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

fix: app submission edge cases (#4550) #833

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

ColinBuyck
Copy link
Collaborator

This PR addresses bloom-housing#2173

  • Addresses the issue in full
  • Addresses only certain aspects of the issue

Description

This PR protects two duplicate submission paths. The first being spam clicking the submission button which was caused by the button loadingMessage prop not sufficiently blocking the submission call which is captured here (bloom-housing/ui-seeds#98) and is now handled with a !submitting check at the top of the submit function. The second path was clicking back on the confirmation screen and clicking submit again. This is handled by redirecting the user if they already have a confirmation code since that is only saved on submitted applications.

Note that the ticket also includes reference to a "warning" but after talking to produce these two fixes are the desired behavior for handling this ticket.

How Can This Be Tested/Reviewed?

This can be tested on the public side by logging in, fill out an application, and attempt to spam click the submit button. Then, click the back button on the confirmation page and notice the toast and redirect. You should end up on the my applications page where you will only see one submission for your attempted spam click.

Then, repeat this process as a signed out user and you should be redirected to the listings page and only see one submission in the applications table (via db).

Author Checklist:

  • Added QA notes to the issue with applicable URLs
  • Reviewed in a desktop view
  • Reviewed in a mobile view
  • Reviewed considering accessibility
  • Added tests covering the changes
  • Made corresponding changes to the documentation
  • Ran yarn generate:client and/or created a migration when required

Review Process:

  • Read and understand the issue
  • Ensure the author has added QA notes
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Either (1) explicitly ask a clarifying question, (2) request changes, or (3) approve the PR, even if there are very small remaining changes, if you don't need to re-review after the updates

* fix: block duplicate submissions

* fix: redirect with toast

* fix: linting

* fix: copy update

---------

Co-authored-by: Eric McGarry <[email protected]>
@ColinBuyck ColinBuyck added 1 review needed release A PR that is pulling over a commit(s) from core labels Jan 23, 2025
Copy link

netlify bot commented Jan 23, 2025

Deploy Preview for housing-sanjoseca-gov ready!

Name Link
🔨 Latest commit af63fc7
🔍 Latest deploy log https://app.netlify.com/sites/housing-sanjoseca-gov/deploys/6792861594befb000835c708
😎 Deploy Preview https://deploy-preview-833--housing-sanjoseca-gov.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

🎏

@ColinBuyck ColinBuyck merged commit 07eb52d into main Feb 6, 2025
37 checks passed
@ColinBuyck ColinBuyck deleted the release/2173/app-submission-edge-cases branch February 6, 2025 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge release A PR that is pulling over a commit(s) from core
Development

Successfully merging this pull request may close these issues.

2 participants