Gracefully handle file upload persistence errors by clearing state#275
Gracefully handle file upload persistence errors by clearing state#275alexluckett merged 29 commits intomainfrom
Conversation
91dd96c to
ad767cd
Compare
1e59cc3 to
43b39ad
Compare
a8f4c04 to
ed79a7a
Compare
ed79a7a to
17ac0b9
Compare
| const notificationEmail = | ||
| metadata.notificationEmail ?? 'defraforms@defra.gov.uk' |
There was a problem hiding this comment.
This is one of the key changes
this used to be: definition.outputEmail and was swapped to metadata.notificationEmail.
There was a problem hiding this comment.
Is the fallback needed here?
We would'nt be inside this function if there was no notificationEmail given this line
… operation (requires hapi toolkit)
c390319 to
585a89f
Compare
fc8ca08 to
ee024f5
Compare
02d3673 to
1db48cb
Compare
jbarnsley10
left a comment
There was a problem hiding this comment.
Great work. My only question would be 'is it a sensible user experience where multiple files from say multiple upload pages have disappeared?' i.e. does it handle one error at a time, or all at once etc
Also there's a minor Sonar issue on a negated condition.
Approving
a3d2fa4 to
b23cc00
Compare
davidjamesstone
left a comment
There was a problem hiding this comment.
Awesome - love it
Scullyon
left a comment
There was a problem hiding this comment.
Looks really good, just a minor comment in one of the files and also wondered the same thing as Jez.
|
|
||
| getStateKeys() { | ||
| const extraStateKeys = | ||
| this.component instanceof FileUploadField ? ['upload'] : [] |
There was a problem hiding this comment.
Should this be in here or would the component itself be a better location for its own state keys? This method could then obtain the data from the component.
There was a problem hiding this comment.
Really good idea, that would be a better place to do it. I'll leave this as a follow-up though as this has been hanging around a bit over the holidays.
|



Proposed change
Files may disappear if they've expired (unexpected bugs with retrieveal keys, file expiry on S3, etc). Currently form submissions can go through with invalid file upload state as we don't re-check it before submission.
This PR:
onSubmitfunction within the component and removes special handling withinSummaryPageControllerdefinition.outputEmailfeature, in favour ofmetadata.notificationEmailJira ticket: https://eaflood.atlassian.net/browse/DF-719
Type of change
Checklist
README.mdanddocs/*(where appropriate, e.g. new features).npm run test).npm run lint).npm run format).