Skip to content

Commit aeba58b

Browse files
authored
Merge pull request #1023 from Vlad0n20/fix/ENG-11519
fix(collections): fix 400 on cedar metadata re-submit
2 parents 8028544 + bdbad69 commit aeba58b

5 files changed

Lines changed: 52 additions & 17 deletions

File tree

src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.spec.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@ import { of, throwError } from 'rxjs';
99
import { ComponentFixture, TestBed } from '@angular/core/testing';
1010

1111
import { CreateCollectionSubmission } from '@osf/features/collections/store/add-to-collection/add-to-collection.actions';
12-
import { CedarMetadataAttributes, CedarRecordDataBinding } from '@osf/features/metadata/models';
13-
import { CreateCedarMetadataRecord } from '@osf/features/metadata/store';
12+
import {
13+
CedarMetadataAttributes,
14+
CedarMetadataRecordData,
15+
CedarRecordDataBinding,
16+
} from '@osf/features/metadata/models';
17+
import { CreateCedarMetadataRecord, UpdateCedarMetadataRecord } from '@osf/features/metadata/store';
1418
import { UpdateProjectPublicStatus } from '@osf/features/project/overview/store';
1519
import { ResourceType } from '@osf/shared/enums/resource-type.enum';
1620
import { CollectionSubmissionPayload } from '@osf/shared/models/collections/collection-submission-payload.model';
@@ -29,6 +33,15 @@ const MOCK_CEDAR_DATA: CedarRecordDataBinding = {
2933
isPublished: true,
3034
};
3135

36+
const MOCK_EXISTING_CEDAR_RECORD: CedarMetadataRecordData = {
37+
id: 'cedar-record-1',
38+
attributes: { metadata: {} as CedarMetadataAttributes, is_published: true },
39+
relationships: {
40+
template: { data: { type: 'cedar-metadata-templates', id: 'template-1' } },
41+
target: { data: { type: 'nodes', id: 'project-1' } },
42+
},
43+
};
44+
3245
describe('AddToCollectionConfirmationDialogComponent', () => {
3346
let component: AddToCollectionConfirmationDialogComponent;
3447
let fixture: ComponentFixture<AddToCollectionConfirmationDialogComponent>;
@@ -40,6 +53,7 @@ describe('AddToCollectionConfirmationDialogComponent', () => {
4053
payload?: CollectionSubmissionPayload;
4154
project?: { id: string; isPublic: boolean };
4255
cedarData?: CedarRecordDataBinding | null;
56+
existingCedarRecord?: CedarMetadataRecordData | null;
4357
};
4458
};
4559

@@ -116,7 +130,7 @@ describe('AddToCollectionConfirmationDialogComponent', () => {
116130
expect(dialogRef.close).toHaveBeenCalledWith(true);
117131
});
118132

119-
it('should create Cedar record before submission when cedarData is present', () => {
133+
it('should create Cedar record before submission when cedarData is present and no existing record', () => {
120134
dialogConfig.data.cedarData = MOCK_CEDAR_DATA;
121135
vi.spyOn(store, 'dispatch').mockReturnValue(of(void 0));
122136

@@ -125,6 +139,22 @@ describe('AddToCollectionConfirmationDialogComponent', () => {
125139
expect(store.dispatch).toHaveBeenCalledWith(
126140
new CreateCedarMetadataRecord(MOCK_CEDAR_DATA, 'project-1', ResourceType.Project)
127141
);
142+
expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdateCedarMetadataRecord));
143+
expect(store.dispatch).toHaveBeenCalledWith(new CreateCollectionSubmission(MOCK_PAYLOAD));
144+
expect(dialogRef.close).toHaveBeenCalledWith(true);
145+
});
146+
147+
it('should update existing Cedar record instead of creating when existingCedarRecord is provided', () => {
148+
dialogConfig.data.cedarData = MOCK_CEDAR_DATA;
149+
dialogConfig.data.existingCedarRecord = MOCK_EXISTING_CEDAR_RECORD;
150+
vi.spyOn(store, 'dispatch').mockReturnValue(of(void 0));
151+
152+
component.handleAddToCollectionConfirm();
153+
154+
expect(store.dispatch).toHaveBeenCalledWith(
155+
new UpdateCedarMetadataRecord(MOCK_CEDAR_DATA, 'cedar-record-1', 'project-1', ResourceType.Project)
156+
);
157+
expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(CreateCedarMetadataRecord));
128158
expect(store.dispatch).toHaveBeenCalledWith(new CreateCollectionSubmission(MOCK_PAYLOAD));
129159
expect(dialogRef.close).toHaveBeenCalledWith(true);
130160
});

src/app/features/collections/components/add-to-collection/add-to-collection-confirmation-dialog/add-to-collection-confirmation-dialog.component.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import { ChangeDetectionStrategy, Component, DestroyRef, inject, signal } from '
1111
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
1212

1313
import { CreateCollectionSubmission } from '@osf/features/collections/store/add-to-collection/add-to-collection.actions';
14-
import { CedarRecordDataBinding } from '@osf/features/metadata/models';
15-
import { CreateCedarMetadataRecord } from '@osf/features/metadata/store';
14+
import { CedarMetadataRecordData, CedarRecordDataBinding } from '@osf/features/metadata/models';
15+
import { CreateCedarMetadataRecord, UpdateCedarMetadataRecord } from '@osf/features/metadata/store';
1616
import { UpdateProjectPublicStatus } from '@osf/features/project/overview/store';
1717
import { ResourceType } from '@osf/shared/enums/resource-type.enum';
18+
import { ProjectModel } from '@osf/shared/models/projects/projects.model';
1819
import { ToastService } from '@osf/shared/services/toast.service';
1920

2021
@Component({
@@ -34,29 +35,33 @@ export class AddToCollectionConfirmationDialogComponent {
3435
createCollectionSubmission: CreateCollectionSubmission,
3536
updateProjectPublicStatus: UpdateProjectPublicStatus,
3637
createCedarRecord: CreateCedarMetadataRecord,
38+
updateCedarRecord: UpdateCedarMetadataRecord,
3739
});
3840

3941
handleAddToCollectionConfirm(): void {
4042
const payload = this.config.data.payload;
41-
const project = this.config.data.project;
43+
const project = this.config.data.project as ProjectModel | null | undefined;
4244
const cedarData = this.config.data.cedarData as CedarRecordDataBinding | null | undefined;
45+
const existingCedarRecord = this.config.data.existingCedarRecord as CedarMetadataRecordData | null | undefined;
4346

4447
if (!payload || !project) return;
4548

4649
this.isSubmitting.set(true);
47-
const projectPayload = [{ id: project.id as string, public: true }];
50+
const projectPayload = [{ id: project.id, public: true }];
4851

4952
const updatePublicStatus$: Observable<unknown> = project.isPublic
5053
? of(null)
5154
: this.actions.updateProjectPublicStatus(projectPayload);
5255

53-
const createCedar$: Observable<unknown> = cedarData
54-
? this.actions.createCedarRecord(cedarData, project.id as string, ResourceType.Project)
56+
const saveCedar$: Observable<unknown> = cedarData
57+
? existingCedarRecord
58+
? this.actions.updateCedarRecord(cedarData, existingCedarRecord.id!, project.id, ResourceType.Project)
59+
: this.actions.createCedarRecord(cedarData, project.id, ResourceType.Project)
5560
: of(null);
5661

5762
updatePublicStatus$
5863
.pipe(
59-
switchMap(() => createCedar$),
64+
switchMap(() => saveCedar$),
6065
switchMap(() => this.actions.createCollectionSubmission(payload)),
6166
takeUntilDestroyed(this.destroyRef)
6267
)

src/app/features/collections/components/add-to-collection/add-to-collection.component.spec.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,13 +270,16 @@ describe('AddToCollectionComponent', () => {
270270
expect(result).toBeUndefined();
271271
});
272272

273-
it('should open confirmation dialog in new submission mode', () => {
273+
it('should open confirmation dialog in new submission mode with existingCedarRecord in data', () => {
274274
const { component, dialogService } = setup();
275275
component.handleAddToCollection();
276276

277277
expect(dialogService.open).toHaveBeenCalledWith(
278278
expect.any(Function),
279-
expect.objectContaining({ header: 'collections.addToCollection.confirmationDialogHeader' })
279+
expect.objectContaining({
280+
header: 'collections.addToCollection.confirmationDialogHeader',
281+
data: expect.objectContaining({ existingCedarRecord: null }),
282+
})
280283
);
281284
});
282285

src/app/features/collections/components/add-to-collection/add-to-collection.component.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,21 +245,18 @@ export class AddToCollectionComponent implements CanDeactivateComponent {
245245
payload,
246246
project: this.selectedProject(),
247247
cedarData: this.pendingCedarData(),
248+
existingCedarRecord: this.existingCedarRecord(),
248249
},
249250
})
250251
.onClose.pipe(
251252
filter((res) => !!res),
252-
switchMap(() => this.saveCedarRecordIfNeeded()),
253253
takeUntilDestroyed(this.destroyRef)
254254
)
255255
.subscribe({
256256
next: () => {
257257
this.allowNavigation.set(true);
258258
this.router.navigate([this.selectedProject()?.id, 'overview']);
259259
},
260-
error: () => {
261-
this.toastService.showError('collections.addToCollection.updateError');
262-
},
263260
});
264261
}
265262
}

src/app/features/collections/components/add-to-collection/collection-metadata-step/collection-metadata-step.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export class CollectionMetadataStepComponent {
135135
const template = this.cedarTemplate();
136136
if (!editor || !template) return;
137137

138-
const currentMetadata = editor.currentMetadata;
138+
const currentMetadata = editor.currentMetadata ?? this.cedarFormData();
139139
const isValid = !!editor.dataQualityReport?.isValid;
140140

141141
if (currentMetadata) {

0 commit comments

Comments
 (0)