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

[SIMPLE-FORMS] fix: submission failures for form 20-10207 #19447

Merged

Conversation

pennja
Copy link
Contributor

@pennja pennja commented Nov 13, 2024

Summary

  • This work is behind a feature toggle (flipper): YES
  • This PR addresses the issue of document upload errors in the SubmissionArchive service while processing form 20-10207 with attachments.
  • Changes include:
    • Adding test coverage for file existence checks
    • Implementing a guard clause to handle missing files
    • Updating the uploads controller to pass the correct attachment file paths instead of UUIDs
  • To reproduce the bug, attempt to submit a 20-10207 form with attachments. This should trigger an error during the upload process.
  • The solution involves ensuring that the system checks for the existence of the each attachment file before attempting to process it. This prevents errors from occurring when an attachment is missing and provides a clearer error message to the user.
  • I work for the Veteran Facing Forms team, which owns the maintenance of this component.

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Prior to this change, the system would throw an error if a document was uploaded without the corresponding file being present, leading to a poor user experience.
  • To verify the changes, follow these steps:
    1. Run the unit tests, there is now no way to reproduce this error with the fix.
      OR
    2. Submit a form 20-10207 form successfully with attachments and verify that the submission is processed correctly

What areas of the site does it impact?

  • This change impacts the attachment upload functionality within the SubmissionArchive service, as well as the UploadsController.

Acceptance criteria

  • I fixed unit tests and integration tests for each feature.
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution.
  • Documentation has been updated (link to documentation).
  • No sensitive information is captured in logging, hardcoded, or specs.
  • Feature has a monitor built into Datadog.
  • I logged into a local build and verified all authenticated routes work as expected.

Requested Feedback

I would appreciate feedback on the error handling implementation and whether the test coverage is sufficient. Additionally, if there are any concerns regarding the feature toggle rollout, please let me know.

@pennja pennja requested review from a team as code owners November 13, 2024 20:05
@va-vfs-bot va-vfs-bot temporarily deployed to jap/simple-forms/1858-10207-submission-resulting-in-failure/main/main November 13, 2024 20:18 Inactive
@pennja pennja changed the title [SIMPLE-FORMS] fix: [SIMPLE-FORMS] fix: submission failures for form 20-10207 Nov 14, 2024
Copy link

Backend-review-group approval confirmed.

@RachalCassity RachalCassity self-assigned this Nov 14, 2024
@pennja pennja enabled auto-merge (squash) November 14, 2024 17:12
@pennja pennja force-pushed the jap/simple-forms/1858-10207-submission-resulting-in-failure branch from 25c22a2 to cd23741 Compare November 14, 2024 17:18
@pennja pennja merged commit 95afe01 into master Nov 14, 2024
21 checks passed
@pennja pennja deleted the jap/simple-forms/1858-10207-submission-resulting-in-failure branch November 14, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants