From e1199673258678c3acc3f3a20723f75dd31ecc39 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Mon, 29 Jun 2020 17:24:00 +0200 Subject: [PATCH 1/8] fix issue where beforeEach wouldn't wait for dropBitstream to end --- .../item-bitstreams.component.spec.ts | 18 +++++++++--------- src/app/shared/mocks/request.service.mock.ts | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts index 5aa085a42c..125fc1fb0c 100644 --- a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts @@ -191,15 +191,15 @@ describe('ItemBitstreamsComponent', () => { }); describe('when dropBitstream is called', () => { - const event = { - fromIndex: 0, - toIndex: 50, - // tslint:disable-next-line:no-empty - finish: () => {} - }; - - beforeEach(() => { - comp.dropBitstream(bundle, event); + beforeEach((done) => { + comp.dropBitstream(bundle, { + fromIndex: 0, + toIndex: 50, + // tslint:disable-next-line:no-empty + finish: () => { + done(); + } + }) }); it('should send out a patch for the move operation', () => { diff --git a/src/app/shared/mocks/request.service.mock.ts b/src/app/shared/mocks/request.service.mock.ts index 6a3f182868..385195bc77 100644 --- a/src/app/shared/mocks/request.service.mock.ts +++ b/src/app/shared/mocks/request.service.mock.ts @@ -11,7 +11,7 @@ export function getMockRequestService(requestEntry$: Observable = getByUUID: requestEntry$, uriEncodeBody: jasmine.createSpy('uriEncodeBody'), isCachedOrPending: false, - removeByHrefSubstring: jasmine.createSpy('removeByHrefSubstring').and.returnValue(observableOf(true)), + removeByHrefSubstring: observableOf(true), hasByHrefObservable: observableOf(false) }); } From 066e6cd142b69a1be5fe9b6509ec4f06ce3ebb81 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Mon, 29 Jun 2020 17:42:16 +0200 Subject: [PATCH 2/8] fixed a number of instances in community-list-service.spec.ts where async code was presumed synchronous --- .../community-list-service.spec.ts | 131 +++++++++++------- 1 file changed, 84 insertions(+), 47 deletions(-) diff --git a/src/app/community-list-page/community-list-service.spec.ts b/src/app/community-list-page/community-list-service.spec.ts index 6b7ab2bd77..f4955b2a36 100644 --- a/src/app/community-list-page/community-list-service.spec.ts +++ b/src/app/community-list-page/community-list-service.spec.ts @@ -210,13 +210,16 @@ describe('CommunityListService', () => { let flatNodeList; describe('None expanded: should return list containing only flatnodes of the test top communities page 1 and 2', () => { let findTopSpy; - beforeEach(() => { + beforeEach((done) => { findTopSpy = spyOn(communityDataServiceStub, 'findTop').and.callThrough(); service.getNextPageTopCommunities(); - const sub = service.loadCommunities(null) - .subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.loadCommunities(null) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('flatnode list should contain just flatnodes of top community list page 1 and 2', () => { expect(findTopSpy).toHaveBeenCalled(); @@ -236,10 +239,13 @@ describe('CommunityListService', () => { describe('should transform all communities in a list of flatnodes with possible subcoms and collections as subflatnodes if they\'re expanded', () => { let flatNodeList; describe('None expanded: should return list containing only flatnodes of the test top communities', () => { - beforeEach(() => { - const sub = service.loadCommunities(null) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + beforeEach((done) => { + service.loadCommunities(null) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as top community list', () => { expect(flatNodeList.length).toEqual(mockListOfTopCommunitiesPage1.length); @@ -256,7 +262,7 @@ describe('CommunityListService', () => { }); }); describe('All top expanded, all page 1: should return list containing flatnodes of the communities in the test list and all its possible page-limited children (subcommunities and collections)', () => { - beforeEach(() => { + beforeEach((done) => { const expandedNodes = []; mockListOfTopCommunitiesPage1.map((community: Community) => { const communityFlatNode = toFlatNode(community, observableOf(true), 0, true, null); @@ -264,9 +270,12 @@ describe('CommunityListService', () => { communityFlatNode.currentCommunityPage = 1; expandedNodes.push(communityFlatNode); }); - const sub = service.loadCommunities(expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.loadCommunities(expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as top community list and size of its possible page-limited children', () => { expect(flatNodeList.length).toEqual(mockListOfTopCommunitiesPage1.length + mockSubcommunities1Page1.length + mockSubcommunities1Page1.length); @@ -281,14 +290,17 @@ describe('CommunityListService', () => { }); }); describe('Just first top comm expanded, all page 1: should return list containing flatnodes of the communities in the test list and all its possible page-limited children (subcommunities and collections)', () => { - beforeEach(() => { + beforeEach((done) => { const communityFlatNode = toFlatNode(mockListOfTopCommunitiesPage1[0], observableOf(true), 0, true, null); communityFlatNode.currentCollectionPage = 1; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - const sub = service.loadCommunities(expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.loadCommunities(expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as top community list and size of page-limited children of first top community', () => { expect(flatNodeList.length).toEqual(mockListOfTopCommunitiesPage1.length + mockSubcommunities1Page1.length); @@ -300,14 +312,17 @@ describe('CommunityListService', () => { }); }); describe('Just second top comm expanded, collections at page 2: should return list containing flatnodes of the communities in the test list and all its possible page-limited children (subcommunities and collections)', () => { - beforeEach(() => { + beforeEach((done) => { const communityFlatNode = toFlatNode(mockListOfTopCommunitiesPage1[1], observableOf(true), 0, true, null); communityFlatNode.currentCollectionPage = 2; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - const sub = service.loadCommunities(expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.loadCommunities(expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as top community list and size of page-limited children of second top community', () => { expect(flatNodeList.length).toEqual(mockListOfTopCommunitiesPage1.length + mockCollectionsPage1.length + mockCollectionsPage2.length); @@ -333,10 +348,13 @@ describe('CommunityListService', () => { }); let flatNodeList; describe('None expanded: should return list containing only flatnodes of the communities in the test list', () => { - beforeEach(() => { - const sub = service.transformListOfCommunities(new PaginatedList(new PageInfo(), listOfCommunities), 0, null, null) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + beforeEach((done) => { + service.transformListOfCommunities(new PaginatedList(new PageInfo(), listOfCommunities), 0, null, null) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as community test list', () => { expect(flatNodeList.length).toEqual(listOfCommunities.length); @@ -353,7 +371,7 @@ describe('CommunityListService', () => { }); }); describe('All top expanded, all page 1: should return list containing flatnodes of the communities in the test list and all its possible page-limited children (subcommunities and collections)', () => { - beforeEach(() => { + beforeEach((done) => { const expandedNodes = []; listOfCommunities.map((community: Community) => { const communityFlatNode = toFlatNode(community, observableOf(true), 0, true, null); @@ -361,9 +379,12 @@ describe('CommunityListService', () => { communityFlatNode.currentCommunityPage = 1; expandedNodes.push(communityFlatNode); }); - const sub = service.transformListOfCommunities(new PaginatedList(new PageInfo(), listOfCommunities), 0, null, expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.transformListOfCommunities(new PaginatedList(new PageInfo(), listOfCommunities), 0, null, expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be as big as community test list and size of its possible children', () => { expect(flatNodeList.length).toEqual(listOfCommunities.length + mockSubcommunities1Page1.length + mockSubcommunities1Page1.length); @@ -397,10 +418,13 @@ describe('CommunityListService', () => { }); let flatNodeList; describe('should return list containing only flatnode corresponding to that community', () => { - beforeEach(() => { - const sub = service.transformCommunity(communityWithNoSubcomsOrColls, 0, null, null) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + beforeEach((done) => { + service.transformCommunity(communityWithNoSubcomsOrColls, 0, null, null) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('length of flatnode list should be 1', () => { expect(flatNodeList.length).toEqual(1); @@ -426,10 +450,14 @@ describe('CommunityListService', () => { }); let flatNodeList; describe('should return list containing only flatnode corresponding to that community', () => { - beforeAll(() => { - const sub = service.transformCommunity(communityWithSubcoms, 0, null, null) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + beforeAll((done) => { + service.transformCommunity(communityWithSubcoms, 0, null, null) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); + }); it('length of flatnode list should be 1', () => { expect(flatNodeList.length).toEqual(1); @@ -455,14 +483,17 @@ describe('CommunityListService', () => { } }); let flatNodeList; - beforeEach(() => { + beforeEach((done) => { const communityFlatNode = toFlatNode(communityWithSubcoms, observableOf(true), 0, true, null); communityFlatNode.currentCollectionPage = 1; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - const sub = service.transformCommunity(communityWithSubcoms, 0, null, expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.transformCommunity(communityWithSubcoms, 0, null, expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('list of flatnodes is length is 1 + nrOfSubcoms & first flatnode is of expanded test community', () => { expect(flatNodeList.length).toEqual(1 + mockSubcommunities1Page1.length); @@ -485,7 +516,7 @@ describe('CommunityListService', () => { describe('should return list containing flatnodes of that community, its collections of the first two pages', () => { let communityWithCollections; let flatNodeList; - beforeEach(() => { + beforeEach((done) => { communityWithCollections = Object.assign(new Community(), { id: '9076bd16-e69a-48d6-9e41-0238cb40d863', uuid: '9076bd16-e69a-48d6-9e41-0238cb40d863', @@ -500,9 +531,12 @@ describe('CommunityListService', () => { communityFlatNode.currentCollectionPage = 2; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - const sub = service.transformCommunity(communityWithCollections, 0, null, expandedNodes) - .pipe(take(1)).subscribe((value) => flatNodeList = value); - sub.unsubscribe(); + service.transformCommunity(communityWithCollections, 0, null, expandedNodes) + .pipe(take(1)) + .subscribe((value) => { + flatNodeList = value; + done(); + }); }); it('list of flatnodes is length is 1 + nrOfCollections & first flatnode is of expanded test community', () => { expect(flatNodeList.length).toEqual(1 + mockCollectionsPage1.length + mockCollectionsPage2.length); @@ -533,7 +567,7 @@ describe('CommunityListService', () => { describe('getIsExpandable', () => { describe('should return true', () => { - it('if community has subcommunities', () => { + it('if community has subcommunities', (done) => { const communityWithSubcoms = Object.assign(new Community(), { id: '7669c72a-3f2a-451f-a3b9-9210e7a4c02f', uuid: '7669c72a-3f2a-451f-a3b9-9210e7a4c02f', @@ -546,9 +580,10 @@ describe('CommunityListService', () => { }); service.getIsExpandable(communityWithSubcoms).pipe(take(1)).subscribe((result) => { expect(result).toEqual(true); + done(); }); }); - it('if community has collections', () => { + it('if community has collections', (done) => { const communityWithCollections = Object.assign(new Community(), { id: '9076bd16-e69a-48d6-9e41-0238cb40d863', uuid: '9076bd16-e69a-48d6-9e41-0238cb40d863', @@ -561,11 +596,12 @@ describe('CommunityListService', () => { }); service.getIsExpandable(communityWithCollections).pipe(take(1)).subscribe((result) => { expect(result).toEqual(true); + done(); }); }); }); describe('should return false', () => { - it('if community has neither subcommunities nor collections', () => { + it('if community has neither subcommunities nor collections', (done) => { const communityWithNoSubcomsOrColls = Object.assign(new Community(), { id: 'efbf25e1-2d8c-4c28-8f3e-2e04c215be24', uuid: 'efbf25e1-2d8c-4c28-8f3e-2e04c215be24', @@ -578,6 +614,7 @@ describe('CommunityListService', () => { }); service.getIsExpandable(communityWithNoSubcomsOrColls).pipe(take(1)).subscribe((result) => { expect(result).toEqual(false); + done(); }); }); }); From b1e44a7fa583da100dc3609dd40be5b1d9e2b5da Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Sun, 3 May 2020 01:53:46 +0200 Subject: [PATCH 3/8] 70599: Improvements comcol tree --- .../community-list-datasource.ts | 5 +- .../community-list-service.ts | 119 +++++++----------- .../community-list.component.ts | 18 ++- .../builders/remote-data-build.service.ts | 7 +- 4 files changed, 60 insertions(+), 89 deletions(-) diff --git a/src/app/community-list-page/community-list-datasource.ts b/src/app/community-list-page/community-list-datasource.ts index 3a9d9f2077..4974b2c4fe 100644 --- a/src/app/community-list-page/community-list-datasource.ts +++ b/src/app/community-list-page/community-list-datasource.ts @@ -1,3 +1,4 @@ +import { FindListOptions } from '../core/data/request.models'; import { CommunityListService, FlatNode } from './community-list-service'; import { CollectionViewer, DataSource } from '@angular/cdk/typings/collections'; import { BehaviorSubject, Observable, } from 'rxjs'; @@ -21,10 +22,10 @@ export class CommunityListDatasource implements DataSource { return this.communityList$.asObservable(); } - loadCommunities(expandedNodes: FlatNode[]) { + loadCommunities(findOptions: FindListOptions, expandedNodes: FlatNode[]) { this.loading$.next(true); - this.communityListService.loadCommunities(expandedNodes).pipe( + this.communityListService.loadCommunities(findOptions, expandedNodes).pipe( take(1), finalize(() => this.loading$.next(false)), ).subscribe((flatNodes: FlatNode[]) => { diff --git a/src/app/community-list-page/community-list-service.ts b/src/app/community-list-page/community-list-service.ts index be04887e71..70db048d3e 100644 --- a/src/app/community-list-page/community-list-service.ts +++ b/src/app/community-list-page/community-list-service.ts @@ -4,11 +4,12 @@ import { combineLatest as observableCombineLatest } from 'rxjs/internal/observab import { Observable, of as observableOf } from 'rxjs'; import { AppState } from '../app.reducer'; import { CommunityDataService } from '../core/data/community-data.service'; -import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model'; -import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; -import { catchError, filter, map, switchMap, take } from 'rxjs/operators'; +import { FindListOptions } from '../core/data/request.models'; +import { map, flatMap } from 'rxjs/operators'; import { Community } from '../core/shared/community.model'; import { Collection } from '../core/shared/collection.model'; +import { getSucceededRemoteData } from '../core/shared/operators'; +import { PageInfo } from '../core/shared/page-info.model'; import { hasValue, isNotEmpty } from '../shared/empty.util'; import { RemoteData } from '../core/data/remote-data'; import { PaginatedList } from '../core/data/paginated-list'; @@ -46,8 +47,7 @@ export class ShowMoreFlatNode { // Helper method to combine an flatten an array of observables of flatNode arrays export const combineAndFlatten = (obsList: Array>): Observable => observableCombineLatest(...obsList).pipe( - map((matrix: FlatNode[][]) => - matrix.reduce((combinedList, currentList: FlatNode[]) => [...combinedList, ...currentList])) + map((matrix: any[][]) => [].concat(...matrix)) ); /** @@ -99,6 +99,8 @@ const communityListStateSelector = (state: AppState) => state.communityList; const expandedNodesSelector = createSelector(communityListStateSelector, (communityList: CommunityListState) => communityList.expandedNodes); const loadingNodeSelector = createSelector(communityListStateSelector, (communityList: CommunityListState) => communityList.loadingNode); +export const MAX_COMCOLS_PER_PAGE = 2; + /** * Service class for the community list, responsible for the creating of the flat list used by communityList dataSource * and connection to the store to retrieve and save the state of the community list @@ -107,26 +109,8 @@ const loadingNodeSelector = createSelector(communityListStateSelector, (communit @Injectable() export class CommunityListService { - // page-limited list of top-level communities - payloads$: Array>>; - - topCommunitiesConfig: PaginationComponentOptions; - topCommunitiesSortConfig: SortOptions; - - maxSubCommunitiesPerPage: number; - maxCollectionsPerPage: number; - constructor(private communityDataService: CommunityDataService, private collectionDataService: CollectionDataService, private store: Store) { - this.topCommunitiesConfig = new PaginationComponentOptions(); - this.topCommunitiesConfig.id = 'top-level-pagination'; - this.topCommunitiesConfig.pageSize = 10; - this.topCommunitiesConfig.currentPage = 1; - this.topCommunitiesSortConfig = new SortOptions('dc.title', SortDirection.ASC); - this.initTopCommunityList(); - - this.maxSubCommunitiesPerPage = 3; - this.maxCollectionsPerPage = 3; } saveCommunityListStateToStore(expandedNodes: FlatNode[], loadingNode: FlatNode): void { @@ -141,57 +125,46 @@ export class CommunityListService { return this.store.select(loadingNodeSelector); } - /** - * Increases the payload so it contains the next page of top level communities - */ - getNextPageTopCommunities(): void { - this.topCommunitiesConfig.currentPage = this.topCommunitiesConfig.currentPage + 1; - this.payloads$ = [...this.payloads$, this.communityDataService.findTop({ - currentPage: this.topCommunitiesConfig.currentPage, - elementsPerPage: this.topCommunitiesConfig.pageSize, - sort: { - field: this.topCommunitiesSortConfig.field, - direction: this.topCommunitiesSortConfig.direction - } - }).pipe( - take(1), - map((results) => results.payload), - )]; - } - /** * Gets all top communities, limited by page, and transforms this in a list of flatNodes. * @param expandedNodes List of expanded nodes; if a node is not expanded its subCommunities and collections need * not be added to the list */ - loadCommunities(expandedNodes: FlatNode[]): Observable { - const res = this.payloads$.map((payload) => { - return payload.pipe( - take(1), - switchMap((result: PaginatedList) => { - return this.transformListOfCommunities(result, 0, null, expandedNodes); - }), - catchError(() => observableOf([])), - ); - }); - return combineAndFlatten(res); + loadCommunities(findOptions: FindListOptions, expandedNodes: FlatNode[]): Observable { + const currentPage = findOptions.currentPage; + const topCommunities = []; + for (let i = 1; i <= currentPage; i++) { + const pagination: FindListOptions = Object.assign({}, findOptions, { currentPage: i }); + topCommunities.push(this.getTopCommunities(pagination)); + } + const topComs$ = observableCombineLatest(...topCommunities).pipe( + map((coms: Array>) => { + const newPages: Community[][] = coms.map((unit: PaginatedList) => unit.page); + const newPage: Community[] = [].concat(...newPages); + let newPageInfo = new PageInfo(); + if (coms && coms.length > 0) { + newPageInfo = Object.assign({}, coms[0].pageInfo, { currentPage }) + } + return new PaginatedList(newPageInfo, newPage); + }) + ); + return topComs$.pipe(flatMap((topComs: PaginatedList) => this.transformListOfCommunities(topComs, 0, null, expandedNodes))); }; /** * Puts the initial top level communities in a list to be called upon */ - private initTopCommunityList(): void { - this.payloads$ = [this.communityDataService.findTop({ - currentPage: this.topCommunitiesConfig.currentPage, - elementsPerPage: this.topCommunitiesConfig.pageSize, + private getTopCommunities(options: FindListOptions): Observable> { + return this.communityDataService.findTop({ + currentPage: options.currentPage, + elementsPerPage: MAX_COMCOLS_PER_PAGE, sort: { - field: this.topCommunitiesSortConfig.field, - direction: this.topCommunitiesSortConfig.direction + field: options.sort.field, + direction: options.sort.direction } }).pipe( - take(1), map((results) => results.payload), - )]; + ); } /** @@ -206,16 +179,15 @@ export class CommunityListService { parent: FlatNode, expandedNodes: FlatNode[]): Observable { if (isNotEmpty(listOfPaginatedCommunities.page)) { - let currentPage = this.topCommunitiesConfig.currentPage; + let currentPage = listOfPaginatedCommunities.currentPage; if (isNotEmpty(parent)) { currentPage = expandedNodes.find((node: FlatNode) => node.id === parent.id).currentCommunityPage; } - const isNotAllCommunities = (listOfPaginatedCommunities.totalElements > (listOfPaginatedCommunities.elementsPerPage * currentPage)); let obsList = listOfPaginatedCommunities.page .map((community: Community) => { return this.transformCommunity(community, level, parent, expandedNodes) }); - if (isNotAllCommunities && listOfPaginatedCommunities.currentPage > currentPage) { + if (currentPage < listOfPaginatedCommunities.totalPages && currentPage === listOfPaginatedCommunities.currentPage) { obsList = [...obsList, observableOf([showMoreFlatNode('community', level, parent)])]; } @@ -252,13 +224,12 @@ export class CommunityListService { let subcoms = []; for (let i = 1; i <= currentCommunityPage; i++) { const nextSetOfSubcommunitiesPage = this.communityDataService.findByParent(community.uuid, { - elementsPerPage: this.maxSubCommunitiesPerPage, + elementsPerPage: MAX_COMCOLS_PER_PAGE, currentPage: i }) .pipe( - filter((rd: RemoteData>) => rd.hasSucceeded), - take(1), - switchMap((rd: RemoteData>) => + getSucceededRemoteData(), + flatMap((rd: RemoteData>) => this.transformListOfCommunities(rd.payload, level + 1, communityFlatNode, expandedNodes)) ); @@ -271,16 +242,15 @@ export class CommunityListService { let collections = []; for (let i = 1; i <= currentCollectionPage; i++) { const nextSetOfCollectionsPage = this.collectionDataService.findByParent(community.uuid, { - elementsPerPage: this.maxCollectionsPerPage, + elementsPerPage: MAX_COMCOLS_PER_PAGE, currentPage: i }) .pipe( - filter((rd: RemoteData>) => rd.hasSucceeded), - take(1), + getSucceededRemoteData(), map((rd: RemoteData>) => { let nodes = rd.payload.page .map((collection: Collection) => toFlatNode(collection, observableOf(false), level + 1, false, communityFlatNode)); - if ((rd.payload.elementsPerPage * currentCollectionPage) < rd.payload.totalElements && rd.payload.currentPage > currentCollectionPage) { + if (currentCollectionPage < rd.payload.totalPages && currentCollectionPage === rd.payload.currentPage) { nodes = [...nodes, showMoreFlatNode('collection', level + 1, communityFlatNode)]; } return nodes; @@ -305,21 +275,18 @@ export class CommunityListService { let hasColls$: Observable; hasSubcoms$ = this.communityDataService.findByParent(community.uuid, { elementsPerPage: 1 }) .pipe( - filter((rd: RemoteData>) => rd.hasSucceeded), - take(1), + getSucceededRemoteData(), map((results) => results.payload.totalElements > 0), ); hasColls$ = this.collectionDataService.findByParent(community.uuid, { elementsPerPage: 1 }) .pipe( - filter((rd: RemoteData>) => rd.hasSucceeded), - take(1), + getSucceededRemoteData(), map((results) => results.payload.totalElements > 0), ); let hasChildren$: Observable; hasChildren$ = observableCombineLatest(hasSubcoms$, hasColls$).pipe( - take(1), map(([hasSubcoms, hasColls]: [boolean, boolean]) => { if (hasSubcoms || hasColls) { return true; diff --git a/src/app/community-list-page/community-list/community-list.component.ts b/src/app/community-list-page/community-list/community-list.component.ts index ddcd49cd1c..f672eae151 100644 --- a/src/app/community-list-page/community-list/community-list.component.ts +++ b/src/app/community-list-page/community-list/community-list.component.ts @@ -1,5 +1,7 @@ import { Component, OnDestroy, OnInit } from '@angular/core'; import { take } from 'rxjs/operators'; +import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model'; +import { FindListOptions } from '../../core/data/request.models'; import { CommunityListService, FlatNode } from '../community-list-service'; import { CommunityListDatasource } from '../community-list-datasource'; import { FlatTreeControl } from '@angular/cdk/tree'; @@ -27,7 +29,13 @@ export class CommunityListComponent implements OnInit, OnDestroy { dataSource: CommunityListDatasource; + paginationConfig: FindListOptions; + constructor(private communityListService: CommunityListService) { + this.paginationConfig = new FindListOptions(); + this.paginationConfig.elementsPerPage = 2; + this.paginationConfig.currentPage = 1; + this.paginationConfig.sort = new SortOptions('dc.title', SortDirection.ASC); } ngOnInit() { @@ -37,7 +45,7 @@ export class CommunityListComponent implements OnInit, OnDestroy { }); this.communityListService.getExpandedNodesFromStore().pipe(take(1)).subscribe((result) => { this.expandedNodes = [...result]; - this.dataSource.loadCommunities(this.expandedNodes); + this.dataSource.loadCommunities(this.paginationConfig, this.expandedNodes); }); } @@ -74,7 +82,7 @@ export class CommunityListComponent implements OnInit, OnDestroy { node.currentCommunityPage = 1; } } - this.dataSource.loadCommunities(this.expandedNodes); + this.dataSource.loadCommunities(this.paginationConfig, this.expandedNodes); } /** @@ -94,10 +102,10 @@ export class CommunityListComponent implements OnInit, OnDestroy { const parentNodeInExpandedNodes = this.expandedNodes.find((node2: FlatNode) => node.parent.id === node2.id); parentNodeInExpandedNodes.currentCommunityPage++; } - this.dataSource.loadCommunities(this.expandedNodes); + this.dataSource.loadCommunities(this.paginationConfig, this.expandedNodes); } else { - this.communityListService.getNextPageTopCommunities(); - this.dataSource.loadCommunities(this.expandedNodes); + this.paginationConfig.currentPage++; + this.dataSource.loadCommunities(this.paginationConfig, this.expandedNodes); } } diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index 88d1890de2..6c9f40888f 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -150,12 +150,7 @@ export class RemoteDataBuildService { filterSuccessfulResponses(), map((response: DSOSuccessResponse) => { if (hasValue((response as DSOSuccessResponse).pageInfo)) { - const resPageInfo = (response as DSOSuccessResponse).pageInfo; - if (isNotEmpty(resPageInfo) && resPageInfo.currentPage >= 0) { - return Object.assign({}, resPageInfo, { currentPage: resPageInfo.currentPage + 1 }); - } else { - return resPageInfo; - } + return (response as DSOSuccessResponse).pageInfo; } }) ); From 1dfe7ec170137ca806000425fe82ec2332df7e78 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Mon, 4 May 2020 10:47:19 +0200 Subject: [PATCH 4/8] 70599: Test fixes comcol tree after changes --- .../community-list-service.spec.ts | 42 ++++++++++++------- .../community-list.component.spec.ts | 20 ++++----- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/app/community-list-page/community-list-service.spec.ts b/src/app/community-list-page/community-list-service.spec.ts index f4955b2a36..accd0f23a5 100644 --- a/src/app/community-list-page/community-list-service.spec.ts +++ b/src/app/community-list-page/community-list-service.spec.ts @@ -1,21 +1,19 @@ -import { of as observableOf } from 'rxjs'; -import { TestBed, inject, async } from '@angular/core/testing'; +import { inject, TestBed } from '@angular/core/testing'; import { Store } from '@ngrx/store'; +import { of as observableOf } from 'rxjs'; +import { take } from 'rxjs/operators'; import { AppState } from '../app.reducer'; +import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; +import { PaginatedList } from '../core/data/paginated-list'; +import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { StoreMock } from '../shared/testing/store.mock'; import { CommunityListService, FlatNode, toFlatNode } from './community-list-service'; import { CollectionDataService } from '../core/data/collection-data.service'; -import { PaginatedList } from '../core/data/paginated-list'; -import { PageInfo } from '../core/shared/page-info.model'; import { CommunityDataService } from '../core/data/community-data.service'; -import { - createFailedRemoteDataObject$, - createSuccessfulRemoteDataObject$ -} from '../shared/remote-data.utils'; import { Community } from '../core/shared/community.model'; import { Collection } from '../core/shared/collection.model'; -import { take } from 'rxjs/operators'; import { FindListOptions } from '../core/data/request.models'; +import { PageInfo } from '../core/shared/page-info.model'; describe('CommunityListService', () => { let store: StoreMock; @@ -212,9 +210,11 @@ describe('CommunityListService', () => { let findTopSpy; beforeEach((done) => { findTopSpy = spyOn(communityDataServiceStub, 'findTop').and.callThrough(); - service.getNextPageTopCommunities(); - service.loadCommunities(null) + service.loadCommunities({ + currentPage: 2, + sort: new SortOptions('dc.title', SortDirection.ASC) + }, null) .pipe(take(1)) .subscribe((value) => { flatNodeList = value; @@ -240,7 +240,10 @@ describe('CommunityListService', () => { let flatNodeList; describe('None expanded: should return list containing only flatnodes of the test top communities', () => { beforeEach((done) => { - service.loadCommunities(null) + service.loadCommunities({ + currentPage: 1, + sort: new SortOptions('dc.title', SortDirection.ASC) + }, null) .pipe(take(1)) .subscribe((value) => { flatNodeList = value; @@ -270,7 +273,10 @@ describe('CommunityListService', () => { communityFlatNode.currentCommunityPage = 1; expandedNodes.push(communityFlatNode); }); - service.loadCommunities(expandedNodes) + service.loadCommunities({ + currentPage: 1, + sort: new SortOptions('dc.title', SortDirection.ASC) + }, expandedNodes) .pipe(take(1)) .subscribe((value) => { flatNodeList = value; @@ -295,7 +301,10 @@ describe('CommunityListService', () => { communityFlatNode.currentCollectionPage = 1; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - service.loadCommunities(expandedNodes) + service.loadCommunities({ + currentPage: 1, + sort: new SortOptions('dc.title', SortDirection.ASC) + }, expandedNodes) .pipe(take(1)) .subscribe((value) => { flatNodeList = value; @@ -317,7 +326,10 @@ describe('CommunityListService', () => { communityFlatNode.currentCollectionPage = 2; communityFlatNode.currentCommunityPage = 1; const expandedNodes = [communityFlatNode]; - service.loadCommunities(expandedNodes) + service.loadCommunities({ + currentPage: 1, + sort: new SortOptions('dc.title', SortDirection.ASC) + }, expandedNodes) .pipe(take(1)) .subscribe((value) => { flatNodeList = value; diff --git a/src/app/community-list-page/community-list/community-list.component.spec.ts b/src/app/community-list-page/community-list/community-list.component.spec.ts index a91c5fa057..f4c5f959d1 100644 --- a/src/app/community-list-page/community-list/community-list.component.spec.ts +++ b/src/app/community-list-page/community-list/community-list.component.spec.ts @@ -114,15 +114,9 @@ describe('CommunityListComponent', () => { beforeEach(async(() => { communityListServiceStub = { - topPageSize: 2, - topCurrentPage: 1, - collectionPageSize: 2, - subcommunityPageSize: 2, + pageSize: 2, expandedNodes: [], loadingNode: null, - getNextPageTopCommunities() { - this.topCurrentPage++; - }, getLoadingNodeFromStore() { return observableOf(this.loadingNode); }, @@ -133,12 +127,12 @@ describe('CommunityListComponent', () => { this.expandedNodes = expandedNodes; this.loadingNode = loadingNode; }, - loadCommunities(expandedNodes) { + loadCommunities(options, expandedNodes) { let flatnodes; let showMoreTopComNode = false; flatnodes = [...mockTopFlatnodesUnexpanded]; - const currentPage = this.topCurrentPage; - const elementsPerPage = this.topPageSize; + const currentPage = options.currentPage; + const elementsPerPage = this.pageSize; let endPageIndex = (currentPage * elementsPerPage); if (endPageIndex >= flatnodes.length) { endPageIndex = flatnodes.length; @@ -171,14 +165,14 @@ describe('CommunityListComponent', () => { collFlatnodes = [...collFlatnodes, toFlatNode(coll, observableOf(false), topNode.level + 1, false, topNode)]; }); if (isNotEmpty(subComFlatnodes)) { - const endSubComIndex = this.subcommunityPageSize * expandedParent.currentCommunityPage; + const endSubComIndex = this.pageSize * expandedParent.currentCommunityPage; flatnodes = [...flatnodes, ...subComFlatnodes.slice(0, endSubComIndex)]; if (subComFlatnodes.length > endSubComIndex) { flatnodes = [...flatnodes, showMoreFlatNode('community', topNode.level + 1, expandedParent)]; } } if (isNotEmpty(collFlatnodes)) { - const endColIndex = this.collectionPageSize * expandedParent.currentCollectionPage; + const endColIndex = this.pageSize * expandedParent.currentCollectionPage; flatnodes = [...flatnodes, ...collFlatnodes.slice(0, endColIndex)]; if (collFlatnodes.length > endColIndex) { flatnodes = [...flatnodes, showMoreFlatNode('collection', topNode.level + 1, expandedParent)]; @@ -225,6 +219,8 @@ describe('CommunityListComponent', () => { it('should render a cdk tree with the first elementsPerPage (2) nr of top level communities, unexpanded', () => { const expandableNodesFound = fixture.debugElement.queryAll(By.css('.expandable-node a')); const childlessNodesFound = fixture.debugElement.queryAll(By.css('.childless-node a')); + console.log('expandableNodesFound', expandableNodesFound) + console.log('childlessNodesFound', childlessNodesFound) const allNodes = [...expandableNodesFound, ...childlessNodesFound]; expect(allNodes.length).toEqual(2); mockTopFlatnodesUnexpanded.slice(0, 2).map((topFlatnode: FlatNode) => { From 53779cf69e4df943b756f044cf2f74d4f6de1f7a Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Thu, 11 Jun 2020 11:52:58 +0200 Subject: [PATCH 5/8] remove console logs --- .../community-list/community-list.component.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app/community-list-page/community-list/community-list.component.spec.ts b/src/app/community-list-page/community-list/community-list.component.spec.ts index f4c5f959d1..ef9e89ff1b 100644 --- a/src/app/community-list-page/community-list/community-list.component.spec.ts +++ b/src/app/community-list-page/community-list/community-list.component.spec.ts @@ -219,8 +219,6 @@ describe('CommunityListComponent', () => { it('should render a cdk tree with the first elementsPerPage (2) nr of top level communities, unexpanded', () => { const expandableNodesFound = fixture.debugElement.queryAll(By.css('.expandable-node a')); const childlessNodesFound = fixture.debugElement.queryAll(By.css('.childless-node a')); - console.log('expandableNodesFound', expandableNodesFound) - console.log('childlessNodesFound', childlessNodesFound) const allNodes = [...expandableNodesFound, ...childlessNodesFound]; expect(allNodes.length).toEqual(2); mockTopFlatnodesUnexpanded.slice(0, 2).map((topFlatnode: FlatNode) => { From 94e3f2d5e0f5618342afc2f562b7d3cfabe80a53 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Fri, 19 Jun 2020 13:17:46 +0200 Subject: [PATCH 6/8] Feedback processed: - pagesize comcol changed to 50 & - loadCommunities inside ngZone.runOutsideAngular - Chevron size small unexpanded and large expanded when logged in fixed --- ...pandable-admin-sidebar-section.component.scss | 4 ++-- .../community-list-datasource.ts | 16 ++++++++++------ .../community-list-service.ts | 2 +- .../community-list/community-list.component.ts | 7 ++++--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/app/+admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.scss b/src/app/+admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.scss index 37fe15bd40..1f6e288608 100644 --- a/src/app/+admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.scss +++ b/src/app/+admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.scss @@ -1,4 +1,4 @@ -::ng-deep { +:host ::ng-deep { .fa-chevron-right { padding-left: $spacer/2; font-size: 0.5rem; @@ -16,4 +16,4 @@ display: flex; flex-direction: column; } -} \ No newline at end of file +} diff --git a/src/app/community-list-page/community-list-datasource.ts b/src/app/community-list-page/community-list-datasource.ts index 4974b2c4fe..8cd0c6b0af 100644 --- a/src/app/community-list-page/community-list-datasource.ts +++ b/src/app/community-list-page/community-list-datasource.ts @@ -1,3 +1,4 @@ +import { NgZone } from '@angular/core'; import { FindListOptions } from '../core/data/request.models'; import { CommunityListService, FlatNode } from './community-list-service'; import { CollectionViewer, DataSource } from '@angular/cdk/typings/collections'; @@ -15,7 +16,8 @@ export class CommunityListDatasource implements DataSource { private communityList$ = new BehaviorSubject([]); public loading$ = new BehaviorSubject(false); - constructor(private communityListService: CommunityListService) { + constructor(private communityListService: CommunityListService, + private zone: NgZone) { } connect(collectionViewer: CollectionViewer): Observable { @@ -25,11 +27,13 @@ export class CommunityListDatasource implements DataSource { loadCommunities(findOptions: FindListOptions, expandedNodes: FlatNode[]) { this.loading$.next(true); - this.communityListService.loadCommunities(findOptions, expandedNodes).pipe( - take(1), - finalize(() => this.loading$.next(false)), - ).subscribe((flatNodes: FlatNode[]) => { - this.communityList$.next(flatNodes); + this.zone.runOutsideAngular(() => { + this.communityListService.loadCommunities(findOptions, expandedNodes).pipe( + take(1), + finalize(() => this.loading$.next(false)), + ).subscribe((flatNodes: FlatNode[]) => { + this.communityList$.next(flatNodes); + }); }); } diff --git a/src/app/community-list-page/community-list-service.ts b/src/app/community-list-page/community-list-service.ts index 70db048d3e..a5c3506e3d 100644 --- a/src/app/community-list-page/community-list-service.ts +++ b/src/app/community-list-page/community-list-service.ts @@ -99,7 +99,7 @@ const communityListStateSelector = (state: AppState) => state.communityList; const expandedNodesSelector = createSelector(communityListStateSelector, (communityList: CommunityListState) => communityList.expandedNodes); const loadingNodeSelector = createSelector(communityListStateSelector, (communityList: CommunityListState) => communityList.loadingNode); -export const MAX_COMCOLS_PER_PAGE = 2; +export const MAX_COMCOLS_PER_PAGE = 50; /** * Service class for the community list, responsible for the creating of the flat list used by communityList dataSource diff --git a/src/app/community-list-page/community-list/community-list.component.ts b/src/app/community-list-page/community-list/community-list.component.ts index f672eae151..be96ff1a0a 100644 --- a/src/app/community-list-page/community-list/community-list.component.ts +++ b/src/app/community-list-page/community-list/community-list.component.ts @@ -1,4 +1,4 @@ -import { Component, OnDestroy, OnInit } from '@angular/core'; +import { Component, NgZone, OnDestroy, OnInit } from '@angular/core'; import { take } from 'rxjs/operators'; import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model'; import { FindListOptions } from '../../core/data/request.models'; @@ -31,7 +31,8 @@ export class CommunityListComponent implements OnInit, OnDestroy { paginationConfig: FindListOptions; - constructor(private communityListService: CommunityListService) { + constructor(private communityListService: CommunityListService, + private zone: NgZone) { this.paginationConfig = new FindListOptions(); this.paginationConfig.elementsPerPage = 2; this.paginationConfig.currentPage = 1; @@ -39,7 +40,7 @@ export class CommunityListComponent implements OnInit, OnDestroy { } ngOnInit() { - this.dataSource = new CommunityListDatasource(this.communityListService); + this.dataSource = new CommunityListDatasource(this.communityListService, this.zone); this.communityListService.getLoadingNodeFromStore().pipe(take(1)).subscribe((result) => { this.loadingNode = result; }); From 6433e211a8bdcd1c0ad3769fa192eb4ed446b6ac Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Fri, 19 Jun 2020 13:59:21 +0200 Subject: [PATCH 7/8] Feedback processed: - change so state change behaviourSubjects back in NgZone.run --- src/app/community-list-page/community-list-datasource.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/community-list-page/community-list-datasource.ts b/src/app/community-list-page/community-list-datasource.ts index 8cd0c6b0af..4ffb16759d 100644 --- a/src/app/community-list-page/community-list-datasource.ts +++ b/src/app/community-list-page/community-list-datasource.ts @@ -25,14 +25,13 @@ export class CommunityListDatasource implements DataSource { } loadCommunities(findOptions: FindListOptions, expandedNodes: FlatNode[]) { - this.loading$.next(true); - this.zone.runOutsideAngular(() => { + this.loading$.next(true); this.communityListService.loadCommunities(findOptions, expandedNodes).pipe( take(1), - finalize(() => this.loading$.next(false)), + finalize(() => this.zone.run(() => this.loading$.next(false))), ).subscribe((flatNodes: FlatNode[]) => { - this.communityList$.next(flatNodes); + this.zone.run(() => this.communityList$.next(flatNodes)); }); }); } From f992fe1afd57b5019fe39f82fb42379494ddb42b Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Tue, 30 Jun 2020 10:48:50 +0200 Subject: [PATCH 8/8] Feedback: - set initial loading moved outside of of NgZone --- src/app/community-list-page/community-list-datasource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/community-list-page/community-list-datasource.ts b/src/app/community-list-page/community-list-datasource.ts index 4ffb16759d..b77cbb5246 100644 --- a/src/app/community-list-page/community-list-datasource.ts +++ b/src/app/community-list-page/community-list-datasource.ts @@ -25,8 +25,8 @@ export class CommunityListDatasource implements DataSource { } loadCommunities(findOptions: FindListOptions, expandedNodes: FlatNode[]) { + this.loading$.next(true); this.zone.runOutsideAngular(() => { - this.loading$.next(true); this.communityListService.loadCommunities(findOptions, expandedNodes).pipe( take(1), finalize(() => this.zone.run(() => this.loading$.next(false))),