From 9b8ada0326049ad2e17d6684a304b2a99202a931 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Tue, 6 Apr 2021 10:06:15 +0200 Subject: [PATCH 01/42] [CST-4009] Discovery result sort options not reflecting what is configured --- .../my-dspace-page.component.html | 4 + .../my-dspace-page.component.spec.ts | 24 +++++- .../my-dspace-page.component.ts | 34 ++++++++- src/app/core/core.module.ts | 2 + .../search-filters/search-config.model.ts | 75 ++++++++++++++++++ .../search-config.resource-type.ts | 9 +++ .../core/shared/search/search.service.spec.ts | 50 ++++++++++++ src/app/core/shared/search/search.service.ts | 76 +++++++++++++------ .../search-settings.component.html | 6 +- .../search-settings.component.spec.ts | 51 ++++++------- .../search-settings.component.ts | 19 ++--- .../search-sidebar.component.html | 2 +- .../search-sidebar.component.ts | 12 +++ 13 files changed, 292 insertions(+), 72 deletions(-) create mode 100644 src/app/core/shared/search/search-filters/search-config.model.ts create mode 100644 src/app/core/shared/search/search-filters/search-config.resource-type.ts 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 c911e2c319..32e3a0d710 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.html +++ b/src/app/+my-dspace-page/my-dspace-page.component.html @@ -6,6 +6,8 @@ [configurationList]="(configurationList$ | async)" [resultCount]="(resultsRD$ | async)?.payload.totalElements" [viewModeList]="viewModeList" + [searchOptions]="(searchOptions$ | async)" + [sortOptions]="(sortOptions$ | async)" [refreshFilters]="refreshFilters.asObservable()" [inPlaceSearch]="inPlaceSearch">
@@ -28,6 +30,8 @@ [resultCount]="(resultsRD$ | async)?.payload.totalElements" (toggleSidebar)="closeSidebar()" [ngClass]="{'active': !(isSidebarCollapsed() | async)}" + [searchOptions]="(searchOptions$ | async)" + [sortOptions]="(sortOptions$ | async)" [refreshFilters]="refreshFilters.asObservable()" [inPlaceSearch]="inPlaceSearch"> diff --git a/src/app/+my-dspace-page/my-dspace-page.component.spec.ts b/src/app/+my-dspace-page/my-dspace-page.component.spec.ts index 59581d0da8..909239b61c 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.spec.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.spec.ts @@ -45,6 +45,7 @@ describe('MyDSpacePageComponent', () => { pagination.id = 'mydspace-results-pagination'; pagination.currentPage = 1; pagination.pageSize = 10; + const sortOption = { name: 'score', metadata: null }; const sort: SortOptions = new SortOptions('score', SortDirection.DESC); const mockResults = createSuccessfulRemoteDataObject$(['test', 'data']); const searchServiceStub = jasmine.createSpyObj('SearchService', { @@ -52,7 +53,8 @@ describe('MyDSpacePageComponent', () => { getEndpoint: observableOf('discover/search/objects'), getSearchLink: '/mydspace', getScopes: observableOf(['test-scope']), - setServiceOptions: {} + setServiceOptions: {}, + getSearchConfigurationFor: createSuccessfulRemoteDataObject$({ sortOptions: [sortOption]}) }); const configurationParam = 'default'; const queryParam = 'test query'; @@ -188,4 +190,24 @@ describe('MyDSpacePageComponent', () => { }); }); + + describe('when stable', () => { + + beforeEach(() => { + fixture.detectChanges(); + }); + + it('should have initialized the sortOptions$ observable', (done) => { + + comp.sortOptions$.subscribe((sortOptions) => { + + expect(sortOptions.length).toEqual(2); + expect(sortOptions[0]).toEqual(new SortOptions('score', SortDirection.ASC)); + expect(sortOptions[1]).toEqual(new SortOptions('score', SortDirection.DESC)); + done(); + }); + + }); + + }); }); 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 5ee2a47d9f..ba5c0e5cc7 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.ts @@ -7,8 +7,8 @@ import { OnInit } from '@angular/core'; -import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs'; -import { map, switchMap, tap, } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, Observable, Subject, Subscription } from 'rxjs'; +import { map, switchMap, take, tap } from 'rxjs/operators'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; @@ -19,7 +19,7 @@ import { PaginatedSearchOptions } from '../shared/search/paginated-search-option import { SearchService } from '../core/shared/search/search.service'; import { SidebarService } from '../shared/sidebar/sidebar.service'; import { hasValue } from '../shared/empty.util'; -import { getFirstSucceededRemoteData } from '../core/shared/operators'; +import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators'; import { MyDSpaceResponseParsingService } from '../core/data/mydspace-response-parsing.service'; import { SearchConfigurationOption } from '../shared/search/search-switch-configuration/search-configuration-option.model'; import { RoleType } from '../core/roles/role-types'; @@ -29,6 +29,8 @@ import { ViewMode } from '../core/shared/view-mode.model'; import { MyDSpaceRequest } from '../core/data/request.models'; import { SearchResult } from '../shared/search/search-result.model'; import { Context } from '../core/shared/context.model'; +import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; +import { SearchConfig } from '../core/shared/search/search-filters/search-config.model'; export const MYDSPACE_ROUTE = '/mydspace'; export const SEARCH_CONFIG_SERVICE: InjectionToken = new InjectionToken('searchConfigurationService'); @@ -71,6 +73,11 @@ export class MyDSpacePageComponent implements OnInit { */ searchOptions$: Observable; + /** + * The current available sort options + */ + sortOptions$: Observable; + /** * The current relevant scopes */ @@ -151,6 +158,27 @@ export class MyDSpacePageComponent implements OnInit { }) ); + this.sortOptions$ = this.context$.pipe( + switchMap((context) => this.service.getSearchConfigurationFor(null, context)), + getFirstSucceededRemoteDataPayload(), + map((searchConfig: SearchConfig) => { + const sortOptions = []; + searchConfig.sortOptions.forEach(sortOption => { + sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); + sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); + }); + return sortOptions; + })); + + combineLatest([ + this.sortOptions$, + this.searchConfigService.paginatedSearchOptions + ]).pipe(take(1)) + .subscribe(([sortOptions, searchOptions]) => { + const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); + this.searchConfigService.paginatedSearchOptions.next(updateValue); + }); + } /** diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index f73bfd0bdf..62265fdedf 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -162,6 +162,7 @@ import { ShortLivedToken } from './auth/models/short-lived-token.model'; import { UsageReport } from './statistics/models/usage-report.model'; import { RootDataService } from './data/root-data.service'; import { Root } from './data/root.model'; +import { SearchConfig } from './shared/search/search-filters/search-config.model'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -342,6 +343,7 @@ export const models = Registration, UsageReport, Root, + SearchConfig ]; @NgModule({ diff --git a/src/app/core/shared/search/search-filters/search-config.model.ts b/src/app/core/shared/search/search-filters/search-config.model.ts new file mode 100644 index 0000000000..dd7a799f37 --- /dev/null +++ b/src/app/core/shared/search/search-filters/search-config.model.ts @@ -0,0 +1,75 @@ +import { autoserialize, deserialize } from 'cerialize'; + +import { SEARCH_CONFIG } from './search-config.resource-type'; +import { typedObject } from '../../../cache/builders/build-decorators'; +import { CacheableObject } from '../../../cache/object-cache.reducer'; +import { HALLink } from '../../hal-link.model'; +import { ResourceType } from '../../resource-type'; + +/** + * The configuration for a search + */ +@typedObject +export class SearchConfig implements CacheableObject { + static type = SEARCH_CONFIG; + + /** + * The id of this search configuration. + */ + @autoserialize + id: string; + + /** + * The configured filters. + */ + @autoserialize + filters: FilterConfig[]; + + /** + * The configured sort options. + */ + @autoserialize + sortOptions: SortOption[]; + + /** + * The object type. + */ + @autoserialize + type: ResourceType; + + /** + * The {@link HALLink}s for this Item + */ + @deserialize + _links: { + facets: HALLink; + objects: HALLink; + self: HALLink; + }; +} + +/** + * Interface to model filter's configuration. + */ +export interface FilterConfig { + filter: string; + hasFacets: boolean; + operators: OperatorConfig[]; + openByDefault: boolean; + pageSize: number; + type: string; +} + +/** + * Interface to model sort option's configuration. + */ +export interface SortOption { + name: string; +} + +/** + * Interface to model operator's configuration. + */ +export interface OperatorConfig { + operator: string; +} diff --git a/src/app/core/shared/search/search-filters/search-config.resource-type.ts b/src/app/core/shared/search/search-filters/search-config.resource-type.ts new file mode 100644 index 0000000000..967a654006 --- /dev/null +++ b/src/app/core/shared/search/search-filters/search-config.resource-type.ts @@ -0,0 +1,9 @@ +import {ResourceType} from '../../resource-type'; + +/** + * The resource type for SearchConfig + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const SEARCH_CONFIG = new ResourceType('discover'); diff --git a/src/app/core/shared/search/search.service.spec.ts b/src/app/core/shared/search/search.service.spec.ts index 06208094bd..d09444ad6c 100644 --- a/src/app/core/shared/search/search.service.spec.ts +++ b/src/app/core/shared/search/search.service.spec.ts @@ -230,5 +230,55 @@ describe('SearchService', () => { expect((searchService as any).requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ href: requestUrl }), true); }); }); + + describe('when getSearchConfigurationFor is called without a scope', () => { + const endPoint = 'http://endpoint.com/test/config'; + beforeEach(() => { + spyOn((searchService as any).halService, 'getEndpoint').and.returnValue(observableOf(endPoint)); + spyOn((searchService as any).rdb, 'buildFromHref').and.callThrough(); + /* tslint:disable:no-empty */ + searchService.getSearchConfigurationFor(null).subscribe((t) => { + }); // subscribe to make sure all methods are called + /* tslint:enable:no-empty */ + }); + + it('should call getEndpoint on the halService', () => { + expect((searchService as any).halService.getEndpoint).toHaveBeenCalled(); + }); + + it('should send out the request on the request service', () => { + expect((searchService as any).requestService.send).toHaveBeenCalled(); + }); + + it('should call send containing a request with the correct request url', () => { + expect((searchService as any).requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ href: endPoint }), true); + }); + }); + + describe('when getSearchConfigurationFor is called with a scope', () => { + const endPoint = 'http://endpoint.com/test/config'; + const scope = 'test'; + const requestUrl = endPoint + '?scope=' + scope; + beforeEach(() => { + spyOn((searchService as any).halService, 'getEndpoint').and.returnValue(observableOf(endPoint)); + /* tslint:disable:no-empty */ + searchService.getSearchConfigurationFor(scope).subscribe((t) => { + }); // subscribe to make sure all methods are called + /* tslint:enable:no-empty */ + }); + + it('should call getEndpoint on the halService', () => { + expect((searchService as any).halService.getEndpoint).toHaveBeenCalled(); + }); + + it('should send out the request on the request service', () => { + expect((searchService as any).requestService.send).toHaveBeenCalled(); + }); + + it('should call send containing a request with the correct request url', () => { + expect((searchService as any).requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ href: requestUrl }), true); + }); + }); + }); }); diff --git a/src/app/core/shared/search/search.service.ts b/src/app/core/shared/search/search.service.ts index b380a70d44..7747717830 100644 --- a/src/app/core/shared/search/search.service.ts +++ b/src/app/core/shared/search/search.service.ts @@ -37,12 +37,19 @@ import { ListableObject } from '../../../shared/object-collection/shared/listabl import { getSearchResultFor } from '../../../shared/search/search-result-element-decorator'; import { FacetConfigResponse } from '../../../shared/search/facet-config-response.model'; import { FacetValues } from '../../../shared/search/facet-values.model'; +import { SearchConfig } from './search-filters/search-config.model'; /** * Service that performs all general actions that have to do with the search page */ @Injectable() export class SearchService implements OnDestroy { + + /** + * Endpoint link path for retrieving search configurations + */ + private configurationLinkPath = 'discover/search'; + /** * Endpoint link path for retrieving general search results */ @@ -224,6 +231,24 @@ export class SearchService implements OnDestroy { ); } + private getConfigUrl(url: string, scope?: string, configurationName?: string) { + const args: string[] = []; + + if (isNotEmpty(scope)) { + args.push(`scope=${scope}`); + } + + if (isNotEmpty(configurationName)) { + args.push(`configuration=${configurationName}`); + } + + if (isNotEmpty(args)) { + url = new URLCombiner(url, `?${args.join('&')}`).toString(); + } + + return url; + } + /** * Request the filter configuration for a given scope or the whole repository * @param {string} scope UUID of the object for which config the filter config is requested, when no scope is provided the configuration for the whole repository is loaded @@ -232,33 +257,17 @@ export class SearchService implements OnDestroy { */ getConfig(scope?: string, configurationName?: string): Observable> { const href$ = this.halService.getEndpoint(this.facetLinkPathPrefix).pipe( - map((url: string) => { - const args: string[] = []; - - if (isNotEmpty(scope)) { - args.push(`scope=${scope}`); - } - - if (isNotEmpty(configurationName)) { - args.push(`configuration=${configurationName}`); - } - - if (isNotEmpty(args)) { - url = new URLCombiner(url, `?${args.join('&')}`).toString(); - } - - return url; - }), + map((url: string) => this.getConfigUrl(url, scope, configurationName)), ); href$.pipe(take(1)).subscribe((url: string) => { - let request = new this.request(this.requestService.generateRequestId(), url); - request = Object.assign(request, { - getResponseParser(): GenericConstructor { - return FacetConfigResponseParsingService; - } - }); - this.requestService.send(request, true); + let request = new this.request(this.requestService.generateRequestId(), url); + request = Object.assign(request, { + getResponseParser(): GenericConstructor { + return FacetConfigResponseParsingService; + } + }); + this.requestService.send(request, true); }); return this.rdb.buildFromHref(href$).pipe( @@ -397,6 +406,25 @@ export class SearchService implements OnDestroy { }); } + /** + * Request the search configuration for a given scope or the whole repository + * @param {string} scope UUID of the object for which config the filter config is requested, when no scope is provided the configuration for the whole repository is loaded + * @param {string} configurationName the name of the configuration + * @returns {Observable>} The found configuration + */ + getSearchConfigurationFor(scope?: string, configurationName?: string ): Observable> { + const href$ = this.halService.getEndpoint(this.configurationLinkPath).pipe( + map((url: string) => this.getConfigUrl(url, scope, configurationName)), + ); + + href$.pipe(take(1)).subscribe((url: string) => { + const request = new this.request(this.requestService.generateRequestId(), url); + this.requestService.send(request, true); + }); + + return this.rdb.buildFromHref(href$); + } + /** * @returns {string} The base path to the search page */ diff --git a/src/app/shared/search/search-settings/search-settings.component.html b/src/app/shared/search/search-settings/search-settings.component.html index f6806e6843..a31678743d 100644 --- a/src/app/shared/search/search-settings/search-settings.component.html +++ b/src/app/shared/search/search-settings/search-settings.component.html @@ -1,4 +1,4 @@ - +

{{ 'search.sidebar.settings.title' | translate}}

-
- \ No newline at end of file + diff --git a/src/app/shared/search/search-settings/search-settings.component.spec.ts b/src/app/shared/search/search-settings/search-settings.component.spec.ts index cd4a872815..221e3a0dea 100644 --- a/src/app/shared/search/search-settings/search-settings.component.spec.ts +++ b/src/app/shared/search/search-settings/search-settings.component.spec.ts @@ -12,7 +12,6 @@ import { EnumKeysPipe } from '../../utils/enum-keys-pipe'; import { By } from '@angular/platform-browser'; import { SearchFilterService } from '../../../core/shared/search/search-filter.service'; import { VarDirective } from '../../utils/var.directive'; -import { take } from 'rxjs/operators'; import { SEARCH_CONFIG_SERVICE } from '../../../+my-dspace-page/my-dspace-page.component'; import { SidebarService } from '../../sidebar/sidebar.service'; import { SidebarServiceStub } from '../../testing/sidebar-service.stub'; @@ -81,7 +80,7 @@ describe('SearchSettingsComponent', () => { provide: SEARCH_CONFIG_SERVICE, useValue: { paginatedSearchOptions: observableOf(paginatedSearchOptions), - getCurrentScope: observableOf('test-id') + getCurrentScope: observableOf('test-id'), } }, ], @@ -93,6 +92,14 @@ describe('SearchSettingsComponent', () => { fixture = TestBed.createComponent(SearchSettingsComponent); comp = fixture.componentInstance; + comp.sortOptions = [ + new SortOptions('score', SortDirection.DESC), + new SortOptions('dc.title', SortDirection.ASC), + new SortOptions('dc.title', SortDirection.DESC) + ]; + + comp.searchOptions = paginatedSearchOptions; + // SearchPageComponent test instance fixture.detectChanges(); searchServiceObject = (comp as any).service; @@ -101,34 +108,24 @@ describe('SearchSettingsComponent', () => { }); - it('it should show the order settings with the respective selectable options', (done) => { - (comp as any).searchOptions$.pipe(take(1)).subscribe((options) => { - fixture.detectChanges(); - const orderSetting = fixture.debugElement.query(By.css('div.result-order-settings')); - expect(orderSetting).toBeDefined(); - const childElements = orderSetting.queryAll(By.css('option')); - expect(childElements.length).toEqual(comp.searchOptionPossibilities.length); - done(); - }); + it('it should show the order settings with the respective selectable options', () => { + fixture.detectChanges(); + const orderSetting = fixture.debugElement.query(By.css('div.result-order-settings')); + expect(orderSetting).toBeDefined(); + const childElements = orderSetting.queryAll(By.css('option')); + expect(childElements.length).toEqual(comp.sortOptions.length); }); - it('it should show the size settings', (done) => { - (comp as any).searchOptions$.pipe(take(1)).subscribe((options) => { - fixture.detectChanges(); - const pageSizeSetting = fixture.debugElement.query(By.css('page-size-settings')); - expect(pageSizeSetting).toBeDefined(); - done(); - } - ); + it('it should show the size settings', () => { + fixture.detectChanges(); + const pageSizeSetting = fixture.debugElement.query(By.css('page-size-settings')); + expect(pageSizeSetting).toBeDefined(); }); - it('should have the proper order value selected by default', (done) => { - (comp as any).searchOptions$.pipe(take(1)).subscribe((options) => { - fixture.detectChanges(); - const orderSetting = fixture.debugElement.query(By.css('div.result-order-settings')); - const childElementToBeSelected = orderSetting.query(By.css('option[value="0"][selected="selected"]')); - expect(childElementToBeSelected).toBeDefined(); - done(); - }); + it('should have the proper order value selected by default', () => { + fixture.detectChanges(); + const orderSetting = fixture.debugElement.query(By.css('div.result-order-settings')); + const childElementToBeSelected = orderSetting.query(By.css('option[value="score,DESC"][selected="selected"]')); + expect(childElementToBeSelected).toBeDefined(); }); }); diff --git a/src/app/shared/search/search-settings/search-settings.component.ts b/src/app/shared/search/search-settings/search-settings.component.ts index 45d7c7b432..d85f234aa3 100644 --- a/src/app/shared/search/search-settings/search-settings.component.ts +++ b/src/app/shared/search/search-settings/search-settings.component.ts @@ -1,9 +1,8 @@ -import { Component, Inject, OnInit } from '@angular/core'; +import { Component, Inject, Input } from '@angular/core'; import { SearchService } from '../../../core/shared/search/search.service'; -import { SortDirection, SortOptions } from '../../../core/cache/models/sort-options.model'; +import { SortOptions } from '../../../core/cache/models/sort-options.model'; import { ActivatedRoute, NavigationExtras, Router } from '@angular/router'; import { PaginatedSearchOptions } from '../paginated-search-options.model'; -import { Observable } from 'rxjs'; import { SearchConfigurationService } from '../../../core/shared/search/search-configuration.service'; import { SEARCH_CONFIG_SERVICE } from '../../../+my-dspace-page/my-dspace-page.component'; @@ -16,16 +15,17 @@ import { SEARCH_CONFIG_SERVICE } from '../../../+my-dspace-page/my-dspace-page.c /** * This component represents the part of the search sidebar that contains the general search settings. */ -export class SearchSettingsComponent implements OnInit { +export class SearchSettingsComponent { + /** * The configuration for the current paginated search results */ - searchOptions$: Observable; + @Input() searchOptions: PaginatedSearchOptions; /** * All sort options that are shown in the settings */ - searchOptionPossibilities = [new SortOptions('score', SortDirection.DESC), new SortOptions('dc.title', SortDirection.ASC), new SortOptions('dc.title', SortDirection.DESC)]; + @Input() sortOptions: SortOptions[]; constructor(private service: SearchService, private route: ActivatedRoute, @@ -33,13 +33,6 @@ export class SearchSettingsComponent implements OnInit { @Inject(SEARCH_CONFIG_SERVICE) public searchConfigurationService: SearchConfigurationService) { } - /** - * Initialize paginated search options - */ - ngOnInit(): void { - this.searchOptions$ = this.searchConfigurationService.paginatedSearchOptions; - } - /** * Method to change the current sort field and direction * @param {Event} event Change event containing the sort direction and sort field diff --git a/src/app/shared/search/search-sidebar/search-sidebar.component.html b/src/app/shared/search/search-sidebar/search-sidebar.component.html index 74abeadfd8..624d094d22 100644 --- a/src/app/shared/search/search-sidebar/search-sidebar.component.html +++ b/src/app/shared/search/search-sidebar/search-sidebar.component.html @@ -12,7 +12,7 @@
- +
diff --git a/src/app/shared/search/search-sidebar/search-sidebar.component.ts b/src/app/shared/search/search-sidebar/search-sidebar.component.ts index 2060e0f345..7f134cacd3 100644 --- a/src/app/shared/search/search-sidebar/search-sidebar.component.ts +++ b/src/app/shared/search/search-sidebar/search-sidebar.component.ts @@ -2,6 +2,8 @@ import { Component, EventEmitter, Input, Output } from '@angular/core'; import { SearchConfigurationOption } from '../search-switch-configuration/search-configuration-option.model'; import { Observable } from 'rxjs'; +import { PaginatedSearchOptions } from '../paginated-search-options.model'; +import { SortOptions } from '../../../core/cache/models/sort-options.model'; /** * This component renders a simple item page. @@ -45,6 +47,16 @@ export class SearchSidebarComponent { */ @Input() inPlaceSearch; + /** + * The configuration for the current paginated search results + */ + @Input() searchOptions: PaginatedSearchOptions; + + /** + * All sort options that are shown in the settings + */ + @Input() sortOptions: SortOptions[]; + /** * Emits when the search filters values may be stale, and so they must be refreshed. */ From 4d85c0270f31332fad98e6effb68bf50e68b0ea7 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Tue, 6 Apr 2021 10:30:26 +0200 Subject: [PATCH 02/42] [CST-4009] sorting options translations --- src/assets/i18n/en.json5 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 2924a3a47e..20a21f93e8 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3037,8 +3037,13 @@ "sorting.dc.title.DESC": "Title Descending", - "sorting.score.DESC": "Relevance", + "sorting.score.ASC": "Relevance Ascending", + "sorting.score.DESC": "Relevance Descending", + + "sorting.dc.date.issued.ASC": "Date Issued Ascending", + + "sorting.dc.date.issued.DESC": "Date Issued Descending", "statistics.title": "Statistics", From b4686deb63e1df21fb0bf3889a4b32ac52e038d4 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 6 Apr 2021 17:49:06 +0200 Subject: [PATCH 03/42] [CST-4009] Retrieve sort options also for search page --- src/app/+search-page/search.component.html | 7 +++- src/app/+search-page/search.component.spec.ts | 24 ++++++++++- src/app/+search-page/search.component.ts | 40 ++++++++++++++++--- 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/app/+search-page/search.component.html b/src/app/+search-page/search.component.html index 7cb16caebe..d8aa25e4a3 100644 --- a/src/app/+search-page/search.component.html +++ b/src/app/+search-page/search.component.html @@ -31,11 +31,14 @@ + [searchOptions]="(searchOptions$ | async)" + [sortOptions]="(sortOptions$ | async)" + (toggleSidebar)="closeSidebar()"> diff --git a/src/app/+search-page/search.component.spec.ts b/src/app/+search-page/search.component.spec.ts index 989aed403d..06061c1d40 100644 --- a/src/app/+search-page/search.component.spec.ts +++ b/src/app/+search-page/search.component.spec.ts @@ -40,12 +40,14 @@ const pagination: PaginationComponentOptions = new PaginationComponentOptions(); pagination.id = 'search-results-pagination'; pagination.currentPage = 1; pagination.pageSize = 10; +const sortOption = { name: 'score', metadata: null }; const sort: SortOptions = new SortOptions('score', SortDirection.DESC); const mockResults = createSuccessfulRemoteDataObject$(['test', 'data']); const searchServiceStub = jasmine.createSpyObj('SearchService', { search: mockResults, getSearchLink: '/search', - getScopes: observableOf(['test-scope']) + getScopes: observableOf(['test-scope']), + getSearchConfigurationFor: createSuccessfulRemoteDataObject$({ sortOptions: [sortOption]}) }); const configurationParam = 'default'; const queryParam = 'test query'; @@ -181,4 +183,24 @@ describe('SearchComponent', () => { }); }); + + describe('when stable', () => { + + beforeEach(() => { + fixture.detectChanges(); + }); + + it('should have initialized the sortOptions$ observable', (done) => { + + comp.sortOptions$.subscribe((sortOptions) => { + + expect(sortOptions.length).toEqual(2); + expect(sortOptions[0]).toEqual(new SortOptions('score', SortDirection.ASC)); + expect(sortOptions[1]).toEqual(new SortOptions('score', SortDirection.DESC)); + done(); + }); + + }); + + }); }); diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index 84077ebdc8..43535278a1 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -1,14 +1,14 @@ import { ChangeDetectionStrategy, Component, Inject, Input, OnInit } from '@angular/core'; -import { BehaviorSubject, Observable, Subscription } from 'rxjs'; -import { startWith, switchMap, } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, Observable, Subscription } from 'rxjs'; +import { map, startWith, switchMap, take, } from 'rxjs/operators'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; import { DSpaceObject } from '../core/shared/dspace-object.model'; import { pushInOut } from '../shared/animations/push'; import { HostWindowService } from '../shared/host-window.service'; import { SidebarService } from '../shared/sidebar/sidebar.service'; -import { hasValue, isNotEmpty } from '../shared/empty.util'; -import { getFirstSucceededRemoteData } from '../core/shared/operators'; +import { hasValue, isEmpty } from '../shared/empty.util'; +import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators'; import { RouteService } from '../core/services/route.service'; import { SEARCH_CONFIG_SERVICE } from '../+my-dspace-page/my-dspace-page.component'; import { PaginatedSearchOptions } from '../shared/search/paginated-search-options.model'; @@ -18,6 +18,8 @@ import { SearchService } from '../core/shared/search/search.service'; import { currentPath } from '../shared/utils/route.utils'; import { Router } from '@angular/router'; import { Context } from '../core/shared/context.model'; +import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; +import { SearchConfig } from '../core/shared/search/search-filters/search-config.model'; @Component({ selector: 'ds-search', @@ -47,6 +49,11 @@ export class SearchComponent implements OnInit { */ searchOptions$: Observable; + /** + * The current available sort options + */ + sortOptions$: Observable; + /** * The current relevant scopes */ @@ -129,9 +136,32 @@ export class SearchComponent implements OnInit { this.scopeListRD$ = this.searchConfigService.getCurrentScope('').pipe( switchMap((scopeId) => this.service.getScopes(scopeId)) ); - if (!isNotEmpty(this.configuration$)) { + if (isEmpty(this.configuration$)) { this.configuration$ = this.routeService.getRouteParameterValue('configuration'); } + + this.sortOptions$ = this.configuration$.pipe( + switchMap((configuration) => this.service.getSearchConfigurationFor(null, configuration)), + getFirstSucceededRemoteDataPayload(), + map((searchConfig: SearchConfig) => { + const sortOptions = []; + searchConfig.sortOptions.forEach(sortOption => { + sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); + sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); + }); + console.log(searchConfig, sortOptions); + return sortOptions; + })); + + combineLatest([ + this.sortOptions$, + this.searchConfigService.paginatedSearchOptions + ]).pipe(take(1)) + .subscribe(([sortOptions, searchOptions]) => { + const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); + console.log(updateValue); + this.searchConfigService.paginatedSearchOptions.next(updateValue); + }); } /** From 9c85328b17889fa4ce33de334015ff0a84929e54 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 7 Apr 2021 15:45:22 +0200 Subject: [PATCH 04/42] 77998: xdescribe fixes --- .../browse-by-decorator.ts | 7 ++ .../browse-by-switcher.component.spec.ts | 11 +-- .../browse-by-switcher.component.ts | 10 +- .../shared/item-relationships-utils.ts | 6 ++ .../core/cache/builders/build-decorators.ts | 14 +++ .../core/cache/builders/link.service.spec.ts | 91 ++++++++----------- src/app/core/cache/builders/link.service.ts | 18 ++-- .../core/data/relationship.service.spec.ts | 12 +-- src/app/core/data/relationship.service.ts | 10 +- .../cookies/browser-klaro.service.spec.ts | 5 +- ...ta-representation-loader.component.spec.ts | 31 +++++-- ...etadata-representation-loader.component.ts | 14 ++- .../metadata-representation.decorator.ts | 7 ++ 13 files changed, 136 insertions(+), 100 deletions(-) diff --git a/src/app/+browse-by/+browse-by-switcher/browse-by-decorator.ts b/src/app/+browse-by/+browse-by-switcher/browse-by-decorator.ts index 0143377922..efb4a4a9f4 100644 --- a/src/app/+browse-by/+browse-by-switcher/browse-by-decorator.ts +++ b/src/app/+browse-by/+browse-by-switcher/browse-by-decorator.ts @@ -1,4 +1,6 @@ import { hasNoValue } from '../../shared/empty.util'; +import { InjectionToken } from '@angular/core'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; export enum BrowseByType { Title = 'title', @@ -8,6 +10,11 @@ export enum BrowseByType { export const DEFAULT_BROWSE_BY_TYPE = BrowseByType.Metadata; +export const BROWSE_BY_COMPONENT_FACTORY = new InjectionToken<(browseByType) => GenericConstructor>('getComponentByBrowseByType', { + providedIn: 'root', + factory: () => getComponentByBrowseByType +}); + const map = new Map(); /** diff --git a/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.spec.ts b/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.spec.ts index 545b766c88..f340237e26 100644 --- a/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.spec.ts +++ b/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.spec.ts @@ -2,12 +2,11 @@ import { BrowseBySwitcherComponent } from './browse-by-switcher.component'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import * as decorator from './browse-by-decorator'; import { BehaviorSubject } from 'rxjs'; import { environment } from '../../../environments/environment'; -import createSpy = jasmine.createSpy; +import { BROWSE_BY_COMPONENT_FACTORY } from './browse-by-decorator'; -xdescribe('BrowseBySwitcherComponent', () => { +describe('BrowseBySwitcherComponent', () => { let comp: BrowseBySwitcherComponent; let fixture: ComponentFixture; @@ -23,7 +22,8 @@ xdescribe('BrowseBySwitcherComponent', () => { TestBed.configureTestingModule({ declarations: [BrowseBySwitcherComponent], providers: [ - { provide: ActivatedRoute, useValue: activatedRouteStub } + { provide: ActivatedRoute, useValue: activatedRouteStub }, + { provide: BROWSE_BY_COMPONENT_FACTORY, useValue: jasmine.createSpy('getComponentByBrowseByType').and.returnValue(null) } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -32,7 +32,6 @@ xdescribe('BrowseBySwitcherComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(BrowseBySwitcherComponent); comp = fixture.componentInstance; - spyOnProperty(decorator, 'getComponentByBrowseByType').and.returnValue(createSpy('getComponentByItemType')); })); types.forEach((type) => { @@ -43,7 +42,7 @@ xdescribe('BrowseBySwitcherComponent', () => { }); it(`should call getComponentByBrowseByType with type "${type.type}"`, () => { - expect(decorator.getComponentByBrowseByType).toHaveBeenCalledWith(type.type); + expect((comp as any).getComponentByBrowseByType).toHaveBeenCalledWith(type.type); }); }); }); diff --git a/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.ts b/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.ts index 6cfeebf796..043a4ce90a 100644 --- a/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.ts +++ b/src/app/+browse-by/+browse-by-switcher/browse-by-switcher.component.ts @@ -1,10 +1,11 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, Inject, OnInit } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { Observable } from 'rxjs'; import { BrowseByTypeConfig } from '../../../config/browse-by-type-config.interface'; import { map } from 'rxjs/operators'; -import { getComponentByBrowseByType } from './browse-by-decorator'; +import { BROWSE_BY_COMPONENT_FACTORY } from './browse-by-decorator'; import { environment } from '../../../environments/environment'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; @Component({ selector: 'ds-browse-by-switcher', @@ -20,7 +21,8 @@ export class BrowseBySwitcherComponent implements OnInit { */ browseByComponent: Observable; - public constructor(protected route: ActivatedRoute) { + public constructor(protected route: ActivatedRoute, + @Inject(BROWSE_BY_COMPONENT_FACTORY) private getComponentByBrowseByType: (browseByType) => GenericConstructor) { } /** @@ -32,7 +34,7 @@ export class BrowseBySwitcherComponent implements OnInit { const id = params.id; return environment.browseBy.types.find((config: BrowseByTypeConfig) => config.id === id); }), - map((config: BrowseByTypeConfig) => getComponentByBrowseByType(config.type)) + map((config: BrowseByTypeConfig) => this.getComponentByBrowseByType(config.type)) ); } diff --git a/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts b/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts index a8e2136131..b4c3da2cdc 100644 --- a/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts +++ b/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts @@ -9,6 +9,12 @@ import { getFirstSucceededRemoteData } from '../../../../core/shared/operators'; import { hasValue } from '../../../../shared/empty.util'; +import { InjectionToken } from '@angular/core'; + +export const PAGINATED_RELATIONS_TO_ITEMS_OPERATOR = new InjectionToken<(thisId: string) => (source: Observable>>) => Observable>>>('paginatedRelationsToItems', { + providedIn: 'root', + factory: () => paginatedRelationsToItems +}); /** * Operator for comparing arrays using a mapping function diff --git a/src/app/core/cache/builders/build-decorators.ts b/src/app/core/cache/builders/build-decorators.ts index 1ea09b6684..b561ababde 100644 --- a/src/app/core/cache/builders/build-decorators.ts +++ b/src/app/core/cache/builders/build-decorators.ts @@ -8,6 +8,20 @@ import { TypedObject, getResourceTypeValueFor } from '../object-cache.reducer'; +import { InjectionToken } from '@angular/core'; + +export const DATA_SERVICE_FACTORY = new InjectionToken<(resourceType: ResourceType) => GenericConstructor>('getDataServiceFor', { + providedIn: 'root', + factory: () => getDataServiceFor +}); +export const LINK_DEFINITION_FACTORY = new InjectionToken<(source: GenericConstructor, linkName: keyof T['_links']) => LinkDefinition>('getLinkDefinition', { + providedIn: 'root', + factory: () => getLinkDefinition +}); +export const LINK_DEFINITION_MAP_FACTORY = new InjectionToken<(source: GenericConstructor) => Map>>('getLinkDefinitions', { + providedIn: 'root', + factory: () => getLinkDefinitions +}); const resolvedLinkKey = Symbol('resolvedLink'); diff --git a/src/app/core/cache/builders/link.service.spec.ts b/src/app/core/cache/builders/link.service.spec.ts index 1c41cfee86..a6d9c59492 100644 --- a/src/app/core/cache/builders/link.service.spec.ts +++ b/src/app/core/cache/builders/link.service.spec.ts @@ -5,15 +5,9 @@ import { FindListOptions } from '../../data/request.models'; import { HALLink } from '../../shared/hal-link.model'; import { HALResource } from '../../shared/hal-resource.model'; import { ResourceType } from '../../shared/resource-type'; -import * as decorators from './build-decorators'; import { LinkService } from './link.service'; - -const spyOnFunction = (obj: T, func: keyof T) => { - const spy = jasmine.createSpy(func as string); - spyOnProperty(obj, func, 'get').and.returnValue(spy); - - return spy; -}; +import { DATA_SERVICE_FACTORY, LINK_DEFINITION_FACTORY, LINK_DEFINITION_MAP_FACTORY } from './build-decorators'; +import { isEmpty } from 'rxjs/operators'; const TEST_MODEL = new ResourceType('testmodel'); let result: any; @@ -51,7 +45,7 @@ let testDataService: TestDataService; let testModel: TestModel; -xdescribe('LinkService', () => { +describe('LinkService', () => { let service: LinkService; beforeEach(() => { @@ -76,6 +70,30 @@ xdescribe('LinkService', () => { providers: [LinkService, { provide: TestDataService, useValue: testDataService + }, { + provide: DATA_SERVICE_FACTORY, + useValue: jasmine.createSpy('getDataServiceFor').and.returnValue(TestDataService), + }, { + provide: LINK_DEFINITION_FACTORY, + useValue: jasmine.createSpy('getLinkDefinition').and.returnValue({ + resourceType: TEST_MODEL, + linkName: 'predecessor', + propertyName: 'predecessor' + }), + }, { + provide: LINK_DEFINITION_MAP_FACTORY, + useValue: jasmine.createSpy('getLinkDefinitions').and.returnValue([ + { + resourceType: TEST_MODEL, + linkName: 'predecessor', + propertyName: 'predecessor', + }, + { + resourceType: TEST_MODEL, + linkName: 'successor', + propertyName: 'successor', + } + ]), }] }); service = TestBed.inject(LinkService); @@ -84,12 +102,6 @@ xdescribe('LinkService', () => { describe('resolveLink', () => { describe(`when the linkdefinition concerns a single object`, () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor' - }); - spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); }); it('should call dataservice.findByHref with the correct href and nested links', () => { @@ -98,13 +110,12 @@ xdescribe('LinkService', () => { }); describe(`when the linkdefinition concerns a list`, () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ + ((service as any).getLinkDefinition as jasmine.Spy).and.returnValue({ resourceType: TEST_MODEL, linkName: 'predecessor', propertyName: 'predecessor', isList: true }); - spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); service.resolveLink(testModel, followLink('predecessor', { some: 'options ' } as any, true, true, true, followLink('successor'))); }); it('should call dataservice.findAllByHref with the correct href, findListOptions, and nested links', () => { @@ -113,21 +124,15 @@ xdescribe('LinkService', () => { }); describe('either way', () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor' - }); - spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); result = service.resolveLink(testModel, followLink('predecessor', {}, true, true, true, followLink('successor'))); }); it('should call getLinkDefinition with the correct model and link', () => { - expect(decorators.getLinkDefinition).toHaveBeenCalledWith(testModel.constructor as any, 'predecessor'); + expect((service as any).getLinkDefinition).toHaveBeenCalledWith(testModel.constructor as any, 'predecessor'); }); it('should call getDataServiceFor with the correct resource type', () => { - expect(decorators.getDataServiceFor).toHaveBeenCalledWith(TEST_MODEL); + expect((service as any).getDataServiceFor).toHaveBeenCalledWith(TEST_MODEL); }); it('should return the model with the resolved link', () => { @@ -140,7 +145,7 @@ xdescribe('LinkService', () => { describe(`when the specified link doesn't exist on the model's class`, () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue(undefined); + ((service as any).getLinkDefinition as jasmine.Spy).and.returnValue(undefined); }); it('should throw an error', () => { expect(() => { @@ -151,12 +156,7 @@ xdescribe('LinkService', () => { describe(`when there is no dataservice for the resourcetype in the link`, () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor' - }); - spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(undefined); + ((service as any).getDataServiceFor as jasmine.Spy).and.returnValue(undefined); }); it('should throw an error', () => { expect(() => { @@ -188,18 +188,6 @@ xdescribe('LinkService', () => { beforeEach(() => { testModel.predecessor = 'predecessor value' as any; testModel.successor = 'successor value' as any; - spyOnFunction(decorators, 'getLinkDefinitions').and.returnValue([ - { - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor', - }, - { - resourceType: TEST_MODEL, - linkName: 'successor', - propertyName: 'successor', - } - ]); }); it('should return a new version of the object without any resolved links', () => { @@ -231,16 +219,10 @@ xdescribe('LinkService', () => { } } }); - spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); }); describe('resolving the available link', () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor' - }); result = service.resolveLinks(testModel, followLink('predecessor')); }); @@ -251,7 +233,7 @@ xdescribe('LinkService', () => { describe('resolving the missing link', () => { beforeEach(() => { - spyOnFunction(decorators, 'getLinkDefinition').and.returnValue({ + ((service as any).getLinkDefinition as jasmine.Spy).and.returnValue({ resourceType: TEST_MODEL, linkName: 'successor', propertyName: 'successor' @@ -259,8 +241,11 @@ xdescribe('LinkService', () => { result = service.resolveLinks(testModel, followLink('successor')); }); - it('should return the model with no resolved link', () => { - expect(result.successor).toBeUndefined(); + it('should resolve to an empty observable', (done) => { + result.successor.pipe(isEmpty()).subscribe((v) => { + expect(v).toEqual(true); + done(); + }); }); }); }); diff --git a/src/app/core/cache/builders/link.service.ts b/src/app/core/cache/builders/link.service.ts index 56a1154b77..29f8633da5 100644 --- a/src/app/core/cache/builders/link.service.ts +++ b/src/app/core/cache/builders/link.service.ts @@ -1,17 +1,18 @@ -import { Injectable, Injector } from '@angular/core'; +import { Inject, Injectable, Injector } from '@angular/core'; import { hasNoValue, hasValue, isNotEmpty } from '../../../shared/empty.util'; import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; import { GenericConstructor } from '../../shared/generic-constructor'; import { HALResource } from '../../shared/hal-resource.model'; import { - getDataServiceFor, - getLinkDefinition, - getLinkDefinitions, + DATA_SERVICE_FACTORY, + LINK_DEFINITION_FACTORY, + LINK_DEFINITION_MAP_FACTORY, LinkDefinition } from './build-decorators'; import { RemoteData } from '../../data/remote-data'; import { Observable } from 'rxjs/internal/Observable'; import { EMPTY } from 'rxjs'; +import { ResourceType } from '../../shared/resource-type'; /** * A Service to handle the resolving and removing @@ -24,6 +25,9 @@ export class LinkService { constructor( protected parentInjector: Injector, + @Inject(DATA_SERVICE_FACTORY) private getDataServiceFor: (resourceType: ResourceType) => GenericConstructor, + @Inject(LINK_DEFINITION_FACTORY) private getLinkDefinition: (source: GenericConstructor, linkName: keyof T['_links']) => LinkDefinition, + @Inject(LINK_DEFINITION_MAP_FACTORY) private getLinkDefinitions: (source: GenericConstructor) => Map>, ) { } @@ -49,12 +53,12 @@ export class LinkService { * @param linkToFollow the {@link FollowLinkConfig} to resolve */ public resolveLinkWithoutAttaching(model, linkToFollow: FollowLinkConfig): Observable> { - const matchingLinkDef = getLinkDefinition(model.constructor, linkToFollow.name); + const matchingLinkDef = this.getLinkDefinition(model.constructor, linkToFollow.name); if (hasNoValue(matchingLinkDef)) { throw new Error(`followLink('${linkToFollow.name}') was used for a ${model.constructor.name}, but there is no property on ${model.constructor.name} models with an @link() for ${linkToFollow.name}`); } else { - const provider = getDataServiceFor(matchingLinkDef.resourceType); + const provider = this.getDataServiceFor(matchingLinkDef.resourceType); if (hasNoValue(provider)) { throw new Error(`The @link() for ${linkToFollow.name} on ${model.constructor.name} models uses the resource type ${matchingLinkDef.resourceType.value.toUpperCase()}, but there is no service with an @dataService(${matchingLinkDef.resourceType.value.toUpperCase()}) annotation in order to retrieve it`); @@ -104,7 +108,7 @@ export class LinkService { */ public removeResolvedLinks(model: T): T { const result = Object.assign(new (model.constructor as GenericConstructor)(), model); - const linkDefs = getLinkDefinitions(model.constructor as GenericConstructor); + const linkDefs = this.getLinkDefinitions(model.constructor as GenericConstructor); if (isNotEmpty(linkDefs)) { linkDefs.forEach((linkDef: LinkDefinition) => { result[linkDef.propertyName] = undefined; diff --git a/src/app/core/data/relationship.service.spec.ts b/src/app/core/data/relationship.service.spec.ts index 0a8ec48464..0f7dd319c3 100644 --- a/src/app/core/data/relationship.service.spec.ts +++ b/src/app/core/data/relationship.service.spec.ts @@ -1,5 +1,4 @@ import { of as observableOf } from 'rxjs'; -import * as ItemRelationshipsUtils from '../../+item-page/simple/item-types/shared/item-relationships-utils'; import { followLink } from '../../shared/utils/follow-link-config.model'; import { ObjectCacheService } from '../cache/object-cache.service'; import { RelationshipType } from '../shared/item-relationships/relationship-type.model'; @@ -15,9 +14,9 @@ import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-servic import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { getMockRemoteDataBuildServiceHrefMap } from '../../shared/mocks/remote-data-build.service.mock'; import { getMockRequestService } from '../../shared/mocks/request.service.mock'; -import { createPaginatedList, spyOnOperator } from '../../shared/testing/utils.test'; +import { createPaginatedList } from '../../shared/testing/utils.test'; -xdescribe('RelationshipService', () => { +describe('RelationshipService', () => { let service: RelationshipService; let requestService: RequestService; @@ -132,7 +131,8 @@ xdescribe('RelationshipService', () => { null, null, null, - null + null, + jasmine.createSpy('paginatedRelationsToItems').and.returnValue((v) => v), ); } @@ -195,8 +195,6 @@ xdescribe('RelationshipService', () => { const rd$ = createSuccessfulRemoteDataObject$(relationsList); spyOn(service, 'getItemRelationshipsByLabel').and.returnValue(rd$); - - spyOnOperator(ItemRelationshipsUtils, 'paginatedRelationsToItems').and.returnValue((v) => v); }); it('should call getItemRelationshipsByLabel with the correct params', (done) => { @@ -225,7 +223,7 @@ xdescribe('RelationshipService', () => { mockLabel, mockOptions ).subscribe((result) => { - expect(ItemRelationshipsUtils.paginatedRelationsToItems).toHaveBeenCalledWith(mockItem.uuid); + expect((service as any).paginatedRelationsToItems).toHaveBeenCalledWith(mockItem.uuid); done(); }); }); diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index 95512be678..da1ff790df 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -1,11 +1,10 @@ import { HttpClient, HttpHeaders } from '@angular/common/http'; -import { Injectable } from '@angular/core'; +import { Inject, Injectable } from '@angular/core'; import { MemoizedSelector, select, Store } from '@ngrx/store'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, startWith, switchMap, take, tap } from 'rxjs/operators'; import { - compareArraysUsingIds, - paginatedRelationsToItems, + compareArraysUsingIds, PAGINATED_RELATIONS_TO_ITEMS_OPERATOR, relationsToItems } from '../../+item-page/simple/item-types/shared/item-relationships-utils'; import { AppState, keySelector } from '../../app.reducer'; @@ -87,7 +86,8 @@ export class RelationshipService extends DataService { protected notificationsService: NotificationsService, protected http: HttpClient, protected comparator: DefaultChangeAnalyzer, - protected appStore: Store) { + protected appStore: Store, + @Inject(PAGINATED_RELATIONS_TO_ITEMS_OPERATOR) private paginatedRelationsToItems: (thisId: string) => (source: Observable>>) => Observable>>) { super(); } @@ -254,7 +254,7 @@ export class RelationshipService extends DataService { * @param options */ getRelatedItemsByLabel(item: Item, label: string, options?: FindListOptions): Observable>> { - return this.getItemRelationshipsByLabel(item, label, options, true, true, followLink('leftItem'), followLink('rightItem'), followLink('relationshipType')).pipe(paginatedRelationsToItems(item.uuid)); + return this.getItemRelationshipsByLabel(item, label, options, true, true, followLink('leftItem'), followLink('rightItem'), followLink('relationshipType')).pipe(this.paginatedRelationsToItems(item.uuid)); } /** diff --git a/src/app/shared/cookies/browser-klaro.service.spec.ts b/src/app/shared/cookies/browser-klaro.service.spec.ts index 7cf8a190f4..2155fb1bad 100644 --- a/src/app/shared/cookies/browser-klaro.service.spec.ts +++ b/src/app/shared/cookies/browser-klaro.service.spec.ts @@ -1,5 +1,4 @@ import { TestBed } from '@angular/core/testing'; - import { BrowserKlaroService, COOKIE_MDFIELD } from './browser-klaro.service'; import { getMockTranslateService } from '../mocks/translate.service.mock'; import { of as observableOf } from 'rxjs'; @@ -13,7 +12,7 @@ import { getTestScheduler } from 'jasmine-marbles'; import { MetadataValue } from '../../core/shared/metadata.models'; import { cloneDeep } from 'lodash'; -xdescribe('BrowserKlaroService', () => { +describe('BrowserKlaroService', () => { let translateService; let ePersonService; let authService; @@ -81,7 +80,7 @@ xdescribe('BrowserKlaroService', () => { } } }, - apps: [{ + services: [{ name: appName, purposes: [purpose] }], diff --git a/src/app/shared/metadata-representation/metadata-representation-loader.component.spec.ts b/src/app/shared/metadata-representation/metadata-representation-loader.component.spec.ts index bd5031ddc5..7edf1a700e 100644 --- a/src/app/shared/metadata-representation/metadata-representation-loader.component.spec.ts +++ b/src/app/shared/metadata-representation/metadata-representation-loader.component.spec.ts @@ -1,15 +1,15 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; -import { ChangeDetectionStrategy, ComponentFactoryResolver, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; import { Context } from '../../core/shared/context.model'; import { MetadataRepresentation, MetadataRepresentationType } from '../../core/shared/metadata-representation/metadata-representation.model'; import { MetadataRepresentationLoaderComponent } from './metadata-representation-loader.component'; -import { PlainTextMetadataListElementComponent } from '../object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component'; -import { spyOnExported } from '../testing/utils.test'; import { MetadataRepresentationDirective } from './metadata-representation.directive'; -import * as metadataRepresentationDecorator from './metadata-representation.decorator'; +import { METADATA_REPRESENTATION_COMPONENT_FACTORY } from './metadata-representation.decorator'; +import { ThemeService } from '../theme-support/theme.service'; +import { PlainTextMetadataListElementComponent } from '../object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component'; const testType = 'TestType'; const testContext = Context.Search; @@ -29,16 +29,30 @@ class TestType implements MetadataRepresentation { } } -xdescribe('MetadataRepresentationLoaderComponent', () => { +describe('MetadataRepresentationLoaderComponent', () => { let comp: MetadataRepresentationLoaderComponent; let fixture: ComponentFixture; + let themeService: ThemeService; + const themeName = 'test-theme'; beforeEach(waitForAsync(() => { + themeService = jasmine.createSpyObj('themeService', { + getThemeName: themeName, + }); TestBed.configureTestingModule({ imports: [], declarations: [MetadataRepresentationLoaderComponent, PlainTextMetadataListElementComponent, MetadataRepresentationDirective], schemas: [NO_ERRORS_SCHEMA], - providers: [ComponentFactoryResolver] + providers: [ + { + provide: METADATA_REPRESENTATION_COMPONENT_FACTORY, + useValue: jasmine.createSpy('getMetadataRepresentationComponent').and.returnValue(PlainTextMetadataListElementComponent) + }, + { + provide: ThemeService, + useValue: themeService, + } + ] }).overrideComponent(MetadataRepresentationLoaderComponent, { set: { changeDetection: ChangeDetectionStrategy.Default, @@ -53,15 +67,12 @@ xdescribe('MetadataRepresentationLoaderComponent', () => { comp.mdRepresentation = new TestType(); comp.context = testContext; - - spyOnExported(metadataRepresentationDecorator, 'getMetadataRepresentationComponent').and.returnValue(PlainTextMetadataListElementComponent); fixture.detectChanges(); - })); describe('When the component is rendered', () => { it('should call the getMetadataRepresentationComponent function with the right entity type, representation type and context', () => { - expect(metadataRepresentationDecorator.getMetadataRepresentationComponent).toHaveBeenCalledWith(testType, testRepresentationType, testContext); + expect((comp as any).getMetadataRepresentationComponent).toHaveBeenCalledWith(testType, testRepresentationType, testContext, themeName); }); }); }); diff --git a/src/app/shared/metadata-representation/metadata-representation-loader.component.ts b/src/app/shared/metadata-representation/metadata-representation-loader.component.ts index 976dfdbda8..7077949809 100644 --- a/src/app/shared/metadata-representation/metadata-representation-loader.component.ts +++ b/src/app/shared/metadata-representation/metadata-representation-loader.component.ts @@ -1,6 +1,9 @@ -import { Component, ComponentFactoryResolver, Input, OnInit, ViewChild } from '@angular/core'; -import { MetadataRepresentation } from '../../core/shared/metadata-representation/metadata-representation.model'; -import { getMetadataRepresentationComponent } from './metadata-representation.decorator'; +import { Component, ComponentFactoryResolver, Inject, Input, OnInit, ViewChild } from '@angular/core'; +import { + MetadataRepresentation, + MetadataRepresentationType +} from '../../core/shared/metadata-representation/metadata-representation.model'; +import { METADATA_REPRESENTATION_COMPONENT_FACTORY } from './metadata-representation.decorator'; import { Context } from '../../core/shared/context.model'; import { GenericConstructor } from '../../core/shared/generic-constructor'; import { MetadataRepresentationListElementComponent } from '../object-list/metadata-representation-list-element/metadata-representation-list-element.component'; @@ -45,7 +48,8 @@ export class MetadataRepresentationLoaderComponent implements OnInit { constructor( private componentFactoryResolver: ComponentFactoryResolver, - private themeService: ThemeService + private themeService: ThemeService, + @Inject(METADATA_REPRESENTATION_COMPONENT_FACTORY) private getMetadataRepresentationComponent: (entityType: string, mdRepresentationType: MetadataRepresentationType, context: Context, theme: string) => GenericConstructor, ) { } @@ -68,6 +72,6 @@ export class MetadataRepresentationLoaderComponent implements OnInit { * @returns {string} */ private getComponent(): GenericConstructor { - return getMetadataRepresentationComponent(this.mdRepresentation.itemType, this.mdRepresentation.representationType, this.context, this.themeService.getThemeName()); + return this.getMetadataRepresentationComponent(this.mdRepresentation.itemType, this.mdRepresentation.representationType, this.context, this.themeService.getThemeName()); } } diff --git a/src/app/shared/metadata-representation/metadata-representation.decorator.ts b/src/app/shared/metadata-representation/metadata-representation.decorator.ts index 30bb507b49..0b5bea33d9 100644 --- a/src/app/shared/metadata-representation/metadata-representation.decorator.ts +++ b/src/app/shared/metadata-representation/metadata-representation.decorator.ts @@ -1,6 +1,13 @@ import { MetadataRepresentationType } from '../../core/shared/metadata-representation/metadata-representation.model'; import { hasNoValue, hasValue } from '../empty.util'; import { Context } from '../../core/shared/context.model'; +import { InjectionToken } from '@angular/core'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; + +export const METADATA_REPRESENTATION_COMPONENT_FACTORY = new InjectionToken<(entityType: string, mdRepresentationType: MetadataRepresentationType, context: Context, theme: string) => GenericConstructor>('getMetadataRepresentationComponent', { + providedIn: 'root', + factory: () => getMetadataRepresentationComponent +}); export const map = new Map(); From d54b7d9f7c8cbcffea2c9165e35861d4b7d2a1c1 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Thu, 8 Apr 2021 15:43:07 +0200 Subject: [PATCH 05/42] [CST-4009] fix sort options on pagination change --- .../my-dspace-page.component.ts | 29 +++------ src/app/+search-page/search.component.ts | 29 ++------- .../search/search-configuration.service.ts | 63 +++++++++++++++++-- src/assets/i18n/en.json5 | 4 ++ 4 files changed, 77 insertions(+), 48 deletions(-) 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 ba5c0e5cc7..de51d9afd3 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.ts @@ -31,6 +31,8 @@ import { SearchResult } from '../shared/search/search-result.model'; import { Context } from '../core/shared/context.model'; import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; import { SearchConfig } from '../core/shared/search/search-filters/search-config.model'; +import {RouteService} from '../core/services/route.service'; +import {Router} from '@angular/router'; export const MYDSPACE_ROUTE = '/mydspace'; export const SEARCH_CONFIG_SERVICE: InjectionToken = new InjectionToken('searchConfigurationService'); @@ -116,7 +118,9 @@ export class MyDSpacePageComponent implements OnInit { constructor(private service: SearchService, private sidebarService: SidebarService, private windowService: HostWindowService, - @Inject(SEARCH_CONFIG_SERVICE) public searchConfigService: MyDSpaceConfigurationService) { + @Inject(SEARCH_CONFIG_SERVICE) public searchConfigService: MyDSpaceConfigurationService, + private routeService: RouteService, + private router: Router) { this.isXsOrSm$ = this.windowService.isXsOrSm(); this.service.setServiceOptions(MyDSpaceResponseParsingService, MyDSpaceRequest); } @@ -158,26 +162,11 @@ export class MyDSpacePageComponent implements OnInit { }) ); - this.sortOptions$ = this.context$.pipe( - switchMap((context) => this.service.getSearchConfigurationFor(null, context)), - getFirstSucceededRemoteDataPayload(), - map((searchConfig: SearchConfig) => { - const sortOptions = []; - searchConfig.sortOptions.forEach(sortOption => { - sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); - sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); - }); - return sortOptions; - })); + const configuration$ = this.routeService.getRouteParameterValue('configuration'); - combineLatest([ - this.sortOptions$, - this.searchConfigService.paginatedSearchOptions - ]).pipe(take(1)) - .subscribe(([sortOptions, searchOptions]) => { - const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); - this.searchConfigService.paginatedSearchOptions.next(updateValue); - }); + this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(configuration$, this.service); + + this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$, this.router); } diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index 43535278a1..c5813a2fd1 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -16,10 +16,9 @@ import { SearchResult } from '../shared/search/search-result.model'; import { SearchConfigurationService } from '../core/shared/search/search-configuration.service'; import { SearchService } from '../core/shared/search/search.service'; import { currentPath } from '../shared/utils/route.utils'; -import { Router } from '@angular/router'; +import { Router} from '@angular/router'; import { Context } from '../core/shared/context.model'; -import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; -import { SearchConfig } from '../core/shared/search/search-filters/search-config.model'; +import { SortOptions } from '../core/cache/models/sort-options.model'; @Component({ selector: 'ds-search', @@ -140,28 +139,10 @@ export class SearchComponent implements OnInit { this.configuration$ = this.routeService.getRouteParameterValue('configuration'); } - this.sortOptions$ = this.configuration$.pipe( - switchMap((configuration) => this.service.getSearchConfigurationFor(null, configuration)), - getFirstSucceededRemoteDataPayload(), - map((searchConfig: SearchConfig) => { - const sortOptions = []; - searchConfig.sortOptions.forEach(sortOption => { - sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); - sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); - }); - console.log(searchConfig, sortOptions); - return sortOptions; - })); + this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(this.configuration$, this.service); + + this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$, this.router); - combineLatest([ - this.sortOptions$, - this.searchConfigService.paginatedSearchOptions - ]).pipe(take(1)) - .subscribe(([sortOptions, searchOptions]) => { - const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); - console.log(updateValue); - this.searchConfigService.paginatedSearchOptions.next(updateValue); - }); } /** diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index edd3982319..c114ee6616 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -1,8 +1,15 @@ import { Injectable, OnDestroy } from '@angular/core'; -import { ActivatedRoute, Params } from '@angular/router'; +import { ActivatedRoute, NavigationExtras, Params, Router } from '@angular/router'; -import { BehaviorSubject, combineLatest as observableCombineLatest, merge as observableMerge, Observable, Subscription } from 'rxjs'; -import { filter, map, startWith } from 'rxjs/operators'; +import { + BehaviorSubject, + combineLatest, + combineLatest as observableCombineLatest, + merge as observableMerge, + Observable, + Subscription +} from 'rxjs'; +import { distinctUntilChanged, filter, map, startWith, switchMap, take } from 'rxjs/operators'; import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model'; import { SearchOptions } from '../../../shared/search/search-options.model'; import { PaginatedSearchOptions } from '../../../shared/search/paginated-search-options.model'; @@ -11,9 +18,12 @@ import { RemoteData } from '../../data/remote-data'; import { DSpaceObjectType } from '../dspace-object-type.model'; import { SortDirection, SortOptions } from '../../cache/models/sort-options.model'; import { RouteService } from '../../services/route.service'; -import { getFirstSucceededRemoteData } from '../operators'; +import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../operators'; import { hasNoValue, hasValue, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util'; import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { SearchConfig } from './search-filters/search-config.model'; +import { SearchService } from './search.service'; +import { of } from 'rxjs/internal/observable/of'; /** * Service that performs all actions that have to do with the current search configuration @@ -209,6 +219,51 @@ export class SearchConfigurationService implements OnDestroy { return this.routeService.getQueryParamsWithPrefix('f.'); } + /** + * Creates an observable of SortOptions[] every time the configuration$ stream emits. + * @param configuration$ + * @param service + */ + getConfigurationSortOptionsObservable(configuration$: Observable, service: SearchService): Observable { + return configuration$.pipe( + distinctUntilChanged(), + switchMap((configuration) => service.getSearchConfigurationFor(null, configuration)), + getFirstSucceededRemoteDataPayload(), + map((searchConfig: SearchConfig) => { + const sortOptions = []; + searchConfig.sortOptions.forEach(sortOption => { + sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); + sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); + }); + return sortOptions; + })); + } + + /** + * Every time sortOptions change (after a configuration change) it update the navigation with the default sort option + * and emit the new paginateSearchOptions value. + * @param configuration$ + * @param service + */ + initializeSortOptionsFromConfiguration(sortOptions$: Observable, router: Router) { + const subscription = sortOptions$.pipe(switchMap((sortOptions) => combineLatest([ + of(sortOptions), + this.paginatedSearchOptions.pipe(take(1)) + ]))).subscribe(([sortOptions, searchOptions]) => { + const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); + const navigationExtras: NavigationExtras = { + queryParams: { + sortDirection: updateValue.sort.direction, + sortField: updateValue.sort.field, + }, + queryParamsHandling: 'merge' + }; + router.navigate([], navigationExtras); + this.paginatedSearchOptions.next(updateValue); + }); + this.subs.push(subscription); + } + /** * Sets up a subscription to all necessary parameters to make sure the searchOptions emits a new value every time they update * @param {SearchOptions} defaults Default values for when no parameters are available diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 20a21f93e8..bcf2a92666 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3045,6 +3045,10 @@ "sorting.dc.date.issued.DESC": "Date Issued Descending", + "sorting.dc.date.accessioned.ASC": "Accessioned Date Ascending", + + "sorting.dc.date.accessioned.DESC": "Accessioned Date Descending", + "statistics.title": "Statistics", From 23fe338c5d68fc15e3e2f3abfb6af0ec825f1a69 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Thu, 8 Apr 2021 16:18:31 +0200 Subject: [PATCH 06/42] [CST-4009] fix sort options on pagination change --- .../+my-dspace-page/my-dspace-page.component.ts | 17 +++++++---------- src/app/+search-page/search.component.ts | 2 +- .../search/search-configuration.service.ts | 13 +++++-------- 3 files changed, 13 insertions(+), 19 deletions(-) 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 de51d9afd3..b95a296ed1 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.ts @@ -7,8 +7,8 @@ import { OnInit } from '@angular/core'; -import { BehaviorSubject, combineLatest, Observable, Subject, Subscription } from 'rxjs'; -import { map, switchMap, take, tap } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs'; +import { map, switchMap, tap } from 'rxjs/operators'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; @@ -19,7 +19,7 @@ import { PaginatedSearchOptions } from '../shared/search/paginated-search-option import { SearchService } from '../core/shared/search/search.service'; import { SidebarService } from '../shared/sidebar/sidebar.service'; import { hasValue } from '../shared/empty.util'; -import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators'; +import { getFirstSucceededRemoteData } from '../core/shared/operators'; import { MyDSpaceResponseParsingService } from '../core/data/mydspace-response-parsing.service'; import { SearchConfigurationOption } from '../shared/search/search-switch-configuration/search-configuration-option.model'; import { RoleType } from '../core/roles/role-types'; @@ -29,10 +29,8 @@ import { ViewMode } from '../core/shared/view-mode.model'; import { MyDSpaceRequest } from '../core/data/request.models'; import { SearchResult } from '../shared/search/search-result.model'; import { Context } from '../core/shared/context.model'; -import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; -import { SearchConfig } from '../core/shared/search/search-filters/search-config.model'; -import {RouteService} from '../core/services/route.service'; -import {Router} from '@angular/router'; +import { SortOptions } from '../core/cache/models/sort-options.model'; +import { RouteService } from '../core/services/route.service'; export const MYDSPACE_ROUTE = '/mydspace'; export const SEARCH_CONFIG_SERVICE: InjectionToken = new InjectionToken('searchConfigurationService'); @@ -119,8 +117,7 @@ export class MyDSpacePageComponent implements OnInit { private sidebarService: SidebarService, private windowService: HostWindowService, @Inject(SEARCH_CONFIG_SERVICE) public searchConfigService: MyDSpaceConfigurationService, - private routeService: RouteService, - private router: Router) { + private routeService: RouteService) { this.isXsOrSm$ = this.windowService.isXsOrSm(); this.service.setServiceOptions(MyDSpaceResponseParsingService, MyDSpaceRequest); } @@ -166,7 +163,7 @@ export class MyDSpacePageComponent implements OnInit { this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(configuration$, this.service); - this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$, this.router); + this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$); } diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index c5813a2fd1..bffa958ee5 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -141,7 +141,7 @@ export class SearchComponent implements OnInit { this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(this.configuration$, this.service); - this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$, this.router); + this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$); } diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index 339eefdc6b..e10ab668cc 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -1,5 +1,5 @@ import { Injectable, OnDestroy } from '@angular/core'; -import { ActivatedRoute, NavigationExtras, Params, Router } from '@angular/router'; +import { ActivatedRoute, Params } from '@angular/router'; import { BehaviorSubject, @@ -230,20 +230,17 @@ export class SearchConfigurationService implements OnDestroy { * @param configuration$ * @param service */ - initializeSortOptionsFromConfiguration(sortOptions$: Observable, router: Router) { + initializeSortOptionsFromConfiguration(sortOptions$: Observable) { const subscription = sortOptions$.pipe(switchMap((sortOptions) => combineLatest([ of(sortOptions), this.paginatedSearchOptions.pipe(take(1)) ]))).subscribe(([sortOptions, searchOptions]) => { const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); - const navigationExtras: NavigationExtras = { - queryParams: { + this.paginationService.updateRoute(this.paginationID, + { sortDirection: updateValue.sort.direction, sortField: updateValue.sort.field, - }, - queryParamsHandling: 'merge' - }; - router.navigate([], navigationExtras); + }); this.paginatedSearchOptions.next(updateValue); }); this.subs.push(subscription); From a2e00bbd9f8217590242584e03a38507bc2d5a72 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 8 Apr 2021 17:54:27 +0200 Subject: [PATCH 07/42] 78243: edit item page fine-grained permission checks --- .../collection-page-administrator.guard.ts | 4 +- .../community-page-administrator.guard.ts | 4 +- .../edit-item-page.routing.module.ts | 26 ++++++--- .../item-operation.component.html | 18 +++--- .../item-operation/itemOperation.model.ts | 10 +++- .../item-page-bitstreams.guard.ts | 31 +++++++++++ .../item-page-collection-mapper.guard.ts | 31 +++++++++++ .../item-page-metadata.guard.ts} | 14 ++--- .../item-page-reinstate.guard.ts | 4 +- .../item-page-relationships.guard.ts | 31 +++++++++++ .../edit-item-page/item-page-status.guard.ts | 32 +++++++++++ .../item-page-version-history.guard.ts | 31 +++++++++++ .../item-page-withdraw.guard.ts | 4 +- .../item-status/item-status.component.ts | 55 ++++++++----------- .../item-page-administrator.guard.ts | 4 +- .../collection-administrator.guard.ts | 4 +- .../community-administrator.guard.ts | 4 +- ... => dso-page-single-feature.guard.spec.ts} | 6 +- .../dso-page-single-feature.guard.ts | 27 +++++++++ ...uard.ts => dso-page-some-feature.guard.ts} | 4 +- .../group-administrator.guard.ts | 4 +- ...ingle-feature-authorization.guard.spec.ts} | 6 +- .../single-feature-authorization.guard.ts | 27 +++++++++ .../site-administrator.guard.ts | 4 +- .../site-register.guard.ts | 4 +- ...ts => some-feature-authorization.guard.ts} | 22 ++++---- .../data/feature-authorization/feature-id.ts | 7 +++ src/app/core/shared/operators.ts | 19 ++++++- .../endpoint-mocking-rest.service.ts | 10 +++- ...e-item-can-manage-bitstreams-response.json | 51 +++++++++++++++++ .../dspace-rest/mocks/response-map.mock.ts | 2 + src/assets/i18n/en.json5 | 2 + 32 files changed, 402 insertions(+), 100 deletions(-) create mode 100644 src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts create mode 100644 src/app/+item-page/edit-item-page/item-page-collection-mapper.guard.ts rename src/app/+item-page/{item-page-edit-metadata.guard.ts => edit-item-page/item-page-metadata.guard.ts} (59%) create mode 100644 src/app/+item-page/edit-item-page/item-page-relationships.guard.ts create mode 100644 src/app/+item-page/edit-item-page/item-page-status.guard.ts create mode 100644 src/app/+item-page/edit-item-page/item-page-version-history.guard.ts rename src/app/core/data/feature-authorization/feature-authorization-guard/{dso-page-feature.guard.spec.ts => dso-page-single-feature.guard.spec.ts} (93%) create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.ts rename src/app/core/data/feature-authorization/feature-authorization-guard/{dso-page-feature.guard.ts => dso-page-some-feature.guard.ts} (90%) rename src/app/core/data/feature-authorization/feature-authorization-guard/{feature-authorization.guard.spec.ts => single-feature-authorization.guard.spec.ts} (92%) create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.ts rename src/app/core/data/feature-authorization/feature-authorization-guard/{feature-authorization.guard.ts => some-feature-authorization.guard.ts} (64%) create mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-bitstreams-response.json diff --git a/src/app/+collection-page/collection-page-administrator.guard.ts b/src/app/+collection-page/collection-page-administrator.guard.ts index 748cca81cb..c7866515b2 100644 --- a/src/app/+collection-page/collection-page-administrator.guard.ts +++ b/src/app/+collection-page/collection-page-administrator.guard.ts @@ -4,7 +4,7 @@ import { Collection } from '../core/shared/collection.model'; import { CollectionPageResolver } from './collection-page.resolver'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { Observable, of as observableOf } from 'rxjs'; -import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; import { AuthService } from '../core/auth/auth.service'; @@ -14,7 +14,7 @@ import { AuthService } from '../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Collection} pages requiring administrator rights */ -export class CollectionPageAdministratorGuard extends DsoPageFeatureGuard { +export class CollectionPageAdministratorGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: CollectionPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/+community-page/community-page-administrator.guard.ts b/src/app/+community-page/community-page-administrator.guard.ts index fad4a78f07..fd7ce5f7bf 100644 --- a/src/app/+community-page/community-page-administrator.guard.ts +++ b/src/app/+community-page/community-page-administrator.guard.ts @@ -4,7 +4,7 @@ import { Community } from '../core/shared/community.model'; import { CommunityPageResolver } from './community-page.resolver'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { Observable, of as observableOf } from 'rxjs'; -import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; import { AuthService } from '../core/auth/auth.service'; @@ -14,7 +14,7 @@ import { AuthService } from '../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Community} pages requiring administrator rights */ -export class CommunityPageAdministratorGuard extends DsoPageFeatureGuard { +export class CommunityPageAdministratorGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: CommunityPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts index b7d650d8c3..2535e42216 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts @@ -31,8 +31,13 @@ import { } from './edit-item-page.routing-paths'; import { ItemPageReinstateGuard } from './item-page-reinstate.guard'; import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; -import { ItemPageEditMetadataGuard } from '../item-page-edit-metadata.guard'; +import { ItemPageMetadataGuard } from './item-page-metadata.guard'; import { ItemPageAdministratorGuard } from '../item-page-administrator.guard'; +import { ItemPageStatusGuard } from './item-page-status.guard'; +import { ItemPageBitstreamsGuard } from './item-page-bitstreams.guard'; +import { ItemPageRelationshipsGuard } from './item-page-relationships.guard'; +import { ItemPageVersionHistoryGuard } from './item-page-version-history.guard'; +import { ItemPageCollectionMapperGuard } from './item-page-collection-mapper.guard'; /** * Routing module that handles the routing for the Edit Item page administrator functionality @@ -60,25 +65,25 @@ import { ItemPageAdministratorGuard } from '../item-page-administrator.guard'; path: 'status', component: ItemStatusComponent, data: { title: 'item.edit.tabs.status.title', showBreadcrumbs: true }, - canActivate: [ItemPageAdministratorGuard] + canActivate: [ItemPageStatusGuard] }, { path: 'bitstreams', component: ItemBitstreamsComponent, data: { title: 'item.edit.tabs.bitstreams.title', showBreadcrumbs: true }, - canActivate: [ItemPageAdministratorGuard] + canActivate: [ItemPageBitstreamsGuard] }, { path: 'metadata', component: ItemMetadataComponent, data: { title: 'item.edit.tabs.metadata.title', showBreadcrumbs: true }, - canActivate: [ItemPageEditMetadataGuard] + canActivate: [ItemPageMetadataGuard] }, { path: 'relationships', component: ItemRelationshipsComponent, data: { title: 'item.edit.tabs.relationships.title', showBreadcrumbs: true }, - canActivate: [ItemPageEditMetadataGuard] + canActivate: [ItemPageRelationshipsGuard] }, /* TODO - uncomment & fix when view page exists { @@ -96,13 +101,13 @@ import { ItemPageAdministratorGuard } from '../item-page-administrator.guard'; path: 'versionhistory', component: ItemVersionHistoryComponent, data: { title: 'item.edit.tabs.versionhistory.title', showBreadcrumbs: true }, - canActivate: [ItemPageAdministratorGuard] + canActivate: [ItemPageVersionHistoryGuard] }, { path: 'mapper', component: ItemCollectionMapperComponent, data: { title: 'item.edit.tabs.item-mapper.title', showBreadcrumbs: true }, - canActivate: [ItemPageAdministratorGuard] + canActivate: [ItemPageCollectionMapperGuard] } ] }, @@ -175,7 +180,12 @@ import { ItemPageAdministratorGuard } from '../item-page-administrator.guard'; ItemPageReinstateGuard, ItemPageWithdrawGuard, ItemPageAdministratorGuard, - ItemPageEditMetadataGuard, + ItemPageMetadataGuard, + ItemPageStatusGuard, + ItemPageBitstreamsGuard, + ItemPageRelationshipsGuard, + ItemPageVersionHistoryGuard, + ItemPageCollectionMapperGuard, ] }) export class EditItemPageRoutingModule { diff --git a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html index e3989f02f3..ecbc19aea8 100644 --- a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html +++ b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html @@ -3,13 +3,15 @@ {{'item.edit.tabs.status.buttons.' + operation.operationKey + '.label' | translate}} -
- - {{'item.edit.tabs.status.buttons.' + operation.operationKey + '.button' | translate}} - -
-
- - {{'item.edit.tabs.status.buttons.' + operation.operationKey + '.button' | translate}} +
+ + + + +
diff --git a/src/app/+item-page/edit-item-page/item-operation/itemOperation.model.ts b/src/app/+item-page/edit-item-page/item-operation/itemOperation.model.ts index 105889d42d..33302dcba6 100644 --- a/src/app/+item-page/edit-item-page/item-operation/itemOperation.model.ts +++ b/src/app/+item-page/edit-item-page/item-operation/itemOperation.model.ts @@ -1,3 +1,5 @@ +import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; + /** * Represents an item operation used on the edit item page with a key, an operation URL to which will be navigated * when performing the action and an option to disable the operation. @@ -7,11 +9,15 @@ export class ItemOperation { operationKey: string; operationUrl: string; disabled: boolean; + authorized: boolean; + featureID: FeatureID; - constructor(operationKey: string, operationUrl: string) { + constructor(operationKey: string, operationUrl: string, featureID?: FeatureID, disabled = false, authorized = true) { this.operationKey = operationKey; this.operationUrl = operationUrl; - this.setDisabled(false); + this.featureID = featureID; + this.authorized = authorized; + this.setDisabled(disabled); } /** diff --git a/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts b/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts new file mode 100644 index 0000000000..bf4a6dc681 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; +import { Item } from '../../core/shared/item.model'; +import { ItemPageResolver } from '../item-page.resolver'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage bitstreams rights + */ +export class ItemPageBitstreamsGuard extends DsoPageSingleFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check manage bitstreams authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.CanManageBitstreams); + } +} diff --git a/src/app/+item-page/edit-item-page/item-page-collection-mapper.guard.ts b/src/app/+item-page/edit-item-page/item-page-collection-mapper.guard.ts new file mode 100644 index 0000000000..2380377aea --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-collection-mapper.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from '../item-page.resolver'; +import { Item } from '../../core/shared/item.model'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage mappings rights + */ +export class ItemPageCollectionMapperGuard extends DsoPageSingleFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check manage mappings authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.CanManageMappings); + } +} diff --git a/src/app/+item-page/item-page-edit-metadata.guard.ts b/src/app/+item-page/edit-item-page/item-page-metadata.guard.ts similarity index 59% rename from src/app/+item-page/item-page-edit-metadata.guard.ts rename to src/app/+item-page/edit-item-page/item-page-metadata.guard.ts index a9b870b1cd..a6846bec4e 100644 --- a/src/app/+item-page/item-page-edit-metadata.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-metadata.guard.ts @@ -1,12 +1,12 @@ import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; -import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; -import { ItemPageResolver } from './item-page.resolver'; -import { Item } from '../core/shared/item.model'; -import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from '../item-page.resolver'; +import { Item } from '../../core/shared/item.model'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { Observable, of as observableOf } from 'rxjs'; -import { FeatureID } from '../core/data/feature-authorization/feature-id'; -import { AuthService } from '../core/auth/auth.service'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -14,7 +14,7 @@ import { AuthService } from '../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring edit metadata rights */ -export class ItemPageEditMetadataGuard extends DsoPageFeatureGuard { +export class ItemPageMetadataGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts index 0288e30b0a..88c9c20b12 100644 --- a/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { DsoPageFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { Item } from '../../core/shared/item.model'; import { ItemPageResolver } from '../item-page.resolver'; import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; @@ -14,7 +14,7 @@ import { AuthService } from '../../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring reinstate rights */ -export class ItemPageReinstateGuard extends DsoPageFeatureGuard { +export class ItemPageReinstateGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/+item-page/edit-item-page/item-page-relationships.guard.ts b/src/app/+item-page/edit-item-page/item-page-relationships.guard.ts new file mode 100644 index 0000000000..77da92ae02 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-relationships.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from '../item-page.resolver'; +import { Item } from '../../core/shared/item.model'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage relationships rights + */ +export class ItemPageRelationshipsGuard extends DsoPageSingleFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check manage relationships authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.CanManageRelationships); + } +} diff --git a/src/app/+item-page/edit-item-page/item-page-status.guard.ts b/src/app/+item-page/edit-item-page/item-page-status.guard.ts new file mode 100644 index 0000000000..98f963a4be --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-status.guard.ts @@ -0,0 +1,32 @@ +import { Injectable } from '@angular/core'; +import { Item } from '../../core/shared/item.model'; +import { ItemPageResolver } from '../item-page.resolver'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; +import { DsoPageSomeFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring any of the rights required for + * the status page + */ +export class ItemPageStatusGuard extends DsoPageSomeFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check authorization rights + */ + getFeatureIDs(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf([FeatureID.CanManageMappings, FeatureID.WithdrawItem, FeatureID.ReinstateItem, FeatureID.CanManagePolicies, FeatureID.CanMakePrivate, FeatureID.CanDelete, FeatureID.CanMove]); + } +} diff --git a/src/app/+item-page/edit-item-page/item-page-version-history.guard.ts b/src/app/+item-page/edit-item-page/item-page-version-history.guard.ts new file mode 100644 index 0000000000..dccdd9e641 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-version-history.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from '../item-page.resolver'; +import { Item } from '../../core/shared/item.model'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { AuthService } from '../../core/auth/auth.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage versions rights + */ +export class ItemPageVersionHistoryGuard extends DsoPageSingleFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check manage versions authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.CanManageVersions); + } +} diff --git a/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts index 243f751974..de9b7d0147 100644 --- a/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts @@ -1,4 +1,4 @@ -import { DsoPageFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { Item } from '../../core/shared/item.model'; import { Injectable } from '@angular/core'; import { ItemPageResolver } from '../item-page.resolver'; @@ -14,7 +14,7 @@ import { AuthService } from '../../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring withdraw rights */ -export class ItemPageWithdrawGuard extends DsoPageFeatureGuard { +export class ItemPageWithdrawGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index 2745fc8df7..51aa24ea6c 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -4,7 +4,7 @@ import { Item } from '../../../core/shared/item.model'; import { ActivatedRoute } from '@angular/router'; import { ItemOperation } from '../item-operation/itemOperation.model'; import { distinctUntilChanged, first, map } from 'rxjs/operators'; -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, of } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { getItemEditRoute, getItemPageRoute } from '../../item-page-routing-paths'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; @@ -78,42 +78,33 @@ export class ItemStatusComponent implements OnInit { The value is supposed to be a href for the button */ const operations = []; - operations.push(new ItemOperation('authorizations', this.getCurrentUrl(item) + '/authorizations')); - operations.push(new ItemOperation('mappedCollections', this.getCurrentUrl(item) + '/mapper')); - operations.push(undefined); - // Store the index of the "withdraw" or "reinstate" operation, because it's added asynchronously - const indexOfWithdrawReinstate = operations.length - 1; - if (item.isDiscoverable) { - operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private')); + operations.push(new ItemOperation('authorizations', this.getCurrentUrl(item) + '/authorizations', FeatureID.CanManagePolicies, true)); + operations.push(new ItemOperation('mappedCollections', this.getCurrentUrl(item) + '/mapper', FeatureID.CanManageMappings, true)); + if (item.isWithdrawn) { + operations.push(new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate', FeatureID.ReinstateItem, true)); } else { - operations.push(new ItemOperation('public', this.getCurrentUrl(item) + '/public')); + operations.push(new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate', FeatureID.WithdrawItem, true)); } - operations.push(new ItemOperation('delete', this.getCurrentUrl(item) + '/delete')); - operations.push(new ItemOperation('move', this.getCurrentUrl(item) + '/move')); + if (item.isDiscoverable) { + operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private', FeatureID.CanMakePrivate, true)); + } else { + operations.push(new ItemOperation('public', this.getCurrentUrl(item) + '/public', FeatureID.CanMakePrivate, true)); + } + operations.push(new ItemOperation('delete', this.getCurrentUrl(item) + '/delete', FeatureID.CanDelete, true)); + operations.push(new ItemOperation('move', this.getCurrentUrl(item) + '/move', FeatureID.CanMove, true)); this.operations$.next(operations); - if (item.isWithdrawn) { - this.authorizationService.isAuthorized(FeatureID.ReinstateItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { - const newOperations = [...this.operations$.value]; - if (authorized) { - newOperations[indexOfWithdrawReinstate] = new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate'); - } else { - newOperations[indexOfWithdrawReinstate] = undefined; - } - this.operations$.next(newOperations); - }); - } else { - this.authorizationService.isAuthorized(FeatureID.WithdrawItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { - const newOperations = [...this.operations$.value]; - if (authorized) { - newOperations[indexOfWithdrawReinstate] = new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw'); - } else { - newOperations[indexOfWithdrawReinstate] = undefined; - } - this.operations$.next(newOperations); - }); - } + observableCombineLatest(operations.map((operation) => { + if (hasValue(operation.featureID)) { + return this.authorizationService.isAuthorized(operation.featureID, item.self).pipe( + distinctUntilChanged(), + map((authorized) => new ItemOperation(operation.operationKey, operation.operationUrl, operation.featureID, !authorized, authorized)) + ); + } else { + return of(operation); + } + })).subscribe((ops) => this.operations$.next(ops)); }); this.itemPageRoute$ = this.itemRD$.pipe( getAllSucceededRemoteDataPayload(), diff --git a/src/app/+item-page/item-page-administrator.guard.ts b/src/app/+item-page/item-page-administrator.guard.ts index c90502472e..5d3464aa75 100644 --- a/src/app/+item-page/item-page-administrator.guard.ts +++ b/src/app/+item-page/item-page-administrator.guard.ts @@ -3,7 +3,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { ItemPageResolver } from './item-page.resolver'; import { Item } from '../core/shared/item.model'; -import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard'; import { Observable, of as observableOf } from 'rxjs'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; import { AuthService } from '../core/auth/auth.service'; @@ -14,7 +14,7 @@ import { AuthService } from '../core/auth/auth.service'; /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring administrator rights */ -export class ItemPageAdministratorGuard extends DsoPageFeatureGuard { +export class ItemPageAdministratorGuard extends DsoPageSingleFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard.ts index bc39397ed9..b41a322cb6 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { AuthService } from '../../../auth/auth.service'; @@ -13,7 +13,7 @@ import { FeatureID } from '../feature-id'; @Injectable({ providedIn: 'root' }) -export class CollectionAdministratorGuard extends FeatureAuthorizationGuard { +export class CollectionAdministratorGuard extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { super(authorizationService, router, authService); } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/community-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/community-administrator.guard.ts index afb1fea63d..2ab77a00cc 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/community-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/community-administrator.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { AuthService } from '../../../auth/auth.service'; @@ -13,7 +13,7 @@ import { FeatureID } from '../feature-id'; @Injectable({ providedIn: 'root' }) -export class CommunityAdministratorGuard extends FeatureAuthorizationGuard { +export class CommunityAdministratorGuard extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { super(authorizationService, router, authService); } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts similarity index 93% rename from src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts index f98e3f1837..06677d1f22 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts @@ -4,14 +4,14 @@ import { RemoteData } from '../../remote-data'; import { Observable, of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { DSpaceObject } from '../../../shared/dspace-object.model'; -import { DsoPageFeatureGuard } from './dso-page-feature.guard'; +import { DsoPageSingleFeatureGuard } from './dso-page-single-feature.guard'; import { FeatureID } from '../feature-id'; import { AuthService } from '../../../auth/auth.service'; /** * Test implementation of abstract class DsoPageAdministratorGuard */ -class DsoPageFeatureGuardImpl extends DsoPageFeatureGuard { +class DsoPageFeatureGuardImpl extends DsoPageSingleFeatureGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, protected router: Router, @@ -26,7 +26,7 @@ class DsoPageFeatureGuardImpl extends DsoPageFeatureGuard { } describe('DsoPageAdministratorGuard', () => { - let guard: DsoPageFeatureGuard; + let guard: DsoPageSingleFeatureGuard; let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.ts new file mode 100644 index 0000000000..3fc90f9069 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.ts @@ -0,0 +1,27 @@ +import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { DsoPageSomeFeatureGuard } from './dso-page-some-feature.guard'; +import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +import { FeatureID } from '../feature-id'; +import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; + +/** + * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature + * This guard utilizes a resolver to retrieve the relevant object to check authorizations for + */ +export abstract class DsoPageSingleFeatureGuard extends DsoPageSomeFeatureGuard { + /** + * The features to check authorization for + */ + getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return this.getFeatureID(route, state).pipe( + map((featureID) => [featureID]), + ); + } + + /** + * The type of feature to check authorization for + * Override this method to define a feature + */ + abstract getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; +} diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts similarity index 90% rename from src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts index c50dd7f95d..7b7cb4c196 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts @@ -5,15 +5,15 @@ import { Observable } from 'rxjs'; import { getAllSucceededRemoteDataPayload } from '../../../shared/operators'; import { map } from 'rxjs/operators'; import { DSpaceObject } from '../../../shared/dspace-object.model'; -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; import { AuthService } from '../../../auth/auth.service'; import { hasNoValue, hasValue } from '../../../../shared/empty.util'; +import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; /** * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature * This guard utilizes a resolver to retrieve the relevant object to check authorizations for */ -export abstract class DsoPageFeatureGuard extends FeatureAuthorizationGuard { +export abstract class DsoPageSomeFeatureGuard extends SomeFeatureAuthorizationGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, protected router: Router, diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/group-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/group-administrator.guard.ts index 3fee767fdc..5afd572326 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/group-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/group-administrator.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { AuthService } from '../../../auth/auth.service'; @@ -13,7 +13,7 @@ import { FeatureID } from '../feature-id'; @Injectable({ providedIn: 'root' }) -export class GroupAdministratorGuard extends FeatureAuthorizationGuard { +export class GroupAdministratorGuard extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { super(authorizationService, router, authService); } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts similarity index 92% rename from src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts index 2c6f4b0717..1fa5498f12 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts @@ -1,4 +1,4 @@ -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { Observable, of as observableOf } from 'rxjs'; @@ -9,7 +9,7 @@ import { AuthService } from '../../../auth/auth.service'; * Test implementation of abstract class FeatureAuthorizationGuard * Provide the return values of the overwritten getters as constructor arguments */ -class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { +class FeatureAuthorizationGuardImpl extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService, @@ -33,7 +33,7 @@ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { } describe('FeatureAuthorizationGuard', () => { - let guard: FeatureAuthorizationGuard; + let guard: SingleFeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.ts new file mode 100644 index 0000000000..cb71d2f418 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.ts @@ -0,0 +1,27 @@ +import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +import { FeatureID } from '../feature-id'; +import { Observable } from 'rxjs'; +import { map} from 'rxjs/operators'; +import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; + +/** + * Abstract Guard for preventing unauthorized activating and loading of routes when a user + * doesn't have authorized rights on a specific feature and/or object. + * Override the desired getters in the parent class for checking specific authorization on a feature and/or object. + */ +export abstract class SingleFeatureAuthorizationGuard extends SomeFeatureAuthorizationGuard { + /** + * The features to check authorization for + */ + getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return this.getFeatureID(route, state).pipe( + map((featureID) => [featureID]), + ); + } + + /** + * The type of feature to check authorization for + * Override this method to define a feature + */ + abstract getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; +} diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts index bb678ebf33..cc6f50c161 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { FeatureID } from '../feature-id'; import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; @@ -13,7 +13,7 @@ import { AuthService } from '../../../auth/auth.service'; @Injectable({ providedIn: 'root' }) -export class SiteAdministratorGuard extends FeatureAuthorizationGuard { +export class SiteAdministratorGuard extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { super(authorizationService, router, authService); } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts index 709d9ff266..bdbb8250e2 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts @@ -1,4 +1,4 @@ -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; import { Injectable } from '@angular/core'; import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; @@ -13,7 +13,7 @@ import { AuthService } from '../../../auth/auth.service'; @Injectable({ providedIn: 'root' }) -export class SiteRegisterGuard extends FeatureAuthorizationGuard { +export class SiteRegisterGuard extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { super(authorizationService, router, authService); } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts similarity index 64% rename from src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts index 86b75b637e..3a6cf745c9 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts @@ -2,16 +2,16 @@ import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTr import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; -import { returnForbiddenUrlTreeOrLoginOnFalse } from '../../../shared/operators'; +import { returnForbiddenUrlTreeOrLoginOnAllFalse } from '../../../shared/operators'; import { switchMap } from 'rxjs/operators'; import { AuthService } from '../../../auth/auth.service'; /** * Abstract Guard for preventing unauthorized activating and loading of routes when a user - * doesn't have authorized rights on a specific feature and/or object. - * Override the desired getters in the parent class for checking specific authorization on a feature and/or object. + * doesn't have authorized rights on any of the specified features and/or object. + * Override the desired getters in the parent class for checking specific authorization on a list of features and/or object. */ -export abstract class FeatureAuthorizationGuard implements CanActivate { +export abstract class SomeFeatureAuthorizationGuard implements CanActivate { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { @@ -22,17 +22,19 @@ export abstract class FeatureAuthorizationGuard implements CanActivate { * Redirect the user to the unauthorized page when he/she's not authorized for the given feature */ canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableCombineLatest(this.getFeatureID(route, state), this.getObjectUrl(route, state), this.getEPersonUuid(route, state)).pipe( - switchMap(([featureID, objectUrl, ePersonUuid]) => this.authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid)), - returnForbiddenUrlTreeOrLoginOnFalse(this.router, this.authService, state.url) + return observableCombineLatest(this.getFeatureIDs(route, state), this.getObjectUrl(route, state), this.getEPersonUuid(route, state)).pipe( + switchMap(([featureIDs, objectUrl, ePersonUuid]) => + observableCombineLatest(...featureIDs.map((featureID) => this.authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid))) + ), + returnForbiddenUrlTreeOrLoginOnAllFalse(this.router, this.authService, state.url) ); } /** - * The type of feature to check authorization for - * Override this method to define a feature + * The features to check authorization for + * Override this method to define a list of features */ - abstract getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; + abstract getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; /** * The URL of the object to check if the user has authorized rights for diff --git a/src/app/core/data/feature-authorization/feature-id.ts b/src/app/core/data/feature-authorization/feature-id.ts index e3473a895e..df77dd8949 100644 --- a/src/app/core/data/feature-authorization/feature-id.ts +++ b/src/app/core/data/feature-authorization/feature-id.ts @@ -12,4 +12,11 @@ export enum FeatureID { CanManageGroups = 'canManageGroups', IsCollectionAdmin = 'isCollectionAdmin', IsCommunityAdmin = 'isCommunityAdmin', + CanManageVersions = 'canManageVersions', + CanManageBitstreams = 'canManageBitstreams', + CanManageRelationships = 'canManageRelationships', + CanManageMappings = 'canManageMappings', + CanManagePolicies = 'canManagePolicies', + CanMakePrivate = 'canMakePrivate', + CanMove = 'canMove', } diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index dd610b6ca7..2d0ab70e2c 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -201,10 +201,23 @@ export const redirectOn4xx = (router: Router, authService: AuthService) => */ export const returnForbiddenUrlTreeOrLoginOnFalse = (router: Router, authService: AuthService, redirectUrl: string) => (source: Observable): Observable => + source.pipe( + map((authorized) => [authorized]), + returnForbiddenUrlTreeOrLoginOnAllFalse(router, authService, redirectUrl), + ); + +/** + * Operator that returns a UrlTree to a forbidden page or the login page when the booleans received are all false + * @param router The router used to navigate to a forbidden page + * @param authService The AuthService used to determine whether or not the user is logged in + * @param redirectUrl The URL to redirect back to after logging in + */ +export const returnForbiddenUrlTreeOrLoginOnAllFalse = (router: Router, authService: AuthService, redirectUrl: string) => + (source: Observable): Observable => observableCombineLatest(source, authService.isAuthenticated()).pipe( - map(([authorized, authenticated]: [boolean, boolean]) => { - if (authorized) { - return authorized; + map(([authorizedList, authenticated]: [boolean[], boolean]) => { + if (authorizedList.indexOf(true) > -1) { + return true; } else { if (authenticated) { return router.parseUrl(getForbiddenRoute()); diff --git a/src/app/shared/mocks/dspace-rest/endpoint-mocking-rest.service.ts b/src/app/shared/mocks/dspace-rest/endpoint-mocking-rest.service.ts index 278a392e18..8d621ad4be 100644 --- a/src/app/shared/mocks/dspace-rest/endpoint-mocking-rest.service.ts +++ b/src/app/shared/mocks/dspace-rest/endpoint-mocking-rest.service.ts @@ -97,8 +97,14 @@ export class EndpointMockingRestService extends DspaceRestService { * the mock response if there is one, undefined otherwise */ private getMockData(urlStr: string): any { - const url = new URL(urlStr); - const key = url.pathname.slice(environment.rest.nameSpace.length); + let key; + if (this.mockResponseMap.has(urlStr)) { + key = urlStr; + } else { + // didn't find an exact match for the url, try to match only the endpoint without namespace and parameters + const url = new URL(urlStr); + key = url.pathname.slice(environment.rest.nameSpace.length); + } if (this.mockResponseMap.has(key)) { // parse and stringify to clone the object to ensure that any changes made // to it afterwards don't affect future calls diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-bitstreams-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-bitstreams-response.json new file mode 100644 index 0000000000..1642691672 --- /dev/null +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-bitstreams-response.json @@ -0,0 +1,51 @@ +{ + "_embedded": { + "authorizations": [ + { + "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canManageBitstreams_core.item_96715576-3748-4761-ad45-001646632963", + "type": "authorization", + "_links": { + "eperson": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageBitstreams_core.item_96715576-3748-4761-ad45-001646632963/eperson" + }, + "feature": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageBitstreams_core.item_96715576-3748-4761-ad45-001646632963/feature" + }, + "object": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageBitstreams_core.item_96715576-3748-4761-ad45-001646632963/object" + }, + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageBitstreams_core.item_96715576-3748-4761-ad45-001646632963" + } + }, + "_embedded": { + "feature": { + "id": "canManageBitstreams", + "description": "It can be used to verify if the bitstreams of the specified objects can be managed", + "type": "feature", + "resourcetypes": [ + "core.item", + "core.bundle" + ], + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/features/canManageBitstreams" + } + } + } + } + } + ] + }, + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canManageBitstreams" + } + }, + "page": { + "size": 20, + "totalElements": 1, + "totalPages": 1, + "number": 0 + } +} diff --git a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts index 92f04a7bc5..4cc5c39b7d 100644 --- a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts +++ b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts @@ -2,6 +2,7 @@ import { InjectionToken } from '@angular/core'; // import mockSubmissionResponse from './mock-submission-response.json'; // import mockPublicationResponse from './mock-publication-response.json'; // import mockUntypedItemResponse from './mock-untyped-item-response.json'; +import mockFeatureItemCanManageBitstreamsResponse from './mock-feature-item-can-manage-bitstreams-response.json'; export class ResponseMapMock extends Map {} @@ -16,4 +17,5 @@ export const mockResponseMap: ResponseMapMock = new Map([ // [ '/config/submissionforms/traditionalpageone', mockSubmissionResponse ] // [ '/api/pid/find', mockPublicationResponse ], // [ '/api/pid/find', mockUntypedItemResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canManageBitstreams&embed=feature', mockFeatureItemCanManageBitstreamsResponse ], ]); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 2924a3a47e..abecfce291 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1761,6 +1761,8 @@ "item.edit.tabs.status.buttons.reinstate.label": "Reinstate item into the repository", + "item.edit.tabs.status.buttons.unauthorized": "You don't have permission to perform this action", + "item.edit.tabs.status.buttons.withdraw.button": "Withdraw...", "item.edit.tabs.status.buttons.withdraw.label": "Withdraw item from the repository", From 3504feedf3178589dcb7d4c123c243b6a8adff69 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 9 Apr 2021 13:34:04 +0200 Subject: [PATCH 08/42] 78243: edit item page fine-grained permission checks - mocks + tests --- .../item-operation.component.spec.ts | 14 +-- .../dso-page-single-feature.guard.spec.ts | 8 +- .../dso-page-some-feature.guard.spec.ts | 87 ++++++++++++++ .../dso-page-some-feature.guard.ts | 2 +- ...single-feature-authorization.guard.spec.ts | 8 +- .../some-feature-authorization.guard.spec.ts | 110 ++++++++++++++++++ ...feature-item-can-delete-none-response.json | 13 +++ .../mock-feature-item-can-move-response.json | 50 ++++++++ .../dspace-rest/mocks/response-map.mock.ts | 4 + 9 files changed, 280 insertions(+), 16 deletions(-) create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.spec.ts create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.spec.ts create mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json create mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json diff --git a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts index 00e7f8452a..7570119b3a 100644 --- a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts @@ -28,19 +28,19 @@ describe('ItemOperationComponent', () => { }); it('should render operation row', () => { - const span = fixture.debugElement.query(By.css('span')).nativeElement; + const span = fixture.debugElement.query(By.css('.action-label span')).nativeElement; expect(span.textContent).toContain('item.edit.tabs.status.buttons.key1.label'); - const link = fixture.debugElement.query(By.css('a')).nativeElement; - expect(link.href).toContain('url1'); - expect(link.textContent).toContain('item.edit.tabs.status.buttons.key1.button'); + const button = fixture.debugElement.query(By.css('button')).nativeElement; + expect(button.textContent).toContain('item.edit.tabs.status.buttons.key1.button'); }); it('should render disabled operation row', () => { itemOperation.setDisabled(true); fixture.detectChanges(); - const span = fixture.debugElement.query(By.css('span')).nativeElement; + const span = fixture.debugElement.query(By.css('.action-label span')).nativeElement; expect(span.textContent).toContain('item.edit.tabs.status.buttons.key1.label'); - const span2 = fixture.debugElement.query(By.css('span.btn-danger')).nativeElement; - expect(span2.textContent).toContain('item.edit.tabs.status.buttons.key1.button'); + const button = fixture.debugElement.query(By.css('button')).nativeElement; + expect(button.disabled).toBeTrue(); + expect(button.textContent).toContain('item.edit.tabs.status.buttons.key1.button'); }); }); diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts index 06677d1f22..6c1f330c69 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-single-feature.guard.spec.ts @@ -9,9 +9,9 @@ import { FeatureID } from '../feature-id'; import { AuthService } from '../../../auth/auth.service'; /** - * Test implementation of abstract class DsoPageAdministratorGuard + * Test implementation of abstract class DsoPageSingleFeatureGuard */ -class DsoPageFeatureGuardImpl extends DsoPageSingleFeatureGuard { +class DsoPageSingleFeatureGuardImpl extends DsoPageSingleFeatureGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, protected router: Router, @@ -25,7 +25,7 @@ class DsoPageFeatureGuardImpl extends DsoPageSingleFeatureGuard { } } -describe('DsoPageAdministratorGuard', () => { +describe('DsoPageSingleFeatureGuard', () => { let guard: DsoPageSingleFeatureGuard; let authorizationService: AuthorizationDataService; let router: Router; @@ -62,7 +62,7 @@ describe('DsoPageAdministratorGuard', () => { }, parent: parentRoute }; - guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, authService, undefined); + guard = new DsoPageSingleFeatureGuardImpl(resolver, authorizationService, router, authService, undefined); } beforeEach(() => { diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.spec.ts new file mode 100644 index 0000000000..071b1b0731 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.spec.ts @@ -0,0 +1,87 @@ +import { AuthorizationDataService } from '../authorization-data.service'; +import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; +import { RemoteData } from '../../remote-data'; +import { Observable, of as observableOf } from 'rxjs'; +import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { FeatureID } from '../feature-id'; +import { AuthService } from '../../../auth/auth.service'; +import { DsoPageSomeFeatureGuard } from './dso-page-some-feature.guard'; + +/** + * Test implementation of abstract class DsoPageSomeFeatureGuard + */ +class DsoPageSomeFeatureGuardImpl extends DsoPageSomeFeatureGuard { + constructor(protected resolver: Resolve>, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService, + protected featureIDs: FeatureID[]) { + super(resolver, authorizationService, router, authService); + } + + getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.featureIDs); + } +} + +describe('DsoPageSomeFeatureGuard', () => { + let guard: DsoPageSomeFeatureGuard; + let authorizationService: AuthorizationDataService; + let router: Router; + let authService: AuthService; + let resolver: Resolve>; + let object: DSpaceObject; + let route; + let parentRoute; + + function init() { + object = { + self: 'test-selflink' + } as DSpaceObject; + + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true) + }); + router = jasmine.createSpyObj('router', { + parseUrl: {} + }); + resolver = jasmine.createSpyObj('resolver', { + resolve: createSuccessfulRemoteDataObject$(object) + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true) + }); + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0' + } + }; + route = { + params: { + }, + parent: parentRoute + }; + guard = new DsoPageSomeFeatureGuardImpl(resolver, authorizationService, router, authService, []); + } + + beforeEach(() => { + init(); + }); + + describe('getObjectUrl', () => { + it('should return the resolved object\'s selflink', (done) => { + guard.getObjectUrl(route, undefined).subscribe((selflink) => { + expect(selflink).toEqual(object.self); + done(); + }); + }); + }); + + describe('getRouteWithDSOId', () => { + it('should return the route that has the UUID of the DSO', () => { + const foundRoute = (guard as any).getRouteWithDSOId(route); + expect(foundRoute).toBe(parentRoute); + }); + }); +}); diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts index 7b7cb4c196..8683709345 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard.ts @@ -10,7 +10,7 @@ import { hasNoValue, hasValue } from '../../../../shared/empty.util'; import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; /** - * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature + * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for any specific feature in a list * This guard utilizes a resolver to retrieve the relevant object to check authorizations for */ export abstract class DsoPageSomeFeatureGuard extends SomeFeatureAuthorizationGuard { diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts index 1fa5498f12..635aa3530b 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard.spec.ts @@ -6,10 +6,10 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro import { AuthService } from '../../../auth/auth.service'; /** - * Test implementation of abstract class FeatureAuthorizationGuard + * Test implementation of abstract class SingleFeatureAuthorizationGuard * Provide the return values of the overwritten getters as constructor arguments */ -class FeatureAuthorizationGuardImpl extends SingleFeatureAuthorizationGuard { +class SingleFeatureAuthorizationGuardImpl extends SingleFeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService, @@ -32,7 +32,7 @@ class FeatureAuthorizationGuardImpl extends SingleFeatureAuthorizationGuard { } } -describe('FeatureAuthorizationGuard', () => { +describe('SingleFeatureAuthorizationGuard', () => { let guard: SingleFeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; let router: Router; @@ -56,7 +56,7 @@ describe('FeatureAuthorizationGuard', () => { authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true) }); - guard = new FeatureAuthorizationGuardImpl(authorizationService, router, authService, featureId, objectUrl, ePersonUuid); + guard = new SingleFeatureAuthorizationGuardImpl(authorizationService, router, authService, featureId, objectUrl, ePersonUuid); } beforeEach(() => { diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.spec.ts new file mode 100644 index 0000000000..90153d2d14 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.spec.ts @@ -0,0 +1,110 @@ +import { AuthorizationDataService } from '../authorization-data.service'; +import { FeatureID } from '../feature-id'; +import { Observable, of as observableOf } from 'rxjs'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { AuthService } from '../../../auth/auth.service'; +import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; + +/** + * Test implementation of abstract class SomeFeatureAuthorizationGuard + * Provide the return values of the overwritten getters as constructor arguments + */ +class SomeFeatureAuthorizationGuardImpl extends SomeFeatureAuthorizationGuard { + constructor(protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService, + protected featureIds: FeatureID[], + protected objectUrl: string, + protected ePersonUuid: string) { + super(authorizationService, router, authService); + } + + getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.featureIds); + } + + getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.objectUrl); + } + + getEPersonUuid(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.ePersonUuid); + } +} + +describe('SomeFeatureAuthorizationGuard', () => { + let guard: SomeFeatureAuthorizationGuard; + let authorizationService: AuthorizationDataService; + let router: Router; + let authService: AuthService; + + let featureIds: FeatureID[]; + let authorizedFeatureIds: FeatureID[]; + let objectUrl: string; + let ePersonUuid: string; + + function init() { + featureIds = [FeatureID.LoginOnBehalfOf, FeatureID.CanDelete]; + authorizedFeatureIds = []; + objectUrl = 'fake-object-url'; + ePersonUuid = 'fake-eperson-uuid'; + + authorizationService = Object.assign({ + isAuthorized(featureId?: FeatureID): Observable { + return observableOf(authorizedFeatureIds.indexOf(featureId) > -1); + } + }); + router = jasmine.createSpyObj('router', { + parseUrl: {} + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true) + }); + guard = new SomeFeatureAuthorizationGuardImpl(authorizationService, router, authService, featureIds, objectUrl, ePersonUuid); + } + + beforeEach(() => { + init(); + }); + + describe('canActivate', () => { + describe('when the user isn\'t authorized', () => { + beforeEach(() => { + authorizedFeatureIds = []; + }); + + it('should not return true', (done) => { + guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + expect(result).not.toEqual(true); + done(); + }); + }); + }); + + describe('when the user is authorized for at least one of the guard\'s features', () => { + beforeEach(() => { + authorizedFeatureIds = [featureIds[0]]; + }); + + it('should return true', (done) => { + guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + + describe('when the user is authorized for all of the guard\'s features', () => { + beforeEach(() => { + authorizedFeatureIds = featureIds; + }); + + it('should return true', (done) => { + guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + }); +}); diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json new file mode 100644 index 0000000000..51968bd5da --- /dev/null +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json @@ -0,0 +1,13 @@ +{ + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canDelete" + } + }, + "page": { + "size": 20, + "totalElements": 0, + "totalPages": 1, + "number": 0 + } +} diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json new file mode 100644 index 0000000000..692751d671 --- /dev/null +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json @@ -0,0 +1,50 @@ +{ + "_embedded": { + "authorizations": [ + { + "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963", + "type": "authorization", + "_links": { + "eperson": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/eperson" + }, + "feature": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/feature" + }, + "object": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/object" + }, + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963" + } + }, + "_embedded": { + "feature": { + "id": "canMove", + "description": "It can be used to verify if the user is allowed to move items", + "type": "feature", + "resourcetypes": [ + "core.item" + ], + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/features/canMove" + } + } + } + } + } + ] + }, + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canMove" + } + }, + "page": { + "size": 20, + "totalElements": 1, + "totalPages": 1, + "number": 0 + } +} diff --git a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts index 4cc5c39b7d..f276c24bbf 100644 --- a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts +++ b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts @@ -3,6 +3,8 @@ import { InjectionToken } from '@angular/core'; // import mockPublicationResponse from './mock-publication-response.json'; // import mockUntypedItemResponse from './mock-untyped-item-response.json'; import mockFeatureItemCanManageBitstreamsResponse from './mock-feature-item-can-manage-bitstreams-response.json'; +import mockFeatureItemCanMoveResponse from './mock-feature-item-can-move-response.json'; +import mockFeatureItemCanDeleteNoneResponse from './mock-feature-item-can-delete-none-response.json'; export class ResponseMapMock extends Map {} @@ -18,4 +20,6 @@ export const mockResponseMap: ResponseMapMock = new Map([ // [ '/api/pid/find', mockPublicationResponse ], // [ '/api/pid/find', mockUntypedItemResponse ], [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canManageBitstreams&embed=feature', mockFeatureItemCanManageBitstreamsResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canMove&embed=feature', mockFeatureItemCanMoveResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canDelete&embed=feature', mockFeatureItemCanDeleteNoneResponse ], ]); From 5fca681222f349cb50d0108f336905816a449d72 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 7 Apr 2021 17:07:26 +0200 Subject: [PATCH 09/42] 78001: RelationshipEffects test fixes --- src/app/core/shared/operators.ts | 8 +- .../relationship.effects.spec.ts | 136 ++++++++---------- .../relationship.effects.ts | 9 +- 3 files changed, 72 insertions(+), 81 deletions(-) diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index dd610b6ca7..5cef7e5e09 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -1,6 +1,6 @@ import { Router, UrlTree } from '@angular/router'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; -import { filter, find, map, mergeMap, switchMap, take, takeWhile, tap } from 'rxjs/operators'; +import { debounceTime, filter, find, map, mergeMap, switchMap, take, takeWhile, tap } from 'rxjs/operators'; import { hasNoValue, hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util'; import { SearchResult } from '../../shared/search/search-result.model'; import { PaginatedList } from '../data/paginated-list.model'; @@ -15,6 +15,12 @@ import { DSpaceObject } from './dspace-object.model'; import { getForbiddenRoute, getPageNotFoundRoute } from '../../app-routing-paths'; import { getEndUserAgreementPath } from '../../info/info-routing-paths'; import { AuthService } from '../auth/auth.service'; +import { InjectionToken } from '@angular/core'; + +export const DEBOUNCE_TIME_OPERATOR = new InjectionToken<(dueTime: number) => (source: Observable) => Observable>('debounceTime', { + providedIn: 'root', + factory: () => debounceTime +}); /** * This file contains custom RxJS operators that can be used in multiple places diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts index 5e2ffd30fb..e988eba9eb 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.spec.ts @@ -1,10 +1,7 @@ import { TestBed, waitForAsync } from '@angular/core/testing'; - import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; -import { TestScheduler } from 'rxjs/testing'; import { provideMockActions } from '@ngrx/effects/testing'; import { Store } from '@ngrx/store'; - import { RelationshipEffects } from './relationship.effects'; import { AddRelationshipAction, RelationshipActionTypes, RemoveRelationshipAction } from './relationship.actions'; import { Item } from '../../../../../core/shared/item.model'; @@ -23,6 +20,9 @@ import { RequestService } from '../../../../../core/data/request.service'; import { NotificationsService } from '../../../../notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; import { SelectableListService } from '../../../../object-list/selectable-list/selectable-list.service'; +import { cold, hot } from 'jasmine-marbles'; +import { DEBOUNCE_TIME_OPERATOR } from '../../../../../core/shared/operators'; +import { last } from 'rxjs/operators'; describe('RelationshipEffects', () => { let relationEffects: RelationshipEffects; @@ -51,7 +51,6 @@ describe('RelationshipEffects', () => { let notificationsService; let translateService; let selectableListService; - let testScheduler: TestScheduler; function init() { testUUID1 = '20e24c2f-a00a-467c-bdee-c929e79bf08d'; @@ -131,6 +130,7 @@ describe('RelationshipEffects', () => { { provide: NotificationsService, useValue: notificationsService }, { provide: TranslateService, useValue: translateService }, { provide: SelectableListService, useValue: selectableListService }, + { provide: DEBOUNCE_TIME_OPERATOR, useValue: jasmine.createSpy('debounceTime').and.returnValue((v) => v.pipe(last())) }, ], }); })); @@ -140,9 +140,6 @@ describe('RelationshipEffects', () => { identifier = (relationEffects as any).createIdentifier(leftItem, rightItem, relationshipType.leftwardType); spyOn((relationEffects as any), 'addRelationship').and.stub(); spyOn((relationEffects as any), 'removeRelationship').and.stub(); - testScheduler = new TestScheduler((actual, expected) => { - expect(actual).toEqual(expected); - }); }); describe('mapLastActions$', () => { @@ -151,15 +148,13 @@ describe('RelationshipEffects', () => { let action; it('should set the current value debounceMap and the value of the initialActionMap to ADD_RELATIONSHIP', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--a-|', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('--b-|', { b: undefined }); - flush(); - // TODO check following expectations with the implementation - // expect((relationEffects as any).initialActionMap[identifier]).toBe(action.type); - // expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); - }); + action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(action.type); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); }); }); @@ -172,14 +167,14 @@ describe('RelationshipEffects', () => { }); it('should set the current value debounceMap to ADD_RELATIONSHIP but not change the value of the initialActionMap', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--a-|', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('--b-|', { b: undefined }); - flush(); - expect((relationEffects as any).initialActionMap[identifier]).toBe(testActionType); - expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); - }); + action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(testActionType); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); }); }); @@ -188,30 +183,26 @@ describe('RelationshipEffects', () => { describe('When the last value in the debounceMap is also an ADD_RELATIONSHIP action', () => { beforeEach(() => { (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.ADD_RELATIONSHIP; + ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); }); it('should call addRelationship on the effect', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--a-|', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('--b-|', { b: undefined }); - flush(); - expect((relationEffects as any).addRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234', undefined); - }); + action = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).addRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234', undefined); }); }); describe('When the last value in the debounceMap is instead a REMOVE_RELATIONSHIP action', () => { - it('should not call removeRelationship or addRelationship on the effect', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - const actiona = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - const actionb = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--ab-|', { a: actiona, b: actionb }); - expectObservable(relationEffects.mapLastActions$).toBe('--bb-|', { b: undefined }); - flush(); - expect((relationEffects as any).addRelationship).not.toHaveBeenCalled(); - expect((relationEffects as any).removeRelationship).not.toHaveBeenCalled(); - }); + const actiona = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + const actionb = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--ab-|', { a: actiona, b: actionb }); + const expected = cold('--bb-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).addRelationship).not.toHaveBeenCalled(); + expect((relationEffects as any).removeRelationship).not.toHaveBeenCalled(); }); }); }); @@ -222,15 +213,13 @@ describe('RelationshipEffects', () => { let action; it('should set the current value debounceMap and the value of the initialActionMap to REMOVE_RELATIONSHIP', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--a-|', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('--b-|', { b: undefined }); - flush(); - // TODO check following expectations with the implementation - // expect((relationEffects as any).initialActionMap[identifier]).toBe(action.type); - // expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); - }); + action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(action.type); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); }); }); @@ -238,20 +227,19 @@ describe('RelationshipEffects', () => { let action; const testActionType = 'TEST_TYPE'; beforeEach(() => { + ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); (relationEffects as any).initialActionMap[identifier] = testActionType; (relationEffects as any).debounceMap[identifier] = new BehaviorSubject(testActionType); }); it('should set the current value debounceMap to REMOVE_RELATIONSHIP but not change the value of the initialActionMap', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--a-|', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('--b-|', { b: undefined }); - flush(); - // TODO check following expectations with the implementation - // expect((relationEffects as any).initialActionMap[identifier]).toBe(testActionType); - // expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); - }); + action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + + expect((relationEffects as any).initialActionMap[identifier]).toBe(testActionType); + expect((relationEffects as any).debounceMap[identifier].value).toBe(action.type); }); }); @@ -259,32 +247,28 @@ describe('RelationshipEffects', () => { let action; describe('When the last value in the debounceMap is also an REMOVE_RELATIONSHIP action', () => { beforeEach(() => { + ((relationEffects as any).debounceTime as jasmine.Spy).and.returnValue((v) => v); (relationEffects as any).initialActionMap[identifier] = RelationshipActionTypes.REMOVE_RELATIONSHIP; }); it('should call removeRelationship on the effect', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('a', { a: action }); - expectObservable(relationEffects.mapLastActions$).toBe('b', { b: undefined }); - flush(); - expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234',); - }); + action = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--a-|', { a: action }); + const expected = cold('--b-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).removeRelationship).toHaveBeenCalledWith(leftItem, rightItem, relationshipType.leftwardType, '1234',); }); }); describe('When the last value in the debounceMap is instead a ADD_RELATIONSHIP action', () => { - it('should not call addRelationship or removeRelationship on the effect', () => { - testScheduler.run(({ hot, expectObservable, flush }) => { - const actionb = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - const actiona = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); - actions = hot('--ab-|', { a: actiona, b: actionb }); - expectObservable(relationEffects.mapLastActions$).toBe('--bb-|', { b: undefined }); - flush(); - expect((relationEffects as any).addRelationship).not.toHaveBeenCalled(); - expect((relationEffects as any).removeRelationship).not.toHaveBeenCalled(); - }); + const actionb = new RemoveRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + const actiona = new AddRelationshipAction(leftItem, rightItem, relationshipType.leftwardType, '1234'); + actions = hot('--ab-|', { a: actiona, b: actionb }); + const expected = cold('--bb-|', { b: undefined }); + expect(relationEffects.mapLastActions$).toBeObservable(expected); + expect((relationEffects as any).addRelationship).not.toHaveBeenCalled(); + expect((relationEffects as any).removeRelationship).not.toHaveBeenCalled(); }); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts index 5a20ed8b51..a810cb0ad9 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/relationship.effects.ts @@ -1,11 +1,11 @@ -import { Injectable } from '@angular/core'; +import { Inject, Injectable } from '@angular/core'; import { Actions, Effect, ofType } from '@ngrx/effects'; -import { debounceTime, filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; +import { filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; import { BehaviorSubject, Observable } from 'rxjs'; import { RelationshipService } from '../../../../../core/data/relationship.service'; import { getRemoteDataPayload, - getFirstSucceededRemoteData + getFirstSucceededRemoteData, DEBOUNCE_TIME_OPERATOR } from '../../../../../core/shared/operators'; import { AddRelationshipAction, @@ -71,7 +71,7 @@ export class RelationshipEffects { this.initialActionMap[identifier] = action.type; this.debounceMap[identifier] = new BehaviorSubject(action.type); this.debounceMap[identifier].pipe( - debounceTime(DEBOUNCE_TIME), + this.debounceTime(DEBOUNCE_TIME), take(1) ).subscribe( (type) => { @@ -159,6 +159,7 @@ export class RelationshipEffects { private notificationsService: NotificationsService, private translateService: TranslateService, private selectableListService: SelectableListService, + @Inject(DEBOUNCE_TIME_OPERATOR) private debounceTime: (dueTime: number) => (source: Observable) => Observable, ) { } From fd437eb7ee0481750520589c0ff94e74245638c4 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 16 Apr 2021 11:36:33 +0200 Subject: [PATCH 10/42] 78243: edit-item-page mocks, item-status operation refactoring --- .../item-status/item-status.component.ts | 27 +++++----- src/app/core/shared/operators.ts | 2 +- ...feature-item-can-delete-none-response.json | 13 ----- ...re-item-can-manage-mappings-response.json} | 18 +++---- ...tem-can-manage-relationships-response.json | 50 +++++++++++++++++++ ...ure-item-can-manage-versions-response.json | 50 +++++++++++++++++++ .../dspace-rest/mocks/response-map.mock.ts | 10 ++-- src/assets/i18n/en.json5 | 2 +- 8 files changed, 132 insertions(+), 40 deletions(-) delete mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json rename src/app/shared/mocks/dspace-rest/mocks/{mock-feature-item-can-move-response.json => mock-feature-item-can-manage-mappings-response.json} (56%) create mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-relationships-response.json create mode 100644 src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-versions-response.json diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index 51aa24ea6c..f95d2d1517 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -3,8 +3,8 @@ import { fadeIn, fadeInOut } from '../../../shared/animations/fade'; import { Item } from '../../../core/shared/item.model'; import { ActivatedRoute } from '@angular/router'; import { ItemOperation } from '../item-operation/itemOperation.model'; -import { distinctUntilChanged, first, map } from 'rxjs/operators'; -import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, of } from 'rxjs'; +import { distinctUntilChanged, first, map, mergeMap, toArray } from 'rxjs/operators'; +import { BehaviorSubject, Observable, from as observableFrom } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { getItemEditRoute, getItemPageRoute } from '../../item-page-routing-paths'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; @@ -95,16 +95,19 @@ export class ItemStatusComponent implements OnInit { this.operations$.next(operations); - observableCombineLatest(operations.map((operation) => { - if (hasValue(operation.featureID)) { - return this.authorizationService.isAuthorized(operation.featureID, item.self).pipe( - distinctUntilChanged(), - map((authorized) => new ItemOperation(operation.operationKey, operation.operationUrl, operation.featureID, !authorized, authorized)) - ); - } else { - return of(operation); - } - })).subscribe((ops) => this.operations$.next(ops)); + observableFrom(operations).pipe( + mergeMap((operation) => { + if (hasValue(operation.featureID)) { + return this.authorizationService.isAuthorized(operation.featureID, item.self).pipe( + distinctUntilChanged(), + map((authorized) => new ItemOperation(operation.operationKey, operation.operationUrl, operation.featureID, !authorized, authorized)) + ); + } else { + return [operation]; + } + }), + toArray() + ).subscribe((ops) => this.operations$.next(ops)); }); this.itemPageRoute$ = this.itemRD$.pipe( getAllSucceededRemoteDataPayload(), diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 2d0ab70e2c..fc3f2651e1 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -216,7 +216,7 @@ export const returnForbiddenUrlTreeOrLoginOnAllFalse = (router: Router, authServ (source: Observable): Observable => observableCombineLatest(source, authService.isAuthenticated()).pipe( map(([authorizedList, authenticated]: [boolean[], boolean]) => { - if (authorizedList.indexOf(true) > -1) { + if (authorizedList.some((b: boolean) => b === true)) { return true; } else { if (authenticated) { diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json deleted file mode 100644 index 51968bd5da..0000000000 --- a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-delete-none-response.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "_links": { - "self": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canDelete" - } - }, - "page": { - "size": 20, - "totalElements": 0, - "totalPages": 1, - "number": 0 - } -} diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-mappings-response.json similarity index 56% rename from src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json rename to src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-mappings-response.json index 692751d671..c186ef8cc4 100644 --- a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-move-response.json +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-mappings-response.json @@ -2,33 +2,33 @@ "_embedded": { "authorizations": [ { - "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963", + "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canManageMappings_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067", "type": "authorization", "_links": { "eperson": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/eperson" + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageMappings_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/eperson" }, "feature": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/feature" + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageMappings_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/feature" }, "object": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963/object" + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageMappings_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/object" }, "self": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canMove_core.item_96715576-3748-4761-ad45-001646632963" + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageMappings_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067" } }, "_embedded": { "feature": { - "id": "canMove", - "description": "It can be used to verify if the user is allowed to move items", + "id": "canManageMappings", + "description": "It can be used to verify if the mappings of the specified objects can be managed", "type": "feature", "resourcetypes": [ "core.item" ], "_links": { "self": { - "href": "https://api7.dspace.org/server/api/authz/features/canMove" + "href": "https://api7.dspace.org/server/api/authz/features/canManageMappings" } } } @@ -38,7 +38,7 @@ }, "_links": { "self": { - "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canMove" + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/e98b0f27-5c19-49a0-960d-eb6ad5287067&feature=canManageMappings" } }, "page": { diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-relationships-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-relationships-response.json new file mode 100644 index 0000000000..b6de452dd2 --- /dev/null +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-relationships-response.json @@ -0,0 +1,50 @@ +{ + "_embedded": { + "authorizations": [ + { + "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canManageRelationships_core.item_047556d1-3d01-4c53-bc68-0cee7ad7ed4e", + "type": "authorization", + "_links": { + "eperson": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageRelationships_core.item_047556d1-3d01-4c53-bc68-0cee7ad7ed4e/eperson" + }, + "feature": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageRelationships_core.item_047556d1-3d01-4c53-bc68-0cee7ad7ed4e/feature" + }, + "object": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageRelationships_core.item_047556d1-3d01-4c53-bc68-0cee7ad7ed4e/object" + }, + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageRelationships_core.item_047556d1-3d01-4c53-bc68-0cee7ad7ed4e" + } + }, + "_embedded": { + "feature": { + "id": "canManageRelationships", + "description": "It can be used to verify if the relationships of the specified objects can be managed", + "type": "feature", + "resourcetypes": [ + "core.item" + ], + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/features/canManageRelationships" + } + } + } + } + } + ] + }, + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/047556d1-3d01-4c53-bc68-0cee7ad7ed4e&feature=canManageRelationships" + } + }, + "page": { + "size": 20, + "totalElements": 1, + "totalPages": 1, + "number": 0 + } +} diff --git a/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-versions-response.json b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-versions-response.json new file mode 100644 index 0000000000..55eb69a7bf --- /dev/null +++ b/src/app/shared/mocks/dspace-rest/mocks/mock-feature-item-can-manage-versions-response.json @@ -0,0 +1,50 @@ +{ + "_embedded": { + "authorizations": [ + { + "id": "cd824a61-95be-4e16-bccd-51fea26707d0_canManageVersions_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067", + "type": "authorization", + "_links": { + "eperson": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageVersions_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/eperson" + }, + "feature": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageVersions_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/feature" + }, + "object": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageVersions_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067/object" + }, + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/cd824a61-95be-4e16-bccd-51fea26707d0_canManageVersions_core.item_e98b0f27-5c19-49a0-960d-eb6ad5287067" + } + }, + "_embedded": { + "feature": { + "id": "canManageVersions", + "description": "It can be used to verify if the versions of the specified objects can be managed", + "type": "feature", + "resourcetypes": [ + "core.item" + ], + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/features/canManageVersions" + } + } + } + } + } + ] + }, + "_links": { + "self": { + "href": "https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/e98b0f27-5c19-49a0-960d-eb6ad5287067&feature=canManageVersions" + } + }, + "page": { + "size": 20, + "totalElements": 1, + "totalPages": 1, + "number": 0 + } +} diff --git a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts index f276c24bbf..a746416e77 100644 --- a/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts +++ b/src/app/shared/mocks/dspace-rest/mocks/response-map.mock.ts @@ -3,8 +3,9 @@ import { InjectionToken } from '@angular/core'; // import mockPublicationResponse from './mock-publication-response.json'; // import mockUntypedItemResponse from './mock-untyped-item-response.json'; import mockFeatureItemCanManageBitstreamsResponse from './mock-feature-item-can-manage-bitstreams-response.json'; -import mockFeatureItemCanMoveResponse from './mock-feature-item-can-move-response.json'; -import mockFeatureItemCanDeleteNoneResponse from './mock-feature-item-can-delete-none-response.json'; +import mockFeatureItemCanManageRelationshipsResponse from './mock-feature-item-can-manage-relationships-response.json'; +import mockFeatureItemCanManageVersionsResponse from './mock-feature-item-can-manage-versions-response.json'; +import mockFeatureItemCanManageMappingsResponse from './mock-feature-item-can-manage-mappings-response.json'; export class ResponseMapMock extends Map {} @@ -20,6 +21,7 @@ export const mockResponseMap: ResponseMapMock = new Map([ // [ '/api/pid/find', mockPublicationResponse ], // [ '/api/pid/find', mockUntypedItemResponse ], [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canManageBitstreams&embed=feature', mockFeatureItemCanManageBitstreamsResponse ], - [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canMove&embed=feature', mockFeatureItemCanMoveResponse ], - [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/96715576-3748-4761-ad45-001646632963&feature=canDelete&embed=feature', mockFeatureItemCanDeleteNoneResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/047556d1-3d01-4c53-bc68-0cee7ad7ed4e&feature=canManageRelationships&embed=feature', mockFeatureItemCanManageRelationshipsResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/e98b0f27-5c19-49a0-960d-eb6ad5287067&feature=canManageVersions&embed=feature', mockFeatureItemCanManageVersionsResponse ], + [ 'https://api7.dspace.org/server/api/authz/authorizations/search/object?uri=https://api7.dspace.org/server/api/core/items/e98b0f27-5c19-49a0-960d-eb6ad5287067&feature=canManageMappings&embed=feature', mockFeatureItemCanManageMappingsResponse ], ]); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index abecfce291..818606372b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1761,7 +1761,7 @@ "item.edit.tabs.status.buttons.reinstate.label": "Reinstate item into the repository", - "item.edit.tabs.status.buttons.unauthorized": "You don't have permission to perform this action", + "item.edit.tabs.status.buttons.unauthorized": "You're not authorized to perform this action", "item.edit.tabs.status.buttons.withdraw.button": "Withdraw...", From 25f02b99d323fe630d0b482fdf3a02597b9e20b0 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 16 Apr 2021 12:31:22 +0200 Subject: [PATCH 11/42] 78243: message change --- src/assets/i18n/en.json5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 818606372b..600c884938 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1516,7 +1516,7 @@ "item.edit.breadcrumbs": "Edit Item", - "item.edit.tabs.disabled.tooltip": "You don't have permission to access this tab", + "item.edit.tabs.disabled.tooltip": "You're not authorized to access this tab", "item.edit.tabs.mapper.head": "Collection Mapper", From add2aac9349573bcbcbb3db3035ef3f7595a99e5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 19 Apr 2021 15:41:33 +0000 Subject: [PATCH 12/42] Bump ssri from 6.0.1 to 6.0.2 Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2. - [Release notes](https://github.com/npm/ssri/releases) - [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md) - [Commits](https://github.com/npm/ssri/compare/v6.0.1...v6.0.2) Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 29e0ee4648..7c8b54f47c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10792,9 +10792,9 @@ sshpk@^1.7.0: tweetnacl "~0.14.0" ssri@^6.0.0, ssri@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/ssri/-/ssri-6.0.1.tgz#2a3c41b28dd45b62b63676ecb74001265ae9edd8" - integrity sha512-3Wge10hNcT1Kur4PDFwEieXSCMCJs/7WvSACcrMYrNp+b8kDL1/0wJch5Ni2WrtwEa2IO8OsVfeKIciKCDx/QA== + version "6.0.2" + resolved "https://registry.yarnpkg.com/ssri/-/ssri-6.0.2.tgz#157939134f20464e7301ddba3e90ffa8f7728ac5" + integrity sha512-cepbSq/neFK7xB6A50KHN0xHDotYzq58wWCa5LeWqnPrHG8GzfEjO/4O8kpmcGW+oaxkvhEJCWgbgNk4/ZV93Q== dependencies: figgy-pudding "^3.5.1" From ed7454ffc9230b535f0a63d0b1f2972793935571 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 20 Apr 2021 09:37:28 +0200 Subject: [PATCH 13/42] Fix visibility issue with collapsed navbar menu --- .../header-nav-wrapper/header-navbar-wrapper.component.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/header-nav-wrapper/header-navbar-wrapper.component.scss b/src/app/header-nav-wrapper/header-navbar-wrapper.component.scss index b297979fd0..a2ebd0d41a 100644 --- a/src/app/header-nav-wrapper/header-navbar-wrapper.component.scss +++ b/src/app/header-nav-wrapper/header-navbar-wrapper.component.scss @@ -5,3 +5,7 @@ position: sticky; } } + +:host { + z-index: var(--ds-nav-z-index); +} From a205aa02b3316d97c1bc9650987375f827cc79b2 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 15 Apr 2021 13:01:31 +0200 Subject: [PATCH 14/42] [CST-4009] Retrieve configuration by search config service --- src/app/+my-dspace-page/my-dspace-page.component.ts | 2 +- src/app/+search-page/search.component.ts | 2 +- src/assets/i18n/en.json5 | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) 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 b95a296ed1..a5dcfe96be 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.ts @@ -159,7 +159,7 @@ export class MyDSpacePageComponent implements OnInit { }) ); - const configuration$ = this.routeService.getRouteParameterValue('configuration'); + const configuration$ = this.searchConfigService.getCurrentConfiguration('workspace'); this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(configuration$, this.service); diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index bffa958ee5..ce08d2c365 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -136,7 +136,7 @@ export class SearchComponent implements OnInit { switchMap((scopeId) => this.service.getScopes(scopeId)) ); if (isEmpty(this.configuration$)) { - this.configuration$ = this.routeService.getRouteParameterValue('configuration'); + this.configuration$ = this.searchConfigService.getCurrentConfiguration('default'); } this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(this.configuration$, this.service); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 4ea4d1de42..a63d57d0d8 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3053,6 +3053,10 @@ "sorting.dc.date.accessioned.DESC": "Accessioned Date Descending", + "sorting.lastModified.ASC": "Last modified Ascending", + + "sorting.lastModified.DESC": "Last modified Descending", + "statistics.title": "Statistics", From b6ab3d2067031dfed44e4015112bd7e46d8758c8 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Apr 2021 16:06:15 +0200 Subject: [PATCH 15/42] [CST-4087] fix issue with mydspace result default order --- .../shared/search/search-configuration.service.ts | 11 +++++++---- .../search/search-filters/search-config.model.ts | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index e10ab668cc..5de30fc4a7 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -21,7 +21,7 @@ import { RouteService } from '../../services/route.service'; import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../operators'; import { hasNoValue, hasValue, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util'; import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; -import { SearchConfig } from './search-filters/search-config.model'; +import { SearchConfig, SortOption } from './search-filters/search-config.model'; import { SearchService } from './search.service'; import { of } from 'rxjs/internal/observable/of'; import { PaginationService } from '../../pagination/pagination.service'; @@ -216,9 +216,12 @@ export class SearchConfigurationService implements OnDestroy { getFirstSucceededRemoteDataPayload(), map((searchConfig: SearchConfig) => { const sortOptions = []; - searchConfig.sortOptions.forEach(sortOption => { - sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); - sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); + searchConfig.sortOptions.forEach((sortOption: SortOption) => { + console.log(sortOption); + const firstOrder = (sortOption.sortOrder.toLowerCase() === SortDirection.ASC.toLowerCase()) ? SortDirection.ASC : SortDirection.DESC; + const secondOrder = (sortOption.sortOrder.toLowerCase() !== SortDirection.ASC.toLowerCase()) ? SortDirection.ASC : SortDirection.DESC; + sortOptions.push(new SortOptions(sortOption.name, firstOrder)); + sortOptions.push(new SortOptions(sortOption.name, secondOrder)); }); return sortOptions; })); diff --git a/src/app/core/shared/search/search-filters/search-config.model.ts b/src/app/core/shared/search/search-filters/search-config.model.ts index dd7a799f37..725761fe7b 100644 --- a/src/app/core/shared/search/search-filters/search-config.model.ts +++ b/src/app/core/shared/search/search-filters/search-config.model.ts @@ -65,6 +65,7 @@ export interface FilterConfig { */ export interface SortOption { name: string; + sortOrder: string; } /** From 1aa659e6b7e9899bbccc6b4858a688d638163b19 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 23 Apr 2021 13:28:41 +0200 Subject: [PATCH 16/42] 78849: Fix cache/re-request issue & other improvements --- .../core/data/eperson-registration.service.ts | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index fd55c031d8..b8614e62b5 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -3,7 +3,7 @@ import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { GetRequest, PostRequest } from './request.models'; import { Observable } from 'rxjs'; -import { filter, find, map, take } from 'rxjs/operators'; +import { filter, find, map, skipWhile, take } from 'rxjs/operators'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { Registration } from '../shared/registration.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteData } from '../shared/operators'; @@ -60,9 +60,9 @@ export class EpersonRegistrationService { const requestId = this.requestService.generateRequestId(); - const hrefObs = this.getRegistrationEndpoint(); + const href$ = this.getRegistrationEndpoint(); - hrefObs.pipe( + href$.pipe( find((href: string) => hasValue(href)), map((href: string) => { const request = new PostRequest(requestId, href, registration); @@ -82,27 +82,28 @@ export class EpersonRegistrationService { searchByToken(token: string): Observable { const requestId = this.requestService.generateRequestId(); - const hrefObs = this.getTokenSearchEndpoint(token); - - hrefObs.pipe( + const href$ = this.getTokenSearchEndpoint(token).pipe( find((href: string) => hasValue(href)), - map((href: string) => { - const request = new GetRequest(requestId, href); - Object.assign(request, { - getResponseParser(): GenericConstructor { - return RegistrationResponseParsingService; - } - }); - this.requestService.send(request, true); - }) - ).subscribe(); + ) - return this.rdbService.buildFromRequestUUID(requestId).pipe( + href$.subscribe((href: string) => { + const request = new GetRequest(requestId, href); + Object.assign(request, { + getResponseParser(): GenericConstructor { + return RegistrationResponseParsingService; + } + }); + this.requestService.send(request, true); + }); + + return this.rdbService.buildSingle(href$).pipe( + skipWhile((rd: RemoteData) => rd.isStale), getFirstSucceededRemoteData(), map((restResponse: RemoteData) => { - return Object.assign(new Registration(), {email: restResponse.payload.email, token: token, user: restResponse.payload.user}); + return Object.assign(new Registration(), { + email: restResponse.payload.email, token: token, user: restResponse.payload.user + }); }), - take(1), ); } From 7d0ea04b3e9d5b3bd75aa45ce70266aa2a4db5d1 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 23 Apr 2021 15:39:15 +0200 Subject: [PATCH 17/42] 78849: Add unit tests for EPerson registration caching --- .../data/eperson-registration.service.spec.ts | 61 +++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.spec.ts b/src/app/core/data/eperson-registration.service.spec.ts index fe3a1958eb..cffd3266e5 100644 --- a/src/app/core/data/eperson-registration.service.spec.ts +++ b/src/app/core/data/eperson-registration.service.spec.ts @@ -1,15 +1,18 @@ import { RequestService } from './request.service'; import { EpersonRegistrationService } from './eperson-registration.service'; import { RestResponse } from '../cache/response.models'; -import { RequestEntry } from './request.reducer'; +import { RequestEntry, RequestEntryState } from './request.reducer'; import { cold } from 'jasmine-marbles'; import { PostRequest } from './request.models'; import { Registration } from '../shared/registration.model'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; -import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { createPendingRemoteDataObject, createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; import { of as observableOf } from 'rxjs/internal/observable/of'; +import { TestScheduler } from 'rxjs/testing'; + +fdescribe('EpersonRegistrationService', () => { + let testScheduler; -describe('EpersonRegistrationService', () => { let service: EpersonRegistrationService; let requestService: RequestService; @@ -29,6 +32,12 @@ describe('EpersonRegistrationService', () => { rd = createSuccessfulRemoteDataObject(registrationWithUser); halService = new HALEndpointServiceStub('rest-url'); + testScheduler = new TestScheduler((actual, expected) => { + // asserting the two objects are equal + // e.g. using chai. + expect(actual).toEqual(expected); + }); + requestService = jasmine.createSpyObj('requestService', { generateRequestId: 'request-id', send: {}, @@ -36,7 +45,8 @@ describe('EpersonRegistrationService', () => { { a: Object.assign(new RequestEntry(), { response: new RestResponse(true, 200, 'Success') }) }) }); rdbService = jasmine.createSpyObj('rdbService', { - buildFromRequestUUID: observableOf(rd) + buildSingle: observableOf(rd), + buildFromRequestUUID: observableOf(rd), }); service = new EpersonRegistrationService( requestService, @@ -88,6 +98,49 @@ describe('EpersonRegistrationService', () => { })); }); + + it('should return the original registration if it was already cached', () => { + testScheduler.run(({ cold, expectObservable }) => { + rdbService.buildSingle.and.returnValue(cold('a-b-c', { + a: createSuccessfulRemoteDataObject(registrationWithUser), + b: createPendingRemoteDataObject(), + c: createSuccessfulRemoteDataObject(new Registration()) + })); + + expectObservable( + service.searchByToken('test-token') + ).toBe('(a|)', { + a: Object.assign(new Registration(), { + email: registrationWithUser.email, + token: 'test-token', + user: registrationWithUser.user + }) + }); + }); + }); + + it('should re-request the registration if it was already cached but stale', () => { + const rdCachedStale = createSuccessfulRemoteDataObject(new Registration()); + rdCachedStale.state = RequestEntryState.SuccessStale; + + testScheduler.run(({ cold, expectObservable }) => { + rdbService.buildSingle.and.returnValue(cold('a-b-c', { + a: rdCachedStale, + b: createPendingRemoteDataObject(), + c: createSuccessfulRemoteDataObject(registrationWithUser), + })); + + expectObservable( + service.searchByToken('test-token') + ).toBe('----(c|)', { + c: Object.assign(new Registration(), { + email: registrationWithUser.email, + token: 'test-token', + user: registrationWithUser.user + }) + }); + }); + }); }); }); From 7c0d9acbf1bca7a340cf040d80b00a80322f1c1e Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Fri, 23 Apr 2021 16:46:44 +0200 Subject: [PATCH 18/42] [CST-4009] default sort option configured with default sort order --- .../my-dspace-page.component.spec.ts | 2 +- .../my-dspace-page.component.ts | 6 +-- src/app/+search-page/search.component.spec.ts | 2 +- src/app/+search-page/search.component.ts | 11 ++-- .../search/search-configuration.service.ts | 51 +++++++++++-------- 5 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/app/+my-dspace-page/my-dspace-page.component.spec.ts b/src/app/+my-dspace-page/my-dspace-page.component.spec.ts index 909239b61c..b4b75b42a0 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.spec.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.spec.ts @@ -45,7 +45,7 @@ describe('MyDSpacePageComponent', () => { pagination.id = 'mydspace-results-pagination'; pagination.currentPage = 1; pagination.pageSize = 10; - const sortOption = { name: 'score', metadata: null }; + const sortOption = { name: 'score', sortOrder: 'DESC', metadata: null }; const sort: SortOptions = new SortOptions('score', SortDirection.DESC); const mockResults = createSuccessfulRemoteDataObject$(['test', 'data']); const searchServiceStub = jasmine.createSpyObj('SearchService', { 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 a5dcfe96be..3ded17191e 100644 --- a/src/app/+my-dspace-page/my-dspace-page.component.ts +++ b/src/app/+my-dspace-page/my-dspace-page.component.ts @@ -160,10 +160,10 @@ export class MyDSpacePageComponent implements OnInit { ); const configuration$ = this.searchConfigService.getCurrentConfiguration('workspace'); + const searchConfig$ = this.searchConfigService.getConfigurationSearchConfigObservable(configuration$, this.service); - this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(configuration$, this.service); - - this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$); + this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(searchConfig$); + this.searchConfigService.initializeSortOptionsFromConfiguration(searchConfig$); } diff --git a/src/app/+search-page/search.component.spec.ts b/src/app/+search-page/search.component.spec.ts index 06061c1d40..bcbf3f0f77 100644 --- a/src/app/+search-page/search.component.spec.ts +++ b/src/app/+search-page/search.component.spec.ts @@ -40,7 +40,7 @@ const pagination: PaginationComponentOptions = new PaginationComponentOptions(); pagination.id = 'search-results-pagination'; pagination.currentPage = 1; pagination.pageSize = 10; -const sortOption = { name: 'score', metadata: null }; +const sortOption = { name: 'score', sortOrder: 'DESC', metadata: null }; const sort: SortOptions = new SortOptions('score', SortDirection.DESC); const mockResults = createSuccessfulRemoteDataObject$(['test', 'data']); const searchServiceStub = jasmine.createSpyObj('SearchService', { diff --git a/src/app/+search-page/search.component.ts b/src/app/+search-page/search.component.ts index ce08d2c365..b817c82a57 100644 --- a/src/app/+search-page/search.component.ts +++ b/src/app/+search-page/search.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectionStrategy, Component, Inject, Input, OnInit } from '@angular/core'; -import { BehaviorSubject, combineLatest, Observable, Subscription } from 'rxjs'; -import { map, startWith, switchMap, take, } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subscription } from 'rxjs'; +import { startWith, switchMap } from 'rxjs/operators'; import { PaginatedList } from '../core/data/paginated-list.model'; import { RemoteData } from '../core/data/remote-data'; import { DSpaceObject } from '../core/shared/dspace-object.model'; @@ -8,7 +8,7 @@ import { pushInOut } from '../shared/animations/push'; import { HostWindowService } from '../shared/host-window.service'; import { SidebarService } from '../shared/sidebar/sidebar.service'; import { hasValue, isEmpty } from '../shared/empty.util'; -import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators'; +import { getFirstSucceededRemoteData } from '../core/shared/operators'; import { RouteService } from '../core/services/route.service'; import { SEARCH_CONFIG_SERVICE } from '../+my-dspace-page/my-dspace-page.component'; import { PaginatedSearchOptions } from '../shared/search/paginated-search-options.model'; @@ -139,9 +139,10 @@ export class SearchComponent implements OnInit { this.configuration$ = this.searchConfigService.getCurrentConfiguration('default'); } - this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(this.configuration$, this.service); + const searchConfig$ = this.searchConfigService.getConfigurationSearchConfigObservable(this.configuration$, this.service); - this.searchConfigService.initializeSortOptionsFromConfiguration(this.sortOptions$); + this.sortOptions$ = this.searchConfigService.getConfigurationSortOptionsObservable(searchConfig$); + this.searchConfigService.initializeSortOptionsFromConfiguration(searchConfig$); } diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index 5de30fc4a7..c209d79e40 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -21,7 +21,7 @@ import { RouteService } from '../../services/route.service'; import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../operators'; import { hasNoValue, hasValue, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util'; import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; -import { SearchConfig, SortOption } from './search-filters/search-config.model'; +import { SearchConfig } from './search-filters/search-config.model'; import { SearchService } from './search.service'; import { of } from 'rxjs/internal/observable/of'; import { PaginationService } from '../../pagination/pagination.service'; @@ -205,40 +205,33 @@ export class SearchConfigurationService implements OnDestroy { } /** - * Creates an observable of SortOptions[] every time the configuration$ stream emits. + * Creates an observable of SearchConfig every time the configuration$ stream emits. * @param configuration$ * @param service */ - getConfigurationSortOptionsObservable(configuration$: Observable, service: SearchService): Observable { + getConfigurationSearchConfigObservable(configuration$: Observable, service: SearchService): Observable { return configuration$.pipe( distinctUntilChanged(), switchMap((configuration) => service.getSearchConfigurationFor(null, configuration)), - getFirstSucceededRemoteDataPayload(), - map((searchConfig: SearchConfig) => { - const sortOptions = []; - searchConfig.sortOptions.forEach((sortOption: SortOption) => { - console.log(sortOption); - const firstOrder = (sortOption.sortOrder.toLowerCase() === SortDirection.ASC.toLowerCase()) ? SortDirection.ASC : SortDirection.DESC; - const secondOrder = (sortOption.sortOrder.toLowerCase() !== SortDirection.ASC.toLowerCase()) ? SortDirection.ASC : SortDirection.DESC; - sortOptions.push(new SortOptions(sortOption.name, firstOrder)); - sortOptions.push(new SortOptions(sortOption.name, secondOrder)); - }); - return sortOptions; - })); + getFirstSucceededRemoteDataPayload()); } /** - * Every time sortOptions change (after a configuration change) it update the navigation with the default sort option + * Every time searchConfig change (after a configuration change) it update the navigation with the default sort option * and emit the new paginateSearchOptions value. * @param configuration$ * @param service */ - initializeSortOptionsFromConfiguration(sortOptions$: Observable) { - const subscription = sortOptions$.pipe(switchMap((sortOptions) => combineLatest([ - of(sortOptions), + initializeSortOptionsFromConfiguration(searchConfig$: Observable) { + const subscription = searchConfig$.pipe(switchMap((searchConfig) => combineLatest([ + of(searchConfig), this.paginatedSearchOptions.pipe(take(1)) - ]))).subscribe(([sortOptions, searchOptions]) => { - const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { sort: sortOptions[0]}); + ]))).subscribe(([searchConfig, searchOptions]) => { + const field = searchConfig.sortOptions[0].name; + const direction = searchConfig.sortOptions[0].sortOrder.toLowerCase() === SortDirection.ASC.toLowerCase() ? SortDirection.ASC : SortDirection.DESC; + const updateValue = Object.assign(new PaginatedSearchOptions({}), searchOptions, { + sort: new SortOptions(field, direction) + }); this.paginationService.updateRoute(this.paginationID, { sortDirection: updateValue.sort.direction, @@ -249,6 +242,22 @@ export class SearchConfigurationService implements OnDestroy { this.subs.push(subscription); } + /** + * Creates an observable of available SortOptions[] every time the searchConfig$ stream emits. + * @param searchConfig$ + * @param service + */ + getConfigurationSortOptionsObservable(searchConfig$: Observable): Observable { + return searchConfig$.pipe(map((searchConfig) => { + const sortOptions = []; + searchConfig.sortOptions.forEach(sortOption => { + sortOptions.push(new SortOptions(sortOption.name, SortDirection.ASC)); + sortOptions.push(new SortOptions(sortOption.name, SortDirection.DESC)); + }); + return sortOptions; + })); + } + /** * Sets up a subscription to all necessary parameters to make sure the searchOptions emits a new value every time they update * @param {SearchOptions} defaults Default values for when no parameters are available From 8433f49ed92c9c5ad8faf101c1ea649b348a6b82 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 09:09:51 +0200 Subject: [PATCH 19/42] 78991: Initialize slider handle width --- src/styles/_custom_variables.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/src/styles/_custom_variables.scss b/src/styles/_custom_variables.scss index 56cf4abca9..298be09f67 100644 --- a/src/styles/_custom_variables.scss +++ b/src/styles/_custom_variables.scss @@ -78,4 +78,5 @@ --ds-breadcrumb-link-active-color: #{darken($cyan, 30%)}; --ds-slider-color: #{$green}; + --ds-slider-handle-width: 18px; } From eb9a7a15d6d008bf63b944211a447c7e4be50b99 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 09:55:28 +0200 Subject: [PATCH 20/42] 78991: Specify filter operator for (date) ranges --- .../+my-dspace-page/my-dspace-configuration.service.spec.ts | 5 ++++- .../core/shared/search/search-configuration.service.spec.ts | 5 ++++- src/app/core/shared/search/search-configuration.service.ts | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/app/+my-dspace-page/my-dspace-configuration.service.spec.ts b/src/app/+my-dspace-page/my-dspace-configuration.service.spec.ts index 87a2f8a9dd..fa278da967 100644 --- a/src/app/+my-dspace-page/my-dspace-configuration.service.spec.ts +++ b/src/app/+my-dspace-page/my-dspace-configuration.service.spec.ts @@ -27,7 +27,10 @@ describe('MyDSpaceConfigurationService', () => { scope: '' }); - const backendFilters = [new SearchFilter('f.namedresourcetype', ['another value']), new SearchFilter('f.dateSubmitted', ['[2013 TO 2018]'])]; + const backendFilters = [ + new SearchFilter('f.namedresourcetype', ['another value']), + new SearchFilter('f.dateSubmitted', ['[2013 TO 2018]'], 'equals') + ]; const spy = jasmine.createSpyObj('RouteService', { getQueryParameterValue: observableOf(value1), diff --git a/src/app/core/shared/search/search-configuration.service.spec.ts b/src/app/core/shared/search/search-configuration.service.spec.ts index 061182c2fc..805ecd0486 100644 --- a/src/app/core/shared/search/search-configuration.service.spec.ts +++ b/src/app/core/shared/search/search-configuration.service.spec.ts @@ -23,7 +23,10 @@ describe('SearchConfigurationService', () => { scope: '' }); - const backendFilters = [new SearchFilter('f.author', ['another value']), new SearchFilter('f.date', ['[2013 TO 2018]'])]; + const backendFilters = [ + new SearchFilter('f.author', ['another value']), + new SearchFilter('f.date', ['[2013 TO 2018]'], 'equals') + ]; const routeService = jasmine.createSpyObj('RouteService', { getQueryParameterValue: observableOf(value1), diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index 798a0de287..7983bec64d 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -168,7 +168,7 @@ export class SearchConfigurationService implements OnDestroy { if (hasNoValue(filters.find((f) => f.key === realKey))) { const min = filterParams[realKey + '.min'] ? filterParams[realKey + '.min'][0] : '*'; const max = filterParams[realKey + '.max'] ? filterParams[realKey + '.max'][0] : '*'; - filters.push(new SearchFilter(realKey, ['[' + min + ' TO ' + max + ']'])); + filters.push(new SearchFilter(realKey, ['[' + min + ' TO ' + max + ']'], 'equals')); } } else { filters.push(new SearchFilter(key, filterParams[key])); From 4fa6a3e97651a4e57dad2fb93c2f53db97e323ad Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 10:53:37 +0200 Subject: [PATCH 21/42] 78991: Set fallback max date to the current year --- .../search-range-filter/search-range-filter.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts index c3139c0217..62b1cb98a6 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts @@ -56,7 +56,7 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple /** * Fallback maximum for the range */ - max = 2018; + max = new Date().getFullYear(); /** * The current range of the filter From 6c2a3431c1a8af0bbb5541d4edf1bd7d12d2ea26 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 11:41:43 +0200 Subject: [PATCH 22/42] 78991: Fix range handle lines & remove duplicate CSS variable --- .../search-range-filter/search-range-filter.component.scss | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss index 2b5029fce2..2c98280e7f 100644 --- a/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss +++ b/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.scss @@ -8,18 +8,16 @@ ::ng-deep { - --ds-slider-handle-width: 18px; - html:not([dir=rtl]) .noUi-horizontal .noUi-handle { right: calc(var(--ds-slider-handle-width) / -2); } .noUi-horizontal .noUi-handle { width: var(--ds-slider-handle-width); &:before { - left: calc(calc(calc(var(--ds-slider-handle-width) - 2) / 2) - 2); + left: calc(((var(--ds-slider-handle-width) - 2px) / 2) - 2px); } &:after { - left: calc(calc(calc(var(--ds-slider-handle-width) - 2) / 2) + 2); + left: calc(((var(--ds-slider-handle-width) - 2px) / 2) + 2px); } &:focus { outline: none; From 4634d2a4a81afe311841199a4e5a08554c7d7c56 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 11:46:16 +0200 Subject: [PATCH 23/42] 78991: Don't initialize SearchFilterComponent with closed=true --- .../search-filters/search-filter/search-filter.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/shared/search/search-filters/search-filter/search-filter.component.ts b/src/app/shared/search/search-filters/search-filter/search-filter.component.ts index 76a8a0d323..31ace10a7d 100644 --- a/src/app/shared/search/search-filters/search-filter/search-filter.component.ts +++ b/src/app/shared/search/search-filters/search-filter/search-filter.component.ts @@ -35,7 +35,7 @@ export class SearchFilterComponent implements OnInit { /** * True when the filter is 100% collapsed in the UI */ - closed = true; + closed: boolean; /** * Emits true when the filter is currently collapsed in the store From 5f45e93d12d3efa42322dfc268424a660c122e17 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 13:24:26 +0200 Subject: [PATCH 24/42] 78994: Remove setItem method --- ...dynamic-lookup-relation-modal.component.ts | 44 ++----------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts index 4ed972b2fa..6d3b14d8e8 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts @@ -12,11 +12,8 @@ import { RelationshipOptions } from '../../models/relationship-options.model'; import { SearchResult } from '../../../../search/search-result.model'; import { Item } from '../../../../../core/shared/item.model'; import { - getAllSucceededRemoteData, - getAllSucceededRemoteDataPayload, - getRemoteDataPayload -} from '../../../../../core/shared/operators'; -import { AddRelationshipAction, RemoveRelationshipAction, UpdateRelationshipNameVariantAction } from './relationship.actions'; + AddRelationshipAction, RemoveRelationshipAction, UpdateRelationshipNameVariantAction, +} from './relationship.actions'; import { RelationshipService } from '../../../../../core/data/relationship.service'; import { RelationshipTypeService } from '../../../../../core/data/relationship-type.service'; import { Store } from '@ngrx/store'; @@ -27,12 +24,7 @@ import { ExternalSource } from '../../../../../core/shared/external-source.model import { ExternalSourceService } from '../../../../../core/data/external-source.service'; import { Router } from '@angular/router'; import { RemoteDataBuildService } from '../../../../../core/cache/builders/remote-data-build.service'; -import { followLink } from '../../../../utils/follow-link-config.model'; -import { SubmissionObject } from '../../../../../core/submission/models/submission-object.model'; -import { Collection } from '../../../../../core/shared/collection.model'; -import { SubmissionService } from '../../../../../submission/submission.service'; -import { SubmissionObjectDataService } from '../../../../../core/submission/submission-object-data.service'; -import { RemoteData } from '../../../../../core/data/remote-data'; +import { getAllSucceededRemoteDataPayload } from '../../../../../core/shared/operators'; @Component({ selector: 'ds-dynamic-lookup-relation-modal', @@ -122,10 +114,6 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy */ totalExternal$: Observable; - /** - * List of subscriptions to unsubscribe from - */ - private subs: Subscription[] = []; constructor( public modal: NgbActiveModal, @@ -136,17 +124,14 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy private lookupRelationService: LookupRelationService, private searchConfigService: SearchConfigurationService, private rdbService: RemoteDataBuildService, - private submissionService: SubmissionService, - private submissionObjectService: SubmissionObjectDataService, private zone: NgZone, private store: Store, - private router: Router + private router: Router, ) { } ngOnInit(): void { - this.setItem(); this.selection$ = this.selectableListService .getSelectableList(this.listId) .pipe(map((listState: SelectableListState) => hasValue(listState) && hasValue(listState.selection) ? listState.selection : [])); @@ -206,24 +191,6 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy }); } - /** - * Initialize this.item$ based on this.model.submissionId - */ - private setItem() { - const submissionObject$ = this.submissionObjectService - .findById(this.submissionId, true, true, followLink('item'), followLink('collection')).pipe( - getAllSucceededRemoteData(), - getRemoteDataPayload() - ); - - const item$ = submissionObject$.pipe(switchMap((submissionObject: SubmissionObject) => (submissionObject.item as Observable>).pipe(getAllSucceededRemoteData(), getRemoteDataPayload()))); - const collection$ = submissionObject$.pipe(switchMap((submissionObject: SubmissionObject) => (submissionObject.collection as Observable>).pipe(getAllSucceededRemoteData(), getRemoteDataPayload()))); - - this.subs.push(item$.subscribe((item) => this.item = item)); - this.subs.push(collection$.subscribe((collection) => this.collection = collection)); - - } - /** * Add a subscription updating relationships with name variants * @param sri The search result to track name variants for @@ -279,8 +246,5 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy ngOnDestroy() { this.router.navigate([], {}); Object.values(this.subMap).forEach((subscription) => subscription.unsubscribe()); - this.subs - .filter((sub) => hasValue(sub)) - .forEach((sub) => sub.unsubscribe()); } } From d62d9b0f485212ca6a31f0ee5b9d7afa63d024f0 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 15:13:27 +0200 Subject: [PATCH 25/42] 78994: Update unit tests --- .../dynamic-lookup-relation-modal.component.spec.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts index 47dd857d53..19d4760183 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.spec.ts @@ -21,8 +21,6 @@ import { createPaginatedList } from '../../../../testing/utils.test'; import { ExternalSourceService } from '../../../../../core/data/external-source.service'; import { LookupRelationService } from '../../../../../core/data/lookup-relation.service'; import { RemoteDataBuildService } from '../../../../../core/cache/builders/remote-data-build.service'; -import { SubmissionService } from '../../../../../submission/submission.service'; -import { SubmissionObjectDataService } from '../../../../../core/submission/submission-object-data.service'; import { WorkspaceItem } from '../../../../../core/submission/models/workspaceitem.model'; import { Collection } from '../../../../../core/shared/collection.model'; @@ -46,8 +44,6 @@ describe('DsDynamicLookupRelationModalComponent', () => { let lookupRelationService; let rdbService; let submissionId; - let submissionService; - let submissionObjectDataService; const externalSources = [ Object.assign(new ExternalSource(), { @@ -99,12 +95,6 @@ describe('DsDynamicLookupRelationModalComponent', () => { rdbService = jasmine.createSpyObj('rdbService', { aggregate: createSuccessfulRemoteDataObject$(externalSources) }); - submissionService = jasmine.createSpyObj('SubmissionService', { - dispatchSave: jasmine.createSpy('dispatchSave') - }); - submissionObjectDataService = jasmine.createSpyObj('SubmissionObjectDataService', { - findById: createSuccessfulRemoteDataObject$(testWSI) - }); submissionId = '1234'; } @@ -129,8 +119,6 @@ describe('DsDynamicLookupRelationModalComponent', () => { }, { provide: RelationshipTypeService, useValue: {} }, { provide: RemoteDataBuildService, useValue: rdbService }, - { provide: SubmissionService, useValue: submissionService }, - { provide: SubmissionObjectDataService, useValue: submissionObjectDataService }, { provide: Store, useValue: { // tslint:disable-next-line:no-empty From aca1c86455292b1ecddb02d1688dae8d02bf5c95 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 15:02:50 +0200 Subject: [PATCH 26/42] 78994: Provide Item's owning collection to relation modal --- .../edit-relationship-list.component.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts index 2082143e05..3f9637cdc9 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -18,7 +18,7 @@ import { RelationshipType } from '../../../../core/shared/item-relationships/rel import { getAllSucceededRemoteData, getRemoteDataPayload, - getFirstSucceededRemoteData, + getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload, } from '../../../../core/shared/operators'; import { ItemType } from '../../../../core/shared/item-relationships/item-type.model'; import { DsDynamicLookupRelationModalComponent } from '../../../../shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component'; @@ -29,6 +29,7 @@ import { SearchResult } from '../../../../shared/search/search-result.model'; import { followLink } from '../../../../shared/utils/follow-link-config.model'; import { PaginatedList } from '../../../../core/data/paginated-list.model'; import { RemoteData } from '../../../../core/data/remote-data'; +import { Collection } from '../../../../core/shared/collection.model'; @Component({ selector: 'ds-edit-relationship-list', @@ -146,6 +147,11 @@ export class EditRelationshipListComponent implements OnInit { modalComp.repeatable = true; modalComp.listId = this.listId; modalComp.item = this.item; + this.item.owningCollection.pipe( + getFirstSucceededRemoteDataPayload() + ).subscribe((collection: Collection) => { + modalComp.collection = collection; + }); modalComp.select = (...selectableObjects: SearchResult[]) => { selectableObjects.forEach((searchResult) => { const relatedItem: Item = searchResult.indexableObject; From 0fe199a97c05a05f5f26847f778ffeab8acdc99f Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 26 Apr 2021 16:49:07 +0200 Subject: [PATCH 27/42] 78849: Fix unit test --- .../data/eperson-registration.service.spec.ts | 55 +++++-------------- 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.spec.ts b/src/app/core/data/eperson-registration.service.spec.ts index cffd3266e5..cebe7ffa80 100644 --- a/src/app/core/data/eperson-registration.service.spec.ts +++ b/src/app/core/data/eperson-registration.service.spec.ts @@ -1,16 +1,16 @@ import { RequestService } from './request.service'; import { EpersonRegistrationService } from './eperson-registration.service'; import { RestResponse } from '../cache/response.models'; -import { RequestEntry, RequestEntryState } from './request.reducer'; +import { RequestEntry } from './request.reducer'; import { cold } from 'jasmine-marbles'; import { PostRequest } from './request.models'; import { Registration } from '../shared/registration.model'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; -import { createPendingRemoteDataObject, createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; import { of as observableOf } from 'rxjs/internal/observable/of'; import { TestScheduler } from 'rxjs/testing'; -fdescribe('EpersonRegistrationService', () => { +describe('EpersonRegistrationService', () => { let testScheduler; let service: EpersonRegistrationService; @@ -96,51 +96,26 @@ fdescribe('EpersonRegistrationService', () => { user: registrationWithUser.user }) })); - }); - it('should return the original registration if it was already cached', () => { + it('should use cached responses and /registrations/search/findByToken?', () => { testScheduler.run(({ cold, expectObservable }) => { - rdbService.buildSingle.and.returnValue(cold('a-b-c', { - a: createSuccessfulRemoteDataObject(registrationWithUser), - b: createPendingRemoteDataObject(), - c: createSuccessfulRemoteDataObject(new Registration()) - })); + rdbService.buildSingle.and.returnValue(cold('a', { a: rd })); - expectObservable( - service.searchByToken('test-token') - ).toBe('(a|)', { - a: Object.assign(new Registration(), { - email: registrationWithUser.email, - token: 'test-token', - user: registrationWithUser.user - }) + service.searchByToken('test-token'); + + expect(requestService.send).toHaveBeenCalledWith( + jasmine.objectContaining({ + uuid: 'request-id', method: 'GET', + href: 'rest-url/registrations/search/findByToken?token=test-token', + }), true + ); + expectObservable(rdbService.buildSingle.calls.argsFor(0)[0]).toBe('(a|)', { + a: 'rest-url/registrations/search/findByToken?token=test-token' }); }); }); - it('should re-request the registration if it was already cached but stale', () => { - const rdCachedStale = createSuccessfulRemoteDataObject(new Registration()); - rdCachedStale.state = RequestEntryState.SuccessStale; - - testScheduler.run(({ cold, expectObservable }) => { - rdbService.buildSingle.and.returnValue(cold('a-b-c', { - a: rdCachedStale, - b: createPendingRemoteDataObject(), - c: createSuccessfulRemoteDataObject(registrationWithUser), - })); - - expectObservable( - service.searchByToken('test-token') - ).toBe('----(c|)', { - c: Object.assign(new Registration(), { - email: registrationWithUser.email, - token: 'test-token', - user: registrationWithUser.user - }) - }); - }); - }); }); }); From bdc2dd5f9ca98ddb0e89371e02201cdb5ef1d94a Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Tue, 27 Apr 2021 09:53:38 +0200 Subject: [PATCH 28/42] [CST-4009] fixed search configuration stream --- src/app/core/shared/search/search-configuration.service.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/app/core/shared/search/search-configuration.service.ts b/src/app/core/shared/search/search-configuration.service.ts index c209d79e40..6a1373c87e 100644 --- a/src/app/core/shared/search/search-configuration.service.ts +++ b/src/app/core/shared/search/search-configuration.service.ts @@ -18,7 +18,10 @@ import { RemoteData } from '../../data/remote-data'; import { DSpaceObjectType } from '../dspace-object-type.model'; import { SortDirection, SortOptions } from '../../cache/models/sort-options.model'; import { RouteService } from '../../services/route.service'; -import { getFirstSucceededRemoteData, getFirstSucceededRemoteDataPayload } from '../operators'; +import { + getAllSucceededRemoteDataPayload, + getFirstSucceededRemoteData +} from '../operators'; import { hasNoValue, hasValue, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util'; import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { SearchConfig } from './search-filters/search-config.model'; @@ -213,7 +216,7 @@ export class SearchConfigurationService implements OnDestroy { return configuration$.pipe( distinctUntilChanged(), switchMap((configuration) => service.getSearchConfigurationFor(null, configuration)), - getFirstSucceededRemoteDataPayload()); + getAllSucceededRemoteDataPayload()); } /** From ad7824460b713bc3fd85d725cac8069838f1ae4a Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Wed, 28 Apr 2021 15:28:02 +0200 Subject: [PATCH 29/42] [CST-4009] update en.json5 --- src/assets/i18n/en.json5 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index a63d57d0d8..23c889847f 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3008,6 +3008,8 @@ "search.results.empty": "Your search returned no results.", + "default.search.results.head": "Search Results", + "search.sidebar.close": "Back to results", @@ -3041,9 +3043,9 @@ "sorting.dc.title.DESC": "Title Descending", - "sorting.score.ASC": "Relevance Ascending", + "sorting.score.ASC": "Least Relevant", - "sorting.score.DESC": "Relevance Descending", + "sorting.score.DESC": "Most Relevant", "sorting.dc.date.issued.ASC": "Date Issued Ascending", From d2b44318fa05cd31ee4b84e9333a7cb2e6eb91d2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 28 Apr 2021 21:57:44 +0000 Subject: [PATCH 30/42] Bump elliptic from 6.5.3 to 6.5.4 Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](https://github.com/indutny/elliptic/compare/v6.5.3...v6.5.4) Signed-off-by: dependabot[bot] --- yarn.lock | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/yarn.lock b/yarn.lock index 29e0ee4648..6428d3b153 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2440,10 +2440,10 @@ bluebird@^3.5.1, bluebird@^3.5.3, bluebird@^3.5.5: resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f" integrity sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg== -bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.4.0: - version "4.11.9" - resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.11.9.tgz#26d556829458f9d1e81fc48952493d0ba3507828" - integrity sha512-E6QoYqCKZfgatHTdHzs1RRKP7ip4vvm+EyRUeE2RF0NblwVvb0p6jSVeNTOFxPn26QXN2o6SMfNxKp6kU8zQaw== +bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.11.9: + version "4.12.0" + resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.12.0.tgz#775b3f278efbb9718eec7361f483fb36fbbfea88" + integrity sha512-c98Bf3tPniI+scsdk237ku1Dc3ujXQTSgyiPUDEOe7tRkhrqridvh8klBv0HCEso1OLOYcHuCv/cS6DNxKH+ZA== bn.js@^5.0.0, bn.js@^5.1.1: version "5.1.3" @@ -2533,7 +2533,7 @@ braces@^3.0.1, braces@^3.0.2, braces@~3.0.2: dependencies: fill-range "^7.0.1" -brorand@^1.0.1: +brorand@^1.0.1, brorand@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f" integrity sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8= @@ -4370,17 +4370,17 @@ electron-to-chromium@^1.3.621: integrity sha512-AJT0Fm1W0uZlMVVkkJrcCVvczDuF8tPm3bwzQf5WO8AaASB2hwTRP7B8pU5rqjireH+ib6am8+hH5/QkXzzYKw== elliptic@^6.5.3: - version "6.5.3" - resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.3.tgz#cb59eb2efdaf73a0bd78ccd7015a62ad6e0f93d6" - integrity sha512-IMqzv5wNQf+E6aHeIqATs0tOLeOTwj1QKbRcS3jBbYkl5oLAserA8yJTT7/VyHUYG91PRmPyeQDObKLPpeS4dw== + version "6.5.4" + resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.4.tgz#da37cebd31e79a1367e941b592ed1fbebd58abbb" + integrity sha512-iLhC6ULemrljPZb+QutR5TQGB+pdW6KGD5RSegS+8sorOZT+rdQFbsQFJgvN3eRqNALqJer4oQ16YvJHlU8hzQ== dependencies: - bn.js "^4.4.0" - brorand "^1.0.1" + bn.js "^4.11.9" + brorand "^1.1.0" hash.js "^1.0.0" - hmac-drbg "^1.0.0" - inherits "^2.0.1" - minimalistic-assert "^1.0.0" - minimalistic-crypto-utils "^1.0.0" + hmac-drbg "^1.0.1" + inherits "^2.0.4" + minimalistic-assert "^1.0.1" + minimalistic-crypto-utils "^1.0.1" emoji-regex@^7.0.1: version "7.0.3" @@ -5557,7 +5557,7 @@ hex-color-regex@^1.1.0: resolved "https://registry.yarnpkg.com/hex-color-regex/-/hex-color-regex-1.1.0.tgz#4c06fccb4602fe2602b3c93df82d7e7dbf1a8a8e" integrity sha512-l9sfDFsuqtOqKDsQdqrMRk0U85RZc0RtOR9yPI7mRVOa4FsR/BVnZ0shmQRM96Ji99kYZP/7hn1cedc1+ApsTQ== -hmac-drbg@^1.0.0: +hmac-drbg@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1" integrity sha1-0nRXAQJabHdabFRXk+1QL8DGSaE= @@ -7393,7 +7393,7 @@ minimalistic-assert@^1.0.0, minimalistic-assert@^1.0.1: resolved "https://registry.yarnpkg.com/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz#2e194de044626d4a10e7f7fbc00ce73e83e4d5c7" integrity sha512-UtJcAD4yEaGtjPezWuO9wC4nwUnVH/8/Im3yEHQP4b67cXlD/Qr9hdITCU1xDbSEXg2XKNaP8jsReV7vQd00/A== -minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1: +minimalistic-crypto-utils@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a" integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo= From 781a88bc4c2a4478a99e31cb468486611e2e0289 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 29 Apr 2021 13:29:18 +0200 Subject: [PATCH 31/42] 78849: Fix double notification on submit --- .../forgot-password-form/forgot-password-form.component.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/forgot-password/forgot-password-form/forgot-password-form.component.ts b/src/app/forgot-password/forgot-password-form/forgot-password-form.component.ts index 98188a07e8..707c70f19c 100644 --- a/src/app/forgot-password/forgot-password-form/forgot-password-form.component.ts +++ b/src/app/forgot-password/forgot-password-form/forgot-password-form.component.ts @@ -11,6 +11,7 @@ import { Store } from '@ngrx/store'; import { CoreState } from '../../core/core.reducers'; import { RemoteData } from '../../core/data/remote-data'; import { EPerson } from '../../core/eperson/models/eperson.model'; +import { getFirstCompletedRemoteData } from '../../core/shared/operators'; @Component({ selector: 'ds-forgot-password-form', @@ -70,7 +71,9 @@ export class ForgotPasswordFormComponent { */ submit() { if (!this.isInValid) { - this.ePersonDataService.patchPasswordWithToken(this.user, this.token, this.password).subscribe((response: RemoteData) => { + this.ePersonDataService.patchPasswordWithToken(this.user, this.token, this.password).pipe( + getFirstCompletedRemoteData() + ).subscribe((response: RemoteData) => { if (response.hasSucceeded) { this.notificationsService.success( this.translateService.instant(this.NOTIFICATIONS_PREFIX + '.success.title'), From 032231f10e563cf7a898ebdf731385c08fd48c49 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 29 Apr 2021 13:57:31 +0200 Subject: [PATCH 32/42] 78849: Fix lint issues --- src/app/core/data/eperson-registration.service.spec.ts | 5 +++-- src/app/core/data/eperson-registration.service.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.spec.ts b/src/app/core/data/eperson-registration.service.spec.ts index cebe7ffa80..2860880803 100644 --- a/src/app/core/data/eperson-registration.service.spec.ts +++ b/src/app/core/data/eperson-registration.service.spec.ts @@ -99,8 +99,8 @@ describe('EpersonRegistrationService', () => { }); it('should use cached responses and /registrations/search/findByToken?', () => { - testScheduler.run(({ cold, expectObservable }) => { - rdbService.buildSingle.and.returnValue(cold('a', { a: rd })); + testScheduler.run(({ tscold, expectObservable }) => { + rdbService.buildSingle.and.returnValue(tscold('a', { a: rd })); service.searchByToken('test-token'); @@ -119,3 +119,4 @@ describe('EpersonRegistrationService', () => { }); }); +/**/ diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index b8614e62b5..8ea43b3c3f 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -84,7 +84,7 @@ export class EpersonRegistrationService { const href$ = this.getTokenSearchEndpoint(token).pipe( find((href: string) => hasValue(href)), - ) + ); href$.subscribe((href: string) => { const request = new GetRequest(requestId, href); From a24cfe4cc74d5988bf18886d93699baeb4e40d36 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 29 Apr 2021 14:15:03 +0200 Subject: [PATCH 33/42] 78243: Feedback 2021-04-29 --- src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts | 2 +- .../edit-item-page/item-status/item-status.component.ts | 2 +- src/app/core/data/feature-authorization/feature-id.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts b/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts index bf4a6dc681..764c6ac7c8 100644 --- a/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-bitstreams.guard.ts @@ -26,6 +26,6 @@ export class ItemPageBitstreamsGuard extends DsoPageSingleFeatureGuard { * Check manage bitstreams authorization rights */ getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageBitstreams); + return observableOf(FeatureID.CanManageBitstreamBundles); } } diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index f95d2d1517..f01f5c1f7a 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -83,7 +83,7 @@ export class ItemStatusComponent implements OnInit { if (item.isWithdrawn) { operations.push(new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate', FeatureID.ReinstateItem, true)); } else { - operations.push(new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate', FeatureID.WithdrawItem, true)); + operations.push(new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw', FeatureID.WithdrawItem, true)); } if (item.isDiscoverable) { operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private', FeatureID.CanMakePrivate, true)); diff --git a/src/app/core/data/feature-authorization/feature-id.ts b/src/app/core/data/feature-authorization/feature-id.ts index 9a3d13183b..6d070fcd4c 100644 --- a/src/app/core/data/feature-authorization/feature-id.ts +++ b/src/app/core/data/feature-authorization/feature-id.ts @@ -14,7 +14,7 @@ export enum FeatureID { IsCommunityAdmin = 'isCommunityAdmin', CanDownload = 'canDownload', CanManageVersions = 'canManageVersions', - CanManageBitstreams = 'canManageBitstreams', + CanManageBitstreamBundles = 'canManageBitstreamBundles', CanManageRelationships = 'canManageRelationships', CanManageMappings = 'canManageMappings', CanManagePolicies = 'canManagePolicies', From b2b077868e46b73636c7cbdcc99e816b475a0ed8 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 29 Apr 2021 14:26:33 +0200 Subject: [PATCH 34/42] 78994: Disable no-shadowed-variable --- src/app/core/data/eperson-registration.service.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/eperson-registration.service.spec.ts b/src/app/core/data/eperson-registration.service.spec.ts index 2860880803..768d83c024 100644 --- a/src/app/core/data/eperson-registration.service.spec.ts +++ b/src/app/core/data/eperson-registration.service.spec.ts @@ -98,9 +98,10 @@ describe('EpersonRegistrationService', () => { })); }); + // tslint:disable:no-shadowed-variable it('should use cached responses and /registrations/search/findByToken?', () => { - testScheduler.run(({ tscold, expectObservable }) => { - rdbService.buildSingle.and.returnValue(tscold('a', { a: rd })); + testScheduler.run(({ cold, expectObservable }) => { + rdbService.buildSingle.and.returnValue(cold('a', { a: rd })); service.searchByToken('test-token'); From 60009144a117a526716e29226ad9df1331066727 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 29 Apr 2021 14:28:37 +0200 Subject: [PATCH 35/42] 78994: Remove unused import --- src/app/core/data/eperson-registration.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/data/eperson-registration.service.ts b/src/app/core/data/eperson-registration.service.ts index 8ea43b3c3f..adf01b0ce9 100644 --- a/src/app/core/data/eperson-registration.service.ts +++ b/src/app/core/data/eperson-registration.service.ts @@ -3,7 +3,7 @@ import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { GetRequest, PostRequest } from './request.models'; import { Observable } from 'rxjs'; -import { filter, find, map, skipWhile, take } from 'rxjs/operators'; +import { filter, find, map, skipWhile } from 'rxjs/operators'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { Registration } from '../shared/registration.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteData } from '../shared/operators'; From b5342e0fab6220c6dc5ff722c86a91911a12f10f Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Mon, 3 May 2021 18:41:56 +0200 Subject: [PATCH 36/42] 79218: Remove duplicate menu item --- .../admin-sidebar/admin-sidebar.component.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index a9c0a6648d..dbe8ac1042 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -243,20 +243,6 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { } as OnClickMenuItemModel, }, - /* Curation tasks */ - { - id: 'curation_tasks', - active: false, - visible: isCollectionAdmin, - model: { - type: MenuItemType.LINK, - text: 'menu.section.curation_task', - link: '' - } as LinkMenuItemModel, - icon: 'filter', - index: 7 - }, - /* Statistics */ { id: 'statistics_task', From fe4fe9e8d34fff27a9def8ad483f6170f09176d8 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Mon, 3 May 2021 19:07:02 +0200 Subject: [PATCH 37/42] 79218: Comment out all disabled menu options --- .../admin-sidebar/admin-sidebar.component.ts | 194 +++++++++--------- 1 file changed, 101 insertions(+), 93 deletions(-) diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index dbe8ac1042..e172f9717b 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -179,17 +179,18 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { link: '/processes/new' } as LinkMenuItemModel, }, - { - id: 'new_item_version', - parentID: 'new', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.new_item_version', - link: '' - } as LinkMenuItemModel, - }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'new_item_version', + // parentID: 'new', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.new_item_version', + // link: '' + // } as LinkMenuItemModel, + // }, /* Edit */ { @@ -244,32 +245,34 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { }, /* Statistics */ - { - id: 'statistics_task', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.statistics_task', - link: '' - } as LinkMenuItemModel, - icon: 'chart-bar', - index: 8 - }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'statistics_task', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.statistics_task', + // link: '' + // } as LinkMenuItemModel, + // icon: 'chart-bar', + // index: 8 + // }, /* Control Panel */ - { - id: 'control_panel', - active: false, - visible: isSiteAdmin, - model: { - type: MenuItemType.LINK, - text: 'menu.section.control_panel', - link: '' - } as LinkMenuItemModel, - icon: 'cogs', - index: 9 - }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'control_panel', + // active: false, + // visible: isSiteAdmin, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.control_panel', + // link: '' + // } as LinkMenuItemModel, + // icon: 'cogs', + // index: 9 + // }, /* Processes */ { @@ -310,42 +313,45 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { index: 3, shouldPersistOnRouteChange: true }, - { - id: 'export_community', - parentID: 'export', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.export_community', - link: '' - } as LinkMenuItemModel, - shouldPersistOnRouteChange: true - }, - { - id: 'export_collection', - parentID: 'export', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.export_collection', - link: '' - } as LinkMenuItemModel, - shouldPersistOnRouteChange: true - }, - { - id: 'export_item', - parentID: 'export', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.export_item', - link: '' - } as LinkMenuItemModel, - shouldPersistOnRouteChange: true - }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'export_community', + // parentID: 'export', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.export_community', + // link: '' + // } as LinkMenuItemModel, + // shouldPersistOnRouteChange: true + // }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'export_collection', + // parentID: 'export', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.export_collection', + // link: '' + // } as LinkMenuItemModel, + // shouldPersistOnRouteChange: true + // }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'export_item', + // parentID: 'export', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.export_item', + // link: '' + // } as LinkMenuItemModel, + // shouldPersistOnRouteChange: true + // }, ]; menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, menuSection)); @@ -392,17 +398,18 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { icon: 'file-import', index: 2 }, - { - id: 'import_batch', - parentID: 'import', - active: false, - visible: true, - model: { - type: MenuItemType.LINK, - text: 'menu.section.import_batch', - link: '' - } as LinkMenuItemModel, - } + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'import_batch', + // parentID: 'import', + // active: false, + // visible: true, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.import_batch', + // link: '' + // } as LinkMenuItemModel, + // } ]; menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, { shouldPersistOnRouteChange: true @@ -549,17 +556,18 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { link: '/access-control/groups' } as LinkMenuItemModel, }, - { - id: 'access_control_authorizations', - parentID: 'access_control', - active: false, - visible: authorized, - model: { - type: MenuItemType.LINK, - text: 'menu.section.access_control_authorizations', - link: '' - } as LinkMenuItemModel, - }, + // TODO: enable this menu item once the feature has been implemented + // { + // id: 'access_control_authorizations', + // parentID: 'access_control', + // active: false, + // visible: authorized, + // model: { + // type: MenuItemType.LINK, + // text: 'menu.section.access_control_authorizations', + // link: '' + // } as LinkMenuItemModel, + // }, { id: 'access_control', active: false, From 340f9518cda741c2590bcd64a27cef1b1d95b33d Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Tue, 4 May 2021 10:31:55 +0200 Subject: [PATCH 38/42] 78991: Fix SearchOptions typing --- src/app/shared/search/search-options.model.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/app/shared/search/search-options.model.ts b/src/app/shared/search/search-options.model.ts index fb4e8d4caf..053b9225f7 100644 --- a/src/app/shared/search/search-options.model.ts +++ b/src/app/shared/search/search-options.model.ts @@ -13,10 +13,15 @@ export class SearchOptions { scope?: string; query?: string; dsoTypes?: DSpaceObjectType[]; - filters?: any; - fixedFilter?: any; + filters?: SearchFilter[]; + fixedFilter?: string; - constructor(options: {configuration?: string, scope?: string, query?: string, dsoTypes?: DSpaceObjectType[], filters?: SearchFilter[], fixedFilter?: any}) { + constructor( + options: { + configuration?: string, scope?: string, query?: string, dsoTypes?: DSpaceObjectType[], filters?: SearchFilter[], + fixedFilter?: string + } + ) { this.configuration = options.configuration; this.scope = options.scope; this.query = options.query; From e682997195eb2ef759f964a3d6d464431ffa9331 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Tue, 4 May 2021 10:34:05 +0200 Subject: [PATCH 39/42] 78991: URI-encode SearchOptions query values --- .../paginated-search-options.model.spec.ts | 13 +++++++---- .../search/search-options.model.spec.ts | 16 +++++++++---- src/app/shared/search/search-options.model.ts | 23 ++++++++++++++----- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/app/shared/search/paginated-search-options.model.spec.ts b/src/app/shared/search/paginated-search-options.model.spec.ts index b75d2b8fab..2b8fe7aeb5 100644 --- a/src/app/shared/search/paginated-search-options.model.spec.ts +++ b/src/app/shared/search/paginated-search-options.model.spec.ts @@ -8,7 +8,11 @@ describe('PaginatedSearchOptions', () => { let options: PaginatedSearchOptions; const sortOptions = new SortOptions('test.field', SortDirection.DESC); const pageOptions = Object.assign(new PaginationComponentOptions(), { pageSize: 40, page: 1 }); - const filters = [new SearchFilter('f.test', ['value']), new SearchFilter('f.example', ['another value', 'second value'])]; + const filters = [ + new SearchFilter('f.test', ['value']), + new SearchFilter('f.example', ['another value', 'second value']), + new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), + ]; const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; @@ -31,12 +35,13 @@ describe('PaginatedSearchOptions', () => { 'sort=test.field,DESC&' + 'page=0&' + 'size=40&' + - 'query=search query&' + + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + 'f.test=value&' + - 'f.example=another value&' + - 'f.example=second value' + 'f.example=another%20value&' + + 'f.example=second%20value&' + + 'f.range=%5B2002%20TO%202021%5D,equals' ); }); diff --git a/src/app/shared/search/search-options.model.spec.ts b/src/app/shared/search/search-options.model.spec.ts index 62fe732218..d20ebac6bf 100644 --- a/src/app/shared/search/search-options.model.spec.ts +++ b/src/app/shared/search/search-options.model.spec.ts @@ -4,8 +4,13 @@ import { SearchFilter } from './search-filter.model'; import { DSpaceObjectType } from '../../core/shared/dspace-object-type.model'; describe('SearchOptions', () => { - let options: PaginatedSearchOptions; - const filters = [new SearchFilter('f.test', ['value']), new SearchFilter('f.example', ['another value', 'second value'])]; + let options: SearchOptions; + + const filters = [ + new SearchFilter('f.test', ['value']), + new SearchFilter('f.example', ['another value', 'second value']), + new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), + ]; const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; @@ -18,12 +23,13 @@ describe('SearchOptions', () => { it('should generate a string with all parameters that are present', () => { const outcome = options.toRestUrl(baseUrl); expect(outcome).toEqual('www.rest.com?' + - 'query=search query&' + + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + 'f.test=value&' + - 'f.example=another value&' + - 'f.example=second value' + 'f.example=another%20value&' + + 'f.example=second%20value&' + + 'f.range=%5B2002%20TO%202021%5D,equals' ); }); diff --git a/src/app/shared/search/search-options.model.ts b/src/app/shared/search/search-options.model.ts index 053b9225f7..fad4002caf 100644 --- a/src/app/shared/search/search-options.model.ts +++ b/src/app/shared/search/search-options.model.ts @@ -38,27 +38,38 @@ export class SearchOptions { */ toRestUrl(url: string, args: string[] = []): string { if (isNotEmpty(this.configuration)) { - args.push(`configuration=${this.configuration}`); + args.push(`configuration=${encodeURIComponent(this.configuration)}`); } if (isNotEmpty(this.fixedFilter)) { - args.push(this.fixedFilter); + let fixedFilter: string; + const match = this.fixedFilter.match(/^([^=]+=)(.+)$/); + + if (match) { + fixedFilter = match[1] + encodeURIComponent(match[2]); + } else { + fixedFilter = encodeURIComponent(this.fixedFilter); + } + + args.push(fixedFilter); } if (isNotEmpty(this.query)) { - args.push(`query=${this.query}`); + args.push(`query=${encodeURIComponent(this.query)}`); } if (isNotEmpty(this.scope)) { - args.push(`scope=${this.scope}`); + args.push(`scope=${encodeURIComponent(this.scope)}`); } if (isNotEmpty(this.dsoTypes)) { this.dsoTypes.forEach((dsoType: string) => { - args.push(`dsoType=${dsoType}`); + args.push(`dsoType=${encodeURIComponent(dsoType)}`); }); } if (isNotEmpty(this.filters)) { this.filters.forEach((filter: SearchFilter) => { filter.values.forEach((value) => { const filterValue = value.includes(',') ? `${value}` : value + (filter.operator ? ',' + filter.operator : ''); - args.push(`${filter.key}=${filterValue}`); + + // we don't want commas to get URI-encoded + args.push(`${filter.key}=${encodeURIComponent(filterValue).replace(/%2C/g, ',')}`); }); }); } From b9a8bfb2bd3fcd51065fa7c821ab546ae604df5b Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Tue, 4 May 2021 10:44:06 +0200 Subject: [PATCH 40/42] 78991: Also test SearchOptions.fixedFilter --- .../search/paginated-search-options.model.spec.ts | 5 ++++- src/app/shared/search/search-options.model.spec.ts | 10 +++++++++- src/app/shared/search/search-options.model.ts | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/app/shared/search/paginated-search-options.model.spec.ts b/src/app/shared/search/paginated-search-options.model.spec.ts index 2b8fe7aeb5..0523e2b139 100644 --- a/src/app/shared/search/paginated-search-options.model.spec.ts +++ b/src/app/shared/search/paginated-search-options.model.spec.ts @@ -13,6 +13,7 @@ describe('PaginatedSearchOptions', () => { new SearchFilter('f.example', ['another value', 'second value']), new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), ]; + const fixedFilter = 'f.fixed=1234 5678,equals'; const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; @@ -23,7 +24,8 @@ describe('PaginatedSearchOptions', () => { filters: filters, query: query, scope: scope, - dsoTypes: [DSpaceObjectType.ITEM] + dsoTypes: [DSpaceObjectType.ITEM], + fixedFilter: fixedFilter, }); }); @@ -35,6 +37,7 @@ describe('PaginatedSearchOptions', () => { 'sort=test.field,DESC&' + 'page=0&' + 'size=40&' + + 'f.fixed=1234%205678,equals&' + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + diff --git a/src/app/shared/search/search-options.model.spec.ts b/src/app/shared/search/search-options.model.spec.ts index d20ebac6bf..7fb73daa40 100644 --- a/src/app/shared/search/search-options.model.spec.ts +++ b/src/app/shared/search/search-options.model.spec.ts @@ -11,11 +11,18 @@ describe('SearchOptions', () => { new SearchFilter('f.example', ['another value', 'second value']), new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), ]; + const fixedFilter = 'f.fixed=1234 5678,equals'; const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; beforeEach(() => { - options = new SearchOptions({ filters: filters, query: query, scope: scope, dsoTypes: [DSpaceObjectType.ITEM] }); + options = new SearchOptions({ + filters: filters, + query: query, + scope: scope, + dsoTypes: [DSpaceObjectType.ITEM], + fixedFilter: fixedFilter, + }); }); describe('when toRestUrl is called', () => { @@ -23,6 +30,7 @@ describe('SearchOptions', () => { it('should generate a string with all parameters that are present', () => { const outcome = options.toRestUrl(baseUrl); expect(outcome).toEqual('www.rest.com?' + + 'f.fixed=1234%205678,equals&' + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + diff --git a/src/app/shared/search/search-options.model.ts b/src/app/shared/search/search-options.model.ts index fad4002caf..4f56e48534 100644 --- a/src/app/shared/search/search-options.model.ts +++ b/src/app/shared/search/search-options.model.ts @@ -45,7 +45,7 @@ export class SearchOptions { const match = this.fixedFilter.match(/^([^=]+=)(.+)$/); if (match) { - fixedFilter = match[1] + encodeURIComponent(match[2]); + fixedFilter = match[1] + encodeURIComponent(match[2]).replace(/%2C/g, ','); } else { fixedFilter = encodeURIComponent(this.fixedFilter); } From 15ad31bd8474d84a51defe6339bca8cb356be1ef Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Tue, 4 May 2021 11:04:04 +0200 Subject: [PATCH 41/42] 78991: Clean up SearchOptions.toRestUrl * Extract "selective encoding" ops into separate methods * Add comments to clarify regex * Use hasValue() instead of just checking on 'match' * Leave only the last comma of filter values unencoded --- .../paginated-search-options.model.spec.ts | 8 ++-- .../search/search-options.model.spec.ts | 8 ++-- src/app/shared/search/search-options.model.ts | 41 ++++++++++++------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/app/shared/search/paginated-search-options.model.spec.ts b/src/app/shared/search/paginated-search-options.model.spec.ts index 0523e2b139..9881cc1149 100644 --- a/src/app/shared/search/paginated-search-options.model.spec.ts +++ b/src/app/shared/search/paginated-search-options.model.spec.ts @@ -10,10 +10,10 @@ describe('PaginatedSearchOptions', () => { const pageOptions = Object.assign(new PaginationComponentOptions(), { pageSize: 40, page: 1 }); const filters = [ new SearchFilter('f.test', ['value']), - new SearchFilter('f.example', ['another value', 'second value']), - new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), + new SearchFilter('f.example', ['another value', 'second value']), // should be split into two arguments, spaces should be URI-encoded + new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), // value should be URI-encoded, ',equals' should not ]; - const fixedFilter = 'f.fixed=1234 5678,equals'; + const fixedFilter = 'f.fixed=1234,5678,equals'; // '=' and ',equals' should not be URI-encoded const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; @@ -37,7 +37,7 @@ describe('PaginatedSearchOptions', () => { 'sort=test.field,DESC&' + 'page=0&' + 'size=40&' + - 'f.fixed=1234%205678,equals&' + + 'f.fixed=1234%2C5678,equals&' + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + diff --git a/src/app/shared/search/search-options.model.spec.ts b/src/app/shared/search/search-options.model.spec.ts index 7fb73daa40..8bed046736 100644 --- a/src/app/shared/search/search-options.model.spec.ts +++ b/src/app/shared/search/search-options.model.spec.ts @@ -8,10 +8,10 @@ describe('SearchOptions', () => { const filters = [ new SearchFilter('f.test', ['value']), - new SearchFilter('f.example', ['another value', 'second value']), - new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), + new SearchFilter('f.example', ['another value', 'second value']), // should be split into two arguments, spaces should be URI-encoded + new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'), // value should be URI-encoded, ',equals' should not ]; - const fixedFilter = 'f.fixed=1234 5678,equals'; + const fixedFilter = 'f.fixed=1234,5678,equals'; // '=' and ',equals' should not be URI-encoded const query = 'search query'; const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47'; const baseUrl = 'www.rest.com'; @@ -30,7 +30,7 @@ describe('SearchOptions', () => { it('should generate a string with all parameters that are present', () => { const outcome = options.toRestUrl(baseUrl); expect(outcome).toEqual('www.rest.com?' + - 'f.fixed=1234%205678,equals&' + + 'f.fixed=1234%2C5678,equals&' + 'query=search%20query&' + 'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' + 'dsoType=ITEM&' + diff --git a/src/app/shared/search/search-options.model.ts b/src/app/shared/search/search-options.model.ts index 4f56e48534..591e4fcb04 100644 --- a/src/app/shared/search/search-options.model.ts +++ b/src/app/shared/search/search-options.model.ts @@ -1,4 +1,4 @@ -import { isNotEmpty } from '../empty.util'; +import { hasValue, isNotEmpty } from '../empty.util'; import { URLCombiner } from '../../core/url-combiner/url-combiner'; import { SearchFilter } from './search-filter.model'; import { DSpaceObjectType } from '../../core/shared/dspace-object-type.model'; @@ -41,16 +41,7 @@ export class SearchOptions { args.push(`configuration=${encodeURIComponent(this.configuration)}`); } if (isNotEmpty(this.fixedFilter)) { - let fixedFilter: string; - const match = this.fixedFilter.match(/^([^=]+=)(.+)$/); - - if (match) { - fixedFilter = match[1] + encodeURIComponent(match[2]).replace(/%2C/g, ','); - } else { - fixedFilter = encodeURIComponent(this.fixedFilter); - } - - args.push(fixedFilter); + args.push(this.encodedFixedFilter); } if (isNotEmpty(this.query)) { args.push(`query=${encodeURIComponent(this.query)}`); @@ -67,9 +58,7 @@ export class SearchOptions { this.filters.forEach((filter: SearchFilter) => { filter.values.forEach((value) => { const filterValue = value.includes(',') ? `${value}` : value + (filter.operator ? ',' + filter.operator : ''); - - // we don't want commas to get URI-encoded - args.push(`${filter.key}=${encodeURIComponent(filterValue).replace(/%2C/g, ',')}`); + args.push(`${filter.key}=${this.encodeFilterQueryValue(filterValue)}`); }); }); } @@ -78,4 +67,28 @@ export class SearchOptions { } return url; } + + get encodedFixedFilter(): string { + // expected format: 'arg=value' + // -> split the query agument into (arg=)(value) and only encode 'value' + const match = this.fixedFilter.match(/^([^=]+=)(.+)$/); + + if (hasValue(match)) { + return match[1] + this.encodeFilterQueryValue(match[2]); + } else { + return this.encodeFilterQueryValue(this.fixedFilter); + } + } + + encodeFilterQueryValue(filterQueryValue: string): string { + // expected format: 'value' or 'value,operator' + // -> split into (value)(,operator) and only encode 'value' + const match = filterQueryValue.match(/^(.*)(,\w+)$/); + + if (hasValue(match)) { + return encodeURIComponent(match[1]) + match[2]; + } else { + return encodeURIComponent(filterQueryValue); + } + } } From 042afd9bb8ad8603b3bc258b12709222cec20d08 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Tue, 4 May 2021 11:46:04 +0200 Subject: [PATCH 42/42] 79220: Fix edit group navigation bug --- .../group-registry/group-form/group-form.component.spec.ts | 7 +++++++ .../group-registry/group-form/group-form.component.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts index d213a071d7..5f0f570044 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts @@ -210,4 +210,11 @@ describe('GroupFormComponent', () => { }); }); + describe('ngOnDestroy', () => { + it('does NOT call router.navigate', () => { + component.ngOnDestroy(); + expect(router.navigate).toHaveBeenCalledTimes(0); + }); + }); + }); diff --git a/src/app/access-control/group-registry/group-form/group-form.component.ts b/src/app/access-control/group-registry/group-form/group-form.component.ts index 9b74d26fe8..c2c694f445 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.ts @@ -405,7 +405,7 @@ export class GroupFormComponent implements OnInit, OnDestroy { */ @HostListener('window:beforeunload') ngOnDestroy(): void { - this.onCancel(); + this.groupDataService.cancelEditGroup(); this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe()); }