Skip to content

Commit cdbfda5

Browse files
authored
[ENG-10626] User unable to Update embargoed registration (#959)
- Ticket: https://openscience.atlassian.net/browse/ENG-10626 - Feature flag: n/a ## Summary of Changes 1. Added logic with `allowUpdates`. 2. Fixed freeze in registry update.
1 parent 012ec5b commit cdbfda5

13 files changed

Lines changed: 95 additions & 25 deletions

File tree

src/app/features/metadata/components/metadata-registry-info/metadata-registry-info.component.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ describe('MetadataRegistryInfoComponent', () => {
1919
iri: 'https://example.com/registry',
2020
reviewsWorkflow: 'standard',
2121
allowSubmissions: true,
22+
allowUpdates: true,
2223
};
2324

2425
beforeEach(() => {

src/app/features/registries/components/registry-provider-hero/registry-provider-hero.component.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ describe('RegistryProviderHeroComponent', () => {
3737
iri: '',
3838
reviewsWorkflow: '',
3939
allowSubmissions: false,
40+
allowUpdates: true,
4041
};
4142

4243
beforeEach(() => {

src/app/features/registries/pages/justification/justification.component.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,23 +165,20 @@ export class JustificationComponent implements OnDestroy {
165165

166166
private initStepValidation(): void {
167167
effect(() => {
168-
const currentIndex = this.currentStepIndex();
169-
const pages = this.pages();
170-
const revisionData = this.schemaResponseRevisionData();
171168
const stepState = untracked(() => this.stepsState());
172169

173-
if (currentIndex > 0) {
170+
if (this.currentStepIndex() > 0) {
174171
this.actions.updateStepState('0', true, stepState?.[0]?.touched || false);
175172
}
176173

177-
if (pages.length && currentIndex > 0 && revisionData) {
178-
for (let i = 1; i < currentIndex; i++) {
179-
const pageStep = pages[i - 1];
174+
if (this.pages().length && this.currentStepIndex() > 0 && this.schemaResponseRevisionData()) {
175+
for (let i = 1; i < this.currentStepIndex(); i++) {
176+
const pageStep = this.pages()[i - 1];
180177
const isStepInvalid =
181178
pageStep?.questions?.some((question) => {
182-
const questionData = revisionData[question.responseKey!];
179+
const questionData = this.schemaResponseRevisionData()[question.responseKey!];
183180
return question.required && (Array.isArray(questionData) ? !questionData.length : !questionData);
184-
}) || false;
181+
}) ?? false;
185182
this.actions.updateStepState(i.toString(), isStepInvalid, stepState?.[i]?.touched || false);
186183
}
187184
}

src/app/features/registries/pages/registries-provider-search/registries-provider-search.component.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const MOCK_PROVIDER: RegistryProviderDetails = {
3636
iri: 'http://iri.example.com',
3737
reviewsWorkflow: 'pre-moderation',
3838
allowSubmissions: true,
39+
allowUpdates: true,
3940
};
4041

4142
describe('RegistriesProviderSearchComponent', () => {

src/app/features/registry/components/registry-revisions/registry-revisions.component.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
<p-button
3232
class="w-full"
3333
styleClass="w-full"
34-
(onClick)="updateRegistration.emit(registry()?.id!)"
34+
(onClick)="startUpdateRegistration()"
3535
[label]="'common.buttons.update' | translate"
3636
[loading]="isSubmitting()"
3737
/>

src/app/features/registry/components/registry-revisions/registry-revisions.component.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,28 @@ describe('RegistryRevisionsComponent', () => {
168168
expect(spy).toHaveBeenCalledWith(1);
169169
});
170170

171+
it('should emit updateRegistration with registry id on startUpdateRegistration', () => {
172+
const { component } = setup();
173+
const spy = vi.fn();
174+
component.updateRegistration.subscribe(spy);
175+
176+
component.startUpdateRegistration();
177+
178+
expect(spy).toHaveBeenCalledWith(MOCK_REGISTRY.id);
179+
});
180+
181+
it('should not emit updateRegistration when registry id is missing on startUpdateRegistration', () => {
182+
const { fixture, component } = setup();
183+
const spy = vi.fn();
184+
component.updateRegistration.subscribe(spy);
185+
fixture.componentRef.setInput('registry', { ...MOCK_REGISTRY, id: '' });
186+
fixture.detectChanges();
187+
188+
component.startUpdateRegistration();
189+
190+
expect(spy).not.toHaveBeenCalled();
191+
});
192+
171193
it('should emit continueUpdate on continueUpdateHandler', () => {
172194
const { component } = setup();
173195
const spy = vi.fn();

src/app/features/registry/components/registry-revisions/registry-revisions.component.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ export class RegistryRevisionsComponent {
7575
});
7676
});
7777

78+
startUpdateRegistration() {
79+
const registryId = this.registry()?.id;
80+
81+
if (!registryId) {
82+
return;
83+
}
84+
85+
this.updateRegistration.emit(registryId);
86+
}
87+
7888
emitOpenRevision(index: number) {
7989
this.openRevision.emit(index);
8090
}

src/app/features/registry/pages/registry-overview/registry-overview.component.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ <h4 class="mb-1">
8484
(continueUpdate)="onContinueUpdateRegistration()"
8585
[isModeration]="isModeration()"
8686
[isSubmitting]="isSchemaResponsesLoading()"
87-
[canEdit]="hasAdminAccess()"
87+
[canEdit]="canUpdate()"
8888
>
8989
</osf-registry-revisions>
9090
</div>

src/app/features/registry/pages/registry-overview/registry-overview.component.spec.ts

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { SchemaResponse } from '@osf/shared/models/registration/schema-response.
2020
import { CustomDialogService } from '@osf/shared/services/custom-dialog.service';
2121
import { ToastService } from '@osf/shared/services/toast.service';
2222
import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
23+
import { RegistrationProviderSelectors } from '@osf/shared/stores/registration-provider';
2324

2425
import { MOCK_REGISTRATION_OVERVIEW_MODEL } from '@testing/mocks/registration-overview-model.mock';
2526
import { createMockSchemaResponse } from '@testing/mocks/schema-response.mock';
@@ -28,7 +29,7 @@ import { CustomDialogServiceMock } from '@testing/providers/custom-dialog-provid
2829
import { LoaderServiceMock, provideLoaderServiceMock } from '@testing/providers/loader-service.mock';
2930
import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock';
3031
import { RouterMockBuilder } from '@testing/providers/router-provider.mock';
31-
import { provideMockStore } from '@testing/providers/store-provider.mock';
32+
import { BaseSetupOverrides, mergeSignalOverrides, provideMockStore } from '@testing/providers/store-provider.mock';
3233
import { ToastServiceMock } from '@testing/providers/toast-provider.mock';
3334
import { ViewOnlyLinkHelperMock } from '@testing/providers/view-only-link-helper.mock';
3435

@@ -44,7 +45,7 @@ import { RegistrySelectors } from '../../store/registry';
4445

4546
import { RegistryOverviewComponent } from './registry-overview.component';
4647

47-
interface SetupOverrides {
48+
interface SetupOverrides extends BaseSetupOverrides {
4849
registry?: RegistrationOverviewModel | null;
4950
schemaResponses?: SchemaResponse[];
5051
queryParams?: Record<string, string>;
@@ -67,6 +68,19 @@ function setup(overrides: SetupOverrides = {}) {
6768
const mockLoaderService = new LoaderServiceMock();
6869
const mockToastService = ToastServiceMock.simple();
6970
const mockViewOnlyHelper = ViewOnlyLinkHelperMock.simple(overrides.hasViewOnly);
71+
const signalDefaults = [
72+
{ selector: RegistrySelectors.getRegistry, value: registry },
73+
{ selector: RegistrySelectors.isRegistryLoading, value: false },
74+
{ selector: RegistrySelectors.isRegistryAnonymous, value: false },
75+
{ selector: RegistrySelectors.getSchemaResponses, value: schemaResponses },
76+
{ selector: RegistrySelectors.isSchemaResponsesLoading, value: false },
77+
{ selector: RegistrySelectors.getSchemaBlocks, value: [] },
78+
{ selector: RegistrySelectors.isSchemaBlocksLoading, value: false },
79+
{ selector: RegistrySelectors.areReviewActionsLoading, value: false },
80+
{ selector: RegistrySelectors.getSchemaResponse, value: schemaResponses[0] ?? null },
81+
{ selector: RegistrySelectors.hasAdminAccess, value: false },
82+
{ selector: RegistrationProviderSelectors.allowUpdates, value: false },
83+
];
7084

7185
TestBed.configureTestingModule({
7286
imports: [
@@ -94,18 +108,7 @@ function setup(overrides: SetupOverrides = {}) {
94108
MockProvider(ToastService, mockToastService),
95109
MockProvider(ViewOnlyLinkHelperService, mockViewOnlyHelper),
96110
provideMockStore({
97-
signals: [
98-
{ selector: RegistrySelectors.getRegistry, value: registry },
99-
{ selector: RegistrySelectors.isRegistryLoading, value: false },
100-
{ selector: RegistrySelectors.isRegistryAnonymous, value: false },
101-
{ selector: RegistrySelectors.getSchemaResponses, value: schemaResponses },
102-
{ selector: RegistrySelectors.isSchemaResponsesLoading, value: false },
103-
{ selector: RegistrySelectors.getSchemaBlocks, value: [] },
104-
{ selector: RegistrySelectors.isSchemaBlocksLoading, value: false },
105-
{ selector: RegistrySelectors.areReviewActionsLoading, value: false },
106-
{ selector: RegistrySelectors.getSchemaResponse, value: schemaResponses[0] ?? null },
107-
{ selector: RegistrySelectors.hasAdminAccess, value: false },
108-
],
111+
signals: mergeSignalOverrides(signalDefaults, overrides.selectorOverrides),
109112
}),
110113
],
111114
});
@@ -181,6 +184,30 @@ describe('RegistryOverviewComponent', () => {
181184
expect(component.canMakeDecision()).toBe(false);
182185
});
183186

187+
it('should compute canUpdate as true when admin access and provider updates are allowed', () => {
188+
const { component } = setup({
189+
registry: MOCK_REGISTRATION_OVERVIEW_MODEL,
190+
selectorOverrides: [
191+
{ selector: RegistrySelectors.hasAdminAccess, value: true },
192+
{ selector: RegistrationProviderSelectors.allowUpdates, value: true },
193+
],
194+
});
195+
196+
expect(component.canUpdate()).toBe(true);
197+
});
198+
199+
it('should compute canUpdate as false when provider updates are not allowed', () => {
200+
const { component } = setup({
201+
registry: MOCK_REGISTRATION_OVERVIEW_MODEL,
202+
selectorOverrides: [
203+
{ selector: RegistrySelectors.hasAdminAccess, value: true },
204+
{ selector: RegistrationProviderSelectors.allowUpdates, value: false },
205+
],
206+
});
207+
208+
expect(component.canUpdate()).toBe(false);
209+
});
210+
184211
it('should compute isInitialState from reviewsState', () => {
185212
const { component } = setup({
186213
registry: { ...MOCK_REGISTRATION_OVERVIEW_MODEL, reviewsState: RegistrationReviewStates.Initial },

src/app/features/registry/pages/registry-overview/registry-overview.component.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { ToastService } from '@osf/shared/services/toast.service';
3737
import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
3838
import { GetBookmarksCollectionId } from '@osf/shared/stores/bookmarks';
3939
import { GetBibliographicContributors } from '@osf/shared/stores/contributors';
40+
import { RegistrationProviderSelectors } from '@osf/shared/stores/registration-provider';
4041

4142
import { ArchivingMessageComponent } from '../../components/archiving-message/archiving-message.component';
4243
import { RegistrationOverviewToolbarComponent } from '../../components/registration-overview-toolbar/registration-overview-toolbar.component';
@@ -98,6 +99,7 @@ export class RegistryOverviewComponent implements OnInit, OnDestroy {
9899
readonly areReviewActionsLoading = select(RegistrySelectors.areReviewActionsLoading);
99100
readonly currentRevision = select(RegistrySelectors.getSchemaResponse);
100101
readonly hasAdminAccess = select(RegistrySelectors.hasAdminAccess);
102+
readonly allowUpdates = select(RegistrationProviderSelectors.allowUpdates);
101103

102104
readonly selectedRevisionIndex = signal(0);
103105

@@ -112,6 +114,8 @@ export class RegistryOverviewComponent implements OnInit, OnDestroy {
112114
() => !this.registry()?.archiving && !this.registry()?.withdrawn && this.isModeration()
113115
);
114116

117+
readonly canUpdate = computed(() => this.hasAdminAccess() && this.allowUpdates());
118+
115119
isRootRegistration = computed(() => {
116120
const rootId = this.registry()?.rootParentId;
117121
return !rootId || rootId === this.registry()?.id;

0 commit comments

Comments
 (0)