-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 👍
There was a problem hiding this 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?
editor.planx.uk/src/pages/SubmissionDownload/helpers/downloadZip.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 👍
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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 👍
🎟️ Trello ticket: https://trello.com/c/EeJb0QVC/2614-harden-download-application-files-endpoint
In this PR:
/download-application-files
endpoint is called, which checks whether the email matches the submission email, and downloads the zip file if so.To test 🧪
Send
component.team_settings
tableScreenshots
Still to do:
test.todos
to fill in