diff --git a/cypress/e2e/item-edit.cy.ts b/cypress/e2e/item-edit.cy.ts index b13d5a4695..60e1b924ee 100644 --- a/cypress/e2e/item-edit.cy.ts +++ b/cypress/e2e/item-edit.cy.ts @@ -18,6 +18,11 @@ describe('Edit Item > Edit Metadata tab', () => { // tag must be loaded cy.get('ds-edit-item-page').should('be.visible'); + // wait for all the ds-dso-edit-metadata-value components to be rendered + cy.get('ds-dso-edit-metadata-value div[role="row"]').each(($row: HTMLDivElement) => { + cy.wrap($row).find('div[role="cell"]').should('be.visible'); + }); + // Analyze for accessibility issues testA11y('ds-edit-item-page'); }); diff --git a/cypress/e2e/submission.cy.ts b/cypress/e2e/submission.cy.ts index 7123d84134..0ac7003a8b 100644 --- a/cypress/e2e/submission.cy.ts +++ b/cypress/e2e/submission.cy.ts @@ -137,7 +137,7 @@ describe('New Submission page', () => { // Upload our DSpace logo via drag & drop onto submission form // cy.get('div#section_upload') - cy.get('div.ds-document-drop-zone').selectFile('src/assets/images/dspace-logo.png', { + cy.get('div.ds-document-drop-zone').selectFile('src/assets/images/dspace-logo.svg', { action: 'drag-drop', }); diff --git a/src/app/access-control/access-control-routes.ts b/src/app/access-control/access-control-routes.ts index a7cce461ef..07b6f6c4ff 100644 --- a/src/app/access-control/access-control-routes.ts +++ b/src/app/access-control/access-control-routes.ts @@ -1,16 +1,13 @@ import { AbstractControl } from '@angular/forms'; -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { DYNAMIC_ERROR_MESSAGES_MATCHER, DynamicErrorMessagesMatcher, } from '@ng-dynamic-forms/core'; import { i18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; -import { GroupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard'; -import { SiteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; +import { groupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard'; +import { siteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; import { EPERSON_PATH, GROUP_PATH, @@ -20,7 +17,7 @@ import { EPeopleRegistryComponent } from './epeople-registry/epeople-registry.co import { EPersonFormComponent } from './epeople-registry/eperson-form/eperson-form.component'; import { EPersonResolver } from './epeople-registry/eperson-resolver.service'; import { GroupFormComponent } from './group-registry/group-form/group-form.component'; -import { GroupPageGuard } from './group-registry/group-page.guard'; +import { groupPageGuard } from './group-registry/group-page.guard'; import { GroupsRegistryComponent } from './group-registry/groups-registry.component'; /** @@ -28,7 +25,7 @@ import { GroupsRegistryComponent } from './group-registry/groups-registry.compon */ export const ValidateEmailErrorStateMatcher: DynamicErrorMessagesMatcher = (control: AbstractControl, model: any, hasFocus: boolean) => { - return (control.touched && !hasFocus) || (control.errors?.emailTaken && hasFocus); + return ( control.touched && !hasFocus ) || ( control.errors?.emailTaken && hasFocus ); }; const providers = [ @@ -46,7 +43,7 @@ export const ROUTES: Route[] = [ }, providers, data: { title: 'admin.access-control.epeople.title', breadcrumbKey: 'admin.access-control.epeople' }, - canActivate: mapToCanActivate([SiteAdministratorGuard]), + canActivate: [siteAdministratorGuard], }, { path: `${EPERSON_PATH}/create`, @@ -56,7 +53,7 @@ export const ROUTES: Route[] = [ }, providers, data: { title: 'admin.access-control.epeople.add.title', breadcrumbKey: 'admin.access-control.epeople.add' }, - canActivate: mapToCanActivate([SiteAdministratorGuard]), + canActivate: [siteAdministratorGuard], }, { path: `${EPERSON_PATH}/:id/edit`, @@ -67,7 +64,7 @@ export const ROUTES: Route[] = [ }, providers, data: { title: 'admin.access-control.epeople.edit.title', breadcrumbKey: 'admin.access-control.epeople.edit' }, - canActivate: mapToCanActivate([SiteAdministratorGuard]), + canActivate: [siteAdministratorGuard], }, { path: GROUP_PATH, @@ -77,7 +74,7 @@ export const ROUTES: Route[] = [ }, providers, data: { title: 'admin.access-control.groups.title', breadcrumbKey: 'admin.access-control.groups' }, - canActivate: mapToCanActivate([GroupAdministratorGuard]), + canActivate: [groupAdministratorGuard], }, { path: `${GROUP_PATH}/create`, @@ -90,7 +87,7 @@ export const ROUTES: Route[] = [ title: 'admin.access-control.groups.title.addGroup', breadcrumbKey: 'admin.access-control.groups.addGroup', }, - canActivate: mapToCanActivate([GroupAdministratorGuard]), + canActivate: [groupAdministratorGuard], }, { path: `${GROUP_PATH}/:groupId/edit`, @@ -103,7 +100,7 @@ export const ROUTES: Route[] = [ title: 'admin.access-control.groups.title.singleGroup', breadcrumbKey: 'admin.access-control.groups.singleGroup', }, - canActivate: mapToCanActivate([GroupPageGuard]), + canActivate: [groupPageGuard], }, { path: 'bulk-access', @@ -112,6 +109,6 @@ export const ROUTES: Route[] = [ breadcrumb: i18nBreadcrumbResolver, }, data: { title: 'admin.access-control.bulk-access.title', breadcrumbKey: 'admin.access-control.bulk-access' }, - canActivate: mapToCanActivate([SiteAdministratorGuard]), + canActivate: [siteAdministratorGuard], }, ]; diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html index 1b0386dee2..591d80bdd5 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.html +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.html @@ -20,25 +20,33 @@ - - {{eperson.id}} + + {{epersonDTO.eperson.id}} - - {{ dsoNameService.getName(eperson) }} + + {{ dsoNameService.getName(epersonDTO.eperson) }} - {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}
- {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }} + {{messagePrefix + '.table.email' | translate}}: {{ epersonDTO.eperson.email ? epersonDTO.eperson.email : '-' }}
+ {{messagePrefix + '.table.netid' | translate}}: {{ epersonDTO.eperson.netid ? epersonDTO.eperson.netid : '-' }}
- +
diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts index 9cc7727557..9a6b2b4f05 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.spec.ts @@ -222,13 +222,13 @@ describe('MembersListComponent', () => { describe('if first delete button is pressed', () => { beforeEach(() => { + spyOn(component, 'search').and.callThrough(); const deleteButton: DebugElement = fixture.debugElement.query(By.css('#ePeopleMembersOfGroup tbody .fa-trash-alt')); deleteButton.nativeElement.click(); fixture.detectChanges(); }); - it('then no ePerson remains as a member of the active group.', () => { - const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr')); - expect(epersonsFound.length).toEqual(0); + it('should trigger the search to add the user back to the search table', () => { + expect(component.search).toHaveBeenCalled(); }); }); }); @@ -264,13 +264,13 @@ describe('MembersListComponent', () => { describe('if first add button is pressed', () => { beforeEach(() => { + spyOn(component, 'search').and.callThrough(); const addButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus')); addButton.nativeElement.click(); fixture.detectChanges(); }); - it('then all (two) ePersons are member of the active group. No non-members left', () => { - epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr')); - expect(epersonsFound.length).toEqual(0); + it('should trigger the search to remove the user from the search table', () => { + expect(component.search).toHaveBeenCalled(); }); }); }); diff --git a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts index 4a707ef159..9b123ae447 100644 --- a/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts +++ b/src/app/access-control/group-registry/group-form/members-list/members-list.component.ts @@ -24,21 +24,29 @@ import { } from '@ngx-translate/core'; import { BehaviorSubject, + combineLatest as observableCombineLatest, Observable, + ObservedValueOf, + of as observableOf, Subscription, } from 'rxjs'; import { + defaultIfEmpty, map, switchMap, take, } from 'rxjs/operators'; import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service'; -import { PaginatedList } from '../../../../core/data/paginated-list.model'; +import { + buildPaginatedList, + PaginatedList, +} from '../../../../core/data/paginated-list.model'; import { RemoteData } from '../../../../core/data/remote-data'; import { EPersonDataService } from '../../../../core/eperson/eperson-data.service'; import { GroupDataService } from '../../../../core/eperson/group-data.service'; import { EPerson } from '../../../../core/eperson/models/eperson.model'; +import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model'; import { Group } from '../../../../core/eperson/models/group.model'; import { PaginationService } from '../../../../core/pagination/pagination.service'; import { @@ -137,7 +145,7 @@ export class MembersListComponent implements OnInit, OnDestroy { /** * List of EPeople members of currently active group being edited */ - ePeopleMembersOfGroup: BehaviorSubject> = new BehaviorSubject>(undefined); + ePeopleMembersOfGroup: BehaviorSubject> = new BehaviorSubject(undefined); /** * Pagination config used to display the list of EPeople that are result of EPeople search @@ -226,10 +234,35 @@ export class MembersListComponent implements OnInit, OnDestroy { return rd; } }), - getRemoteDataPayload()) - .subscribe((paginatedListOfEPersons: PaginatedList) => { - this.ePeopleMembersOfGroup.next(paginatedListOfEPersons); - })); + switchMap((epersonListRD: RemoteData>) => { + const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => { + const dto$: Observable = observableCombineLatest( + this.isMemberOfGroup(member), (isMember: ObservedValueOf>) => { + const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel(); + epersonDtoModel.eperson = member; + epersonDtoModel.ableToDelete = isMember; + return epersonDtoModel; + }); + return dto$; + })]); + return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => { + return buildPaginatedList(epersonListRD.payload.pageInfo, dtos); + })); + }), + ).subscribe((paginatedListOfDTOs: PaginatedList) => { + this.ePeopleMembersOfGroup.next(paginatedListOfDTOs); + }), + ); + } + + /** + * We always return true since this is only used by the top section (which represents all the users part of the group + * in {@link MembersListComponent}) + * + * @param possibleMember EPerson that is a possible member (being tested) of the group currently being edited + */ + isMemberOfGroup(possibleMember: EPerson): Observable { + return observableOf(true); } /** diff --git a/src/app/access-control/group-registry/group-page.guard.spec.ts b/src/app/access-control/group-registry/group-page.guard.spec.ts index b1648f59ec..3024e42d64 100644 --- a/src/app/access-control/group-registry/group-page.guard.spec.ts +++ b/src/app/access-control/group-registry/group-page.guard.spec.ts @@ -1,14 +1,24 @@ +import { + TestBed, + waitForAsync, +} from '@angular/core/testing'; import { ActivatedRouteSnapshot, Router, + UrlTree, } from '@angular/router'; -import { of as observableOf } from 'rxjs'; +import { + Observable, + of as observableOf, +} from 'rxjs'; import { AuthService } from '../../core/auth/auth.service'; import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; import { HALEndpointService } from '../../core/shared/hal-endpoint.service'; -import { GroupPageGuard } from './group-page.guard'; +import { groupPageGuard } from './group-page.guard'; + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; // Increase timeout to 10 seconds describe('GroupPageGuard', () => { const groupsEndpointUrl = 'https://test.org/api/eperson/groups'; @@ -20,42 +30,54 @@ describe('GroupPageGuard', () => { }, } as unknown as ActivatedRouteSnapshot; - let guard: GroupPageGuard; let halEndpointService: HALEndpointService; let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; - beforeEach(() => { + function init() { halEndpointService = jasmine.createSpyObj(['getEndpoint']); - (halEndpointService as any).getEndpoint.and.returnValue(observableOf(groupsEndpointUrl)); + ( halEndpointService as any ).getEndpoint.and.returnValue(observableOf(groupsEndpointUrl)); authorizationService = jasmine.createSpyObj(['isAuthorized']); // NOTE: value is set in beforeEach router = jasmine.createSpyObj(['parseUrl']); - (router as any).parseUrl.and.returnValue = {}; + ( router as any ).parseUrl.and.returnValue = {}; authService = jasmine.createSpyObj(['isAuthenticated']); - (authService as any).isAuthenticated.and.returnValue(observableOf(true)); + ( authService as any ).isAuthenticated.and.returnValue(observableOf(true)); - guard = new GroupPageGuard(halEndpointService, authorizationService, router, authService); - }); + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: HALEndpointService, useValue: halEndpointService }, + ], + }); + } + + beforeEach(waitForAsync(() => { + init(); + })); it('should be created', () => { - expect(guard).toBeTruthy(); + expect(groupPageGuard).toBeTruthy(); }); describe('canActivate', () => { describe('when the current user can manage the group', () => { beforeEach(() => { - (authorizationService as any).isAuthorized.and.returnValue(observableOf(true)); + ( authorizationService as any ).isAuthorized.and.returnValue(observableOf(true)); }); it('should return true', (done) => { - guard.canActivate( - routeSnapshotWithGroupId, { url: 'current-url' } as any, - ).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return groupPageGuard()(routeSnapshotWithGroupId, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { expect(authorizationService.isAuthorized).toHaveBeenCalledWith( FeatureID.CanManageGroup, groupEndpointUrl, undefined, ); @@ -71,15 +93,18 @@ describe('GroupPageGuard', () => { }); it('should not return true', (done) => { - guard.canActivate( - routeSnapshotWithGroupId, { url: 'current-url' } as any, - ).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return groupPageGuard()(routeSnapshotWithGroupId, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { expect(authorizationService.isAuthorized).toHaveBeenCalledWith( FeatureID.CanManageGroup, groupEndpointUrl, undefined, ); expect(result).not.toBeTrue(); done(); }); + }); }); }); diff --git a/src/app/access-control/group-registry/group-page.guard.ts b/src/app/access-control/group-registry/group-page.guard.ts index 928271887c..c52bed9c48 100644 --- a/src/app/access-control/group-registry/group-page.guard.ts +++ b/src/app/access-control/group-registry/group-page.guard.ts @@ -1,7 +1,7 @@ -import { Injectable } from '@angular/core'; +import { inject } from '@angular/core'; import { ActivatedRouteSnapshot, - Router, + CanActivateFn, RouterStateSnapshot, } from '@angular/router'; import { @@ -10,34 +10,29 @@ import { } from 'rxjs'; import { map } from 'rxjs/operators'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { SomeFeatureAuthorizationGuard } from '../../core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard'; +import { + someFeatureAuthorizationGuard, + StringGuardParamFn, +} from '../../core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; import { HALEndpointService } from '../../core/shared/hal-endpoint.service'; -@Injectable({ - providedIn: 'root', -}) -export class GroupPageGuard extends SomeFeatureAuthorizationGuard { +const defaultGroupPageGetObjectUrl: StringGuardParamFn = ( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot, +): Observable => { + const halEndpointService = inject(HALEndpointService); + const groupsEndpoint = 'groups'; - protected groupsEndpoint = 'groups'; + return halEndpointService.getEndpoint(groupsEndpoint).pipe( + map(groupsUrl => `${groupsUrl}/${route?.params?.groupId}`), + ); +}; - constructor(protected halEndpointService: HALEndpointService, - protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf([FeatureID.CanManageGroup]); - } - - getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return this.halEndpointService.getEndpoint(this.groupsEndpoint).pipe( - map(groupsUrl => `${groupsUrl}/${route?.params?.groupId}`), - ); - } - -} +export const groupPageGuard = ( + getObjectUrl = defaultGroupPageGetObjectUrl, + getEPersonUuid?: StringGuardParamFn, +): CanActivateFn => someFeatureAuthorizationGuard( + () => observableOf([FeatureID.CanManageGroup]), + getObjectUrl, + getEPersonUuid); diff --git a/src/app/admin/admin-notify-dashboard/admin-notify-dashboard-routes.ts b/src/app/admin/admin-notify-dashboard/admin-notify-dashboard-routes.ts index c193148cc4..0316913cf6 100644 --- a/src/app/admin/admin-notify-dashboard/admin-notify-dashboard-routes.ts +++ b/src/app/admin/admin-notify-dashboard/admin-notify-dashboard-routes.ts @@ -1,18 +1,15 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { i18nBreadcrumbResolver } from '../../core/breadcrumbs/i18n-breadcrumb.resolver'; import { notifyInfoGuard } from '../../core/coar-notify/notify-info/notify-info.guard'; -import { SiteAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; +import { siteAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; import { AdminNotifyDashboardComponent } from './admin-notify-dashboard.component'; import { AdminNotifyIncomingComponent } from './admin-notify-logs/admin-notify-incoming/admin-notify-incoming.component'; import { AdminNotifyOutgoingComponent } from './admin-notify-logs/admin-notify-outgoing/admin-notify-outgoing.component'; export const ROUTES: Route[] = [ { - canActivate: [...mapToCanActivate([SiteAdministratorGuard]), notifyInfoGuard], + canActivate: [siteAdministratorGuard, notifyInfoGuard], path: '', resolve: { breadcrumb: i18nBreadcrumbResolver, @@ -30,7 +27,7 @@ export const ROUTES: Route[] = [ breadcrumb: i18nBreadcrumbResolver, }, component: AdminNotifyIncomingComponent, - canActivate: [...mapToCanActivate([SiteAdministratorGuard]), notifyInfoGuard], + canActivate: [siteAdministratorGuard, notifyInfoGuard], data: { title: 'admin.notify.dashboard.page.title', breadcrumbKey: 'admin.notify.dashboard', @@ -42,7 +39,7 @@ export const ROUTES: Route[] = [ breadcrumb: i18nBreadcrumbResolver, }, component: AdminNotifyOutgoingComponent, - canActivate: [...mapToCanActivate([SiteAdministratorGuard]), notifyInfoGuard], + canActivate: [siteAdministratorGuard, notifyInfoGuard], data: { title: 'admin.notify.dashboard.page.title', breadcrumbKey: 'admin.notify.dashboard', diff --git a/src/app/app-routes.ts b/src/app/app-routes.ts index 8b7f6acd47..29a78364b5 100644 --- a/src/app/app-routes.ts +++ b/src/app/app-routes.ts @@ -1,6 +1,5 @@ import { InMemoryScrollingOptions, - mapToCanActivate, Route, RouterConfigOptions, } from '@angular/router'; @@ -26,12 +25,12 @@ import { COLLECTION_MODULE_PATH } from './collection-page/collection-page-routin import { COMMUNITY_MODULE_PATH } from './community-page/community-page-routing-paths'; import { authBlockingGuard } from './core/auth/auth-blocking.guard'; import { authenticatedGuard } from './core/auth/authenticated.guard'; -import { GroupAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/group-administrator.guard'; -import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; -import { SiteRegisterGuard } from './core/data/feature-authorization/feature-authorization-guard/site-register.guard'; -import { EndUserAgreementCurrentUserGuard } from './core/end-user-agreement/end-user-agreement-current-user.guard'; +import { groupAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/group-administrator.guard'; +import { siteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; +import { siteRegisterGuard } from './core/data/feature-authorization/feature-authorization-guard/site-register.guard'; +import { endUserAgreementCurrentUserGuard } from './core/end-user-agreement/end-user-agreement-current-user.guard'; import { reloadGuard } from './core/reload/reload.guard'; -import { ForgotPasswordCheckGuard } from './core/rest-property/forgot-password-check-guard.guard'; +import { forgotPasswordCheckGuard } from './core/rest-property/forgot-password-check-guard.guard'; import { ServerCheckGuard } from './core/server-check/server-check.guard'; import { ThemedForbiddenComponent } from './forbidden/themed-forbidden.component'; import { ITEM_MODULE_PATH } from './item-page/item-page-routing-paths'; @@ -66,105 +65,105 @@ export const APP_ROUTES: Route[] = [ .then((m) => m.ROUTES), data: { showBreadcrumbs: false }, providers: [provideSuggestionNotificationsState()], - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'community-list', loadChildren: () => import('./community-list-page/community-list-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'id', loadChildren: () => import('./lookup-by-id/lookup-by-id-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'handle', loadChildren: () => import('./lookup-by-id/lookup-by-id-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: REGISTER_PATH, loadChildren: () => import('./register-page/register-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([SiteRegisterGuard]), + canActivate: [siteRegisterGuard], }, { path: FORGOT_PASSWORD_PATH, loadChildren: () => import('./forgot-password/forgot-password-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard, ForgotPasswordCheckGuard]), + canActivate: [endUserAgreementCurrentUserGuard, forgotPasswordCheckGuard], }, { path: COMMUNITY_MODULE_PATH, loadChildren: () => import('./community-page/community-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: COLLECTION_MODULE_PATH, loadChildren: () => import('./collection-page/collection-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: ITEM_MODULE_PATH, loadChildren: () => import('./item-page/item-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'entities/:entity-type', loadChildren: () => import('./item-page/item-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: LEGACY_BITSTREAM_MODULE_PATH, loadChildren: () => import('./bitstream-page/bitstream-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: BITSTREAM_MODULE_PATH, loadChildren: () => import('./bitstream-page/bitstream-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'mydspace', loadChildren: () => import('./my-dspace-page/my-dspace-page-routes') .then((m) => m.ROUTES), providers: [provideSuggestionNotificationsState()], - canActivate: [authenticatedGuard, ...mapToCanActivate([EndUserAgreementCurrentUserGuard])], + canActivate: [authenticatedGuard, endUserAgreementCurrentUserGuard], }, { path: 'search', loadChildren: () => import('./search-page/search-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'browse', loadChildren: () => import('./browse-by/browse-by-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: ADMIN_MODULE_PATH, loadChildren: () => import('./admin/admin-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([SiteAdministratorGuard, EndUserAgreementCurrentUserGuard]), + canActivate: [siteAdministratorGuard, endUserAgreementCurrentUserGuard], }, { path: NOTIFICATIONS_MODULE_PATH, loadChildren: () => import('./quality-assurance-notifications-pages/notifications-pages-routes') .then((m) => m.ROUTES), providers: [provideSuggestionNotificationsState()], - canActivate: [authenticatedGuard, ...mapToCanActivate([EndUserAgreementCurrentUserGuard])], + canActivate: [authenticatedGuard, endUserAgreementCurrentUserGuard], }, { path: 'login', @@ -181,47 +180,47 @@ export const APP_ROUTES: Route[] = [ loadChildren: () => import('./submit-page/submit-page-routes') .then((m) => m.ROUTES), providers: [provideSubmissionState()], - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'import-external', loadChildren: () => import('./import-external-page/import-external-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: 'workspaceitems', loadChildren: () => import('./workspaceitems-edit-page/workspaceitems-edit-page-routes') .then((m) => m.ROUTES), providers: [provideSubmissionState()], - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: WORKFLOW_ITEM_MODULE_PATH, providers: [provideSubmissionState()], loadChildren: () => import('./workflowitems-edit-page/workflowitems-edit-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: PROFILE_MODULE_PATH, loadChildren: () => import('./profile-page/profile-page-routes') .then((m) => m.ROUTES), providers: [provideSuggestionNotificationsState()], - canActivate: [authenticatedGuard, ...mapToCanActivate([EndUserAgreementCurrentUserGuard])], + canActivate: [authenticatedGuard, endUserAgreementCurrentUserGuard], }, { path: PROCESS_MODULE_PATH, loadChildren: () => import('./process-page/process-page-routes') .then((m) => m.ROUTES), - canActivate: [authenticatedGuard, ...mapToCanActivate([EndUserAgreementCurrentUserGuard])], + canActivate: [authenticatedGuard, endUserAgreementCurrentUserGuard], }, { path: SUGGESTION_MODULE_PATH, loadChildren: () => import('./suggestions-page/suggestions-page-routes') .then((m) => m.ROUTES), providers: [provideSuggestionNotificationsState()], - canActivate: [authenticatedGuard, ...mapToCanActivate([EndUserAgreementCurrentUserGuard])], + canActivate: [authenticatedGuard, endUserAgreementCurrentUserGuard], }, { path: INFO_MODULE_PATH, @@ -230,7 +229,7 @@ export const APP_ROUTES: Route[] = [ { path: REQUEST_COPY_MODULE_PATH, loadChildren: () => import('./request-copy/request-copy-routes').then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: FORBIDDEN_PATH, @@ -240,7 +239,7 @@ export const APP_ROUTES: Route[] = [ path: 'statistics', loadChildren: () => import('./statistics-page/statistics-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([EndUserAgreementCurrentUserGuard]), + canActivate: [endUserAgreementCurrentUserGuard], }, { path: HEALTH_PAGE_PATH, @@ -250,7 +249,7 @@ export const APP_ROUTES: Route[] = [ { path: ACCESS_CONTROL_MODULE_PATH, loadChildren: () => import('./access-control/access-control-routes').then((m) => m.ROUTES), - canActivate: mapToCanActivate([GroupAdministratorGuard, EndUserAgreementCurrentUserGuard]), + canActivate: [groupAdministratorGuard, endUserAgreementCurrentUserGuard], }, { path: 'subscriptions', diff --git a/src/app/app.component.ts b/src/app/app.component.ts index cdf45f50a8..b87073c034 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -32,6 +32,7 @@ import { Observable, } from 'rxjs'; import { + delay, distinctUntilChanged, take, withLatestFrom, @@ -136,7 +137,10 @@ export class AppComponent implements OnInit, AfterViewInit { } ngAfterViewInit() { - this.router.events.subscribe((event) => { + this.router.events.pipe( + // delay(0) to prevent "Expression has changed after it was checked" errors + delay(0), + ).subscribe((event) => { if (event instanceof NavigationStart) { distinctNext(this.isRouteLoading$, true); } else if ( diff --git a/src/app/bitstream-page/bitstream-page-authorizations.guard.spec.ts b/src/app/bitstream-page/bitstream-page-authorizations.guard.spec.ts new file mode 100644 index 0000000000..9b4d019058 --- /dev/null +++ b/src/app/bitstream-page/bitstream-page-authorizations.guard.spec.ts @@ -0,0 +1,81 @@ +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { + Observable, + of as observableOf, +} from 'rxjs'; +import { AuthService } from 'src/app/core/auth/auth.service'; +import { AuthorizationDataService } from 'src/app/core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from 'src/app/core/data/feature-authorization/feature-id'; + +import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import { Bitstream } from '../core/shared/bitstream.model'; +import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; +import { bitstreamPageAuthorizationsGuard } from './bitstream-page-authorizations.guard'; + +describe('bitstreamPageAuthorizationsGuard', () => { + let authorizationService: AuthorizationDataService; + let authService: AuthService; + let router: Router; + let route; + let parentRoute; + let bitstreamService: BitstreamDataService; + let bitstream: Bitstream; + let uuid = '1234-abcdef-54321-fedcba'; + let bitstreamSelfLink = 'test.url/1234-abcdef-54321-fedcba'; + + beforeEach(() => { + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true), + }); + router = jasmine.createSpyObj('router', { + parseUrl: {}, + navigateByUrl: undefined, + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + }); + + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0', + }, + }; + route = { + params: {}, + parent: parentRoute, + }; + bitstream = new Bitstream(); + bitstream.uuid = uuid; + bitstream._links = { self: { href: bitstreamSelfLink } } as any; + bitstreamService = jasmine.createSpyObj('bitstreamService', { findById: createSuccessfulRemoteDataObject$(bitstream) }); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: BitstreamDataService, useValue: bitstreamService }, + ], + }); + }); + + it('should call authorizationService.isAuthorized with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return bitstreamPageAuthorizationsGuard(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith( + FeatureID.CanManagePolicies, + bitstreamSelfLink, + undefined, + ); + done(); + }); + + }); +}); diff --git a/src/app/bitstream-page/bitstream-page-authorizations.guard.ts b/src/app/bitstream-page/bitstream-page-authorizations.guard.ts new file mode 100644 index 0000000000..134693977f --- /dev/null +++ b/src/app/bitstream-page/bitstream-page-authorizations.guard.ts @@ -0,0 +1,16 @@ +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; + +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 { bitstreamPageResolver } from './bitstream-page.resolver'; + +/** + * Guard for preventing unauthorized access to certain {@link Bitstream} pages requiring specific authorizations. + * Checks authorization rights for managing policies. + */ +export const bitstreamPageAuthorizationsGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => bitstreamPageResolver, + () => observableOf(FeatureID.CanManagePolicies), + ); diff --git a/src/app/bitstream-page/bitstream-page-routes.ts b/src/app/bitstream-page/bitstream-page-routes.ts index 73848a7f4e..6d5e312db5 100644 --- a/src/app/bitstream-page/bitstream-page-routes.ts +++ b/src/app/bitstream-page/bitstream-page-routes.ts @@ -10,8 +10,9 @@ import { resourcePolicyTargetResolver } from '../shared/resource-policies/resolv import { BitstreamAuthorizationsComponent } from './bitstream-authorizations/bitstream-authorizations.component'; import { BitstreamDownloadPageComponent } from './bitstream-download-page/bitstream-download-page.component'; import { bitstreamPageResolver } from './bitstream-page.resolver'; +import { bitstreamPageAuthorizationsGuard } from './bitstream-page-authorizations.guard'; import { ThemedEditBitstreamPageComponent } from './edit-bitstream-page/themed-edit-bitstream-page.component'; -import { legacyBitstreamUrlResolver } from './legacy-bitstream-url.resolver'; +import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; const EDIT_BITSTREAM_PATH = ':id/edit'; const EDIT_BITSTREAM_AUTHORIZATIONS_PATH = ':id/authorizations'; @@ -23,18 +24,12 @@ export const ROUTES: Route[] = [ { // Resolve XMLUI bitstream download URLs path: 'handle/:prefix/:suffix/:filename', - component: BitstreamDownloadPageComponent, - resolve: { - bitstream: legacyBitstreamUrlResolver, - }, + canActivate: [legacyBitstreamURLRedirectGuard], }, { // Resolve JSPUI bitstream download URLs path: ':prefix/:suffix/:sequence_id/:filename', - component: BitstreamDownloadPageComponent, - resolve: { - bitstream: legacyBitstreamUrlResolver, - }, + canActivate: [legacyBitstreamURLRedirectGuard], }, { // Resolve angular bitstream download URLs @@ -55,6 +50,7 @@ export const ROUTES: Route[] = [ }, { path: EDIT_BITSTREAM_AUTHORIZATIONS_PATH, + canActivate: [bitstreamPageAuthorizationsGuard], children: [ { path: 'create', diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts new file mode 100644 index 0000000000..1eb0e00b85 --- /dev/null +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.spec.ts @@ -0,0 +1,158 @@ +import { cold } from 'jasmine-marbles'; +import { EMPTY } from 'rxjs'; + +import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; +import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import { RemoteData } from '../core/data/remote-data'; +import { RequestEntryState } from '../core/data/request-entry-state.model'; +import { BrowserHardRedirectService } from '../core/services/browser-hard-redirect.service'; +import { HardRedirectService } from '../core/services/hard-redirect.service'; +import { Bitstream } from '../core/shared/bitstream.model'; +import { RouterStub } from '../shared/testing/router.stub'; +import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; + +describe('legacyBitstreamURLRedirectGuard', () => { + let resolver: any; + let bitstreamDataService: BitstreamDataService; + let remoteDataMocks: { [type: string]: RemoteData }; + let route; + let state; + let hardRedirectService: HardRedirectService; + let router: RouterStub; + + let bitstream: Bitstream; + + beforeEach(() => { + route = { + params: {}, + queryParams: {}, + }; + router = new RouterStub(); + hardRedirectService = new BrowserHardRedirectService(window.location); + state = {}; + bitstream = Object.assign(new Bitstream(), { + uuid: 'bitstream-id', + }); + remoteDataMocks = { + RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined), + ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined), + Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, bitstream, 200), + NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204), + Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), + }; + bitstreamDataService = { + findByItemHandle: () => undefined, + } as any; + resolver = legacyBitstreamURLRedirectGuard; + }); + + describe(`resolve`, () => { + describe(`For JSPUI-style URLs`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + sequence_id: '5', + }, + }); + }); + it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { + resolver(route, state, bitstreamDataService, hardRedirectService, router); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + route.params.sequence_id, + route.params.filename, + ); + }); + }); + + describe(`For XMLUI-style URLs`, () => { + describe(`when there is a sequenceId query parameter`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + }, + queryParams: { + sequenceId: '5', + }, + }); + }); + it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => { + resolver(route, state, bitstreamDataService, hardRedirectService, router); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + route.queryParams.sequenceId, + route.params.filename, + ); + }); + }); + describe(`when there's no sequenceId query parameter`, () => { + beforeEach(() => { + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); + route = Object.assign({}, route, { + params: { + prefix: '123456789', + suffix: '1234', + filename: 'some-file.pdf', + }, + }); + }); + it(`should call findByItemHandle with the handle, and filename from the route`, () => { + resolver(route, state, bitstreamDataService, hardRedirectService, router); + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( + `${route.params.prefix}/${route.params.suffix}`, + undefined, + route.params.filename, + ); + }); + }); + }); + describe('should return and complete after the RemoteData has...', () => { + it('...failed', () => { + spyOn(router, 'createUrlTree').and.callThrough(); + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.Error, + })); + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); + }); + }); + + it('...succeeded without content', () => { + spyOn(router, 'createUrlTree').and.callThrough(); + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.NoContent, + })); + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); + }); + }); + + it('...succeeded', () => { + spyOn(hardRedirectService, 'redirect'); + spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.Success, + })); + resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => { + expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); + expect(hardRedirectService.redirect).toHaveBeenCalledWith(new URL(`/bitstreams/${bitstream.uuid}/download`, window.location.origin).href, 301); + }); + }); + }); + }); +}); diff --git a/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts new file mode 100644 index 0000000000..78403ed7e3 --- /dev/null +++ b/src/app/bitstream-page/legacy-bitstream-url-redirect.guard.ts @@ -0,0 +1,57 @@ +import { inject } from '@angular/core'; +import { + ActivatedRouteSnapshot, + CanActivateFn, + Router, + RouterStateSnapshot, + UrlTree, +} from '@angular/router'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; + +import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; +import { BitstreamDataService } from '../core/data/bitstream-data.service'; +import { RemoteData } from '../core/data/remote-data'; +import { HardRedirectService } from '../core/services/hard-redirect.service'; +import { Bitstream } from '../core/shared/bitstream.model'; +import { getFirstCompletedRemoteData } from '../core/shared/operators'; +import { hasNoValue } from '../shared/empty.util'; + +/** + * Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the + * bitstream. In production mode the status code will also be set the status code to 301 marking it as a permanent URL + * redirect for bots to the regular bitstream download Page. + * + * @returns Either a {@link UrlTree} to the 404 page when the url isn't a valid format or false in order to make the + * user wait until the {@link HardRedirectService#redirect} was performed + */ +export const legacyBitstreamURLRedirectGuard: CanActivateFn = ( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot, + bitstreamDataService: BitstreamDataService = inject(BitstreamDataService), + serverHardRedirectService: HardRedirectService = inject(HardRedirectService), + router: Router = inject(Router), +): Observable => { + const prefix = route.params.prefix; + const suffix = route.params.suffix; + const filename = route.params.filename; + let sequenceId = route.params.sequence_id; + if (hasNoValue(sequenceId)) { + sequenceId = route.queryParams.sequenceId; + } + return bitstreamDataService.findByItemHandle( + `${prefix}/${suffix}`, + sequenceId, + filename, + ).pipe( + getFirstCompletedRemoteData(), + map((rd: RemoteData) => { + if (rd.hasSucceeded && !rd.hasNoContent) { + serverHardRedirectService.redirect(new URL(`/bitstreams/${rd.payload.uuid}/download`, serverHardRedirectService.getCurrentOrigin()).href, 301); + return false; + } else { + return router.createUrlTree([PAGE_NOT_FOUND_PATH]); + } + }), + ); +}; diff --git a/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts b/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts deleted file mode 100644 index ec67d530df..0000000000 --- a/src/app/bitstream-page/legacy-bitstream-url.resolver.spec.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { EMPTY } from 'rxjs'; -import { TestScheduler } from 'rxjs/testing'; - -import { BitstreamDataService } from '../core/data/bitstream-data.service'; -import { RemoteData } from '../core/data/remote-data'; -import { RequestEntryState } from '../core/data/request-entry-state.model'; -import { legacyBitstreamUrlResolver } from './legacy-bitstream-url.resolver'; - -describe(`legacyBitstreamUrlResolver`, () => { - let resolver: any; - let bitstreamDataService: BitstreamDataService; - let testScheduler; - let remoteDataMocks; - let route; - let state; - - beforeEach(() => { - testScheduler = new TestScheduler((actual, expected) => { - expect(actual).toEqual(expected); - }); - - route = { - params: {}, - queryParams: {}, - }; - state = {}; - remoteDataMocks = { - RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined), - ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined), - Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, {}, 200), - Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), - }; - bitstreamDataService = { - findByItemHandle: () => undefined, - } as any; - resolver = legacyBitstreamUrlResolver; - }); - - describe(`resolve`, () => { - describe(`For JSPUI-style URLs`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - sequence_id: '5', - }, - }); - }); - it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { - testScheduler.run(() => { - resolver(route, state, bitstreamDataService); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - route.params.sequence_id, - route.params.filename, - ); - }); - }); - }); - - describe(`For XMLUI-style URLs`, () => { - describe(`when there is a sequenceId query parameter`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - }, - queryParams: { - sequenceId: '5', - }, - }); - }); - it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => { - testScheduler.run(() => { - resolver(route, state, bitstreamDataService); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - route.queryParams.sequenceId, - route.params.filename, - ); - }); - }); - }); - describe(`when there's no sequenceId query parameter`, () => { - beforeEach(() => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(EMPTY); - route = Object.assign({}, route, { - params: { - prefix: '123456789', - suffix: '1234', - filename: 'some-file.pdf', - }, - }); - }); - it(`should call findByItemHandle with the handle, and filename from the route`, () => { - testScheduler.run(() => { - resolver(route, state, bitstreamDataService); - expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( - `${route.params.prefix}/${route.params.suffix}`, - undefined, - route.params.filename, - ); - }); - }); - }); - }); - describe(`should return and complete after the remotedata has...`, () => { - it(`...failed`, () => { - testScheduler.run(({ cold, expectObservable }) => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { - a: remoteDataMocks.RequestPending, - b: remoteDataMocks.ResponsePending, - c: remoteDataMocks.Error, - })); - const expected = '----(c|)'; - const values = { - c: remoteDataMocks.Error, - }; - - expectObservable(resolver(route, state, bitstreamDataService)).toBe(expected, values); - }); - }); - it(`...succeeded`, () => { - testScheduler.run(({ cold, expectObservable }) => { - spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { - a: remoteDataMocks.RequestPending, - b: remoteDataMocks.ResponsePending, - c: remoteDataMocks.Success, - })); - const expected = '----(c|)'; - const values = { - c: remoteDataMocks.Success, - }; - - expectObservable(resolver(route, state, bitstreamDataService)).toBe(expected, values); - }); - }); - }); - }); -}); diff --git a/src/app/bitstream-page/legacy-bitstream-url.resolver.ts b/src/app/bitstream-page/legacy-bitstream-url.resolver.ts deleted file mode 100644 index 8b9b1127b1..0000000000 --- a/src/app/bitstream-page/legacy-bitstream-url.resolver.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { inject } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - RouterStateSnapshot, -} from '@angular/router'; -import { Observable } from 'rxjs'; - -import { BitstreamDataService } from '../core/data/bitstream-data.service'; -import { RemoteData } from '../core/data/remote-data'; -import { Bitstream } from '../core/shared/bitstream.model'; -import { getFirstCompletedRemoteData } from '../core/shared/operators'; -import { hasNoValue } from '../shared/empty.util'; - -/** - * Resolve a bitstream based on the handle of the item, and the sequence id or the filename of the - * bitstream - * - * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot - * @param {RouterStateSnapshot} state The current RouterStateSnapshot - * @param {BitstreamDataService} bitstreamDataService - * @returns Observable<> Emits the found bitstream based on the parameters in - * current route, or an error if something went wrong - */ -export const legacyBitstreamUrlResolver: ResolveFn> = ( - route: ActivatedRouteSnapshot, - state: RouterStateSnapshot, - bitstreamDataService: BitstreamDataService = inject(BitstreamDataService), -): Observable> => { - const prefix = route.params.prefix; - const suffix = route.params.suffix; - const filename = route.params.filename; - - let sequenceId = route.params.sequence_id; - if (hasNoValue(sequenceId)) { - sequenceId = route.queryParams.sequenceId; - } - - return bitstreamDataService.findByItemHandle( - `${prefix}/${suffix}`, - sequenceId, - filename, - ).pipe( - getFirstCompletedRemoteData(), - ); -}; diff --git a/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.html b/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.html index 3dd9b6b25a..e2c0c52827 100644 --- a/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.html +++ b/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.html @@ -7,8 +7,10 @@ } }}
- diff --git a/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.ts b/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.ts index 6ba5dd2c4b..ca6efa16b9 100644 --- a/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.ts +++ b/src/app/browse-by/browse-by-taxonomy/browse-by-taxonomy.component.ts @@ -14,7 +14,10 @@ import { Params, RouterLink, } from '@angular/router'; -import { TranslateModule } from '@ngx-translate/core'; +import { + TranslateModule, + TranslateService, +} from '@ngx-translate/core'; import { BehaviorSubject, Observable, @@ -124,6 +127,11 @@ export class BrowseByTaxonomyComponent implements OnInit, OnChanges, OnDestroy { */ browseDefinition$: Observable; + /** + * Browse description + */ + description: string; + /** * Subscriptions to track */ @@ -131,6 +139,7 @@ export class BrowseByTaxonomyComponent implements OnInit, OnChanges, OnDestroy { public constructor( protected route: ActivatedRoute, + protected translate: TranslateService, ) { } @@ -141,9 +150,11 @@ export class BrowseByTaxonomyComponent implements OnInit, OnChanges, OnDestroy { }), ); this.subs.push(this.browseDefinition$.subscribe((browseDefinition: HierarchicalBrowseDefinition) => { + this.selectedItems = []; this.facetType = browseDefinition.facetType; this.vocabularyName = browseDefinition.vocabulary; this.vocabularyOptions = { name: this.vocabularyName, closed: true }; + this.description = this.translate.instant(`browse.metadata.${this.vocabularyName}.tree.descrption`); })); this.subs.push(this.scope$.subscribe(() => { this.updateQueryParams(); diff --git a/src/app/collection-page/collection-page-administrator.guard.ts b/src/app/collection-page/collection-page-administrator.guard.ts index 63e969b7c8..30edc72fc6 100644 --- a/src/app/collection-page/collection-page-administrator.guard.ts +++ b/src/app/collection-page/collection-page-administrator.guard.ts @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../core/auth/auth.service'; -import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../core/data/remote-data'; -import { Collection } from '../core/shared/collection.model'; import { collectionPageResolver } from './collection-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Collection} pages requiring administrator rights + * Check administrator authorization rights */ -export class CollectionPageAdministratorGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = collectionPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check administrator authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } -} +export const collectionPageAdministratorGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => collectionPageResolver, + () => observableOf(FeatureID.AdministratorOf), + ); diff --git a/src/app/collection-page/collection-page-routes.ts b/src/app/collection-page/collection-page-routes.ts index 889b910d6a..f2dadc3fbe 100644 --- a/src/app/collection-page/collection-page-routes.ts +++ b/src/app/collection-page/collection-page-routes.ts @@ -1,7 +1,4 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { browseByGuard } from '../browse-by/browse-by-guard'; import { browseByI18nBreadcrumbResolver } from '../browse-by/browse-by-i18n-breadcrumb.resolver'; @@ -15,7 +12,7 @@ import { dsoEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; import { LinkMenuItemModel } from '../shared/menu/menu-item/models/link.model'; import { MenuItemType } from '../shared/menu/menu-item-type.model'; import { collectionPageResolver } from './collection-page.resolver'; -import { CollectionPageAdministratorGuard } from './collection-page-administrator.guard'; +import { collectionPageAdministratorGuard } from './collection-page-administrator.guard'; import { COLLECTION_CREATE_PATH, COLLECTION_EDIT_PATH, @@ -65,7 +62,7 @@ export const ROUTES: Route[] = [ path: COLLECTION_EDIT_PATH, loadChildren: () => import('./edit-collection-page/edit-collection-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([CollectionPageAdministratorGuard]), + canActivate: [collectionPageAdministratorGuard], }, { path: 'delete', diff --git a/src/app/collection-page/edit-collection-page/edit-collection-page-routes.ts b/src/app/collection-page/edit-collection-page/edit-collection-page-routes.ts index cf550c3223..19dbaa616b 100644 --- a/src/app/collection-page/edit-collection-page/edit-collection-page-routes.ts +++ b/src/app/collection-page/edit-collection-page/edit-collection-page-routes.ts @@ -1,10 +1,7 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { i18nBreadcrumbResolver } from '../../core/breadcrumbs/i18n-breadcrumb.resolver'; -import { CollectionAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard'; +import { collectionAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/collection-administrator.guard'; import { ResourcePolicyCreateComponent } from '../../shared/resource-policies/create/resource-policy-create.component'; import { ResourcePolicyEditComponent } from '../../shared/resource-policies/edit/resource-policy-edit.component'; import { resourcePolicyResolver } from '../../shared/resource-policies/resolvers/resource-policy.resolver'; @@ -30,7 +27,7 @@ export const ROUTES: Route[] = [ }, data: { breadcrumbKey: 'collection.edit' }, component: EditCollectionPageComponent, - canActivate: mapToCanActivate([CollectionAdministratorGuard]), + canActivate: [collectionAdministratorGuard], children: [ { path: '', diff --git a/src/app/community-page/community-page-administrator.guard.ts b/src/app/community-page/community-page-administrator.guard.ts index 23d429b484..ecbc9b86c0 100644 --- a/src/app/community-page/community-page-administrator.guard.ts +++ b/src/app/community-page/community-page-administrator.guard.ts @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../core/auth/auth.service'; -import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../core/data/remote-data'; -import { Community } from '../core/shared/community.model'; import { communityPageResolver } from './community-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Community} pages requiring administrator rights + * Check administrator authorization rights */ -export class CommunityPageAdministratorGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = communityPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check administrator authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } -} +export const communityPageAdministratorGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => communityPageResolver, + () => observableOf(FeatureID.AdministratorOf), + ); diff --git a/src/app/community-page/community-page-routes.ts b/src/app/community-page/community-page-routes.ts index 656b96c311..d9505c53b1 100644 --- a/src/app/community-page/community-page-routes.ts +++ b/src/app/community-page/community-page-routes.ts @@ -1,7 +1,4 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { browseByGuard } from '../browse-by/browse-by-guard'; import { browseByI18nBreadcrumbResolver } from '../browse-by/browse-by-i18n-breadcrumb.resolver'; @@ -14,7 +11,7 @@ import { dsoEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; import { LinkMenuItemModel } from '../shared/menu/menu-item/models/link.model'; import { MenuItemType } from '../shared/menu/menu-item-type.model'; import { communityPageResolver } from './community-page.resolver'; -import { CommunityPageAdministratorGuard } from './community-page-administrator.guard'; +import { communityPageAdministratorGuard } from './community-page-administrator.guard'; import { COMMUNITY_CREATE_PATH, COMMUNITY_EDIT_PATH, @@ -62,7 +59,7 @@ export const ROUTES: Route[] = [ path: COMMUNITY_EDIT_PATH, loadChildren: () => import('./edit-community-page/edit-community-page-routes') .then((m) => m.ROUTES), - canActivate: mapToCanActivate([CommunityPageAdministratorGuard]), + canActivate: [communityPageAdministratorGuard], }, { path: 'delete', diff --git a/src/app/community-page/edit-community-page/edit-community-page-routes.ts b/src/app/community-page/edit-community-page/edit-community-page-routes.ts index a15312a216..2402c2037d 100644 --- a/src/app/community-page/edit-community-page/edit-community-page-routes.ts +++ b/src/app/community-page/edit-community-page/edit-community-page-routes.ts @@ -1,10 +1,7 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { i18nBreadcrumbResolver } from '../../core/breadcrumbs/i18n-breadcrumb.resolver'; -import { CommunityAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/community-administrator.guard'; +import { communityAdministratorGuard } from '../../core/data/feature-authorization/feature-authorization-guard/community-administrator.guard'; import { ResourcePolicyCreateComponent } from '../../shared/resource-policies/create/resource-policy-create.component'; import { ResourcePolicyEditComponent } from '../../shared/resource-policies/edit/resource-policy-edit.component'; import { resourcePolicyResolver } from '../../shared/resource-policies/resolvers/resource-policy.resolver'; @@ -28,7 +25,7 @@ export const ROUTES: Route[] = [ }, data: { breadcrumbKey: 'community.edit' }, component: EditCommunityPageComponent, - canActivate: mapToCanActivate([CommunityAdministratorGuard]), + canActivate: [communityAdministratorGuard], children: [ { path: '', diff --git a/src/app/core/data/base/delete-data.spec.ts b/src/app/core/data/base/delete-data.spec.ts index 53d651402f..f3fa72a285 100644 --- a/src/app/core/data/base/delete-data.spec.ts +++ b/src/app/core/data/base/delete-data.spec.ts @@ -209,6 +209,11 @@ describe('DeleteDataImpl', () => { method: RestRequestMethod.DELETE, href: 'some-href?copyVirtualMetadata=a©VirtualMetadata=b©VirtualMetadata=c', })); + + const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1]; + callback(); + expect(service.invalidateByHref).toHaveBeenCalledWith('some-href'); + done(); }); }); diff --git a/src/app/core/data/base/delete-data.ts b/src/app/core/data/base/delete-data.ts index e758ee40fa..c47a6af847 100644 --- a/src/app/core/data/base/delete-data.ts +++ b/src/app/core/data/base/delete-data.ts @@ -75,15 +75,16 @@ export class DeleteDataImpl extends IdentifiableDataS deleteByHref(href: string, copyVirtualMetadata?: string[]): Observable> { const requestId = this.requestService.generateRequestId(); + let deleteHref: string = href; if (copyVirtualMetadata) { copyVirtualMetadata.forEach((id) => - href += (href.includes('?') ? '&' : '?') + deleteHref += (deleteHref.includes('?') ? '&' : '?') + 'copyVirtualMetadata=' + id, ); } - const request = new DeleteRequest(requestId, href); + const request = new DeleteRequest(requestId, deleteHref); if (hasValue(this.responseMsToLive)) { request.responseMsToLive = this.responseMsToLive; } diff --git a/src/app/core/data/base/identifiable-data.service.spec.ts b/src/app/core/data/base/identifiable-data.service.spec.ts index 528d6c4945..9281a5c0eb 100644 --- a/src/app/core/data/base/identifiable-data.service.spec.ts +++ b/src/app/core/data/base/identifiable-data.service.spec.ts @@ -5,10 +5,7 @@ * * http://www.dspace.org/license/ */ -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { of as observableOf } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock'; @@ -18,14 +15,14 @@ import { followLink } from '../../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../../cache/object-cache.service'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; -import { FindListOptions } from '../find-list-options.model'; import { RemoteData } from '../remote-data'; import { RequestService } from '../request.service'; import { RequestEntryState } from '../request-entry-state.model'; import { EMBED_SEPARATOR } from './base-data.service'; import { IdentifiableDataService } from './identifiable-data.service'; -const endpoint = 'https://rest.api/core'; +const base = 'https://rest.api/core'; +const endpoint = 'test'; class TestService extends IdentifiableDataService { constructor( @@ -34,11 +31,7 @@ class TestService extends IdentifiableDataService { protected objectCache: ObjectCacheService, protected halService: HALEndpointService, ) { - super(undefined, requestService, rdbService, objectCache, halService); - } - - public getBrowseEndpoint(options: FindListOptions = {}, linkPath: string = this.linkPath): Observable { - return observableOf(endpoint); + super(endpoint, requestService, rdbService, objectCache, halService); } } @@ -55,7 +48,7 @@ describe('IdentifiableDataService', () => { function initTestService(): TestService { requestService = getMockRequestService(); - halService = new HALEndpointServiceStub('url') as any; + halService = new HALEndpointServiceStub(base) as any; rdbService = getMockRemoteDataBuildService(); objectCache = { @@ -147,4 +140,12 @@ describe('IdentifiableDataService', () => { expect(result).toEqual(expected); }); }); + + describe('invalidateById', () => { + it('should invalidate the correct resource by href', () => { + spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true)); + service.invalidateById('123'); + expect(service.invalidateByHref).toHaveBeenCalledWith(`${base}/${endpoint}/123`); + }); + }); }); diff --git a/src/app/core/data/base/identifiable-data.service.ts b/src/app/core/data/base/identifiable-data.service.ts index da3167903e..ba368d21ca 100644 --- a/src/app/core/data/base/identifiable-data.service.ts +++ b/src/app/core/data/base/identifiable-data.service.ts @@ -6,7 +6,11 @@ * http://www.dspace.org/license/ */ import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + map, + switchMap, + take, +} from 'rxjs/operators'; import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; @@ -81,4 +85,19 @@ export class IdentifiableDataService extends BaseData return this.getEndpoint().pipe( map((endpoint: string) => this.getIDHref(endpoint, resourceID, ...linksToFollow))); } + + /** + * Invalidate a cached resource by its identifier + * @param resourceID the identifier of the resource to invalidate + */ + invalidateById(resourceID: string): Observable { + const ok$ = this.getIDHrefObs(resourceID).pipe( + take(1), + switchMap((href) => this.invalidateByHref(href)), + ); + + ok$.subscribe(); + + return ok$; + } } diff --git a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts index 4048efe4ff..4ada344beb 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts @@ -72,7 +72,7 @@ describe('AuthorizationDataService', () => { const ePersonUuid = 'fake-eperson-uuid'; function createExpected(providedObjectUrl: string, providedEPersonUuid?: string, providedFeatureId?: FeatureID): FindListOptions { - const searchParams = [new RequestParam('uri', providedObjectUrl)]; + const searchParams = [new RequestParam('uri', providedObjectUrl, false)]; if (hasValue(providedFeatureId)) { searchParams.push(new RequestParam('feature', providedFeatureId)); } diff --git a/src/app/core/data/feature-authorization/authorization-data.service.ts b/src/app/core/data/feature-authorization/authorization-data.service.ts index 93d8f6b3ec..e5efbae671 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -147,7 +147,8 @@ export class AuthorizationDataService extends BaseDataService imp if (isNotEmpty(options.searchParams)) { params = [...options.searchParams]; } - params.push(new RequestParam('uri', objectUrl)); + // TODO fix encode the uri parameter in the self link in the backend and set encodeValue to true afterwards + params.push(new RequestParam('uri', objectUrl, false)); if (hasValue(featureId)) { params.push(new RequestParam('feature', featureId)); } 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 5af9f18b33..1b1b4a9d6c 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,35 +1,13 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user * isn't a Collection administrator + * Check group management rights */ -@Injectable({ - providedIn: 'root', -}) -export class CollectionAdministratorGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check group management rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.IsCollectionAdmin); - } -} +export const collectionAdministratorGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.IsCollectionAdmin)); 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 2092fce110..6d7dac314e 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,35 +1,13 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user * isn't a Community administrator + * Check group management rights */ -@Injectable({ - providedIn: 'root', -}) -export class CommunityAdministratorGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check group management rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.IsCommunityAdmin); - } -} +export const communityAdministratorGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.IsCommunityAdmin)); 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 2cd9fefa93..18292bb943 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 @@ -1,8 +1,8 @@ +import { TestBed } from '@angular/core/testing'; import { - ActivatedRouteSnapshot, ResolveFn, Router, - RouterStateSnapshot, + UrlTree, } from '@angular/router'; import { Observable, @@ -12,52 +12,39 @@ import { import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { AuthService } from '../../../auth/auth.service'; import { DSpaceObject } from '../../../shared/dspace-object.model'; -import { Item } from '../../../shared/item.model'; import { RemoteData } from '../../remote-data'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { DsoPageSingleFeatureGuard } from './dso-page-single-feature.guard'; +import { dsoPageSingleFeatureGuard } from './dso-page-single-feature.guard'; +import { + defaultDSOGetObjectUrl, + getRouteWithDSOId, +} from './dso-page-some-feature.guard'; -const object = { - self: 'test-selflink', -} as DSpaceObject; - -const testResolver: ResolveFn> = () => createSuccessfulRemoteDataObject$(object); - -/** - * Test implementation of abstract class DsoPageSingleFeatureGuard - */ -class DsoPageSingleFeatureGuardImpl extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = testResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService, - protected featureID: FeatureID) { - super(authorizationService, router, authService); - } - - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(this.featureID); - } -} describe('DsoPageSingleFeatureGuard', () => { - let guard: DsoPageSingleFeatureGuard; let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; + let resolver: ResolveFn>; + let object: DSpaceObject; let route; let parentRoute; + let featureId: FeatureID; + function init() { + object = { + self: 'test-selflink', + } as DSpaceObject; + authorizationService = jasmine.createSpyObj('authorizationService', { isAuthorized: observableOf(true), }); router = jasmine.createSpyObj('router', { parseUrl: {}, }); + resolver = () => createSuccessfulRemoteDataObject$(object); authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true), }); @@ -71,16 +58,25 @@ describe('DsoPageSingleFeatureGuard', () => { }, parent: parentRoute, }; - guard = new DsoPageSingleFeatureGuardImpl(authorizationService, router, authService, undefined); + + featureId = FeatureID.LoginOnBehalfOf; + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + ], + }); } beforeEach(() => { init(); }); - describe('getObjectUrl', () => { + describe('defaultDSOGetObjectUrl', () => { it('should return the resolved object\'s selflink', (done) => { - guard.getObjectUrl(route, undefined).subscribe((selflink) => { + defaultDSOGetObjectUrl(resolver)(route, undefined).subscribe((selflink) => { expect(selflink).toEqual(object.self); done(); }); @@ -89,8 +85,23 @@ describe('DsoPageSingleFeatureGuard', () => { describe('getRouteWithDSOId', () => { it('should return the route that has the UUID of the DSO', () => { - const foundRoute = (guard as any).getRouteWithDSOId(route); + const foundRoute = getRouteWithDSOId(route); expect(foundRoute).toBe(parentRoute); }); }); + + describe('dsoPageSingleFeatureGuard', () => { + it('should call authorizationService.isAuthenticated with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return dsoPageSingleFeatureGuard( + () => resolver, () => observableOf(featureId), + )(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe(() => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, object.self, undefined); + done(); + }); + }); + }); }); 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 index 1f75df846b..5073a38653 100644 --- 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 @@ -1,31 +1,27 @@ import { ActivatedRouteSnapshot, + CanActivateFn, + ResolveFn, RouterStateSnapshot, } from '@angular/router'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { RemoteData } from '../../remote-data'; import { FeatureID } from '../feature-id'; -import { DsoPageSomeFeatureGuard } from './dso-page-some-feature.guard'; +import { dsoPageSomeFeatureGuard } from './dso-page-some-feature.guard'; +import { SingleFeatureGuardParamFn } from './single-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 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; -} +export const dsoPageSingleFeatureGuard = ( + getResolveFn: () => ResolveFn>, + getFeatureID: SingleFeatureGuardParamFn, +): CanActivateFn => dsoPageSomeFeatureGuard( + getResolveFn, + (route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => getFeatureID(route, state).pipe( + map((featureID: FeatureID) => [featureID]), + )); 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 index 4e741b5b71..08f1c96b29 100644 --- 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 @@ -1,8 +1,8 @@ +import { TestBed } from '@angular/core/testing'; import { - ActivatedRouteSnapshot, ResolveFn, Router, - RouterStateSnapshot, + UrlTree, } from '@angular/router'; import { Observable, @@ -12,53 +12,39 @@ import { import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { AuthService } from '../../../auth/auth.service'; import { DSpaceObject } from '../../../shared/dspace-object.model'; -import { Item } from '../../../shared/item.model'; import { RemoteData } from '../../remote-data'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { DsoPageSomeFeatureGuard } from './dso-page-some-feature.guard'; +import { + defaultDSOGetObjectUrl, + dsoPageSomeFeatureGuard, + getRouteWithDSOId, +} from './dso-page-some-feature.guard'; -const object = { - self: 'test-selflink', -} as DSpaceObject; -const testResolver: ResolveFn> = () => createSuccessfulRemoteDataObject$(object); - -/** - * Test implementation of abstract class DsoPageSomeFeatureGuard - */ -class DsoPageSomeFeatureGuardImpl extends DsoPageSomeFeatureGuard { - - protected resolver: ResolveFn> = testResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService, - protected featureIDs: FeatureID[]) { - super(authorizationService, router, authService); - } - - getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(this.featureIDs); - } -} - -describe('DsoPageSomeFeatureGuard', () => { - let guard: DsoPageSomeFeatureGuard; +describe('dsoPageSomeFeatureGuard and its functions', () => { let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; - + let resolver: ResolveFn>; + let object: DSpaceObject; let route; let parentRoute; + let featureIds: FeatureID[]; + function init() { + object = { + self: 'test-selflink', + } as DSpaceObject; + featureIds = [FeatureID.LoginOnBehalfOf, FeatureID.CanDelete]; authorizationService = jasmine.createSpyObj('authorizationService', { isAuthorized: observableOf(true), }); router = jasmine.createSpyObj('router', { parseUrl: {}, }); + resolver = () => createSuccessfulRemoteDataObject$(object); authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true), }); @@ -72,16 +58,25 @@ describe('DsoPageSomeFeatureGuard', () => { }, parent: parentRoute, }; - guard = new DsoPageSomeFeatureGuardImpl(authorizationService, router, authService, []); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + ], + }); + } beforeEach(() => { init(); }); - describe('getObjectUrl', () => { + + describe('defaultDSOGetObjectUrl', () => { it('should return the resolved object\'s selflink', (done) => { - guard.getObjectUrl(route, undefined).subscribe((selflink) => { + defaultDSOGetObjectUrl(resolver)(route, undefined).subscribe((selflink) => { expect(selflink).toEqual(object.self); done(); }); @@ -90,8 +85,26 @@ describe('DsoPageSomeFeatureGuard', () => { describe('getRouteWithDSOId', () => { it('should return the route that has the UUID of the DSO', () => { - const foundRoute = (guard as any).getRouteWithDSOId(route); + const foundRoute = getRouteWithDSOId(route); expect(foundRoute).toBe(parentRoute); }); }); + + + describe('dsoPageSomeFeatureGuard', () => { + it('should call authorizationService.isAuthenticated with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return dsoPageSomeFeatureGuard( + () => resolver, () => observableOf(featureIds), + )(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe(() => { + featureIds.forEach((featureId: FeatureID) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, object.self, undefined); + }); + done(); + }); + }); + }); }); 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 c887b8ae2a..7469f113b4 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 @@ -1,7 +1,7 @@ import { ActivatedRouteSnapshot, + CanActivateFn, ResolveFn, - Router, RouterStateSnapshot, } from '@angular/router'; import { Observable } from 'rxjs'; @@ -11,47 +11,50 @@ import { hasNoValue, hasValue, } from '../../../../shared/empty.util'; -import { AuthService } from '../../../auth/auth.service'; import { DSpaceObject } from '../../../shared/dspace-object.model'; import { getAllSucceededRemoteDataPayload } from '../../../shared/operators'; import { RemoteData } from '../../remote-data'; -import { AuthorizationDataService } from '../authorization-data.service'; -import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; +import { FeatureID } from '../feature-id'; +import { + someFeatureAuthorizationGuard, + SomeFeatureGuardParamFn, + StringGuardParamFn, +} from './some-feature-authorization.guard'; + +export declare type DSOGetObjectURlFn = (resolve: ResolveFn>) => StringGuardParamFn; + /** - * 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 + * Method to resolve resolve (parent) route that contains the UUID of the DSO + * @param route The current route */ -export abstract class DsoPageSomeFeatureGuard extends SomeFeatureAuthorizationGuard { - - protected abstract resolver: ResolveFn>; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); +export const getRouteWithDSOId = (route: ActivatedRouteSnapshot): ActivatedRouteSnapshot => { + let routeWithDSOId = route; + while (hasNoValue(routeWithDSOId.params.id) && hasValue(routeWithDSOId.parent)) { + routeWithDSOId = routeWithDSOId.parent; } + return routeWithDSOId; +}; - /** - * Check authorization rights for the object resolved using the provided resolver - */ - getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - const routeWithObjectID = this.getRouteWithDSOId(route); - return (this.resolver(routeWithObjectID, state) as Observable>).pipe( + + +export const defaultDSOGetObjectUrl: DSOGetObjectURlFn = (resolve: ResolveFn>): StringGuardParamFn => { + return (route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => { + const routeWithObjectID = getRouteWithDSOId(route); + return (resolve(routeWithObjectID, state) as Observable>).pipe( getAllSucceededRemoteDataPayload(), map((dso) => dso.self), ); - } + }; +}; - /** - * Method to resolve (parent) route that contains the UUID of the DSO - * @param route The current route - */ - protected getRouteWithDSOId(route: ActivatedRouteSnapshot): ActivatedRouteSnapshot { - let routeWithDSOId = route; - while (hasNoValue(routeWithDSOId.params.id) && hasValue(routeWithDSOId.parent)) { - routeWithDSOId = routeWithDSOId.parent; - } - return routeWithDSOId; - } -} +/** + * 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 const dsoPageSomeFeatureGuard = ( + getResolveFn: () => ResolveFn>, + getFeatureIDs: SomeFeatureGuardParamFn, + getObjectUrl: DSOGetObjectURlFn = defaultDSOGetObjectUrl, + getEPersonUuid?: StringGuardParamFn, +): CanActivateFn => someFeatureAuthorizationGuard((route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => getFeatureIDs(route, state), getObjectUrl(getResolveFn()), getEPersonUuid); 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 5f32e26851..9641d0aace 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,35 +1,12 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have group * management rights */ -@Injectable({ - providedIn: 'root', -}) -export class GroupAdministratorGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check group management rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageGroups); - } -} +export const groupAdministratorGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.CanManageGroups)); 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 e789f8c473..7c15fa4cdf 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 @@ -1,7 +1,10 @@ import { - ActivatedRouteSnapshot, + TestBed, + waitForAsync, +} from '@angular/core/testing'; +import { Router, - RouterStateSnapshot, + UrlTree, } from '@angular/router'; import { Observable, @@ -11,37 +14,9 @@ import { import { AuthService } from '../../../auth/auth.service'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; -/** - * Test implementation of abstract class SingleFeatureAuthorizationGuard - * Provide the return values of the overwritten getters as constructor arguments - */ -class SingleFeatureAuthorizationGuardImpl extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService, - protected featureId: FeatureID, - protected objectUrl: string, - protected ePersonUuid: string) { - super(authorizationService, router, authService); - } - - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(this.featureId); - } - - getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(this.objectUrl); - } - - getEPersonUuid(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(this.ePersonUuid); - } -} - -describe('SingleFeatureAuthorizationGuard', () => { - let guard: SingleFeatureAuthorizationGuard; +describe('singleFeatureAuthorizationGuard', () => { let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; @@ -64,17 +39,36 @@ describe('SingleFeatureAuthorizationGuard', () => { authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true), }); - guard = new SingleFeatureAuthorizationGuardImpl(authorizationService, router, authService, featureId, objectUrl, ePersonUuid); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + ], + }); } - beforeEach(() => { + beforeEach(waitForAsync(() => { init(); - }); + })); describe('canActivate', () => { - it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { - guard.canActivate(undefined, { url: 'current-url' } as any).subscribe(); - expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); + it('should call authorizationService.isAuthenticated with the appropriate arguments', (done: DoneFn) => { + const result$ = TestBed.runInInjectionContext(() => { + return singleFeatureAuthorizationGuard( + () => observableOf(featureId), + () => observableOf(objectUrl), + () => observableOf(ePersonUuid), + )(undefined, { url: 'current-url' } as any); + }) as Observable; + + + result$.subscribe(() => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); + done(); + }); }); + }); }); 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 index cd9f615aa7..995dcb6f5c 100644 --- 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 @@ -1,31 +1,35 @@ import { ActivatedRouteSnapshot, + CanActivateFn, RouterStateSnapshot, } from '@angular/router'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { FeatureID } from '../feature-id'; -import { SomeFeatureAuthorizationGuard } from './some-feature-authorization.guard'; +import { + someFeatureAuthorizationGuard, + StringGuardParamFn, +} from './some-feature-authorization.guard'; + +export declare type SingleFeatureGuardParamFn = (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => Observable; /** - * 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. + * Guard for preventing unauthorized activating and loading of routes when a user doesn't have + * authorized rights on a specific feature and/or object. + * + * @param getFeatureID The feature to check authorization for + * @param getObjectUrl The URL of the object to check if the user has authorized rights for, + * Optional, if not provided, the {@link Site}'s URL will be assumed + * @param getEPersonUuid The UUID of the user to check authorization rights for. + * Optional, if not provided, the authenticated user's UUID will be assumed. */ -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; -} +export const singleFeatureAuthorizationGuard = ( + getFeatureID: SingleFeatureGuardParamFn, + getObjectUrl?: StringGuardParamFn, + getEPersonUuid?: StringGuardParamFn, +): CanActivateFn => someFeatureAuthorizationGuard( + (route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => getFeatureID(route, state).pipe( + map((featureID: FeatureID) => [featureID]), + ), getObjectUrl, getEPersonUuid); 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 e4f0705def..4caa1f806d 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,33 +1,12 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator * rights to the {@link Site} */ -@Injectable({ providedIn: 'root' }) -export class SiteAdministratorGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check administrator authorization rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } -} +export const siteAdministratorGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.AdministratorOf)); 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 a1a78dc67d..ee08532d38 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,33 +1,12 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have registration * rights to the {@link Site} */ -@Injectable({ providedIn: 'root' }) -export class SiteRegisterGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check registration authorization rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.EPersonRegistration); - } -} +export const siteRegisterGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.EPersonRegistration)); 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 index 53d77cadad..79e023bdd0 100644 --- 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 @@ -1,7 +1,10 @@ import { - ActivatedRouteSnapshot, + TestBed, + waitForAsync, +} from '@angular/core/testing'; +import { Router, - RouterStateSnapshot, + UrlTree, } from '@angular/router'; import { Observable, @@ -11,37 +14,9 @@ import { import { AuthService } from '../../../auth/auth.service'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -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); - } -} +import { someFeatureAuthorizationGuard } from './some-feature-authorization.guard'; describe('SomeFeatureAuthorizationGuard', () => { - let guard: SomeFeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; let router: Router; let authService: AuthService; @@ -62,18 +37,27 @@ describe('SomeFeatureAuthorizationGuard', () => { 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); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + ], + }); } - beforeEach(() => { + beforeEach(waitForAsync(() => { init(); - }); + })); describe('canActivate', () => { describe('when the user isn\'t authorized', () => { @@ -82,7 +66,16 @@ describe('SomeFeatureAuthorizationGuard', () => { }); it('should not return true', (done) => { - guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + + const result$ = TestBed.runInInjectionContext(() => { + return someFeatureAuthorizationGuard( + () => observableOf(featureIds), + () => observableOf(objectUrl), + () => observableOf(ePersonUuid), + )(undefined, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { expect(result).not.toEqual(true); done(); }); @@ -95,7 +88,16 @@ describe('SomeFeatureAuthorizationGuard', () => { }); it('should return true', (done) => { - guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + + const result$ = TestBed.runInInjectionContext(() => { + return someFeatureAuthorizationGuard( + () => observableOf(featureIds), + () => observableOf(objectUrl), + () => observableOf(ePersonUuid), + )(undefined, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(true); done(); }); @@ -108,7 +110,16 @@ describe('SomeFeatureAuthorizationGuard', () => { }); it('should return true', (done) => { - guard.canActivate(undefined, { url: 'current-url' } as any).subscribe((result) => { + + const result$ = TestBed.runInInjectionContext(() => { + return someFeatureAuthorizationGuard( + () => observableOf(featureIds), + () => observableOf(objectUrl), + () => observableOf(ePersonUuid), + )(undefined, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(true); done(); }); diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts index 229321452f..53e5e582eb 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard.ts @@ -1,5 +1,7 @@ +import { inject } from '@angular/core'; import { ActivatedRouteSnapshot, + CanActivateFn, Router, RouterStateSnapshot, UrlTree, @@ -16,49 +18,39 @@ import { returnForbiddenUrlTreeOrLoginOnAllFalse } from '../../../shared/authori import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; +export declare type SomeFeatureGuardParamFn = (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => Observable; +export declare type StringGuardParamFn = (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => Observable; +export const defaultStringGuardParamFn = () => observableOf(undefined); + /** - * Abstract Guard for preventing unauthorized activating and loading of routes when a user - * 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. + * Guard for preventing unauthorized activating and loading of routes when a user doesn't have + * authorized rights on any of the specified features and/or object. + + * @param getFeatureIDs The features to check authorization for + * @param getObjectUrl The URL of the object to check if the user has authorized rights for, + * Optional, if not provided, the {@link Site}'s URL will be assumed + * @param getEPersonUuid The UUID of the user to check authorization rights for. + * Optional, if not provided, the authenticated user's UUID will be assumed. */ -export abstract class SomeFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - } - - /** - * True when user has authorization rights for the feature and object provided - * Redirect the user to the unauthorized page when they are not authorized for the given feature - */ - canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - 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))), +export const someFeatureAuthorizationGuard = ( + getFeatureIDs: SomeFeatureGuardParamFn, + getObjectUrl: StringGuardParamFn = defaultStringGuardParamFn, + getEPersonUuid: StringGuardParamFn = defaultStringGuardParamFn, +): CanActivateFn => { + return (route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => { + const authorizationService = inject(AuthorizationDataService); + const router = inject(Router); + const authService = inject(AuthService); + return observableCombineLatest([ + getFeatureIDs(route, state), + getObjectUrl(route, state), + getEPersonUuid(route, state), + ]).pipe( + switchMap(([featureIDs, objectUrl, ePersonUuid]: [FeatureID[], string, string]) => + observableCombineLatest(featureIDs.map((featureID) => authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid))), ), - returnForbiddenUrlTreeOrLoginOnAllFalse(this.router, this.authService, state.url), + returnForbiddenUrlTreeOrLoginOnAllFalse(router, authService, state.url), ); - } + }; +}; - /** - * The features to check authorization for - * Override this method to define a list of features - */ - abstract getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; - - /** - * The URL of the object to check if the user has authorized rights for - * Override this method to define an object URL. If not provided, the {@link Site}'s URL will be used - */ - getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(undefined); - } - - /** - * The UUID of the user to check authorization rights for - * Override this method to define an {@link EPerson} UUID. If not provided, the authenticated user's UUID will be used. - */ - getEPersonUuid(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(undefined); - } -} diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/statistics-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/statistics-administrator.guard.ts index b301d550a1..21cafeaba3 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/statistics-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/statistics-administrator.guard.ts @@ -1,35 +1,12 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../../auth/auth.service'; -import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; -import { SingleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from './single-feature-authorization.guard'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have group * management rights */ -@Injectable({ - providedIn: 'root', -}) -export class StatisticsAdministratorGuard extends SingleFeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check group management rights - */ - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanViewUsageStatistics); - } -} +export const statisticsAdministratorGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.CanViewUsageStatistics)); diff --git a/src/app/core/data/object-updates/object-updates.reducer.ts b/src/app/core/data/object-updates/object-updates.reducer.ts index e014889850..cadae9ae83 100644 --- a/src/app/core/data/object-updates/object-updates.reducer.ts +++ b/src/app/core/data/object-updates/object-updates.reducer.ts @@ -61,6 +61,8 @@ export interface VirtualMetadataSource { export interface RelationshipIdentifiable extends Identifiable { nameVariant?: string; + originalItem: Item; + originalIsLeft: boolean relatedItem: Item; relationship: Relationship; type: RelationshipType; diff --git a/src/app/core/data/object-updates/object-updates.service.ts b/src/app/core/data/object-updates/object-updates.service.ts index 1f036ddfb1..75b554d87b 100644 --- a/src/app/core/data/object-updates/object-updates.service.ts +++ b/src/app/core/data/object-updates/object-updates.service.ts @@ -15,6 +15,7 @@ import { filter, map, switchMap, + take, } from 'rxjs/operators'; import { @@ -212,8 +213,14 @@ export class ObjectUpdatesService { * @param url The page's URL for which the changes are saved * @param field An updated field for the page's object */ - saveAddFieldUpdate(url: string, field: Identifiable) { + saveAddFieldUpdate(url: string, field: Identifiable): Observable { + const update$: Observable = this.getFieldUpdatesExclusive(url, [field]).pipe( + filter((fieldUpdates: FieldUpdates) => fieldUpdates[field.uuid].changeType === FieldChangeType.ADD), + take(1), + map(() => true), + ); this.saveFieldUpdate(url, field, FieldChangeType.ADD); + return update$; } /** @@ -221,8 +228,14 @@ export class ObjectUpdatesService { * @param url The page's URL for which the changes are saved * @param field An updated field for the page's object */ - saveRemoveFieldUpdate(url: string, field: Identifiable) { + saveRemoveFieldUpdate(url: string, field: Identifiable): Observable { + const update$: Observable = this.getFieldUpdatesExclusive(url, [field]).pipe( + filter((fieldUpdates: FieldUpdates) => fieldUpdates[field.uuid].changeType === FieldChangeType.REMOVE), + take(1), + map(() => true), + ); this.saveFieldUpdate(url, field, FieldChangeType.REMOVE); + return update$; } /** diff --git a/src/app/core/data/relationship-data.service.spec.ts b/src/app/core/data/relationship-data.service.spec.ts index 753d07b21e..1fa30ae7e4 100644 --- a/src/app/core/data/relationship-data.service.spec.ts +++ b/src/app/core/data/relationship-data.service.spec.ts @@ -128,6 +128,7 @@ describe('RelationshipDataService', () => { const itemService = jasmine.createSpyObj('itemService', { findById: (uuid) => createSuccessfulRemoteDataObject(relatedItems.find((relatedItem) => relatedItem.id === uuid)), findByHref: createSuccessfulRemoteDataObject$(relatedItems[0]), + getIDHrefObs: (uuid: string) => observableOf(`https://demo.dspace.org/server/api/core/items/${uuid}`), }); const getRequestEntry$ = (successful: boolean) => { @@ -244,6 +245,16 @@ describe('RelationshipDataService', () => { }); }); + describe('searchByItemsAndType', () => { + it('should call addDependency for each item to invalidate the request when one of the items is update', () => { + spyOn(service as any, 'addDependency'); + + service.searchByItemsAndType(relationshipType.id, item.id, relationshipType.leftwardType, ['item-id-1', 'item-id-2']); + + expect((service as any).addDependency).toHaveBeenCalledTimes(2); + }); + }); + describe('resolveMetadataRepresentation', () => { const parentItem: Item = Object.assign(new Item(), { id: 'parent-item', diff --git a/src/app/core/data/relationship-data.service.ts b/src/app/core/data/relationship-data.service.ts index 17d0fa6634..0ab9a0a3a2 100644 --- a/src/app/core/data/relationship-data.service.ts +++ b/src/app/core/data/relationship-data.service.ts @@ -155,8 +155,11 @@ export class RelationshipDataService extends IdentifiableDataService> { + deleteRelationship(id: string, copyVirtualMetadata: string, shouldRefresh = true): Observable> { return this.getRelationshipEndpoint(id).pipe( isNotEmptyOperator(), take(1), @@ -167,7 +170,11 @@ export class RelationshipDataService extends IdentifiableDataService this.rdbService.buildFromRequestUUID(restRequest.uuid)), getFirstCompletedRemoteData(), - tap(() => this.refreshRelationshipItemsInCacheByRelationship(id)), + tap(() => { + if (shouldRefresh) { + this.refreshRelationshipItemsInCacheByRelationship(id); + } + }), ); } @@ -178,8 +185,11 @@ export class RelationshipDataService extends IdentifiableDataService> { + addRelationship(typeId: string, item1: Item, item2: Item, leftwardValue?: string, rightwardValue?: string, shouldRefresh = true): Observable> { const options: HttpOptions = Object.create({}); let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'text/uri-list'); @@ -194,8 +204,12 @@ export class RelationshipDataService extends IdentifiableDataService this.rdbService.buildFromRequestUUID(restRequest.uuid)), getFirstCompletedRemoteData(), - tap(() => this.refreshRelationshipItemsInCache(item1)), - tap(() => this.refreshRelationshipItemsInCache(item2)), + tap(() => { + if (shouldRefresh) { + this.refreshRelationshipItemsInCache(item1); + this.refreshRelationshipItemsInCache(item2); + } + }), ) as Observable>; } @@ -223,7 +237,7 @@ export class RelationshipDataService extends IdentifiableDataService + this.getItemRelationshipsByLabel(item, label, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), + ); } /** @@ -548,13 +574,18 @@ export class RelationshipDataService extends IdentifiableDataService>> = this.searchBy( 'byItemsAndType', { searchParams: searchParams, }, ) as Observable>>; + arrayOfItemIds.forEach((itemId: string) => { + this.addDependency(searchRD$, this.itemService.getIDHrefObs(encodeURIComponent(itemId))); + }); + + return searchRD$; } /** diff --git a/src/app/core/data/version-history-data.service.spec.ts b/src/app/core/data/version-history-data.service.spec.ts index a4e2a2c436..607b938561 100644 --- a/src/app/core/data/version-history-data.service.spec.ts +++ b/src/app/core/data/version-history-data.service.spec.ts @@ -69,6 +69,10 @@ describe('VersionHistoryDataService', () => { }, }, }); + const version1WithDraft = Object.assign(new Version(), { + ...version1, + versionhistory: createSuccessfulRemoteDataObject$(versionHistoryDraft), + }); const versions = [version1, version2]; versionHistory.versions = createSuccessfulRemoteDataObject$(createPaginatedList(versions)); const item1 = Object.assign(new Item(), { @@ -190,21 +194,18 @@ describe('VersionHistoryDataService', () => { }); describe('hasDraftVersion$', () => { - beforeEach(waitForAsync(() => { - versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(version1)); - })); it('should return false if draftVersion is false', fakeAsync(() => { - versionService.getHistoryFromVersion.and.returnValue(of(versionHistory)); + versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(version1)); service.hasDraftVersion$('href').subscribe((res) => { expect(res).toBeFalse(); }); })); + it('should return true if draftVersion is true', fakeAsync(() => { - versionService.getHistoryFromVersion.and.returnValue(of(versionHistoryDraft)); + versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(version1WithDraft)); service.hasDraftVersion$('href').subscribe((res) => { expect(res).toBeTrue(); }); })); }); - }); diff --git a/src/app/core/data/version-history-data.service.ts b/src/app/core/data/version-history-data.service.ts index 6b9e855146..cc93e275ba 100644 --- a/src/app/core/data/version-history-data.service.ts +++ b/src/app/core/data/version-history-data.service.ts @@ -1,17 +1,22 @@ import { HttpHeaders } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { + combineLatest, Observable, - of, + of as observableOf, } from 'rxjs'; import { filter, + find, map, switchMap, take, } from 'rxjs/operators'; -import { hasValueOperator } from '../../shared/empty.util'; +import { + hasValue, + hasValueOperator, +} from '../../shared/empty.util'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model'; import { @@ -29,7 +34,6 @@ import { getFirstSucceededRemoteDataPayload, getRemoteDataPayload, } from '../shared/operators'; -import { sendRequest } from '../shared/request.operators'; import { Version } from '../shared/version.model'; import { VersionHistory } from '../shared/version-history.model'; import { IdentifiableDataService } from './base/identifiable-data.service'; @@ -38,7 +42,6 @@ import { PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; import { PostRequest } from './request.models'; import { RequestService } from './request.service'; -import { RestRequest } from './rest-request.model'; import { VersionDataService } from './version-data.service'; /** @@ -100,19 +103,31 @@ export class VersionHistoryDataService extends IdentifiableDataService> { + const requestId = this.requestService.generateRequestId(); const requestOptions: HttpOptions = Object.create({}); let requestHeaders = new HttpHeaders(); requestHeaders = requestHeaders.append('Content-Type', 'text/uri-list'); requestOptions.headers = requestHeaders; - return this.halService.getEndpoint(this.versionsEndpoint).pipe( + this.halService.getEndpoint(this.versionsEndpoint).pipe( take(1), map((endpointUrl: string) => (summary?.length > 0) ? `${endpointUrl}?summary=${summary}` : `${endpointUrl}`), - map((endpointURL: string) => new PostRequest(this.requestService.generateRequestId(), endpointURL, itemHref, requestOptions)), - sendRequest(this.requestService), - switchMap((restRequest: RestRequest) => this.rdbService.buildFromRequestUUID(restRequest.uuid)), + find((href: string) => hasValue(href)), + ).subscribe((href) => { + const request = new PostRequest(requestId, href, itemHref, requestOptions); + if (hasValue(this.responseMsToLive)) { + request.responseMsToLive = this.responseMsToLive; + } + + this.requestService.send(request); + }); + + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, (versionRD) => combineLatest([ + this.requestService.setStaleByHrefSubstring(versionRD.payload._links.self.href), + this.requestService.setStaleByHrefSubstring(versionRD.payload._links.versionhistory.href), + ])).pipe( getFirstCompletedRemoteData(), - ) as Observable>; + ); } /** @@ -151,7 +166,7 @@ export class VersionHistoryDataService extends IdentifiableDataService res.versionhistory), getFirstSucceededRemoteDataPayload(), switchMap((versionHistoryRD) => this.getLatestVersionFromHistory$(versionHistoryRD)), - ) : of(null); + ) : observableOf(null); } /** @@ -162,8 +177,8 @@ export class VersionHistoryDataService extends IdentifiableDataService { return version ? this.getLatestVersion$(version).pipe( take(1), - switchMap((latestVersion) => of(version.version === latestVersion.version)), - ) : of(null); + switchMap((latestVersion) => observableOf(version.version === latestVersion.version)), + ) : observableOf(null); } /** @@ -172,17 +187,22 @@ export class VersionHistoryDataService extends IdentifiableDataService { - return this.versionDataService.findByHref(versionHref, true, true, followLink('versionhistory')).pipe( + return this.versionDataService.findByHref(versionHref, false, true, followLink('versionhistory')).pipe( getFirstCompletedRemoteData(), - switchMap((res) => { - if (res.hasSucceeded && !res.hasNoContent) { - return of(res).pipe( - getFirstSucceededRemoteDataPayload(), - switchMap((version) => this.versionDataService.getHistoryFromVersion(version)), - map((versionHistory) => versionHistory ? versionHistory.draftVersion : false), + switchMap((versionRD: RemoteData) => { + if (versionRD.hasSucceeded && !versionRD.hasNoContent) { + return versionRD.payload.versionhistory.pipe( + getFirstCompletedRemoteData(), + map((versionHistoryRD: RemoteData) => { + if (versionHistoryRD.hasSucceeded && !versionHistoryRD.hasNoContent) { + return versionHistoryRD.payload.draftVersion; + } else { + return false; + } + }), ); } else { - return of(false); + return observableOf(false); } }), ); diff --git a/src/app/core/end-user-agreement/abstract-end-user-agreement.guard.ts b/src/app/core/end-user-agreement/abstract-end-user-agreement.guard.ts deleted file mode 100644 index 2937011a38..0000000000 --- a/src/app/core/end-user-agreement/abstract-end-user-agreement.guard.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, - UrlTree, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; - -import { environment } from '../../../environments/environment'; -import { returnEndUserAgreementUrlTreeOnFalse } from '../shared/authorized.operators'; - -/** - * An abstract guard for redirecting users to the user agreement page if a certain condition is met - * That condition is defined by abstract method hasAccepted - */ -export abstract class AbstractEndUserAgreementGuard { - - constructor(protected router: Router) { - } - - /** - * True when the user agreement has been accepted - * The user will be redirected to the End User Agreement page if they haven't accepted it before - * A redirect URL will be provided with the navigation so the component can redirect the user back to the blocked route - * when they're finished accepting the agreement - */ - canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - if (!environment.info.enableEndUserAgreement) { - return observableOf(true); - } - return this.hasAccepted().pipe( - returnEndUserAgreementUrlTreeOnFalse(this.router, state.url), - ); - } - - /** - * This abstract method determines how the User Agreement has to be accepted before the user is allowed to visit - * the desired route - */ - abstract hasAccepted(): Observable; - -} diff --git a/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.spec.ts b/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.spec.ts index a801fb7a6f..640859c7a4 100644 --- a/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.spec.ts +++ b/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.spec.ts @@ -1,13 +1,14 @@ +import { TestBed } from '@angular/core/testing'; import { Router, UrlTree, } from '@angular/router'; +import { Observable } from 'rxjs'; import { EndUserAgreementService } from './end-user-agreement.service'; -import { EndUserAgreementCookieGuard } from './end-user-agreement-cookie.guard'; +import { endUserAgreementCookieGuard } from './end-user-agreement-cookie.guard'; -describe('EndUserAgreementCookieGuard', () => { - let guard: EndUserAgreementCookieGuard; +describe('endUserAgreementCookieGuard', () => { let endUserAgreementService: EndUserAgreementService; let router: Router; @@ -21,14 +22,22 @@ describe('EndUserAgreementCookieGuard', () => { parseUrl: new UrlTree(), createUrlTree: new UrlTree(), }); - - guard = new EndUserAgreementCookieGuard(endUserAgreementService, router); + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: router }, + { provide: EndUserAgreementService, useValue: endUserAgreementService }, + ], + }); }); describe('canActivate', () => { describe('when the cookie has been accepted', () => { it('should return true', (done) => { - guard.canActivate(undefined, { url: Object.assign({ url: 'redirect' }) } as any).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCookieGuard(undefined, { url: Object.assign({ url: 'redirect' }) } as any); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(true); done(); }); @@ -41,7 +50,11 @@ describe('EndUserAgreementCookieGuard', () => { }); it('should return a UrlTree', (done) => { - guard.canActivate(undefined, Object.assign({ url: 'redirect' })).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCookieGuard(undefined, { url: Object.assign({ url: 'redirect' }) } as any); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(jasmine.any(UrlTree)); done(); }); diff --git a/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.ts b/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.ts index 3eccae3ca9..2da38fb22d 100644 --- a/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.ts +++ b/src/app/core/end-user-agreement/end-user-agreement-cookie.guard.ts @@ -1,29 +1,19 @@ -import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { inject } from '@angular/core'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AbstractEndUserAgreementGuard } from './abstract-end-user-agreement.guard'; +import { endUserAgreementGuard } from './end-user-agreement.guard'; import { EndUserAgreementService } from './end-user-agreement.service'; + /** - * A guard redirecting users to the end agreement page when the user agreement cookie hasn't been accepted + * Guard for preventing unauthorized access to certain pages + * requiring the end user agreement to have been accepted in a cookie */ -@Injectable({ providedIn: 'root' }) -export class EndUserAgreementCookieGuard extends AbstractEndUserAgreementGuard { - - constructor(protected endUserAgreementService: EndUserAgreementService, - protected router: Router) { - super(router); - } - - /** - * True when the user agreement cookie has been accepted - */ - hasAccepted(): Observable { - return observableOf(this.endUserAgreementService.isCookieAccepted()); - } - -} +export const endUserAgreementCookieGuard: CanActivateFn = + endUserAgreementGuard( + () => { + const endUserAgreementService = inject(EndUserAgreementService); + return observableOf(endUserAgreementService.isCookieAccepted()); + }, + ); diff --git a/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.spec.ts b/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.spec.ts index 88722bd799..ae1f63884f 100644 --- a/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.spec.ts +++ b/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.spec.ts @@ -1,16 +1,18 @@ +import { TestBed } from '@angular/core/testing'; import { Router, UrlTree, } from '@angular/router'; -import { of as observableOf } from 'rxjs'; +import { + Observable, + of as observableOf, +} from 'rxjs'; import { environment } from '../../../environments/environment.test'; import { EndUserAgreementService } from './end-user-agreement.service'; -import { EndUserAgreementCurrentUserGuard } from './end-user-agreement-current-user.guard'; - -describe('EndUserAgreementGuard', () => { - let guard: EndUserAgreementCurrentUserGuard; +import { endUserAgreementCurrentUserGuard } from './end-user-agreement-current-user.guard'; +describe('endUserAgreementGuard', () => { let endUserAgreementService: EndUserAgreementService; let router: Router; @@ -18,19 +20,30 @@ describe('EndUserAgreementGuard', () => { endUserAgreementService = jasmine.createSpyObj('endUserAgreementService', { hasCurrentUserAcceptedAgreement: observableOf(true), }); + router = jasmine.createSpyObj('router', { navigateByUrl: {}, parseUrl: new UrlTree(), createUrlTree: new UrlTree(), }); - guard = new EndUserAgreementCurrentUserGuard(endUserAgreementService, router); + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: router }, + { provide: EndUserAgreementService, useValue: endUserAgreementService }, + ], + }); + }); describe('canActivate', () => { describe('when the user has accepted the agreement', () => { it('should return true', (done) => { - guard.canActivate(undefined, Object.assign({ url: 'redirect' })).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCurrentUserGuard(undefined, Object.assign({ url: 'redirect' })); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(true); done(); }); @@ -43,7 +56,11 @@ describe('EndUserAgreementGuard', () => { }); it('should return a UrlTree', (done) => { - guard.canActivate(undefined, Object.assign({ url: 'redirect' })).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCurrentUserGuard(undefined, Object.assign({ url: 'redirect' })); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(jasmine.any(UrlTree)); done(); }); @@ -53,7 +70,12 @@ describe('EndUserAgreementGuard', () => { describe('when the end user agreement is disabled', () => { it('should return true', (done) => { environment.info.enableEndUserAgreement = false; - guard.canActivate(undefined, Object.assign({ url: 'redirect' })).subscribe((result) => { + + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCurrentUserGuard(undefined, Object.assign({ url: 'redirect' })); + }) as Observable; + + result$.subscribe((result) => { expect(result).toEqual(true); done(); }); @@ -61,7 +83,11 @@ describe('EndUserAgreementGuard', () => { it('should not resolve to the end user agreement page', (done) => { environment.info.enableEndUserAgreement = false; - guard.canActivate(undefined, Object.assign({ url: 'redirect' })).subscribe((result) => { + const result$ = TestBed.runInInjectionContext(() => { + return endUserAgreementCurrentUserGuard(undefined, Object.assign({ url: 'redirect' })); + }) as Observable; + + result$.subscribe((result) => { expect(router.navigateByUrl).not.toHaveBeenCalled(); done(); }); diff --git a/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.ts b/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.ts index 566f049450..7c190a08b3 100644 --- a/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.ts +++ b/src/app/core/end-user-agreement/end-user-agreement-current-user.guard.ts @@ -1,34 +1,25 @@ -import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { inject } from '@angular/core'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; import { environment } from '../../../environments/environment'; -import { AbstractEndUserAgreementGuard } from './abstract-end-user-agreement.guard'; +import { endUserAgreementGuard } from './end-user-agreement.guard'; import { EndUserAgreementService } from './end-user-agreement.service'; + /** - * A guard redirecting logged in users to the end agreement page when they haven't accepted the latest user agreement + * Guard for preventing unauthorized access to certain pages + * requiring the end user agreement to have been accepted by the current user + */ -@Injectable({ providedIn: 'root' }) -export class EndUserAgreementCurrentUserGuard extends AbstractEndUserAgreementGuard { +export const endUserAgreementCurrentUserGuard: CanActivateFn = + endUserAgreementGuard( + () => { + const endUserAgreementService = inject(EndUserAgreementService); + if (!environment.info.enableEndUserAgreement) { + return observableOf(true); + } - constructor(protected endUserAgreementService: EndUserAgreementService, - protected router: Router) { - super(router); - } - - /** - * True when the currently logged in user has accepted the agreements or when the user is not currently authenticated - */ - hasAccepted(): Observable { - if (!environment.info.enableEndUserAgreement) { - return observableOf(true); - } - - return this.endUserAgreementService.hasCurrentUserAcceptedAgreement(true); - } - -} + return endUserAgreementService.hasCurrentUserAcceptedAgreement(true); + }, + ); diff --git a/src/app/core/end-user-agreement/end-user-agreement.guard.ts b/src/app/core/end-user-agreement/end-user-agreement.guard.ts new file mode 100644 index 0000000000..911dd54e25 --- /dev/null +++ b/src/app/core/end-user-agreement/end-user-agreement.guard.ts @@ -0,0 +1,34 @@ +import { inject } from '@angular/core'; +import { + ActivatedRouteSnapshot, + CanActivateFn, + Router, + RouterStateSnapshot, + UrlTree, +} from '@angular/router'; +import { + Observable, + of as observableOf, +} from 'rxjs'; + +import { environment } from '../../../environments/environment'; +import { returnEndUserAgreementUrlTreeOnFalse } from '../shared/authorized.operators'; + +export declare type HasAcceptedGuardParamFn = () => Observable; +/** + * Guard for preventing activating when the user has not accepted the EndUserAgreement + * @param hasAccepted Function determining if the EndUserAgreement has been accepted + */ +export const endUserAgreementGuard = ( + hasAccepted: HasAcceptedGuardParamFn, +): CanActivateFn => { + return (route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable => { + const router = inject(Router); + if (!environment.info.enableEndUserAgreement) { + return observableOf(true); + } + return hasAccepted().pipe( + returnEndUserAgreementUrlTreeOnFalse(router, state.url), + ); + }; +}; diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index e0672e3207..0de6de7407 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -107,13 +107,17 @@ export class EPersonDataService extends IdentifiableDataService impleme * @param scope Scope of the EPeople search, default byMetadata * @param query Query of search * @param options Options of search request + * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's + * no valid cached version. Defaults to true + * @param reRequestOnStale Whether or not the request should automatically be re- + * requested after the response becomes stale */ - public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean): Observable>> { + public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean, reRequestOnStale = true): Observable>> { switch (scope) { case 'metadata': - return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale); case 'email': - return this.getEPersonByEmail(query.trim()).pipe( + return this.getEPersonByEmail(query.trim(), useCachedVersionIfAvailable, reRequestOnStale).pipe( map((rd: RemoteData) => { if (rd.hasSucceeded) { // Turn the single EPerson or NoContent in to a PaginatedList @@ -145,7 +149,7 @@ export class EPersonDataService extends IdentifiableDataService impleme }), ); default: - return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable); + return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale); } } diff --git a/src/app/core/rest-property/forgot-password-check-guard.guard.ts b/src/app/core/rest-property/forgot-password-check-guard.guard.ts index cc74e8039f..49727dda11 100644 --- a/src/app/core/rest-property/forgot-password-check-guard.guard.ts +++ b/src/app/core/rest-property/forgot-password-check-guard.guard.ts @@ -1,37 +1,11 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../auth/auth.service'; -import { AuthorizationDataService } from '../data/feature-authorization/authorization-data.service'; -import { SingleFeatureAuthorizationGuard } from '../data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard'; +import { singleFeatureAuthorizationGuard } from '../data/feature-authorization/feature-authorization-guard/single-feature-authorization.guard'; import { FeatureID } from '../data/feature-authorization/feature-id'; -@Injectable({ - providedIn: 'root', -}) /** * Guard that checks if the forgot-password feature is enabled */ -export class ForgotPasswordCheckGuard extends SingleFeatureAuthorizationGuard { - - constructor( - protected readonly authorizationService: AuthorizationDataService, - protected readonly router: Router, - protected readonly authService: AuthService, - ) { - super(authorizationService, router, authService); - } - - getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return of(FeatureID.EPersonForgotPassword); - } - -} +export const forgotPasswordCheckGuard: CanActivateFn = + singleFeatureAuthorizationGuard(() => observableOf(FeatureID.EPersonForgotPassword)); diff --git a/src/app/core/submission/workflowitem-data.service.ts b/src/app/core/submission/workflowitem-data.service.ts index c7a6b48231..bb0acb5761 100644 --- a/src/app/core/submission/workflowitem-data.service.ts +++ b/src/app/core/submission/workflowitem-data.service.ts @@ -29,14 +29,12 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { NoContent } from '../shared/NoContent.model'; import { getFirstCompletedRemoteData } from '../shared/operators'; import { WorkflowItem } from './models/workflowitem.model'; -import { WorkspaceItem } from './models/workspaceitem.model'; /** * A service that provides methods to make REST requests with workflow items endpoint. */ @Injectable({ providedIn: 'root' }) export class WorkflowItemDataService extends IdentifiableDataService implements SearchData, DeleteData { - protected linkPath = 'workflowitems'; protected searchByItemLinkPath = 'item'; protected responseMsToLive = 10 * 1000; @@ -50,7 +48,7 @@ export class WorkflowItemDataService extends IdentifiableDataService[]): Observable> { + public findByItem(uuid: string, useCachedVersionIfAvailable = false, reRequestOnStale = true, options: FindListOptions = {}, ...linksToFollow: FollowLinkConfig[]): Observable> { const findListOptions = new FindListOptions(); findListOptions.searchParams = [new RequestParam('uuid', uuid)]; const href$ = this.searchData.getSearchByHref(this.searchByItemLinkPath, findListOptions, ...linksToFollow); @@ -134,7 +132,7 @@ export class WorkflowItemDataService extends IdentifiableDataService>} * Return an observable that emits response from the server */ - public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig[]): Observable>> { + public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig[]): Observable>> { return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } diff --git a/src/app/core/submission/workspaceitem-data.service.spec.ts b/src/app/core/submission/workspaceitem-data.service.spec.ts index 43a61bef99..8837792a78 100644 --- a/src/app/core/submission/workspaceitem-data.service.spec.ts +++ b/src/app/core/submission/workspaceitem-data.service.spec.ts @@ -2,13 +2,18 @@ import { HttpClient, HttpHeaders, } from '@angular/common/http'; +import { waitForAsync } from '@angular/core/testing'; import { Store } from '@ngrx/store'; import { cold, getTestScheduler, hot, } from 'jasmine-marbles'; -import { of as observableOf } from 'rxjs'; +import { + of as observableOf, + of, +} from 'rxjs'; +import { map } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; import { getMockHrefOnlyDataService } from '../../shared/mocks/href-only-data.service.mock'; @@ -151,12 +156,17 @@ describe('WorkspaceitemDataService test', () => { }); describe('findByItem', () => { - it('should proxy the call to UpdateDataServiceImpl.findByHref', () => { + it('should proxy the call to UpdateDataServiceImpl.findByHref', waitForAsync(() => { scheduler.schedule(() => service.findByItem('1234-1234', true, true, pageInfo)); scheduler.flush(); - const searchUrl = service.getIDHref('item', [new RequestParam('uuid', '1234-1234')]); - expect((service as any).findByHref).toHaveBeenCalledWith(searchUrl, true, true); - }); + const searchUrl$ = + of('https://rest.api/rest/api/submission/workspaceitems/search/item') + .pipe(map(href => service.buildHrefFromFindOptions(href, { searchParams: [new RequestParam('uuid', '1234-1234')] }, []))); + searchUrl$.subscribe((url) => { + expect(url).toEqual('https://rest.api/rest/api/submission/workspaceitems/search/item?uuid=1234-1234'); + }); + expect((service as any).findByHref).toHaveBeenCalled(); + })); it('should return a RemoteData for the search', () => { const result = service.findByItem('1234-1234', true, true, pageInfo); diff --git a/src/app/core/submission/workspaceitem-data.service.ts b/src/app/core/submission/workspaceitem-data.service.ts index 1e22c87d59..17aafaf651 100644 --- a/src/app/core/submission/workspaceitem-data.service.ts +++ b/src/app/core/submission/workspaceitem-data.service.ts @@ -45,7 +45,7 @@ export class WorkspaceitemDataService extends IdentifiableDataService; - private searchData: SearchData; + private searchData: SearchDataImpl; constructor( protected comparator: DSOChangeAnalyzer, @@ -91,7 +91,7 @@ export class WorkspaceitemDataService extends IdentifiableDataService[]): Observable> { const findListOptions = new FindListOptions(); findListOptions.searchParams = [new RequestParam('uuid', uuid)]; - const href$ = this.getIDHref(this.searchByItemLinkPath, findListOptions, ...linksToFollow); + const href$ = this.searchData.getSearchByHref(this.searchByItemLinkPath, findListOptions, ...linksToFollow); return this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } diff --git a/src/app/footer/footer.component.html b/src/app/footer/footer.component.html index 35487f91ae..4841674852 100644 --- a/src/app/footer/footer.component.html +++ b/src/app/footer/footer.component.html @@ -84,7 +84,7 @@
diff --git a/src/app/health-page/health-page-routes.ts b/src/app/health-page/health-page-routes.ts index 4c02bc548f..f87cf8e3d3 100644 --- a/src/app/health-page/health-page-routes.ts +++ b/src/app/health-page/health-page-routes.ts @@ -1,10 +1,7 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { i18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; -import { SiteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; +import { siteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; import { HealthPageComponent } from './health-page.component'; export const ROUTES: Route[] = [ @@ -15,7 +12,7 @@ export const ROUTES: Route[] = [ breadcrumbKey: 'health', title: 'health-page.title', }, - canActivate: mapToCanActivate([SiteAdministratorGuard]), + canActivate: [siteAdministratorGuard], component: HealthPageComponent, }, ]; diff --git a/src/app/info/notify-info/notify-info.component.html b/src/app/info/notify-info/notify-info.component.html index 3370f83d03..d54a86b568 100644 --- a/src/app/info/notify-info/notify-info.component.html +++ b/src/app/info/notify-info/notify-info.component.html @@ -1,18 +1,13 @@
- - {{ 'coar-notify-support.title' | translate }} - - -

{{ 'coar-notify-support.title' | translate }}

-

+

{{ 'coar-notify-support.title' | translate }}

+

-

{{ 'coar-notify-support.ldn-inbox.title' | translate }}

-

+

{{ 'coar-notify-support.ldn-inbox.title' | translate }}

+

-

{{ 'coar-notify-support.message-moderation.title' | translate }}

-

- {{ 'coar-notify-support.message-moderation.content' | translate }} - {{ 'coar-notify-support.message-moderation.feedback-form' | translate }} -

- +

{{ 'coar-notify-support.message-moderation.title' | translate }}

+

+ {{ 'coar-notify-support.message-moderation.content' | translate }} + {{ 'coar-notify-support.message-moderation.feedback-form' | translate }} +

diff --git a/src/app/info/notify-info/notify-info.component.spec.ts b/src/app/info/notify-info/notify-info.component.spec.ts index 010227b326..ecd584cfb0 100644 --- a/src/app/info/notify-info/notify-info.component.spec.ts +++ b/src/app/info/notify-info/notify-info.component.spec.ts @@ -16,7 +16,9 @@ describe('NotifyInfoComponent', () => { let notifyInfoServiceSpy: any; beforeEach(async () => { - notifyInfoServiceSpy = jasmine.createSpyObj('NotifyInfoService', ['getCoarLdnLocalInboxUrls']); + notifyInfoServiceSpy = jasmine.createSpyObj('NotifyInfoService', { + getCoarLdnLocalInboxUrls: of([]), + }); await TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), NotifyInfoComponent], @@ -31,8 +33,7 @@ describe('NotifyInfoComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(NotifyInfoComponent); component = fixture.componentInstance; - component.coarRestApiUrl = of([]); - spyOn(component, 'generateCoarRestApiLinksHTML').and.returnValue(of('')); + component.coarRestApiUrls$ = of(''); fixture.detectChanges(); }); diff --git a/src/app/info/notify-info/notify-info.component.ts b/src/app/info/notify-info/notify-info.component.ts index 38bbf44595..a9b4661fad 100644 --- a/src/app/info/notify-info/notify-info.component.ts +++ b/src/app/info/notify-info/notify-info.component.ts @@ -8,7 +8,6 @@ import { TranslateModule } from '@ngx-translate/core'; import { map, Observable, - of, } from 'rxjs'; import { NotifyInfoService } from '../../core/coar-notify/notify-info/notify-info.service'; @@ -31,26 +30,17 @@ export class NotifyInfoComponent implements OnInit { /** * Observable containing the COAR REST INBOX API URLs. */ - coarRestApiUrl: Observable = of([]); + coarRestApiUrls$: Observable; - constructor(private notifyInfoService: NotifyInfoService) {} + constructor( + protected notifyInfoService: NotifyInfoService, + ) { + } ngOnInit() { - this.coarRestApiUrl = this.notifyInfoService.getCoarLdnLocalInboxUrls(); - } - - /** - * Generates HTML code for COAR REST API links. - * @returns An Observable that emits the generated HTML code. - */ - generateCoarRestApiLinksHTML() { - return this.coarRestApiUrl.pipe( - // transform the data into HTML - map((urls) => { - return urls.map(url => ` - ${url} - `).join(','); - }), + this.coarRestApiUrls$ = this.notifyInfoService.getCoarLdnLocalInboxUrls().pipe( + map((urls: string[]) => urls.map((url: string) => `${url}`).join(', ')), ); } + } diff --git a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 77c3fdaafb..69b234fbaf 100644 --- a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -55,6 +55,10 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl */ updates$: Observable; + hasChanges$: Observable; + + isReinstatable$: Observable; + /** * Route to the item's page */ @@ -101,10 +105,9 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl } this.discardTimeOut = environment.item.edit.undoTimeout; - this.url = this.router.url; - if (this.url.indexOf('?') > 0) { - this.url = this.url.substr(0, this.url.indexOf('?')); - } + this.url = this.router.url.split('?')[0]; + this.hasChanges$ = this.hasChanges(); + this.isReinstatable$ = this.isReinstatable(); this.hasChanges().pipe(first()).subscribe((hasChanges) => { if (!hasChanges) { this.initializeOriginalFields(); diff --git a/src/app/item-page/edit-item-page/edit-item-page-routes.ts b/src/app/item-page/edit-item-page/edit-item-page-routes.ts index 1b1e43a883..a7189f9888 100644 --- a/src/app/item-page/edit-item-page/edit-item-page-routes.ts +++ b/src/app/item-page/edit-item-page/edit-item-page-routes.ts @@ -1,7 +1,4 @@ -import { - mapToCanActivate, - Route, -} from '@angular/router'; +import { Route } from '@angular/router'; import { i18nBreadcrumbResolver } from '../../core/breadcrumbs/i18n-breadcrumb.resolver'; import { ThemedDsoEditMetadataComponent } from '../../dso-shared/dso-edit-metadata/themed-dso-edit-metadata.component'; @@ -27,17 +24,21 @@ import { ItemCollectionMapperComponent } from './item-collection-mapper/item-col import { ItemCurateComponent } from './item-curate/item-curate.component'; import { ItemDeleteComponent } from './item-delete/item-delete.component'; import { ItemMoveComponent } from './item-move/item-move.component'; -import { ItemPageAccessControlGuard } from './item-page-access-control.guard'; -import { ItemPageBitstreamsGuard } from './item-page-bitstreams.guard'; -import { ItemPageCollectionMapperGuard } from './item-page-collection-mapper.guard'; -import { ItemPageCurateGuard } from './item-page-curate.guard'; -import { ItemPageMetadataGuard } from './item-page-metadata.guard'; -import { ItemPageRegisterDoiGuard } from './item-page-register-doi.guard'; -import { ItemPageReinstateGuard } from './item-page-reinstate.guard'; -import { ItemPageRelationshipsGuard } from './item-page-relationships.guard'; -import { ItemPageStatusGuard } from './item-page-status.guard'; -import { ItemPageVersionHistoryGuard } from './item-page-version-history.guard'; -import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; +import { itemPageAccessControlGuard } from './item-page-access-control.guard'; +import { itemPageBitstreamsGuard } from './item-page-bitstreams.guard'; +import { itemPageCollectionMapperGuard } from './item-page-collection-mapper.guard'; +import { itemPageCurateGuard } from './item-page-curate.guard'; +import { itemPageDeleteGuard } from './item-page-delete.guard'; +import { itemPageEditAuthorizationsGuard } from './item-page-edit-authorizations.guard'; +import { itemPageMetadataGuard } from './item-page-metadata.guard'; +import { itemPageMoveGuard } from './item-page-move.guard'; +import { itemPagePrivateGuard } from './item-page-private.guard'; +import { itemPageRegisterDoiGuard } from './item-page-register-doi.guard'; +import { itemPageReinstateGuard } from './item-page-reinstate.guard'; +import { itemPageRelationshipsGuard } from './item-page-relationships.guard'; +import { itemPageStatusGuard } from './item-page-status.guard'; +import { itemPageVersionHistoryGuard } from './item-page-version-history.guard'; +import { itemPageWithdrawGuard } from './item-page-withdraw.guard'; import { ItemPrivateComponent } from './item-private/item-private.component'; import { ItemPublicComponent } from './item-public/item-public.component'; import { ItemRegisterDoiComponent } from './item-register-doi/item-register-doi.component'; @@ -72,31 +73,31 @@ export const ROUTES: Route[] = [ path: 'status', component: ThemedItemStatusComponent, data: { title: 'item.edit.tabs.status.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageStatusGuard]), + canActivate: [itemPageStatusGuard], }, { path: 'bitstreams', component: ItemBitstreamsComponent, data: { title: 'item.edit.tabs.bitstreams.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageBitstreamsGuard]), + canActivate: [itemPageBitstreamsGuard], }, { path: 'metadata', component: ThemedDsoEditMetadataComponent, data: { title: 'item.edit.tabs.metadata.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageMetadataGuard]), + canActivate: [itemPageMetadataGuard], }, { path: 'curate', component: ItemCurateComponent, data: { title: 'item.edit.tabs.curate.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageCurateGuard]), + canActivate: [itemPageCurateGuard], }, { path: 'relationships', component: ItemRelationshipsComponent, data: { title: 'item.edit.tabs.relationships.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageRelationshipsGuard]), + canActivate: [itemPageRelationshipsGuard], }, /* TODO - uncomment & fix when view page exists { @@ -114,19 +115,19 @@ export const ROUTES: Route[] = [ path: 'versionhistory', component: ItemVersionHistoryComponent, data: { title: 'item.edit.tabs.versionhistory.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageVersionHistoryGuard]), + canActivate: [itemPageVersionHistoryGuard], }, { path: 'access-control', component: ItemAccessControlComponent, data: { title: 'item.edit.tabs.access-control.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageAccessControlGuard]), + canActivate: [itemPageAccessControlGuard], }, { path: 'mapper', component: ItemCollectionMapperComponent, data: { title: 'item.edit.tabs.item-mapper.title', showBreadcrumbs: true }, - canActivate: mapToCanActivate([ItemPageCollectionMapperGuard]), + canActivate: [itemPageCollectionMapperGuard], }, ], }, @@ -137,16 +138,17 @@ export const ROUTES: Route[] = [ { path: ITEM_EDIT_WITHDRAW_PATH, component: ItemWithdrawComponent, - canActivate: mapToCanActivate([ItemPageWithdrawGuard]), + canActivate: [itemPageWithdrawGuard], }, { path: ITEM_EDIT_REINSTATE_PATH, component: ItemReinstateComponent, - canActivate: mapToCanActivate([ItemPageReinstateGuard]), + canActivate: [itemPageReinstateGuard], }, { path: ITEM_EDIT_PRIVATE_PATH, component: ItemPrivateComponent, + canActivate: [itemPagePrivateGuard], }, { path: ITEM_EDIT_PUBLIC_PATH, @@ -155,16 +157,18 @@ export const ROUTES: Route[] = [ { path: ITEM_EDIT_DELETE_PATH, component: ItemDeleteComponent, + canActivate: [itemPageDeleteGuard], }, { path: ITEM_EDIT_MOVE_PATH, component: ItemMoveComponent, data: { title: 'item.edit.move.title' }, + canActivate: [itemPageMoveGuard], }, { path: ITEM_EDIT_REGISTER_DOI_PATH, component: ItemRegisterDoiComponent, - canActivate: mapToCanActivate([ItemPageRegisterDoiGuard]), + canActivate: [itemPageRegisterDoiGuard], data: { title: 'item.edit.register-doi.title' }, }, { @@ -192,6 +196,7 @@ export const ROUTES: Route[] = [ data: { title: 'item.edit.authorizations.title' }, }, ], + canActivate: [itemPageEditAuthorizationsGuard], }, ], }, diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html index bb0c3e3760..88d984c19f 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html @@ -6,21 +6,21 @@ class="fas fa-upload">  {{"item.edit.bitstreams.upload-button" | translate}} - - - - - - diff --git a/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts b/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts index dd1e08c98d..1655856a3e 100644 --- a/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts +++ b/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line max-classes-per-file import { AsyncPipe, NgForOf, @@ -68,6 +69,34 @@ import { ModifyItemOverviewComponent } from '../modify-item-overview/modify-item import { AbstractSimpleItemActionComponent } from '../simple-item-action/abstract-simple-item-action.component'; import { VirtualMetadata } from '../virtual-metadata/virtual-metadata.component'; +/** + * Data Transfer Object used to prevent the HTML template to call function returning Observables + */ +class RelationshipTypeDTO { + + relationshipType: RelationshipType; + + isSelected$: Observable; + + label$: Observable; + + relationshipDTOs$: Observable; + +} + +/** + * Data Transfer Object used to prevent the HTML template to call function returning Observables + */ +class RelationshipDTO { + + relationship: Relationship; + + relatedItem$: Observable; + + virtualMetadata$: Observable; + +} + @Component({ selector: 'ds-item-delete', templateUrl: '../item-delete/item-delete.component.html', @@ -106,7 +135,7 @@ export class ItemDeleteComponent * A list of the relationship types for which this item has relations as an observable. * The list doesn't contain duplicates. */ - types$: BehaviorSubject = new BehaviorSubject([]); + typeDTOs$: BehaviorSubject = new BehaviorSubject([]); /** * A map which stores the relationships of this item for each type as observable lists @@ -135,6 +164,8 @@ export class ItemDeleteComponent */ private subs: Subscription[] = []; + public isDeleting$: BehaviorSubject = new BehaviorSubject(false); + constructor(protected route: ActivatedRoute, protected router: Router, protected notificationsService: NotificationsService, @@ -189,13 +220,24 @@ export class ItemDeleteComponent ), ); }), - ).subscribe((types: RelationshipType[]) => this.types$.next(types))); + ).subscribe((types: RelationshipType[]) => this.typeDTOs$.next(types.map((relationshipType: RelationshipType) => Object.assign(new RelationshipTypeDTO(), { + relationshipType: relationshipType, + isSelected$: this.isSelected(relationshipType), + label$: this.getLabel(relationshipType), + relationshipDTOs$: this.getRelationships(relationshipType).pipe( + map((relationships: Relationship[]) => relationships.map((relationship: Relationship) => Object.assign(new RelationshipDTO(), { + relationship: relationship, + relatedItem$: this.getRelatedItem(relationship), + virtualMetadata$: this.getVirtualMetadata(relationship), + } as RelationshipDTO))), + ), + }))))); } - this.subs.push(this.types$.pipe( + this.subs.push(this.typeDTOs$.pipe( take(1), - ).subscribe((types) => - this.objectUpdatesService.initialize(this.url, types, this.item.lastModified), + ).subscribe((types: RelationshipTypeDTO[]) => + this.objectUpdatesService.initialize(this.url, types.map((relationshipTypeDto: RelationshipTypeDTO) => relationshipTypeDto.relationshipType), this.item.lastModified), )); } @@ -368,34 +410,33 @@ export class ItemDeleteComponent * @param selected whether the type should be selected */ setSelected(type: RelationshipType, selected: boolean): void { - this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected); + if (this.isDeleting$.value === false) { + this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected); + } } /** * Perform the delete operation */ - performAction() { - - this.subs.push(this.types$.pipe( - switchMap((types) => + performAction(): void { + this.isDeleting$.next(true); + this.subs.push(this.typeDTOs$.pipe( + switchMap((types: RelationshipTypeDTO[]) => combineLatest( - types.map((type) => this.isSelected(type)), + types.map((type: RelationshipTypeDTO) => type.isSelected$), ).pipe( defaultIfEmpty([]), - map((selection) => types.filter( - (type, index) => selection[index], + map((selection: boolean[]) => types.filter( + (type: RelationshipTypeDTO, index: number) => selection[index], )), - map((selectedTypes) => selectedTypes.map((type) => type.id)), + map((selectedDtoTypes: RelationshipTypeDTO[]) => selectedDtoTypes.map((typeDto: RelationshipTypeDTO) => typeDto.relationshipType.id)), ), ), - switchMap((types) => - this.itemDataService.delete(this.item.id, types).pipe(getFirstCompletedRemoteData()), - ), - ).subscribe( - (rd: RemoteData) => { - this.notify(rd.hasSucceeded); - }, - )); + switchMap((types: string[]) => this.itemDataService.delete(this.item.id, types)), + getFirstCompletedRemoteData(), + ).subscribe((rd: RemoteData) => { + this.notify(rd.hasSucceeded); + })); } /** @@ -405,10 +446,10 @@ export class ItemDeleteComponent notify(succeeded: boolean) { if (succeeded) { this.notificationsService.success(this.translateService.get('item.edit.' + this.messageKey + '.success')); - this.router.navigate(['']); + void this.router.navigate(['']); } else { this.notificationsService.error(this.translateService.get('item.edit.' + this.messageKey + '.error')); - this.router.navigate([getItemEditRoute(this.item)]); + void this.router.navigate([getItemEditRoute(this.item)]); } } diff --git a/src/app/item-page/edit-item-page/item-page-access-control.guard.ts b/src/app/item-page/edit-item-page/item-page-access-control.guard.ts index 42ee1f3d15..bcf5ea9a13 100644 --- a/src/app/item-page/edit-item-page/item-page-access-control.guard.ts +++ b/src/app/item-page/edit-item-page/item-page-access-control.guard.ts @@ -1,43 +1,15 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring administrator rights */ -export class ItemPageAccessControlGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check administrator authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } -} +export const itemPageAccessControlGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.AdministratorOf), + ); 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 396064b524..32af7e603e 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 @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage bitstreams rights + * Check manage bitstreams authorization rights */ -export class ItemPageBitstreamsGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super( authorizationService, router, authService); - } - - /** - * Check manage bitstreams authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageBitstreamBundles); - } -} +export const itemPageBitstreamsGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanManageBitstreamBundles), + ); 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 index aee0a60371..56d9675d92 100644 --- 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 @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage mappings rights + * Check manage mappings authorization rights */ -export class ItemPageCollectionMapperGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check manage mappings authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageMappings); - } -} +export const itemPageCollectionMapperGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanManageMappings), + ); diff --git a/src/app/item-page/edit-item-page/item-page-curate.guard.ts b/src/app/item-page/edit-item-page/item-page-curate.guard.ts index 392bc5c523..40cfe00c2f 100644 --- a/src/app/item-page/edit-item-page/item-page-curate.guard.ts +++ b/src/app/item-page/edit-item-page/item-page-curate.guard.ts @@ -1,43 +1,15 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring administrator rights */ -export class ItemPageCurateGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check administrator authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } -} +export const itemPageCurateGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.AdministratorOf), + ); diff --git a/src/app/item-page/edit-item-page/item-page-delete.guard.spec.ts b/src/app/item-page/edit-item-page/item-page-delete.guard.spec.ts new file mode 100644 index 0000000000..d2d7954c6f --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-delete.guard.spec.ts @@ -0,0 +1,94 @@ +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { + Observable, + of as observableOf, +} from 'rxjs'; +import { AuthService } from 'src/app/core/auth/auth.service'; +import { AuthorizationDataService } from 'src/app/core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from 'src/app/core/data/feature-authorization/feature-id'; + +import { APP_DATA_SERVICES_MAP } from '../../../config/app-config.interface'; +import { ItemDataService } from '../../core/data/item-data.service'; +import { Item } from '../../core/shared/item.model'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { itemPageDeleteGuard } from './item-page-delete.guard'; + +describe('itemPageDeleteGuard', () => { + let authorizationService: AuthorizationDataService; + let authService: AuthService; + let router: Router; + let route; + let parentRoute; + let store: Store; + let itemService: ItemDataService; + let item: Item; + let uuid = '1234-abcdef-54321-fedcba'; + let itemSelfLink = 'test.url/1234-abcdef-54321-fedcba'; + + beforeEach(() => { + + store = jasmine.createSpyObj('store', { + dispatch: {}, + pipe: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true), + }); + router = jasmine.createSpyObj('router', { + parseUrl: {}, + navigateByUrl: undefined, + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + }); + + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0', + }, + }; + route = { + params: {}, + parent: parentRoute, + }; + item = new Item(); + item.uuid = uuid; + item._links = { self: { href: itemSelfLink } } as any; + itemService = jasmine.createSpyObj('itemService', { findById: createSuccessfulRemoteDataObject$(item) }); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: Store, useValue: store }, + { provide: APP_DATA_SERVICES_MAP, useValue: {} }, + { provide: TranslateService, useValue: getMockTranslateService() }, + { provide: ItemDataService, useValue: itemService }, + ], + }); + }); + + it('should call authorizationService.isAuthorized with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return itemPageDeleteGuard(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith( + FeatureID.CanDelete, + itemSelfLink, // This value is retrieved from the itemDataService.findById's return item's self link + undefined, // dsoPageSingleFeatureGuard never provides a function to retrieve a person ID + ); + done(); + }); + + }); +}); diff --git a/src/app/item-page/edit-item-page/item-page-delete.guard.ts b/src/app/item-page/edit-item-page/item-page-delete.guard.ts new file mode 100644 index 0000000000..99d79ca68f --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-delete.guard.ts @@ -0,0 +1,16 @@ +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; + +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 { itemPageResolver } from '../item-page.resolver'; + +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring specific authorizations. + * Checks authorization rights for deleting items. + */ +export const itemPageDeleteGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanDelete), + ); diff --git a/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.spec.ts b/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.spec.ts new file mode 100644 index 0000000000..f03e79ad36 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.spec.ts @@ -0,0 +1,94 @@ +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { + Observable, + of as observableOf, +} from 'rxjs'; +import { AuthService } from 'src/app/core/auth/auth.service'; +import { AuthorizationDataService } from 'src/app/core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from 'src/app/core/data/feature-authorization/feature-id'; + +import { APP_DATA_SERVICES_MAP } from '../../../config/app-config.interface'; +import { ItemDataService } from '../../core/data/item-data.service'; +import { Item } from '../../core/shared/item.model'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { itemPageEditAuthorizationsGuard } from './item-page-edit-authorizations.guard'; + +describe('itemPageEditAuthorizationsGuard', () => { + let authorizationService: AuthorizationDataService; + let authService: AuthService; + let router: Router; + let route; + let parentRoute; + let store: Store; + let itemService: ItemDataService; + let item: Item; + let uuid = '1234-abcdef-54321-fedcba'; + let itemSelfLink = 'test.url/1234-abcdef-54321-fedcba'; + + beforeEach(() => { + + store = jasmine.createSpyObj('store', { + dispatch: {}, + pipe: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true), + }); + router = jasmine.createSpyObj('router', { + parseUrl: {}, + navigateByUrl: undefined, + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + }); + + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0', + }, + }; + route = { + params: {}, + parent: parentRoute, + }; + item = new Item(); + item.uuid = uuid; + item._links = { self: { href: itemSelfLink } } as any; + itemService = jasmine.createSpyObj('itemService', { findById: createSuccessfulRemoteDataObject$(item) }); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: Store, useValue: store }, + { provide: APP_DATA_SERVICES_MAP, useValue: {} }, + { provide: TranslateService, useValue: getMockTranslateService() }, + { provide: ItemDataService, useValue: itemService }, + ], + }); + }); + + it('should call authorizationService.isAuthorized with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return itemPageEditAuthorizationsGuard(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith( + FeatureID.CanManagePolicies, + itemSelfLink, // This value is retrieved from the itemDataService.findById's return item's self link + undefined, // dsoPageSingleFeatureGuard never provides a function to retrieve a person ID + ); + done(); + }); + + }); +}); diff --git a/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.ts b/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.ts new file mode 100644 index 0000000000..c5032ac604 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-edit-authorizations.guard.ts @@ -0,0 +1,16 @@ +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; + +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 { itemPageResolver } from '../item-page.resolver'; + +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring specific authorizations. + * Checks authorization rights for managing policies. + */ +export const itemPageEditAuthorizationsGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanManagePolicies), + ); diff --git a/src/app/item-page/edit-item-page/item-page-metadata.guard.ts b/src/app/item-page/edit-item-page/item-page-metadata.guard.ts index 1d6d75df2a..f058eb7359 100644 --- a/src/app/item-page/edit-item-page/item-page-metadata.guard.ts +++ b/src/app/item-page/edit-item-page/item-page-metadata.guard.ts @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring edit metadata rights + * Check edit metadata authorization rights */ -export class ItemPageMetadataGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check edit metadata authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanEditMetadata); - } -} +export const itemPageMetadataGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanEditMetadata), + ); diff --git a/src/app/item-page/edit-item-page/item-page-move.guard.spec.ts b/src/app/item-page/edit-item-page/item-page-move.guard.spec.ts new file mode 100644 index 0000000000..d30f23817a --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-move.guard.spec.ts @@ -0,0 +1,94 @@ +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { + Observable, + of as observableOf, +} from 'rxjs'; +import { AuthService } from 'src/app/core/auth/auth.service'; +import { AuthorizationDataService } from 'src/app/core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from 'src/app/core/data/feature-authorization/feature-id'; + +import { APP_DATA_SERVICES_MAP } from '../../../config/app-config.interface'; +import { ItemDataService } from '../../core/data/item-data.service'; +import { Item } from '../../core/shared/item.model'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { itemPageMoveGuard } from './item-page-move.guard'; + +describe('itemPageMoveGuard', () => { + let authorizationService: AuthorizationDataService; + let authService: AuthService; + let router: Router; + let route; + let parentRoute; + let store: Store; + let itemService: ItemDataService; + let item: Item; + let uuid = '1234-abcdef-54321-fedcba'; + let itemSelfLink = 'test.url/1234-abcdef-54321-fedcba'; + + beforeEach(() => { + + store = jasmine.createSpyObj('store', { + dispatch: {}, + pipe: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true), + }); + router = jasmine.createSpyObj('router', { + parseUrl: {}, + navigateByUrl: undefined, + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + }); + + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0', + }, + }; + route = { + params: {}, + parent: parentRoute, + }; + item = new Item(); + item.uuid = uuid; + item._links = { self: { href: itemSelfLink } } as any; + itemService = jasmine.createSpyObj('itemService', { findById: createSuccessfulRemoteDataObject$(item) }); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: Store, useValue: store }, + { provide: APP_DATA_SERVICES_MAP, useValue: {} }, + { provide: TranslateService, useValue: getMockTranslateService() }, + { provide: ItemDataService, useValue: itemService }, + ], + }); + }); + + it('should call authorizationService.isAuthorized with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return itemPageMoveGuard(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith( + FeatureID.CanMove, + itemSelfLink, // This value is retrieved from the itemDataService.findById's return item's self link + undefined, // dsoPageSingleFeatureGuard never provides a function to retrieve a person ID + ); + done(); + }); + + }); +}); diff --git a/src/app/item-page/edit-item-page/item-page-move.guard.ts b/src/app/item-page/edit-item-page/item-page-move.guard.ts new file mode 100644 index 0000000000..307201ef90 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-move.guard.ts @@ -0,0 +1,16 @@ +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; + +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 { itemPageResolver } from '../item-page.resolver'; + +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring specific authorizations. + * Checks authorization rights for moving items. + */ +export const itemPageMoveGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanMove), + ); diff --git a/src/app/item-page/edit-item-page/item-page-private.guard.spec.ts b/src/app/item-page/edit-item-page/item-page-private.guard.spec.ts new file mode 100644 index 0000000000..3e573923d8 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-private.guard.spec.ts @@ -0,0 +1,94 @@ +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { + Observable, + of as observableOf, +} from 'rxjs'; +import { AuthService } from 'src/app/core/auth/auth.service'; +import { AuthorizationDataService } from 'src/app/core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from 'src/app/core/data/feature-authorization/feature-id'; + +import { APP_DATA_SERVICES_MAP } from '../../../config/app-config.interface'; +import { ItemDataService } from '../../core/data/item-data.service'; +import { Item } from '../../core/shared/item.model'; +import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { itemPagePrivateGuard } from './item-page-private.guard'; + +describe('itemPagePrivateGuard', () => { + let authorizationService: AuthorizationDataService; + let authService: AuthService; + let router: Router; + let route; + let parentRoute; + let store: Store; + let itemService: ItemDataService; + let item: Item; + let uuid = '1234-abcdef-54321-fedcba'; + let itemSelfLink = 'test.url/1234-abcdef-54321-fedcba'; + + beforeEach(() => { + + store = jasmine.createSpyObj('store', { + dispatch: {}, + pipe: observableOf(true), + }); + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true), + }); + router = jasmine.createSpyObj('router', { + parseUrl: {}, + navigateByUrl: undefined, + }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + }); + + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0', + }, + }; + route = { + params: {}, + parent: parentRoute, + }; + item = new Item(); + item.uuid = uuid; + item._links = { self: { href: itemSelfLink } } as any; + itemService = jasmine.createSpyObj('itemService', { findById: createSuccessfulRemoteDataObject$(item) }); + + TestBed.configureTestingModule({ + providers: [ + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: Router, useValue: router }, + { provide: AuthService, useValue: authService }, + { provide: Store, useValue: store }, + { provide: APP_DATA_SERVICES_MAP, useValue: {} }, + { provide: TranslateService, useValue: getMockTranslateService() }, + { provide: ItemDataService, useValue: itemService }, + ], + }); + }); + + it('should call authorizationService.isAuthorized with the appropriate arguments', (done) => { + const result$ = TestBed.runInInjectionContext(() => { + return itemPagePrivateGuard(route, { url: 'current-url' } as any); + }) as Observable; + + result$.subscribe((result) => { + expect(authorizationService.isAuthorized).toHaveBeenCalledWith( + FeatureID.CanMakePrivate, + itemSelfLink, // This value is retrieved from the itemDataService.findById's return item's self link + undefined, // dsoPageSingleFeatureGuard never provides a function to retrieve a person ID + ); + done(); + }); + + }); +}); diff --git a/src/app/item-page/edit-item-page/item-page-private.guard.ts b/src/app/item-page/edit-item-page/item-page-private.guard.ts new file mode 100644 index 0000000000..64626542ff --- /dev/null +++ b/src/app/item-page/edit-item-page/item-page-private.guard.ts @@ -0,0 +1,16 @@ +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; + +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 { itemPageResolver } from '../item-page.resolver'; + +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring specific authorizations. + * Checks authorization rights for making items private. + */ +export const itemPagePrivateGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanMakePrivate), + ); diff --git a/src/app/item-page/edit-item-page/item-page-register-doi.guard.ts b/src/app/item-page/edit-item-page/item-page-register-doi.guard.ts index 18c7939852..9bedca518b 100644 --- a/src/app/item-page/edit-item-page/item-page-register-doi.guard.ts +++ b/src/app/item-page/edit-item-page/item-page-register-doi.guard.ts @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring DOI registration rights + * Check DOI registration authorization rights */ -export class ItemPageRegisterDoiGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check DOI registration authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanRegisterDOI); - } -} +export const itemPageRegisterDoiGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.CanRegisterDOI), + ); 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 aacf2f3e7d..3e60158d0a 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,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring reinstate rights + * Check reinstate authorization rights */ -export class ItemPageReinstateGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check reinstate authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.ReinstateItem); - } -} +export const itemPageReinstateGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.ReinstateItem), + ); 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 index 5d16ae8751..fe107977db 100644 --- 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 @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage relationships rights + * Check manage relationships authorization rights */ -export class ItemPageRelationshipsGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check manage relationships authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageRelationships); - } -} +export const itemPageRelationshipsGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => 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 index 372b072e19..deeb2dbb5e 100644 --- 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 @@ -1,44 +1,17 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSomeFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard'; +import { dsoPageSomeFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-some-feature.guard'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; -import { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring any of the rights required for * the status page + * Check authorization rights */ -export class ItemPageStatusGuard extends DsoPageSomeFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(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, FeatureID.CanRegisterDOI]); - } -} +export const itemPageStatusGuard: CanActivateFn = + dsoPageSomeFeatureGuard( + () => itemPageResolver, + () => observableOf([FeatureID.CanManageMappings, FeatureID.WithdrawItem, FeatureID.ReinstateItem, FeatureID.CanManagePolicies, FeatureID.CanMakePrivate, FeatureID.CanDelete, FeatureID.CanMove, FeatureID.CanRegisterDOI]), + ); 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 index dc512527ad..99d581dce6 100644 --- 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 @@ -1,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring manage versions rights + * Check manage versions authorization rights */ -export class ItemPageVersionHistoryGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check manage versions authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.CanManageVersions); - } -} +export const itemPageVersionHistoryGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => 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 464386edfc..8e41b1c653 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,43 +1,16 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - ResolveFn, - Router, - RouterStateSnapshot, -} from '@angular/router'; -import { - Observable, - of as observableOf, -} from 'rxjs'; +import { CanActivateFn } from '@angular/router'; +import { of as observableOf } from 'rxjs'; -import { AuthService } from '../../core/auth/auth.service'; -import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { DsoPageSingleFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-single-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 { RemoteData } from '../../core/data/remote-data'; -import { Item } from '../../core/shared/item.model'; import { itemPageResolver } from '../item-page.resolver'; -@Injectable({ - providedIn: 'root', -}) /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring withdraw rights + * Check withdraw authorization rights */ -export class ItemPageWithdrawGuard extends DsoPageSingleFeatureGuard { - - protected resolver: ResolveFn> = itemPageResolver; - - constructor(protected authorizationService: AuthorizationDataService, - protected router: Router, - protected authService: AuthService) { - super(authorizationService, router, authService); - } - - /** - * Check withdraw authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.WithdrawItem); - } -} +export const itemPageWithdrawGuard: CanActivateFn = + dsoPageSingleFeatureGuard( + () => itemPageResolver, + () => observableOf(FeatureID.WithdrawItem), + ); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts new file mode 100644 index 0000000000..4809a4f730 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts @@ -0,0 +1,412 @@ +import { TestBed } from '@angular/core/testing'; +import { TranslateModule } from '@ngx-translate/core'; +import { of as observableOf } from 'rxjs'; +import { v4 as uuidv4 } from 'uuid'; + +import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { FieldChangeType } from '../../../core/data/object-updates/field-change-type.model'; +import { FieldUpdate } from '../../../core/data/object-updates/field-update.model'; +import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model'; +import { + DeleteRelationship, + RelationshipIdentifiable, +} from '../../../core/data/object-updates/object-updates.reducer'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { RelationshipDataService } from '../../../core/data/relationship-data.service'; +import { Item } from '../../../core/shared/item.model'; +import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { + createFailedRemoteDataObject, + createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$, +} from '../../../shared/remote-data.utils'; +import { EntityTypeDataServiceStub } from '../../../shared/testing/entity-type-data.service.stub'; +import { ItemDataServiceStub } from '../../../shared/testing/item-data.service.stub'; +import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; +import { ObjectUpdatesServiceStub } from '../../../shared/testing/object-updates.service.stub'; +import { RelationshipDataServiceStub } from '../../../shared/testing/relationship-data.service.stub'; +import { EditItemRelationshipsService } from './edit-item-relationships.service'; + +describe('EditItemRelationshipsService', () => { + let service: EditItemRelationshipsService; + + let itemService: ItemDataServiceStub; + let objectUpdatesService: ObjectUpdatesServiceStub; + let notificationsService: NotificationsServiceStub; + let relationshipService: RelationshipDataServiceStub; + let entityTypeDataService: EntityTypeDataServiceStub; + + let currentItem: Item; + + let relationshipItem1: Item; + let relationshipIdentifiable1: RelationshipIdentifiable; + let relationship1: Relationship; + + let relationshipItem2: Item; + let relationshipIdentifiable2: RelationshipIdentifiable; + let relationship2: Relationship; + + let orgUnitType: ItemType; + let orgUnitToOrgUnitType: RelationshipType; + + beforeEach(() => { + itemService = new ItemDataServiceStub(); + objectUpdatesService = new ObjectUpdatesServiceStub(); + notificationsService = new NotificationsServiceStub(); + relationshipService = new RelationshipDataServiceStub(); + entityTypeDataService = new EntityTypeDataServiceStub(); + + TestBed.configureTestingModule({ + imports: [ + TranslateModule.forRoot(), + ], + providers: [ + { provide: ItemDataService, useValue: itemService }, + { provide: ObjectUpdatesService, useValue: objectUpdatesService }, + { provide: NotificationsService, useValue: notificationsService }, + { provide: RelationshipDataService, useValue: relationshipService }, + { provide: EntityTypeDataService, useValue: entityTypeDataService }, + ], + }); + service = TestBed.inject(EditItemRelationshipsService); + }); + + beforeEach(() => { + currentItem = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink1', + }, + }, + }); + + relationshipItem1 = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink2', + }, + }, + }); + relationshipIdentifiable1 = { + originalItem: currentItem, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable; + relationship1 = Object.assign(new Relationship(), { + _links: { + leftItem: currentItem._links.self, + rightItem: relationshipItem1._links.self, + }, + }); + + relationshipItem2 = Object.assign(new Item(), { + uuid: uuidv4(), + metadata: { + 'dspace.entity.type': 'OrgUnit', + }, + _links: { + self: { + href: 'selfLink3', + }, + }, + }); + relationshipIdentifiable2 = { + originalItem: currentItem, + relatedItem: relationshipItem2, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem2.uuid}`, + } as RelationshipIdentifiable; + relationship2 = Object.assign(new Relationship(), { + _links: { + leftItem: currentItem._links.self, + rightItem: relationshipItem2._links.self, + }, + }); + + orgUnitType = Object.assign(new ItemType(), { + id: '2', + label: 'OrgUnit', + }); + orgUnitToOrgUnitType = Object.assign(new RelationshipType(), { + id: '1', + leftMaxCardinality: null, + leftMinCardinality: 0, + leftType: createSuccessfulRemoteDataObject$(orgUnitType), + leftwardType: 'isOrgUnitOfOrgUnit', + rightMaxCardinality: null, + rightMinCardinality: 0, + rightType: createSuccessfulRemoteDataObject$(orgUnitType), + rightwardType: 'isOrgUnitOfOrgUnit', + uuid: 'relationshiptype-1', + }); + }); + + describe('submit', () => { + let fieldUpdateAddRelationship1: FieldUpdate; + let fieldUpdateRemoveRelationship2: FieldUpdate; + + beforeEach(() => { + fieldUpdateAddRelationship1 = { + changeType: FieldChangeType.ADD, + field: relationshipIdentifiable1, + }; + fieldUpdateRemoveRelationship2 = { + changeType: FieldChangeType.REMOVE, + field: relationshipIdentifiable2, + }; + + spyOn(service, 'addRelationship').withArgs(relationshipIdentifiable1).and.returnValue(createSuccessfulRemoteDataObject$(relationship1)); + spyOn(service, 'deleteRelationship').withArgs(relationshipIdentifiable2 as DeleteRelationship).and.returnValue(createSuccessfulRemoteDataObject$({})); + spyOn(itemService, 'invalidateByHref').and.callThrough(); + }); + + it('should support performing multiple relationships manipulations in one submit() call', () => { + spyOn(objectUpdatesService, 'getFieldUpdates').and.returnValue(observableOf({ + [`1-${relationshipItem1.uuid}`]: fieldUpdateAddRelationship1, + [`1-${relationshipItem2.uuid}`]: fieldUpdateRemoveRelationship2, + } as FieldUpdates)); + service.submit(currentItem, `/entities/orgunit/${currentItem.uuid}/edit/relationships`); + + expect(service.addRelationship).toHaveBeenCalledWith(relationshipIdentifiable1); + expect(service.deleteRelationship).toHaveBeenCalledWith(relationshipIdentifiable2 as DeleteRelationship); + + expect(itemService.invalidateByHref).toHaveBeenCalledWith(currentItem.self); + expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem1.self); + expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem2.self); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + }); + }); + + describe('deleteRelationship', () => { + beforeEach(() => { + spyOn(relationshipService, 'deleteRelationship').and.callThrough(); + }); + + it('should pass "all" as copyVirtualMetadata when the user want to keep the data on both sides', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: true, + keepRightVirtualMetadata: true, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'all', false); + }); + + it('should pass "left" as copyVirtualMetadata when the user only want to keep the data on the left side', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: true, + keepRightVirtualMetadata: false, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'left', false); + }); + + it('should pass "right" as copyVirtualMetadata when the user only want to keep the data on the right side', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: false, + keepRightVirtualMetadata: true, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'right', false); + }); + + it('should pass "none" as copyVirtualMetadata when the user doesn\'t want to keep the virtual metadata', () => { + service.deleteRelationship({ + uuid: relationshipItem1.uuid, + keepLeftVirtualMetadata: false, + keepRightVirtualMetadata: false, + } as DeleteRelationship); + + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationshipItem1.uuid, 'none', false); + }); + }); + + describe('addRelationship', () => { + beforeEach(() => { + spyOn(relationshipService, 'addRelationship').and.callThrough(); + }); + + it('should call the addRelationship from relationshipService correctly when original item is on the right', () => { + service.addRelationship({ + originalItem: currentItem, + originalIsLeft: false, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable); + expect(relationshipService.addRelationship).toHaveBeenCalledWith(orgUnitToOrgUnitType.id, relationshipItem1, currentItem, undefined, null, false); + }); + + it('should call the addRelationship from relationshipService correctly when original item is on the left', () => { + service.addRelationship({ + originalItem: currentItem, + originalIsLeft: true, + relatedItem: relationshipItem1, + type: orgUnitToOrgUnitType, + uuid: `1-${relationshipItem1.uuid}`, + } as RelationshipIdentifiable); + + expect(relationshipService.addRelationship).toHaveBeenCalledWith(orgUnitToOrgUnitType.id, currentItem, relationshipItem1, null, undefined, false); + }); + }); + + describe('isProvidedItemTypeLeftType', () => { + it('should return true if the provided item corresponds to the left type of the relationship', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }), + }); + const itemType = Object.assign(new ItemType(), { id: 'leftType' } ); + const item = Object.assign(new Item(), { uuid: 'item-uuid' }); + + const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item); + result.subscribe((resultValue) => { + expect(resultValue).toBeTrue(); + done(); + }); + }); + + it('should return false if the provided item corresponds to the right type of the relationship', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }), + }); + const itemType = Object.assign(new ItemType(), { id: 'rightType' } ); + const item = Object.assign(new Item(), { uuid: 'item-uuid' }); + + const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item); + result.subscribe((resultValue) => { + expect(resultValue).toBeFalse(); + done(); + }); + }); + + it('should return undefined if the provided item corresponds does not match any of the relationship types', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }), + }); + const itemType = Object.assign(new ItemType(), { id: 'something-else' } ); + const item = Object.assign(new Item(), { uuid: 'item-uuid' }); + + const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item); + result.subscribe((resultValue) => { + expect(resultValue).toBeUndefined(); + done(); + }); + }); + }); + + describe('relationshipMatchesBothSameTypes', () => { + it('should return true if both left and right type of the relationship type are the same and match the provided itemtype', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }), + rightType: createSuccessfulRemoteDataObject$({ id:'sameType' }), + leftwardType: 'isDepartmentOfDivision', + rightwardType: 'isDivisionOfDepartment', + }); + const itemType = Object.assign(new ItemType(), { id: 'sameType' } ); + + const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType); + result.subscribe((resultValue) => { + expect(resultValue).toBeTrue(); + done(); + }); + }); + it('should return false if both left and right type of the relationship type are the same and match the provided itemtype but the leftwardType & rightwardType is identical', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'sameType' }), + leftwardType: 'isOrgUnitOfOrgUnit', + rightwardType: 'isOrgUnitOfOrgUnit', + }); + const itemType = Object.assign(new ItemType(), { id: 'sameType' }); + + const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType); + result.subscribe((resultValue) => { + expect(resultValue).toBeFalse(); + done(); + }); + }); + it('should return false if both left and right type of the relationship type are the same and do not match the provided itemtype', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'sameType' }), + leftwardType: 'isDepartmentOfDivision', + rightwardType: 'isDivisionOfDepartment', + }); + const itemType = Object.assign(new ItemType(), { id: 'something-else' } ); + + const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType); + result.subscribe((resultValue) => { + expect(resultValue).toBeFalse(); + done(); + }); + }); + it('should return false if both left and right type of the relationship type are different', (done) => { + const relationshipType = Object.assign(new RelationshipType(), { + leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }), + rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }), + leftwardType: 'isAuthorOfPublication', + rightwardType: 'isPublicationOfAuthor', + }); + const itemType = Object.assign(new ItemType(), { id: 'leftType' } ); + + const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType); + result.subscribe((resultValue) => { + expect(resultValue).toBeFalse(); + done(); + }); + }); + }); + + describe('displayNotifications', () => { + it('should show one success notification when multiple requests succeeded', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createSuccessfulRemoteDataObject({}), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + }); + + it('should show one success notification even when some requests failed', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed'), + createSuccessfulRemoteDataObject({}), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + expect(notificationsService.error).toHaveBeenCalledTimes(1); + }); + + it('should show a separate error notification for each failed request', () => { + service.displayNotifications([ + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed 1'), + createSuccessfulRemoteDataObject({}), + createFailedRemoteDataObject('Request Failed 2'), + ]); + + expect(notificationsService.success).toHaveBeenCalledTimes(1); + expect(notificationsService.error).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts new file mode 100644 index 0000000000..e66562fe19 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts @@ -0,0 +1,267 @@ +import { Injectable } from '@angular/core'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { TranslateService } from '@ngx-translate/core'; +import { + BehaviorSubject, + combineLatest as observableCombineLatest, + EMPTY, + Observable, + Subscription, +} from 'rxjs'; +import { + concatMap, + map, + switchMap, + take, + toArray, +} from 'rxjs/operators'; + +import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { FieldChangeType } from '../../../core/data/object-updates/field-change-type.model'; +import { FieldUpdate } from '../../../core/data/object-updates/field-update.model'; +import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model'; +import { + DeleteRelationship, + RelationshipIdentifiable, +} from '../../../core/data/object-updates/object-updates.reducer'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { RelationshipDataService } from '../../../core/data/relationship-data.service'; +import { RemoteData } from '../../../core/data/remote-data'; +import { Item } from '../../../core/shared/item.model'; +import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { NoContent } from '../../../core/shared/NoContent.model'; +import { + getFirstSucceededRemoteData, + getRemoteDataPayload, +} from '../../../core/shared/operators'; +import { hasValue } from '../../../shared/empty.util'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; + +@Injectable({ + providedIn: 'root', +}) +export class EditItemRelationshipsService { + public notificationsPrefix = 'item.edit.relationships.notifications.'; + + public isSaving$: BehaviorSubject = new BehaviorSubject(false); + + constructor( + public itemService: ItemDataService, + public objectUpdatesService: ObjectUpdatesService, + public notificationsService: NotificationsService, + protected modalService: NgbModal, + public relationshipService: RelationshipDataService, + public entityTypeService: EntityTypeDataService, + public translateService: TranslateService, + ) { } + + + /** + * Resolve the currently selected related items back to relationships and send a delete request for each of the relationships found + * Make sure the lists are refreshed afterwards and notifications are sent for success and errors + */ + public submit(item: Item, url: string): void { + this.isSaving$.next(true); + this.objectUpdatesService.getFieldUpdates(url, [], true).pipe( + map((fieldUpdates: FieldUpdates) => + Object.values(fieldUpdates) + .filter((fieldUpdate: FieldUpdate) => hasValue(fieldUpdate)) + .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD || fieldUpdate.changeType === FieldChangeType.REMOVE), + ), + take(1), + // emit each update in the array separately + switchMap((updates) => updates), + // process each update one by one, while waiting for the previous to finish + concatMap((update: FieldUpdate) => { + if (update.changeType === FieldChangeType.REMOVE) { + return this.deleteRelationship(update.field as DeleteRelationship).pipe( + take(1), + switchMap((deleteRD: RemoteData) => { + if (deleteRD.hasSucceeded) { + return this.itemService.invalidateByHref((update.field as DeleteRelationship).relatedItem._links.self.href).pipe( + map(() => deleteRD), + ); + } + return [deleteRD]; + }), + ); + } else if (update.changeType === FieldChangeType.ADD) { + return this.addRelationship(update.field as RelationshipIdentifiable).pipe( + take(1), + switchMap((relationshipRD: RemoteData) => { + if (relationshipRD.hasSucceeded) { + // Set the newly related item to stale, so its relationships will update to include + // the new one. Only set the current item to stale at the very end so we only do it + // once + const { leftItem, rightItem } = relationshipRD.payload._links; + if (leftItem.href === item.self) { + return this.itemService.invalidateByHref(rightItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } else { + return this.itemService.invalidateByHref(leftItem.href).pipe( + // when it's invalidated, emit the original relationshipRD for use in the pipe below + map(() => relationshipRD), + ); + } + } else { + return [relationshipRD]; + } + }), + ); + } else { + return EMPTY; + } + }), + toArray(), + switchMap((responses) => { + // once all relationships are made and all related items have been invalidated, invalidate + // the current item + return this.itemService.invalidateByHref(item.self).pipe( + map(() => responses), + ); + }), + ).subscribe((responses) => { + if (responses.length > 0) { + this.initializeOriginalFields(item, url); + this.displayNotifications(responses); + this.modalService.dismissAll(); + this.isSaving$.next(false); + } + }); + } + + /** + * Sends all initial values of this item to the object updates service + */ + public initializeOriginalFields(item: Item, url: string): Subscription { + return this.relationshipService.getRelatedItems(item).pipe( + take(1), + ).subscribe((items: Item[]) => { + this.objectUpdatesService.initialize(url, items, item.lastModified); + }); + } + + deleteRelationship(deleteRelationship: DeleteRelationship): Observable> { + let copyVirtualMetadata: string; + if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'all'; + } else if (deleteRelationship.keepLeftVirtualMetadata) { + copyVirtualMetadata = 'left'; + } else if (deleteRelationship.keepRightVirtualMetadata) { + copyVirtualMetadata = 'right'; + } else { + copyVirtualMetadata = 'none'; + } + + return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata, false); + } + + addRelationship(addRelationship: RelationshipIdentifiable): Observable> { + let leftItem: Item; + let rightItem: Item; + let leftwardValue: string; + let rightwardValue: string; + if (addRelationship.originalIsLeft) { + leftItem = addRelationship.originalItem; + rightItem = addRelationship.relatedItem; + leftwardValue = null; + rightwardValue = addRelationship.nameVariant; + } else { + leftItem = addRelationship.relatedItem; + rightItem = addRelationship.originalItem; + leftwardValue = addRelationship.nameVariant; + rightwardValue = null; + } + return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue, false); + } + + /** + * Display notifications + * - Error notification for each failed response with their message + * - Success notification in case there's at least one successful response + * @param responses + */ + displayNotifications(responses: RemoteData[]): void { + const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); + const successfulResponses = responses.filter((response: RemoteData) => response.hasSucceeded); + + failedResponses.forEach((response: RemoteData) => { + this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); + }); + if (successfulResponses.length > 0) { + this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); + } + } + + isProvidedItemTypeLeftType(relationshipType: RelationshipType, itemType: ItemType, item: Item): Observable { + return this.getRelationshipLeftAndRightType(relationshipType).pipe( + map(([leftType, rightType]: [ItemType, ItemType]) => { + if (leftType.id === itemType.id) { + return true; + } + + if (rightType.id === itemType.id) { + return false; + } + + // should never happen... + console.warn(`The item ${item.uuid} is not on the right or the left side of relationship type ${relationshipType.uuid}`); + return undefined; + }), + ); + } + + /** + * Whether both side of the relationship need to be displayed on the edit relationship page or not. + * + * @param relationshipType The relationship type + * @param itemType The item type + */ + shouldDisplayBothRelationshipSides(relationshipType: RelationshipType, itemType: ItemType): Observable { + return this.getRelationshipLeftAndRightType(relationshipType).pipe( + map(([leftType, rightType]: [ItemType, ItemType]) => { + return leftType.id === itemType.id && rightType.id === itemType.id && relationshipType.leftwardType !== relationshipType.rightwardType; + }), + ); + } + + protected getRelationshipLeftAndRightType(relationshipType: RelationshipType): Observable<[ItemType, ItemType]> { + const leftType$: Observable = relationshipType.leftType.pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + ); + + const rightType$: Observable = relationshipType.rightType.pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + ); + + return observableCombineLatest([ + leftType$, + rightType$, + ]); + } + + + + /** + * Get translated notification title + * @param key + */ + getNotificationTitle(key: string): string { + return this.translateService.instant(this.notificationsPrefix + key + '.title'); + } + + /** + * Get translated notification content + * @param key + */ + getNotificationContent(key: string): string { + return this.translateService.instant(this.notificationsPrefix + key + '.content'); + } +} diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.html b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.html new file mode 100644 index 0000000000..ed7fb190f2 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.html @@ -0,0 +1,31 @@ + + + + + + + + + diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.scss b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.spec.ts new file mode 100644 index 0000000000..077f609633 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.spec.ts @@ -0,0 +1,122 @@ +import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { + ComponentFixture, + TestBed, + waitForAsync, +} from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { cold } from 'jasmine-marbles'; +import { of as observableOf } from 'rxjs'; + +import { Item } from '../../../../core/shared/item.model'; +import { ItemType } from '../../../../core/shared/item-relationships/item-type.model'; +import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; +import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { EditItemRelationshipsService } from '../edit-item-relationships.service'; +import { EditRelationshipListComponent } from '../edit-relationship-list/edit-relationship-list.component'; +import { EditRelationshipListWrapperComponent } from './edit-relationship-list-wrapper.component'; + +describe('EditRelationshipListWrapperComponent', () => { + let editItemRelationshipsService: EditItemRelationshipsService; + let comp: EditRelationshipListWrapperComponent; + let fixture: ComponentFixture; + + const leftType = Object.assign(new ItemType(), { id: 'leftType', label: 'leftTypeString' }); + const rightType = Object.assign(new ItemType(), { id: 'rightType', label: 'rightTypeString' }); + + const relationshipType = Object.assign(new RelationshipType(), { + id: '1', + leftMaxCardinality: null, + leftMinCardinality: 0, + leftType: createSuccessfulRemoteDataObject$(leftType), + leftwardType: 'isOrgUnitOfOrgUnit', + rightMaxCardinality: null, + rightMinCardinality: 0, + rightType: createSuccessfulRemoteDataObject$(rightType), + rightwardType: 'isOrgUnitOfOrgUnit', + uuid: 'relationshiptype-1', + }); + + const item = Object.assign(new Item(), { uuid: 'item-uuid' }); + + beforeEach(waitForAsync(() => { + + editItemRelationshipsService = jasmine.createSpyObj('editItemRelationshipsService', { + isProvidedItemTypeLeftType: observableOf(true), + shouldDisplayBothRelationshipSides: observableOf(false), + }); + + + TestBed.configureTestingModule({ + imports: [ + EditRelationshipListWrapperComponent, + ], + providers: [ + { provide: EditItemRelationshipsService, useValue: editItemRelationshipsService }, + ], + schemas: [ + CUSTOM_ELEMENTS_SCHEMA, + ], + }).overrideComponent(EditRelationshipListWrapperComponent, { + remove: { + imports: [ + EditRelationshipListComponent, + ], + }, + }).compileComponents(); + + })); + + beforeEach(() => { + fixture = TestBed.createComponent(EditRelationshipListWrapperComponent); + comp = fixture.componentInstance; + comp.relationshipType = relationshipType; + comp.itemType = leftType; + comp.item = item; + + fixture.detectChanges(); + }); + + describe('onInit', () => { + it('should render the component', () => { + expect(comp).toBeTruthy(); + }); + it('should set currentItemIsLeftItem$ and bothItemsMatchType$ based on the provided relationshipType, itemType and item', () => { + expect(editItemRelationshipsService.isProvidedItemTypeLeftType).toHaveBeenCalledWith(relationshipType, leftType, item); + expect(editItemRelationshipsService.shouldDisplayBothRelationshipSides).toHaveBeenCalledWith(relationshipType, leftType); + + expect(comp.currentItemIsLeftItem$.getValue()).toEqual(true); + expect(comp.shouldDisplayBothRelationshipSides$).toBeObservable(cold('(a|)', { a: false })); + }); + }); + + describe('when the current item is left', () => { + it('should render one relationship list section', () => { + const relationshipLists = fixture.debugElement.queryAll(By.css('ds-edit-relationship-list')); + expect(relationshipLists.length).toEqual(1); + }); + }); + + describe('when the current item is right', () => { + it('should render one relationship list section', () => { + (editItemRelationshipsService.isProvidedItemTypeLeftType as jasmine.Spy).and.returnValue(observableOf(false)); + comp.ngOnInit(); + fixture.detectChanges(); + + const relationshipLists = fixture.debugElement.queryAll(By.css('ds-edit-relationship-list')); + expect(relationshipLists.length).toEqual(1); + }); + }); + + describe('when the current item is both left and right', () => { + it('should render two relationship list sections', () => { + (editItemRelationshipsService.shouldDisplayBothRelationshipSides as jasmine.Spy).and.returnValue(observableOf(true)); + comp.ngOnInit(); + fixture.detectChanges(); + + const relationshipLists = fixture.debugElement.queryAll(By.css('ds-edit-relationship-list')); + expect(relationshipLists.length).toEqual(2); + }); + }); + +}); diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.ts b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.ts new file mode 100644 index 0000000000..943a147546 --- /dev/null +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.ts @@ -0,0 +1,111 @@ +import { + AsyncPipe, + NgIf, +} from '@angular/common'; +import { + Component, + EventEmitter, + Input, + OnDestroy, + OnInit, + Output, +} from '@angular/core'; +import { + BehaviorSubject, + Observable, + Subscription, +} from 'rxjs'; + +import { Item } from '../../../../core/shared/item.model'; +import { ItemType } from '../../../../core/shared/item-relationships/item-type.model'; +import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; +import { hasValue } from '../../../../shared/empty.util'; +import { EditItemRelationshipsService } from '../edit-item-relationships.service'; +import { EditRelationshipListComponent } from '../edit-relationship-list/edit-relationship-list.component'; + +@Component({ + selector: 'ds-edit-relationship-list-wrapper', + styleUrls: ['./edit-relationship-list-wrapper.component.scss'], + templateUrl: './edit-relationship-list-wrapper.component.html', + standalone: true, + imports: [ + AsyncPipe, + EditRelationshipListComponent, + NgIf, + ], +}) +/** + * A component creating a list of editable relationships of a certain type + * The relationships are rendered as a list of related items + */ +export class EditRelationshipListWrapperComponent implements OnInit, OnDestroy { + + /** + * The item to display related items for + */ + @Input() item: Item; + + @Input() itemType: ItemType; + + /** + * The URL to the current page + * Used to fetch updates for the current item from the store + */ + @Input() url: string; + + /** + * The label of the relationship-type we're rendering a list for + */ + @Input() relationshipType: RelationshipType; + + /** + * If updated information has changed + */ + @Input() hasChanges!: Observable; + + /** + * The event emmiter to submit the new information + */ + @Output() submitModal: EventEmitter = new EventEmitter(); + + /** + * Observable that emits true if {@link itemType} is on the left-hand side of {@link relationshipType}, + * false if it is on the right-hand side and undefined in the rare case that it is on neither side. + */ + currentItemIsLeftItem$: BehaviorSubject = new BehaviorSubject(undefined); + + + isLeftItem$ = new BehaviorSubject(true); + + isRightItem$ = new BehaviorSubject(false); + + shouldDisplayBothRelationshipSides$: Observable; + + /** + * Array to track all subscriptions and unsubscribe them onDestroy + * @type {Array} + */ + private subs: Subscription[] = []; + + constructor( + protected editItemRelationshipsService: EditItemRelationshipsService, + ) { + } + + + ngOnInit(): void { + this.subs.push(this.editItemRelationshipsService.isProvidedItemTypeLeftType(this.relationshipType, this.itemType, this.item) + .subscribe((nextValue: boolean) => { + this.currentItemIsLeftItem$.next(nextValue); + })); + + this.shouldDisplayBothRelationshipSides$ = this.editItemRelationshipsService.shouldDisplayBothRelationshipSides(this.relationshipType, this.itemType); + } + + + ngOnDestroy(): void { + this.subs + .filter((subscription) => hasValue(subscription)) + .forEach((subscription) => subscription.unsubscribe()); + } +} diff --git a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html index 0a99a5820f..e32938590f 100644 --- a/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html +++ b/src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html @@ -1,5 +1,5 @@

- {{getRelationshipMessageKey$ | async | translate}} + {{relationshipMessageKey$ | async | translate}} - - + +
+ +
+
+
+
- - -
- -
-
- -
-
-
- - - -
+
+
+
+
- -
+ + + + {{ 'item.edit.relationships.no-entity-type' | translate }} + + + + + + + + +
+ + + +
+
diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 106edb08ef..28380b3f27 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -13,12 +13,10 @@ import { Router, } from '@angular/router'; import { TranslateModule } from '@ngx-translate/core'; -import { getTestScheduler } from 'jasmine-marbles'; import { combineLatest as observableCombineLatest, of as observableOf, } from 'rxjs'; -import { TestScheduler } from 'rxjs/testing'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { RestResponse } from '../../../core/cache/response.models'; @@ -33,6 +31,7 @@ import { Item } from '../../../core/shared/item.model'; import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { AlertComponent } from '../../../shared/alert/alert.component'; import { getMockThemeService } from '../../../shared/mocks/theme-service.mock'; import { INotification, @@ -44,6 +43,7 @@ import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$, } from '../../../shared/remote-data.utils'; +import { ItemDataServiceStub } from '../../../shared/testing/item-data.service.stub'; import { relationshipTypes } from '../../../shared/testing/relationship-types.mock'; import { RouterStub } from '../../../shared/testing/router.stub'; import { createPaginatedList } from '../../../shared/testing/utils.test'; @@ -72,12 +72,11 @@ const notificationsService = jasmine.createSpyObj('notificationsService', const router = new RouterStub(); let relationshipTypeService; let routeStub; -let itemService; +let itemService: ItemDataServiceStub; const url = 'http://test-url.com/test-url'; router.url = url; -let scheduler: TestScheduler; let item; let author1; let author2; @@ -157,10 +156,7 @@ describe('ItemRelationshipsComponent', () => { changeType: FieldChangeType.REMOVE, }; - itemService = jasmine.createSpyObj('itemService', { - findByHref: createSuccessfulRemoteDataObject$(item), - findById: createSuccessfulRemoteDataObject$(item), - }); + itemService = new ItemDataServiceStub(); routeStub = { data: observableOf({}), parent: { @@ -228,7 +224,6 @@ describe('ItemRelationshipsComponent', () => { }, ); - scheduler = getTestScheduler(); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), ItemRelationshipsComponent], providers: [ @@ -247,10 +242,18 @@ describe('ItemRelationshipsComponent', () => { ], schemas: [ NO_ERRORS_SCHEMA, ], + }).overrideComponent(ItemRelationshipsComponent, { + remove: { + imports: [ + AlertComponent, + ], + }, }).compileComponents(); })); beforeEach(() => { + spyOn(itemService, 'findByHref').and.returnValue(item); + spyOn(itemService, 'findById').and.returnValue(item); fixture = TestBed.createComponent(ItemRelationshipsComponent); comp = fixture.componentInstance; de = fixture.debugElement; @@ -285,7 +288,7 @@ describe('ItemRelationshipsComponent', () => { }); it('it should delete the correct relationship', () => { - expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid, 'left'); + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid, 'left', false); }); }); diff --git a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts index ea9b571cd6..7b0cc46647 100644 --- a/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -2,6 +2,7 @@ import { AsyncPipe, NgForOf, NgIf, + NgTemplateOutlet, } from '@angular/common'; import { ChangeDetectorRef, @@ -17,63 +18,55 @@ import { TranslateService, } from '@ngx-translate/core'; import { - combineLatest as observableCombineLatest, + BehaviorSubject, Observable, - of as observableOf, - zip as observableZip, } from 'rxjs'; import { + distinctUntilChanged, map, - startWith, - switchMap, - take, } from 'rxjs/operators'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { EntityTypeDataService } from '../../../core/data/entity-type-data.service'; import { ItemDataService } from '../../../core/data/item-data.service'; -import { FieldChangeType } from '../../../core/data/object-updates/field-change-type.model'; -import { FieldUpdate } from '../../../core/data/object-updates/field-update.model'; -import { FieldUpdates } from '../../../core/data/object-updates/field-updates.model'; -import { - DeleteRelationship, - RelationshipIdentifiable, -} from '../../../core/data/object-updates/object-updates.reducer'; import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; import { PaginatedList } from '../../../core/data/paginated-list.model'; import { RelationshipDataService } from '../../../core/data/relationship-data.service'; import { RelationshipTypeDataService } from '../../../core/data/relationship-type-data.service'; -import { RemoteData } from '../../../core/data/remote-data'; import { RequestService } from '../../../core/data/request.service'; -import { Item } from '../../../core/shared/item.model'; import { ItemType } from '../../../core/shared/item-relationships/item-type.model'; -import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; -import { NoContent } from '../../../core/shared/NoContent.model'; import { getFirstSucceededRemoteData, getRemoteDataPayload, } from '../../../core/shared/operators'; -import { hasValue } from '../../../shared/empty.util'; +import { AlertComponent } from '../../../shared/alert/alert.component'; +import { AlertType } from '../../../shared/alert/alert-type'; import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { followLink } from '../../../shared/utils/follow-link-config.model'; import { VarDirective } from '../../../shared/utils/var.directive'; +import { compareArraysUsingIds } from '../../simple/item-types/shared/item-relationships-utils'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; +import { EditItemRelationshipsService } from './edit-item-relationships.service'; import { EditRelationshipListComponent } from './edit-relationship-list/edit-relationship-list.component'; +import { EditRelationshipListWrapperComponent } from './edit-relationship-list-wrapper/edit-relationship-list-wrapper.component'; @Component({ selector: 'ds-item-relationships', styleUrls: ['./item-relationships.component.scss'], templateUrl: './item-relationships.component.html', imports: [ - ThemedLoadingComponent, + AlertComponent, AsyncPipe, - TranslateModule, - NgIf, EditRelationshipListComponent, NgForOf, + NgIf, + NgTemplateOutlet, + ThemedLoadingComponent, + TranslateModule, VarDirective, + EditRelationshipListWrapperComponent, ], standalone: true, }) @@ -91,7 +84,13 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { /** * The item's entity type as an observable */ - entityType$: Observable; + entityType$: BehaviorSubject = new BehaviorSubject(undefined); + + get isSaving$(): BehaviorSubject { + return this.editItemRelationshipsService.isSaving$; + } + + readonly AlertType = AlertType; constructor( public itemService: ItemDataService, @@ -107,6 +106,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { protected relationshipTypeService: RelationshipTypeDataService, public cdr: ChangeDetectorRef, protected modalService: NgbModal, + protected editItemRelationshipsService: EditItemRelationshipsService, ) { super(itemService, objectUpdatesService, router, notificationsService, translateService, route); } @@ -118,18 +118,18 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { const label = this.item.firstMetadataValue('dspace.entity.type'); if (label !== undefined) { - this.relationshipTypes$ = this.relationshipTypeService.searchByEntityType(label, true, true, ...this.getRelationshipTypeFollowLinks()) - .pipe( - map((relationshipTypes: PaginatedList) => relationshipTypes.page), - ); - - this.entityType$ = this.entityTypeService.getEntityTypeByLabel(label).pipe( - getFirstSucceededRemoteData(), - getRemoteDataPayload(), + this.relationshipTypes$ = this.relationshipTypeService.searchByEntityType(label, true, true, ...this.getRelationshipTypeFollowLinks()).pipe( + map((relationshipTypes: PaginatedList) => relationshipTypes.page), + distinctUntilChanged(compareArraysUsingIds()), ); + this.entityTypeService.getEntityTypeByLabel(label).pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + ).subscribe((type) => this.entityType$.next(type)); + } else { - this.entityType$ = observableOf(undefined); + this.entityType$.next(undefined); } } @@ -145,127 +145,24 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { * Make sure the lists are refreshed afterwards and notifications are sent for success and errors */ public submit(): void { - - // Get all the relationships that should be removed - const removedRelationshipIDs$: Observable = this.relationshipService.getItemRelationshipsArray(this.item).pipe( - startWith([]), - map((relationships: Relationship[]) => relationships.map((relationship) => - Object.assign(new Relationship(), relationship, { uuid: relationship.id }), - )), - switchMap((relationships: Relationship[]) => { - return this.objectUpdatesService.getFieldUpdatesExclusive(this.url, relationships) as Observable; - }), - map((fieldUpdates: FieldUpdates) => - Object.values(fieldUpdates) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE) - .map((fieldUpdate: FieldUpdate) => fieldUpdate.field as DeleteRelationship), - ), - ); - - const addRelatedItems$: Observable = this.objectUpdatesService.getFieldUpdates(this.url, []).pipe( - map((fieldUpdates: FieldUpdates) => - Object.values(fieldUpdates) - .filter((fieldUpdate: FieldUpdate) => hasValue(fieldUpdate)) - .filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.ADD) - .map((fieldUpdate: FieldUpdate) => fieldUpdate.field as RelationshipIdentifiable), - ), - ); - - observableCombineLatest( - removedRelationshipIDs$, - addRelatedItems$, - ).pipe( - take(1), - ).subscribe(([removeRelationshipIDs, addRelatedItems]) => { - const actions = [ - this.deleteRelationships(removeRelationshipIDs), - this.addRelationships(addRelatedItems), - ]; - actions.forEach((action) => - action.subscribe((response) => { - if (response.length > 0) { - this.initializeOriginalFields(); - this.cdr.detectChanges(); - this.displayNotifications(response); - this.modalService.dismissAll(); - } - }), - ); - }); + this.editItemRelationshipsService.submit(this.item, this.url); } - deleteRelationships(deleteRelationshipIDs: DeleteRelationship[]): Observable[]> { - return observableZip(...deleteRelationshipIDs.map((deleteRelationship) => { - let copyVirtualMetadata: string; - if (deleteRelationship.keepLeftVirtualMetadata && deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'all'; - } else if (deleteRelationship.keepLeftVirtualMetadata) { - copyVirtualMetadata = 'left'; - } else if (deleteRelationship.keepRightVirtualMetadata) { - copyVirtualMetadata = 'right'; - } else { - copyVirtualMetadata = 'none'; - } - return this.relationshipService.deleteRelationship(deleteRelationship.uuid, copyVirtualMetadata); - }, - )); - } - - addRelationships(addRelatedItems: RelationshipIdentifiable[]): Observable[]> { - return observableZip(...addRelatedItems.map((addRelationship) => - this.entityType$.pipe( - switchMap((entityType) => this.entityTypeService.isLeftType(addRelationship.type, entityType)), - switchMap((isLeftType) => { - let leftItem: Item; - let rightItem: Item; - let leftwardValue: string; - let rightwardValue: string; - if (isLeftType) { - leftItem = this.item; - rightItem = addRelationship.relatedItem; - leftwardValue = null; - rightwardValue = addRelationship.nameVariant; - } else { - leftItem = addRelationship.relatedItem; - rightItem = this.item; - leftwardValue = addRelationship.nameVariant; - rightwardValue = null; - } - return this.relationshipService.addRelationship(addRelationship.type.id, leftItem, rightItem, leftwardValue, rightwardValue); - }), - ), - )); - } - - /** - * Display notifications - * - Error notification for each failed response with their message - * - Success notification in case there's at least one successful response - * @param responses - */ - displayNotifications(responses: RemoteData[]) { - const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); - const successfulResponses = responses.filter((response: RemoteData) => response.hasSucceeded); - - failedResponses.forEach((response: RemoteData) => { - this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); - }); - if (successfulResponses.length > 0) { - this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); - } - } /** * Sends all initial values of this item to the object updates service */ public initializeOriginalFields() { - return this.relationshipService.getRelatedItems(this.item).pipe( - take(1), - ).subscribe((items: Item[]) => { - this.objectUpdatesService.initialize(this.url, items, this.item.lastModified); - }); + return this.editItemRelationshipsService.initializeOriginalFields(this.item, this.url); } + /** + * Method to prevent unnecessary for loop re-rendering + */ + trackById(index: number, relationshipType: RelationshipType): string { + return relationshipType.id; + } + getRelationshipTypeFollowLinks() { return [ followLink('leftType'), diff --git a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts index 45759aa90b..a722d07c9d 100644 --- a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts +++ b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts @@ -37,6 +37,7 @@ describe('ModifyItemOverviewComponent', () => { fixture = TestBed.createComponent(ModifyItemOverviewComponent); comp = fixture.componentInstance; comp.item = mockItem; + comp.ngOnChanges(); fixture.detectChanges(); }); diff --git a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts index 2d4b622ea9..3bd5024837 100644 --- a/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts +++ b/src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.ts @@ -5,7 +5,7 @@ import { import { Component, Input, - OnInit, + OnChanges, } from '@angular/core'; import { TranslateModule } from '@ngx-translate/core'; @@ -21,12 +21,12 @@ import { MetadataMap } from '../../../core/shared/metadata.models'; /** * Component responsible for rendering a table containing the metadatavalues from the to be edited item */ -export class ModifyItemOverviewComponent implements OnInit { +export class ModifyItemOverviewComponent implements OnChanges { @Input() item: Item; metadata: MetadataMap; - ngOnInit(): void { - this.metadata = this.item.metadata; + ngOnChanges(): void { + this.metadata = this.item?.metadata; } } diff --git a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html index b83b93d8f1..927f7dbb48 100644 --- a/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html +++ b/src/app/item-page/edit-item-page/virtual-metadata/virtual-metadata.component.html @@ -5,9 +5,9 @@