feat: allow team editors to download a zip of application data from the editor submissions log #3750
Conversation
|
Removed vultr server and associated DNS entries |
| externalPlanningSiteName: external_planning_site_name | ||
| externalPlanningSiteUrl: external_planning_site_url | ||
| submissionEmail: submission_email | ||
| } |
There was a problem hiding this comment.
In order to re-use the existing /download-application-files/:sessionId endpoint, we need team settings like submissionEmail to be available in the editor when the team store is initialised - is this okay? Am I forgetting any specific reasons that settings were omitted from this query and not initialised?
There was a problem hiding this comment.
I think this is cool, I remember I had a bit of trouble with the query, but think that was due to an issue on timing of changes with submissionEmail than anything code related that would stop this happening. All the settings should be defined in the database and able to be read, nothing gets generated after this... unless you edit a setting.
| }/download-application-files/${ | ||
| submission.sessionId | ||
| }?localAuthority=${teamSlug}&email=${submissionEmail}`; | ||
| window.open(zipUrl, "_blank"); |
There was a problem hiding this comment.
This is really simple! Is window.open() actually too simple / let's get council folks to test on staging to confirm that their council browser settings won't block this?
We also know that we want to "harden" /download-application-files/:sessionId endpoint in the near future (this Trello ticket) - so we'll ideally want a solution that works for both places here so that we can ensure send to email & submission log zips are totally consistent for end uers ! I've attached this PR to the Trello ticket for future reference.
There was a problem hiding this comment.
Only one way to find out! This is the approach I would have gone with initially also 👍
| const showDownloadButton = | ||
| canUserEditTeam(teamSlug) && | ||
| submission.status === "Success" && | ||
| submissionEmail && |
There was a problem hiding this comment.
It's admittedly a bit of a weird requirement that submissionEmail needs to be set to be able to download application data that was originally sent to any back-office system (because submissionEmail is "send to email" specific) - but now that this is self-service in the editor, I think it's ultimately okay and won't be a blocker?
This MVP re-uses the /download-application-files/:sessionId endpoint, but as other comments here also say we know we want to change/"harden" this soon so will be able to theoretically drop this requirement in next iteration!
There was a problem hiding this comment.
Yeah it's a bit odd as it stands but I totally get it!
I think a better approach would be to hit the API and use the user's JWT permissions to control access as opposed to the fact they have access to the submission_email column.
The reason for this is that team_settings doesn't have access controls between teams, but the lowcal_sessions table could / should.
What I think this would look like -
- Set up permissions on
lowcal_sessionstable so thatteamEditorscan access sessions from their team - Make a new route, with middleware to check it's an authenticated request, but use the same controller (might need to look at which client is making the request?)
- Hit the new route here
router.get("/download-submission-files/:sessionId", useTeamEditorAuth, downloadApplicationFiles);This works fine for now, but when we revisit this I think it's worth tightening this up a little 👍
|
Possible quick usability win here: wrapping the download icon in a tooltip. I'll branch off to add this. |
There was a problem hiding this comment.
Works as expected for me, although it is hard for me to review the quality of the files, only testing the initialising of the team settings, and the use of the button to trigger a download.
@jessicamcinchak do you think that's something I should do before approving or it's unrelated to the PR?
| externalPlanningSiteName: external_planning_site_name | ||
| externalPlanningSiteUrl: external_planning_site_url | ||
| submissionEmail: submission_email | ||
| } |
There was a problem hiding this comment.
I think this is cool, I remember I had a bit of trouble with the query, but think that was due to an issue on timing of changes with submissionEmail than anything code related that would stop this happening. All the settings should be defined in the database and able to be read, nothing gets generated after this... unless you edit a setting.
| const showDownloadButton = | ||
| canUserEditTeam(teamSlug) && | ||
| submission.status === "Success" && | ||
| submissionEmail && |
There was a problem hiding this comment.
Yeah it's a bit odd as it stands but I totally get it!
I think a better approach would be to hit the API and use the user's JWT permissions to control access as opposed to the fact they have access to the submission_email column.
The reason for this is that team_settings doesn't have access controls between teams, but the lowcal_sessions table could / should.
What I think this would look like -
- Set up permissions on
lowcal_sessionstable so thatteamEditorscan access sessions from their team - Make a new route, with middleware to check it's an authenticated request, but use the same controller (might need to look at which client is making the request?)
- Hit the new route here
router.get("/download-submission-files/:sessionId", useTeamEditorAuth, downloadApplicationFiles);This works fine for now, but when we revisit this I think it's worth tightening this up a little 👍
Folks using the existing Uniform connector have been having issues accessing application files in their DMS and more often requesting that a zip be sent to them via Slack, etc - this should make application data easier to self-serve from the editor ! See this thread for more context: https://opensystemslab.slack.com/archives/C5Q59R3HB/p1727791569016769
Changes:
Test flow: