buttercup/buttercup-desktop

cannot delete custom fields

sfisque opened this issue · 7 comments

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.

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!

@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.

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

Do you have any ideas?

@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.

@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.

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.

Editing a custom field results in the addition of a new custom field and the retention of the one being edited. I think this observation is related to this bug.