From 8401f939cc0c82c45cf71f8cf68b61b672ded22f Mon Sep 17 00:00:00 2001 From: suzanadirla Date: Thu, 2 Nov 2017 18:50:24 +0200 Subject: [PATCH] [ACA-9380] fix Incorrect behavior when doing a multiple selection Copy or Move and only some of the items fail to be copied (#31) * [ACA-938] Incorrect behavior when doing a multiple selection Copy or Move and only some of the items fail to be copied * [ACA-938] Incorrect behavior when doing a multiple selection Copy or Move and only some of the items fail to be copied unit tests changes according to partial action changes --- .../directives/node-copy.directive.spec.ts | 76 +++++++++++++++++++ .../common/directives/node-copy.directive.ts | 19 ++++- .../services/node-actions.service.spec.ts | 33 ++++---- .../common/services/node-actions.service.ts | 30 +++----- src/assets/i18n/de.json | 6 +- src/assets/i18n/en.json | 8 +- src/assets/i18n/es.json | 6 +- src/assets/i18n/fr.json | 6 +- src/assets/i18n/it.json | 6 +- src/assets/i18n/ja.json | 6 +- src/assets/i18n/nb.json | 6 +- src/assets/i18n/nl.json | 6 +- src/assets/i18n/pt-BR.json | 6 +- src/assets/i18n/ru.json | 6 +- src/assets/i18n/zh-CN.json | 6 +- 15 files changed, 156 insertions(+), 70 deletions(-) diff --git a/src/app/common/directives/node-copy.directive.spec.ts b/src/app/common/directives/node-copy.directive.spec.ts index 84297332d..113df96da 100644 --- a/src/app/common/directives/node-copy.directive.spec.ts +++ b/src/app/common/directives/node-copy.directive.spec.ts @@ -116,6 +116,82 @@ describe('NodeCopyDirective', () => { ); }); + it('notifies partially copy of one node out of a multiple selection of nodes', () => { + spyOn(service, 'copyNodes').and.returnValue(Observable.of('OPERATION.SUCCES.CONTENT.COPY')); + + component.selection = [ + { entry: { id: 'node-to-copy-1', name: 'name1' } }, + { entry: { id: 'node-to-copy-2', name: 'name2' } }]; + const createdItems = [ + { entry: { id: 'copy-of-node-1', name: 'name1' } }]; + + fixture.detectChanges(); + element.triggerEventHandler('click', null); + service.contentCopied.next(createdItems); + + expect(service.copyNodes).toHaveBeenCalled(); + expect(notificationService.openSnackMessageAction).toHaveBeenCalledWith( + 'APP.MESSAGES.INFO.NODE_COPY.PARTIAL_SINGULAR', 'Undo', 10000 + ); + }); + + it('notifies partially copy of more nodes out of a multiple selection of nodes', () => { + spyOn(service, 'copyNodes').and.returnValue(Observable.of('OPERATION.SUCCES.CONTENT.COPY')); + + component.selection = [ + { entry: { id: 'node-to-copy-0', name: 'name0' } }, + { entry: { id: 'node-to-copy-1', name: 'name1' } }, + { entry: { id: 'node-to-copy-2', name: 'name2' } }]; + const createdItems = [ + { entry: { id: 'copy-of-node-0', name: 'name0' } }, + { entry: { id: 'copy-of-node-1', name: 'name1' } }]; + + fixture.detectChanges(); + element.triggerEventHandler('click', null); + service.contentCopied.next(createdItems); + + expect(service.copyNodes).toHaveBeenCalled(); + expect(notificationService.openSnackMessageAction).toHaveBeenCalledWith( + 'APP.MESSAGES.INFO.NODE_COPY.PARTIAL_PLURAL', 'Undo', 10000 + ); + }); + + it('notifies of failed copy of multiple nodes', () => { + spyOn(service, 'copyNodes').and.returnValue(Observable.of('OPERATION.SUCCES.CONTENT.COPY')); + + component.selection = [ + { entry: { id: 'node-to-copy-0', name: 'name0' } }, + { entry: { id: 'node-to-copy-1', name: 'name1' } }, + { entry: { id: 'node-to-copy-2', name: 'name2' } }]; + const createdItems = []; + + fixture.detectChanges(); + element.triggerEventHandler('click', null); + service.contentCopied.next(createdItems); + + expect(service.copyNodes).toHaveBeenCalled(); + expect(notificationService.openSnackMessageAction).toHaveBeenCalledWith( + 'APP.MESSAGES.INFO.NODE_COPY.FAIL_PLURAL', '', 3000 + ); + }); + + it('notifies of failed copy of one node', () => { + spyOn(service, 'copyNodes').and.returnValue(Observable.of('OPERATION.SUCCES.CONTENT.COPY')); + + component.selection = [ + { entry: { id: 'node-to-copy', name: 'name' } }]; + const createdItems = []; + + fixture.detectChanges(); + element.triggerEventHandler('click', null); + service.contentCopied.next(createdItems); + + expect(service.copyNodes).toHaveBeenCalled(); + expect(notificationService.openSnackMessageAction).toHaveBeenCalledWith( + 'APP.MESSAGES.INFO.NODE_COPY.FAIL_SINGULAR', '', 3000 + ); + }); + it('notifies error if success message was not emitted', () => { spyOn(service, 'copyNodes').and.returnValue(Observable.of('')); diff --git a/src/app/common/directives/node-copy.directive.ts b/src/app/common/directives/node-copy.directive.ts index 23c1b41e3..2e771f252 100644 --- a/src/app/common/directives/node-copy.directive.ts +++ b/src/app/common/directives/node-copy.directive.ts @@ -60,14 +60,27 @@ export class NodeCopyDirective { } private toastMessage(info: any, newItems?: MinimalNodeEntity[]) { - const numberOfCopiedItems = newItems ? newItems.length : ''; + const numberOfCopiedItems = newItems ? newItems.length : 0; + const failedItems = this.selection.length - numberOfCopiedItems; let i18nMessageString = 'APP.MESSAGES.ERRORS.GENERIC'; if (typeof info === 'string') { if (info.toLowerCase().indexOf('succes') !== -1) { + let i18MessageSuffix; + + if (failedItems) { + if (numberOfCopiedItems) { + i18MessageSuffix = ( numberOfCopiedItems === 1 ) ? 'PARTIAL_SINGULAR' : 'PARTIAL_PLURAL'; + + } else { + i18MessageSuffix = ( failedItems === 1 ) ? 'FAIL_SINGULAR' : 'FAIL_PLURAL'; + } + + } else { + i18MessageSuffix = ( numberOfCopiedItems === 1 ) ? 'SINGULAR' : 'PLURAL'; + } - const i18MessageSuffix = ( numberOfCopiedItems === 1 ) ? 'SINGULAR' : 'PLURAL'; i18nMessageString = `APP.MESSAGES.INFO.NODE_COPY.${i18MessageSuffix}`; } @@ -86,7 +99,7 @@ export class NodeCopyDirective { const undo = (numberOfCopiedItems > 0) ? 'Undo' : ''; const withUndo = (numberOfCopiedItems > 0) ? '_WITH_UNDO' : ''; - this.translation.get(i18nMessageString, { number: numberOfCopiedItems }).subscribe(message => { + this.translation.get(i18nMessageString, { success: numberOfCopiedItems, failed: failedItems }).subscribe(message => { this.notification.openSnackMessageAction(message, undo, NodeActionsService[`SNACK_MESSAGE_DURATION${withUndo}`]) .onAction() .subscribe(() => this.deleteCopy(newItems)); diff --git a/src/app/common/services/node-actions.service.spec.ts b/src/app/common/services/node-actions.service.spec.ts index 5de14c8b8..04861ec90 100644 --- a/src/app/common/services/node-actions.service.spec.ts +++ b/src/app/common/services/node-actions.service.spec.ts @@ -369,8 +369,8 @@ describe('NodeActionsService', () => { spyOnSuccess.calls.reset(); spyOnError.calls.reset(); copyObservable.toPromise().then( - () => { - spyOnSuccess(); + (response) => { + spyOnSuccess(response); }, () => { spyOnError(); @@ -382,8 +382,9 @@ describe('NodeActionsService', () => { { targetParentId: folderDestination.entry.id, name: undefined } ); }).then(() => { - expect(spyOnSuccess.calls.count()).toEqual(0); - expect(spyOnError.calls.count()).toEqual(1); + expect(spyOnSuccess.calls.count()).toEqual(1); + expect(spyOnSuccess).toHaveBeenCalledWith(permissionError); + expect(spyOnError.calls.count()).toEqual(0); }); })); @@ -401,16 +402,16 @@ describe('NodeActionsService', () => { spyOnError.calls.reset(); copyObservable.toPromise() .then( - () => { - spyOnSuccess(); + (response) => { + spyOnSuccess(response); }, () => { spyOnError(); }) .then( () => { - expect(spyOnSuccess).not.toHaveBeenCalled(); - expect(spyOnError).toHaveBeenCalled(); + expect(spyOnSuccess).toHaveBeenCalledWith(permissionError); + expect(spyOnError).not.toHaveBeenCalled(); expect(spyContentAction).toHaveBeenCalled(); expect(spyFolderAction).not.toHaveBeenCalled(); @@ -694,7 +695,7 @@ describe('NodeActionsService', () => { }); })); - it('should throw permission error in case it occurs', async(() => { + it('should not throw permission error, to be able to show message in case of partial move of files', async(() => { spyOnDocumentListServiceAction = spyOn(documentListService, 'moveNode').and .returnValue(Observable.throw(permissionError)); @@ -711,8 +712,8 @@ describe('NodeActionsService', () => { .then(() => { expect(spyOnDocumentListServiceAction).toHaveBeenCalled(); - expect(spyOnSuccess).not.toHaveBeenCalledWith(permissionError); - expect(spyOnError).toHaveBeenCalledWith(permissionError); + expect(spyOnSuccess).toHaveBeenCalledWith(permissionError); + expect(spyOnError).not.toHaveBeenCalledWith(permissionError); }); })); @@ -750,15 +751,15 @@ describe('NodeActionsService', () => { spyOnError.calls.reset(); }); - it('should throw permission error in case it occurs on folder move', async(() => { + it('should not throw permission error in case it occurs on folder move', async(() => { spyOnDocumentListServiceAction = spyOn(documentListService, 'moveNode').and .returnValue(Observable.throw(permissionError)); const moveFolderActionObservable = service.moveFolderAction(folderToMove.entry, folderDestinationId); moveFolderActionObservable.toPromise() .then( - () => { - spyOnSuccess(); + (value) => { + spyOnSuccess(value); }, (error) => { spyOnError(error); @@ -766,8 +767,8 @@ describe('NodeActionsService', () => { .then(() => { expect(spyOnDocumentListServiceAction).toHaveBeenCalled(); - expect(spyOnSuccess).not.toHaveBeenCalled(); - expect(spyOnError).toHaveBeenCalledWith(permissionError); + expect(spyOnSuccess).toHaveBeenCalledWith(permissionError); + expect(spyOnError).not.toHaveBeenCalled(); }); })); diff --git a/src/app/common/services/node-actions.service.ts b/src/app/common/services/node-actions.service.ts index 227039904..837274980 100644 --- a/src/app/common/services/node-actions.service.ts +++ b/src/app/common/services/node-actions.service.ts @@ -103,11 +103,11 @@ export class NodeActionsService { (newContent) => { observable.next(`OPERATION.SUCCES.${type.toUpperCase()}.${action.toUpperCase()}`); + const processedData = this.processResponse(newContent); if (action === 'copy') { - this.contentCopied.next(newContent); + this.contentCopied.next(processedData.succeeded); } else if (action === 'move') { - const processedData = this.processResponse(newContent); this.contentMoved.next(processedData); } @@ -219,7 +219,8 @@ export class NodeActionsService { if (errStatusCode && errStatusCode === 409) { return this.copyContentAction(contentEntry, selectionId, this.getNewNameFrom(_oldName, contentEntry.name)); } else { - return Observable.throw(err || 'Server error'); + // do not throw error, to be able to show message in case of partial copy of files + return Observable.of(err || 'Server error'); } }); } @@ -268,7 +269,8 @@ export class NodeActionsService { }); } else { - return Observable.throw(err || 'Server error'); + // do not throw error, to be able to show message in case of partial copy of files + return Observable.of(err || 'Server error'); } }); } @@ -367,7 +369,8 @@ export class NodeActionsService { return Observable.zip(...batch); }); } else { - return Observable.throw(err); + // do not throw error, to be able to show message in case of partial move of files + return Observable.of(err); } }); } @@ -382,19 +385,8 @@ export class NodeActionsService { return { itemMoved, initialParentId }; }) .catch((err) => { - let errStatusCode; - try { - const {error: {statusCode}} = JSON.parse(err.message); - errStatusCode = statusCode; - } catch (e) { // - } - - if (errStatusCode && errStatusCode === 409) { - // do not throw error, to be able to show message in case of partial move of files - return Observable.of(err); - } else { - return Observable.throw(err); - } + // do not throw error, to be able to show message in case of partial move of files + return Observable.of(err); }); } @@ -413,7 +405,7 @@ export class NodeActionsService { } }, (err) => { - return Observable.throw(err || 'Server error'); + return Observable.of(err || 'Server error'); }); return matchedNodes; } diff --git a/src/assets/i18n/de.json b/src/assets/i18n/de.json index 319a451dc..9e6753071 100644 --- a/src/assets/i18n/de.json +++ b/src/assets/i18n/de.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "{{ success }} Elemente gelöscht, {{ failed }} konnte(n) nicht gelöscht werden" }, "NODE_COPY": { - "SINGULAR": "{{ number }} Element kopiert.", - "PLURAL": "{{ number }} Elemente kopiert." + "SINGULAR": "{{ success }} Element kopiert.", + "PLURAL": "{{ success }} Elemente kopiert." }, "NODE_MOVE": { "SINGULAR": "{{ success }} Element verschoben.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index c5483baab..5ad48ea50 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -205,8 +205,12 @@ "PARTIAL_PLURAL": "Deleted {{ success }} items, {{ failed }} couldn't be deleted" }, "NODE_COPY": { - "SINGULAR": "Copied {{ number }} item", - "PLURAL": "Copied {{ number }} items" + "SINGULAR": "Copied {{ success }} item", + "PLURAL": "Copied {{ success }} items", + "PARTIAL_SINGULAR": "Copied {{ success }} item, {{ failed }} couldn't be copied.", + "PARTIAL_PLURAL": "Copied {{ success }} items, {{ failed }} couldn't be copied.", + "FAIL_SINGULAR": "{{ failed }} item couldn't be copied.", + "FAIL_PLURAL": "{{ failed }} items couldn't be copied." }, "NODE_MOVE": { "SINGULAR": "Moved {{ success }} item.", diff --git a/src/assets/i18n/es.json b/src/assets/i18n/es.json index c9a4ca3ca..804d7221d 100644 --- a/src/assets/i18n/es.json +++ b/src/assets/i18n/es.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "Se han eliminado {{ success }} elementos, {{ failed }} no se han podido eliminar" }, "NODE_COPY": { - "SINGULAR": "Se ha copiado {{ number }} elemento", - "PLURAL": "Se han copiado {{ number }} elementos" + "SINGULAR": "Se ha copiado {{ success }} elemento", + "PLURAL": "Se han copiado {{ success }} elementos" }, "NODE_MOVE": { "SINGULAR": "Se ha movido {{ success }} elemento.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/fr.json b/src/assets/i18n/fr.json index dc8530d04..d4bdfb7de 100644 --- a/src/assets/i18n/fr.json +++ b/src/assets/i18n/fr.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "{{ success }} éléments supprimés, {{ failed }} n'a/n'ont pas pu être supprimé(s)" }, "NODE_COPY": { - "SINGULAR": "{{ number }} élément copié", - "PLURAL": "{{ number }} éléments copiés" + "SINGULAR": "{{ success }} élément copié", + "PLURAL": "{{ success }} éléments copiés" }, "NODE_MOVE": { "SINGULAR": "{{ success }} élément déplacé.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/it.json b/src/assets/i18n/it.json index e30a62882..a6ba1541e 100644 --- a/src/assets/i18n/it.json +++ b/src/assets/i18n/it.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "Elementi {{ success }} eliminati, impossibile eliminare {{ failed }}" }, "NODE_COPY": { - "SINGULAR": "Copiato {{ number }} elemento", - "PLURAL": "Copiati {{ number }} elementi" + "SINGULAR": "Copiato {{ success }} elemento", + "PLURAL": "Copiati {{ success }} elementi" }, "NODE_MOVE": { "SINGULAR": "Spostato {{ success }} elemento.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/ja.json b/src/assets/i18n/ja.json index d25551c6d..e15849443 100644 --- a/src/assets/i18n/ja.json +++ b/src/assets/i18n/ja.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "{{ success }} 件のアイテムを削除しましたが、{{ failed }} 件は削除できませんでした" }, "NODE_COPY": { - "SINGULAR": "{{ number }} 件のアイテムをコピーしました", - "PLURAL": "{{ number }} 件のアイテムをコピーしました" + "SINGULAR": "{{ success }} 件のアイテムをコピーしました", + "PLURAL": "{{ success }} 件のアイテムをコピーしました" }, "NODE_MOVE": { "SINGULAR": "{{ success }} 件のアイテムを移動しました。", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/nb.json b/src/assets/i18n/nb.json index 6c96cca5a..cc43b70ad 100644 --- a/src/assets/i18n/nb.json +++ b/src/assets/i18n/nb.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "Slettet {{ success }} elementer, {{ failed }} kunne ikke slettes" }, "NODE_COPY": { - "SINGULAR": "Kopierte {{ number }} element", - "PLURAL": "Kopierte {{ number }} elementer" + "SINGULAR": "Kopierte {{ success }} element", + "PLURAL": "Kopierte {{ success }} elementer" }, "NODE_MOVE": { "SINGULAR": "Flyttet {{ success }} element.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/nl.json b/src/assets/i18n/nl.json index 33b5764c2..95fab1cd9 100644 --- a/src/assets/i18n/nl.json +++ b/src/assets/i18n/nl.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "{{ success }} items verwijderd, kan {{ failed }} niet verwijderen" }, "NODE_COPY": { - "SINGULAR": "{{ number }} item gekopieerd", - "PLURAL": "{{ number }} items gekopieerd" + "SINGULAR": "{{ success }} item gekopieerd", + "PLURAL": "{{ success }} items gekopieerd" }, "NODE_MOVE": { "SINGULAR": "{{ success }} item verplaatst.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/pt-BR.json b/src/assets/i18n/pt-BR.json index 96e9819a2..351eedb79 100644 --- a/src/assets/i18n/pt-BR.json +++ b/src/assets/i18n/pt-BR.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "Itens {{ success }} excluídos, não foi possível excluir {{ failed }}" }, "NODE_COPY": { - "SINGULAR": "Item {{ number }} copiado", - "PLURAL": "Itens {{ number }} copiados" + "SINGULAR": "Item {{ success }} copiado", + "PLURAL": "Itens {{ success }} copiados" }, "NODE_MOVE": { "SINGULAR": "Item {{ success }} movido.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/ru.json b/src/assets/i18n/ru.json index 286f0a988..ad6f834ca 100644 --- a/src/assets/i18n/ru.json +++ b/src/assets/i18n/ru.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "Удалено элементов: {{ success }}, не удалось удалить: {{ failed }}" }, "NODE_COPY": { - "SINGULAR": "Скопирован {{ number }} элемент", - "PLURAL": "Скопировано элементов: {{ number }}" + "SINGULAR": "Скопирован {{ success }} элемент", + "PLURAL": "Скопировано элементов: {{ success }}" }, "NODE_MOVE": { "SINGULAR": "Перемещен {{ success }} элемент.", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/assets/i18n/zh-CN.json b/src/assets/i18n/zh-CN.json index aeb79a47b..069508a25 100644 --- a/src/assets/i18n/zh-CN.json +++ b/src/assets/i18n/zh-CN.json @@ -184,8 +184,8 @@ "PARTIAL_PLURAL": "已删除 {{ success }} 项目，{{ failed }} 无法删除" }, "NODE_COPY": { - "SINGULAR": "已复制 {{ number }} 个项目", - "PLURAL": "已复制 {{ number }} 个项目" + "SINGULAR": "已复制 {{ success }} 个项目", + "PLURAL": "已复制 {{ success }} 个项目" }, "NODE_MOVE": { "SINGULAR": "已移动 {{ success }} 项目。", @@ -199,4 +199,4 @@ } } } -} \ No newline at end of file +}