diff --git a/setup-jest.ts b/setup-jest.ts index 0ec261638..9fbefed7c 100644 --- a/setup-jest.ts +++ b/setup-jest.ts @@ -52,3 +52,11 @@ jest.mock('@newrelic/browser-agent/loaders/browser-agent', () => ({ stop: jest.fn(), })), })); + +if (!globalThis.structuredClone) { + Object.defineProperty(globalThis, 'structuredClone', { + value: (value: T): T => JSON.parse(JSON.stringify(value)) as T, + writable: true, + configurable: true, + }); +} diff --git a/src/app/features/contributors/contributors.component.spec.ts b/src/app/features/contributors/contributors.component.spec.ts index 249c8e8b3..d47ee0199 100644 --- a/src/app/features/contributors/contributors.component.spec.ts +++ b/src/app/features/contributors/contributors.component.spec.ts @@ -33,16 +33,6 @@ describe('Component: Contributors', () => { const mockContributors: ContributorModel[] = [MOCK_CONTRIBUTOR, MOCK_CONTRIBUTOR_WITHOUT_HISTORY]; - beforeAll(() => { - if (typeof (globalThis as any).structuredClone !== 'function') { - Object.defineProperty(globalThis as any, 'structuredClone', { - configurable: true, - writable: true, - value: (o: unknown) => JSON.parse(JSON.stringify(o)), - }); - } - }); - beforeEach(async () => { jest.useFakeTimers(); diff --git a/src/app/features/metadata/dialogs/contributors-dialog/contributors-dialog.component.spec.ts b/src/app/features/metadata/dialogs/contributors-dialog/contributors-dialog.component.spec.ts index b1e61b48e..cbb3b0b73 100644 --- a/src/app/features/metadata/dialogs/contributors-dialog/contributors-dialog.component.spec.ts +++ b/src/app/features/metadata/dialogs/contributors-dialog/contributors-dialog.component.spec.ts @@ -28,16 +28,6 @@ describe('ContributorsDialogComponent', () => { const mockContributors: ContributorModel[] = [MOCK_CONTRIBUTOR]; - beforeAll(() => { - if (typeof (globalThis as any).structuredClone !== 'function') { - Object.defineProperty(globalThis as any, 'structuredClone', { - configurable: true, - writable: true, - value: (o: unknown) => JSON.parse(JSON.stringify(o)), - }); - } - }); - beforeEach(async () => { mockCustomDialogService = CustomDialogServiceMockBuilder.create().build(); diff --git a/src/app/features/moderation/components/moderators-list/moderators-list.component.spec.ts b/src/app/features/moderation/components/moderators-list/moderators-list.component.spec.ts index 1da622c33..f19023271 100644 --- a/src/app/features/moderation/components/moderators-list/moderators-list.component.spec.ts +++ b/src/app/features/moderation/components/moderators-list/moderators-list.component.spec.ts @@ -41,16 +41,6 @@ describe('ModeratorsListComponent', () => { const mockModerators: ModeratorModel[] = MOCK_MODERATORS; - beforeAll(() => { - if (typeof (globalThis as any).structuredClone !== 'function') { - Object.defineProperty(globalThis as any, 'structuredClone', { - configurable: true, - writable: true, - value: (o: unknown) => JSON.parse(JSON.stringify(o)), - }); - } - }); - beforeEach(async () => { mockActivatedRoute = ActivatedRouteMockBuilder.create() .withParams({ providerId: mockProviderId }) diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.html b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.html index e29db002e..dd2c13d9b 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.html +++ b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.html @@ -19,6 +19,6 @@
- +
diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.spec.ts b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.spec.ts index 6af60f53a..80a4473af 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.spec.ts +++ b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.spec.ts @@ -7,57 +7,55 @@ import { TextInputComponent } from '@osf/shared/components/text-input/text-input import { ArrayInputComponent } from './array-input.component'; -import { OSFTestingModule } from '@testing/osf.testing.module'; +import { provideOSFCore } from '@testing/osf.testing.provider'; describe('ArrayInputComponent', () => { let component: ArrayInputComponent; let fixture: ComponentFixture; - let formArray: FormArray; + let formArray: FormArray>; - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [ArrayInputComponent, MockComponent(TextInputComponent), OSFTestingModule], - }).compileComponents(); + function setup(overrides?: { withValidators?: boolean; formArray?: FormArray> }) { + TestBed.configureTestingModule({ + imports: [ArrayInputComponent, MockComponent(TextInputComponent)], + providers: [provideOSFCore()], + }); fixture = TestBed.createComponent(ArrayInputComponent); component = fixture.componentInstance; - formArray = new FormArray([new FormControl('test')]); - fixture.componentRef.setInput('formArray', formArray); + formArray = + overrides?.formArray ?? new FormArray>([new FormControl('test', { nonNullable: true })]); + fixture.componentRef.setInput('formArray', formArray as FormArray); fixture.componentRef.setInput('inputPlaceholder', 'Enter value'); - fixture.componentRef.setInput('validators', [Validators.required]); + if (overrides?.withValidators ?? true) { + fixture.componentRef.setInput('validators', [Validators.required]); + } fixture.detectChanges(); - }); - - it('should create', () => { - expect(component).toBeTruthy(); - }); - - it('should have correct input values', () => { - expect(component.formArray()).toBe(formArray); - expect(component.inputPlaceholder()).toBe('Enter value'); - expect(component.validators()).toEqual([Validators.required]); - }); + } it('should add new control to form array', () => { + setup(); const initialLength = formArray.length; component.add(); expect(formArray.length).toBe(initialLength + 1); - expect(formArray.at(formArray.length - 1)).toBeInstanceOf(FormControl); + const newControl = formArray.at(formArray.length - 1); + expect(newControl.value).toBe(''); + expect(newControl.hasError('required')).toBe(true); }); - it('should add control with correct validators', () => { + it('should add control without validators when validators input is not set', () => { + setup({ withValidators: false }); component.add(); const newControl = formArray.at(formArray.length - 1); - expect(newControl.hasError('required')).toBe(true); + expect(newControl.errors).toBeNull(); }); it('should remove control at specified index', () => { - component.add(); + setup(); component.add(); const initialLength = formArray.length; @@ -67,9 +65,8 @@ describe('ArrayInputComponent', () => { }); it('should not remove control if only one control exists', () => { - const singleControlArray = new FormArray([new FormControl('only')]); - fixture.componentRef.setInput('formArray', singleControlArray); - fixture.detectChanges(); + const singleControlArray = new FormArray>([new FormControl('only', { nonNullable: true })]); + setup({ formArray: singleControlArray }); const initialLength = singleControlArray.length; @@ -77,27 +74,4 @@ describe('ArrayInputComponent', () => { expect(singleControlArray.length).toBe(initialLength); }); - - it('should handle multiple add and remove operations', () => { - const initialLength = formArray.length; - - component.add(); - component.add(); - component.add(); - - expect(formArray.length).toBe(initialLength + 3); - - component.remove(1); - component.remove(2); - - expect(formArray.length).toBe(initialLength + 1); - }); - - it('should create controls with nonNullable true', () => { - component.add(); - - const newControl = formArray.at(formArray.length - 1); - expect(newControl.value).toBe(''); - expect(newControl.hasError('required')).toBe(true); - }); }); diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.ts b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.ts index 4e32dd133..024eb0a03 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.ts +++ b/src/app/features/preprints/components/stepper/author-assertion-step/array-input/array-input.component.ts @@ -15,11 +15,11 @@ import { TextInputComponent } from '@osf/shared/components/text-input/text-input changeDetection: ChangeDetectionStrategy.OnPush, }) export class ArrayInputComponent { - formArray = input.required>(); - inputPlaceholder = input.required(); - validators = input.required(); + readonly formArray = input.required>(); + readonly inputPlaceholder = input.required(); + readonly validators = input([]); - add() { + add(): void { this.formArray().push( new FormControl('', { nonNullable: true, @@ -28,9 +28,11 @@ export class ArrayInputComponent { ); } - remove(index: number) { - if (this.formArray().length > 1) { - this.formArray().removeAt(index); + remove(index: number): void { + const formArray = this.formArray(); + + if (formArray.length > 1) { + formArray.removeAt(index); } } } diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.html b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.html index 31c97a19d..6840ebace 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.html +++ b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.html @@ -224,7 +224,7 @@

{{ 'preprints.preprintStepper.authorAssertions.publicPreregistration.title' styleClass="w-full" [label]="'common.buttons.back' | translate" severity="info" - (click)="backButtonClicked()" + (onClick)="backButtonClicked()" /> {{ 'preprints.preprintStepper.authorAssertions.publicPreregistration.title' tooltipPosition="top" [disabled]="authorAssertionsForm.invalid" [loading]="isUpdatingPreprint()" - (click)="nextButtonClicked()" + (onClick)="nextButtonClicked()" /> diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.spec.ts b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.spec.ts index bba0d23d6..7fb094c34 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.spec.ts @@ -1,10 +1,15 @@ -import { MockComponents, MockProvider } from 'ng-mocks'; +import { Store } from '@ngxs/store'; + +import { MockComponents, MockDirective, MockProvider } from 'ng-mocks'; + +import { Textarea } from 'primeng/textarea'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { FormControl } from '@angular/forms'; -import { ApplicabilityStatus } from '@osf/features/preprints/enums'; +import { ApplicabilityStatus, PreregLinkInfo } from '@osf/features/preprints/enums'; import { PreprintModel } from '@osf/features/preprints/models'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { PreprintStepperSelectors, UpdatePreprint } from '@osf/features/preprints/store/preprint-stepper'; import { FormSelectComponent } from '@osf/shared/components/form-select/form-select.component'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { ToastService } from '@osf/shared/services/toast.service'; @@ -13,75 +18,275 @@ import { ArrayInputComponent } from './array-input/array-input.component'; import { AuthorAssertionsStepComponent } from './author-assertions-step.component'; import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; -import { TranslationServiceMock } from '@testing/mocks/translation.service.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { CustomConfirmationServiceMockBuilder } from '@testing/providers/custom-confirmation-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { + CustomConfirmationServiceMock, + CustomConfirmationServiceMockType, +} from '@testing/providers/custom-confirmation-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('AuthorAssertionsStepComponent', () => { let component: AuthorAssertionsStepComponent; let fixture: ComponentFixture; - let toastServiceMock: ReturnType; - let customConfirmationServiceMock: ReturnType; + let store: Store; + let toastServiceMock: ToastServiceMockType; + let customConfirmationServiceMock: CustomConfirmationServiceMockType; const mockPreprint: PreprintModel = PREPRINT_MOCK; - beforeEach(async () => { - toastServiceMock = ToastServiceMockBuilder.create().build(); - customConfirmationServiceMock = CustomConfirmationServiceMockBuilder.create().build(); + const populatedPreprint: PreprintModel = { + ...mockPreprint, + hasCoi: true, + coiStatement: 'Author is a board member of the funder.', + hasDataLinks: ApplicabilityStatus.Applicable, + dataLinks: ['https://data.example/ds1', 'https://data.example/ds2'], + whyNoData: null, + hasPreregLinks: ApplicabilityStatus.Applicable, + preregLinks: ['https://prereg.example/reg1'], + whyNoPrereg: null, + preregLinkInfo: PreregLinkInfo.Both, + }; + + const cleanPreprint: PreprintModel = { + ...mockPreprint, + hasCoi: false, + coiStatement: null, + hasDataLinks: ApplicabilityStatus.NotApplicable, + dataLinks: [], + whyNoData: null, + hasPreregLinks: ApplicabilityStatus.NotApplicable, + preregLinks: [], + whyNoPrereg: null, + preregLinkInfo: null, + }; + + const defaultSignals: SignalOverride[] = [ + { selector: PreprintStepperSelectors.getPreprint, value: mockPreprint }, + { selector: PreprintStepperSelectors.isPreprintSubmitting, value: false }, + ]; + + function setup(overrides?: { selectorOverrides?: SignalOverride[]; detectChanges?: boolean }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + toastServiceMock = ToastServiceMock.simple(); + customConfirmationServiceMock = CustomConfirmationServiceMock.simple(); - await TestBed.configureTestingModule({ + TestBed.configureTestingModule({ imports: [ AuthorAssertionsStepComponent, - OSFTestingModule, - MockComponents(ArrayInputComponent, FormSelectComponent), + ...MockComponents(ArrayInputComponent, FormSelectComponent), + MockDirective(Textarea), ], providers: [ - TranslationServiceMock, + provideOSFCore(), MockProvider(ToastService, toastServiceMock), MockProvider(CustomConfirmationService, customConfirmationServiceMock), - provideMockStore({ - signals: [ - { - selector: PreprintStepperSelectors.getPreprint, - value: mockPreprint, - }, - { - selector: PreprintStepperSelectors.isPreprintSubmitting, - value: false, - }, - ], - }), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(AuthorAssertionsStepComponent); component = fixture.componentInstance; - }); + if (overrides?.detectChanges ?? false) { + fixture.detectChanges(); + } + } + + it('should create and initialize form with preprint defaults', () => { + setup(); - it('should create', () => { expect(component).toBeTruthy(); + expect(component.authorAssertionsForm.controls.hasCoi.value).toBe(false); + expect(component.authorAssertionsForm.controls.coiStatement.value).toBeNull(); + expect(component.authorAssertionsForm.controls.hasDataLinks.value).toBe(ApplicabilityStatus.NotApplicable); + expect(component.authorAssertionsForm.controls.hasPreregLinks.value).toBe(ApplicabilityStatus.NotApplicable); + expect(component.hasCoiValue()).toBe(false); + expect(component.hasDataLinks()).toBe(ApplicabilityStatus.NotApplicable); + expect(component.hasPreregLinks()).toBe(ApplicabilityStatus.NotApplicable); + }); + + it('should hydrate form from a preprint that has real data', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: populatedPreprint }], + }); + + const controls = component.authorAssertionsForm.controls; + expect(controls.hasCoi.value).toBe(true); + expect(controls.coiStatement.value).toBe('Author is a board member of the funder.'); + expect(controls.hasDataLinks.value).toBe(ApplicabilityStatus.Applicable); + expect(controls.dataLinks.length).toBe(2); + expect(controls.hasPreregLinks.value).toBe(ApplicabilityStatus.Applicable); + expect(controls.preregLinks.length).toBe(1); + expect(controls.preregLinkInfo.value).toBe(PreregLinkInfo.Both); + }); + + it('should enable coiStatement control when hasCoi becomes true', () => { + setup({ detectChanges: true }); + component.authorAssertionsForm.controls.hasCoi.setValue(true); + fixture.detectChanges(); + + expect(component.authorAssertionsForm.controls.coiStatement.enabled).toBe(true); }); - it('should initialize form with preprint data', () => { - expect(component.authorAssertionsForm.get('hasCoi')?.value).toBe(false); - expect(component.authorAssertionsForm.get('coiStatement')?.value).toBeNull(); - expect(component.authorAssertionsForm.get('hasDataLinks')?.value).toBe(ApplicabilityStatus.NotApplicable); - expect(component.authorAssertionsForm.get('hasPreregLinks')?.value).toBe(ApplicabilityStatus.NotApplicable); + it('should disable and clear coiStatement control when hasCoi becomes false', () => { + setup({ detectChanges: true }); + const { hasCoi, coiStatement } = component.authorAssertionsForm.controls; + hasCoi.setValue(true); + coiStatement.setValue('Some statement'); + fixture.detectChanges(); + + hasCoi.setValue(false); + fixture.detectChanges(); + + expect(coiStatement.value).toBeNull(); + expect(coiStatement.disabled).toBe(true); + }); + + it('should enable whyNoData and clear dataLinks when hasDataLinks is Unavailable', () => { + setup({ detectChanges: true }); + const { hasDataLinks, whyNoData, dataLinks } = component.authorAssertionsForm.controls; + dataLinks.push(new FormControl('https://existing.example')); + + hasDataLinks.setValue(ApplicabilityStatus.Unavailable); + fixture.detectChanges(); + + expect(whyNoData.enabled).toBe(true); + expect(dataLinks.length).toBe(0); + }); + + it('should add an empty dataLinks entry and clear whyNoData when hasDataLinks is Applicable', () => { + setup({ detectChanges: true }); + const { hasDataLinks, whyNoData, dataLinks } = component.authorAssertionsForm.controls; + hasDataLinks.setValue(ApplicabilityStatus.Unavailable); + whyNoData.setValue('No data available'); + fixture.detectChanges(); + + hasDataLinks.setValue(ApplicabilityStatus.Applicable); + fixture.detectChanges(); + + expect(dataLinks.length).toBe(1); + expect(whyNoData.value).toBeNull(); + }); + + it('should enable whyNoPrereg and clear preregLinks/preregLinkInfo when hasPreregLinks is Unavailable', () => { + setup({ detectChanges: true }); + const { hasPreregLinks, whyNoPrereg, preregLinkInfo, preregLinks } = component.authorAssertionsForm.controls; + preregLinks.push(new FormControl('https://existing.example')); + preregLinkInfo.setValue(PreregLinkInfo.Both); + + hasPreregLinks.setValue(ApplicabilityStatus.Unavailable); + fixture.detectChanges(); + + expect(whyNoPrereg.enabled).toBe(true); + expect(preregLinks.length).toBe(0); + expect(preregLinkInfo.value).toBeNull(); + }); + + it('should add an empty preregLinks entry and enable preregLinkInfo when hasPreregLinks is Applicable', () => { + setup({ detectChanges: true }); + const { hasPreregLinks, preregLinks, preregLinkInfo } = component.authorAssertionsForm.controls; + hasPreregLinks.setValue(ApplicabilityStatus.Unavailable); + fixture.detectChanges(); + + hasPreregLinks.setValue(ApplicabilityStatus.Applicable); + fixture.detectChanges(); + + expect(preregLinks.length).toBe(1); + expect(preregLinkInfo.enabled).toBe(true); }); - it('should emit nextClicked when nextButtonClicked is called', () => { + it('should return early in nextButtonClicked when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + }); const emitSpy = jest.spyOn(component.nextClicked, 'emit'); + (store.dispatch as jest.Mock).mockClear(); + + component.nextButtonClicked(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePreprint)); + expect(emitSpy).not.toHaveBeenCalled(); + }); + + it('should dispatch UpdatePreprint, show success toast, and emit next on valid submission', () => { + setup(); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); + component.authorAssertionsForm.patchValue({ + hasCoi: true, + coiStatement: 'COI', + hasDataLinks: ApplicabilityStatus.Applicable, + hasPreregLinks: ApplicabilityStatus.Applicable, + preregLinkInfo: PreregLinkInfo.Both, + }); + component.authorAssertionsForm.controls.dataLinks.push(new FormControl('https://data.example')); + component.authorAssertionsForm.controls.preregLinks.push(new FormControl('https://prereg.example')); + (store.dispatch as jest.Mock).mockClear(); + component.nextButtonClicked(); + expect(store.dispatch).toHaveBeenCalledWith( + new UpdatePreprint(mockPreprint.id, { + hasCoi: true, + coiStatement: 'COI', + hasDataLinks: ApplicabilityStatus.Applicable, + whyNoData: null, + dataLinks: ['https://data.example'], + hasPreregLinks: ApplicabilityStatus.Applicable, + whyNoPrereg: null, + preregLinks: ['https://prereg.example'], + preregLinkInfo: PreregLinkInfo.Both, + }) + ); expect(toastServiceMock.showSuccess).toHaveBeenCalledWith( 'preprints.preprintStepper.common.successMessages.preprintSaved' ); expect(emitSpy).toHaveBeenCalled(); }); - it('should show confirmation dialog when backButtonClicked is called with changes', () => { + it('should omit preregLinkInfo from the UpdatePreprint payload when it is empty', () => { + setup(); + component.authorAssertionsForm.patchValue({ + hasPreregLinks: ApplicabilityStatus.Applicable, + preregLinkInfo: null, + }); + component.authorAssertionsForm.controls.preregLinks.push(new FormControl('https://prereg.example')); + (store.dispatch as jest.Mock).mockClear(); + + component.nextButtonClicked(); + + expect(store.dispatch).toHaveBeenCalledWith( + new UpdatePreprint(mockPreprint.id, expect.objectContaining({ preregLinkInfo: undefined })) + ); + }); + + it('should return early in backButtonClicked when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + }); + const emitSpy = jest.spyOn(component.backClicked, 'emit'); + + component.backButtonClicked(); + + expect(customConfirmationServiceMock.confirmContinue).not.toHaveBeenCalled(); + expect(emitSpy).not.toHaveBeenCalled(); + }); + + it('should emit back immediately when there are no unsaved changes', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: cleanPreprint }], + }); + const emitSpy = jest.spyOn(component.backClicked, 'emit'); + + component.backButtonClicked(); + + expect(customConfirmationServiceMock.confirmContinue).not.toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalled(); + }); + + it('should handle discard confirmation callbacks when there are unsaved changes', () => { + setup(); + const emitSpy = jest.spyOn(component.backClicked, 'emit'); component.authorAssertionsForm.patchValue({ hasCoi: true }); component.backButtonClicked(); @@ -92,20 +297,13 @@ describe('AuthorAssertionsStepComponent', () => { onConfirm: expect.any(Function), onReject: expect.any(Function), }); - }); - it('should expose readonly properties', () => { - expect(component.CustomValidators).toBeDefined(); - expect(component.ApplicabilityStatus).toBe(ApplicabilityStatus); - expect(component.inputLimits).toBeDefined(); - expect(component.INPUT_VALIDATION_MESSAGES).toBeDefined(); - expect(component.preregLinkOptions).toBeDefined(); - expect(component.linkValidators).toBeDefined(); - }); + const { onReject } = customConfirmationServiceMock.confirmContinue.mock.calls[0][0]; + onReject(); + expect(emitSpy).not.toHaveBeenCalled(); - it('should have correct signal values', () => { - expect(component.hasCoiValue()).toBe(false); - expect(component.hasDataLinks()).toBe(ApplicabilityStatus.NotApplicable); - expect(component.hasPreregLinks()).toBe(ApplicabilityStatus.NotApplicable); + const { onConfirm } = customConfirmationServiceMock.confirmContinue.mock.calls[0][0]; + onConfirm(); + expect(emitSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.ts b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.ts index 97ca4a946..e8838344a 100644 --- a/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.ts +++ b/src/app/features/preprints/components/stepper/author-assertion-step/author-assertions-step.component.ts @@ -40,89 +40,88 @@ import { ArrayInputComponent } from './array-input/array-input.component'; @Component({ selector: 'osf-author-assertions-step', imports: [ + Button, Card, - FormsModule, + Message, RadioButton, - ReactiveFormsModule, Textarea, - Message, - TranslatePipe, - NgClass, - Button, Tooltip, + NgClass, + FormsModule, + ReactiveFormsModule, ArrayInputComponent, FormSelectComponent, + TranslatePipe, ], templateUrl: './author-assertions-step.component.html', styleUrl: './author-assertions-step.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class AuthorAssertionsStepComponent { - private toastService = inject(ToastService); - private confirmationService = inject(CustomConfirmationService); - private actions = createDispatchMap({ updatePreprint: UpdatePreprint }); + private readonly toastService = inject(ToastService); + private readonly confirmationService = inject(CustomConfirmationService); + private readonly actions = createDispatchMap({ updatePreprint: UpdatePreprint }); - readonly CustomValidators = CustomValidators; readonly ApplicabilityStatus = ApplicabilityStatus; readonly inputLimits = formInputLimits; readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; readonly preregLinkOptions = preregLinksOptions; readonly linkValidators = [CustomValidators.linkValidator(), CustomValidators.requiredTrimmed()]; - createdPreprint = select(PreprintStepperSelectors.getPreprint); - isUpdatingPreprint = select(PreprintStepperSelectors.isPreprintSubmitting); + readonly createdPreprint = select(PreprintStepperSelectors.getPreprint); + readonly isUpdatingPreprint = select(PreprintStepperSelectors.isPreprintSubmitting); readonly authorAssertionsForm = new FormGroup({ - hasCoi: new FormControl(this.createdPreprint()!.hasCoi || false, { + hasCoi: new FormControl(this.createdPreprint()?.hasCoi ?? false, { nonNullable: true, validators: [], }), - coiStatement: new FormControl(this.createdPreprint()!.coiStatement, { + coiStatement: new FormControl(this.createdPreprint()?.coiStatement ?? null, { nonNullable: false, validators: [], }), hasDataLinks: new FormControl( - this.createdPreprint()!.hasDataLinks || ApplicabilityStatus.NotApplicable, + this.createdPreprint()?.hasDataLinks ?? ApplicabilityStatus.NotApplicable, { nonNullable: true, validators: [], } ), dataLinks: new FormArray( - this.createdPreprint()!.dataLinks?.map((link) => new FormControl(link)) || [] + this.createdPreprint()?.dataLinks?.map((link) => new FormControl(link)) || [] ), - whyNoData: new FormControl(this.createdPreprint()!.whyNoData, { + whyNoData: new FormControl(this.createdPreprint()?.whyNoData ?? null, { nonNullable: false, validators: [], }), hasPreregLinks: new FormControl( - this.createdPreprint()!.hasPreregLinks || ApplicabilityStatus.NotApplicable, + this.createdPreprint()?.hasPreregLinks ?? ApplicabilityStatus.NotApplicable, { nonNullable: true, validators: [], } ), preregLinks: new FormArray( - this.createdPreprint()!.preregLinks?.map((link) => new FormControl(link)) || [] + this.createdPreprint()?.preregLinks?.map((link) => new FormControl(link)) || [] ), - whyNoPrereg: new FormControl(this.createdPreprint()!.whyNoPrereg, { + whyNoPrereg: new FormControl(this.createdPreprint()?.whyNoPrereg ?? null, { nonNullable: false, validators: [], }), - preregLinkInfo: new FormControl(this.createdPreprint()!.preregLinkInfo, { + preregLinkInfo: new FormControl(this.createdPreprint()?.preregLinkInfo ?? null, { nonNullable: false, validators: [], }), }); hasCoiValue = toSignal(this.authorAssertionsForm.controls['hasCoi'].valueChanges, { - initialValue: this.createdPreprint()!.hasCoi || false, + initialValue: this.createdPreprint()?.hasCoi ?? false, }); hasDataLinks = toSignal(this.authorAssertionsForm.controls['hasDataLinks'].valueChanges, { - initialValue: this.createdPreprint()!.hasDataLinks || ApplicabilityStatus.NotApplicable, + initialValue: this.createdPreprint()?.hasDataLinks ?? ApplicabilityStatus.NotApplicable, }); hasPreregLinks = toSignal(this.authorAssertionsForm.controls['hasPreregLinks'].valueChanges, { - initialValue: this.createdPreprint()!.hasPreregLinks || ApplicabilityStatus.NotApplicable, + initialValue: this.createdPreprint()?.hasPreregLinks ?? ApplicabilityStatus.NotApplicable, }); nextClicked = output(); @@ -194,7 +193,13 @@ export class AuthorAssertionsStepComponent { }); } - nextButtonClicked() { + nextButtonClicked(): void { + const preprintId = this.createdPreprint()?.id; + + if (!preprintId) { + return; + } + const formValue = this.authorAssertionsForm.getRawValue(); const hasCoi = formValue.hasCoi; @@ -210,7 +215,7 @@ export class AuthorAssertionsStepComponent { const preregLinkInfo = formValue.preregLinkInfo || undefined; this.actions - .updatePreprint(this.createdPreprint()!.id, { + .updatePreprint(preprintId, { hasCoi, coiStatement, hasDataLinks, @@ -229,9 +234,15 @@ export class AuthorAssertionsStepComponent { }); } - backButtonClicked() { + backButtonClicked(): void { + const preprint = this.createdPreprint(); + + if (!preprint) { + return; + } + const formValue = this.authorAssertionsForm.getRawValue(); - const changedFields = findChangedFields(formValue, this.createdPreprint()!); + const changedFields = findChangedFields(formValue, preprint); if (!Object.keys(changedFields).length) { this.backClicked.emit(); @@ -248,7 +259,7 @@ export class AuthorAssertionsStepComponent { }); } - private disableAndClearValidators(control: AbstractControl) { + private disableAndClearValidators(control: AbstractControl): void { if (control instanceof FormArray) { while (control.length !== 0) { control.removeAt(0); @@ -261,12 +272,12 @@ export class AuthorAssertionsStepComponent { control.disable(); } - private enableAndSetValidators(control: AbstractControl, validators: ValidatorFn[]) { + private enableAndSetValidators(control: AbstractControl, validators: ValidatorFn[]): void { control.setValidators(validators); control.enable(); } - private addAtLeastOneControl(formArray: FormArray) { + private addAtLeastOneControl(formArray: FormArray): void { if (formArray.controls.length > 0) return; formArray.push( diff --git a/src/app/features/preprints/components/stepper/file-step/file-step.component.html b/src/app/features/preprints/components/stepper/file-step/file-step.component.html index 8a95bdcec..ee8b37860 100644 --- a/src/app/features/preprints/components/stepper/file-step/file-step.component.html +++ b/src/app/features/preprints/components/stepper/file-step/file-step.component.html @@ -4,7 +4,7 @@

{{ 'preprints.preprintStepper.file.title' | translate }}

{{ 'preprints.preprintStepper.file.uploadDescription' - | translate: { preprintWord: provider()?.preprintWord | titlecase } + | translate: { preprintWord: provider().preprintWord | titlecase } }}

{{ 'preprints.preprintStepper.file.note' | translate }}

@@ -56,7 +56,7 @@

{{ 'preprints.preprintStepper.file.title' | translate }}

(onClick)="fileInput.click()" /> - + } } @@ -64,25 +64,27 @@

{{ 'preprints.preprintStepper.file.title' | translate }}

@if (selectedFileSource() === PreprintFileSource.Project && !preprintFile() && !isPreprintFileLoading()) {
-

{{ 'preprints.preprintStepper.file.projectSelection.description' | translate }}

- {{ 'preprints.preprintStepper.file.projectSelection.subDescription' | translate }} + {{ 'preprints.preprintStepper.projectSelection.description' | translate }} +

+

+ {{ 'preprints.preprintStepper.projectSelection.subDescription' | translate }}

@@ -154,11 +156,9 @@

{{ 'preprints.preprintStepper.file.title' | translate }}

class="w-6 md:w-9rem" styleClass="w-full" [label]="'common.buttons.next' | translate" - [disabled]="!preprintFile() || versionFileMode()" + [disabled]="!canProceedToNext()" [pTooltip]=" - !preprintFile() || versionFileMode() - ? ('preprints.preprintStepper.common.validation.fillRequiredFields' | translate) - : '' + !canProceedToNext() ? ('preprints.preprintStepper.common.validation.fillRequiredFields' | translate) : '' " tooltipPosition="top" (onClick)="nextButtonClicked()" diff --git a/src/app/features/preprints/components/stepper/file-step/file-step.component.spec.ts b/src/app/features/preprints/components/stepper/file-step/file-step.component.spec.ts index 1471c9a39..37d2b0841 100644 --- a/src/app/features/preprints/components/stepper/file-step/file-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/file-step/file-step.component.spec.ts @@ -1,120 +1,197 @@ +import { Store } from '@ngxs/store'; + import { MockComponents, MockProvider } from 'ng-mocks'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { SelectChangeEvent } from 'primeng/select'; + +import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; import { PreprintFileSource } from '@osf/features/preprints/enums'; import { PreprintModel, PreprintProviderDetails } from '@osf/features/preprints/models'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { + CopyFileFromProject, + FetchAvailableProjects, + FetchPreprintFilesLinks, + FetchPreprintPrimaryFile, + FetchProjectFilesByLink, + PreprintStepperSelectors, + ReuploadFile, + SetPreprintStepperCurrentFolder, + SetProjectRootFolder, + SetSelectedPreprintFileSource, + UploadFile, +} from '@osf/features/preprints/store/preprint-stepper'; import { FilesTreeComponent } from '@osf/shared/components/files-tree/files-tree.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; +import { FileModel } from '@osf/shared/models/files/file.model'; +import { FileFolderModel } from '@osf/shared/models/files/file-folder.model'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { ToastService } from '@osf/shared/services/toast.service'; -import { FileFolderModel } from '@shared/models/files/file-folder.model'; import { FileStepComponent } from './file-step.component'; import { OSF_FILE_MOCK } from '@testing/mocks/osf-file.mock'; import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; import { PREPRINT_PROVIDER_DETAILS_MOCK } from '@testing/mocks/preprint-provider-details'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { CustomConfirmationServiceMockBuilder } from '@testing/providers/custom-confirmation-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { + CustomConfirmationServiceMock, + CustomConfirmationServiceMockType, +} from '@testing/providers/custom-confirmation-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('FileStepComponent', () => { let component: FileStepComponent; let fixture: ComponentFixture; - let toastServiceMock: ReturnType; - let confirmationServiceMock: ReturnType; + let store: Store; + let toastServiceMock: ToastServiceMockType; + let confirmationServiceMock: CustomConfirmationServiceMockType; + const originalPointerEvent = (globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent; const mockProvider: PreprintProviderDetails = PREPRINT_PROVIDER_DETAILS_MOCK; const mockPreprint: PreprintModel = PREPRINT_MOCK; const mockProjectFiles: FileFolderModel[] = [OSF_FILE_MOCK]; const mockPreprintFile: FileFolderModel = OSF_FILE_MOCK; - + const mockCurrentFolder: FileFolderModel = OSF_FILE_MOCK; const mockAvailableProjects = [ - { id: 'project-1', title: 'Test Project 1' }, - { id: 'project-2', title: 'Test Project 2' }, + { id: 'project-1', name: 'Test Project 1' }, + { id: 'project-2', name: 'Test Project 2' }, ]; - beforeEach(async () => { - toastServiceMock = ToastServiceMockBuilder.create().build(); - confirmationServiceMock = CustomConfirmationServiceMockBuilder.create().build(); + const defaultSignals: SignalOverride[] = [ + { selector: PreprintStepperSelectors.getPreprint, value: mockPreprint }, + { selector: PreprintStepperSelectors.getSelectedFileSource, value: PreprintFileSource.None }, + { selector: PreprintStepperSelectors.getUploadLink, value: 'upload-link' }, + { selector: PreprintStepperSelectors.getPreprintFile, value: mockPreprintFile }, + { selector: PreprintStepperSelectors.isPreprintFilesLoading, value: false }, + { selector: PreprintStepperSelectors.getAvailableProjects, value: mockAvailableProjects }, + { selector: PreprintStepperSelectors.areAvailableProjectsLoading, value: false }, + { selector: PreprintStepperSelectors.getProjectFiles, value: mockProjectFiles }, + { selector: PreprintStepperSelectors.getFilesTotalCount, value: 1 }, + { selector: PreprintStepperSelectors.areProjectFilesLoading, value: false }, + { selector: PreprintStepperSelectors.getCurrentFolder, value: mockCurrentFolder }, + { selector: PreprintStepperSelectors.isCurrentFolderLoading, value: false }, + ]; - await TestBed.configureTestingModule({ - imports: [FileStepComponent, ...MockComponents(IconComponent, FilesTreeComponent), OSFTestingModule], + function setup(overrides?: { + selectorOverrides?: SignalOverride[]; + provider?: PreprintProviderDetails; + detectChanges?: boolean; + }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + toastServiceMock = ToastServiceMock.simple(); + confirmationServiceMock = CustomConfirmationServiceMock.simple(); + + TestBed.configureTestingModule({ + imports: [FileStepComponent, ...MockComponents(IconComponent, FilesTreeComponent)], providers: [ + provideOSFCore(), MockProvider(ToastService, toastServiceMock), MockProvider(CustomConfirmationService, confirmationServiceMock), - provideMockStore({ - signals: [ - { - selector: PreprintStepperSelectors.getPreprint, - value: mockPreprint, - }, - { - selector: PreprintStepperSelectors.getSelectedProviderId, - value: 'provider-1', - }, - { - selector: PreprintStepperSelectors.getSelectedFileSource, - value: PreprintFileSource.None, - }, - { - selector: PreprintStepperSelectors.getUploadLink, - value: 'upload-link', - }, - { - selector: PreprintStepperSelectors.getPreprintFile, - value: mockPreprintFile, - }, - { - selector: PreprintStepperSelectors.isPreprintFilesLoading, - value: false, - }, - { - selector: PreprintStepperSelectors.getAvailableProjects, - value: mockAvailableProjects, - }, - { - selector: PreprintStepperSelectors.areAvailableProjectsLoading, - value: false, - }, - { - selector: PreprintStepperSelectors.getProjectFiles, - value: mockProjectFiles, - }, - { - selector: PreprintStepperSelectors.areProjectFilesLoading, - value: false, - }, - { - selector: PreprintStepperSelectors.getCurrentFolder, - value: null, - }, - ], - }), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(FileStepComponent); component = fixture.componentInstance; - fixture.componentRef.setInput('provider', mockProvider); - fixture.detectChanges(); + fixture.componentRef.setInput('provider', overrides?.provider ?? mockProvider); + if (overrides?.detectChanges ?? true) { + fixture.detectChanges(); + } + } + + beforeAll(() => { + if (!(globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent) { + (globalThis as unknown as { PointerEvent: typeof Event }).PointerEvent = MouseEvent as unknown as typeof Event; + } + }); + + afterAll(() => { + if (originalPointerEvent) { + (globalThis as unknown as { PointerEvent: typeof Event }).PointerEvent = originalPointerEvent; + } else { + delete (globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent; + } }); it('should create', () => { + setup(); expect(component).toBeTruthy(); }); - it('should initialize with correct values', () => { - expect(component.provider()).toBe(mockProvider); - expect(component.preprint()).toBe(mockPreprint); - expect(component.selectedFileSource()).toBe(PreprintFileSource.None); - expect(component.preprintFile()).toBe(mockPreprintFile); + it('should compute state values', () => { + setup({ detectChanges: false }); + expect(component.preprintHasPrimaryFile()).toBe(true); + expect(component.isFileSourceSelected()).toBe(false); + expect(component.canProceedToNext()).toBe(true); + expect(component.cancelSourceOptionButtonVisible()).toBe(false); + }); + + it('should dispatch fetch links and primary file fetch in ngOnInit', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprintFile, value: null }], + detectChanges: false, + }); + + component.ngOnInit(); + + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintFilesLinks()); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintPrimaryFile()); + }); + + it('should not dispatch primary file fetch in ngOnInit without primary file id', () => { + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getPreprint, value: { ...mockPreprint, primaryFileId: null } }, + ], + detectChanges: false, + }); + + component.ngOnInit(); + + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintFilesLinks()); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchPreprintPrimaryFile)); + }); + + it('should dispatch available projects from debounced projectNameControl value', fakeAsync(() => { + setup(); + (store.dispatch as jest.Mock).mockClear(); + + component.projectNameControl.setValue('project-search'); + tick(500); + + expect(store.dispatch).toHaveBeenCalledWith(new FetchAvailableProjects('project-search')); + })); + + it('should skip available projects dispatch when value equals selectedProjectId', fakeAsync(() => { + setup(); + (store.dispatch as jest.Mock).mockClear(); + component.selectedProjectId.set('project-1'); + + component.projectNameControl.setValue('project-1'); + tick(500); + + expect(store.dispatch).not.toHaveBeenCalledWith(new FetchAvailableProjects('project-1')); + })); + + it('should handle selectFileSource for project and computer source', () => { + setup({ detectChanges: false }); + + component.selectFileSource(PreprintFileSource.Project); + expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.Project)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchAvailableProjects(null)); + + (store.dispatch as jest.Mock).mockClear(); + component.selectFileSource(PreprintFileSource.Computer); + + expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.Computer)); + expect(store.dispatch).not.toHaveBeenCalledWith(new FetchAvailableProjects(null)); }); - it('should emit backClicked when backButtonClicked is called', () => { + it('should emit backClicked', () => { + setup({ detectChanges: false }); const emitSpy = jest.spyOn(component.backClicked, 'emit'); component.backButtonClicked(); @@ -122,7 +199,8 @@ describe('FileStepComponent', () => { expect(emitSpy).toHaveBeenCalled(); }); - it('should emit nextClicked when nextButtonClicked is called with primary file', () => { + it('should handle nextButtonClicked for allowed and blocked states', () => { + setup({ detectChanges: false }); const emitSpy = jest.spyOn(component.nextClicked, 'emit'); component.nextButtonClicked(); @@ -130,82 +208,158 @@ describe('FileStepComponent', () => { expect(toastServiceMock.showSuccess).toHaveBeenCalledWith( 'preprints.preprintStepper.common.successMessages.preprintSaved' ); - expect(emitSpy).toHaveBeenCalled(); - }); + expect(emitSpy).toHaveBeenCalledTimes(1); - it('should not emit nextClicked when nextButtonClicked is called without primary file', () => { - const emitSpy = jest.spyOn(component.nextClicked, 'emit'); - jest.spyOn(component, 'preprint').mockReturnValue({ ...mockPreprint, primaryFileId: null }); + component.versionFileMode.set(true); + toastServiceMock.showSuccess.mockClear(); component.nextButtonClicked(); - expect(emitSpy).not.toHaveBeenCalled(); + expect(toastServiceMock.showSuccess).not.toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledTimes(1); + + jest.spyOn(component, 'preprint').mockReturnValue({ ...mockPreprint, primaryFileId: null } as PreprintModel); + component.versionFileMode.set(false); + + component.nextButtonClicked(); + + expect(toastServiceMock.showSuccess).not.toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledTimes(1); }); - it('should handle file selection for upload', () => { - const mockFile = new File(['test'], 'test.pdf', { type: 'application/pdf' }); - const mockEvent = { - target: { - files: [mockFile], - }, - } as any; + it('should skip file upload dispatches when no file is selected', () => { + setup({ detectChanges: false }); - component.onFileSelected(mockEvent); + const input = document.createElement('input'); + Object.defineProperty(input, 'files', { value: [] }); + component.onFileSelected({ target: input } as unknown as Event); - expect(mockFile).toBeDefined(); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UploadFile)); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(ReuploadFile)); }); - it('should handle file selection for reupload', () => { - component.versionFileMode.set(true); + it('should handle upload and reupload flows in onFileSelected', () => { + setup({ detectChanges: false }); + const file = new File(['file-body'], 'test.txt'); + const input = document.createElement('input'); + Object.defineProperty(input, 'files', { value: [file] }); - const mockFile = new File(['test'], 'test.pdf', { type: 'application/pdf' }); - const mockEvent = { - target: { - files: [mockFile], - }, - } as any; + component.onFileSelected({ target: input } as unknown as Event); + expect(store.dispatch).toHaveBeenCalledWith(new UploadFile(file)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintPrimaryFile()); - component.onFileSelected(mockEvent); + (store.dispatch as jest.Mock).mockClear(); + component.versionFileMode.set(true); + component.onFileSelected({ target: input } as unknown as Event); + + expect(store.dispatch).toHaveBeenCalledWith(new ReuploadFile(file)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintPrimaryFile()); expect(component.versionFileMode()).toBe(false); }); - it('should handle version file confirmation', () => { - confirmationServiceMock.confirmContinue.mockImplementation(({ onConfirm }) => { - onConfirm(); - }); + it('should handle selectProject with and without current folder files link', () => { + setup({ detectChanges: false }); + + component.selectProject({ + value: 'project-1', + originalEvent: new PointerEvent('click'), + } as SelectChangeEvent); + + expect(store.dispatch).toHaveBeenCalledWith(new SetProjectRootFolder('project-1')); + expect(store.dispatch).toHaveBeenCalledWith(new FetchProjectFilesByLink(mockCurrentFolder.links.filesLink, 1)); + expect(component.selectedProjectId()).toBe('project-1'); + + jest.spyOn(component, 'currentFolder').mockReturnValue(null); + (store.dispatch as jest.Mock).mockClear(); + + component.selectProject({ + value: 'project-1', + originalEvent: new PointerEvent('click'), + } as SelectChangeEvent); + + expect(store.dispatch).toHaveBeenCalledWith(new SetProjectRootFolder('project-1')); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchProjectFilesByLink)); + }); + + it('should return early in selectProject when original event is not pointer event', () => { + setup({ detectChanges: false }); + + component.selectProject({ + value: 'project-1', + originalEvent: new Event('change'), + } as SelectChangeEvent); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(SetProjectRootFolder)); + }); + + it('should dispatch copy file from project and preprint file fetch', () => { + setup({ detectChanges: false }); + const projectFile = OSF_FILE_MOCK as unknown as FileModel; + + component.selectProjectFile(projectFile); + + expect(store.dispatch).toHaveBeenCalledWith(new CopyFileFromProject(projectFile)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintPrimaryFile()); + }); + + it('should set version mode and reset selected source on version file confirmation', () => { + setup({ detectChanges: false }); component.versionFile(); + const options = confirmationServiceMock.confirmContinue.mock.calls[0][0]; + options.onConfirm(); - expect(confirmationServiceMock.confirmContinue).toHaveBeenCalledWith({ - headerKey: 'preprints.preprintStepper.file.versionFile.header', - messageKey: 'preprints.preprintStepper.file.versionFile.message', - onConfirm: expect.any(Function), - onReject: expect.any(Function), - }); expect(component.versionFileMode()).toBe(true); + expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.None)); }); - it('should handle cancel button click', () => { - jest.spyOn(component, 'preprintFile').mockReturnValue(null); + it('should not change mode or selected source on version file reject', () => { + setup({ detectChanges: false }); - component.cancelButtonClicked(); + component.versionFile(); + const options = confirmationServiceMock.confirmContinue.mock.calls[0][0]; + (store.dispatch as jest.Mock).mockClear(); + options.onReject(); - expect(component.preprintFile()).toBeNull(); + expect(component.versionFileMode()).toBe(false); + expect(store.dispatch).not.toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.None)); }); - it('should not handle cancel button click when preprint file exists', () => { + it('should handle cancelButtonClicked for file present and file missing states', () => { + setup({ detectChanges: false }); + component.cancelButtonClicked(); + expect(store.dispatch).not.toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.None)); - expect(component.preprintFile()).toBeDefined(); + jest.spyOn(component, 'preprintFile').mockReturnValue(null); + (store.dispatch as jest.Mock).mockClear(); + + component.cancelButtonClicked(); + expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintFileSource(PreprintFileSource.None)); }); - it('should expose readonly properties', () => { - expect(component.PreprintFileSource).toBe(PreprintFileSource); + it('should handle setCurrentFolder for unchanged and changed folders', () => { + setup({ detectChanges: false }); + + component.setCurrentFolder(mockCurrentFolder); + expect(store.dispatch).not.toHaveBeenCalledWith(new SetPreprintStepperCurrentFolder(mockCurrentFolder)); + expect(store.dispatch).not.toHaveBeenCalledWith(new FetchProjectFilesByLink(mockCurrentFolder.links.filesLink, 1)); + + (store.dispatch as jest.Mock).mockClear(); + const nextFolder = { ...mockCurrentFolder, id: 'folder-2' } as FileFolderModel; + + component.setCurrentFolder(nextFolder); + + expect(store.dispatch).toHaveBeenCalledWith(new SetPreprintStepperCurrentFolder(nextFolder)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchProjectFilesByLink(nextFolder.links.filesLink, 1)); }); - it('should have correct form control', () => { - expect(component.projectNameControl).toBeDefined(); - expect(component.projectNameControl.value).toBeNull(); + it('should dispatch files load in onLoadFiles', () => { + setup({ detectChanges: false }); + + component.onLoadFiles({ link: '/v2/nodes/node-456/files/', page: 3 }); + + expect(store.dispatch).toHaveBeenCalledWith(new FetchProjectFilesByLink('/v2/nodes/node-456/files/', 3)); }); }); diff --git a/src/app/features/preprints/components/stepper/file-step/file-step.component.ts b/src/app/features/preprints/components/stepper/file-step/file-step.component.ts index 9f556128a..b8f771b61 100644 --- a/src/app/features/preprints/components/stepper/file-step/file-step.component.ts +++ b/src/app/features/preprints/components/stepper/file-step/file-step.component.ts @@ -42,6 +42,7 @@ import { } from '@osf/features/preprints/store/preprint-stepper'; import { FilesTreeComponent } from '@osf/shared/components/files-tree/files-tree.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; +import { ClearFileDirective } from '@osf/shared/directives/clear-file.directive'; import { StringOrNull } from '@osf/shared/helpers/types.helper'; import { FileModel } from '@osf/shared/models/files/file.model'; import { FileFolderModel } from '@osf/shared/models/files/file-folder.model'; @@ -52,16 +53,17 @@ import { ToastService } from '@osf/shared/services/toast.service'; selector: 'osf-file-step', imports: [ Button, - TitleCasePipe, - NgClass, + Card, Tooltip, Skeleton, - IconComponent, - Card, Select, + NgClass, ReactiveFormsModule, + IconComponent, FilesTreeComponent, + TitleCasePipe, TranslatePipe, + ClearFileDirective, ], templateUrl: './file-step.component.html', styleUrl: './file-step.component.scss', @@ -70,6 +72,8 @@ import { ToastService } from '@osf/shared/services/toast.service'; export class FileStepComponent implements OnInit { private toastService = inject(ToastService); private customConfirmationService = inject(CustomConfirmationService); + private destroyRef = inject(DestroyRef); + private actions = createDispatchMap({ setSelectedFileSource: SetSelectedPreprintFileSource, getPreprintFilesLinks: FetchPreprintFilesLinks, @@ -82,13 +86,11 @@ export class FileStepComponent implements OnInit { copyFileFromProject: CopyFileFromProject, setCurrentFolder: SetPreprintStepperCurrentFolder, }); - private destroyRef = inject(DestroyRef); readonly PreprintFileSource = PreprintFileSource; - provider = input.required(); + provider = input.required(); preprint = select(PreprintStepperSelectors.getPreprint); - providerId = select(PreprintStepperSelectors.getSelectedProviderId); selectedFileSource = select(PreprintStepperSelectors.getSelectedFileSource); fileUploadLink = select(PreprintStepperSelectors.getUploadLink); @@ -120,12 +122,15 @@ export class FileStepComponent implements OnInit { backClicked = output(); isFileSourceSelected = computed(() => this.selectedFileSource() !== PreprintFileSource.None); + canProceedToNext = computed(() => !!this.preprintFile() && !this.versionFileMode()); ngOnInit() { this.actions.getPreprintFilesLinks(); + if (this.preprintHasPrimaryFile() && !this.preprintFile()) { this.actions.fetchPreprintFile(); } + this.projectNameControl.valueChanges .pipe(debounceTime(500), distinctUntilChanged(), takeUntilDestroyed(this.destroyRef)) .subscribe((projectNameOrId) => { @@ -150,7 +155,7 @@ export class FileStepComponent implements OnInit { } nextButtonClicked() { - if (!this.preprint()?.primaryFileId) { + if (!this.canProceedToNext() || !this.preprint()?.primaryFileId) { return; } @@ -163,20 +168,14 @@ export class FileStepComponent implements OnInit { const file = input.files?.[0]; if (!file) return; - if (this.versionFileMode()) { + const isVersionFileMode = this.versionFileMode(); + + if (isVersionFileMode) { this.versionFileMode.set(false); - this.actions.reuploadFile(file).subscribe({ - next: () => { - this.actions.fetchPreprintFile(); - }, - }); - } else { - this.actions.uploadFile(file).subscribe({ - next: () => { - this.actions.fetchPreprintFile(); - }, - }); } + + const uploadAction = isVersionFileMode ? this.actions.reuploadFile(file) : this.actions.uploadFile(file); + uploadAction.subscribe(() => this.actions.fetchPreprintFile()); } selectProject(event: SelectChangeEvent) { @@ -185,6 +184,7 @@ export class FileStepComponent implements OnInit { } this.selectedProjectId.set(event.value); + this.actions .setProjectRootFolder(event.value) .pipe( @@ -201,11 +201,7 @@ export class FileStepComponent implements OnInit { } selectProjectFile(file: FileModel) { - this.actions.copyFileFromProject(file).subscribe({ - next: () => { - this.actions.fetchPreprintFile(); - }, - }); + this.actions.copyFileFromProject(file).subscribe(() => this.actions.fetchPreprintFile()); } versionFile() { @@ -232,6 +228,7 @@ export class FileStepComponent implements OnInit { if (this.currentFolder()?.id === folder.id) { return; } + this.actions.setCurrentFolder(folder); this.actions.getProjectFilesByLink(folder.links.filesLink, 1); } diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.html b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.html index 090082de5..8866ee45c 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.html +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.html @@ -6,16 +6,14 @@

{{ 'preprints.preprintStepper.metadata.affiliatedInstitutionsTitle' | transl class="mt-3" [innerHTML]=" 'preprints.preprintStepper.metadata.affiliatedInstitutionsDescription' - | translate: { preprintWord: provider()?.preprintWord } + | translate: { preprintWord: provider().preprintWord } " >

diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.spec.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.spec.ts index 68cff07db..048833d6b 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.spec.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.spec.ts @@ -1,128 +1,144 @@ +import { Store } from '@ngxs/store'; + import { MockComponent } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { ReviewsState } from '@osf/features/preprints/enums'; -import { PreprintProviderDetails } from '@osf/features/preprints/models'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { PreprintModel, PreprintProviderDetails } from '@osf/features/preprints/models'; +import { PreprintStepperSelectors, SetInstitutionsChanged } from '@osf/features/preprints/store/preprint-stepper'; import { AffiliatedInstitutionSelectComponent } from '@osf/shared/components/affiliated-institution-select/affiliated-institution-select.component'; +import { ResourceType } from '@osf/shared/enums/resource-type.enum'; import { Institution } from '@osf/shared/models/institutions/institutions.model'; -import { InstitutionsSelectors } from '@shared/stores/institutions'; +import { + FetchResourceInstitutions, + FetchUserInstitutions, + InstitutionsSelectors, + UpdateResourceInstitutions, +} from '@shared/stores/institutions'; import { PreprintsAffiliatedInstitutionsComponent } from './preprints-affiliated-institutions.component'; import { MOCK_INSTITUTION } from '@testing/mocks/institution.mock'; +import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; import { PREPRINT_PROVIDER_DETAILS_MOCK } from '@testing/mocks/preprint-provider-details'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; describe('PreprintsAffiliatedInstitutionsComponent', () => { let component: PreprintsAffiliatedInstitutionsComponent; let fixture: ComponentFixture; + let store: Store; const mockProvider: PreprintProviderDetails = PREPRINT_PROVIDER_DETAILS_MOCK; - const mockPreprint: any = { id: 'preprint-1', reviewsState: ReviewsState.Pending }; + const mockPreprint: PreprintModel = PREPRINT_MOCK; const mockUserInstitutions: Institution[] = [MOCK_INSTITUTION]; const mockResourceInstitutions: Institution[] = [MOCK_INSTITUTION]; - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [ - PreprintsAffiliatedInstitutionsComponent, - OSFTestingModule, - MockComponent(AffiliatedInstitutionSelectComponent), - ], - providers: [ - provideMockStore({ - signals: [ - { - selector: InstitutionsSelectors.getUserInstitutions, - value: mockUserInstitutions, - }, - { - selector: InstitutionsSelectors.areUserInstitutionsLoading, - value: false, - }, - { - selector: InstitutionsSelectors.getResourceInstitutions, - value: mockResourceInstitutions, - }, - { - selector: InstitutionsSelectors.areResourceInstitutionsLoading, - value: false, - }, - { - selector: InstitutionsSelectors.areResourceInstitutionsSubmitting, - value: false, - }, - { - selector: PreprintStepperSelectors.getInstitutionsChanged, - value: false, - }, - ], - }), - ], - }).compileComponents(); - + const defaultSignals: SignalOverride[] = [ + { selector: InstitutionsSelectors.getUserInstitutions, value: mockUserInstitutions }, + { selector: InstitutionsSelectors.areUserInstitutionsLoading, value: false }, + { selector: InstitutionsSelectors.getResourceInstitutions, value: mockResourceInstitutions }, + { selector: InstitutionsSelectors.areResourceInstitutionsLoading, value: false }, + { selector: InstitutionsSelectors.areResourceInstitutionsSubmitting, value: false }, + { selector: PreprintStepperSelectors.getInstitutionsChanged, value: false }, + ]; + + function setup(overrides?: { + selectorOverrides?: SignalOverride[]; + preprint?: PreprintModel; + detectChanges?: boolean; + }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + + TestBed.configureTestingModule({ + imports: [PreprintsAffiliatedInstitutionsComponent, MockComponent(AffiliatedInstitutionSelectComponent)], + providers: [provideOSFCore(), provideMockStore({ signals })], + }); + + store = TestBed.inject(Store); fixture = TestBed.createComponent(PreprintsAffiliatedInstitutionsComponent); component = fixture.componentInstance; fixture.componentRef.setInput('provider', mockProvider); - fixture.componentRef.setInput('preprint', mockPreprint); - fixture.detectChanges(); - }); + fixture.componentRef.setInput('preprint', overrides?.preprint ?? mockPreprint); + if (overrides?.detectChanges ?? true) { + fixture.detectChanges(); + } + } it('should create', () => { + setup(); expect(component).toBeTruthy(); }); - it('should initialize with correct values', () => { - expect(component.provider()).toBe(mockProvider); - expect(component.preprint()!.id).toBe('preprint-1'); - expect(component.userInstitutions()).toBe(mockUserInstitutions); - expect(component.areUserInstitutionsLoading()).toBe(false); - expect(component.resourceInstitutions()).toBe(mockResourceInstitutions); - expect(component.areResourceInstitutionsLoading()).toBe(false); - expect(component.areResourceInstitutionsSubmitting()).toBe(false); + it('should compute loading state when any loading flag is true', () => { + setup({ + selectorOverrides: [{ selector: InstitutionsSelectors.areResourceInstitutionsSubmitting, value: true }], + }); + expect(component.isLoading()).toBe(true); }); - it('should initialize selectedInstitutions with resource institutions', () => { + it('should initialize selected institutions from resource institutions effect', () => { + setup(); expect(component.selectedInstitutions()).toEqual(mockResourceInstitutions); }); - it('should handle institutions change', () => { - const newInstitutions = [MOCK_INSTITUTION]; + it('should keep selected institutions empty when resource institutions are empty', () => { + setup({ + selectorOverrides: [{ selector: InstitutionsSelectors.getResourceInstitutions, value: [] }], + }); + expect(component.selectedInstitutions()).toEqual([]); + }); - component.onInstitutionsChange(newInstitutions); + it('should dispatch fetch actions on init lifecycle', () => { + setup(); - expect(component.selectedInstitutions()).toEqual(newInstitutions); + expect(store.dispatch).toHaveBeenCalledWith(new FetchUserInstitutions()); + expect(store.dispatch).toHaveBeenCalledWith(new FetchResourceInstitutions(mockPreprint.id, ResourceType.Preprint)); }); - it('should handle effect for resource institutions', () => { - const newResourceInstitutions = [MOCK_INSTITUTION]; + it('should auto-apply user institutions on create flow when institutions not changed', () => { + setup({ + preprint: { ...mockPreprint, reviewsState: ReviewsState.Initial }, + }); - jest.spyOn(component, 'resourceInstitutions').mockReturnValue(newResourceInstitutions); - component.ngOnInit(); - - expect(component.selectedInstitutions()).toEqual(newResourceInstitutions); + expect(store.dispatch).toHaveBeenCalledWith(new SetInstitutionsChanged(true)); + expect(store.dispatch).toHaveBeenCalledWith( + new UpdateResourceInstitutions(mockPreprint.id, ResourceType.Preprint, mockUserInstitutions) + ); }); - it('should not update selectedInstitutions when resource institutions is empty', () => { - const initialInstitutions = component.selectedInstitutions(); + it('should not auto-apply user institutions when not in create flow', () => { + setup({ + preprint: { ...mockPreprint, reviewsState: ReviewsState.Pending }, + }); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(SetInstitutionsChanged)); + expect(store.dispatch).not.toHaveBeenCalledWith( + new UpdateResourceInstitutions(mockPreprint.id, ResourceType.Preprint, mockUserInstitutions) + ); + }); - jest.spyOn(component, 'resourceInstitutions').mockReturnValue([]); - component.ngOnInit(); + it('should not auto-apply user institutions when institutions already changed', () => { + setup({ + preprint: { ...mockPreprint, reviewsState: ReviewsState.Initial }, + selectorOverrides: [{ selector: PreprintStepperSelectors.getInstitutionsChanged, value: true }], + }); - expect(component.selectedInstitutions()).toEqual(initialInstitutions); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(SetInstitutionsChanged)); + expect(store.dispatch).not.toHaveBeenCalledWith( + new UpdateResourceInstitutions(mockPreprint.id, ResourceType.Preprint, mockUserInstitutions) + ); }); - it('should handle multiple institution changes', () => { - const firstChange = [MOCK_INSTITUTION]; - const secondChange = [MOCK_INSTITUTION]; + it('should dispatch update institutions on selection change', () => { + setup(); + const updatedInstitutions = [MOCK_INSTITUTION]; - component.onInstitutionsChange(firstChange); - expect(component.selectedInstitutions()).toEqual(firstChange); + component.onInstitutionsChange(updatedInstitutions); - component.onInstitutionsChange(secondChange); - expect(component.selectedInstitutions()).toEqual(secondChange); + expect(store.dispatch).toHaveBeenCalledWith( + new UpdateResourceInstitutions(mockPreprint.id, ResourceType.Preprint, updatedInstitutions) + ); }); }); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.ts index fc8444c93..084d50f9e 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-affiliated-institutions/preprints-affiliated-institutions.component.ts @@ -4,7 +4,7 @@ import { TranslatePipe } from '@ngx-translate/core'; import { Card } from 'primeng/card'; -import { ChangeDetectionStrategy, Component, effect, input, OnInit, signal } from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, effect, input, OnInit, signal } from '@angular/core'; import { ReviewsState } from '@osf/features/preprints/enums'; import { PreprintModel, PreprintProviderDetails } from '@osf/features/preprints/models'; @@ -27,17 +27,17 @@ import { changeDetection: ChangeDetectionStrategy.OnPush, }) export class PreprintsAffiliatedInstitutionsComponent implements OnInit { - provider = input.required(); - preprint = input.required(); + readonly provider = input.required(); + readonly preprint = input.required(); - selectedInstitutions = signal([]); + readonly selectedInstitutions = signal([]); - userInstitutions = select(InstitutionsSelectors.getUserInstitutions); - areUserInstitutionsLoading = select(InstitutionsSelectors.areUserInstitutionsLoading); - resourceInstitutions = select(InstitutionsSelectors.getResourceInstitutions); - areResourceInstitutionsLoading = select(InstitutionsSelectors.areResourceInstitutionsLoading); - areResourceInstitutionsSubmitting = select(InstitutionsSelectors.areResourceInstitutionsSubmitting); - institutionsChanged = select(PreprintStepperSelectors.getInstitutionsChanged); + readonly userInstitutions = select(InstitutionsSelectors.getUserInstitutions); + readonly areUserInstitutionsLoading = select(InstitutionsSelectors.areUserInstitutionsLoading); + readonly resourceInstitutions = select(InstitutionsSelectors.getResourceInstitutions); + readonly areResourceInstitutionsLoading = select(InstitutionsSelectors.areResourceInstitutionsLoading); + readonly areResourceInstitutionsSubmitting = select(InstitutionsSelectors.areResourceInstitutionsSubmitting); + readonly institutionsChanged = select(PreprintStepperSelectors.getInstitutionsChanged); private readonly actions = createDispatchMap({ fetchUserInstitutions: FetchUserInstitutions, @@ -46,6 +46,13 @@ export class PreprintsAffiliatedInstitutionsComponent implements OnInit { setInstitutionsChanged: SetInstitutionsChanged, }); + isLoading = computed( + () => + this.areUserInstitutionsLoading() || + this.areResourceInstitutionsLoading() || + this.areResourceInstitutionsSubmitting() + ); + constructor() { effect(() => { const resourceInstitutions = this.resourceInstitutions(); @@ -56,7 +63,7 @@ export class PreprintsAffiliatedInstitutionsComponent implements OnInit { effect(() => { const userInstitutions = this.userInstitutions(); - const isCreateFlow = this.preprint()?.reviewsState === ReviewsState.Initial; + const isCreateFlow = this.preprint().reviewsState === ReviewsState.Initial; if (userInstitutions.length > 0 && isCreateFlow && !this.institutionsChanged()) { this.actions.setInstitutionsChanged(true); @@ -67,7 +74,7 @@ export class PreprintsAffiliatedInstitutionsComponent implements OnInit { ngOnInit() { this.actions.fetchUserInstitutions(); - this.actions.fetchResourceInstitutions(this.preprint()!.id, ResourceType.Preprint); + this.actions.fetchResourceInstitutions(this.preprint().id, ResourceType.Preprint); } onInstitutionsChange(institutions: Institution[]): void { diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.spec.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.spec.ts index 6da10dfb3..580bf3eee 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.spec.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.spec.ts @@ -1,103 +1,253 @@ +import { Store } from '@ngxs/store'; + import { MockComponent, MockProvider } from 'ng-mocks'; +import { of } from 'rxjs'; + import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { UserSelectors } from '@core/store/user'; import { ContributorsTableComponent } from '@osf/shared/components/contributors'; +import { AddContributorType } from '@osf/shared/enums/contributors/add-contributor-type.enum'; +import { ResourceType } from '@osf/shared/enums/resource-type.enum'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; import { ToastService } from '@osf/shared/services/toast.service'; -import { ContributorModel } from '@shared/models/contributors/contributor.model'; -import { ContributorsSelectors } from '@shared/stores/contributors'; +import { + BulkAddContributors, + BulkUpdateContributors, + ContributorsSelectors, + DeleteContributor, + GetAllContributors, + LoadMoreContributors, +} from '@shared/stores/contributors'; import { PreprintsContributorsComponent } from './preprints-contributors.component'; -import { MOCK_CONTRIBUTOR } from '@testing/mocks/contributors.mock'; -import { MOCK_USER } from '@testing/mocks/data.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { CustomConfirmationServiceMockBuilder } from '@testing/providers/custom-confirmation-provider.mock'; -import { CustomDialogServiceMockBuilder } from '@testing/providers/custom-dialog-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; +import { MOCK_CONTRIBUTOR, MOCK_CONTRIBUTOR_ADD } from '@testing/mocks/contributors.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { + CustomConfirmationServiceMock, + CustomConfirmationServiceMockType, +} from '@testing/providers/custom-confirmation-provider.mock'; +import { CustomDialogServiceMock, CustomDialogServiceMockType } from '@testing/providers/custom-dialog-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('PreprintsContributorsComponent', () => { let component: PreprintsContributorsComponent; let fixture: ComponentFixture; - let toastServiceMock: ReturnType; - let confirmationServiceMock: ReturnType; - let mockCustomDialogService: ReturnType; - - const mockContributors: ContributorModel[] = [MOCK_CONTRIBUTOR]; - const mockCurrentUser = MOCK_USER; - - beforeEach(async () => { - toastServiceMock = ToastServiceMockBuilder.create().build(); - confirmationServiceMock = CustomConfirmationServiceMockBuilder.create().build(); - mockCustomDialogService = CustomDialogServiceMockBuilder.create().build(); - - await TestBed.configureTestingModule({ - imports: [PreprintsContributorsComponent, OSFTestingModule, MockComponent(ContributorsTableComponent)], + let store: Store; + let dialogMock: CustomDialogServiceMockType; + let confirmationMock: CustomConfirmationServiceMockType; + let toastMock: ToastServiceMockType; + + const mockContributors = [MOCK_CONTRIBUTOR]; + const preprintId = 'preprint-1'; + const defaultSignals: SignalOverride[] = [ + { selector: ContributorsSelectors.getContributors, value: mockContributors }, + { selector: ContributorsSelectors.isContributorsLoading, value: false }, + { selector: ContributorsSelectors.isContributorsLoadingMore, value: false }, + { selector: ContributorsSelectors.getContributorsPageSize, value: 10 }, + { selector: ContributorsSelectors.getContributorsTotalCount, value: 1 }, + ]; + + function setup(overrides?: { + preprintId?: string; + selectorOverrides?: SignalOverride[]; + addDialogCloseValue?: unknown; + addUnregisteredDialogCloseValue?: unknown; + }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + const addDialogClose$ = of(overrides?.addDialogCloseValue); + const addUnregisteredDialogClose$ = of(overrides?.addUnregisteredDialogCloseValue); + + dialogMock = CustomDialogServiceMock.create() + .withOpen( + jest.fn((component: unknown) => { + const isUnregisteredDialog = + typeof component === 'function' && `${component}`.includes('AddUnregisteredContributorDialogComponent'); + return { + onClose: isUnregisteredDialog ? addUnregisteredDialogClose$ : addDialogClose$, + } as never; + }) + ) + .build(); + confirmationMock = CustomConfirmationServiceMock.simple(); + toastMock = ToastServiceMock.simple(); + + TestBed.configureTestingModule({ + imports: [PreprintsContributorsComponent, MockComponent(ContributorsTableComponent)], providers: [ - MockProvider(ToastService, toastServiceMock), - MockProvider(CustomConfirmationService, confirmationServiceMock), - MockProvider(CustomDialogService, mockCustomDialogService), - provideMockStore({ - signals: [ - { - selector: ContributorsSelectors.getContributors, - value: mockContributors, - }, - { - selector: ContributorsSelectors.isContributorsLoading, - value: false, - }, - { - selector: UserSelectors.getCurrentUser, - value: mockCurrentUser, - }, - ], - }), + provideOSFCore(), + MockProvider(CustomDialogService, dialogMock), + MockProvider(CustomConfirmationService, confirmationMock), + MockProvider(ToastService, toastMock), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(PreprintsContributorsComponent); component = fixture.componentInstance; - fixture.componentRef.setInput('preprintId', 'preprint-1'); + fixture.componentRef.setInput( + 'preprintId', + overrides && 'preprintId' in overrides ? overrides.preprintId : preprintId + ); + fixture.detectChanges(); + } + + it('should fetch contributors when preprint id exists', () => { + setup(); + + expect(store.dispatch).toHaveBeenCalledWith(new GetAllContributors(preprintId, ResourceType.Preprint)); + }); + + it('should clone initial contributors into editable contributors', () => { + setup(); + + expect(component.contributors()).toEqual(mockContributors); + expect(component.contributors()).not.toBe(mockContributors); + }); + + it('should compute hasChanges correctly', () => { + setup(); + expect(component.hasChanges).toBe(false); + + component.contributors.set([{ ...MOCK_CONTRIBUTOR, fullName: 'Updated Name' }]); + expect(component.hasChanges).toBe(true); + + component.contributors.set([]); + expect(component.hasChanges).toBe(true); + }); + + it('should compute table params from selector values', () => { + setup(); + + expect(component.tableParams().totalRecords).toBe(1); + expect(component.tableParams().rows).toBe(10); + expect(component.tableParams().paginator).toBe(false); + expect(component.tableParams().scrollable).toBe(true); + }); + + it('should reset contributors on cancel', () => { + setup(); + component.contributors.set([{ ...MOCK_CONTRIBUTOR, fullName: 'Changed' }]); + + component.cancel(); + + expect(component.contributors()).toEqual(mockContributors); }); - it('should create', () => { - expect(component).toBeTruthy(); + it('should save changed contributors and show success toast', () => { + setup(); + component.contributors.set([{ ...MOCK_CONTRIBUTOR, fullName: 'Updated Name' }]); + + component.save(); + + expect(store.dispatch).toHaveBeenCalledWith( + new BulkUpdateContributors(preprintId, ResourceType.Preprint, [{ ...MOCK_CONTRIBUTOR, fullName: 'Updated Name' }]) + ); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'project.contributors.toastMessages.multipleUpdateSuccessMessage' + ); + }); + + it('should open add contributor dialog', () => { + setup(); + + component.openAddContributorDialog(); + + expect(dialogMock.open).toHaveBeenCalled(); }); - it('should remove contributor with confirmation', () => { - const contributorToRemove = mockContributors[0]; + it('should ignore empty add contributor dialog result', () => { + setup({ addDialogCloseValue: null }); - confirmationServiceMock.confirmDelete.mockImplementation(({ onConfirm }) => { - onConfirm(); + component.openAddContributorDialog(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(BulkAddContributors)); + }); + + it('should open unregistered dialog when add dialog returns unregistered type', () => { + setup({ + addDialogCloseValue: { type: AddContributorType.Unregistered, data: [MOCK_CONTRIBUTOR_ADD] }, + }); + const openUnregisteredSpy = jest.spyOn(component, 'openAddUnregisteredContributorDialog'); + + component.openAddContributorDialog(); + + expect(openUnregisteredSpy).toHaveBeenCalled(); + }); + + it('should add contributors when add dialog returns registered type', () => { + setup({ + addDialogCloseValue: { type: AddContributorType.Registered, data: [MOCK_CONTRIBUTOR_ADD] }, }); - component.removeContributor(contributorToRemove); + component.openAddContributorDialog(); + + expect(store.dispatch).toHaveBeenCalledWith( + new BulkAddContributors(preprintId, ResourceType.Preprint, [MOCK_CONTRIBUTOR_ADD]) + ); + expect(toastMock.showSuccess).toHaveBeenCalledWith('project.contributors.toastMessages.multipleAddSuccessMessage'); + }); - expect(confirmationServiceMock.confirmDelete).toHaveBeenCalledWith({ + it('should open registered dialog when unregistered dialog returns registered type', () => { + setup({ + addUnregisteredDialogCloseValue: { type: AddContributorType.Registered, data: [MOCK_CONTRIBUTOR_ADD] }, + }); + const openRegisteredSpy = jest.spyOn(component, 'openAddContributorDialog'); + + component.openAddUnregisteredContributorDialog(); + + expect(openRegisteredSpy).toHaveBeenCalled(); + }); + + it('should add unregistered contributor and show named toast', () => { + setup({ + addUnregisteredDialogCloseValue: { + type: AddContributorType.Unregistered, + data: [{ ...MOCK_CONTRIBUTOR_ADD, fullName: 'Jane Doe' }], + }, + }); + + component.openAddUnregisteredContributorDialog(); + + expect(store.dispatch).toHaveBeenCalledWith( + new BulkAddContributors(preprintId, ResourceType.Preprint, [{ ...MOCK_CONTRIBUTOR_ADD, fullName: 'Jane Doe' }]) + ); + expect(toastMock.showSuccess).toHaveBeenCalledWith('project.contributors.toastMessages.addSuccessMessage', { + name: 'Jane Doe', + }); + }); + + it('should open delete confirmation and delete contributor on confirm', () => { + setup(); + + component.removeContributor(MOCK_CONTRIBUTOR); + + expect(confirmationMock.confirmDelete).toHaveBeenCalledWith({ headerKey: 'project.contributors.removeDialog.title', messageKey: 'project.contributors.removeDialog.message', - messageParams: { name: contributorToRemove.fullName }, + messageParams: { name: MOCK_CONTRIBUTOR.fullName }, acceptLabelKey: 'common.buttons.remove', onConfirm: expect.any(Function), }); - }); - it('should expose readonly properties', () => { - expect(component.destroyRef).toBeDefined(); - expect(component.customDialogService).toBeDefined(); - expect(component.toastService).toBeDefined(); - expect(component.customConfirmationService).toBeDefined(); - expect(component.actions).toBeDefined(); - }); + const { onConfirm } = confirmationMock.confirmDelete.mock.calls[0][0]; + onConfirm(); - it('should handle effect for contributors', () => { - component.ngOnInit(); + expect(store.dispatch).toHaveBeenCalledWith( + new DeleteContributor(preprintId, ResourceType.Preprint, MOCK_CONTRIBUTOR.userId) + ); + expect(toastMock.showSuccess).toHaveBeenCalledWith('project.contributors.removeDialog.successMessage', { + name: MOCK_CONTRIBUTOR.fullName, + }); + }); - expect(component).toBeTruthy(); + it('should load more contributors', () => { + setup({ preprintId }); + component.loadMoreContributors(); + expect(store.dispatch).toHaveBeenCalledWith(new LoadMoreContributors(preprintId, ResourceType.Preprint)); }); }); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.ts index 3fbfa30d6..aad0b7305 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-contributors/preprints-contributors.component.ts @@ -5,7 +5,6 @@ import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; import { Card } from 'primeng/card'; import { Message } from 'primeng/message'; -import { TableModule } from 'primeng/table'; import { filter } from 'rxjs'; @@ -21,7 +20,6 @@ import { signal, } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { FormsModule } from '@angular/forms'; import { AddContributorDialogComponent, @@ -49,25 +47,33 @@ import { TableParameters } from '@shared/models/table-parameters.model'; @Component({ selector: 'osf-preprints-contributors', - imports: [FormsModule, TableModule, ContributorsTableComponent, TranslatePipe, Card, Button, Message], + imports: [Button, Card, Message, ContributorsTableComponent, TranslatePipe], templateUrl: './preprints-contributors.component.html', styleUrl: './preprints-contributors.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class PreprintsContributorsComponent implements OnInit { - preprintId = input(''); + readonly preprintId = input.required(); readonly destroyRef = inject(DestroyRef); readonly customDialogService = inject(CustomDialogService); readonly toastService = inject(ToastService); readonly customConfirmationService = inject(CustomConfirmationService); - initialContributors = select(ContributorsSelectors.getContributors); - contributors = signal([]); - contributorsTotalCount = select(ContributorsSelectors.getContributorsTotalCount); - isContributorsLoading = select(ContributorsSelectors.isContributorsLoading); - isLoadingMore = select(ContributorsSelectors.isContributorsLoadingMore); - pageSize = select(ContributorsSelectors.getContributorsPageSize); + readonly initialContributors = select(ContributorsSelectors.getContributors); + readonly contributors = signal([]); + readonly contributorsTotalCount = select(ContributorsSelectors.getContributorsTotalCount); + readonly isContributorsLoading = select(ContributorsSelectors.isContributorsLoading); + readonly isLoadingMore = select(ContributorsSelectors.isContributorsLoadingMore); + readonly pageSize = select(ContributorsSelectors.getContributorsPageSize); + + readonly actions = createDispatchMap({ + getContributors: GetAllContributors, + deleteContributor: DeleteContributor, + bulkUpdateContributors: BulkUpdateContributors, + bulkAddContributors: BulkAddContributors, + loadMoreContributors: LoadMoreContributors, + }); readonly tableParams = computed(() => ({ ...DEFAULT_TABLE_PARAMS, @@ -78,14 +84,6 @@ export class PreprintsContributorsComponent implements OnInit { rows: this.pageSize(), })); - actions = createDispatchMap({ - getContributors: GetAllContributors, - deleteContributor: DeleteContributor, - bulkUpdateContributors: BulkUpdateContributors, - bulkAddContributors: BulkAddContributors, - loadMoreContributors: LoadMoreContributors, - }); - get hasChanges(): boolean { return JSON.stringify(this.initialContributors()) !== JSON.stringify(this.contributors()); } @@ -155,9 +153,12 @@ export class PreprintsContributorsComponent implements OnInit { } else { const params = { name: res.data[0].fullName }; - this.actions.bulkAddContributors(this.preprintId(), ResourceType.Preprint, res.data).subscribe({ - next: () => this.toastService.showSuccess('project.contributors.toastMessages.addSuccessMessage', params), - }); + this.actions + .bulkAddContributors(this.preprintId(), ResourceType.Preprint, res.data) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(() => + this.toastService.showSuccess('project.contributors.toastMessages.addSuccessMessage', params) + ); } }); } @@ -172,12 +173,10 @@ export class PreprintsContributorsComponent implements OnInit { this.actions .deleteContributor(this.preprintId(), ResourceType.Preprint, contributor.userId) .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe({ - next: () => { - this.toastService.showSuccess('project.contributors.removeDialog.successMessage', { - name: contributor.fullName, - }); - }, + .subscribe(() => { + this.toastService.showSuccess('project.contributors.removeDialog.successMessage', { + name: contributor.fullName, + }); }); }, }); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.html b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.html index 3d20423c3..624123cc0 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.html +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.html @@ -1,8 +1,12 @@ +@let preprint = createdPreprint(); +

{{ 'preprints.preprintStepper.metadata.title' | translate }}

-
- -
+@if (preprint) { +
+ +
+}

{{ 'shared.license.title' | translate }}

@@ -14,6 +18,7 @@

{{ 'shared.license.title' | translate }}

{{ 'common.links.helpGuide' | translate }}.

+ {{ 'shared.license.title' | translate }}

/> -
- -
+@if (preprint) { +
+ +
-
- -
+
+ +
+}
@@ -47,10 +58,12 @@

{{ 'preprints.preprintStepper.metadata.tagsTitle' | translate }}

{{ 'preprints.preprintStepper.metadata.publicationDoi.title' | translate }}

+ @let doiControl = metadataForm.controls['doi']; + @if (doiControl.errors?.['pattern'] && (doiControl.touched || doiControl.dirty)) { - {{ 'preprints.preprintStepper.metadata.publicationDoi.patternError' | translate }} + + {{ 'preprints.preprintStepper.metadata.publicationDoi.patternError' | translate }} }
@@ -91,7 +104,7 @@

{{ 'preprints.preprintStepper.metadata.publicationCitationTitle styleClass="w-full" [label]="'common.buttons.back' | translate" severity="info" - (click)="backButtonClicked()" + (onClick)="backButtonClicked()" /> {{ 'preprints.preprintStepper.metadata.publicationCitationTitle tooltipPosition="top" [disabled]="metadataForm.invalid || !createdPreprint()?.licenseId" [loading]="isUpdatingPreprint()" - (click)="nextButtonClicked()" + (onClick)="nextButtonClicked()" /> diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.spec.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.spec.ts index ee60f9c14..8c906c5a4 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.spec.ts @@ -1,18 +1,24 @@ +import { Store } from '@ngxs/store'; + import { MockComponents, MockProvider } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { FormControl, FormGroup } from '@angular/forms'; -import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { formInputLimits } from '@osf/features/preprints/constants'; -import { PreprintProviderDetails } from '@osf/features/preprints/models'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { PreprintModel, PreprintProviderDetails } from '@osf/features/preprints/models'; +import { + FetchLicenses, + PreprintStepperSelectors, + SaveLicense, + UpdatePreprint, +} from '@osf/features/preprints/store/preprint-stepper'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { LicenseComponent } from '@osf/shared/components/license/license.component'; import { TextInputComponent } from '@osf/shared/components/text-input/text-input.component'; import { INPUT_VALIDATION_MESSAGES } from '@osf/shared/constants/input-validation-messages.const'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { ToastService } from '@osf/shared/services/toast.service'; +import { LicenseModel } from '@shared/models/license/license.model'; import { PreprintsAffiliatedInstitutionsComponent } from './preprints-affiliated-institutions/preprints-affiliated-institutions.component'; import { PreprintsContributorsComponent } from './preprints-contributors/preprints-contributors.component'; @@ -22,31 +28,39 @@ import { PreprintsMetadataStepComponent } from './preprints-metadata-step.compon import { MOCK_LICENSE } from '@testing/mocks/license.mock'; import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; import { PREPRINT_PROVIDER_DETAILS_MOCK } from '@testing/mocks/preprint-provider-details'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { CustomConfirmationServiceMockBuilder } from '@testing/providers/custom-confirmation-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMockBuilder } from '@testing/providers/toast-provider.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { + CustomConfirmationServiceMock, + CustomConfirmationServiceMockType, +} from '@testing/providers/custom-confirmation-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('PreprintsMetadataStepComponent', () => { let component: PreprintsMetadataStepComponent; let fixture: ComponentFixture; - let toastServiceMock: ReturnType; - let customConfirmationServiceMock: ReturnType; + let store: Store; + let toastServiceMock: ToastServiceMockType; + let customConfirmationServiceMock: CustomConfirmationServiceMockType; const mockProvider: PreprintProviderDetails = PREPRINT_PROVIDER_DETAILS_MOCK; - const mockPreprint = PREPRINT_MOCK; - const mockLicenses = [MOCK_LICENSE]; + const mockPreprint: PreprintModel = PREPRINT_MOCK; + const mockLicenses: LicenseModel[] = [MOCK_LICENSE]; + + const defaultSignals: SignalOverride[] = [ + { selector: PreprintStepperSelectors.getLicenses, value: mockLicenses }, + { selector: PreprintStepperSelectors.getPreprint, value: mockPreprint }, + { selector: PreprintStepperSelectors.isPreprintSubmitting, value: false }, + ]; - beforeEach(async () => { - toastServiceMock = ToastServiceMockBuilder.create().withShowSuccess(jest.fn()).build(); - customConfirmationServiceMock = CustomConfirmationServiceMockBuilder.create() - .withConfirmContinue(jest.fn()) - .build(); + function setup(overrides?: { selectorOverrides?: SignalOverride[]; detectChanges?: boolean }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + toastServiceMock = ToastServiceMock.simple(); + customConfirmationServiceMock = CustomConfirmationServiceMock.simple(); - await TestBed.configureTestingModule({ + TestBed.configureTestingModule({ imports: [ PreprintsMetadataStepComponent, - OSFTestingModule, ...MockComponents( PreprintsContributorsComponent, IconComponent, @@ -57,125 +71,199 @@ describe('PreprintsMetadataStepComponent', () => { ), ], providers: [ + provideOSFCore(), MockProvider(ToastService, toastServiceMock), MockProvider(CustomConfirmationService, customConfirmationServiceMock), - provideNoopAnimations(), - provideMockStore({ - signals: [ - { - selector: PreprintStepperSelectors.getLicenses, - value: mockLicenses, - }, - { - selector: PreprintStepperSelectors.getPreprint, - value: mockPreprint, - }, - { - selector: PreprintStepperSelectors.isPreprintSubmitting, - value: false, - }, - ], - }), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(PreprintsMetadataStepComponent); component = fixture.componentInstance; fixture.componentRef.setInput('provider', mockProvider); - fixture.detectChanges(); - }); + if (overrides?.detectChanges ?? true) { + fixture.detectChanges(); + } + } it('should create', () => { + setup(); expect(component).toBeTruthy(); }); - it('should initialize with correct default values', () => { + it('should initialize defaults and form values', () => { + setup(); + expect(component.inputLimits).toBe(formInputLimits); expect(component.INPUT_VALIDATION_MESSAGES).toBe(INPUT_VALIDATION_MESSAGES); expect(component.today).toBeInstanceOf(Date); + expect(component.metadataForm.controls.doi.value).toBe(mockPreprint.doi); + expect(component.metadataForm.controls.customPublicationCitation.value).toBe( + mockPreprint.customPublicationCitation + ); + expect(component.metadataForm.controls.tags.value).toEqual(mockPreprint.tags); }); - it('should initialize form with correct structure', () => { - fixture.detectChanges(); - expect(component.metadataForm).toBeInstanceOf(FormGroup); - expect(component.metadataForm.controls['doi']).toBeInstanceOf(FormControl); - expect(component.metadataForm.controls['originalPublicationDate']).toBeInstanceOf(FormControl); - expect(component.metadataForm.controls['customPublicationCitation']).toBeInstanceOf(FormControl); - expect(component.metadataForm.controls['tags']).toBeInstanceOf(FormControl); - expect(component.metadataForm.controls['subjects']).toBeInstanceOf(FormControl); + it('should dispatch fetch licenses on init', () => { + setup(); + expect(store.dispatch).toHaveBeenCalledWith(new FetchLicenses(mockProvider.id)); }); - it('should return licenses from store', () => { - const licenses = component.licenses(); - expect(licenses).toBe(mockLicenses); - }); + it('should auto-select default license and dispatch save when license has no required fields', () => { + const licenseWithoutFields = { ...MOCK_LICENSE, id: 'license-no-fields', requiredFields: [] }; + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getLicenses, value: [licenseWithoutFields] }, + { + selector: PreprintStepperSelectors.getPreprint, + value: { ...mockPreprint, licenseId: null, defaultLicenseId: 'license-no-fields' }, + }, + ], + }); - it('should return created preprint from store', () => { - const preprint = component.createdPreprint(); - expect(preprint).toBe(mockPreprint); + expect(component.defaultLicense()).toBe('license-no-fields'); + expect(store.dispatch).toHaveBeenCalledWith(new SaveLicense('license-no-fields', undefined)); }); - it('should return submission state from store', () => { - const isSubmitting = component.isUpdatingPreprint(); - expect(isSubmitting).toBe(false); - }); + it('should auto-select default license without dispatching save when license requires fields', () => { + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getLicenses, value: [MOCK_LICENSE] }, + { + selector: PreprintStepperSelectors.getPreprint, + value: { ...mockPreprint, licenseId: null, defaultLicenseId: MOCK_LICENSE.id }, + }, + ], + }); - it('should return provider input', () => { - const provider = component.provider(); - expect(provider).toBe(mockProvider); + expect(component.defaultLicense()).toBe(MOCK_LICENSE.id); + expect(store.dispatch).not.toHaveBeenCalledWith(new SaveLicense(MOCK_LICENSE.id, undefined)); }); - it('should handle next button click with valid form', () => { - fixture.detectChanges(); + it('should return early in nextButtonClicked when form is invalid', () => { + setup(); const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); + (store.dispatch as jest.Mock).mockClear(); + + component.metadataForm.patchValue({ subjects: [] }); + component.nextButtonClicked(); + + expect(nextClickedSpy).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePreprint)); + }); - component.metadataForm.patchValue({ - subjects: [{ id: 'subject1', name: 'Test Subject' }], + it('should return early in nextButtonClicked when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + detectChanges: false, }); + component.initForm(); + component.metadataForm.patchValue({ subjects: [{ id: 'subject-1', name: 'Subject 1' }] }); + const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); + (store.dispatch as jest.Mock).mockClear(); component.nextButtonClicked(); - expect(nextClickedSpy).toHaveBeenCalled(); + expect(nextClickedSpy).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePreprint)); }); - it('should not proceed with next button click when form is invalid', () => { - fixture.detectChanges(); + it('should update preprint and emit success in nextButtonClicked', () => { + setup(); const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); - - component.metadataForm.patchValue({ - subjects: [], - }); + component.metadataForm.patchValue({ subjects: [{ id: 'subject-1', name: 'Subject 1' }] }); + (store.dispatch as jest.Mock).mockClear(); component.nextButtonClicked(); - expect(nextClickedSpy).not.toHaveBeenCalled(); + expect(store.dispatch).toHaveBeenCalledWith(expect.any(UpdatePreprint)); + expect(toastServiceMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.common.successMessages.preprintSaved' + ); + expect(nextClickedSpy).toHaveBeenCalled(); + }); + + it('should dispatch save license from createLicense', () => { + setup({ detectChanges: false }); + component.createLicense({ id: MOCK_LICENSE.id, licenseOptions: { year: '2024', copyrightHolders: 'A' } }); + expect(store.dispatch).toHaveBeenCalledWith( + new SaveLicense(MOCK_LICENSE.id, { year: '2024', copyrightHolders: 'A' }) + ); + }); + + it('should dispatch save license in selectLicense only when required fields are absent', () => { + setup({ detectChanges: false }); + const noFields = { ...MOCK_LICENSE, id: 'no-fields', requiredFields: [] }; + (store.dispatch as jest.Mock).mockClear(); + + component.selectLicense(noFields); + expect(store.dispatch).toHaveBeenCalledWith(new SaveLicense('no-fields', undefined)); + + (store.dispatch as jest.Mock).mockClear(); + component.selectLicense(MOCK_LICENSE); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(SaveLicense)); + }); + + it('should update tags form control', () => { + setup(); + component.updateTags(['alpha', 'beta']); + expect(component.metadataForm.controls.tags.value).toEqual(['alpha', 'beta']); }); - it('should handle back button click with changes', () => { - component.metadataForm.patchValue({ - doi: 'new-doi', + it('should return early in backButtonClicked when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + detectChanges: false, }); + component.initForm(); + const backClickedSpy = jest.spyOn(component.backClicked, 'emit'); component.backButtonClicked(); - expect(customConfirmationServiceMock.confirmContinue).toHaveBeenCalled(); + expect(backClickedSpy).not.toHaveBeenCalled(); + expect(customConfirmationServiceMock.confirmContinue).not.toHaveBeenCalled(); }); - it('should handle select license without required fields', () => { - const license = mockLicenses[0]; + it('should emit back when there are no changes in backButtonClicked', () => { + setup(); + const backClickedSpy = jest.spyOn(component.backClicked, 'emit'); + component.metadataForm.patchValue({ subjects: [{ id: 'subject-1', name: 'Subject 1' }] }); + + component.backButtonClicked(); - expect(() => component.selectLicense(license)).not.toThrow(); + expect(backClickedSpy).toHaveBeenCalled(); + expect(customConfirmationServiceMock.confirmContinue).not.toHaveBeenCalled(); }); - it('should handle select license with required fields', () => { - const license = mockLicenses[0]; + it('should request confirmation and emit on confirm when there are changes in backButtonClicked', () => { + setup(); + const backClickedSpy = jest.spyOn(component.backClicked, 'emit'); + component.metadataForm.patchValue({ doi: '10.9999/changed', subjects: [{ id: 'subject-1', name: 'Subject 1' }] }); + + component.backButtonClicked(); + + expect(customConfirmationServiceMock.confirmContinue).toHaveBeenCalledWith({ + headerKey: 'common.discardChanges.header', + messageKey: 'common.discardChanges.message', + onConfirm: expect.any(Function), + onReject: expect.any(Function), + }); - expect(() => component.selectLicense(license)).not.toThrow(); + const { onConfirm } = customConfirmationServiceMock.confirmContinue.mock.calls[0][0]; + onConfirm(); + expect(backClickedSpy).toHaveBeenCalled(); }); - it('should handle edge case with empty licenses', () => { - const licenses = component.licenses(); - expect(licenses).toBeDefined(); - expect(Array.isArray(licenses)).toBe(true); + it('should not emit on reject when there are changes in backButtonClicked', () => { + setup(); + const backClickedSpy = jest.spyOn(component.backClicked, 'emit'); + component.metadataForm.patchValue({ doi: '10.9999/changed', subjects: [{ id: 'subject-1', name: 'Subject 1' }] }); + + component.backButtonClicked(); + + const { onReject } = customConfirmationServiceMock.confirmContinue.mock.calls[0][0]; + onReject(); + expect(backClickedSpy).not.toHaveBeenCalled(); }); }); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.ts index 3d55f43ad..66be42590 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-metadata-step.component.ts @@ -15,7 +15,6 @@ import { FormControl, FormGroup, ReactiveFormsModule, Validators } from '@angula import { formInputLimits } from '@osf/features/preprints/constants'; import { MetadataForm, PreprintModel, PreprintProviderDetails } from '@osf/features/preprints/models'; import { - CreatePreprint, FetchLicenses, PreprintStepperSelectors, SaveLicense, @@ -39,21 +38,21 @@ import { PreprintsSubjectsComponent } from './preprints-subjects/preprints-subje @Component({ selector: 'osf-preprints-metadata', imports: [ - PreprintsContributorsComponent, Button, Card, - ReactiveFormsModule, Message, - TranslatePipe, DatePicker, - IconComponent, InputText, - TextInputComponent, Tooltip, + ReactiveFormsModule, + IconComponent, LicenseComponent, TagsInputComponent, PreprintsSubjectsComponent, + PreprintsContributorsComponent, PreprintsAffiliatedInstitutionsComponent, + TextInputComponent, + TranslatePipe, ], templateUrl: './preprints-metadata-step.component.html', styleUrl: './preprints-metadata-step.component.scss', @@ -62,27 +61,29 @@ import { PreprintsSubjectsComponent } from './preprints-subjects/preprints-subje export class PreprintsMetadataStepComponent implements OnInit { private customConfirmationService = inject(CustomConfirmationService); private toastService = inject(ToastService); + + provider = input.required(); + nextClicked = output(); + backClicked = output(); + private actions = createDispatchMap({ - createPreprint: CreatePreprint, updatePreprint: UpdatePreprint, fetchLicenses: FetchLicenses, saveLicense: SaveLicense, }); - metadataForm!: FormGroup; - inputLimits = formInputLimits; - readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; - today = new Date(); - licenses = select(PreprintStepperSelectors.getLicenses); createdPreprint = select(PreprintStepperSelectors.getPreprint); isUpdatingPreprint = select(PreprintStepperSelectors.isPreprintSubmitting); - provider = input.required(); - nextClicked = output(); - backClicked = output(); + metadataForm!: FormGroup; + today = new Date(); + defaultLicense = signal(undefined); + readonly inputLimits = formInputLimits; + readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; + constructor() { effect(() => { const licenses = this.licenses(); @@ -101,7 +102,7 @@ export class PreprintsMetadataStepComponent implements OnInit { } ngOnInit() { - this.actions.fetchLicenses(); + this.actions.fetchLicenses(this.provider().id); this.initForm(); } @@ -136,11 +137,16 @@ export class PreprintsMetadataStepComponent implements OnInit { return; } - const model = this.metadataForm.value; + const preprint = this.createdPreprint(); - const changedFields = findChangedFields(model, this.createdPreprint()!); + if (!preprint) { + return; + } + + const model = this.metadataForm.value; + const changedFields = findChangedFields(model, preprint); - this.actions.updatePreprint(this.createdPreprint()!.id, changedFields).subscribe({ + this.actions.updatePreprint(preprint.id, changedFields).subscribe({ complete: () => { this.toastService.showSuccess('preprints.preprintStepper.common.successMessages.preprintSaved'); this.nextClicked.emit(); @@ -156,6 +162,7 @@ export class PreprintsMetadataStepComponent implements OnInit { if (license.requiredFields.length) { return; } + this.actions.saveLicense(license.id); } @@ -166,9 +173,14 @@ export class PreprintsMetadataStepComponent implements OnInit { } backButtonClicked() { - const formValue = this.metadataForm.value; - delete formValue.subjects; - const changedFields = findChangedFields(formValue, this.createdPreprint()!); + const preprint = this.createdPreprint(); + + if (!preprint) { + return; + } + + const { subjects: _subjects, ...formValue } = this.metadataForm.value; + const changedFields = findChangedFields(formValue, preprint); if (!Object.keys(changedFields).length) { this.backClicked.emit(); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.spec.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.spec.ts index 1c9a4abe3..f820cf809 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.spec.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.spec.ts @@ -1,167 +1,85 @@ +import { Store } from '@ngxs/store'; + import { MockComponent } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { FormControl } from '@angular/forms'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; import { SubjectsComponent } from '@osf/shared/components/subjects/subjects.component'; -import { SubjectModel } from '@osf/shared/models/subject/subject.model'; -import { SubjectsSelectors } from '@osf/shared/stores/subjects'; +import { ResourceType } from '@osf/shared/enums/resource-type.enum'; +import { + FetchChildrenSubjects, + FetchSelectedSubjects, + FetchSubjects, + SubjectsSelectors, + UpdateResourceSubjects, +} from '@osf/shared/stores/subjects'; import { PreprintsSubjectsComponent } from './preprints-subjects.component'; import { SUBJECTS_MOCK } from '@testing/mocks/subject.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; +import { provideOSFCore } from '@testing/osf.testing.provider'; import { provideMockStore } from '@testing/providers/store-provider.mock'; describe('PreprintsSubjectsComponent', () => { let component: PreprintsSubjectsComponent; let fixture: ComponentFixture; + let store: Store; - const mockSubjects: SubjectModel[] = SUBJECTS_MOCK; + const mockSubjects = SUBJECTS_MOCK; - const mockFormControl = new FormControl([]); + beforeEach(() => { + const control = new FormControl([]); - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [PreprintsSubjectsComponent, OSFTestingModule, MockComponent(SubjectsComponent)], + TestBed.configureTestingModule({ + imports: [PreprintsSubjectsComponent, MockComponent(SubjectsComponent)], providers: [ + provideOSFCore(), provideMockStore({ signals: [ - { selector: PreprintStepperSelectors.getSelectedProviderId, value: 'test-provider-id' }, { selector: SubjectsSelectors.getSelectedSubjects, value: mockSubjects }, { selector: SubjectsSelectors.areSelectedSubjectsLoading, value: false }, ], }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(PreprintsSubjectsComponent); component = fixture.componentInstance; - + fixture.componentRef.setInput('control', control); + fixture.componentRef.setInput('providerId', 'test-provider-id'); fixture.componentRef.setInput('preprintId', 'test-preprint-id'); - fixture.componentRef.setInput('control', mockFormControl); - - fixture.detectChanges(); - }); - - describe('Component Creation', () => { - it('should create', () => { - expect(component).toBeTruthy(); - }); - - it('should be an instance of PreprintsSubjectsComponent', () => { - expect(component).toBeInstanceOf(PreprintsSubjectsComponent); - }); - }); - - it('should have required inputs', () => { - expect(component.preprintId()).toBe('test-preprint-id'); - expect(component.control()).toBe(mockFormControl); - }); - - it('should have NGXS selectors defined', () => { - expect(component.selectedSubjects).toBeDefined(); - expect(component.isSubjectsUpdating).toBeDefined(); - expect(component['selectedProviderId']).toBeDefined(); - }); - - it('should have actions defined', () => { - expect(component.actions).toBeDefined(); - expect(component.actions.fetchSubjects).toBeDefined(); - expect(component.actions.fetchSelectedSubjects).toBeDefined(); - expect(component.actions.fetchChildrenSubjects).toBeDefined(); - expect(component.actions.updateResourceSubjects).toBeDefined(); - }); - - it('should have INPUT_VALIDATION_MESSAGES constant', () => { - expect(component.INPUT_VALIDATION_MESSAGES).toBeDefined(); - }); - - it('should get selected subjects from store', () => { - expect(component.selectedSubjects()).toEqual(mockSubjects); - }); - - it('should get subjects loading state from store', () => { - expect(component.isSubjectsUpdating()).toBe(false); - }); - - it('should get selected provider ID from store', () => { - expect(component['selectedProviderId']()).toBe('test-provider-id'); - }); - - it('should call getSubjectChildren with parent ID', () => { - const parentId = 'parent-123'; - - expect(() => component.getSubjectChildren(parentId)).not.toThrow(); - }); - - it('should call searchSubjects with search term', () => { - const searchTerm = 'mathematics'; - - expect(() => component.searchSubjects(searchTerm)).not.toThrow(); - }); - - it('should handle null control gracefully', () => { - const nullControl = new FormControl(null); - fixture.componentRef.setInput('control', nullControl); - - expect(() => component.updateControlState(mockSubjects)).not.toThrow(); - }); - - it('should mark control as touched and dirty', () => { - const freshControl = new FormControl([]); - fixture.componentRef.setInput('control', freshControl); - - component.updateControlState(mockSubjects); - - expect(freshControl.touched).toBe(true); - expect(freshControl.dirty).toBe(true); - }); - - it('should render subjects component', () => { - const subjectsComponent = fixture.nativeElement.querySelector('osf-subjects'); - expect(subjectsComponent).toBeTruthy(); - }); - - it('should handle control with required error', () => { - mockFormControl.setErrors({ required: true }); - mockFormControl.markAsTouched(); - mockFormControl.markAsDirty(); fixture.detectChanges(); - - expect(component).toBeTruthy(); - expect(mockFormControl.errors).toEqual({ required: true }); }); - it('should not show error message when control is valid', () => { - mockFormControl.setErrors(null); - fixture.detectChanges(); - - const errorMessage = fixture.nativeElement.querySelector('p-message'); - expect(errorMessage).toBeFalsy(); + it('should fetch provider subjects and selected subjects on init when ids exist', () => { + expect(store.dispatch).toHaveBeenCalledWith(new FetchSubjects(ResourceType.Preprint, 'test-provider-id')); + expect(store.dispatch).toHaveBeenCalledWith(new FetchSelectedSubjects('test-preprint-id', ResourceType.Preprint)); + expect(component.control().value).toEqual(mockSubjects); }); - it('should handle empty preprintId', () => { - fixture.componentRef.setInput('preprintId', ''); + it('should dispatch child subjects fetch', () => { + component.getSubjectChildren('parent-123'); - expect(() => component.ngOnInit()).not.toThrow(); + expect(store.dispatch).toHaveBeenCalledWith(new FetchChildrenSubjects('parent-123')); }); - it('should handle undefined preprintId', () => { - fixture.componentRef.setInput('preprintId', undefined); - - expect(() => component.ngOnInit()).not.toThrow(); + it('should search subjects', () => { + component.searchSubjects('math'); + expect(store.dispatch).toHaveBeenCalledWith(new FetchSubjects(ResourceType.Preprint, 'test-provider-id', 'math')); }); - it('should handle empty subjects array', () => { - const emptySubjects: SubjectModel[] = []; + it('should update control state and resource subjects when preprint id exists', () => { + const control = component.control(); - expect(() => component.updateSelectedSubjects(emptySubjects)).not.toThrow(); - expect(mockFormControl.value).toEqual(emptySubjects); - }); + component.updateSelectedSubjects(mockSubjects); - it('should handle null subjects', () => { - expect(() => component.updateControlState(null as any)).not.toThrow(); + expect(control.value).toEqual(mockSubjects); + expect(control.touched).toBe(true); + expect(control.dirty).toBe(true); + expect(store.dispatch).toHaveBeenCalledWith( + new UpdateResourceSubjects('test-preprint-id', ResourceType.Preprint, mockSubjects) + ); }); }); diff --git a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.ts b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.ts index 0d18ceb5c..cad361ca3 100644 --- a/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.ts +++ b/src/app/features/preprints/components/stepper/preprints-metadata-step/preprints-subjects/preprints-subjects.component.ts @@ -8,7 +8,6 @@ import { Message } from 'primeng/message'; import { ChangeDetectionStrategy, Component, effect, input, OnInit } from '@angular/core'; import { FormControl } from '@angular/forms'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; import { SubjectsComponent } from '@osf/shared/components/subjects/subjects.component'; import { INPUT_VALIDATION_MESSAGES } from '@osf/shared/constants/input-validation-messages.const'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; @@ -29,21 +28,22 @@ import { SubjectModel } from '@shared/models/subject/subject.model'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class PreprintsSubjectsComponent implements OnInit { - preprintId = input(); + readonly control = input.required(); + readonly providerId = input.required(); + readonly preprintId = input.required(); - private readonly selectedProviderId = select(PreprintStepperSelectors.getSelectedProviderId); - selectedSubjects = select(SubjectsSelectors.getSelectedSubjects); - isSubjectsUpdating = select(SubjectsSelectors.areSelectedSubjectsLoading); - control = input.required(); + readonly selectedSubjects = select(SubjectsSelectors.getSelectedSubjects); + readonly isSubjectsUpdating = select(SubjectsSelectors.areSelectedSubjectsLoading); - readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; - actions = createDispatchMap({ + readonly actions = createDispatchMap({ fetchSubjects: FetchSubjects, fetchSelectedSubjects: FetchSelectedSubjects, fetchChildrenSubjects: FetchChildrenSubjects, updateResourceSubjects: UpdateResourceSubjects, }); + readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; + constructor() { effect(() => { this.updateControlState(this.selectedSubjects()); @@ -51,30 +51,28 @@ export class PreprintsSubjectsComponent implements OnInit { } ngOnInit(): void { - this.actions.fetchSubjects(ResourceType.Preprint, this.selectedProviderId()!); + this.actions.fetchSubjects(ResourceType.Preprint, this.providerId()); this.actions.fetchSelectedSubjects(this.preprintId()!, ResourceType.Preprint); } - getSubjectChildren(parentId: string) { + getSubjectChildren(parentId: string): void { this.actions.fetchChildrenSubjects(parentId); } - searchSubjects(search: string) { - this.actions.fetchSubjects(ResourceType.Preprint, this.selectedProviderId()!, search); + searchSubjects(search: string): void { + this.actions.fetchSubjects(ResourceType.Preprint, this.providerId(), search); } - updateSelectedSubjects(subjects: SubjectModel[]) { + updateSelectedSubjects(subjects: SubjectModel[]): void { this.updateControlState(subjects); - - this.actions.updateResourceSubjects(this.preprintId()!, ResourceType.Preprint, subjects); + this.actions.updateResourceSubjects(this.preprintId(), ResourceType.Preprint, subjects); } - updateControlState(value: SubjectModel[]) { - if (this.control()) { - this.control().setValue(value); - this.control().markAsTouched(); - this.control().markAsDirty(); - this.control().updateValueAndValidity(); - } + updateControlState(value: SubjectModel[]): void { + const control = this.control(); + control.setValue(value); + control.markAsTouched(); + control.markAsDirty(); + control.updateValueAndValidity(); } } diff --git a/src/app/features/preprints/components/stepper/review-step/review-step.component.html b/src/app/features/preprints/components/stepper/review-step/review-step.component.html index c3d45f941..236dc3d20 100644 --- a/src/app/features/preprints/components/stepper/review-step/review-step.component.html +++ b/src/app/features/preprints/components/stepper/review-step/review-step.component.html @@ -5,7 +5,7 @@

{{ 'preprints.preprintStepper.review.title' | translate | titlecase }}

{{ 'preprints.preprintStepper.review.workflowDescription' - | translate: { providerName: provider()?.name, reviewsWorkflow: provider()?.reviewsWorkflow } + | translate: { providerName: provider().name, reviewsWorkflow: provider().reviewsWorkflow } }}

@@ -28,30 +28,32 @@

{{ 'preprints.preprintStepper.review.sections.titleAndAbstract.title' | tran

{{ 'preprints.preprintStepper.review.sections.titleAndAbstract.service' - | translate: { preprintWord: provider()?.preprintWord | titlecase } + | translate: { preprintWord: provider().preprintWord | titlecase } }}

Provider logo -

{{ provider()?.name }}

+

{{ provider().name }}

-
-

{{ 'preprints.preprintStepper.common.labels.title' | translate }}

-

{{ preprint()!.title }}

-
+ @if (preprint(); as preprint) { +
+

{{ 'preprints.preprintStepper.common.labels.title' | translate }}

+

{{ preprint.title }}

+
-
-

{{ 'preprints.preprintStepper.common.labels.abstract' | translate }}

- -
+
+

{{ 'preprints.preprintStepper.common.labels.abstract' | translate }}

+ +
+ } @@ -143,7 +145,7 @@

{{ 'preprints.preprintStepper.review.sections.metadata.publicationCitation' -@if (provider()?.assertionsEnabled) { +@if (provider().assertionsEnabled) {

{{ 'preprints.preprintStepper.review.sections.authorAssertions.title' | translate }}

@@ -222,7 +224,7 @@

{{ 'preprints.preprintStepper.review.sections.supplements.title' | translate }}

@if (preprintProject()) { -

{{ preprintProject()?.name | fixSpecialChar }}

+

{{ preprintProject()?.name }}

} @else {

{{ 'preprints.preprintStepper.review.sections.supplements.noSupplements' | translate }}

} @@ -235,15 +237,15 @@

{{ 'preprints.preprintStepper.review.sections.supplements.title' | translate styleClass="w-full" [label]="'common.buttons.cancel' | translate" severity="info" - (click)="cancelSubmission()" [disabled]="isPreprintSubmitting()" + (onClick)="cancelSubmission()" /> diff --git a/src/app/features/preprints/components/stepper/review-step/review-step.component.spec.ts b/src/app/features/preprints/components/stepper/review-step/review-step.component.spec.ts index 3187a6bdb..4dad236d3 100644 --- a/src/app/features/preprints/components/stepper/review-step/review-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/review-step/review-step.component.spec.ts @@ -1,18 +1,34 @@ -import { MockComponents, MockPipe } from 'ng-mocks'; +import { Store } from '@ngxs/store'; + +import { MockComponents, MockPipe, MockProvider } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; import { PreprintProviderDetails } from '@osf/features/preprints/models'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { + FetchLicenses, + FetchPreprintProject, + PreprintStepperSelectors, + SubmitPreprint, + UpdatePreprint, + UpdatePrimaryFileRelationship, +} from '@osf/features/preprints/store/preprint-stepper'; import { AffiliatedInstitutionsViewComponent } from '@osf/shared/components/affiliated-institutions-view/affiliated-institutions-view.component'; import { ContributorsListComponent } from '@osf/shared/components/contributors-list/contributors-list.component'; import { LicenseDisplayComponent } from '@osf/shared/components/license-display/license-display.component'; +import { ResourceType } from '@osf/shared/enums/resource-type.enum'; +import { InterpolatePipe } from '@osf/shared/pipes/interpolate.pipe'; import { ToastService } from '@osf/shared/services/toast.service'; -import { InterpolatePipe } from '@shared/pipes/interpolate.pipe'; -import { ContributorsSelectors } from '@shared/stores/contributors'; -import { InstitutionsSelectors } from '@shared/stores/institutions'; -import { SubjectsSelectors } from '@shared/stores/subjects'; +import { + ContributorsSelectors, + GetBibliographicContributors, + LoadMoreBibliographicContributors, +} from '@osf/shared/stores/contributors'; +import { FetchResourceInstitutions, InstitutionsSelectors } from '@osf/shared/stores/institutions'; +import { FetchSelectedSubjects, SubjectsSelectors } from '@osf/shared/stores/subjects'; + +import { ReviewsState } from '../../../enums'; import { ReviewStepComponent } from './review-step.component'; @@ -23,83 +39,209 @@ import { OSF_FILE_MOCK } from '@testing/mocks/osf-file.mock'; import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; import { PREPRINT_PROVIDER_DETAILS_MOCK } from '@testing/mocks/preprint-provider-details'; import { SUBJECTS_MOCK } from '@testing/mocks/subject.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { RouterMock } from '@testing/providers/router-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { RouterMock, RouterMockType } from '@testing/providers/router-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('ReviewStepComponent', () => { let component: ReviewStepComponent; let fixture: ComponentFixture; - let router: jest.Mocked; + let store: Store; + let routerMock: RouterMockType; + let toastMock: ToastServiceMockType; const mockProvider: PreprintProviderDetails = PREPRINT_PROVIDER_DETAILS_MOCK; const mockPreprint = PREPRINT_MOCK; const mockPreprintFile = OSF_FILE_MOCK; - const mockContributors = [MOCK_CONTRIBUTOR]; - const mockSubjects = SUBJECTS_MOCK; - const mockInstitutions = [MOCK_INSTITUTION]; - const mockLicense = MOCK_LICENSE; - const mockPreprintProject = { - id: 'project-id', - name: 'Test Project', - }; - - beforeEach(async () => { - await TestBed.configureTestingModule({ + + const defaultSignals: SignalOverride[] = [ + { selector: PreprintStepperSelectors.getPreprint, value: mockPreprint }, + { selector: PreprintStepperSelectors.getPreprintFile, value: mockPreprintFile }, + { selector: PreprintStepperSelectors.isPreprintSubmitting, value: false }, + { selector: PreprintStepperSelectors.getPreprintLicense, value: MOCK_LICENSE }, + { selector: PreprintStepperSelectors.getPreprintProject, value: { id: 'project-id', name: 'Test Project' } }, + { selector: ContributorsSelectors.getBibliographicContributors, value: [MOCK_CONTRIBUTOR] }, + { selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false }, + { selector: ContributorsSelectors.hasMoreBibliographicContributors, value: false }, + { selector: SubjectsSelectors.getSelectedSubjects, value: SUBJECTS_MOCK }, + { selector: InstitutionsSelectors.getResourceInstitutions, value: [MOCK_INSTITUTION] }, + ]; + + function setup(overrides?: { + selectorOverrides?: SignalOverride[]; + provider?: PreprintProviderDetails; + detectChanges?: boolean; + }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + routerMock = RouterMock.create().build(); + toastMock = ToastServiceMock.simple(); + + TestBed.configureTestingModule({ imports: [ ReviewStepComponent, - OSFTestingModule, ...MockComponents(AffiliatedInstitutionsViewComponent, ContributorsListComponent, LicenseDisplayComponent), MockPipe(InterpolatePipe), ], providers: [ - { provide: Router, useValue: RouterMock.create().build() }, - { provide: ToastService, useValue: ToastServiceMock.simple() }, - provideMockStore({ - signals: [ - { selector: PreprintStepperSelectors.getPreprint, value: mockPreprint }, - { selector: PreprintStepperSelectors.getPreprintFile, value: mockPreprintFile }, - { selector: PreprintStepperSelectors.isPreprintSubmitting, value: false }, - { selector: PreprintStepperSelectors.getPreprintLicense, value: mockLicense }, - { selector: PreprintStepperSelectors.getPreprintProject, value: mockPreprintProject }, - { selector: ContributorsSelectors.getBibliographicContributors, value: mockContributors }, - { selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false }, - { selector: ContributorsSelectors.hasMoreBibliographicContributors, value: false }, - { selector: SubjectsSelectors.getSelectedSubjects, value: mockSubjects }, - { selector: InstitutionsSelectors.getResourceInstitutions, value: mockInstitutions }, - ], - }), + provideOSFCore(), + MockProvider(Router, routerMock), + MockProvider(ToastService, toastMock), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(ReviewStepComponent); component = fixture.componentInstance; - router = TestBed.inject(Router) as jest.Mocked; + fixture.componentRef.setInput('provider', overrides && 'provider' in overrides ? overrides.provider : mockProvider); + if (overrides?.detectChanges ?? true) { + fixture.detectChanges(); + } + } + + it('should dispatch initial fetch actions when preprint exists', () => { + setup(); + + expect(store.dispatch).toHaveBeenCalledWith(new FetchLicenses(mockProvider.id)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintProject()); + expect(store.dispatch).toHaveBeenCalledWith( + new GetBibliographicContributors(mockPreprint.id, ResourceType.Preprint) + ); + expect(store.dispatch).toHaveBeenCalledWith(new FetchSelectedSubjects(mockPreprint.id, ResourceType.Preprint)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchResourceInstitutions(mockPreprint.id, ResourceType.Preprint)); + }); + + it('should skip preprint-dependent fetches when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + }); - fixture.componentRef.setInput('provider', mockProvider); + expect(store.dispatch).toHaveBeenCalledWith(new FetchLicenses(mockProvider.id)); + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintProject()); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(GetBibliographicContributors)); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchSelectedSubjects)); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchResourceInstitutions)); }); - it('should have required provider input', () => { - expect(component.provider()).toEqual(mockProvider); + it('should expose license options record from preprint', () => { + setup(); + expect(component.licenseOptionsRecord()).toEqual(mockPreprint.licenseOptions ?? {}); }); - it('should create license options record', () => { - const licenseOptionsRecord = component.licenseOptionsRecord(); - expect(licenseOptionsRecord).toEqual({ copyrightHolders: 'John Doe', year: '2023' }); + it('should expose empty license options record when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + detectChanges: false, + }); + expect(component.licenseOptionsRecord()).toEqual({}); }); - it('should handle cancelSubmission method', () => { + it('should navigate to preprints list on cancel', () => { + setup(); + component.cancelSubmission(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/preprints'); + + expect(routerMock.navigateByUrl).toHaveBeenCalledWith('/preprints'); + }); + + it('should return early in submitPreprint when required data is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + detectChanges: false, + }); + + component.submitPreprint(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePrimaryFileRelationship)); + expect(toastMock.showSuccess).not.toHaveBeenCalled(); + expect(routerMock.navigate).not.toHaveBeenCalled(); + }); + + it('should return early in submitPreprint when no file id is available', () => { + const preprintWithoutPrimaryFileId = { ...mockPreprint, primaryFileId: null }; + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getPreprint, value: preprintWithoutPrimaryFileId }, + { selector: PreprintStepperSelectors.getPreprintFile, value: null }, + ], + }); + + component.submitPreprint(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePrimaryFileRelationship)); + expect(toastMock.showSuccess).not.toHaveBeenCalled(); + expect(routerMock.navigate).not.toHaveBeenCalled(); + }); + + it('should publish directly when provider has no reviews workflow', () => { + setup({ + provider: { ...mockProvider, reviewsWorkflow: null }, + }); + + component.submitPreprint(); + + expect(store.dispatch).toHaveBeenCalledWith(new UpdatePrimaryFileRelationship(mockPreprintFile.id)); + expect(store.dispatch).toHaveBeenCalledWith(new UpdatePreprint(mockPreprint.id, { isPublished: true })); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.common.successMessages.preprintSubmitted' + ); + expect(routerMock.navigate).toHaveBeenCalledWith(['/preprints', mockProvider.id, mockPreprint.id]); + }); + + it('should submit preprint when workflow exists and reviews state is not accepted', () => { + const preprintPending = { ...mockPreprint, reviewsState: ReviewsState.Pending }; + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: preprintPending }], + }); + + component.submitPreprint(); + + expect(store.dispatch).toHaveBeenCalledWith(new UpdatePrimaryFileRelationship(mockPreprintFile.id)); + expect(store.dispatch).toHaveBeenCalledWith(new SubmitPreprint()); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.common.successMessages.preprintSubmitted' + ); + expect(routerMock.navigate).toHaveBeenCalledWith(['/preprints', mockProvider.id, mockPreprint.id]); }); - it('should handle submitting state', () => { - expect(component.isPreprintSubmitting()).toBe(false); + it('should skip submit/update when reviews state is accepted and still navigate', () => { + const preprintAccepted = { ...mockPreprint, reviewsState: ReviewsState.Accepted }; + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: preprintAccepted }], + }); + + component.submitPreprint(); + + expect(store.dispatch).toHaveBeenCalledWith(new UpdatePrimaryFileRelationship(mockPreprintFile.id)); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(SubmitPreprint)); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(UpdatePreprint)); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.common.successMessages.preprintSubmitted' + ); + expect(routerMock.navigate).toHaveBeenCalledWith(['/preprints', mockProvider.id, mockPreprint.id]); }); - it('should have proper method signatures', () => { - expect(typeof component.submitPreprint).toBe('function'); - expect(typeof component.cancelSubmission).toBe('function'); + it('should load more contributors when preprint id exists', () => { + setup(); + + component.loadMoreContributors(); + + expect(store.dispatch).toHaveBeenCalledWith( + new LoadMoreBibliographicContributors(mockPreprint.id, ResourceType.Preprint) + ); + }); + + it('should load more contributors with undefined id when preprint is missing', () => { + setup({ + selectorOverrides: [{ selector: PreprintStepperSelectors.getPreprint, value: null }], + detectChanges: false, + }); + + component.loadMoreContributors(); + + expect(store.dispatch).toHaveBeenCalledWith( + new LoadMoreBibliographicContributors(undefined, ResourceType.Preprint) + ); }); }); diff --git a/src/app/features/preprints/components/stepper/review-step/review-step.component.ts b/src/app/features/preprints/components/stepper/review-step/review-step.component.ts index 99a0375d4..5a37192fb 100644 --- a/src/app/features/preprints/components/stepper/review-step/review-step.component.ts +++ b/src/app/features/preprints/components/stepper/review-step/review-step.component.ts @@ -26,7 +26,6 @@ import { AffiliatedInstitutionsViewComponent } from '@osf/shared/components/affi import { ContributorsListComponent } from '@osf/shared/components/contributors-list/contributors-list.component'; import { LicenseDisplayComponent } from '@osf/shared/components/license-display/license-display.component'; import { TruncatedTextComponent } from '@osf/shared/components/truncated-text/truncated-text.component'; -import { FixSpecialCharPipe } from '@osf/shared/pipes/fix-special-char.pipe'; import { ToastService } from '@osf/shared/services/toast.service'; import { ResourceType } from '@shared/enums/resource-type.enum'; import { @@ -40,26 +39,28 @@ import { FetchSelectedSubjects, SubjectsSelectors } from '@shared/stores/subject @Component({ selector: 'osf-review-step', imports: [ + Button, Card, - TruncatedTextComponent, Tag, - DatePipe, - Button, - TitleCasePipe, - TranslatePipe, AffiliatedInstitutionsViewComponent, ContributorsListComponent, LicenseDisplayComponent, - FixSpecialCharPipe, + TruncatedTextComponent, + DatePipe, + TitleCasePipe, + TranslatePipe, ], templateUrl: './review-step.component.html', styleUrl: './review-step.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class ReviewStepComponent implements OnInit { - private router = inject(Router); - private toastService = inject(ToastService); - private actions = createDispatchMap({ + private readonly router = inject(Router); + private readonly toastService = inject(ToastService); + + readonly provider = input.required(); + + private readonly actions = createDispatchMap({ getBibliographicContributors: GetBibliographicContributors, fetchSubjects: FetchSelectedSubjects, fetchLicenses: FetchLicenses, @@ -71,41 +72,55 @@ export class ReviewStepComponent implements OnInit { loadMoreBibliographicContributors: LoadMoreBibliographicContributors, }); - provider = input.required(); + readonly preprint = select(PreprintStepperSelectors.getPreprint); + readonly preprintFile = select(PreprintStepperSelectors.getPreprintFile); + readonly isPreprintSubmitting = select(PreprintStepperSelectors.isPreprintSubmitting); - preprint = select(PreprintStepperSelectors.getPreprint); - preprintFile = select(PreprintStepperSelectors.getPreprintFile); - isPreprintSubmitting = select(PreprintStepperSelectors.isPreprintSubmitting); - - bibliographicContributors = select(ContributorsSelectors.getBibliographicContributors); - areContributorsLoading = select(ContributorsSelectors.isBibliographicContributorsLoading); - hasMoreBibliographicContributors = select(ContributorsSelectors.hasMoreBibliographicContributors); - subjects = select(SubjectsSelectors.getSelectedSubjects); - affiliatedInstitutions = select(InstitutionsSelectors.getResourceInstitutions); - license = select(PreprintStepperSelectors.getPreprintLicense); - preprintProject = select(PreprintStepperSelectors.getPreprintProject); - licenseOptionsRecord = computed(() => (this.preprint()?.licenseOptions ?? {}) as Record); + readonly bibliographicContributors = select(ContributorsSelectors.getBibliographicContributors); + readonly areContributorsLoading = select(ContributorsSelectors.isBibliographicContributorsLoading); + readonly hasMoreBibliographicContributors = select(ContributorsSelectors.hasMoreBibliographicContributors); + readonly subjects = select(SubjectsSelectors.getSelectedSubjects); + readonly affiliatedInstitutions = select(InstitutionsSelectors.getResourceInstitutions); + readonly license = select(PreprintStepperSelectors.getPreprintLicense); + readonly preprintProject = select(PreprintStepperSelectors.getPreprintProject); + readonly licenseOptionsRecord = computed(() => (this.preprint()?.licenseOptions ?? {}) as Record); readonly ApplicabilityStatus = ApplicabilityStatus; readonly PreregLinkInfo = PreregLinkInfo; ngOnInit(): void { - this.actions.getBibliographicContributors(this.preprint()?.id, ResourceType.Preprint); - this.actions.fetchSubjects(this.preprint()!.id, ResourceType.Preprint); - this.actions.fetchLicenses(); + this.actions.fetchLicenses(this.provider().id); this.actions.fetchPreprintProject(); - this.actions.fetchResourceInstitutions(this.preprint()!.id, ResourceType.Preprint); + + const preprintId = this.preprint()?.id; + if (!preprintId) { + return; + } + + this.actions.getBibliographicContributors(preprintId, ResourceType.Preprint); + this.actions.fetchSubjects(preprintId, ResourceType.Preprint); + this.actions.fetchResourceInstitutions(preprintId, ResourceType.Preprint); } - submitPreprint() { - const preprint = this.preprint()!; - const preprintFile = this.preprintFile()!; + submitPreprint(): void { + const preprint = this.preprint(); + const provider = this.provider(); + + if (!preprint) { + return; + } + + const preprintFileId = this.preprintFile()?.id ?? preprint.primaryFileId; + + if (!preprintFileId) { + return; + } this.actions - .updatePrimaryFileRelationship(preprintFile?.id ?? preprint.primaryFileId) + .updatePrimaryFileRelationship(preprintFileId) .pipe( switchMap(() => { - if (!this.provider()?.reviewsWorkflow) { + if (!provider.reviewsWorkflow) { return this.actions.updatePreprint(preprint.id, { isPublished: true }); } @@ -116,13 +131,13 @@ export class ReviewStepComponent implements OnInit { }), tap(() => { this.toastService.showSuccess('preprints.preprintStepper.common.successMessages.preprintSubmitted'); - this.router.navigate(['/preprints', this.provider()!.id, preprint.id]); + this.router.navigate(['/preprints', provider.id, preprint.id]); }) ) .subscribe(); } - cancelSubmission() { + cancelSubmission(): void { this.router.navigateByUrl('/preprints'); } diff --git a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.html b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.html index fb3d8b8c9..f90168727 100644 --- a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.html +++ b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.html @@ -15,7 +15,7 @@

{{ 'preprints.preprintStepper.supplements.title' | translate }}

styleClass="w-full" [label]="'preprints.preprintStepper.supplements.options.connectExisting' | translate" severity="secondary" - (click)="selectSupplementOption(SupplementOptions.ConnectExistingProject)" + (onClick)="selectSupplementOption(SupplementOptions.ConnectExistingProject)" /> @@ -33,26 +33,26 @@

{{ 'preprints.preprintStepper.supplements.title' | translate }}

- {{ 'preprints.preprintStepper.supplements.projectSelection.description' | translate }} + {{ 'preprints.preprintStepper.projectSelection.description' | translate }}

- {{ 'preprints.preprintStepper.supplements.projectSelection.subDescription' | translate }} + {{ 'preprints.preprintStepper.projectSelection.subDescription' | translate }}

@@ -88,7 +88,7 @@

{{ 'preprints.preprintStepper.supplements.title' | translate }}

styleClass="w-full" [label]="'common.buttons.back' | translate" severity="info" - (click)="backButtonClicked()" + (onClick)="backButtonClicked()" /> {{ 'preprints.preprintStepper.supplements.title' | translate }}

[label]="'common.buttons.next' | translate" [disabled]="isNextButtonDisabled()" [loading]="isPreprintSubmitting()" - (click)="nextButtonClicked()" + (onClick)="nextButtonClicked()" /> diff --git a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.spec.ts b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.spec.ts index 8f7123c13..554477a13 100644 --- a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.spec.ts @@ -1,125 +1,344 @@ -import { MockComponent, MockProvider } from 'ng-mocks'; +import { Store } from '@ngxs/store'; -import { ConfirmationService } from 'primeng/api'; +import { MockComponent, MockProvider } from 'ng-mocks'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; +import { SupplementOptions } from '@osf/features/preprints/enums'; +import { + ConnectProject, + CreateNewProject, + DisconnectProject, + FetchAvailableProjects, + FetchPreprintProject, + PreprintStepperSelectors, +} from '@osf/features/preprints/store/preprint-stepper'; import { AddProjectFormComponent } from '@osf/shared/components/add-project-form/add-project-form.component'; +import { ProjectFormControls } from '@osf/shared/enums/create-project-form-controls.enum'; +import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { ToastService } from '@osf/shared/services/toast.service'; import { SupplementsStepComponent } from './supplements-step.component'; -import { TranslateServiceMock } from '@testing/mocks/translate.service.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; +import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { + CustomConfirmationServiceMock, + CustomConfirmationServiceMockType, +} from '@testing/providers/custom-confirmation-provider.mock'; +import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; describe('SupplementsStepComponent', () => { let component: SupplementsStepComponent; let fixture: ComponentFixture; - let mockToastService: ReturnType; + let store: Store; + let toastMock: ToastServiceMockType; + let confirmationMock: CustomConfirmationServiceMockType; + const originalPointerEvent = (globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent; + + const defaultSignals: SignalOverride[] = [ + { selector: PreprintStepperSelectors.getPreprint, value: { ...PREPRINT_MOCK, nodeId: null } }, + { selector: PreprintStepperSelectors.isPreprintSubmitting, value: false }, + { selector: PreprintStepperSelectors.getAvailableProjects, value: [] }, + { selector: PreprintStepperSelectors.areAvailableProjectsLoading, value: false }, + { selector: PreprintStepperSelectors.getPreprintProject, value: null }, + { selector: PreprintStepperSelectors.isPreprintProjectLoading, value: false }, + ]; - beforeEach(async () => { - mockToastService = ToastServiceMock.simple(); + function setup(overrides?: { selectorOverrides?: SignalOverride[]; detectChanges?: boolean }) { + const signals = mergeSignalOverrides(defaultSignals, overrides?.selectorOverrides); + toastMock = ToastServiceMock.simple(); + confirmationMock = CustomConfirmationServiceMock.simple(); - await TestBed.configureTestingModule({ - imports: [SupplementsStepComponent, MockComponent(AddProjectFormComponent), OSFTestingModule], + TestBed.configureTestingModule({ + imports: [SupplementsStepComponent, MockComponent(AddProjectFormComponent)], providers: [ - provideMockStore({ - signals: [ - { - selector: PreprintStepperSelectors.getPreprint, - value: {}, - }, - { - selector: PreprintStepperSelectors.isPreprintSubmitting, - value: false, - }, - { - selector: PreprintStepperSelectors.getAvailableProjects, - value: [], - }, - { - selector: PreprintStepperSelectors.areAvailableProjectsLoading, - value: false, - }, - { - selector: PreprintStepperSelectors.getPreprintProject, - value: null, - }, - { - selector: PreprintStepperSelectors.isPreprintProjectLoading, - value: false, - }, - ], - }), - TranslateServiceMock, - MockProvider(ConfirmationService, { - confirm: jest.fn(), - close: jest.fn(), - }), - { provide: ToastService, useValue: mockToastService }, + provideOSFCore(), + MockProvider(ToastService, toastMock), + MockProvider(CustomConfirmationService, confirmationMock), + provideMockStore({ signals }), ], - }).compileComponents(); + }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(SupplementsStepComponent); component = fixture.componentInstance; - fixture.detectChanges(); + if (overrides?.detectChanges ?? true) { + fixture.detectChanges(); + } + } + + beforeAll(() => { + if (!(globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent) { + (globalThis as unknown as { PointerEvent: typeof Event }).PointerEvent = MouseEvent as unknown as typeof Event; + } + }); + + afterAll(() => { + if (originalPointerEvent) { + (globalThis as unknown as { PointerEvent: typeof Event }).PointerEvent = originalPointerEvent; + } else { + delete (globalThis as unknown as { PointerEvent?: typeof Event }).PointerEvent; + } }); it('should create', () => { + setup(); expect(component).toBeTruthy(); }); - it('should call getAvailableProjects when project name changes after debounce', () => { - jest.useFakeTimers(); + it('should fetch preprint project in constructor effect when node id exists and differs from selected project', () => { + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getPreprint, value: { ...PREPRINT_MOCK, nodeId: 'node-1' } }, + { selector: PreprintStepperSelectors.getPreprintProject, value: { id: 'project-1', name: 'Test Project' } }, + ], + }); - const getAvailableProjectsSpy = jest.fn(); - Object.defineProperty(component, 'actions', { - value: { getAvailableProjects: getAvailableProjectsSpy }, - writable: true, + expect(store.dispatch).toHaveBeenCalledWith(new FetchPreprintProject()); + }); + + it('should skip preprint project fetch when node id matches selected project id', () => { + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getPreprint, value: { ...PREPRINT_MOCK, nodeId: 'node-1' } }, + { selector: PreprintStepperSelectors.getPreprintProject, value: { id: 'node-1', name: 'Node Project' } }, + ], }); - component.ngOnInit(); - component.projectNameControl.setValue('test-project'); - jest.advanceTimersByTime(500); + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchPreprintProject)); + }); - expect(getAvailableProjectsSpy).toHaveBeenCalledWith('test-project'); - jest.useRealTimers(); + it('should skip preprint project fetch when node id is absent', () => { + setup({ + selectorOverrides: [ + { selector: PreprintStepperSelectors.getPreprint, value: { ...PREPRINT_MOCK, nodeId: null } }, + ], + }); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(FetchPreprintProject)); }); - it('should not call getAvailableProjects if value is the same as selectedProjectId', () => { - jest.useFakeTimers(); - const getAvailableProjectsSpy = jest.fn(); + it('should dispatch available projects from debounced project search', fakeAsync(() => { + setup(); + (store.dispatch as jest.Mock).mockClear(); + + component.projectNameControl.setValue('search-query'); + tick(500); + + expect(store.dispatch).toHaveBeenCalledTimes(1); + expect(store.dispatch).toHaveBeenCalledWith(new FetchAvailableProjects('search-query')); + })); + + it('should not dispatch before the debounce window elapses', fakeAsync(() => { + setup(); + (store.dispatch as jest.Mock).mockClear(); - Object.defineProperty(component, 'actions', { - value: { getAvailableProjects: getAvailableProjectsSpy }, - writable: true, + component.projectNameControl.setValue('search-query'); + tick(300); + + expect(store.dispatch).not.toHaveBeenCalledWith(new FetchAvailableProjects('search-query')); + tick(200); + })); + + it('should skip available projects dispatch when value equals selected project id', fakeAsync(() => { + setup(); + (store.dispatch as jest.Mock).mockClear(); + component.selectedProjectId.set('project-1'); + + component.projectNameControl.setValue('project-1'); + tick(500); + + expect(store.dispatch).not.toHaveBeenCalledWith(new FetchAvailableProjects('project-1')); + })); + + it('should select supplement option and reset create form for create-new option', () => { + setup({ detectChanges: false }); + component.createProjectForm.patchValue({ + [ProjectFormControls.Title]: 'Project', + [ProjectFormControls.StorageLocation]: 'region-1', }); - jest.spyOn(component, 'selectedProjectId').mockReturnValue('test-project'); - component.ngOnInit(); - component.projectNameControl.setValue('test-project'); - jest.advanceTimersByTime(500); + component.selectSupplementOption(SupplementOptions.CreateNewProject); - expect(getAvailableProjectsSpy).not.toHaveBeenCalled(); - jest.useRealTimers(); + expect(component.selectedSupplementOption()).toBe(SupplementOptions.CreateNewProject); + expect(component.createProjectForm.controls.title.value).toBe(''); + expect(component.createProjectForm.controls.storageLocation.value).toBe(''); + expect(store.dispatch).toHaveBeenCalledWith(new FetchAvailableProjects(null)); }); - it('should handle empty values', () => { - jest.useFakeTimers(); - const getAvailableProjectsSpy = jest.fn(); - Object.defineProperty(component, 'actions', { - value: { getAvailableProjects: getAvailableProjectsSpy }, - writable: true, + it('should select supplement option without resetting form for connect-existing option', () => { + setup({ detectChanges: false }); + component.createProjectForm.patchValue({ [ProjectFormControls.Title]: 'Keep me' }); + + component.selectSupplementOption(SupplementOptions.ConnectExistingProject); + + expect(component.selectedSupplementOption()).toBe(SupplementOptions.ConnectExistingProject); + expect(component.createProjectForm.controls.title.value).toBe('Keep me'); + }); + + it('should dispatch connect project and show success toast on project selection', () => { + setup({ detectChanges: false }); + + component.selectProject({ + value: 'project-1', + originalEvent: new PointerEvent('click'), + } as never); + + expect(component.selectedProjectId()).toBe('project-1'); + expect(store.dispatch).toHaveBeenCalledWith(new ConnectProject('project-1')); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.supplements.successMessages.projectConnected' + ); + }); + + it('should return early in selectProject when event is not pointer event', () => { + setup({ detectChanges: false }); + + component.selectProject({ + value: 'project-1', + originalEvent: new Event('change'), + } as never); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(ConnectProject)); + expect(toastMock.showSuccess).not.toHaveBeenCalled(); + }); + + it('should disconnect project on confirmation and show success toast', () => { + setup({ detectChanges: false }); + component.selectedProjectId.set('project-1'); + + component.disconnectProject(); + + expect(confirmationMock.confirmDelete).toHaveBeenCalledWith({ + headerKey: 'preprints.preprintStepper.supplements.disconnectProject.header', + messageKey: 'preprints.preprintStepper.supplements.disconnectProject.message', + onConfirm: expect.any(Function), }); - component.ngOnInit(); - component.projectNameControl.setValue(''); - jest.advanceTimersByTime(500); + const { onConfirm } = confirmationMock.confirmDelete.mock.calls[0][0]; + onConfirm(); - expect(getAvailableProjectsSpy).toHaveBeenCalledWith(''); - jest.useRealTimers(); + expect(store.dispatch).toHaveBeenCalledWith(new DisconnectProject()); + expect(component.selectedProjectId()).toBeNull(); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.supplements.successMessages.projectDisconnected' + ); + }); + + it('should not dispatch disconnect or clear selection when disconnect is cancelled', () => { + setup({ detectChanges: false }); + component.selectedProjectId.set('project-1'); + + component.disconnectProject(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(DisconnectProject)); + expect(component.selectedProjectId()).toBe('project-1'); + expect(toastMock.showSuccess).not.toHaveBeenCalled(); + }); + + it('should return early when create project form is invalid', () => { + setup({ detectChanges: false }); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); + + component.submitCreateProjectForm(); + + expect(store.dispatch).not.toHaveBeenCalledWith(expect.any(CreateNewProject)); + expect(emitSpy).not.toHaveBeenCalled(); + }); + + it('should create project, show success and emit next when form is valid', () => { + setup({ detectChanges: false }); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); + component.createProjectForm.patchValue({ + [ProjectFormControls.Title]: 'New Project', + [ProjectFormControls.StorageLocation]: 'region-1', + [ProjectFormControls.Affiliations]: ['inst-1'], + [ProjectFormControls.Description]: 'Description', + [ProjectFormControls.Template]: 'template-id', + }); + + component.submitCreateProjectForm(); + + expect(store.dispatch).toHaveBeenCalledWith( + new CreateNewProject('New Project', 'Description', 'template-id', 'region-1', ['inst-1']) + ); + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.supplements.successMessages.projectCreated' + ); + expect(emitSpy).toHaveBeenCalled(); + }); + + it('should submit create project form path in nextButtonClicked for create-new option', () => { + setup({ detectChanges: false }); + const createSpy = jest.spyOn(component, 'submitCreateProjectForm'); + component.selectedSupplementOption.set(SupplementOptions.CreateNewProject); + + component.nextButtonClicked(); + + expect(createSpy).toHaveBeenCalled(); + }); + + it('should emit next and show saved toast in nextButtonClicked for non-create option', () => { + setup({ detectChanges: false }); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); + component.selectedSupplementOption.set(SupplementOptions.ConnectExistingProject); + + component.nextButtonClicked(); + + expect(toastMock.showSuccess).toHaveBeenCalledWith( + 'preprints.preprintStepper.common.successMessages.preprintSaved' + ); + expect(emitSpy).toHaveBeenCalled(); + }); + + it('should handle discard-changes confirmation callbacks in backButtonClicked', () => { + setup({ detectChanges: false }); + const emitSpy = jest.spyOn(component.backClicked, 'emit'); + component.selectedSupplementOption.set(SupplementOptions.CreateNewProject); + component.createProjectForm.patchValue({ [ProjectFormControls.Title]: 'Has data' }); + + component.backButtonClicked(); + + expect(confirmationMock.confirmContinue).toHaveBeenCalledWith({ + headerKey: 'preprints.preprintStepper.supplements.discardChanges.header', + messageKey: 'preprints.preprintStepper.supplements.discardChanges.message', + onConfirm: expect.any(Function), + onReject: expect.any(Function), + }); + + const { onReject } = confirmationMock.confirmContinue.mock.calls[0][0]; + onReject(); + expect(emitSpy).not.toHaveBeenCalled(); + + const { onConfirm } = confirmationMock.confirmContinue.mock.calls[0][0]; + onConfirm(); + + expect(emitSpy).toHaveBeenCalledTimes(1); + }); + + it('should emit back immediately in backButtonClicked when no create form data', () => { + setup({ detectChanges: false }); + const emitSpy = jest.spyOn(component.backClicked, 'emit'); + + component.backButtonClicked(); + + expect(emitSpy).toHaveBeenCalled(); + expect(confirmationMock.confirmContinue).not.toHaveBeenCalled(); + }); + + it('should compute next button disabled state for create and connect options', () => { + setup({ detectChanges: false }); + component.selectedSupplementOption.set(SupplementOptions.CreateNewProject); + expect(component.isNextButtonDisabled()).toBe(true); + + component.createProjectForm.patchValue({ + [ProjectFormControls.Title]: 'Valid title', + [ProjectFormControls.StorageLocation]: 'region-1', + }); + expect(component.isNextButtonDisabled()).toBe(false); + component.selectedSupplementOption.set(SupplementOptions.ConnectExistingProject); + expect(component.isNextButtonDisabled()).toBe(false); }); }); diff --git a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.ts b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.ts index c4e58c40b..04fbb95c3 100644 --- a/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.ts +++ b/src/app/features/preprints/components/stepper/supplements-step/supplements-step.component.ts @@ -61,29 +61,33 @@ import { ProjectForm } from '@shared/models/projects/create-project-form.model'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class SupplementsStepComponent implements OnInit { - private customConfirmationService = inject(CustomConfirmationService); + private readonly customConfirmationService = inject(CustomConfirmationService); private readonly toastService = inject(ToastService); - private actions = createDispatchMap({ + private readonly destroyRef = inject(DestroyRef); + + private readonly actions = createDispatchMap({ getAvailableProjects: FetchAvailableProjects, connectProject: ConnectProject, disconnectProject: DisconnectProject, fetchPreprintProject: FetchPreprintProject, createNewProject: CreateNewProject, }); - private destroyRef = inject(DestroyRef); - - readonly SupplementOptions = SupplementOptions; - createdPreprint = select(PreprintStepperSelectors.getPreprint); - isPreprintSubmitting = select(PreprintStepperSelectors.isPreprintSubmitting); - availableProjects = select(PreprintStepperSelectors.getAvailableProjects); - areAvailableProjectsLoading = select(PreprintStepperSelectors.areAvailableProjectsLoading); - preprintProject = select(PreprintStepperSelectors.getPreprintProject); - isPreprintProjectLoading = select(PreprintStepperSelectors.isPreprintProjectLoading); + readonly createdPreprint = select(PreprintStepperSelectors.getPreprint); + readonly isPreprintSubmitting = select(PreprintStepperSelectors.isPreprintSubmitting); + readonly availableProjects = select(PreprintStepperSelectors.getAvailableProjects); + readonly areAvailableProjectsLoading = select(PreprintStepperSelectors.areAvailableProjectsLoading); + readonly preprintProject = select(PreprintStepperSelectors.getPreprintProject); + readonly isPreprintProjectLoading = select(PreprintStepperSelectors.isPreprintProjectLoading); selectedSupplementOption = signal(SupplementOptions.None); selectedProjectId = signal(null); + nextClicked = output(); + backClicked = output(); + + readonly SupplementOptions = SupplementOptions; + readonly projectNameControl = new FormControl(null); readonly createProjectForm = new FormGroup({ [ProjectFormControls.Title]: new FormControl('', { @@ -124,20 +128,16 @@ export class SupplementsStepComponent implements OnInit { return; } - untracked(() => { - const preprintProject = this.preprintProject(); - if (preprint.nodeId === preprintProject?.id) { - return; - } - }); + const shouldFetchPreprintProject = untracked(() => preprint.nodeId !== this.preprintProject()?.id); + + if (!shouldFetchPreprintProject) { + return; + } this.actions.fetchPreprintProject(); }); } - nextClicked = output(); - backClicked = output(); - ngOnInit() { this.projectNameControl.valueChanges .pipe(debounceTime(500), distinctUntilChanged(), takeUntilDestroyed(this.destroyRef)) @@ -164,6 +164,7 @@ export class SupplementsStepComponent implements OnInit { if (!(event.originalEvent instanceof PointerEvent)) { return; } + this.selectedProjectId.set(event.value); this.actions.connectProject(event.value).subscribe({ diff --git a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.html b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.html index 01bd85ba5..9091466b8 100644 --- a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.html +++ b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.html @@ -64,6 +64,6 @@

{{ 'preprints.preprintStepper.titleAndAbstract.title' | translate }}

tooltipPosition="top" [disabled]="titleAndAbstractForm.invalid" [loading]="isUpdatingPreprint()" - (click)="nextButtonClicked()" + (onClick)="nextButtonClicked()" /> diff --git a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.spec.ts b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.spec.ts index a972d6439..95edb3149 100644 --- a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.spec.ts +++ b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.spec.ts @@ -1,14 +1,20 @@ -import { MockComponent } from 'ng-mocks'; +import { MockComponent, MockDirective, MockProvider } from 'ng-mocks'; + +import { Textarea } from 'primeng/textarea'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ActivatedRoute } from '@angular/router'; import { TitleAndAbstractStepComponent } from '@osf/features/preprints/components'; import { PreprintStepperSelectors } from '@osf/features/preprints/store/preprint-stepper'; import { TextInputComponent } from '@osf/shared/components/text-input/text-input.component'; +import { ToastService } from '@osf/shared/services/toast.service'; import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; describe('TitleAndAbstractStepComponent', () => { let component: TitleAndAbstractStepComponent; @@ -16,19 +22,20 @@ describe('TitleAndAbstractStepComponent', () => { const mockPreprint = PREPRINT_MOCK; - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [TitleAndAbstractStepComponent, OSFTestingModule, MockComponent(TextInputComponent)], + function setup(overrides?: { createdPreprint?: typeof mockPreprint | null; providerId?: string }) { + const mockToastService = ToastServiceMock.simple(); + + TestBed.configureTestingModule({ + imports: [TitleAndAbstractStepComponent, MockComponent(TextInputComponent), MockDirective(Textarea)], providers: [ + provideOSFCore(), + MockProvider(ActivatedRoute, ActivatedRouteMockBuilder.create().build()), + MockProvider(ToastService, mockToastService), provideMockStore({ signals: [ { selector: PreprintStepperSelectors.getPreprint, - value: null, - }, - { - selector: PreprintStepperSelectors.getSelectedProviderId, - value: 'provider-1', + value: overrides && 'createdPreprint' in overrides ? overrides.createdPreprint : null, }, { selector: PreprintStepperSelectors.isPreprintSubmitting, @@ -37,137 +44,78 @@ describe('TitleAndAbstractStepComponent', () => { ], }), ], - }).compileComponents(); + }); fixture = TestBed.createComponent(TitleAndAbstractStepComponent); component = fixture.componentInstance; - }); - - it('should create', () => { - expect(component).toBeTruthy(); - }); - - it('should initialize form with empty values', () => { - expect(component.titleAndAbstractForm.get('title')?.value).toBe(''); - expect(component.titleAndAbstractForm.get('description')?.value).toBe(''); - }); - - it('should have form invalid when fields are empty', () => { - expect(component.titleAndAbstractForm.invalid).toBe(true); - }); - - it('should have form valid when fields are filled correctly', () => { + fixture.componentRef.setInput( + 'providerId', + overrides && 'providerId' in overrides ? overrides.providerId : 'provider-1' + ); + fixture.detectChanges(); + } + + function fillValidForm() { component.titleAndAbstractForm.patchValue({ title: 'Valid Title', description: 'Valid description with sufficient length', }); - expect(component.titleAndAbstractForm.valid).toBe(true); - }); + } - it('should validate title max length', () => { - const longTitle = 'a'.repeat(513); - component.titleAndAbstractForm.patchValue({ - title: longTitle, - description: 'Valid description', - }); - expect(component.titleAndAbstractForm.get('title')?.hasError('maxlength')).toBe(true); - }); - - it('should validate description is required', () => { - component.titleAndAbstractForm.patchValue({ - title: 'Valid Title', - description: '', - }); - expect(component.titleAndAbstractForm.get('description')?.hasError('required')).toBe(true); - }); + it('should initialize form with empty values', () => { + setup(); - it('should not proceed when form is invalid', () => { - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); - component.nextButtonClicked(); - expect(nextClickedSpy).not.toHaveBeenCalled(); + expect(component.titleAndAbstractForm.controls.title.value).toBe(''); + expect(component.titleAndAbstractForm.controls.description.value).toBe(''); + expect(component.titleAndAbstractForm.invalid).toBe(true); }); - it('should emit nextClicked when form is valid and no existing preprint', () => { - component.titleAndAbstractForm.patchValue({ - title: 'Valid Title', - description: 'Valid description with sufficient length', - }); + it('should enforce title and description validation', () => { + setup(); - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); - component.nextButtonClicked(); - expect(nextClickedSpy).toHaveBeenCalled(); - }); + component.titleAndAbstractForm.patchValue({ title: 'a'.repeat(513), description: 'Valid description' }); + expect(component.titleAndAbstractForm.controls.title.hasError('maxlength')).toBe(true); - it('should initialize form with existing preprint data', () => { - component.titleAndAbstractForm.patchValue({ - title: mockPreprint.title, - description: mockPreprint.description, - }); - expect(component.titleAndAbstractForm.get('title')?.value).toBe(mockPreprint.title); - expect(component.titleAndAbstractForm.get('description')?.value).toBe(mockPreprint.description); - }); + component.titleAndAbstractForm.patchValue({ title: 'Valid title', description: 'Short' }); + expect(component.titleAndAbstractForm.controls.description.hasError('minlength')).toBe(true); - it('should emit nextClicked when form is valid and preprint exists', () => { - component.titleAndAbstractForm.patchValue({ - title: mockPreprint.title, - description: mockPreprint.description, - }); - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); - component.nextButtonClicked(); - expect(nextClickedSpy).toHaveBeenCalled(); + component.titleAndAbstractForm.patchValue({ title: 'Valid title', description: '' }); + expect(component.titleAndAbstractForm.controls.description.hasError('required')).toBe(true); }); - it('should emit nextClicked when form is valid and no existing preprint', () => { - component.titleAndAbstractForm.patchValue({ - title: 'Test Title', - description: 'Test description with sufficient length', - }); - - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); - component.nextButtonClicked(); + it('should patch form with existing preprint values', () => { + setup({ createdPreprint: mockPreprint }); - expect(nextClickedSpy).toHaveBeenCalled(); + expect(component.titleAndAbstractForm.controls.title.value).toBe(mockPreprint.title); + expect(component.titleAndAbstractForm.controls.description.value).toBe(mockPreprint.description); }); - it('should emit nextClicked when form is valid and preprint exists', () => { - jest.spyOn(component, 'createdPreprint').mockReturnValue(mockPreprint); - - component.titleAndAbstractForm.patchValue({ - title: 'Updated Title', - description: 'Updated description with sufficient length', - }); + it('should not dispatch or emit when form is invalid', () => { + setup(); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); component.nextButtonClicked(); - expect(nextClickedSpy).toHaveBeenCalled(); + expect(emitSpy).not.toHaveBeenCalled(); }); - it('should not emit nextClicked when form is invalid', () => { - const nextClickedSpy = jest.spyOn(component.nextClicked, 'emit'); + it('should create preprint and emit next when form is valid and no preprint exists', () => { + setup({ createdPreprint: null, providerId: 'provider-1' }); + fillValidForm(); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); component.nextButtonClicked(); - expect(nextClickedSpy).not.toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalled(); }); - it('should have correct form validation for title and description', () => { - component.titleAndAbstractForm.patchValue({ - title: '', - description: 'Valid description', - }); - expect(component.titleAndAbstractForm.get('title')?.hasError('required')).toBe(true); + it('should update preprint and emit next when form is valid and preprint exists', () => { + setup({ createdPreprint: mockPreprint }); + fillValidForm(); + const emitSpy = jest.spyOn(component.nextClicked, 'emit'); - component.titleAndAbstractForm.patchValue({ - title: 'Valid Title', - description: 'Short', - }); - expect(component.titleAndAbstractForm.get('description')?.hasError('minlength')).toBe(true); + component.nextButtonClicked(); - component.titleAndAbstractForm.patchValue({ - title: 'Valid Title', - description: 'Valid description with sufficient length', - }); - expect(component.titleAndAbstractForm.valid).toBe(true); + expect(emitSpy).toHaveBeenCalled(); }); }); diff --git a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.ts b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.ts index cadca4dc6..c15c59dd0 100644 --- a/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.ts +++ b/src/app/features/preprints/components/stepper/title-and-abstract-step/title-and-abstract-step.component.ts @@ -8,7 +8,7 @@ import { Message } from 'primeng/message'; import { Textarea } from 'primeng/textarea'; import { Tooltip } from 'primeng/tooltip'; -import { ChangeDetectionStrategy, Component, effect, inject, output } from '@angular/core'; +import { ChangeDetectionStrategy, Component, effect, inject, input, output } from '@angular/core'; import { FormControl, FormGroup, FormsModule, ReactiveFormsModule, Validators } from '@angular/forms'; import { RouterLink } from '@angular/router'; @@ -27,30 +27,36 @@ import { ToastService } from '@osf/shared/services/toast.service'; @Component({ selector: 'osf-title-and-abstract-step', imports: [ - Card, - FormsModule, Button, + Card, Textarea, RouterLink, - ReactiveFormsModule, Tooltip, Message, - TranslatePipe, + FormsModule, + ReactiveFormsModule, TextInputComponent, + TranslatePipe, ], templateUrl: './title-and-abstract-step.component.html', styleUrl: './title-and-abstract-step.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class TitleAndAbstractStepComponent { - private toastService = inject(ToastService); + private readonly toastService = inject(ToastService); + + readonly providerId = input.required(); + readonly nextClicked = output(); - private actions = createDispatchMap({ + private readonly actions = createDispatchMap({ createPreprint: CreatePreprint, updatePreprint: UpdatePreprint, }); - inputLimits = formInputLimits; + readonly createdPreprint = select(PreprintStepperSelectors.getPreprint); + readonly isUpdatingPreprint = select(PreprintStepperSelectors.isPreprintSubmitting); + + readonly inputLimits = formInputLimits; readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; titleAndAbstractForm = new FormGroup({ @@ -68,12 +74,6 @@ export class TitleAndAbstractStepComponent { }), }); - createdPreprint = select(PreprintStepperSelectors.getPreprint); - providerId = select(PreprintStepperSelectors.getSelectedProviderId); - - isUpdatingPreprint = select(PreprintStepperSelectors.isPreprintSubmitting); - nextClicked = output(); - constructor() { effect(() => { const createdPreprint = this.createdPreprint(); @@ -86,22 +86,23 @@ export class TitleAndAbstractStepComponent { }); } - nextButtonClicked() { + nextButtonClicked(): void { if (this.titleAndAbstractForm.invalid) { return; } - const model = this.titleAndAbstractForm.value; + const model = this.titleAndAbstractForm.getRawValue(); + const createdPreprint = this.createdPreprint(); - if (this.createdPreprint()) { - this.actions.updatePreprint(this.createdPreprint()!.id, model).subscribe({ + if (createdPreprint) { + this.actions.updatePreprint(createdPreprint.id, model).subscribe({ complete: () => { this.nextClicked.emit(); this.toastService.showSuccess('preprints.preprintStepper.common.successMessages.preprintSaved'); }, }); } else { - this.actions.createPreprint(model.title!, model.description!, this.providerId()!).subscribe({ + this.actions.createPreprint(model.title, model.description, this.providerId()).subscribe({ complete: () => { this.nextClicked.emit(); this.toastService.showSuccess('preprints.preprintStepper.common.successMessages.preprintSaved'); diff --git a/src/app/features/preprints/pages/create-new-version/create-new-version.component.html b/src/app/features/preprints/pages/create-new-version/create-new-version.component.html index cbd1794d8..42bc5e0e6 100644 --- a/src/app/features/preprints/pages/create-new-version/create-new-version.component.html +++ b/src/app/features/preprints/pages/create-new-version/create-new-version.component.html @@ -39,7 +39,7 @@

} @case (PreprintSteps.Review) { - + } } diff --git a/src/app/features/preprints/pages/create-new-version/create-new-version.component.ts b/src/app/features/preprints/pages/create-new-version/create-new-version.component.ts index 122819273..70176a373 100644 --- a/src/app/features/preprints/pages/create-new-version/create-new-version.component.ts +++ b/src/app/features/preprints/pages/create-new-version/create-new-version.component.ts @@ -31,12 +31,7 @@ import { FileStepComponent, ReviewStepComponent } from '../../components'; import { createNewVersionStepsConst } from '../../constants'; import { PreprintSteps } from '../../enums'; import { GetPreprintProviderById, PreprintProvidersSelectors } from '../../store/preprint-providers'; -import { - FetchPreprintById, - PreprintStepperSelectors, - ResetPreprintStepperState, - SetSelectedPreprintProviderId, -} from '../../store/preprint-stepper'; +import { FetchPreprintById, PreprintStepperSelectors, ResetPreprintStepperState } from '../../store/preprint-stepper'; @Component({ selector: 'osf-create-new-version', @@ -48,31 +43,31 @@ import { export class CreateNewVersionComponent implements OnDestroy, CanDeactivateComponent { @HostBinding('class') classes = 'flex-1 flex flex-column w-full'; - private route = inject(ActivatedRoute); - private router = inject(Router); - private brandService = inject(BrandService); - private headerStyleHelper = inject(HeaderStyleService); - private browserTabHelper = inject(BrowserTabService); + private readonly route = inject(ActivatedRoute); + private readonly router = inject(Router); + private readonly brandService = inject(BrandService); + private readonly headerStyleHelper = inject(HeaderStyleService); + private readonly browserTabHelper = inject(BrowserTabService); - private providerId = toSignal(this.route.params.pipe(map((params) => params['providerId']))); - private preprintId = toSignal(this.route.params.pipe(map((params) => params['preprintId']))); + private readonly providerId = toSignal(this.route.params.pipe(map((params) => params['providerId']))); + private readonly preprintId = toSignal(this.route.params.pipe(map((params) => params['preprintId']))); - private actions = createDispatchMap({ + private readonly actions = createDispatchMap({ getPreprintProviderById: GetPreprintProviderById, - setSelectedPreprintProviderId: SetSelectedPreprintProviderId, - resetState: ResetPreprintStepperState, fetchPreprint: FetchPreprintById, + resetState: ResetPreprintStepperState, }); - readonly PreprintSteps = PreprintSteps; - readonly newVersionSteps = createNewVersionStepsConst; + readonly preprintProvider = select(PreprintProvidersSelectors.getPreprintProviderDetails(this.providerId())); + readonly isPreprintProviderLoading = select(PreprintProvidersSelectors.isPreprintProviderDetailsLoading); + readonly hasBeenSubmitted = select(PreprintStepperSelectors.hasBeenSubmitted); - preprintProvider = select(PreprintProvidersSelectors.getPreprintProviderDetails(this.providerId())); - isPreprintProviderLoading = select(PreprintProvidersSelectors.isPreprintProviderDetailsLoading); - hasBeenSubmitted = select(PreprintStepperSelectors.hasBeenSubmitted); currentStep = signal(createNewVersionStepsConst[0]); isWeb = toSignal(inject(IS_WEB)); + readonly PreprintSteps = PreprintSteps; + readonly newVersionSteps = createNewVersionStepsConst; + constructor() { this.actions.getPreprintProviderById(this.providerId()); this.actions.fetchPreprint(this.preprintId()); @@ -81,7 +76,6 @@ export class CreateNewVersionComponent implements OnDestroy, CanDeactivateCompon const provider = this.preprintProvider(); if (provider) { - this.actions.setSelectedPreprintProviderId(provider.id); this.brandService.applyBranding(provider.brand); this.headerStyleHelper.applyHeaderStyles( provider.brand.primaryColor, diff --git a/src/app/features/preprints/pages/my-preprints/my-preprints.component.spec.ts b/src/app/features/preprints/pages/my-preprints/my-preprints.component.spec.ts index 31b3f4ef7..06b3456ac 100644 --- a/src/app/features/preprints/pages/my-preprints/my-preprints.component.spec.ts +++ b/src/app/features/preprints/pages/my-preprints/my-preprints.component.spec.ts @@ -67,6 +67,7 @@ describe('MyPreprintsComponent', () => { store = TestBed.inject(Store); jest.spyOn(store, 'dispatch'); + store = TestBed.inject(Store); fixture = TestBed.createComponent(MyPreprintsComponent); component = fixture.componentInstance; fixture.detectChanges(); @@ -161,6 +162,7 @@ describe('MyPreprintsComponent', () => { sortOrder: 'asc', }); + queryParamsSubject.next({ page: '2', size: '20', search: 'test', sortColumn: 'title', sortOrder: 'asc' }); fixture.detectChanges(); expect(component.searchControl.value).toBe('test'); @@ -168,7 +170,6 @@ describe('MyPreprintsComponent', () => { expect(component.sortOrder()).toBe(SortOrder.Asc); expect(component.tableParams().rows).toBe(20); expect(component.tableParams().firstRowIndex).toBe(20); - expect(store.dispatch).toHaveBeenCalledWith( new FetchMyPreprints(2, 20, { searchValue: 'test', diff --git a/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.spec.ts b/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.spec.ts index 321d511cd..12b7e4720 100644 --- a/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.spec.ts +++ b/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.spec.ts @@ -9,7 +9,6 @@ import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header import { PreprintProviderShortInfo } from '../../models'; import { GetPreprintProvidersAllowingSubmissions, PreprintProvidersSelectors } from '../../store/preprint-providers'; -import { PreprintStepperSelectors, SetSelectedPreprintProviderId } from '../../store/preprint-stepper'; import { SelectPreprintServiceComponent } from './select-preprint-service.component'; @@ -29,7 +28,6 @@ describe('SelectPreprintServiceComponent', () => { const defaultSignals: SignalOverride[] = [ { selector: PreprintProvidersSelectors.getPreprintProvidersAllowingSubmissions, value: mockProviders }, { selector: PreprintProvidersSelectors.arePreprintProvidersAllowingSubmissionsLoading, value: false }, - { selector: PreprintStepperSelectors.getSelectedProviderId, value: null }, ]; function setup(overrides?: { selectorOverrides?: SignalOverride[] }) { @@ -68,26 +66,24 @@ describe('SelectPreprintServiceComponent', () => { component.toggleProviderSelection(mockProvider); - expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintProviderId(mockProvider.id)); + expect(component.selectedProviderId()).toBe(mockProvider.id); }); - it('should dispatch deselect action when toggling the already selected provider', () => { - setup({ - selectorOverrides: [{ selector: PreprintStepperSelectors.getSelectedProviderId, value: mockProvider.id }], - }); + it('should deselect when toggling the already selected provider', () => { + setup(); + component.selectedProviderId.set(mockProvider.id); component.toggleProviderSelection(mockProvider); - expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintProviderId(null)); + expect(component.selectedProviderId()).toBeNull(); }); - it('should dispatch select action when toggling a different provider', () => { - setup({ - selectorOverrides: [{ selector: PreprintStepperSelectors.getSelectedProviderId, value: 'other-provider' }], - }); + it('should select when toggling a different provider', () => { + setup(); + component.selectedProviderId.set('other-provider'); component.toggleProviderSelection(mockProvider); - expect(store.dispatch).toHaveBeenCalledWith(new SetSelectedPreprintProviderId(mockProvider.id)); + expect(component.selectedProviderId()).toBe(mockProvider.id); }); }); diff --git a/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.ts b/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.ts index e47218751..842d11f5e 100644 --- a/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.ts +++ b/src/app/features/preprints/pages/select-preprint-service/select-preprint-service.component.ts @@ -8,7 +8,7 @@ import { Skeleton } from 'primeng/skeleton'; import { Tooltip } from 'primeng/tooltip'; import { NgClass } from '@angular/common'; -import { ChangeDetectionStrategy, Component, HostBinding } from '@angular/core'; +import { ChangeDetectionStrategy, Component, HostBinding, signal } from '@angular/core'; import { RouterLink } from '@angular/router'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; @@ -16,7 +16,6 @@ import { SafeHtmlPipe } from '@osf/shared/pipes/safe-html.pipe'; import { PreprintProviderShortInfo } from '../../models'; import { GetPreprintProvidersAllowingSubmissions, PreprintProvidersSelectors } from '../../store/preprint-providers'; -import { PreprintStepperSelectors, SetSelectedPreprintProviderId } from '../../store/preprint-stepper'; @Component({ selector: 'osf-select-preprint-service', @@ -30,12 +29,12 @@ export class SelectPreprintServiceComponent { private actions = createDispatchMap({ getPreprintProvidersAllowingSubmissions: GetPreprintProvidersAllowingSubmissions, - setSelectedPreprintProviderId: SetSelectedPreprintProviderId, }); preprintProvidersAllowingSubmissions = select(PreprintProvidersSelectors.getPreprintProvidersAllowingSubmissions); areProvidersLoading = select(PreprintProvidersSelectors.arePreprintProvidersAllowingSubmissionsLoading); - selectedProviderId = select(PreprintStepperSelectors.getSelectedProviderId); + + selectedProviderId = signal(null); skeletonArray = new Array(8); constructor() { @@ -44,10 +43,10 @@ export class SelectPreprintServiceComponent { toggleProviderSelection(provider: PreprintProviderShortInfo): void { if (provider.id === this.selectedProviderId()) { - this.actions.setSelectedPreprintProviderId(null); + this.selectedProviderId.set(null); return; } - this.actions.setSelectedPreprintProviderId(provider.id); + this.selectedProviderId.set(provider.id); } } diff --git a/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.html b/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.html index 2fa6ce3cc..38a89d790 100644 --- a/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.html +++ b/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.html @@ -36,7 +36,7 @@

@switch (currentStep().value) { @case (PreprintSteps.TitleAndAbstract) { - + } @case (PreprintSteps.File) { diff --git a/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.ts b/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.ts index 21e5c3ac2..bb0e6f7a2 100644 --- a/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.ts +++ b/src/app/features/preprints/pages/submit-preprint-stepper/submit-preprint-stepper.component.ts @@ -40,12 +40,7 @@ import { import { submitPreprintSteps } from '../../constants'; import { PreprintSteps } from '../../enums'; import { GetPreprintProviderById, PreprintProvidersSelectors } from '../../store/preprint-providers'; -import { - DeletePreprint, - PreprintStepperSelectors, - ResetPreprintStepperState, - SetSelectedPreprintProviderId, -} from '../../store/preprint-stepper'; +import { DeletePreprint, PreprintStepperSelectors, ResetPreprintStepperState } from '../../store/preprint-stepper'; @Component({ selector: 'osf-submit-preprint-stepper', @@ -77,7 +72,6 @@ export class SubmitPreprintStepperComponent implements OnDestroy, CanDeactivateC private actions = createDispatchMap({ getPreprintProviderById: GetPreprintProviderById, - setSelectedPreprintProviderId: SetSelectedPreprintProviderId, resetState: ResetPreprintStepperState, deletePreprint: DeletePreprint, }); @@ -109,7 +103,6 @@ export class SubmitPreprintStepperComponent implements OnDestroy, CanDeactivateC const provider = this.preprintProvider(); if (provider) { - this.actions.setSelectedPreprintProviderId(provider.id); this.brandService.applyBranding(provider.brand); this.headerStyleHelper.applyHeaderStyles( provider.brand.primaryColor, diff --git a/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.html b/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.html index 6a82b8ad0..6e19e1bd8 100644 --- a/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.html +++ b/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.html @@ -36,16 +36,16 @@

@switch (currentStep().value) { @case (PreprintSteps.TitleAndAbstract) { - + } @case (PreprintSteps.File) { } @case (PreprintSteps.Metadata) { } @case (PreprintSteps.AuthorAssertions) { diff --git a/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.ts b/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.ts index 455e67cb9..eacb8c11e 100644 --- a/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.ts +++ b/src/app/features/preprints/pages/update-preprint-stepper/update-preprint-stepper.component.ts @@ -39,12 +39,7 @@ import { import { submitPreprintSteps } from '../../constants'; import { PreprintSteps, ProviderReviewsWorkflow, ReviewsState } from '../../enums'; import { GetPreprintProviderById, PreprintProvidersSelectors } from '../../store/preprint-providers'; -import { - FetchPreprintById, - PreprintStepperSelectors, - ResetPreprintStepperState, - SetSelectedPreprintProviderId, -} from '../../store/preprint-stepper'; +import { FetchPreprintById, PreprintStepperSelectors, ResetPreprintStepperState } from '../../store/preprint-stepper'; @Component({ selector: 'osf-update-preprint-stepper', @@ -76,7 +71,6 @@ export class UpdatePreprintStepperComponent implements OnDestroy, CanDeactivateC private actions = createDispatchMap({ getPreprintProviderById: GetPreprintProviderById, - setSelectedPreprintProviderId: SetSelectedPreprintProviderId, resetState: ResetPreprintStepperState, fetchPreprint: FetchPreprintById, }); @@ -129,7 +123,6 @@ export class UpdatePreprintStepperComponent implements OnDestroy, CanDeactivateC const provider = this.preprintProvider(); if (provider) { - this.actions.setSelectedPreprintProviderId(provider.id); this.brandService.applyBranding(provider.brand); this.headerStyleHelper.applyHeaderStyles( provider.brand.primaryColor, diff --git a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.actions.ts b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.actions.ts index 23d62a456..4e35cfd0c 100644 --- a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.actions.ts +++ b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.actions.ts @@ -6,12 +6,6 @@ import { LicenseOptions } from '@osf/shared/models/license/license.model'; import { PreprintFileSource } from '../../enums'; import { PreprintModel } from '../../models'; -export class SetSelectedPreprintProviderId { - static readonly type = '[Preprint Stepper] Set Selected Preprint Provider Id'; - - constructor(public id: StringOrNull) {} -} - export class CreatePreprint { static readonly type = '[Preprint Stepper] Create Preprint'; @@ -98,6 +92,8 @@ export class FetchProjectFilesByLink { export class FetchLicenses { static readonly type = '[Preprint Stepper] Fetch Licenses'; + + constructor(public providerId: string) {} } export class SaveLicense { diff --git a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.model.ts b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.model.ts index dc64f8289..7fdbee39c 100644 --- a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.model.ts +++ b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.model.ts @@ -1,4 +1,3 @@ -import { StringOrNull } from '@osf/shared/helpers/types.helper'; import { IdNameModel } from '@osf/shared/models/common/id-name.model'; import { FileModel } from '@osf/shared/models/files/file.model'; import { FileFolderModel } from '@osf/shared/models/files/file-folder.model'; @@ -10,7 +9,6 @@ import { PreprintFileSource } from '../../enums'; import { PreprintFilesLinks, PreprintModel } from '../../models'; export interface PreprintStepperStateModel { - selectedProviderId: StringOrNull; preprint: AsyncStateModel; fileSource: PreprintFileSource; preprintFilesLinks: AsyncStateModel; @@ -25,7 +23,6 @@ export interface PreprintStepperStateModel { } export const DEFAULT_PREPRINT_STEPPER_STATE: PreprintStepperStateModel = { - selectedProviderId: null, preprint: { data: null, isLoading: false, diff --git a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.selectors.ts b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.selectors.ts index 43f5e74c6..f88c8eaf9 100644 --- a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.selectors.ts +++ b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.selectors.ts @@ -4,11 +4,6 @@ import { PreprintStepperState, PreprintStepperStateModel } from '@osf/features/p import { UserPermissions } from '@osf/shared/enums/user-permissions.enum'; export class PreprintStepperSelectors { - @Selector([PreprintStepperState]) - static getSelectedProviderId(state: PreprintStepperStateModel) { - return state.selectedProviderId; - } - @Selector([PreprintStepperState]) static getPreprint(state: PreprintStepperStateModel) { return state.preprint.data; diff --git a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.state.ts b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.state.ts index 1e1eb3bd1..cc817558d 100644 --- a/src/app/features/preprints/store/preprint-stepper/preprint-stepper.state.ts +++ b/src/app/features/preprints/store/preprint-stepper/preprint-stepper.state.ts @@ -41,7 +41,6 @@ import { SetPreprintStepperCurrentFolder, SetProjectRootFolder, SetSelectedPreprintFileSource, - SetSelectedPreprintProviderId, SubmitPreprint, UpdatePreprint, UpdatePrimaryFileRelationship, @@ -61,11 +60,6 @@ export class PreprintStepperState { private licensesService = inject(PreprintLicensesService); private preprintProjectsService = inject(PreprintsProjectsService); - @Action(SetSelectedPreprintProviderId) - setSelectedPreprintProviderId(ctx: StateContext, action: SetSelectedPreprintProviderId) { - ctx.patchState({ selectedProviderId: action.id }); - } - @Action(CreatePreprint) createPreprint(ctx: StateContext, action: CreatePreprint) { ctx.setState(patch({ preprint: patch({ isSubmitting: true }) })); @@ -323,12 +317,10 @@ export class PreprintStepperState { } @Action(FetchLicenses) - fetchLicenses(ctx: StateContext) { - const providerId = ctx.getState().selectedProviderId; - if (!providerId) return; + fetchLicenses(ctx: StateContext, action: FetchLicenses) { ctx.setState(patch({ licenses: patch({ isLoading: true }) })); - return this.licensesService.getLicenses(providerId).pipe( + return this.licensesService.getLicenses(action.providerId).pipe( tap((licenses) => { ctx.setState(patch({ licenses: patch({ isLoading: false, data: licenses }) })); }), 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 a354ad6b6..338a3fab0 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 @@ -2,8 +2,7 @@ import { Store } from '@ngxs/store'; import { MockComponents, MockProvider } from 'ng-mocks'; -import { signal, WritableSignal } from '@angular/core'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { TestBed } from '@angular/core/testing'; import { FormGroup } from '@angular/forms'; import { ActivatedRoute, Router } from '@angular/router'; @@ -22,38 +21,27 @@ import { CustomStepComponent } from './custom-step.component'; import { MOCK_REGISTRIES_PAGE, MOCK_STEPS_DATA } from '@testing/mocks/registries.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; -import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; -import { ToastServiceMock, ToastServiceMockType } from '@testing/providers/toast-provider.mock'; +import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; +import { + BaseSetupOverrides, + mergeSignalOverrides, + provideMockStore, + SignalOverride, +} from '@testing/providers/store-provider.mock'; +import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; type StepsState = Record; -describe('CustomStepComponent', () => { - let component: CustomStepComponent; - let fixture: ComponentFixture; - let store: Store; - let routeBuilder: ActivatedRouteMockBuilder; - let mockRouter: RouterMockType; - let toastMock: ToastServiceMockType; - let pagesSignal: WritableSignal; - let stepsStateSignal: WritableSignal; - - function createComponent( - page: PageSchema, - stepsData: Record = {}, - stepsState: StepsState = {} - ): ComponentFixture { - pagesSignal.set([page]); - stepsStateSignal.set(stepsState); - const f = TestBed.createComponent(CustomStepComponent); - f.componentRef.setInput('stepsData', stepsData); - f.componentRef.setInput('filesLink', 'files-link'); - f.componentRef.setInput('projectId', 'project'); - f.componentRef.setInput('provider', 'provider'); - f.detectChanges(); - return f; - } +interface SetupOverrides extends BaseSetupOverrides { + pages?: PageSchema[]; + stepsState?: StepsState; + stepsData?: Record; + filesLink?: string; + projectId?: string; + provider?: string; +} +describe('CustomStepComponent', () => { function createPage( questions: PageSchema['questions'] = [], sections: PageSchema['sections'] = undefined @@ -61,12 +49,20 @@ describe('CustomStepComponent', () => { return { id: 'p', title: 'P', questions, sections }; } - beforeEach(() => { - toastMock = ToastServiceMock.simple(); - routeBuilder = ActivatedRouteMockBuilder.create().withParams({ step: 1 }); - mockRouter = RouterMockBuilder.create().withUrl('/registries/drafts/id/1').build(); - pagesSignal = signal([MOCK_REGISTRIES_PAGE]); - stepsStateSignal = signal({}); + function setup(overrides: SetupOverrides = {}) { + const routeBuilder = ActivatedRouteMockBuilder.create().withParams(overrides.routeParams ?? { step: 1 }); + if (overrides.hasParent === false) { + routeBuilder.withNoParent(); + } + + const mockRouter = RouterMockBuilder.create().withUrl('/registries/drafts/id/1').build(); + const toastMock = ToastServiceMock.simple(); + + const defaultSignals: SignalOverride[] = [ + { selector: RegistriesSelectors.getPagesSchema, value: overrides.pages ?? [MOCK_REGISTRIES_PAGE] }, + { selector: RegistriesSelectors.getStepsState, value: overrides.stepsState ?? {} }, + ]; + const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides); TestBed.configureTestingModule({ imports: [CustomStepComponent, ...MockComponents(InfoIconComponent, FilesControlComponent)], @@ -75,53 +71,48 @@ describe('CustomStepComponent', () => { MockProvider(ToastService, toastMock), MockProvider(ActivatedRoute, routeBuilder.build()), MockProvider(Router, mockRouter), - provideMockStore({ - signals: [ - { selector: RegistriesSelectors.getPagesSchema, value: pagesSignal }, - { selector: RegistriesSelectors.getStepsState, value: stepsStateSignal }, - ], - }), + provideMockStore({ signals }), ], }); - store = TestBed.inject(Store); - fixture = TestBed.createComponent(CustomStepComponent); - component = fixture.componentInstance; - fixture.componentRef.setInput('stepsData', MOCK_STEPS_DATA); - fixture.componentRef.setInput('filesLink', 'files-link'); - fixture.componentRef.setInput('projectId', 'project'); - fixture.componentRef.setInput('provider', 'provider'); + const store = TestBed.inject(Store); + const fixture = TestBed.createComponent(CustomStepComponent); + const component = fixture.componentInstance; + fixture.componentRef.setInput('stepsData', overrides.stepsData ?? MOCK_STEPS_DATA); + fixture.componentRef.setInput('filesLink', overrides.filesLink ?? 'files-link'); + fixture.componentRef.setInput('projectId', overrides.projectId ?? 'project'); + fixture.componentRef.setInput('provider', overrides.provider ?? 'provider'); fixture.detectChanges(); - }); + return { component, fixture, store, routeBuilder, mockRouter, toastMock }; + } it('should create', () => { + const { component } = setup(); expect(component).toBeTruthy(); }); - it('should initialize stepForm when page available', () => { - expect(Object.keys(component['stepForm'].controls)).toContain('field1'); - expect(Object.keys(component['stepForm'].controls)).toContain('field2'); - }); - it('should emit back on first step', () => { + const { component } = setup(); const backSpy = jest.spyOn(component.back, 'emit'); component.goBack(); expect(backSpy).toHaveBeenCalled(); }); it('should navigate to previous step on step > 1', () => { + const { component, mockRouter } = setup(); component.step.set(2); component.goBack(); expect(mockRouter.navigate).toHaveBeenCalledWith(['../', 1], { relativeTo: expect.anything() }); }); it('should navigate to next step within pages', () => { - pagesSignal.set([MOCK_REGISTRIES_PAGE, MOCK_REGISTRIES_PAGE]); + const { component, mockRouter } = setup({ pages: [MOCK_REGISTRIES_PAGE, MOCK_REGISTRIES_PAGE] }); component.goNext(); expect(mockRouter.navigate).toHaveBeenCalledWith(['../', 2], { relativeTo: expect.anything() }); }); it('should emit next on last step', () => { + const { component } = setup(); const nextSpy = jest.spyOn(component.next, 'emit'); component.step.set(1); component.goNext(); @@ -129,12 +120,14 @@ describe('CustomStepComponent', () => { }); it('should dispatch updateStepState on ngOnDestroy', () => { + const { component, store } = setup(); (store.dispatch as jest.Mock).mockClear(); component.ngOnDestroy(); expect(store.dispatch).toHaveBeenCalledWith(expect.any(UpdateStepState)); }); it('should emit updateAction and dispatch setUpdatedFields when fields changed', () => { + const { component, store } = setup(); const emitSpy = jest.spyOn(component.updateAction, 'emit'); component['stepForm'].get('field1')?.setValue('changed'); (store.dispatch as jest.Mock).mockClear(); @@ -146,6 +139,7 @@ describe('CustomStepComponent', () => { }); it('should not emit updateAction when no fields changed', () => { + const { component, store } = setup(); const emitSpy = jest.spyOn(component.updateAction, 'emit'); (store.dispatch as jest.Mock).mockClear(); @@ -156,6 +150,7 @@ describe('CustomStepComponent', () => { }); it('should skip saveStepState when form has no controls', () => { + const { component, store } = setup(); component.stepForm = new FormGroup({}); (store.dispatch as jest.Mock).mockClear(); @@ -165,6 +160,7 @@ describe('CustomStepComponent', () => { }); it('should attach file and emit updateAction', () => { + const { component } = setup(); const emitSpy = jest.spyOn(component.updateAction, 'emit'); const mockFile = { id: 'new-file', @@ -181,6 +177,7 @@ describe('CustomStepComponent', () => { }); it('should not attach duplicate file', () => { + const { component } = setup(); component.attachedFiles['field1'] = [{ file_id: 'file-1', name: 'existing.txt' }]; const emitSpy = jest.spyOn(component.updateAction, 'emit'); @@ -191,6 +188,7 @@ describe('CustomStepComponent', () => { }); it('should show warning when attachment limit reached', () => { + const { component, toastMock } = setup(); component.attachedFiles['field1'] = Array.from({ length: 5 }, (_, i) => ({ file_id: `f-${i}`, name: `f-${i}` })); const mockFile = { @@ -206,6 +204,7 @@ describe('CustomStepComponent', () => { }); it('should remove file and emit updateAction', () => { + const { component } = setup(); const emitSpy = jest.spyOn(component.updateAction, 'emit'); component.attachedFiles['field1'] = [ { file_id: 'f1', name: 'a' }, @@ -220,12 +219,14 @@ describe('CustomStepComponent', () => { }); it('should skip non-existent questionKey', () => { + const { component } = setup(); const emitSpy = jest.spyOn(component.updateAction, 'emit'); component.removeFromAttachedFiles({ file_id: 'f1' }, 'nonexistent'); expect(emitSpy).not.toHaveBeenCalled(); }); it('should save step state and update step on route param change', () => { + const { component, store, routeBuilder } = setup(); (store.dispatch as jest.Mock).mockClear(); routeBuilder.withParams({ step: 2 }); @@ -234,65 +235,85 @@ describe('CustomStepComponent', () => { }); it('should mark form touched when stepsState has invalid for current step', () => { - const f = createComponent(MOCK_REGISTRIES_PAGE, MOCK_STEPS_DATA, { - 1: { invalid: true, touched: true }, + const { component } = setup({ + stepsState: { 1: { invalid: true, touched: true } }, }); - expect(f.componentInstance['stepForm'].get('field1')?.touched).toBe(true); + expect(component['stepForm'].get('field1')?.touched).toBe(true); }); it('should initialize checkbox control with empty array default', () => { - const page = createPage([ - { id: 'q', displayText: '', responseKey: 'cbField', fieldType: FieldType.Checkbox, required: true }, - ]); - const f = createComponent(page); - expect(f.componentInstance['stepForm'].get('cbField')?.value).toEqual([]); + const { component } = setup({ + pages: [ + createPage([ + { id: 'q', displayText: '', responseKey: 'cbField', fieldType: FieldType.Checkbox, required: true }, + ]), + ], + stepsData: {}, + }); + expect(component['stepForm'].get('cbField')?.value).toEqual([]); }); it('should initialize radio control with required validator', () => { - const page = createPage([ - { id: 'q', displayText: '', responseKey: 'radioField', fieldType: FieldType.Radio, required: true }, - ]); - const f = createComponent(page); - expect(f.componentInstance['stepForm'].get('radioField')?.valid).toBe(false); + const { component } = setup({ + pages: [ + createPage([ + { id: 'q', displayText: '', responseKey: 'radioField', fieldType: FieldType.Radio, required: true }, + ]), + ], + stepsData: {}, + }); + expect(component['stepForm'].get('radioField')?.valid).toBe(false); }); it('should initialize file control and populate attachedFiles', () => { - const page = createPage([ - { id: 'q', displayText: '', responseKey: 'fileField', fieldType: FieldType.File, required: false }, - ]); const files: FilePayloadJsonApi[] = [ { file_id: 'f1', file_name: 'doc.pdf', file_urls: { html: '', download: '' }, file_hashes: { sha256: '' } }, ]; - const f = createComponent(page, { fileField: files }); + const { component } = setup({ + pages: [ + createPage([ + { id: 'q', displayText: '', responseKey: 'fileField', fieldType: FieldType.File, required: false }, + ]), + ], + stepsData: { fileField: files }, + }); - expect(f.componentInstance.attachedFiles['fileField'].length).toBe(1); - expect(f.componentInstance.attachedFiles['fileField'][0].name).toBe('doc.pdf'); + expect(component.attachedFiles['fileField'].length).toBe(1); + expect(component.attachedFiles['fileField'][0].name).toBe('doc.pdf'); }); it('should skip unknown field types', () => { - const page = createPage([ - { id: 'q', displayText: '', responseKey: 'unknownField', fieldType: 'unknown' as FieldType, required: false }, - ]); - const f = createComponent(page); - expect(f.componentInstance['stepForm'].get('unknownField')).toBeNull(); + const { component } = setup({ + pages: [ + createPage([ + { id: 'q', displayText: '', responseKey: 'unknownField', fieldType: 'unknown' as FieldType, required: false }, + ]), + ], + stepsData: {}, + }); + expect(component['stepForm'].get('unknownField')).toBeNull(); }); it('should include section questions', () => { - const page = createPage( - [], - [ - { - id: 's1', - title: 'S', - questions: [ - { id: 'q', displayText: '', responseKey: 'secField', fieldType: FieldType.Text, required: false }, - ], - }, - ] - ); - const f = createComponent(page, { secField: 'val' }); - - expect(f.componentInstance['stepForm'].get('secField')).toBeDefined(); - expect(f.componentInstance['stepForm'].get('secField')?.value).toBe('val'); + const { component } = setup({ + pages: [ + createPage( + [], + [ + { + id: 's1', + title: 'S', + questions: [ + { id: 'q', displayText: '', responseKey: 'secField', fieldType: FieldType.Text, required: false }, + ], + }, + ] + ), + ], + stepsData: { secField: 'val' }, + }); + + expect(component['stepForm'].get('secField')).toBeDefined(); + expect(component['stepForm'].get('secField')?.value).toBe('val'); }); }); diff --git a/src/app/features/registries/components/registries-metadata-step/registries-contributors/registries-contributors.component.spec.ts b/src/app/features/registries/components/registries-metadata-step/registries-contributors/registries-contributors.component.spec.ts index aecb80277..703f8e7ba 100644 --- a/src/app/features/registries/components/registries-metadata-step/registries-contributors/registries-contributors.component.spec.ts +++ b/src/app/features/registries/components/registries-metadata-step/registries-contributors/registries-contributors.component.spec.ts @@ -55,16 +55,6 @@ describe('RegistriesContributorsComponent', () => { const initialContributors: ContributorModel[] = [MOCK_CONTRIBUTOR, MOCK_CONTRIBUTOR_WITHOUT_HISTORY]; - beforeAll(() => { - if (typeof (globalThis as any).structuredClone !== 'function') { - Object.defineProperty(globalThis as any, 'structuredClone', { - configurable: true, - writable: true, - value: (o: unknown) => JSON.parse(JSON.stringify(o)), - }); - } - }); - beforeEach(() => { mockCustomDialogService = CustomDialogServiceMockBuilder.create().withDefaultOpen().build(); mockCustomConfirmationService = CustomConfirmationServiceMockBuilder.create().build(); diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 23f79bf7f..6e2433aa5 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -2175,11 +2175,6 @@ "uploadFromComputer": "Upload From Your Computer", "selectFromProject": "Select From Existing OSF Project", "uploadFileButton": "Upload file", - "projectSelection": { - "title": "Project Title", - "description": "This will make your project public, if it is not already", - "subDescription": "The projects and components for which you have admin access are listed below." - }, "versionFile": { "header": "Add a new preprint file", "message": "This will allow a new version of the preprint file to be uploaded to the preprint. The existing file will be retained as a version of the preprint." @@ -2227,11 +2222,6 @@ "connectExisting": "Connect An Existing OSF Project", "createNew": "Create A New OSF Project" }, - "projectSelection": { - "title": "Select Project", - "description": "This will make your project public, if it is not already", - "subDescription": "The projects and components for which you have admin access are listed below." - }, "disconnectProject": { "header": "Disconnect supplemental material", "message": "This will disconnect the selected project. You can select new supplemental material or re-add the same supplemental material at a later date." @@ -2284,6 +2274,11 @@ } } }, + "projectSelection": { + "selectProject": "Select Project", + "description": "This will make your project public, if it is not already", + "subDescription": "The projects and components for which you have admin access are listed below." + }, "common": { "validation": { "fillRequiredFields": "Fill in “Required” fields to continue" diff --git a/src/styles/overrides/select.scss b/src/styles/overrides/select.scss index 92375d05c..6aa14352c 100644 --- a/src/styles/overrides/select.scss +++ b/src/styles/overrides/select.scss @@ -45,3 +45,9 @@ min-width: 16rem; } } + +#project-select { + .p-select-label { + font-size: 0.875rem; + } +}