From 1616c3847f49498d8b30139dbd2205641086b6af Mon Sep 17 00:00:00 2001 From: nsemets Date: Fri, 17 Apr 2026 18:43:41 +0300 Subject: [PATCH 1/3] fix(justification): updated init step validation to fix freeze issue --- .../justification/justification.component.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/app/features/registries/pages/justification/justification.component.ts b/src/app/features/registries/pages/justification/justification.component.ts index 610dfd668..96ddf6e12 100644 --- a/src/app/features/registries/pages/justification/justification.component.ts +++ b/src/app/features/registries/pages/justification/justification.component.ts @@ -165,23 +165,20 @@ export class JustificationComponent implements OnDestroy { private initStepValidation(): void { effect(() => { - const currentIndex = this.currentStepIndex(); - const pages = this.pages(); - const revisionData = this.schemaResponseRevisionData(); const stepState = untracked(() => this.stepsState()); - if (currentIndex > 0) { + if (this.currentStepIndex() > 0) { this.actions.updateStepState('0', true, stepState?.[0]?.touched || false); } - if (pages.length && currentIndex > 0 && revisionData) { - for (let i = 1; i < currentIndex; i++) { - const pageStep = pages[i - 1]; + if (this.pages().length && this.currentStepIndex() > 0 && this.schemaResponseRevisionData()) { + for (let i = 1; i < this.currentStepIndex(); i++) { + const pageStep = this.pages()[i - 1]; const isStepInvalid = pageStep?.questions?.some((question) => { - const questionData = revisionData[question.responseKey!]; + const questionData = this.schemaResponseRevisionData()[question.responseKey!]; return question.required && (Array.isArray(questionData) ? !questionData.length : !questionData); - }) || false; + }) ?? false; this.actions.updateStepState(i.toString(), isStepInvalid, stepState?.[i]?.touched || false); } } From f51f412cb44aec45090a991c5195d6ec3ab34563 Mon Sep 17 00:00:00 2001 From: nsemets Date: Mon, 20 Apr 2026 12:19:39 +0300 Subject: [PATCH 2/3] fix(registry-update): updated logic with registry update and tests --- .../metadata-registry-info.component.spec.ts | 1 + .../registry-provider-hero.component.spec.ts | 1 + ...gistries-provider-search.component.spec.ts | 1 + .../registry-revisions.component.html | 2 +- .../registry-revisions.component.spec.ts | 22 ++++++++ .../registry-revisions.component.ts | 10 ++++ .../registry-overview.component.html | 2 +- .../registry-overview.component.spec.ts | 55 ++++++++++++++----- .../registry-overview.component.ts | 4 ++ .../mappers/registration-provider.mapper.ts | 1 + .../provider/registry-provider.model.ts | 1 + .../registration-provider.selectors.ts | 5 ++ 12 files changed, 89 insertions(+), 16 deletions(-) diff --git a/src/app/features/metadata/components/metadata-registry-info/metadata-registry-info.component.spec.ts b/src/app/features/metadata/components/metadata-registry-info/metadata-registry-info.component.spec.ts index 4a0f25eb2..34f80a903 100644 --- a/src/app/features/metadata/components/metadata-registry-info/metadata-registry-info.component.spec.ts +++ b/src/app/features/metadata/components/metadata-registry-info/metadata-registry-info.component.spec.ts @@ -19,6 +19,7 @@ describe('MetadataRegistryInfoComponent', () => { iri: 'https://example.com/registry', reviewsWorkflow: 'standard', allowSubmissions: true, + allowUpdates: true, }; beforeEach(() => { diff --git a/src/app/features/registries/components/registry-provider-hero/registry-provider-hero.component.spec.ts b/src/app/features/registries/components/registry-provider-hero/registry-provider-hero.component.spec.ts index abdc2a725..8fdba2575 100644 --- a/src/app/features/registries/components/registry-provider-hero/registry-provider-hero.component.spec.ts +++ b/src/app/features/registries/components/registry-provider-hero/registry-provider-hero.component.spec.ts @@ -37,6 +37,7 @@ describe('RegistryProviderHeroComponent', () => { iri: '', reviewsWorkflow: '', allowSubmissions: false, + allowUpdates: true, }; beforeEach(() => { diff --git a/src/app/features/registries/pages/registries-provider-search/registries-provider-search.component.spec.ts b/src/app/features/registries/pages/registries-provider-search/registries-provider-search.component.spec.ts index e30b7d3c0..2e2fe0908 100644 --- a/src/app/features/registries/pages/registries-provider-search/registries-provider-search.component.spec.ts +++ b/src/app/features/registries/pages/registries-provider-search/registries-provider-search.component.spec.ts @@ -36,6 +36,7 @@ const MOCK_PROVIDER: RegistryProviderDetails = { iri: 'http://iri.example.com', reviewsWorkflow: 'pre-moderation', allowSubmissions: true, + allowUpdates: true, }; describe('RegistriesProviderSearchComponent', () => { diff --git a/src/app/features/registry/components/registry-revisions/registry-revisions.component.html b/src/app/features/registry/components/registry-revisions/registry-revisions.component.html index 794707631..6a50baf07 100644 --- a/src/app/features/registry/components/registry-revisions/registry-revisions.component.html +++ b/src/app/features/registry/components/registry-revisions/registry-revisions.component.html @@ -31,7 +31,7 @@ diff --git a/src/app/features/registry/components/registry-revisions/registry-revisions.component.spec.ts b/src/app/features/registry/components/registry-revisions/registry-revisions.component.spec.ts index 24b0310b9..47d0b876a 100644 --- a/src/app/features/registry/components/registry-revisions/registry-revisions.component.spec.ts +++ b/src/app/features/registry/components/registry-revisions/registry-revisions.component.spec.ts @@ -168,6 +168,28 @@ describe('RegistryRevisionsComponent', () => { expect(spy).toHaveBeenCalledWith(1); }); + it('should emit updateRegistration with registry id on startUpdateRegistration', () => { + const { component } = setup(); + const spy = vi.fn(); + component.updateRegistration.subscribe(spy); + + component.startUpdateRegistration(); + + expect(spy).toHaveBeenCalledWith(MOCK_REGISTRY.id); + }); + + it('should not emit updateRegistration when registry id is missing on startUpdateRegistration', () => { + const { fixture, component } = setup(); + const spy = vi.fn(); + component.updateRegistration.subscribe(spy); + fixture.componentRef.setInput('registry', { ...MOCK_REGISTRY, id: '' }); + fixture.detectChanges(); + + component.startUpdateRegistration(); + + expect(spy).not.toHaveBeenCalled(); + }); + it('should emit continueUpdate on continueUpdateHandler', () => { const { component } = setup(); const spy = vi.fn(); diff --git a/src/app/features/registry/components/registry-revisions/registry-revisions.component.ts b/src/app/features/registry/components/registry-revisions/registry-revisions.component.ts index 46560bbcf..1a88fab9e 100644 --- a/src/app/features/registry/components/registry-revisions/registry-revisions.component.ts +++ b/src/app/features/registry/components/registry-revisions/registry-revisions.component.ts @@ -75,6 +75,16 @@ export class RegistryRevisionsComponent { }); }); + startUpdateRegistration() { + const registryId = this.registry()?.id; + + if (!registryId) { + return; + } + + this.updateRegistration.emit(registryId); + } + emitOpenRevision(index: number) { this.openRevision.emit(index); } diff --git a/src/app/features/registry/pages/registry-overview/registry-overview.component.html b/src/app/features/registry/pages/registry-overview/registry-overview.component.html index b7d67b3ca..4712f0918 100644 --- a/src/app/features/registry/pages/registry-overview/registry-overview.component.html +++ b/src/app/features/registry/pages/registry-overview/registry-overview.component.html @@ -84,7 +84,7 @@

(continueUpdate)="onContinueUpdateRegistration()" [isModeration]="isModeration()" [isSubmitting]="isSchemaResponsesLoading()" - [canEdit]="hasAdminAccess()" + [canEdit]="canUpdate()" > diff --git a/src/app/features/registry/pages/registry-overview/registry-overview.component.spec.ts b/src/app/features/registry/pages/registry-overview/registry-overview.component.spec.ts index d4626ee13..d8833dd28 100644 --- a/src/app/features/registry/pages/registry-overview/registry-overview.component.spec.ts +++ b/src/app/features/registry/pages/registry-overview/registry-overview.component.spec.ts @@ -20,6 +20,7 @@ import { SchemaResponse } from '@osf/shared/models/registration/schema-response. import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; import { ToastService } from '@osf/shared/services/toast.service'; import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; +import { RegistrationProviderSelectors } from '@osf/shared/stores/registration-provider'; import { MOCK_REGISTRATION_OVERVIEW_MODEL } from '@testing/mocks/registration-overview-model.mock'; import { createMockSchemaResponse } from '@testing/mocks/schema-response.mock'; @@ -28,7 +29,7 @@ import { CustomDialogServiceMock } from '@testing/providers/custom-dialog-provid import { LoaderServiceMock, provideLoaderServiceMock } from '@testing/providers/loader-service.mock'; import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { BaseSetupOverrides, mergeSignalOverrides, provideMockStore } from '@testing/providers/store-provider.mock'; import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; import { ViewOnlyLinkHelperMock } from '@testing/providers/view-only-link-helper.mock'; @@ -44,7 +45,7 @@ import { RegistrySelectors } from '../../store/registry'; import { RegistryOverviewComponent } from './registry-overview.component'; -interface SetupOverrides { +interface SetupOverrides extends BaseSetupOverrides { registry?: RegistrationOverviewModel | null; schemaResponses?: SchemaResponse[]; queryParams?: Record; @@ -67,6 +68,19 @@ function setup(overrides: SetupOverrides = {}) { const mockLoaderService = new LoaderServiceMock(); const mockToastService = ToastServiceMock.simple(); const mockViewOnlyHelper = ViewOnlyLinkHelperMock.simple(overrides.hasViewOnly); + const signalDefaults = [ + { selector: RegistrySelectors.getRegistry, value: registry }, + { selector: RegistrySelectors.isRegistryLoading, value: false }, + { selector: RegistrySelectors.isRegistryAnonymous, value: false }, + { selector: RegistrySelectors.getSchemaResponses, value: schemaResponses }, + { selector: RegistrySelectors.isSchemaResponsesLoading, value: false }, + { selector: RegistrySelectors.getSchemaBlocks, value: [] }, + { selector: RegistrySelectors.isSchemaBlocksLoading, value: false }, + { selector: RegistrySelectors.areReviewActionsLoading, value: false }, + { selector: RegistrySelectors.getSchemaResponse, value: schemaResponses[0] ?? null }, + { selector: RegistrySelectors.hasAdminAccess, value: false }, + { selector: RegistrationProviderSelectors.allowUpdates, value: false }, + ]; TestBed.configureTestingModule({ imports: [ @@ -94,18 +108,7 @@ function setup(overrides: SetupOverrides = {}) { MockProvider(ToastService, mockToastService), MockProvider(ViewOnlyLinkHelperService, mockViewOnlyHelper), provideMockStore({ - signals: [ - { selector: RegistrySelectors.getRegistry, value: registry }, - { selector: RegistrySelectors.isRegistryLoading, value: false }, - { selector: RegistrySelectors.isRegistryAnonymous, value: false }, - { selector: RegistrySelectors.getSchemaResponses, value: schemaResponses }, - { selector: RegistrySelectors.isSchemaResponsesLoading, value: false }, - { selector: RegistrySelectors.getSchemaBlocks, value: [] }, - { selector: RegistrySelectors.isSchemaBlocksLoading, value: false }, - { selector: RegistrySelectors.areReviewActionsLoading, value: false }, - { selector: RegistrySelectors.getSchemaResponse, value: schemaResponses[0] ?? null }, - { selector: RegistrySelectors.hasAdminAccess, value: false }, - ], + signals: mergeSignalOverrides(signalDefaults, overrides.selectorOverrides), }), ], }); @@ -181,6 +184,30 @@ describe('RegistryOverviewComponent', () => { expect(component.canMakeDecision()).toBe(false); }); + it('should compute canUpdate as true when admin access and provider updates are allowed', () => { + const { component } = setup({ + registry: MOCK_REGISTRATION_OVERVIEW_MODEL, + selectorOverrides: [ + { selector: RegistrySelectors.hasAdminAccess, value: true }, + { selector: RegistrationProviderSelectors.allowUpdates, value: true }, + ], + }); + + expect(component.canUpdate()).toBe(true); + }); + + it('should compute canUpdate as false when provider updates are not allowed', () => { + const { component } = setup({ + registry: MOCK_REGISTRATION_OVERVIEW_MODEL, + selectorOverrides: [ + { selector: RegistrySelectors.hasAdminAccess, value: true }, + { selector: RegistrationProviderSelectors.allowUpdates, value: false }, + ], + }); + + expect(component.canUpdate()).toBe(false); + }); + it('should compute isInitialState from reviewsState', () => { const { component } = setup({ registry: { ...MOCK_REGISTRATION_OVERVIEW_MODEL, reviewsState: RegistrationReviewStates.Initial }, diff --git a/src/app/features/registry/pages/registry-overview/registry-overview.component.ts b/src/app/features/registry/pages/registry-overview/registry-overview.component.ts index 927fc222a..fcf3bbac3 100644 --- a/src/app/features/registry/pages/registry-overview/registry-overview.component.ts +++ b/src/app/features/registry/pages/registry-overview/registry-overview.component.ts @@ -37,6 +37,7 @@ import { ToastService } from '@osf/shared/services/toast.service'; import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; import { GetBookmarksCollectionId } from '@osf/shared/stores/bookmarks'; import { GetBibliographicContributors } from '@osf/shared/stores/contributors'; +import { RegistrationProviderSelectors } from '@osf/shared/stores/registration-provider'; import { ArchivingMessageComponent } from '../../components/archiving-message/archiving-message.component'; import { RegistrationOverviewToolbarComponent } from '../../components/registration-overview-toolbar/registration-overview-toolbar.component'; @@ -98,6 +99,7 @@ export class RegistryOverviewComponent implements OnInit, OnDestroy { readonly areReviewActionsLoading = select(RegistrySelectors.areReviewActionsLoading); readonly currentRevision = select(RegistrySelectors.getSchemaResponse); readonly hasAdminAccess = select(RegistrySelectors.hasAdminAccess); + readonly allowUpdates = select(RegistrationProviderSelectors.allowUpdates); readonly selectedRevisionIndex = signal(0); @@ -112,6 +114,8 @@ export class RegistryOverviewComponent implements OnInit, OnDestroy { () => !this.registry()?.archiving && !this.registry()?.withdrawn && this.isModeration() ); + readonly canUpdate = computed(() => this.hasAdminAccess() && this.allowUpdates()); + isRootRegistration = computed(() => { const rootId = this.registry()?.rootParentId; return !rootId || rootId === this.registry()?.id; diff --git a/src/app/shared/mappers/registration-provider.mapper.ts b/src/app/shared/mappers/registration-provider.mapper.ts index d97c2ebf8..8feb41303 100644 --- a/src/app/shared/mappers/registration-provider.mapper.ts +++ b/src/app/shared/mappers/registration-provider.mapper.ts @@ -35,6 +35,7 @@ export class RegistrationProviderMapper { iri: response.links.iri, reviewsWorkflow: response.attributes.reviews_workflow, allowSubmissions: response.attributes.allow_submissions, + allowUpdates: response.attributes.allow_updates, }; } } diff --git a/src/app/shared/models/provider/registry-provider.model.ts b/src/app/shared/models/provider/registry-provider.model.ts index 1c914acd9..9ccec98f6 100644 --- a/src/app/shared/models/provider/registry-provider.model.ts +++ b/src/app/shared/models/provider/registry-provider.model.ts @@ -11,4 +11,5 @@ export interface RegistryProviderDetails { iri: string; reviewsWorkflow: string; allowSubmissions: boolean; + allowUpdates: boolean; } diff --git a/src/app/shared/stores/registration-provider/registration-provider.selectors.ts b/src/app/shared/stores/registration-provider/registration-provider.selectors.ts index 61010f5cb..d960ebd4e 100644 --- a/src/app/shared/stores/registration-provider/registration-provider.selectors.ts +++ b/src/app/shared/stores/registration-provider/registration-provider.selectors.ts @@ -13,4 +13,9 @@ export class RegistrationProviderSelectors { static isBrandedProviderLoading(state: RegistrationProviderStateModel) { return state.currentBrandedProvider.isLoading; } + + @Selector([RegistrationProviderState]) + static allowUpdates(state: RegistrationProviderStateModel) { + return state.currentBrandedProvider.data?.allowUpdates ?? false; + } } From 21dae6576c052719836d170431d2817d029156f6 Mon Sep 17 00:00:00 2001 From: nsemets Date: Tue, 5 May 2026 11:26:01 +0300 Subject: [PATCH 3/3] fix(registration-card): updated logic for updates button --- .../registration-card.component.html | 15 +- .../registration-card.component.spec.ts | 262 +++++++++++------- .../registration-card.component.ts | 65 ++--- .../registration/registration.mapper.ts | 2 + .../registration/registration-card.model.ts | 1 + .../registration-json-api.model.ts | 5 +- src/testing/mocks/registration.mock.ts | 1 + 7 files changed, 201 insertions(+), 150 deletions(-) diff --git a/src/app/shared/components/registration-card/registration-card.component.html b/src/app/shared/components/registration-card/registration-card.component.html index 823b0b300..6f304d0e5 100644 --- a/src/app/shared/components/registration-card/registration-card.component.html +++ b/src/app/shared/components/registration-card/registration-card.component.html @@ -5,7 +5,7 @@

- {{ (registrationData().title | fixSpecialChar) || ('project.registrations.card.noTitle' | translate) }} + {{ registrationData().title || ('project.registrations.card.noTitle' | translate) }}

@if (!isDraft()) { @@ -49,12 +49,11 @@

@if (isDraft()) { - @if (hasWriteAccess) { + @if (hasWriteAccess()) { [routerLink]="['/registries/drafts/', registrationData().id, 'metadata']" /> } - @if (hasAdminAccess) { + @if (hasAdminAccess()) { routerLinkActive="router-link-active" [label]="'common.buttons.view' | translate" > - @if (showButtons) { - @if (isApproved) { + @if (showButtons()) { + @if (isApproved()) { } - @if (isInProgress || isUnapproved) { + @if (isInProgress() || isUnapproved()) {
- @if (isApproved) { + @if (isApproved()) {

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

{ let component: RegistrationCardComponent; let fixture: ComponentFixture; + let store: Store; + let routerMock: RouterMockType; const mockRegistrationData: RegistrationCard = MOCK_REGISTRATION; - beforeEach(() => { + const defaultSignals: SignalOverride[] = [ + { selector: RegistriesSelectors.getSchemaResponse, value: signal({ id: 'revision-id' }) }, + ]; + + type SetupOverrides = BaseSetupOverrides & { + registrationData?: RegistrationCard; + isDraft?: boolean; + }; + + function setup(overrides: SetupOverrides = {}): void { + routerMock = RouterMockBuilder.create().build(); + TestBed.configureTestingModule({ imports: [ RegistrationCardComponent, @@ -34,133 +58,165 @@ describe('RegistrationCardComponent', () => { ], providers: [ provideOSFCore(), - provideRouter([]), - provideMockStore({ - signals: [{ selector: RegistriesSelectors.getSchemaResponse, value: signal(null) }], - }), + MockProvider(Router, routerMock), + provideMockStore({ signals: mergeSignalOverrides(defaultSignals, overrides.selectorOverrides) }), ], }); + store = TestBed.inject(Store); fixture = TestBed.createComponent(RegistrationCardComponent); component = fixture.componentInstance; - }); + fixture.componentRef.setInput('registrationData', overrides.registrationData ?? mockRegistrationData); + fixture.componentRef.setInput('isDraft', overrides.isDraft ?? false); + fixture.detectChanges(); + } it('should create', () => { - expect(component).toBeTruthy(); - }); + setup(); - it('should have registrationData as required input', () => { - fixture.componentRef.setInput('registrationData', mockRegistrationData); - expect(component.registrationData()).toEqual(mockRegistrationData); + expect(component).toBeTruthy(); }); - it('should compute isAccepted correctly when reviewsState is Accepted', () => { - const testData = { - ...mockRegistrationData, - reviewsState: RegistrationReviewStates.Accepted, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + it.each([ + [[UserPermissions.Read, UserPermissions.Write, UserPermissions.Admin], true, true], + [[UserPermissions.Write], false, true], + [[UserPermissions.Admin], true, false], + ] as [UserPermissions[], boolean, boolean][])( + 'should identify user permissions', + (currentUserPermissions, hasAdminAccess, hasWriteAccess) => { + setup({ + registrationData: { + ...mockRegistrationData, + currentUserPermissions, + }, + }); + + expect(component.hasAdminAccess()).toBe(hasAdminAccess); + expect(component.hasWriteAccess()).toBe(hasWriteAccess); + } + ); + + it.each([ + [RegistrationReviewStates.Accepted, true], + [RegistrationReviewStates.Pending, true], + [RegistrationReviewStates.Embargo, true], + [RegistrationReviewStates.Withdrawn, false], + ] as [RegistrationReviewStates, boolean][])( + 'should identify update-eligible review states', + (reviewsState, eligible) => { + setup({ + registrationData: { + ...mockRegistrationData, + reviewsState, + }, + }); + + expect(component.isAccepted()).toBe(reviewsState === RegistrationReviewStates.Accepted); + expect(component.isPending()).toBe(reviewsState === RegistrationReviewStates.Pending); + expect(component.isEmbargo()).toBe(reviewsState === RegistrationReviewStates.Embargo); + expect(component.showButtons()).toBe(eligible); + } + ); + + it.each([ + [RevisionReviewStates.Approved, true, false, false], + [RevisionReviewStates.Unapproved, false, true, false], + [RevisionReviewStates.RevisionInProgress, false, false, true], + ] as [RevisionReviewStates, boolean, boolean, boolean][])( + 'should identify revision states', + (revisionState, isApproved, isUnapproved, isInProgress) => { + setup({ + registrationData: { + ...mockRegistrationData, + revisionState, + }, + }); + + expect(component.isApproved()).toBe(isApproved); + expect(component.isUnapproved()).toBe(isUnapproved); + expect(component.isInProgress()).toBe(isInProgress); + } + ); + + it.each([ + [null, true], + [mockRegistrationData.id, true], + ['different-root-id', false], + ] as [string | null, boolean][])('should identify root registrations', (rootParentId, isRootRegistration) => { + setup({ + registrationData: { + ...mockRegistrationData, + rootParentId, + }, + }); - expect(component.isAccepted).toBe(true); + expect(component.isRootRegistration()).toBe(isRootRegistration); }); - it('should compute isPending correctly when reviewsState is Pending', () => { - const testData = { - ...mockRegistrationData, - reviewsState: RegistrationReviewStates.Pending, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + it.each([ + ['updates are disabled', { allowUpdates: false }], + ['user lacks admin access', { currentUserPermissions: [UserPermissions.Write] }], + ['registration is not root', { rootParentId: 'different-root-id' }], + ] as [string, Partial][])('should hide update buttons when %s', (_label, registrationData) => { + setup({ + registrationData: { + ...mockRegistrationData, + ...registrationData, + }, + }); - expect(component.isPending).toBe(true); + expect(component.showButtons()).toBe(false); }); - it('should compute isApproved correctly when revisionState is Approved', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.Approved, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + it('should dispatch create schema response and navigate to justification page on updateRegistration', () => { + setup(); + (store.dispatch as Mock).mockClear(); - expect(component.isApproved).toBe(true); - }); + component.updateRegistration(mockRegistrationData.id); - it('should compute isUnapproved correctly when revisionState is Unapproved', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.Unapproved, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); - - expect(component.isUnapproved).toBe(true); + expect(store.dispatch).toHaveBeenCalledWith(new CreateSchemaResponse(mockRegistrationData.id)); + expect(routerMock.navigate).toHaveBeenCalledWith(['/registries/revisions/revision-id/justification']); }); - it('should compute isInProgress correctly when revisionState is RevisionInProgress', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.RevisionInProgress, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); - - expect(component.isInProgress).toBe(true); - }); + it('should dispatch fetch schema responses and navigate to review page for unapproved revision', () => { + setup({ + registrationData: { + ...mockRegistrationData, + revisionState: RevisionReviewStates.Unapproved, + }, + }); + (store.dispatch as Mock).mockClear(); - it('should compute isAccepted as false when reviewsState is not Accepted', () => { - const testData = { - ...mockRegistrationData, - reviewsState: RegistrationReviewStates.Pending, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + component.continueUpdateRegistration(mockRegistrationData.id); - expect(component.isAccepted).toBe(false); + expect(store.dispatch).toHaveBeenCalledWith(new FetchAllSchemaResponses(mockRegistrationData.id)); + expect(routerMock.navigate).toHaveBeenCalledWith(['/registries/revisions/revision-id/review']); }); - it('should compute isPending as false when reviewsState is not Pending', () => { - const testData = { - ...mockRegistrationData, - reviewsState: RegistrationReviewStates.Accepted, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); - - expect(component.isPending).toBe(false); - }); + it('should dispatch fetch schema responses and navigate to justification page for non-unapproved revision', () => { + setup({ + registrationData: { + ...mockRegistrationData, + revisionState: RevisionReviewStates.RevisionInProgress, + }, + }); + (store.dispatch as Mock).mockClear(); - it('should compute isApproved as false when revisionState is not Approved', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.Unapproved, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + component.continueUpdateRegistration(mockRegistrationData.id); - expect(component.isApproved).toBe(false); + expect(store.dispatch).toHaveBeenCalledWith(new FetchAllSchemaResponses(mockRegistrationData.id)); + expect(routerMock.navigate).toHaveBeenCalledWith(['/registries/revisions/revision-id/justification']); }); - it('should compute isUnapproved as false when revisionState is not Unapproved', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.Approved, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); - - expect(component.isUnapproved).toBe(false); - }); + it('should not navigate when schema response is not present', () => { + setup({ + selectorOverrides: [{ selector: RegistriesSelectors.getSchemaResponse, value: signal(null) }], + }); + (store.dispatch as Mock).mockClear(); - it('should compute isInProgress as false when revisionState is not RevisionInProgress', () => { - const testData = { - ...mockRegistrationData, - revisionState: RevisionReviewStates.Approved, - }; - fixture.componentRef.setInput('registrationData', testData); - fixture.detectChanges(); + component.updateRegistration(mockRegistrationData.id); - expect(component.isInProgress).toBe(false); + expect(store.dispatch).toHaveBeenCalledWith(new CreateSchemaResponse(mockRegistrationData.id)); + expect(routerMock.navigate).not.toHaveBeenCalled(); }); }); diff --git a/src/app/shared/components/registration-card/registration-card.component.ts b/src/app/shared/components/registration-card/registration-card.component.ts index e9ba0f72a..56c0bf7e5 100644 --- a/src/app/shared/components/registration-card/registration-card.component.ts +++ b/src/app/shared/components/registration-card/registration-card.component.ts @@ -8,7 +8,7 @@ import { Card } from 'primeng/card'; import { tap } from 'rxjs'; import { DatePipe } from '@angular/common'; -import { ChangeDetectionStrategy, Component, inject, input, output } from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, inject, input, output } from '@angular/core'; import { Router, RouterLink } from '@angular/router'; import { CreateSchemaResponse, FetchAllSchemaResponses, RegistriesSelectors } from '@osf/features/registries/store'; @@ -16,7 +16,6 @@ import { RegistrationReviewStates } from '@osf/shared/enums/registration-review- import { RevisionReviewStates } from '@osf/shared/enums/revision-review-states.enum'; import { UserPermissions } from '@osf/shared/enums/user-permissions.enum'; import { RegistrationCard } from '@osf/shared/models/registration/registration-card.model'; -import { FixSpecialCharPipe } from '@osf/shared/pipes/fix-special-char.pipe'; import { ContributorsListComponent } from '../contributors-list/contributors-list.component'; import { DataResourcesComponent } from '../data-resources/data-resources.component'; @@ -37,7 +36,6 @@ import { TruncatedTextComponent } from '../truncated-text/truncated-text.compone IconComponent, TruncatedTextComponent, ContributorsListComponent, - FixSpecialCharPipe, ], templateUrl: './registration-card.component.html', styleUrl: './registration-card.component.scss', @@ -52,6 +50,7 @@ export class RegistrationCardComponent { readonly deleteDraft = output(); private router = inject(Router); + schemaResponse = select(RegistriesSelectors.getSchemaResponse); actions = createDispatchMap({ @@ -59,46 +58,36 @@ export class RegistrationCardComponent { createSchemaResponse: CreateSchemaResponse, }); - get hasAdminAccess(): boolean { - return this.registrationData().currentUserPermissions.includes(UserPermissions.Admin); - } - - get hasWriteAccess(): boolean { - return this.registrationData().currentUserPermissions.includes(UserPermissions.Write); - } - - get isAccepted(): boolean { - return this.registrationData().reviewsState === RegistrationReviewStates.Accepted; - } - - get isPending(): boolean { - return this.registrationData().reviewsState === RegistrationReviewStates.Pending; - } - - get isApproved(): boolean { - return this.registrationData().revisionState === RevisionReviewStates.Approved; - } + readonly hasAdminAccess = computed(() => + this.registrationData().currentUserPermissions.includes(UserPermissions.Admin) + ); - get isUnapproved(): boolean { - return this.registrationData().revisionState === RevisionReviewStates.Unapproved; - } + readonly hasWriteAccess = computed(() => + this.registrationData().currentUserPermissions.includes(UserPermissions.Write) + ); - get isInProgress(): boolean { - return this.registrationData().revisionState === RevisionReviewStates.RevisionInProgress; - } + readonly isAccepted = computed(() => this.registrationData().reviewsState === RegistrationReviewStates.Accepted); + readonly isPending = computed(() => this.registrationData().reviewsState === RegistrationReviewStates.Pending); + readonly isApproved = computed(() => this.registrationData().revisionState === RevisionReviewStates.Approved); + readonly isUnapproved = computed(() => this.registrationData().revisionState === RevisionReviewStates.Unapproved); + readonly isEmbargo = computed(() => this.registrationData().reviewsState === RegistrationReviewStates.Embargo); - get isEmbargo(): boolean { - return this.registrationData().reviewsState === RegistrationReviewStates.Embargo; - } + readonly isInProgress = computed( + () => this.registrationData().revisionState === RevisionReviewStates.RevisionInProgress + ); - get isRootRegistration(): boolean { + readonly isRootRegistration = computed(() => { const registration = this.registrationData(); return !registration.rootParentId || registration.id === registration.rootParentId; - } + }); - get showButtons(): boolean { - return this.isRootRegistration && (this.isAccepted || this.isPending || this.isEmbargo) && this.hasAdminAccess; - } + readonly showButtons = computed( + () => + this.isRootRegistration() && + (this.isAccepted() || this.isPending() || this.isEmbargo()) && + this.hasAdminAccess() && + this.registrationData().allowUpdates + ); updateRegistration(id: string): void { this.actions @@ -125,11 +114,15 @@ export class RegistrationCardComponent { private navigateToJustificationPage(): void { const revisionId = this.schemaResponse()?.id; + if (!revisionId) return; + this.router.navigate([`/registries/revisions/${revisionId}/justification`]); } private navigateToJustificationReview(): void { const revisionId = this.schemaResponse()?.id; + if (!revisionId) return; + this.router.navigate([`/registries/revisions/${revisionId}/review`]); } } diff --git a/src/app/shared/mappers/registration/registration.mapper.ts b/src/app/shared/mappers/registration/registration.mapper.ts index 41fbe6ef4..740b6f8b1 100644 --- a/src/app/shared/mappers/registration/registration.mapper.ts +++ b/src/app/shared/mappers/registration/registration.mapper.ts @@ -73,6 +73,7 @@ export class RegistrationMapper { public: registration.attributes.public, contributors: ContributorsMapper.getContributors(registration.embeds?.bibliographic_contributors?.data), currentUserPermissions: registration.attributes.current_user_permissions, + allowUpdates: registration.embeds?.provider?.data?.attributes?.allow_updates ?? false, }; } @@ -97,6 +98,7 @@ export class RegistrationMapper { contributors: ContributorsMapper.getContributors(registration?.embeds?.bibliographic_contributors?.data), rootParentId: registration.relationships.root?.data?.id, currentUserPermissions: registration.attributes.current_user_permissions, + allowUpdates: registration.embeds?.provider?.data?.attributes?.allow_updates ?? false, }; } diff --git a/src/app/shared/models/registration/registration-card.model.ts b/src/app/shared/models/registration/registration-card.model.ts index 65c5dd6cd..b3174e8ac 100644 --- a/src/app/shared/models/registration/registration-card.model.ts +++ b/src/app/shared/models/registration/registration-card.model.ts @@ -26,4 +26,5 @@ export interface RegistrationCard { hasSupplements?: boolean; rootParentId?: string | null; currentUserPermissions: UserPermissions[]; + allowUpdates: boolean; } diff --git a/src/app/shared/models/registration/registration-json-api.model.ts b/src/app/shared/models/registration/registration-json-api.model.ts index 1e38892af..db3e46f74 100644 --- a/src/app/shared/models/registration/registration-json-api.model.ts +++ b/src/app/shared/models/registration/registration-json-api.model.ts @@ -5,6 +5,7 @@ import { UserPermissions } from '@osf/shared/enums/user-permissions.enum'; import { ApiData, MetaJsonApi, PaginationLinksJsonApi } from '../common/json-api.model'; import { ContributorDataJsonApi } from '../contributors/contributor-response-json-api.model'; import { LicenseRecordJsonApi } from '../license/licenses-json-api.model'; +import { RegistrationProviderAttributesJsonApi } from '../provider/registration-provider-json-api.model'; export interface DraftRegistrationResponseJsonApi { data: DraftRegistrationDataJsonApi; @@ -138,9 +139,7 @@ export interface RegistrationEmbedsJsonApi { }; provider?: { data: { - attributes: { - name: string; - }; + attributes: RegistrationProviderAttributesJsonApi; }; }; } diff --git a/src/testing/mocks/registration.mock.ts b/src/testing/mocks/registration.mock.ts index 684c0f1c1..a774d3e5a 100644 --- a/src/testing/mocks/registration.mock.ts +++ b/src/testing/mocks/registration.mock.ts @@ -25,4 +25,5 @@ export const MOCK_REGISTRATION: RegistrationCard = { hasPapers: false, hasSupplements: true, currentUserPermissions: [UserPermissions.Admin, UserPermissions.Write, UserPermissions.Read], + allowUpdates: true, };