Fix bug related to upload button permission (#1838)

* Add the permission check on the activiti button

* Fix code after unit test

* Add basic documentation

* Improve the code and fix the missing file issue

* Fix unit test after refactoring

* remove duplicate log

ng2-components/ng2-alfresco-upload/src/components/upload-button.component.html

@@ -1,6 +1,6 @@
 <form>
     <!--Files Upload-->
-    <div *ngIf="!uploadFolders" [class.mdl-button--file--disabled]="isDisabled()" class="mdl-button mdl-js-button mdl-button--raised mdl-button--colored mdl-button--file">
+    <div *ngIf="!uploadFolders" [class.mdl-button--file--disabled]="isButtonDisabled()" class="mdl-button mdl-js-button mdl-button--raised mdl-button--colored mdl-button--file">
         <md-icon>file_upload</md-icon>
 
         <!--Multiple Files Upload-->
@@ -11,7 +11,7 @@
                    (change)="onFilesAdded($event)"
                    multiple="multiple"
                    accept="{{acceptedFilesType}}"
-                   [attr.disabled]="isDisabled()"
+                   [attr.disabled]="isButtonDisabled()"
                    #uploadFiles>
         </span>
 
@@ -22,13 +22,13 @@
             <input id="upload-single-file" data-automation-id="upload-single-file" type="file" name="uploadFiles"
                    (change)="onFilesAdded($event)"
                    accept="{{acceptedFilesType}}"
-                   [attr.disabled]="isDisabled()"
+                   [attr.disabled]="isButtonDisabled()"
                    #uploadFiles>
         </span>
     </div>
 
     <!--Folders Upload-->
-    <div *ngIf="uploadFolders" [class.mdl-button--file--disabled]="isDisabled()" class="mdl-button mdl-js-button mdl-button--raised mdl-button--colored mdl-button--file">
+    <div *ngIf="uploadFolders" [class.mdl-button--file--disabled]="isButtonDisabled()" class="mdl-button mdl-js-button mdl-button--raised mdl-button--colored mdl-button--file">
         <md-icon>file_upload</md-icon>
         <label id="uploadFolder-label" *ngIf="!staticTitle" for="uploadFolder">{{'FILE_UPLOAD.BUTTON.UPLOAD_FOLDER' | translate}}</label>
         <label id="uploadFolder-label-static" *ngIf="staticTitle" for="uploadFolder">{{staticTitle}}</label>
@@ -36,7 +36,7 @@
                (change)="onDirectoryAdded($event)"
                multiple="multiple"
                accept="{{acceptedFilesType}}"
-               [attr.disabled]="isDisabled()"
+               [attr.disabled]="isButtonDisabled()"
                webkitdirectory directory
                multiple #uploadFolders>
     </div>

ng2-components/ng2-alfresco-upload/src/components/upload-button.component.spec.ts

@@ -17,7 +17,7 @@
 
 import { ComponentFixture, TestBed, async } from '@angular/core/testing';
 import { UploadButtonComponent } from './upload-button.component';
-import { DebugElement }    from '@angular/core';
+import { DebugElement, SimpleChange }    from '@angular/core';
 import { CoreModule, AlfrescoTranslationService, NotificationService } from 'ng2-alfresco-core';
 import { TranslationMock } from '../assets/translation.service.mock';
 import { UploadService } from '../services/upload.service';
@@ -175,6 +175,7 @@ describe('UploadButtonComponent', () => {
 
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
 
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
         component.onFilesAdded(fakeEvent);
         let compiled = fixture.debugElement.nativeElement;
         fixture.detectChanges();
@@ -188,6 +189,7 @@ describe('UploadButtonComponent', () => {
 
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
 
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
         component.onFilesAdded(fakeEvent);
         let compiled = fixture.debugElement.nativeElement;
         fixture.detectChanges();
@@ -196,11 +198,13 @@ describe('UploadButtonComponent', () => {
     });
 
     it('should call uploadFile with the default root folder', () => {
+        component.rootFolderId = '-root-';
         component.currentFolderPath = '/root-fake-/sites-fake/folder-fake';
         component.onSuccess = null;
 
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
 
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
         uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue');
 
         fixture.detectChanges();
@@ -215,6 +219,8 @@ describe('UploadButtonComponent', () => {
         component.onSuccess = null;
 
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
+
         uploadService.uploadFilesInTheQueue = jasmine.createSpy('uploadFilesInTheQueue');
 
         fixture.detectChanges();
@@ -224,12 +230,14 @@ describe('UploadButtonComponent', () => {
     });
 
     it('should create a folder and emit an File uploaded event', (done) => {
+        component.rootFolderId = '-my-';
         component.currentFolderPath = '/fake-root-path';
-        fixture.detectChanges();
 
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
         spyOn(uploadService, 'callApiCreateFolder').and.returnValue(fakeResolvePromise);
 
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
+        fixture.detectChanges();
         component.onSuccess.subscribe(e => {
             expect(e.value).toEqual('File uploaded');
             done();
@@ -245,8 +253,12 @@ describe('UploadButtonComponent', () => {
     });
 
     it('should emit an onError event when the folder already exist', (done) => {
+        component.rootFolderId = '-my-';
         spyOn(uploadService, 'getFolderNode').and.returnValue(Observable.of(fakeFolderNodeWithPermission));
         spyOn(uploadService, 'callApiCreateFolder').and.returnValue(fakeRejectPromise);
+
+        component.ngOnChanges({ rootFolderId: new SimpleChange(null, component.rootFolderId) });
+
         component.onError.subscribe(e => {
             expect(e.value).toEqual('FILE_UPLOAD.MESSAGES.FOLDER_ALREADY_EXIST');
             done();

ng2-components/ng2-alfresco-upload/src/components/upload-button.component.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { Component, ElementRef, Input, Output, EventEmitter, OnInit, OnChanges } from '@angular/core';
+import { Component, ElementRef, Input, Output, EventEmitter, OnInit, OnChanges, SimpleChanges } from '@angular/core';
 import { Subject } from 'rxjs/Rx';
 import { AlfrescoTranslationService, LogService, NotificationService } from 'ng2-alfresco-core';
 import { UploadService } from '../services/upload.service';
@@ -98,7 +98,8 @@ export class UploadButtonComponent implements OnInit, OnChanges {
     @Output()
     permissionEvent: EventEmitter<PermissionModel> = new EventEmitter<PermissionModel>();
 
-    private disableButton: boolean = false;
+    private hasPermission: boolean = false;
+
     private permissionValue: Subject<boolean> = new Subject<boolean>();
 
     constructor(private el: ElementRef,
@@ -111,26 +112,33 @@ export class UploadButtonComponent implements OnInit, OnChanges {
         }
     }
 
-    isDisabled(): boolean {
-        return this.disabled || this.disableButton;
-    }
-
     ngOnInit() {
-        this.permissionValue.subscribe((hasPermission: boolean) => {
+        this.permissionValue.subscribe((permission: boolean) => {
-            if (!hasPermission && this.disableWithNoPermission) {
+            this.hasPermission = permission;
-                this.disableButton = true;
-            } else {
-                this.disableButton = undefined;
-            }
         });
     }
 
-    ngOnChanges(changes) {
+    ngOnChanges(changes: SimpleChanges) {
+        let rootFolderId = changes['rootFolderId'];
+        if (rootFolderId && rootFolderId.currentValue) {
             this.checkPermission();
+        }
         let formFields = this.createFormFields();
         this.uploadService.setOptions(formFields, this.versioning);
     }
 
+    isButtonDisabled(): boolean {
+        return this.isForceDisable() || this.isDisableWithNoPermission();
+    }
+
+    isForceDisable(): boolean {
+        return this.disabled ? true : undefined;
+    }
+
+    isDisableWithNoPermission(): boolean {
+        return !this.hasPermission && this.disableWithNoPermission ? true : undefined;
+    }
+
     /**
      * Method called when files are dropped in the drag area.
      *
@@ -138,15 +146,12 @@ export class UploadButtonComponent implements OnInit, OnChanges {
      */
     onFilesAdded($event: any): void {
         let files = $event.currentTarget.files;
-        this.permissionValue.subscribe((hasPermission: boolean) => {
+
-            if (hasPermission) {
+        if (this.hasPermission) {
             this.uploadFiles(this.currentFolderPath, files);
         } else {
             this.permissionEvent.emit(new PermissionModel({type: 'content', action: 'upload', permission: 'create'}));
         }
-        });
-
-        this.checkPermission();
         // reset the value of the input file
         $event.target.value = '';
     }
@@ -158,8 +163,7 @@ export class UploadButtonComponent implements OnInit, OnChanges {
      */
     onDirectoryAdded($event: any): void {
         let files = $event.currentTarget.files;
-        this.permissionValue.subscribe((hasPermission: boolean) => {
+        if (this.hasPermission) {
-            if (hasPermission) {
             let hashMapDir = this.convertIntoHashMap(files);
 
             hashMapDir.forEach((filesDir, directoryPath) => {
@@ -181,16 +185,12 @@ export class UploadButtonComponent implements OnInit, OnChanges {
                                     this._showErrorNotificationBar(errorMessage);
                                 }
                             }
-                                this.logService.error(error);
                         }
                     );
             });
         } else {
             this.permissionEvent.emit(new PermissionModel({type: 'content', action: 'upload', permission: 'create'}));
         }
-        });
-
-        this.checkPermission();
         // reset the value of the input file
         $event.target.value = '';
     }
@@ -320,11 +320,11 @@ export class UploadButtonComponent implements OnInit, OnChanges {
     checkPermission() {
         if (this.rootFolderId) {
             this.uploadService.getFolderNode(this.rootFolderId).subscribe(
-                res => {
+                (res) => {
                     this.permissionValue.next(this.hasCreatePermission(res));
                 },
-                error => {
+                (error) => {
-                    this.logService.error(error);
+                    this.onError.emit(error);
                 }
             );
         }