Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/components/form/Form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { defaultMockDataElementId } from 'src/__mocks__/getInstanceDataMock';
import { defaultDataTypeMock } from 'src/__mocks__/getLayoutSetsMock';
import { Form } from 'src/components/form/Form';
import { type BackendValidationIssue, BackendValidationSeverity } from 'src/features/validation';
import { fetchFormData } from 'src/queries/queries';
import { renderWithInstanceAndLayout } from 'src/test/renderWithProviders';
import type { CompExternal, ILayout } from 'src/layout/layout';
import type { CompSummaryExternal } from 'src/layout/Summary/config.generated';
Expand Down Expand Up @@ -224,19 +225,20 @@ describe('Form', () => {
});

async function render(layout = mockComponents, validationIssues: BackendValidationIssue[] = []) {
jest.mocked(fetchFormData).mockImplementation(async () => ({
Group: [
{
prop1: 'value1',
prop2: 'value2',
prop3: 'value3',
},
],
}));

await renderWithInstanceAndLayout({
renderer: () => <Form />,
initialPage: 'FormLayout',
queries: {
fetchFormData: async () => ({
Group: [
{
prop1: 'value1',
prop2: 'value2',
prop3: 'value3',
},
],
}),
fetchLayouts: () =>
Promise.resolve({
FormLayout: {
Expand Down
9 changes: 8 additions & 1 deletion src/components/form/Form.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { useEffect, useMemo } from 'react';
import { useLocation, useNavigate, useSearchParams } from 'react-router-dom';

import { useQueryClient } from '@tanstack/react-query';

import { Flex } from 'src/app-components/Flex/Flex';
import classes from 'src/components/form/Form.module.css';
import { MessageBanner } from 'src/components/form/MessageBanner';
Expand All @@ -14,6 +16,7 @@ import { FileScanResults } from 'src/features/attachments/types';
import { useExpandedWidthLayouts, useLayoutLookups } from 'src/features/form/layout/LayoutsContext';
import { useUiConfigContext } from 'src/features/form/layout/UiConfigContext';
import { usePageSettings } from 'src/features/form/layoutSettings/LayoutSettingsContext';
import { invalidateFormDataQueries } from 'src/features/formData/useFormDataQuery';
import { useLaxInstanceId } from 'src/features/instance/InstanceContext';
import { useLanguage } from 'src/features/language/useLanguage';
import { useOnFormSubmitValidation } from 'src/features/validation/callbacks/onFormSubmitValidation';
Expand Down Expand Up @@ -241,6 +244,7 @@ function HandleNavigationFocusComponent() {
const validate = useQueryKey(SearchParams.Validate)?.toLocaleLowerCase() === 'true';
const navigate = useNavigate();
const searchStringRef = useAsRef(useLocation().search);
const queryClient = useQueryClient();

React.useEffect(() => {
(async () => {
Expand All @@ -250,10 +254,13 @@ function HandleNavigationFocusComponent() {
location.delete(SearchParams.ExitSubform);
const baseHash = window.location.hash.slice(1).split('?')[0];
const nextLocation = location.size > 0 ? `${baseHash}?${location.toString()}` : baseHash;
if (exitSubform) {
invalidateFormDataQueries(queryClient);
}
Comment on lines +257 to +259
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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 -- src

Length 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.

navigate(nextLocation, { replace: true });
}
})();
}, [navigate, searchStringRef, exitSubform, validate, onFormSubmitValidation]);
}, [navigate, searchStringRef, exitSubform, validate, onFormSubmitValidation, queryClient]);

return null;
}
13 changes: 7 additions & 6 deletions src/features/datamodel/DataModelsProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { useApplicationMetadata } from 'src/features/applicationMetadata/Applica
import { getFirstDataElementId } from 'src/features/applicationMetadata/appMetadataUtils';
import { useCustomValidationConfigQuery } from 'src/features/customValidation/useCustomValidationQuery';
import { UpdateDataElementIdsForCypress } from 'src/features/datamodel/DataElementIdsForCypress';
import { useCurrentDataModelName, useDataModelUrl } from 'src/features/datamodel/useBindingSchema';
import { useCurrentDataModelName } from 'src/features/datamodel/useBindingSchema';
import { useDataModelSchemaQuery } from 'src/features/datamodel/useDataModelSchemaQuery';
import {
getAllReferencedDataTypes,
Expand All @@ -31,6 +31,7 @@ import {
instanceQueries,
useInstanceDataElements,
useInstanceDataQueryArgs,
useLaxInstanceId,
} from 'src/features/instance/InstanceContext';
import { MissingRolesError } from 'src/features/instantiate/containers/MissingRolesError';
import { useIsPdf } from 'src/hooks/useIsPdf';
Expand Down Expand Up @@ -320,22 +321,22 @@ function LoadInitialData({ dataType, overrideDataElement }: LoaderProps & { over
const dataElements = useInstanceDataElements(dataType);
const dataElementId = overrideDataElement ?? getFirstDataElementId(dataElements, dataType);
const metaData = useApplicationMetadata();
const instanceId = useLaxInstanceId();

const url = useDataModelUrl({
const { data, error } = useFormDataQuery({
dataType,
dataElementId,
instanceId,
prefillFromQueryParams: getValidPrefillDataFromQueryParams(metaData, dataType),
});

const { data, error } = useFormDataQuery(url);

useEffect(() => {
if (!data || !url) {
if (!data) {
return;
}
sessionStorage.removeItem('queryParams');
setInitialData(dataType, data);
}, [data, dataType, metaData.id, setInitialData, url]);
}, [data, dataType, metaData.id, setInitialData]);

useEffect(() => {
setDataElementId(dataType, dataElementId ?? null);
Expand Down
6 changes: 0 additions & 6 deletions src/features/datamodel/useAvailableDataModels.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { useApplicationMetadata } from 'src/features/applicationMetadata/ApplicationMetadataProvider';
import type { IDataType } from 'src/types/shared';

export function useAvailableDataModels() {
const dataTypes = useApplicationMetadata().dataTypes;
return dataTypes.filter((dataType) => getDataTypeVariant(dataType) === DataTypeVariant.DataModel);
}

export enum DataTypeVariant {
Pdf = 'pdf',
Attachment = 'attachment',
Expand Down
94 changes: 2 additions & 92 deletions src/features/datamodel/useBindingSchema.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useMemo } from 'react';
import { useMemo } from 'react';

import type { JSONSchema7 } from 'json-schema';

Expand All @@ -10,18 +10,8 @@ import {
} from 'src/features/applicationMetadata/appMetadataUtils';
import { DataModels } from 'src/features/datamodel/DataModelsProvider';
import { useLayoutSets } from 'src/features/form/layoutSets/LayoutSetsProvider';
import { useInstanceDataQuery, useLaxInstanceId } from 'src/features/instance/InstanceContext';
import { useInstanceDataQuery } from 'src/features/instance/InstanceContext';
import { useProcessTaskId } from 'src/features/instance/useProcessTaskId';
import { useCurrentLanguage } from 'src/features/language/LanguageProvider';
import { useAllowAnonymous } from 'src/features/stateless/getAllowAnonymous';
import { useAsRef } from 'src/hooks/useAsRef';
import {
getAnonymousStatelessDataModelUrl,
getStatefulDataModelUrl,
getStatelessDataModelUrl,
getStatelessDataModelUrlWithPrefill,
} from 'src/utils/urls/appUrlHelper';
import { getUrlWithLanguage } from 'src/utils/urls/urlHelper';
import type { IDataModelReference } from 'src/layout/common.generated';
import type { IDataModelBindings } from 'src/layout/layout';

Expand Down Expand Up @@ -49,86 +39,6 @@ export function useCurrentDataModelDataElementId() {
}).data;
}

type DataModelDeps = {
language: string;
isAnonymous: boolean;
isStateless: boolean;
instanceId?: string;
};

type DataModelProps = {
dataType?: string;
dataElementId?: string;
language?: string;
prefillFromQueryParams?: string;
};

function getDataModelUrl({
dataType,
dataElementId,
language,
isAnonymous,
isStateless,
instanceId,
prefillFromQueryParams,
}: DataModelDeps & DataModelProps) {
if (prefillFromQueryParams && !isAnonymous && isStateless && dataType) {
return getUrlWithLanguage(getStatelessDataModelUrlWithPrefill(dataType, prefillFromQueryParams), language);
}

if (isStateless && isAnonymous && dataType) {
return getUrlWithLanguage(getAnonymousStatelessDataModelUrl(dataType), language);
}

if (isStateless && !isAnonymous && dataType) {
return getUrlWithLanguage(getStatelessDataModelUrl(dataType), language);
}

if (instanceId && dataElementId) {
return getUrlWithLanguage(getStatefulDataModelUrl(instanceId, dataElementId), language);
}

return undefined;
}

export function useGetDataModelUrl() {
const isAnonymous = useAllowAnonymous();
const isStateless = useApplicationMetadata().isStatelessApp;
const instanceId = useLaxInstanceId();
const currentLanguage = useAsRef(useCurrentLanguage());

return useCallback(
({ dataType, dataElementId, language }: DataModelProps) =>
getDataModelUrl({
dataType,
dataElementId,
language: language ?? currentLanguage.current,
isAnonymous,
isStateless,
instanceId,
}),
[currentLanguage, instanceId, isAnonymous, isStateless],
);
}

// We assume that the first data element of the correct type is the one we should use, same as isDataTypeWritable
export function useDataModelUrl({ dataType, dataElementId, language, prefillFromQueryParams }: DataModelProps) {
const isAnonymous = useAllowAnonymous();
const isStateless = useApplicationMetadata().isStatelessApp;
const instanceId = useLaxInstanceId();
const currentLanguage = useAsRef(useCurrentLanguage());

return getDataModelUrl({
dataType,
dataElementId,
language: language ?? currentLanguage.current,
isAnonymous,
isStateless,
instanceId,
prefillFromQueryParams,
});
}

export function useCurrentDataModelName() {
const overriddenDataModelType = useTaskStore((state) => state.overriddenDataModelType);

Expand Down
6 changes: 3 additions & 3 deletions src/features/expressions/shared-context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { screen } from '@testing-library/react';
import { getApplicationMetadataMock } from 'src/__mocks__/getApplicationMetadataMock';
import { getInstanceDataMock } from 'src/__mocks__/getInstanceDataMock';
import { getSharedTests } from 'src/features/expressions/shared';
import { fetchApplicationMetadata } from 'src/queries/queries';
import { fetchApplicationMetadata, fetchFormData } from 'src/queries/queries';
import { renderWithInstanceAndLayout } from 'src/test/renderWithProviders';
import { NodesInternal } from 'src/utils/layout/NodesContext';
import { splitDashedKey } from 'src/utils/splitDashedKey';
Expand Down Expand Up @@ -93,6 +93,8 @@ describe('Expressions shared context tests', () => {

const applicationMetadata = getApplicationMetadataMock(instance ? {} : { onEntry: { show: 'stateless' } });
jest.mocked(fetchApplicationMetadata).mockImplementation(async () => applicationMetadata);
// TODO(Datamodels): add support for multiple data models
jest.mocked(fetchFormData).mockImplementation(async () => dataModel ?? {});

if (instanceDataElements) {
for (const element of instanceDataElements) {
Expand All @@ -111,8 +113,6 @@ describe('Expressions shared context tests', () => {
renderer: () => <TestContexts />,
queries: {
fetchLayouts: async () => layouts!,
// TODO(Datamodels): add support for multiple data models
fetchFormData: async () => dataModel ?? {},
...(instance ? { fetchInstanceData: async () => instance } : {}),
...(frontendSettings ? { fetchApplicationSettings: async () => frontendSettings } : {}),
},
Expand Down
42 changes: 23 additions & 19 deletions src/features/expressions/shared-functions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ import {
isRepeatingComponent,
RepeatingComponents,
} from 'src/features/form/layout/utils/repeating';
import { fetchApplicationMetadata, fetchInstanceData, fetchProcessState, fetchUserProfile } from 'src/queries/queries';
import {
fetchApplicationMetadata,
fetchFormData,
fetchInstanceData,
fetchProcessState,
fetchUserProfile,
} from 'src/queries/queries';
import { AppQueries } from 'src/queries/types';
import {
renderWithInstanceAndLayout,
Expand Down Expand Up @@ -292,23 +298,6 @@ describe('Expressions shared function tests', () => {
profile.profileSettingPreference.language = profileSettings.language;
}

async function fetchFormData(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)}`);
}

// Clear localstorage, because LanguageProvider uses it to cache selected languages
localStorage.clear();

Expand All @@ -332,6 +321,22 @@ describe('Expressions shared function tests', () => {
}
return instanceData;
});
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)}`);
});

const toRender = (
<ExpressionRunner
Expand All @@ -347,7 +352,6 @@ describe('Expressions shared function tests', () => {
sets: [{ id: 'layout-set', dataType: 'default', tasks: ['Task_1'] }, getSubFormLayoutSetMock()],
}),
fetchLayouts: async () => layouts,
fetchFormData,
...(frontendSettings ? { fetchApplicationSettings: async () => frontendSettings } : {}),
fetchTextResources: async () => ({
language: 'nb',
Expand Down
Loading
Loading