diff --git a/lib/content-services/src/lib/aspect-list/aspect-list-dialog.component.spec.ts b/lib/content-services/src/lib/aspect-list/aspect-list-dialog.component.spec.ts index ef97a81ff5..ff62ca31b2 100644 --- a/lib/content-services/src/lib/aspect-list/aspect-list-dialog.component.spec.ts +++ b/lib/content-services/src/lib/aspect-list/aspect-list-dialog.component.spec.ts @@ -20,7 +20,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { AspectListDialogComponent } from './aspect-list-dialog.component'; import { of, Subject } from 'rxjs'; import { AspectListDialogComponentData } from './aspect-list-dialog-data.interface'; -import { AspectListService } from './services/aspect-list.service'; +import { AspectListService, StandardAspectsWhere } from './services/aspect-list.service'; import { delay } from 'rxjs/operators'; import { AspectEntry, Node } from '@alfresco/js-api'; import { NodesApiService } from '../common/services/nodes-api.service'; @@ -144,8 +144,8 @@ describe('AspectListDialogComponent', () => { describe('Without passing node id', () => { beforeEach(() => { aspectListService = TestBed.inject(AspectListService); - spyOn(aspectListService, 'getAllAspects').and.returnValue( - of({ standardAspectPaging: { list: { entries: aspectListMock } }, customAspectPaging: { list: { entries: customAspectListMock } } }) + spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => + of({ list: { entries: opts.where === StandardAspectsWhere ? aspectListMock : customAspectListMock } }) ); fixture.detectChanges(); }); @@ -251,11 +251,10 @@ describe('AspectListDialogComponent', () => { data.nodeId = 'fake-node-id'; aspectListService = TestBed.inject(AspectListService); nodeService = TestBed.inject(NodesApiService); - spyOn(aspectListService, 'getAllAspects').and.returnValue( - of({ standardAspectPaging: { list: { entries: aspectListMock } }, customAspectPaging: { list: { entries: customAspectListMock } } }) - ); spyOn(aspectListService, 'getVisibleAspects').and.returnValue(['frs:AspectOne']); - spyOn(aspectListService, 'getAspects').and.returnValue(of({ list: { entries: customAspectListMock } })); + spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => + of({ list: { entries: opts.where === StandardAspectsWhere ? aspectListMock : customAspectListMock } }) + ); spyOn(nodeService, 'getNode').and.returnValue( of(new Node({ id: 'fake-node-id', aspectNames: ['frs:AspectOne', 'cst:customAspect'] })).pipe(delay(0)) ); diff --git a/lib/content-services/src/lib/aspect-list/aspect-list.component.spec.ts b/lib/content-services/src/lib/aspect-list/aspect-list.component.spec.ts index 857a681922..0264683b39 100644 --- a/lib/content-services/src/lib/aspect-list/aspect-list.component.spec.ts +++ b/lib/content-services/src/lib/aspect-list/aspect-list.component.spec.ts @@ -19,15 +19,14 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { NodesApiService } from '../common/services/nodes-api.service'; import { AspectListComponent } from './aspect-list.component'; import { AspectListService, CustomAspectsWhere, StandardAspectsWhere } from './services/aspect-list.service'; -import { EMPTY, of } from 'rxjs'; -import { AspectEntry, Pagination } from '@alfresco/js-api'; +import { NEVER, of } from 'rxjs'; +import { AspectEntry } from '@alfresco/js-api'; import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { MatExpansionPanelHarness } from '@angular/material/expansion/testing'; import { MatTableHarness } from '@angular/material/table/testing'; import { MatCheckboxHarness } from '@angular/material/checkbox/testing'; import { MatProgressSpinnerHarness } from '@angular/material/progress-spinner/testing'; -import { CustomAspectPaging } from './interfaces/custom-aspect-paging.interface'; import { provideApiTesting } from '../testing/providers'; const aspectListMock: AspectEntry[] = [ @@ -111,11 +110,6 @@ const customAspectListMock: AspectEntry[] = [ } ]; -const allAspectsMock: CustomAspectPaging = { - standardAspectPaging: { list: { entries: aspectListMock } }, - customAspectPaging: { list: { entries: customAspectListMock } } -}; - describe('AspectListComponent', () => { let loader: HarnessLoader; let component: AspectListComponent; @@ -138,8 +132,8 @@ describe('AspectListComponent', () => { describe('Loading', () => { it('should show the loading spinner when result is loading', async () => { - spyOn(nodeService, 'getNode').and.returnValue(EMPTY); - spyOn(aspectListService, 'getAspects').and.returnValue(EMPTY); + spyOn(nodeService, 'getNode').and.returnValue(NEVER); + spyOn(aspectListService, 'getAspects').and.returnValue(NEVER); fixture.detectChanges(); expect(await loader.hasHarness(MatProgressSpinnerHarness)).toBe(true); @@ -148,8 +142,9 @@ describe('AspectListComponent', () => { describe('When passing a node id', () => { beforeEach(() => { - spyOn(aspectListService, 'getAllAspects').and.returnValue(of(allAspectsMock)); - spyOn(aspectListService, 'getAspects').and.returnValue(of({ list: { entries: customAspectListMock } })); + spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => + of({ list: { entries: opts.where === StandardAspectsWhere ? aspectListMock : customAspectListMock } }) + ); spyOn(aspectListService, 'getVisibleAspects').and.returnValue(['frs:AspectOne']); spyOn(nodeService, 'getNode').and.returnValue(of({ id: 'fake-node-id', aspectNames: ['frs:AspectOne', 'stored:aspect'] } as any)); component.nodeId = 'fake-node-id'; @@ -252,10 +247,18 @@ describe('AspectListComponent', () => { }); it('should load aspects with 0 skip count as pagination by default', () => { - expect(aspectListService.getAllAspects).toHaveBeenCalledWith( - { where: StandardAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 }, - { where: CustomAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 } - ); + expect(aspectListService.getAspects).toHaveBeenCalledWith(jasmine.anything(), { + where: StandardAspectsWhere, + include: ['properties'], + skipCount: 0, + maxItems: 100 + }); + expect(aspectListService.getAspects).toHaveBeenCalledWith(jasmine.anything(), { + where: CustomAspectsWhere, + include: ['properties'], + skipCount: 0, + maxItems: 100 + }); }); }); @@ -271,7 +274,9 @@ describe('AspectListComponent', () => { describe('When no node id is passed', () => { beforeEach(() => { - spyOn(aspectListService, 'getAllAspects').and.returnValue(of(allAspectsMock)); + spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => + of({ list: { entries: opts.where === StandardAspectsWhere ? aspectListMock : customAspectListMock } }) + ); }); afterEach(() => { @@ -294,35 +299,73 @@ describe('AspectListComponent', () => { it('should load aspects with 0 skip count as pagination by default', () => { fixture.detectChanges(); - expect(aspectListService.getAllAspects).toHaveBeenCalledWith( - { where: StandardAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 }, - { where: CustomAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 } - ); + expect(aspectListService.getAspects).toHaveBeenCalledWith(jasmine.anything(), { + where: StandardAspectsWhere, + include: ['properties'], + skipCount: 0, + maxItems: 100 + }); + expect(aspectListService.getAspects).toHaveBeenCalledWith(jasmine.anything(), { + where: CustomAspectsWhere, + include: ['properties'], + skipCount: 0, + maxItems: 100 + }); }); }); - it('should load next batch of aspects if not all items were returned by first call', (done) => { - fixture.detectChanges(); - const moreItemsPagination: Pagination = { count: 2, hasMoreItems: true }; - const allAspectsWithMoreItems: CustomAspectPaging = { - standardAspectPaging: { list: { entries: aspectListMock, pagination: moreItemsPagination } }, - customAspectPaging: { list: { entries: customAspectListMock, pagination: moreItemsPagination } } - }; - const getAspectsSpy = spyOn(aspectListService, 'getAllAspects').and.returnValues(of(allAspectsWithMoreItems), of(allAspectsMock)); - spyOn(aspectListService, 'getAspects').and.returnValues( - of({ list: { entries: aspectListMock } }), - of({ list: { entries: customAspectListMock } }) - ); + it('should keep requesting a type until its hasMoreItems is false, without re-querying an exhausted type', (done) => { + const getAspectsSpy = spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => { + if (opts.where === StandardAspectsWhere) { + return opts.skipCount === 0 + ? of({ list: { entries: aspectListMock, pagination: { count: 2, hasMoreItems: true } } }) + : of({ list: { entries: aspectListMock, pagination: { count: 2, hasMoreItems: false } } }); + } + return of({ list: { entries: customAspectListMock, pagination: { count: 2, hasMoreItems: false } } }); + }); + component.ngOnInit(); component.aspects$.subscribe(() => { - expect(getAspectsSpy.calls.argsFor(0)[0]).toEqual({ where: StandardAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 }); - expect(getAspectsSpy.calls.argsFor(0)[1]).toEqual({ where: CustomAspectsWhere, include: ['properties'], skipCount: 0, maxItems: 100 }); - expect(getAspectsSpy.calls.argsFor(1)[0]).toEqual({ where: StandardAspectsWhere, include: ['properties'], skipCount: 2, maxItems: 100 }); - expect(getAspectsSpy.calls.argsFor(1)[1]).toEqual({ where: CustomAspectsWhere, include: ['properties'], skipCount: 2, maxItems: 100 }); + const standardCalls = getAspectsSpy.calls.allArgs().filter(([, opts]) => opts.where === StandardAspectsWhere); + const customCalls = getAspectsSpy.calls.allArgs().filter(([, opts]) => opts.where === CustomAspectsWhere); + expect(standardCalls.map(([, opts]) => opts.skipCount)).toEqual([0, 2]); + expect(customCalls.map(([, opts]) => opts.skipCount)).toEqual([0]); done(); }); + }); + it('should page through all aspects (standard and custom) when categorising node aspects', (done) => { + const standardPage1: AspectEntry[] = [{ entry: { id: 'std:first', title: 'First', properties: [] } }]; + const standardPage2: AspectEntry[] = [{ entry: { id: 'std:second', title: 'Second', properties: [] } }]; + spyOn(aspectListService, 'getVisibleAspects').and.returnValue([]); + spyOn(nodeService, 'getNode').and.returnValue(of({ id: 'node-id', aspectNames: ['std:second'] } as any)); + const getAspectsSpy = spyOn(aspectListService, 'getAspects').and.callFake((_visibleAspects, opts) => { + if (opts.where === StandardAspectsWhere) { + return opts.skipCount === 0 + ? of({ list: { entries: standardPage1, pagination: { count: 1, hasMoreItems: true } } }) + : of({ list: { entries: standardPage2, pagination: { count: 1, hasMoreItems: false } } }); + } + return of({ list: { entries: [], pagination: { count: 0, hasMoreItems: false } } }); + }); + component.nodeId = 'node-id'; component.ngOnInit(); - fixture.detectChanges(); + + component.aspects$.subscribe(() => { + expect(getAspectsSpy).toHaveBeenCalledWith(jasmine.anything(), { + where: StandardAspectsWhere, + include: ['properties'], + skipCount: 0, + maxItems: 100 + }); + expect(getAspectsSpy).toHaveBeenCalledWith(jasmine.anything(), { + where: StandardAspectsWhere, + include: ['properties'], + skipCount: 1, + maxItems: 100 + }); + expect(component.nodeAspects).toContain('std:second'); + expect(component.notDisplayedAspects).not.toContain('std:second'); + done(); + }); }); }); diff --git a/lib/content-services/src/lib/aspect-list/aspect-list.component.ts b/lib/content-services/src/lib/aspect-list/aspect-list.component.ts index a36651ab83..7240f00971 100644 --- a/lib/content-services/src/lib/aspect-list/aspect-list.component.ts +++ b/lib/content-services/src/lib/aspect-list/aspect-list.component.ts @@ -17,11 +17,11 @@ import { Component, DestroyRef, EventEmitter, inject, Input, OnInit, Output, ViewEncapsulation } from '@angular/core'; import { NodesApiService } from '../common/services/nodes-api.service'; -import { EMPTY, Observable, zip } from 'rxjs'; -import { concatMap, expand, map, reduce, take, tap } from 'rxjs/operators'; +import { EMPTY, forkJoin, Observable } from 'rxjs'; +import { expand, map, reduce } from 'rxjs/operators'; import { AspectListService, CustomAspectsWhere, StandardAspectsWhere } from './services/aspect-list.service'; import { MatCheckboxChange, MatCheckboxModule } from '@angular/material/checkbox'; -import { AspectEntry, ContentPagingQuery, ListAspectsOpts } from '@alfresco/js-api'; +import { AspectEntry, ListAspectsOpts, Node } from '@alfresco/js-api'; import { CommonModule } from '@angular/common'; import { MatExpansionModule } from '@angular/material/expansion'; import { MatTableModule } from '@angular/material/table'; @@ -66,46 +66,19 @@ export class AspectListComponent implements OnInit { private readonly destroyRef = inject(DestroyRef); - private customAspectsLoaded = 0; - private standardAspectsLoaded = 0; - private hasMoreAspects = false; - ngOnInit(): void { - let aspects$: Observable; - if (this.nodeId) { - const node$ = this.nodeApiService.getNode(this.nodeId); - const customAspect$ = this.aspectListService - .getAspects(this.aspectListService.getVisibleAspects(), { - where: CustomAspectsWhere, - include: ['properties'], - skipCount: 0, - maxItems: 100 - }) - .pipe(map((customAspects) => customAspects?.list?.entries.flatMap((customAspect) => customAspect.entry.id))); - aspects$ = zip(node$, customAspect$).pipe( - tap(([node, customAspects]) => { - this.nodeAspects = node.aspectNames.filter( - (aspect) => this.aspectListService.getVisibleAspects().includes(aspect) || customAspects.includes(aspect) - ); - this.nodeAspectStatus = [...this.nodeAspects]; - this.notDisplayedAspects = node.aspectNames.filter( - (aspect) => !this.aspectListService.getVisibleAspects().includes(aspect) && !customAspects.includes(aspect) - ); - this.valueChanged.emit([...this.nodeAspects, ...this.notDisplayedAspects]); - this.updateCounter.emit(this.nodeAspects.length); - }), - concatMap(() => this.loadAspects({ skipCount: this.standardAspectsLoaded }, { skipCount: this.customAspectsLoaded })), - takeUntilDestroyed(this.destroyRef) - ); - } else { - aspects$ = this.loadAspects({ skipCount: this.standardAspectsLoaded }, { skipCount: this.customAspectsLoaded }); - } - this.aspects$ = aspects$.pipe( - expand(() => - this.hasMoreAspects ? this.loadAspects({ skipCount: this.standardAspectsLoaded }, { skipCount: this.customAspectsLoaded }) : EMPTY - ), - map((aspects) => aspects.filter((aspect) => !this.excludedAspects.includes(aspect.entry.id))), - reduce((acc, aspects) => [...acc, ...aspects]) + const allAspects$ = this.loadAllAspects(); + const displayAspects$ = this.nodeId + ? forkJoin([this.nodeApiService.getNode(this.nodeId), allAspects$]).pipe( + map(([node, allAspects]) => { + this.categoriseNodeAspects(node, allAspects); + return allAspects; + }) + ) + : allAspects$; + this.aspects$ = displayAspects$.pipe( + map((aspects) => aspects.filter((aspect) => !(this.excludedAspects ?? []).includes(aspect.entry.id))), + takeUntilDestroyed(this.destroyRef) ); } @@ -158,32 +131,38 @@ export class AspectListComponent implements OnInit { } } - private loadAspects(standardAspectsPagination?: ContentPagingQuery, customAspectsPagination?: ContentPagingQuery): Observable { - const standardAspectOpts: ListAspectsOpts = { - where: StandardAspectsWhere, - include: ['properties'], - skipCount: standardAspectsPagination?.skipCount ?? 0, - maxItems: 100 - }; - const customAspectOpts: ListAspectsOpts = { - where: CustomAspectsWhere, - include: ['properties'], - skipCount: customAspectsPagination?.skipCount ?? 0, - maxItems: 100 - }; - return this.aspectListService.getAllAspects(standardAspectOpts, customAspectOpts).pipe( - take(1), - tap((aspectsPaging) => { - this.customAspectsLoaded += aspectsPaging.customAspectPaging?.list?.pagination?.count ?? 0; - this.standardAspectsLoaded += aspectsPaging.standardAspectPaging?.list?.pagination?.count ?? 0; - this.hasMoreAspects = - aspectsPaging.customAspectPaging?.list?.pagination?.hasMoreItems || - aspectsPaging.standardAspectPaging?.list?.pagination?.hasMoreItems; - }), - map((aspectsPaging) => [ - ...(aspectsPaging.standardAspectPaging?.list?.entries ?? []), - ...(aspectsPaging.customAspectPaging?.list?.entries ?? []) - ]) + private loadAllAspects(): Observable { + return forkJoin([this.loadAllAspectsOfType(StandardAspectsWhere), this.loadAllAspectsOfType(CustomAspectsWhere)]).pipe( + map(([standardAspects, customAspects]) => [...standardAspects, ...customAspects]) ); } + + private loadAllAspectsOfType(where: string): Observable { + const visibleAspects = this.aspectListService.getVisibleAspects(); + const fetchPage = (skipCount: number): Observable<{ entries: AspectEntry[]; skipCount: number; hasMoreItems: boolean }> => { + const opts: ListAspectsOpts = { where, include: ['properties'], skipCount, maxItems: 100 }; + return this.aspectListService.getAspects(visibleAspects, opts).pipe( + map((aspectPaging) => ({ + entries: aspectPaging?.list?.entries ?? [], + skipCount: skipCount + (aspectPaging?.list?.pagination?.count ?? 0), + hasMoreItems: aspectPaging?.list?.pagination?.hasMoreItems ?? false + })) + ); + }; + return fetchPage(0).pipe( + expand((page) => (page.hasMoreItems ? fetchPage(page.skipCount) : EMPTY)), + reduce((allEntries, page) => [...allEntries, ...page.entries], [] as AspectEntry[]) + ); + } + + private categoriseNodeAspects(node: Node, allAspects: AspectEntry[]): void { + const allAspectIds = allAspects.map((aspect) => aspect.entry.id); + const visibleAspects = this.aspectListService.getVisibleAspects(); + const aspectNames = node.aspectNames ?? []; + this.nodeAspects = aspectNames.filter((aspect) => visibleAspects.includes(aspect) || allAspectIds.includes(aspect)); + this.nodeAspectStatus = [...this.nodeAspects]; + this.notDisplayedAspects = aspectNames.filter((aspect) => !visibleAspects.includes(aspect) && !allAspectIds.includes(aspect)); + this.valueChanged.emit([...this.nodeAspects, ...this.notDisplayedAspects]); + this.updateCounter.emit(this.nodeAspects.length); + } }