From 2721e977529b5b7511996d6a9b05c9f12a0424ff Mon Sep 17 00:00:00 2001 From: Ondrej Kosarko Date: Tue, 23 Jun 2026 17:40:22 +0000 Subject: [PATCH 1/3] fix(submission): keep dc.type dropdown display after change (ufal/clarin-dspace#1377 -> ufal/dspace-angular#156) The "type of resource" scrollable dropdown blanked out after changing its value when the backend was slow: the reloaded model value came back with an empty display but populated value/authority, so currentValue rendered ''. Fall back to the underlying value when display is empty, matching the existing inputFormatter convention. Add a fakeAsync regression test that drives an empty-display value through group.valueChanges (the real reload entry point) and asserts the rendered input keeps its text, reproducing the original blank-after-latency bug. --- ...amic-scrollable-dropdown.component.spec.ts | 43 +++++++++++++++++++ .../dynamic-scrollable-dropdown.component.ts | 4 +- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts index 116e4a0f463..126e360a1ff 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts @@ -5,6 +5,7 @@ import { ComponentFixture, fakeAsync, inject, TestBed, tick, waitForAsync, } fro import { By } from '@angular/platform-browser'; import { NgbModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateModule } from '@ngx-translate/core'; +import { of as observableOf } from 'rxjs'; import { InfiniteScrollModule } from 'ngx-infinite-scroll'; import { DynamicFormLayoutService, DynamicFormsCoreModule, DynamicFormValidationService } from '@ng-dynamic-forms/core'; import { DynamicFormsNGBootstrapUIModule } from '@ng-dynamic-forms/ui-ng-bootstrap'; @@ -15,6 +16,7 @@ import { VocabularyServiceStub } from '../../../../../testing/vocabulary-service import { DsDynamicScrollableDropdownComponent } from './dynamic-scrollable-dropdown.component'; import { DynamicScrollableDropdownModel } from './dynamic-scrollable-dropdown.model'; import { VocabularyEntry } from '../../../../../../core/submission/vocabularies/models/vocabulary-entry.model'; +import { FormFieldMetadataValueObject } from '../../../models/form-field-metadata-value.model'; import { createTestComponent, hasClass } from '../../../../../testing/utils.test'; import { mockDynamicFormLayoutService, @@ -209,6 +211,47 @@ describe('Dynamic Dynamic Scrollable Dropdown component', () => { expect(scrollableDropdownComp.optionsList).toEqual(vocabularyServiceStub.getList()); expect(scrollableDropdownComp.model.value).toEqual(modelValue); }); + + it('should fall back to the underlying value when display is empty on value change', () => { + const valueWithEmptyDisplay = { display: '', value: 'Corpus' }; + scrollableDropdownComp.setCurrentValue(valueWithEmptyDisplay); + + let currentValue; + scrollableDropdownComp.currentValue.subscribe((v) => currentValue = v); + + expect(currentValue).toBe('Corpus'); + }); + + it('should fall back to the underlying value when display is empty on init', () => { + spyOn(vocabularyServiceStub, 'getVocabularyEntryByID').and.returnValue(observableOf(new VocabularyEntry())); + (scrollableDropdownComp.model as any).value = Object.assign( + new FormFieldMetadataValueObject(), + { display: '', value: 'Corpus', authority: 'corpus-auth' } + ); + scrollableDropdownComp.setCurrentValue(scrollableDropdownComp.model.value, true); + + let currentValue; + scrollableDropdownComp.currentValue.subscribe((v) => currentValue = v); + + expect(currentValue).toBe('Corpus'); + }); + + // Regression for ufal/clarin-dspace#1377: after a dc.type change the section + // reloads and pushes a new value with an empty display through the form control. + // The rendered input must not blank out. + it('should keep the rendered value when a value with empty display arrives via valueChanges', fakeAsync(() => { + const reloadedValue = Object.assign( + new FormFieldMetadataValueObject(), + { display: '', value: 'Corpus', authority: 'corpus-auth' } + ); + + scrollableDropdownComp.group.get(scrollableDropdownComp.model.id).setValue(reloadedValue); + tick(); + scrollableDropdownFixture.detectChanges(); + + const input = scrollableDropdownFixture.debugElement.query(By.css('input.form-control')).nativeElement; + expect(input.value).toBe('Corpus'); + })); }); }); }); diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts index b284fa4fc5e..4c5f3069b31 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts @@ -276,7 +276,7 @@ export class DsDynamicScrollableDropdownComponent extends DsDynamicVocabularyCom if (init && !this.useFindAllService) { result = this.getInitValueFromModel().pipe( - map((formValue: FormFieldMetadataValueObject) => formValue.display) + map((formValue: FormFieldMetadataValueObject) => formValue.display || formValue.value) ); } else { if (isEmpty(value)) { @@ -286,7 +286,7 @@ export class DsDynamicScrollableDropdownComponent extends DsDynamicVocabularyCom } else if (this.useFindAllService) { result = observableOf(value[this.model.displayKey]); } else { - result = observableOf(value.display); + result = observableOf(value.display || value.value); } } From 907fe83b1f53f23885f39c8e793201c5b69a572f Mon Sep 17 00:00:00 2001 From: Ondrej Kosarko Date: Wed, 24 Jun 2026 08:27:55 +0000 Subject: [PATCH 2/3] test(submission): make empty-display init test deterministic Address Copilot PR review: the init-path test previously relied on isNotEmpty(new VocabularyEntry()) being false so getInitValueFromModel would fall back to the raw model.value. That couples the test to VocabularyEntry having no initialized fields. Stub getInitValueFromModel directly to emit an empty-display value (built bypassing the FFMVO constructor, which would otherwise fill display via `display || value`), so the test deterministically exercises the setCurrentValue fallback it is meant to guard. Co-Authored-By: Claude Opus 4.8 --- .../dynamic-scrollable-dropdown.component.spec.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts index 126e360a1ff..3df5cedc06a 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts @@ -223,12 +223,15 @@ describe('Dynamic Dynamic Scrollable Dropdown component', () => { }); it('should fall back to the underlying value when display is empty on init', () => { - spyOn(vocabularyServiceStub, 'getVocabularyEntryByID').and.returnValue(observableOf(new VocabularyEntry())); - (scrollableDropdownComp.model as any).value = Object.assign( + // Build the value bypassing the FormFieldMetadataValueObject constructor (which would + // otherwise apply its own `display || value`), so the init branch genuinely receives an + // empty display and exercises the fallback in setCurrentValue. + const emptyDisplayValue = Object.assign( new FormFieldMetadataValueObject(), - { display: '', value: 'Corpus', authority: 'corpus-auth' } + { display: '', value: 'Corpus' } ); - scrollableDropdownComp.setCurrentValue(scrollableDropdownComp.model.value, true); + spyOn(scrollableDropdownComp, 'getInitValueFromModel').and.returnValue(observableOf(emptyDisplayValue)); + scrollableDropdownComp.setCurrentValue(emptyDisplayValue, true); let currentValue; scrollableDropdownComp.currentValue.subscribe((v) => currentValue = v); From 359bae7a16e3b7bb9fc230c0b668bd42d27bc02c Mon Sep 17 00:00:00 2001 From: Ondrej Kosarko Date: Wed, 24 Jun 2026 09:26:46 +0000 Subject: [PATCH 3/3] test(type-bind): stop re-spying mock methods that are already spies The DSDynamicTypeBindRelationService spec called spyOn() on formBuilderService.getTypeBindModel / getTypeBindModelUpdates, but the mock (getMockFormBuilderService) already creates those as jasmine spies via createSpyObj. Because src/test.ts runs with teardown.destroyAfterEach:false and karma randomizes spec order, certain orderings hit the second spy installation and threw "getTypeBindModel has already been spied upon", failing 6 tests non-deterministically (green on clarin-v7, red here only due to a different random ordering). Reconfigure the existing spies with .and.* instead of re-spying, which is order-independent. Co-Authored-By: Claude Opus 4.8 --- ...-dynamic-type-bind-relation.service.spec.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 7311d19e949..bd1e81f55a7 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 @@ -86,7 +86,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => { it('Should not push undefined bind models', () => { const testModel = mockInputWithTypeBindModel; testModel.typeBindRelations = getTypeBindRelations(['boundType']); - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined); const relatedModels = service.getRelatedFormModel(testModel); @@ -143,7 +143,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => { opposingMatch: HIDDEN_MATCHER.match, onChange: jasmine.createSpy('onChange') }; - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined); const hasMatch = service.matchesCondition(relation, visibleMatcher); @@ -159,7 +159,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => { opposingMatch: MATCH_VISIBLE, onChange: jasmine.createSpy('onChange') }; - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined); const hasMatch = service.matchesCondition(relation, hiddenMatcher); @@ -185,8 +185,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => { onChange: jasmine.createSpy('onChange') }; (service as any).dynamicMatchers = [visibleMatcher]; - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined); - spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable()); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined); + ((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable()); const subscriptions = service.subscribeRelations(testModel, dcTypeControl); expect(subscriptions.length).toBe(1); @@ -220,8 +220,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => { onChange: jasmine.createSpy('onChange') }; (service as any).dynamicMatchers = [hiddenMatcher]; - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined); - spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable()); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined); + ((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable()); const subscriptions = service.subscribeRelations(testModel, dcTypeControl); expect(subscriptions.length).toBe(1); @@ -254,8 +254,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => { onChange: jasmine.createSpy('onChange') }; (service as any).dynamicMatchers = [visibleMatcher]; - spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined); - spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable()); + ((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined); + ((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable()); const subscriptions = service.subscribeRelations(testModel, dcTypeControl); expect(visibleMatcher.onChange).toHaveBeenCalledWith(false, testModel, dcTypeControl, jasmine.anything());