Skip to content

Commit 8580e90

Browse files
authored
fix(submission): keep dc.type dropdown display after change (#157)
* fix(submission): keep dc.type dropdown display after change (ufal/clarin-dspace#1377 -> #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. * 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. * 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.
1 parent d0b5ecf commit 8580e90

3 files changed

Lines changed: 57 additions & 11 deletions

File tree

src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-type-bind-relation.service.spec.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
8686
it('Should not push undefined bind models', () => {
8787
const testModel = mockInputWithTypeBindModel;
8888
testModel.typeBindRelations = getTypeBindRelations(['boundType']);
89-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined);
89+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined);
9090

9191
const relatedModels = service.getRelatedFormModel(testModel);
9292

@@ -143,7 +143,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
143143
opposingMatch: HIDDEN_MATCHER.match,
144144
onChange: jasmine.createSpy('onChange')
145145
};
146-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined);
146+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined);
147147

148148
const hasMatch = service.matchesCondition(relation, visibleMatcher);
149149

@@ -159,7 +159,7 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
159159
opposingMatch: MATCH_VISIBLE,
160160
onChange: jasmine.createSpy('onChange')
161161
};
162-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.returnValue(undefined);
162+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.returnValue(undefined);
163163

164164
const hasMatch = service.matchesCondition(relation, hiddenMatcher);
165165

@@ -185,8 +185,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
185185
onChange: jasmine.createSpy('onChange')
186186
};
187187
(service as any).dynamicMatchers = [visibleMatcher];
188-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined);
189-
spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable());
188+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined);
189+
((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable());
190190

191191
const subscriptions = service.subscribeRelations(testModel, dcTypeControl);
192192
expect(subscriptions.length).toBe(1);
@@ -220,8 +220,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
220220
onChange: jasmine.createSpy('onChange')
221221
};
222222
(service as any).dynamicMatchers = [hiddenMatcher];
223-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined);
224-
spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable());
223+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined);
224+
((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable());
225225

226226
const subscriptions = service.subscribeRelations(testModel, dcTypeControl);
227227
expect(subscriptions.length).toBe(1);
@@ -254,8 +254,8 @@ describe('DSDynamicTypeBindRelationService test suite', () => {
254254
onChange: jasmine.createSpy('onChange')
255255
};
256256
(service as any).dynamicMatchers = [visibleMatcher];
257-
spyOn((service as any).formBuilderService, 'getTypeBindModel').and.callFake(() => bindModelAvailable ? bindModel : undefined);
258-
spyOn((service as any).formBuilderService, 'getTypeBindModelUpdates').and.returnValue(bindModelUpdates$.asObservable());
257+
((service as any).formBuilderService.getTypeBindModel as jasmine.Spy).and.callFake(() => bindModelAvailable ? bindModel : undefined);
258+
((service as any).formBuilderService.getTypeBindModelUpdates as jasmine.Spy).and.returnValue(bindModelUpdates$.asObservable());
259259

260260
const subscriptions = service.subscribeRelations(testModel, dcTypeControl);
261261
expect(visibleMatcher.onChange).toHaveBeenCalledWith(false, testModel, dcTypeControl, jasmine.anything());

src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ComponentFixture, fakeAsync, inject, TestBed, tick, waitForAsync, } fro
55
import { By } from '@angular/platform-browser';
66
import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
77
import { TranslateModule } from '@ngx-translate/core';
8+
import { of as observableOf } from 'rxjs';
89
import { InfiniteScrollModule } from 'ngx-infinite-scroll';
910
import { DynamicFormLayoutService, DynamicFormsCoreModule, DynamicFormValidationService } from '@ng-dynamic-forms/core';
1011
import { DynamicFormsNGBootstrapUIModule } from '@ng-dynamic-forms/ui-ng-bootstrap';
@@ -15,6 +16,7 @@ import { VocabularyServiceStub } from '../../../../../testing/vocabulary-service
1516
import { DsDynamicScrollableDropdownComponent } from './dynamic-scrollable-dropdown.component';
1617
import { DynamicScrollableDropdownModel } from './dynamic-scrollable-dropdown.model';
1718
import { VocabularyEntry } from '../../../../../../core/submission/vocabularies/models/vocabulary-entry.model';
19+
import { FormFieldMetadataValueObject } from '../../../models/form-field-metadata-value.model';
1820
import { createTestComponent, hasClass } from '../../../../../testing/utils.test';
1921
import {
2022
mockDynamicFormLayoutService,
@@ -209,6 +211,50 @@ describe('Dynamic Dynamic Scrollable Dropdown component', () => {
209211
expect(scrollableDropdownComp.optionsList).toEqual(vocabularyServiceStub.getList());
210212
expect(scrollableDropdownComp.model.value).toEqual(modelValue);
211213
});
214+
215+
it('should fall back to the underlying value when display is empty on value change', () => {
216+
const valueWithEmptyDisplay = { display: '', value: 'Corpus' };
217+
scrollableDropdownComp.setCurrentValue(valueWithEmptyDisplay);
218+
219+
let currentValue;
220+
scrollableDropdownComp.currentValue.subscribe((v) => currentValue = v);
221+
222+
expect(currentValue).toBe('Corpus');
223+
});
224+
225+
it('should fall back to the underlying value when display is empty on init', () => {
226+
// Build the value bypassing the FormFieldMetadataValueObject constructor (which would
227+
// otherwise apply its own `display || value`), so the init branch genuinely receives an
228+
// empty display and exercises the fallback in setCurrentValue.
229+
const emptyDisplayValue = Object.assign(
230+
new FormFieldMetadataValueObject(),
231+
{ display: '', value: 'Corpus' }
232+
);
233+
spyOn(scrollableDropdownComp, 'getInitValueFromModel').and.returnValue(observableOf(emptyDisplayValue));
234+
scrollableDropdownComp.setCurrentValue(emptyDisplayValue, true);
235+
236+
let currentValue;
237+
scrollableDropdownComp.currentValue.subscribe((v) => currentValue = v);
238+
239+
expect(currentValue).toBe('Corpus');
240+
});
241+
242+
// Regression for ufal/clarin-dspace#1377: after a dc.type change the section
243+
// reloads and pushes a new value with an empty display through the form control.
244+
// The rendered input must not blank out.
245+
it('should keep the rendered value when a value with empty display arrives via valueChanges', fakeAsync(() => {
246+
const reloadedValue = Object.assign(
247+
new FormFieldMetadataValueObject(),
248+
{ display: '', value: 'Corpus', authority: 'corpus-auth' }
249+
);
250+
251+
scrollableDropdownComp.group.get(scrollableDropdownComp.model.id).setValue(reloadedValue);
252+
tick();
253+
scrollableDropdownFixture.detectChanges();
254+
255+
const input = scrollableDropdownFixture.debugElement.query(By.css('input.form-control')).nativeElement;
256+
expect(input.value).toBe('Corpus');
257+
}));
212258
});
213259
});
214260
});

src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ export class DsDynamicScrollableDropdownComponent extends DsDynamicVocabularyCom
276276

277277
if (init && !this.useFindAllService) {
278278
result = this.getInitValueFromModel().pipe(
279-
map((formValue: FormFieldMetadataValueObject) => formValue.display)
279+
map((formValue: FormFieldMetadataValueObject) => formValue.display || formValue.value)
280280
);
281281
} else {
282282
if (isEmpty(value)) {
@@ -286,7 +286,7 @@ export class DsDynamicScrollableDropdownComponent extends DsDynamicVocabularyCom
286286
} else if (this.useFindAllService) {
287287
result = observableOf(value[this.model.displayKey]);
288288
} else {
289-
result = observableOf(value.display);
289+
result = observableOf(value.display || value.value);
290290
}
291291
}
292292

0 commit comments

Comments
 (0)