[Bug Report] Deleted fields in edits are reset when updating the edit
This affects production (v0.5.3), as well as current develop.
To Reproduce
Steps to reproduce the behavior:
- Existing performer with (for example) height set to 170 cm.
- Make an edit to remove the height, and modify (change/add, but not delete) at least one other field.
- Click to update the edit.
- Height is silently restored to the form field. When the update is submitted, the deleted value is restored to the edit.
This is applicable to at least height and disambiguation, but likely others, and on other entities too.
Expected behavior
Deleted fields in an edit to carry over when updating the edit.
Additional context
This feels like a repeat issue.
As far as I can tell, null denotes an empty value and undefined denotes omitted value (field was not provided, ignore).
However, it seems the code uses ?? (Nullish coalescing) and ||,
which means both values are treated the same - as ignored, so the next value in the chain is applied.
https://github.com/stashapp/stash-box/blob/45ce23359b0d65e1c2334cd54e089f6566f1a571/frontend/src/pages/performers/performerForm/PerformerForm.tsx#L155 https://github.com/stashapp/stash-box/blob/45ce23359b0d65e1c2334cd54e089f6566f1a571/frontend/src/pages/performers/performerForm/PerformerForm.tsx#L168
Potential solution
I'm not sure what the best fix is, but the simplest approach I could think of,
was to replace ?? with filtering to the first value that !== undefined in order.
For example,
disambiguation: [initial?.disambiguation, performer?.disambiguation].find((v) => v !== undefined),
height: [initial?.height, performer?.height].find((v) => v !== undefined),
// handling of height shouldn't be any different from disambiguation.
// if empty strings exist in the chain they should probably be converted to null.
Probably in a function instead (and maybe a loop instead of Array.find()).
This type of solution may have current and/or future consequences with regard to field merges, as part of merging entities.
Currently, at least for merging performer - the initial prop to only used to provide the combined aliases and images fields.
https://github.com/stashapp/stash-box/blob/45ce23359b0d65e1c2334cd54e089f6566f1a571/frontend/src/pages/performers/PerformerMerge.tsx#L166-L169