From 9a2a62255feb0c9de07d8727290a90251bbdd7f4 Mon Sep 17 00:00:00 2001 From: Dharan <14145706+dhrn@users.noreply.github.com> Date: Tue, 8 Jun 2021 13:30:04 +0530 Subject: [PATCH] [ADF-5369] HTTP 500 response in adf-upload-button is emitted as a success event (#7087) * [ADF-5369] HTTP 500 response in adf-upload-button is emitted as a success event * [ci:force] unit test fixed --- .../app/components/files/files.component.ts | 4 +- .../src/lib/mock/upload.service.mock.ts | 58 ++++++++++++++ .../components/base-upload/upload-base.ts | 5 +- .../upload-button.component.spec.ts | 62 ++++++++------- .../upload-drag-area.component.spec.ts | 75 ++++++++++++++++++- .../upload/components/upload-files.event.ts | 5 +- lib/core/services/upload.service.spec.ts | 2 +- lib/core/services/upload.service.ts | 27 +++---- 8 files changed, 189 insertions(+), 49 deletions(-) create mode 100644 lib/content-services/src/lib/mock/upload.service.mock.ts diff --git a/demo-shell/src/app/components/files/files.component.ts b/demo-shell/src/app/components/files/files.component.ts index b4a67901c19..434128141f4 100644 --- a/demo-shell/src/app/components/files/files.component.ts +++ b/demo-shell/src/app/components/files/files.component.ts @@ -404,8 +404,8 @@ export class FilesComponent implements OnInit, OnChanges, OnDestroy { }); } - openSnackMessageError(message: string) { - this.notificationService.showError(message); + openSnackMessageError(error: any) { + this.notificationService.showError(error.value || error); } openSnackMessageInfo(message: string) { diff --git a/lib/content-services/src/lib/mock/upload.service.mock.ts b/lib/content-services/src/lib/mock/upload.service.mock.ts new file mode 100644 index 00000000000..280ddac86db --- /dev/null +++ b/lib/content-services/src/lib/mock/upload.service.mock.ts @@ -0,0 +1,58 @@ +/*! + * @license + * Copyright 2019 Alfresco Software, Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export const mockUploadSuccessPromise = { + on: function (event, callback) { + if (event === 'success') { + callback(); + } + return this; + }, + catch: function (callback) { + callback(); + return this; + }, + then: function (callback) { + callback(); + return this; + }, + next: function (callback) { + callback(); + return this; + } +}; + +export const mockUploadErrorPromise = { + on: function (event, callback) { + if (event === 'error') { + callback(); + } + return this; + }, + catch: function (callback) { + callback(); + return this; + }, + then: function (callback) { + callback(); + return this; + }, + next: function (callback) { + callback(); + return this; + } +}; diff --git a/lib/content-services/src/lib/upload/components/base-upload/upload-base.ts b/lib/content-services/src/lib/upload/components/base-upload/upload-base.ts index fd57dab35a5..5867c879d1b 100644 --- a/lib/content-services/src/lib/upload/components/base-upload/upload-base.ts +++ b/lib/content-services/src/lib/upload/components/base-upload/upload-base.ts @@ -126,14 +126,15 @@ export abstract class UploadBase implements OnInit, OnDestroy { const event = new UploadFilesEvent( [...filteredFiles], this.uploadService, - this.success + this.success, + this.error ); this.beginUpload.emit(event); if (!event.defaultPrevented) { if (filteredFiles.length > 0) { this.uploadService.addToQueue(...filteredFiles); - this.uploadService.uploadFilesInTheQueue(this.success); + this.uploadService.uploadFilesInTheQueue(this.success, this.error); } } }); diff --git a/lib/content-services/src/lib/upload/components/upload-button.component.spec.ts b/lib/content-services/src/lib/upload/components/upload-button.component.spec.ts index c68c55df0e3..0341f229334 100644 --- a/lib/content-services/src/lib/upload/components/upload-button.component.spec.ts +++ b/lib/content-services/src/lib/upload/components/upload-button.component.spec.ts @@ -23,6 +23,7 @@ import { UploadButtonComponent } from './upload-button.component'; import { NodeEntry } from '@alfresco/js-api'; import { ContentTestingModule } from '../../testing/content.testing.module'; import { TranslateModule } from '@ngx-translate/core'; +import { mockUploadErrorPromise, mockUploadSuccessPromise } from '../../mock/upload.service.mock'; describe('UploadButtonComponent', () => { @@ -124,40 +125,32 @@ describe('UploadButtonComponent', () => { it('should call uploadFile with the default root folder', () => { component.rootFolderId = '-root-'; - component.success = null; - spyOn(contentService, 'getNode').and.returnValue(of(fakeFolderNodeWithPermission)); + spyOn(uploadService, 'uploadFilesInTheQueue').and.stub(); component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); - fixture.detectChanges(); component.onFilesAdded(fakeEvent); - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalled(); }); it('should call uploadFile with a custom root folder', () => { component.rootFolderId = '-my-'; - component.success = null; - spyOn(contentService, 'getNode').and.returnValue(of(fakeFolderNodeWithPermission)); - component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); - - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); + spyOn(uploadService, 'uploadFilesInTheQueue').and.stub(); + component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); fixture.detectChanges(); component.onFilesAdded(fakeEvent); - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalled(); }); it('should not call uploadFiles if rootFolderId is null', () => { component.rootFolderId = null; + spyOn(uploadService, 'uploadFilesInTheQueue').and.stub(); component.ngOnChanges({ rootFolderId: new SimpleChange(null, null, true) }); - - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); - fixture.detectChanges(); component.onFilesAdded(fakeEvent); @@ -351,6 +344,7 @@ describe('UploadButtonComponent', () => { let fakeNodeWithNoPermission; beforeEach(() => { + spyOn(uploadService, 'uploadFilesInTheQueue').and.stub(); fakeNodeWithNoPermission = { entry: {} }; @@ -361,9 +355,6 @@ describe('UploadButtonComponent', () => { spyOn(contentService, 'getNode').and.returnValue(of(fakeNodeWithNoPermission)); component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); - - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); - fixture.detectChanges(); component.onFilesAdded(fakeEvent); @@ -375,9 +366,6 @@ describe('UploadButtonComponent', () => { spyOn(contentService, 'getNode').and.returnValue(throwError('error')); component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); - - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); - fixture.detectChanges(); component.onFilesAdded(fakeEvent); @@ -407,9 +395,6 @@ describe('UploadButtonComponent', () => { spyOn(contentService, 'getNode').and.returnValue(of(fakeNodeWithNoPermission)); component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); - - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); - fixture.detectChanges(); component.onFilesAdded(fakeEvent); @@ -421,13 +406,40 @@ describe('UploadButtonComponent', () => { spyOn(contentService, 'getNode').and.returnValue(of(fakeFolderNodeWithPermission)); component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); + fixture.detectChanges(); - uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue'); + component.onFilesAdded(fakeEvent); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalled(); + }); + }); + describe('Events', () => { + beforeEach(() => { + spyOn(contentService, 'getNode').and.returnValue(of(fakeFolderNodeWithPermission)); + component.rootFolderId = 'nodeId'; + component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId, true) }); fixture.detectChanges(); + }); + + it('should emit an success for successful upload of a file', (done) => { + spyOn(uploadService, 'getUploadPromise').and.returnValue(mockUploadSuccessPromise); + + component.success.subscribe((success) => { + expect(success).not.toBeNull(); + done(); + }); component.onFilesAdded(fakeEvent); - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalled(); + }); + + it('should emit error if upload errored', (done) => { + spyOn(uploadService, 'getUploadPromise').and.returnValue(mockUploadErrorPromise); + + component.error.subscribe((error) => { + expect(error).not.toBeNull(); + done(); + }); + component.onFilesAdded(fakeEvent); }); }); }); diff --git a/lib/content-services/src/lib/upload/components/upload-drag-area.component.spec.ts b/lib/content-services/src/lib/upload/components/upload-drag-area.component.spec.ts index 8a996310433..60596bdced5 100644 --- a/lib/content-services/src/lib/upload/components/upload-drag-area.component.spec.ts +++ b/lib/content-services/src/lib/upload/components/upload-drag-area.component.spec.ts @@ -20,6 +20,7 @@ import { FileModel, UploadService, setupTestBed } from '@alfresco/adf-core'; import { UploadDragAreaComponent } from './upload-drag-area.component'; import { ContentTestingModule } from '../../testing/content.testing.module'; import { TranslateModule } from '@ngx-translate/core'; +import { mockUploadSuccessPromise, mockUploadErrorPromise } from '../../mock/upload.service.mock'; function getFakeShareDataRow(allowableOperations = ['delete', 'update', 'create']) { return { @@ -222,6 +223,7 @@ describe('UploadDragAreaComponent', () => { it('should only upload those files whose fileTypes are in acceptedFilesType', async(() => { spyOn(uploadService, 'uploadFilesInTheQueue'); component.success = null; + component.error = null; component.acceptedFilesType = '.jpg,.pdf'; fixture.detectChanges(); const files: File[] = [ @@ -231,7 +233,7 @@ describe('UploadDragAreaComponent', () => { ]; component.onFilesDropped(files); fixture.whenStable().then(() => { - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null, null); const filesCalledWith = addToQueueSpy.calls.mostRecent().args; expect(filesCalledWith.length).toBe(2, 'Files should contain two elements'); expect(filesCalledWith[0].name).toBe('phobos.jpg'); @@ -242,12 +244,13 @@ describe('UploadDragAreaComponent', () => { it('should upload a file if fileType is in acceptedFilesType', async(() => { spyOn(uploadService, 'uploadFilesInTheQueue'); component.success = null; + component.error = null; component.acceptedFilesType = '.png'; fixture.detectChanges(); fixture.whenStable().then(() => { component.onFilesDropped([new File(['fakefake'], 'file-fake.png', { type: 'image/png' })]); - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null, null); }); })); @@ -281,23 +284,25 @@ describe('UploadDragAreaComponent', () => { it('should not upload a file if fileType is not in acceptedFilesType', async(() => { component.success = null; + component.error = null; component.acceptedFilesType = '.pdf'; fixture.detectChanges(); spyOn(uploadService, 'uploadFilesInTheQueue'); fixture.whenStable().then(() => { component.onFilesDropped([new File(['fakefake'], 'file-fake.png', { type: 'image/png' })]); - expect(uploadService.uploadFilesInTheQueue).not.toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).not.toHaveBeenCalledWith(null, null); }); })); it('should upload a file with a custom root folder ID when dropped', async(() => { component.success = null; + component.error = null; fixture.detectChanges(); spyOn(uploadService, 'uploadFilesInTheQueue'); component.onFilesDropped([new File(['fakefake'], 'file-fake.png', { type: 'image/png' })]); - expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null); + expect(uploadService.uploadFilesInTheQueue).toHaveBeenCalledWith(null, null); })); it('should upload a file when user has create permission on target folder', async(() => { @@ -446,5 +451,67 @@ describe('UploadDragAreaComponent', () => { component.onUploadFiles(fakeCustomEvent); }); + + it('should emit success if successful of upload a file', (done) => { + spyOn(uploadService, 'getUploadPromise').and.returnValue(mockUploadSuccessPromise); + const fakeItem = { + fullPath: '/folder-fake/file-fake.png', + isDirectory: false, + isFile: true, + relativeFolder: '/', + name: 'file-fake.png', + file: (callbackFile) => { + const fileFake = new File(['fakefake'], 'file-fake.png', { type: 'image/png' }); + callbackFile(fileFake); + } + }; + + fixture.detectChanges(); + + component.success.subscribe((success) => { + expect(success).not.toBeNull(); + done(); + }); + + const fakeCustomEvent: CustomEvent = new CustomEvent('CustomEvent', { + detail: { + data: getFakeShareDataRow(), + files: [fakeItem] + } + }); + + component.onUploadFiles(fakeCustomEvent); + }); + + it('should emit error if upload errored', (done) => { + spyOn(uploadService, 'getUploadPromise').and.returnValue(mockUploadErrorPromise); + const fakeItem = { + fullPath: '/folder-fake/file-fake.png', + isDirectory: false, + isFile: true, + relativeFolder: '/', + name: 'file-fake.png', + file: (callbackFile) => { + const fileFake = new File(['fakefake'], 'file-fake.png', { type: 'image/png' }); + callbackFile(fileFake); + } + }; + + fixture.detectChanges(); + + component.error.subscribe((error) => { + expect(error).not.toBeNull(); + done(); + }); + + const fakeCustomEvent: CustomEvent = new CustomEvent('CustomEvent', { + detail: { + data: getFakeShareDataRow(), + files: [fakeItem] + } + }); + + component.onUploadFiles(fakeCustomEvent); + }); }); }); diff --git a/lib/content-services/src/lib/upload/components/upload-files.event.ts b/lib/content-services/src/lib/upload/components/upload-files.event.ts index afa965aa102..3b974c91584 100644 --- a/lib/content-services/src/lib/upload/components/upload-files.event.ts +++ b/lib/content-services/src/lib/upload/components/upload-files.event.ts @@ -32,7 +32,8 @@ export class UploadFilesEvent { constructor( public files: Array, private uploadService: UploadService, - private callback: EventEmitter + private successEmitter: EventEmitter, + private errorEmitter: EventEmitter ) {} pauseUpload() { @@ -42,7 +43,7 @@ export class UploadFilesEvent { resumeUpload() { if (this.files && this.files.length > 0) { this.uploadService.addToQueue(...this.files); - this.uploadService.uploadFilesInTheQueue(this.callback); + this.uploadService.uploadFilesInTheQueue(this.successEmitter, this.errorEmitter); } } } diff --git a/lib/core/services/upload.service.spec.ts b/lib/core/services/upload.service.spec.ts index 7122d9753c8..fd98dfcd43b 100644 --- a/lib/core/services/upload.service.spec.ts +++ b/lib/core/services/upload.service.spec.ts @@ -195,7 +195,7 @@ describe('UploadService', () => { { parentId: '-root-' } ); service.addToQueue(fileFake); - service.uploadFilesInTheQueue(emitter); + service.uploadFilesInTheQueue(null, emitter); expect(jasmine.Ajax.requests.mostRecent().url) .toBe('http://localhost:9876/ecm/alfresco/api/-default-/public/alfresco/versions/1/nodes/-root-/children?autoRename=true&include=allowableOperations'); diff --git a/lib/core/services/upload.service.ts b/lib/core/services/upload.service.ts index 1ca237ba124..5648fe0f1c3 100644 --- a/lib/core/services/upload.service.ts +++ b/lib/core/services/upload.service.ts @@ -166,9 +166,10 @@ export class UploadService { /** * Finds all the files in the queue that are not yet uploaded and uploads them into the directory folder. - * @param emitter Emitter to invoke on file status change + * @param successEmitter Emitter to invoke on file success status change + * @param errorEmitter Emitter to invoke on file error status change */ - uploadFilesInTheQueue(emitter?: EventEmitter): void { + uploadFilesInTheQueue(successEmitter?: EventEmitter, errorEmitter?: EventEmitter): void { if (!this.activeTask) { const file = this.queue.find( (currentFile) => currentFile.status === FileUploadStatus.Pending @@ -176,13 +177,13 @@ export class UploadService { if (file) { this.onUploadStarting(file); - const promise = this.beginUpload(file, emitter); + const promise = this.beginUpload(file, successEmitter, errorEmitter); this.activeTask = promise; this.cache[file.name] = promise; const next = () => { this.activeTask = null; - setTimeout(() => this.uploadFilesInTheQueue(emitter), 100); + setTimeout(() => this.uploadFilesInTheQueue(successEmitter, errorEmitter), 100); }; promise.next = next; @@ -270,7 +271,7 @@ export class UploadService { } } - private beginUpload(file: FileModel, emitter: EventEmitter): any { + private beginUpload(file: FileModel, successEmitter?: EventEmitter, errorEmitter?: EventEmitter): any { const promise = this.getUploadPromise(file); promise .on('progress', (progress: FileUploadProgress) => { @@ -278,14 +279,14 @@ export class UploadService { }) .on('abort', () => { this.onUploadAborted(file); - if (emitter) { - emitter.emit({ value: 'File aborted' }); + if (successEmitter) { + successEmitter.emit({ value: 'File aborted' }); } }) .on('error', (err) => { this.onUploadError(file, err); - if (emitter) { - emitter.emit({ value: 'Error file uploaded' }); + if (errorEmitter) { + errorEmitter.emit({ value: 'Error file uploaded' }); } }) .on('success', (data) => { @@ -296,13 +297,13 @@ export class UploadService { } else { this.deleteAbortedNodeVersion(data.entry.id, data.entry.properties['cm:versionLabel']); } - if (emitter) { - emitter.emit({ value: 'File deleted' }); + if (successEmitter) { + successEmitter.emit({ value: 'File deleted' }); } } else { this.onUploadComplete(file, data); - if (emitter) { - emitter.emit({ value: data }); + if (successEmitter) { + successEmitter.emit({ value: data }); } } })