Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/app/shared/upload/uploader/uploader-complete-event.model.ts
Original file line number Diff line number Diff line change
@@ -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, when available
*/
fileName?: string;
}
Comment thread
jr-rk marked this conversation as resolved.
38 changes: 38 additions & 0 deletions src/app/shared/upload/uploader/uploader.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,44 @@ 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' });
}));

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
Expand Down
8 changes: 8 additions & 0 deletions src/app/shared/upload/uploader/uploader.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Comment thread
jr-rk marked this conversation as resolved.

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

/**
* 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<UploaderCompleteEvent> = new EventEmitter<UploaderCompleteEvent>();

/**
* The function to call on error occurred
*/
Expand Down Expand Up @@ -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 });
}
Comment thread
jr-rk marked this conversation as resolved.
};
this.uploader.onErrorItem = (item: any, response: any, status: any, headers: any) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
[enableDragOverDocument]="enableDragOverDocument"
[onBeforeUpload]="onBeforeUpload"
[uploadFilesOptions]="uploadFilesOptions"
(onCompleteItem)="onCompleteItem($event)"
(onUploadError)="onUploadError()"></ds-uploader>
(onCompleteItemWithFile)="onCompleteItem($event)"
(onUploadError)="onUploadError($event)"></ds-uploader>
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
});
Comment thread
jr-rk marked this conversation as resolved.

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(
Expand All @@ -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');
});
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
}
});
Expand All @@ -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<string> {
return isNotEmpty(fileName)
? this.translate.get(`submission.sections.upload.${suffix}-file`, { fileName })
: this.translate.get(`submission.sections.upload.${suffix}`);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/assets/i18n/en.json5
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
[enableDragOverDocument]="enableDragOverDocument"
[onBeforeUpload]="onBeforeUpload"
[uploadFilesOptions]="uploadFilesOptions"
(onCompleteItem)="onCompleteItem($event)"
(onUploadError)="onUploadError()"></ds-uploader>
(onCompleteItemWithFile)="onCompleteItem($event)"
(onUploadError)="onUploadError($event)"></ds-uploader>
<!-- DATASHARE - end -->
Loading