From f167d5a62911596df404d20c5a1d9088b0c8ab2e Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 1 Sep 2020 14:25:50 +0200 Subject: [PATCH] fix issue where combining entities and authority control in the same field wouldn't work --- .../models/lookup/dynamic-lookup.component.ts | 1 + .../objects/submission-objects.effects.ts | 59 +++++-------------- ...ubmission-section-cc-licenses.component.ts | 8 ++- .../section-container.component.html | 1 + .../sections/form/section-form.component.ts | 4 +- .../submission/sections/sections.directive.ts | 9 ++- .../sections/sections.service.spec.ts | 17 ++++-- .../submission/sections/sections.service.ts | 35 +++++++++-- 8 files changed, 76 insertions(+), 58 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts index 3d946caa5e..90319ee64d 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.ts @@ -81,6 +81,7 @@ export class DsDynamicLookupComponent extends DsDynamicVocabularyComponent imple */ public hasAuthorityValue() { return hasValue(this.model.value) + && typeof this.model.value === 'object' && this.model.value.hasAuthority(); } diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 0f4158a760..2a69a61a8c 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -2,10 +2,19 @@ import { Injectable } from '@angular/core'; import { Actions, Effect, ofType } from '@ngrx/effects'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; -import { union } from 'lodash'; +import { isEqual, union } from 'lodash'; import { from as observableFrom, Observable, of as observableOf } from 'rxjs'; -import { catchError, filter, map, mergeMap, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; +import { + catchError, + filter, + map, + mergeMap, + switchMap, + take, + tap, + withLatestFrom +} from 'rxjs/operators'; import { SubmissionObject } from '../../core/submission/models/submission-object.model'; import { WorkflowItem } from '../../core/submission/models/workflowitem.model'; import { WorkspaceitemSectionUploadObject } from '../../core/submission/models/workspaceitem-section-upload.model'; @@ -49,8 +58,6 @@ import { RemoteData } from '../../core/data/remote-data'; import { getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'; import { SubmissionObjectDataService } from '../../core/submission/submission-object-data.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; -import { normalizeSectionData } from '../../core/submission/submission-response-parsing.service'; -import { difference } from '../../shared/object.util'; @Injectable() export class SubmissionObjectEffects { @@ -72,9 +79,7 @@ export class SubmissionObjectEffects { if (sectionDefinition.sectionType !== SectionsType.SubmissionForm) { sectionData = (isNotUndefined(action.payload.sections) && isNotUndefined(action.payload.sections[sectionId])) ? action.payload.sections[sectionId] : Object.create(null); } else { - // Normalize item metadata before to init section - // TODO to review after https://github.com/DSpace/dspace-angular/issues/818 is resolved - sectionData = normalizeSectionData(action.payload.item.metadata); + sectionData = action.payload.item.metadata; } const sectionErrors = null; mappedActions.push( @@ -253,7 +258,7 @@ export class SubmissionObjectEffects { @Effect() addAllMetadataToSectionData = this.actions$.pipe( ofType(SubmissionObjectActionTypes.UPDATE_SECTION_DATA), switchMap((action: UpdateSectionDataAction) => { - return this.sectionService.getSectionState(action.payload.submissionId, action.payload.sectionId) + return this.sectionService.getSectionState(action.payload.submissionId, action.payload.sectionId, SectionsType.Upload) .pipe(map((section: SubmissionSectionObject) => [action, section]), take(1)); }), filter(([action, section]: [UpdateSectionDataAction, SubmissionSectionObject]) => section.sectionType === SectionsType.SubmissionForm), @@ -271,15 +276,8 @@ export class SubmissionObjectEffects { return item$.pipe( map((item: Item) => item.metadata), - map((metadata: any) => { - if (!this.isEqual(action.payload.data, normalizeSectionData(metadata))) { - // Normalize item metadata before to update section - // TODO to review after https://github.com/DSpace/dspace-angular/issues/818 is resolved - return new UpdateSectionDataAction(action.payload.submissionId, action.payload.sectionId, normalizeSectionData(metadata), action.payload.errors) - } else { - return new UpdateSectionDataSuccessAction(); - } - }) + filter((metadata) => !isEqual(action.payload.data, metadata)), + map((metadata: any) => new UpdateSectionDataAction(action.payload.submissionId, action.payload.sectionId, metadata, action.payload.errors)) ); } else { return observableOf(new UpdateSectionDataSuccessAction()); @@ -397,31 +395,4 @@ export class SubmissionObjectEffects { } return mappedActions; } - - /** - * Check if the section data has been enriched by the server - * - * @param sectionData - * the section metadata retrieved from the server - * @param itemData - * the item data retrieved from the server - */ - isEqual(sectionData: any, itemData: any): boolean { - const diffResult = []; - - // compare current form data state with section data retrieved from store - const diffObj = difference(sectionData, itemData); - - // iterate over differences to check whether they are actually different - Object.keys(diffObj) - .forEach((key) => { - diffObj[key].forEach((value) => { - if (value.hasOwnProperty('value')) { - diffResult.push(value); - } - }); - }); - return isEmpty(diffResult); - } - } diff --git a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts index d455bd5e22..9e0cfa9efa 100644 --- a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts +++ b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts @@ -1,6 +1,10 @@ import { Component, Inject } from '@angular/core'; import { Observable, of as observableOf, Subscription } from 'rxjs'; -import { Field, Option, SubmissionCcLicence } from '../../../core/submission/models/submission-cc-license.model'; +import { + Field, + Option, + SubmissionCcLicence +} from '../../../core/submission/models/submission-cc-license.model'; import { getRemoteDataPayload, getSucceededRemoteData } from '../../../core/shared/operators'; import { distinctUntilChanged, filter, map, take } from 'rxjs/operators'; import { SubmissionCcLicenseDataService } from '../../../core/submission/submission-cc-license-data.service'; @@ -228,7 +232,7 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent onSectionInit(): void { this.pathCombiner = new JsonPatchOperationPathCombiner('sections', this.sectionData.id); this.subscriptions.push( - this.sectionService.getSectionState(this.submissionId, this.sectionData.id).pipe( + this.sectionService.getSectionState(this.submissionId, this.sectionData.id, SectionsType.CcLicense).pipe( filter((sectionState) => { return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errors)) }), diff --git a/src/app/submission/sections/container/section-container.component.html b/src/app/submission/sections/container/section-container.component.html index fb29f606e6..8262d51f6f 100644 --- a/src/app/submission/sections/container/section-container.component.html +++ b/src/app/submission/sections/container/section-container.component.html @@ -3,6 +3,7 @@ [ngClass]="{ 'section-focus' : sectionRef.isSectionActive() }" [mandatory]="sectionData.mandatory" [submissionId]="submissionId" + [sectionType]="sectionData.sectionType" [sectionId]="sectionData.id"> this.formConfig = config), flatMap(() => observableCombineLatest( - this.sectionService.getSectionData(this.submissionId, this.sectionData.id), + this.sectionService.getSectionData(this.submissionId, this.sectionData.id, this.sectionData.sectionType), this.submissionObjectService.getHrefByID(this.submissionId).pipe(take(1)).pipe( switchMap((href: string) => { this.objectCache.remove(href); @@ -318,7 +318,7 @@ export class SubmissionSectionformComponent extends SectionModelComponent { /** * Subscribe to section state */ - this.sectionService.getSectionState(this.submissionId, this.sectionData.id).pipe( + this.sectionService.getSectionState(this.submissionId, this.sectionData.id, this.sectionData.sectionType).pipe( filter((sectionState: SubmissionSectionObject) => { return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errors)) }), diff --git a/src/app/submission/sections/sections.directive.ts b/src/app/submission/sections/sections.directive.ts index 0efb7225aa..1b0a6bfbc6 100644 --- a/src/app/submission/sections/sections.directive.ts +++ b/src/app/submission/sections/sections.directive.ts @@ -9,6 +9,7 @@ import { hasValue, isNotEmpty, isNotNull } from '../../shared/empty.util'; import { SubmissionSectionError, SubmissionSectionObject } from '../objects/submission-objects.reducer'; import parseSectionErrorPaths, { SectionErrorPath } from '../utils/parseSectionErrorPaths'; import { SubmissionService } from '../submission.service'; +import { SectionsType } from './sections-type'; /** * Directive for handling generic section functionality @@ -31,6 +32,12 @@ export class SectionsDirective implements OnDestroy, OnInit { */ @Input() sectionId: string; + /** + * The section type + * @type {SectionsType} + */ + @Input() sectionType: SectionsType; + /** * The submission id * @type {string} @@ -104,7 +111,7 @@ export class SectionsDirective implements OnDestroy, OnInit { })); this.subs.push( - this.sectionService.getSectionState(this.submissionId, this.sectionId).pipe( + this.sectionService.getSectionState(this.submissionId, this.sectionId, this.sectionType).pipe( map((state: SubmissionSectionObject) => state.errors)) .subscribe((errors: SubmissionSectionError[]) => { if (isNotEmpty(errors)) { diff --git a/src/app/submission/sections/sections.service.spec.ts b/src/app/submission/sections/sections.service.spec.ts index e5cb3ddc09..5c7bff13ce 100644 --- a/src/app/submission/sections/sections.service.spec.ts +++ b/src/app/submission/sections/sections.service.spec.ts @@ -14,7 +14,11 @@ import { NotificationsServiceStub } from '../../shared/testing/notifications-ser import { SubmissionServiceStub } from '../../shared/testing/submission-service.stub'; import { getMockTranslateService } from '../../shared/mocks/translate.service.mock'; import { SectionsService } from './sections.service'; -import { mockSectionsData, mockSectionsErrors, mockSubmissionState } from '../../shared/mocks/submission.mock'; +import { + mockSectionsData, + mockSectionsErrors, + mockSubmissionState +} from '../../shared/mocks/submission.mock'; import { DisableSectionAction, EnableSectionAction, @@ -23,12 +27,17 @@ import { SectionStatusChangeAction, UpdateSectionDataAction } from '../objects/submission-objects.actions'; -import { FormAddError, FormClearErrorsAction, FormRemoveErrorAction } from '../../shared/form/form.actions'; +import { + FormAddError, + FormClearErrorsAction, + FormRemoveErrorAction +} from '../../shared/form/form.actions'; import parseSectionErrors from '../utils/parseSectionErrors'; import { SubmissionScopeType } from '../../core/submission/submission-scope-type'; import { SubmissionSectionError } from '../objects/submission-objects.reducer'; import { getMockScrollToService } from '../../shared/mocks/scroll-to-service.mock'; import { storeModuleConfig } from '../../app.reducer'; +import { SectionsType } from './sections-type'; describe('SectionsService test suite', () => { let notificationsServiceStub: NotificationsServiceStub; @@ -151,7 +160,7 @@ describe('SectionsService test suite', () => { b: sectionData[sectionId] }); - expect(service.getSectionData(submissionId, sectionId)).toBeObservable(expected); + expect(service.getSectionData(submissionId, sectionId, SectionsType.SubmissionForm)).toBeObservable(expected); }); }); @@ -175,7 +184,7 @@ describe('SectionsService test suite', () => { b: sectionState }); - expect(service.getSectionState(submissionId, sectionId)).toBeObservable(expected); + expect(service.getSectionState(submissionId, sectionId, SectionsType.SubmissionForm)).toBeObservable(expected); }); }); diff --git a/src/app/submission/sections/sections.service.ts b/src/app/submission/sections/sections.service.ts index 52ae941893..9d7fee3bb3 100644 --- a/src/app/submission/sections/sections.service.ts +++ b/src/app/submission/sections/sections.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { combineLatest, Observable } from 'rxjs'; -import { distinctUntilChanged, filter, map, take } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map, take, tap } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; import { ScrollToConfigOptions, ScrollToService } from '@nicky-lenaers/ngx-scroll-to'; @@ -25,6 +25,8 @@ import { FormAddError, FormClearErrorsAction, FormRemoveErrorAction } from '../. import { NotificationsService } from '../../shared/notifications/notifications.service'; import { SubmissionService } from '../submission.service'; import { WorkspaceitemSectionDataType } from '../../core/submission/models/workspaceitem-sections.model'; +import { SectionsType } from './sections-type'; +import { normalizeSectionData } from '../../core/submission/submission-response-parsing.service'; /** * A service that provides methods used in submission process. @@ -129,12 +131,23 @@ export class SectionsService { * The submission id * @param sectionId * The section id + * @param sectionType + * The type of section to retrieve * @return Observable * observable of [WorkspaceitemSectionDataType] */ - public getSectionData(submissionId: string, sectionId: string): Observable { + public getSectionData(submissionId: string, sectionId: string, sectionType: SectionsType): Observable { return this.store.select(submissionSectionDataFromIdSelector(submissionId, sectionId)).pipe( - distinctUntilChanged()); + map((sectionData: WorkspaceitemSectionDataType) => { + if (sectionType === SectionsType.SubmissionForm) { + return normalizeSectionData(sectionData) + } + else { + return sectionData; + } + }), + distinctUntilChanged(), + ); } /** @@ -159,14 +172,26 @@ export class SectionsService { * The submission id * @param sectionId * The section id + * @param sectionType + * The type of section to retrieve * @return Observable * observable of [SubmissionSectionObject] */ - public getSectionState(submissionId: string, sectionId: string): Observable { + public getSectionState(submissionId: string, sectionId: string, sectionType: SectionsType): Observable { return this.store.select(submissionSectionFromIdSelector(submissionId, sectionId)).pipe( filter((sectionObj: SubmissionSectionObject) => hasValue(sectionObj)), map((sectionObj: SubmissionSectionObject) => sectionObj), - distinctUntilChanged(), + map((sectionState: SubmissionSectionObject) => { + if (hasValue(sectionState.data) && sectionType === SectionsType.SubmissionForm) { + return Object.assign({}, sectionState, { + data: normalizeSectionData(sectionState.data) + }) + } + else { + return sectionState; + } + }), + distinctUntilChanged() ); }