Refactor: template parser answer list support

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

@@ -5,7 +5,7 @@ import { arrayToHashmap, groupJsonByKey, IContentsEntry } from "../../utils";
 import BaseProcessor from "../base";
 
 export class FlowParserProcessor extends BaseProcessor<FlowTypes.FlowTypeWithData> {
-  public cacheVersion = 20230818.3;
+  public cacheVersion = 20240220.2;
 
   public parsers: { [flowType in FlowTypes.FlowType]: Parsers.DefaultParser } = {
     data_list: new Parsers.DataListParser(this),

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

@@ -44,13 +44,16 @@ export class DefaultParser<
       this.queue.shift();
     }
     // Process queue
+    let rowNumber = 1;
     while (this.queue.length > 0) {
+      // Start rowNumber from 2 to match sheet (without header row)
+      rowNumber++;
       const row = this.queue[0];
       try {
         const processed = new RowProcessor(row, this, rowDefaultValues).run();
         // some rows may be omitted during processing so ignore
         if (processed) {
-          const postProcessed = this.postProcessRow(processed);
+          const postProcessed = this.postProcessRow(processed, rowNumber);
           if (postProcessed) {
             processedRows.push(postProcessed);
           }
@@ -87,7 +90,7 @@ export class DefaultParser<
 
   /** Overridable method called by parser to apply any additional processing
    * on each individual row. By default the original row is simply returned */
-  public postProcessRow(row: any) {
+  public postProcessRow(row: any, rowNumber: number) {
     return row;
   }
 

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

@@ -1,3 +1,179 @@
-describe("template Parser", () => {
-  it("TODO - add tests", () => expect(true).toEqual(true));
+import { FlowTypes } from "data-models";
+import { TemplateParser } from "./template.parser";
+
+const ROW_BASE: FlowTypes.TemplateRow = {
+  _nested_name: "",
+  name: "",
+  type: "" as any,
+};
+
+describe("Template Parser PostProcessor", () => {
+  let parser: TemplateParser;
+  beforeEach(() => {
+    parser = new TemplateParser({} as any);
+  });
+
+  it("Remove empty rows", () => {
+    const res = parser.postProcessRow({} as any);
+    expect(res).toEqual(undefined);
+  });
+
+  it("Replaces empty row type with set_variable", () => {
+    const res = parser.postProcessRow({ ...ROW_BASE, type: "" as any });
+    expect(res.type).toEqual("set_variable");
+  });
+
+  // TODO - notify authors of change (can now auto gen multiple)
+  it("Generates default row names when omitted for multiple rows", () => {
+    const res = parser.run({
+      flow_name: "test_name_generate_multiple",
+      flow_type: "template",
+      rows: [
+        { ...ROW_BASE, name: "", type: "accordion" },
+        { ...ROW_BASE, name: "", type: "accordion" },
+      ],
+    });
+    expect(res.rows).toEqual([
+      // Note - suffix numbers start at 2 as flow sheet would typically have row 1 as header row
+      {
+        _nested_name: "accordion_2",
+        name: "accordion_2",
+        type: "accordion",
+      },
+      {
+        _nested_name: "accordion_3",
+        name: "accordion_3",
+        type: "accordion",
+      },
+    ]);
+  });
+
+  it("Converts rows with _list in name to templated list", () => {
+    // CASE 0 - Do not convert rows without _list in name
+    const case0 = parser.postProcessRow({
+      ...ROW_BASE,
+      value: "item_1; item_2; item_3;",
+    });
+    expect(case0.value).toEqual("item_1; item_2; item_3;");
+
+    // CASE 1 - List items defined inline
+    const case1 = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "test_list",
+      value: "item_1; item_2; item_3;",
+    });
+    expect(case1.value).toEqual(["item_1", "item_2", "item_3"]);
+    // CASE 2 - List refers to variable (do not parse)
+    const case2 = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "test_list",
+      value: "@local.some_list",
+    });
+    expect(case2.value).toEqual("@local.some_list");
+    // CASE 3 - List items include variables
+    const case3 = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "test_list",
+      value: "@local.item_1; @local.item_2",
+    });
+    expect(case3.value).toEqual(["@local.item_1", "@local.item_2"]);
+    // CASE 4 - List items as json (mix dynamic and missing)
+    const case4 = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "test_list",
+      value: "key_1a: textValue | key_1b: @local.value; key_2a:  | key_2b: 5",
+    });
+    expect(case4.value).toEqual([
+      { key_1a: "textValue", key_1b: "@local.value" },
+      { key_2a: "", key_2b: "5" },
+    ]);
+  });
+
+  it("Converts rows with _collection in name to templated collection", () => {
+    const res = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "test_collection",
+      value: "key_1:value_1; key_2:value_2",
+    });
+    expect(res.value).toEqual({ key_1: "value_1", key_2: "value_2" });
+  });
+
+  it("Parses parameter lists", () => {
+    const res = parser.postProcessRow({
+      ...ROW_BASE,
+      // NOTE - list already array due to defaultParser initial parse
+      parameter_list: ["key_1:val_1", "key_2: val_trailing_spaces   ", "key_3_no_value"] as any,
+    });
+    // TODO - anymore advanced cases?
+    expect(res.parameter_list).toEqual({
+      key_1: "val_1",
+      key_2: "val_trailing_spaces",
+      key_3_no_value: "true",
+    });
+  });
+
+  it("Replaces action list self-references", () => {
+    const res = parser.postProcessRow({
+      ...ROW_BASE,
+      name: "my_action_list",
+      action_list: [
+        { trigger: "click", action_id: "", args: ["@local.my_action_list", "some_value"] },
+      ],
+    });
+    expect(res.action_list[0].args).toEqual(["this.value", "some_value"]);
+  });
+
+  it("Extracts dynamic fields", () => {
+    const res = parser.postProcessRow({
+      ...ROW_BASE,
+      value: "@local.dynamic_value",
+    });
+    expect(res._dynamicFields).toEqual({
+      value: [
+        {
+          fullExpression: "@local.dynamic_value",
+          matchedExpression: "@local.dynamic_value",
+          type: "local",
+          fieldName: "dynamic_value",
+        },
+      ],
+    });
+  });
+
+  it("Extracts dynamic dependencies", () => {
+    const res = parser.postProcessRow({
+      ...ROW_BASE,
+      value: "@local.dynamic_value",
+    });
+    expect(res._dynamicDependencies).toEqual({ "@local.dynamic_value": ["value"] });
+  });
+
+  it("Creates nested path names for child rows", () => {
+    const rows: FlowTypes.TemplateRow[] = [
+      {
+        _nested_name: "my_items",
+        name: "my_items",
+        type: "items",
+        // Handle case of both named and unnamed nested row names
+        rows: [
+          {
+            ...ROW_BASE,
+            type: "text",
+            name: "named_text",
+          },
+          { ...ROW_BASE, type: "text" },
+        ],
+      },
+    ];
+    const res = parser.run({ flow_type: "template", flow_name: "test_nested", rows });
+    const itemRows = res.rows[0].rows;
+    const nestedNames = itemRows.map((n) => n._nested_name);
+    expect(nestedNames).toEqual(["my_items.named_text", "my_items.text_2"]);
+  });
+});
+
+describe("Template Parser [QC]", () => {
+  // TODO - confirm what checks to include and add to code
+  // it("Ensures answer_list parameters refer to list variables", () => {
+  // });
 });

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

@@ -9,7 +9,7 @@ import {
 } from "../../../utils";
 
 export class TemplateParser extends DefaultParser {
-  postProcessRow(row: FlowTypes.TemplateRow, nestedPath?: string) {
+  postProcessRow(row: FlowTypes.TemplateRow, rowNumber = 1, nestedPath?: string) {
     // remove empty rows
     if (Object.keys(row).length === 0) {
       return;
@@ -28,16 +28,16 @@ export class TemplateParser extends DefaultParser {
       if (row.type === "template") {
         row.name = row.value;
       } else {
-        row.name = row.type;
+        row.name = `${row.type}_${rowNumber}`;
       }
     }
     // track path to row when nested
     row._nested_name = nestedPath ? `${nestedPath}.${row.name}` : row.name;
-
     // convert any variables (local/global) list or collection strings (e.g. 'my_list_1')
+    // in similar way to how top-level properties get converted by default parser
     if (row.value && typeof row.value === "string") {
-      if (row.name?.includes("_list") && row.value && typeof row.value === "string") {
-        row.value = parseAppDataListString(row.value);
+      if (row.name?.includes("_list")) {
+        row.value = this.parseTemplateList(row.value);
       }
       if (row.name?.includes("_collection") && row.value && typeof row.value === "string") {
         row.value = parseAppDataCollectionString(row.value);
@@ -58,14 +58,19 @@ export class TemplateParser extends DefaultParser {
 
     // handle nested rows in same way
     if (row.rows) {
-      row.rows = row.rows.map((r) => this.postProcessRow(r, row._nested_name));
+      row.rows = row.rows.map((r, i) => this.postProcessRow(r, i + 1, row._nested_name));
     }
 
     if (row.exclude_from_translation) {
       row.exclude_from_translation = this.parseExcludeFromTranslation(
         row.exclude_from_translation as any
       );
     }
+
+    const errors = this.qualityControlCheck(row);
+    if (errors.length > 0) {
+      throw JSON.stringify(errors, null, 2);
+    }
     return row;
   }
 
@@ -74,6 +79,36 @@ export class TemplateParser extends DefaultParser {
     return flowsWithOverrides;
   }
 
+  /**
+   * Ensure any local variables defined with `_list` in their name are correctly
+   * parsed into list format
+   */
+  private parseTemplateList(value: any) {
+    // Assume all falsy values indicate an empty array
+    if (!value) return [];
+
+    // Assume any non-string values already parsed
+    if (typeof value !== "string") return value;
+
+    // HACK - use list separator to infer whether an actual list or not
+    // E.g. avoid parsing reference `my_list : @local.some_other_list`
+    // TODO - make clear to authors new convention
+    if (!value.includes(";")) return value;
+
+    // HACK - assume any list with | characters designed as parameter list
+    const isCollectionList = value.includes("|");
+
+    // convert to array
+    let parsed: any[] = parseAppDataListString(value);
+
+    // map array elements if collection list
+    if (isCollectionList) {
+      parsed = parsed.map((el: string) => parseAppDataCollectionString(el, "|"));
+    }
+    return parsed;
+  }
+
+  /** Convert parameter list string array (as provided by default parser) to key-value pairs */
   private parseParameterList(parameterList: string[]) {
     const parameterObj: FlowTypes.TemplateRow["parameter_list"] = {};
     parameterList.forEach((p) => {
@@ -116,4 +151,10 @@ export class TemplateParser extends DefaultParser {
       return action;
     });
   }
+
+  private qualityControlCheck(row: FlowTypes.TemplateRow) {
+    const errors: string[] = [];
+
+    return errors;
+  }
 }

packages/scripts/src/commands/app-data/convert/utils/app-data-string.utils.ts

@@ -1,33 +1,6 @@
-import chalk from "chalk";
 import { FlowTypes } from "data-models";
 import { setNestedProperty, booleanStringToBoolean } from "../utils";
 
-/**
- * Xls data represents nested objects in the following ways
- * ';' - pre-processing with '_list' columns to format as array
- * '|' - post-processing specific item into set of arguments / parameters
- * ':' - modifiers or properties of an argument
- *
- * As the pipe and colon characters may or may not exist for a particular string
- * it is impossible to know any given data needs to be formatted as string or array
- * to remain consistent with the rest of the column. As such all strings will be
- * treated as arrays, and deeply nested objects extracted in future processing stages
- *
- * original:  db_lookup:first |app_events:event_id | app_launch | before:7:day'
- * nest 1:    [db_lookup:first ,app_events:event_id , app_launch , before:7:day]
- * nest 2:    [[db_lookup,first] ,[app_events,event_id] , [app_launch] , [before,7,day]]
- *
- */
-export function parsAppDataString(str: string): string[][] {
-  if (str.includes(";")) {
-    console.error(chalk.red('lists should be pre-processed, but ";" found'));
-    process.exit(1);
-  }
-  const nest1 = str.split("|").map((d) => d.trim());
-  const nest2 = nest1.map((el) => el.split(":").map((d) => d.trim()));
-  return nest2;
-}
-
 /**
  * Convert app data map string to object
  * @param str list string with key-value pairs, e.g