From d17701cfc0d48f279b1ed76347ac3ddf8007ae5c Mon Sep 17 00:00:00 2001 From: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:09:38 +0200 Subject: [PATCH 1/3] feat: include file name in submission upload notifications Multi-file drag-and-drop uploads showed generic 'Upload successful'/'Upload failed' toasts, so users could not tell which file each notification referred to. Add a non-breaking onCompleteItemWithFile output on UploaderComponent that carries the client-side file name alongside the parsed response, and use it (plus the existing error payload) in SubmissionUploadFilesComponent to render per-file notifications, falling back to the generic keys when no name is available. Co-Authored-By: Claude Opus 4.8 --- .../uploader/uploader-complete-event.model.ts | 15 +++++ .../uploader/uploader.component.spec.ts | 20 +++++++ .../upload/uploader/uploader.component.ts | 8 +++ .../submission-upload-files.component.html | 4 +- .../submission-upload-files.component.spec.ts | 60 +++++++++++++++++-- .../submission-upload-files.component.ts | 37 +++++++++--- src/assets/i18n/en.json5 | 4 ++ .../submission-upload-files.component.html | 4 +- 8 files changed, 136 insertions(+), 16 deletions(-) create mode 100644 src/app/shared/upload/uploader/uploader-complete-event.model.ts diff --git a/src/app/shared/upload/uploader/uploader-complete-event.model.ts b/src/app/shared/upload/uploader/uploader-complete-event.model.ts new file mode 100644 index 00000000000..20c6b0e9e12 --- /dev/null +++ b/src/app/shared/upload/uploader/uploader-complete-event.model.ts @@ -0,0 +1,15 @@ +/** + * An interface that represents a completed single-file upload, carrying both the + * parsed response body and the client-side file name of the file that completed. + */ +export interface UploaderCompleteEvent { + /** + * The parsed response body (e.g. a WorkspaceItem in the submission workflow) + */ + response: any; + + /** + * The client-side name of the file that completed uploading + */ + fileName: string; +} diff --git a/src/app/shared/upload/uploader/uploader.component.spec.ts b/src/app/shared/upload/uploader/uploader.component.spec.ts index 90563f10802..bf8a3674808 100644 --- a/src/app/shared/upload/uploader/uploader.component.spec.ts +++ b/src/app/shared/upload/uploader/uploader.component.spec.ts @@ -66,6 +66,26 @@ describe('Chips component', () => { expect(app).toBeDefined(); })); + it('should emit both onCompleteItem and onCompleteItemWithFile on a completed upload', inject([UploaderComponent], (app: UploaderComponent) => { + app.uploadFilesOptions = Object.assign(new UploaderOptions(), { + url: 'http://test', + authToken: null, + disableMultipart: false, + itemAlias: null, + }); + app.ngOnInit(); + app.ngAfterViewInit(); + + spyOn(app.onCompleteItem, 'emit'); + spyOn(app.onCompleteItemWithFile, 'emit'); + + const parsed = { foo: 'bar' }; + app.uploader.onCompleteItem({ file: { name: 'test.pdf' } } as any, JSON.stringify(parsed), 200, {}); + + expect(app.onCompleteItem.emit).toHaveBeenCalledWith(parsed); + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: 'test.pdf' }); + })); + }); // declare a test component diff --git a/src/app/shared/upload/uploader/uploader.component.ts b/src/app/shared/upload/uploader/uploader.component.ts index 804200d220b..cdd0b40c572 100644 --- a/src/app/shared/upload/uploader/uploader.component.ts +++ b/src/app/shared/upload/uploader/uploader.component.ts @@ -33,6 +33,7 @@ import { isNotEmpty, isUndefined, } from '../../empty.util'; +import { UploaderCompleteEvent } from './uploader-complete-event.model'; import { UploaderOptions } from './uploader-options.model'; import { UploaderProperties } from './uploader-properties.model'; @@ -92,6 +93,12 @@ export class UploaderComponent implements OnInit, AfterViewInit { */ @Output() onCompleteItem: EventEmitter = new EventEmitter(); + /** + * The function to call when upload is completed, carrying the parsed response together with the + * client-side file name. Emitted alongside {@link onCompleteItem} so existing consumers are unaffected. + */ + @Output() onCompleteItemWithFile: EventEmitter = new EventEmitter(); + /** * The function to call on error occurred */ @@ -195,6 +202,7 @@ export class UploaderComponent implements OnInit, AfterViewInit { if (isNotEmpty(response)) { const responsePath = JSON.parse(response); this.onCompleteItem.emit(responsePath); + this.onCompleteItemWithFile.emit({ response: responsePath, fileName: item?.file?.name }); } }; this.uploader.onErrorItem = (item: any, response: any, status: any, headers: any) => { diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.html b/src/app/submission/form/submission-upload-files/submission-upload-files.component.html index dfad8c422ec..5e7be8fdf7d 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.html +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.html @@ -4,5 +4,5 @@ [enableDragOverDocument]="enableDragOverDocument" [onBeforeUpload]="onBeforeUpload" [uploadFilesOptions]="uploadFilesOptions" - (onCompleteItem)="onCompleteItem($event)" - (onUploadError)="onUploadError()"> + (onCompleteItemWithFile)="onCompleteItem($event)" + (onUploadError)="onUploadError($event)"> diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts index 72ae72b8462..d7dbb4b4ff4 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts @@ -167,7 +167,10 @@ describe('SubmissionUploadFilesComponent Component', () => { const expectedErrors: any = mockUploadResponse1ParsedErrors; fixture.detectChanges(); - comp.onCompleteItem(Object.assign({}, uploadRestResponse, { sections: mockSectionsData })); + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + fileName: 'test.pdf', + }); Object.keys(mockSectionsData).forEach((sectionId) => { expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( @@ -183,15 +186,43 @@ describe('SubmissionUploadFilesComponent Component', () => { }); + it('should include the file name in the success notification content', () => { + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + fileName: 'test.pdf', + }); + + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-successful-file', + { fileName: 'test.pdf' }, + ); + }); + + it('should fall back to the generic success key when no file name is available', () => { + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + fileName: undefined, + }); + + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-successful'); + }); + it('should show an error notification and call updateSectionData if unsuccessful', () => { const responseErrors = mockUploadResponse2Errors; const expectedErrors: any = mockUploadResponse2ParsedErrors; fixture.detectChanges(); - comp.onCompleteItem(Object.assign({}, uploadRestResponse, { - sections: mockSectionsData, - errors: responseErrors.errors, - })); + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { + sections: mockSectionsData, + errors: responseErrors.errors, + }), + fileName: 'test.pdf', + }); Object.keys(mockSectionsData).forEach((sectionId) => { expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith( @@ -207,6 +238,25 @@ describe('SubmissionUploadFilesComponent Component', () => { }); }); + + describe('on upload error', () => { + it('should show an error notification including the file name when available', () => { + comp.onUploadError({ item: { file: { name: 'broken.zip' } } }); + + expect(notificationsServiceStub.error).toHaveBeenCalled(); + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', + { fileName: 'broken.zip' }, + ); + }); + + it('should fall back to the generic error key when no file name is available', () => { + comp.onUploadError(); + + expect(notificationsServiceStub.error).toHaveBeenCalled(); + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-failed'); + }); + }); }); }); diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts index 3632ec6760d..510aaae1758 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts @@ -26,6 +26,8 @@ import { } from '../../../shared/empty.util'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { UploaderComponent } from '../../../shared/upload/uploader/uploader.component'; +import { UploaderCompleteEvent } from '../../../shared/upload/uploader/uploader-complete-event.model'; +import { UploaderError } from '../../../shared/upload/uploader/uploader-error.model'; import { UploaderOptions } from '../../../shared/upload/uploader/uploader-options.model'; import { SectionsService } from '../../sections/sections.service'; import { SectionsType } from '../../sections/sections-type'; @@ -133,10 +135,13 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy { /** * Parse the submission object retrieved from REST after upload * - * @param workspaceitem - * The submission object retrieved from REST + * @param event + * The completed upload event, carrying the submission object retrieved from REST and the + * client-side name of the file that completed */ - public onCompleteItem(workspaceitem: WorkspaceItem) { + public onCompleteItem(event: UploaderCompleteEvent) { + const workspaceitem = event.response as WorkspaceItem; + const fileName = event.fileName; // Checks if upload section is enabled so do upload this.subs.push( this.uploadEnabled @@ -159,9 +164,9 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy { if (isUpload) { // Look for errors on upload if ((isEmpty(sectionErrors))) { - this.notificationsService.success(null, this.translate.get('submission.sections.upload.upload-successful')); + this.notificationsService.success(null, this.getNotificationContent('upload-successful', fileName)); } else { - this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed')); + this.notificationsService.error(null, this.getNotificationContent('upload-failed', fileName)); } } }); @@ -176,9 +181,27 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy { /** * Show error notification on upload fails + * + * @param error + * The upload error, carrying the file that failed to upload (when available) + */ + public onUploadError(error?: UploaderError) { + this.notificationsService.error(null, this.getNotificationContent('upload-failed', error?.item?.file?.name)); + } + + /** + * Build the translated notification content for an upload outcome, including the file name when + * available. Falls back to the generic (file-name-less) message when the file name is missing. + * + * @param suffix + * The i18n key suffix under `submission.sections.upload.` (e.g. `upload-successful`) + * @param fileName + * The name of the file the notification refers to, if known */ - public onUploadError() { - this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed')); + private getNotificationContent(suffix: string, fileName?: string): Observable { + return isNotEmpty(fileName) + ? this.translate.get(`submission.sections.upload.${suffix}-file`, { fileName }) + : this.translate.get(`submission.sections.upload.${suffix}`); } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 0cb2f77e239..33af3e19080 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -5444,8 +5444,12 @@ "submission.sections.upload.upload-failed": "Upload failed", + "submission.sections.upload.upload-failed-file": "Upload failed for file \"{{fileName}}\"", + "submission.sections.upload.upload-successful": "Upload successful", + "submission.sections.upload.upload-successful-file": "File \"{{fileName}}\" uploaded successfully", + "submission.sections.accesses.form.discoverable-description": "When checked, this item will be discoverable in search/browse. When unchecked, the item will only be available via a direct link and will never appear in search/browse.", "submission.sections.accesses.form.discoverable-label": "Discoverable", diff --git a/src/themes/datashare/app/submission/form/submission-upload-files/submission-upload-files.component.html b/src/themes/datashare/app/submission/form/submission-upload-files/submission-upload-files.component.html index 39b338bf410..6ede15875dc 100644 --- a/src/themes/datashare/app/submission/form/submission-upload-files/submission-upload-files.component.html +++ b/src/themes/datashare/app/submission/form/submission-upload-files/submission-upload-files.component.html @@ -9,6 +9,6 @@ [enableDragOverDocument]="enableDragOverDocument" [onBeforeUpload]="onBeforeUpload" [uploadFilesOptions]="uploadFilesOptions" - (onCompleteItem)="onCompleteItem($event)" - (onUploadError)="onUploadError()"> + (onCompleteItemWithFile)="onCompleteItem($event)" + (onUploadError)="onUploadError($event)"> \ No newline at end of file From 5532ff152ec5640bf982de5cd05a88ba810f8951 Mon Sep 17 00:00:00 2001 From: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:11:43 +0200 Subject: [PATCH 2/3] fix: make UploaderCompleteEvent fileName optional to match emitter The emitter can pass undefined when the FileItem carries no file name; the interface claimed it was always present. Also cover the missing-file-name emit in the uploader spec. Co-Authored-By: Claude Opus 4.8 --- .../uploader/uploader-complete-event.model.ts | 4 ++-- .../upload/uploader/uploader.component.spec.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/app/shared/upload/uploader/uploader-complete-event.model.ts b/src/app/shared/upload/uploader/uploader-complete-event.model.ts index 20c6b0e9e12..aee97950017 100644 --- a/src/app/shared/upload/uploader/uploader-complete-event.model.ts +++ b/src/app/shared/upload/uploader/uploader-complete-event.model.ts @@ -9,7 +9,7 @@ export interface UploaderCompleteEvent { response: any; /** - * The client-side name of the file that completed uploading + * The client-side name of the file that completed uploading, when available */ - fileName: string; + fileName?: string; } diff --git a/src/app/shared/upload/uploader/uploader.component.spec.ts b/src/app/shared/upload/uploader/uploader.component.spec.ts index bf8a3674808..505633368a2 100644 --- a/src/app/shared/upload/uploader/uploader.component.spec.ts +++ b/src/app/shared/upload/uploader/uploader.component.spec.ts @@ -86,6 +86,24 @@ describe('Chips component', () => { expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: 'test.pdf' }); })); + it('should emit onCompleteItemWithFile with an undefined fileName when the item has no file name', inject([UploaderComponent], (app: UploaderComponent) => { + app.uploadFilesOptions = Object.assign(new UploaderOptions(), { + url: 'http://test', + authToken: null, + disableMultipart: false, + itemAlias: null, + }); + app.ngOnInit(); + app.ngAfterViewInit(); + + spyOn(app.onCompleteItemWithFile, 'emit'); + + const parsed = { foo: 'bar' }; + app.uploader.onCompleteItem(undefined, JSON.stringify(parsed), 200, {}); + + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: undefined }); + })); + }); // declare a test component From cd62034989c142f955f27058a1b0de5169f2cb96 Mon Sep 17 00:00:00 2001 From: Juraj Roka <95219754+jr-rk@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:48:37 +0200 Subject: [PATCH 3/3] refactor: address Copilot review on upload notification typing Omit fileName from UploaderCompleteEvent when unknown instead of emitting an explicit undefined, type the onUploadError output as EventEmitter, and drop the explicit undefined fileName from the fallback spec. Co-Authored-By: Claude Opus 4.8 --- src/app/shared/upload/uploader/uploader.component.spec.ts | 4 ++-- src/app/shared/upload/uploader/uploader.component.ts | 6 ++++-- .../submission-upload-files.component.spec.ts | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/app/shared/upload/uploader/uploader.component.spec.ts b/src/app/shared/upload/uploader/uploader.component.spec.ts index 505633368a2..b715dd26c40 100644 --- a/src/app/shared/upload/uploader/uploader.component.spec.ts +++ b/src/app/shared/upload/uploader/uploader.component.spec.ts @@ -86,7 +86,7 @@ describe('Chips component', () => { expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: 'test.pdf' }); })); - it('should emit onCompleteItemWithFile with an undefined fileName when the item has no file name', inject([UploaderComponent], (app: UploaderComponent) => { + it('should emit onCompleteItemWithFile without a fileName when the item has no file name', inject([UploaderComponent], (app: UploaderComponent) => { app.uploadFilesOptions = Object.assign(new UploaderOptions(), { url: 'http://test', authToken: null, @@ -101,7 +101,7 @@ describe('Chips component', () => { const parsed = { foo: 'bar' }; app.uploader.onCompleteItem(undefined, JSON.stringify(parsed), 200, {}); - expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: undefined }); + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed }); })); }); diff --git a/src/app/shared/upload/uploader/uploader.component.ts b/src/app/shared/upload/uploader/uploader.component.ts index cdd0b40c572..e3532d1456b 100644 --- a/src/app/shared/upload/uploader/uploader.component.ts +++ b/src/app/shared/upload/uploader/uploader.component.ts @@ -34,6 +34,7 @@ import { isUndefined, } from '../../empty.util'; import { UploaderCompleteEvent } from './uploader-complete-event.model'; +import { UploaderError } from './uploader-error.model'; import { UploaderOptions } from './uploader-options.model'; import { UploaderProperties } from './uploader-properties.model'; @@ -102,7 +103,7 @@ export class UploaderComponent implements OnInit, AfterViewInit { /** * The function to call on error occurred */ - @Output() onUploadError: EventEmitter = new EventEmitter(); + @Output() onUploadError: EventEmitter = new EventEmitter(); /** * The function to call when a file is selected @@ -202,7 +203,8 @@ export class UploaderComponent implements OnInit, AfterViewInit { if (isNotEmpty(response)) { const responsePath = JSON.parse(response); this.onCompleteItem.emit(responsePath); - this.onCompleteItemWithFile.emit({ response: responsePath, fileName: item?.file?.name }); + const fileName = item?.file?.name; + this.onCompleteItemWithFile.emit(hasValue(fileName) ? { response: responsePath, fileName } : { response: responsePath }); } }; this.uploader.onErrorItem = (item: any, response: any, status: any, headers: any) => { diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts index d7dbb4b4ff4..15a3e6e5c1e 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts @@ -205,7 +205,6 @@ describe('SubmissionUploadFilesComponent Component', () => { comp.onCompleteItem({ response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), - fileName: undefined, }); expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-successful');