Skip to content

Commit d17701c

Browse files
jr-rkclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 0f0555e commit d17701c

8 files changed

Lines changed: 136 additions & 16 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* An interface that represents a completed single-file upload, carrying both the
3+
* parsed response body and the client-side file name of the file that completed.
4+
*/
5+
export interface UploaderCompleteEvent {
6+
/**
7+
* The parsed response body (e.g. a WorkspaceItem in the submission workflow)
8+
*/
9+
response: any;
10+
11+
/**
12+
* The client-side name of the file that completed uploading
13+
*/
14+
fileName: string;
15+
}

src/app/shared/upload/uploader/uploader.component.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,26 @@ describe('Chips component', () => {
6666
expect(app).toBeDefined();
6767
}));
6868

69+
it('should emit both onCompleteItem and onCompleteItemWithFile on a completed upload', inject([UploaderComponent], (app: UploaderComponent) => {
70+
app.uploadFilesOptions = Object.assign(new UploaderOptions(), {
71+
url: 'http://test',
72+
authToken: null,
73+
disableMultipart: false,
74+
itemAlias: null,
75+
});
76+
app.ngOnInit();
77+
app.ngAfterViewInit();
78+
79+
spyOn(app.onCompleteItem, 'emit');
80+
spyOn(app.onCompleteItemWithFile, 'emit');
81+
82+
const parsed = { foo: 'bar' };
83+
app.uploader.onCompleteItem({ file: { name: 'test.pdf' } } as any, JSON.stringify(parsed), 200, {});
84+
85+
expect(app.onCompleteItem.emit).toHaveBeenCalledWith(parsed);
86+
expect(app.onCompleteItemWithFile.emit).toHaveBeenCalledWith({ response: parsed, fileName: 'test.pdf' });
87+
}));
88+
6989
});
7090

7191
// declare a test component

src/app/shared/upload/uploader/uploader.component.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
isNotEmpty,
3434
isUndefined,
3535
} from '../../empty.util';
36+
import { UploaderCompleteEvent } from './uploader-complete-event.model';
3637
import { UploaderOptions } from './uploader-options.model';
3738
import { UploaderProperties } from './uploader-properties.model';
3839

@@ -92,6 +93,12 @@ export class UploaderComponent implements OnInit, AfterViewInit {
9293
*/
9394
@Output() onCompleteItem: EventEmitter<any> = new EventEmitter<any>();
9495

96+
/**
97+
* The function to call when upload is completed, carrying the parsed response together with the
98+
* client-side file name. Emitted alongside {@link onCompleteItem} so existing consumers are unaffected.
99+
*/
100+
@Output() onCompleteItemWithFile: EventEmitter<UploaderCompleteEvent> = new EventEmitter<UploaderCompleteEvent>();
101+
95102
/**
96103
* The function to call on error occurred
97104
*/
@@ -195,6 +202,7 @@ export class UploaderComponent implements OnInit, AfterViewInit {
195202
if (isNotEmpty(response)) {
196203
const responsePath = JSON.parse(response);
197204
this.onCompleteItem.emit(responsePath);
205+
this.onCompleteItemWithFile.emit({ response: responsePath, fileName: item?.file?.name });
198206
}
199207
};
200208
this.uploader.onErrorItem = (item: any, response: any, status: any, headers: any) => {

src/app/submission/form/submission-upload-files/submission-upload-files.component.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
[enableDragOverDocument]="enableDragOverDocument"
55
[onBeforeUpload]="onBeforeUpload"
66
[uploadFilesOptions]="uploadFilesOptions"
7-
(onCompleteItem)="onCompleteItem($event)"
8-
(onUploadError)="onUploadError()"></ds-uploader>
7+
(onCompleteItemWithFile)="onCompleteItem($event)"
8+
(onUploadError)="onUploadError($event)"></ds-uploader>

src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ describe('SubmissionUploadFilesComponent Component', () => {
167167
const expectedErrors: any = mockUploadResponse1ParsedErrors;
168168
fixture.detectChanges();
169169

170-
comp.onCompleteItem(Object.assign({}, uploadRestResponse, { sections: mockSectionsData }));
170+
comp.onCompleteItem({
171+
response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }),
172+
fileName: 'test.pdf',
173+
});
171174

172175
Object.keys(mockSectionsData).forEach((sectionId) => {
173176
expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith(
@@ -183,15 +186,43 @@ describe('SubmissionUploadFilesComponent Component', () => {
183186

184187
});
185188

189+
it('should include the file name in the success notification content', () => {
190+
fixture.detectChanges();
191+
192+
comp.onCompleteItem({
193+
response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }),
194+
fileName: 'test.pdf',
195+
});
196+
197+
expect(translateService.get).toHaveBeenCalledWith(
198+
'submission.sections.upload.upload-successful-file',
199+
{ fileName: 'test.pdf' },
200+
);
201+
});
202+
203+
it('should fall back to the generic success key when no file name is available', () => {
204+
fixture.detectChanges();
205+
206+
comp.onCompleteItem({
207+
response: Object.assign({}, uploadRestResponse, { sections: mockSectionsData }),
208+
fileName: undefined,
209+
});
210+
211+
expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-successful');
212+
});
213+
186214
it('should show an error notification and call updateSectionData if unsuccessful', () => {
187215
const responseErrors = mockUploadResponse2Errors;
188216
const expectedErrors: any = mockUploadResponse2ParsedErrors;
189217
fixture.detectChanges();
190218

191-
comp.onCompleteItem(Object.assign({}, uploadRestResponse, {
192-
sections: mockSectionsData,
193-
errors: responseErrors.errors,
194-
}));
219+
comp.onCompleteItem({
220+
response: Object.assign({}, uploadRestResponse, {
221+
sections: mockSectionsData,
222+
errors: responseErrors.errors,
223+
}),
224+
fileName: 'test.pdf',
225+
});
195226

196227
Object.keys(mockSectionsData).forEach((sectionId) => {
197228
expect(sectionsServiceStub.updateSectionData).toHaveBeenCalledWith(
@@ -207,6 +238,25 @@ describe('SubmissionUploadFilesComponent Component', () => {
207238

208239
});
209240
});
241+
242+
describe('on upload error', () => {
243+
it('should show an error notification including the file name when available', () => {
244+
comp.onUploadError({ item: { file: { name: 'broken.zip' } } });
245+
246+
expect(notificationsServiceStub.error).toHaveBeenCalled();
247+
expect(translateService.get).toHaveBeenCalledWith(
248+
'submission.sections.upload.upload-failed-file',
249+
{ fileName: 'broken.zip' },
250+
);
251+
});
252+
253+
it('should fall back to the generic error key when no file name is available', () => {
254+
comp.onUploadError();
255+
256+
expect(notificationsServiceStub.error).toHaveBeenCalled();
257+
expect(translateService.get).toHaveBeenCalledWith('submission.sections.upload.upload-failed');
258+
});
259+
});
210260
});
211261
});
212262

src/app/submission/form/submission-upload-files/submission-upload-files.component.ts

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import {
2626
} from '../../../shared/empty.util';
2727
import { NotificationsService } from '../../../shared/notifications/notifications.service';
2828
import { UploaderComponent } from '../../../shared/upload/uploader/uploader.component';
29+
import { UploaderCompleteEvent } from '../../../shared/upload/uploader/uploader-complete-event.model';
30+
import { UploaderError } from '../../../shared/upload/uploader/uploader-error.model';
2931
import { UploaderOptions } from '../../../shared/upload/uploader/uploader-options.model';
3032
import { SectionsService } from '../../sections/sections.service';
3133
import { SectionsType } from '../../sections/sections-type';
@@ -133,10 +135,13 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy {
133135
/**
134136
* Parse the submission object retrieved from REST after upload
135137
*
136-
* @param workspaceitem
137-
* The submission object retrieved from REST
138+
* @param event
139+
* The completed upload event, carrying the submission object retrieved from REST and the
140+
* client-side name of the file that completed
138141
*/
139-
public onCompleteItem(workspaceitem: WorkspaceItem) {
142+
public onCompleteItem(event: UploaderCompleteEvent) {
143+
const workspaceitem = event.response as WorkspaceItem;
144+
const fileName = event.fileName;
140145
// Checks if upload section is enabled so do upload
141146
this.subs.push(
142147
this.uploadEnabled
@@ -159,9 +164,9 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy {
159164
if (isUpload) {
160165
// Look for errors on upload
161166
if ((isEmpty(sectionErrors))) {
162-
this.notificationsService.success(null, this.translate.get('submission.sections.upload.upload-successful'));
167+
this.notificationsService.success(null, this.getNotificationContent('upload-successful', fileName));
163168
} else {
164-
this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed'));
169+
this.notificationsService.error(null, this.getNotificationContent('upload-failed', fileName));
165170
}
166171
}
167172
});
@@ -176,9 +181,27 @@ export class SubmissionUploadFilesComponent implements OnChanges, OnDestroy {
176181

177182
/**
178183
* Show error notification on upload fails
184+
*
185+
* @param error
186+
* The upload error, carrying the file that failed to upload (when available)
187+
*/
188+
public onUploadError(error?: UploaderError) {
189+
this.notificationsService.error(null, this.getNotificationContent('upload-failed', error?.item?.file?.name));
190+
}
191+
192+
/**
193+
* Build the translated notification content for an upload outcome, including the file name when
194+
* available. Falls back to the generic (file-name-less) message when the file name is missing.
195+
*
196+
* @param suffix
197+
* The i18n key suffix under `submission.sections.upload.` (e.g. `upload-successful`)
198+
* @param fileName
199+
* The name of the file the notification refers to, if known
179200
*/
180-
public onUploadError() {
181-
this.notificationsService.error(null, this.translate.get('submission.sections.upload.upload-failed'));
201+
private getNotificationContent(suffix: string, fileName?: string): Observable<string> {
202+
return isNotEmpty(fileName)
203+
? this.translate.get(`submission.sections.upload.${suffix}-file`, { fileName })
204+
: this.translate.get(`submission.sections.upload.${suffix}`);
182205
}
183206

184207
/**

src/assets/i18n/en.json5

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5444,8 +5444,12 @@
54445444

54455445
"submission.sections.upload.upload-failed": "Upload failed",
54465446

5447+
"submission.sections.upload.upload-failed-file": "Upload failed for file \"{{fileName}}\"",
5448+
54475449
"submission.sections.upload.upload-successful": "Upload successful",
54485450

5451+
"submission.sections.upload.upload-successful-file": "File \"{{fileName}}\" uploaded successfully",
5452+
54495453
"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.",
54505454

54515455
"submission.sections.accesses.form.discoverable-label": "Discoverable",

src/themes/datashare/app/submission/form/submission-upload-files/submission-upload-files.component.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99
[enableDragOverDocument]="enableDragOverDocument"
1010
[onBeforeUpload]="onBeforeUpload"
1111
[uploadFilesOptions]="uploadFilesOptions"
12-
(onCompleteItem)="onCompleteItem($event)"
13-
(onUploadError)="onUploadError()"></ds-uploader>
12+
(onCompleteItemWithFile)="onCompleteItem($event)"
13+
(onUploadError)="onUploadError($event)"></ds-uploader>
1414
<!-- DATASHARE - end -->

0 commit comments

Comments
 (0)