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

refactor: migrate attachment validator to ts #1181

Merged
merged 16 commits into from
Mar 4, 2021

Conversation

tshuli
Copy link
Contributor

@tshuli tshuli commented Feb 18, 2021

Problem

Part of #7

Solution

Improvements

  • Migrates attachment validation to Typescript

Unit test coverage

Attachment validation
    check for responses on hidden fields should disallow responses submitted for hidden fields
      ✓ when response contains filename
      ✓ when response contains answer
      ✓ when response contains file content

    Validation of attachment size
      ✓ should respect the attachmentSize from formField
      ✓ should allow attachment with valid size
      ✓ should disallow attachment that exceeds size

    Required or optional
      ✓ should disallow submission with no attachment if it is required 
      ✓ should allow submission with attachment if it is required 
      ✓ should allow submission with attachment if it is optional 
      ✓ should allow submission with no attachment if it is not required
      ✓ should disallow submission with no answer if it is required 
      ✓ should allow submission with no answer if it is not required 
      ✓ should disallow when it is not required but with answer and no attachment

Tests

  • Create two required attachment fields with 1mb size limit each. Hide one of them behind logic. Check that normal submission using frontend succeeds for both visible and hidden field.
  • Check that following validation works when submitting programatically
  • Attachment size cannot be above the specified limit. Use the AngularJS inspector to modify field size limit to 2mb on frontend. Upload a file of >1mb. Check that submission is rejected "File size more than limit"
  • Attachment cannot be empty if required. First submit a form with the second attachment field hidden. Programatically resubmit with the second field unhidden but with no answer or attachment. Check that submission is rejected "Answer is empty string"
  • Now inject an answer into the response for the unhidden field but with no attachment content. Check that submission is rejected "No attachment content"

@tshuli tshuli changed the base branch from develop to refactor/number-validator February 18, 2021 06:29
@tshuli tshuli force-pushed the refactor/number-validator branch 2 times, most recently from cbcd31b to 9d82b1c Compare February 21, 2021 07:27
@tshuli tshuli force-pushed the refactor/attachment-validator branch from 49f3af2 to 7cf050c Compare February 22, 2021 05:47
@tshuli tshuli changed the base branch from refactor/number-validator to develop February 22, 2021 05:47
@tshuli tshuli requested a review from karrui February 22, 2021 06:16
@tshuli tshuli marked this pull request as ready for review February 22, 2021 06:16
@tshuli tshuli force-pushed the refactor/attachment-validator branch from 7cf050c to afd7d79 Compare February 23, 2021 05:44
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

was AttachmentValidator.js deleted?

Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

some refactors requested on the tests!

@tshuli
Copy link
Contributor Author

tshuli commented Mar 3, 2021

@mantariksh thanks for reviewing, comments addressed for re-review

@tshuli tshuli requested a review from mantariksh March 3, 2021 06:07
@tshuli tshuli force-pushed the refactor/attachment-validator branch from ebabbb7 to b413fcb Compare March 4, 2021 01:50
@tshuli tshuli force-pushed the refactor/attachment-validator branch from b32cb7a to 9c44655 Compare March 4, 2021 02:28
@tshuli
Copy link
Contributor Author

tshuli commented Mar 4, 2021

Addressed comments, for re-review @mantariksh

@tshuli tshuli requested a review from mantariksh March 4, 2021 02:57
@tshuli tshuli merged commit 95544a6 into develop Mar 4, 2021
@tshuli tshuli mentioned this pull request Mar 10, 2021
@karrui karrui deleted the refactor/attachment-validator branch April 21, 2021 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants