Skip to content

[Bug Report] Deleted fields in edits are reset when updating the edit #879

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

Open
peolic opened this issue Jan 1, 2025 · 0 comments
Open
Labels
help wanted Extra attention is needed

Comments

@peolic
Copy link
Contributor

peolic commented Jan 1, 2025

This affects production (v0.5.3), as well as current develop.

To Reproduce

Steps to reproduce the behavior:

  1. Existing performer with (for example) height set to 170 cm.
  2. Make an edit to remove the height, and modify (change/add, but not delete) at least one other field.
  3. Click to update the edit.
  4. 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.

disambiguation: initial?.disambiguation ?? performer?.disambiguation,
height: initial?.height || performer?.height,

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.

initial={{
aliases,
images,
}}

@peolic peolic added the help wanted Extra attention is needed label Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant