Merge remote-tracking branch 'origin/v3-ui' into feature/automation-branching-ux

packages/backend-core/src/sql/sql.ts

@@ -71,18 +71,6 @@ function prioritisedArraySort(toSort: string[], priorities: string[]) {
   })
 }
 
-function getTableName(table?: Table): string | undefined {
-  // SQS uses the table ID rather than the table name
-  if (
-    table?.sourceType === TableSourceType.INTERNAL ||
-    table?.sourceId === INTERNAL_TABLE_SOURCE_ID
-  ) {
-    return table?._id
-  } else {
-    return table?.name
-  }
-}
-
 function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] {
   if (Array.isArray(query)) {
     return query.map((q: SqlQuery) => convertBooleans(q) as SqlQuery)
@@ -99,6 +87,13 @@ function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] {
   return query
 }
 
+function isSqs(table: Table): boolean {
+  return (
+    table.sourceType === TableSourceType.INTERNAL ||
+    table.sourceId === INTERNAL_TABLE_SOURCE_ID
+  )
+}
+
 class InternalBuilder {
   private readonly client: SqlClient
   private readonly query: QueryJson
@@ -180,15 +175,13 @@ class InternalBuilder {
   }
 
   private generateSelectStatement(): (string | Knex.Raw)[] | "*" {
-    const { meta, endpoint, resource, tableAliases } = this.query
+    const { meta, endpoint, resource } = this.query
 
     if (!resource || !resource.fields || resource.fields.length === 0) {
       return "*"
     }
 
-    const alias = tableAliases?.[endpoint.entityId]
+    const alias = this.getTableName(endpoint.entityId)
-      ? tableAliases?.[endpoint.entityId]
-      : endpoint.entityId
     const schema = meta.table.schema
     if (!this.isFullSelectStatementRequired()) {
       return [this.knex.raw(`${this.quote(alias)}.*`)]
@@ -813,17 +806,48 @@ class InternalBuilder {
     return query
   }
 
+  isSqs(): boolean {
+    return isSqs(this.table)
+  }
+
+  getTableName(tableOrName?: Table | string): string {
+    let table: Table
+    if (typeof tableOrName === "string") {
+      const name = tableOrName
+      if (this.query.table?.name === name) {
+        table = this.query.table
+      } else if (this.query.meta.table?.name === name) {
+        table = this.query.meta.table
+      } else if (!this.query.meta.tables?.[name]) {
+        // This can legitimately happen in custom queries, where the user is
+        // querying against a table that may not have been imported into
+        // Budibase.
+        return name
+      } else {
+        table = this.query.meta.tables[name]
+      }
+    } else if (tableOrName) {
+      table = tableOrName
+    } else {
+      table = this.table
+    }
+
+    let name = table.name
+    if (isSqs(table) && table._id) {
+      // SQS uses the table ID rather than the table name
+      name = table._id
+    }
+    const aliases = this.query.tableAliases || {}
+    return aliases[name] ? aliases[name] : name
+  }
+
   addDistinctCount(query: Knex.QueryBuilder): Knex.QueryBuilder {
-    const primary = this.table.primary
+    if (!this.table.primary) {
-    const aliases = this.query.tableAliases
-    const aliased =
-      this.table.name && aliases?.[this.table.name]
-        ? aliases[this.table.name]
-        : this.table.name
-    if (!primary) {
       throw new Error("SQL counting requires primary key to be supplied")
     }
-    return query.countDistinct(`${aliased}.${primary[0]} as total`)
+    return query.countDistinct(
+      `${this.getTableName()}.${this.table.primary[0]} as __bb_total`
+    )
   }
 
   addAggregations(
@@ -831,12 +855,14 @@ class InternalBuilder {
     aggregations: Aggregation[]
   ): Knex.QueryBuilder {
     const fields = this.query.resource?.fields || []
+    const tableName = this.getTableName()
     if (fields.length > 0) {
-      query = query.groupBy(fields.map(field => `${this.table.name}.${field}`))
+      query = query.groupBy(fields.map(field => `${tableName}.${field}`))
+      query = query.select(fields.map(field => `${tableName}.${field}`))
     }
     for (const aggregation of aggregations) {
       const op = aggregation.calculationType
-      const field = `${this.table.name}.${aggregation.field} as ${aggregation.name}`
+      const field = `${tableName}.${aggregation.field} as ${aggregation.name}`
       switch (op) {
         case CalculationType.COUNT:
           query = query.count(field)
@@ -861,10 +887,7 @@ class InternalBuilder {
   addSorting(query: Knex.QueryBuilder): Knex.QueryBuilder {
     let { sort, resource } = this.query
     const primaryKey = this.table.primary
-    const tableName = getTableName(this.table)
+    const aliased = this.getTableName()
-    const aliases = this.query.tableAliases
-    const aliased =
-      tableName && aliases?.[tableName] ? aliases[tableName] : this.table?.name
     if (!Array.isArray(primaryKey)) {
       throw new Error("Sorting requires primary key to be specified for table")
     }
@@ -1508,23 +1531,40 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
     return results.length ? results : [{ [operation.toLowerCase()]: true }]
   }
 
+  private getTableName(
+    table: Table,
+    aliases?: Record<string, string>
+  ): string | undefined {
+    let name = table.name
+    if (
+      table.sourceType === TableSourceType.INTERNAL ||
+      table.sourceId === INTERNAL_TABLE_SOURCE_ID
+    ) {
+      if (!table._id) {
+        return
+      }
+      // SQS uses the table ID rather than the table name
+      name = table._id
+    }
+    return aliases?.[name] || name
+  }
+
   convertJsonStringColumns<T extends Record<string, any>>(
     table: Table,
     results: T[],
     aliases?: Record<string, string>
   ): T[] {
-    const tableName = getTableName(table)
+    const tableName = this.getTableName(table, aliases)
     for (const [name, field] of Object.entries(table.schema)) {
       if (!this._isJsonColumn(field)) {
         continue
       }
-      const aliasedTableName = (tableName && aliases?.[tableName]) || tableName
+      const fullName = `${tableName}.${name}` as keyof T
-      const fullName = `${aliasedTableName}.${name}`
       for (let row of results) {
-        if (typeof row[fullName as keyof T] === "string") {
+        if (typeof row[fullName] === "string") {
-          row[fullName as keyof T] = JSON.parse(row[fullName])
+          row[fullName] = JSON.parse(row[fullName])
         }
-        if (typeof row[name as keyof T] === "string") {
+        if (typeof row[name] === "string") {
           row[name as keyof T] = JSON.parse(row[name])
         }
       }

packages/builder/src/pages/builder/app/[application]/data/table/[tableId]/[viewId]/index.svelte

@@ -3,6 +3,7 @@
   import { admin, themeStore } from "stores/portal"
   import { Grid } from "@budibase/frontend-core"
   import { API } from "api"
+  import { notifications } from "@budibase/bbui"
   import GridCreateEditRowModal from "components/backend/DataTable/modals/grid/GridCreateEditRowModal.svelte"
   import GridFilterButton from "components/backend/DataTable/buttons/grid/GridFilterButton.svelte"
   import GridManageAccessButton from "components/backend/DataTable/buttons/grid/GridManageAccessButton.svelte"
@@ -26,14 +27,12 @@
   $: currentTheme = $themeStore?.theme
   $: darkMode = !currentTheme.includes("light")
 
-  $: currentTheme = $themeStore?.theme
-  $: darkMode = !currentTheme.includes("light")
-
   const makeRowActionButtons = actions => {
     return (actions || []).map(action => ({
       text: action.name,
       onClick: async row => {
         await rowActions.trigger(id, action.id, row._id)
+        notifications.success("Row action triggered successfully")
       },
     }))
   }

packages/builder/src/pages/builder/app/[application]/data/table/[tableId]/index.svelte

@@ -1,5 +1,5 @@
 <script>
-  import { Banner } from "@budibase/bbui"
+  import { Banner, notifications } from "@budibase/bbui"
   import {
     datasources,
     tables,
@@ -67,6 +67,7 @@
       text: action.name,
       onClick: async row => {
         await rowActions.trigger(id, action.id, row._id)
+        notifications.success("Row action triggered successfully")
       },
     }))
   }

packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte

@@ -98,7 +98,6 @@
                   align="right"
                   offset={5}
                   size="S"
-                  quiet
                   animate={false}
                   on:mouseenter={() => ($hoveredRowId = row._id)}
                 />

packages/server/src/api/routes/tests/viewV2.spec.ts

@@ -2458,6 +2458,93 @@ describe.each([
               expect("_id" in row).toBe(false)
             }
           })
+
+          it("should be able to group by a basic field", async () => {
+            const view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              schema: {
+                quantity: {
+                  visible: true,
+                  field: "quantity",
+                },
+                "Total Price": {
+                  visible: true,
+                  calculationType: CalculationType.SUM,
+                  field: "price",
+                },
+              },
+            })
+
+            const response = await config.api.viewV2.search(view.id, {
+              query: {},
+            })
+
+            const priceByQuantity: Record<number, number> = {}
+            for (const row of rows) {
+              priceByQuantity[row.quantity] ??= 0
+              priceByQuantity[row.quantity] += row.price
+            }
+
+            for (const row of response.rows) {
+              expect(row["Total Price"]).toEqual(priceByQuantity[row.quantity])
+            }
+          })
+
+          it.each([
+            CalculationType.COUNT,
+            CalculationType.SUM,
+            CalculationType.AVG,
+            CalculationType.MIN,
+            CalculationType.MAX,
+          ])("should be able to calculate $type", async type => {
+            const view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              schema: {
+                aggregate: {
+                  visible: true,
+                  calculationType: type,
+                  field: "price",
+                },
+              },
+            })
+
+            const response = await config.api.viewV2.search(view.id, {
+              query: {},
+            })
+
+            function calculate(
+              type: CalculationType,
+              numbers: number[]
+            ): number {
+              switch (type) {
+                case CalculationType.COUNT:
+                  return numbers.length
+                case CalculationType.SUM:
+                  return numbers.reduce((a, b) => a + b, 0)
+                case CalculationType.AVG:
+                  return numbers.reduce((a, b) => a + b, 0) / numbers.length
+                case CalculationType.MIN:
+                  return Math.min(...numbers)
+                case CalculationType.MAX:
+                  return Math.max(...numbers)
+              }
+            }
+
+            const prices = rows.map(row => row.price)
+            const expected = calculate(type, prices)
+            const actual = response.rows[0].aggregate
+
+            if (type === CalculationType.AVG) {
+              // The average calculation can introduce floating point rounding
+              // errors, so we need to compare to within a small margin of
+              // error.
+              expect(actual).toBeCloseTo(expected)
+            } else {
+              expect(actual).toEqual(expected)
+            }
+          })
         })
     })
 

packages/server/src/automations/tests/scenarios/branching.spec.ts

@@ -17,44 +17,65 @@ describe("Branching automations", () => {
   afterAll(setup.afterAll)
 
   it("should run a multiple nested branching automation", async () => {
+    const firstLogId = "11111111-1111-1111-1111-111111111111"
+    const branch1LogId = "22222222-2222-2222-2222-222222222222"
+    const branch2LogId = "33333333-3333-3333-3333-333333333333"
+    const branch2Id = "44444444-4444-4444-4444-444444444444"
+
     const builder = createAutomationBuilder({
       name: "Test Trigger with Loop and Create Row",
     })
 
     const results = await builder
       .appAction({ fields: {} })
-      .serverLog({ text: "Starting automation" })
+      .serverLog(
+        { text: "Starting automation" },
+        { stepName: "FirstLog", stepId: firstLogId }
+      )
       .branch({
         topLevelBranch1: {
           steps: stepBuilder =>
-            stepBuilder.serverLog({ text: "Branch 1" }).branch({
+            stepBuilder
+              .serverLog(
+                { text: "Branch 1" },
+                { stepId: "66666666-6666-6666-6666-666666666666" }
+              )
+              .branch({
                 branch1: {
                   steps: stepBuilder =>
-                  stepBuilder.serverLog({ text: "Branch 1.1" }),
+                    stepBuilder.serverLog(
+                      { text: "Branch 1.1" },
+                      { stepId: branch1LogId }
+                    ),
                   condition: {
-                  equal: { "{{steps.1.success}}": true },
+                    equal: { [`{{ steps.${firstLogId}.success }}`]: true },
                   },
                 },
                 branch2: {
                   steps: stepBuilder =>
-                  stepBuilder.serverLog({ text: "Branch 1.2" }),
+                    stepBuilder.serverLog(
+                      { text: "Branch 1.2" },
+                      { stepId: branch2LogId }
+                    ),
                   condition: {
-                  equal: { "{{steps.1.success}}": false },
+                    equal: { [`{{ steps.${firstLogId}.success }}`]: false },
                   },
                 },
               }),
           condition: {
-            equal: { "{{steps.1.success}}": true },
+            equal: { [`{{ steps.${firstLogId}.success }}`]: true },
           },
         },
         topLevelBranch2: {
-          steps: stepBuilder => stepBuilder.serverLog({ text: "Branch 2" }),
+          steps: stepBuilder =>
+            stepBuilder.serverLog({ text: "Branch 2" }, { stepId: branch2Id }),
           condition: {
-            equal: { "{{steps.1.success}}": false },
+            equal: { [`{{ steps.${firstLogId}.success }}`]: false },
           },
         },
       })
       .run()
+
     expect(results.steps[3].outputs.status).toContain("branch1 branch taken")
     expect(results.steps[4].outputs.message).toContain("Branch 1.1")
   })

packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts

@@ -64,18 +64,18 @@ class BaseStepBuilder {
     stepId: TStep,
     stepSchema: Omit<AutomationStep, "id" | "stepId" | "inputs">,
     inputs: AutomationStepInputs<TStep>,
-    stepName?: string
+    opts?: { stepName?: string; stepId?: string }
   ): this {
-    const id = uuidv4()
+    const id = opts?.stepId || uuidv4()
     this.steps.push({
       ...stepSchema,
       inputs: inputs as any,
       id,
       stepId,
-      name: stepName || stepSchema.name,
+      name: opts?.stepName || stepSchema.name,
     })
-    if (stepName) {
+    if (opts?.stepName) {
-      this.stepNames[id] = stepName
+      this.stepNames[id] = opts.stepName
     }
     return this
   }
@@ -95,7 +95,6 @@ class BaseStepBuilder {
       })
       branchStepInputs.children![key] = stepBuilder.build()
     })
-
     const branchStep: AutomationStep = {
       ...definition,
       id: uuidv4(),
@@ -106,80 +105,98 @@ class BaseStepBuilder {
   }
 
   // STEPS
-  createRow(inputs: CreateRowStepInputs, opts?: { stepName?: string }): this {
+  createRow(
+    inputs: CreateRowStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.CREATE_ROW,
       BUILTIN_ACTION_DEFINITIONS.CREATE_ROW,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
-  updateRow(inputs: UpdateRowStepInputs, opts?: { stepName?: string }): this {
+  updateRow(
+    inputs: UpdateRowStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.UPDATE_ROW,
       BUILTIN_ACTION_DEFINITIONS.UPDATE_ROW,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
-  deleteRow(inputs: DeleteRowStepInputs, opts?: { stepName?: string }): this {
+  deleteRow(
+    inputs: DeleteRowStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.DELETE_ROW,
       BUILTIN_ACTION_DEFINITIONS.DELETE_ROW,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
   sendSmtpEmail(
     inputs: SmtpEmailStepInputs,
-    opts?: { stepName?: string }
+    opts?: { stepName?: string; stepId?: string }
   ): this {
     return this.step(
       AutomationActionStepId.SEND_EMAIL_SMTP,
       BUILTIN_ACTION_DEFINITIONS.SEND_EMAIL_SMTP,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
   executeQuery(
     inputs: ExecuteQueryStepInputs,
-    opts?: { stepName?: string }
+    opts?: { stepName?: string; stepId?: string }
   ): this {
     return this.step(
       AutomationActionStepId.EXECUTE_QUERY,
       BUILTIN_ACTION_DEFINITIONS.EXECUTE_QUERY,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
-  queryRows(inputs: QueryRowsStepInputs, opts?: { stepName?: string }): this {
+  queryRows(
+    inputs: QueryRowsStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.QUERY_ROWS,
       BUILTIN_ACTION_DEFINITIONS.QUERY_ROWS,
       inputs,
-      opts?.stepName
+      opts
     )
   }
-  loop(inputs: LoopStepInputs, opts?: { stepName?: string }): this {
+  loop(
+    inputs: LoopStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.LOOP,
       BUILTIN_ACTION_DEFINITIONS.LOOP,
       inputs,
-      opts?.stepName
+      opts
     )
   }
 
-  serverLog(input: ServerLogStepInputs, opts?: { stepName?: string }): this {
+  serverLog(
+    input: ServerLogStepInputs,
+    opts?: { stepName?: string; stepId?: string }
+  ): this {
     return this.step(
       AutomationActionStepId.SERVER_LOG,
       BUILTIN_ACTION_DEFINITIONS.SERVER_LOG,
       input,
-      opts?.stepName
+      opts
     )
   }
 

packages/server/src/definitions/automations.ts

@@ -15,7 +15,8 @@ export interface TriggerOutput {
 
 export interface AutomationContext extends AutomationResults {
   steps: any[]
-  stepsByName?: Record<string, any>
+  stepsById: Record<string, any>
+  stepsByName: Record<string, any>
   env?: Record<string, string>
   trigger: any
 }

packages/server/src/sdk/app/rows/utils.ts

@@ -57,8 +57,8 @@ export function getSQLClient(datasource: Datasource): SqlClient {
 export function processRowCountResponse(
   response: DatasourcePlusQueryResponse
 ): number {
-  if (response && response.length === 1 && "total" in response[0]) {
+  if (response && response.length === 1 && "__bb_total" in response[0]) {
-    const total = response[0].total
+    const total = response[0].__bb_total
     return typeof total === "number" ? total : parseInt(total)
   } else {
     throw new Error("Unable to count rows in query - no count response")

packages/server/src/threads/automation.ts

@@ -74,7 +74,7 @@ class Orchestrator {
   private job: Job
   private loopStepOutputs: LoopStep[]
   private stopped: boolean
-  private executionOutput: AutomationContext
+  private executionOutput: Omit<AutomationContext, "stepsByName" | "stepsById">
 
   constructor(job: AutomationJob) {
     let automation = job.data.automation
@@ -91,6 +91,7 @@ class Orchestrator {
     // step zero is never used as the template string is zero indexed for customer facing
     this.context = {
       steps: [{}],
+      stepsById: {},
       stepsByName: {},
       trigger: triggerOutput,
     }
@@ -457,8 +458,9 @@ class Orchestrator {
         inputs: steps[stepToLoopIndex].inputs,
       })
 
+      this.context.stepsById[steps[stepToLoopIndex].id] = tempOutput
       const stepName = steps[stepToLoopIndex].name || steps[stepToLoopIndex].id
-      this.context.stepsByName![stepName] = tempOutput
+      this.context.stepsByName[stepName] = tempOutput
       this.context.steps[this.context.steps.length] = tempOutput
       this.context.steps = this.context.steps.filter(
         item => !item.hasOwnProperty.call(item, "currentItem")
@@ -517,7 +519,10 @@ class Orchestrator {
         Object.entries(filter).forEach(([_, value]) => {
           Object.entries(value).forEach(([field, _]) => {
             const updatedField = field.replace("{{", "{{ literal ")
-            const fromContext = processStringSync(updatedField, this.context)
+            const fromContext = processStringSync(
+              updatedField,
+              this.processContext(this.context)
+            )
             toFilter[field] = fromContext
           })
         })
@@ -563,9 +568,9 @@ class Orchestrator {
         }
 
         const stepFn = await this.getStepFunctionality(step.stepId)
-        let inputs = await this.addContextAndProcess(
+        let inputs = await processObject(
           originalStepInput,
-          this.context
+          this.processContext(this.context)
         )
 
         inputs = automationUtils.cleanInputValues(inputs, step.schema.inputs)
@@ -594,16 +599,16 @@ class Orchestrator {
     return null
   }
 
-  private async addContextAndProcess(inputs: any, context: any) {
+  private processContext(context: AutomationContext) {
     const processContext = {
       ...context,
       steps: {
         ...context.steps,
+        ...context.stepsById,
         ...context.stepsByName,
       },
     }
-
+    return processContext
-    return processObject(inputs, processContext)
   }
 
   private handleStepOutput(
@@ -623,6 +628,7 @@ class Orchestrator {
     } else {
       this.updateExecutionOutput(step.id, step.stepId, step.inputs, outputs)
       this.context.steps[this.context.steps.length] = outputs
+      this.context.stepsById![step.id] = outputs
       const stepName = step.name || step.id
       this.context.stepsByName![stepName] = outputs
     }

packages/server/src/utilities/rowProcessor/index.ts

@@ -426,6 +426,25 @@ export async function coreOutputProcessing(
         }
       }
     }
+
+    if (sdk.views.isView(source)) {
+      const calculationFields = Object.keys(
+        helpers.views.calculationFields(source)
+      )
+
+      // We ensure all calculation fields are returned as numbers.  During the
+      // testing of this feature it was discovered that the COUNT operation
+      // returns a string for MySQL, MariaDB, and Postgres. But given that all
+      // calculation fields should be numbers, we blanket make sure of that
+      // here.
+      for (const key of calculationFields) {
+        for (const row of rows) {
+          if (typeof row[key] === "string") {
+            row[key] = parseFloat(row[key])
+          }
+        }
+      }
+    }
   }
 
   if (!isUserMetadataTable(table._id!)) {