From 6f1537925689e0b6b8a09acf8c0655d39facb677 Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Wed, 13 Nov 2024 17:32:47 +0100 Subject: [PATCH 1/2] [DSC-2028] move default projection to search manager, refactor services, adapt tests --- .../collection-page/collection-page.component.spec.ts | 8 ++++---- src/app/collection-page/collection-page.component.ts | 9 +++------ src/app/core/browse/search-manager.ts | 5 +++-- src/app/my-dspace-page/my-dspace-page.component.html | 1 - src/app/my-dspace-page/my-dspace-page.component.ts | 4 ---- .../abstract-browse-elements.component.ts | 10 ++++++---- .../browse-most-elements.component.ts | 2 +- .../themed-browse-most-elements.component.ts | 2 +- src/app/shared/search/search.component.ts | 2 +- 9 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/app/collection-page/collection-page.component.spec.ts b/src/app/collection-page/collection-page.component.spec.ts index 15fce2cd9ac..8e873925738 100644 --- a/src/app/collection-page/collection-page.component.spec.ts +++ b/src/app/collection-page/collection-page.component.spec.ts @@ -14,7 +14,6 @@ import { RouterStub } from '../shared/testing/router.stub'; import { environment } from 'src/environments/environment.test'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { Collection } from '../core/shared/collection.model'; -import { SearchService } from '../core/shared/search/search.service'; import { By } from '@angular/platform-browser'; import { FormsModule } from '@angular/forms'; import { RouterTestingModule } from '@angular/router/testing'; @@ -22,6 +21,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { VarDirective } from '../shared/utils/var.directive'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; import { Bitstream } from '../core/shared/bitstream.model'; +import { SearchManager } from '../core/browse/search-manager'; describe('CollectionPageComponent', () => { let component: CollectionPageComponent; @@ -33,7 +33,7 @@ describe('CollectionPageComponent', () => { let paginationServiceSpy: jasmine.SpyObj; let authorizationDataServiceSpy: jasmine.SpyObj; let dsoNameServiceSpy: jasmine.SpyObj; - let searchServiceSpy: jasmine.SpyObj; + let searchServiceSpy: jasmine.SpyObj; let aroute = new ActivatedRouteStub(); let router = new RouterStub(); @@ -44,7 +44,7 @@ describe('CollectionPageComponent', () => { paginationServiceSpy = jasmine.createSpyObj('PaginationService', ['getCurrentPagination', 'getCurrentSort', 'clearPagination']); authorizationDataServiceSpy = jasmine.createSpyObj('AuthorizationDataService', ['isAuthorized']); collectionDataServiceSpy = jasmine.createSpyObj('CollectionDataService', ['findById', 'getAuthorizedCollection']); - searchServiceSpy = jasmine.createSpyObj('SearchService', ['search']); + searchServiceSpy = jasmine.createSpyObj('SearchManager', ['search']); dsoNameServiceSpy = jasmine.createSpyObj('DSONameService', ['getName']); await TestBed.configureTestingModule({ @@ -58,7 +58,7 @@ describe('CollectionPageComponent', () => { { provide: PaginationService, useValue: paginationServiceSpy }, { provide: AuthorizationDataService, useValue: authorizationDataServiceSpy }, { provide: DSONameService, useValue: dsoNameServiceSpy }, - { provide: SearchService, useValue: searchServiceSpy }, + { provide: SearchManager, useValue: searchServiceSpy }, { provide: APP_CONFIG, useValue: environment }, { provide: PLATFORM_ID, useValue: 'browser' }, ] diff --git a/src/app/collection-page/collection-page.component.ts b/src/app/collection-page/collection-page.component.ts index e894d66a4af..ccb23beec70 100644 --- a/src/app/collection-page/collection-page.component.ts +++ b/src/app/collection-page/collection-page.component.ts @@ -4,9 +4,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, Subject } from 'rxjs'; import { filter, map, mergeMap, startWith, switchMap, take } from 'rxjs/operators'; import { PaginatedSearchOptions } from '../shared/search/models/paginated-search-options.model'; -import { SearchService } from '../core/shared/search/search.service'; import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; -import { CollectionDataService } from '../core/data/collection-data.service'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; import { Bitstream } from '../core/shared/bitstream.model'; @@ -30,6 +28,7 @@ import { redirectOn4xx } from '../core/shared/authorized.operators'; import { BROWSE_LINKS_TO_FOLLOW } from '../core/browse/browse.service'; import { DSONameService } from '../core/breadcrumbs/dso-name.service'; import { APP_CONFIG, AppConfig } from '../../../src/config/app-config.interface'; +import { SearchManager } from '../core/browse/search-manager'; @Component({ selector: 'ds-collection-page', @@ -64,8 +63,7 @@ export class CollectionPageComponent implements OnInit { constructor( @Inject(PLATFORM_ID) private platformId: Object, - private collectionDataService: CollectionDataService, - private searchService: SearchService, + private searchManager: SearchManager, private route: ActivatedRoute, private router: Router, private authService: AuthService, @@ -113,14 +111,13 @@ export class CollectionPageComponent implements OnInit { getFirstSucceededRemoteData(), map((rd) => rd.payload.id), switchMap((id: string) => { - return this.searchService.search( + return this.searchManager.search( new PaginatedSearchOptions({ scope: id, pagination: currentPagination, sort: currentSort, dsoTypes: [DSpaceObjectType.ITEM], forcedEmbeddedKeys: ['metrics'], - projection: 'preventMetadataSecurity' }), null, true, true, ...BROWSE_LINKS_TO_FOLLOW) .pipe(toDSpaceObjectListRD()) as Observable>>; }), diff --git a/src/app/core/browse/search-manager.ts b/src/app/core/browse/search-manager.ts index c7fcdb54cb6..c8946037304 100644 --- a/src/app/core/browse/search-manager.ts +++ b/src/app/core/browse/search-manager.ts @@ -45,7 +45,7 @@ export class SearchManager { * @returns {Observable>>} */ getBrowseItemsFor(filterValue: string, filterAuthority: string, options: BrowseEntrySearchOptions, ...linksToFollow: FollowLinkConfig[]): Observable>> { - const browseOptions = Object.assign({}, options, { projection: 'preventMetadataSecurity' }); + const browseOptions = Object.assign({}, options, { projection: options.projection ?? 'preventMetadataSecurity' }); return this.browseService.getBrowseItemsFor(filterValue, filterAuthority, browseOptions, ...linksToFollow) .pipe(this.completeWithExtraData()); } @@ -67,7 +67,8 @@ export class SearchManager { useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig[]): Observable>> { - return this.searchService.search(searchOptions, responseMsToLive, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow) + const optionsWithDefaultProjection = Object.assign(new PaginatedSearchOptions({}), searchOptions, { projection: searchOptions.projection ?? 'preventMetadataSecurity' }); + return this.searchService.search(optionsWithDefaultProjection, responseMsToLive, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow) .pipe(this.completeSearchObjectsWithExtraData()); } diff --git a/src/app/my-dspace-page/my-dspace-page.component.html b/src/app/my-dspace-page/my-dspace-page.component.html index 617c69b5f7f..29563075d9e 100644 --- a/src/app/my-dspace-page/my-dspace-page.component.html +++ b/src/app/my-dspace-page/my-dspace-page.component.html @@ -9,7 +9,6 @@ [configurationList]="(configurationList$ | async)" [context]="context" [viewModeList]="viewModeList" - [projection]="projection" [showThumbnails]="false" [selectable]="(currentConfiguration$ | async) === workflowType" [selectionConfig]="{ repeatable: true, listId: listId }" diff --git a/src/app/my-dspace-page/my-dspace-page.component.ts b/src/app/my-dspace-page/my-dspace-page.component.ts index 89924b3e276..2e6a1c11281 100644 --- a/src/app/my-dspace-page/my-dspace-page.component.ts +++ b/src/app/my-dspace-page/my-dspace-page.component.ts @@ -60,10 +60,6 @@ export class MyDSpacePageComponent implements OnInit { */ roleTypeEnum = RoleType; - /** - * Projection to use during the search - */ - projection = 'preventMetadataSecurity'; /** * List of available view mode diff --git a/src/app/shared/browse-most-elements/abstract-browse-elements.component.ts b/src/app/shared/browse-most-elements/abstract-browse-elements.component.ts index 6bd9f6f63aa..8e9314851fe 100644 --- a/src/app/shared/browse-most-elements/abstract-browse-elements.component.ts +++ b/src/app/shared/browse-most-elements/abstract-browse-elements.component.ts @@ -3,7 +3,6 @@ import { CollectionElementLinkType } from '../object-collection/collection-eleme import { Component, Input, OnChanges, OnInit, PLATFORM_ID, inject } from '@angular/core'; import { isPlatformServer } from '@angular/common'; -import { SearchService } from '../../core/shared/search/search.service'; import { PaginatedSearchOptions } from '../search/models/paginated-search-options.model'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { SearchResult } from '../search/models/search-result.model'; @@ -21,6 +20,7 @@ import { BehaviorSubject, Observable, mergeMap } from 'rxjs'; import { Item } from '../../core/shared/item.model'; import { getItemPageRoute } from '../../item-page/item-page-routing-paths'; import { TopSection } from '../../core/layout/models/section.model'; +import { SearchManager } from '../../core/browse/search-manager'; @Component({ template: '' @@ -29,7 +29,7 @@ export abstract class AbstractBrowseElementsComponent implements OnInit, OnChang protected readonly appConfig = inject(APP_CONFIG); protected readonly platformId = inject(PLATFORM_ID); - protected readonly searchService = inject(SearchService); + protected readonly searchManager = inject(SearchManager); protected abstract followMetricsLink: boolean; // to be overridden protected abstract followThumbnailLink: boolean; // to be overridden @@ -47,7 +47,7 @@ export abstract class AbstractBrowseElementsComponent implements OnInit, OnChang /** * Optional projection to use during the search */ - @Input() projection = 'preventMetadataSecurity'; + @Input() projection; /** * Whether to show the badge label or not @@ -96,10 +96,12 @@ export abstract class AbstractBrowseElementsComponent implements OnInit, OnChang this.paginatedSearchOptions = Object.assign(new PaginatedSearchOptions({}), this.paginatedSearchOptions, { projection: this.projection }); + this.paginatedSearchOptions$ = new BehaviorSubject(this.paginatedSearchOptions); + this.searchResults$ = this.paginatedSearchOptions$.asObservable().pipe( mergeMap((paginatedSearchOptions) => - this.searchService.search(paginatedSearchOptions, null, true, true, ...followLinks), + this.searchManager.search(paginatedSearchOptions, null, true, true, ...followLinks), ), getAllCompletedRemoteData(), ); diff --git a/src/app/shared/browse-most-elements/browse-most-elements.component.ts b/src/app/shared/browse-most-elements/browse-most-elements.component.ts index 198b413072c..3e1f0efa62a 100644 --- a/src/app/shared/browse-most-elements/browse-most-elements.component.ts +++ b/src/app/shared/browse-most-elements/browse-most-elements.component.ts @@ -25,7 +25,7 @@ export class BrowseMostElementsComponent implements OnInit, OnChanges { /** * Optional projection to use during the search */ - @Input() projection = 'preventMetadataSecurity'; + @Input() projection; /** * Whether to show the badge label or not diff --git a/src/app/shared/browse-most-elements/themed-browse-most-elements.component.ts b/src/app/shared/browse-most-elements/themed-browse-most-elements.component.ts index 79379816c2f..f07a6050ec5 100644 --- a/src/app/shared/browse-most-elements/themed-browse-most-elements.component.ts +++ b/src/app/shared/browse-most-elements/themed-browse-most-elements.component.ts @@ -19,7 +19,7 @@ export class ThemedBrowseMostElementsComponent extends ThemedComponent Date: Thu, 14 Nov 2024 10:49:28 +0100 Subject: [PATCH 2/2] [DSC-2028] adapt findById in metadata link view, fix tests --- .../rendering-types/crisref/crisref.component.spec.ts | 4 ++-- .../metadata-link-view.component.spec.ts | 10 +++++----- .../metadata-link-view/metadata-link-view.component.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metadata/rendering-types/crisref/crisref.component.spec.ts b/src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metadata/rendering-types/crisref/crisref.component.spec.ts index 67e87b3d17d..a524beafeac 100644 --- a/src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metadata/rendering-types/crisref/crisref.component.spec.ts +++ b/src/app/cris-layout/cris-layout-matrix/cris-layout-box-container/boxes/metadata/rendering-types/crisref/crisref.component.spec.ts @@ -20,7 +20,7 @@ describe('CrisrefComponent', () => { let fixture: ComponentFixture; const itemService = jasmine.createSpyObj('ItemDataService', { - findById: jasmine.createSpy('findById') + findByIdWithProjections: jasmine.createSpy('findByIdWithProjections') }); const metadataValue = Object.assign(new MetadataValue(), { 'value': 'test item title', @@ -106,7 +106,7 @@ describe('CrisrefComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(CrisrefComponent); component = fixture.componentInstance; - itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(testPerson)); + itemService.findByIdWithProjections.and.returnValue(createSuccessfulRemoteDataObject$(testPerson)); fixture.detectChanges(); }); diff --git a/src/app/shared/metadata-link-view/metadata-link-view.component.spec.ts b/src/app/shared/metadata-link-view/metadata-link-view.component.spec.ts index cfac48cfb14..81391f98a17 100644 --- a/src/app/shared/metadata-link-view/metadata-link-view.component.spec.ts +++ b/src/app/shared/metadata-link-view/metadata-link-view.component.spec.ts @@ -90,7 +90,7 @@ describe('MetadataLinkViewComponent', () => { }); itemService = jasmine.createSpyObj('ItemDataService', { - findById: jasmine.createSpy('findById') + findByIdWithProjections: jasmine.createSpy('findByIdWithProjections') }); beforeEach(waitForAsync(() => { @@ -110,7 +110,7 @@ describe('MetadataLinkViewComponent', () => { describe('Check metadata without authority', () => { beforeEach(() => { fixture = TestBed.createComponent(MetadataLinkViewComponent); - itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(testOrgunit)); + itemService.findByIdWithProjections.and.returnValue(createSuccessfulRemoteDataObject$(testOrgunit)); component = fixture.componentInstance; component.item = testPerson; component.metadata = testMetadataValueWithoutAuthority; @@ -135,7 +135,7 @@ describe('MetadataLinkViewComponent', () => { describe('when item is found with orcid', () => { beforeEach(() => { fixture = TestBed.createComponent(MetadataLinkViewComponent); - itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(testPerson)); + itemService.findByIdWithProjections.and.returnValue(createSuccessfulRemoteDataObject$(testPerson)); component = fixture.componentInstance; component.item = testPerson; component.metadata = testMetadataValueWithAuthority; @@ -162,7 +162,7 @@ describe('MetadataLinkViewComponent', () => { describe('when item is found without orcid', () => { beforeEach(() => { fixture = TestBed.createComponent(MetadataLinkViewComponent); - itemService.findById.and.returnValue(createSuccessfulRemoteDataObject$(testOrgunit)); + itemService.findByIdWithProjections.and.returnValue(createSuccessfulRemoteDataObject$(testOrgunit)); component = fixture.componentInstance; component.item = testPerson; component.metadata = testMetadataValueWithAuthority; @@ -189,7 +189,7 @@ describe('MetadataLinkViewComponent', () => { describe('when item is not found', () => { beforeEach(() => { fixture = TestBed.createComponent(MetadataLinkViewComponent); - itemService.findById.and.returnValue(createFailedRemoteDataObject$()); + itemService.findByIdWithProjections.and.returnValue(createFailedRemoteDataObject$()); component = fixture.componentInstance; component.item = testPerson; component.metadata = testMetadataValueWithAuthority; diff --git a/src/app/shared/metadata-link-view/metadata-link-view.component.ts b/src/app/shared/metadata-link-view/metadata-link-view.component.ts index c38e1cc2dac..0b17c8eb9de 100644 --- a/src/app/shared/metadata-link-view/metadata-link-view.component.ts +++ b/src/app/shared/metadata-link-view/metadata-link-view.component.ts @@ -85,7 +85,7 @@ export class MetadataLinkViewComponent implements OnInit { const linksToFollow = [followLink('thumbnail')]; if (Metadata.hasValidAuthority(metadataValue.authority)) { - return this.itemService.findById(metadataValue.authority, true, false, ...linksToFollow).pipe( + return this.itemService.findByIdWithProjections(metadataValue.authority, ['preventMetadataSecurity'], true, false, ...linksToFollow).pipe( getFirstCompletedRemoteData(), map((itemRD: RemoteData) => this.createMetadataView(itemRD, metadataValue)) );