Refactor useFormDataQuery #3588
Conversation
cammiida
added
kind/other
cammiida
force-pushed
the
refactor/useFormDataQuery
branch
2 times, most recently
from
5ef1fd0 to
7b9668e
Compare
cammiida
force-pushed
the
refactor/useFormDataQuery
branch
from
d590884 to
0b420cc
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
cammiida
force-pushed
the
refactor/useFormDataQuery
branch
from
0324de2 to
83394a5
Compare
|
cammiida
force-pushed
the
refactor/useFormDataQuery
branch
from
83394a5 to
3d65218
Compare
📝 WalkthroughWalkthroughRefactors form-data fetching to a parameterized React Query system, replaces URL-based hooks, updates cache keying, and adjusts components to use instanceId-aware queries. Introduces Signing “OnBehalfOf” type and related API changes. Consolidates stateless URL builders. Removes legacy helpers and test harness features. Broad test updates mock fetchFormData centrally. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/datamodel/useBindingSchema.tsx (1)
31-40: Fix stateless flow — selector is skipped when query is disableduseInstanceDataQuery({ select }) is disabled in stateless apps so the selector never runs and the hook returns undefined; getCurrentTaskDataElementId expects instance data and may throw.
- Change src/features/datamodel/useBindingSchema.tsx to stop relying on the query's select callback. Read the instance directly: const { data: instance } = useInstanceDataQuery(); then:
- if (overriddenDataElementId) return overriddenDataElementId;
- return getCurrentTaskDataElementId({ application, dataElements: instance?.data ?? [], taskId, layoutSets });
- OR, alternatively, make getCurrentTaskDataElementId tolerant of undefined dataElements and handle the fallback there.
- Add a unit test for the stateless case asserting the hook returns the expected dataElementId (or the intended fallback) when no instance is present.
🧹 Nitpick comments (38)
src/features/validation/expressionValidation/ExpressionValidation.test.tsx (1)
71-76: Add cleanup to avoid cross‑test leakage of spies.Centralize restoration to prevent mocks from leaking across parameterized tests.
beforeEach(() => { jest.spyOn(FD, 'useDebounced').mockRestore(); jest.spyOn(DataModels, 'useExpressionValidationConfig').mockRestore(); jest.spyOn(Validation, 'useUpdateDataModelValidations').mockRestore(); }); + +afterEach(() => { + jest.restoreAllMocks(); +});src/layout/SigneeList/api.ts (1)
58-58: AvoidtoSortedunless you guarantee ES2023/polyfill
Array.prototype.toSortedmay not be available in all runtimes. Use a non‑mutating copy + sort for broader compatibility.Apply this diff:
- return parsed.signeeStates.toSorted((a, b) => (a.name ?? '').localeCompare(b.name ?? '')); + return [...parsed.signeeStates].sort((a, b) => (a.name ?? '').localeCompare(b.name ?? ''));src/layout/SigningActions/api.ts (1)
88-88: Guard for missing taskId before optimistic update
taskIdis part of the query key; if absent,setQueryDatawill target a mismatched key. Early‑return or skip the optimistic update whentaskIdis falsy (see diff above).src/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsx (1)
149-155: Align with chooser: parse JSON that’s either object or nullAfter adjusting the chooser to emit JSON strings, keep
JSON.parsebut ensure it maps toOnBehalfOf | null. Optional: add a tiny guard to avoid runtime errors from unexpected values.Apply this diff:
- onChange={(e) => { - setOnBehalfOf(JSON.parse(e.target.value)); + onChange={(e) => { + const v = e.target.value; + // v is "null" or a JSON object string + setOnBehalfOf((v === 'null' ? null : JSON.parse(v)) as OnBehalfOf | null); setOnBehalfOfError(false); }}src/layout/Likert/LikertTestUtils.tsx (1)
129-131: Drop any in AxiosResponse cast.Use unknown instead of any to align with TS hygiene.
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - fetchOptions: async () => ({ data: mockOptions, headers: {} }) as AxiosResponse<IRawOption[], any>, + fetchOptions: async () => + ({ data: mockOptions, headers: {} }) as AxiosResponse<IRawOption[], unknown>,src/layout/Map/MapComponent.test.tsx (1)
56-59: Prefer resilient mocking for multiple calls and isolate between tests.If the component triggers more than one form-data fetch, mockImplementationOnce may be insufficient and lead to flakes. Consider mockResolvedValueOnce for each expected call or set a per-test default and reset in afterEach.
- jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ + jest.mocked(fetchFormData).mockResolvedValueOnce({ myCoords: '59.2641592,10.4036248', -})); +});src/layout/MultipleSelect/MultipleSelectComponent.test.tsx (1)
42-42: Same note: one-time mock may be brittle if fetch count changes.Use mockResolvedValueOnce or add afterEach(() => jest.resetAllMocks()) to keep tests hermetic.
src/features/options/useGetOptions.ts (2)
1-1: Re-introduce useCallback for setData; import was removed.Without memoization, setData identity changes every render, defeating downstream memoization.
-import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useCallback } from 'react';
81-91: Memoize derived values and setData to reduce churn.selectedValues/currentValues and setData now change each render; restore stable identities.
- const currentValues = value && value.length > 0 ? (valueType === 'multi' ? value.split(',') : [value]) : []; - const selectedValues = currentValues.filter((value) => options.find((option) => option.value === value)); - - function setData(values: string[]) { - if (valueType === 'single') { - setValue('simpleBinding', values.at(0)); - } else if (valueType === 'multi') { - setValue('simpleBinding', values.join(',')); - } - } + const currentValues = useMemo( + () => (value && value.length > 0 ? (valueType === 'multi' ? value.split(',') : [value]) : []), + [value, valueType], + ); + const selectedValues = useMemo( + () => currentValues.filter((v) => options.some((o) => o.value === v)), + [currentValues, options], + ); + const setData = useCallback( + (values: string[]) => { + if (valueType === 'single') { + setValue('simpleBinding', values.at(0)); + } else { + setValue('simpleBinding', values.join(',')); + } + }, + [setValue, valueType], + );src/layout/Input/InputComponent.test.tsx (2)
22-23: Ensure fetchFormData is a real Jest mock/spy before calling mockImplementationOnce.jest.mocked(...) is only a type helper; without a module mock or spy, these calls will be no-ops or fail. Either spy on the module export or explicitly mock the module.
Recommended approach (spy):
- import { fetchFormData } from 'src/queries/queries'; + import * as Queries from 'src/queries/queries';- jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ some: { field: 'some value' } })); + jest.spyOn(Queries, 'fetchFormData').mockResolvedValueOnce({ some: { field: 'some value' } });Repeat similarly for other occurrences in this file.
Also applies to: 60-61, 171-172, 190-191
13-19: Stabilize tests by resetting mocks between cases.Prevents order coupling when using mockImplementationOnce across multiple tests.
describe('InputComponent', () => { + afterEach(() => { + jest.resetAllMocks(); + });src/layout/TextArea/TextAreaComponent.test.tsx (1)
15-19: Back the jest.mocked(...) calls with a real mock/spy.As above, switch to a spy or mock the module to guarantee these per-test overrides apply.
- import { fetchFormData } from 'src/queries/queries'; + import * as Queries from 'src/queries/queries'; - jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ myTextArea: initialText })); + jest.spyOn(Queries, 'fetchFormData').mockResolvedValueOnce({ myTextArea: initialText });Also applies to: 29-33, 47-49
src/features/validation/ValidationPlugin.test.tsx (1)
42-44: Confirm fetchFormData is mocked/spied before using mockImplementationOnce.Mirror the spy-based pattern to avoid silent failures.
- import { fetchFormData } from 'src/queries/queries'; + import * as Queries from 'src/queries/queries'; - jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ TextField: text })); + jest.spyOn(Queries, 'fetchFormData').mockResolvedValueOnce({ TextField: text });src/utils/urls/appUrlHelper.ts (1)
51-73: Keep URL param construction consistent across helpers.Most helpers use getQueryStringFromObject; consider standardizing for consistency and easier testing.
src/components/form/Form.tsx (1)
242-242: Remove unused dependency/variable.onFormSubmitValidation isn’t used in this component; it unnecessarily triggers the effect.
- const onFormSubmitValidation = useOnFormSubmitValidation(); ... - }, [navigate, searchStringRef, exitSubform, validate, onFormSubmitValidation, queryClient]); + }, [navigate, searchStringRef, exitSubform, validate, queryClient]);Also applies to: 263-263
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx (1)
151-153: Use mockImplementationOnce and reset mocks to avoid cross-test bleed.Make this per-invocation or reset after each test; current persistent implementation can affect other suites.
- jest.mocked(fetchFormData).mockImplementation(async () => ({ + jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ group: [{ field1: 'field1-row0', field2: 'field2-row0', field3: 'field3-row0', [ALTINN_ROW_ID]: 'abc123' }], }));Also add:
afterEach(() => { jest.resetAllMocks(); });src/features/expressions/shared-context.test.tsx (1)
96-98: Use mockImplementationOnce for per-case mocks; queries module is already globally mocked.
- Replace in src/features/expressions/shared-context.test.tsx (around lines 96–98):
- jest.mocked(fetchFormData).mockImplementation(async () => dataModel ?? {}); + jest.mocked(fetchFormData).mockImplementationOnce(async () => dataModel ?? {});
- Confirmation: src/setupTests.ts already mocks 'src/queries/queries' at line 115, so no additional module-mocking setup is needed.
src/components/form/Form.test.tsx (1)
228-237: Prefer mockImplementationOnce or reset mocks between testsThe test sets a persistent fetchFormData mock; src/setupTests.ts defines a global fetchFormData mock and jest.config.js does not enable resetMocks, so the persistent implementation can leak across tests.
- Replace the local mock in src/components/form/Form.test.tsx (around lines 228–237):
- jest.mocked(fetchFormData).mockImplementation(async () => ({ + jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ Group: [ { prop1: 'value1', prop2: 'value2', prop3: 'value3', }, ], }));
- Or enable global reset by adding to src/setupTests.ts:
afterEach(() => { jest.resetAllMocks(); });src/layout/Group/SummaryGroupComponent.test.tsx (1)
23-28: fetchFormData is already a jest.fn in setupTests — add per-file mock resetsetupTests.ts mocks src/queries/queries and defines fetchFormData as jest.fn (src/setupTests.ts:115–126, fetchFormData at line 125). Add a per-file reset to avoid cross-test leakage: in src/layout/Group/SummaryGroupComponent.test.tsx add
afterEach(() => { jest.resetAllMocks(); });
src/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsx (1)
177-186: Fix key casing: checkboxBindingData model binding expects 'checkboxBinding' (lowercase b). Mock uses 'checkBoxBinding', which may break any code reading this field.
Apply:
- { [ALTINN_ROW_ID]: uuidv4(), checkBoxBinding: 'option.value', prop1: 'test row 0' }, - { [ALTINN_ROW_ID]: uuidv4(), checkBoxBinding: 'option.value', prop1: 'test row 1' }, - { [ALTINN_ROW_ID]: uuidv4(), checkBoxBinding: 'option.value', prop1: 'test row 2' }, - { [ALTINN_ROW_ID]: uuidv4(), checkBoxBinding: 'option.value', prop1: 'test row 3' }, + { [ALTINN_ROW_ID]: uuidv4(), checkboxBinding: 'option.value', prop1: 'test row 0' }, + { [ALTINN_ROW_ID]: uuidv4(), checkboxBinding: 'option.value', prop1: 'test row 1' }, + { [ALTINN_ROW_ID]: uuidv4(), checkboxBinding: 'option.value', prop1: 'test row 2' }, + { [ALTINN_ROW_ID]: uuidv4(), checkboxBinding: 'option.value', prop1: 'test row 3' },src/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsx (1)
16-16: Confirm mocking strategy for fetchFormDataSame note: jest.mocked(fetchFormData) requires a prior mock/spy. If there’s no global mock in setup, prefer spying on the module object.
Example:
-import { fetchFormData } from 'src/queries/queries'; +import * as queries from 'src/queries/queries'; ... - jest.mocked(fetchFormData).mockImplementation(async () => ({ + jest.spyOn(queries, 'fetchFormData').mockImplementation(async () => ({ MyGroup: existingRows ?? [], }));Also applies to: 77-80
src/utils/layout/all.test.tsx (1)
16-16: Stabilize query mocking via spy to avoid ESM mocking pitfallsIf the queries module isn’t auto-mocked, jest.mocked(fetchFormData) will not be a jest.fn. Use jest.spyOn for robustness across ESM.
Example:
-import { fetchApplicationMetadata, fetchFormData, fetchInstanceData, fetchProcessState } from 'src/queries/queries'; +import * as queries from 'src/queries/queries'; ... - jest.mocked(fetchApplicationMetadata).mockImplementation(async () => set.app.getAppMetadata()); - jest.mocked(fetchProcessState).mockImplementation(async () => mainSet.simulateProcessData()); - jest.mocked(fetchInstanceData).mockImplementation(async () => set.simulateInstance()); - jest.mocked(fetchFormData).mockImplementation(async (url) => set.getModel({ url }).simulateDataModel()); + jest.spyOn(queries, 'fetchApplicationMetadata').mockImplementation(async () => set.app.getAppMetadata()); + jest.spyOn(queries, 'fetchProcessState').mockImplementation(async () => mainSet.simulateProcessData()); + jest.spyOn(queries, 'fetchInstanceData').mockImplementation(async () => set.simulateInstance()); + jest.spyOn(queries, 'fetchFormData').mockImplementation(async (url) => set.getModel({ url }).simulateDataModel());Also applies to: 151-151
src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsx (1)
3-3: Make fetchFormData a spy/mocked export before overridingMirror the spy-based approach to ensure .mockImplementation is valid under ESM.
Example:
-import { fetchFormData } from 'src/queries/queries'; +import * as queries from 'src/queries/queries'; ... - jest.mocked(fetchFormData).mockImplementation(async () => ({ + jest.spyOn(queries, 'fetchFormData').mockImplementation(async () => ({ multipageGroup: [ { [ALTINN_ROW_ID]: 'abc123', prop1: 'prop1', prop2: 'prop2', prop3: 'prop3' }, { [ALTINN_ROW_ID]: 'def456', prop1: 'prop4', prop2: 'prop5', prop3: 'prop6' }, ], }));Also applies to: 17-17, 88-103
src/layout/Datepicker/DatepickerComponent.test.tsx (1)
9-9: Scope the one-off form-data mock safelySame ESM consideration: prefer spying on the module export for the one-time mock.
Example:
-import { fetchFormData } from 'src/queries/queries'; +import * as queries from 'src/queries/queries'; ... - jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ myDate: '2022-12-31' })); + jest.spyOn(queries, 'fetchFormData').mockImplementationOnce(async () => ({ myDate: '2022-12-31' }));Also applies to: 124-126
src/layout/Dropdown/DropdownComponent.test.tsx (2)
12-12: Default mock should use a spy for durabilityFor consistency and to avoid relying on global auto-mocking, spy on queries.fetchFormData.
Example:
-import { fetchFormData } from 'src/queries/queries'; +import * as queries from 'src/queries/queries'; ... - jest.mocked(fetchFormData).mockImplementation(async () => ({ + jest.spyOn(queries, 'fetchFormData').mockImplementation(async () => ({ ...getFormDataMockForRepGroup(), }));Also applies to: 51-54
309-310: One-off mock: switch to spyAligns with the default spy approach in this file.
- jest.mocked(fetchFormData).mockImplementationOnce(async () => ({ simpleBinding: '', label: '', metadata: '' })); + jest.spyOn(queries, 'fetchFormData').mockImplementationOnce(async () => ({ simpleBinding: '', label: '', metadata: '' }));src/features/expressions/shared-functions.test.tsx (1)
324-339: Use URL parsing instead of brittle regexesMakes the mock robust to ordering/casing and future URL changes; also handles uppercase GUIDs.
- jest.mocked(fetchFormData).mockImplementation(async (url: string) => { - if (!dataModels) { - return dataModel ?? {}; - } - - const statelessDataType = url.match(/dataType=([\w-]+)&/)?.[1]; - const statefulDataElementId = url.match(/data\/([a-f0-9-]+)\?/)?.[1]; - - const model = dataModels.find( - (dm) => dm.dataElement.dataType === statelessDataType || dm.dataElement.id === statefulDataElementId, - ); - if (model) { - return model.data; - } - throw new Error(`Datamodel ${url} not found in ${JSON.stringify(dataModels)}`); - }); + jest.spyOn(queries, 'fetchFormData').mockImplementation(async (url: string) => { + if (!dataModels) return dataModel ?? {}; + const u = new URL(url, 'http://localhost'); + const statelessDataType = u.searchParams.get('dataType') ?? undefined; + const statefulDataElementId = u.pathname.match(/\/data\/([^/?#]+)/)?.[1]; + const model = dataModels.find( + (dm) => dm.dataElement.dataType === statelessDataType || dm.dataElement.id === statefulDataElementId, + ); + if (model) return model.data; + throw new Error(`Datamodel ${url} not found in ${JSON.stringify(dataModels)}`); + });If not already, change the import to a module namespace:
-import { - fetchApplicationMetadata, - fetchFormData, - fetchInstanceData, - fetchProcessState, - fetchUserProfile, -} from 'src/queries/queries'; +import * as queries from 'src/queries/queries';And update other mocks similarly with jest.spyOn(queries, ...).
src/layout/FileUpload/FileUploadComponent.test.tsx (2)
492-510: Initialize id before using it inside the mock closureMinor ordering nit: the fetchApplicationMetadata mock uses id from the outer scope. Move id creation above the mock for clarity and to avoid subtle ordering pitfalls.
Apply this diff:
- jest.mocked(fetchApplicationMetadata).mockImplementationOnce(async () => - getIncomingApplicationMetadataMock((a) => { - a.dataTypes.push({ - id, - allowedContentTypes: ['image/png'], - maxCount: 4, - minCount: 1, - }); - }), - ); - jest.mocked(fetchInstanceData).mockImplementation(async () => - getInstanceDataMock((i) => { - i.data.push(...attachments); - }), - ); - - const id = uuidv4(); - const attachments = attachmentsGenerator(id); + const id = uuidv4(); + const attachments = attachmentsGenerator(id); + jest.mocked(fetchApplicationMetadata).mockImplementationOnce(async () => + getIncomingApplicationMetadataMock((a) => { + a.dataTypes.push({ + id, + allowedContentTypes: ['image/png'], + maxCount: 4, + minCount: 1, + }); + }), + ); + jest.mocked(fetchInstanceData).mockImplementation(async () => + getInstanceDataMock((i) => { + i.data.push(...attachments); + }), + );
319-321: Avoid wrapping findByRole in waitForfindByRole already waits; wrapping it in waitFor adds noise.
Apply this diff:
- const saveButton = await waitFor(() => screen.findByRole('button', { name: 'Lagre' })); + const saveButton = await screen.findByRole('button', { name: 'Lagre' });src/features/formData/FormDataReaders.test.tsx (3)
61-74: Don’t assume absolute URLs in testsnew URL(url) throws for relative URLs. Make parsing resilient so future changes to the URL builder don’t break tests.
Apply this diff:
- jest.mocked(fetchFormData).mockImplementation(async (url) => { - const path = new URL(url).pathname; + jest.mocked(fetchFormData).mockImplementation(async (url) => { + const path = (() => { + try { + return new URL(url).pathname; + } catch { + return new URL(url, 'http://localhost').pathname; + } + })();
196-205: Restore real timers after using fake timersThis test enables fake timers but doesn’t restore them, which can leak across tests.
Apply this diff:
- it('advanced, should fetch data from multiple models, handle failures', async () => { - jest.useFakeTimers(); + it('advanced, should fetch data from multiple models, handle failures', async () => { + jest.useFakeTimers(); const missingError = new Error('This should fail when fetching'); - (missingError as AxiosError).isAxiosError = true; + (missingError as AxiosError).isAxiosError = true; const model2Promise = new Promise((resolve) => { setTimeout(() => resolve({ name: 'Universe' }), 100); }); - await render({ + try { + await render({ ids: ['test1', 'test2', 'test3', 'testDefault', 'testMissing', 'testMissingWithDefault'], /* ...rest of props... */ - }); + }); + } finally { + jest.useRealTimers(); + }Also applies to: 199-199
112-120: Remove unused urlFor helperurlFor is no longer used. Drop it to reduce confusion.
Apply this diff:
- function urlFor(dataModelName: string) { - for (const [uuid, name] of Object.entries(idToNameMap)) { - if (name === dataModelName) { - return `https://local.altinn.cloud/ttd/test/instances/${instanceId}/data/${uuid}?includeRowId=true&language=nb`; - } - } - return false; - } @@ - return { ...utils, urlFor }; + return { ...utils };Also applies to: 144-145
src/setupTests.ts (1)
115-126: Also mock fetchExternalApi to prevent real network calls in testsIt’s included in IgnoredQueriesAndMutations but left unmocked; if reached, tests could hit the network.
Apply this diff:
import type { doProcessNext, fetchApplicationMetadata, + fetchExternalApi, fetchFormData, fetchInstanceData, fetchProcessState, fetchUserProfile, } from 'src/queries/queries'; @@ fetchUserProfile: jest.fn<typeof fetchUserProfile>(async () => getProfileMock()), fetchInstanceData: jest.fn<typeof fetchInstanceData>(async () => getInstanceDataMock()), fetchFormData: jest.fn<typeof fetchFormData>(async () => ({})), + fetchExternalApi: jest.fn<typeof fetchExternalApi>(async () => ({} as unknown)),If you intentionally want fetchExternalApi to be real during tests, disregard this and confirm no test paths call it.
src/features/datamodel/DataModelsProvider.tsx (1)
340-340: Trim effect deps to avoid unnecessary rerunsmetaData.id isn’t used inside the effect; removing it avoids redundant re-runs when app metadata changes.
Apply this diff:
- }, [data, dataType, metaData.id, setInitialData]); + }, [data, dataType, setInitialData]);src/features/formData/FormDataWrite.tsx (1)
112-116: Defensive: the dataType check is redundantnewDataModels entries always include dataType in all code paths. You can drop the guard.
Apply this diff:
- if (!dataType) { - continue; - }src/features/formData/FormDataReaders.tsx (1)
106-110: Memoize availableModels and guard against transient undefinedavailableModels is recomputed every render and changes identity, which can trigger downstream renders. Wrap in useMemo and defensively handle missing metadata (during initial render) to avoid churn.
Apply:
- const dataTypes = useApplicationMetadata().dataTypes; - const availableModels = dataTypes - .filter((dataType) => getDataTypeVariant(dataType) === DataTypeVariant.DataModel) - .map((dm) => dm.id); + const dataTypes = useApplicationMetadata().dataTypes; + const availableModels = useMemo( + () => + (dataTypes ?? []) + .filter((dataType) => getDataTypeVariant(dataType) === DataTypeVariant.DataModel) + .map((dm) => dm.id), + [dataTypes], + );src/features/formData/useFormDataQuery.tsx (2)
85-93: Use HTTP status for 403 detection, not message matchingMatching '403' in error.message is brittle. Prefer response.status.
Apply:
- if (error && isAxiosError(error)) { - if (error.message?.includes('403')) { + if (error && isAxiosError(error)) { + if (error.response?.status === 403) { window.logInfo('Current party is missing roles'); } else { window.logError('Fetching form data failed:\n', error); }
99-107: Type‑safe access to RequiredAuthenticationLevelGuard response.data before indexing to avoid runtime/type issues with unknown payloads.
Apply:
-async function maybeAuthenticationRedirect(error: AxiosError) { - if (error.response && error.response.status === 403 && error.response.data) { - const reqAuthLevel = error.response.data['RequiredAuthenticationLevel']; +async function maybeAuthenticationRedirect(error: AxiosError) { + if (error.response && error.response.status === 403 && error.response.data) { + const data = error.response.data as { RequiredAuthenticationLevel?: string } | undefined; + const reqAuthLevel = data?.RequiredAuthenticationLevel; if (reqAuthLevel) { await putWithoutConfig(invalidateCookieUrl); redirectToUpgrade(reqAuthLevel); } } }If you want stricter safety across the file, add a top‑level type:
type AuthErrorData = { RequiredAuthenticationLevel?: string };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
src/components/form/Form.test.tsx(2 hunks)src/components/form/Form.tsx(4 hunks)src/features/datamodel/DataModelsProvider.tsx(3 hunks)src/features/datamodel/useAvailableDataModels.tsx(0 hunks)src/features/datamodel/useBindingSchema.tsx(2 hunks)src/features/expressions/shared-context.test.tsx(2 hunks)src/features/expressions/shared-functions.test.tsx(2 hunks)src/features/formData/FormData.test.tsx(5 hunks)src/features/formData/FormDataReaders.test.tsx(4 hunks)src/features/formData/FormDataReaders.tsx(3 hunks)src/features/formData/FormDataWrite.tsx(2 hunks)src/features/formData/useDataModelBindings.test.tsx(2 hunks)src/features/formData/useDataModelBindings.ts(1 hunks)src/features/formData/useFormDataQuery.tsx(1 hunks)src/features/navigation/AppNavigation.test.tsx(2 hunks)src/features/options/useGetOptions.test.tsx(2 hunks)src/features/options/useGetOptions.ts(2 hunks)src/features/receipt/ReceiptContainer.test.tsx(0 hunks)src/features/validation/ValidationPlugin.test.tsx(3 hunks)src/features/validation/expressionValidation/ExpressionValidation.test.tsx(2 hunks)src/layout/Address/AddressComponent.test.tsx(5 hunks)src/layout/Checkboxes/CheckboxesContainerComponent.test.tsx(6 hunks)src/layout/Datepicker/DatepickerComponent.test.tsx(2 hunks)src/layout/Dropdown/DropdownComponent.test.tsx(3 hunks)src/layout/FileUpload/FileUploadComponent.test.tsx(6 hunks)src/layout/Group/SummaryGroupComponent.test.tsx(2 hunks)src/layout/Input/InputComponent.test.tsx(5 hunks)src/layout/Likert/LikertTestUtils.tsx(3 hunks)src/layout/Map/MapComponent.test.tsx(2 hunks)src/layout/MultipleSelect/MultipleSelectComponent.test.tsx(2 hunks)src/layout/RadioButtons/ControlledRadioGroup.test.tsx(4 hunks)src/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsx(5 hunks)src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsx(3 hunks)src/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsx(2 hunks)src/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsx(2 hunks)src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx(3 hunks)src/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsx(4 hunks)src/layout/SigneeList/api.ts(1 hunks)src/layout/SigningActions/OnBehalfOfChooser.tsx(2 hunks)src/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsx(4 hunks)src/layout/SigningActions/api.ts(2 hunks)src/layout/Subform/utils.ts(1 hunks)src/layout/TextArea/TextAreaComponent.test.tsx(3 hunks)src/queries/types.ts(1 hunks)src/setupTests.ts(2 hunks)src/test/renderWithProviders.tsx(1 hunks)src/utils/layout/all.test.tsx(2 hunks)src/utils/maybeAuthenticationRedirect.ts(0 hunks)src/utils/urls/appUrlHelper.ts(1 hunks)test/e2e/integration/subform-test/subform.ts(1 hunks)
💤 Files with no reviewable changes (3)
- src/features/receipt/ReceiptContainer.test.tsx
- src/features/datamodel/useAvailableDataModels.tsx
- src/utils/maybeAuthenticationRedirect.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganyand unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options viaqueryOptions
Files:
src/features/options/useGetOptions.test.tsxsrc/layout/Map/MapComponent.test.tsxsrc/components/form/Form.tsxsrc/features/formData/useDataModelBindings.test.tsxtest/e2e/integration/subform-test/subform.tssrc/layout/Likert/LikertTestUtils.tsxsrc/layout/Subform/utils.tssrc/features/validation/ValidationPlugin.test.tsxsrc/layout/Group/SummaryGroupComponent.test.tsxsrc/layout/Datepicker/DatepickerComponent.test.tsxsrc/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsxsrc/features/formData/useDataModelBindings.tssrc/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsxsrc/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsxsrc/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsxsrc/features/validation/expressionValidation/ExpressionValidation.test.tsxsrc/layout/TextArea/TextAreaComponent.test.tsxsrc/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsxsrc/layout/FileUpload/FileUploadComponent.test.tsxsrc/components/form/Form.test.tsxsrc/utils/layout/all.test.tsxsrc/layout/Input/InputComponent.test.tsxsrc/features/options/useGetOptions.tssrc/queries/types.tssrc/layout/MultipleSelect/MultipleSelectComponent.test.tsxsrc/test/renderWithProviders.tsxsrc/features/formData/FormDataReaders.tsxsrc/features/formData/FormDataWrite.tsxsrc/features/navigation/AppNavigation.test.tsxsrc/layout/RadioButtons/ControlledRadioGroup.test.tsxsrc/features/expressions/shared-context.test.tsxsrc/layout/SigningActions/api.tssrc/layout/Dropdown/DropdownComponent.test.tsxsrc/layout/Address/AddressComponent.test.tsxsrc/features/expressions/shared-functions.test.tsxsrc/utils/urls/appUrlHelper.tssrc/layout/Checkboxes/CheckboxesContainerComponent.test.tsxsrc/features/formData/useFormDataQuery.tsxsrc/layout/SigningActions/OnBehalfOfChooser.tsxsrc/features/formData/FormData.test.tsxsrc/setupTests.tssrc/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsxsrc/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsxsrc/layout/SigneeList/api.tssrc/features/datamodel/useBindingSchema.tsxsrc/features/formData/FormDataReaders.test.tsxsrc/features/datamodel/DataModelsProvider.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, use
renderWithProvidersfromsrc/test/renderWithProviders.tsxto supply required form layout context
Files:
src/features/options/useGetOptions.test.tsxsrc/layout/Map/MapComponent.test.tsxsrc/features/formData/useDataModelBindings.test.tsxsrc/features/validation/ValidationPlugin.test.tsxsrc/layout/Group/SummaryGroupComponent.test.tsxsrc/layout/Datepicker/DatepickerComponent.test.tsxsrc/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsxsrc/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsxsrc/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsxsrc/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsxsrc/features/validation/expressionValidation/ExpressionValidation.test.tsxsrc/layout/TextArea/TextAreaComponent.test.tsxsrc/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsxsrc/layout/FileUpload/FileUploadComponent.test.tsxsrc/components/form/Form.test.tsxsrc/utils/layout/all.test.tsxsrc/layout/Input/InputComponent.test.tsxsrc/layout/MultipleSelect/MultipleSelectComponent.test.tsxsrc/features/navigation/AppNavigation.test.tsxsrc/layout/RadioButtons/ControlledRadioGroup.test.tsxsrc/features/expressions/shared-context.test.tsxsrc/layout/Dropdown/DropdownComponent.test.tsxsrc/layout/Address/AddressComponent.test.tsxsrc/features/expressions/shared-functions.test.tsxsrc/layout/Checkboxes/CheckboxesContainerComponent.test.tsxsrc/features/formData/FormData.test.tsxsrc/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsxsrc/features/formData/FormDataReaders.test.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context
Applied to files:
src/features/options/useGetOptions.test.tsxsrc/layout/Map/MapComponent.test.tsxsrc/features/formData/useDataModelBindings.test.tsxsrc/layout/Likert/LikertTestUtils.tsxsrc/features/validation/ValidationPlugin.test.tsxsrc/layout/Group/SummaryGroupComponent.test.tsxsrc/layout/Datepicker/DatepickerComponent.test.tsxsrc/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsxsrc/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsxsrc/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsxsrc/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsxsrc/features/validation/expressionValidation/ExpressionValidation.test.tsxsrc/layout/TextArea/TextAreaComponent.test.tsxsrc/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsxsrc/layout/FileUpload/FileUploadComponent.test.tsxsrc/components/form/Form.test.tsxsrc/utils/layout/all.test.tsxsrc/layout/Input/InputComponent.test.tsxsrc/layout/MultipleSelect/MultipleSelectComponent.test.tsxsrc/test/renderWithProviders.tsxsrc/features/navigation/AppNavigation.test.tsxsrc/layout/RadioButtons/ControlledRadioGroup.test.tsxsrc/features/expressions/shared-context.test.tsxsrc/layout/Dropdown/DropdownComponent.test.tsxsrc/layout/Address/AddressComponent.test.tsxsrc/features/expressions/shared-functions.test.tsxsrc/layout/Checkboxes/CheckboxesContainerComponent.test.tsxsrc/features/formData/FormData.test.tsxsrc/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsxsrc/features/formData/FormDataReaders.test.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.
Applied to files:
src/features/formData/FormDataReaders.tsxsrc/features/expressions/shared-context.test.tsxsrc/features/expressions/shared-functions.test.tsxsrc/utils/urls/appUrlHelper.tssrc/features/formData/useFormDataQuery.tsxsrc/setupTests.tssrc/features/datamodel/useBindingSchema.tsxsrc/features/datamodel/DataModelsProvider.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In the Altinn app-frontend-react codebase, "stateless" refers to "instanceless" applications. While stateless/instanceless apps don't have server-side instance data, they still require data models to be loaded and processed for local state management.
Applied to files:
src/features/datamodel/useBindingSchema.tsx
🧬 Code graph analysis (39)
src/features/options/useGetOptions.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)
src/layout/Map/MapComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/components/form/Form.tsx (1)
src/features/formData/useFormDataQuery.tsx (1)
invalidateFormDataQueries(59-61)
src/features/formData/useDataModelBindings.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Likert/LikertTestUtils.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Subform/utils.ts (1)
src/features/formData/useFormDataQuery.tsx (1)
useFormDataQuery(71-97)
src/features/validation/ValidationPlugin.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Group/SummaryGroupComponent.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Datepicker/DatepickerComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)
src/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)
src/features/validation/expressionValidation/ExpressionValidation.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/TextArea/TextAreaComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsx (3)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)src/test/renderWithProviders.tsx (1)
renderWithInstanceAndLayout(589-656)
src/layout/FileUpload/FileUploadComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/components/form/Form.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/utils/layout/all.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Input/InputComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/layout/MultipleSelect/MultipleSelectComponent.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/features/formData/FormDataReaders.tsx (4)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
useApplicationMetadata(67-67)src/features/datamodel/useAvailableDataModels.tsx (1)
getDataTypeVariant(9-19)src/features/instance/InstanceContext.tsx (1)
useLaxInstanceId(58-62)src/features/formData/useFormDataQuery.tsx (1)
useFormDataQuery(71-97)
src/features/formData/FormDataWrite.tsx (1)
src/features/formData/useFormDataQuery.tsx (1)
formDataQueries(32-57)
src/features/navigation/AppNavigation.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/RadioButtons/ControlledRadioGroup.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/test/renderWithProviders.tsx (1)
renderGenericComponentTest(672-722)
src/features/expressions/shared-context.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/SigningActions/api.ts (4)
src/features/language/LanguageProvider.tsx (1)
useCurrentLanguage(79-79)src/features/profile/ProfileProvider.tsx (1)
useProfile(49-49)src/queries/queries.ts (1)
doPerformAction(166-182)src/layout/SigneeList/api.ts (2)
signingQueries(34-50)SigneeState(32-32)
src/layout/Dropdown/DropdownComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/__mocks__/getFormDataMockForRepGroup.ts (1)
getFormDataMockForRepGroup(5-20)
src/layout/Address/AddressComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/features/expressions/shared-functions.test.tsx (1)
src/queries/queries.ts (1)
fetchFormData(286-286)
src/layout/Checkboxes/CheckboxesContainerComponent.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/test/renderWithProviders.tsx (1)
renderGenericComponentTest(672-722)
src/features/formData/useFormDataQuery.tsx (10)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/language/LanguageProvider.tsx (1)
useCurrentLanguage(79-79)src/features/stateless/getAllowAnonymous.ts (1)
useAllowAnonymous(6-26)src/features/party/PartiesProvider.tsx (1)
useSelectedParty(189-189)src/utils/isAxiosError.ts (1)
isAxiosError(10-12)src/utils/network/networking.ts (1)
putWithoutConfig(48-55)src/utils/urls/appUrlHelper.ts (4)
invalidateCookieUrl(10-10)redirectToUpgrade(186-188)getStatelessDataModelUrl(51-73)getStatefulDataModelUrl(75-76)src/utils/urls/urlHelper.ts (1)
getUrlWithLanguage(137-151)src/features/instance/InstanceContext.tsx (1)
useLaxInstanceId(58-62)src/hooks/useAsRef.ts (1)
useAsRef(12-16)
src/layout/SigningActions/OnBehalfOfChooser.tsx (2)
src/layout/SigneeList/api.ts (1)
SigneeState(32-32)src/layout/SigningActions/api.ts (2)
AuthorizedOrganizationDetails(26-26)OnBehalfOf(85-85)
src/features/formData/FormData.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/setupTests.ts (2)
src/queries/types.ts (1)
IgnoredQueriesAndMutations(3-12)src/queries/queries.ts (6)
fetchApplicationMetadata(244-244)fetchProcessState(240-240)doProcessNext(93-94)fetchUserProfile(280-280)fetchInstanceData(235-238)fetchFormData(286-286)
src/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/features/formData/types.ts (1)
ALTINN_ROW_ID(39-39)
src/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsx (1)
src/layout/SigningActions/api.ts (1)
OnBehalfOf(85-85)
src/features/formData/FormDataReaders.test.tsx (2)
src/queries/queries.ts (1)
fetchFormData(286-286)src/layout/Likert/LikertTestUtils.tsx (1)
render(114-147)
src/features/datamodel/DataModelsProvider.tsx (3)
src/features/instance/InstanceContext.tsx (1)
useLaxInstanceId(58-62)src/features/formData/useFormDataQuery.tsx (1)
useFormDataQuery(71-97)src/features/datamodel/utils.ts (1)
getValidPrefillDataFromQueryParams(159-191)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Install
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
- GitHub Check: Install
🔇 Additional comments (32)
test/e2e/integration/subform-test/subform.ts (1)
210-212: Stability wait LGTM; prefer a more deterministic guard before clicking.Good addition for flake reduction. Please confirm
cy.waitForLoad()isn’t a fixed timeout but waits on a concrete signal (network idle/spinner gone). Optionally, you can assert the target control is interactable instead of a generic wait:- cy.waitForLoad(); + cy.findAllByRole('button', { name: /slett/i }).last().should('be.visible').and('not.be.disabled');src/features/validation/expressionValidation/ExpressionValidation.test.tsx (1)
11-11: Global mock detected — confirm fetchFormData is mocked or switch to namespace + spy.src/setupTests.ts contains jest.mock('src/queries/queries') (around line 115); verify whether that mock replaces fetchFormData with a jest.fn. If it does, keep the named import and use jest.mocked(fetchFormData); if it does not, change the import and spy on the export:
-import { fetchFormData } from 'src/queries/queries'; +import * as Queries from 'src/queries/queries';src/layout/SigneeList/api.ts (1)
36-47: Key includes taskId but fetch doesn’t use it—confirm intent and avoid accidental duplicationIncluding
taskIdin the query key while the fetch only depends on(partyId, instanceGuid)is fine for scoping optimistic updates, but it will create per-task caches of identical data. Confirm this is intentional and won’t inflate memory or cause stale duplicates when tasks change. If not intentional, droptaskIdfrom the key or add it to the fetch.src/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsx (1)
99-102: Nice auto‑selection when a single unsigned org existsAuto‑initializing
onBehalfOfimproves UX. With the chooser fix, this remains compatible.src/layout/RadioButtons/ControlledRadioGroup.test.tsx (1)
259-261: Good: per-test one-time mock used here.This avoids pollution and keeps the test deterministic.
src/features/options/useGetOptions.ts (2)
264-265: useGetOptionsUsingDmb memo depends on an unstable “set”.Once setData is memoized above, this stabilizes. If not, the spread object here will be recreated each render. Re-run tests after applying the refactor.
86-89: Keep undefined to clear the simpleBinding (don’t use an empty string).
Altinn app-frontend treats undefined as removal/clear of the bound value; '' would set an empty string — keeping values.at(0) (which may be undefined) is correct here.src/features/options/useGetOptions.test.tsx (2)
11-12: Type-only import pattern looks fine.Using “type fetchOptions” and typeof fetchOptions in annotations is correct with Jest typings.
69-77: Good shift to one-off fetchFormData mock.Hermetic and aligned with the new centralized mock strategy.
src/layout/Likert/LikertTestUtils.tsx (1)
3-3: No change needed — fetchFormData is mocked in src/setupTests.ts.
setupTests.ts calls jest.mock('src/queries/queries', ...) and defines fetchFormData: jest.fn(async () => ({})), so jest.mocked(fetchFormData).mockImplementationOnce(...) is safe.src/features/navigation/AppNavigation.test.tsx (1)
45-48: Mocking pattern looks good — unable to verify project-wide mock cleanupOne-off mock tied to rawOrder is fine; no project-level Jest config (jest.config.* or package.json 'jest' object) setting restoreMocks/resetMocks/clearMocks was found. Confirm either set restoreMocks/resetMocks/clearMocks in Jest config or add a global afterEach in your test setup (e.g., setupTests.*) that calls jest.resetAllMocks()/jest.restoreAllMocks().
src/layout/Input/InputComponent.test.tsx (1)
3-3: LGTM on imports.Using @jest/globals and importing the fetcher locally aligns with the new mocking pattern.
Also applies to: 9-9
src/layout/TextArea/TextAreaComponent.test.tsx (1)
3-3: LGTM on imports.Matches the updated repo-wide test scaffolding.
Also applies to: 9-9
src/test/renderWithProviders.tsx (1)
50-56: Exporting ExtendedRenderOptions is fine; treat as a public API change.Looks good. Verify downstream imports were updated to import the type (not re-declare).
src/layout/Subform/utils.ts (1)
23-27: Refactor to useFormDataQuery looks good.Destructure + error handling reads clean and aligns with the new query API.
src/layout/RepeatingGroup/Summary2/RepeatingGroupTableSummary/RepeatingGroupTableSummary.test.tsx (1)
155-157: Type-safe spy return: nice touch.The explicit ReturnType cast on useNavigateToComponent keeps the mock aligned with the hook signature.
src/queries/types.ts (1)
3-12: Approve — IgnoredQueriesAndMutations export and ignore-list extension verified.The type is exported in src/queries/types.ts and is consumed in src/setupTests.ts (jest.requireActual type param at line 117); AppQueries/AppMutations reference it (src/queries/types.ts:18–19). No further changes required.
src/features/formData/useDataModelBindings.test.tsx (1)
96-96: LGTM — scoped mock per render is correct.
src/setupTests.ts mocks 'src/queries/queries' and defines fetchFormData as jest.fn (src/setupTests.ts:115). Using mockImplementationOnce in the test prevents leakage.src/features/expressions/shared-functions.test.tsx (1)
23-29: LGTM: centralizing query importsConsolidated imports are fine and align with test infra refactor.
src/layout/RepeatingGroup/Summary/SummaryRepeatingGroup.test.tsx (1)
8-8: LGTM: deterministic mock data for snapshotStable ALTINN_ROW_ID and concise mock keep the snapshot deterministic. Ensure fetchFormData is a mock/spy as noted in other files.
Also applies to: 24-32
src/layout/RepeatingGroup/Table/RepeatingGroupTable.test.tsx (1)
3-3: No change required — fetchFormData is mocked in setupTests.tssrc/setupTests.ts declares jest.mock('src/queries/queries') and defines fetchFormData: jest.fn(...) (see src/setupTests.ts lines 115 and 125), so jest.mocked(fetchFormData).mockImplementation(...) in the tests is valid.
src/layout/FileUpload/FileUploadComponent.test.tsx (1)
206-304: LGTM: per-test fetchFormData mocking is correct and aligns with setupTestsThe new jest.mocked(fetchFormData).mockImplementationOnce(...) calls are scoped and predictable. Good move away from per-render query overrides.
src/layout/Address/AddressComponent.test.tsx (1)
3-3: LGTM: centralized fetchFormData mocking in testsImports and scoped mocks of fetchFormData are consistent with the new test strategy. No issues found.
Also applies to: 9-9, 110-111, 134-135, 171-174, 196-199
src/layout/Checkboxes/CheckboxesContainerComponent.test.tsx (2)
50-74: LGTM: render() now centrally mocks fetchFormDataGood consolidation; keeps individual tests clean and focused.
307-344: LGTM: validation scenarios wired to mocked form dataThe required validations now rely on fetchFormData mocks rather than ad-hoc queries. Looks good.
src/features/formData/FormData.test.tsx (2)
217-226: LGTM: realistic initial form data in rendering testsSeeding obj1/obj2 improves signal vs. empty data. Good.
384-389: LGTM: locking tests seed initial data predictablyScoped fetchFormData mock is appropriate here.
src/features/datamodel/DataModelsProvider.tsx (1)
325-331: LGTM: parameterized useFormDataQuery with instanceIdPassing instanceId and removing URL coupling is the right direction. Error propagation is intact.
Also applies to: 333-341
src/features/formData/FormDataReaders.tsx (1)
205-210: Good migration to param-based useFormDataQueryPassing a structured params object aligns with our TanStack Query guideline and simplifies cache-keying. LGTM.
src/layout/RepeatingGroup/Container/RepeatingGroupContainer.test.tsx (2)
179-189: Scenario shift to edit existing row improves test determinismEditing the last existing row and asserting editIndex reduces flakiness vs. creating a new row. Nice.
101-108: Global mock for 'src/queries/queries' is present — no change neededsrc/setupTests.ts defines jest.mock('src/queries/queries', ...) (around line 115), so jest.mocked(fetchFormData).mockImplementation(...) in the test is valid.
src/features/formData/useFormDataQuery.tsx (1)
32-57: Good centralization of form-data queriesThe formDataQueries registry with queryOptions and skipToken is idiomatic TanStack Query v5 and matches our guideline to centralize keys/options.
| if (exitSubform) { | ||
| invalidateFormDataQueries(queryClient); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Await query invalidation (or mark as intentionally fire-and-forget).
Not awaiting the async invalidate may race with navigation. Either await it or explicitly discard the promise.
- if (exitSubform) {
- invalidateFormDataQueries(queryClient);
- }
+ if (exitSubform) {
+ await invalidateFormDataQueries(queryClient);
+ }Optional: If you prefer non-blocking, do void invalidateFormDataQueries(queryClient);.
Also consider scoping invalidation to the current instance (if supported by formDataQueries) to avoid sweeping cache busts.
🏁 Script executed:
#!/usr/bin/env bash
# Inspect available formDataQueries keys to see if instance-scoped invalidation exists
rg -nP "formDataQueries\.(allKey|listKey|formData)" -C2 -- srcLength of output: 1953
Await or explicitly discard invalidateFormDataQueries; prefer scoped invalidation
Don't call invalidateFormDataQueries(queryClient) without handling the promise — await it or explicitly discard it (void) to avoid racing with navigation.
- if (exitSubform) {
- invalidateFormDataQueries(queryClient);
- }
+ if (exitSubform) {
+ await invalidateFormDataQueries(queryClient);
+ }If non-blocking is intended: void invalidateFormDataQueries(queryClient);
Note: invalidateFormDataQueries currently invalidates the global key (formDataQueries.allKey) — see src/features/formData/useFormDataQuery.tsx (lines ~59–61). For instance-scoped invalidation use formDataQueries.formDataKey(...) or update the helper to accept params.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/form/Form.tsx around lines 257-259, the call to
invalidateFormDataQueries(queryClient) is unawaited which can race with
navigation — either await the promise or explicitly discard it with void to show
non-blocking intent; preferably perform a scoped invalidation instead of the
global key by calling the form-specific key helper (e.g.
formDataQueries.formDataKey(...) ) or update invalidateFormDataQueries to accept
an identifier so you can invalidate only the affected form; implement one of
these: await invalidateFormDataQueries(queryClient), void
invalidateFormDataQueries(queryClient), or replace the call with a form-scoped
invalidate using the formDataQueries.formDataKey(...) (or extend the helper to
accept params) to avoid global invalidation.
| if (!dataType) { | ||
| continue; | ||
| } | ||
| const queryKey = getFormDataQueryKey(url); | ||
| const queryKey = formDataQueries.formDataKey({ | ||
| dataType, | ||
| dataElementId, | ||
| isAnonymous: false, | ||
| instanceId, | ||
| }); | ||
| queryClient.setQueryData(queryKey, data); | ||
| } | ||
| } |
There was a problem hiding this comment.
Cache key mismatch: isAnonymous hardcoded to false when updating query cache
Form data queries include isAnonymous in the key. Hardcoding false means setQueryData may not update the actual cache entry (e.g., anonymous/stateless flows), causing stale reads or unnecessary refetches.
Apply this diff to update all matching formData query entries regardless of isAnonymous:
- const queryKey = formDataQueries.formDataKey({
- dataType,
- dataElementId,
- isAnonymous: false,
- instanceId,
- });
- queryClient.setQueryData(queryKey, data);
+ const entries = queryClient.getQueryCache().findAll({ queryKey: formDataQueries.allKey });
+ for (const entry of entries) {
+ const [, params] = entry.queryKey as [string, { dataType?: string; dataElementId?: string | null; instanceId?: string }];
+ if (params?.dataType === dataType && params?.dataElementId === dataElementId && params?.instanceId === instanceId) {
+ queryClient.setQueryData(entry.queryKey, data);
+ }
+ }Alternatively, compute isAnonymous from the same source as useFormDataQuery and pass it to formDataQueries.formDataKey(..) to target a single key. Let me know if you prefer that; I can wire it up.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!dataType) { | |
| continue; | |
| } | |
| const queryKey = getFormDataQueryKey(url); | |
| const queryKey = formDataQueries.formDataKey({ | |
| dataType, | |
| dataElementId, | |
| isAnonymous: false, | |
| instanceId, | |
| }); | |
| queryClient.setQueryData(queryKey, data); | |
| } | |
| } | |
| if (!dataType) { | |
| continue; | |
| } | |
| const entries = queryClient.getQueryCache().findAll({ queryKey: formDataQueries.allKey }); | |
| for (const entry of entries) { | |
| const [, params] = entry.queryKey as [string, { dataType?: string; dataElementId?: string | null; instanceId?: string }]; | |
| if (params?.dataType === dataType && params?.dataElementId === dataElementId && params?.instanceId === instanceId) { | |
| queryClient.setQueryData(entry.queryKey, data); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/features/formData/FormDataWrite.tsx around lines 112 to 123, the cache
key is built with isAnonymous hardcoded to false which can miss the actual
cached entry; either (A) update the logic to setQueryData for both isAnonymous
values by iterating [true, false] and calling formDataQueries.formDataKey(...)
for each before calling queryClient.setQueryData, or (B) retrieve/compute
isAnonymous from the same source/useFormDataQuery hook used elsewhere and pass
that computed boolean into formDataQueries.formDataKey(...) so you target the
single correct cache entry; implement one of these fixes and remove the
hardcoded false.
| return { formData: formData as Output<B, DA>['formData'], setValue, setValues, isValid }; | ||
| } |
There was a problem hiding this comment.
Return object is no longer memoized; expect gratuitous re-renders.
Many consumers rely on referential stability for { formData, setValue, setValues, isValid }. Re-add memoization.
- return { formData: formData as Output<B, DA>['formData'], setValue, setValues, isValid };
+ return useMemo(
+ () => ({ formData: formData as Output<B, DA>['formData'], setValue, setValues, isValid }),
+ [formData, setValue, setValues, isValid],
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { formData: formData as Output<B, DA>['formData'], setValue, setValues, isValid }; | |
| } | |
| return useMemo( | |
| () => ({ formData: formData as Output<B, DA>['formData'], setValue, setValues, isValid }), | |
| [formData, setValue, setValues, isValid], | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/features/formData/useDataModelBindings.ts around lines 53-54, the
returned object { formData, setValue, setValues, isValid } is no longer memoized
causing gratuitous re-renders; wrap the returned object in React.useMemo and
return that memoized value instead, using [formData, setValue, setValues,
isValid] as the dependency array so the object identity remains stable while
types are preserved.
| export const formDataQueries = { | ||
| allKey: ['formData'], | ||
| formDataKey: (formDataKeyParams: FormDataKeyParams) => [...formDataQueries.allKey, formDataKeyParams], | ||
| formData: ({ enabled = true, options, language, ...formDataKeyParams }: FormDataParams) => { | ||
| const url = getDataModelUrl({ ...formDataKeyParams, language }); | ||
| const queryKey = formDataQueries.formDataKey(formDataKeyParams); | ||
|
|
||
| const queryFn = enabled && url ? () => fetchFormData(url, options) : skipToken; | ||
|
|
||
| if (!formDataKeyParams.dataElementId) { | ||
| // We need to refetch for stateless apps as caching will break some apps. | ||
| // See this issue: https://github.com/Altinn/app-frontend-react/issues/2564 | ||
| return queryOptions({ | ||
| queryKey, | ||
| queryFn, | ||
| gcTime: 0, | ||
| }); | ||
| } | ||
|
|
||
| if (isStateless) { | ||
| // We need to refetch for stateless apps as caching will break some apps. | ||
| // See this issue: https://github.com/Altinn/app-frontend-react/issues/2564 | ||
| return { | ||
| return queryOptions({ | ||
| queryKey, | ||
| queryFn, | ||
| gcTime: 0, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| queryKey, | ||
| queryFn, | ||
| refetchInterval: false, | ||
| }; | ||
| } | ||
|
|
||
| export function useFormDataQueryKey(url: string | undefined) { | ||
| return useMemoDeepEqual(() => getFormDataQueryKey(url), [url]); | ||
| } | ||
|
|
||
| const formDataQueryKeys = { | ||
| all: ['fetchFormData'] as const, | ||
| withUrl: (url: string | undefined) => [...formDataQueryKeys.all, url ? getFormDataCacheKeyUrl(url) : url] as const, | ||
| refetchInterval: false, | ||
| }); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Language is not part of the query key — risk of stale cache on language switch
getUrlWithLanguage varies the URL by language, but language is excluded from the query key. React Query won’t refetch on language change, leading to stale data (especially for stateless prefill or server-side language-dependent behavior).
Apply:
-type FormDataKeyParams = {
+type FormDataKeyParams = {
isAnonymous: boolean;
dataType?: string;
dataElementId?: string;
prefillFromQueryParams?: string;
instanceId?: string;
-};
+ language?: string;
+};
@@
- formData: ({ enabled = true, options, language, ...formDataKeyParams }: FormDataParams) => {
- const url = getDataModelUrl({ ...formDataKeyParams, language });
- const queryKey = formDataQueries.formDataKey(formDataKeyParams);
+ formData: ({ enabled = true, options, language, ...formDataKeyParams }: FormDataParams) => {
+ const url = getDataModelUrl({ ...formDataKeyParams, language });
+ const queryKey = formDataQueries.formDataKey({ ...formDataKeyParams, language });Optionally, invalidate on language change if you prefer not to key by language.
Also applies to: 71-82
| result = validations; | ||
| }); | ||
| jest.spyOn(Validation, 'useUpdateDataModelValidations').mockImplementation(() => updateDataModelValidations); | ||
| jest.mocked(fetchFormData).mockImplementation(async () => formData); |
There was a problem hiding this comment.
Use jest.spyOn instead of jest.mocked to stub fetchFormData.
Without a prior module mock, this line will throw (mockImplementation is undefined). Spy on the module export and resolve with formData.
Apply this diff:
-jest.mocked(fetchFormData).mockImplementation(async () => formData);
+jest.spyOn(Queries, 'fetchFormData').mockResolvedValue(formData);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mocked(fetchFormData).mockImplementation(async () => formData); | |
| jest.spyOn(Queries, 'fetchFormData').mockResolvedValue(formData); |
🤖 Prompt for AI Agents
In src/features/validation/expressionValidation/ExpressionValidation.test.tsx
around line 86, the test uses jest.mocked(fetchFormData).mockImplementation(...)
which fails because the module export wasn't mocked; replace this with a
jest.spyOn(...) on the module that exports fetchFormData and stub it to resolve
with formData (e.g., spyOn(module,
'fetchFormData').mockResolvedValue(formData)), and ensure the spy is restored
after the test (restore or mockRestore) to avoid cross-test pollution.
| }: Props = {}) => { | ||
| jest | ||
| .mocked(fetchFormData) | ||
| .mockImplementation(async () => (formData ? { myRadio: formData, ...groupData } : { ...groupData })); | ||
| return await renderGenericComponentTest({ | ||
| type: 'RadioButtons', |
There was a problem hiding this comment.
Persistent mockImplementation in render() leaks across tests.
This overrides fetchFormData for the entire file run, risking cross-test coupling. Use mockImplementationOnce per render call or reset in afterEach.
- jest
- .mocked(fetchFormData)
- .mockImplementation(async () => (formData ? { myRadio: formData, ...groupData } : { ...groupData }));
+ jest
+ .mocked(fetchFormData)
+ .mockImplementationOnce(async () => (formData ? { myRadio: formData, ...groupData } : { ...groupData }));Additionally, add in this file:
// outside the render() helper
afterEach(() => {
jest.resetAllMocks();
});🤖 Prompt for AI Agents
In src/layout/RadioButtons/ControlledRadioGroup.test.tsx around lines 46 to 51,
the test helper uses jest.mocked(fetchFormData).mockImplementation which
persists across tests and leaks state; change the test helper to use
mockImplementationOnce for each render call (so each render only overrides
fetchFormData once) or explicitly call fetchFormData.mockClear()/mockReset()
before setting the implementation, and add an afterEach(() => {
jest.resetAllMocks(); }) at file scope (outside the render helper) to ensure
mocks are reset between tests.
| const { instanceOwnerPartyId, instanceGuid, taskId } = useParams(); | ||
| const selectedLanguage = useCurrentLanguage(); | ||
| const queryClient = useQueryClient(); | ||
| const currentUserPartyId = useProfile()?.partyId.toString(); |
There was a problem hiding this comment.
NPE risk: .toString() on possibly undefined
useProfile()?.partyId.toString() will throw if partyId is undefined. Use optional chaining on partyId as well.
Apply this diff:
- const currentUserPartyId = useProfile()?.partyId.toString();
+ const currentUserPartyId = useProfile()?.partyId?.toString();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentUserPartyId = useProfile()?.partyId.toString(); | |
| const currentUserPartyId = useProfile()?.partyId?.toString(); |
🤖 Prompt for AI Agents
In src/layout/SigningActions/api.ts around line 91, calling
useProfile()?.partyId.toString() can throw if partyId is undefined; change to
safely access partyId (e.g., useProfile()?.partyId?.toString() or const
currentUserPartyId = useProfile()?.partyId ? useProfile()!.partyId.toString() :
undefined) so you only call toString() when partyId is present, or provide a
default value via nullish coalescing (?.toString() ?? '') depending on desired
behavior.
| onSuccess: (_data, onBehalfOf) => { | ||
| // optimistically update data in cache | ||
| const signeeListQueryKey = signingQueries.signeeList(instanceOwnerPartyId, instanceGuid, taskId).queryKey; | ||
| queryClient.setQueryData<SigneeState[]>(signeeListQueryKey, (signeeList) => { | ||
| const partyId = onBehalfOf?.partyId ?? currentUserPartyId; | ||
| if (!signeeList || !partyId) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return signeeList.map((signee) => ({ ...signee, hasSigned: signee.partyId.toString() === partyId })); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Optimistic update overwrites other signees and can clear cache
- Overwrites
hasSignedtofalsefor everyone except the selected party. - Returns
undefinedwhen data/partyId is missing, wiping the cache entry.
Preserve existing flags and no-op when inputs are missing.
Apply this diff:
- onSuccess: (_data, onBehalfOf) => {
- // optimistically update data in cache
- const signeeListQueryKey = signingQueries.signeeList(instanceOwnerPartyId, instanceGuid, taskId).queryKey;
- queryClient.setQueryData<SigneeState[]>(signeeListQueryKey, (signeeList) => {
- const partyId = onBehalfOf?.partyId ?? currentUserPartyId;
- if (!signeeList || !partyId) {
- return undefined;
- }
-
- return signeeList.map((signee) => ({ ...signee, hasSigned: signee.partyId.toString() === partyId }));
- });
-
- // Refetch all queries related to signing to ensure we have the latest data
- queryClient.invalidateQueries({ queryKey: signingQueries.all });
- },
+ onSuccess: (_data, onBehalfOf) => {
+ // Optimistically update data in cache
+ if (!instanceOwnerPartyId || !instanceGuid || !taskId) {
+ queryClient.invalidateQueries({ queryKey: signingQueries.all });
+ return;
+ }
+ const signeeListQueryKey = signingQueries.signeeList(instanceOwnerPartyId, instanceGuid, taskId).queryKey;
+ queryClient.setQueryData<SigneeState[]>(signeeListQueryKey, (signeeList) => {
+ const partyId = onBehalfOf?.partyId ?? currentUserPartyId;
+ if (!signeeList || !partyId) return signeeList;
+ return signeeList.map((signee) =>
+ signee.partyId.toString() === partyId ? { ...signee, hasSigned: true } : signee,
+ );
+ });
+ // Ensure latest data
+ queryClient.invalidateQueries({ queryKey: signingQueries.all });
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onSuccess: (_data, onBehalfOf) => { | |
| // optimistically update data in cache | |
| const signeeListQueryKey = signingQueries.signeeList(instanceOwnerPartyId, instanceGuid, taskId).queryKey; | |
| queryClient.setQueryData<SigneeState[]>(signeeListQueryKey, (signeeList) => { | |
| const partyId = onBehalfOf?.partyId ?? currentUserPartyId; | |
| if (!signeeList || !partyId) { | |
| return undefined; | |
| } | |
| return signeeList.map((signee) => ({ ...signee, hasSigned: signee.partyId.toString() === partyId })); | |
| }); | |
| onSuccess: (_data, onBehalfOf) => { | |
| // Optimistically update data in cache | |
| if (!instanceOwnerPartyId || !instanceGuid || !taskId) { | |
| queryClient.invalidateQueries({ queryKey: signingQueries.all }); | |
| return; | |
| } | |
| const signeeListQueryKey = signingQueries.signeeList(instanceOwnerPartyId, instanceGuid, taskId).queryKey; | |
| queryClient.setQueryData<SigneeState[]>(signeeListQueryKey, (signeeList) => { | |
| const partyId = onBehalfOf?.partyId ?? currentUserPartyId; | |
| if (!signeeList || !partyId) return signeeList; | |
| return signeeList.map((signee) => | |
| signee.partyId.toString() === partyId ? { ...signee, hasSigned: true } : signee, | |
| ); | |
| }); | |
| // Ensure latest data | |
| queryClient.invalidateQueries({ queryKey: signingQueries.all }); | |
| }, |
🤖 Prompt for AI Agents
In src/layout/SigningActions/api.ts around lines 105 to 116, the optimistic
update currently overwrites other signees' flags and returns undefined (clearing
the cache) when data or partyId is missing; change it to no-op when signeeList
or partyId is missing by returning the original signeeList, and modify the
mapping so you only set hasSigned to true for the matching partyId while
preserving all other properties (including any existing hasSigned values) for
non-matching signees.
| checked={onBehalfOf?.orgNoOrSsn === ''} | ||
| /> |
There was a problem hiding this comment.
Fix value shape to match JSON.parse consumer and treat “Myself” as null
PanelAwaitingCurrentUserSignature does JSON.parse(e.target.value). Current values ('' for “Myself”, org.partyId for orgs) will either throw or produce a number, not an OnBehalfOf. Emit JSON strings:
- “Myself” →
"null"so mutation omitsonBehalfOf. - Org →
{"orgNoOrSsn": "...", "partyId": "..."}.
Also update the “Myself” checked logic to onBehalfOf === null.
Apply this diff:
- value=''
+ value='null'
label={mySelf}
name='onBehalfOf'
key={currentUserSignee.partyId}
onChange={onChange}
- checked={onBehalfOf?.orgNoOrSsn === ''}
+ checked={onBehalfOf === null}
/>
...
- <RadioButton
- value={org.partyId}
+ <RadioButton
+ value={JSON.stringify({ orgNoOrSsn: org.orgNumber, partyId: String(org.partyId) })}
label={org.orgName}
name='onBehalfOf'
key={org.partyId}
onChange={onChange}
- checked={onBehalfOf?.orgNoOrSsn === org.orgNumber}
+ checked={onBehalfOf?.orgNoOrSsn === org.orgNumber}
/>Also applies to: 51-57
| export const getStatelessDataModelUrl = ({ | ||
| dataType, | ||
| prefillFromQueryParams, | ||
| isAnonymous = false, | ||
| }: { | ||
| dataType: string; | ||
| prefillFromQueryParams?: string; | ||
| isAnonymous?: boolean; | ||
| }) => { | ||
| const queryParams = new URLSearchParams({ | ||
| dataType, | ||
| includeRowId: 'true', | ||
| }); | ||
| if (isAnonymous) { | ||
| return `${appPath}/v1/data/anonymous?${queryParams}`; | ||
| } | ||
|
|
||
| export const getStatelessDataModelUrlWithPrefill = (dataType: string, prefillFromQueryParams: string) => | ||
| `${appPath}/v1/data?dataType=${dataType}&includeRowId=true&prefill=${prefillFromQueryParams}`; | ||
| if (prefillFromQueryParams) { | ||
| queryParams.append('prefill', prefillFromQueryParams); | ||
| } | ||
|
|
||
| export const getStatelessDataModelUrl = (dataType: string) => | ||
| `${appPath}/v1/data?dataType=${dataType}&includeRowId=true`; | ||
| return `${appPath}/v1/data?${queryParams}`; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Anonymous branch ignores prefill and always sets includeRowId — likely API mismatch.
- prefillFromQueryParams is dropped for isAnonymous (early return).
- includeRowId is sent to the anonymous endpoint; confirm API accepts/needs it.
Recommend supporting prefill for anonymous and only adding includeRowId for non-anonymous.
-export const getStatelessDataModelUrl = ({
- dataType,
- prefillFromQueryParams,
- isAnonymous = false,
-}: {
- dataType: string;
- prefillFromQueryParams?: string;
- isAnonymous?: boolean;
-}) => {
- const queryParams = new URLSearchParams({
- dataType,
- includeRowId: 'true',
- });
- if (isAnonymous) {
- return `${appPath}/v1/data/anonymous?${queryParams}`;
- }
-
- if (prefillFromQueryParams) {
- queryParams.append('prefill', prefillFromQueryParams);
- }
-
- return `${appPath}/v1/data?${queryParams}`;
-};
+export const getStatelessDataModelUrl = ({
+ dataType,
+ prefillFromQueryParams,
+ isAnonymous = false,
+}: {
+ dataType: string;
+ prefillFromQueryParams?: string;
+ isAnonymous?: boolean;
+}) => {
+ const queryParams = new URLSearchParams();
+ queryParams.set('dataType', dataType);
+ if (prefillFromQueryParams) {
+ queryParams.set('prefill', prefillFromQueryParams);
+ }
+ if (!isAnonymous) {
+ queryParams.set('includeRowId', 'true');
+ return `${appPath}/v1/data?${queryParams}`;
+ }
+ return `${appPath}/v1/data/anonymous?${queryParams}`;
+};🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- show file: src/utils/urls/appUrlHelper.ts ----"
if [ -f src/utils/urls/appUrlHelper.ts ]; then
sed -n '1,240p' src/utils/urls/appUrlHelper.ts || true
else
echo "NOT FOUND: src/utils/urls/appUrlHelper.ts"
fi
echo
echo "---- search for usages of getStatelessDataModelUrl ----"
rg -n --hidden -S 'getStatelessDataModelUrl' -g '!:node_modules/**' || true
echo
echo "---- search for endpoints and related params (includeRowId, prefill, /v1/data/anonymous) ----"
rg -n --hidden -S '/v1/data/anonymous|/v1/data\b|includeRowId|prefill' -g '!:node_modules/**' || true
echo
echo "---- find appPath definition/exports ----"
rg -n --hidden -S '\bappPath\b' -g '!:node_modules/**' || trueLength of output: 10677
Fix anonymous URL builder: preserve prefill and do not set includeRowId for anonymous calls
getStatelessDataModelUrl unconditionally sets includeRowId='true' and returns early when isAnonymous, which drops prefillFromQueryParams; change to always set dataType, append prefill when present (including for anonymous), and only add includeRowId for non-anonymous requests.
File: src/utils/urls/appUrlHelper.ts — getStatelessDataModelUrl (≈ lines 51–73).
🤖 Prompt for AI Agents
In src/utils/urls/appUrlHelper.ts around lines 51 to 73, the function currently
always sets includeRowId='true' and returns early for anonymous requests, which
drops prefillFromQueryParams; change it to always initialize URLSearchParams
with dataType, then if prefillFromQueryParams is provided append the prefill
param (regardless of isAnonymous), and only append includeRowId='true' when
isAnonymous is false before building the final URL; finally, return the
anonymous or non-anonymous path with the constructed query string.
Description
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests