diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts index eab9114900..30c490a45c 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.spec.ts @@ -42,6 +42,7 @@ describe('GroupFormComponent', () => { let ePersonDataServiceStub: any; let groupsDataServiceStub: any; let authorizationService: AuthorizationDataService; + let notificationService: NotificationsServiceStub; let router; let groups; @@ -76,6 +77,9 @@ describe('GroupFormComponent', () => { editGroup(group: Group) { this.activeGroup = group }, + updateGroup(group: Group) { + return null; + }, cancelEditGroup(): void { this.activeGroup = null; }, @@ -96,6 +100,7 @@ describe('GroupFormComponent', () => { builderService = getMockFormBuilderService(); translateService = getMockTranslateService(); router = new RouterMock(); + notificationService = new NotificationsServiceStub(); TestBed.configureTestingModule({ imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule, TranslateModule.forRoot({ @@ -109,7 +114,7 @@ describe('GroupFormComponent', () => { providers: [GroupFormComponent, { provide: EPersonDataService, useValue: ePersonDataServiceStub }, { provide: GroupDataService, useValue: groupsDataServiceStub }, - { provide: NotificationsService, useValue: new NotificationsServiceStub() }, + { provide: NotificationsService, useValue: notificationService }, { provide: FormBuilderService, useValue: builderService }, { provide: DSOChangeAnalyzer, useValue: {} }, { provide: HttpClient, useValue: {} }, @@ -154,6 +159,34 @@ describe('GroupFormComponent', () => { }); })); }); + describe('with active Group', () => { + beforeEach(() => { + spyOn(groupsDataServiceStub, 'getActiveGroup').and.returnValue(observableOf(expected)); + spyOn(groupsDataServiceStub, 'updateGroup').and.returnValue(observableOf(new RestResponse(true, 200, 'OK'))); + component.groupName.value = 'newGroupName'; + component.onSubmit(); + fixture.detectChanges(); + }); + + it('should emit the existing group using the correct new values', async(() => { + const expected2 = Object.assign(new Group(), { + name: 'newGroupName', + metadata: { + 'dc.description': [ + { + value: groupDescription + } + ], + }, + }); + fixture.whenStable().then(() => { + expect(component.submitForm.emit).toHaveBeenCalledWith(expected2); + }); + })); + it('should emit success notification', () => { + expect(notificationService.success).toHaveBeenCalled(); + }) + }); }); }); diff --git a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts index 55d520cafb..4346fed9c7 100644 --- a/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts +++ b/src/app/+admin/admin-access-control/group-registry/group-form/group-form.component.ts @@ -193,7 +193,7 @@ export class GroupFormComponent implements OnInit, OnDestroy { if (group === null) { this.createNewGroup(values); } else { - this.editGroup(group, values); + this.editGroup(group); } } ); @@ -246,14 +246,37 @@ export class GroupFormComponent implements OnInit, OnDestroy { } /** - * // TODO - * @param group - * @param values + * Edit existing Group based on given values from form and old Group + * @param group Group to edit and old values contained within */ - editGroup(group: Group, values) { - // TODO (backend) - console.log('TODO implement editGroup', values); - this.notificationsService.error('TODO implement editGroup (not yet implemented in backend) '); + editGroup(group: Group) { + const editedGroup = Object.assign(new Group(), { + id: group.id, + metadata: { + 'dc.description': [ + { + value: (hasValue(this.groupDescription.value) ? this.groupDescription.value : group.firstMetadataValue('dc.description')) + } + ], + }, + name: (hasValue(this.groupName.value) ? this.groupName.value : group.name), + _links: group._links, + }); + const response = this.groupDataService.updateGroup(editedGroup); + response.pipe(take(1)).subscribe((restResponse: RestResponse) => { + console.log('resp', restResponse) + if (restResponse.isSuccessful) { + this.notificationsService.success(this.translateService.get(this.messagePrefix + '.notification.edited.success', { name: editedGroup.name })); + this.submitForm.emit(editedGroup); + } else { + this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.edited.failure', { name: editedGroup.name })); + this.cancelForm.emit(); + } + }); + + if (this.groupName.value != null && this.groupName.value !== group.name) { + this.showNotificationIfNameInUse(editedGroup, 'edited'); + } } /** diff --git a/src/app/core/eperson/group-data.service.ts b/src/app/core/eperson/group-data.service.ts index 4543f33cf0..be89afb7cc 100644 --- a/src/app/core/eperson/group-data.service.ts +++ b/src/app/core/eperson/group-data.service.ts @@ -2,6 +2,7 @@ import { HttpClient, HttpHeaders } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { createSelector, select, Store } from '@ngrx/store'; +import { Operation } from 'fast-json-patch/lib/core'; import { Observable } from 'rxjs'; import { filter, map, take, tap } from 'rxjs/operators'; import { @@ -21,12 +22,19 @@ import { DataService } from '../data/data.service'; import { DSOChangeAnalyzer } from '../data/dso-change-analyzer.service'; import { PaginatedList } from '../data/paginated-list'; import { RemoteData } from '../data/remote-data'; -import { CreateRequest, DeleteRequest, FindListOptions, FindListRequest, PostRequest } from '../data/request.models'; +import { + CreateRequest, + DeleteRequest, + FindListOptions, + FindListRequest, + PatchRequest, + PostRequest +} from '../data/request.models'; import { RequestService } from '../data/request.service'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { getResponseFromEntry } from '../shared/operators'; +import { getRemoteDataPayload, getResponseFromEntry, getSucceededRemoteData } from '../shared/operators'; import { EPerson } from './models/eperson.model'; import { Group } from './models/group.model'; import { dataService } from '../cache/builders/build-decorators'; @@ -135,26 +143,41 @@ export class GroupDataService extends DataService { } /** - * Create or Update a group - * If the group contains an id, it is assumed the eperson already exists and is updated instead - * @param group The group to create or update + * Add a new patch to the object cache + * The patch is derived from the differences between the given object and its version in the object cache + * @param group The group with changes */ - public createOrUpdateGroup(group: Group): Observable> { - const isUpdate = hasValue(group.id); - if (isUpdate) { - return this.updateGroup(group); - } else { - return this.create(group, null); - } + updateGroup(group: Group): Observable { + const requestId = this.requestService.generateRequestId(); + const oldVersion$ = this.findByHref(group._links.self.href); + oldVersion$.pipe( + getSucceededRemoteData(), + getRemoteDataPayload(), + map((oldGroup: Group) => { + const operations = this.generateOperations(oldGroup, group); + const patchRequest = new PatchRequest(requestId, group._links.self.href, operations); + return this.requestService.configure(patchRequest); + }), + take(1) + ).subscribe(); + + return this.fetchResponse(requestId); } /** - * // TODO - * @param {DSpaceObject} ePerson The given object + * Metadata operations are generated by the difference between old and new Group + * Custom replace operation for the other group Name value + * @param oldGroup + * @param newGroup */ - updateGroup(group: Group): Observable> { - // TODO - return null; + private generateOperations(oldGroup: Group, newGroup: Group): Operation[] { + let operations = this.comparator.diff(oldGroup, newGroup).filter((operation: Operation) => operation.op === 'replace'); + if (hasValue(oldGroup.name) && oldGroup.name !== newGroup.name) { + operations = [...operations, { + op: 'replace', path: '/name', value: newGroup.name + }]; + } + return operations; } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 90efbe3546..3584382c2e 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -327,6 +327,12 @@ "admin.access-control.groups.form.notification.created.failure.groupNameInUse": "Failed to create Group with name: \"{{name}}\", make sure the name is not already in use.", + "admin.access-control.groups.form.notification.edited.failure": "Failed to edit Group \"{{name}}\"", + + "admin.access-control.groups.form.notification.edited.failure.groupNameInUse": "Name \"{{name}}\" already in use!", + + "admin.access-control.groups.form.notification.edited.success": "Successfully edited Group \"{{name}}\"", + "admin.access-control.groups.form.actions.delete": "Delete Group", "admin.access-control.groups.form.delete-group.modal.header": "Delete Group \"{{ dsoName }}\"",