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

GN-4378 Revalidate codelist options list when changing the label #113

Conversation

x-m-el
Copy link
Contributor

@x-m-el x-m-el commented Jun 22, 2023

Overview

Before only the record you changed would get revalidated by ember-changeset (because of auto-validation of ember-changeset being active). However, the uniqueness depends on more than the changed record.
To fix this, the list of labels is revalidated for uniqueness after every change.

connected issues and PRs:

https://binnenland.atlassian.net/browse/GN-4378

How to test/reproduce

Getting the error before:

  1. create a codelist with 2 items and the same label.
  2. adjust the label of the item that does not have an error
  3. the save button would still not be active, even though everything is valid now.

With this change, you will be able to save in step 3.

Challenges/uncertainties

  • ember-changeset does not have a "removeError" function... This is annoying as we add custom errors (that aren't possible with ember-changeset-validation). A workaround is used. PR is open to add this (add #removeError and #removeErrors to Changeset interface adopted-ember-addons/ember-changeset#668), but this will probably take a while to get released.
  • the ChangesetList's changeset array does not track properly. The errors get added correctly, but the UI might sometimes not show the error message even if it is present. I did not find any fix, didn't work:
    • using tracked-built-ins
    • rewriting the array after a changes (with this._changesets = [...this._changesets] in ChangesetList)
      This might be because ChangesetList is a separate class?

@x-m-el x-m-el added the bug Something isn't working label Jun 22, 2023
Copy link
Contributor

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good workaround overall, just a comment.

app/components/codelist-form.js Outdated Show resolved Hide resolved
@elpoelma elpoelma self-requested a review July 3, 2023 12:00
@elpoelma elpoelma requested a review from abeforgit July 3, 2023 12:02
@elpoelma elpoelma enabled auto-merge July 6, 2023 07:52
@elpoelma elpoelma merged commit afa9b7c into master Jul 6, 2023
@elpoelma elpoelma deleted the GN-4378-RB-codelist-saving-is-blocked-when-not-changing-duplicate-value branch July 6, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants