From 7fa592ebe738de4bfc656de68073c0cfd46d9ac9 Mon Sep 17 00:00:00 2001 From: Eugenio Romano Date: Mon, 30 Oct 2017 12:47:03 +0000 Subject: [PATCH] [ADF-1786] fix delete and permission on the same button case (#2565) * fix delete and permission on the same button case * ripristinate space --- .../directives/node-delete.directive.spec.ts | 95 +++++++-- .../src/directives/node-delete.directive.ts | 53 +++-- .../node-permission.directive.spec.ts | 189 +++++++----------- .../directives/node-permission.directive.ts | 65 +----- .../src/directives/node-restore.directive.ts | 6 +- .../ng2-alfresco-documentlist/index.ts | 2 +- .../components/tag-node-list.component.scss | 1 + 7 files changed, 194 insertions(+), 217 deletions(-) diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.spec.ts b/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.spec.ts index 00af6f5fb4..4347f9d738 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.spec.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.spec.ts @@ -18,8 +18,6 @@ import { Component, DebugElement } from '@angular/core'; import { async, ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; -import { Observable } from 'rxjs/Rx'; -import { AlfrescoTranslationService } from '../../index'; import { CoreModule } from '../../index'; import { AlfrescoApiService } from '../services/alfresco-api.service'; import { NotificationService } from '../services/notification.service'; @@ -37,12 +35,26 @@ class TestComponent { done = jasmine.createSpy('done'); } +@Component({ + template: ` +
+
` +}) +class TestWithPermissionsComponent { + selection = []; + + done = jasmine.createSpy('done'); +} + describe('NodeDeleteDirective', () => { let fixture: ComponentFixture; + let fixtureWithPermissions: ComponentFixture; let element: DebugElement; + let elementWithPermissions: DebugElement; let component: TestComponent; + let componentWithPermissions: TestWithPermissionsComponent; let alfrescoApi: AlfrescoApiService; - let translation: AlfrescoTranslationService; let notification: NotificationService; let nodeApi; @@ -52,28 +64,25 @@ describe('NodeDeleteDirective', () => { CoreModule ], declarations: [ - TestComponent + TestComponent, + TestWithPermissionsComponent ] }) - .compileComponents() - .then(() => { - fixture = TestBed.createComponent(TestComponent); - component = fixture.componentInstance; - element = fixture.debugElement.query(By.directive(NodeDeleteDirective)); + .compileComponents() + .then(() => { + fixture = TestBed.createComponent(TestComponent); + fixtureWithPermissions = TestBed.createComponent(TestWithPermissionsComponent); + component = fixture.componentInstance; + componentWithPermissions = fixtureWithPermissions.componentInstance; + element = fixture.debugElement.query(By.directive(NodeDeleteDirective)); + elementWithPermissions = fixtureWithPermissions.debugElement.query(By.directive(NodeDeleteDirective)); - alfrescoApi = TestBed.get(AlfrescoApiService); - nodeApi = alfrescoApi.getInstance().nodes; - translation = TestBed.get(AlfrescoTranslationService); - notification = TestBed.get(NotificationService); - }); + alfrescoApi = TestBed.get(AlfrescoApiService); + nodeApi = alfrescoApi.getInstance().nodes; + notification = TestBed.get(NotificationService); + }); })); - beforeEach(() => { - spyOn(translation, 'get').and.callFake((key) => { - return Observable.of(key); - }); - }); - describe('Delete', () => { beforeEach(() => { spyOn(notification, 'openSnackMessage'); @@ -216,5 +225,51 @@ describe('NodeDeleteDirective', () => { expect(component.done).toHaveBeenCalled(); })); + + it('should disable the button if no node are selected', fakeAsync(() => { + component.selection = []; + + fixture.detectChanges(); + + expect(element.nativeElement.disabled).toEqual(true); + })); + + it('should disable the button if selected node is null', fakeAsync(() => { + component.selection = null; + + fixture.detectChanges(); + + expect(element.nativeElement.disabled).toEqual(true); + })); + + it('should enable the button if nodes are selected', fakeAsync(() => { + component.selection = [ + { entry: { id: '1', name: 'name1' } }, + { entry: { id: '2', name: 'name2' } }, + { entry: { id: '3', name: 'name3' } } + ]; + + fixture.detectChanges(); + + expect(element.nativeElement.disabled).toEqual(false); + })); + + it('should not enable the button if adf-node-permission is present', fakeAsync(() => { + elementWithPermissions.nativeElement.disabled = false; + componentWithPermissions.selection = []; + + fixtureWithPermissions.detectChanges(); + + componentWithPermissions.selection = [ + { entry: { id: '1', name: 'name1' } }, + { entry: { id: '2', name: 'name2' } }, + { entry: { id: '3', name: 'name3' } } + ]; + + fixtureWithPermissions.detectChanges(); + + expect(elementWithPermissions.nativeElement.disabled).toEqual(false); + })); + }); }); diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.ts b/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.ts index 844731b067..07b896c060 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-delete.directive.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { Directive, EventEmitter, HostListener, Input, Output } from '@angular/core'; +import { Directive, ElementRef, EventEmitter, HostListener, Input, OnChanges, Output } from '@angular/core'; import { MinimalNodeEntity, MinimalNodeEntryEntity } from 'alfresco-js-api'; import { Observable } from 'rxjs/Rx'; import { AlfrescoApiService } from '../services/alfresco-api.service'; @@ -30,38 +30,56 @@ interface ProcessedNodeData { interface ProcessStatus { success: ProcessedNodeData[]; failed: ProcessedNodeData[]; + someFailed(); + someSucceeded(); + oneFailed(); + oneSucceeded(); + allSucceeded(); + allFailed(); } @Directive({ selector: '[adf-delete]' }) -export class NodeDeleteDirective { - private nodesApi; - +export class NodeDeleteDirective implements OnChanges { @Input('adf-delete') selection: MinimalNodeEntity[]; - @Input() permanent: boolean = false; + @Input() + permanent: boolean = false; - @Output() delete: EventEmitter = new EventEmitter(); + @Output() + delete: EventEmitter = new EventEmitter(); @HostListener('click') onClick() { this.process(this.selection); } - constructor( - private notification: NotificationService, - private alfrescoApiService: AlfrescoApiService, - private translation: TranslationService - ) { - this.nodesApi = this.alfrescoApiService.getInstance().nodes; + constructor(private notification: NotificationService, + private alfrescoApiService: AlfrescoApiService, + private translation: TranslationService, + private elementRef: ElementRef) { + } + + ngOnChanges() { + if (!this.selection || (this.selection && this.selection.length === 0)) { + this.setDisableAttribute(true); + } else { + if (!this.elementRef.nativeElement.hasAttribute('adf-node-permission')) { + this.setDisableAttribute(false); + } + } + } + + private setDisableAttribute(disable: boolean) { + this.elementRef.nativeElement.disabled = disable; } private process(selection: MinimalNodeEntity[]) { @@ -88,10 +106,9 @@ export class NodeDeleteDirective { } private deleteNode(node: MinimalNodeEntity): Observable { - // shared nodes support const id = ( node.entry).nodeId || node.entry.id; - const promise = this.nodesApi.deleteNode(id, { permanent: this.permanent }); + const promise = this.alfrescoApiService.getInstance().nodes.deleteNode(id, {permanent: this.permanent}); return Observable.fromPromise(promise) .map(() => ({ @@ -152,14 +169,14 @@ export class NodeDeleteDirective { if (status.allFailed && !status.oneFailed) { return this.translation.get( 'CORE.DELETE_NODE.ERROR_PLURAL', - { number: status.failed.length } + {number: status.failed.length} ); } if (status.allSucceeded && !status.oneSucceeded) { return this.translation.get( 'CORE.DELETE_NODE.PLURAL', - { number: status.success.length } + {number: status.success.length} ); } @@ -186,14 +203,14 @@ export class NodeDeleteDirective { if (status.oneFailed && !status.someSucceeded) { return this.translation.get( 'CORE.DELETE_NODE.ERROR_SINGULAR', - { name: status.failed[0].entry.name } + {name: status.failed[0].entry.name} ); } if (status.oneSucceeded && !status.someFailed) { return this.translation.get( 'CORE.DELETE_NODE.SINGULAR', - { name: status.success[0].entry.name } + {name: status.success[0].entry.name} ); } } diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts index e2e47bac89..cd675c5d86 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts @@ -15,150 +15,99 @@ * limitations under the License. */ -import { ChangeDetectorRef, Component, ElementRef, SimpleChange } from '@angular/core'; +import { Component, DebugElement } from '@angular/core'; +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { CoreModule } from '../../index'; import { AlfrescoContentService } from './../services/alfresco-content.service'; -import { NodePermissionDirective, NodePermissionSubject } from './node-permission.directive'; +import { NodePermissionDirective } from './node-permission.directive'; @Component({ - selector: 'adf-text-subject' + template: ` +
+
` }) -class TestComponent implements NodePermissionSubject { - disabled: boolean = false; +class TestComponent { + selection = []; + disabled = false; + done = jasmine.createSpy('done'); } describe('NodePermissionDirective', () => { + let fixture: ComponentFixture; + let element: DebugElement; + let component: TestComponent; + let alfrescoContentService: AlfrescoContentService; - let changeDetectorMock: ChangeDetectorRef; + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [ + CoreModule + ], + declarations: [ + TestComponent + ] + }) + .compileComponents() + .then(() => { + fixture = TestBed.createComponent(TestComponent); + alfrescoContentService = TestBed.get(AlfrescoContentService); + component = fixture.componentInstance; + element = fixture.debugElement.query(By.directive(NodePermissionDirective)); + }); + })); - beforeEach(() => { - changeDetectorMock = { detectChanges: () => {} }; + it('Should be disabled if no nodes are passed', () => { + component.selection = undefined; + + fixture.detectChanges(); + + component.selection = null; + + fixture.detectChanges(); + + expect(element.nativeElement.disabled).toEqual(true); }); - describe('HTML nativeElement as subject', () => { + it('Should be disabled if nodes is an empty array', () => { + component.selection = null; - it('updates element once it is loaded', () => { - const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); - spyOn(directive, 'updateElement').and.stub(); + fixture.detectChanges(); - const nodes = [{}, {}]; - const change = new SimpleChange([], nodes, false); - directive.ngOnChanges({ nodes: change }); + component.selection = []; - expect(directive.updateElement).toHaveBeenCalled(); - }); + fixture.detectChanges(); - it('updates element on nodes change', () => { - const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); - spyOn(directive, 'updateElement').and.stub(); - - const nodes = [{}, {}]; - const change = new SimpleChange([], nodes, false); - directive.ngOnChanges({ nodes: change }); - - expect(directive.updateElement).toHaveBeenCalled(); - }); - - it('updates element only on subsequent change', () => { - const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); - spyOn(directive, 'updateElement').and.stub(); - - const nodes = [{}, {}]; - const change = new SimpleChange([], nodes, true); - directive.ngOnChanges({ nodes: change }); - - expect(directive.updateElement).not.toHaveBeenCalled(); - }); - - it('enables decorated element', () => { - const renderer = jasmine.createSpyObj('renderer', ['removeAttribute']); - const elementRef = new ElementRef({}); - const directive = new NodePermissionDirective(elementRef, renderer, null, changeDetectorMock); - - directive.enableElement(); - - expect(renderer.removeAttribute).toHaveBeenCalledWith(elementRef.nativeElement, 'disabled'); - }); - - it('disables decorated element', () => { - const renderer = jasmine.createSpyObj('renderer', ['setAttribute']); - const elementRef = new ElementRef({}); - const directive = new NodePermissionDirective(elementRef, renderer, null, changeDetectorMock); - - directive.disableElement(); - - expect(renderer.setAttribute).toHaveBeenCalledWith(elementRef.nativeElement, 'disabled', 'true'); - }); - - it('disables element when nodes not available', () => { - const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); - spyOn(directive, 'disableElement').and.stub(); - - directive.nodes = null; - expect(directive.updateElement()).toBeFalsy(); - - directive.nodes = []; - expect(directive.updateElement()).toBeFalsy(); - }); - - it('enables element when all nodes have expected permission', () => { - const contentService = new AlfrescoContentService(null, null, null); - spyOn(contentService, 'hasPermission').and.returnValue(true); - - const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock); - spyOn(directive, 'enableElement').and.stub(); - - directive.nodes = [{}, {}]; - - expect(directive.updateElement()).toBeTruthy(); - expect(directive.enableElement).toHaveBeenCalled(); - }); - - it('disables element when one of the nodes have no permission', () => { - const contentService = new AlfrescoContentService(null, null, null); - spyOn(contentService, 'hasPermission').and.returnValue(false); - - const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock); - spyOn(directive, 'disableElement').and.stub(); - - directive.nodes = [{}, {}]; - - expect(directive.updateElement()).toBeFalsy(); - expect(directive.disableElement).toHaveBeenCalled(); - }); + expect(element.nativeElement.disabled).toEqual(true); }); - describe('Angular component as subject', () => { + it('enables element when all nodes have expected permission', () => { + spyOn(alfrescoContentService, 'hasPermission').and.returnValue(true); - it('disables decorated component', () => { - const contentService = new AlfrescoContentService(null, null, null); - spyOn(contentService, 'hasPermission').and.returnValue(false); - spyOn(changeDetectorMock, 'detectChanges'); + component.selection = null; - let testComponent = new TestComponent(); - testComponent.disabled = false; - const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock, testComponent); - directive.nodes = [{}, {}]; + fixture.detectChanges(); - directive.updateElement(); + component.selection = [{entry: {id: '1', name: 'name1'}}]; - expect(testComponent.disabled).toBeTruthy(); - expect(changeDetectorMock.detectChanges).toHaveBeenCalledTimes(1); - }); + fixture.detectChanges(); - it('enables decorated component', () => { - const contentService = new AlfrescoContentService(null, null, null); - spyOn(contentService, 'hasPermission').and.returnValue(true); - spyOn(changeDetectorMock, 'detectChanges'); + expect(element.nativeElement.disabled).toEqual(false); + }); - let testComponent = new TestComponent(); - testComponent.disabled = true; - const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock, testComponent); - directive.nodes = [{}, {}]; + it('disables element when one of the nodes have no permission', () => { + spyOn(alfrescoContentService, 'hasPermission').and.returnValue(false); - directive.updateElement(); + component.selection = null; - expect(testComponent.disabled).toBeFalsy(); - expect(changeDetectorMock.detectChanges).toHaveBeenCalledTimes(1); - }); + fixture.detectChanges(); + + component.selection = [{entry: {id: '1', name: 'name1'}}]; + + fixture.detectChanges(); + + expect(element.nativeElement.disabled).toEqual(true); }); }); + +}) diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts index e5ca7b7ea7..3046465923 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts @@ -15,9 +15,8 @@ * limitations under the License. */ -import { ChangeDetectorRef, Directive, ElementRef, Host, Inject, Input, OnChanges, Optional, Renderer2, SimpleChanges } from '@angular/core'; +import { Directive, ElementRef, Input, OnChanges, SimpleChanges } from '@angular/core'; import { MinimalNodeEntity } from 'alfresco-js-api'; -import { EXTENDIBLE_COMPONENT } from './../interface/injection.tokens'; import { AlfrescoContentService } from './../services/alfresco-content.service'; export interface NodePermissionSubject { @@ -30,19 +29,13 @@ export interface NodePermissionSubject { export class NodePermissionDirective implements OnChanges { @Input('adf-node-permission') - permission: string = null; + permission: string = null; @Input('adf-nodes') nodes: MinimalNodeEntity[] = []; constructor(private elementRef: ElementRef, - private renderer: Renderer2, - private contentService: AlfrescoContentService, - private changeDetector: ChangeDetectorRef, - - @Host() - @Optional() - @Inject(EXTENDIBLE_COMPONENT) private parentComponent?: NodePermissionSubject) { + private contentService: AlfrescoContentService) { } ngOnChanges(changes: SimpleChanges) { @@ -52,57 +45,17 @@ export class NodePermissionDirective implements OnChanges { } /** - * Updates disabled state for the decorated elememtn - * - * @returns {boolean} True if decorated element got disabled, otherwise False - * @memberof NodePermissionDirective - */ - updateElement(): boolean { - let enable = this.hasPermission(this.nodes, this.permission); - - if (enable) { - this.enable(); - } else { - this.disable(); - } - - return enable; - } - - private enable(): void { - if (this.parentComponent) { - this.parentComponent.disabled = false; - this.changeDetector.detectChanges(); - } else { - this.enableElement(); - } - } - - private disable(): void { - if (this.parentComponent) { - this.parentComponent.disabled = true; - this.changeDetector.detectChanges(); - } else { - this.disableElement(); - } - } - - /** - * Enables decorated element + * Updates disabled state for the decorated element * * @memberof NodePermissionDirective */ - enableElement(): void { - this.renderer.removeAttribute(this.elementRef.nativeElement, 'disabled'); + updateElement(): void { + let hasPermission = this.hasPermission(this.nodes, this.permission); + this.setDisableAttribute(!hasPermission); } - /** - * Disables decorated element - * - * @memberof NodePermissionDirective - */ - disableElement(): void { - this.renderer.setAttribute(this.elementRef.nativeElement, 'disabled', 'true'); + private setDisableAttribute(disable: boolean) { + this.elementRef.nativeElement.disabled = disable; } /** diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-restore.directive.ts b/ng2-components/ng2-alfresco-core/src/directives/node-restore.directive.ts index bd95459c3b..5f6f99a4ed 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-restore.directive.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-restore.directive.ts @@ -32,9 +32,11 @@ export class NodeRestoreDirective { @Input('adf-restore') selection: DeletedNodeEntry[]; - @Input() location: string = ''; + @Input() + location: string = ''; - @Output() restore: EventEmitter = new EventEmitter(); + @Output() + restore: EventEmitter = new EventEmitter(); @HostListener('click') onClick() { diff --git a/ng2-components/ng2-alfresco-documentlist/index.ts b/ng2-components/ng2-alfresco-documentlist/index.ts index a2d646d549..eac8697248 100644 --- a/ng2-components/ng2-alfresco-documentlist/index.ts +++ b/ng2-components/ng2-alfresco-documentlist/index.ts @@ -16,10 +16,10 @@ */ import { NgModule } from '@angular/core'; +import { FlexLayoutModule } from '@angular/flex-layout'; import { CoreModule, TRANSLATION_PROVIDER } from 'ng2-alfresco-core'; import { DataTableModule } from 'ng2-alfresco-datatable'; import { UploadModule } from 'ng2-alfresco-upload'; -import { FlexLayoutModule } from '@angular/flex-layout'; import { BreadcrumbComponent } from './src/components/breadcrumb/breadcrumb.component'; import { DropdownBreadcrumbComponent } from './src/components/breadcrumb/dropdown-breadcrumb.component'; diff --git a/ng2-components/ng2-alfresco-tag/src/components/tag-node-list.component.scss b/ng2-components/ng2-alfresco-tag/src/components/tag-node-list.component.scss index 301042d077..cd6b147342 100644 --- a/ng2-components/ng2-alfresco-tag/src/components/tag-node-list.component.scss +++ b/ng2-components/ng2-alfresco-tag/src/components/tag-node-list.component.scss @@ -1,4 +1,5 @@ .adf-tag-chips-delete { + overflow: visible; cursor: pointer; height: 17px; width: 20px;