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.

Sign up for GitHub

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

Jump to bottom

fix: backend validation does not prevent responses on hidden fields #736

Merged
merged 16 commits into from
Nov 30, 2020
Merged

fix: backend validation does not prevent responses on hidden fields #736

merged 16 commits into from
Nov 30, 2020

Conversation

tshuli
Copy link
Contributor

@tshuli tshuli commented Nov 26, 2020 •
edited
Loading

Problem

Closes #732

Solution

  • Add check for isVisible in validateField()
  • Also updated unit tests

Tests

  • Create a form with Yes/No field, and a Number Field hidden behind it. Attempt to submit a response programatically with the number field still hidden. Submission should fail.
  • Now unhide the Number Field. Attempt to submit a response on the number field normally. Submission should pass.
  • Create email mode form with all field types (including Yes/No) hidden behind a Yes/No field. Submit while keeping the fields hidden. Submission should pass.

@tshuli tshuli force-pushed the fix/form-logic-validation branch from af138bc to a82f694 Compare November 26, 2020 14:08
@tshuli tshuli marked this pull request as ready for review November 27, 2020 10:43
@tshuli tshuli requested a review from liangyuanruo November 27, 2020 10:43
liangyuanruo
liangyuanruo previously approved these changes Nov 30, 2020
View reviewed changes
@liangyuanruo liangyuanruo dismissed their stale review November 30, 2020 02:58

To address additional comments before merging

@tshuli tshuli force-pushed the fix/form-logic-validation branch from addf7ad to c98eab1 Compare November 30, 2020 05:52
@tshuli tshuli requested a review from liangyuanruo November 30, 2020 05:55
liangyuanruo
liangyuanruo reviewed Nov 30, 2020
View reviewed changes
src/types/response/guards.ts
response.answerArray.every((arr: any) => isTableRow(arr))
) {
// Check that all arrays in answerArray have the same length
const subArrLength: number = response.answerArray[0].length
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible that response.answerArray[0] is undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing out, i've added an additional check that response.answerArray[0] must be a tableRow

liangyuanruo
liangyuanruo approved these changes Nov 30, 2020
View reviewed changes
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Approving first, please address remaining comment before merging.

@mantariksh mantariksh mentioned this pull request Nov 30, 2020
Merged
9 tasks
tshuli added 13 commits November 30, 2020 21:41
@tshuli
fix: add check for responses on hidden fields
027ce74
@tshuli
fix: correct isVisible values for tests
790316f
@tshuli
chore: update status code on ValidateFieldError
63b87d0
@tshuli
fix: remove invalid visibility tests from server controller spec beca…
ce52d59 
…use hidden field answers should not even be in the response
@tshuli
chore: add tests for hidden fields in field validator specs
cabc610
@tshuli
refactor: type FieldResponse as union of Response interfaces
9a33337
@tshuli
refactor: use else if
34afb82
@tshuli
chore: tighten type guards for checkbox and table responses
587949b
@tshuli
fix: check for table response on hidden field
92d2404
@tshuli
refactor: TableRow subtype
0600081
@tshuli
fix: concatenate flattened array for table response before evaluating…
71d7662 
… length
@tshuli
refactor: beforeAll and afterAll for jasmine clock
5e57a5d
@tshuli
chore: add check that all arrays have same length
05da7fc
@tshuli tshuli force-pushed the fix/form-logic-validation branch from eae64ee to b455c00 Compare November 30, 2020 13:41
@tshuli tshuli merged commit 0fbfc81 into develop Nov 30, 2020
@tshuli tshuli deleted the fix/form-logic-validation branch November 30, 2020 22:52
@karrui karrui mentioned this pull request Dec 1, 2020
Merged
karrui added a commit that referenced this pull request Dec 1, 2020
@karrui
Revert "fix: backend validation does not prevent responses on hidden …
fead8ce 
…fields (#736)"

This reverts commit 0fbfc81.
@tshuli tshuli restored the fix/form-logic-validation branch December 3, 2020 03:38
@tshuli tshuli mentioned this pull request Dec 7, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liangyuanruo liangyuanruo liangyuanruo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Backend validation does not prevent responses on hidden fields
2 participants
@tshuli @liangyuanruo