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

feat: verify email before downloading application #4000

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

jamdelion
Copy link
Contributor

@jamdelion jamdelion commented Nov 21, 2024

🎟️ Trello ticket: https://trello.com/c/EeJb0QVC/2614-harden-download-application-files-endpoint

In this PR:

  • Added a 'Verify submission email' page, which links from the email the planning officer gets sent when a user submits an application
  • On submission of a valid email address, the /download-application-files endpoint is called, which checks whether the email matches the submission email, and downloads the zip file if so.

To test 🧪

  1. Create a flow that includes a Send component.
  2. In Hasura, add a submission email to the team your flow is in (your email so that you have access to the link) - this is in the team_settings table
  3. Fill out your flow as a user so that the application is submitted.
  4. You should receive a 'New submission' email, containing a 'download application' link.
  5. When you click the link, it should take you to a 'Verify your email' page.
  6. If you fill in the submission email correctly, you should be able to download the application zipfile.
  7. Incorrect email addresses should show an error message.

Screenshots

Before After
Verify email form
With error message
With invalid email validation

Still to do:

  • Plenty of test.todos to fill in
  • Maybe show some sort of visual success when the application downloads correctly?
  • The application summary table could be filled out with more details (from another endpoint request) - e.g. submission date, service name)

@jamdelion jamdelion marked this pull request as ready for review November 27, 2024 10:59
Copy link

github-actions bot commented Nov 27, 2024

Pizza

Deployed d578773 to https://4000.planx.pizza.

Useful links:

@jamdelion jamdelion requested a review from a team November 27, 2024 13:00
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Really solid start ! A few initial code comments and flags for topics here that I think might make for good dev call discussion this week ? Will properly test on pizza next 👍

editor.planx.uk/package.json Outdated Show resolved Hide resolved
editor.planx.uk/src/index.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/pages/VerifyEmail/VerifyEmail.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/pages/FlowEditor/lib/store/editor.ts Outdated Show resolved Hide resolved
editor.planx.uk/src/pages/VerifyEmail/VerifyEmail.tsx Outdated Show resolved Hide resolved
@jamdelion jamdelion requested review from jessicamcinchak and a team December 3, 2024 18:19
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Thanks for updating to use axios here ! A few comments on re-review, overall all working nearly as expected !

Before a final review, I think this one is a particularly good candidate to post to wider team while still in pizza-state for feedback if not already on your radar.

A few bigger-picture questions I have that would influence how/when it's merged:

  • Does the "Download" page look PlanX-y enough / will council's see it as a trustworthy page to download an application from?
    • Do we need to a more-PlanX/Gov UK feeling header or something? I need to remember how Invite to Pay handles this?
  • Will this new workflow break any existing integrations for councils using Send to Email?
    • Eg I have a hunch that Doncaster & Camden's internal IT teams are perhaps running scripts to auto-download email links from PlanX submission emails?
    • What's going to be the process for giving council's a headsup / trying to learn more about who might need to adjust things internally and helping them prepare for that rather than getting bug reports later?

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Testing and working as expected!

One comment about the layout that's worth addressing I think 👍

editor.planx.uk/src/routes/index.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/ui/shared/ErrorWrapper.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Always good to see stories included - appreciate it!

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to see these .todo()s 👍

@jamdelion jamdelion requested a review from DafyddLlyr December 10, 2024 15:05
@jamdelion jamdelion added the demo Demo environment being used by an external team label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
demo Demo environment being used by an external team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants