diff --git a/src/app/features/registries/components/custom-step/custom-step.component.html b/src/app/features/registries/components/custom-step/custom-step.component.html index 022d7d005..e561d3af7 100644 --- a/src/app/features/registries/components/custom-step/custom-step.component.html +++ b/src/app/features/registries/components/custom-step/custom-step.component.html @@ -163,7 +163,7 @@

{{ 'files.actions.uploadFile' | translate }}

[filesViewOnly]="filesViewOnly()" (attachFile)="onAttachFile($event, q.responseKey!)" (openFile)="onOpenFile($event)" - (removeFromAttachedFiles)="removeFromAttachedFiles($event, q.responseKey!)" + (removeFromAttachedFiles)="removeDeletedFileFromAllQuestions($event)" > } diff --git a/src/app/features/registries/components/custom-step/custom-step.component.spec.ts b/src/app/features/registries/components/custom-step/custom-step.component.spec.ts index b1c3b39f9..97f2e05c1 100644 --- a/src/app/features/registries/components/custom-step/custom-step.component.spec.ts +++ b/src/app/features/registries/components/custom-step/custom-step.component.spec.ts @@ -6,6 +6,7 @@ import { Mock } from 'vitest'; import { TestBed } from '@angular/core/testing'; import { FormGroup } from '@angular/forms'; +import { By } from '@angular/platform-browser'; import { ActivatedRoute, Router, UrlTree } from '@angular/router'; import { UserSelectors } from '@osf/core/store/user/user.selectors'; @@ -298,7 +299,7 @@ describe('CustomStepComponent', () => { pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], stepsData: { files: [] }, }); - component.attachedFiles['files'] = [{ file_id: 'file-1', name: 'existing.txt' }]; + component.attachedFiles['files'] = [{ ...filePayload, file_id: 'file-1', name: 'existing.txt' }]; const emitSpy = vi.spyOn(component.updateAction, 'emit'); component.onAttachFile(FileModelMock.simple({ id: 'file-1' }), 'files'); @@ -307,12 +308,28 @@ describe('CustomStepComponent', () => { expect(emitSpy).not.toHaveBeenCalled(); }); + it('should not attach the same file twice before the draft is saved', () => { + const { component } = setup({ + pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], + stepsData: { files: [] }, + }); + const mockFile = FileModelMock.simple({ id: 'new-file', name: 'new.txt' }); + + component.onAttachFile(mockFile, 'files'); + component.onAttachFile(mockFile, 'files'); + + expect(component.attachedFiles['files']).toEqual([ + expect.objectContaining({ file_id: 'new-file', file_name: 'new.txt', name: 'new.txt' }), + ]); + }); + it('should warn when the attachment limit is reached', () => { const { component, toastMock } = setup({ pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], stepsData: { files: [] }, }); component.attachedFiles['files'] = Array.from({ length: FILE_COUNT_ATTACHMENTS_LIMIT }, (_, i) => ({ + ...filePayload, file_id: `f-${i}`, name: `f-${i}`, })); @@ -364,6 +381,65 @@ describe('CustomStepComponent', () => { expect(emitSpy).not.toHaveBeenCalled(); }); + it('should remove a deleted file from file questions on every page', () => { + const otherFile: FilePayloadJsonApi = { ...filePayload, file_id: 'f2', file_name: 'other.pdf' }; + const { component } = setup({ + pages: [ + createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })]), + createPage([createQuestion({ responseKey: 'otherFiles', fieldType: FieldType.File })]), + ], + stepsData: { files: [filePayload, otherFile], otherFiles: [filePayload] }, + }); + const emitSpy = vi.spyOn(component.updateAction, 'emit'); + + component.removeDeletedFileFromAllQuestions('f1'); + + expect(component.attachedFiles['files']).toEqual([{ ...otherFile, name: 'other.pdf' }]); + expect(component.stepForm.get('files')?.value).toEqual([{ ...otherFile, name: 'other.pdf' }]); + expect(emitSpy).toHaveBeenCalledWith({ files: [otherFile], otherFiles: [] }); + }); + + it('should remove a file attached on this page when it is deleted', () => { + const { component } = setup({ + pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], + stepsData: { files: [] }, + }); + component.onAttachFile(FileModelMock.simple({ id: 'new-file' }), 'files'); + const emitSpy = vi.spyOn(component.updateAction, 'emit'); + + component.removeDeletedFileFromAllQuestions('new-file'); + + expect(component.attachedFiles['files']).toEqual([]); + expect(emitSpy).toHaveBeenCalledWith({ files: [] }); + }); + + it('should not emit when the deleted file is not attached', () => { + const { component } = setup({ + pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], + stepsData: { files: [filePayload] }, + }); + const emitSpy = vi.spyOn(component.updateAction, 'emit'); + + component.removeDeletedFileFromAllQuestions('unknown'); + + expect(component.attachedFiles['files']).toHaveLength(1); + expect(emitSpy).not.toHaveBeenCalled(); + }); + + it('should handle a file deleted from the files list', () => { + const { component, fixture } = setup({ + pages: [createPage([createQuestion({ responseKey: 'files', fieldType: FieldType.File })])], + stepsData: { files: [filePayload] }, + }); + const deleteSpy = vi.spyOn(component, 'removeDeletedFileFromAllQuestions'); + + fixture.debugElement + .query(By.directive(FilesControlComponent)) + .triggerEventHandler('removeFromAttachedFiles', 'f1'); + + expect(deleteSpy).toHaveBeenCalledWith('f1'); + }); + it('should open file preview when draftId and guid exist', () => { const urlTree = {} as UrlTree; const { component, mockRouter } = setup({ diff --git a/src/app/features/registries/components/custom-step/custom-step.component.ts b/src/app/features/registries/components/custom-step/custom-step.component.ts index bc4a933df..6b6245a00 100644 --- a/src/app/features/registries/components/custom-step/custom-step.component.ts +++ b/src/app/features/registries/components/custom-step/custom-step.component.ts @@ -38,7 +38,7 @@ import { CustomValidators } from '@osf/shared/helpers/custom-form-validators.hel import { findChangedFields } from '@osf/shared/helpers/find-changed-fields'; import { FileModel } from '@osf/shared/models/files/file.model'; import { FilePayloadJsonApi } from '@osf/shared/models/files/file-payload-json-api.model'; -import { PageSchema } from '@osf/shared/models/registration/page-schema.model'; +import { PageSchema, Question } from '@osf/shared/models/registration/page-schema.model'; import { ToastService } from '@osf/shared/services/toast.service'; import { FilesMapper } from '../../mappers/files.mapper'; @@ -105,6 +105,12 @@ export class CustomStepComponent implements OnDestroy { } return 'shared.files.description'; }); + private readonly fileQuestionKeys = computed(() => + this.pages() + .flatMap((page) => this.getPageQuestions(page)) + .filter((q) => q.fieldType === FieldType.File) + .map((q) => q.responseKey as string) + ); stepForm: FormGroup = this.fb.group({}); attachedFiles: Record = {}; @@ -130,11 +136,34 @@ export class CustomStepComponent implements OnDestroy { return; } - this.attachedFiles[questionKey] = [...this.attachedFiles[questionKey], file]; + const attachedFile = { ...FilesMapper.toFilePayload(file), name: file.name }; + this.attachedFiles[questionKey] = [...this.attachedFiles[questionKey], attachedFile]; this.stepForm.patchValue({ [questionKey]: this.attachedFiles[questionKey] }); this.updateAction.emit(this.getFormPayload()); } + removeDeletedFileFromAllQuestions(fileId: string): void { + const payload: Record = {}; + + for (const key of this.fileQuestionKeys()) { + const files = this.attachedFiles[key] ?? this.getSavedFiles(key); + if (!files.some((f) => f.file_id === fileId)) { + continue; + } + + const remaining = files.filter((f) => f.file_id !== fileId); + if (this.attachedFiles[key]) { + this.attachedFiles[key] = remaining; + this.stepForm.patchValue({ [key]: remaining }); + } + payload[key] = this.mapFilesToPayload(remaining); + } + + if (Object.keys(payload).length) { + this.updateAction.emit(payload); + } + } + onOpenFile(file: FileModel): void { if (this.draftId() && file.guid) { const url = this.router.serializeUrl(this.router.createUrlTree([this.draftId(), 'files', file.guid, 'preview'])); @@ -192,19 +221,13 @@ export class CustomStepComponent implements OnDestroy { private initStepForm(page: PageSchema): void { this.stepForm = this.fb.group({}); this.attachedFiles = {}; - const questions = [ - ...(page.questions || []), - ...(page.sections?.flatMap((section) => section.questions ?? []) ?? []), - ]; - - questions.forEach((q) => { + this.getPageQuestions(page).forEach((q) => { const controlName = q.responseKey as string; const control = this.createControl(q.fieldType!, controlName, q.required); if (!control) return; if (q.fieldType === FieldType.File) { - const files = (this.stepsData()[controlName] as FilePayloadJsonApi[] | undefined) || []; - this.attachedFiles[controlName] = files.map((file) => ({ ...file, name: file.file_name })); + this.attachedFiles[controlName] = this.getSavedFiles(controlName); } this.stepForm.addControl(controlName, control); @@ -270,13 +293,16 @@ export class CustomStepComponent implements OnDestroy { return values; } + private getPageQuestions(page: PageSchema): Question[] { + return [...(page.questions || []), ...(page.sections?.flatMap((section) => section.questions ?? []) ?? [])]; + } + + private getSavedFiles(questionKey: string): AttachedFile[] { + const files = (this.stepsData()[questionKey] as FilePayloadJsonApi[] | undefined) || []; + return files.map((file) => ({ ...file, name: file.file_name })); + } + private mapFilesToPayload(files: AttachedFile[]): FilePayloadJsonApi[] { - return files.map((f) => { - if (f.file_id) { - const { name: _, ...payload } = f; - return payload as FilePayloadJsonApi; - } - return FilesMapper.toFilePayload(f as FileModel); - }); + return files.map(({ name: _, ...payload }) => payload); } } diff --git a/src/app/features/registries/models/attached-file.model.ts b/src/app/features/registries/models/attached-file.model.ts index 458dac9cf..acc6d3b39 100644 --- a/src/app/features/registries/models/attached-file.model.ts +++ b/src/app/features/registries/models/attached-file.model.ts @@ -1,3 +1,3 @@ -import { FileModel } from '@osf/shared/models/files/file.model'; +import { FilePayloadJsonApi } from '@osf/shared/models/files/file-payload-json-api.model'; -export type AttachedFile = Partial; +export type AttachedFile = FilePayloadJsonApi & { name: string };