Merge pull request #2749 from IDEMSInternational/fix/action-param-value

fix: action parameter self-references

packages/scripts/src/commands/app-data/convert/processors/flowParser/flowParser.ts

@@ -4,7 +4,7 @@ import { IConverterPaths, IFlowHashmapByType, IParsedWorkbookData } from "../../
 import { arrayToHashmap, groupJsonByKey, IContentsEntry } from "../../utils";
 import BaseProcessor from "../base";
 
-const cacheVersion = 20241230.0;
+const cacheVersion = 20252801.0;
 
 export class FlowParserProcessor extends BaseProcessor<FlowTypes.FlowTypeWithData> {
   public parsers: { [flowType in FlowTypes.FlowType]: Parsers.DefaultParser } = {

packages/scripts/src/commands/app-data/convert/processors/flowParser/parsers/template.parser.spec.ts

@@ -6,7 +6,7 @@ const ROW_BASE: FlowTypes.TemplateRow = {
   name: "",
   type: "" as any,
 };
-
+/** yarn workspace scripts test -t template.parser.spec.ts */
 describe("Template Parser PostProcessor", () => {
   let parser: TemplateParser;
   beforeEach(() => {
@@ -118,10 +118,17 @@ describe("Template Parser PostProcessor", () => {
       ...ROW_BASE,
       name: "my_action_list",
       action_list: [
-        { trigger: "click", action_id: "", args: ["@local.my_action_list", "some_value"] },
+        {
+          trigger: "click",
+          action_id: "",
+          args: ["@local.my_action_list", "some_value"],
+          params: { param_1: "@local.my_action_list", param_2: "some_value" },
+        },
       ],
     });
+    // Should replace self references in args and params
     expect(res.action_list[0].args).toEqual(["this.value", "some_value"]);
+    expect(res.action_list[0].params).toEqual({ param_1: "this.value", param_2: "some_value" });
   });
 
   it("Extracts dynamic fields", () => {

packages/scripts/src/commands/app-data/convert/processors/flowParser/parsers/template.parser.ts

@@ -138,13 +138,22 @@ export class TemplateParser extends DefaultParser {
       return action_list;
     }
     return action_list.map((action) => {
-      if (rowName && Array.isArray(action.args)) {
-        action.args = action.args.map((arg) => {
-          if (arg === `@local.${rowName}`) {
-            arg = `this.value`;
+      if (rowName) {
+        if (Array.isArray(action.args)) {
+          action.args = action.args.map((arg) => {
+            if (arg === `@local.${rowName}`) {
+              arg = `this.value`;
+            }
+            return arg;
+          });
+        }
+        if (action.params) {
+          for (const [key, value] of Object.entries(action.params)) {
+            if (typeof value === "string" && value === `@local.${rowName}`) {
+              action.params[key] = "this.value";
+            }
           }
-          return arg;
-        });
+        }
       }
       return action;
     });

src/app/shared/components/template/services/instance/template-action.service.spec.ts

@@ -0,0 +1,127 @@
+import { TestBed } from "@angular/core/testing";
+
+import { TemplateActionService } from "./template-action.service";
+import { Injector } from "@angular/core";
+import { TemplateContainerComponent } from "../../template-container.component";
+import { TemplateActionRegistry } from "./template-action.registry";
+import { TemplateRowService } from "./template-row.service";
+import { TemplateNavService } from "../template-nav.service";
+
+// Use a stubbed template action registry to allow registering custom actions in test
+const MockTemplateActionRegistry = new TemplateActionRegistry();
+
+// HACK - templateActionService injects child dependencies from injector
+// stub all injected services with stub (can be replaced as future tests require)
+const mockInjector: Injector = {
+  get: (token: any) => {
+    switch (token) {
+      case TemplateActionRegistry:
+        return MockTemplateActionRegistry;
+      case TemplateNavService:
+        return { handleNavActionsFromChild: () => null, isReady: () => true, ready: () => true };
+      default:
+        return { ready: () => true, isReady: () => true };
+    }
+  },
+};
+
+// use mockTemplateRowService to handle set_local test actions
+class MockTemplateRowService implements Partial<TemplateRowService> {
+  templateRowMap = {
+    mock_row_1: { value: "", _nested_name: "mock_row_1", name: "mock_row_1", type: "" },
+    mock_row_2: { value: "", _nested_name: "mock_row_2", name: "mock_row_2", type: "" },
+  };
+  processRowUpdates = async () => null;
+}
+
+class MockContainer implements Partial<TemplateContainerComponent> {
+  templateRowService = new MockTemplateRowService() as any as TemplateRowService;
+  get templateRowMap() {
+    return this.templateRowService.templateRowMap;
+  }
+}
+
+/**
+ * Call standalone tests via:
+ * yarn ng test --include src/app/shared/components/template/services/instance/template-action.service.spec.ts
+ */
+describe("TemplateActionService", () => {
+  let service: TemplateActionService;
+  let setDataSpy: jasmine.Spy;
+
+  beforeEach(async () => {
+    TestBed.configureTestingModule({
+      imports: [],
+    });
+    service = new TemplateActionService(
+      mockInjector,
+      new MockContainer() as TemplateContainerComponent
+    );
+    // use mock template action registry to allow for custom handlers for testing
+    // allow registry override as single registry persists across actions
+    setDataSpy = jasmine.createSpy("set_data");
+    service["templateActionRegistry"].register(
+      { set_data: async (action) => setDataSpy(action) },
+      true
+    );
+  });
+
+  it("should create", () => {
+    expect(service).toBeTruthy();
+  });
+
+  it("set_local action", async () => {
+    await service.handleActions([
+      { trigger: "click", action_id: "set_local", args: ["mock_row_1", "updated"] },
+    ]);
+    expect(service.container.templateRowMap.mock_row_1.value).toEqual("updated");
+  });
+
+  it("set_self action", async () => {
+    await service.handleActions([
+      { trigger: "click", action_id: "set_self", args: ["mock_row_1", "updated"] },
+    ]);
+    expect(service.container.templateRowMap.mock_row_1.value).toEqual("updated");
+  });
+
+  it("Uses updated args when successive actions change same variable", async () => {
+    const _triggeredBy = { _nested_name: "mock_row_1", name: "mock_row_1", type: "" };
+    await service.handleActions(
+      [
+        { trigger: "click", action_id: "set_self", args: ["mock_row_1", "updated"] },
+        {
+          trigger: "click",
+          action_id: "set_local",
+          args: ["mock_row_2", "this.value"],
+        },
+      ],
+      _triggeredBy
+    );
+    expect(service.container.templateRowMap.mock_row_2.value).toEqual("updated");
+  });
+
+  it("Uses updated params when successive actions change same variable", async () => {
+    const _triggeredBy = { _nested_name: "mock_row_1", name: "mock_row_1", type: "" };
+    await service.handleActions(
+      [
+        { trigger: "click", action_id: "set_self", args: ["mock_row_1", "updated"] },
+        {
+          trigger: "click",
+          action_id: "set_data",
+          args: [],
+          params: { value: "this.value" },
+        },
+      ],
+      _triggeredBy
+    );
+    expect(setDataSpy).toHaveBeenCalledOnceWith({
+      trigger: "click",
+      action_id: "set_data",
+      args: [],
+      params: { value: "updated" },
+      _triggeredBy,
+    });
+  });
+
+  //   TODO - improve test coverage
+});

src/app/shared/components/template/services/instance/template-action.service.ts

@@ -205,14 +205,8 @@ export class TemplateActionService extends SyncServiceBase {
   }
 
   private async processAction(action: FlowTypes.TemplateRowAction) {
-    action.args = action.args.map((arg) => {
-      // HACK - update any self referenced values (see note from template.parser method)
-      if (typeof arg === "string" && arg.startsWith("this.")) {
-        const selfField = arg.split(".")[1];
-        arg = this.container?.templateRowMap[action._triggeredBy?._nested_name]?.[selfField];
-      }
-      return arg;
-    });
+    action = this.hackUpdateActionSelfReferenceValues(action);
+
     const { action_id, args } = action;
 
     // Call any action registered with global handler
@@ -334,6 +328,33 @@ export class TemplateActionService extends SyncServiceBase {
     }
   }
 
+  // HACK - update any self referenced values (see note from template.parser method)
+  // This workaround is required in order for self referenced values in action args and params to
+  // access the up-to-date value, as opposed to the value as it was when the action was originally parsed
+  // See https://github.com/IDEMSInternational/open-app-builder/pull/2749
+  private hackUpdateActionSelfReferenceValues(
+    action: FlowTypes.TemplateRowAction
+  ): FlowTypes.TemplateRowAction {
+    // Update action.args and action.params
+    const currentValue = this.container?.templateRowMap?.[action._triggeredBy?._nested_name]?.value;
+    if (action.args) {
+      action.args = action.args.map((arg) => {
+        if (arg === "this.value") {
+          return currentValue;
+        }
+        return arg;
+      });
+    }
+    if (action.params) {
+      for (const [key, value] of Object.entries(action.params)) {
+        if (value === "this.value") {
+          action.params[key] = currentValue;
+        }
+      }
+    }
+    return action;
+  }
+
   /**
    * Update a local template row
    *