From 45e8977db7d34d7b0684c16f92e9e917f6ac4579 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Wed, 15 Nov 2023 23:31:54 +0100 Subject: [PATCH 1/4] Fixed pagination issues on item mapper --- .../collection-item-mapper.component.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts b/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts index e0e4aaf930..776b82f9b4 100644 --- a/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts +++ b/src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts @@ -144,7 +144,9 @@ export class CollectionItemMapperComponent implements OnInit { this.shouldUpdate$.next(false); } return this.itemDataService.findListByHref(collectionRD.payload._links.mappedItems.href, Object.assign(options, { - sort: this.defaultSortOptions + currentPage: options.pagination.currentPage, + elementsPerPage: options.pagination.pageSize, + sort: this.defaultSortOptions, }),!shouldUpdate, false, followLink('owningCollection')).pipe( getAllSucceededRemoteData() ); From da31c4f253ecc504e65ebc3ebe18b4ce2a0960a2 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 18 Apr 2024 19:27:26 +0200 Subject: [PATCH 2/4] 108555: Removed pageInfoState Input since it wasn't used and almost all the components gave the incorrect type of data to it --- .../browse/bulk-access-browse.component.html | 1 - .../epeople-registry.component.html | 1 - .../eperson-form/eperson-form.component.html | 1 - .../members-list/members-list.component.html | 2 - .../subgroups-list.component.html | 2 - .../groups-registry.component.html | 1 - .../bitstream-formats.component.html | 1 - .../metadata-schema.component.html | 1 - ...rag-and-drop-bitstream-list.component.html | 1 - .../edit-relationship-list.component.html | 1 - .../full-file-section.component.html | 2 - .../versions/item-versions.component.html | 1 - .../overview/process-overview.component.html | 1 - .../object-detail.component.html | 1 - .../object-grid/object-grid.component.html | 1 - .../object-list/object-list.component.html | 1 - .../collection-select.component.html | 1 - .../item-select/item-select.component.html | 1 - .../shared/pagination/pagination.component.ts | 45 +++---------------- 19 files changed, 6 insertions(+), 60 deletions(-) diff --git a/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html b/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html index c716aedb8b..df43878192 100644 --- a/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html +++ b/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html @@ -36,7 +36,6 @@ diff --git a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html index 228449a8a5..9297edb016 100644 --- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html +++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html @@ -47,7 +47,6 @@ @@ -103,7 +102,6 @@ diff --git a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html index d009f0283e..a4b5f4c680 100644 --- a/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html +++ b/src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html @@ -35,7 +35,6 @@ @@ -94,7 +93,6 @@ diff --git a/src/app/access-control/group-registry/groups-registry.component.html b/src/app/access-control/group-registry/groups-registry.component.html index 828aadc95a..401b50fe9d 100644 --- a/src/app/access-control/group-registry/groups-registry.component.html +++ b/src/app/access-control/group-registry/groups-registry.component.html @@ -37,7 +37,6 @@ diff --git a/src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.html b/src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.html index 0a2e9f0f92..0dae271dbb 100644 --- a/src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.html +++ b/src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.html @@ -11,7 +11,6 @@ diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.html b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.html index 557741df80..35fffec5fc 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.html +++ b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.html @@ -16,7 +16,6 @@ diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html b/src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html index f54aa73597..a31ab95348 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html @@ -3,7 +3,6 @@ [hidePagerWhenSinglePage]="true" [hidePaginationDetail]="true" [paginationOptions]="options" - [pageInfoState]="(objectsRD$ | async)?.payload" [collectionSize]="(objectsRD$ | async)?.payload?.totalElements">
diff --git a/src/app/item-page/full/field-components/file-section/full-file-section.component.html b/src/app/item-page/full/field-components/file-section/full-file-section.component.html index 8b8603011d..88fc68f876 100644 --- a/src/app/item-page/full/field-components/file-section/full-file-section.component.html +++ b/src/app/item-page/full/field-components/file-section/full-file-section.component.html @@ -6,7 +6,6 @@ [hideGear]="true" [hidePagerWhenSinglePage]="true" [paginationOptions]="originalOptions" - [pageInfoState]="originals" [collectionSize]="originals?.totalElements" [retainScrollPosition]="true"> @@ -49,7 +48,6 @@ [hideGear]="true" [hidePagerWhenSinglePage]="true" [paginationOptions]="licenseOptions" - [pageInfoState]="licenses" [collectionSize]="licenses?.totalElements" [retainScrollPosition]="true"> diff --git a/src/app/item-page/versions/item-versions.component.html b/src/app/item-page/versions/item-versions.component.html index 176c1f0cde..f9e354a498 100644 --- a/src/app/item-page/versions/item-versions.component.html +++ b/src/app/item-page/versions/item-versions.component.html @@ -10,7 +10,6 @@ [hideGear]="true" [hidePagerWhenSinglePage]="true" [paginationOptions]="options" - [pageInfoState]="versions" [collectionSize]="versions?.totalElements" [retainScrollPosition]="true"> diff --git a/src/app/process-page/overview/process-overview.component.html b/src/app/process-page/overview/process-overview.component.html index 5bf5fee79f..e17ac51892 100644 --- a/src/app/process-page/overview/process-overview.component.html +++ b/src/app/process-page/overview/process-overview.component.html @@ -17,7 +17,6 @@ diff --git a/src/app/shared/object-detail/object-detail.component.html b/src/app/shared/object-detail/object-detail.component.html index d077e2fd2b..98034ca621 100644 --- a/src/app/shared/object-detail/object-detail.component.html +++ b/src/app/shared/object-detail/object-detail.component.html @@ -1,6 +1,5 @@ diff --git a/src/app/shared/object-select/item-select/item-select.component.html b/src/app/shared/object-select/item-select/item-select.component.html index 7f8ff943a3..110434969a 100644 --- a/src/app/shared/object-select/item-select/item-select.component.html +++ b/src/app/shared/object-select/item-select/item-select.component.html @@ -3,7 +3,6 @@ *ngIf="itemsRD?.payload?.totalElements > 0" [paginationOptions]="paginationOptions" [sortOptions]="sortOptions" - [pageInfoState]="itemsRD?.payload" [collectionSize]="itemsRD?.payload?.totalElements" [hidePagerWhenSinglePage]="true" [hideGear]="true"> diff --git a/src/app/shared/pagination/pagination.component.ts b/src/app/shared/pagination/pagination.component.ts index 6da813cbc7..7aa55458b0 100644 --- a/src/app/shared/pagination/pagination.component.ts +++ b/src/app/shared/pagination/pagination.component.ts @@ -13,11 +13,9 @@ import { import { Observable, of as observableOf, Subscription } from 'rxjs'; import { HostWindowService } from '../host-window.service'; -import { HostWindowState } from '../search/host-window.reducer'; import { PaginationComponentOptions } from './pagination-component-options.model'; import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model'; import { hasValue } from '../empty.util'; -import { PageInfo } from '../../core/shared/page-info.model'; import { PaginationService } from '../../core/pagination/pagination.service'; import { map, take } from 'rxjs/operators'; import { RemoteData } from '../../core/data/remote-data'; @@ -47,11 +45,6 @@ export class PaginationComponent implements OnDestroy, OnInit { */ @Input() collectionSize: number; - /** - * Page state of a Remote paginated objects. - */ - @Input() pageInfoState: Observable = undefined; - /** * Configuration for the NgbPagination component. */ @@ -136,18 +129,13 @@ export class PaginationComponent implements OnDestroy, OnInit { /** * Current page. */ - public currentPage$; + public currentPage$: Observable; /** * Current page in the state of a Remote paginated objects. */ public currentPageState: number = undefined; - /** - * An observable of HostWindowState type - */ - public hostWindow: Observable; - /** * ID for the pagination instance. This ID is used in the routing to retrieve the pagination options. * This ID needs to be unique between different pagination components when more than one will be displayed on the same page. @@ -162,7 +150,7 @@ export class PaginationComponent implements OnDestroy, OnInit { /** * Number of items per page. */ - public pageSize$; + public pageSize$: Observable; /** * Declare SortDirection enumeration to use it in the template @@ -183,7 +171,7 @@ export class PaginationComponent implements OnDestroy, OnInit { /** * Name of the field that's used to sort by */ - public sortField$; + public sortField$: Observable; public defaultSortField = 'name'; /** @@ -237,7 +225,7 @@ export class PaginationComponent implements OnDestroy, OnInit { map((currentPagination) => currentPagination.pageSize) ); - let sortOptions; + let sortOptions: SortOptions; if (this.sortOptions) { sortOptions = this.sortOptions; } else { @@ -251,16 +239,6 @@ export class PaginationComponent implements OnDestroy, OnInit { ); } - /** - * @param cdRef - * ChangeDetectorRef is a singleton service provided by Angular. - * @param route - * Route is a singleton service provided by Angular. - * @param router - * Router is a singleton service provided by Angular. - * @param hostWindowService - * the HostWindowService singleton. - */ constructor(private cdRef: ChangeDetectorRef, private paginationService: PaginationService, public hostWindowService: HostWindowService) { @@ -299,17 +277,6 @@ export class PaginationComponent implements OnDestroy, OnInit { this.emitPaginationChange(); } - /** - * Method to change the route to the given sort field - * - * @param sortField - * The sort field being navigated to. - */ - public doSortFieldChange(field: string) { - this.updateParams({ pageId: this.id, page: 1, sortField: field }); - this.emitPaginationChange(); - } - /** * Method to emit a general pagination change event */ @@ -333,8 +300,8 @@ export class PaginationComponent implements OnDestroy, OnInit { if (collectionSize) { showingDetails = this.paginationService.getCurrentPagination(this.id, this.paginationOptions).pipe( map((currentPaginationOptions) => { - let firstItem; - let lastItem; + let firstItem: number; + let lastItem: number; const pageMax = currentPaginationOptions.pageSize * currentPaginationOptions.currentPage; firstItem = currentPaginationOptions.pageSize * (currentPaginationOptions.currentPage - 1) + 1; From 59197cff2d663240cb771d58d1f01cf97d78c3b4 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 18 Apr 2024 19:33:42 +0200 Subject: [PATCH 3/4] 108555: Refactored ItemSelectComponent to not call canSelect every time changes are detected --- .../item-select/item-select.component.html | 10 ++--- .../item-select/item-select.component.spec.ts | 13 ++++--- .../item-select/item-select.component.ts | 39 +++++++------------ .../object-select/object-select.model.ts | 29 ++++++++++++++ .../object-select/object-select.component.ts | 10 +++-- 5 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 src/app/shared/object-select/object-select.model.ts diff --git a/src/app/shared/object-select/item-select/item-select.component.html b/src/app/shared/object-select/item-select/item-select.component.html index 110434969a..719a2f5493 100644 --- a/src/app/shared/object-select/item-select/item-select.component.html +++ b/src/app/shared/object-select/item-select/item-select.component.html @@ -17,17 +17,17 @@ - - + + - - + +
- + {{ dsoNameService.getName(collection) }} {{item.firstMetadataValue(['dc.contributor.author', 'dc.creator', 'dc.contributor.*'])}}{{ dsoNameService.getName(item) }}{{selectItem.dso.firstMetadataValue(['dc.contributor.author', 'dc.creator', 'dc.contributor.*'])}}{{ dsoNameService.getName(selectItem.dso) }}
diff --git a/src/app/shared/object-select/item-select/item-select.component.spec.ts b/src/app/shared/object-select/item-select/item-select.component.spec.ts index 5131060cb2..fabd45e484 100644 --- a/src/app/shared/object-select/item-select/item-select.component.spec.ts +++ b/src/app/shared/object-select/item-select/item-select.component.spec.ts @@ -184,15 +184,16 @@ describe('ItemSelectComponent', () => { beforeEach(() => { comp.featureId = FeatureID.CanManageMappings; spyOn(authorizationDataService, 'isAuthorized').and.returnValue(of(false)); + comp.ngOnInit(); }); - it('should disable the checkbox', waitForAsync(() => { + it('should disable the checkbox', waitForAsync(async () => { fixture.detectChanges(); - fixture.whenStable().then(() => { - const checkbox = fixture.debugElement.query(By.css('input.item-checkbox')).nativeElement; - expect(authorizationDataService.isAuthorized).toHaveBeenCalled(); - expect(checkbox.disabled).toBeTrue(); - }); + await fixture.whenStable(); + + const checkbox = fixture.debugElement.query(By.css('input.item-checkbox')).nativeElement; + expect(authorizationDataService.isAuthorized).toHaveBeenCalled(); + expect(checkbox.disabled).toBeTrue(); })); }); }); diff --git a/src/app/shared/object-select/item-select/item-select.component.ts b/src/app/shared/object-select/item-select/item-select.component.ts index dd0266ff83..e9ec817cf0 100644 --- a/src/app/shared/object-select/item-select/item-select.component.ts +++ b/src/app/shared/object-select/item-select/item-select.component.ts @@ -1,14 +1,13 @@ -import { Component, Input } from '@angular/core'; +import { Component, Input, OnInit } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; -import { ObjectSelectService } from '../object-select.service'; import { ObjectSelectComponent } from '../object-select/object-select.component'; import { hasValueOperator, isNotEmpty } from '../../empty.util'; import { Observable } from 'rxjs'; import { getAllSucceededRemoteDataPayload } from '../../../core/shared/operators'; import { map } from 'rxjs/operators'; import { getItemPageRoute } from '../../../item-page/item-page-routing-paths'; -import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; -import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; +import { PaginatedList } from '../../../core/data/paginated-list.model'; +import { DSpaceObjectSelect } from '../object-select.model'; @Component({ selector: 'ds-item-select', @@ -18,7 +17,7 @@ import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; /** * A component used to select items from a specific list and returning the UUIDs of the selected items */ -export class ItemSelectComponent extends ObjectSelectComponent { +export class ItemSelectComponent extends ObjectSelectComponent implements OnInit { /** * Whether or not to hide the collection column @@ -27,35 +26,25 @@ export class ItemSelectComponent extends ObjectSelectComponent { hideCollection = false; /** - * The routes to the items their pages - * Key: Item ID - * Value: Route to item page + * Collection of all the data that is used to display the {@link Item} in the HTML. + * By collecting this data here it doesn't need to be recalculated on evey change detection. */ - itemPageRoutes$: Observable<{ - [itemId: string]: string - }>; - - constructor( - protected objectSelectService: ObjectSelectService, - protected authorizationService: AuthorizationDataService, - public dsoNameService: DSONameService, - ) { - super(objectSelectService, authorizationService); - } + selectItems$: Observable[]>; ngOnInit(): void { super.ngOnInit(); if (!isNotEmpty(this.confirmButton)) { this.confirmButton = 'item.select.confirm'; } - this.itemPageRoutes$ = this.dsoRD$.pipe( + this.selectItems$ = this.dsoRD$.pipe( hasValueOperator(), getAllSucceededRemoteDataPayload(), - map((items) => { - const itemPageRoutes = {}; - items.page.forEach((item) => itemPageRoutes[item.uuid] = getItemPageRoute(item)); - return itemPageRoutes; - }) + map((items: PaginatedList) => items.page.map((item: Item) => Object.assign(new DSpaceObjectSelect(), { + dso: item, + canSelect$: this.canSelect(item), + selected$: this.getSelected(item.id), + route: getItemPageRoute(item), + } as DSpaceObjectSelect))), ); } diff --git a/src/app/shared/object-select/object-select.model.ts b/src/app/shared/object-select/object-select.model.ts new file mode 100644 index 0000000000..329b419f46 --- /dev/null +++ b/src/app/shared/object-select/object-select.model.ts @@ -0,0 +1,29 @@ +import { Observable } from 'rxjs'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; + +/** + * Class used to collect all the data that that is used by the {@link ObjectSelectComponent} in the HTML. + */ +export class DSpaceObjectSelect { + + /** + * The {@link DSpaceObject} to display + */ + dso: T; + + /** + * Whether the {@link DSpaceObject} can be selected + */ + canSelect$: Observable; + + /** + * Whether the {@link DSpaceObject} is selected + */ + selected$: Observable; + + /** + * The {@link DSpaceObject}'s route + */ + route: string; + +} diff --git a/src/app/shared/object-select/object-select/object-select.component.ts b/src/app/shared/object-select/object-select/object-select.component.ts index 6fb795690d..cc38941fd9 100644 --- a/src/app/shared/object-select/object-select/object-select.component.ts +++ b/src/app/shared/object-select/object-select/object-select.component.ts @@ -9,6 +9,7 @@ import { SortOptions } from '../../../core/cache/models/sort-options.model'; import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { DSpaceObject } from '../../../core/shared/dspace-object.model'; +import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; /** * An abstract component used to select DSpaceObjects from a specific list and returning the UUIDs of the selected DSpaceObjects @@ -17,7 +18,7 @@ import { DSpaceObject } from '../../../core/shared/dspace-object.model'; selector: 'ds-object-select-abstract', template: '' }) -export abstract class ObjectSelectComponent implements OnInit, OnDestroy { +export abstract class ObjectSelectComponent implements OnInit, OnDestroy { /** * A unique key used for the object select service @@ -88,8 +89,11 @@ export abstract class ObjectSelectComponent implements OnInit, OnDestro */ selectedIds$: Observable; - constructor(protected objectSelectService: ObjectSelectService, - protected authorizationService: AuthorizationDataService) { + constructor( + protected objectSelectService: ObjectSelectService, + protected authorizationService: AuthorizationDataService, + public dsoNameService: DSONameService, + ) { } ngOnInit(): void { From 268d2e54fcb0c35216669e52022564a9c5a651d1 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 18 Apr 2024 19:44:17 +0200 Subject: [PATCH 4/4] 108555: Refactored CollectionSelectComponent to not call canSelect every time changes are detected --- .../collection-select.component.html | 6 +-- .../collection-select.component.ts | 37 ++++++++++++------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/app/shared/object-select/collection-select/collection-select.component.html b/src/app/shared/object-select/collection-select/collection-select.component.html index 953f540d59..8142c35b53 100644 --- a/src/app/shared/object-select/collection-select/collection-select.component.html +++ b/src/app/shared/object-select/collection-select/collection-select.component.html @@ -15,9 +15,9 @@ - - - {{ dsoNameService.getName(collection) }} + + + {{ dsoNameService.getName(selectCollection.dso) }} diff --git a/src/app/shared/object-select/collection-select/collection-select.component.ts b/src/app/shared/object-select/collection-select/collection-select.component.ts index 2d36f80274..e9edf19304 100644 --- a/src/app/shared/object-select/collection-select/collection-select.component.ts +++ b/src/app/shared/object-select/collection-select/collection-select.component.ts @@ -1,10 +1,13 @@ -import { Component } from '@angular/core'; +import { Component, OnInit } from '@angular/core'; import { Collection } from '../../../core/shared/collection.model'; import { ObjectSelectComponent } from '../object-select/object-select.component'; -import { isNotEmpty } from '../../empty.util'; -import { ObjectSelectService } from '../object-select.service'; -import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; -import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; +import { isNotEmpty, hasValueOperator } from '../../empty.util'; +import { Observable } from 'rxjs'; +import { DSpaceObjectSelect } from '../object-select.model'; +import { getAllSucceededRemoteDataPayload } from '../../../core/shared/operators'; +import { map } from 'rxjs/operators'; +import { PaginatedList } from '../../../core/data/paginated-list.model'; +import { getCollectionPageRoute } from '../../../collection-page/collection-page-routing-paths'; @Component({ selector: 'ds-collection-select', @@ -15,21 +18,29 @@ import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; /** * A component used to select collections from a specific list and returning the UUIDs of the selected collections */ -export class CollectionSelectComponent extends ObjectSelectComponent { +export class CollectionSelectComponent extends ObjectSelectComponent implements OnInit { - constructor( - protected objectSelectService: ObjectSelectService, - protected authorizationService: AuthorizationDataService, - public dsoNameService: DSONameService, - ) { - super(objectSelectService, authorizationService); - } + /** + * Collection of all the data that is used to display the {@link Collection} in the HTML. + * By collecting this data here it doesn't need to be recalculated on evey change detection. + */ + selectCollections$: Observable[]>; ngOnInit(): void { super.ngOnInit(); if (!isNotEmpty(this.confirmButton)) { this.confirmButton = 'collection.select.confirm'; } + this.selectCollections$ = this.dsoRD$.pipe( + hasValueOperator(), + getAllSucceededRemoteDataPayload(), + map((collections: PaginatedList) => collections.page.map((collection: Collection) => Object.assign(new DSpaceObjectSelect(), { + dso: collection, + canSelect$: this.canSelect(collection), + selected$: this.getSelected(collection.id), + route: getCollectionPageRoute(collection.id), + } as DSpaceObjectSelect))), + ); } }