From 1cd0d2b769c0cbdc2efa41c72ee0e3f4b536d32b Mon Sep 17 00:00:00 2001 From: nsemets Date: Tue, 30 Jun 2026 21:18:47 +0300 Subject: [PATCH] fix(collection): fixed cedar issue --- .../collection-metadata-step.component.html | 37 +- ...collection-metadata-step.component.spec.ts | 333 +++++++++++------- .../collection-metadata-step.component.ts | 38 +- 3 files changed, 263 insertions(+), 145 deletions(-) diff --git a/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.html b/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.html index 0b0cd6498..7c493f920 100644 --- a/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.html +++ b/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.html @@ -9,15 +9,16 @@

{{ 'collections.addToCollection.collectionMetadata' | translate }}

- @if (!isDisabled() && stepperActiveValue() !== targetStepValue()) { + @if (!isDisabled() && !isStepActive()) { @if (collectionMetadataSaved()) { @if (isCedarMode()) { - @if (cedarTemplate()) { + @let cedarTpl = cedarTemplate(); + + @if (cedarTpl) { } } @else { @@ -45,19 +46,23 @@

{{ 'collections.addToCollection.collectionMetadata' | translate }}

- + @if (isCedarMode()) { - @if (cedarTemplate()) { -
- -
+ @let cedarTpl = cedarTemplate(); + + @if (cedarTpl) { + @if (isStepActive()) { +
+ +
+ }
{ let component: CollectionMetadataStepComponent; let fixture: ComponentFixture; + let store: Store; function setup( options: { isCedarMode?: boolean; cedarTemplate?: CedarMetadataDataTemplateJsonApi | null; existingCedarRecord?: CedarMetadataRecordData | null; + stepperActiveValue?: number; + targetStepValue?: number; + isDisabled?: boolean; + primaryCollectionId?: string; + filterOptions?: typeof MOCK_COLLECTIONS_FILTERS_OPTIONS; + collectionSubmission?: CollectionProjectSubmission | null; + selectorOverrides?: SignalOverride[]; } = {} ) { + const signals = mergeSignalOverrides( + [ + { selector: CollectionsSelectors.getCollectionProvider, value: null }, + { selector: CollectionsSelectors.getCollectionProviderLoading, value: false }, + { selector: CollectionsSelectors.getAllFiltersOptions, value: options.filterOptions ?? {} }, + { + selector: AddToCollectionSelectors.getCurrentCollectionSubmission, + value: options.collectionSubmission ?? null, + }, + ], + options.selectorOverrides + ); + TestBed.configureTestingModule({ imports: [CollectionMetadataStepComponent, MockComponents(StepPanel, Step, StepItem)], - providers: [ - provideOSFCore(), - provideMockStore({ - signals: [ - { selector: CollectionsSelectors.getCollectionProvider, value: null }, - { selector: CollectionsSelectors.getCollectionProviderLoading, value: false }, - { selector: CollectionsSelectors.getAllFiltersOptions, value: {} }, - { selector: AddToCollectionSelectors.getCurrentCollectionSubmission, value: null }, - ], - }), - ], + providers: [provideOSFCore(), provideMockStore({ signals })], }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(CollectionMetadataStepComponent); component = fixture.componentInstance; - fixture.componentRef.setInput('stepperActiveValue', 0); - fixture.componentRef.setInput('targetStepValue', 1); - fixture.componentRef.setInput('isDisabled', false); - fixture.componentRef.setInput('primaryCollectionId', 'test-collection-id'); + fixture.componentRef.setInput('stepperActiveValue', options.stepperActiveValue ?? 0); + fixture.componentRef.setInput('targetStepValue', options.targetStepValue ?? 1); + fixture.componentRef.setInput('isDisabled', options.isDisabled ?? false); + fixture.componentRef.setInput('primaryCollectionId', options.primaryCollectionId ?? 'test-collection-id'); if (options.isCedarMode !== undefined) { fixture.componentRef.setInput('isCedarMode', options.isCedarMode); @@ -63,26 +108,31 @@ describe('CollectionMetadataStepComponent', () => { fixture.detectChanges(); } - beforeEach(() => { + it('should create', () => { setup(); - }); - it('should create', () => { expect(component).toBeTruthy(); }); it('should initialize with input values', () => { + setup(); + expect(component.stepperActiveValue()).toBe(0); expect(component.targetStepValue()).toBe(1); expect(component.isDisabled()).toBe(false); expect(component.isCedarMode()).toBe(false); + expect(component.isStepActive()).toBe(false); }); - it('should default isCedarMode to false', () => { - expect(component.isCedarMode()).toBe(false); + it('should dispatch GetCollectionDetails when primaryCollectionId is set', () => { + setup(); + + expect(store.dispatch).toHaveBeenCalledWith(new GetCollectionDetails('test-collection-id')); }); it('should handle save metadata in filter mode', () => { + setup(); + const mockForm = new FormGroup({}); component.collectionMetadataForm.set(mockForm); @@ -96,19 +146,9 @@ describe('CollectionMetadataStepComponent', () => { expect(component.collectionMetadataSaved()).toBe(true); }); - it('should handle form validation', () => { - const validForm = new FormGroup({ - title: new FormControl('Test Collection', [Validators.required]), - }); - const invalidForm = new FormGroup({ - title: new FormControl('', [Validators.required]), - }); - - expect(validForm.valid).toBe(true); - expect(invalidForm.valid).toBe(false); - }); - it('should handle step navigation', () => { + setup(); + const navigateSpy = vi.spyOn(component.stepChange, 'emit'); component.handleEditStep(); @@ -116,119 +156,176 @@ describe('CollectionMetadataStepComponent', () => { expect(navigateSpy).toHaveBeenCalledWith(component.targetStepValue()); }); - it('should handle discard changes in filter mode', () => { - const mockForm = new FormGroup({}); - component.collectionMetadataForm.set(mockForm); + it('should reflect step active state from stepper inputs', () => { + setup(); + + fixture.componentRef.setInput('stepperActiveValue', 3); + fixture.componentRef.setInput('targetStepValue', 3); + fixture.componentRef.setInput('isDisabled', true); + fixture.detectChanges(); + + expect(component.stepperActiveValue()).toBe(3); + expect(component.targetStepValue()).toBe(3); + expect(component.isDisabled()).toBe(true); + expect(component.isStepActive()).toBe(true); + }); + + it('should build available filter entries from collection filter options', () => { + setup({ filterOptions: MOCK_COLLECTIONS_FILTERS_OPTIONS }); + + const entries = component.availableFilterEntries(); + + expect(entries.length).toBeGreaterThan(0); + expect(entries.some((entry) => entry.key === CollectionFilterType.ProgramArea)).toBe(true); + }); + + it('should populate collection metadata form from submission', () => { + setup({ + filterOptions: MOCK_COLLECTIONS_FILTERS_OPTIONS, + collectionSubmission: MOCK_PROJECT_SUBMISSION, + stepperActiveValue: AddToCollectionSteps.CollectionMetadata, + targetStepValue: AddToCollectionSteps.CollectionMetadata, + }); + + expect(component.collectionMetadataForm().get(CollectionFilterType.ProgramArea)?.value).toBe('Science'); + }); + + it('should restore original filter values on discard', () => { + setup({ + filterOptions: MOCK_COLLECTIONS_FILTERS_OPTIONS, + collectionSubmission: MOCK_PROJECT_SUBMISSION, + stepperActiveValue: AddToCollectionSteps.CollectionMetadata, + targetStepValue: AddToCollectionSteps.CollectionMetadata, + }); + + const form = component.collectionMetadataForm(); + form.get(CollectionFilterType.ProgramArea)?.setValue('Technology'); component.collectionMetadataSaved.set(true); component.handleDiscardChanges(); expect(component.collectionMetadataSaved()).toBe(false); + expect(form.get(CollectionFilterType.ProgramArea)?.value).toBe('Science'); }); - it('should have collection metadata form', () => { - expect(component.collectionMetadataForm()).toBeDefined(); - expect(component.collectionMetadataSaved()).toBe(false); + it('should initialize in CEDAR mode', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); + + expect(component.isCedarMode()).toBe(true); + expect(component.cedarTemplate()).toEqual(MOCK_CEDAR_TEMPLATE); }); - it('should handle different input values', () => { - fixture.componentRef.setInput('stepperActiveValue', 2); - fixture.componentRef.setInput('targetStepValue', 3); - fixture.componentRef.setInput('isDisabled', true); - fixture.detectChanges(); + it('should handle discard changes in CEDAR mode without existing record', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); - expect(component.stepperActiveValue()).toBe(2); - expect(component.targetStepValue()).toBe(3); - expect(component.isDisabled()).toBe(true); + component.cedarFormData.set({ field: 'value' }); + component.collectionMetadataSaved.set(true); + + component.handleDiscardChanges(); + + expect(component.collectionMetadataSaved()).toBe(false); + expect(component.cedarFormData()).toEqual({}); }); - describe('CEDAR mode', () => { - beforeEach(() => { - setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); + it('should discard cedar changes to existing record metadata', () => { + setup({ + isCedarMode: true, + cedarTemplate: MOCK_CEDAR_TEMPLATE, + existingCedarRecord: MOCK_CEDAR_RECORD, }); - it('should initialize in CEDAR mode', () => { - expect(component.isCedarMode()).toBe(true); - expect(component.cedarTemplate()).toEqual(MOCK_CEDAR_TEMPLATE); - }); + component.cedarFormData.set({ field: 'edited' }); + component.collectionMetadataSaved.set(true); - it('should handle discard changes in CEDAR mode', () => { - component.cedarFormData.set({ field: 'value' }); - component.collectionMetadataSaved.set(true); + component.handleDiscardChanges(); - component.handleDiscardChanges(); + expect(component.collectionMetadataSaved()).toBe(false); + expect(component.cedarFormData()).toEqual({ field: 'value' }); + }); - expect(component.collectionMetadataSaved()).toBe(false); - expect(component.cedarFormData()).toEqual({}); + it('should populate cedarFormData from existingCedarRecord', () => { + setup({ + isCedarMode: true, + cedarTemplate: MOCK_CEDAR_TEMPLATE, + existingCedarRecord: MOCK_CEDAR_RECORD, }); - it('should handle discard changes with existing record in CEDAR mode', () => { - const existingRecord: CedarMetadataRecordData = { - attributes: { - metadata: { field: 'original' } as unknown as CedarMetadataRecordData['attributes']['metadata'], - is_published: false, - }, - relationships: { - template: { data: { type: 'cedar-metadata-templates', id: 'template-1' } }, - target: { data: { type: 'nodes', id: 'node-1' } }, - }, - }; - fixture.componentRef.setInput('existingCedarRecord', existingRecord); - fixture.detectChanges(); + expect(component.cedarFormData()).toEqual({ field: 'value' }); + }); - component.collectionMetadataSaved.set(true); - component.handleDiscardChanges(); + it('should not overwrite cedarFormData from API when metadata is already saved locally', () => { + setup({ + isCedarMode: true, + cedarTemplate: MOCK_CEDAR_TEMPLATE, + existingCedarRecord: MOCK_CEDAR_RECORD, + }); - expect(component.collectionMetadataSaved()).toBe(false); + component.collectionMetadataSaved.set(true); + component.cedarFormData.set({ field: 'local' }); + + fixture.componentRef.setInput('existingCedarRecord', { + ...MOCK_CEDAR_RECORD, + attributes: { + ...MOCK_CEDAR_RECORD.attributes, + metadata: { field: 'api' } as unknown as CedarMetadataRecordData['attributes']['metadata'], + }, }); + fixture.detectChanges(); - it('should populate cedarFormData from existingCedarRecord', () => { - const existingRecord: CedarMetadataRecordData = { - attributes: { - metadata: { field: 'existing' } as unknown as CedarMetadataRecordData['attributes']['metadata'], - is_published: true, - }, - relationships: { - template: { data: { type: 'cedar-metadata-templates', id: 'template-1' } }, - target: { data: { type: 'nodes', id: 'node-1' } }, - }, - }; - fixture.componentRef.setInput('existingCedarRecord', existingRecord); - fixture.detectChanges(); + expect(component.cedarFormData()).toEqual({ field: 'local' }); + }); - expect(component.cedarFormData()).toEqual({ field: 'existing' }); - }); + it('should not emit cedarDataSaved when handleSaveCedarMetadata is called without editor', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); - it('should emit cedarDataSaved when handleSaveCedarMetadata is called without editor', () => { - const cedarDataSavedSpy = vi.spyOn(component.cedarDataSaved, 'emit'); - const stepChangeSpy = vi.spyOn(component.stepChange, 'emit'); + const cedarDataSavedSpy = vi.spyOn(component.cedarDataSaved, 'emit'); + const stepChangeSpy = vi.spyOn(component.stepChange, 'emit'); - component.handleSaveCedarMetadata(); + component.handleSaveCedarMetadata(); - expect(cedarDataSavedSpy).not.toHaveBeenCalled(); - expect(stepChangeSpy).not.toHaveBeenCalled(); - }); + expect(cedarDataSavedSpy).not.toHaveBeenCalled(); + expect(stepChangeSpy).not.toHaveBeenCalled(); + }); - it('should handle onCedarChange event', () => { - const mockMetadata = { field: 'changed' }; - const mockEditor = { currentMetadata: mockMetadata } as unknown as EventTarget; - const mockEvent = new CustomEvent('change'); - Object.defineProperty(mockEvent, 'target', { value: mockEditor }); + it('should handle onCedarChange event', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); - component.onCedarChange(mockEvent); + const mockMetadata = { field: 'changed' }; + const mockEditor = { currentMetadata: mockMetadata }; + const mockEvent = new CustomEvent('change'); - expect(component.cedarFormData()).toEqual(mockMetadata); - }); + Object.defineProperty(mockEvent, 'target', { value: mockEditor, writable: true }); - it('should not call handleSaveCedarMetadata without template', () => { - fixture.componentRef.setInput('cedarTemplate', null); - fixture.detectChanges(); + component.onCedarChange(mockEvent); - const cedarDataSavedSpy = vi.spyOn(component.cedarDataSaved, 'emit'); + expect(component.cedarFormData()).toEqual(mockMetadata); + }); - component.handleSaveCedarMetadata(); + it('should not update cedarFormData when onCedarChange has no currentMetadata', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); - expect(cedarDataSavedSpy).not.toHaveBeenCalled(); - }); + const mockEvent = new CustomEvent('change', {}); + const mockEditor = {}; + + Object.defineProperty(mockEvent, 'target', { value: mockEditor, writable: true }); + + const initialFormData = component.cedarFormData(); + + component.onCedarChange(mockEvent); + + expect(component.cedarFormData()).toEqual(initialFormData); + }); + + it('should not emit cedarDataSaved without template', () => { + setup({ isCedarMode: true, cedarTemplate: MOCK_CEDAR_TEMPLATE }); + + fixture.componentRef.setInput('cedarTemplate', null); + fixture.detectChanges(); + + const cedarDataSavedSpy = vi.spyOn(component.cedarDataSaved, 'emit'); + + component.handleSaveCedarMetadata(); + + expect(cedarDataSavedSpy).not.toHaveBeenCalled(); }); }); diff --git a/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.ts b/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.ts index 007d6a6d2..02db7617b 100644 --- a/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.ts +++ b/src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.ts @@ -17,6 +17,7 @@ import { input, output, signal, + untracked, viewChild, ViewEncapsulation, } from '@angular/core'; @@ -88,6 +89,8 @@ export class CollectionMetadataStepComponent { private readonly actions = createDispatchMap({ getCollectionDetails: GetCollectionDetails }); + readonly isStepActive = computed(() => this.stepperActiveValue() === this.targetStepValue()); + constructor() { this.setupEffects(); } @@ -102,10 +105,7 @@ export class CollectionMetadataStepComponent { this.cedarFormData.set( record?.attributes?.metadata ? (record.attributes.metadata as Record) : {} ); - const editor = this.cedarEditor()?.nativeElement; - if (editor) { - editor.instanceObject = this.cedarFormData(); - } + this.syncCedarInstance(this.cedarEditor()?.nativeElement); this.collectionMetadataSaved.set(false); return; } @@ -195,17 +195,27 @@ export class CollectionMetadataStepComponent { }); effect(() => { + if (this.collectionMetadataSaved()) return; + const record = this.existingCedarRecord(); if (record?.attributes?.metadata) { - const metadata = record.attributes.metadata as Record; - this.cedarFormData.set(metadata); - const editor = this.cedarEditor()?.nativeElement; - if (editor) editor.instanceObject = metadata; - const viewer = this.cedarViewer()?.nativeElement; - if (viewer) viewer.instanceObject = metadata; + this.cedarFormData.set(record.attributes.metadata as Record); } }); + effect(() => { + if (!this.isStepActive()) return; + + this.existingCedarRecord(); + + this.syncCedarInstance(this.cedarEditor()?.nativeElement); + }); + + effect(() => { + if (this.isStepActive() || !this.collectionMetadataSaved()) return; + this.syncCedarInstance(this.cedarViewer()?.nativeElement); + }); + effect(() => { const filterEntries = this.availableFilterEntries(); if (filterEntries.length && !this.isCedarMode()) { @@ -233,7 +243,7 @@ export class CollectionMetadataStepComponent { }); effect(() => { - if (!this.collectionMetadataSaved() && this.stepperActiveValue() !== AddToCollectionSteps.CollectionMetadata) { + if (!this.collectionMetadataSaved() && !this.isStepActive()) { if (!this.isCedarMode()) { this.collectionMetadataForm().reset(); this.formPopulatedFromSubmission.set(false); @@ -242,6 +252,12 @@ export class CollectionMetadataStepComponent { }); } + private syncCedarInstance(element: CedarEditorElement | undefined): void { + if (element) { + element.instanceObject = untracked(() => this.cedarFormData()); + } + } + private hasFormChanges(form: FormGroup, originalValues: Record): boolean { return Object.keys(originalValues).some((key) => { const currentValue = form.get(key)?.value;