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

Making a postcode component optional through logic changes the validation behaviour #3744

Closed
SilviaAmAm opened this issue Jan 8, 2024 · 2 comments · Fixed by #3750
Closed
Assignees
Labels
bug Something isn't working needs-backport Fix must be backported to stable release branch
Milestone

Comments

@SilviaAmAm
Copy link
Contributor

Product versie / Product version

2.4.2

Omschrijf het probleem / Describe the bug

DH Taiga 632

If a postcode component is made not required by (backend) logic, then the validate in JSON of the component is:

"validate": {
      "required": false
    },

While if it is not touched by the logic, then it is:

"validate": {
   "custom": "",
   "unique": false,
   "pattern": "^[1-9][0-9]{3} ?(?!sa|sd|ss|SA|SD|SS)[a-zA-Z]{2}$",
   "plugins": [],
   "multiple": false,
   "required": true,
   "maxLength": "",
   "minLength": "",
   "customMessage": "Invalid Postcode",
   "customPrivate": false,
   "strictDateValidation": false
 }

The missing pattern/customMessage keys result in the error message being different if an invalid postcode is entered.
Screenshot from 2024-01-08 15-33-35

Stappen om te reproduceren / Steps to reproduce

  • Create a form with two required postcode components.
  • Add a logic rule that says: "If true then make the secode postcode component not required"
  • Save the form

In the SDK, fill in the form and enter 2 invalid postcodes. Notice the different errors.

Verwacht gedrag / Expected behavior

No response

Screen resolution

None

Device

None

OS

None

Browser

No response

@SilviaAmAm SilviaAmAm added bug Something isn't working triage Issue needs to be validated. Remove this label if the issue considered valid. needs-backport Fix must be backported to stable release branch and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Jan 8, 2024
@SilviaAmAm SilviaAmAm added this to the Release 2.4.3 milestone Jan 8, 2024
@SilviaAmAm SilviaAmAm self-assigned this Jan 8, 2024
SilviaAmAm added a commit that referenced this issue Jan 9, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages
SilviaAmAm added a commit that referenced this issue Jan 9, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages
sergei-maertens pushed a commit that referenced this issue Jan 10, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages
sergei-maertens added a commit that referenced this issue Jan 10, 2024
…onent

[#3744] Fix postcode with inconsistent errors
@sergei-maertens sergei-maertens removed the needs-backport Fix must be backported to stable release branch label Jan 10, 2024
@sergei-maertens
Copy link
Member

sergei-maertens commented Jan 10, 2024

Skip backport, reasoning here: #3750 (comment)

Fix is incomplete though, we're not handling old form exports yet.

Old form imports can be corrected manually.

@sergei-maertens
Copy link
Member

Clarified a bit more - we can backport the fix without the data migration. It won't break existing forms (but the bug will still be present). If that's a deal breaker, a manual action is possible where you remove and re-add the logic action.

sergei-maertens pushed a commit that referenced this issue Jan 10, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages

Backport addition: the data migration is dropped, this only applies to
the master branch since we otherwise run into issues with migration
history. The required fixes can be applied manually by removing and
re-adding the "make field required/optional" action.

Backport-of: #3750
sergei-maertens pushed a commit that referenced this issue Jan 10, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages

Backport addition: the data migration is dropped, this only applies to
the master branch since we otherwise run into issues with migration
history. The required fixes can be applied manually by removing and
re-adding the "make field required/optional" action.

Backport-of: #3750
sergei-maertens pushed a commit that referenced this issue Jan 10, 2024
We should not overwrite the whole validate object, because it can contain other custom validation, like patterns/regex and their custom messages

Backport addition: the data migration is dropped, this only applies to
the master branch since we otherwise run into issues with migration
history. The required fixes can be applied manually by removing and
re-adding the "make field required/optional" action.

Backport-of: #3750
@sergei-maertens sergei-maertens added the needs-backport Fix must be backported to stable release branch label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-backport Fix must be backported to stable release branch
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants