Show the uploaded file name in submission upload notifications - #6057
Show the uploaded file name in submission upload notifications#6057milanmajchrak wants to merge 1 commit into
Conversation
Dropping several files into a submission produced identical "Upload successful" toasts, so a single failure inside a batch was impossible to attribute. Each notification now names the file it refers to, falling back to the existing generic messages when no client-side name is available. The uploader gains an additive `onCompleteItemWithFile` output carrying the parsed response together with the file name. The existing `onCompleteItem` is retained and still emits the bare response first, so the other consumers of `ds-uploader` are unaffected. `onUploadError` is retyped from `any` to the existing `UploaderError`, and a single `getNotificationContent()` helper is now the only place an upload notification key appears. Locales that have not translated the two new keys render the generic message via the `default` interpolate param that MissingTranslationHelper already honours, rather than a raw dotted key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@milanmajchrak thanks for submitting this improvement to usability! |
MMilosz
left a comment
There was a problem hiding this comment.
I ran the changes locally without diving into the code. Overall, the functionality works very well. What I verified:
- Properly uses filename received from backend: when uploading
file.txtand re-uploading it, backend names the duplicatefile (1).txtand the exact same name is shown in notification - Batch uploads work as expected
- Error handling works properly
- Non-Latin alphabet filenames display correctly
- Changing to other language (without the new i18n key) shows generic notification
- Poisonous filenames (XSS attempts, unusual encodings, control characters) don't corrupt the UI or cause unexpected behavior
- Missing filenames are handled gracefully
One concern from me: each file upload now triggers 4 additional API requests (see screenshot), which can impact performance especially for batch uploads in peak hours.
Before
Upload completes with a single 201 response.
This PR
After a 201 response, upload fires another 4 requests (I see that the first 201 response already returns filename, in this case it's core-js-banners, without extension).
@milanmajchrak is it possible there's some component re-rendering happening? Or is it intentional?
Other screenshots
All looks good, I'm sharing some screenshots of the different cases tested
Description
Drop three files into a submission and you get three identical "Upload successful" toasts. If one of them fails, there's no way to tell which. This makes each notification name its file. When we don't have a name to show, you get the old generic message, same as before.
Instructions for Reviewers
What changed:
ds-uploadergets a second output,onCompleteItemWithFile, carrying{ response, fileName? }. The oldonCompleteItemstill fires first and still emits the bare response, so the three other places using the uploader don't notice anything.onUploadErroris typed asUploaderErrornow instead ofany, and the upload notification keys all moved into one smallgetNotificationContent()helper.en.json5. Locales that don't have them yet fall back to the generic message instead of printing a rawsubmission.sections.upload.*key — that's what thedefaultinterpolate param is doing.To try it: drop two or three files into the Upload section and check each toast names the right one. For the size-limit path you need
dspace.upload.maxconfigured, otherwise the feature is simply off and the test tells you nothing.Before, both files give you the same message:
After:
Checklist
main— no, this one is a backport ontodtq-dev.role="alert"region and the same escaped rendering path, only the text differs. I haven't put a screen reader on it.Written with some help from Claude Code.