remove redundant logic (#3361)

demo-shell/src/app/components/files/files.component.html

@@ -110,6 +110,7 @@
                     </button>
                     <button mat-icon-button
                             [disabled]="!documentList.selection.length"
+                            (toggle)="documentList.reload()"
                             #favorite="adfFavorite"
                             [adf-node-favorite]="documentList.selection"
                             matTooltip="{{ 'DOCUMENT_LIST.TOOLBAR.FAVORITES' | translate }}">

demo-shell/src/app/components/files/files.component.ts

@@ -54,7 +54,7 @@ export class FilesComponent implements OnInit, OnChanges, OnDestroy {
     showViewer = false;
     showVersions = false;
     displayMode = DisplayMode.List;
-    includeFields = ['isLocked', 'aspectNames'];
+    includeFields = ['isFavorite', 'isLocked', 'aspectNames'];
 
     baseShareUrl = this.appConfig.get<string>('ecmHost') + '/preview/s/';
 

docs/core/node-favorite.directive.md

@@ -49,6 +49,7 @@ export class MyComponent {
 | Name | Type | Description |
 | -- | -- | -- |
 | toggle | `EventEmitter<any>` | Emitted when the favorite setting is complete. |
+| error | `EventEmitter<any>` | Emitted when the favorite setting has fail. |
 
 ## Details
 

lib/core/directives/node-favorite.directive.spec.ts

@@ -32,139 +32,7 @@ describe('NodeFavoriteDirective', () => {
         directive = new NodeFavoriteDirective( alfrescoApiService);
     });
 
-    describe('selection input change event', () => {
+    describe('toggle node as favorite', () => {
-        it('should not call markFavoritesNodes() if input list is empty', () => {
-            spyOn(directive, 'markFavoritesNodes');
-
-            const change = new SimpleChange(null, [], true);
-            directive.ngOnChanges({'selection': change});
-
-            expect(directive.markFavoritesNodes).not.toHaveBeenCalledWith();
-        });
-
-        it('should call markFavoritesNodes() on input change', () => {
-            spyOn(directive, 'markFavoritesNodes');
-
-            let selection = [{ entry: { id: '1', name: 'name1' } }];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            expect(directive.markFavoritesNodes).toHaveBeenCalledWith(selection);
-
-            selection = [
-                { entry: { id: '1', name: 'name1' } },
-                { entry: { id: '2', name: 'name2' } }
-            ];
-
-            change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            expect(directive.markFavoritesNodes).toHaveBeenCalledWith(selection);
-        });
-
-        it('should reset favorites if selection is empty', fakeAsync(() => {
-            spyOn(alfrescoApiService.getInstance().core.favoritesApi, 'getFavorite').and.returnValue(Promise.resolve());
-
-            let selection = [
-                { entry: { id: '1', name: 'name1' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-            tick();
-
-            expect(directive.hasFavorites()).toBe(true);
-
-            change = new SimpleChange(null, [], true);
-            directive.ngOnChanges({'selection': change});
-            tick();
-
-            expect(directive.hasFavorites()).toBe(false);
-        }));
-    });
-
-    describe('markFavoritesNodes()', () => {
-        let favoritesApiSpy;
-
-        beforeEach(() => {
-            favoritesApiSpy = spyOn(alfrescoApiService.getInstance().core.favoritesApi, 'getFavorite')
-                .and.returnValue(Promise.resolve());
-        });
-
-        it('should check each selected node if it is a favorite', fakeAsync(() => {
-            const selection = [
-                { entry: { id: '1', name: 'name1' } },
-                { entry: { id: '2', name: 'name2' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            tick();
-            expect(favoritesApiSpy.calls.count()).toBe(2);
-
-        }));
-
-        it('should not check processed node when another is unselected', fakeAsync(() => {
-            let selection = [
-                { entry: { id: '1', name: 'name1' } },
-                { entry: { id: '2', name: 'name2' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            tick();
-            expect(directive.favorites.length).toBe(2);
-            expect(favoritesApiSpy.calls.count()).toBe(2);
-
-            favoritesApiSpy.calls.reset();
-
-            selection = [
-                { entry: { id: '2', name: 'name2' } }
-            ];
-
-            change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            tick();
-            expect(directive.favorites.length).toBe(1);
-            expect(favoritesApiSpy).not.toHaveBeenCalled();
-        }));
-
-        it('should not check processed nodes when another is selected', fakeAsync(() => {
-            let selection = [
-                { entry: { id: '1', name: 'name1' } },
-                { entry: { id: '2', name: 'name2' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-
-            tick();
-
-            expect(directive.favorites.length).toBe(2);
-            expect(favoritesApiSpy.calls.count()).toBe(2);
-
-            favoritesApiSpy.calls.reset();
-
-            selection = [
-                { entry: { id: '1', name: 'name1' } },
-                { entry: { id: '2', name: 'name2' } },
-                { entry: { id: '3', name: 'name3' } }
-            ];
-
-            change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-            tick();
-
-            expect(directive.favorites.length).toBe(3);
-            expect(favoritesApiSpy.calls.count()).toBe(1);
-        }));
-    });
-
-    describe('toggleFavorite()', () => {
         let removeFavoriteSpy;
         let addFavoriteSpy;
 
@@ -179,8 +47,7 @@ describe('NodeFavoriteDirective', () => {
         });
 
         it('should not perform action if favorites collection is empty', fakeAsync(() => {
-            let change = new SimpleChange(null, [], true);
+            directive.selection = [];
-            directive.ngOnChanges({'selection': change});
             tick();
 
             directive.toggleFavorite();
@@ -192,7 +59,7 @@ describe('NodeFavoriteDirective', () => {
         it('should call addFavorite() if none is a favorite', () => {
             addFavoriteSpy.and.returnValue(Promise.resolve());
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: false } },
                 { entry: { id: '2', name: 'name2', isFavorite: false } }
             ];
@@ -205,7 +72,7 @@ describe('NodeFavoriteDirective', () => {
         it('should call addFavorite() on node that is not a favorite in selection', () => {
             addFavoriteSpy.and.returnValue(Promise.resolve());
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFile: true, isFolder: false, isFavorite: false } },
                 { entry: { id: '2', name: 'name2', isFile: true, isFolder: false, isFavorite: true } }
             ];
@@ -222,7 +89,7 @@ describe('NodeFavoriteDirective', () => {
         it('should call removeFavoriteSite() if all are favorites', () => {
             removeFavoriteSpy.and.returnValue(Promise.resolve());
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: true } },
                 { entry: { id: '2', name: 'name2', isFavorite: true } }
             ];
@@ -236,7 +103,7 @@ describe('NodeFavoriteDirective', () => {
             removeFavoriteSpy.and.returnValue(Promise.resolve());
             spyOn(directive.toggle, 'emit');
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: true } }
             ];
 
@@ -250,7 +117,7 @@ describe('NodeFavoriteDirective', () => {
             addFavoriteSpy.and.returnValue(Promise.resolve());
             spyOn(directive.toggle, 'emit');
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: false } }
             ];
 
@@ -260,66 +127,38 @@ describe('NodeFavoriteDirective', () => {
             expect(directive.toggle.emit).toHaveBeenCalled();
         }));
 
-        it('should set isFavorites items to false', fakeAsync(() => {
+        it('should emit error event when removeFavoriteSite() fails', fakeAsync(() => {
-            removeFavoriteSpy.and.returnValue(Promise.resolve());
+            removeFavoriteSpy.and.returnValue(Promise.reject('error'));
+            spyOn(directive.error, 'emit');
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: true } }
             ];
 
             directive.toggleFavorite();
             tick();
 
-            expect(directive.hasFavorites()).toBe(false);
+            expect(directive.error.emit).toHaveBeenCalledWith('error');
         }));
 
-        it('should set isFavorites items to true', fakeAsync(() => {
+        it('should emit error event when addFavorite() fails', fakeAsync(() => {
-            addFavoriteSpy.and.returnValue(Promise.resolve());
+            addFavoriteSpy.and.returnValue(Promise.reject('error'));
+            spyOn(directive.error, 'emit');
 
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: false } }
             ];
 
             directive.toggleFavorite();
             tick();
 
-            expect(directive.hasFavorites()).toBe(true);
+            expect(directive.error.emit).toHaveBeenCalledWith('error');
-        }));
-    });
-
-    describe('getFavorite()', () => {
-        it('should process node as favorite', fakeAsync(() => {
-            spyOn(alfrescoApiService.getInstance().core.favoritesApi, 'getFavorite').and.returnValue(Promise.resolve());
-
-            const selection = [
-                { entry: { id: '1', name: 'name1' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-            tick();
-
-            expect(directive.favorites[0].entry.isFavorite).toBe(true);
-        }));
-
-        it('should not process node as favorite', fakeAsync(() => {
-            spyOn(alfrescoApiService.getInstance().core.favoritesApi, 'getFavorite').and.returnValue(Promise.reject({}));
-
-            const selection = [
-                { entry: { id: '1', name: 'name1' } }
-            ];
-
-            let change = new SimpleChange(null, selection, true);
-            directive.ngOnChanges({'selection': change});
-            tick();
-
-            expect(directive.favorites[0].entry.isFavorite).toBe(false);
         }));
     });
 
     describe('hasFavorites()', () => {
         it('should return false when favorites collection is empty', () => {
-            directive.favorites = [];
+            directive.selection = [];
 
             const hasFavorites = directive.hasFavorites();
 
@@ -327,7 +166,7 @@ describe('NodeFavoriteDirective', () => {
         });
 
         it('should return false when some are not favorite', () => {
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: true } },
                 { entry: { id: '2', name: 'name2', isFavorite: false } }
             ];
@@ -338,7 +177,7 @@ describe('NodeFavoriteDirective', () => {
         });
 
         it('return true when all are favorite', () => {
-            directive.favorites = [
+            directive.selection = [
                 { entry: { id: '1', name: 'name1', isFavorite: true } },
                 { entry: { id: '2', name: 'name2', isFavorite: true } }
             ];

lib/core/directives/node-favorite.directive.ts

@@ -17,7 +17,7 @@
 
 /* tslint:disable:no-input-rename  */
 
-import { Directive, EventEmitter, HostListener, Input, OnChanges, Output } from '@angular/core';
+import { Directive, EventEmitter, HostListener, Input, Output } from '@angular/core';
 import { FavoriteBody, MinimalNodeEntity } from 'alfresco-js-api';
 import { Observable } from 'rxjs/Observable';
 import { AlfrescoApiService } from '../services/alfresco-api.service';
@@ -28,9 +28,7 @@ import 'rxjs/observable/forkJoin';
     selector: '[adf-node-favorite]',
     exportAs: 'adfFavorite'
 })
-export class NodeFavoriteDirective implements OnChanges {
+export class NodeFavoriteDirective {
-    favorites: any[] = [];
-
     /** Array of nodes to toggle as favorites. */
     @Input('adf-node-favorite')
     selection: MinimalNodeEntity[] = [];
@@ -38,6 +36,9 @@ export class NodeFavoriteDirective implements OnChanges {
     /** Emitted when the favorite setting is complete. */
     @Output() toggle: EventEmitter<any> = new EventEmitter();
 
+    /** Emitted when the favorite setting has fail. */
+    @Output() error: EventEmitter<any> = new EventEmitter();
+
     @HostListener('click')
     onClick() {
         this.toggleFavorite();
@@ -46,103 +47,46 @@ export class NodeFavoriteDirective implements OnChanges {
     constructor(private alfrescoApiService: AlfrescoApiService) {
     }
 
-    ngOnChanges(changes) {
-        if (!changes.selection.currentValue.length) {
-            this.favorites = [];
-
-            return;
-        }
-
-        this.markFavoritesNodes(changes.selection.currentValue);
-    }
-
     toggleFavorite() {
-        if (!this.favorites.length) {
+        if (!this.selection.length) {
             return;
         }
 
-        const every = this.favorites.every((selected) => selected.entry.isFavorite);
+        const every = this.selection.every((selected) => selected.entry.isFavorite);
 
         if (every) {
-            const batch = this.favorites.map((selected) => {
+            const batch = this.selection.map((selected) => {
                 // shared files have nodeId
                 const id = selected.entry.nodeId || selected.entry.id;
 
                 return Observable.fromPromise(this.alfrescoApiService.favoritesApi.removeFavoriteSite('-me-', id));
             });
 
-            Observable.forkJoin(batch).subscribe(() => {
+            Observable.forkJoin(batch).subscribe(
-                this.favorites.map(selected => selected.entry.isFavorite = false);
+                () => this.toggle.emit(),
-                this.toggle.emit();
+                error => this.error.emit(error)
-            });
+            );
         }
 
         if (!every) {
-            const notFavorite = this.favorites.filter((node) => !node.entry.isFavorite);
+            const notFavorite = this.selection.filter((node) => !node.entry.isFavorite);
             const body: FavoriteBody[] = notFavorite.map((node) => this.createFavoriteBody(node));
 
-            Observable.fromPromise(this.alfrescoApiService.favoritesApi.addFavorite('-me-', <any> body))
+            Observable
-                .subscribe(() => {
+                .fromPromise(this.alfrescoApiService.favoritesApi.addFavorite('-me-', <any> body))
-                    notFavorite.map(selected => selected.entry.isFavorite = true);
+                .subscribe(
-                    this.toggle.emit();
+                    () => this.toggle.emit(),
-                });
+                    error => this.error.emit(error)
+                );
         }
     }
 
-    markFavoritesNodes(selection: MinimalNodeEntity[]) {
-        if (selection.length <= this.favorites.length) {
-            const newFavorites = this.reduce(this.favorites, selection);
-            this.favorites = newFavorites;
-        }
-
-        const result = this.diff(selection, this.favorites);
-        const batch = this.getProcessBatch(result);
-
-        Observable.forkJoin(batch).subscribe((data) => {
-            this.favorites.push(...data);
-        });
-    }
-
     hasFavorites(): boolean {
-        if (this.favorites && !this.favorites.length) {
+        if (!this.selection.length) {
             return false;
         }
 
-        return this.favorites.every((selected) => selected.entry.isFavorite);
+        return this.selection.every((selected) => selected.entry.isFavorite);
-    }
-
-    private getProcessBatch(selection): any[] {
-        return selection.map((selected: MinimalNodeEntity) => this.getFavorite(selected));
-    }
-
-    private getFavorite(selected: MinimalNodeEntity): Observable<any> {
-        const { name, isFile, isFolder } = selected.entry;
-        // shared files have nodeId
-        const id = (<any> selected).entry.nodeId || selected.entry.id;
-
-        const promise = this.alfrescoApiService.favoritesApi.getFavorite('-me-', id);
-
-        return Observable.from(promise)
-            .map(() => ({
-                entry: {
-                    id,
-                    isFolder,
-                    isFile,
-                    name,
-                    isFavorite: true
-                }
-            }))
-            .catch(() => {
-                return Observable.of({
-                    entry: {
-                        id,
-                        isFolder,
-                        isFile,
-                        name,
-                        isFavorite: false
-                    }
-                });
-            });
     }
 
     private createFavoriteBody(node): FavoriteBody {
@@ -167,16 +111,4 @@ export class NodeFavoriteDirective implements OnChanges {
 
         return node.entry.isFile ? 'file' : 'folder';
     }
-
-    private diff(list, patch): any[] {
-        const ids = patch.map(item => item.entry.id);
-
-        return list.filter(item => ids.includes(item.entry.id) ? null : item);
-    }
-
-    private reduce(patch, comparator): any[] {
-        const ids = comparator.map(item => item.entry.id);
-
-        return patch.filter(item => ids.includes(item.entry.id) ? item : null);
-    }
 }