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] feat: update form id for form upload flow #34503

Conversation

pennja
Copy link
Contributor

@pennja pennja commented Feb 4, 2025

Summary

  • This work updates some syntax and updates the form-upload-flow form to pass the form id and -UPLOAD as the form id in order to support multiple form uploads at a time.
  • I work for the Veteran Facing Forms team who owns this form

Related issue(s)

  • department-of-veterans-affairs/va.gov-team-forms#1983

Testing done

  • Before the change, users could only have one form upload submission as a saved in progress form.

What areas of the site does it impact?

  • Only the form upload flow is affected by this change

Acceptance criteria

Quality Assurance & Testing

  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user

Requested Feedback

Any

@pennja pennja marked this pull request as ready for review February 4, 2025 21:50
@pennja pennja requested review from a team as code owners February 4, 2025 21:50
@pennja pennja force-pushed the jap/simple-forms/1983-form-upload-look-into-removing-save-in-progress branch from 3147a2f to ea52a2d Compare February 6, 2025 21:20
@pennja pennja requested a review from a team as a code owner February 6, 2025 21:20
@va-vfs-bot va-vfs-bot temporarily deployed to master/jap/simple-forms/1983-form-upload-look-into-removing-save-in-progress/main February 6, 2025 21:21 Inactive
Copy link
Contributor

@Thrillberg Thrillberg left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work Jacob! My comments are mostly just questions from curiosity but I do think it might be worth revisiting what we enter for benefit. Maybe glean something from the form title or subtitle to figure out what benefit it is conferring? 🤔

src/platform/forms/constants.js Show resolved Hide resolved
benefit: `form upload flow`,
title: `form 21P-0518-1 upload`,
description: 'uploaded file for form 21P-0518-1',
trackingPrefix: 'form-21p-0518-1-upload-',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want benefit for each to be different? What does benefit go to or mean? Maybe it should just repeat whatever's in title?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, I can make the update and determine a good answer for you.

Copy link
Contributor Author

@pennja pennja Feb 11, 2025

Choose a reason for hiding this comment

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

Looks like it's used to generate and display applications in progress within the ApplicationsInProgress.jsx component.

Copy link
Contributor

@Thrillberg Thrillberg left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@pennja pennja merged commit 59babf8 into main Feb 11, 2025
83 checks passed
@pennja pennja deleted the jap/simple-forms/1983-form-upload-look-into-removing-save-in-progress branch February 11, 2025 16:32
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.

4 participants