Fixing navigation issues after validating attachment tags (#3753)

* Extracting incremental validation stuff from BackendValidation.tsx and adding it as a separate hook. This should work better than updating initial validations, which broke navigation later in the app.

* Adding cypress test for the tag validation functionality

* Adding an extra assertion that makes sure the validation is still there before the tag is selected

---------

Co-authored-by: Ole Martin Handeland <[email protected]>

Author: olemartinorg

src/features/attachments/AttachmentsStorePlugin.tsx

@@ -13,7 +13,6 @@ import { isAttachmentUploaded, isDataPostError } from 'src/features/attachments/
 import { sortAttachmentsByName } from 'src/features/attachments/sortAttachments';
 import { attachmentSelector } from 'src/features/attachments/tools';
 import { FileScanResults } from 'src/features/attachments/types';
-import { DataModels } from 'src/features/datamodel/DataModelsProvider';
 import { FD } from 'src/features/formData/FormDataWrite';
 import { dataModelPairsToObject } from 'src/features/formData/types';
 import {
@@ -25,12 +24,7 @@ import {
 import { useCurrentLanguage } from 'src/features/language/LanguageProvider';
 import { useLanguage } from 'src/features/language/useLanguage';
 import { backendValidationIssueGroupListToObject } from 'src/features/validation';
-import {
-  mapBackendIssuesToTaskValidations,
-  mapBackendValidationsToValidatorGroups,
-  mapValidatorGroupsToDataModelValidations,
-} from 'src/features/validation/backendValidation/backendValidationUtils';
-import { Validation } from 'src/features/validation/validationContext';
+import { useUpdateIncrementalValidations } from 'src/features/validation/backendValidation/useUpdateIncrementalValidations';
 import { useWaitForState } from 'src/hooks/useWaitForState';
 import { doUpdateAttachmentTags } from 'src/queries/queries';
 import { nodesProduce } from 'src/utils/layout/NodesContext';
@@ -48,7 +42,6 @@ import type {
 import type { AttachmentsSelector } from 'src/features/attachments/tools';
 import type { AttachmentStateInfo } from 'src/features/attachments/types';
 import type { FDActionResult } from 'src/features/formData/FormDataWriteStateMachine';
-import type { BackendFieldValidatorGroups, BackendValidationIssue } from 'src/features/validation';
 import type { DSPropsForSimpleSelector } from 'src/hooks/delayedSelectors';
 import type { IDataModelBindingsList, IDataModelBindingsSimple } from 'src/layout/common.generated';
 import type { RejectedFileError } from 'src/layout/FileUpload/RejectedFileError';
@@ -418,12 +411,7 @@ export class AttachmentsStorePlugin extends NodeDataPlugin<AttachmentsStorePlugi
             update(action);
             try {
               if (appSupportsSetTagsEndpoint(backendVersion)) {
-                await updateTags({
-                  dataElementId,
-                  setTagsRequest: {
-                    tags,
-                  },
-                });
+                await updateTags({ dataElementId, setTagsRequest: { tags } });
               } else {
                 await Promise.all(tagsToAdd.map((tag) => addTag({ dataElementId: attachment.data.id, tagToAdd: tag })));
                 await Promise.all(
@@ -796,8 +784,7 @@ function useAttachmentsRemoveTagMutation() {
 
 function useAttachmentUpdateTagsMutation() {
   const instanceId = useLaxInstanceId();
-  const defaultDataElementId = DataModels.useDefaultDataElementId();
-  const updateBackendValidations = Validation.useUpdateBackendValidations();
+  const updateIncrementalValidations = useUpdateIncrementalValidations();
 
   return useMutation({
     mutationFn: ({ dataElementId, setTagsRequest }: { dataElementId: string; setTagsRequest: SetTagsRequest }) => {
@@ -811,23 +798,9 @@ function useAttachmentUpdateTagsMutation() {
       window.logError('Failed to add tag to attachment:\n', error);
     },
     onSuccess: (data) => {
-      if (!data.validationIssues) {
-        return;
+      if (data.validationIssues) {
+        updateIncrementalValidations(backendValidationIssueGroupListToObject(data.validationIssues));
       }
-
-      const backendValidations: BackendValidationIssue[] = data.validationIssues.reduce(
-        (prev, curr) => [...prev, ...curr.issues],
-        [],
-      );
-      const initialTaskValidations = mapBackendIssuesToTaskValidations(backendValidations);
-      const initialValidatorGroups: BackendFieldValidatorGroups = mapBackendValidationsToValidatorGroups(
-        backendValidations,
-        defaultDataElementId,
-      );
-
-      const dataModelValidations = mapValidatorGroupsToDataModelValidations(initialValidatorGroups);
-
-      updateBackendValidations(dataModelValidations, { initial: backendValidations }, initialTaskValidations);
     },
   });
 }

src/features/formData/FormDataWrite.tsx

@@ -22,7 +22,11 @@ import { getFormDataQueryKey } from 'src/features/formData/useFormDataQuery';
 import { useLaxInstanceId, useOptimisticallyUpdateCachedInstance } from 'src/features/instance/InstanceContext';
 import { useCurrentLanguage } from 'src/features/language/LanguageProvider';
 import { useSelectedParty } from 'src/features/party/PartiesProvider';
-import { type BackendValidationIssueGroups, IgnoredValidators } from 'src/features/validation';
+import {
+  backendValidationIssueGroupListToObject,
+  type BackendValidationIssueGroups,
+  IgnoredValidators,
+} from 'src/features/validation';
 import { useIsUpdatingInitialValidations } from 'src/features/validation/backendValidation/backendValidationQuery';
 import { useAsRef } from 'src/hooks/useAsRef';
 import { useWaitForState } from 'src/hooks/useWaitForState';
@@ -174,7 +178,7 @@ function useFormDataSaveMutation() {
     }
   }
 
-  const mutation = useMutation({
+  return useMutation({
     mutationKey: saveFormDataMutationKey,
     scope: { id: saveFormDataMutationKey[0] },
     mutationFn: async (): Promise<FDSaveFinished | undefined> => {
@@ -303,13 +307,9 @@ function useFormDataSaveMutation() {
           }
         }
 
-        const validationIssueGroups: BackendValidationIssueGroups = Object.fromEntries(
-          validationIssues.map(({ source, issues }) => [source, issues]),
-        );
-
         return {
           newDataModels: dataModelChanges,
-          validationIssues: validationIssueGroups,
+          validationIssues: backendValidationIssueGroupListToObject(validationIssues),
           instance,
           savedData: next,
         };
@@ -358,8 +358,6 @@ function useFormDataSaveMutation() {
       checkForRunawaySaving();
     },
   });
-
-  return mutation;
 }
 
 function useIsSavingFormData() {

src/features/validation/backendValidation/BackendValidation.tsx

@@ -1,64 +1,41 @@
-import { useEffect, useRef } from 'react';
-
-import deepEqual from 'fast-deep-equal';
+import { useEffect } from 'react';
 
 import { DataModels } from 'src/features/datamodel/DataModelsProvider';
 import { FD } from 'src/features/formData/FormDataWrite';
 import { useBackendValidationQuery } from 'src/features/validation/backendValidation/backendValidationQuery';
 import {
-  mapBackendIssuesToFieldValidations,
   mapBackendIssuesToTaskValidations,
   mapBackendValidationsToValidatorGroups,
   mapValidatorGroupsToDataModelValidations,
   useShouldValidateInitial,
 } from 'src/features/validation/backendValidation/backendValidationUtils';
+import { useUpdateIncrementalValidations } from 'src/features/validation/backendValidation/useUpdateIncrementalValidations';
 import { Validation } from 'src/features/validation/validationContext';
-import type { BackendFieldValidatorGroups } from 'src/features/validation';
 
 export function BackendValidation() {
   const updateBackendValidations = Validation.useUpdateBackendValidations();
   const defaultDataElementId = DataModels.useDefaultDataElementId();
   const lastSaveValidations = FD.useLastSaveValidationIssues();
-  const validatorGroups = useRef<BackendFieldValidatorGroups>({});
   const enabled = useShouldValidateInitial();
-  const { data: initialValidations, isFetching } = useBackendValidationQuery({ enabled });
-  const initialValidatorGroups: BackendFieldValidatorGroups = mapBackendValidationsToValidatorGroups(
-    initialValidations,
-    defaultDataElementId,
-  );
-
-  // Map task validations
-  const initialTaskValidations = mapBackendIssuesToTaskValidations(initialValidations);
+  const { data: initialValidations, isFetching: isFetchingInitial } = useBackendValidationQuery({ enabled });
+  const updateIncrementalValidations = useUpdateIncrementalValidations();
 
   // Initial validation
   useEffect(() => {
-    if (!isFetching) {
-      validatorGroups.current = initialValidatorGroups;
+    if (!isFetchingInitial) {
+      const initialTaskValidations = mapBackendIssuesToTaskValidations(initialValidations);
+      const initialValidatorGroups = mapBackendValidationsToValidatorGroups(initialValidations, defaultDataElementId);
       const backendValidations = mapValidatorGroupsToDataModelValidations(initialValidatorGroups);
       updateBackendValidations(backendValidations, { initial: initialValidations }, initialTaskValidations);
     }
-  }, [initialTaskValidations, initialValidations, initialValidatorGroups, isFetching, updateBackendValidations]);
+  }, [defaultDataElementId, initialValidations, isFetchingInitial, updateBackendValidations]);
 
-  // Incremental validation: Update validators and propagate changes to validationcontext
+  // Incremental validation: Update validators and propagate changes to validation context
   useEffect(() => {
     if (lastSaveValidations) {
-      const newValidatorGroups = structuredClone(validatorGroups.current);
-
-      for (const [group, validationIssues] of Object.entries(lastSaveValidations)) {
-        newValidatorGroups[group] = mapBackendIssuesToFieldValidations(validationIssues, defaultDataElementId);
-      }
-
-      if (deepEqual(validatorGroups.current, newValidatorGroups)) {
-        // Dont update any validations, only set last saved validations
-        updateBackendValidations(undefined, { incremental: lastSaveValidations });
-        return;
-      }
-
-      validatorGroups.current = newValidatorGroups;
-      const backendValidations = mapValidatorGroupsToDataModelValidations(validatorGroups.current);
-      updateBackendValidations(backendValidations, { incremental: lastSaveValidations });
+      updateIncrementalValidations(lastSaveValidations);
     }
-  }, [defaultDataElementId, lastSaveValidations, updateBackendValidations]);
+  }, [lastSaveValidations, updateIncrementalValidations]);
 
   return null;
 }

src/features/validation/backendValidation/useUpdateIncrementalValidations.ts

@@ -0,0 +1,47 @@
+import { useCallback } from 'react';
+
+import deepEqual from 'fast-deep-equal';
+
+import { DataModels } from 'src/features/datamodel/DataModelsProvider';
+import { useGetCachedInitialValidations } from 'src/features/validation/backendValidation/backendValidationQuery';
+import {
+  mapBackendIssuesToFieldValidations,
+  mapBackendValidationsToValidatorGroups,
+  mapValidatorGroupsToDataModelValidations,
+} from 'src/features/validation/backendValidation/backendValidationUtils';
+import { Validation } from 'src/features/validation/validationContext';
+import type { BackendValidationIssueGroups } from 'src/features/validation';
+
+/**
+ * Hook for updating incremental validations from various sources (usually the validations updated from last saved data)
+ */
+export function useUpdateIncrementalValidations() {
+  const updateBackendValidations = Validation.useUpdateBackendValidations();
+  const defaultDataElementId = DataModels.useDefaultDataElementId();
+  const getCachedInitialValidations = useGetCachedInitialValidations();
+
+  return useCallback(
+    (lastSaveValidations: BackendValidationIssueGroups) => {
+      const { cachedInitialValidations } = getCachedInitialValidations();
+      const initialValidatorGroups = mapBackendValidationsToValidatorGroups(
+        cachedInitialValidations,
+        defaultDataElementId,
+      );
+
+      const newValidatorGroups = structuredClone(initialValidatorGroups);
+      for (const [group, validationIssues] of Object.entries(lastSaveValidations)) {
+        newValidatorGroups[group] = mapBackendIssuesToFieldValidations(validationIssues, defaultDataElementId);
+      }
+
+      if (deepEqual(initialValidatorGroups, newValidatorGroups)) {
+        // Don't update any validations, only set last saved validations
+        updateBackendValidations(undefined, { incremental: lastSaveValidations });
+        return;
+      }
+
+      const backendValidations = mapValidatorGroupsToDataModelValidations(newValidatorGroups);
+      updateBackendValidations(backendValidations, { incremental: lastSaveValidations });
+    },
+    [defaultDataElementId, getCachedInitialValidations, updateBackendValidations],
+  );
+}

src/features/validation/validationContext.tsx

@@ -144,6 +144,7 @@ function initialCreateStore() {
 const {
   Provider,
   useSelector,
+  useStaticSelector,
   useMemoSelector,
   useLaxShallowSelector,
   useSelectorAsRef,
@@ -359,8 +360,8 @@ export const Validation = {
     }, [s]);
   },
   useValidating: () => useSelector((state) => state.validating!),
-  useUpdateDataModelValidations: () => useSelector((state) => state.updateDataModelValidations),
-  useUpdateBackendValidations: () => useSelector((state) => state.updateBackendValidations),
+  useUpdateDataModelValidations: () => useStaticSelector((state) => state.updateDataModelValidations),
+  useUpdateBackendValidations: () => useStaticSelector((state) => state.updateBackendValidations),
 
   useFullState: <U,>(selector: (state: ValidationContext & Internals) => U): U =>
     useMemoSelector((state) => selector(state)),

test/e2e/integration/expression-validation-test/tags-validation.ts

@@ -0,0 +1,82 @@
+import { AppFrontend } from 'test/e2e/pageobjects/app-frontend';
+
+const appFrontend = new AppFrontend();
+
+describe('Attachment tags validation', () => {
+  beforeEach(() => {
+    cy.intercept('**/active', []).as('noActiveInstances');
+    cy.startAppInstance(appFrontend.apps.expressionValidationTest);
+  });
+
+  it('should update validations when saving tags', () => {
+    cy.gotoNavPage('CV');
+    cy.findByRole('textbox', { name: /alder/i }).type('17');
+
+    // Opt-in to attachment type validation
+    cy.findByRole('radio', { name: /ja/i }).dsCheck();
+
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Vitnemål'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Søknad'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Motivasjonsbrev'");
+
+    cy.get(appFrontend.expressionValidationTest.cvUploader).selectFile('test/e2e/fixtures/test.pdf', { force: true });
+
+    cy.contains('Ferdig lastet').should('be.visible');
+    cy.dsSelect(appFrontend.expressionValidationTest.groupTag, 'Søknad');
+    cy.findByRole('button', { name: /^lagre$/i }).click();
+
+    // Verify "Søknad" validation is removed, but others remain
+    cy.get(appFrontend.errorReport).should('not.contain.text', "Du må laste opp 'Søknad'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Vitnemål'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Motivasjonsbrev'");
+
+    cy.findByRole('button', { name: /rediger/i }).click();
+    cy.dsSelect(appFrontend.expressionValidationTest.groupTag, 'Vitnemål');
+    cy.findByRole('button', { name: /^lagre$/i }).click();
+
+    // Verify "Vitnemål" validation is removed and "Søknad" validation is back
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Søknad'");
+    cy.get(appFrontend.errorReport).should('not.contain.text', "Du må laste opp 'Vitnemål'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Motivasjonsbrev'");
+
+    // Upload second file and tag as "Søknad"
+    cy.get(appFrontend.expressionValidationTest.cvUploader).selectFile('test/e2e/fixtures/test.pdf', { force: true });
+    cy.contains('Ferdig lastet').should('be.visible');
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Søknad'");
+    cy.dsSelect(appFrontend.expressionValidationTest.groupTag, 'Søknad');
+    cy.findAllByRole('button', { name: /^lagre$/i })
+      .last()
+      .click();
+
+    // Verify "Søknad" validation is removed
+    cy.get(appFrontend.errorReport).should('not.contain.text', "Du må laste opp 'Søknad'");
+    cy.get(appFrontend.errorReport).should('not.contain.text', "Du må laste opp 'Vitnemål'");
+    cy.get(appFrontend.errorReport).should('contain.text', "Du må laste opp 'Motivasjonsbrev'");
+
+    // Upload third file and tag as "Motivasjonsbrev"
+    cy.get(appFrontend.expressionValidationTest.cvUploader).selectFile('test/e2e/fixtures/test.pdf', { force: true });
+    cy.contains('Ferdig lastet').should('be.visible');
+    cy.dsSelect(appFrontend.expressionValidationTest.groupTag, 'Motivasjonsbrev');
+    cy.findAllByRole('button', { name: /^lagre$/i })
+      .last()
+      .click();
+
+    // Verify no errors are visible, and tags are visible in the table
+    cy.get(appFrontend.errorReport).should('not.exist');
+    cy.contains('td', 'Søknad').should('be.visible');
+    cy.contains('td', 'Vitnemål').should('be.visible');
+    cy.contains('td', 'Motivasjonsbrev').should('be.visible');
+
+    // Fill in remaining required fields
+    cy.findByRole('textbox', { name: /fornavn/i }).type('Per');
+    cy.findByRole('textbox', { name: /etternavn/i }).type('Hansen');
+    cy.dsSelect(appFrontend.expressionValidationTest.kjønn, 'Mann');
+    cy.findByRole('textbox', { name: /e-post/i }).type('[email protected]');
+    cy.findByRole('textbox', { name: /telefonnummer/i }).type('98765432');
+    cy.dsSelect(appFrontend.expressionValidationTest.bosted, 'Oslo');
+
+    cy.findByRole('button', { name: /neste/i }).click();
+    cy.findByRole('button', { name: /send inn/i }).click();
+    cy.get(appFrontend.receipt.container).should('be.visible');
+  });
+});

test/e2e/pageobjects/app-frontend.ts

@@ -350,6 +350,7 @@ export class AppFrontend {
     kjønn: '#kjonn',
     bosted: '#bosted',
     groupTag: 'input[id^=attachment-tag]',
+    cvUploader: '#vedlegg-cv',
     uploaders: '[id^=Vedlegg-]',
   };