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

Handle required form arrays #484

Merged
merged 2 commits into from
Oct 13, 2021

Conversation

saulipurhonen
Copy link
Contributor

@saulipurhonen saulipurhonen commented Oct 11, 2021

Description

Submitting forms that contain required form array fields caused server errors if these fields were invalid.

Related issues

Closes #477 & #480

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes Made

  • Register a hidden input for required form arrays
  • Display error for invalid field
  • Clear error on appending form array

Testing

  • Integration Tests

@saulipurhonen saulipurhonen force-pushed the bugfix/validate-required-form-arrays branch from 8486d8f to 575ba62 Compare October 11, 2021 09:22
@saulipurhonen saulipurhonen marked this pull request as ready for review October 11, 2021 09:34
@saulipurhonen saulipurhonen self-assigned this Oct 11, 2021
@saulipurhonen saulipurhonen added the bug Something isn't working label Oct 11, 2021
@saulipurhonen saulipurhonen added this to the Open Beta milestone Oct 11, 2021
@saulipurhonen saulipurhonen requested review from hannyle and blankdots and removed request for hannyle October 11, 2021 09:34
@hannyle hannyle self-requested a review October 12, 2021 07:51
Copy link
Contributor

@hannyle hannyle left a comment

Choose a reason for hiding this comment

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

Great! The problem is fixed!

@hannyle
Copy link
Contributor

hannyle commented Oct 12, 2021

One minor thing is for Run form > Experiment Reference, when we Add new item but doesn't fill in anything, Submit the form still links to server error because there is no required field within Experiment Reference.

I remember we have fixed this kind of problem a while ago but for some reason it still exists here. Could we solve it in this PR or should we fix it in another one ?

@saulipurhonen
Copy link
Contributor Author

One minor thing is for Run form > Experiment Reference, when we Add new item but doesn't fill in anything, Submit the form still links to server error because there is no required field within Experiment Reference.

I remember we have fixed this kind of problem a while ago but for some reason it still exists here. Could we solve it in this PR or should we fix it in another one ?

I also remember that this was fixed. Weird thing is that it seems that this error exists also in develop -branch.
If I recall correctly, we fixed it by forcing the first field of array to be required.
I'll take a look into this and see if I could add some minor change into this branch.

@saulipurhonen
Copy link
Contributor Author

One minor thing is for Run form > Experiment Reference, when we Add new item but doesn't fill in anything, Submit the form still links to server error because there is no required field within Experiment Reference.
I remember we have fixed this kind of problem a while ago but for some reason it still exists here. Could we solve it in this PR or should we fix it in another one ?

I also remember that this was fixed. Weird thing is that it seems that this error exists also in develop -branch. If I recall correctly, we fixed it by forcing the first field of array to be required. I'll take a look into this and see if I could add some minor change into this branch.

Introduced a possible fix.

Copy link
Contributor

@hannyle hannyle left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

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

tested and looks good.

For the DAC ( i suspect for the others as well) if the fields are not filled correctly it just complains a item is needed. If there is a quick way to fix it I suggest we make a note of it and fix it as part of #429 or some other issue
Peek 2021-10-13 12-01

I assume it will be addressed as a JSON/form validation error and can come from front-end or backend.

@blankdots
Copy link
Contributor

blankdots commented Oct 13, 2021

feel free to merge, if my comment is not addressed as part of this.

@saulipurhonen
Copy link
Contributor Author

saulipurhonen commented Oct 13, 2021

tested and looks good.

For the DAC ( i suspect for the others as well) if the fields are not filled correctly it just complains a item is needed. If there is a quick way to fix it I suggest we make a note of it and fix it as part of #429 or some other issue ![Peek 2021-10-13 12-01]

I assume it will be addressed as a JSON/form validation error and can come from front-end or backend.

This problem seems to be part of bigger issue. I tested this on develop branch and we don't get a helper text for invalid Contact Email field. I'll create a new issue and note the behavior you mentioned there.

@saulipurhonen saulipurhonen merged commit e6a7612 into develop Oct 13, 2021
@saulipurhonen saulipurhonen deleted the bugfix/validate-required-form-arrays branch October 13, 2021 09:25
@blankdots blankdots mentioned this pull request Apr 7, 2022
6 tasks
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