diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 00ec2fa8f7..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 @@ -41,6 +45,10 @@ jobs: - name: Setup Docker Buildx uses: docker/setup-buildx-action@v1 + # https://github.com/docker/setup-qemu-action + - name: Set up QEMU emulation to build for multiple architectures + 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 +78,7 @@ jobs: with: context: . file: ./Dockerfile + 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' }} 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..09487a7eaa 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 @@ -2,8 +2,8 @@ import { CommonModule } from '@angular/common'; 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 { FormControl, FormGroup, FormsModule, ReactiveFormsModule, Validators } from '@angular/forms'; +import { BrowserModule, By } from '@angular/platform-browser'; import { ActivatedRoute, Router } from '@angular/router'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { Store } from '@ngrx/store'; @@ -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,46 @@ describe('GroupFormComponent', () => { }); }); + describe('delete', () => { + let deleteButton; + + 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(waitForAsync(() => { + deleteButton.click(); + fixture.detectChanges(); + (document as any).querySelector('.modal-footer .confirm').click(); + })); + + it('should call GroupDataService.delete', () => { + expect(groupsDataServiceStub.delete).toHaveBeenCalledWith('active-group'); + }); + }); + + describe('if canceled via modal', () => { + beforeEach(waitForAsync(() => { + deleteButton.click(); + fixture.detectChanges(); + (document as any).querySelector('.modal-footer .cancel').click(); + })); + + it('should not call GroupDataService.delete', () => { + 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..1770762a34 100644 --- a/src/app/access-control/group-registry/groups-registry.component.ts +++ b/src/app/access-control/group-registry/groups-registry.component.ts @@ -9,7 +9,7 @@ import { of as observableOf, Subscription } from 'rxjs'; -import { catchError, map, switchMap, take, tap } from 'rxjs/operators'; +import { catchError, map, switchMap, tap } from 'rxjs/operators'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; @@ -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..1876936efb 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 @@ -5,7 +5,6 @@ import { NotificationsService } from '../../shared/notifications/notifications.s import { CollectionDataService } from '../../core/data/collection-data.service'; import { Collection } from '../../core/shared/collection.model'; import { TranslateService } from '@ngx-translate/core'; -import { RequestService } from '../../core/data/request.service'; /** * Component that represents the page where a user can delete an existing Collection @@ -24,8 +23,7 @@ export class DeleteCollectionPageComponent extends DeleteComColPageComponent { success: {}, error: {} }); - const objectCache = jasmine.createSpyObj('objectCache', { - remove: {} - }); const requestService = jasmine.createSpyObj('requestService', { setStaleByHrefSubstring: {} }); @@ -65,8 +61,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 +90,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..d4396fce17 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 @@ -8,10 +8,9 @@ import { combineLatest as combineLatestObservable, Observable } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { Item } from '../../../core/shared/item.model'; import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators'; -import { switchMap, tap } from 'rxjs/operators'; +import { switchMap } from 'rxjs/operators'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { RequestService } from '../../../core/data/request.service'; import { getCollectionItemTemplateRoute } from '../../collection-page-routing-paths'; @@ -38,8 +37,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/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/delete-community-page/delete-community-page.component.ts b/src/app/community-page/delete-community-page/delete-community-page.component.ts index 0cccc503e1..6e640c64be 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 @@ -5,7 +5,6 @@ import { ActivatedRoute, Router } from '@angular/router'; import { DeleteComColPageComponent } from '../../shared/comcol/comcol-forms/delete-comcol-page/delete-comcol-page.component'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { RequestService } from '../../core/data/request.service'; /** * Component that represents the page where a user can delete an existing Community @@ -24,9 +23,8 @@ export class DeleteCommunityPageComponent extends DeleteComColPageComponent { @@ -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/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index da38d730a5..4db4cba612 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/src/app/core/auth/auth-request.service.ts @@ -12,13 +12,13 @@ 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 */ export abstract class AuthRequestService { protected linkName = 'authn'; - protected browseEndpoint = ''; protected shortlivedtokensEndpoint = 'shortlivedtokens'; constructor(protected halService: HALEndpointService, @@ -27,14 +27,21 @@ 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): string { - return isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; + protected getEndpointByMethod(endpoint: string, method: string, ...linksToFollow: FollowLinkConfig[]): string { + let url = isNotEmpty(method) ? `${endpoint}/${method}` : `${endpoint}`; + if (linksToFollow?.length > 0) { + linksToFollow.forEach((link: FollowLinkConfig, index: number) => { + url += ((index === 0) ? '?' : '&') + `embed=${link.name}`; + }); + } + + return url; } public postToEndpoint(method: string, body?: any, options?: HttpOptions): Observable> { @@ -48,14 +55,14 @@ export abstract class AuthRequestService { distinctUntilChanged()); } - public getRequest(method: string, options?: HttpOptions): 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.spec.ts b/src/app/core/auth/auth.service.spec.ts index 77cc015958..b38d17aecd 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 5ab0edf64f..999ea863df 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -44,13 +44,18 @@ 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'; +import { followLink } from '../../shared/utils/follow-link-config.model'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -211,6 +216,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, followLink('specialGroups')).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 fbe6ed6476..d18b1ccf9a 100644 --- a/src/app/core/auth/models/auth-status.model.ts +++ b/src/app/core/auth/models/auth-status.model.ts @@ -5,6 +5,8 @@ import { IDToUUIDSerializer } from '../../cache/id-to-uuid-serializer'; import { RemoteData } from '../../data/remote-data'; import { EPerson } from '../../eperson/models/eperson.model'; import { EPERSON } from '../../eperson/models/eperson.resource-type'; +import { Group } from '../../eperson/models/group.model'; +import { GROUP } from '../../eperson/models/group.resource-type'; import { HALLink } from '../../shared/hal-link.model'; import { ResourceType } from '../../shared/resource-type'; import { excludeFromEquals } from '../../utilities/equals.decorators'; @@ -13,6 +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 @@ -61,6 +64,7 @@ export class AuthStatus implements CacheableObject { _links: { self: HALLink; eperson: HALLink; + specialGroups: HALLink; }; /** @@ -70,6 +74,13 @@ export class AuthStatus implements CacheableObject { @link(EPERSON) eperson?: Observable>; + /** + * The SpecialGroup of this auth status + * Will be undefined unless the SpecialGroup {@link HALLink} has been resolved. + */ + @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/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.spec.ts b/src/app/core/cache/object-cache.service.spec.ts index bde6831967..f18c262524 100644 --- a/src/app/core/cache/object-cache.service.spec.ts +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -211,25 +211,69 @@ describe('ObjectCacheService', () => { }); }); - describe('has', () => { + describe('hasByHref', () => { + describe('with requestUUID not specified', () => { + describe('getByHref emits an object', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(cacheEntry)); + }); - describe('getByHref emits an object', () => { - beforeEach(() => { - spyOn(service, 'getByHref').and.returnValue(observableOf(cacheEntry)); + it('should return true', () => { + expect(service.hasByHref(selfLink)).toBe(true); + }); }); - it('should return true', () => { - expect(service.hasByHref(selfLink)).toBe(true); + describe('getByHref emits nothing', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(empty()); + }); + + it('should return false', () => { + expect(service.hasByHref(selfLink)).toBe(false); + }); }); }); - describe('getByHref emits nothing', () => { - beforeEach(() => { - spyOn(service, 'getByHref').and.returnValue(empty()); + describe('with requestUUID specified', () => { + describe('getByHref emits an object that includes the specified requestUUID', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, { + requestUUIDs: [ + 'something', + 'something-else', + 'specific-request', + ] + }))); + }); + + it('should return true', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(true); + }); }); - it('should return false', () => { - expect(service.hasByHref(selfLink)).toBe(false); + describe('getByHref emits an object that doesn\'t include the specified requestUUID', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, { + requestUUIDs: [ + 'something', + 'something-else', + ] + }))); + }); + + it('should return true', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(false); + }); + }); + + describe('getByHref emits nothing', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(empty()); + }); + + it('should return false', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(false); + }); }); }); }); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 6d48242178..cdf87e5c1a 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.includes(requestUUID); } 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/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index 81d683b37a..dffc97f294 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -22,6 +22,7 @@ import { import { BitstreamDataService } from './bitstream-data.service'; import { CoreState } from '../core-state.model'; import { FindListOptions } from './find-list-options.model'; +import { Bitstream } from '../shared/bitstream.model'; const LINK_NAME = 'test'; @@ -244,4 +245,75 @@ describe('ComColDataService', () => { }); }); }); + + 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/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index f680fed6a4..64efd58418 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'; @@ -25,9 +29,12 @@ import { RemoteData } from './remote-data'; import { RequestEntryState } from './request-entry-state.model'; import { CoreState } from '../core-state.model'; import { FindListOptions } from './find-list-options.model'; +import { fakeAsync, tick } from '@angular/core/testing'; const endpoint = 'https://rest.api/core'; +const BOOLEAN = { f: false, t: true }; + class TestService extends DataService { constructor( @@ -86,6 +93,9 @@ describe('DataService', () => { }, getObjectBySelfLink: () => { /* empty */ + }, + getByHref: () => { + /* empty */ } } as any; store = {} as Store; @@ -833,4 +843,149 @@ 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 call setStaleByUUID even if not subscribing to returned Observable', fakeAsync(() => { + service.invalidateByHref('some-href'); + tick(); + + expect(getByHrefSpy).toHaveBeenCalledWith('some-href'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); + })); + + 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(createSuccessfulRemoteDataObject$({})); + + service.delete('some-id', ['a', 'b', 'c']).subscribe(rd => { + expect(getIDHrefObsSpy).toHaveBeenCalledWith('some-id'); + expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href', ['a', 'b', 'c']); + }); + }); + + 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 call invalidateByHref even if not subscribing to returned Observable', fakeAsync(() => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.deleteByHref('some-href'); + tick(); + + expect(invalidateByHrefSpy).toHaveBeenCalled(); + })); + + 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..0759cb61ea 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 { AsyncSubject, combineLatest, from as observableFrom, 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'; @@ -21,11 +21,12 @@ import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { getClassForType } from '../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RequestParam } from '../cache/models/request-param.model'; +import { ObjectCacheEntry } from '../cache/object-cache.reducer'; 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 +580,38 @@ 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 { + const done$ = new AsyncSubject(); + + this.objectCache.getByHref(href).pipe( + switchMap((oce: ObjectCacheEntry) => observableFrom(oce.requestUUIDs).pipe( + mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)), + toArray(), + )), + ).subscribe(() => { + done$.next(true); + done$.complete(); + }); + + return done$; + } + /** * Delete an existing DSpace Object on the server * @param objectId The id of the object to be removed @@ -600,6 +633,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 +652,27 @@ export abstract class DataService implements UpdateDa } this.requestService.send(request); - return this.rdbService.buildFromRequestUUID(requestId); + const response$ = this.rdbService.buildFromRequestUUID(requestId); + + const invalidated$ = new AsyncSubject(); + response$.pipe( + getFirstCompletedRemoteData(), + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + return this.invalidateByHref(href); + } else { + return [true]; + } + }) + ).subscribe(() => { + invalidated$.next(true); + invalidated$.complete(); + }); + + return combineLatest([response$, invalidated$]).pipe( + filter(([_, invalidated]) => invalidated), + 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..2d5acb2cb3 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: RequestEntry) => isStale(request.state)), + filter((stale: boolean) => stale), + 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/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/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index 59316c0098..b788a16520 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', @@ -88,6 +92,8 @@ describe('ResourcePolicyService', () => { const resourcePolicyRD = createSuccessfulRemoteDataObject(resourcePolicy); const paginatedListRD = createSuccessfulRemoteDataObject(paginatedList); + const ePersonEndpoint = 'EPERSON_EP'; + beforeEach(() => { scheduler = getTestScheduler(); @@ -105,6 +111,7 @@ describe('ResourcePolicyService', () => { removeByHrefSubstring: {}, getByHref: observableOf(responseCacheEntry), getByUUID: observableOf(responseCacheEntry), + setStaleByHrefSubstring: {}, }); rdbService = jasmine.createSpyObj('rdbService', { buildSingle: hot('a|', { @@ -117,6 +124,11 @@ describe('ResourcePolicyService', () => { a: resourcePolicyRD }) }); + ePersonService = jasmine.createSpyObj('ePersonService', { + getBrowseEndpoint: hot('a', { + a: ePersonEndpoint + }), + }); objectCache = {} as ObjectCacheService; const notificationsService = {} as NotificationsService; const http = {} as HttpClient; @@ -129,7 +141,9 @@ describe('ResourcePolicyService', () => { halService, notificationsService, http, - comparator + comparator, + ePersonService, + groupService ); spyOn((service as any).dataService, 'create').and.callThrough(); @@ -320,4 +334,17 @@ describe('ResourcePolicyService', () => { expect(result).toBeObservable(expected); }); }); + + describe('updateTarget', () => { + it('should create a new PUT request for eperson', () => { + const targetType = 'eperson'; + + const result = service.updateTarget(resourcePolicyId, requestURL, epersonUUID, 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 065e58c53d..b0b7a6bd97 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, 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 { 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'; +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,44 @@ export class ResourcePolicyService { return this.dataService.searchBy(this.searchByResourceMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + /** + * Update the target of the resource policy + * @param resourcePolicyId the ID of the resource policy + * @param resourcePolicyHref the link to the resource policy + * @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, targetUUID: string, targetType: string): Observable> { + + const targetService = targetType === 'eperson' ? this.ePersonService : this.groupService; + + const targetEndpoint$ = targetService.getBrowseEndpoint().pipe( + take(1), + map((endpoint: string) =>`${endpoint}/${targetUUID}`), + ); + + 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(); + + this.requestService.setStaleByHrefSubstring(`${this.dataService.getLinkPath()}/${resourcePolicyId}/${targetType}`); + + targetEndpoint$.subscribe((targetEndpoint) => { + const resourceEndpoint = resourcePolicyHref + '/' + targetType; + const request = new PutRequest(requestId, resourceEndpoint, targetEndpoint, options); + Object.assign(request, { + getResponseParser(): GenericConstructor { + return StatusCodeOnlyResponseParsingService; + } + }); + this.requestService.send(request); + }); + + return this.rdbService.buildFromRequestUUID(requestId); + + } + } 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/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); diff --git a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts index eff51b1019..9aeea8b11e 100644 --- a/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/versioned-item/versioned-item.component.spec.ts @@ -2,7 +2,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { VersionedItemComponent } from './versioned-item.component'; import { VersionHistoryDataService } from '../../../../core/data/version-history-data.service'; -import { TranslateService } from '@ngx-translate/core'; +import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; import { VersionDataService } from '../../../../core/data/version-data.service'; import { NotificationsService } from '../../../../shared/notifications/notifications.service'; import { ItemVersionsSharedService } from '../../../../shared/item/item-versions/item-versions-shared.service'; @@ -19,6 +19,7 @@ import { SearchService } from '../../../../core/shared/search/search.service'; import { ItemDataService } from '../../../../core/data/item-data.service'; import { Version } from '../../../../core/shared/version.model'; import { RouteService } from '../../../../core/services/route.service'; +import { TranslateLoaderMock } from '../../../../shared/testing/translate-loader.mock'; const mockItem: Item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), [])), @@ -57,10 +58,17 @@ describe('VersionedItemComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ declarations: [VersionedItemComponent, DummyComponent], - imports: [RouterTestingModule], + imports: [ + RouterTestingModule, + TranslateModule.forRoot({ + loader: { + provide: TranslateLoader, + useClass: TranslateLoaderMock, + } + }), + ], providers: [ { provide: VersionHistoryDataService, useValue: versionHistoryServiceSpy }, - { provide: TranslateService, useValue: {} }, { provide: VersionDataService, useValue: versionServiceSpy }, { provide: NotificationsService, useValue: {} }, { provide: ItemVersionsSharedService, useValue: {} }, diff --git a/src/app/profile-page/profile-page.component.html b/src/app/profile-page/profile-page.component.html index ccfae0bba1..da31018cad 100644 --- a/src/app/profile-page/profile-page.component.html +++ b/src/app/profile-page/profile-page.component.html @@ -22,12 +22,21 @@ -
+

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

  • {{group.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 f48b894d8d..46f83c964b 100644 --- a/src/app/profile-page/profile-page.component.spec.ts +++ b/src/app/profile-page/profile-page.component.spec.ts @@ -20,6 +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 { EmptySpecialGroupDataMock$, SpecialGroupDataMock$ } from '../shared/testing/special-group.mock'; describe('ProfilePageComponent', () => { let component: ProfilePageComponent; @@ -54,7 +55,8 @@ describe('ProfilePageComponent', () => { }; authService = jasmine.createSpyObj('authService', { - getAuthenticatedUserFromStore: observableOf(user) + getAuthenticatedUserFromStore: observableOf(user), + getSpecialGroupsFromAuthStatus: SpecialGroupDataMock$ }); epersonService = jasmine.createSpyObj('epersonService', { findById: createSuccessfulRemoteDataObject$(user), @@ -235,4 +237,25 @@ describe('ProfilePageComponent', () => { }); }); }); + + describe('check for specialGroups', () => { + it('should contains specialGroups list', () => { + 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('[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/profile-page/profile-page.component.ts b/src/app/profile-page/profile-page.component.ts index fece166a59..7623e9e6ea 100644 --- a/src/app/profile-page/profile-page.component.ts +++ b/src/app/profile-page/profile-page.component.ts @@ -9,11 +9,7 @@ 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'; @@ -45,6 +41,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 +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$ = this.authService.getSpecialGroupsFromAuthStatus(); } /** 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..94f46088aa 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 @@ -7,7 +7,6 @@ import { NotificationsService } from '../../../notifications/notifications.servi import { TranslateService } from '@ngx-translate/core'; import { getFirstCompletedRemoteData } from '../../../../core/shared/operators'; import { NoContent } from '../../../../core/shared/NoContent.model'; -import { RequestService } from '../../../../core/data/request.service'; import { ComColDataService } from '../../../../core/data/comcol-data.service'; import { Community } from '../../../../core/shared/community.model'; import { Collection } from '../../../../core/shared/collection.model'; @@ -41,7 +40,6 @@ export class DeleteComColPageComponent i protected route: ActivatedRoute, protected notifications: NotificationsService, protected translate: TranslateService, - protected requestService: RequestService ) { } @@ -61,7 +59,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/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/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.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} }}

- - +
+
+
+
+ +
+
+

+

+
+
+
+
+ +
+ 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 5cc6397118..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 @@ -242,6 +242,7 @@ describe('ResourcePolicyFormComponent test suite', () => { fixture = TestBed.createComponent(ResourcePolicyFormComponent); comp = fixture.componentInstance; compAsAny = fixture.componentInstance; + compAsAny.resourcePolicy = resourcePolicy; comp.isProcessing = observableOf(false); }); @@ -253,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(); @@ -261,12 +264,12 @@ 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); }); it('should can set grant', () => { - expect(comp.canSetGrant()).toBeTruthy(); + expect(comp.isBeingEdited()).toBeTruthy(); }); it('should not have a target name', () => { @@ -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.canSetGrant()).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 7ae699340e..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 @@ -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,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 { NgbModal, NgbNavChangeEvent } from '@ng-bootstrap/ng-bootstrap'; export interface ResourcePolicyEvent { object: ResourcePolicy; @@ -48,6 +49,7 @@ export interface ResourcePolicyEvent { type: string, uuid: string }; + updateTarget: boolean; } @Component({ @@ -83,6 +85,8 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ @Output() submit: EventEmitter = new EventEmitter(); + @ViewChild('content') content: ElementRef; + /** * The form id * @type {string} @@ -125,6 +129,10 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { */ private subs: Subscription[] = []; + navActiveId: string; + + resourcePolicyTargetUpdated = false; + /** * Initialize instance variables * @@ -133,6 +141,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { * @param {FormService} formService * @param {GroupDataService} groupService * @param {RequestService} requestService + * @param modalService */ constructor( private dsoNameService: DSONameService, @@ -140,6 +149,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { private formService: FormService, private groupService: GroupDataService, private requestService: RequestService, + private modalService: NgbModal, ) { } @@ -151,7 +161,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { this.formId = this.formService.getUniqueId('resource-policy-form'); this.formModel = this.buildResourcePolicyForm(); - if (!this.canSetGrant()) { + if (this.isBeingEdited()) { const epersonRD$ = this.ePersonService.findByHref(this.resourcePolicy._links.eperson.href, false).pipe( getFirstSucceededRemoteData() ); @@ -169,6 +179,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { filter(() => this.isActive), ).subscribe((dsoRD: RemoteData) => { this.resourcePolicyGrant = dsoRD.payload; + this.navActiveId = String(dsoRD.payload.type); this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName()); }) ); @@ -193,19 +204,12 @@ 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 - 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 - 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( @@ -255,8 +259,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); } /** @@ -272,8 +276,10 @@ 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()); } /** @@ -297,6 +303,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { type: this.resourcePolicyGrantType, uuid: this.resourcePolicyGrant.id }; + eventPayload.updateTarget = this.resourcePolicyTargetUpdated; this.submit.emit(eventPayload); }); } @@ -329,4 +336,12 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy { .filter((subscription) => hasValue(subscription)) .forEach((subscription) => subscription.unsubscribe()); } + + onNavChange(changeEvent: NgbNavChangeEvent) { + // if a policy is being edited it should not be possible to switch between group and eperson + if (this.isBeingEdited()) { + changeEvent.preventDefault(); + this.modalService.open(this.content); + } + } } diff --git a/src/app/shared/resource-policies/resource-policies.component.spec.ts b/src/app/shared/resource-policies/resource-policies.component.spec.ts index 894949ba47..fc60229ff4 100644 --- a/src/app/shared/resource-policies/resource-policies.component.spec.ts +++ b/src/app/shared/resource-policies/resource-policies.component.spec.ts @@ -343,6 +343,16 @@ describe('ResourcePoliciesComponent test suite', () => { fixture.detectChanges(); }); + it('should call ResourcePolicyService.delete for the checked policies', () => { + resourcePolicyService.delete.and.returnValue(observableOf(true)); + scheduler = getTestScheduler(); + scheduler.schedule(() => comp.deleteSelectedResourcePolicies()); + scheduler.flush(); + + // only the first one is checked + expect(resourcePolicyService.delete).toHaveBeenCalledWith(resourcePolicy.id); + }); + it('should notify success when delete is successful', () => { resourcePolicyService.delete.and.returnValue(observableOf(true)); diff --git a/src/app/shared/resource-policies/resource-policies.component.ts b/src/app/shared/resource-policies/resource-policies.component.ts index 94c928438a..9313d23c0b 100644 --- a/src/app/shared/resource-policies/resource-policies.component.ts +++ b/src/app/shared/resource-policies/resource-policies.component.ts @@ -157,7 +157,6 @@ export class ResourcePoliciesComponent implements OnInit, OnDestroy { } else { this.notificationsService.error(null, this.translate.get('resource-policies.delete.failure.content')); } - this.requestService.setStaleByHrefSubstring(this.resourceUUID); this.processingDelete$.next(false); }) ); diff --git a/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.spec.ts b/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.spec.ts index 94e05b64fe..fadde46e53 100644 --- a/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.spec.ts +++ b/src/app/shared/search/search-switch-configuration/search-switch-configuration.component.spec.ts @@ -84,7 +84,7 @@ describe('SearchSwitchConfigurationComponent', () => { expect(childElements.length).toEqual(comp.configurationList.length); }); - it('should call onSelect method when selecting an option', () => { + it('should call onSelect method when selecting an option', waitForAsync(() => { fixture.whenStable().then(() => { spyOn(comp, 'onSelect'); select = fixture.debugElement.query(By.css('select')); @@ -94,8 +94,7 @@ describe('SearchSwitchConfigurationComponent', () => { fixture.detectChanges(); expect(comp.onSelect).toHaveBeenCalled(); }); - - }); + })); it('should navigate to the route when selecting an option', () => { spyOn((comp as any), 'getSearchLinkParts').and.returnValue([MYDSPACE_ROUTE]); 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..f1102e0584 --- /dev/null +++ b/src/app/shared/testing/special-group.mock.ts @@ -0,0 +1,56 @@ +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, + 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 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(), [])); + + + diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 520d38cf69..8b162b2adb 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1105,10 +1105,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", @@ -2165,6 +2169,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:", @@ -2886,6 +2906,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", @@ -3139,6 +3161,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", @@ -3161,6 +3187,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", @@ -3575,6 +3611,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", @@ -3589,10 +3641,24 @@ "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.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", @@ -3615,6 +3681,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", @@ -3831,6 +3917,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", diff --git a/src/test.ts b/src/test.ts index dfafd98807..477195418b 100644 --- a/src/test.ts +++ b/src/test.ts @@ -7,6 +7,7 @@ import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; declare const require: any; @@ -17,9 +18,11 @@ getTestBed().initTestEnvironment( { teardown: { destroyAfterEach: false } } ); -// If store is mocked, reset state after each test (see https://ngrx.io/guide/migration/v13) jasmine.getEnv().afterEach(() => { + // If store is mocked, reset state after each test (see https://ngrx.io/guide/migration/v13) getTestBed().inject(MockStore, null)?.resetSelectors(); + // Close any leftover modals + getTestBed().inject(NgbModal, null)?.dismissAll?.(); }); // Then we find all the tests.