fix(mrf be val): skip validation for unchanged mrf responses #7894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Datadog ReportBranch report: ✅ 0 Failed, 1285 Passed, 1 Skipped, 2m 16.27s Total duration (4m 30.22s time saved) |
kevin9foong
changed the title
kevin9foong
changed the title
kevin9foong
commented
Nov 15, 2024
| @@ -191,7 +191,9 @@ const pastOnlyValidatorV3: ResponseValidator<DateResponseV3> = (response) => { | |||
| const answerDate = createMomentFromDateStringV3(answer) | |||
|
|
|||
| return answerDate.isAfter(todayMax) | |||
| ? left(`DateValidatorV3:\t answer does not pass date logic validation`) | |||
| ? left( | |||
There was a problem hiding this comment.
The changes in this file are added to make the logs more specific - not related to the skipping of validation
kevin9foong
commented
Nov 15, 2024
| @@ -1052,131 +1052,4 @@ describe('Email field validation V3', () => { | |||
| new ValidateFieldError('Invalid answer submitted'), | |||
| ) | |||
| }) | |||
|
|
|||
There was a problem hiding this comment.
These TC are removed as they are migrated to the test for checkIsResponseChangedV3 instead, avoiding the need to spy and check if the spied function is invoked
kevin9foong
commented
Nov 15, 2024
| : null | ||
| return prevResponseValue !== responseValue | ||
| case BasicField.Checkbox: | ||
| if (prevResponse.fieldType !== response.fieldType) { |
There was a problem hiding this comment.
this is required due to TS throwing an error as it fails to infer the type of prevResponse is same as response without it, despite being checked in L19 above.
kevin9foong
force-pushed
the
fix/remove-validation-for-unchanged-mrf-responses
branch
from
6ef3ce7 to
7339522
Compare
kevin9foong
changed the title
kevin9foong
deleted the
fix/remove-validation-for-unchanged-mrf-responses
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
During MRF BE Validation, false positives occur for verifiable fields (due to signature being marked as invalid) and date fields with disallow past dates validation.
Details
1st jan 2024, a respondent for step 1 can select the current date1st jan 2024which is valid and passes the disallow past dates validation.3rd jan 2024and the respondent leaves the date field value from step 1 unchanged. This fails the disallow past dates validation since the step 2 submission date of3rd jan 2024is after1st jan 2024.This edge case is considered acceptable and is similar to the current (as of this PR) frontend validation behavior. For example, if the admin tries to submit the form at step 2 where the date field with disallow past dates validation is editable, they would experience a frontend validation failure.
Closes FRM-1909
Solution
During field validation, there is a check if validation is required.
An additional check for if the field response is changed is added. In the event the field is unchanged, then the validation for the field is skipped. This is based on the idea where since a response has been validated in the previous step and is unchanged, it does not need to be re-validated in the subsequent step. This resolves the false positive issue and replicates the same frontend validation behavior described above.
Implementation
A new function
checkIsResponseChangedV3is added to a filefield-validation.utils.tswhich allows for export without having to explicitly export acheckIsResponseChangedV3ForTestif added to theindex.tsfile. This function is used to determine if validation should be skipped.This function is exported and unit tested to ensure it correctly checks for changes for the various field types.
Breaking Changes