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..59cda3e11c7 --- /dev/null +++ b/src/app/shared/upload/uploader/uploader-complete-event.model.ts @@ -0,0 +1,17 @@ +/** + * 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. the submission object returned by REST) + */ + response: any; + + /** + * The client-side name of the file that completed uploading. Present only when a + * non-empty file name is known — an empty file name is never emitted, so the presence + * of this key means a usable name is available. Whitespace-only names are not trimmed. + */ + 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 90762875ca1..a6b0f2fb62a 100644 --- a/src/app/shared/upload/uploader/uploader.component.spec.ts +++ b/src/app/shared/upload/uploader/uploader.component.spec.ts @@ -8,7 +8,7 @@ import { DragService } from '../../../core/drag.service'; import { UploaderOptions } from './uploader-options.model'; import { UploaderComponent } from './uploader.component'; import { FileUploadModule } from 'ng2-file-upload'; -import { TranslateModule } from '@ngx-translate/core'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; import { createTestComponent } from '../../testing/utils.test'; import { HttpXsrfTokenExtractor } from '@angular/common/http'; import { CookieService } from '../../../core/services/cookie.service'; @@ -69,6 +69,233 @@ 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.onCompleteItem.emit).toHaveBeenCalledTimes(1); + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: 'test.pdf' }); + })); + + it('should not emit either completion output when the response body is empty', 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'); + + app.uploader.onCompleteItem({ file: { name: 'test.pdf' } } as any, '', 204, {}); + + expect(app.onCompleteItem.emit).not.toHaveBeenCalled(); + expect(app.onCompleteItemWithFile.emit).not.toHaveBeenCalled(); + })); + + it('should omit fileName from the completion event when the item is undefined', 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, {}); + + const arg = (app.onCompleteItemWithFile.emit as jasmine.Spy).calls.mostRecent().args[0]; + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed }); + expect(Object.keys(arg)).toEqual(['response']); + expect('fileName' in arg).toBeFalse(); + })); + + it('should omit fileName from the completion event when the item has no file', 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({} as any, JSON.stringify(parsed), 200, {}); + + const arg = (app.onCompleteItemWithFile.emit as jasmine.Spy).calls.mostRecent().args[0]; + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed }); + expect(Object.keys(arg)).toEqual(['response']); + expect('fileName' in arg).toBeFalse(); + })); + + it('should omit fileName from the completion event when the file has no 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({ file: {} } as any, JSON.stringify(parsed), 200, {}); + + const arg = (app.onCompleteItemWithFile.emit as jasmine.Spy).calls.mostRecent().args[0]; + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed }); + expect(Object.keys(arg)).toEqual(['response']); + expect('fileName' in arg).toBeFalse(); + })); + + it('should omit fileName from the completion event when the file name is an empty string', 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({ file: { name: '' } } as any, JSON.stringify(parsed), 200, {}); + + const arg = (app.onCompleteItemWithFile.emit as jasmine.Spy).calls.mostRecent().args[0]; + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed }); + expect(Object.keys(arg)).toEqual(['response']); + expect('fileName' in arg).toBeFalse(); + })); + + it('should keep a whitespace-only file name on the completion event', 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({ file: { name: ' ' } } as any, JSON.stringify(parsed), 200, {}); + + expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: ' ' }); + })); + + it('should emit a distinct file name for each of two sequential completed uploads', 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'); + + app.uploader.onCompleteItem({ file: { name: 'first.pdf' } } as any, JSON.stringify({ n: 1 }), 200, {}); + app.uploader.onCompleteItem({ file: { name: 'second.pdf' } } as any, JSON.stringify({ n: 2 }), 200, {}); + + const emitSpy = app.onCompleteItemWithFile.emit as jasmine.Spy; + expect(emitSpy).toHaveBeenCalledTimes(2); + expect(emitSpy.calls.argsFor(0)[0]).toEqual({ response: { n: 1 }, fileName: 'first.pdf' }); + expect(emitSpy.calls.argsFor(1)[0]).toEqual({ response: { n: 2 }, fileName: 'second.pdf' }); + })); + + it('should emit onUploadError with the item, response, status and headers of the failed 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.onUploadError, 'emit'); + + app.uploader.onErrorItem({ file: { name: 'broken.zip' } } as any, 'boom', 500, {}); + + expect(app.onUploadError.emit).toHaveBeenCalledWith({ + item: { file: { name: 'broken.zip' } }, + response: 'boom', + status: 500, + headers: {}, + }); + })); + + it('should emit the un-interpolated size-limit message when a file exceeds the maximum upload size', inject([UploaderComponent], (app: UploaderComponent) => { + app.uploadFilesOptions = Object.assign(new UploaderOptions(), { + url: 'http://test', + authToken: null, + disableMultipart: false, + itemAlias: null, + }); + app.ngOnInit(); + app.ngAfterViewInit(); + + const instantSpy = spyOn(TestBed.inject(TranslateService), 'instant').and.returnValue('SIZE-LIMIT-MSG'); + spyOn(app.onUploadError, 'emit'); + + app.uploader.options.maxFileSize = 1024; + app.uploader.onWhenAddingFileFailed({ name: 'big.zip', size: 2048 } as any, null, app.uploader.options); + + expect(instantSpy).toHaveBeenCalledWith('submission.sections.upload.upload-failed.size-limit-exceeded'); + expect(app.onUploadError.emit).toHaveBeenCalledWith(jasmine.objectContaining({ + status: 400, + response: 'SIZE-LIMIT-MSG', + })); + })); + + it('should not pass interpolation params to the size-limit instant() call', inject([UploaderComponent], (app: UploaderComponent) => { + app.uploadFilesOptions = Object.assign(new UploaderOptions(), { + url: 'http://test', + authToken: null, + disableMultipart: false, + itemAlias: null, + }); + app.ngOnInit(); + app.ngAfterViewInit(); + + const instantSpy = spyOn(TestBed.inject(TranslateService), 'instant').and.returnValue('SIZE-LIMIT-MSG'); + + app.uploader.options.maxFileSize = 1024; + app.uploader.onWhenAddingFileFailed({ name: 'big.zip', size: 2048 } as any, null, app.uploader.options); + + expect(instantSpy.calls.count()).toBe(1); + expect(instantSpy.calls.mostRecent().args.length).toBe(1); + })); + }); // 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 413f10cd411..c25b388748b 100644 --- a/src/app/shared/upload/uploader/uploader.component.ts +++ b/src/app/shared/upload/uploader/uploader.component.ts @@ -15,6 +15,8 @@ import { FileUploader } from 'ng2-file-upload'; import uniqueId from 'lodash/uniqueId'; import { ScrollToService } from '@nicky-lenaers/ngx-scroll-to'; +import { UploaderCompleteEvent } from './uploader-complete-event.model'; +import { UploaderError } from './uploader-error.model'; import { UploaderOptions } from './uploader-options.model'; import { hasValue, isNotEmpty, isUndefined } from '../../empty.util'; import { UploaderProperties } from './uploader-properties.model'; @@ -89,10 +91,16 @@ 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 */ - @Output() onUploadError: EventEmitter = new EventEmitter(); + @Output() onUploadError: EventEmitter = new EventEmitter(); /** * The function to call when a file is selected @@ -201,6 +209,8 @@ export class UploaderComponent implements OnInit, AfterViewInit { if (isNotEmpty(response)) { const responsePath = JSON.parse(response); this.onCompleteItem.emit(responsePath); + const fileName = item?.file?.name; + this.onCompleteItemWithFile.emit(isNotEmpty(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.html b/src/app/submission/form/submission-upload-files/submission-upload-files.component.html index cf916fb413b..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)" + (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 fa7ecebbff6..5c958f14b15 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 @@ -158,7 +158,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( @@ -179,10 +182,13 @@ describe('SubmissionUploadFilesComponent Component', () => { 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( @@ -197,6 +203,213 @@ describe('SubmissionUploadFilesComponent Component', () => { expect(notificationsServiceStub.success).not.toHaveBeenCalled(); }); + + it('should include the file name in the success notification content', () => { + translateService.instant.and.callFake((key: string) => 'T:' + key); + 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', default: 'T:submission.sections.upload.upload-successful' }, + ); + expect(translateService.get).not.toHaveBeenCalledWith('submission.sections.upload.upload-successful'); + expect(notificationsServiceStub.success).toHaveBeenCalledTimes(1); + }); + + it('should fall back to the generic success key when no file name is available', () => { + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + }); + + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-successful'); + expect(translateService.get).not.toHaveBeenCalledWith( + 'submission.sections.upload.upload-successful-file', jasmine.anything()); + expect(notificationsServiceStub.success).toHaveBeenCalledTimes(1); + }); + + it('should fall back to the generic success key when the file name is an empty string', () => { + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + fileName: '', + }); + + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-successful'); + expect(translateService.get).not.toHaveBeenCalledWith( + 'submission.sections.upload.upload-successful-file', jasmine.anything()); + }); + + it('should include the file name in the error notification content when the upload section has errors', () => { + const responseErrors = mockUploadResponse2Errors; + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { + sections: mockSectionsData, + errors: responseErrors.errors + }), + fileName: 'test.pdf', + }); + + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', + { fileName: 'test.pdf', default: 'T:submission.sections.upload.upload-failed' }, + ); + expect(translateService.get).not.toHaveBeenCalledWith('submission.sections.upload.upload-failed'); + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(1); + expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + }); + + it('should fall back to the generic error key when the upload section has errors and no file name is available', () => { + const responseErrors = mockUploadResponse2Errors; + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { + sections: mockSectionsData, + errors: responseErrors.errors + }), + }); + + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-failed'); + expect(translateService.get).not.toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', jasmine.anything()); + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(1); + }); + + it('should not notify when the completion response carries no sections', () => { + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ response: { message: 'forced' }, fileName: 'x.pdf' }); + + expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + expect(notificationsServiceStub.error).not.toHaveBeenCalled(); + }); + + it('should not throw when the completion event is malformed', () => { + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + expect(() => comp.onCompleteItem(undefined as any)).not.toThrow(); + expect(() => comp.onCompleteItem({ response: undefined })).not.toThrow(); + + expect(notificationsServiceStub.success).not.toHaveBeenCalled(); + expect(notificationsServiceStub.error).not.toHaveBeenCalled(); + }); + + it('should raise file-name notifications on the escaped rendering path', () => { + const hostileName = '.pdf'; + translateService.instant.and.callFake((key: string) => 'T:' + key); + fixture.detectChanges(); + + comp.onCompleteItem({ + response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }), + fileName: hostileName, + }); + comp.onUploadError({ item: { file: { name: hostileName } }, response: 'boom', status: 500, headers: {} }); + + // Two arguments only: NotificationsService.success/error(title, content, options?, html = false). + // A 4th positional `true` would move the content to the [innerHTML] branch of + // notification.component.html, where an attacker-controlled file name would be parsed as markup. + expect(notificationsServiceStub.success.calls.mostRecent().args.length).toBe(2); + expect(notificationsServiceStub.error.calls.mostRecent().args.length).toBe(2); + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-successful-file', + jasmine.objectContaining({ fileName: hostileName })); + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', + jasmine.objectContaining({ fileName: hostileName })); + }); + }); + + describe('on upload error', () => { + beforeEach(() => { + // The bare getMockTranslateService() spy returns undefined for every key, which would make the + // size-limit discriminator compare undefined === undefined and take the wrong branch. A real + // TranslateService never returns undefined from instant() - it returns the key when a + // translation is missing - so the callFake below is what makes this spec faithful. + translateService.instant.and.callFake((key: string) => 'T:' + key); + translateService.get.and.callFake((key: string, params?: any) => + observableOf(params ? key + ':' + params.fileName : key)); + }); + + it('should show an error notification including the file name when available', () => { + comp.onUploadError({ item: { file: { name: 'broken.zip' } }, response: 'boom', status: 500, headers: {} }); + + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', + { fileName: 'broken.zip', default: 'T:submission.sections.upload.upload-failed' }, + ); + expect(translateService.get).not.toHaveBeenCalledWith('submission.sections.upload.upload-failed'); + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(1); + }); + + it('should read the file name from item.name when the failed item has no file wrapper', () => { + comp.onUploadError({ item: { name: 'big.zip', size: 9e9 }, response: 'boom', status: 400, headers: {} }); + + expect(translateService.get).toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', + { fileName: 'big.zip', default: 'T:submission.sections.upload.upload-failed' }, + ); + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(1); + }); + + it('should fall back to the generic error key when no file name is available', () => { + comp.onUploadError(); + comp.onUploadError({}); + + expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-failed'); + expect(translateService.get).not.toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', jasmine.anything()); + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(2); + }); + + it('should show the un-interpolated size-limit message when the upload failed because the file is too large', () => { + comp.onUploadError({ + item: { name: 'big.zip', size: 9e9 }, + response: 'T:submission.sections.upload.upload-failed.size-limit-exceeded', + status: 400, + headers: {}, + }); + + expect(notificationsServiceStub.error).toHaveBeenCalledTimes(1); + expect(translateService.get).not.toHaveBeenCalled(); + expect(translateService.instant.calls.mostRecent().args.length).toBe(1); + expect(translateService.instant.calls.mostRecent().args[0]) + .toBe('submission.sections.upload.upload-failed.size-limit-exceeded'); + }); + + it('should not add the file name to the size-limit message', () => { + comp.onUploadError({ + item: { name: 'big.zip', size: 9e9 }, + response: 'T:submission.sections.upload.upload-failed.size-limit-exceeded', + status: 400, + headers: {}, + }); + + expect(translateService.get).not.toHaveBeenCalledWith( + 'submission.sections.upload.upload-failed-file', jasmine.anything()); + // getNotificationContent is the only other producer of notification content and it ALWAYS + // calls translate.get. `get` never being called therefore proves the ternary took the + // size-limit branch and that the raw, un-interpolated size-limit string is what reached + // NotificationsService - without asserting message identity through the notifications stub, + // which AC-T-05 forbids because the shared-spy mock makes such assertions unfalsifiable. + expect(translateService.get).not.toHaveBeenCalled(); + expect(notificationsServiceStub.error.calls.mostRecent().args.length).toBe(2); + }); }); }); }); 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 d16500a8640..1c103323d09 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 @@ -9,6 +9,8 @@ import { hasValue, isEmpty, isNotEmpty } from '../../../shared/empty.util'; import { normalizeSectionData } from '../../../core/submission/submission-response-parsing.service'; import { SubmissionService } from '../../submission.service'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; +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 parseSectionErrors from '../../utils/parseSectionErrors'; import { SubmissionJsonPatchOperationsService } from '../../../core/submission/submission-json-patch-operations.service'; @@ -111,16 +113,19 @@ export class SubmissionUploadFilesComponent implements OnChanges { /** * 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 .pipe(first()) .subscribe((isUploadEnabled) => { - if (isUploadEnabled) { + if (isUploadEnabled && hasValue(workspaceitem)) { const { sections } = workspaceitem; const { errors } = workspaceitem; @@ -137,9 +142,9 @@ export class SubmissionUploadFilesComponent implements OnChanges { 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)); } } }); @@ -153,14 +158,49 @@ export class SubmissionUploadFilesComponent implements OnChanges { } /** - * Show error notification on upload fails + * Show error notification on upload fails. + * + * The client-side size-limit rejection is discriminated FIRST, by comparing the emitted `response` + * against the size-limit message the uploader produced with the very same ONE-ARGUMENT + * `translate.instant(key)` call. Adding interpolation params to either side would make this `===` + * silently false, with no compile error, so the comparison and both `instant()` arities must stay + * exactly as they are. Only when that comparison fails is the default message built, and the + * default message is the only one that carries the file name. + * + * @param error + * The upload error, carrying the file that failed to upload (when available) */ - public onUploadError(event: any) { + public onUploadError(error?: UploaderError) { const errorMessageUploadLimit = this.translate.instant('submission.sections.upload.upload-failed.size-limit-exceeded'); - const defaultErrorMessage = this.translate.instant('submission.sections.upload.upload-failed'); - const errorMessage = event?.response === errorMessageUploadLimit ? errorMessageUploadLimit : defaultErrorMessage; + const isFileSizeLimitError = error?.response === errorMessageUploadLimit; + // `onErrorItem` emits a ng2-file-upload FileItem (name under `file`), `onWhenAddingFileFailed` + // emits a bare FileLikeObject (name at the top level), so both shapes have to be covered. + const fileName = error?.item?.file?.name ?? error?.item?.name; + + this.notificationsService.error(null, isFileSizeLimitError + ? errorMessageUploadLimit + : this.getNotificationContent('upload-failed', fileName)); + } - this.notificationsService.error(null, errorMessage); + /** + * 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. + * The `default` interpolate param is honoured by MissingTranslationHelper, so a locale that has not + * yet translated the `-file` key renders the generic message rather than a raw dotted key. + * + * @param suffix + * The i18n key suffix within the upload section (e.g. `upload-successful`); the helper reads + * `-file` when a file name is known and plain `` otherwise + * @param fileName + * The name of the file the notification refers to, if known + */ + private getNotificationContent(suffix: string, fileName?: string): Observable { + return isNotEmpty(fileName) + ? this.translate.get(`submission.sections.upload.${suffix}-file`, { + fileName, + default: this.translate.instant(`submission.sections.upload.${suffix}`), + }) + : this.translate.get(`submission.sections.upload.${suffix}`); } /** diff --git a/src/assets/i18n/cs.json5 b/src/assets/i18n/cs.json5 index 8d8c4a6035a..9c1a82c6506 100644 --- a/src/assets/i18n/cs.json5 +++ b/src/assets/i18n/cs.json5 @@ -7215,9 +7215,15 @@ // "submission.sections.upload.upload-failed": "Upload failed", "submission.sections.upload.upload-failed": "Nahrání se nezdařilo", + // "submission.sections.upload.upload-failed-file": "Upload failed for file \"{{fileName}}\"", + "submission.sections.upload.upload-failed-file": "Nahrání souboru \"{{fileName}}\" se nezdařilo", + // "submission.sections.upload.upload-successful": "Upload successful", "submission.sections.upload.upload-successful": "Úspěšně nahráno", + // "submission.sections.upload.upload-successful-file": "File \"{{fileName}}\" uploaded successfully", + "submission.sections.upload.upload-successful-file": "Soubor \"{{fileName}}\" byl úspěšně nahrán", + // "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-description": "Když je zaškrtnuto, bude tento záznam zjistitelný ve vyhledávání/prohlížení. Když není zaškrtnuto, záznam bude dostupný pouze prostřednictvím přímého odkazu a nikdy se nezobrazí ve vyhledávání/přehledu.", diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 8def4121098..6f1ec59821f 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -4811,8 +4811,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",