From db3bcf26f12077dcda5d5b3b2d2869b1ff786489 Mon Sep 17 00:00:00 2001 From: Vlad0n20 Date: Mon, 29 Jun 2026 12:21:22 +0200 Subject: [PATCH 1/2] fix(collections): fix 400 on cedar metadata re-submit --- ...tion-confirmation-dialog.component.spec.ts | 36 +++++++++++++++++-- ...ollection-confirmation-dialog.component.ts | 14 +++++--- .../add-to-collection.component.spec.ts | 7 ++-- .../add-to-collection.component.ts | 5 +-- .../collection-metadata-step.component.ts | 2 +- 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.spec.ts b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.spec.ts index 4bd76944d..8ca0ac448 100644 --- a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.spec.ts +++ b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.spec.ts @@ -9,8 +9,12 @@ import { of, throwError } from 'rxjs'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { CreateCollectionSubmission } from '@osf/features/collections/store/add-to-collection/add-to-collection.actions'; -import { CedarMetadataAttributes, CedarRecordDataBinding } from '@osf/features/metadata/models'; -import { CreateCedarMetadataRecord } from '@osf/features/metadata/store'; +import { + CedarMetadataAttributes, + CedarMetadataRecordData, + CedarRecordDataBinding, +} from '@osf/features/metadata/models'; +import { CreateCedarMetadataRecord, UpdateCedarMetadataRecord } from '@osf/features/metadata/store'; import { UpdateProjectPublicStatus } from '@osf/features/project/overview/store'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; import { CollectionSubmissionPayload } from '@osf/shared/models/collections/collection-submission-payload.model'; @@ -29,6 +33,15 @@ const MOCK_CEDAR_DATA: CedarRecordDataBinding = { isPublished: true, }; +const MOCK_EXISTING_CEDAR_RECORD: CedarMetadataRecordData = { + id: 'cedar-record-1', + attributes: { metadata: {} as CedarMetadataAttributes, is_published: true }, + relationships: { + template: { data: { type: 'cedar-metadata-templates', id: 'template-1' } }, + target: { data: { type: 'nodes', id: 'project-1' } }, + }, +}; + describe('AddToCollectionConfirmationDialogComponent', () => { let component: AddToCollectionConfirmationDialogComponent; let fixture: ComponentFixture; @@ -40,6 +53,7 @@ describe('AddToCollectionConfirmationDialogComponent', () => { payload?: CollectionSubmissionPayload; project?: { id: string; isPublic: boolean }; cedarData?: CedarRecordDataBinding | null; + existingCedarRecord?: CedarMetadataRecordData | null; }; }; @@ -116,7 +130,7 @@ describe('AddToCollectionConfirmationDialogComponent', () => { expect(dialogRef.close).toHaveBeenCalledWith(true); }); - it('should create Cedar record before submission when cedarData is present', () => { + it('should create Cedar record before submission when cedarData is present and no existing record', () => { dialogConfig.data.cedarData = MOCK_CEDAR_DATA; vi.spyOn(store, 'dispatch').mockReturnValue(of(void 0)); @@ -125,6 +139,22 @@ describe('AddToCollectionConfirmationDialogComponent', () => { expect(store.dispatch).toHaveBeenCalledWith( new CreateCedarMetadataRecord(MOCK_CEDAR_DATA, 'project-1', ResourceType.Project) ); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdateCedarMetadataRecord)); + expect(store.dispatch).toHaveBeenCalledWith(new CreateCollectionSubmission(MOCK_PAYLOAD)); + expect(dialogRef.close).toHaveBeenCalledWith(true); + }); + + it('should update existing Cedar record instead of creating when existingCedarRecord is provided', () => { + dialogConfig.data.cedarData = MOCK_CEDAR_DATA; + dialogConfig.data.existingCedarRecord = MOCK_EXISTING_CEDAR_RECORD; + vi.spyOn(store, 'dispatch').mockReturnValue(of(void 0)); + + component.handleAddToCollectionConfirm(); + + expect(store.dispatch).toHaveBeenCalledWith( + new UpdateCedarMetadataRecord(MOCK_CEDAR_DATA, 'cedar-record-1', 'project-1', ResourceType.Project) + ); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(CreateCedarMetadataRecord)); expect(store.dispatch).toHaveBeenCalledWith(new CreateCollectionSubmission(MOCK_PAYLOAD)); expect(dialogRef.close).toHaveBeenCalledWith(true); }); diff --git a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts index 400437df7..387a829c5 100644 --- a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts +++ b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts @@ -11,8 +11,8 @@ import { ChangeDetectionStrategy, Component, DestroyRef, inject, signal } from ' import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { CreateCollectionSubmission } from '@osf/features/collections/store/add-to-collection/add-to-collection.actions'; -import { CedarRecordDataBinding } from '@osf/features/metadata/models'; -import { CreateCedarMetadataRecord } from '@osf/features/metadata/store'; +import { CedarMetadataRecordData, CedarRecordDataBinding } from '@osf/features/metadata/models'; +import { CreateCedarMetadataRecord, UpdateCedarMetadataRecord } from '@osf/features/metadata/store'; import { UpdateProjectPublicStatus } from '@osf/features/project/overview/store'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; import { ToastService } from '@osf/shared/services/toast.service'; @@ -34,12 +34,14 @@ export class AddToCollectionConfirmationDialogComponent { createCollectionSubmission: CreateCollectionSubmission, updateProjectPublicStatus: UpdateProjectPublicStatus, createCedarRecord: CreateCedarMetadataRecord, + updateCedarRecord: UpdateCedarMetadataRecord, }); handleAddToCollectionConfirm(): void { const payload = this.config.data.payload; const project = this.config.data.project; const cedarData = this.config.data.cedarData as CedarRecordDataBinding | null | undefined; + const existingCedarRecord = this.config.data.existingCedarRecord as CedarMetadataRecordData | null | undefined; if (!payload || !project) return; @@ -50,13 +52,15 @@ export class AddToCollectionConfirmationDialogComponent { ? of(null) : this.actions.updateProjectPublicStatus(projectPayload); - const createCedar$: Observable = cedarData - ? this.actions.createCedarRecord(cedarData, project.id as string, ResourceType.Project) + const saveCedar$: Observable = cedarData + ? existingCedarRecord + ? this.actions.updateCedarRecord(cedarData, existingCedarRecord.id!, project.id as string, ResourceType.Project) + : this.actions.createCedarRecord(cedarData, project.id as string, ResourceType.Project) : of(null); updatePublicStatus$ .pipe( - switchMap(() => createCedar$), + switchMap(() => saveCedar$), switchMap(() => this.actions.createCollectionSubmission(payload)), takeUntilDestroyed(this.destroyRef) ) diff --git a/src/app/features/collections/components/add-to-collection/add-to-collection.component.spec.ts b/src/app/features/collections/components/add-to-collection/add-to-collection.component.spec.ts index 63f25e48d..6839c9d3c 100644 --- a/src/app/features/collections/components/add-to-collection/add-to-collection.component.spec.ts +++ b/src/app/features/collections/components/add-to-collection/add-to-collection.component.spec.ts @@ -270,13 +270,16 @@ describe('AddToCollectionComponent', () => { expect(result).toBeUndefined(); }); - it('should open confirmation dialog in new submission mode', () => { + it('should open confirmation dialog in new submission mode with existingCedarRecord in data', () => { const { component, dialogService } = setup(); component.handleAddToCollection(); expect(dialogService.open).toHaveBeenCalledWith( expect.any(Function), - expect.objectContaining({ header: 'collections.addToCollection.confirmationDialogHeader' }) + expect.objectContaining({ + header: 'collections.addToCollection.confirmationDialogHeader', + data: expect.objectContaining({ existingCedarRecord: null }), + }) ); }); diff --git a/src/app/features/collections/components/add-to-collection/add-to-collection.component.ts b/src/app/features/collections/components/add-to-collection/add-to-collection.component.ts index af90d8753..ac2ee4e37 100644 --- a/src/app/features/collections/components/add-to-collection/add-to-collection.component.ts +++ b/src/app/features/collections/components/add-to-collection/add-to-collection.component.ts @@ -245,11 +245,11 @@ export class AddToCollectionComponent implements CanDeactivateComponent { payload, project: this.selectedProject(), cedarData: this.pendingCedarData(), + existingCedarRecord: this.existingCedarRecord(), }, }) .onClose.pipe( filter((res) => !!res), - switchMap(() => this.saveCedarRecordIfNeeded()), takeUntilDestroyed(this.destroyRef) ) .subscribe({ @@ -257,9 +257,6 @@ export class AddToCollectionComponent implements CanDeactivateComponent { this.allowNavigation.set(true); this.router.navigate([this.selectedProject()?.id, 'overview']); }, - error: () => { - this.toastService.showError('collections.addToCollection.updateError'); - }, }); } } 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 9b6dfa057..007d6a6d2 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 @@ -135,7 +135,7 @@ export class CollectionMetadataStepComponent { const template = this.cedarTemplate(); if (!editor || !template) return; - const currentMetadata = editor.currentMetadata; + const currentMetadata = editor.currentMetadata ?? this.cedarFormData(); const isValid = !!editor.dataQualityReport?.isValid; if (currentMetadata) { From bdbad69b82eb1bbebd37a76171d52139a4d7b05f Mon Sep 17 00:00:00 2001 From: Vlad0n20 Date: Mon, 29 Jun 2026 16:50:25 +0200 Subject: [PATCH 2/2] fix(collections): fix comment --- .../add-to-collection-confirmation-dialog.component.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts index 387a829c5..52aef57c0 100644 --- a/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts +++ b/src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts @@ -15,6 +15,7 @@ import { CedarMetadataRecordData, CedarRecordDataBinding } from '@osf/features/m import { CreateCedarMetadataRecord, UpdateCedarMetadataRecord } from '@osf/features/metadata/store'; import { UpdateProjectPublicStatus } from '@osf/features/project/overview/store'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; +import { ProjectModel } from '@osf/shared/models/projects/projects.model'; import { ToastService } from '@osf/shared/services/toast.service'; @Component({ @@ -39,14 +40,14 @@ export class AddToCollectionConfirmationDialogComponent { handleAddToCollectionConfirm(): void { const payload = this.config.data.payload; - const project = this.config.data.project; + const project = this.config.data.project as ProjectModel | null | undefined; const cedarData = this.config.data.cedarData as CedarRecordDataBinding | null | undefined; const existingCedarRecord = this.config.data.existingCedarRecord as CedarMetadataRecordData | null | undefined; if (!payload || !project) return; this.isSubmitting.set(true); - const projectPayload = [{ id: project.id as string, public: true }]; + const projectPayload = [{ id: project.id, public: true }]; const updatePublicStatus$: Observable = project.isPublic ? of(null) @@ -54,8 +55,8 @@ export class AddToCollectionConfirmationDialogComponent { const saveCedar$: Observable = cedarData ? existingCedarRecord - ? this.actions.updateCedarRecord(cedarData, existingCedarRecord.id!, project.id as string, ResourceType.Project) - : this.actions.createCedarRecord(cedarData, project.id as string, ResourceType.Project) + ? this.actions.updateCedarRecord(cedarData, existingCedarRecord.id!, project.id, ResourceType.Project) + : this.actions.createCedarRecord(cedarData, project.id, ResourceType.Project) : of(null); updatePublicStatus$