From 44f393d65a54db0ec8c1b4b6663267da783b1f87 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Thu, 28 Apr 2022 11:44:53 +0200 Subject: [PATCH 01/13] [CST-5674] Fixed link in TODO --- .../resource-policies/form/resource-policy-form.component.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 7ae699340e..0a7f0b7a90 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -193,11 +193,11 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ private buildResourcePolicyForm(): DynamicFormControlModel[] { const formModel: DynamicFormControlModel[] = []; - // TODO to be removed when https://jira.lyrasis.org/browse/DS-4477 will be implemented + // TODO to be removed when https://github.com/DSpace/DSpace/issues/7812 will be implemented const policyTypeConf = Object.assign({}, RESOURCE_POLICY_FORM_POLICY_TYPE_CONFIG, { disabled: isNotEmpty(this.resourcePolicy) }); - // TODO to be removed when https://jira.lyrasis.org/browse/DS-4477 will be implemented + // TODO to be removed when https://github.com/DSpace/DSpace/issues/7812 will be implemented const actionConf = Object.assign({}, RESOURCE_POLICY_FORM_ACTION_TYPE_CONFIG, { disabled: isNotEmpty(this.resourcePolicy) }); From e94454be97d7a84502295cf9bfa586aba5b970e0 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 29 Apr 2022 12:11:06 +0200 Subject: [PATCH 02/13] [CST-5674] Update selected ePerson/Group --- .../resource-policies/form/resource-policy-form.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 0a7f0b7a90..07385b46c0 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -274,6 +274,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { updateObjectSelected(object: DSpaceObject, isEPerson: boolean): void { this.resourcePolicyGrant = object; this.resourcePolicyGrantType = isEPerson ? 'eperson' : 'group'; + this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); } /** From 342a62081ccb9f56d1cc519af861b32355fa279d Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 29 Apr 2022 12:17:48 +0200 Subject: [PATCH 03/13] [CST-5674] Enable ePerson/Group editing; prevent switching between ePerson and Group --- .../form/resource-policy-form.component.html | 10 ++-- .../resource-policy-form.component.spec.ts | 4 +- .../form/resource-policy-form.component.ts | 60 +++++++++++-------- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.html b/src/app/shared/resource-policies/form/resource-policy-form.component.html index 42475a955b..51fbd6b526 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.html +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.html @@ -7,16 +7,16 @@ [displayCancel]="false">
- - -
    -
  • + + +
      +
    • {{'resource-policies.form.eperson-group-list.tab.eperson' | translate}}
      • -
    • +
    • {{'resource-policies.form.eperson-group-list.tab.group' | translate}} { }); it('should can set grant', () => { - expect(comp.canSetGrant()).toBeTruthy(); + expect(comp.isBeingEdited()).toBeTruthy(); }); it('should not have a target name', () => { @@ -344,7 +344,7 @@ describe('ResourcePolicyFormComponent test suite', () => { }); it('should not can set grant', () => { - expect(comp.canSetGrant()).toBeFalsy(); + expect(comp.isBeingEdited()).toBeFalsy(); }); it('should have a target name', () => { diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 07385b46c0..16cebfa899 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -41,6 +41,7 @@ import { EPersonDataService } from '../../../core/eperson/eperson-data.service'; import { GroupDataService } from '../../../core/eperson/group-data.service'; import { getFirstSucceededRemoteData } from '../../../core/shared/operators'; import { RequestService } from '../../../core/data/request.service'; +import { NgbNavChangeEvent } from '@ng-bootstrap/ng-bootstrap'; export interface ResourcePolicyEvent { object: ResourcePolicy; @@ -125,6 +126,8 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ private subs: Subscription[] = []; + navActiveId: string; + /** * Initialize instance variables * @@ -151,28 +154,27 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { this.formId = this.formService.getUniqueId('resource-policy-form'); this.formModel = this.buildResourcePolicyForm(); - if (!this.canSetGrant()) { - const epersonRD$ = this.ePersonService.findByHref(this.resourcePolicy._links.eperson.href, false).pipe( - getFirstSucceededRemoteData() - ); - const groupRD$ = this.groupService.findByHref(this.resourcePolicy._links.group.href, false).pipe( - getFirstSucceededRemoteData() - ); - const dsoRD$: Observable> = observableCombineLatest([epersonRD$, groupRD$]).pipe( - map((rdArr: RemoteData[]) => { - return rdArr.find((rd: RemoteData) => isNotEmpty(rd.payload)); - }), - hasValueOperator(), - ); - this.subs.push( - dsoRD$.pipe( - filter(() => this.isActive), - ).subscribe((dsoRD: RemoteData) => { - this.resourcePolicyGrant = dsoRD.payload; - this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); - }) - ); - } + const epersonRD$ = this.ePersonService.findByHref(this.resourcePolicy._links.eperson.href, false).pipe( + getFirstSucceededRemoteData() + ); + const groupRD$ = this.groupService.findByHref(this.resourcePolicy._links.group.href, false).pipe( + getFirstSucceededRemoteData() + ); + const dsoRD$: Observable> = observableCombineLatest([epersonRD$, groupRD$]).pipe( + map((rdArr: RemoteData[]) => { + return rdArr.find((rd: RemoteData) => isNotEmpty(rd.payload)); + }), + hasValueOperator(), + ); + this.subs.push( + dsoRD$.pipe( + filter(() => this.isActive), + ).subscribe((dsoRD: RemoteData) => { + this.resourcePolicyGrant = dsoRD.payload; + this.navActiveId = String(dsoRD.payload.type); + this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); + }) + ); } /** @@ -255,8 +257,8 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { * * @return true if is possible, false otherwise */ - canSetGrant(): boolean { - return isEmpty(this.resourcePolicy); + isBeingEdited(): boolean { + return !isEmpty(this.resourcePolicy); } /** @@ -330,4 +332,14 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { .filter((subscription) => hasValue(subscription)) .forEach((subscription) => subscription.unsubscribe()); } + + onNavChange(changeEvent: NgbNavChangeEvent) { + console.log(`CHANGE ${changeEvent.activeId} -> ${changeEvent.nextId}`); + + if (this.isBeingEdited()) { + // if a policy is being edited it should not be possible to switch between group and eperson + changeEvent.preventDefault(); + // TODO add informative modal + } + } } From d5e5a4a550991b136c686b8d4ee84524221fd66a Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 29 Apr 2022 13:02:22 +0200 Subject: [PATCH 04/13] [CST-5674] Informative modal (WIP) --- .../form/resource-policy-form.component.html | 16 ++++++++++++++++ .../form/resource-policy-form.component.ts | 14 ++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.html b/src/app/shared/resource-policies/form/resource-policy-form.component.html index 51fbd6b526..821d15f414 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.html +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.html @@ -51,3 +51,19 @@
+ + + +
+ + +
+
+ TEXT +
+
+ +
+ diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 16cebfa899..ea49433db0 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core'; +import { Component, ElementRef, EventEmitter, Input, OnDestroy, OnInit, Output, ViewChild } from '@angular/core'; import { Observable, @@ -41,7 +41,7 @@ import { EPersonDataService } from '../../../core/eperson/eperson-data.service'; import { GroupDataService } from '../../../core/eperson/group-data.service'; import { getFirstSucceededRemoteData } from '../../../core/shared/operators'; import { RequestService } from '../../../core/data/request.service'; -import { NgbNavChangeEvent } from '@ng-bootstrap/ng-bootstrap'; +import { NgbModal, NgbNavChangeEvent } from '@ng-bootstrap/ng-bootstrap'; export interface ResourcePolicyEvent { object: ResourcePolicy; @@ -84,6 +84,8 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ @Output() submit: EventEmitter = new EventEmitter(); + @ViewChild('content') content: ElementRef; + /** * The form id * @type {string} @@ -136,6 +138,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { * @param {FormService} formService * @param {GroupDataService} groupService * @param {RequestService} requestService + * @param modalService */ constructor( private dsoNameService: DSONameService, @@ -143,6 +146,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { private formService: FormService, private groupService: GroupDataService, private requestService: RequestService, + private modalService: NgbModal, ) { } @@ -334,12 +338,10 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { } onNavChange(changeEvent: NgbNavChangeEvent) { - console.log(`CHANGE ${changeEvent.activeId} -> ${changeEvent.nextId}`); - + // if a policy is being edited it should not be possible to switch between group and eperson if (this.isBeingEdited()) { - // if a policy is being edited it should not be possible to switch between group and eperson changeEvent.preventDefault(); - // TODO add informative modal + this.modalService.open(this.content); } } } From bb3cc1c619dfe0e2f9a8bfb55380f8f5eb0671e9 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Mon, 2 May 2022 17:02:04 +0200 Subject: [PATCH 05/13] [CST-5674] Edit policy target; modal content; test --- .../resource-policy.service.spec.ts | 8 ++- .../resource-policy.service.ts | 55 +++++++++++++++++-- .../edit/resource-policy-edit.component.ts | 33 +++++++---- .../form/resource-policy-form.component.html | 15 ++++- .../resource-policy-form.component.spec.ts | 13 +++-- .../form/resource-policy-form.component.ts | 5 ++ src/assets/i18n/en.json5 | 10 ++++ 7 files changed, 117 insertions(+), 22 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index 59316c0098..ca62159f59 100644 --- a/src/app/core/resource-policy/resource-policy.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy.service.spec.ts @@ -19,6 +19,8 @@ import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils import { RestResponse } from '../cache/response.models'; import { RequestEntry } from '../data/request-entry.model'; import { FindListOptions } from '../data/find-list-options.model'; +import { EPersonDataService } from '../eperson/eperson-data.service'; +import { GroupDataService } from '../eperson/group-data.service'; describe('ResourcePolicyService', () => { let scheduler: TestScheduler; @@ -28,6 +30,8 @@ describe('ResourcePolicyService', () => { let objectCache: ObjectCacheService; let halService: HALEndpointService; let responseCacheEntry: RequestEntry; + let ePersonService: EPersonDataService; + let groupService: GroupDataService; const resourcePolicy: any = { id: '1', @@ -129,7 +133,9 @@ describe('ResourcePolicyService', () => { halService, notificationsService, http, - comparator + comparator, + ePersonService, + groupService ); spyOn((service as any).dataService, 'create').and.callThrough(); diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index 065e58c53d..f4f70a73e2 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -1,6 +1,6 @@ /* eslint-disable max-classes-per-file */ import { Injectable } from '@angular/core'; -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpHeaders } from '@angular/common/http'; import { Store } from '@ngrx/store'; import { Observable } from 'rxjs'; @@ -23,11 +23,19 @@ import { PaginatedList } from '../data/paginated-list.model'; import { ActionType } from './models/action-type.model'; import { RequestParam } from '../cache/models/request-param.model'; import { isNotEmpty } from '../../shared/empty.util'; -import { map } from 'rxjs/operators'; +import { map, switchMap, take } from 'rxjs/operators'; import { NoContent } from '../shared/NoContent.model'; import { getFirstCompletedRemoteData } from '../shared/operators'; import { CoreState } from '../core-state.model'; import { FindListOptions } from '../data/find-list-options.model'; +import { HttpOptions } from '../dspace-rest/dspace-rest.service'; +import { PostRequest } from '../data/request.models'; +import { GenericConstructor } from '../shared/generic-constructor'; +import { ResponseParsingService } from '../data/parsing.service'; +import { StatusCodeOnlyResponseParsingService } from '../data/status-code-only-response-parsing.service'; +import { HALLink } from '../shared/hal-link.model'; +import { EPersonDataService } from '../eperson/eperson-data.service'; +import { GroupDataService } from '../eperson/group-data.service'; /** @@ -44,7 +52,8 @@ class DataServiceImpl extends DataService { protected halService: HALEndpointService, protected notificationsService: NotificationsService, protected http: HttpClient, - protected comparator: ChangeAnalyzer) { + protected comparator: ChangeAnalyzer, + ) { super(); } @@ -68,7 +77,10 @@ export class ResourcePolicyService { protected halService: HALEndpointService, protected notificationsService: NotificationsService, protected http: HttpClient, - protected comparator: DefaultChangeAnalyzer) { + protected comparator: DefaultChangeAnalyzer, + protected ePersonService: EPersonDataService, + protected groupService: GroupDataService, + ) { this.dataService = new DataServiceImpl(requestService, rdbService, null, objectCache, halService, notificationsService, http, comparator); } @@ -221,4 +233,39 @@ export class ResourcePolicyService { return this.dataService.searchBy(this.searchByResourceMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + /** + * Update the target of the resource policy + * @param resourcePolicyHref the link to the resource policy + * @param uuid the UUID of the target to which the permission is being granted + * @param type the type of the target (eperson or group) to which the permission is being granted + */ + updateTarget(resourcePolicyHref: string, uuid: string, type: string): Observable> { + + const targetService = type === 'eperson' ? this.ePersonService : this.groupService; + + const ep$ = targetService.getBrowseEndpoint().pipe( + take(1), + map((endpoint: string) =>`${endpoint}/${uuid}`), + ); + + const options: HttpOptions = Object.create({}); + let headers = new HttpHeaders(); + headers = headers.append('Content-Type', 'text/uri-list'); + options.headers = headers; + + const requestId = this.requestService.generateRequestId(); + + return ep$.pipe(switchMap((ep) => { + const request = new PostRequest(requestId, resourcePolicyHref, ep, options); + Object.assign(request, { + getResponseParser(): GenericConstructor { + return StatusCodeOnlyResponseParsingService; + } + }); + this.requestService.send(request); + return this.rdbService.buildFromRequestUUID(requestId); + })); + + } + } diff --git a/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts b/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts index a515eef675..d2c1b6dd0d 100644 --- a/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts +++ b/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts @@ -1,7 +1,7 @@ import { Component, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, Observable, of, combineLatest as observableCombineLatest, } from 'rxjs'; import { map, take } from 'rxjs/operators'; import { TranslateService } from '@ngx-translate/core'; @@ -88,16 +88,29 @@ export class ResourcePolicyEditComponent implements OnInit { type: RESOURCE_POLICY.value, _links: this.resourcePolicy._links }); - this.resourcePolicyService.update(updatedObject).pipe( + + const updateTargetSucceeded$ = event.updateTarget ? this.resourcePolicyService.updateTarget( + this.resourcePolicy._links.self.href, event.target.uuid, event.target.type + ).pipe( getFirstCompletedRemoteData(), - ).subscribe((responseRD: RemoteData) => { - this.processing$.next(false); - if (responseRD && responseRD.hasSucceeded) { - this.notificationsService.success(null, this.translate.get('resource-policies.edit.page.success.content')); - this.redirectToAuthorizationsPage(); - } else { - this.notificationsService.error(null, this.translate.get('resource-policies.edit.page.failure.content')); + map((responseRD) => responseRD && responseRD.hasSucceeded) + ) : of(true); + + const updateResourcePolicySucceeded$ = this.resourcePolicyService.update(updatedObject).pipe( + getFirstCompletedRemoteData(), + map((responseRD) => responseRD && responseRD.hasSucceeded) + ); + + observableCombineLatest([updateTargetSucceeded$, updateResourcePolicySucceeded$]).subscribe( + ([updateTargetSucceeded, updateResourcePolicySucceeded]) => { + this.processing$.next(false); + if (updateTargetSucceeded && updateResourcePolicySucceeded) { + this.notificationsService.success(null, this.translate.get('resource-policies.edit.page.success.content')); + this.redirectToAuthorizationsPage(); + } else { + this.notificationsService.error(null, this.translate.get('resource-policies.edit.page.failure.content')); + } } - }); + ); } } diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.html b/src/app/shared/resource-policies/form/resource-policy-form.component.html index 821d15f414..f7aad55ce8 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.html +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.html @@ -55,15 +55,24 @@
- +
- TEXT +
+
+ +
+
+

+

+
+
- +
diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts index c31a65c1f6..456eb6db5e 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts @@ -222,6 +222,8 @@ describe('ResourcePolicyFormComponent test suite', () => { testFixture = createTestComponent(html, TestComponent) as ComponentFixture; testComp = testFixture.componentInstance; + testComp.resourcePolicy = resourcePolicy; + fixture.detectChanges(); }); afterEach(() => { @@ -242,6 +244,7 @@ describe('ResourcePolicyFormComponent test suite', () => { fixture = TestBed.createComponent(ResourcePolicyFormComponent); comp = fixture.componentInstance; compAsAny = fixture.componentInstance; + compAsAny.resourcePolicy = resourcePolicy; comp.isProcessing = observableOf(false); }); @@ -261,7 +264,7 @@ describe('ResourcePolicyFormComponent test suite', () => { expect(compAsAny.buildResourcePolicyForm).toHaveBeenCalled(); expect(compAsAny.initModelsValue).toHaveBeenCalled(); expect(compAsAny.formModel.length).toBe(5); - expect(compAsAny.subs.length).toBe(0); + expect(compAsAny.subs.length).toBe(1); }); @@ -279,7 +282,7 @@ describe('ResourcePolicyFormComponent test suite', () => { expect(compAsAny.reset.emit).toHaveBeenCalled(); }); - it('should update resource policy grant object properly', () => { + it('should update resource policy grant object properly', () => { comp.updateObjectSelected(EPersonMock, true); expect(comp.resourcePolicyGrant).toEqual(EPersonMock); @@ -301,6 +304,7 @@ describe('ResourcePolicyFormComponent test suite', () => { comp = fixture.componentInstance; compAsAny = fixture.componentInstance; comp.resourcePolicy = resourcePolicy; + compAsAny.resourcePolicy = resourcePolicy; comp.isProcessing = observableOf(false); compAsAny.ePersonService.findByHref.and.returnValue( observableOf(createSuccessfulRemoteDataObject({})).pipe(delay(100)) @@ -343,8 +347,8 @@ describe('ResourcePolicyFormComponent test suite', () => { }); }); - it('should not can set grant', () => { - expect(comp.isBeingEdited()).toBeFalsy(); + it('should be being edited', () => { + expect(comp.isBeingEdited()).toBeTrue(); }); it('should have a target name', () => { @@ -398,6 +402,7 @@ describe('ResourcePolicyFormComponent test suite', () => { type: 'group', uuid: GroupMock.id }; + eventPayload.updateTarget = false; scheduler = getTestScheduler(); scheduler.schedule(() => comp.onSubmit()); diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index ea49433db0..2783200d8f 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -49,6 +49,7 @@ export interface ResourcePolicyEvent { type: string, uuid: string }; + updateTarget: boolean; } @Component({ @@ -130,6 +131,8 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { navActiveId: string; + resourcePolicyTargetUpdated = false; + /** * Initialize instance variables * @@ -278,6 +281,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { * Update reference to the eperson or group that will be granted the permission */ updateObjectSelected(object: DSpaceObject, isEPerson: boolean): void { + this.resourcePolicyTargetUpdated = true; this.resourcePolicyGrant = object; this.resourcePolicyGrantType = isEPerson ? 'eperson' : 'group'; this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); @@ -304,6 +308,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { type: this.resourcePolicyGrantType, uuid: this.resourcePolicyGrant.id }; + eventPayload.updateTarget = this.resourcePolicyTargetUpdated; this.submit.emit(eventPayload); }); } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index a7ce942e7d..dfc715b72b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3136,6 +3136,16 @@ "resource-policies.form.eperson-group-list.table.headers.name": "Name", + "resource-policies.form.eperson-group-list.modal.header": "Cannot change type", + + "resource-policies.form.eperson-group-list.modal.text1.toGroup": "It is not possible to replace an ePerson with a group.", + + "resource-policies.form.eperson-group-list.modal.text1.toEPerson": "It is not possible to replace a group with an ePerson.", + + "resource-policies.form.eperson-group-list.modal.text2": "Delete the current resource policy and create a new one with the desired type.", + + "resource-policies.form.eperson-group-list.modal.close": "Ok", + "resource-policies.form.date.end.label": "End Date", "resource-policies.form.date.start.label": "Start Date", From 9173b9db6061ba19032fd28b6e7e1053f18e84e6 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Wed, 4 May 2022 11:35:54 +0200 Subject: [PATCH 06/13] [CST-5674] Fix; enable policy and action type editing; test --- .../resource-policy-form.component.spec.ts | 2 - .../form/resource-policy-form.component.ts | 59 +++++++++---------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts index 456eb6db5e..4a2a3376e0 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts @@ -222,8 +222,6 @@ describe('ResourcePolicyFormComponent test suite', () => { testFixture = createTestComponent(html, TestComponent) as ComponentFixture; testComp = testFixture.componentInstance; - testComp.resourcePolicy = resourcePolicy; - fixture.detectChanges(); }); afterEach(() => { diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 2783200d8f..09be58fca4 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -161,27 +161,31 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { this.formId = this.formService.getUniqueId('resource-policy-form'); this.formModel = this.buildResourcePolicyForm(); - const epersonRD$ = this.ePersonService.findByHref(this.resourcePolicy._links.eperson.href, false).pipe( - getFirstSucceededRemoteData() - ); - const groupRD$ = this.groupService.findByHref(this.resourcePolicy._links.group.href, false).pipe( - getFirstSucceededRemoteData() - ); - const dsoRD$: Observable> = observableCombineLatest([epersonRD$, groupRD$]).pipe( - map((rdArr: RemoteData[]) => { - return rdArr.find((rd: RemoteData) => isNotEmpty(rd.payload)); - }), - hasValueOperator(), - ); - this.subs.push( - dsoRD$.pipe( - filter(() => this.isActive), - ).subscribe((dsoRD: RemoteData) => { - this.resourcePolicyGrant = dsoRD.payload; - this.navActiveId = String(dsoRD.payload.type); - this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); - }) - ); + if (this.isBeingEdited()) { + const epersonRD$ = this.ePersonService.findByHref(this.resourcePolicy._links.eperson.href, false).pipe( + getFirstSucceededRemoteData() + ); + const groupRD$ = this.groupService.findByHref(this.resourcePolicy._links.group.href, false).pipe( + getFirstSucceededRemoteData() + ); + const dsoRD$: Observable> = observableCombineLatest([epersonRD$, groupRD$]).pipe( + map((rdArr: RemoteData[]) => { + return rdArr.find((rd: RemoteData) => isNotEmpty(rd.payload)); + }), + hasValueOperator(), + ); + this.subs.push( + dsoRD$.pipe( + filter(() => this.isActive), + ).subscribe((dsoRD: RemoteData) => { + this.resourcePolicyGrant = dsoRD.payload; + this.navActiveId = String(dsoRD.payload.type); + this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); + }) + ) + } else { + + } } /** @@ -202,19 +206,12 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ private buildResourcePolicyForm(): DynamicFormControlModel[] { const formModel: DynamicFormControlModel[] = []; - // TODO to be removed when https://github.com/DSpace/DSpace/issues/7812 will be implemented - const policyTypeConf = Object.assign({}, RESOURCE_POLICY_FORM_POLICY_TYPE_CONFIG, { - disabled: isNotEmpty(this.resourcePolicy) - }); - // TODO to be removed when https://github.com/DSpace/DSpace/issues/7812 will be implemented - const actionConf = Object.assign({}, RESOURCE_POLICY_FORM_ACTION_TYPE_CONFIG, { - disabled: isNotEmpty(this.resourcePolicy) - }); + formModel.push( new DsDynamicInputModel(RESOURCE_POLICY_FORM_NAME_CONFIG), new DsDynamicTextAreaModel(RESOURCE_POLICY_FORM_DESCRIPTION_CONFIG), - new DynamicSelectModel(policyTypeConf), - new DynamicSelectModel(actionConf) + new DynamicSelectModel(RESOURCE_POLICY_FORM_POLICY_TYPE_CONFIG), + new DynamicSelectModel(RESOURCE_POLICY_FORM_ACTION_TYPE_CONFIG) ); const startDateModel = new DynamicDatePickerModel( From 8c937a55c079c0f29be2b5e1e23ba0a4e0765420 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Thu, 5 May 2022 15:33:53 +0200 Subject: [PATCH 07/13] [CST-5674] Error handling; fixes --- .../core/resource-policy/resource-policy.service.ts | 12 ++++++++---- .../edit/resource-policy-edit.component.ts | 8 ++++++-- src/assets/i18n/en.json5 | 4 ++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index f4f70a73e2..ae77ca84d6 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -235,15 +235,16 @@ export class ResourcePolicyService { /** * Update the target of the resource policy + * @param resourcePolicyId the ID of the resource policy * @param resourcePolicyHref the link to the resource policy * @param uuid the UUID of the target to which the permission is being granted * @param type the type of the target (eperson or group) to which the permission is being granted */ - updateTarget(resourcePolicyHref: string, uuid: string, type: string): Observable> { + updateTarget(resourcePolicyId: string, resourcePolicyHref: string, uuid: string, type: string): Observable> { const targetService = type === 'eperson' ? this.ePersonService : this.groupService; - const ep$ = targetService.getBrowseEndpoint().pipe( + const targetEndpoint$ = targetService.getBrowseEndpoint().pipe( take(1), map((endpoint: string) =>`${endpoint}/${uuid}`), ); @@ -255,8 +256,11 @@ export class ResourcePolicyService { const requestId = this.requestService.generateRequestId(); - return ep$.pipe(switchMap((ep) => { - const request = new PostRequest(requestId, resourcePolicyHref, ep, options); + this.requestService.setStaleByHrefSubstring(`${this.dataService.getLinkPath()}/${resourcePolicyId}/${type}`); + + return targetEndpoint$.pipe(switchMap((targetEndpoint) => { + const resourceEndpoint = resourcePolicyHref + '/' + type + const request = new PostRequest(requestId, resourceEndpoint, targetEndpoint, options); Object.assign(request, { getResponseParser(): GenericConstructor { return StatusCodeOnlyResponseParsingService; diff --git a/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts b/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts index d2c1b6dd0d..1c5a0c4a5f 100644 --- a/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts +++ b/src/app/shared/resource-policies/edit/resource-policy-edit.component.ts @@ -90,7 +90,7 @@ export class ResourcePolicyEditComponent implements OnInit { }); const updateTargetSucceeded$ = event.updateTarget ? this.resourcePolicyService.updateTarget( - this.resourcePolicy._links.self.href, event.target.uuid, event.target.type + this.resourcePolicy.id, this.resourcePolicy._links.self.href, event.target.uuid, event.target.type ).pipe( getFirstCompletedRemoteData(), map((responseRD) => responseRD && responseRD.hasSucceeded) @@ -107,7 +107,11 @@ export class ResourcePolicyEditComponent implements OnInit { if (updateTargetSucceeded && updateResourcePolicySucceeded) { this.notificationsService.success(null, this.translate.get('resource-policies.edit.page.success.content')); this.redirectToAuthorizationsPage(); - } else { + } else if (updateResourcePolicySucceeded) { // everything except target has been updated + this.notificationsService.error(null, this.translate.get('resource-policies.edit.page.target-failure.content')); + } else if (updateTargetSucceeded) { // only target has been updated + this.notificationsService.error(null, this.translate.get('resource-policies.edit.page.other-failure.content')); + } else { // nothing has been updated this.notificationsService.error(null, this.translate.get('resource-policies.edit.page.failure.content')); } } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 91d199061d..3fc6ebe0f7 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3122,6 +3122,10 @@ "resource-policies.edit.page.failure.content": "An error occurred while editing the resource policy.", + "resource-policies.edit.page.target-failure.content": "An error occurred while editing the target (ePerson or group) of the resource policy.", + + "resource-policies.edit.page.other-failure.content": "An error occurred while editing the resource policy. The target (ePerson or group) has been successfully updated.", + "resource-policies.edit.page.success.content": "Operation successful", "resource-policies.edit.page.title": "Edit resource policy", From eb2d9b2fde7ed4751d81a18109f3cd092a100f2c Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Thu, 5 May 2022 17:00:49 +0200 Subject: [PATCH 08/13] [CST-5674] Lint fixes --- src/app/core/resource-policy/resource-policy.service.ts | 2 +- .../resource-policies/form/resource-policy-form.component.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index ae77ca84d6..ca3951109a 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -259,7 +259,7 @@ export class ResourcePolicyService { this.requestService.setStaleByHrefSubstring(`${this.dataService.getLinkPath()}/${resourcePolicyId}/${type}`); return targetEndpoint$.pipe(switchMap((targetEndpoint) => { - const resourceEndpoint = resourcePolicyHref + '/' + type + const resourceEndpoint = resourcePolicyHref + '/' + type; const request = new PostRequest(requestId, resourceEndpoint, targetEndpoint, options); Object.assign(request, { getResponseParser(): GenericConstructor { diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.ts index 09be58fca4..223e610908 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts @@ -182,9 +182,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { this.navActiveId = String(dsoRD.payload.type); this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); }) - ) - } else { - + ); } } From 881af6449542ee174206d5b10ec27b87f0254219 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Tue, 17 May 2022 16:36:40 +0200 Subject: [PATCH 09/13] [CST-5674] POST replaced with PUT --- .../resource-policy.service.ts | 4 +-- .../form/resource-policy-form.component.html | 34 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index ca3951109a..8411647bea 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -29,7 +29,7 @@ import { getFirstCompletedRemoteData } from '../shared/operators'; import { CoreState } from '../core-state.model'; import { FindListOptions } from '../data/find-list-options.model'; import { HttpOptions } from '../dspace-rest/dspace-rest.service'; -import { PostRequest } from '../data/request.models'; +import { PutRequest } from '../data/request.models'; import { GenericConstructor } from '../shared/generic-constructor'; import { ResponseParsingService } from '../data/parsing.service'; import { StatusCodeOnlyResponseParsingService } from '../data/status-code-only-response-parsing.service'; @@ -260,7 +260,7 @@ export class ResourcePolicyService { return targetEndpoint$.pipe(switchMap((targetEndpoint) => { const resourceEndpoint = resourcePolicyHref + '/' + type; - const request = new PostRequest(requestId, resourceEndpoint, targetEndpoint, options); + const request = new PutRequest(requestId, resourceEndpoint, targetEndpoint, options); Object.assign(request, { getResponseParser(): GenericConstructor { return StatusCodeOnlyResponseParsingService; diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.html b/src/app/shared/resource-policies/form/resource-policy-form.component.html index f7aad55ce8..66c1fc400e 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.html +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.html @@ -8,24 +8,22 @@
- - -
- + +
From 51bbbb697e42291f2571dcf767df10fdfa36033f Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 20 May 2022 16:45:10 +0200 Subject: [PATCH 10/13] [CST-5674] add test case for updateTarget --- .../resource-policy.service.spec.ts | 29 +++++++++++++++++++ .../resource-policy.service.ts | 25 ++++++++-------- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index ca62159f59..2b99f26679 100644 --- a/src/app/core/resource-policy/resource-policy.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy.service.spec.ts @@ -92,6 +92,8 @@ describe('ResourcePolicyService', () => { const resourcePolicyRD = createSuccessfulRemoteDataObject(resourcePolicy); const paginatedListRD = createSuccessfulRemoteDataObject(paginatedList); + const ePersonEndpoint = 'EPERSON_EP'; + beforeEach(() => { scheduler = getTestScheduler(); @@ -109,6 +111,7 @@ describe('ResourcePolicyService', () => { removeByHrefSubstring: {}, getByHref: observableOf(responseCacheEntry), getByUUID: observableOf(responseCacheEntry), + setStaleByHrefSubstring: {}, }); rdbService = jasmine.createSpyObj('rdbService', { buildSingle: hot('a|', { @@ -121,6 +124,16 @@ describe('ResourcePolicyService', () => { a: resourcePolicyRD }) }); + ePersonService = jasmine.createSpyObj('ePersonService', { + getBrowseEndpoint: hot('a', { + a: ePersonEndpoint + }), + }); + /*groupService = jasmine.createSpyObj('groupService', { + getBrowseEndpoint: cold('a|', { + a: groupEndpoint + }), + });*/ objectCache = {} as ObjectCacheService; const notificationsService = {} as NotificationsService; const http = {} as HttpClient; @@ -326,4 +339,20 @@ describe('ResourcePolicyService', () => { expect(result).toBeObservable(expected); }); }); + + describe('updateTarget', () => { + it('should create a new PUT request for eperson', () => { + const resourcePolicyId = 'RESOURCE_POLICY_ID'; + const resourcePolicyHref = 'RESOURCE_POLICY_HREF'; + const targetUUID = 'TARGET_UUID'; + const targetType = 'eperson'; + + const result = service.updateTarget(resourcePolicyId, resourcePolicyHref, targetUUID, targetType); + const expected = cold('a|', { + a: resourcePolicyRD + }); + expect(result).toBeObservable(expected); + }); + }); + }); diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index 8411647bea..252a9fdcf8 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -23,7 +23,7 @@ import { PaginatedList } from '../data/paginated-list.model'; import { ActionType } from './models/action-type.model'; import { RequestParam } from '../cache/models/request-param.model'; import { isNotEmpty } from '../../shared/empty.util'; -import { map, switchMap, take } from 'rxjs/operators'; +import { map, take } from 'rxjs/operators'; import { NoContent } from '../shared/NoContent.model'; import { getFirstCompletedRemoteData } from '../shared/operators'; import { CoreState } from '../core-state.model'; @@ -71,7 +71,7 @@ export class ResourcePolicyService { protected searchByResourceMethod = 'resource'; constructor( - protected requestService: RequestService, + public requestService: RequestService, protected rdbService: RemoteDataBuildService, protected objectCache: ObjectCacheService, protected halService: HALEndpointService, @@ -237,16 +237,16 @@ export class ResourcePolicyService { * Update the target of the resource policy * @param resourcePolicyId the ID of the resource policy * @param resourcePolicyHref the link to the resource policy - * @param uuid the UUID of the target to which the permission is being granted - * @param type the type of the target (eperson or group) to which the permission is being granted + * @param targetUUID the UUID of the target to which the permission is being granted + * @param targetType the type of the target (eperson or group) to which the permission is being granted */ - updateTarget(resourcePolicyId: string, resourcePolicyHref: string, uuid: string, type: string): Observable> { + updateTarget(resourcePolicyId: string, resourcePolicyHref: string, targetUUID: string, targetType: string): Observable> { - const targetService = type === 'eperson' ? this.ePersonService : this.groupService; + const targetService = targetType === 'eperson' ? this.ePersonService : this.groupService; const targetEndpoint$ = targetService.getBrowseEndpoint().pipe( take(1), - map((endpoint: string) =>`${endpoint}/${uuid}`), + map((endpoint: string) =>`${endpoint}/${targetUUID}`), ); const options: HttpOptions = Object.create({}); @@ -256,10 +256,10 @@ export class ResourcePolicyService { const requestId = this.requestService.generateRequestId(); - this.requestService.setStaleByHrefSubstring(`${this.dataService.getLinkPath()}/${resourcePolicyId}/${type}`); + this.requestService.setStaleByHrefSubstring(`${this.dataService.getLinkPath()}/${resourcePolicyId}/${targetType}`); - return targetEndpoint$.pipe(switchMap((targetEndpoint) => { - const resourceEndpoint = resourcePolicyHref + '/' + type; + targetEndpoint$.subscribe((targetEndpoint) => { + const resourceEndpoint = resourcePolicyHref + '/' + targetType; const request = new PutRequest(requestId, resourceEndpoint, targetEndpoint, options); Object.assign(request, { getResponseParser(): GenericConstructor { @@ -267,8 +267,9 @@ export class ResourcePolicyService { } }); this.requestService.send(request); - return this.rdbService.buildFromRequestUUID(requestId); - })); + }); + + return this.rdbService.buildFromRequestUUID(requestId); } From d2e881eba343137e330428b258e3bc5cf6008d4f Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 20 May 2022 17:24:05 +0200 Subject: [PATCH 11/13] [CST-5674] fix test --- .../form/resource-policy-form.component.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts index 4a2a3376e0..e555522c79 100644 --- a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts +++ b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts @@ -254,6 +254,8 @@ describe('ResourcePolicyFormComponent test suite', () => { }); it('should init form model properly', () => { + epersonService.findByHref.and.returnValue(observableOf(undefined)); + groupService.findByHref.and.returnValue(observableOf(undefined)); spyOn(compAsAny, 'isFormValid').and.returnValue(observableOf(false)); spyOn(compAsAny, 'initModelsValue').and.callThrough(); spyOn(compAsAny, 'buildResourcePolicyForm').and.callThrough(); From 9d577c5317e518aea03e791f88e1d852adabac91 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 20 May 2022 17:35:22 +0200 Subject: [PATCH 12/13] [CST-5674] fix test (lint) --- src/app/core/resource-policy/resource-policy.service.spec.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index 2b99f26679..b2758e1688 100644 --- a/src/app/core/resource-policy/resource-policy.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy.service.spec.ts @@ -342,12 +342,9 @@ describe('ResourcePolicyService', () => { describe('updateTarget', () => { it('should create a new PUT request for eperson', () => { - const resourcePolicyId = 'RESOURCE_POLICY_ID'; - const resourcePolicyHref = 'RESOURCE_POLICY_HREF'; - const targetUUID = 'TARGET_UUID'; const targetType = 'eperson'; - const result = service.updateTarget(resourcePolicyId, resourcePolicyHref, targetUUID, targetType); + const result = service.updateTarget(resourcePolicyId, requestURL, epersonUUID, targetType); const expected = cold('a|', { a: resourcePolicyRD }); From c206005d425e876881116c3e5ccdcd6a5732051b Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 20 May 2022 17:41:20 +0200 Subject: [PATCH 13/13] [CST-5674] code cleanup --- src/app/core/resource-policy/resource-policy.service.spec.ts | 5 ----- src/app/core/resource-policy/resource-policy.service.ts | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/app/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index b2758e1688..b788a16520 100644 --- a/src/app/core/resource-policy/resource-policy.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy.service.spec.ts @@ -129,11 +129,6 @@ describe('ResourcePolicyService', () => { a: ePersonEndpoint }), }); - /*groupService = jasmine.createSpyObj('groupService', { - getBrowseEndpoint: cold('a|', { - a: groupEndpoint - }), - });*/ objectCache = {} as ObjectCacheService; const notificationsService = {} as NotificationsService; const http = {} as HttpClient; diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index 252a9fdcf8..b0b7a6bd97 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -71,7 +71,7 @@ export class ResourcePolicyService { protected searchByResourceMethod = 'resource'; constructor( - public requestService: RequestService, + protected requestService: RequestService, protected rdbService: RemoteDataBuildService, protected objectCache: ObjectCacheService, protected halService: HALEndpointService,