From 7eab89c5ef09652ad7b2908c6f28ada7317221c9 Mon Sep 17 00:00:00 2001
From: Denys Vuika <denis.vuyka@gmail.com>
Date: Wed, 30 Nov 2016 14:52:06 +0000
Subject: [PATCH] #159 improved error reporting for document-list (#1180)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- expose ‘error’ event
- change path only on successful navigation
- extend demo shell with error handling and reporting
- additional unit tests for document-list
---
 .../app/components/files/files.component.css  |   8 ++
 .../app/components/files/files.component.html |  10 ++
 .../app/components/files/files.component.ts   |  11 ++
 .../breadcrumb/breadcrumb.component.spec.ts   |   9 +-
 .../src/components/document-list.spec.ts      | 119 +++++++++++++-----
 .../src/components/document-list.ts           |  26 ++--
 .../src/data/share-datatable-adapter.spec.ts  |  10 +-
 .../src/data/share-datatable-adapter.ts       |  38 +++---
 8 files changed, 171 insertions(+), 60 deletions(-)

diff --git a/demo-shell-ng2/app/components/files/files.component.css b/demo-shell-ng2/app/components/files/files.component.css
index c2bac0048e..cb4cb685aa 100644
--- a/demo-shell-ng2/app/components/files/files.component.css
+++ b/demo-shell-ng2/app/components/files/files.component.css
@@ -7,3 +7,11 @@
         margin: 0;
     }
 }
+
+.error-message {
+    text-align: left;
+}
+
+.error-message--text {
+    color: #d50000;
+}
diff --git a/demo-shell-ng2/app/components/files/files.component.html b/demo-shell-ng2/app/components/files/files.component.html
index 0c8d3ff4aa..bbfc19e4e0 100644
--- a/demo-shell-ng2/app/components/files/files.component.html
+++ b/demo-shell-ng2/app/components/files/files.component.html
@@ -7,11 +7,18 @@
             [currentFolderPath]="currentPath"
             [target]="documentList">
         </alfresco-document-list-breadcrumb>
+        <div *ngIf="errorMessage" class="error-message">
+            <button (click)="resetError()" class="mdl-button mdl-js-button mdl-button--icon">
+                <i class="material-icons">highlight_off</i>
+            </button>
+            <span class="error-message--text">{{errorMessage}}</span>
+        </div>
         <alfresco-document-list
                 #documentList
                 [currentFolderPath]="currentPath"
                 [contextMenuActions]="true"
                 [contentActions]="true"
+                (error)="onNavigationError($event)"
                 (preview)="showFile($event)"
                 (folderChange)="onFolderChanged($event)">
             <!--
@@ -117,6 +124,9 @@
         <li>
             <button (click)="documentList.currentFolderPath = '/'">Go to root</button>
         </li>
+        <li>
+            <button (click)="documentList.currentFolderPath = '!@£$%^&*()'">Go to the wrong path</button>
+        </li>
         <li>
             <button (click)="fileDialog.toggleShowDialog()">Show/Hide File Dialog</button>
         </li>
diff --git a/demo-shell-ng2/app/components/files/files.component.ts b/demo-shell-ng2/app/components/files/files.component.ts
index 817d7279b0..d8ccb74114 100644
--- a/demo-shell-ng2/app/components/files/files.component.ts
+++ b/demo-shell-ng2/app/components/files/files.component.ts
@@ -38,6 +38,7 @@ declare let __moduleName: string;
 export class FilesComponent implements OnInit {
     currentPath: string = '/Sites/swsdp/documentLibrary';
 
+    errorMessage: string = null;
     fileNodeId: any;
     fileShowed: boolean = false;
     multipleFileUpload: boolean = false;
@@ -120,6 +121,16 @@ export class FilesComponent implements OnInit {
         this.router.navigate(['/activiti/tasksnode', event.value.entry.id]);
     }
 
+    onNavigationError(err: any) {
+        if (err) {
+            this.errorMessage = err.message || 'Navigation error';
+        }
+    }
+
+    resetError() {
+        this.errorMessage = null;
+    }
+
     private setupBpmActions(actions: any[]) {
         actions.map(def => {
             let documentAction = new DocumentActionModel();
diff --git a/ng2-components/ng2-alfresco-documentlist/src/components/breadcrumb/breadcrumb.component.spec.ts b/ng2-components/ng2-alfresco-documentlist/src/components/breadcrumb/breadcrumb.component.spec.ts
index fb4e1f6735..19f7a77ac5 100644
--- a/ng2-components/ng2-alfresco-documentlist/src/components/breadcrumb/breadcrumb.component.spec.ts
+++ b/ng2-components/ng2-alfresco-documentlist/src/components/breadcrumb/breadcrumb.component.spec.ts
@@ -85,15 +85,18 @@ describe('DocumentListBreadcrumb', () => {
         component.onRoutePathClick(node, null);
     });
 
-    it('should update document list on click', () => {
+    it('should update document list on click', (done) => {
         let documentList = new DocumentList(null, null, null);
-        spyOn(documentList, 'displayFolderContent').and.stub();
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.resolve());
 
         let node = <PathNode> { name: 'name', path: '/path' };
         component.target = documentList;
 
         component.onRoutePathClick(node, null);
-        expect(documentList.currentFolderPath).toBe(node.path);
+        setTimeout(() => {
+            expect(documentList.currentFolderPath).toBe(node.path);
+            done();
+        }, 0);
     });
 
     it('should do nothing for same path', () => {
diff --git a/ng2-components/ng2-alfresco-documentlist/src/components/document-list.spec.ts b/ng2-components/ng2-alfresco-documentlist/src/components/document-list.spec.ts
index 81f042cf2c..186f678dd8 100644
--- a/ng2-components/ng2-alfresco-documentlist/src/components/document-list.spec.ts
+++ b/ng2-components/ng2-alfresco-documentlist/src/components/document-list.spec.ts
@@ -15,13 +15,14 @@
  * limitations under the License.
  */
 
-import { NgZone } from '@angular/core';
-import { DataColumn } from 'ng2-alfresco-datatable';
+import { NgZone, TemplateRef } from '@angular/core';
+import { DataTableComponent, DataColumn, DataRowEvent } from 'ng2-alfresco-datatable';
 import { DocumentList } from './document-list';
 import { DocumentListServiceMock } from './../assets/document-list.service.mock';
 import { ContentActionModel } from '../models/content-action.model';
 import { FileNode, FolderNode } from '../assets/document-library.model.mock';
 import { NodeMinimalEntry } from '../models/document-library.model';
+import { ShareDataRow, RowFilter, ImageResolver } from './../data/share-datatable-adapter';
 
 describe('DocumentList', () => {
 
@@ -204,23 +205,14 @@ describe('DocumentList', () => {
     });
 
     it('should display folder content for new folder path', () => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.resolve());
         let newPath = '/some/new/path';
         documentList.currentFolderPath = newPath;
         expect(documentList.displayFolderContent).toHaveBeenCalledWith(newPath);
     });
 
-    it('should not display folder content for same path', () => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
-        documentList.currentFolderPath = '/test';
-        expect(documentList.displayFolderContent).toHaveBeenCalledTimes(1);
-
-        documentList.currentFolderPath = '/test';
-        expect(documentList.displayFolderContent).toHaveBeenCalledTimes(1);
-    });
-
     it('should reset to default path', () => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.resolve());
         documentList.currentFolderPath = null;
 
         expect(documentList.currentFolderPath).toBe(documentList.DEFAULT_ROOT_FOLDER);
@@ -228,7 +220,7 @@ describe('DocumentList', () => {
     });
 
     it('should emit folder changed event', (done) => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.resolve());
         documentList.folderChange.subscribe(e => {
             done();
         });
@@ -237,7 +229,7 @@ describe('DocumentList', () => {
     });
 
     it('should emit folder changed event with folder details', (done) => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.resolve());
 
         let path = '/path';
 
@@ -249,20 +241,6 @@ describe('DocumentList', () => {
         documentList.currentFolderPath = path;
     });
 
-    it('should emit folder changed event only once', () => {
-        spyOn(documentList, 'displayFolderContent').and.stub();
-        let path = '/new/path';
-        let calls = 0;
-        documentList.folderChange.subscribe(e => {
-            calls++;
-        });
-
-        documentList.currentFolderPath = path;
-        documentList.currentFolderPath = path;
-        documentList.currentFolderPath = path;
-        expect(calls).toBe(1);
-    });
-
     it('should execute context action on callback', () => {
         let action = {
             node: {},
@@ -500,4 +478,87 @@ describe('DocumentList', () => {
         expect(documentList.isMobile).toHaveBeenCalled();
         expect(documentList.navigationMode).toBe(DocumentList.SINGLE_CLICK_NAVIGATION);
     });
+
+    it('should emit error on wrong path', (done) => {
+        let raised = false;
+        documentList.error.subscribe(err => raised = true);
+        spyOn(documentList, 'displayFolderContent').and.returnValue(Promise.reject(false));
+
+        documentList.currentFolderPath = 'wrong-path';
+        setTimeout(() => {
+            expect(raised).toBeTruthy();
+            done();
+        }, 0);
+    });
+
+    it('should require dataTable to check empty template', () => {
+        documentList.dataTable = null;
+        expect(documentList.isEmptyTemplateDefined()).toBeFalsy();
+    });
+
+    it('should check [empty folder] template ', () => {
+        documentList.emptyFolderTemplate = <TemplateRef<any>> {};
+        documentList.dataTable = new DataTableComponent();
+        expect(documentList.dataTable).toBeDefined();
+        expect(documentList.isEmptyTemplateDefined()).toBeTruthy();
+
+        documentList.emptyFolderTemplate = null;
+        expect(documentList.isEmptyTemplateDefined()).toBeFalsy();
+    });
+
+    it('should set root path for underlying adapter', () => {
+        documentList.rootPath = 'test';
+        expect(documentList.data.rootPath).toBe('test');
+    });
+
+    it('should set default root path for underlying adapter', () => {
+        documentList.rootPath = null;
+        expect(documentList.data.rootPath).toBe(documentList.data.DEFAULT_ROOT_PATH);
+    });
+
+    it('should fetch root path from underlying adapter', () => {
+        documentList.data.rootPath = 'test';
+        expect(documentList.rootPath).toBe('test');
+    });
+
+    it('should not fetch root path when adapter missing', () => {
+        documentList.data = null;
+        expect(documentList.rootPath).toBeNull();
+    });
+
+    it('should set row filter for underlying adapter', () => {
+        let filter = <RowFilter> {};
+        spyOn(documentList.data, 'setFilter').and.callThrough();
+
+        documentList.rowFilter = filter;
+        expect(documentList.data.setFilter).toHaveBeenCalledWith(filter);
+    });
+
+    it('should set image resolver for underlying adapter', () => {
+        let resolver = <ImageResolver> {};
+        spyOn(documentList.data, 'setImageResolver').and.callThrough();
+
+        documentList.imageResolver = resolver;
+        expect(documentList.data.setImageResolver).toHaveBeenCalledWith(resolver);
+    });
+
+    it('should emit [nodeClick] event on row click', () => {
+        let node = new NodeMinimalEntry();
+        let row = new ShareDataRow(node);
+        let event = <DataRowEvent> { value: row };
+
+        spyOn(documentList, 'onNodeClick').and.callThrough();
+        documentList.onRowClick(event);
+        expect(documentList.onNodeClick).toHaveBeenCalledWith(node);
+    });
+
+    it('should emit [nodeDblClick] event on row double-click', () => {
+        let node = new NodeMinimalEntry();
+        let row = new ShareDataRow(node);
+        let event = <DataRowEvent> { value: row };
+
+        spyOn(documentList, 'onNodeDblClick').and.callThrough();
+        documentList.onRowDblClick(event);
+        expect(documentList.onNodeDblClick).toHaveBeenCalledWith(node);
+    });
 });
diff --git a/ng2-components/ng2-alfresco-documentlist/src/components/document-list.ts b/ng2-components/ng2-alfresco-documentlist/src/components/document-list.ts
index 58b744497d..b6486bfedd 100644
--- a/ng2-components/ng2-alfresco-documentlist/src/components/document-list.ts
+++ b/ng2-components/ng2-alfresco-documentlist/src/components/document-list.ts
@@ -118,6 +118,9 @@ export class DocumentList implements OnInit, AfterContentInit {
     @Output()
     preview: EventEmitter<any> = new EventEmitter();
 
+    @Output()
+    error: EventEmitter<any> = new EventEmitter();
+
     @ViewChild(DataTableComponent)
     dataTable: DataTableComponent;
 
@@ -130,11 +133,15 @@ export class DocumentList implements OnInit, AfterContentInit {
     @Input()
     set currentFolderPath(value: string) {
         if (value !== this._path) {
-            this._path = value || this.DEFAULT_ROOT_FOLDER;
-            this.displayFolderContent(this._path);
-            this.folderChange.emit({
-                path: this.currentFolderPath
-            });
+            const path = value || this.DEFAULT_ROOT_FOLDER;
+            this.displayFolderContent(path)
+                .then(() => {
+                    this._path = path;
+                    this.folderChange.emit({ path: path });
+                })
+                .catch(err => {
+                    this.error.emit(err);
+                });
         }
     }
 
@@ -260,14 +267,17 @@ export class DocumentList implements OnInit, AfterContentInit {
         }
     }
 
-    displayFolderContent(path: string) {
-        this.data.loadPath(path);
+    displayFolderContent(path: string): Promise<any> {
+        return this.data.loadPath(path);
     }
 
     reload() {
         this.ngZone.run(() => {
             if (this.currentFolderPath) {
-                this.displayFolderContent(this.currentFolderPath);
+                this.displayFolderContent(this.currentFolderPath)
+                    .catch(err => {
+                        this.error.emit(err);
+                    });
             }
         });
     }
diff --git a/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.spec.ts b/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.spec.ts
index 96248783b9..22ac0e7170 100644
--- a/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.spec.ts
+++ b/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.spec.ts
@@ -322,7 +322,7 @@ describe('ShareDataTableAdapter', () => {
         expect(value).toBeNull();
     });
 
-    it('should log load error', () => {
+    it('should log load error', (done) => {
         let error = 'My Error';
         documentListService.getFolderReject = true;
         documentListService.getFolderRejectError = error;
@@ -331,10 +331,10 @@ describe('ShareDataTableAdapter', () => {
         spyOn(documentListService, 'getFolder').and.callThrough();
 
         let adapter = new ShareDataTableAdapter(documentListService, null, null);
-        adapter.loadPath('/some/path');
-
-        expect(documentListService.getFolder).toHaveBeenCalled();
-        expect(console.error).toHaveBeenCalledWith(error);
+        adapter.loadPath('/some/path').catch(err => {
+            expect(err).toBe(error);
+            done();
+        });
     });
 
     it('should generate file icon path based on mime type', () => {
diff --git a/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.ts b/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.ts
index 4937ffcf60..c5831c92d1 100644
--- a/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.ts
+++ b/ng2-components/ng2-alfresco-documentlist/src/data/share-datatable-adapter.ts
@@ -203,21 +203,29 @@ export class ShareDataTableAdapter implements DataTableAdapter, PaginationProvid
         this.setSorting(sorting);
     }
 
-    loadPath(path: string) {
-        if (path && this.documentListService) {
-            this.currentPath = path;
-            this.documentListService
-                .getFolder(path, {
-                    maxItems: this._maxItems,
-                    skipCount: this._skipCount,
-                    rootPath: this.rootPath
-                })
-                .subscribe(val => {
-                    this.loadPage(<NodePaging>val);
-                    this.dataLoaded.emit(null);
-                },
-                error => console.error(error));
-        }
+    loadPath(path: string): Promise<any> {
+        return new Promise((resolve, reject) => {
+            if (path && this.documentListService) {
+                this.documentListService
+                    .getFolder(path, {
+                        maxItems: this._maxItems,
+                        skipCount: this._skipCount,
+                        rootPath: this.rootPath
+                    })
+                    .subscribe(val => {
+                        this.currentPath = path;
+                        this.loadPage(<NodePaging>val);
+                        this.dataLoaded.emit(null);
+                        resolve(true);
+                    },
+                    error => {
+                        reject(error);
+                    });
+            } else {
+                resolve(false);
+            }
+        });
+
     }
 
     setFilter(filter: RowFilter) {