Merging v4.8-patches (#2461)

* Showing backend validations that conflicts with frontend-validations (fixes #2412) (#2428) * Fixing the bug - we found some errors, but didn't set the visible to actually show those errors * Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point * Revert "Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point" This reverts commit 71852b9. --------- Co-authored-by: Ole Martin Handeland <[email protected]> * Fixing problems with removal of stale options (#2450) * Showing backend validations that conflicts with frontend-validations (fixes #2412) (#2428) * Fixing the bug - we found some errors, but didn't set the visible to actually show those errors * Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point * Revert "Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point" This reverts commit 71852b9. --------- Co-authored-by: Ole Martin Handeland <[email protected]> * Extracting all the important fixes from bug/stale-options2, but without the new extensive test setup that I'm not quite finished with (along with some remaining bugs I've found that nobody seems to have discovered so far). This quicker fix, if tests pass, should land in production before I'm done with the rest of the test suite and bugfixes. --------- Co-authored-by: Ole Martin Handeland <[email protected]> (cherry picked from commit 155e788) * Fixing hard fail on missing `componentRef` in Summary (#2448) * Showing backend validations that conflicts with frontend-validations (fixes #2412) (#2428) * Fixing the bug - we found some errors, but didn't set the visible to actually show those errors * Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point * Revert "Removing node traversal code and replacing it with more specialized selectors in ValidationStorePlugin, making the code more direct to the point" This reverts commit 71852b9. --------- Co-authored-by: Ole Martin Handeland <[email protected]> * Adding option for rendering all the components when running all apps (will use this to catch errors when rendering). Fixing a mock error which caused the InstanceInformation component to fail when rendering. * Fixing removal of multiline comments (the comment start/end was removed, but not the content inside it) * Adding mock for IDataList so that rendering a plain List component does not fail when looking for data * Adding missing unique key * Adding proper validation for missing summary target instead of letting it fail in runtime (this affected multiple apps and broke them badly) * Ignoring empty files (better to delete them!) --------- Co-authored-by: Ole Martin Handeland <[email protected]> (cherry picked from commit 0904ce3) * Fixing missing nodes in repeating group when UUID has changed (#2453) * Making a mechanism for loading the 'hidden page with a specific test-case' pattern * Using the new function * Storing nodes in the context as soon as they've been added. This makes them available for a ['component', ...] lookup, meaning a hidden expression will not start off as broken and be fixed later, it will resolve correctly immediately * Returning undefined until we have enough information to know if a component is hidden * Waiting until there is no more state to commit in the node hierarchy before allowing items to be removed from the data model. This fixes an issue with loading new data from the backend via a CustomButton in the upcoming test-page for conflicting options. * No longer using a lax traversal selector, implementing a closestId() tool, as 'displayValue' looked into 'item', not 'layout' * Recreating the label saving as an effect-hook, comparing with the form data instead of a previous local value * Preventing a full overwrite of the form data in the data model when going to a hidden test-page * Removing node traversal * Relying a little less on the EffectProps * Making every effect an effect, cleaning up code and 'any' * Nested repeating group titles got their title skewed as if it was showing in full width * Committing what I've worked on until now. Need to context-switch and fix another bug. * The core of the remaining bug was that the row uuid changed, and the node generator was half-way between using row UUIDs and row indexes for everything. When pressing a CustomButton that replaces the rows (but importantly not the _number of rows_), the nodes should stay in place, but just update the UUID in the state. Previously, the entire row was removed, but nodes were not added back (as they were unchanged), so that mixup caused rows without nodes to appear. * Updating comments in the test * Removing a dead end * Fixing bug introduced when hidden expressions can run earlier: This was not wrapped in a condition, so hidden expressions lookup up component values would end up failing hard. * Adding missing form data * Making removal of stale options slower, but safer. This fixes the breaking cypress test. * Adding timeout cleanup --------- Co-authored-by: Ole Martin Handeland <[email protected]> * Fixing a bug causing infinite PATCH requests (#2457) * Changing from 'any' to 'unknown' and fixing the root of the problem * Adding a regression test in Cypress * Fixing data model assumptions --------- Co-authored-by: Ole Martin Handeland <[email protected]> * Fixes after merge --------- Co-authored-by: Ole Martin Handeland <[email protected]>
Altinn · Sep 17, 2024 · b65def9 · b65def9
1 parent 65d04fb
commit b65def9
Show file tree

Hide file tree

Showing 30 changed files with 700 additions and 361 deletions.
diff --git a/src/codegen/ComponentConfig.ts b/src/codegen/ComponentConfig.ts
@@ -295,11 +295,6 @@ export class ComponentConfig {
       from: 'src/layout/layout',
     });
 
-    const BaseRow = new CG.import({
-      import: 'BaseRow',
-      from: 'src/utils/layout/types',
-    });
-
     const isFormComponent = this.config.category === CompCategory.Form;
     const isSummarizable = this.behaviors.isSummarizable;
 
@@ -418,11 +413,11 @@ export class ComponentConfig {
         `pickDirectChildren(state: ${NodeData}<'${this.type}'>, restriction?: ${TraversalRestriction}) {
           return [${pickDirectChildrenBody.join(', ')}];
         }`,
-        `addChild(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}, { pluginKey, metadata }: ${ChildClaim}, row: ${BaseRow} | undefined) {
-          return this.plugins[pluginKey!].addChild(state as any, childNode, metadata, row) as Partial<${NodeData}<'${this.type}'>>;
+        `addChild(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}, { pluginKey, metadata }: ${ChildClaim}, rowIndex: number | undefined) {
+          return this.plugins[pluginKey!].addChild(state as any, childNode, metadata, rowIndex) as Partial<${NodeData}<'${this.type}'>>;
         }`,
-        `removeChild(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}, { pluginKey, metadata }: ${ChildClaim}, row: ${BaseRow} | undefined) {
-          return this.plugins[pluginKey!].removeChild(state as any, childNode, metadata, row) as Partial<${NodeData}<'${this.type}'>>;
+        `removeChild(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}, { pluginKey, metadata }: ${ChildClaim}, rowIndex: number | undefined) {
+          return this.plugins[pluginKey!].removeChild(state as any, childNode, metadata, rowIndex) as Partial<${NodeData}<'${this.type}'>>;
         }`,
         `isChildHidden(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}) {
           return [${isChildHiddenBody.join(', ')}].some((h) => h);
@@ -450,7 +445,7 @@ export class ComponentConfig {
           item: undefined,
           layout: props.item,
           hidden: undefined,
-          row: props.row,
+          rowIndex: props.rowIndex,
           errors: undefined,
         };
         ${itemDef}

diff --git a/src/features/formData/FormDataWrite.tsx b/src/features/formData/FormDataWrite.tsx
@@ -801,6 +801,42 @@ export const FD = {
       return rawRows.map((row: any, index: number) => ({ uuid: row[ALTINN_ROW_ID], index }));
     }),
 
+  /**
+   * Returns the number of rows in a repeating group. This will always be 'fresh', meaning it will update immediately
+   * when a new row is added/removed.
+   */
+  useFreshNumRows: (binding: IDataModelReference | undefined): number =>
+    useMemoSelector((s) => {
+      if (!binding) {
+        return 0;
+      }
+
+      const rawRows = dot.pick(binding.field, s.dataModels[binding.dataType].currentData);
+      if (!Array.isArray(rawRows) || !rawRows.length) {
+        return 0;
+      }
+
+      return rawRows.length;
+    }),
+
+  /**
+   * Get the UUID of a row in a repeating group. This will always be 'fresh', meaning it will update immediately when
+   * a new row is added/removed.
+   */
+  useFreshRowUuid: (binding: IDataModelReference | undefined, index: number): string | undefined =>
+    useMemoSelector((s) => {
+      if (!binding) {
+        return undefined;
+      }
+
+      const rawRows = dot.pick(binding.field, s.dataModels[binding.dataType].currentData);
+      if (!Array.isArray(rawRows) || !rawRows.length) {
+        return undefined;
+      }
+
+      return rawRows[index]?.[ALTINN_ROW_ID];
+    }),
+
   /**
    * Returns a function you can use to debounce saved form data
    * This will work (and return immediately) even if there is no FormDataWriteProvider in the tree.

diff --git a/src/features/formData/jsonPatch/createPatch.test.ts b/src/features/formData/jsonPatch/createPatch.test.ts
@@ -302,12 +302,22 @@ describe('createPatch', () => {
     });
   });
 
-  describe('should avoid overwriting differing simple arrays in current', () => {
+  describe('should overwrite simple arrays in current if the next value is new', () => {
     testPatch({
       prev: { a: ['foo', 'bar', 'baz', 'qux'] },
       next: { a: ['foo', 'bar2', 'baz', 'qux'] },
       current: { a: ['foo', 'bar', 'baz', 'qux'] },
-      final: { a: ['foo', 'bar', 'baz', 'qux'] },
+      final: { a: ['foo', 'bar2', 'baz', 'qux'] },
+      expectedPatch: [{ op: 'replace', path: '/a', value: ['foo', 'bar2', 'baz', 'qux'] }],
+    });
+  });
+
+  describe('should avoid overwriting simple arrays in current if the current value is newer', () => {
+    testPatch({
+      prev: { a: ['foo', 'bar', 'baz', 'qux'] },
+      next: { a: ['foo', 'bar2', 'baz', 'qux'] },
+      current: { a: ['foo', 'bar', 'baz', 'qux2'] },
+      final: { a: ['foo', 'bar', 'baz', 'qux2'] },
       expectedPatch: [],
     });
   });

diff --git a/src/features/formData/jsonPatch/createPatch.ts b/src/features/formData/jsonPatch/createPatch.ts
@@ -43,19 +43,17 @@ export function createPatch({ prev, next, current }: Props<object>): JsonPatch {
   return patch;
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function isObject(value: any): value is object {
+function isObject(value: unknown): value is object {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function compareAny(props: CompareProps<any>) {
+function compareAny(props: CompareProps<unknown>) {
   const { prev, next } = props;
   if (isObject(prev) && isObject(next)) {
-    return compareObjects(props);
+    return compareObjects(props as CompareProps<object>);
   }
   if (Array.isArray(prev) && Array.isArray(next)) {
-    return compareArrays(props);
+    return compareArrays(props as CompareProps<unknown[]>);
   }
   compareValues(props);
 }
@@ -74,8 +72,7 @@ function compareObjects({ prev, next, current, path, ...rest }: CompareProps<obj
  * This comparison function is used to determine if two values in an array can be considered the same. This is used to
  * determine if an item (i.e. a row in a repeating group) has been removed or added.
  */
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function isSameRow(left: any, right: any): boolean {
+function isSameRow(left: unknown, right: unknown): boolean {
   if (isObject(left) && isObject(right)) {
     if (!(ALTINN_ROW_ID in left && ALTINN_ROW_ID in right)) {
       window.logWarn(
@@ -102,20 +99,20 @@ function isSameRow(left: any, right: any): boolean {
  * produce the JsonPatch to create. Do not be fooled by the format returned from getPatch, it is not a JsonPatch
  * even if it looks like it at a glance.
  */
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function compareArrays({ prev, next, current, hasCurrent, patch, path }: CompareProps<any[]>) {
+function compareArrays({ prev, next, current, hasCurrent, patch, path }: CompareProps<unknown[]>) {
   const diff = getPatch(prev, next, isSameRow);
-  const allNextValuesIsString = next.length > 0 && next.every((item) => typeof item === 'string');
-  if (allNextValuesIsString) {
+  const allValues = [...next, ...prev, ...(current || [])];
+  const allValuesAreStrings = allValues.length > 0 && allValues.every((item) => typeof item === 'string');
+  if (allValuesAreStrings) {
     if (next.length > 0) {
-      if (current) {
+      if (current && !deepEqual(current, prev)) {
         // Special case. If we have a current model, and that model is an array of strings that is different from
         // next, we'll keep the current value. This can happen if you for example have a file uploader that saves
         // IDs to a list in the data model. When one attachment got uploaded, and got saved to the backend, we started
         // uploading a new attachment (which got added to the current model), but before the previous save response
         // came back from the backend.
         return;
-      } else {
+      } else if (!current) {
         patch.push({
           op: 'test',
           path: pointer(path),
@@ -128,10 +125,17 @@ function compareArrays({ prev, next, current, hasCurrent, patch, path }: Compare
         value: next,
       });
     } else {
-      patch.push({
-        op: 'remove',
-        path: pointer(path),
-      });
+      if (current && !deepEqual(current, prev)) {
+        // Same special case as above, but for when the next array is empty (i.e. the backend hasn't seen any values in
+        // this array yet, but while saving the user has added some values to the current model, for example by
+        // uploading an attachment which adds a UUID reference).
+        return;
+      } else {
+        patch.push({
+          op: 'remove',
+          path: pointer(path),
+        });
+      }
     }
     return;
   }
@@ -196,8 +200,7 @@ function compareArrays({ prev, next, current, hasCurrent, patch, path }: Compare
   patch.push(...childPatches);
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function compareValues({ prev, next, hasCurrent, current, patch, path }: CompareProps<any>) {
+function compareValues({ prev, next, hasCurrent, current, patch, path }: CompareProps<unknown>) {
   if (prev === next) {
     return;
   }

diff --git a/src/features/options/evalQueryParameters.ts b/src/features/options/evalQueryParameters.ts
@@ -23,7 +23,7 @@ export function resolveQueryParameters(
   queryParameters: IQueryParameters | undefined,
   node: LayoutNode<CompWithBehavior<'canHaveOptions'>>,
   dataSources: ExpressionDataSources,
-) {
+): Record<string, string> | undefined {
   return queryParameters
     ? Object.entries(queryParameters).reduce((obj, [key, expr]) => {
         obj[key] = evalExpr(expr, node, dataSources, {