diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.spec.ts index 883da2295ab..1ac2b497024 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.spec.ts @@ -14,15 +14,18 @@ import { HIDDEN_MATCHER_PROVIDER, REQUIRED_MATCHER_PROVIDER, } from '@ng-dynamic-forms/core'; +import { Subject } from 'rxjs'; import { getMockFormBuilderService } from '../../../mocks/form-builder-service.mock'; import { + dcTypeInputConfig, mockInputWithTypeBindModel, MockRelationModel, } from '../../../mocks/form-models.mock'; import { FormBuilderService } from '../form-builder.service'; import { FormFieldMetadataValueObject } from '../models/form-field-metadata-value.model'; import { DsDynamicTypeBindRelationService } from './ds-dynamic-type-bind-relation.service'; +import { DsDynamicInputModel } from './models/ds-dynamic-input.model'; import { getTypeBindRelations } from './type-bind.utils'; describe('DSDynamicTypeBindRelationService test suite', () => { @@ -87,6 +90,12 @@ describe('DSDynamicTypeBindRelationService test suite', () => { const relatedModels = service.getRelatedFormModel(testModel); expect(relatedModels).toHaveSize(1); }); + it('Should ask the form builder for the model that controls this field', () => { + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + service.getRelatedFormModel(testModel); + expect((service as any).formBuilderService.getTypeBindModel).toHaveBeenCalledWith('edm_type'); + }); }); describe('Test matchesCondition method', () => { @@ -129,6 +138,140 @@ describe('DSDynamicTypeBindRelationService test suite', () => { } }); + it('Expect hasMatch to be true when the controlling model is not registered (field stays hidden)', () => { + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined); + const relation = dynamicFormRelationService.findRelationByMatcher((testModel as any).typeBindRelations, HIDDEN_MATCHER); + expect(service.matchesCondition(relation, HIDDEN_MATCHER)).toBeTruthy(); + }); + + it('Should attach to the controlling model as soon as it is registered, and stop when the caller unsubscribes', () => { + const bindModelUpdates = new Subject(); + const formBuilderServiceSpy: any = (service as any).formBuilderService; + formBuilderServiceSpy.getTypeBindModelUpdates.and.returnValue(bindModelUpdates.asObservable()); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(undefined); + + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + const dcTypeControl = new UntypedFormControl(); + // the caller spreads the result into its own array, so later children must hang off this one + const [subscription] = service.subscribeRelations(testModel, dcTypeControl); + + const controllingModel = new DsDynamicInputModel(dcTypeInputConfig); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(controllingModel); + bindModelUpdates.next('edm_type'); + + controllingModel.value = 'anotherType'; + expect(testModel.hidden).toBeTrue(); + controllingModel.value = 'boundType'; + expect(testModel.hidden).toBeFalse(); + + subscription.unsubscribe(); + + controllingModel.value = 'anotherType'; + expect(testModel.hidden).toBeFalse(); + }); + + it('Should leave a self-bound field untouched instead of hiding it forever', () => { + const formBuilderServiceSpy: any = (service as any).formBuilderService; + const testModel = mockInputWithTypeBindModel; + // the field's own resolves back to the field itself + testModel.typeBindRelations = getTypeBindRelations(['boundType'], testModel.id); + formBuilderServiceSpy.resolveTypeBindModelId.and.returnValue(testModel.id); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(testModel); + testModel.hidden = false; + + const subscriptions = service.subscribeRelations(testModel, new UntypedFormControl()); + + expect(service.getRelatedFormModel(testModel)).toHaveSize(0); + // nothing is evaluated, so the misconfigured field stays usable instead of being hidden forever + expect(testModel.hidden).toBeFalse(); + subscriptions.forEach((subscription) => subscription.unsubscribe()); + }); + + it('Should not hide a field whose controlling model has not been parsed yet, and attach when it is', () => { + // edm_type is not registered yet, so the fallback default model happens to be this field itself + const bindModelUpdates = new Subject(); + const formBuilderServiceSpy: any = (service as any).formBuilderService; + formBuilderServiceSpy.getTypeBindModelUpdates.and.returnValue(bindModelUpdates.asObservable()); + formBuilderServiceSpy.resolveTypeBindModelId.and.returnValue('edm_type'); + + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(testModel); + testModel.hidden = false; + + const [subscription] = service.subscribeRelations(testModel, new UntypedFormControl()); + expect(testModel.hidden).toBeFalse(); + + const controllingModel = new DsDynamicInputModel({ ...dcTypeInputConfig, id: 'edm_type', name: 'edm.type' }); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(controllingModel); + bindModelUpdates.next('edm_type'); + + controllingModel.value = 'anotherType'; + expect(testModel.hidden).toBeTrue(); + + subscription.unsubscribe(); + }); + + it('Should attach the real controlling model even when it was first bound to the default one', () => { + // dc_type is attached as the fallback, so the late edm_type registration must still be picked up + const bindModelUpdates = new Subject(); + const formBuilderServiceSpy: any = (service as any).formBuilderService; + formBuilderServiceSpy.getTypeBindModelUpdates.and.returnValue(bindModelUpdates.asObservable()); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(new DsDynamicInputModel(dcTypeInputConfig)); + + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + const [subscription] = service.subscribeRelations(testModel, new UntypedFormControl()); + + const controllingModel = new DsDynamicInputModel({ + ...dcTypeInputConfig, + id: 'edm_type', + name: 'edm.type', + }); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(controllingModel); + bindModelUpdates.next('edm_type'); + + controllingModel.value = 'anotherType'; + expect(testModel.hidden).toBeTrue(); + controllingModel.value = 'boundType'; + expect(testModel.hidden).toBeFalse(); + + subscription.unsubscribe(); + }); + + it('Should follow a re-registered controlling model and drop the stale one', () => { + // re-parsing the section that holds the controlling field yields a new instance under the same id + const bindModelUpdates = new Subject(); + const formBuilderServiceSpy: any = (service as any).formBuilderService; + formBuilderServiceSpy.getTypeBindModelUpdates.and.returnValue(bindModelUpdates.asObservable()); + + const firstInstance = new DsDynamicInputModel({ ...dcTypeInputConfig, id: 'edm_type', name: 'edm.type' }); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(firstInstance); + + const testModel = mockInputWithTypeBindModel; + testModel.typeBindRelations = getTypeBindRelations(['boundType'], 'edm_type'); + const [subscription] = service.subscribeRelations(testModel, new UntypedFormControl()); + + const secondInstance = new DsDynamicInputModel({ ...dcTypeInputConfig, id: 'edm_type', name: 'edm.type' }); + formBuilderServiceSpy.getTypeBindModel.and.returnValue(secondInstance); + bindModelUpdates.next('edm_type'); + + secondInstance.value = 'boundType'; + expect(testModel.hidden).toBeFalse(); + + // the replaced instance must no longer drive the field + firstInstance.value = 'anotherType'; + expect(testModel.hidden).toBeFalse(); + + secondInstance.value = 'anotherType'; + expect(testModel.hidden).toBeTrue(); + + subscription.unsubscribe(); + }); + }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.ts index 4f8cff747e6..a652017edb4 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.ts @@ -69,13 +69,18 @@ export class DsDynamicTypeBindRelationService { (model as any).typeBindRelations.forEach((relGroup) => relGroup.when.forEach((rel) => { - if (model.id === rel.id) { - throw new Error(`FormControl ${model.id} cannot depend on itself`); + const bindModel: DynamicFormControlModel | undefined = this.formBuilderService.getTypeBindModel(rel?.id); + + if (hasNoValue(bindModel)) { + return; } - const bindModel: DynamicFormControlModel = this.formBuilderService.getTypeBindModel(); + if (bindModel.id === model.id) { + // self-bound field, see isConfiguredToDependOnItself() + return; + } - if (model && !models.some((modelElement) => modelElement === bindModel)) { + if (!models.some((modelElement) => modelElement === bindModel)) { models.push(bindModel); } })); @@ -83,6 +88,25 @@ export class DsDynamicTypeBindRelationService { return models; } + /** + * Whether the configuration binds the model to its own metadata field, i.e. a `` + * pointing at the field it is declared on. Config-only, so it can't be confused with a relation + * that merely falls back to the default model until its real target is parsed. + */ + private isConfiguredToDependOnItself(model: DynamicFormControlModel): boolean { + return ((model as any).typeBindRelations || []).some((relGroup) => + (relGroup.when || []).some((rel) => this.formBuilderService.resolveTypeBindModelId(rel?.id) === model.id)); + } + + /** + * Whether a relation resolves to the model itself *right now* - the misconfiguration above, or + * transiently the default model standing in for a controlling model that isn't parsed yet. + */ + private currentlyResolvesToItself(model: DynamicFormControlModel): boolean { + return ((model as any).typeBindRelations || []).some((relGroup) => + (relGroup.when || []).some((rel) => this.formBuilderService.getTypeBindModel(rel?.id)?.id === model.id)); + } + /** * Return false if the type bind relation (eg. {MATCH_VISIBLE, OR, ['book', 'book part']}) matches the value in * matcher.match or true if the opposite match. Since this is called with regard to actively *hiding* a form @@ -102,7 +126,13 @@ export class DsDynamicTypeBindRelationService { // like relation group component and submission section form component). // This model (DynamicRelationGroupModel) contains eg. mandatory field, formConfiguration, relationFields, // submission scope, form/section type and other high level properties - const bindModel: any = this.formBuilderService.getTypeBindModel(); + const bindModel: any = this.formBuilderService.getTypeBindModel(condition?.id); + + // Nothing registered yet, not even the default fallback - the section holding the controlling + // field hasn't been parsed. Keep MATCH_VISIBLE fields hidden until one shows up. + if (hasNoValue(bindModel)) { + return relation.match === matcher.opposingMatch; + } let values: string[]; let bindModelValue = bindModel.value; @@ -174,45 +204,90 @@ export class DsDynamicTypeBindRelationService { } /** - * Return an array of subscriptions to a calling component + * Return an array of subscriptions to a calling component. + * + * One owning {@link Subscription} rather than the individual children: callers snapshot the + * returned array, and children are still added afterwards when a controlling model shows up late. + * * @param model * @param control */ subscribeRelations(model: DynamicFormControlModel, control: UntypedFormControl): Subscription[] { - const relatedModels = this.getRelatedFormModel(model); - const subscriptions: Subscription[] = []; - - Object.values(relatedModels).forEach((relatedModel: any) => { - - if (hasValue(relatedModel)) { - const initValue = (hasNoValue(relatedModel.value) || typeof relatedModel.value === 'string') ? relatedModel.value : - (Array.isArray(relatedModel.value) ? relatedModel.value : relatedModel.value.value); - - const updateSubject = (relatedModel.type === 'CHECKBOX_GROUP' ? relatedModel.valueUpdates : relatedModel.valueChanges); - const valueChanges = updateSubject.pipe( - startWith(initValue), - ); - - // Build up the subscriptions to watch for changes; - subscriptions.push(valueChanges.subscribe(() => { - // Iterate each matcher - if (hasValue(this.dynamicMatchers)) { - this.dynamicMatchers.forEach((matcher) => { - // Find the relation - const relation = this.dynamicFormRelationService.findRelationByMatcher((model as any).typeBindRelations, matcher); - // If the relation is defined, get matchesCondition result and pass it to the onChange event listener - if (relation !== undefined) { - const hasMatch = this.matchesCondition(relation, matcher); - matcher.onChange(hasMatch, model, control, this.injector); - } - }); + const subscriptions = new Subscription(); + + if (this.isConfiguredToDependOnItself(model)) { + // Upstream throws here, taking down the whole section over one bad field. Evaluating the + // relation instead would hide the field forever, so leave it as rendered and warn. + console.warn(`FormControl ${model.id} cannot depend on itself, ignoring its type bind relation`); + return [subscriptions]; + } + + // keyed by id, compared by identity: re-parsing a section yields a new instance under the same id + const attachedModels = new Map(); + const attachedSubscriptions = new Map(); + + const attachRelatedModels = (relatedModels: DynamicFormControlModel[]) => { + relatedModels.forEach((relatedModel: any) => { + + if (hasValue(relatedModel) && attachedModels.get(relatedModel.id) !== relatedModel) { + const staleSubscription = attachedSubscriptions.get(relatedModel.id); + if (hasValue(staleSubscription)) { + subscriptions.remove(staleSubscription); + staleSubscription.unsubscribe(); } - })); - } - }); + attachedModels.set(relatedModel.id, relatedModel); + + const initValue = (hasNoValue(relatedModel.value) || typeof relatedModel.value === 'string') ? relatedModel.value : + (Array.isArray(relatedModel.value) ? relatedModel.value : relatedModel.value.value); + + const updateSubject = (relatedModel.type === 'CHECKBOX_GROUP' ? relatedModel.valueUpdates : relatedModel.valueChanges); + const valueChanges = updateSubject.pipe( + startWith(initValue), + ); + + // Build up the subscriptions to watch for changes; + const valueChangesSubscription = valueChanges.subscribe(() => this.evaluateRelations(model, control)); + attachedSubscriptions.set(relatedModel.id, valueChangesSubscription); + subscriptions.add(valueChangesSubscription); + } + }); + }; + + attachRelatedModels(this.getRelatedFormModel(model)); + + if (attachedModels.size === 0 && !this.currentlyResolvesToItself(model)) { + // Nothing to listen to yet, so apply the "controlling model missing" fallback once. Skipped + // when the relation resolves to this field itself - matching against our own value would hide + // it while the real controlling model is still unparsed. + this.evaluateRelations(model, control); + } - return subscriptions; + // The controlling model (e.g. `edm_type`) may only be registered by a later + // modelFromConfiguration() call, so attach to it as soon as it shows up. + subscriptions.add(this.formBuilderService.getTypeBindModelUpdates().subscribe(() => { + attachRelatedModels(this.getRelatedFormModel(model)); + })); + + return [subscriptions]; + } + + /** + * Re-evaluate every type bind relation of the given model and notify the matchers of the outcome + */ + private evaluateRelations(model: DynamicFormControlModel, control: UntypedFormControl): void { + if (hasValue(this.dynamicMatchers)) { + // Iterate each matcher + this.dynamicMatchers.forEach((matcher) => { + // Find the relation + const relation = this.dynamicFormRelationService.findRelationByMatcher((model as any).typeBindRelations, matcher); + // If the relation is defined, get matchesCondition result and pass it to the onChange event listener + if (relation !== undefined) { + const hasMatch = this.matchesCondition(relation, matcher); + matcher.onChange(hasMatch, model, control, this.injector); + } + }); + } } } diff --git a/src/app/shared/form/builder/form-builder.service.spec.ts b/src/app/shared/form/builder/form-builder.service.spec.ts index 1e481391915..41b14d6c33d 100644 --- a/src/app/shared/form/builder/form-builder.service.spec.ts +++ b/src/app/shared/form/builder/form-builder.service.spec.ts @@ -32,6 +32,7 @@ import { DynamicTimePickerModel, } from '@ng-dynamic-forms/core'; import { TranslateService } from '@ngx-translate/core'; +import { Subject } from 'rxjs'; import { FormRowModel } from '../../../core/config/models/config-submission-form.model'; import { SubmissionFormsModel } from '../../../core/config/models/config-submission-forms.model'; @@ -39,7 +40,10 @@ import { ConfigurationDataService } from '../../../core/data/configuration-data. import { ConfigurationProperty } from '../../../core/shared/configuration-property.model'; import { VocabularyOptions } from '../../../core/submission/vocabularies/models/vocabulary-options.model'; import { getMockTranslateService } from '../../mocks/translate.service.mock'; -import { createSuccessfulRemoteDataObject$ } from '../../remote-data.utils'; +import { + createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$, +} from '../../remote-data.utils'; import { DynamicDsDatePickerModel } from './ds-dynamic-form-ui/models/date-picker/date-picker.model'; import { DynamicConcatModel } from './ds-dynamic-form-ui/models/ds-dynamic-concat.model'; import { DsDynamicInputModel } from './ds-dynamic-form-ui/models/ds-dynamic-input.model'; @@ -58,32 +62,32 @@ import { FormBuilderService } from './form-builder.service'; import { FormFieldModel } from './models/form-field.model'; import { FormFieldMetadataValueObject } from './models/form-field-metadata-value.model'; +const typeFieldProp = 'submit.type-bind.field'; +const typeFieldTestValue = 'dc.type'; +const submissionId = '1234'; + +function testValidator() { + return { testValidator: { valid: true } }; +} + +function testAsyncValidator() { + return new Promise((resolve) => setTimeout(() => resolve(true), 0)); +} + +const createConfigSuccessSpy = (...values: string[]) => jasmine.createSpyObj('configurationDataService', { + findByPropertyName: createSuccessfulRemoteDataObject$({ + ... new ConfigurationProperty(), + name: typeFieldProp, + values: values, + }), +}); + describe('FormBuilderService test suite', () => { let testModel: DynamicFormControlModel[]; let testFormConfiguration: SubmissionFormsModel; let service: FormBuilderService; let configSpy: ConfigurationDataService; - const typeFieldProp = 'submit.type-bind.field'; - const typeFieldTestValue = 'dc.type'; - - const submissionId = '1234'; - - function testValidator() { - return { testValidator: { valid: true } }; - } - - function testAsyncValidator() { - return new Promise((resolve) => setTimeout(() => resolve(true), 0)); - } - - const createConfigSuccessSpy = (...values: string[]) => jasmine.createSpyObj('configurationDataService', { - findByPropertyName: createSuccessfulRemoteDataObject$({ - ... new ConfigurationProperty(), - name: typeFieldProp, - values: values, - }), - }); beforeEach(() => { configSpy = createConfigSuccessSpy(typeFieldTestValue); @@ -927,3 +931,211 @@ describe('FormBuilderService test suite', () => { }); }); + +describe('FormBuilderService per-field type bind test suite', () => { + + let service: FormBuilderService; + let configSpy: ConfigurationDataService; + + const dcTypeModel = new DsDynamicInputModel({ + id: 'dc_type', name: 'dc.type', repeatable: false, metadataFields: [], + submissionId, hasSelectableMetadata: false, + }); + const edmTypeModel = new DsDynamicInputModel({ + id: 'edm_type', name: 'edm.type', repeatable: false, metadataFields: [], + submissionId, hasSelectableMetadata: false, + }); + + const typeBindFormConfiguration = { + name: 'typeBindFormConfiguration', + rows: [ + { fields: [{ + input: { type: 'onebox' }, label: 'Type', mandatory: 'false', repeatable: false, + hints: '', languageCodes: [], selectableMetadata: [{ metadata: 'edm.type' }], + } as FormFieldModel] } as FormRowModel, + { fields: [{ + input: { type: 'onebox' }, label: 'Language', mandatory: 'false', repeatable: false, + // padded on purpose: the registration path and the relation ids must agree on trimming + hints: '', languageCodes: [], typeBind: ['TEXT'], typeBindField: ' edm.type ', + selectableMetadata: [{ metadata: 'dc.language.iso' }], + } as FormFieldModel] } as FormRowModel, + ], + type: 'submissionform', + _links: { self: { href: 'typeBindFormConfiguration.url' } }, + } as any; + + beforeEach(() => { + // dspace.cfg declares the property twice, so the REST payload repeats the default value + configSpy = createConfigSuccessSpy('dc.type', 'dc.type', 'dc.language.iso=>edm.type'); + TestBed.configureTestingModule({ + imports: [ReactiveFormsModule], + providers: [ + { provide: FormBuilderService, useClass: FormBuilderService }, + { provide: DynamicFormValidationService, useValue: {} }, + { provide: NG_VALIDATORS, useValue: testValidator, multi: true }, + { provide: NG_ASYNC_VALIDATORS, useValue: testAsyncValidator, multi: true }, + { provide: ConfigurationDataService, useValue: configSpy }, + { provide: TranslateService, useValue: getMockTranslateService() }, + ], + }); + service = TestBed.inject(FormBuilderService); + }); + + it('should keep "dc_type" as the default type bind field even when the property is duplicated', () => { + expect(service.getTypeField()).toEqual('dc_type'); + }); + + it('should resolve the controlling model per field', () => { + service.setTypeBindModel(dcTypeModel); + service.setTypeBindModel(edmTypeModel); + + expect(service.getTypeBindModel('dc.language.iso')).toBe(edmTypeModel); + expect(service.getTypeBindModel('edm_type')).toBe(edmTypeModel); + expect(service.getTypeBindModel('dc.contributor.author')).toBe(dcTypeModel); + expect(service.getTypeBindModel()).toBe(dcTypeModel); + }); + + it('should fall back to the default model when the controlling model is not part of the form', () => { + service.setTypeBindModel(dcTypeModel); + expect(service.getTypeBindModel('dc.language.iso')).toBe(dcTypeModel); + }); + + it('should emit every registered type bind model id', (done) => { + const emitted: string[] = []; + service.getTypeBindModelUpdates().subscribe((id: string) => { + emitted.push(id); + if (emitted.length === 2) { + expect(emitted).toEqual(['dc_type', 'edm_type']); + done(); + } + }); + service.setTypeBindModel(dcTypeModel); + service.setTypeBindModel(edmTypeModel); + }); + + it('should bind dc.language.iso to edm.type when parsing the form configuration', () => { + const formModel = service.modelFromConfiguration(submissionId, typeBindFormConfiguration, 'testScopeUUID'); + const languageModel = service.findById('dc_language_iso', formModel) as DsDynamicInputModel; + + expect(languageModel.typeBindRelations[0].when[0].id).toEqual('edm_type'); + expect(service.getTypeBindModel('dc.language.iso').id).toEqual('edm_type'); + }); + + it('should not let a previous submission\'s controlling model answer lookups', () => { + service.modelFromConfiguration(submissionId, typeBindFormConfiguration, 'testScopeUUID'); + expect(service.getTypeBindModel('dc.language.iso').id).toEqual('edm_type'); + + // another collection, whose form has no edm.type field at all + service.modelFromConfiguration('other-submission', { + name: 'plainFormConfiguration', + rows: [ + { fields: [{ + input: { type: 'onebox' }, label: 'Type', mandatory: 'false', repeatable: false, + hints: '', languageCodes: [], selectableMetadata: [{ metadata: 'dc.type' }], + } as FormFieldModel] } as FormRowModel, + ], + type: 'submissionform', + _links: { self: { href: 'plainFormConfiguration.url' } }, + } as any, 'testScopeUUID'); + + expect(service.getTypeBindModel('dc.language.iso').id).toEqual('dc_type'); + }); +}); + +describe('FormBuilderService per-field type bind with a slow configuration response', () => { + + let service: FormBuilderService; + let configResponse: Subject; + + const overrideOnlyFormConfiguration = { + name: 'overrideOnlyFormConfiguration', + rows: [ + { fields: [{ + input: { type: 'onebox' }, label: 'Type', mandatory: 'false', repeatable: false, + hints: '', languageCodes: [], selectableMetadata: [{ metadata: 'edm.type' }], + } as FormFieldModel] } as FormRowModel, + { fields: [{ + // no : the binding exists only in the submit.type-bind.field property + input: { type: 'onebox' }, label: 'Language', mandatory: 'false', repeatable: false, + hints: '', languageCodes: [], typeBind: ['TEXT'], + selectableMetadata: [{ metadata: 'dc.language.iso' }], + } as FormFieldModel] } as FormRowModel, + ], + type: 'submissionform', + _links: { self: { href: 'overrideOnlyFormConfiguration.url' } }, + } as any; + + beforeEach(() => { + configResponse = new Subject(); + TestBed.configureTestingModule({ + imports: [ReactiveFormsModule], + providers: [ + { provide: FormBuilderService, useClass: FormBuilderService }, + { provide: DynamicFormValidationService, useValue: {} }, + { provide: NG_VALIDATORS, useValue: testValidator, multi: true }, + { provide: NG_ASYNC_VALIDATORS, useValue: testAsyncValidator, multi: true }, + { + provide: ConfigurationDataService, + useValue: jasmine.createSpyObj('configurationDataService', { + findByPropertyName: configResponse.asObservable(), + }), + }, + { provide: TranslateService, useValue: getMockTranslateService() }, + ], + }); + service = TestBed.inject(FormBuilderService); + }); + + it('should register the controlling model once the property arrives after the form was parsed', () => { + service.modelFromConfiguration(submissionId, overrideOnlyFormConfiguration, 'testScopeUUID'); + // the property has not arrived yet, so edm.type is not known to be a controlling field + expect(service.getTypeBindModel('dc.language.iso')).toBeUndefined(); + + configResponse.next(createSuccessfulRemoteDataObject({ + ... new ConfigurationProperty(), + name: 'submit.type-bind.field', + values: ['dc.language.iso=>edm.type', ' dc.type '], + })); + configResponse.complete(); + + expect(service.getTypeField()).toEqual('dc_type'); + expect(service.getTypeBindModel('dc.language.iso').id).toEqual('edm_type'); + }); + + it('should register a controlling model declared only by a padded ', () => { + // no A=>B entry here, so the padded XML attribute is the only source of the controlling model id + configResponse.next(createSuccessfulRemoteDataObject({ + ... new ConfigurationProperty(), + name: 'submit.type-bind.field', + values: ['dc.type'], + })); + configResponse.complete(); + + const paddedConfiguration = { + ...overrideOnlyFormConfiguration, + rows: [ + overrideOnlyFormConfiguration.rows[0], + { fields: [{ + ...overrideOnlyFormConfiguration.rows[1].fields[0], + typeBindField: ' edm.type ', + } as FormFieldModel] } as FormRowModel, + ], + } as any; + + service.modelFromConfiguration(submissionId, paddedConfiguration, 'testScopeUUID'); + + expect(service.getTypeBindModel('edm_type')?.id).toEqual('edm_type'); + }); + + it('should ignore blank and malformed values', () => { + configResponse.next(createSuccessfulRemoteDataObject({ + ... new ConfigurationProperty(), + name: 'submit.type-bind.field', + values: ['', ' ', '=>', 'a=>b=>c', undefined, 'dc.type'], + })); + configResponse.complete(); + + expect(service.getTypeField()).toEqual('dc_type'); + expect((service as any).typeFields.has('a')).toBeFalse(); + }); +}); diff --git a/src/app/shared/form/builder/form-builder.service.ts b/src/app/shared/form/builder/form-builder.service.ts index 66c500f23dc..a3dc3626a57 100644 --- a/src/app/shared/form/builder/form-builder.service.ts +++ b/src/app/shared/form/builder/form-builder.service.ts @@ -27,7 +27,12 @@ import { import isObject from 'lodash/isObject'; import isString from 'lodash/isString'; import mergeWith from 'lodash/mergeWith'; +import { + Observable, + Subject, +} from 'rxjs'; +import { FormRowModel } from '../../../core/config/models/config-submission-form.model'; import { SubmissionFormsModel } from '../../../core/config/models/config-submission-forms.model'; import { ConfigurationDataService } from '../../../core/data/configuration-data.service'; import { VIRTUAL_METADATA_PREFIX } from '../../../core/shared/metadata.models'; @@ -55,13 +60,37 @@ import { DynamicQualdropModel } from './ds-dynamic-form-ui/models/ds-dynamic-qua import { DynamicRowArrayModel } from './ds-dynamic-form-ui/models/ds-dynamic-row-array-model'; import { DynamicRelationGroupModel } from './ds-dynamic-form-ui/models/relation-group/dynamic-relation-group.model'; import { DYNAMIC_FORM_CONTROL_TYPE_TAG } from './ds-dynamic-form-ui/models/tag/dynamic-tag.model'; +import { FormFieldModel } from './models/form-field.model'; import { FormFieldMetadataValueObject } from './models/form-field-metadata-value.model'; import { RowParser } from './parsers/row-parser'; +/** + * Key of the default entry in the type field map, e.g. {'default' -> 'dc_type'} + */ +export const TYPE_BIND_DEFAULT_KEY = 'default'; + +/** + * Separator in `submit.type-bind.field`, e.g. `dc.language.iso=>edm.type` + */ +const TYPE_BIND_FIELD_SEPARATOR = '=>'; + +/** + * Controlling model id used when `submit.type-bind.field` declares no default + */ +const TYPE_BIND_DEFAULT_MODEL_ID = 'dc_type'; + @Injectable({ providedIn: 'root' }) export class FormBuilderService extends DynamicFormService { - private typeBindModel: DynamicFormControlModel; + /** + * The models that control type binding, keyed by model id (`dc_type`, `edm_type`) + */ + private typeBindModel: Map; + + /** + * Emits the id of a type bind model whenever one is registered + */ + private typeBindModelUpdates: Subject; /** * This map contains the active forms model @@ -74,9 +103,25 @@ export class FormBuilderService extends DynamicFormService { private formGroups: Map; /** - * This is the field to use for type binding + * The submission {@link typeBindModel} belongs to; the registry is scoped to it + */ + private typeBindModelSubmissionId: string; + + /** + * Rows parsed before `submit.type-bind.field` arrived, re-scanned once it does + */ + private typeBindParsedRows: DynamicFormControlModel[][]; + + /** + * Whether `submit.type-bind.field` has been processed, i.e. whether {@link typeFields} can still change + */ + private typeBindConfigLoaded: boolean; + + /** + * The fields to use for type binding: TYPE_BIND_DEFAULT_KEY -> default controlling model id, plus + * one entry per bound field, e.g. `dc.language.iso` -> `edm_type` */ - private typeField: string; + private typeFields: Map; constructor( componentService: DynamicFormComponentService, @@ -87,12 +132,18 @@ export class FormBuilderService extends DynamicFormService { super(componentService, validationService); this.formModels = new Map(); this.formGroups = new Map(); + this.typeFields = new Map(); + this.typeBindModel = new Map(); + this.typeBindParsedRows = []; + this.typeBindModelUpdates = new Subject(); + + this.typeFields.set(TYPE_BIND_DEFAULT_KEY, TYPE_BIND_DEFAULT_MODEL_ID); + // without a config service the type field map can never change + this.typeBindConfigLoaded = hasNoValue(this.configService); // If optional config service was passed, perform an initial set of type field (default dc_type) for type binds if (hasValue(this.configService)) { this.setTypeBindFieldFromConfig(); } - - } createDynamicFormControlEvent(control: UntypedFormControl, group: UntypedFormGroup, model: DynamicFormControlModel, type: string): DynamicFormControlEvent { @@ -104,12 +155,47 @@ export class FormBuilderService extends DynamicFormService { return { $event, context, control: control, group: group, model: model, type }; } - getTypeBindModel() { - return this.typeBindModel; + /** + * Get the model of the field controlling the type binding of a bound field, falling back to the + * default one when the target isn't part of the current form. + * + * @param typeBindFieldRef the bound field's own metadata name (mapped through + * `submit.type-bind.field`), or the controlling model id when `` is set + */ + getTypeBindModel(typeBindFieldRef?: string): DynamicFormControlModel | undefined { + return this.typeBindModel.get(this.resolveTypeBindModelId(typeBindFieldRef)) + ?? this.typeBindModel.get(this.getDefaultTypeBindModelId()); + } + + /** + * Which model id a type bind reference points at, from configuration only. Unlike + * {@link getTypeBindModel} the answer doesn't depend on what has been parsed so far. + */ + resolveTypeBindModelId(typeBindFieldRef?: string): string { + return this.typeFields.get(typeBindFieldRef) ?? typeBindFieldRef ?? this.getDefaultTypeBindModelId(); + } + + /** + * Model id of the default controlling field + */ + private getDefaultTypeBindModelId(): string { + return this.typeFields.get(TYPE_BIND_DEFAULT_KEY) ?? TYPE_BIND_DEFAULT_MODEL_ID; } setTypeBindModel(model: DynamicFormControlModel) { - this.typeBindModel = model; + if (this.typeBindModel.get(model.id) === model) { + return; + } + this.typeBindModel.set(model.id, model); + this.typeBindModelUpdates.next(model.id); + } + + /** + * Emits the id of every type bind model as it is registered, so fields whose controlling model is + * parsed later can still attach to it. + */ + getTypeBindModelUpdates(): Observable { + return this.typeBindModelUpdates.asObservable(); } findById(id: string, groupModel: DynamicFormControlModel[], arrayIndex = null): DynamicFormControlModel | null { @@ -311,16 +397,65 @@ export class FormBuilderService extends DynamicFormService { }); } - if (hasNoValue(typeBindModel)) { - typeBindModel = this.findById(this.typeField, rows); - } + this.resetTypeBindModelsOnSubmissionChange(submissionId); if (hasValue(typeBindModel)) { this.setTypeBindModel(typeBindModel); + } else { + if (!this.typeBindConfigLoaded) { + // bounded on purpose: once the property is in, every controlling id is known at parse time + this.typeBindParsedRows.push(rows); + } + this.registerTypeBindModels(this.getTypeBindModelIds(rawData), rows); } return rows; } + /** + * Drop the models registered by another submission. The registry has to survive across sections of + * one submission (a controlling field may live in another section), but not across submissions. + */ + private resetTypeBindModelsOnSubmissionChange(submissionId: string): void { + if (this.typeBindModelSubmissionId !== submissionId) { + this.typeBindModelSubmissionId = submissionId; + this.typeBindModel.clear(); + this.typeBindParsedRows = []; + } + } + + /** + * Register every model of the given rows whose id is one of the given controlling field ids + */ + private registerTypeBindModels(modelIds: string[], rows: DynamicFormControlModel[]): void { + modelIds.forEach((typeBindModelId: string) => { + const foundModel = this.findById(typeBindModelId, rows); + if (hasValue(foundModel)) { + this.setTypeBindModel(foundModel); + } + }); + } + + /** + * Ids of every model that can control type binding here: the `submit.type-bind.field` values plus + * every `` of this configuration, read straight from the REST payload so + * they are known even before that property arrives. + */ + private getTypeBindModelIds(rawData: any): string[] { + const ids = new Set(this.typeFields.values()); + const collectFromRows = (formRows: FormRowModel[]): void => { + (formRows || []).forEach((formRow: FormRowModel) => (formRow?.fields || []).forEach((field: FormFieldModel) => { + // trim like FieldParser.getTypeBindFieldRef, or a padded value registers ' edm_type ' + const typeBindField = field?.typeBindField?.trim(); + if (isNotEmpty(typeBindField)) { + ids.add(typeBindField.replace(/\./g, '_')); + } + collectFromRows(field?.rows); + })); + }; + collectFromRows(rawData?.rows); + return Array.from(ids); + } + isModelInCustomGroup(model: DynamicFormControlModel): boolean { return this.isCustomGroup((model as any).parent); } @@ -512,38 +647,65 @@ export class FormBuilderService extends DynamicFormService { } /** - * Get the type bind field from config + * Get the type bind field(s) from config, e.g. + * `submit.type-bind.field = dc.type, dc.language.iso=>edm.type`: the default controlling field plus + * one `=>` entry per overridden field. The property may be declared + * in both dspace.cfg and local.cfg, so duplicates and order must not matter. */ setTypeBindFieldFromConfig(): void { this.configService.findByPropertyName('submit.type-bind.field').pipe( getFirstCompletedRemoteData(), ).subscribe((remoteData: any) => { + // whatever the outcome, the type field map cannot change any more + this.typeBindConfigLoaded = true; // make sure we got a success response from the backend if (!remoteData.hasSucceeded) { - this.typeField = 'dc_type'; + this.typeFields.set(TYPE_BIND_DEFAULT_KEY, TYPE_BIND_DEFAULT_MODEL_ID); + this.typeBindParsedRows = []; return; } - // Read type bind value from response and set if non-empty - const typeFieldConfig = remoteData.payload.values[0]; - if (isEmpty(typeFieldConfig)) { - this.typeField = 'dc_type'; - } else { - this.typeField = typeFieldConfig.replace(/\./g, '_'); + const typeFieldConfigValues: string[] = remoteData.payload.values || []; + typeFieldConfigValues.forEach((rawValue: string) => { + const typeFieldConfig = rawValue?.trim(); + if (isEmpty(typeFieldConfig)) { + return; + } + if (!typeFieldConfig.includes(TYPE_BIND_FIELD_SEPARATOR)) { + // `dc.type`: the default controlling field + this.typeFields.set(TYPE_BIND_DEFAULT_KEY, typeFieldConfig.replace(/\./g, '_')); + return; + } + // `dc.language.iso=>edm.type`: this field is controlled by another one + const parts = typeFieldConfig.split(TYPE_BIND_FIELD_SEPARATOR).map((part: string) => part.trim()); + if (parts.length !== 2 || parts.some((part: string) => isEmpty(part))) { + console.warn(`Ignoring malformed submit.type-bind.field value "${rawValue}", expected "${TYPE_BIND_FIELD_SEPARATOR}"`); + return; + } + this.typeFields.set(parts[0], parts[1].replace(/\./g, '_')); + }); + if (hasNoValue(this.typeFields.get(TYPE_BIND_DEFAULT_KEY))) { + this.typeFields.set(TYPE_BIND_DEFAULT_KEY, TYPE_BIND_DEFAULT_MODEL_ID); } + // forms parsed before the property arrived didn't know the `A=>B` overrides yet + const typeBindModelIds = Array.from(this.typeFields.values()); + this.typeBindParsedRows.forEach((rows: DynamicFormControlModel[]) => this.registerTypeBindModels(typeBindModelIds, rows)); + this.typeBindParsedRows = []; }); } /** - * Get type field. If the type isn't already set, and a ConfigurationDataService is provided, set (with subscribe) - * from back end. Otherwise, get/set a default "dc_type" value + * Get the default type field. If it isn't already set, and a ConfigurationDataService is provided, + * set (with subscribe) from back end. Otherwise, get/set a default "dc_type" value */ getTypeField(): string { - if (hasValue(this.configService) && hasNoValue(this.typeField)) { - this.setTypeBindFieldFromConfig(); - } else if (hasNoValue(this.typeField)) { - this.typeField = 'dc_type'; + if (hasNoValue(this.typeFields.get(TYPE_BIND_DEFAULT_KEY))) { + if (hasValue(this.configService)) { + this.setTypeBindFieldFromConfig(); + } else { + this.typeFields.set(TYPE_BIND_DEFAULT_KEY, TYPE_BIND_DEFAULT_MODEL_ID); + } } - return this.typeField; + return this.getDefaultTypeBindModelId(); } } diff --git a/src/app/shared/form/builder/models/form-field.model.spec.ts b/src/app/shared/form/builder/models/form-field.model.spec.ts new file mode 100644 index 00000000000..ce3f1333760 --- /dev/null +++ b/src/app/shared/form/builder/models/form-field.model.spec.ts @@ -0,0 +1,34 @@ +import { Deserialize } from 'cerialize'; + +import { FormFieldModel } from './form-field.model'; + +describe('FormFieldModel', () => { + + it('should deserialize the type bind configuration coming from submission-forms.xml', () => { + // shape of a single field in GET /api/config/submissionforms/ + const field = Deserialize({ + label: 'Pick the languages of the TEXT', + mandatory: 'true', + repeatable: true, + selectableMetadata: [{ metadata: 'dc.language.iso' }], + typeBind: ['TEXT'], + typeBindField: 'edm.type', + }, FormFieldModel) as FormFieldModel; + + expect(field.typeBind).toEqual(['TEXT']); + // without @autoserialize on typeBindField the per-field binding is dropped before the parser sees it + expect(field.typeBindField).toEqual('edm.type'); + }); + + it('should leave typeBindField undefined when the field has no ', () => { + const field = Deserialize({ + label: 'Title', + mandatory: 'true', + repeatable: false, + selectableMetadata: [{ metadata: 'dc.title' }], + typeBind: [], + }, FormFieldModel) as FormFieldModel; + + expect(field.typeBindField).toBeUndefined(); + }); +}); diff --git a/src/app/shared/form/builder/models/form-field.model.ts b/src/app/shared/form/builder/models/form-field.model.ts index a168124bec8..3c6a6830385 100644 --- a/src/app/shared/form/builder/models/form-field.model.ts +++ b/src/app/shared/form/builder/models/form-field.model.ts @@ -132,6 +132,13 @@ export class FormFieldModel { @autoserialize typeBind: string[]; + /** + * The field controlling the type binding of this field, from ``. + * Undefined when the attribute is absent: the `submit.type-bind.field` default applies then. + */ + @autoserialize + typeBindField?: string; + /** * Containing the value for this metadata field */ diff --git a/src/app/shared/form/builder/parsers/field-parser.ts b/src/app/shared/form/builder/parsers/field-parser.ts index df0f0389bc5..e26e2ccd07e 100644 --- a/src/app/shared/form/builder/parsers/field-parser.ts +++ b/src/app/shared/form/builder/parsers/field-parser.ts @@ -95,7 +95,7 @@ export abstract class FieldParser { hasSelectableMetadata: isNotEmpty(this.configData.selectableMetadata), isDraggable, typeBindRelations: isNotEmpty(this.configData.typeBind) ? getTypeBindRelations(this.configData.typeBind, - this.parserOptions.typeField) : null, + this.getTypeBindFieldRef()) : null, groupFactory: () => { let model; if ((arrayCounter === 0)) { @@ -290,6 +290,21 @@ export abstract class FieldParser { } } + /** + * The field controlling the type binding of this field: the model id when submission-forms.xml + * declares ``, otherwise this field's own metadata name, which + * {@link FormBuilderService#getTypeBindModel} maps through `submit.type-bind.field` later - that + * property is fetched asynchronously and may not have arrived while the form is parsed. + */ + protected getTypeBindFieldRef(): string { + const typeBindField = this.configData.typeBindField?.trim(); + if (isNotEmpty(typeBindField)) { + // input ids don't allow dots + return typeBindField.replace(/\./g, '_'); + } + return this.getFieldId() || this.parserOptions.typeField; + } + protected initModel(id?: string, label = true, labelEmpty = false, setErrors = true, hint = true) { const controlModel = Object.create(null); @@ -338,7 +353,7 @@ export abstract class FieldParser { // If typeBind is configured if (isNotEmpty(this.configData.typeBind)) { (controlModel as DsDynamicInputModel).typeBindRelations = getTypeBindRelations(this.configData.typeBind, - this.parserOptions.typeField); + this.getTypeBindFieldRef()); } return controlModel; diff --git a/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts b/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts index b2a50395e1a..d008d72f586 100644 --- a/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts +++ b/src/app/shared/form/builder/parsers/onebox-field-parser.spec.ts @@ -152,4 +152,25 @@ describe('OneboxFieldParser test suite', () => { }); }); + describe('type binding', () => { + it('should use the field named in as the type bind relation id', () => { + field1.typeBind = ['TEXT']; + field1.typeBindField = 'edm.type'; + const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); + + const fieldModel = parser.parse() as DsDynamicInputModel; + + expect(fieldModel.typeBindRelations[0].when[0].id).toBe('edm_type'); + }); + + it('should fall back to the field\'s own metadata name when no is configured', () => { + field1.typeBind = ['TEXT']; + const parser = new OneboxFieldParser(submissionId, field1, initFormValues, parserOptions, translateService); + + const fieldModel = parser.parse() as DsDynamicInputModel; + + expect(fieldModel.typeBindRelations[0].when[0].id).toBe('title'); + }); + }); + }); diff --git a/src/app/shared/mocks/form-builder-service.mock.ts b/src/app/shared/mocks/form-builder-service.mock.ts index 9a0f358da66..ce60600f2ce 100644 --- a/src/app/shared/mocks/form-builder-service.mock.ts +++ b/src/app/shared/mocks/form-builder-service.mock.ts @@ -2,13 +2,14 @@ import { UntypedFormControl, UntypedFormGroup, } from '@angular/forms'; +import { EMPTY } from 'rxjs'; import { DsDynamicInputModel } from '../form/builder/ds-dynamic-form-ui/models/ds-dynamic-input.model'; import { FormBuilderService } from '../form/builder/form-builder.service'; export function getMockFormBuilderService(): FormBuilderService { - return jasmine.createSpyObj('FormBuilderService', { + const formBuilderService = jasmine.createSpyObj('FormBuilderService', { modelFromConfiguration: [], createFormGroup: new UntypedFormGroup({}), getValueFromModel: {}, @@ -45,5 +46,12 @@ export function getMockFormBuilderService(): FormBuilderService { ], }, ), + getTypeBindModelUpdates: EMPTY, + resolveTypeBindModelId: undefined, }); + + // as the real implementation behaves for a reference the type field map does not remap + formBuilderService.resolveTypeBindModelId.and.callFake((typeBindFieldRef?: string) => typeBindFieldRef ?? 'dc_type'); + + return formBuilderService; }