Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cannot delete custom fields #1342

Open
sfisque opened this issue Jun 13, 2024 · 6 comments
Open

cannot delete custom fields #1342

sfisque opened this issue Jun 13, 2024 · 6 comments

Comments

@sfisque
Copy link

sfisque commented Jun 13, 2024

OS: MacOS 11.7.10
Ver: Desktop @ v2.27.0
Core @ v7.7.0

Flags: installed

attempting to delete a custom field appears to work, but after saving, the deleted field re-appears.

additionally, renaming a custom field does not rename the original field, but duplicates it with a new name, after clicking save.

@quintushr
Copy link
Contributor

I have started looking into this bug. I can reproduce it, but I don't yet understand where it comes from. I suspect the core is returning changes that do not include deletions and only when a new field is saved.

Feel free to let me know if you need any adjustments or further assistance!

@perry-mitchell
Copy link
Member

@quintushr I suspect the issue will be in this method: https://github.com/buttercup/buttercup-core/blob/master/source/facades/vault.ts#L74

It's been around a long time, but it's definitely high time we fixed it 😅 - There's good test coverage there so it should be safe to modify for a fix, and then potentially test such a fix.

@quintushr
Copy link
Contributor

@perry-mitchell I added a test (buttercup/buttercup-core@f6231fa), but it works and does not reproduce the bug.

Do you have any ideas?

@perry-mitchell
Copy link
Member

@quintushr Hmm, did you try with only a single vault consume call? Doing everything in one working state before consuming the facade to process changes?

The desktop does it all in one, for instance. I also partially suspect the UI portion of being the culprit here, but this is good work as we need to narrow it down anyway.

Another way of looking at this is making the change in the desktop while debugging and logging what is actually being sent to the consume method - then see if you can/need to replicate that in the test. It may very well end up being a UI bug.

@quintushr
Copy link
Contributor

@perry-mitchell I think I know the source of the problem, but I don't understand where it occurs in the core. Indeed, in the desktop version, after creating an attribute, any changes involving deletions disappear, which is why the fields reappear.

I'm sorry for my slowness; I'm not very familiar with the environment and Electron, which makes the task more difficult for me, and I am having trouble with the architecture.

Thanks again for your help.

@perry-mitchell
Copy link
Member

Thanks @quintushr - that sounds about right. There's some questionable logic in the core for resolving "missing" properties and how to handle them. It needs to decide if they're missing intentionally or if it's a merge and they should be added.

Completely fine- it's not a small codebase and the health of some of the code has deteriorated. I'm working on bringing it up to date to make it easier to contribute, but it's a long process.

I'm currently working on fixing the release process due to code signing changes and after that I'll try to help here if I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants