From cc9548b25a7179b26b121688a2879c220854ec31 Mon Sep 17 00:00:00 2001 From: Suzana Dirla Date: Mon, 13 Aug 2018 16:36:12 +0300 Subject: [PATCH] [ADF-3362][ADF-3361] Search on document picker with custom site list - fix (#3592) * [ADF-3362] Search on document picker with custom site list - fix * refactor & fix ADF-3361 & ADF-3362 * [ADF-3361][ADF-3362] set spy on the right method * [ADF-3361][ADF-3362] fix tests * [ADF-3361][ADF-3362] set constant * [ADF-3361][ADF-3362] remove unused method * [ADF-3361][ADF-3362] more relevant tests related to the fix * [ADF-3362] refactor method * [ADF-3362] fix tslint errors * remove pagination override --- ...tent-node-selector-panel.component.spec.ts | 50 +++++++++++++--- .../content-node-selector-panel.component.ts | 6 +- .../components/document-list.component.ts | 18 +----- .../services/custom-resources.service.ts | 60 ++++++++++--------- 4 files changed, 81 insertions(+), 53 deletions(-) diff --git a/lib/content-services/content-node-selector/content-node-selector-panel.component.spec.ts b/lib/content-services/content-node-selector/content-node-selector-panel.component.spec.ts index 114f346c0a..1991c98f24 100644 --- a/lib/content-services/content-node-selector/content-node-selector-panel.component.spec.ts +++ b/lib/content-services/content-node-selector/content-node-selector-panel.component.spec.ts @@ -28,6 +28,7 @@ import { NodePaging } from 'alfresco-js-api'; import { ContentTestingModule } from '../testing/content.testing.module'; import { DocumentListService } from '../document-list/services/document-list.service'; import { DocumentListComponent } from '../document-list/components/document-list.component'; +import { CustomResourcesService } from '../document-list/services/custom-resources.service'; const ONE_FOLDER_RESULT = { list: { @@ -295,7 +296,8 @@ describe('ContentNodeSelectorComponent', () => { const sitesService = TestBed.get(SitesService); spyOn(sitesService, 'getSites').and.returnValue(of({ list: { entries: [] } })); - getCorrespondingNodeIdsSpy = spyOn(component.documentList, 'getCorrespondingNodeIds').and + const customResourcesService = TestBed.get(CustomResourcesService); + getCorrespondingNodeIdsSpy = spyOn(customResourcesService, 'getCorrespondingNodeIds').and .callFake(id => { if (id === '-sites-') { return of(['123456testId', '09876543testId']); @@ -349,7 +351,8 @@ describe('ContentNodeSelectorComponent', () => { expect(cnSearchSpy).toHaveBeenCalled(); expect(cnSearchSpy.calls.count()).toBe(2); - expect(cnSearchSpy).toHaveBeenCalledWith('vegeta', '-sites-', 0, 25); + expect(cnSearchSpy).toHaveBeenCalledWith('vegeta', undefined, 0, 25); + expect(cnSearchSpy).toHaveBeenCalledWith('vegeta', '-sites-', 0, 25, ['123456testId', '09876543testId']); })); it('should call the content node selector\'s search with the right parameters on changing the site selectbox\'s value from a custom dropdown menu', fakeAsync(() => { @@ -366,10 +369,21 @@ describe('ContentNodeSelectorComponent', () => { expect(cnSearchSpy).toHaveBeenCalled(); expect(cnSearchSpy.calls.count()).toBe(2); + expect(cnSearchSpy).toHaveBeenCalledWith('vegeta', undefined, 0, 25); expect(cnSearchSpy).toHaveBeenCalledWith('vegeta', '-sites-', 0, 25, ['123456testId', '09876543testId']); })); - it('should get the corresponding node ids before the search call on changing the site selectbox\'s value from a custom dropdown menu', fakeAsync(() => { + it('should get the corresponding node ids on search when a known alias is selected from dropdown', fakeAsync(() => { + typeToSearchBox('vegeta'); + + tick(debounceSearch); + + component.siteChanged( { entry: { guid: '-sites-' } }); + expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(1, 'getCorrespondingNodeIdsSpy calls count should be one after the site changes to known alias \'-sites\-'); + expect(getCorrespondingNodeIdsSpy.calls.mostRecent().args[0]).toEqual('-sites-'); + })); + + it('should get the corresponding node ids on search when a known alias is selected from CUSTOM dropdown', fakeAsync(() => { component.dropdownSiteList = { list: { entries: [ { entry: { guid: '-sites-' } }, { entry: { guid: 'namek' } }] } }; fixture.detectChanges(); @@ -377,15 +391,37 @@ describe('ContentNodeSelectorComponent', () => { tick(debounceSearch); - expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(1, 'getCorrespondingNodeIdsSpy calls count should be one after only one search'); + component.siteChanged( { entry: { guid: '-sites-' } }); + expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(1); + expect(getCorrespondingNodeIdsSpy.calls.mostRecent().args[0]).toEqual('-sites-'); + })); + + it('should NOT get the corresponding node ids on search when NOTHING is selected from dropdown', fakeAsync(() => { + component.dropdownSiteList = { list: { entries: [ { entry: { guid: '-sites-' } }, { entry: { guid: 'namek' } }] } }; + fixture.detectChanges(); + + typeToSearchBox('vegeta'); + + tick(debounceSearch); + + expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(0, 'getCorrespondingNodeIdsSpy calls count should be 0 when no site is selected'); + })); + + it('should NOT get the corresponding node ids on search when NO known alias is selected from dropdown', fakeAsync(() => { + typeToSearchBox('vegeta'); + tick(debounceSearch); + + expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(0, 'getCorrespondingNodeIdsSpy should not be called'); component.siteChanged( { entry: { guid: 'namek' } }); - expect(getCorrespondingNodeIdsSpy.calls.count()).toBe(2, 'getCorrespondingNodeIdsSpy calls count should be two after the site change'); - expect(getCorrespondingNodeIdsSpy.calls.allArgs()).toEqual([[undefined], ['namek']]); + expect(getCorrespondingNodeIdsSpy).not.toHaveBeenCalled(); })); - it('should NOT get the corresponding node ids before the search call on changing the site selectbox\'s value from default dropdown menu', fakeAsync(() => { + it('should NOT get the corresponding node ids on search when NO known alias is selected from CUSTOM dropdown', fakeAsync(() => { + component.dropdownSiteList = { list: { entries: [ { entry: { guid: '-sites-' } }, { entry: { guid: 'namek' } }] } }; + fixture.detectChanges(); + typeToSearchBox('vegeta'); tick(debounceSearch); diff --git a/lib/content-services/content-node-selector/content-node-selector-panel.component.ts b/lib/content-services/content-node-selector/content-node-selector-panel.component.ts index deab20c527..cf8f93f0a1 100644 --- a/lib/content-services/content-node-selector/content-node-selector-panel.component.ts +++ b/lib/content-services/content-node-selector/content-node-selector-panel.component.ts @@ -28,6 +28,7 @@ import { ImageResolver } from '../document-list/data/image-resolver.model'; import { ContentNodeSelectorService } from './content-node-selector.service'; import { debounceTime } from 'rxjs/operators'; import { BehaviorSubject } from 'rxjs'; +import { CustomResourcesService } from '../document-list/services/custom-resources.service'; export type ValidationFunction = (entry: MinimalNodeEntryEntity) => boolean; @@ -120,6 +121,7 @@ export class ContentNodeSelectorPanelComponent implements OnInit, PaginatedCompo constructor(private contentNodeSelectorService: ContentNodeSelectorService, private apiService: AlfrescoApiService, + private customResourcesService: CustomResourcesService, private preferences: UserPreferencesService) { this.searchInput.valueChanges .pipe( @@ -255,8 +257,8 @@ export class ContentNodeSelectorPanelComponent implements OnInit, PaginatedCompo private querySearch(): void { this.loadingSearchResults = true; - if (this.dropdownSiteList) { - this.documentList.getCorrespondingNodeIds(this.siteId) + if (this.customResourcesService.hasCorrespondingNodeIds(this.siteId)) { + this.customResourcesService.getCorrespondingNodeIds(this.siteId) .subscribe(nodeIds => { this.contentNodeSelectorService.search(this.searchTerm, this.siteId, this.skipCount, this.pageSize, nodeIds) .subscribe(this.showSearchResults.bind(this)); diff --git a/lib/content-services/document-list/components/document-list.component.ts b/lib/content-services/document-list/components/document-list.component.ts index 7bdc5c6340..db08e3458e 100644 --- a/lib/content-services/document-list/components/document-list.component.ts +++ b/lib/content-services/document-list/components/document-list.component.ts @@ -27,7 +27,7 @@ import { } from '@alfresco/adf-core'; import { MinimalNodeEntity, MinimalNodeEntryEntity, NodePaging } from 'alfresco-js-api'; -import { Observable, Subject, BehaviorSubject, Subscription, of } from 'rxjs'; +import { Subject, BehaviorSubject, Subscription, of } from 'rxjs'; import { ShareDataRow } from './../data/share-data-row.model'; import { ShareDataTableAdapter } from './../data/share-datatable-adapter'; import { presetsDefaultModel } from '../models/preset.model'; @@ -813,22 +813,6 @@ export class DocumentListComponent implements OnInit, OnChanges, OnDestroy, Afte this.pagination.value.skipCount = 0; } - // TODO: remove it from here - getCorrespondingNodeIds(nodeId: string): Observable { - if (this.customResourcesService.isCustomSource(nodeId)) { - return this.customResourcesService.getCorrespondingNodeIds(nodeId, this.pagination.getValue()); - } else if (nodeId) { - return new Observable(observer => { - this.documentListService.getFolderNode(nodeId, this.includeFields) - .subscribe((node: MinimalNodeEntryEntity) => { - observer.next([node.id]); - observer.complete(); - }); - }); - } - - } - ngOnDestroy() { this.subscriptions.forEach(s => s.unsubscribe()); this.subscriptions = []; diff --git a/lib/content-services/document-list/services/custom-resources.service.ts b/lib/content-services/document-list/services/custom-resources.service.ts index dae3002c7c..1cde8a171e 100644 --- a/lib/content-services/document-list/services/custom-resources.service.ts +++ b/lib/content-services/document-list/services/custom-resources.service.ts @@ -30,7 +30,7 @@ import { } from 'alfresco-js-api'; import { Injectable } from '@angular/core'; import { Observable, from, of, throwError } from 'rxjs'; -import { catchError } from 'rxjs/operators'; +import { catchError, map } from 'rxjs/operators'; @Injectable() export class CustomResourcesService { @@ -261,6 +261,17 @@ export class CustomResourcesService { return isCustomSources; } + isSupportedSource(folderId: string): boolean { + let isSupportedSources = false; + const sources = ['-my-', '-root-', '-shared-']; + + if (sources.indexOf(folderId) > -1) { + isSupportedSources = true; + } + + return isSupportedSources; + } + /** * Gets a folder's contents. * @param nodeId ID of the target folder node @@ -292,42 +303,37 @@ export class CustomResourcesService { * @param pagination Specifies how to paginate the results * @returns List of node IDs */ - getCorrespondingNodeIds(nodeId: string, pagination: PaginationModel): Observable { - if (nodeId === '-trashcan-') { - return from(this.apiService.nodesApi.getDeletedNodes() - .then(result => result.list.entries.map(node => node.entry.id))); + getCorrespondingNodeIds(nodeId: string, pagination: PaginationModel = {}): Observable { + if (this.isCustomSource(nodeId)) { - } else if (nodeId === '-sharedlinks-') { - return from(this.apiService.sharedLinksApi.findSharedLinks() - .then(result => result.list.entries.map(node => node.entry.nodeId))); + return this.loadFolderByNodeId(nodeId, pagination, []) + .pipe(map(result => result.list.entries.map((node: any) => { + if (nodeId === '-sharedlinks-') { + return node.entry.nodeId; - } else if (nodeId === '-sites-') { - return from(this.apiService.sitesApi.getSites() - .then(result => result.list.entries.map(node => node.entry.guid))); + } else if (nodeId === '-sites-' || nodeId === '-mysites-') { + return node.entry.guid; - } else if (nodeId === '-mysites-') { - return from(this.apiService.peopleApi.getSiteMembership('-me-') - .then(result => result.list.entries.map(node => node.entry.guid))); + } else if (nodeId === '-favorites-') { + return node.entry.targetGuid; + } - } else if (nodeId === '-favorites-') { - return from(this.apiService.favoritesApi.getFavorites('-me-') - .then(result => result.list.entries.map(node => node.entry.targetGuid))); - - } else if (nodeId === '-recent-') { - return new Observable(observer => { - this.getRecentFiles('-me-', pagination) - .subscribe((recentFiles) => { - let recentFilesIdS = recentFiles.list.entries.map(node => node.entry.id); - observer.next(recentFilesIdS); - observer.complete(); - }); - }); + return node.entry.id; + }))); + } else if (nodeId) { + // cases when nodeId is '-my-', '-root-' or '-shared-' + return from(this.apiService.nodesApi.getNode(nodeId) + .then(node => [node.entry.id])); } return of([]); } + hasCorrespondingNodeIds(nodeId: string): boolean { + return this.isCustomSource(nodeId) || this.isSupportedSource(nodeId); + } + private getIncludesFields(includeFields: string[]): string[] { return ['path', 'properties', 'allowableOperations', 'permissions', ...includeFields] .filter((element, index, array) => index === array.indexOf(element));