From a5bf0d27e40721f937e4244fdf96022ffc85ee16 Mon Sep 17 00:00:00 2001
From: Vito <vito.albano@alfresco.com>
Date: Wed, 21 Nov 2018 16:50:11 +0000
Subject: [PATCH] [ADF-3763] fixed subscription for node child and invalid node
 id (#3991)

* [ADF-3763] fixed subscription for node child and invalid node id

* [ADF-3763] removed fdescribe

* [ADF-3763] added line for updating documentation
---
 .../tree-view/tree-view-sample.component.html | 13 ++++-
 .../tree-view/tree-view-sample.component.ts   | 11 +++++
 docs/content-services/tree-view.component.md  |  1 +
 .../components/tree-view.component.html       |  9 ++--
 .../components/tree-view.component.spec.ts    | 47 ++++++++++++++++++-
 .../components/tree-view.component.ts         | 22 +++++----
 .../tree-view/data/tree-view-datasource.ts    | 38 ++++++++++-----
 7 files changed, 112 insertions(+), 29 deletions(-)

diff --git a/demo-shell/src/app/components/tree-view/tree-view-sample.component.html b/demo-shell/src/app/components/tree-view/tree-view-sample.component.html
index 9dfb0850e5..ca75436205 100644
--- a/demo-shell/src/app/components/tree-view/tree-view-sample.component.html
+++ b/demo-shell/src/app/components/tree-view/tree-view-sample.component.html
@@ -1,10 +1,19 @@
 <div>TREE VIEW TEST</div>
 <mat-form-field class="example-full-width">
-    <input matInput placeholder="Node Id" [(ngModel)]="nodeIdSample">
+    <input matInput placeholder="Node Id"
+                [(ngModel)]="nodeIdSample"
+                (ngModelChange)="reset()">
 </mat-form-field>
 <span>
     CLICKED NODE: {{clickedNodeName}}
 </span>
 
-<adf-tree-view-list [nodeId]="nodeIdSample" (nodeClicked)="onClick($event)">
+<adf-tree-view-list *ngIf="!errorMessage; else erroOccurred"
+        [nodeId]="nodeIdSample"
+        (nodeClicked)="onClick($event)"
+        (error)="onErrorOccurred($event)">
 </adf-tree-view-list>
+<ng-template #erroOccurred>
+    <span>An Error Occurred </span>
+    <span>{{errorMessage}}</span>
+</ng-template>
diff --git a/demo-shell/src/app/components/tree-view/tree-view-sample.component.ts b/demo-shell/src/app/components/tree-view/tree-view-sample.component.ts
index fea4caae6e..512de3b858 100644
--- a/demo-shell/src/app/components/tree-view/tree-view-sample.component.ts
+++ b/demo-shell/src/app/components/tree-view/tree-view-sample.component.ts
@@ -25,10 +25,21 @@ import { Component } from '@angular/core';
 export class TreeViewSampleComponent {
 
     clickedNodeName: string = '';
+    errorMessage = '';
 
     nodeIdSample: string = '-my-';
 
     onClick(node) {
         this.clickedNodeName = node.entry.name;
     }
+
+    reset() {
+        this.clickedNodeName = '';
+        this.errorMessage = '';
+    }
+
+    onErrorOccurred(error) {
+        this.clickedNodeName = '';
+        this.errorMessage = error;
+    }
 }
diff --git a/docs/content-services/tree-view.component.md b/docs/content-services/tree-view.component.md
index b94509b211..d81c0108c2 100644
--- a/docs/content-services/tree-view.component.md
+++ b/docs/content-services/tree-view.component.md
@@ -30,4 +30,5 @@ Shows the folder and subfolders of a node as a tree view.
 
 | Name | Type | Description |
 | ---- | ---- | ----------- |
+| error | [`EventEmitter`](https://angular.io/api/core/EventEmitter)`<any>` | Emitted when an invalid node id is given. |
 | nodeClicked | [`EventEmitter`](https://angular.io/api/core/EventEmitter)`<`[`NodeEntry`](https://github.com/Alfresco/alfresco-js-api/blob/master/src/alfresco-core-rest-api/docs/NodeEntry.md)`>` | Emitted when a node in the tree view is clicked. |
diff --git a/lib/content-services/tree-view/components/tree-view.component.html b/lib/content-services/tree-view/components/tree-view.component.html
index 92b1504f8b..78aaeafa44 100644
--- a/lib/content-services/tree-view/components/tree-view.component.html
+++ b/lib/content-services/tree-view/components/tree-view.component.html
@@ -1,9 +1,12 @@
-<mat-tree class="adf-tree-view-main" [dataSource]="dataSource" [treeControl]="treeControl" *ngIf="nodeId; else missingNodeId">
-    <mat-tree-node class="adf-tree-view-node" *matTreeNodeDef="let treeNode" id="{{treeNode.name + '-tree-node'}}"
+<mat-tree class="adf-tree-view-main" [dataSource]="dataSource"
+          [treeControl]="treeControl" *ngIf="nodeId; else missingNodeId">
+    <mat-tree-node class="adf-tree-view-node"
+        *matTreeNodeDef="let treeNode" id="{{treeNode.name + '-tree-node'}}"
         matTreeNodePadding [matTreeNodePaddingIndent]="15">
         {{treeNode.name}}
     </mat-tree-node>
-    <mat-tree-node class="adf-tree-view-node" id="{{treeNode.name + '-tree-child-node'}}" *matTreeNodeDef="let treeNode; when: hasChild"
+    <mat-tree-node class="adf-tree-view-node"
+        id="{{treeNode.name + '-tree-child-node'}}" *matTreeNodeDef="let treeNode; when: hasChild"
         matTreeNodePadding [matTreeNodePaddingIndent]="15">
         <button id="{{'button-'+treeNode.name}}" (click)="onNodeClicked(treeNode.node)"
                 mat-icon-button [attr.aria-label]="'toggle ' + treeNode.filename" matTreeNodeToggle>
diff --git a/lib/content-services/tree-view/components/tree-view.component.spec.ts b/lib/content-services/tree-view/components/tree-view.component.spec.ts
index 45b949f62d..43760d2a64 100644
--- a/lib/content-services/tree-view/components/tree-view.component.spec.ts
+++ b/lib/content-services/tree-view/components/tree-view.component.spec.ts
@@ -20,9 +20,10 @@ import { setupTestBed } from '@alfresco/adf-core';
 import { TreeViewComponent } from './tree-view.component';
 import { ContentTestingModule } from '../../testing/content.testing.module';
 import { TreeViewService } from '../services/tree-view.service';
-import { of } from 'rxjs';
+import { of, throwError } from 'rxjs';
 import { TreeBaseNode } from '../models/tree-view.model';
 import { NodeEntry } from 'alfresco-js-api';
+import { SimpleChange } from '@angular/core';
 
 describe('TreeViewComponent', () => {
 
@@ -73,6 +74,8 @@ describe('TreeViewComponent', () => {
             component = fixture.componentInstance;
             spyOn(treeService, 'getTreeNodes').and.callFake((nodeId) => returnRootOrChildrenNode(nodeId));
             component.nodeId = '9999999';
+            const changeNodeId = new SimpleChange(null, '9999999', true);
+            component.ngOnChanges({ 'nodeId': changeNodeId });
             fixture.detectChanges();
         }));
 
@@ -95,6 +98,24 @@ describe('TreeViewComponent', () => {
             });
         }));
 
+        it('should show only the correct subfolders when the nodeId is changed', async(() => {
+            component.nodeId = 'fake-second-id';
+            const changeNodeId = new SimpleChange('9999999', 'fake-second-id', true);
+            component.ngOnChanges({ 'nodeId': changeNodeId });
+            fixture.detectChanges();
+            fixture.whenStable().then(() => {
+                let rootFolderButton: HTMLButtonElement = <HTMLButtonElement> element.querySelector('#button-fake-next-child-name');
+                expect(rootFolderButton).not.toBeNull();
+                rootFolderButton.click();
+                fixture.detectChanges();
+                fixture.whenStable().then(() => {
+                    expect(element.querySelector('#fake-child-name-tree-child-node')).not.toBeNull();
+                    expect(element.querySelector('#fake-second-name-tree-child-node')).not.toBeNull();
+                    expect(element.querySelectorAll('mat-tree-node').length).toBe(4);
+                });
+            });
+        }));
+
         it('should throw a nodeClicked event when a node is clicked', (done) => {
             component.nodeClicked.subscribe((nodeClicked: NodeEntry) => {
                 expect(nodeClicked).toBeDefined();
@@ -178,4 +199,28 @@ describe('TreeViewComponent', () => {
             });
         }));
     });
+
+    describe('When invalid nodeId is given', () => {
+
+        beforeEach(async(() => {
+            fixture = TestBed.createComponent(TreeViewComponent);
+            treeService = TestBed.get(TreeViewService);
+            spyOn(treeService, 'getTreeNodes').and.callFake((nodeId) => throwError('Invalid Node Id'));
+            fixture.componentInstance.nodeId = 'Poopoovic';
+        }));
+
+        afterEach(() => {
+            fixture.destroy();
+        });
+
+        it('should raise an error event when invalid nodeId is provided', (done) => {
+            fixture.componentInstance.error.subscribe((error) => {
+                expect(error).toBe('Invalid Node Id');
+                done();
+            });
+            const changeNodeId = new SimpleChange(null, 'Poopoovic', true);
+            fixture.componentInstance.ngOnChanges({ 'nodeId': changeNodeId });
+            fixture.detectChanges();
+        });
+    });
 });
diff --git a/lib/content-services/tree-view/components/tree-view.component.ts b/lib/content-services/tree-view/components/tree-view.component.ts
index 3c9bd1eeaa..7f64232db1 100644
--- a/lib/content-services/tree-view/components/tree-view.component.ts
+++ b/lib/content-services/tree-view/components/tree-view.component.ts
@@ -16,7 +16,7 @@
  */
 
 import { FlatTreeControl } from '@angular/cdk/tree';
-import { Component, Input, OnInit, OnChanges, SimpleChanges, Output, EventEmitter } from '@angular/core';
+import { Component, Input, OnChanges, SimpleChanges, Output, EventEmitter } from '@angular/core';
 import { TreeBaseNode } from '../models/tree-view.model';
 import { TreeViewDataSource } from '../data/tree-view-datasource';
 import { TreeViewService } from '../services/tree-view.service';
@@ -28,7 +28,7 @@ import { NodeEntry } from 'alfresco-js-api';
     styleUrls: ['tree-view.component.scss']
 })
 
-export class TreeViewComponent implements OnInit, OnChanges {
+export class TreeViewComponent implements OnChanges {
 
     /** Identifier of the node to display. */
     @Input()
@@ -38,6 +38,10 @@ export class TreeViewComponent implements OnInit, OnChanges {
     @Output()
     nodeClicked: EventEmitter<NodeEntry> = new EventEmitter();
 
+    /** Emitted when an invalid node id is given. */
+    @Output()
+    error: EventEmitter<any> = new EventEmitter();
+
     treeControl: FlatTreeControl<TreeBaseNode>;
     dataSource: TreeViewDataSource;
 
@@ -46,16 +50,12 @@ export class TreeViewComponent implements OnInit, OnChanges {
         this.dataSource = new TreeViewDataSource(this.treeControl, this.treeViewService);
     }
 
-    ngOnInit() {
-        if (this.nodeId) {
-            this.loadTreeNode();
-        }
-    }
-
     ngOnChanges(changes: SimpleChanges) {
-        if (changes['nodeId'].currentValue &&
+        if (changes['nodeId'] && changes['nodeId'].currentValue &&
             changes['nodeId'].currentValue !== changes['nodeId'].previousValue) {
             this.loadTreeNode();
+        } else {
+            this.dataSource.data = [];
         }
     }
 
@@ -74,6 +74,8 @@ export class TreeViewComponent implements OnInit, OnChanges {
             .subscribe(
                 (treeNode: TreeBaseNode[]) => {
                     this.dataSource.data = treeNode;
-                });
+                },
+                (error) => this.error.emit(error)
+            );
     }
 }
diff --git a/lib/content-services/tree-view/data/tree-view-datasource.ts b/lib/content-services/tree-view/data/tree-view-datasource.ts
index 3eac69d7a9..832943de23 100644
--- a/lib/content-services/tree-view/data/tree-view-datasource.ts
+++ b/lib/content-services/tree-view/data/tree-view-datasource.ts
@@ -28,6 +28,8 @@ export class TreeViewDataSource {
 
     treeNodes: TreeBaseNode[];
     dataChange = new BehaviorSubject<TreeBaseNode[]>([]);
+    childrenSubscription = null;
+    changeSubscription = null;
 
     get data(): TreeBaseNode[] {
         return this.treeNodes;
@@ -44,7 +46,7 @@ export class TreeViewDataSource {
     }
 
     connect(collectionViewer: CollectionViewer): Observable<TreeBaseNode[]> {
-        this.treeControl.expansionModel.onChange!.subscribe(change => {
+        this.changeSubscription = this.treeControl.expansionModel.onChange.subscribe(change => {
             if ((change as SelectionChange<TreeBaseNode>).added &&
                 (change as SelectionChange<TreeBaseNode>).added.length > 0) {
                 this.expandTreeNodes(change as SelectionChange<TreeBaseNode>);
@@ -55,6 +57,15 @@ export class TreeViewDataSource {
         return merge(collectionViewer.viewChange, this.dataChange).pipe(map(() => this.data));
     }
 
+    disconnect() {
+        if (this.childrenSubscription) {
+            this.childrenSubscription.unsubscribe();
+        }
+        if (this.changeSubscription) {
+            this.changeSubscription.unsubscribe();
+        }
+    }
+
     private expandTreeNodes(change: SelectionChange<TreeBaseNode>) {
         change.added.forEach(node => this.expandNode(node));
     }
@@ -64,19 +75,20 @@ export class TreeViewDataSource {
     }
 
     private expandNode(node: TreeBaseNode) {
-        this.treeViewService.getTreeNodes(node.nodeId).subscribe((children) => {
-            const index = this.data.indexOf(node);
-            if (!children || index < 0) {
-                node.expandable = false;
-                return;
-            }
-            const nodes = children.map(actualNode => {
-                actualNode.level = node.level + 1;
-                return actualNode;
+        this.childrenSubscription = this.treeViewService.getTreeNodes(node.nodeId)
+            .subscribe((children) => {
+                const index = this.data.indexOf(node);
+                if (!children || index < 0) {
+                    node.expandable = false;
+                    return;
+                }
+                const nodes = children.map(actualNode => {
+                    actualNode.level = node.level + 1;
+                    return actualNode;
+                });
+                this.data.splice(index + 1, 0, ...nodes);
+                this.dataChange.next(this.data);
             });
-            this.data.splice(index + 1, 0, ...nodes);
-            this.dataChange.next(this.data);
-        });
     }
 
     toggleNode(node: TreeBaseNode) {