From c19d12c5c00df7301b5edafa16fd5c38323068a9 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 7 Apr 2022 17:15:07 +0200 Subject: [PATCH 01/37] 90252: Invalidate requests containing DSO on DataService.delete Keep track of a list of request UUIDs in the object cache (most recent in front) When deleting a DSO, mark all of these as stale --- .../core/cache/object-cache.reducer.spec.ts | 4 +- src/app/core/cache/object-cache.reducer.ts | 10 +- src/app/core/cache/object-cache.service.ts | 4 +- .../bitstream-format-data.service.spec.ts | 17 ++- src/app/core/data/data.service.spec.ts | 137 +++++++++++++++++- src/app/core/data/data.service.ts | 59 +++++++- src/app/core/data/request.service.spec.ts | 35 ++++- src/app/core/data/request.service.ts | 17 ++- src/app/shared/mocks/request.service.mock.ts | 1 + 9 files changed, 266 insertions(+), 18 deletions(-) diff --git a/src/app/core/cache/object-cache.reducer.spec.ts b/src/app/core/cache/object-cache.reducer.spec.ts index 61d587b6de..82e2da58b1 100644 --- a/src/app/core/cache/object-cache.reducer.spec.ts +++ b/src/app/core/cache/object-cache.reducer.spec.ts @@ -41,7 +41,7 @@ describe('objectCacheReducer', () => { alternativeLinks: [altLink1, altLink2], timeCompleted: new Date().getTime(), msToLive: 900000, - requestUUID: requestUUID1, + requestUUIDs: [requestUUID1], patches: [], isDirty: false, }, @@ -55,7 +55,7 @@ describe('objectCacheReducer', () => { alternativeLinks: [altLink3, altLink4], timeCompleted: new Date().getTime(), msToLive: 900000, - requestUUID: selfLink2, + requestUUIDs: [selfLink2], patches: [], isDirty: false } diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index 9001d334ce..1a42408f72 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -63,9 +63,11 @@ export class ObjectCacheEntry implements CacheEntry { msToLive: number; /** - * The UUID of the request that caused this entry to be added + * The UUIDs of the requests that caused this entry to be added + * New UUIDs should be added to the front of the array + * to make retrieving the latest UUID easier. */ - requestUUID: string; + requestUUIDs: string[]; /** * An array of patches that were made on the client side to this entry, but haven't been sent to the server yet @@ -156,11 +158,11 @@ function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheActio data: action.payload.objectToCache, timeCompleted: action.payload.timeCompleted, msToLive: action.payload.msToLive, - requestUUID: action.payload.requestUUID, + requestUUIDs: [action.payload.requestUUID, ...(existing.requestUUIDs || [])], isDirty: isNotEmpty(existing.patches), patches: existing.patches || [], alternativeLinks: [...(existing.alternativeLinks || []), ...newAltLinks] - } + } as ObjectCacheEntry }); } diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 6d48242178..00f124b2f6 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -197,7 +197,7 @@ export class ObjectCacheService { */ getRequestUUIDBySelfLink(selfLink: string): Observable { return this.getByHref(selfLink).pipe( - map((entry: ObjectCacheEntry) => entry.requestUUID), + map((entry: ObjectCacheEntry) => entry.requestUUIDs[0]), distinctUntilChanged()); } @@ -282,7 +282,7 @@ export class ObjectCacheService { let result = false; this.getByHref(href).subscribe((entry: ObjectCacheEntry) => { if (isNotEmpty(requestUUID)) { - result = entry.requestUUID === requestUUID; + result = entry.requestUUIDs[0] === requestUUID; // todo: may make more sense to do entry.requestUUIDs.includes(requestUUID) instead } else { result = true; } diff --git a/src/app/core/data/bitstream-format-data.service.spec.ts b/src/app/core/data/bitstream-format-data.service.spec.ts index c1ebf90a47..30ef79ee6d 100644 --- a/src/app/core/data/bitstream-format-data.service.spec.ts +++ b/src/app/core/data/bitstream-format-data.service.spec.ts @@ -37,7 +37,12 @@ describe('BitstreamFormatDataService', () => { } } as Store; - const objectCache = {} as ObjectCacheService; + const requestUUIDs = ['some', 'uuid']; + + const objectCache = jasmine.createSpyObj('objectCache', { + getByHref: observableOf({ requestUUIDs }) + }) as ObjectCacheService; + const halEndpointService = { getEndpoint(linkPath: string): Observable { return cold('a', { a: bitstreamFormatsEndpoint }); @@ -76,6 +81,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -96,6 +102,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -118,6 +125,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -139,6 +147,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -163,6 +172,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -186,6 +196,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -209,6 +220,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -231,6 +243,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -253,6 +266,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: cold('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); @@ -273,6 +287,7 @@ describe('BitstreamFormatDataService', () => { send: {}, getByHref: observableOf(responseCacheEntry), getByUUID: hot('a', { a: responseCacheEntry }), + setStaleByUUID: observableOf(true), generateRequestId: 'request-id', removeByHrefSubstring: {} }); diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index f680fed6a4..7022196c1a 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -11,7 +11,11 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { DSpaceObject } from '../shared/dspace-object.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Item } from '../shared/item.model'; -import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { + createFailedRemoteDataObject, + createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$, +} from '../../shared/remote-data.utils'; import { ChangeAnalyzer } from './change-analyzer'; import { DataService } from './data.service'; import { PatchRequest } from './request.models'; @@ -28,6 +32,8 @@ import { FindListOptions } from './find-list-options.model'; const endpoint = 'https://rest.api/core'; +const BOOLEAN = { f: false, t: true }; + class TestService extends DataService { constructor( @@ -86,6 +92,9 @@ describe('DataService', () => { }, getObjectBySelfLink: () => { /* empty */ + }, + getByHref: () => { + /* empty */ } } as any; store = {} as Store; @@ -833,4 +842,130 @@ describe('DataService', () => { }); }); + + describe('invalidateByHref', () => { + let getByHrefSpy: jasmine.Spy; + + beforeEach(() => { + getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ + requestUUIDs: ['request1', 'request2', 'request3'] + })); + + }); + + it('should call setStaleByUUID for every request associated with this DSO', (done) => { + service.invalidateByHref('some-href').subscribe((ok) => { + expect(ok).toBeTrue(); + expect(getByHrefSpy).toHaveBeenCalledWith('some-href'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); + done(); + }); + }); + + it('should return an Observable that only emits true once all requests are stale', () => { + testScheduler.run(({ cold, expectObservable }) => { + requestService.setStaleByUUID.and.callFake((uuid) => { + switch (uuid) { // fake requests becoming stale at different times + case 'request1': + return cold('--(t|)', BOOLEAN); + case 'request2': + return cold('----(t|)', BOOLEAN); + case 'request3': + return cold('------(t|)', BOOLEAN); + } + }); + + const done$ = service.invalidateByHref('some-href'); + + // emit true as soon as the final request is stale + expectObservable(done$).toBe('------(t|)', BOOLEAN); + }); + }); + }); + + describe('delete', () => { + let MOCK_SUCCEEDED_RD; + let MOCK_FAILED_RD; + + let invalidateByHrefSpy: jasmine.Spy; + let buildFromRequestUUIDSpy: jasmine.Spy; + let getIDHrefObsSpy: jasmine.Spy; + let deleteByHrefSpy: jasmine.Spy; + + beforeEach(() => { + invalidateByHrefSpy = spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true)); + buildFromRequestUUIDSpy = spyOn(rdbService, 'buildFromRequestUUID').and.callThrough(); + getIDHrefObsSpy = spyOn(service, 'getIDHrefObs').and.callThrough(); + deleteByHrefSpy = spyOn(service, 'deleteByHref').and.callThrough(); + + MOCK_SUCCEEDED_RD = createSuccessfulRemoteDataObject({}); + MOCK_FAILED_RD = createFailedRemoteDataObject('something went wrong'); + }); + + it('should retrieve href by ID and call deleteByHref', () => { + getIDHrefObsSpy.and.returnValue(observableOf('some-href')); + buildFromRequestUUIDSpy.and.returnValue(null); + + service.delete('some-id').subscribe(rd => { + expect(getIDHrefObsSpy).toHaveBeenCalledWith('some-id'); + expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href'); + }); + }); + + describe('deleteByHref', () => { + it('should call invalidateByHref if the DELETE request succeeds', (done) => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.deleteByHref('some-href').subscribe(rd => { + expect(rd).toBe(MOCK_SUCCEEDED_RD); + expect(invalidateByHrefSpy).toHaveBeenCalled(); + done(); + }); + }); + + it('should not call invalidateByHref if the DELETE request fails', (done) => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_FAILED_RD)); + + service.deleteByHref('some-href').subscribe(rd => { + expect(rd).toBe(MOCK_FAILED_RD); + expect(invalidateByHrefSpy).not.toHaveBeenCalled(); + done(); + }); + }); + + it('should wait for invalidateByHref before emitting', () => { + testScheduler.run(({ cold, expectObservable }) => { + buildFromRequestUUIDSpy.and.returnValue( + cold('(r|)', { r: MOCK_SUCCEEDED_RD}) // RD emits right away + ); + invalidateByHrefSpy.and.returnValue( + cold('----(t|)', BOOLEAN) // but we pretend that setting requests to stale takes longer + ); + + const done$ = service.deleteByHref('some-href'); + expectObservable(done$).toBe( + '----(r|)', { r: MOCK_SUCCEEDED_RD} // ...and expect the returned Observable to wait until that's done + ); + }); + }); + + it('should wait for the DELETE request to resolve before emitting', () => { + testScheduler.run(({ cold, expectObservable }) => { + buildFromRequestUUIDSpy.and.returnValue( + cold('----(r|)', { r: MOCK_SUCCEEDED_RD}) // the request takes a while + ); + invalidateByHrefSpy.and.returnValue( + cold('(t|)', BOOLEAN) // but we pretend that setting to stale happens sooner + ); // e.g.: maybe already stale before this call? + + const done$ = service.deleteByHref('some-href'); + expectObservable(done$).toBe( + '----(r|)', { r: MOCK_SUCCEEDED_RD} // ...and expect the returned Observable to wait for the request + ); + }); + }); + }); + }); }); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 310ad704ec..ca4d7e42de 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; import { Operation } from 'fast-json-patch'; -import { Observable, of as observableOf } from 'rxjs'; +import { combineLatest, from, Observable, of as observableOf } from 'rxjs'; import { distinctUntilChanged, filter, @@ -12,7 +12,7 @@ import { takeWhile, switchMap, tap, - skipWhile, + skipWhile, toArray } from 'rxjs/operators'; import { hasValue, isNotEmpty, isNotEmptyOperator } from '../../shared/empty.util'; import { NotificationOptions } from '../../shared/notifications/models/notification-options.model'; @@ -25,7 +25,7 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { DSpaceSerializer } from '../dspace-rest/dspace.serializer'; import { DSpaceObject } from '../shared/dspace-object.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { getRemoteDataPayload, getFirstSucceededRemoteData, } from '../shared/operators'; +import { getRemoteDataPayload, getFirstSucceededRemoteData, getFirstCompletedRemoteData } from '../shared/operators'; import { URLCombiner } from '../url-combiner/url-combiner'; import { ChangeAnalyzer } from './change-analyzer'; import { PaginatedList } from './paginated-list.model'; @@ -579,6 +579,37 @@ export abstract class DataService implements UpdateDa return result$; } + /** + * Invalidate an existing DSpaceObject by marking all requests it is included in as stale + * @param objectId The id of the object to be invalidated + * @return An Observable that will emit `true` once all requests are stale + */ + invalidate(objectId: string): Observable { + return this.getIDHrefObs(objectId).pipe( + switchMap((href: string) => this.invalidateByHref(href)) + ); + } + + /** + * Invalidate an existing DSpaceObject by marking all requests it is included in as stale + * @param href The self link of the object to be invalidated + * @return An Observable that will emit `true` once all requests are stale + */ + invalidateByHref(href: string): Observable { + return this.objectCache.getByHref(href).pipe( + map(oce => oce.requestUUIDs), + switchMap(requestUUIDs => { + return from(requestUUIDs).pipe( + mergeMap(requestUUID => this.requestService.setStaleByUUID(requestUUID)), + toArray(), + ); + }), + map(areRequestsStale => areRequestsStale.every(Boolean)), + distinctUntilChanged(), + takeWhile(allStale => allStale === false, true), + ); + } + /** * Delete an existing DSpace Object on the server * @param objectId The id of the object to be removed @@ -600,6 +631,7 @@ export abstract class DataService implements UpdateDa * metadata should be saved as real metadata * @return A RemoteData observable with an empty payload, but still representing the state of the request: statusCode, * errorMessage, timeCompleted, etc + * Only emits once all request related to the DSO has been invalidated. */ deleteByHref(href: string, copyVirtualMetadata?: string[]): Observable> { const requestId = this.requestService.generateRequestId(); @@ -618,7 +650,26 @@ export abstract class DataService implements UpdateDa } this.requestService.send(request); - return this.rdbService.buildFromRequestUUID(requestId); + const response$ = this.rdbService.buildFromRequestUUID(requestId); + + const invalidated$ = response$.pipe( + getFirstCompletedRemoteData(), + switchMap(rd => { + if (rd.hasSucceeded) { + return this.invalidateByHref(href); + } else { + return [true]; + } + }) + ); + + return combineLatest([response$, invalidated$]).pipe( + filter(([_, invalidated]) => invalidated), + tap(() => { + console.log(`DataService.deleteByHref() href=${href} done.`); + }), + map(([response, _]) => response), + ); } /** diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index a49761ae5d..fe35d840d7 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -8,7 +8,7 @@ import { defaultUUID, getMockUUIDService } from '../../shared/mocks/uuid.service import { ObjectCacheService } from '../cache/object-cache.service'; import { coreReducers} from '../core.reducers'; import { UUIDService } from '../shared/uuid.service'; -import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; +import { RequestConfigureAction, RequestExecuteAction, RequestStaleAction } from './request.actions'; import { DeleteRequest, GetRequest, @@ -19,7 +19,7 @@ import { PutRequest } from './request.models'; import { RequestService } from './request.service'; -import { TestBed, waitForAsync } from '@angular/core/testing'; +import { fakeAsync, TestBed, waitForAsync } from '@angular/core/testing'; import { storeModuleConfig } from '../../app.reducer'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { RequestEntryState } from './request-entry-state.model'; @@ -426,7 +426,7 @@ describe('RequestService', () => { describe('and it is cached', () => { describe('in the ObjectCache', () => { beforeEach(() => { - (objectCache.getByHref as any).and.returnValue(observableOf({ requestUUID: 'some-uuid' })); + (objectCache.getByHref as any).and.returnValue(observableOf({ requestUUIDs: ['some-uuid'] })); spyOn(serviceAsAny, 'hasByHref').and.returnValue(false); spyOn(serviceAsAny, 'hasByUUID').and.returnValue(true); }); @@ -596,4 +596,33 @@ describe('RequestService', () => { }); }); + describe('setStaleByUUID', () => { + let dispatchSpy: jasmine.Spy; + let getByUUIDSpy: jasmine.Spy; + + beforeEach(() => { + dispatchSpy = spyOn(store, 'dispatch'); + getByUUIDSpy = spyOn(service, 'getByUUID').and.callThrough(); + }); + + it('should dispatch a RequestStaleAction', () => { + service.setStaleByUUID('something'); + const firstAction = dispatchSpy.calls.argsFor(0)[0]; + expect(firstAction).toBeInstanceOf(RequestStaleAction); + expect(firstAction.payload).toEqual({ uuid: 'something' }); + }); + + it('should return an Observable that emits true as soon as the request is stale', fakeAsync(() => { + dispatchSpy.and.callFake(() => { /* empty */ }); // don't actually set as stale + getByUUIDSpy.and.returnValue(cold('a-b--c--d-', { // but fake the state in the cache + a: { state: RequestEntryState.ResponsePending }, + b: { state: RequestEntryState.Success }, + c: { state: RequestEntryState.SuccessStale }, + d: { state: RequestEntryState.Error }, + })); + + const done$ = service.setStaleByUUID('something'); + expect(done$).toBeObservable(cold('-----(t|)', { t: true })); + })); + }); }); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 3903bcfc99..b8b1d1eba9 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -311,6 +311,21 @@ export class RequestService { ); } + /** + * Mark a request as stale + * @param uuid the UUID of the request + * @return an Observable that will emit true once the Request becomes stale + */ + setStaleByUUID(uuid: string): Observable { + this.store.dispatch(new RequestStaleAction(uuid)); + + return this.getByUUID(uuid).pipe( + map(request => isStale(request.state)), + filter(stale => stale === true), + take(1), + ); + } + /** * Check if a GET request is in the cache or if it's still pending * @param {GetRequest} request The request to check @@ -339,7 +354,7 @@ export class RequestService { .subscribe((entry: ObjectCacheEntry) => { // if the object cache has a match, check if the request that the object came with is // still valid - inObjCache = this.hasByUUID(entry.requestUUID); + inObjCache = this.hasByUUID(entry.requestUUIDs[0]); }).unsubscribe(); // we should send the request if it isn't cached diff --git a/src/app/shared/mocks/request.service.mock.ts b/src/app/shared/mocks/request.service.mock.ts index f07aec587e..bce5b2d466 100644 --- a/src/app/shared/mocks/request.service.mock.ts +++ b/src/app/shared/mocks/request.service.mock.ts @@ -13,6 +13,7 @@ export function getMockRequestService(requestEntry$: Observable = isCachedOrPending: false, removeByHrefSubstring: observableOf(true), setStaleByHrefSubstring: observableOf(true), + setStaleByUUID: observableOf(true), hasByHref$: observableOf(false) }); } From 9699491269f1c47c239b745fea1252eeb316c4a4 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Wed, 13 Apr 2022 12:03:27 +0200 Subject: [PATCH 02/37] 90252: Remove old cleanup code & add tests to confirm that DataService.delete is called --- .../group-form/group-form.component.spec.ts | 60 ++++++++++++++- .../group-form/group-form.component.ts | 12 +-- .../groups-registry.component.html | 2 +- .../groups-registry.component.spec.ts | 31 +++++++- .../groups-registry.component.ts | 12 --- .../metadata-registry.component.ts | 2 - .../metadata-schema.component.ts | 3 - .../delete-collection-page.component.ts | 3 +- .../collection-metadata.component.spec.ts | 26 +++---- .../collection-metadata.component.ts | 23 +----- .../delete-community-page.component.ts | 3 +- src/app/core/data/comcol-data.service.spec.ts | 73 ++++++++++++++++++ .../core/eperson/eperson-data.service.spec.ts | 9 +-- .../core/registry/registry.service.spec.ts | 8 ++ .../item-bitstreams.component.ts | 22 ------ .../comcol-form/comcol-form.component.spec.ts | 22 ++++-- .../comcol-form/comcol-form.component.ts | 1 - .../delete-comcol-page.component.spec.ts | 8 -- .../delete-comcol-page.component.ts | 2 - .../item-versions-delete-modal.component.html | 4 +- .../item-versions-delete-modal.component.ts | 10 ++- .../item-versions.component.spec.ts | 77 +++++++++++++++++-- .../item-versions/item-versions.component.ts | 74 +++++++++--------- .../resource-policies.component.spec.ts | 10 +++ .../resource-policies.component.ts | 1 - 25 files changed, 333 insertions(+), 165 deletions(-) diff --git a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts index 2307f3c6fa..bd27a7a825 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts @@ -3,9 +3,9 @@ import { HttpClient } from '@angular/common/http'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { FormsModule, ReactiveFormsModule, FormArray, FormControl, FormGroup,Validators, NG_VALIDATORS, NG_ASYNC_VALIDATORS } from '@angular/forms'; -import { BrowserModule } from '@angular/platform-browser'; +import { BrowserModule, By } from '@angular/platform-browser'; import { ActivatedRoute, Router } from '@angular/router'; -import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; +import { NgbModal, NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { Store } from '@ngrx/store'; import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core'; import { Observable, of as observableOf } from 'rxjs'; @@ -27,7 +27,7 @@ import { FormBuilderService } from '../../../shared/form/builder/form-builder.se import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { GroupMock, GroupMock2 } from '../../../shared/testing/group-mock'; import { GroupFormComponent } from './group-form.component'; -import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { getMockFormBuilderService } from '../../../shared/mocks/form-builder-service.mock'; import { getMockTranslateService } from '../../../shared/mocks/translate.service.mock'; import { TranslateLoaderMock } from '../../../shared/testing/translate-loader.mock'; @@ -35,6 +35,7 @@ import { RouterMock } from '../../../shared/mocks/router.mock'; import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; import { Operation } from 'fast-json-patch'; import { ValidateGroupExists } from './validators/group-exists.validator'; +import { NoContent } from '../../../core/shared/NoContent.model'; describe('GroupFormComponent', () => { let component: GroupFormComponent; @@ -87,6 +88,9 @@ describe('GroupFormComponent', () => { patch(group: Group, operations: Operation[]) { return null; }, + delete(objectId: string, copyVirtualMetadata?: string[]): Observable> { + return createSuccessfulRemoteDataObject$({}); + }, cancelEditGroup(): void { this.activeGroup = null; }, @@ -348,4 +352,54 @@ describe('GroupFormComponent', () => { }); }); + describe('delete', () => { + let deleteButton; + let confirmButton; + let cancelButton; + + beforeEach(() => { + component.initialisePage(); + + component.canEdit$ = observableOf(true); + component.groupBeingEdited = { + permanent: false + } as Group; + + fixture.detectChanges(); + deleteButton = fixture.debugElement.query(By.css('.delete-button')).nativeElement; + + spyOn(groupsDataServiceStub, 'delete').and.callThrough(); + spyOn(groupsDataServiceStub, 'getActiveGroup').and.returnValue(observableOf({ id: 'active-group' })); + }); + + describe('if confirmed via modal', () => { + beforeEach(() => { + deleteButton.click(); + fixture.detectChanges(); + confirmButton = (document as any).querySelector('.modal-footer .confirm'); + }); + + it('should call GroupDataService.delete', () => { + confirmButton.click(); + fixture.detectChanges(); + + expect(groupsDataServiceStub.delete).toHaveBeenCalledWith('active-group'); + }); + }); + + describe('if canceled via modal', () => { + beforeEach(() => { + deleteButton.click(); + fixture.detectChanges(); + cancelButton = (document as any).querySelector('.modal-footer .cancel'); + }); + + it('should not call GroupDataService.delete', () => { + cancelButton.click(); + fixture.detectChanges(); + + expect(groupsDataServiceStub.delete).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/src/app/access-control/group-registry/group-form/group-form.component.ts b/src/app/access-control/group-registry/group-form/group-form.component.ts index 826b7dbe69..b0178f1294 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.ts @@ -426,7 +426,7 @@ export class GroupFormComponent implements OnInit, OnDestroy { .subscribe((rd: RemoteData) => { if (rd.hasSucceeded) { this.notificationsService.success(this.translateService.get(this.messagePrefix + '.notification.deleted.success', { name: group.name })); - this.reset(); + this.onCancel(); } else { this.notificationsService.error( this.translateService.get(this.messagePrefix + '.notification.deleted.failure.title', { name: group.name }), @@ -439,16 +439,6 @@ export class GroupFormComponent implements OnInit, OnDestroy { }); } - /** - * This method will ensure that the page gets reset and that the cache is cleared - */ - reset() { - this.groupDataService.getBrowseEndpoint().pipe(take(1)).subscribe((href: string) => { - this.requestService.removeByHrefSubstring(href); - }); - this.onCancel(); - } - /** * Cancel the current edit when component is destroyed & unsub all subscriptions */ diff --git a/src/app/access-control/group-registry/groups-registry.component.html b/src/app/access-control/group-registry/groups-registry.component.html index e791b7f2a0..237dedde09 100644 --- a/src/app/access-control/group-registry/groups-registry.component.html +++ b/src/app/access-control/group-registry/groups-registry.component.html @@ -79,7 +79,7 @@ diff --git a/src/app/access-control/group-registry/groups-registry.component.spec.ts b/src/app/access-control/group-registry/groups-registry.component.spec.ts index 0b30a551fd..99586ee5f0 100644 --- a/src/app/access-control/group-registry/groups-registry.component.spec.ts +++ b/src/app/access-control/group-registry/groups-registry.component.spec.ts @@ -31,6 +31,7 @@ import { RouterMock } from '../../shared/mocks/router.mock'; import { PaginationService } from '../../core/pagination/pagination.service'; import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { NoContent } from '../../core/shared/NoContent.model'; describe('GroupRegistryComponent', () => { let component: GroupsRegistryComponent; @@ -145,7 +146,10 @@ describe('GroupRegistryComponent', () => { totalPages: 1, currentPage: 1 }), [result])); - } + }, + delete(objectId: string, copyVirtualMetadata?: string[]): Observable> { + return createSuccessfulRemoteDataObject$({}); + }, }; dsoDataServiceStub = { findByHref(href: string): Observable> { @@ -301,4 +305,29 @@ describe('GroupRegistryComponent', () => { }); }); }); + + describe('delete', () => { + let deleteButton; + + beforeEach(fakeAsync(() => { + spyOn(groupsDataServiceStub, 'delete').and.callThrough(); + + setIsAuthorized(true, true); + + // force rerender after setup changes + component.search({ query: '' }); + tick(); + fixture.detectChanges(); + + // only mockGroup[0] is deletable, so we should only get one button + deleteButton = fixture.debugElement.query(By.css('.btn-delete')).nativeElement; + })); + + it('should call GroupDataService.delete', () => { + deleteButton.click(); + fixture.detectChanges(); + + expect(groupsDataServiceStub.delete).toHaveBeenCalledWith(mockGroups[0].id); + }); + }); }); diff --git a/src/app/access-control/group-registry/groups-registry.component.ts b/src/app/access-control/group-registry/groups-registry.component.ts index da861518da..29ec1770a7 100644 --- a/src/app/access-control/group-registry/groups-registry.component.ts +++ b/src/app/access-control/group-registry/groups-registry.component.ts @@ -199,7 +199,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { if (rd.hasSucceeded) { this.deletedGroupsIds = [...this.deletedGroupsIds, group.group.id]; this.notificationsService.success(this.translateService.get(this.messagePrefix + 'notification.deleted.success', { name: group.group.name })); - this.reset(); } else { this.notificationsService.error( this.translateService.get(this.messagePrefix + 'notification.deleted.failure.title', { name: group.group.name }), @@ -209,17 +208,6 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy { } } - /** - * This method will set everything to stale, which will cause the lists on this page to update. - */ - reset() { - this.groupService.getBrowseEndpoint().pipe( - take(1) - ).subscribe((href: string) => { - this.requestService.setStaleByHrefSubstring(href); - }); - } - /** * Get the members (epersons embedded value of a group) * @param group diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-registry.component.ts b/src/app/admin/admin-registries/metadata-registry/metadata-registry.component.ts index 8574c4678b..857034604e 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-registry.component.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-registry.component.ts @@ -128,7 +128,6 @@ export class MetadataRegistryComponent { * Delete all the selected metadata schemas */ deleteSchemas() { - this.registryService.clearMetadataSchemaRequests().subscribe(); this.registryService.getSelectedMetadataSchemas().pipe(take(1)).subscribe( (schemas) => { const tasks$ = []; @@ -148,7 +147,6 @@ export class MetadataRegistryComponent { } this.registryService.deselectAllMetadataSchema(); this.registryService.cancelEditMetadataSchema(); - this.forceUpdateSchemas(); }); } ); diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts index 8a2086d5e2..d0827e6e4d 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-schema.component.ts @@ -174,15 +174,12 @@ export class MetadataSchemaComponent implements OnInit { const failedResponses = responses.filter((response: RemoteData) => response.hasFailed); if (successResponses.length > 0) { this.showNotification(true, successResponses.length); - this.registryService.clearMetadataFieldRequests(); - } if (failedResponses.length > 0) { this.showNotification(false, failedResponses.length); } this.registryService.deselectAllMetadataField(); this.registryService.cancelEditMetadataField(); - this.forceUpdateFields(); }); } ); diff --git a/src/app/collection-page/delete-collection-page/delete-collection-page.component.ts b/src/app/collection-page/delete-collection-page/delete-collection-page.component.ts index 3e30373070..4c3e003773 100644 --- a/src/app/collection-page/delete-collection-page/delete-collection-page.component.ts +++ b/src/app/collection-page/delete-collection-page/delete-collection-page.component.ts @@ -24,8 +24,7 @@ export class DeleteCollectionPageComponent extends DeleteComColPageComponent { success: {}, error: {} }); - const objectCache = jasmine.createSpyObj('objectCache', { - remove: {} - }); const requestService = jasmine.createSpyObj('requestService', { setStaleByHrefSubstring: {} }); @@ -65,8 +62,7 @@ describe('CollectionMetadataComponent', () => { { provide: ItemTemplateDataService, useValue: itemTemplateServiceStub }, { provide: ActivatedRoute, useValue: { parent: { data: observableOf({ dso: createSuccessfulRemoteDataObject(collection) }) } } }, { provide: NotificationsService, useValue: notificationsService }, - { provide: ObjectCacheService, useValue: objectCache }, - { provide: RequestService, useValue: requestService } + { provide: RequestService, useValue: requestService }, ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -95,21 +91,19 @@ describe('CollectionMetadataComponent', () => { }); describe('deleteItemTemplate', () => { - describe('when delete returns a success', () => { - beforeEach(() => { - (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(true)); - comp.deleteItemTemplate(); - }); + beforeEach(() => { + (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(true)); + comp.deleteItemTemplate(); + }); + it('should call ItemTemplateService.deleteByCollectionID', () => { + expect(itemTemplateService.deleteByCollectionID).toHaveBeenCalledWith(template, 'collection-id'); + }); + + describe('when delete returns a success', () => { it('should display a success notification', () => { expect(notificationsService.success).toHaveBeenCalled(); }); - - it('should reset related object and request cache', () => { - expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collectionTemplateHref); - expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(template.self); - expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collection.self); - }); }); describe('when delete returns a failure', () => { diff --git a/src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts b/src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts index cfaad3767e..57248cdaed 100644 --- a/src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts @@ -38,8 +38,7 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent this.itemTemplateService.getCollectionEndpoint(collection.id)), - ); - - combineLatestObservable(collection$, template$, templateHref$).pipe( - switchMap(([collection, template, templateHref]) => { - return this.itemTemplateService.deleteByCollectionID(template, collection.uuid).pipe( - tap((success: boolean) => { - if (success) { - this.objectCache.remove(templateHref); - this.objectCache.remove(template.self); - this.requestService.setStaleByHrefSubstring(template.self); - this.requestService.setStaleByHrefSubstring(templateHref); - this.requestService.setStaleByHrefSubstring(collection.self); - } - }) - ); + combineLatestObservable(collection$, template$).pipe( + switchMap(([collection, template]) => { + return this.itemTemplateService.deleteByCollectionID(template, collection.uuid); }) ).subscribe((success: boolean) => { if (success) { diff --git a/src/app/community-page/delete-community-page/delete-community-page.component.ts b/src/app/community-page/delete-community-page/delete-community-page.component.ts index 0cccc503e1..3160d94be9 100644 --- a/src/app/community-page/delete-community-page/delete-community-page.component.ts +++ b/src/app/community-page/delete-community-page/delete-community-page.component.ts @@ -24,9 +24,8 @@ export class DeleteCommunityPageComponent extends DeleteComColPageComponent { }); }); }); + + describe('deleteLogo', () => { + let dso; + + beforeEach(() => { + dso = { + _links: { + logo: { + href: 'logo-href' + } + } + }; + }); + + describe('when DSO has no logo', () => { + beforeEach(() => { + dso.logo = undefined; + }); + + it('should return a failed RD', (done) => { + service.deleteLogo(dso).subscribe(rd => { + expect(rd.hasFailed).toBeTrue(); + expect(bitstreamDataService.deleteByHref).not.toHaveBeenCalled(); + done(); + }); + }); + }); + + describe('when DSO has a logo', () => { + let logo; + + beforeEach(() => { + logo = Object.assign(new Bitstream, { + id: 'logo-id', + _links: { + self: { + href: 'logo-href', + } + } + }); + }); + + describe('that can be retrieved', () => { + beforeEach(() => { + dso.logo = createSuccessfulRemoteDataObject$(logo); + }); + + it('should call BitstreamDataService.deleteByHref', (done) => { + service.deleteLogo(dso).subscribe(rd => { + expect(rd.hasSucceeded).toBeTrue(); + expect(bitstreamDataService.deleteByHref).toHaveBeenCalledWith('logo-href'); + done(); + }); + }); + }); + + describe('that cannot be retrieved', () => { + beforeEach(() => { + dso.logo = createFailedRemoteDataObject$(logo); + }); + + it('should not call BitstreamDataService.deleteByHref', (done) => { + service.deleteLogo(dso).subscribe(rd => { + expect(rd.hasFailed).toBeTrue(); + expect(bitstreamDataService.deleteByHref).not.toHaveBeenCalled(); + done(); + }); + }); + }); + }); + }); }); diff --git a/src/app/core/eperson/eperson-data.service.spec.ts b/src/app/core/eperson/eperson-data.service.spec.ts index e123253e2b..80cb92716a 100644 --- a/src/app/core/eperson/eperson-data.service.spec.ts +++ b/src/app/core/eperson/eperson-data.service.spec.ts @@ -21,7 +21,7 @@ import { EPersonDataService } from './eperson-data.service'; import { EPerson } from './models/eperson.model'; import { EPersonMock, EPersonMock2 } from '../../shared/testing/eperson.mock'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; -import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { createNoContentRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { getMockRemoteDataBuildServiceHrefMap } from '../../shared/mocks/remote-data-build.service.mock'; import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; import { getMockRequestService } from '../../shared/mocks/request.service.mock'; @@ -287,13 +287,12 @@ describe('EPersonDataService', () => { describe('deleteEPerson', () => { beforeEach(() => { - spyOn(service, 'findById').and.returnValue(createSuccessfulRemoteDataObject$(EPersonMock)); + spyOn(service, 'delete').and.returnValue(createNoContentRemoteDataObject$()); service.deleteEPerson(EPersonMock).subscribe(); }); - it('should send DeleteRequest', () => { - const expected = new DeleteRequest(requestService.generateRequestId(), epersonsEndpoint + '/' + EPersonMock.uuid); - expect(requestService.send).toHaveBeenCalledWith(expected); + it('should call DataService.delete with the EPerson\'s UUID', () => { + expect(service.delete).toHaveBeenCalledWith(EPersonMock.id); }); }); diff --git a/src/app/core/registry/registry.service.spec.ts b/src/app/core/registry/registry.service.spec.ts index db52a0547e..e9dfbe7e2c 100644 --- a/src/app/core/registry/registry.service.spec.ts +++ b/src/app/core/registry/registry.service.spec.ts @@ -386,6 +386,10 @@ describe('RegistryService', () => { result = registryService.deleteMetadataSchema(mockSchemasList[0].id); }); + it('should defer to MetadataSchemaDataService.delete', () => { + expect(metadataSchemaService.delete).toHaveBeenCalledWith(`${mockSchemasList[0].id}`); + }); + it('should return a successful response', () => { result.subscribe((response: RemoteData) => { expect(response.hasSucceeded).toBe(true); @@ -400,6 +404,10 @@ describe('RegistryService', () => { result = registryService.deleteMetadataField(mockFieldsList[0].id); }); + it('should defer to MetadataFieldDataService.delete', () => { + expect(metadataFieldService.delete).toHaveBeenCalledWith(`${mockFieldsList[0].id}`); + }); + it('should return a successful response', () => { result.subscribe((response: RemoteData) => { expect(response.hasSucceeded).toBe(true); diff --git a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts index 7599f95ed6..0c7dfb1e34 100644 --- a/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts +++ b/src/app/item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts @@ -147,7 +147,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme // Perform the setup actions from above in order and display notifications removedResponses$.pipe(take(1)).subscribe((responses: RemoteData[]) => { this.displayNotifications('item.edit.bitstreams.notifications.remove', responses); - this.reset(); this.submitting = false; }); } @@ -242,27 +241,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme ); } - /** - * De-cache the current item (it should automatically reload due to itemUpdateSubscription) - */ - reset() { - this.refreshItemCache(); - } - - /** - * Remove the current item's cache from object- and request-cache - */ - refreshItemCache() { - this.bundles$.pipe(take(1)).subscribe((bundles: Bundle[]) => { - bundles.forEach((bundle: Bundle) => { - this.objectCache.remove(bundle.self); - this.requestService.removeByHrefSubstring(bundle.self); - }); - this.objectCache.remove(this.item.self); - this.requestService.removeByHrefSubstring(this.item.self); - }); - } - /** * Unsubscribe from open subscriptions whenever the component gets destroyed */ diff --git a/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.spec.ts b/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.spec.ts index dc146e1fd7..054c161da6 100644 --- a/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.spec.ts +++ b/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.spec.ts @@ -55,6 +55,9 @@ describe('ComColFormComponent', () => { }) ]; + const logo = { + id: 'logo' + }; const logoEndpoint = 'rest/api/logo/endpoint'; const dsoService = Object.assign({ getLogoEndpoint: () => observableOf(logoEndpoint), @@ -207,7 +210,7 @@ describe('ComColFormComponent', () => { beforeEach(() => { initComponent(Object.assign(new Community(), { id: 'community-id', - logo: createSuccessfulRemoteDataObject$({}), + logo: createSuccessfulRemoteDataObject$(logo), _links: { self: { href: 'community-self' }, logo: { href: 'community-logo' }, @@ -225,28 +228,31 @@ describe('ComColFormComponent', () => { describe('submit with logo marked for deletion', () => { beforeEach(() => { + spyOn(dsoService, 'deleteLogo').and.callThrough(); comp.markLogoForDeletion = true; }); + it('should call dsoService.deleteLogo on the DSO', () => { + comp.onSubmit(); + fixture.detectChanges(); + + expect(dsoService.deleteLogo).toHaveBeenCalledWith(comp.dso); + }); + describe('when dsoService.deleteLogo returns a successful response', () => { beforeEach(() => { - spyOn(dsoService, 'deleteLogo').and.returnValue(createSuccessfulRemoteDataObject$({})); + dsoService.deleteLogo.and.returnValue(createSuccessfulRemoteDataObject$({})); comp.onSubmit(); }); it('should display a success notification', () => { expect(notificationsService.success).toHaveBeenCalled(); }); - - it('should remove the object\'s cache', () => { - expect(requestServiceStub.removeByHrefSubstring).toHaveBeenCalled(); - expect(objectCacheStub.remove).toHaveBeenCalled(); - }); }); describe('when dsoService.deleteLogo returns an error response', () => { beforeEach(() => { - spyOn(dsoService, 'deleteLogo').and.returnValue(createFailedRemoteDataObject$('Error', 500)); + dsoService.deleteLogo.and.returnValue(createFailedRemoteDataObject$('Error', 500)); comp.onSubmit(); }); diff --git a/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.ts b/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.ts index c3ba9dcaee..29be240753 100644 --- a/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.ts +++ b/src/app/shared/comcol/comcol-forms/comcol-form/comcol-form.component.ts @@ -184,7 +184,6 @@ export class ComColFormComponent implements On } this.dso.logo = undefined; this.uploadFilesOptions.method = RestRequestMethod.POST; - this.refreshCache(); this.finish.emit(); }); } diff --git a/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.spec.ts b/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.spec.ts index bdc2637886..bc73e4134b 100644 --- a/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.spec.ts +++ b/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.spec.ts @@ -68,7 +68,6 @@ describe('DeleteComColPageComponent', () => { { delete: createNoContentRemoteDataObject$(), findByHref: jasmine.createSpy('findByHref'), - refreshCache: jasmine.createSpy('refreshCache') }); routerStub = { @@ -79,10 +78,6 @@ describe('DeleteComColPageComponent', () => { data: observableOf(community) }; - requestServiceStub = jasmine.createSpyObj('RequestService', { - removeByHrefSubstring: jasmine.createSpy('removeByHrefSubstring') - }); - translateServiceStub = jasmine.createSpyObj('TranslateService', { instant: jasmine.createSpy('instant') }); @@ -99,7 +94,6 @@ describe('DeleteComColPageComponent', () => { { provide: ActivatedRoute, useValue: routeStub }, { provide: NotificationsService, useValue: new NotificationsServiceStub() }, { provide: TranslateService, useValue: translateServiceStub }, - { provide: RequestService, useValue: requestServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -159,7 +153,6 @@ describe('DeleteComColPageComponent', () => { scheduler.flush(); fixture.detectChanges(); expect(notificationsService.error).toHaveBeenCalled(); - expect(dsoDataService.refreshCache).not.toHaveBeenCalled(); expect(router.navigate).toHaveBeenCalled(); }); @@ -169,7 +162,6 @@ describe('DeleteComColPageComponent', () => { scheduler.flush(); fixture.detectChanges(); expect(notificationsService.success).toHaveBeenCalled(); - expect(dsoDataService.refreshCache).toHaveBeenCalled(); expect(router.navigate).toHaveBeenCalled(); }); diff --git a/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts b/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts index f781b71f3f..46b8f63bc1 100644 --- a/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts +++ b/src/app/shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component.ts @@ -41,7 +41,6 @@ export class DeleteComColPageComponent i protected route: ActivatedRoute, protected notifications: NotificationsService, protected translate: TranslateService, - protected requestService: RequestService ) { } @@ -61,7 +60,6 @@ export class DeleteComColPageComponent i if (response.hasSucceeded) { const successMessage = this.translate.instant((dso as any).type + '.delete.notification.success'); this.notifications.success(successMessage); - this.dsoDataService.refreshCache(dso); } else { const errorMessage = this.translate.instant((dso as any).type + '.delete.notification.fail'); this.notifications.error(errorMessage); diff --git a/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.html b/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.html index 0c0b72272f..6c54a15162 100644 --- a/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.html +++ b/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.html @@ -8,12 +8,12 @@

{{ "item.version.delete.modal.text" | translate : {version: versionNumber} }}

- -
+ + +
+
+

{{'profile.special.groups.head' | translate}}

+
    +
  • {{specialGroup.name}}
    • +
+
+
+ diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index f48b894d8d..5ba1deedcf 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -13,13 +13,17 @@ import { NotificationsService } from '../shared/notifications/notifications.serv import { authReducer } from '../core/auth/auth.reducer'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { createPaginatedList } from '../shared/testing/utils.test'; -import { BehaviorSubject, of as observableOf } from 'rxjs'; +import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; import { AuthService } from '../core/auth/auth.service'; import { RestResponse } from '../core/cache/response.models'; import { provideMockStore } from '@ngrx/store/testing'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { getTestScheduler } from 'jasmine-marbles'; import { By } from '@angular/platform-browser'; +import { RemoteData } from '../core/data/remote-data'; +import { PaginatedList } from '../core/data/paginated-list.model'; +import { Group } from '../core/eperson/models/group.model'; +import { SpecialGroupData } from '../shared/testing/special-group.mock'; describe('ProfilePageComponent', () => { let component: ProfilePageComponent; @@ -235,4 +239,20 @@ describe('ProfilePageComponent', () => { }); }); }); + + describe('check for specialGroups', () => { + it('should contains specialGroups list', () => { + component.specialGroupsRD$ = SpecialGroupData; + fixture.detectChanges(); + const specialGroupsEle = fixture.debugElement.query(By.css('#specialGroups')); + expect(specialGroupsEle).toBeTruthy(); + }); + + it('should not contains specialGroups list', () => { + component.specialGroupsRD$ = null; + fixture.detectChanges(); + const specialGroupsEle = fixture.debugElement.query(By.css('#specialGroups')); + expect(specialGroupsEle).toBeFalsy(); + }); + }); }); diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index fece166a59..574f8c17a4 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -20,6 +20,8 @@ import { AuthService } from '../core/auth/auth.service'; import { Operation } from 'fast-json-patch'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; +import { SpecialGroupData } from '../shared/testing/special-group.mock'; + @Component({ selector: 'ds-profile-page', @@ -45,6 +47,11 @@ export class ProfilePageComponent implements OnInit { */ groupsRD$: Observable>>; + /** + * The special groups the user belongs to + */ + specialGroupsRD$: Observable>>; + /** * Prefix for the notification messages of this component */ @@ -88,6 +95,7 @@ export class ProfilePageComponent implements OnInit { ); this.groupsRD$ = this.user$.pipe(switchMap((user: EPerson) => user.groups)); this.canChangePassword$ = this.user$.pipe(switchMap((user: EPerson) => this.authorizationService.isAuthorized(FeatureID.CanChangePassword, user._links.self.href))); + this.specialGroupsRD$ = SpecialGroupData; } /** diff --git a/src/app/shared/testing/auth-request-service.stub.ts b/src/app/shared/testing/auth-request-service.stub.ts index 0dc57427dd..0094324518 100644 --- a/src/app/shared/testing/auth-request-service.stub.ts +++ b/src/app/shared/testing/auth-request-service.stub.ts @@ -34,6 +34,9 @@ export class AuthRequestServiceStub { }, eperson: { href: this.mockUser._links.self.href + }, + specialGroups: { + href: this.mockUser._links.self.href } }; } else { @@ -62,6 +65,9 @@ export class AuthRequestServiceStub { }, eperson: { href: this.mockUser._links.self.href + }, + specialGroups: { + href: this.mockUser._links.self.href } }; } else { diff --git a/src/app/shared/testing/special-group.mock.ts b/src/app/shared/testing/special-group.mock.ts new file mode 100644 index 0000000000..76d5bf60ea --- /dev/null +++ b/src/app/shared/testing/special-group.mock.ts @@ -0,0 +1,64 @@ +import { EPersonMock } from './eperson.mock'; +import { of } from 'rxjs'; +import { RemoteData } from '../../core/data/remote-data'; +import { environment } from '../../../environments/environment'; +import { RequestEntryState } from '../../core/data/request.reducer'; +import { PageInfo } from '../../core/shared/page-info.model'; +import { buildPaginatedList } from '../../core/data/paginated-list.model'; +import { Group } from '../../core/eperson/models/group.model'; + + +export const SpecialGroupMock2: Group = Object.assign(new Group(), { + handle: null, + subgroups: [], + epersons: [], + permanent: true, + selfRegistered: false, + _links: { + self: { + href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid2', + }, + subgroups: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid2/subgroups' }, + object: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid2/object' }, + epersons: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid2/epersons' } + }, + _name: 'testgroupname2', + id: 'testgroupid2', + uuid: 'testgroupid2', + type: 'specialGroups', + // object: createSuccessfulRemoteDataObject$({ name: 'testspecialGroupsid2objectName'}) +}); + +export const SpecialGroupMock: Group = Object.assign(new Group(), { + handle: null, + subgroups: [SpecialGroupMock2], + epersons: [EPersonMock], + selfRegistered: false, + permanent: false, + _links: { + self: { + href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid', + }, + subgroups: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid/subgroups' }, + object: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid2/object' }, + epersons: { href: 'https://rest.api/server/api/eperson/specialGroups/testgroupid/epersons' } + }, + _name: 'testgroupname', + id: 'testgroupid', + uuid: 'testgroupid', + type: 'specialGroups', +}); + +export const SpecialGroupData = of(new RemoteData( + new Date().getTime(), + environment.cache.msToLive.default, + new Date().getTime(), + RequestEntryState.Success, + undefined, + buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock]), + 200 + )); + + + + diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index c3c68a6882..25c69d372a 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2861,6 +2861,8 @@ "profile.groups.head": "Authorization groups you belong to", + "profile.special.groups.head": "Authorization special groups you belong to", + "profile.head": "Update Profile", "profile.metadata.form.error.firstname.required": "First Name is required", From a34e7428c3ad7fb1d441ae9d3f532a44522fcce2 Mon Sep 17 00:00:00 2001 From: Pratik Rajkotiya Date: Wed, 27 Apr 2022 19:11:49 +0530 Subject: [PATCH 10/37] [CST-5738] remove unused variable. --- config/config.yml | 4 ++-- src/app/profile-page/profile-page.component.spec.ts | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/config/config.yml b/config/config.yml index b5eecd112f..3866797f5d 100644 --- a/config/config.yml +++ b/config/config.yml @@ -1,5 +1,5 @@ rest: - ssl: true - host: api7.dspace.org + ssl: false + host: localhost:8080 port: 443 nameSpace: /server diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 5ba1deedcf..6fad24eb34 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -20,9 +20,6 @@ import { provideMockStore } from '@ngrx/store/testing'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { getTestScheduler } from 'jasmine-marbles'; import { By } from '@angular/platform-browser'; -import { RemoteData } from '../core/data/remote-data'; -import { PaginatedList } from '../core/data/paginated-list.model'; -import { Group } from '../core/eperson/models/group.model'; import { SpecialGroupData } from '../shared/testing/special-group.mock'; describe('ProfilePageComponent', () => { From 44f393d65a54db0ec8c1b4b6663267da783b1f87 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Thu, 28 Apr 2022 11:44:53 +0200 Subject: [PATCH 11/37] [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 2e979afd22fdfe19427761189d0e64fa10bb4b32 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Thu, 28 Apr 2022 16:48:24 +0200 Subject: [PATCH 12/37] 90948: Show notifications when failing to create/delete role Groups --- .../collection-roles.component.spec.ts | 3 ++ .../community-roles.component.spec.ts | 3 ++ .../comcol-role/comcol-role.component.html | 2 +- .../comcol-role/comcol-role.component.spec.ts | 30 +++++++++++++++++++ .../comcol-role/comcol-role.component.ts | 25 ++++++++++++++-- src/assets/i18n/en.json5 | 4 +++ 6 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/app/collection-page/edit-collection-page/collection-roles/collection-roles.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-roles/collection-roles.component.spec.ts index 985290a592..5a8ca5b7ab 100644 --- a/src/app/collection-page/edit-collection-page/collection-roles/collection-roles.component.spec.ts +++ b/src/app/collection-page/edit-collection-page/collection-roles/collection-roles.component.spec.ts @@ -13,6 +13,8 @@ import { RouterTestingModule } from '@angular/router/testing'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { ComcolModule } from '../../../shared/comcol/comcol.module'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; describe('CollectionRolesComponent', () => { @@ -79,6 +81,7 @@ describe('CollectionRolesComponent', () => { { provide: ActivatedRoute, useValue: route }, { provide: RequestService, useValue: requestService }, { provide: GroupDataService, useValue: groupDataService }, + { provide: NotificationsService, useClass: NotificationsServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); diff --git a/src/app/community-page/edit-community-page/community-roles/community-roles.component.spec.ts b/src/app/community-page/edit-community-page/community-roles/community-roles.component.spec.ts index d1188df02d..baea5fb9d0 100644 --- a/src/app/community-page/edit-community-page/community-roles/community-roles.component.spec.ts +++ b/src/app/community-page/edit-community-page/community-roles/community-roles.component.spec.ts @@ -13,6 +13,8 @@ import { RouterTestingModule } from '@angular/router/testing'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { ComcolModule } from '../../../shared/comcol/comcol.module'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub'; describe('CommunityRolesComponent', () => { @@ -64,6 +66,7 @@ describe('CommunityRolesComponent', () => { { provide: ActivatedRoute, useValue: route }, { provide: RequestService, useValue: requestService }, { provide: GroupDataService, useValue: groupDataService }, + { provide: NotificationsService, useClass: NotificationsServiceStub } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); diff --git a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.html b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.html index b0a319b4ff..905186a232 100644 --- a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.html +++ b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.html @@ -4,7 +4,7 @@ *ngVar="group$ | async as group">
- {{'comcol-role.edit.' + (comcolRole$ | async)?.name + '.name' | translate}} + {{ roleName$ | async }}
diff --git a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.spec.ts b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.spec.ts index 175abe48e4..59fc95b67f 100644 --- a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.spec.ts +++ b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.spec.ts @@ -10,6 +10,8 @@ import { RouterTestingModule } from '@angular/router/testing'; import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../../remote-data.utils'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { ComcolModule } from '../../../comcol.module'; +import { NotificationsService } from '../../../../notifications/notifications.service'; +import { NotificationsServiceStub } from '../../../../testing/notifications-service.stub'; describe('ComcolRoleComponent', () => { @@ -20,6 +22,7 @@ describe('ComcolRoleComponent', () => { let group; let statusCode; let comcolRole; + let notificationsService; const requestService = { hasByHref$: () => observableOf(true) }; @@ -40,6 +43,7 @@ describe('ComcolRoleComponent', () => { providers: [ { provide: GroupDataService, useValue: groupService }, { provide: RequestService, useValue: requestService }, + { provide: NotificationsService, useClass: NotificationsServiceStub } ], schemas: [ NO_ERRORS_SCHEMA ] @@ -59,12 +63,14 @@ describe('ComcolRoleComponent', () => { fixture = TestBed.createComponent(ComcolRoleComponent); comp = fixture.componentInstance; de = fixture.debugElement; + notificationsService = TestBed.inject(NotificationsService); comcolRole = { name: 'test role name', href: 'test role link', }; comp.comcolRole = comcolRole; + comp.roleName$ = observableOf(comcolRole.name); fixture.detectChanges(); }); @@ -101,6 +107,18 @@ describe('ComcolRoleComponent', () => { done(); }); }); + + describe('when a group cannot be created', () => { + beforeEach(() => { + groupService.createComcolGroup.and.returnValue(createFailedRemoteDataObject$()); + de.query(By.css('.btn.create')).nativeElement.click(); + }); + + it('should show an error notification', (done) => { + expect(notificationsService.error).toHaveBeenCalled(); + done(); + }); + }); }); describe('when the related group is the Anonymous group', () => { @@ -169,5 +187,17 @@ describe('ComcolRoleComponent', () => { done(); }); }); + + describe('when a group cannot be deleted', () => { + beforeEach(() => { + groupService.deleteComcolGroup.and.returnValue(createFailedRemoteDataObject$()); + de.query(By.css('.btn.delete')).nativeElement.click(); + }); + + it('should show an error notification', (done) => { + expect(notificationsService.error).toHaveBeenCalled(); + done(); + }); + }); }); }); diff --git a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.ts b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.ts index 7ed88fae1c..3091dd0cf0 100644 --- a/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.ts +++ b/src/app/shared/comcol/comcol-forms/edit-comcol-page/comcol-role/comcol-role.component.ts @@ -12,6 +12,8 @@ import { HALLink } from '../../../../../core/shared/hal-link.model'; import { getGroupEditRoute } from '../../../../../access-control/access-control-routing-paths'; import { hasNoValue, hasValue } from '../../../../empty.util'; import { NoContent } from '../../../../../core/shared/NoContent.model'; +import { NotificationsService } from '../../../../notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; /** * Component for managing a community or collection role. @@ -64,9 +66,16 @@ export class ComcolRoleComponent implements OnInit { */ hasCustomGroup$: Observable; + /** + * The human-readable name of this role + */ + roleName$: Observable; + constructor( protected requestService: RequestService, protected groupService: GroupDataService, + protected notificationsService: NotificationsService, + protected translateService: TranslateService, ) { } @@ -101,7 +110,12 @@ export class ComcolRoleComponent implements OnInit { this.groupService.clearGroupsRequests(); this.requestService.setStaleByHrefSubstring(this.comcolRole.href); } else { - // TODO show error notification + this.notificationsService.error( + this.roleName$.pipe( + switchMap(role => this.translateService.get('comcol-role.edit.create.error.title', { role })) + ), + `${rd.statusCode} ${rd.errorMessage}` + ); } }); } @@ -117,7 +131,12 @@ export class ComcolRoleComponent implements OnInit { this.groupService.clearGroupsRequests(); this.requestService.setStaleByHrefSubstring(this.comcolRole.href); } else { - // TODO show error notification + this.notificationsService.error( + this.roleName$.pipe( + switchMap(role => this.translateService.get('comcol-role.edit.delete.error.title', { role })) + ), + rd.errorMessage + ); } }); } @@ -154,5 +173,7 @@ export class ComcolRoleComponent implements OnInit { this.hasCustomGroup$ = this.group$.pipe( map((group: Group) => hasValue(group) && group.name !== 'Anonymous'), ); + + this.roleName$ = this.translateService.get(`comcol-role.edit.${this.comcolRole.name}.name`); } } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index f742273edb..9121be759b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1082,10 +1082,14 @@ "comcol-role.edit.create": "Create", + "comcol-role.edit.create.error.title": "Failed to create a group for the '{{ role }}' role", + "comcol-role.edit.restrict": "Restrict", "comcol-role.edit.delete": "Delete", + "comcol-role.edit.delete.error.title": "Failed to delete the '{{ role }}' role's group", + "comcol-role.edit.community-admin.name": "Administrators", From e94454be97d7a84502295cf9bfa586aba5b970e0 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 29 Apr 2022 12:11:06 +0200 Subject: [PATCH 13/37] [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 14/37] [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 0f80b6533b89e98f4161a3dc4d164ddda9f10f0f Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 29 Apr 2022 12:49:07 +0200 Subject: [PATCH 15/37] [CST-5738] Add method to retrieve embedded special groups --- src/app/core/auth/auth-request.service.ts | 12 +++++--- src/app/core/auth/auth.service.spec.ts | 22 ++++++++++++++ src/app/core/auth/auth.service.ts | 22 +++++++++++++- src/app/core/auth/models/auth-status.model.ts | 6 ++-- .../profile-page.component.spec.ts | 9 +++--- .../profile-page/profile-page.component.ts | 10 ++----- src/app/shared/testing/special-group.mock.ts | 29 +++++++------------ 7 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/app/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index da38d730a5..516502ca6e 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/src/app/core/auth/auth-request.service.ts @@ -18,7 +18,6 @@ import { RestRequest } from '../data/rest-request.model'; */ export abstract class AuthRequestService { protected linkName = 'authn'; - protected browseEndpoint = ''; protected shortlivedtokensEndpoint = 'shortlivedtokens'; constructor(protected halService: HALEndpointService, @@ -33,8 +32,13 @@ export abstract class AuthRequestService { ); } - protected getEndpointByMethod(endpoint: string, method: string): string { - return isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; + protected getEndpointByMethod(endpoint: string, method: string, embed = false): string { + let url = isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; + if (embed) { + url += '?embed=specialGroups'; + } + + return url; } public postToEndpoint(method: string, body?: any, options?: HttpOptions): Observable> { @@ -48,7 +52,7 @@ export abstract class AuthRequestService { distinctUntilChanged()); } - public getRequest(method: string, options?: HttpOptions): Observable> { + public getRequest(method: string, options?: HttpOptions, embed = false): Observable> { return this.halService.getEndpoint(this.linkName).pipe( filter((href: string) => isNotEmpty(href)), map((endpointURL) => this.getEndpointByMethod(endpointURL, method)), diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index ced8bb94c8..d63e300ce0 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -32,6 +32,8 @@ import { TranslateService } from '@ngx-translate/core'; import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub'; import { SetUserAsIdleAction, UnsetUserAsIdleAction } from './auth.actions'; +import { SpecialGroupDataMock, SpecialGroupDataMock$ } from '../../shared/testing/special-group.mock'; +import { cold } from 'jasmine-marbles'; describe('AuthService test', () => { @@ -56,6 +58,13 @@ describe('AuthService test', () => { let linkService; let hardRedirectService; + const AuthStatusWithSpecialGroups = Object.assign(new AuthStatus(), { + uuid: 'test', + authenticated: true, + okay: true, + specialGroups: SpecialGroupDataMock$ + }); + function init() { mockStore = jasmine.createSpyObj('store', { dispatch: {}, @@ -511,6 +520,19 @@ describe('AuthService test', () => { expect((authService as any).navigateToRedirectUrl).toHaveBeenCalled(); }); }); + + describe('getSpecialGroupsFromAuthStatus', () => { + beforeEach(() => { + spyOn(authRequest, 'getRequest').and.returnValue(createSuccessfulRemoteDataObject$(AuthStatusWithSpecialGroups)); + }); + + it('should call navigateToRedirectUrl with no url', () => { + const expectRes = cold('(a|)', { + a: SpecialGroupDataMock + }); + expect(authService.getSpecialGroupsFromAuthStatus()).toBeObservable(expectRes); + }); + }); }); describe('when user is not logged in', () => { diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index f89fa21681..2d2c61490c 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -44,13 +44,17 @@ import { import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { RouteService } from '../services/route.service'; import { EPersonDataService } from '../eperson/eperson-data.service'; -import { getAllSucceededRemoteDataPayload } from '../shared/operators'; +import { getAllSucceededRemoteDataPayload, getFirstCompletedRemoteData } from '../shared/operators'; import { AuthMethod } from './models/auth.method'; import { HardRedirectService } from '../services/hard-redirect.service'; import { RemoteData } from '../data/remote-data'; import { environment } from '../../../environments/environment'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; +import { buildPaginatedList, PaginatedList } from '../data/paginated-list.model'; +import { Group } from '../eperson/models/group.model'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { PageInfo } from '../shared/page-info.model'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -211,6 +215,22 @@ export class AuthService { this.store.dispatch(new CheckAuthenticationTokenAction()); } + /** + * Return the special groups list embedded in the AuthStatus model + */ + public getSpecialGroupsFromAuthStatus(): Observable>> { + return this.authRequestService.getRequest('status', null, true).pipe( + getFirstCompletedRemoteData(), + switchMap((status: RemoteData) => { + if (status.hasSucceeded) { + return status.payload.specialGroups; + } else { + return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(),[])); + } + }) + ); + } + /** * Checks if token is present into storage and is not expired */ diff --git a/src/app/core/auth/models/auth-status.model.ts b/src/app/core/auth/models/auth-status.model.ts index 2634182a55..d18b1ccf9a 100644 --- a/src/app/core/auth/models/auth-status.model.ts +++ b/src/app/core/auth/models/auth-status.model.ts @@ -15,7 +15,7 @@ import { AUTH_STATUS } from './auth-status.resource-type'; import { AuthTokenInfo } from './auth-token-info.model'; import { AuthMethod } from './auth.method'; import { CacheableObject } from '../../cache/cacheable-object.model'; - +import { PaginatedList } from '../../data/paginated-list.model'; /** * Object that represents the authenticated status of a user @@ -78,8 +78,8 @@ export class AuthStatus implements CacheableObject { * The SpecialGroup of this auth status * Will be undefined unless the SpecialGroup {@link HALLink} has been resolved. */ - @link(GROUP) - specialGroups?: Observable>; + @link(GROUP, true) + specialGroups?: Observable>>; /** * True if the token is valid, false if there was no token or the token wasn't valid diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 6fad24eb34..1bc9a78ad5 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -13,14 +13,14 @@ import { NotificationsService } from '../shared/notifications/notifications.serv import { authReducer } from '../core/auth/auth.reducer'; import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; import { createPaginatedList } from '../shared/testing/utils.test'; -import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; +import { BehaviorSubject, of as observableOf } from 'rxjs'; import { AuthService } from '../core/auth/auth.service'; import { RestResponse } from '../core/cache/response.models'; import { provideMockStore } from '@ngrx/store/testing'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { getTestScheduler } from 'jasmine-marbles'; import { By } from '@angular/platform-browser'; -import { SpecialGroupData } from '../shared/testing/special-group.mock'; +import { SpecialGroupDataMock$ } from '../shared/testing/special-group.mock'; describe('ProfilePageComponent', () => { let component: ProfilePageComponent; @@ -55,7 +55,8 @@ describe('ProfilePageComponent', () => { }; authService = jasmine.createSpyObj('authService', { - getAuthenticatedUserFromStore: observableOf(user) + getAuthenticatedUserFromStore: observableOf(user), + getSpecialGroupsFromAuthStatus: SpecialGroupDataMock$ }); epersonService = jasmine.createSpyObj('epersonService', { findById: createSuccessfulRemoteDataObject$(user), @@ -239,8 +240,6 @@ describe('ProfilePageComponent', () => { describe('check for specialGroups', () => { it('should contains specialGroups list', () => { - component.specialGroupsRD$ = SpecialGroupData; - fixture.detectChanges(); const specialGroupsEle = fixture.debugElement.query(By.css('#specialGroups')); expect(specialGroupsEle).toBeTruthy(); }); diff --git a/src/app/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index 574f8c17a4..7623e9e6ea 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -9,19 +9,13 @@ import { RemoteData } from '../core/data/remote-data'; import { PaginatedList } from '../core/data/paginated-list.model'; import { filter, switchMap, tap } from 'rxjs/operators'; import { EPersonDataService } from '../core/eperson/eperson-data.service'; -import { - getAllSucceededRemoteData, - getRemoteDataPayload, - getFirstCompletedRemoteData -} from '../core/shared/operators'; +import { getAllSucceededRemoteData, getFirstCompletedRemoteData, getRemoteDataPayload } from '../core/shared/operators'; import { hasValue, isNotEmpty } from '../shared/empty.util'; import { followLink } from '../shared/utils/follow-link-config.model'; import { AuthService } from '../core/auth/auth.service'; import { Operation } from 'fast-json-patch'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; -import { SpecialGroupData } from '../shared/testing/special-group.mock'; - @Component({ selector: 'ds-profile-page', @@ -95,7 +89,7 @@ export class ProfilePageComponent implements OnInit { ); this.groupsRD$ = this.user$.pipe(switchMap((user: EPerson) => user.groups)); this.canChangePassword$ = this.user$.pipe(switchMap((user: EPerson) => this.authorizationService.isAuthorized(FeatureID.CanChangePassword, user._links.self.href))); - this.specialGroupsRD$ = SpecialGroupData; + this.specialGroupsRD$ = this.authService.getSpecialGroupsFromAuthStatus(); } /** diff --git a/src/app/shared/testing/special-group.mock.ts b/src/app/shared/testing/special-group.mock.ts index 76d5bf60ea..d8ec8a83cc 100644 --- a/src/app/shared/testing/special-group.mock.ts +++ b/src/app/shared/testing/special-group.mock.ts @@ -1,12 +1,11 @@ -import { EPersonMock } from './eperson.mock'; -import { of } from 'rxjs'; -import { RemoteData } from '../../core/data/remote-data'; -import { environment } from '../../../environments/environment'; -import { RequestEntryState } from '../../core/data/request.reducer'; -import { PageInfo } from '../../core/shared/page-info.model'; -import { buildPaginatedList } from '../../core/data/paginated-list.model'; -import { Group } from '../../core/eperson/models/group.model'; +import { Observable } from 'rxjs'; +import { EPersonMock } from './eperson.mock'; +import { PageInfo } from '../../core/shared/page-info.model'; +import { buildPaginatedList, PaginatedList } from '../../core/data/paginated-list.model'; +import { Group } from '../../core/eperson/models/group.model'; +import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../remote-data.utils'; +import { RemoteData } from '../../core/data/remote-data'; export const SpecialGroupMock2: Group = Object.assign(new Group(), { handle: null, @@ -49,16 +48,8 @@ export const SpecialGroupMock: Group = Object.assign(new Group(), { type: 'specialGroups', }); -export const SpecialGroupData = of(new RemoteData( - new Date().getTime(), - environment.cache.msToLive.default, - new Date().getTime(), - RequestEntryState.Success, - undefined, - buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock]), - 200 - )); - - +export const SpecialGroupDataMock: RemoteData> = createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock])); +export const SpecialGroupDataMock$: Observable>> = createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock])); + From d5e5a4a550991b136c686b8d4ee84524221fd66a Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 29 Apr 2022 13:02:22 +0200 Subject: [PATCH 16/37] [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 aa7ceec15aaa331335a67e6d595c075d14918c25 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Fri, 29 Apr 2022 19:15:40 +0200 Subject: [PATCH 17/37] [CST-5738] Fix retrieving of embed from auth status model --- src/app/core/auth/auth-request.service.ts | 19 +++++++++++-------- src/app/core/auth/auth.service.ts | 3 ++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/app/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index 516502ca6e..d0fec5fa71 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/src/app/core/auth/auth-request.service.ts @@ -12,6 +12,7 @@ import { AuthStatus } from './models/auth-status.model'; import { ShortLivedToken } from './models/short-lived-token.model'; import { URLCombiner } from '../url-combiner/url-combiner'; import { RestRequest } from '../data/rest-request.model'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; /** * Abstract service to send authentication requests @@ -26,16 +27,18 @@ export abstract class AuthRequestService { ) { } - protected fetchRequest(request: RestRequest): Observable> { - return this.rdbService.buildFromRequestUUID(request.uuid).pipe( + protected fetchRequest(request: RestRequest, ...linksToFollow: FollowLinkConfig[]): Observable> { + return this.rdbService.buildFromRequestUUID(request.uuid, ...linksToFollow).pipe( getFirstCompletedRemoteData(), ); } - protected getEndpointByMethod(endpoint: string, method: string, embed = false): string { + protected getEndpointByMethod(endpoint: string, method: string, ...linksToFollow: FollowLinkConfig[]): string { let url = isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; - if (embed) { - url += '?embed=specialGroups'; + if (linksToFollow && linksToFollow.length > 0) { + linksToFollow.forEach((link: FollowLinkConfig, index: number) => { + url += ((index === 0) ? '?' : '&') + `embed=${link.name}`; + }); } return url; @@ -52,14 +55,14 @@ export abstract class AuthRequestService { distinctUntilChanged()); } - public getRequest(method: string, options?: HttpOptions, embed = false): Observable> { + public getRequest(method: string, options?: HttpOptions, ...linksToFollow: FollowLinkConfig[]): Observable> { return this.halService.getEndpoint(this.linkName).pipe( filter((href: string) => isNotEmpty(href)), - map((endpointURL) => this.getEndpointByMethod(endpointURL, method)), + map((endpointURL) => this.getEndpointByMethod(endpointURL, method, ...linksToFollow)), distinctUntilChanged(), map((endpointURL: string) => new GetRequest(this.requestService.generateRequestId(), endpointURL, undefined, options)), tap((request: GetRequest) => this.requestService.send(request)), - mergeMap((request: GetRequest) => this.fetchRequest(request)), + mergeMap((request: GetRequest) => this.fetchRequest(request, ...linksToFollow)), distinctUntilChanged()); } diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 2d2c61490c..7796094e39 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -55,6 +55,7 @@ import { buildPaginatedList, PaginatedList } from '../data/paginated-list.model' import { Group } from '../eperson/models/group.model'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { PageInfo } from '../shared/page-info.model'; +import { followLink } from '../../shared/utils/follow-link-config.model'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -219,7 +220,7 @@ export class AuthService { * Return the special groups list embedded in the AuthStatus model */ public getSpecialGroupsFromAuthStatus(): Observable>> { - return this.authRequestService.getRequest('status', null, true).pipe( + return this.authRequestService.getRequest('status', null, followLink('specialGroups')).pipe( getFirstCompletedRemoteData(), switchMap((status: RemoteData) => { if (status.hasSucceeded) { From bb3cc1c619dfe0e2f9a8bfb55380f8f5eb0671e9 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Mon, 2 May 2022 17:02:04 +0200 Subject: [PATCH 18/37] [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 19/37] [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 20/37] [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 21/37] [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 724fc3c6f045e8ff839cc0c080492139279e81c6 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Mon, 9 May 2022 09:24:17 +0200 Subject: [PATCH 22/37] [CST-5738] Revert unintentional change on config.yml --- config/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config.yml b/config/config.yml index 3866797f5d..b5eecd112f 100644 --- a/config/config.yml +++ b/config/config.yml @@ -1,5 +1,5 @@ rest: - ssl: false - host: localhost:8080 + ssl: true + host: api7.dspace.org port: 443 nameSpace: /server From 42c459a51d5ebcf2eff3423fe239968e24dfcd9c Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Thu, 12 May 2022 16:13:43 +0200 Subject: [PATCH 23/37] [CST-5303] added labels into i18n --- src/assets/i18n/en.json5 | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 9bdf41346d..bb2bcf8ebc 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2158,6 +2158,22 @@ "item.preview.dc.title": "Title:", + "item.preview.dc.type": "Type:", + + "item.preview.oaire.citation.issue" : "Issue", + + "item.preview.oaire.citation.volume" : "Volume", + + "item.preview.dc.relation.issn" : "ISSN", + + "item.preview.dc.identifier.isbn" : "ISBN", + + "item.preview.dc.identifier": "Identifier:", + + "item.preview.dc.relation.ispartof" : "Journal or Serie", + + "item.preview.dc.identifier.doi" : "DOI", + "item.preview.person.familyName": "Surname:", "item.preview.person.givenName": "Name:", @@ -3568,6 +3584,22 @@ "submission.import-external.source.arxiv": "arXiv", + "submission.import-external.source.ads": "NASA/ADS", + + "submission.import-external.source.cinii": "CiNii", + + "submission.import-external.source.crossref": "CrossRef", + + "submission.import-external.source.scielo": "SciELO", + + "submission.import-external.source.scopus": "Scopus", + + "submission.import-external.source.vufind": "VuFind", + + "submission.import-external.source.wos": "Web Of Science", + + "submission.import-external.source.epo": "European Patent Office (EPO)", + "submission.import-external.source.loading": "Loading ...", "submission.import-external.source.sherpaJournal": "SHERPA Journals", @@ -3582,10 +3614,14 @@ "submission.import-external.source.pubmed": "Pubmed", + "submission.import-external.source.pubmedeu": "Pubmed Europe", + "submission.import-external.source.lcname": "Library of Congress Names", "submission.import-external.preview.title": "Item Preview", + "submission.import-external.preview.title.Publication": "Publication Preview", + "submission.import-external.preview.subtitle": "The metadata below was imported from an external source. It will be pre-filled when you start the submission.", "submission.import-external.preview.button.import": "Start submission", @@ -3824,6 +3860,18 @@ "submission.sections.describe.relationship-lookup.selection-tab.title.arxiv": "Search Results", + "submission.sections.describe.relationship-lookup.selection-tab.title.crossref": "Search Results", + + "submission.sections.describe.relationship-lookup.selection-tab.title.epo": "Search Results", + + "submission.sections.describe.relationship-lookup.selection-tab.title.scopus": "Search Results", + + "submission.sections.describe.relationship-lookup.selection-tab.title.scielo": "Search Results", + + "submission.sections.describe.relationship-lookup.selection-tab.title.wos": "Search Results", + + "submission.sections.describe.relationship-lookup.selection-tab.title": "Search Results", + "submission.sections.describe.relationship-lookup.name-variant.notification.content": "Would you like to save \"{{ value }}\" as a name variant for this person so you and others can reuse it for future submissions? If you don\'t you can still use it for this submission.", "submission.sections.describe.relationship-lookup.name-variant.notification.confirm": "Save a new name variant", From 1f9b8ba92a4fcf09e2f0c6cb8661f7b8b6fdd637 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 17 May 2022 11:16:46 +0200 Subject: [PATCH 24/37] Fix issue where you'd get stuck in an infinite redirect loop when going to the page of an item with an entity type containing certain special characters --- src/app/item-page/item-page.resolver.spec.ts | 87 ++++++++++++++++++++ src/app/item-page/item-page.resolver.ts | 10 ++- 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 src/app/item-page/item-page.resolver.spec.ts diff --git a/src/app/item-page/item-page.resolver.spec.ts b/src/app/item-page/item-page.resolver.spec.ts new file mode 100644 index 0000000000..533f97d0c0 --- /dev/null +++ b/src/app/item-page/item-page.resolver.spec.ts @@ -0,0 +1,87 @@ +import { ItemPageResolver } from './item-page.resolver'; +import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils'; +import { DSpaceObject } from '../core/shared/dspace-object.model'; +import { MetadataValueFilter } from '../core/shared/metadata.models'; +import { first } from 'rxjs/operators'; +import { Router } from '@angular/router'; +import { TestBed } from '@angular/core/testing'; +import { RouterTestingModule } from '@angular/router/testing'; + +describe('ItemPageResolver', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [RouterTestingModule.withRoutes([{ + path: 'entities/:entity-type/:id', + component: {} as any + }])] + }); + }); + + describe('resolve', () => { + let resolver: ItemPageResolver; + let itemService: any; + let store: any; + let router: any; + + const uuid = '1234-65487-12354-1235'; + let item: DSpaceObject; + + function runTestsWithEntityType(entityType: string) { + beforeEach(() => { + router = TestBed.inject(Router); + item = Object.assign(new DSpaceObject(), { + uuid: uuid, + firstMetadataValue(_keyOrKeys: string | string[], _valueFilter?: MetadataValueFilter): string { + return entityType; + } + }); + itemService = { + findById: (_id: string) => createSuccessfulRemoteDataObject$(item) + }; + store = jasmine.createSpyObj('store', { + dispatch: {}, + }); + resolver = new ItemPageResolver(itemService, store, router); + }); + + it('should redirect to the correct route for the entity type', (done) => { + spyOn(item, 'firstMetadataValue').and.returnValue(entityType); + spyOn(router, 'navigateByUrl').and.callThrough(); + + resolver.resolve({ params: { id: uuid } } as any, { url: router.parseUrl(`/items/${uuid}`).toString() } as any) + .pipe(first()) + .subscribe( + () => { + expect(router.navigateByUrl).toHaveBeenCalledWith(router.parseUrl(`/entities/${entityType}/${uuid}`).toString()); + done(); + } + ); + }); + + it('should not redirect if we’re already on the correct route', (done) => { + spyOn(item, 'firstMetadataValue').and.returnValue(entityType); + spyOn(router, 'navigateByUrl').and.callThrough(); + + resolver.resolve({ params: { id: uuid } } as any, { url: router.parseUrl(`/entities/${entityType}/${uuid}`).toString() } as any) + .pipe(first()) + .subscribe( + () => { + expect(router.navigateByUrl).not.toHaveBeenCalled(); + done(); + } + ); + }); + } + + describe('when normal entity type is provided', () => { + runTestsWithEntityType('publication'); + }); + + describe('when entity type contains a special character', () => { + runTestsWithEntityType('alligator,loki'); + runTestsWithEntityType('🐊'); + runTestsWithEntityType(' '); + }); + + }); +}); diff --git a/src/app/item-page/item-page.resolver.ts b/src/app/item-page/item-page.resolver.ts index 7edffc5357..e9b287406e 100644 --- a/src/app/item-page/item-page.resolver.ts +++ b/src/app/item-page/item-page.resolver.ts @@ -1,5 +1,5 @@ import { Injectable } from '@angular/core'; -import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { Observable } from 'rxjs'; import { RemoteData } from '../core/data/remote-data'; import { ItemDataService } from '../core/data/item-data.service'; @@ -35,8 +35,14 @@ export class ItemPageResolver extends ItemResolver { return super.resolve(route, state).pipe( map((rd: RemoteData) => { if (rd.hasSucceeded && hasValue(rd.payload)) { - const itemRoute = getItemPageRoute(rd.payload); const thisRoute = state.url; + + // Angular uses a custom function for encodeURIComponent, (e.g. it doesn't encode commas + // or semicolons) and thisRoute has been encoded with that function. If we want to compare + // it with itemRoute, we have to run itemRoute through Angular's version as well to ensure + // the same characters are encoded the same way. + const itemRoute = this.router.parseUrl(getItemPageRoute(rd.payload)).toString(); + if (!thisRoute.startsWith(itemRoute)) { const itemId = rd.payload.uuid; const subRoute = thisRoute.substring(thisRoute.indexOf(itemId) + itemId.length, thisRoute.length); From 881af6449542ee174206d5b10ec27b87f0254219 Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Tue, 17 May 2022 16:36:40 +0200 Subject: [PATCH 25/37] [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 618ff0ce19607900bfd165aec9ae7ef01a9b19a2 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Tue, 17 May 2022 17:45:27 +0200 Subject: [PATCH 26/37] [CST-5303] added missing labels --- src/assets/i18n/en.json5 | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index bb2bcf8ebc..5bae41d0a1 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3644,6 +3644,26 @@ "submission.sections.describe.relationship-lookup.external-source.import-button-title.isProjectOfPublication": "Project", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.none = "Import remote item", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Event": "Import remote event", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Product": "Import remote product", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Equipment": "Import remote equipment", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.OrgUnit = "Import remote organizational unit", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Funding": "Import remote fund", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Person = "Import remote person", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Patent": "Import remote patent", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Project = "Import remote project", + + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Publication = "Import remote publication", + "submission.sections.describe.relationship-lookup.external-source.import-modal.isProjectOfPublication.added.new-entity": "New Entity Added!", "submission.sections.describe.relationship-lookup.external-source.import-modal.isProjectOfPublication.title": "Project", From a732f1534de03fbdc89aad33af769dc50f40fb72 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Wed, 18 May 2022 09:33:08 +0200 Subject: [PATCH 27/37] [CST-5303] fix wrong format --- src/assets/i18n/en.json5 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 5bae41d0a1..dffbbd4b7e 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3644,7 +3644,7 @@ "submission.sections.describe.relationship-lookup.external-source.import-button-title.isProjectOfPublication": "Project", - "submission.sections.describe.relationship-lookup.external-source.import-button-title.none = "Import remote item", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.none": "Import remote item", "submission.sections.describe.relationship-lookup.external-source.import-button-title.Event": "Import remote event", @@ -3652,17 +3652,17 @@ "submission.sections.describe.relationship-lookup.external-source.import-button-title.Equipment": "Import remote equipment", - "submission.sections.describe.relationship-lookup.external-source.import-button-title.OrgUnit = "Import remote organizational unit", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.OrgUnit": "Import remote organizational unit", "submission.sections.describe.relationship-lookup.external-source.import-button-title.Funding": "Import remote fund", - "submission.sections.describe.relationship-lookup.external-source.import-button-title.Person = "Import remote person", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Person": "Import remote person", "submission.sections.describe.relationship-lookup.external-source.import-button-title.Patent": "Import remote patent", - "submission.sections.describe.relationship-lookup.external-source.import-button-title.Project = "Import remote project", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Project": "Import remote project", - "submission.sections.describe.relationship-lookup.external-source.import-button-title.Publication = "Import remote publication", + "submission.sections.describe.relationship-lookup.external-source.import-button-title.Publication": "Import remote publication", "submission.sections.describe.relationship-lookup.external-source.import-modal.isProjectOfPublication.added.new-entity": "New Entity Added!", From 50184c87848f6d9e69180b5a377346ab91caafbb Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 18 May 2022 19:27:23 +0200 Subject: [PATCH 28/37] [CST-5738] fix LGTM alert --- src/app/core/auth/auth-request.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index d0fec5fa71..4db4cba612 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/src/app/core/auth/auth-request.service.ts @@ -35,7 +35,7 @@ export abstract class AuthRequestService { protected getEndpointByMethod(endpoint: string, method: string, ...linksToFollow: FollowLinkConfig[]): string { let url = isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; - if (linksToFollow && linksToFollow.length > 0) { + if (linksToFollow?.length > 0) { linksToFollow.forEach((link: FollowLinkConfig, index: number) => { url += ((index === 0) ? '?' : '&') + `embed=${link.name}`; }); From 305203dba7fb2212171ed19df4d39895b235493b Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 19 May 2022 19:08:08 +0200 Subject: [PATCH 29/37] [CST-5738] hide special group heading when are not present --- src/app/profile-page/profile-page.component.html | 14 ++++++-------- .../profile-page/profile-page.component.spec.ts | 13 ++++++++++--- src/app/shared/testing/special-group.mock.ts | 1 + 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/app/profile-page/profile-page.component.html b/src/app/profile-page/profile-page.component.html index ffdf802a94..da31018cad 100644 --- a/src/app/profile-page/profile-page.component.html +++ b/src/app/profile-page/profile-page.component.html @@ -22,7 +22,7 @@
-
+

{{'profile.groups.head' | translate}}

  • {{group.name}}
    • @@ -31,13 +31,11 @@ -
    -
    -

    {{'profile.special.groups.head' | translate}}

    -
      -
    • {{specialGroup.name}}
      • -
    -
    +
    +

    {{'profile.special.groups.head' | translate}}

    +
      +
    • {{specialGroup.name}}
      • +
    diff --git a/src/app/profile-page/profile-page.component.spec.ts b/src/app/profile-page/profile-page.component.spec.ts index 1bc9a78ad5..46f83c964b 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -20,7 +20,7 @@ import { provideMockStore } from '@ngrx/store/testing'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; import { getTestScheduler } from 'jasmine-marbles'; import { By } from '@angular/platform-browser'; -import { SpecialGroupDataMock$ } from '../shared/testing/special-group.mock'; +import { EmptySpecialGroupDataMock$, SpecialGroupDataMock$ } from '../shared/testing/special-group.mock'; describe('ProfilePageComponent', () => { let component: ProfilePageComponent; @@ -240,14 +240,21 @@ describe('ProfilePageComponent', () => { describe('check for specialGroups', () => { it('should contains specialGroups list', () => { - const specialGroupsEle = fixture.debugElement.query(By.css('#specialGroups')); + const specialGroupsEle = fixture.debugElement.query(By.css('[data-test="specialGroups"]')); expect(specialGroupsEle).toBeTruthy(); }); it('should not contains specialGroups list', () => { component.specialGroupsRD$ = null; fixture.detectChanges(); - const specialGroupsEle = fixture.debugElement.query(By.css('#specialGroups')); + const specialGroupsEle = fixture.debugElement.query(By.css('[data-test="specialGroups"]')); + expect(specialGroupsEle).toBeFalsy(); + }); + + it('should not contains specialGroups list', () => { + component.specialGroupsRD$ = EmptySpecialGroupDataMock$; + fixture.detectChanges(); + const specialGroupsEle = fixture.debugElement.query(By.css('[data-test="specialGroups"]')); expect(specialGroupsEle).toBeFalsy(); }); }); diff --git a/src/app/shared/testing/special-group.mock.ts b/src/app/shared/testing/special-group.mock.ts index d8ec8a83cc..f1102e0584 100644 --- a/src/app/shared/testing/special-group.mock.ts +++ b/src/app/shared/testing/special-group.mock.ts @@ -50,6 +50,7 @@ export const SpecialGroupMock: Group = Object.assign(new Group(), { export const SpecialGroupDataMock: RemoteData> = createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock])); export const SpecialGroupDataMock$: Observable>> = createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [SpecialGroupMock2,SpecialGroupMock])); +export const EmptySpecialGroupDataMock$: Observable>> = createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])); From 383485b16ddbcb8e534862d0432c29998477f19f Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 20 May 2022 10:53:50 +0200 Subject: [PATCH 30/37] Modal outputs as EventEmitters instead of Subjects --- .../confirmation-modal.component.spec.ts | 24 +++++++++---------- .../confirmation-modal.component.ts | 9 ++++--- .../idle-modal/idle-modal.component.spec.ts | 12 +++++----- .../shared/idle-modal/idle-modal.component.ts | 7 +++--- .../item-versions-delete-modal.component.ts | 9 ++++--- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/app/shared/confirmation-modal/confirmation-modal.component.spec.ts b/src/app/shared/confirmation-modal/confirmation-modal.component.spec.ts index b2ef7cec99..d899dd8ef8 100644 --- a/src/app/shared/confirmation-modal/confirmation-modal.component.spec.ts +++ b/src/app/shared/confirmation-modal/confirmation-modal.component.spec.ts @@ -46,27 +46,27 @@ describe('ConfirmationModalComponent', () => { describe('confirmPressed', () => { beforeEach(() => { - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); component.confirmPressed(); }); it('should call the close method on the active modal', () => { expect(modalStub.close).toHaveBeenCalled(); }); - it('behaviour subject should have true as next', () => { - expect(component.response.next).toHaveBeenCalledWith(true); + it('behaviour subject should emit true', () => { + expect(component.response.emit).toHaveBeenCalledWith(true); }); }); describe('cancelPressed', () => { beforeEach(() => { - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); component.cancelPressed(); }); it('should call the close method on the active modal', () => { expect(modalStub.close).toHaveBeenCalled(); }); - it('behaviour subject should have false as next', () => { - expect(component.response.next).toHaveBeenCalledWith(false); + it('behaviour subject should emit false', () => { + expect(component.response.emit).toHaveBeenCalledWith(false); }); }); @@ -88,7 +88,7 @@ describe('ConfirmationModalComponent', () => { describe('when the click method emits on cancel button', () => { beforeEach(fakeAsync(() => { spyOn(component, 'close'); - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); debugElement.query(By.css('button.cancel')).triggerEventHandler('click', { preventDefault: () => {/**/ } @@ -99,15 +99,15 @@ describe('ConfirmationModalComponent', () => { it('should call the close method on the component', () => { expect(component.close).toHaveBeenCalled(); }); - it('behaviour subject should have false as next', () => { - expect(component.response.next).toHaveBeenCalledWith(false); + it('behaviour subject should emit false', () => { + expect(component.response.emit).toHaveBeenCalledWith(false); }); }); describe('when the click method emits on confirm button', () => { beforeEach(fakeAsync(() => { spyOn(component, 'close'); - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); debugElement.query(By.css('button.confirm')).triggerEventHandler('click', { preventDefault: () => {/**/ } @@ -118,8 +118,8 @@ describe('ConfirmationModalComponent', () => { it('should call the close method on the component', () => { expect(component.close).toHaveBeenCalled(); }); - it('behaviour subject should have true as next', () => { - expect(component.response.next).toHaveBeenCalledWith(true); + it('behaviour subject should emit false', () => { + expect(component.response.emit).toHaveBeenCalledWith(true); }); }); diff --git a/src/app/shared/confirmation-modal/confirmation-modal.component.ts b/src/app/shared/confirmation-modal/confirmation-modal.component.ts index c18025427a..4fa4858600 100644 --- a/src/app/shared/confirmation-modal/confirmation-modal.component.ts +++ b/src/app/shared/confirmation-modal/confirmation-modal.component.ts @@ -1,6 +1,5 @@ -import { Component, Input, Output } from '@angular/core'; +import { Component, EventEmitter, Input, Output } from '@angular/core'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; -import { Subject } from 'rxjs'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; @Component({ @@ -24,7 +23,7 @@ export class ConfirmationModalComponent { * An event fired when the cancel or confirm button is clicked, with respectively false or true */ @Output() - response: Subject = new Subject(); + response = new EventEmitter(); constructor(protected activeModal: NgbActiveModal) { } @@ -33,7 +32,7 @@ export class ConfirmationModalComponent { * Confirm the action that led to the modal */ confirmPressed() { - this.response.next(true); + this.response.emit(true); this.close(); } @@ -41,7 +40,7 @@ export class ConfirmationModalComponent { * Cancel the action that led to the modal and close modal */ cancelPressed() { - this.response.next(false); + this.response.emit(false); this.close(); } diff --git a/src/app/shared/idle-modal/idle-modal.component.spec.ts b/src/app/shared/idle-modal/idle-modal.component.spec.ts index 847bf6ac4f..7ea0b96d5b 100644 --- a/src/app/shared/idle-modal/idle-modal.component.spec.ts +++ b/src/app/shared/idle-modal/idle-modal.component.spec.ts @@ -46,7 +46,7 @@ describe('IdleModalComponent', () => { describe('extendSessionPressed', () => { beforeEach(fakeAsync(() => { - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); component.extendSessionPressed(); })); it('should set idle to false', () => { @@ -55,8 +55,8 @@ describe('IdleModalComponent', () => { it('should close the modal', () => { expect(modalStub.close).toHaveBeenCalled(); }); - it('response \'closed\' should have true as next', () => { - expect(component.response.next).toHaveBeenCalledWith(true); + it('response \'closed\' should emit true', () => { + expect(component.response.emit).toHaveBeenCalledWith(true); }); }); @@ -74,7 +74,7 @@ describe('IdleModalComponent', () => { describe('closePressed', () => { beforeEach(fakeAsync(() => { - spyOn(component.response, 'next'); + spyOn(component.response, 'emit'); component.closePressed(); })); it('should set idle to false', () => { @@ -83,8 +83,8 @@ describe('IdleModalComponent', () => { it('should close the modal', () => { expect(modalStub.close).toHaveBeenCalled(); }); - it('response \'closed\' should have true as next', () => { - expect(component.response.next).toHaveBeenCalledWith(true); + it('response \'closed\' should emit true', () => { + expect(component.response.emit).toHaveBeenCalledWith(true); }); }); diff --git a/src/app/shared/idle-modal/idle-modal.component.ts b/src/app/shared/idle-modal/idle-modal.component.ts index 35fafcf5cf..4873137ff1 100644 --- a/src/app/shared/idle-modal/idle-modal.component.ts +++ b/src/app/shared/idle-modal/idle-modal.component.ts @@ -1,8 +1,7 @@ -import { Component, OnInit, Output } from '@angular/core'; +import { Component, EventEmitter, OnInit, Output } from '@angular/core'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; import { environment } from '../../../environments/environment'; import { AuthService } from '../../core/auth/auth.service'; -import { Subject } from 'rxjs'; import { hasValue } from '../empty.util'; import { Store } from '@ngrx/store'; import { AppState } from '../../app.reducer'; @@ -29,7 +28,7 @@ export class IdleModalComponent implements OnInit { * An event fired when the modal is closed */ @Output() - response: Subject = new Subject(); + response = new EventEmitter(); constructor(private activeModal: NgbActiveModal, private authService: AuthService, @@ -84,6 +83,6 @@ export class IdleModalComponent implements OnInit { */ closeModal() { this.activeModal.close(); - this.response.next(true); + this.response.emit(true); } } diff --git a/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.ts b/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.ts index c6e83c65c6..3aa1bbd49f 100644 --- a/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.ts +++ b/src/app/shared/item/item-versions/item-versions-delete-modal/item-versions-delete-modal.component.ts @@ -1,6 +1,5 @@ -import { Component, Output } from '@angular/core'; +import { Component, EventEmitter, Output } from '@angular/core'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; -import { Subject } from 'rxjs'; @Component({ selector: 'ds-item-versions-delete-modal', @@ -12,7 +11,7 @@ export class ItemVersionsDeleteModalComponent { * An event fired when the cancel or confirm button is clicked, with respectively false or true */ @Output() - response: Subject = new Subject(); + response = new EventEmitter(); versionNumber: number; @@ -21,12 +20,12 @@ export class ItemVersionsDeleteModalComponent { } onModalClose() { - this.response.next(false); + this.response.emit(false); this.activeModal.dismiss(); } onModalSubmit() { - this.response.next(true); + this.response.emit(true); this.activeModal.close(); } From 51bbbb697e42291f2571dcf767df10fdfa36033f Mon Sep 17 00:00:00 2001 From: Davide Negretti Date: Fri, 20 May 2022 16:45:10 +0200 Subject: [PATCH 31/37] [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 32/37] [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 33/37] [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 34/37] [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, From 3e2cd387df8415f38655fbd81d5f53abbae712a1 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 23 May 2022 12:30:23 +0200 Subject: [PATCH 35/37] added missing configs --- src/assets/i18n/en.json5 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index dffbbd4b7e..006de7345c 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3622,6 +3622,16 @@ "submission.import-external.preview.title.Publication": "Publication Preview", + "submission.import-external.preview.title.none": "Item Preview", + + "submission.import-external.preview.title.Journal": "Journal Preview", + + "submission.import-external.preview.title.OrgUnit": "Organizational Unit Preview", + + "submission.import-external.preview.title.Person": "Person Preview", + + "submission.import-external.preview.title.Project": "Project Preview", + "submission.import-external.preview.subtitle": "The metadata below was imported from an external source. It will be pre-filled when you start the submission.", "submission.import-external.preview.button.import": "Start submission", From 7f6c17df75e36145d50665d4d4b3831371380c66 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 25 May 2022 15:02:38 +0200 Subject: [PATCH 36/37] add support for multiple platforms to docker build --- .github/workflows/docker.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 00ec2fa8f7..89de307516 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -41,6 +41,10 @@ jobs: - name: Setup Docker Buildx uses: docker/setup-buildx-action@v1 + # https://github.com/docker/setup-qemu-action + - name: Set up QEMU + uses: docker/setup-qemu-action@v2 + # https://github.com/docker/login-action - name: Login to DockerHub # Only login if not a PR, as PRs only trigger a Docker build and not a push @@ -70,6 +74,7 @@ jobs: with: context: . file: ./Dockerfile + platforms: linux/amd64,linux/arm64 # For pull requests, we run the Docker build (to ensure no PR changes break the build), # but we ONLY do an image push to DockerHub if it's NOT a PR push: ${{ github.event_name != 'pull_request' }} From d719a025977090b6a0231fcd2a02e834617a8e6e Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 27 May 2022 09:08:52 -0500 Subject: [PATCH 37/37] Only build PRs in AMD64, as a simple sanity check. --- .github/workflows/docker.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 89de307516..64303ca8bb 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -31,6 +31,10 @@ jobs: # We turn off 'latest' tag by default. TAGS_FLAVOR: | latest=false + # Architectures / Platforms for which we will build Docker images + # If this is a PR, we ONLY build for AMD64. For PRs we only do a sanity check test to ensure Docker builds work. + # If this is NOT a PR (e.g. a tag or merge commit), also build for ARM64. + PLATFORMS: linux/amd64${{ github.event_name != 'pull_request' && ', linux/arm64' || '' }} steps: # https://github.com/actions/checkout @@ -42,7 +46,7 @@ jobs: uses: docker/setup-buildx-action@v1 # https://github.com/docker/setup-qemu-action - - name: Set up QEMU + - name: Set up QEMU emulation to build for multiple architectures uses: docker/setup-qemu-action@v2 # https://github.com/docker/login-action @@ -74,7 +78,7 @@ jobs: with: context: . file: ./Dockerfile - platforms: linux/amd64,linux/arm64 + platforms: ${{ env.PLATFORMS }} # For pull requests, we run the Docker build (to ensure no PR changes break the build), # but we ONLY do an image push to DockerHub if it's NOT a PR push: ${{ github.event_name != 'pull_request' }}