From cc2605a9dd7b7a8eedb9889205aa060d1130f5b7 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 21 Oct 2024 10:04:03 +0100
Subject: [PATCH 01/34] WIP

---
 packages/backend-core/src/sql/sql.ts               |  2 +-
 .../server/src/api/routes/tests/search.spec.ts     | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 949d7edf1b..0a9d55cf59 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -236,7 +236,7 @@ class InternalBuilder {
     const alias = this.getTableName(endpoint.entityId)
     const schema = meta.table.schema
     if (!this.isFullSelectStatementRequired()) {
-      return [this.knex.raw(`${this.quote(alias)}.*`)]
+      return [this.knex.raw("??", [`${alias}.*`])]
     }
     // get just the fields for this table
     return resource.fields
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index e0143c5938..1c36a6577c 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -48,14 +48,14 @@ import { generateRowIdField } from "../../../integrations/utils"
 import { cloneDeep } from "lodash/fp"
 
 describe.each([
-  ["in-memory", undefined],
-  ["lucene", undefined],
-  ["sqs", undefined],
+  // ["in-memory", undefined],
+  // ["lucene", undefined],
+  // ["sqs", undefined],
   [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
-  [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
+  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("search (%s)", (name, dsProvider) => {
   const isSqs = name === "sqs"
   const isLucene = name === "lucene"

From 44bd00a0d7be259b0d4a54b8421f6537337d948c Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 21 Oct 2024 18:20:52 +0100
Subject: [PATCH 02/34] Making progress on converting raw calls to use
 bindings.

---
 packages/backend-core/src/sql/sql.ts          | 138 ++++++++----------
 .../src/api/routes/tests/search.spec.ts       |  52 +++----
 .../src/api/routes/tests/viewV2.spec.ts       |  12 +-
 3 files changed, 90 insertions(+), 112 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 0a9d55cf59..ec22ac37f9 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -258,30 +258,38 @@ class InternalBuilder {
         const columnSchema = schema[column]
 
         if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) {
-          return this.knex.raw(
-            `${this.quotedIdentifier(
-              [table, column].join(".")
-            )}::money::numeric as ${this.quote(field)}`
-          )
+          return this.knex.raw("??::money::numeric as ??", [
+            [table, column].join("."),
+            field,
+          ])
         }
 
         if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) {
           // Time gets returned as timestamp from mssql, not matching the expected
           // HH:mm format
-          return this.knex.raw(`CONVERT(varchar, ${field}, 108) as "${field}"`)
+
+          // TODO: figure out how to express this safely without string
+          // interpolation.
+          return this.knex.raw(`CONVERT(varchar, ??, 108) as "${field}"`, [
+            field,
+          ])
         }
 
-        const quoted = table
-          ? `${this.quote(table)}.${this.quote(column)}`
-          : this.quote(field)
-        return this.knex.raw(quoted)
+        if (table) {
+          return this.knex.raw("??", [`${table}.${column}`])
+        } else {
+          return this.knex.raw("??", [field])
+        }
       })
   }
 
   // OracleDB can't use character-large-objects (CLOBs) in WHERE clauses,
   // so when we use them we need to wrap them in to_char(). This function
   // converts a field name to the appropriate identifier.
-  private convertClobs(field: string, opts?: { forSelect?: boolean }): string {
+  private convertClobs(
+    field: string,
+    opts?: { forSelect?: boolean }
+  ): Knex.Raw {
     if (this.client !== SqlClient.ORACLE) {
       throw new Error(
         "you've called convertClobs on a DB that's not Oracle, this is a mistake"
@@ -290,7 +298,7 @@ class InternalBuilder {
     const parts = this.splitIdentifier(field)
     const col = parts.pop()!
     const schema = this.table.schema[col]
-    let identifier = this.quotedIdentifier(field)
+    let identifier = this.knex.raw("??", [field])
 
     if (
       schema.type === FieldType.STRING ||
@@ -301,9 +309,9 @@ class InternalBuilder {
       schema.type === FieldType.BARCODEQR
     ) {
       if (opts?.forSelect) {
-        identifier = `to_char(${identifier}) as ${this.quotedIdentifier(col)}`
+        identifier = this.knex.raw("to_char(??) as ??", [identifier, col])
       } else {
-        identifier = `to_char(${identifier})`
+        identifier = this.knex.raw("to_char(??)", [identifier])
       }
     }
     return identifier
@@ -485,9 +493,7 @@ class InternalBuilder {
             .where(
               `${throughAlias}.${manyToMany.from}`,
               "=",
-              mainKnex.raw(
-                this.quotedIdentifier(`${fromAlias}.${manyToMany.fromPrimary}`)
-              )
+              mainKnex.raw(`??`, [`${fromAlias}.${manyToMany.fromPrimary}`])
             )
           // in SQS the same junction table is used for different many-to-many relationships between the
           // two same tables, this is needed to avoid rows ending up in all columns
@@ -516,7 +522,7 @@ class InternalBuilder {
           subQuery = subQuery.where(
             toKey,
             "=",
-            mainKnex.raw(this.quotedIdentifier(foreignKey))
+            mainKnex.raw("??", [foreignKey])
           )
 
           query = query.where(q => {
@@ -736,39 +742,29 @@ class InternalBuilder {
         ArrayOperator.ONE_OF,
         (q, key: string, array) => {
           if (this.client === SqlClient.ORACLE) {
+            // @ts-ignore
             key = this.convertClobs(key)
-            array = Array.isArray(array) ? array : [array]
-            const binding = new Array(array.length).fill("?").join(",")
-            return q.whereRaw(`${key} IN (${binding})`, array)
-          } else {
-            return q[fnc](key, Array.isArray(array) ? array : [array])
           }
+          return q[fnc](key, Array.isArray(array) ? array : [array])
         },
         (q, key: string[], array) => {
           if (this.client === SqlClient.ORACLE) {
-            const keyStr = `(${key.map(k => this.convertClobs(k)).join(",")})`
-            const binding = `(${array
-              .map((a: any) => `(${new Array(a.length).fill("?").join(",")})`)
-              .join(",")})`
-            return q.whereRaw(`${keyStr} IN ${binding}`, array.flat())
-          } else {
-            return q[fnc](key, Array.isArray(array) ? array : [array])
+            // @ts-ignore
+            key = key.map(k => this.convertClobs(k))
           }
+          return q[fnc](key, Array.isArray(array) ? array : [array])
         }
       )
     }
     if (filters.string) {
       iterate(filters.string, BasicOperator.STRING, (q, key, value) => {
-        const fnc = allOr ? "orWhere" : "where"
         // postgres supports ilike, nothing else does
         if (this.client === SqlClient.POSTGRES) {
+          const fnc = allOr ? "orWhere" : "where"
           return q[fnc](key, "ilike", `${value}%`)
         } else {
-          const rawFnc = `${fnc}Raw`
-          // @ts-ignore
-          return q[rawFnc](`LOWER(${this.quotedIdentifier(key)}) LIKE ?`, [
-            `${value.toLowerCase()}%`,
-          ])
+          const fnc = allOr ? "orWhereRaw" : "whereRaw"
+          return q[fnc](`LOWER(??) LIKE ?`, [key, `${value.toLowerCase()}%`])
         }
       })
     }
@@ -795,48 +791,33 @@ class InternalBuilder {
 
         const schema = this.getFieldSchema(key)
 
+        let rawKey: string | Knex.Raw = key
+        let high = value.high
+        let low = value.low
+
         if (this.client === SqlClient.ORACLE) {
-          // @ts-ignore
-          key = this.knex.raw(this.convertClobs(key))
+          rawKey = this.convertClobs(key)
+        } else if (
+          this.client === SqlClient.SQL_LITE &&
+          schema?.type === FieldType.BIGINT
+        ) {
+          rawKey = this.knex.raw("CAST(?? AS INTEGER)", [key])
+          high = this.knex.raw("CAST(? AS INTEGER)", [value.high])
+          low = this.knex.raw("CAST(? AS INTEGER)", [value.low])
         }
 
         if (lowValid && highValid) {
-          if (
-            schema?.type === FieldType.BIGINT &&
-            this.client === SqlClient.SQL_LITE
-          ) {
-            return q.whereRaw(
-              `CAST(${key} AS INTEGER) BETWEEN CAST(? AS INTEGER) AND CAST(? AS INTEGER)`,
-              [value.low, value.high]
-            )
-          } else {
-            const fnc = allOr ? "orWhereBetween" : "whereBetween"
-            return q[fnc](key, [value.low, value.high])
-          }
+          const fnc = allOr ? "orWhereBetween" : "whereBetween"
+          // @ts-ignore
+          return q[fnc](rawKey, [low, high])
         } else if (lowValid) {
-          if (
-            schema?.type === FieldType.BIGINT &&
-            this.client === SqlClient.SQL_LITE
-          ) {
-            return q.whereRaw(`CAST(${key} AS INTEGER) >= CAST(? AS INTEGER)`, [
-              value.low,
-            ])
-          } else {
-            const fnc = allOr ? "orWhere" : "where"
-            return q[fnc](key, ">=", value.low)
-          }
+          const fnc = allOr ? "orWhere" : "where"
+          // @ts-ignore
+          return q[fnc](rawKey, ">=", low)
         } else if (highValid) {
-          if (
-            schema?.type === FieldType.BIGINT &&
-            this.client === SqlClient.SQL_LITE
-          ) {
-            return q.whereRaw(`CAST(${key} AS INTEGER) <= CAST(? AS INTEGER)`, [
-              value.high,
-            ])
-          } else {
-            const fnc = allOr ? "orWhere" : "where"
-            return q[fnc](key, "<=", value.high)
-          }
+          const fnc = allOr ? "orWhere" : "where"
+          // @ts-ignore
+          return q[fnc](rawKey, "<=", high)
         }
         return q
       })
@@ -976,9 +957,7 @@ class InternalBuilder {
         const selectFields = qualifiedFields.map(field =>
           this.convertClobs(field, { forSelect: true })
         )
-        query = query
-          .groupByRaw(groupByFields.join(", "))
-          .select(this.knex.raw(selectFields.join(", ")))
+        query = query.groupBy(groupByFields).select(selectFields)
       } else {
         query = query.groupBy(qualifiedFields).select(qualifiedFields)
       }
@@ -990,11 +969,10 @@ class InternalBuilder {
           if (this.client === SqlClient.ORACLE) {
             const field = this.convertClobs(`${tableName}.${aggregation.field}`)
             query = query.select(
-              this.knex.raw(
-                `COUNT(DISTINCT ${field}) as ${this.quotedIdentifier(
-                  aggregation.name
-                )}`
-              )
+              this.knex.raw(`COUNT(DISTINCT ??) as ??`, [
+                field,
+                aggregation.name,
+              ])
             )
           } else {
             query = query.countDistinct(
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 1c36a6577c..718ad47a31 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -48,14 +48,14 @@ import { generateRowIdField } from "../../../integrations/utils"
 import { cloneDeep } from "lodash/fp"
 
 describe.each([
-  // ["in-memory", undefined],
-  // ["lucene", undefined],
-  // ["sqs", undefined],
+  ["in-memory", undefined],
+  ["lucene", undefined],
+  ["sqs", undefined],
   [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
-  // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
+  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("search (%s)", (name, dsProvider) => {
   const isSqs = name === "sqs"
   const isLucene = name === "lucene"
@@ -155,24 +155,24 @@ describe.each([
 
   describe.each([
     ["table", createTable],
-    [
-      "view",
-      async (schema: TableSchema) => {
-        const tableId = await createTable(schema)
-        const viewId = await createView(
-          tableId,
-          Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
-            const field = schema[fieldName]
-            viewSchema[fieldName] = {
-              visible: field.visible ?? true,
-              readonly: false,
-            }
-            return viewSchema
-          }, {})
-        )
-        return viewId
-      },
-    ],
+    // [
+    //   "view",
+    //   async (schema: TableSchema) => {
+    //     const tableId = await createTable(schema)
+    //     const viewId = await createView(
+    //       tableId,
+    //       Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
+    //         const field = schema[fieldName]
+    //         viewSchema[fieldName] = {
+    //           visible: field.visible ?? true,
+    //           readonly: false,
+    //         }
+    //         return viewSchema
+    //       }, {})
+    //     )
+    //     return viewId
+    //   },
+    // ],
   ])("from %s", (sourceType, createTableOrView) => {
     const isView = sourceType === "view"
 
@@ -792,7 +792,7 @@ describe.each([
         })
       })
 
-    describe.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => {
+    describe.only.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           name: { name: "name", type: FieldType.STRING },
diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index dfd4f50bd1..0c18307909 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -35,12 +35,12 @@ import { quotas } from "@budibase/pro"
 import { db, roles, features } from "@budibase/backend-core"
 
 describe.each([
-  ["lucene", undefined],
-  ["sqs", undefined],
-  [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  // ["lucene", undefined],
+  // ["sqs", undefined],
+  // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
+  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
   [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("/v2/views (%s)", (name, dsProvider) => {
   const config = setup.getConfig()

From 4545493cd514a8c53a0f5f21f9a512454225f1ad Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Tue, 22 Oct 2024 11:48:38 +0100
Subject: [PATCH 03/34] Checkpoint, more raws converted.

---
 packages/backend-core/src/sql/sql.ts          | 69 ++++++++++---------
 .../src/api/routes/tests/search.spec.ts       |  7 +-
 2 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index ec22ac37f9..a7a1de2673 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -258,7 +258,9 @@ class InternalBuilder {
         const columnSchema = schema[column]
 
         if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) {
-          return this.knex.raw("??::money::numeric as ??", [
+          // TODO: figure out how to express this safely without string
+          // interpolation.
+          return this.knex.raw(`??::money::numeric as "${field}"`, [
             [table, column].join("."),
             field,
           ])
@@ -632,18 +634,16 @@ class InternalBuilder {
     }
 
     const like = (q: Knex.QueryBuilder, key: string, value: any) => {
-      const fuzzyOr = filters?.fuzzyOr
-      const fnc = fuzzyOr || allOr ? "orWhere" : "where"
-      // postgres supports ilike, nothing else does
-      if (this.client === SqlClient.POSTGRES) {
-        return q[fnc](key, "ilike", `%${value}%`)
-      } else {
-        const rawFnc = `${fnc}Raw`
-        // @ts-ignore
-        return q[rawFnc](`LOWER(${this.quotedIdentifier(key)}) LIKE ?`, [
-          `%${value.toLowerCase()}%`,
-        ])
+      if (filters?.fuzzyOr || allOr) {
+        q = q.or
       }
+      if (
+        this.client === SqlClient.ORACLE ||
+        this.client === SqlClient.SQL_LITE
+      ) {
+        return q.whereRaw(`LOWER(??) LIKE ?`, [key, `%${value.toLowerCase()}%`])
+      }
+      return q.whereILike(key, this.knex.raw("?", [`%${value}%`]))
     }
 
     const contains = (mode: AnySearchFilter, any: boolean = false) => {
@@ -1069,17 +1069,20 @@ class InternalBuilder {
 
   private buildJsonField(field: string): string {
     const parts = field.split(".")
-    let tableField: string, unaliased: string
+    let unaliased: string
+
+    let tableField: string
     if (parts.length > 1) {
       const alias = parts.shift()!
       unaliased = parts.join(".")
-      tableField = `${this.quote(alias)}.${this.quote(unaliased)}`
+      tableField = `${alias}.${unaliased}`
     } else {
       unaliased = parts.join(".")
-      tableField = this.quote(unaliased)
+      tableField = unaliased
     }
+
     const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
-    return `'${unaliased}'${separator}${tableField}`
+    return this.knex.raw(`?${separator}??`, [unaliased, tableField]).toString()
   }
 
   maxFunctionParameters() {
@@ -1175,13 +1178,13 @@ class InternalBuilder {
       subQuery = subQuery.where(
         correlatedTo,
         "=",
-        knex.raw(this.quotedIdentifier(correlatedFrom))
+        knex.raw("??", [correlatedFrom])
       )
 
-      const standardWrap = (select: string): Knex.QueryBuilder => {
+      const standardWrap = (select: Knex.Raw): Knex.QueryBuilder => {
         subQuery = subQuery.select(`${toAlias}.*`).limit(getRelationshipLimit())
         // @ts-ignore - the from alias syntax isn't in Knex typing
-        return knex.select(knex.raw(select)).from({
+        return knex.select(select).from({
           [toAlias]: subQuery,
         })
       }
@@ -1191,12 +1194,12 @@ class InternalBuilder {
           // need to check the junction table document is to the right column, this is just for SQS
           subQuery = this.addJoinFieldCheck(subQuery, relationship)
           wrapperQuery = standardWrap(
-            `json_group_array(json_object(${fieldList}))`
+            this.knex.raw(`json_group_array(json_object(${fieldList}))`)
           )
           break
         case SqlClient.POSTGRES:
           wrapperQuery = standardWrap(
-            `json_agg(json_build_object(${fieldList}))`
+            this.knex.raw(`json_agg(json_build_object(${fieldList}))`)
           )
           break
         case SqlClient.MARIADB:
@@ -1210,21 +1213,25 @@ class InternalBuilder {
         case SqlClient.MY_SQL:
         case SqlClient.ORACLE:
           wrapperQuery = standardWrap(
-            `json_arrayagg(json_object(${fieldList}))`
+            this.knex.raw(`json_arrayagg(json_object(${fieldList}))`)
           )
           break
-        case SqlClient.MS_SQL:
+        case SqlClient.MS_SQL: {
+          const comparatorQuery = knex
+            .select(`${fromAlias}.*`)
+            // @ts-ignore - from alias syntax not TS supported
+            .from({
+              [fromAlias]: subQuery
+                .select(`${toAlias}.*`)
+                .limit(getRelationshipLimit()),
+            })
+
           wrapperQuery = knex.raw(
-            `(SELECT ${this.quote(toAlias)} = (${knex
-              .select(`${fromAlias}.*`)
-              // @ts-ignore - from alias syntax not TS supported
-              .from({
-                [fromAlias]: subQuery
-                  .select(`${toAlias}.*`)
-                  .limit(getRelationshipLimit()),
-              })} FOR JSON PATH))`
+            `(SELECT ?? = (${comparatorQuery} FOR JSON PATH))`,
+            [toAlias]
           )
           break
+        }
         default:
           throw new Error(`JSON relationships not implement for ${sqlClient}`)
       }
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 718ad47a31..74bf0eaf53 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -792,10 +792,11 @@ describe.each([
         })
       })
 
-    describe.only.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => {
+    const stringTypes = [FieldType.STRING, FieldType.LONGFORM] as const
+    describe.only.each(stringTypes)("%s", type => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
-          name: { name: "name", type: FieldType.STRING },
+          name: { name: "name", type },
         })
         await createRows([{ name: "foo" }, { name: "bar" }])
       })
@@ -1602,7 +1603,7 @@ describe.each([
         })
       })
 
-    describe.each([FieldType.ARRAY, FieldType.OPTIONS])("%s", () => {
+    describe("arrays", () => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           numbers: {

From 56a68db1d4e9ed2046c25b0708afc37e1f3522db Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Tue, 22 Oct 2024 18:33:44 +0100
Subject: [PATCH 04/34] Checkpoint EOD: fixed a bunch more raw cases, some test
 failures to fix tomorrow.

---
 packages/backend-core/src/sql/sql.ts          | 184 +++++++++++-------
 .../src/api/routes/tests/search.spec.ts       |   4 +-
 packages/types/src/sdk/search.ts              |   2 +-
 3 files changed, 114 insertions(+), 76 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index a7a1de2673..20ef5fab59 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -13,6 +13,7 @@ import SqlTableQueryBuilder from "./sqlTable"
 import {
   Aggregation,
   AnySearchFilter,
+  ArrayFilter,
   ArrayOperator,
   BasicOperator,
   BBReferenceFieldMetadata,
@@ -152,30 +153,24 @@ class InternalBuilder {
     return this.query.meta.table
   }
 
+  get knexClient(): Knex.Client {
+    return this.knex.client as Knex.Client
+  }
+
   getFieldSchema(key: string): FieldSchema | undefined {
     const { column } = this.splitter.run(key)
     return this.table.schema[column]
   }
 
   private quoteChars(): [string, string] {
-    switch (this.client) {
-      case SqlClient.ORACLE:
-      case SqlClient.POSTGRES:
-        return ['"', '"']
-      case SqlClient.MS_SQL:
-        return ["[", "]"]
-      case SqlClient.MARIADB:
-      case SqlClient.MY_SQL:
-      case SqlClient.SQL_LITE:
-        return ["`", "`"]
-    }
+    const wrapped = this.knexClient.wrapIdentifier("foo", {})
+    return [wrapped[0], wrapped[wrapped.length - 1]]
   }
 
-  // Takes a string like foo and returns a quoted string like [foo] for SQL Server
-  // and "foo" for Postgres.
+  // Takes a string like foo and returns a quoted string like [foo] for SQL
+  // Server and "foo" for Postgres.
   private quote(str: string): string {
-    const [start, end] = this.quoteChars()
-    return `${start}${str}${end}`
+    return this.knexClient.wrapIdentifier(str, {})
   }
 
   private isQuoted(key: string): boolean {
@@ -193,6 +188,21 @@ class InternalBuilder {
     return key.map(part => this.quote(part)).join(".")
   }
 
+  // Unfortuantely we cannot rely on knex's identifier escaping because it trims
+  // the identifier string before escaping it, which breaks cases for us where
+  // columns that start or end with a space aren't referenced correctly anymore.
+  //
+  // So whenever you're using an identifier binding in knex, e.g. knex.raw("??
+  // as ?", ["foo", "bar"]), you need to make sure you call this:
+  //
+  //   knex.raw("?? as ?", [this.quotedIdentifier("foo"), "bar"])
+  //
+  // Issue we filed against knex about this:
+  //   https://github.com/knex/knex/issues/6143
+  private rawQuotedIdentifier(key: string): Knex.Raw {
+    return this.knex.raw(this.quotedIdentifier(key))
+  }
+
   // Turns an identifier like a.b.c or `a`.`b`.`c` into ["a", "b", "c"]
   private splitIdentifier(key: string): string[] {
     const [start, end] = this.quoteChars()
@@ -261,7 +271,7 @@ class InternalBuilder {
           // TODO: figure out how to express this safely without string
           // interpolation.
           return this.knex.raw(`??::money::numeric as "${field}"`, [
-            [table, column].join("."),
+            this.rawQuotedIdentifier([table, column].join(".")),
             field,
           ])
         }
@@ -273,14 +283,14 @@ class InternalBuilder {
           // TODO: figure out how to express this safely without string
           // interpolation.
           return this.knex.raw(`CONVERT(varchar, ??, 108) as "${field}"`, [
-            field,
+            this.rawQuotedIdentifier(field),
           ])
         }
 
         if (table) {
-          return this.knex.raw("??", [`${table}.${column}`])
+          return this.rawQuotedIdentifier(`${table}.${column}`)
         } else {
-          return this.knex.raw("??", [field])
+          return this.rawQuotedIdentifier(field)
         }
       })
   }
@@ -300,7 +310,7 @@ class InternalBuilder {
     const parts = this.splitIdentifier(field)
     const col = parts.pop()!
     const schema = this.table.schema[col]
-    let identifier = this.knex.raw("??", [field])
+    let identifier = this.rawQuotedIdentifier(field)
 
     if (
       schema.type === FieldType.STRING ||
@@ -311,7 +321,10 @@ class InternalBuilder {
       schema.type === FieldType.BARCODEQR
     ) {
       if (opts?.forSelect) {
-        identifier = this.knex.raw("to_char(??) as ??", [identifier, col])
+        identifier = this.knex.raw("to_char(??) as ??", [
+          identifier,
+          this.rawQuotedIdentifier(col),
+        ])
       } else {
         identifier = this.knex.raw("to_char(??)", [identifier])
       }
@@ -437,7 +450,6 @@ class InternalBuilder {
     filterKey: string,
     whereCb: (filterKey: string, query: Knex.QueryBuilder) => Knex.QueryBuilder
   ): Knex.QueryBuilder {
-    const mainKnex = this.knex
     const { relationships, endpoint, tableAliases: aliases } = this.query
     const tableName = endpoint.entityId
     const fromAlias = aliases?.[tableName] || tableName
@@ -459,8 +471,8 @@ class InternalBuilder {
         relationship.to &&
         relationship.tableName
       ) {
-        const joinTable = mainKnex
-          .select(mainKnex.raw(1))
+        const joinTable = this.knex
+          .select(this.knex.raw(1))
           .from({ [toAlias]: relatedTableName })
         let subQuery = joinTable.clone()
         const manyToMany = validateManyToMany(relationship)
@@ -495,7 +507,7 @@ class InternalBuilder {
             .where(
               `${throughAlias}.${manyToMany.from}`,
               "=",
-              mainKnex.raw(`??`, [`${fromAlias}.${manyToMany.fromPrimary}`])
+              this.rawQuotedIdentifier(`${fromAlias}.${manyToMany.fromPrimary}`)
             )
           // in SQS the same junction table is used for different many-to-many relationships between the
           // two same tables, this is needed to avoid rows ending up in all columns
@@ -524,7 +536,7 @@ class InternalBuilder {
           subQuery = subQuery.where(
             toKey,
             "=",
-            mainKnex.raw("??", [foreignKey])
+            this.rawQuotedIdentifier(foreignKey)
           )
 
           query = query.where(q => {
@@ -641,15 +653,30 @@ class InternalBuilder {
         this.client === SqlClient.ORACLE ||
         this.client === SqlClient.SQL_LITE
       ) {
-        return q.whereRaw(`LOWER(??) LIKE ?`, [key, `%${value.toLowerCase()}%`])
+        return q.whereRaw(`LOWER(??) LIKE ?`, [
+          this.rawQuotedIdentifier(key),
+          `%${value.toLowerCase()}%`,
+        ])
       }
-      return q.whereILike(key, this.knex.raw("?", [`%${value}%`]))
+      return q.whereILike(
+        // @ts-expect-error knex types are wrong, raw is fine here
+        this.rawQuotedIdentifier(key),
+        this.knex.raw("?", [`%${value}%`])
+      )
     }
 
-    const contains = (mode: AnySearchFilter, any: boolean = false) => {
-      const rawFnc = allOr ? "orWhereRaw" : "whereRaw"
-      const not = mode === filters?.notContains ? "NOT " : ""
-      function stringifyArray(value: Array<any>, quoteStyle = '"'): string {
+    const contains = (mode: ArrayFilter, any = false) => {
+      function addModifiers<T extends {}, Q>(q: Knex.QueryBuilder<T, Q>) {
+        if (allOr || mode === filters?.containsAny) {
+          q = q.or
+        }
+        if (mode === filters?.notContains) {
+          q = q.not
+        }
+        return q
+      }
+
+      function stringifyArray(value: any[], quoteStyle = '"'): string {
         for (let i in value) {
           if (typeof value[i] === "string") {
             value[i] = `${quoteStyle}${value[i]}${quoteStyle}`
@@ -657,60 +684,64 @@ class InternalBuilder {
         }
         return `[${value.join(",")}]`
       }
+
       if (this.client === SqlClient.POSTGRES) {
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
           const wrap = any ? "" : "'"
           const op = any ? "\\?| array" : "@>"
-          const fieldNames = key.split(/\./g)
-          const table = fieldNames[0]
-          const col = fieldNames[1]
-          return q[rawFnc](
-            `${not}COALESCE("${table}"."${col}"::jsonb ${op} ${wrap}${stringifyArray(
-              value,
-              any ? "'" : '"'
-            )}${wrap}, FALSE)`
+
+          const stringifiedArray = stringifyArray(value, any ? "'" : '"')
+          return addModifiers(q).whereRaw(
+            `COALESCE(??::jsonb ${op} ${wrap}${stringifiedArray}${wrap}, FALSE)`,
+            [this.rawQuotedIdentifier(key)]
           )
         })
       } else if (
         this.client === SqlClient.MY_SQL ||
         this.client === SqlClient.MARIADB
       ) {
-        const jsonFnc = any ? "JSON_OVERLAPS" : "JSON_CONTAINS"
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
-          return q[rawFnc](
-            `${not}COALESCE(${jsonFnc}(${key}, '${stringifyArray(
-              value
-            )}'), FALSE)`
-          )
+          return addModifiers(q).whereRaw(`COALESCE(?(??, ?), FALSE)`, [
+            this.knex.raw(any ? "JSON_OVERLAPS" : "JSON_CONTAINS"),
+            this.rawQuotedIdentifier(key),
+            stringifyArray(value),
+          ])
         })
       } else {
-        const andOr = mode === filters?.containsAny ? " OR " : " AND "
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
-          let statement = ""
-          const identifier = this.quotedIdentifier(key)
-          for (let i in value) {
-            if (typeof value[i] === "string") {
-              value[i] = `%"${value[i].toLowerCase()}"%`
-            } else {
-              value[i] = `%${value[i]}%`
-            }
-            statement += `${
-              statement ? andOr : ""
-            }COALESCE(LOWER(${identifier}), '') LIKE ?`
-          }
-
-          if (statement === "") {
+          if (value.length === 0) {
             return q
           }
 
-          if (not) {
-            return q[rawFnc](
-              `(NOT (${statement}) OR ${identifier} IS NULL)`,
-              value
-            )
-          } else {
-            return q[rawFnc](statement, value)
+          q = addModifiers(q).where(subQuery => {
+            subQuery.where(subSubQuery => {
+              for (const elem of value) {
+                if (mode === filters?.containsAny) {
+                  subSubQuery = subSubQuery.or
+                } else {
+                  subSubQuery = subSubQuery.and
+                }
+
+                const lower =
+                  typeof elem === "string" ? `"${elem.toLowerCase()}"` : elem
+
+                subSubQuery = subSubQuery.whereLike(
+                  // @ts-expect-error knex types are wrong, raw is fine here
+                  this.knex.raw(`COALESCE(LOWER(??), '')`, [
+                    this.rawQuotedIdentifier(key),
+                  ]),
+                  `%${lower}%`
+                )
+              }
+            })
+          })
+
+          if (mode === filters?.notContains) {
+            // @ts-expect-error knex types are wrong, raw is fine here
+            q.or.whereNull(this.rawQuotedIdentifier(key))
           }
+
+          return q
         })
       }
     }
@@ -764,7 +795,10 @@ class InternalBuilder {
           return q[fnc](key, "ilike", `${value}%`)
         } else {
           const fnc = allOr ? "orWhereRaw" : "whereRaw"
-          return q[fnc](`LOWER(??) LIKE ?`, [key, `${value.toLowerCase()}%`])
+          return q[fnc](`LOWER(??) LIKE ?`, [
+            this.rawQuotedIdentifier(key),
+            `${value.toLowerCase()}%`,
+          ])
         }
       })
     }
@@ -801,7 +835,9 @@ class InternalBuilder {
           this.client === SqlClient.SQL_LITE &&
           schema?.type === FieldType.BIGINT
         ) {
-          rawKey = this.knex.raw("CAST(?? AS INTEGER)", [key])
+          rawKey = this.knex.raw("CAST(?? AS INTEGER)", [
+            this.rawQuotedIdentifier(key),
+          ])
           high = this.knex.raw("CAST(? AS INTEGER)", [value.high])
           low = this.knex.raw("CAST(? AS INTEGER)", [value.low])
         }
@@ -1082,7 +1118,9 @@ class InternalBuilder {
     }
 
     const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
-    return this.knex.raw(`?${separator}??`, [unaliased, tableField]).toString()
+    return this.knex
+      .raw(`?${separator}??`, [unaliased, this.rawQuotedIdentifier(tableField)])
+      .toString()
   }
 
   maxFunctionParameters() {
@@ -1178,7 +1216,7 @@ class InternalBuilder {
       subQuery = subQuery.where(
         correlatedTo,
         "=",
-        knex.raw("??", [correlatedFrom])
+        this.rawQuotedIdentifier(correlatedFrom)
       )
 
       const standardWrap = (select: Knex.Raw): Knex.QueryBuilder => {
@@ -1228,7 +1266,7 @@ class InternalBuilder {
 
           wrapperQuery = knex.raw(
             `(SELECT ?? = (${comparatorQuery} FOR JSON PATH))`,
-            [toAlias]
+            [this.rawQuotedIdentifier(toAlias)]
           )
           break
         }
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 74bf0eaf53..146791a268 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -691,7 +691,7 @@ describe.each([
           ])
         })
 
-        it("should not match the session user id in a multi user field", async () => {
+        it.only("should not match the session user id in a multi user field", async () => {
           await expectQuery({
             notContains: { multi_user: ["{{ [user]._id }}"] },
             notEmpty: { multi_user: true },
@@ -793,7 +793,7 @@ describe.each([
       })
 
     const stringTypes = [FieldType.STRING, FieldType.LONGFORM] as const
-    describe.only.each(stringTypes)("%s", type => {
+    describe.each(stringTypes)("%s", type => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           name: { name: "name", type },
diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts
index d41bb0fb99..d700b1b41d 100644
--- a/packages/types/src/sdk/search.ts
+++ b/packages/types/src/sdk/search.ts
@@ -49,7 +49,7 @@ type BasicFilter<T = any> = Record<string, T> & {
   [InternalSearchFilterOperator.COMPLEX_ID_OPERATOR]?: never
 }
 
-type ArrayFilter = Record<string, any[]> & {
+export type ArrayFilter = Record<string, any[]> & {
   [InternalSearchFilterOperator.COMPLEX_ID_OPERATOR]?: {
     id: string[]
     values: string[]

From 309506adab54ca89cb0612650afbddedcf8761b1 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Wed, 23 Oct 2024 15:05:41 +0100
Subject: [PATCH 05/34] wip

---
 .../server/src/api/routes/tests/search.spec.ts     | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 146791a268..9aa482fb6c 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -48,13 +48,13 @@ import { generateRowIdField } from "../../../integrations/utils"
 import { cloneDeep } from "lodash/fp"
 
 describe.each([
-  ["in-memory", undefined],
-  ["lucene", undefined],
-  ["sqs", undefined],
-  [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  // ["in-memory", undefined],
+  // ["lucene", undefined],
+  // ["sqs", undefined],
+  // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
+  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
   [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("search (%s)", (name, dsProvider) => {
   const isSqs = name === "sqs"

From aaf4022f25b4998bf69066d9e7f8749fa6f2cbef Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Wed, 23 Oct 2024 16:22:07 +0100
Subject: [PATCH 06/34] Finally fix notContains tests.

---
 packages/backend-core/src/sql/sql.ts          | 24 +++++++++++++------
 .../src/api/routes/tests/search.spec.ts       | 16 ++++++-------
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 20ef5fab59..69e5391eb2 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -713,7 +713,11 @@ class InternalBuilder {
             return q
           }
 
-          q = addModifiers(q).where(subQuery => {
+          q = q.where(subQuery => {
+            if (mode === filters?.notContains) {
+              subQuery = subQuery.not
+            }
+
             subQuery.where(subSubQuery => {
               for (const elem of value) {
                 if (mode === filters?.containsAny) {
@@ -733,13 +737,19 @@ class InternalBuilder {
                   `%${lower}%`
                 )
               }
-            })
-          })
 
-          if (mode === filters?.notContains) {
-            // @ts-expect-error knex types are wrong, raw is fine here
-            q.or.whereNull(this.rawQuotedIdentifier(key))
-          }
+              return subSubQuery
+            })
+
+            if (mode === filters?.notContains) {
+              subQuery = subQuery.or.whereNull(
+                // @ts-expect-error knex types are wrong, raw is fine here
+                this.rawQuotedIdentifier(key)
+              )
+            }
+
+            return subQuery
+          })
 
           return q
         })
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 9aa482fb6c..b319248ff7 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -48,13 +48,13 @@ import { generateRowIdField } from "../../../integrations/utils"
 import { cloneDeep } from "lodash/fp"
 
 describe.each([
-  // ["in-memory", undefined],
-  // ["lucene", undefined],
-  // ["sqs", undefined],
-  // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  ["in-memory", undefined],
+  ["lucene", undefined],
+  ["sqs", undefined],
+  [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
+  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
   [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("search (%s)", (name, dsProvider) => {
   const isSqs = name === "sqs"
@@ -691,7 +691,7 @@ describe.each([
           ])
         })
 
-        it.only("should not match the session user id in a multi user field", async () => {
+        it("should not match the session user id in a multi user field", async () => {
           await expectQuery({
             notContains: { multi_user: ["{{ [user]._id }}"] },
             notEmpty: { multi_user: true },

From a120ce4e144d9ab859d8ef750f5a347b464c2ec0 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Wed, 23 Oct 2024 17:07:42 +0100
Subject: [PATCH 07/34] More refactoring.

---
 packages/backend-core/src/sql/sql.ts | 83 +++++++++++++++++-----------
 1 file changed, 51 insertions(+), 32 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 69e5391eb2..b331a4da61 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -162,6 +162,12 @@ class InternalBuilder {
     return this.table.schema[column]
   }
 
+  private supportsILike(): boolean {
+    return !(
+      this.client === SqlClient.ORACLE || this.client === SqlClient.SQL_LITE
+    )
+  }
+
   private quoteChars(): [string, string] {
     const wrapped = this.knexClient.wrapIdentifier("foo", {})
     return [wrapped[0], wrapped[wrapped.length - 1]]
@@ -566,7 +572,7 @@ class InternalBuilder {
     filters = this.parseFilters({ ...filters })
     const aliases = this.query.tableAliases
     // if all or specified in filters, then everything is an or
-    const allOr = filters.allOr
+    const shouldOr = filters.allOr
     const isSqlite = this.client === SqlClient.SQL_LITE
     const tableName = isSqlite ? this.table._id! : this.table.name
 
@@ -630,7 +636,7 @@ class InternalBuilder {
             value
           )
         } else if (shouldProcessRelationship) {
-          if (allOr) {
+          if (shouldOr) {
             query = query.or
           }
           query = builder.addRelationshipForFilter(
@@ -646,7 +652,7 @@ class InternalBuilder {
     }
 
     const like = (q: Knex.QueryBuilder, key: string, value: any) => {
-      if (filters?.fuzzyOr || allOr) {
+      if (filters?.fuzzyOr || shouldOr) {
         q = q.or
       }
       if (
@@ -667,7 +673,7 @@ class InternalBuilder {
 
     const contains = (mode: ArrayFilter, any = false) => {
       function addModifiers<T extends {}, Q>(q: Knex.QueryBuilder<T, Q>) {
-        if (allOr || mode === filters?.containsAny) {
+        if (shouldOr || mode === filters?.containsAny) {
           q = q.or
         }
         if (mode === filters?.notContains) {
@@ -777,38 +783,46 @@ class InternalBuilder {
     }
 
     if (filters.oneOf) {
-      const fnc = allOr ? "orWhereIn" : "whereIn"
       iterate(
         filters.oneOf,
         ArrayOperator.ONE_OF,
         (q, key: string, array) => {
+          if (shouldOr) {
+            q = q.or
+          }
           if (this.client === SqlClient.ORACLE) {
             // @ts-ignore
             key = this.convertClobs(key)
           }
-          return q[fnc](key, Array.isArray(array) ? array : [array])
+          return q.whereIn(key, Array.isArray(array) ? array : [array])
         },
         (q, key: string[], array) => {
+          if (shouldOr) {
+            q = q.or
+          }
           if (this.client === SqlClient.ORACLE) {
             // @ts-ignore
             key = key.map(k => this.convertClobs(k))
           }
-          return q[fnc](key, Array.isArray(array) ? array : [array])
+          return q.whereIn(key, Array.isArray(array) ? array : [array])
         }
       )
     }
     if (filters.string) {
       iterate(filters.string, BasicOperator.STRING, (q, key, value) => {
-        // postgres supports ilike, nothing else does
-        if (this.client === SqlClient.POSTGRES) {
-          const fnc = allOr ? "orWhere" : "where"
-          return q[fnc](key, "ilike", `${value}%`)
-        } else {
-          const fnc = allOr ? "orWhereRaw" : "whereRaw"
-          return q[fnc](`LOWER(??) LIKE ?`, [
+        if (shouldOr) {
+          q = q.or
+        }
+        if (
+          this.client === SqlClient.ORACLE ||
+          this.client === SqlClient.SQL_LITE
+        ) {
+          return q.whereRaw(`LOWER(??) LIKE ?`, [
             this.rawQuotedIdentifier(key),
             `${value.toLowerCase()}%`,
           ])
+        } else {
+          return q.whereILike(key, `${value}%`)
         }
       })
     }
@@ -852,37 +866,42 @@ class InternalBuilder {
           low = this.knex.raw("CAST(? AS INTEGER)", [value.low])
         }
 
+        if (shouldOr) {
+          q = q.or
+        }
+
         if (lowValid && highValid) {
-          const fnc = allOr ? "orWhereBetween" : "whereBetween"
           // @ts-ignore
-          return q[fnc](rawKey, [low, high])
+          return q.whereBetween(rawKey, [low, high])
         } else if (lowValid) {
-          const fnc = allOr ? "orWhere" : "where"
           // @ts-ignore
-          return q[fnc](rawKey, ">=", low)
+          return q.where(rawKey, ">=", low)
         } else if (highValid) {
-          const fnc = allOr ? "orWhere" : "where"
           // @ts-ignore
-          return q[fnc](rawKey, "<=", high)
+          return q.where(rawKey, "<=", high)
         }
         return q
       })
     }
     if (filters.equal) {
       iterate(filters.equal, BasicOperator.EQUAL, (q, key, value) => {
-        const fnc = allOr ? "orWhereRaw" : "whereRaw"
+        if (shouldOr) {
+          q = q.or
+        }
         if (this.client === SqlClient.MS_SQL) {
-          return q[fnc](
-            `CASE WHEN ${this.quotedIdentifier(key)} = ? THEN 1 ELSE 0 END = 1`,
-            [value]
-          )
-        } else if (this.client === SqlClient.ORACLE) {
-          const identifier = this.convertClobs(key)
-          return q[fnc](`(${identifier} IS NOT NULL AND ${identifier} = ?)`, [
+          return q.whereRaw(`CASE WHEN ?? = ? THEN 1 ELSE 0 END = 1`, [
+            this.quotedIdentifier(key),
             value,
           ])
+        } else if (this.client === SqlClient.ORACLE) {
+          const identifier = this.convertClobs(key)
+          return q.where(subq =>
+            // @ts-expect-error knex types are wrong, raw is fine here
+            subq.whereNotNull(identifier).andWhere(identifier, value)
+          )
         } else {
-          return q[fnc](`COALESCE(${this.quotedIdentifier(key)} = ?, FALSE)`, [
+          return q.whereRaw(`COALESCE(?? = ?, FALSE)`, [
+            this.rawQuotedIdentifier(key),
             value,
           ])
         }
@@ -890,7 +909,7 @@ class InternalBuilder {
     }
     if (filters.notEqual) {
       iterate(filters.notEqual, BasicOperator.NOT_EQUAL, (q, key, value) => {
-        const fnc = allOr ? "orWhereRaw" : "whereRaw"
+        const fnc = shouldOr ? "orWhereRaw" : "whereRaw"
         if (this.client === SqlClient.MS_SQL) {
           return q[fnc](
             `CASE WHEN ${this.quotedIdentifier(key)} = ? THEN 1 ELSE 0 END = 0`,
@@ -911,13 +930,13 @@ class InternalBuilder {
     }
     if (filters.empty) {
       iterate(filters.empty, BasicOperator.EMPTY, (q, key) => {
-        const fnc = allOr ? "orWhereNull" : "whereNull"
+        const fnc = shouldOr ? "orWhereNull" : "whereNull"
         return q[fnc](key)
       })
     }
     if (filters.notEmpty) {
       iterate(filters.notEmpty, BasicOperator.NOT_EMPTY, (q, key) => {
-        const fnc = allOr ? "orWhereNotNull" : "whereNotNull"
+        const fnc = shouldOr ? "orWhereNotNull" : "whereNotNull"
         return q[fnc](key)
       })
     }

From ebcbadfd3ae3fd53e5ddabfc73fff18cdd26dcb3 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Wed, 23 Oct 2024 17:21:14 +0100
Subject: [PATCH 08/34] remove all of the `fnc` variables

---
 packages/backend-core/src/sql/sql.ts          | 59 ++++++++++---------
 .../src/api/routes/tests/search.spec.ts       |  2 +-
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index b331a4da61..b579454f3a 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -719,12 +719,12 @@ class InternalBuilder {
             return q
           }
 
-          q = q.where(subQuery => {
+          return q.where(subQuery => {
             if (mode === filters?.notContains) {
               subQuery = subQuery.not
             }
 
-            subQuery.where(subSubQuery => {
+            return subQuery.where(subSubQuery => {
               for (const elem of value) {
                 if (mode === filters?.containsAny) {
                   subSubQuery = subSubQuery.or
@@ -743,21 +743,8 @@ class InternalBuilder {
                   `%${lower}%`
                 )
               }
-
-              return subSubQuery
             })
-
-            if (mode === filters?.notContains) {
-              subQuery = subQuery.or.whereNull(
-                // @ts-expect-error knex types are wrong, raw is fine here
-                this.rawQuotedIdentifier(key)
-              )
-            }
-
-            return subQuery
           })
-
-          return q
         })
       }
     }
@@ -890,7 +877,7 @@ class InternalBuilder {
         }
         if (this.client === SqlClient.MS_SQL) {
           return q.whereRaw(`CASE WHEN ?? = ? THEN 1 ELSE 0 END = 1`, [
-            this.quotedIdentifier(key),
+            this.rawQuotedIdentifier(key),
             value,
           ])
         } else if (this.client === SqlClient.ORACLE) {
@@ -909,20 +896,30 @@ class InternalBuilder {
     }
     if (filters.notEqual) {
       iterate(filters.notEqual, BasicOperator.NOT_EQUAL, (q, key, value) => {
-        const fnc = shouldOr ? "orWhereRaw" : "whereRaw"
+        if (shouldOr) {
+          q = q.or
+        }
         if (this.client === SqlClient.MS_SQL) {
-          return q[fnc](
-            `CASE WHEN ${this.quotedIdentifier(key)} = ? THEN 1 ELSE 0 END = 0`,
-            [value]
-          )
+          return q.whereRaw(`CASE WHEN ?? = ? THEN 1 ELSE 0 END = 0`, [
+            this.rawQuotedIdentifier(key),
+            value,
+          ])
         } else if (this.client === SqlClient.ORACLE) {
           const identifier = this.convertClobs(key)
-          return q[fnc](
-            `(${identifier} IS NOT NULL AND ${identifier} != ?) OR ${identifier} IS NULL`,
-            [value]
+          return (
+            q
+              .where(subq =>
+                subq.not
+                  // @ts-expect-error knex types are wrong, raw is fine here
+                  .whereNull(identifier)
+                  .and.where(identifier, "!=", value)
+              )
+              // @ts-expect-error knex types are wrong, raw is fine here
+              .or.whereNull(identifier)
           )
         } else {
-          return q[fnc](`COALESCE(${this.quotedIdentifier(key)} != ?, TRUE)`, [
+          return q.whereRaw(`COALESCE(?? != ?, TRUE)`, [
+            this.rawQuotedIdentifier(key),
             value,
           ])
         }
@@ -930,14 +927,18 @@ class InternalBuilder {
     }
     if (filters.empty) {
       iterate(filters.empty, BasicOperator.EMPTY, (q, key) => {
-        const fnc = shouldOr ? "orWhereNull" : "whereNull"
-        return q[fnc](key)
+        if (shouldOr) {
+          q = q.or
+        }
+        return q.whereNull(key)
       })
     }
     if (filters.notEmpty) {
       iterate(filters.notEmpty, BasicOperator.NOT_EMPTY, (q, key) => {
-        const fnc = shouldOr ? "orWhereNotNull" : "whereNotNull"
-        return q[fnc](key)
+        if (shouldOr) {
+          q = q.or
+        }
+        return q.whereNotNull(key)
       })
     }
     if (filters.contains) {
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index b319248ff7..74bf0eaf53 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -793,7 +793,7 @@ describe.each([
       })
 
     const stringTypes = [FieldType.STRING, FieldType.LONGFORM] as const
-    describe.each(stringTypes)("%s", type => {
+    describe.only.each(stringTypes)("%s", type => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           name: { name: "name", type },

From 9c9d2560b2f8ef6ebc95af1bd8c12a35561cadd4 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 11:17:31 +0200
Subject: [PATCH 09/34] Remove fetchEnriched

---
 .../server/src/api/routes/tests/rowAction.spec.ts     |  2 +-
 packages/server/src/tests/utilities/api/automation.ts | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts
index 5cec83fe23..2b6b0f01c9 100644
--- a/packages/server/src/api/routes/tests/rowAction.spec.ts
+++ b/packages/server/src/api/routes/tests/rowAction.spec.ts
@@ -958,7 +958,7 @@ describe("/rowsActions", () => {
       // document was not being cleaned up. This meant there existed code paths
       // that would find it and try to reference the tables within it, resulting
       // in errors.
-      await config.api.automation.fetchEnriched({
+      await config.api.automation.fetch({
         status: 200,
       })
     })
diff --git a/packages/server/src/tests/utilities/api/automation.ts b/packages/server/src/tests/utilities/api/automation.ts
index 11c041d52b..9d9a27e891 100644
--- a/packages/server/src/tests/utilities/api/automation.ts
+++ b/packages/server/src/tests/utilities/api/automation.ts
@@ -23,17 +23,6 @@ export class AutomationAPI extends TestAPI {
     })
   }
 
-  fetchEnriched = async (
-    expectations?: Expectations
-  ): Promise<FetchAutomationResponse> => {
-    return await this._get<FetchAutomationResponse>(
-      `/api/automations?enrich=true`,
-      {
-        expectations,
-      }
-    )
-  }
-
   post = async (
     body: Automation,
     expectations?: Expectations

From ccdb03244966022e25d7a50b5f786dced0e5660b Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 11:30:59 +0200
Subject: [PATCH 10/34] Move sdk rowaction code

---
 .../src/sdk/app/{rowActions.ts => rowActions/crud.ts}  | 10 +++++-----
 packages/server/src/sdk/app/rowActions/index.ts        |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)
 rename packages/server/src/sdk/app/{rowActions.ts => rowActions/crud.ts} (96%)
 create mode 100644 packages/server/src/sdk/app/rowActions/index.ts

diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions/crud.ts
similarity index 96%
rename from packages/server/src/sdk/app/rowActions.ts
rename to packages/server/src/sdk/app/rowActions/crud.ts
index de73969696..1153677685 100644
--- a/packages/server/src/sdk/app/rowActions.ts
+++ b/packages/server/src/sdk/app/rowActions/crud.ts
@@ -7,11 +7,11 @@ import {
   User,
   VirtualDocumentType,
 } from "@budibase/types"
-import { generateRowActionsID } from "../../db/utils"
-import automations from "./automations"
-import { definitions as TRIGGER_DEFINITIONS } from "../../automations/triggerInfo"
-import * as triggers from "../../automations/triggers"
-import sdk from ".."
+import { generateRowActionsID } from "../../../db/utils"
+import automations from "../automations"
+import { definitions as TRIGGER_DEFINITIONS } from "../../../automations/triggerInfo"
+import * as triggers from "../../../automations/triggers"
+import sdk from "../.."
 
 async function ensureUniqueAndThrow(
   doc: TableRowActions,
diff --git a/packages/server/src/sdk/app/rowActions/index.ts b/packages/server/src/sdk/app/rowActions/index.ts
new file mode 100644
index 0000000000..8baf08477f
--- /dev/null
+++ b/packages/server/src/sdk/app/rowActions/index.ts
@@ -0,0 +1 @@
+export * from "./crud"

From f2e46863f63befe90ea4f0d64c744b63786fa29f Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 11:53:59 +0200
Subject: [PATCH 11/34] Add test deleting external ds

---
 .../src/api/routes/tests/rowAction.spec.ts    | 59 ++++++++++++++++++-
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts
index 2b6b0f01c9..5246304906 100644
--- a/packages/server/src/api/routes/tests/rowAction.spec.ts
+++ b/packages/server/src/api/routes/tests/rowAction.spec.ts
@@ -1,17 +1,20 @@
 import _ from "lodash"
 import tk from "timekeeper"
 
+import { context, roles } from "@budibase/backend-core"
+import { automations } from "@budibase/pro"
 import {
   CreateRowActionRequest,
   DocumentType,
   PermissionLevel,
   RowActionResponse,
+  TableRowActions,
 } from "@budibase/types"
 import * as setup from "./utilities"
 import { generator, mocks } from "@budibase/backend-core/tests"
 import { Expectations } from "../../../tests/utilities/api/base"
-import { roles } from "@budibase/backend-core"
-import { automations } from "@budibase/pro"
+import { DatabaseName, getDatasource } from "../../../integrations/tests/utils"
+import { generateRowActionsID } from "../../../db/utils"
 
 const expectAutomationId = () =>
   expect.stringMatching(`^${DocumentType.AUTOMATION}_.+`)
@@ -962,5 +965,57 @@ describe("/rowsActions", () => {
         status: 200,
       })
     })
+
+    it.each([
+      [
+        "external",
+        async () => {
+          const ds = await config.createDatasource({
+            datasource: await getDatasource(DatabaseName.POSTGRES),
+          })
+          const table = await config.api.table.save(
+            setup.structures.tableForDatasource(ds)
+          )
+          return table
+        },
+      ],
+    ])(
+      "should delete all the row actions (and automations) for its tables when a datasource is deleted",
+      async (_, getTable) => {
+        async function getRowActionsFromDb(tableId: string) {
+          return await context.doInAppContext(config.getAppId(), async () => {
+            const db = context.getAppDB()
+            const tableDoc = await db.tryGet<TableRowActions>(
+              generateRowActionsID(tableId)
+            )
+            return tableDoc
+          })
+        }
+
+        const table = await getTable()
+        const tableId = table._id!
+
+        await config.api.rowAction.save(tableId, {
+          name: generator.guid(),
+        })
+        await config.api.rowAction.save(tableId, {
+          name: generator.guid(),
+        })
+
+        const { actions } = (await getRowActionsFromDb(tableId))!
+        expect(Object.entries(actions)).toHaveLength(2)
+
+        const { automations } = await config.api.automation.fetch()
+        expect(automations).toHaveLength(2)
+
+        const datasource = await config.api.datasource.get(table.sourceId)
+        await config.api.datasource.delete(datasource)
+
+        const automationsResp = await config.api.automation.fetch()
+        expect(automationsResp.automations).toHaveLength(0)
+
+        expect(await getRowActionsFromDb(tableId)).toBeUndefined()
+      }
+    )
   })
 })

From 0695888659f0637218c725e04dffdc0acadf29e7 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 11:01:35 +0100
Subject: [PATCH 12/34] wip

---
 packages/backend-core/src/sql/sql.ts          | 57 +++++++++++++------
 .../src/api/routes/tests/search.spec.ts       |  4 +-
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index b579454f3a..684f276145 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -99,6 +99,23 @@ function isSqs(table: Table): boolean {
   )
 }
 
+function escapeQuotes(value: string, quoteChar = '"'): string {
+  return value.replace(new RegExp(quoteChar, "g"), `${quoteChar}${quoteChar}`)
+}
+
+function wrap(value: string, quoteChar = '"'): string {
+  return `${quoteChar}${escapeQuotes(value, quoteChar)}${quoteChar}`
+}
+
+function stringifyArray(value: any[], quoteStyle = '"'): string {
+  for (let i in value) {
+    if (typeof value[i] === "string") {
+      value[i] = wrap(value[i], quoteStyle)
+    }
+  }
+  return `[${value.join(",")}]`
+}
+
 const allowEmptyRelationships: Record<SearchFilterKey, boolean> = {
   [BasicOperator.EQUAL]: false,
   [BasicOperator.NOT_EQUAL]: true,
@@ -194,6 +211,15 @@ class InternalBuilder {
     return key.map(part => this.quote(part)).join(".")
   }
 
+  private quotedValue(value: string): string {
+    const formatter = this.knexClient.formatter(this.knexClient.queryBuilder())
+    return formatter.wrap(value, false)
+  }
+
+  private rawQuotedValue(value: string): Knex.Raw {
+    return this.knex.raw(this.quotedValue(value))
+  }
+
   // Unfortuantely we cannot rely on knex's identifier escaping because it trims
   // the identifier string before escaping it, which breaks cases for us where
   // columns that start or end with a space aren't referenced correctly anymore.
@@ -682,25 +708,20 @@ class InternalBuilder {
         return q
       }
 
-      function stringifyArray(value: any[], quoteStyle = '"'): string {
-        for (let i in value) {
-          if (typeof value[i] === "string") {
-            value[i] = `${quoteStyle}${value[i]}${quoteStyle}`
-          }
-        }
-        return `[${value.join(",")}]`
-      }
-
       if (this.client === SqlClient.POSTGRES) {
         iterate(mode, ArrayOperator.CONTAINS, (q, key, value) => {
-          const wrap = any ? "" : "'"
-          const op = any ? "\\?| array" : "@>"
-
-          const stringifiedArray = stringifyArray(value, any ? "'" : '"')
-          return addModifiers(q).whereRaw(
-            `COALESCE(??::jsonb ${op} ${wrap}${stringifiedArray}${wrap}, FALSE)`,
-            [this.rawQuotedIdentifier(key)]
-          )
+          q = addModifiers(q)
+          if (any) {
+            return q.whereRaw(`COALESCE(??::jsonb \\?| array??, FALSE)`, [
+              this.rawQuotedIdentifier(key),
+              this.knex.raw(stringifyArray(value, "'")),
+            ])
+          } else {
+            return q.whereRaw(`COALESCE(??::jsonb @> '??', FALSE)`, [
+              this.rawQuotedIdentifier(key),
+              this.knex.raw(stringifyArray(value)),
+            ])
+          }
         })
       } else if (
         this.client === SqlClient.MY_SQL ||
@@ -710,7 +731,7 @@ class InternalBuilder {
           return addModifiers(q).whereRaw(`COALESCE(?(??, ?), FALSE)`, [
             this.knex.raw(any ? "JSON_OVERLAPS" : "JSON_CONTAINS"),
             this.rawQuotedIdentifier(key),
-            stringifyArray(value),
+            this.knex.raw(wrap(stringifyArray(value))),
           ])
         })
       } else {
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 74bf0eaf53..b22b1545be 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -793,7 +793,7 @@ describe.each([
       })
 
     const stringTypes = [FieldType.STRING, FieldType.LONGFORM] as const
-    describe.only.each(stringTypes)("%s", type => {
+    describe.each(stringTypes)("%s", type => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           name: { name: "name", type },
@@ -2163,7 +2163,7 @@ describe.each([
       })
     })
 
-    describe("multi user", () => {
+    describe.only("multi user", () => {
       let user1: User
       let user2: User
 

From 7b0419773cd9f07af246d1d1584dafa695f34c1b Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 12:08:17 +0200
Subject: [PATCH 13/34] Implement deletion

---
 packages/server/src/api/controllers/datasource.ts | 2 ++
 packages/server/src/sdk/app/rowActions/index.ts   | 1 +
 packages/server/src/sdk/app/rowActions/utils.ts   | 9 +++++++++
 3 files changed, 12 insertions(+)
 create mode 100644 packages/server/src/sdk/app/rowActions/utils.ts

diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts
index baab008da8..794e2dfddd 100644
--- a/packages/server/src/api/controllers/datasource.ts
+++ b/packages/server/src/api/controllers/datasource.ts
@@ -267,6 +267,8 @@ export async function destroy(ctx: UserCtx) {
   const datasource = await sdk.datasources.get(datasourceId)
   // Delete all queries for the datasource
 
+  await sdk.rowActions.deleteAllForDatasource(datasourceId)
+
   if (datasource.type === dbCore.BUDIBASE_DATASOURCE_TYPE) {
     await destroyInternalTablesBySourceId(datasourceId)
   } else {
diff --git a/packages/server/src/sdk/app/rowActions/index.ts b/packages/server/src/sdk/app/rowActions/index.ts
index 8baf08477f..b0dbd426df 100644
--- a/packages/server/src/sdk/app/rowActions/index.ts
+++ b/packages/server/src/sdk/app/rowActions/index.ts
@@ -1 +1,2 @@
 export * from "./crud"
+export * from "./utils"
diff --git a/packages/server/src/sdk/app/rowActions/utils.ts b/packages/server/src/sdk/app/rowActions/utils.ts
new file mode 100644
index 0000000000..d4e5a4610d
--- /dev/null
+++ b/packages/server/src/sdk/app/rowActions/utils.ts
@@ -0,0 +1,9 @@
+import sdk from "../../../sdk"
+
+export async function deleteAllForDatasource(datasourceId: string) {
+  const allTables = await sdk.tables.getAllTables()
+  const tables = allTables.filter(t => t.sourceId === datasourceId)
+  for (const table of Object.values(tables)) {
+    await sdk.rowActions.deleteAll(table._id!)
+  }
+}

From bbc9a2798de2f307f6aca87bf0fd4c35b7964939 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 12:08:44 +0200
Subject: [PATCH 14/34] Add test for internal

---
 .../src/api/routes/tests/rowAction.spec.ts      | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts
index 5246304906..513953cbfd 100644
--- a/packages/server/src/api/routes/tests/rowAction.spec.ts
+++ b/packages/server/src/api/routes/tests/rowAction.spec.ts
@@ -1,7 +1,11 @@
 import _ from "lodash"
 import tk from "timekeeper"
 
-import { context, roles } from "@budibase/backend-core"
+import {
+  context,
+  DEFAULT_BB_DATASOURCE_ID,
+  roles,
+} from "@budibase/backend-core"
 import { automations } from "@budibase/pro"
 import {
   CreateRowActionRequest,
@@ -967,6 +971,17 @@ describe("/rowsActions", () => {
     })
 
     it.each([
+      [
+        "internal",
+        async () => {
+          await config.api.application.addSampleData(config.getAppId())
+          const tables = await config.api.table.fetch()
+          const table = tables.find(
+            t => t.sourceId === DEFAULT_BB_DATASOURCE_ID
+          )!
+          return table
+        },
+      ],
       [
         "external",
         async () => {

From e14918c105185e751e05b72e88d1b1ccea14feb7 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 11:20:31 +0100
Subject: [PATCH 15/34] Fix notContains tests again.

---
 packages/backend-core/src/sql/sql.ts                | 12 ++++++++++--
 packages/server/src/api/routes/tests/search.spec.ts |  2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 684f276145..8922ce055b 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -740,12 +740,12 @@ class InternalBuilder {
             return q
           }
 
-          return q.where(subQuery => {
+          q = q.where(subQuery => {
             if (mode === filters?.notContains) {
               subQuery = subQuery.not
             }
 
-            return subQuery.where(subSubQuery => {
+            subQuery = subQuery.where(subSubQuery => {
               for (const elem of value) {
                 if (mode === filters?.containsAny) {
                   subSubQuery = subSubQuery.or
@@ -765,7 +765,15 @@ class InternalBuilder {
                 )
               }
             })
+            if (mode === filters?.notContains) {
+              subQuery = subQuery.or.whereNull(
+                // @ts-expect-error knex types are wrong, raw is fine here
+                this.rawQuotedIdentifier(key)
+              )
+            }
+            return subQuery
           })
+          return q
         })
       }
     }
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index b22b1545be..b319248ff7 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -2163,7 +2163,7 @@ describe.each([
       })
     })
 
-    describe.only("multi user", () => {
+    describe("multi user", () => {
       let user1: User
       let user2: User
 

From ae6f2f93820f44bca9e6cd797b9ffa65efa8d1da Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 12:22:51 +0200
Subject: [PATCH 16/34] Fix test

---
 packages/server/src/api/routes/tests/rowAction.spec.ts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts
index 513953cbfd..14a1812195 100644
--- a/packages/server/src/api/routes/tests/rowAction.spec.ts
+++ b/packages/server/src/api/routes/tests/rowAction.spec.ts
@@ -974,6 +974,7 @@ describe("/rowsActions", () => {
       [
         "internal",
         async () => {
+          await config.newTenant()
           await config.api.application.addSampleData(config.getAppId())
           const tables = await config.api.table.fetch()
           const table = tables.find(
@@ -985,6 +986,7 @@ describe("/rowsActions", () => {
       [
         "external",
         async () => {
+          await config.newTenant()
           const ds = await config.createDatasource({
             datasource: await getDatasource(DatabaseName.POSTGRES),
           })

From 8dda1a51cfed47c2c56fdd6a75582c22f609a21d Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 12:39:20 +0200
Subject: [PATCH 17/34] Remove row actions when unfetching tables

---
 packages/server/src/sdk/app/datasources/plus.ts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/packages/server/src/sdk/app/datasources/plus.ts b/packages/server/src/sdk/app/datasources/plus.ts
index 31ec51c728..a8a21e6906 100644
--- a/packages/server/src/sdk/app/datasources/plus.ts
+++ b/packages/server/src/sdk/app/datasources/plus.ts
@@ -9,6 +9,7 @@ import * as datasources from "./datasources"
 import tableSdk from "../tables"
 import { getIntegration } from "../../../integrations"
 import { context } from "@budibase/backend-core"
+import sdk from "../.."
 
 function checkForSchemaErrors(schema: Record<string, Table>) {
   const errors: Record<string, string> = {}
@@ -96,6 +97,15 @@ export async function buildSchemaFromSource(
   const datasource = await datasources.get(datasourceId)
 
   const { tables, errors } = await buildFilteredSchema(datasource, tablesFilter)
+
+  const oldTables = datasource.entities || {}
+  const tablesToRemove = Object.keys(oldTables).filter(
+    t => !Object.keys(tables).includes(t)
+  )
+  for (const table of tablesToRemove) {
+    await sdk.rowActions.deleteAll(oldTables[table]._id!)
+  }
+
   datasource.entities = tables
 
   datasources.setDefaultDisplayColumns(datasource)

From 0736812293361ff53936a82f58682c0298aae9e3 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 11:39:57 +0100
Subject: [PATCH 18/34] Add SQL injection tests.

---
 packages/backend-core/src/sql/sql.ts          | 14 +++---
 .../src/api/routes/tests/search.spec.ts       | 50 +++++++++++++++++++
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 8922ce055b..883f7a4d5f 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -887,13 +887,13 @@ class InternalBuilder {
         }
 
         if (lowValid && highValid) {
-          // @ts-ignore
+          // @ts-expect-error knex types are wrong, raw is fine here
           return q.whereBetween(rawKey, [low, high])
         } else if (lowValid) {
-          // @ts-ignore
+          // @ts-expect-error knex types are wrong, raw is fine here
           return q.where(rawKey, ">=", low)
         } else if (highValid) {
-          // @ts-ignore
+          // @ts-expect-error knex types are wrong, raw is fine here
           return q.where(rawKey, "<=", high)
         }
         return q
@@ -1132,9 +1132,11 @@ class InternalBuilder {
         } else {
           let composite = `${aliased}.${key}`
           if (this.client === SqlClient.ORACLE) {
-            query = query.orderByRaw(
-              `${this.convertClobs(composite)} ${direction} nulls ${nulls}`
-            )
+            query = query.orderByRaw(`?? ?? nulls ??`, [
+              this.convertClobs(composite),
+              direction,
+              nulls,
+            ])
           } else {
             query = query.orderBy(composite, direction, nulls)
           }
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index b319248ff7..dfee63fa4d 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -3471,5 +3471,55 @@ describe.each([
           ])
         })
       })
+
+    describe("SQL injection", () => {
+      const badStrings = [
+        "1; DROP TABLE test;",
+        "1; DELETE FROM test;",
+        "1; UPDATE test SET name = 'foo';",
+        "1; INSERT INTO test (name) VALUES ('foo');",
+        "' OR '1'='1' --",
+        "'; DROP TABLE users; --",
+        "' OR 1=1 --",
+        "' UNION SELECT null, null, null; --",
+        "' AND (SELECT COUNT(*) FROM users) > 0 --",
+        "\"; EXEC xp_cmdshell('dir'); --",
+        "\"' OR 'a'='a",
+        "OR 1=1;",
+        "'; SHUTDOWN --",
+      ]
+
+      describe.only.each(badStrings)("bad string: %s", badString => {
+        it("should not allow SQL injection as a field name", async () => {
+          const tableOrViewId = await createTableOrView({
+            [badString]: {
+              name: badString,
+              type: FieldType.STRING,
+            },
+          })
+
+          await config.api.row.search(
+            tableOrViewId,
+            { query: {} },
+            { status: 200 }
+          )
+        })
+
+        it("should not allow SQL injection as a field value", async () => {
+          const tableOrViewId = await createTableOrView({
+            foo: {
+              name: "foo",
+              type: FieldType.STRING,
+            },
+          })
+
+          await config.api.row.search(
+            tableOrViewId,
+            { query: { equal: { foo: badString } } },
+            { status: 200 }
+          )
+        })
+      })
+    })
   })
 })

From 47e16dd84467d3f9b72fad11d6e4563ce8d1e7b0 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 13:07:13 +0200
Subject: [PATCH 19/34] Typing emitter

---
 .../server/src/api/controllers/row/index.ts   | 19 +++++++++--------
 .../server/src/events/AutomationEmitter.ts    |  4 ++--
 packages/types/src/sdk/koa.ts                 | 21 ++++++++++++++++++-
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts
index 4d40476b7c..d3b1d3d5b2 100644
--- a/packages/server/src/api/controllers/row/index.ts
+++ b/packages/server/src/api/controllers/row/index.ts
@@ -7,6 +7,7 @@ import * as internal from "./internal"
 import * as external from "./external"
 import { isExternalTableID } from "../../../integrations/utils"
 import {
+  AutomationEventType,
   Ctx,
   DeleteRow,
   DeleteRowRequest,
@@ -64,15 +65,15 @@ export async function patch(
       ctx.throw(404, "Row not found")
     }
     ctx.status = 200
-    ctx.eventEmitter &&
-      ctx.eventEmitter.emitRow({
-        eventName: `row:update`,
-        appId,
-        row,
-        table,
-        oldRow,
-        user: sdk.users.getUserContextBindings(ctx.user),
-      })
+
+    ctx.eventEmitter?.emitRow({
+      eventName: AutomationEventType.ROW_UPDATE,
+      appId,
+      row,
+      table,
+      oldRow,
+      user: sdk.users.getUserContextBindings(ctx.user),
+    })
     ctx.message = `${table.name} updated successfully.`
     ctx.body = row
     gridSocket?.emitRowUpdate(ctx, row)
diff --git a/packages/server/src/events/AutomationEmitter.ts b/packages/server/src/events/AutomationEmitter.ts
index a63273bdc0..ec1ea7f28c 100644
--- a/packages/server/src/events/AutomationEmitter.ts
+++ b/packages/server/src/events/AutomationEmitter.ts
@@ -1,12 +1,12 @@
 import { rowEmission, tableEmission } from "./utils"
 import mainEmitter from "./index"
 import env from "../environment"
-import { Table, Row, DocumentType, App } from "@budibase/types"
+import { Table, Row, DocumentType, App, ContextEmitter } from "@budibase/types"
 import { context } from "@budibase/backend-core"
 
 const MAX_AUTOMATIONS_ALLOWED = 5
 
-class AutomationEmitter {
+class AutomationEmitter implements ContextEmitter {
   chainCount: number
   metadata: { automationChainCount: number }
 
diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts
index a7df701171..8add047c0b 100644
--- a/packages/types/src/sdk/koa.ts
+++ b/packages/types/src/sdk/koa.ts
@@ -1,5 +1,14 @@
 import { Context, Request } from "koa"
-import { User, Role, UserRoles, Account, ConfigType } from "../documents"
+import {
+  User,
+  Role,
+  UserRoles,
+  Account,
+  ConfigType,
+  Row,
+  Table,
+  AutomationEventType,
+} from "../documents"
 import { FeatureFlag, License } from "../sdk"
 import { Files } from "formidable"
 
@@ -40,6 +49,7 @@ export interface UserCtx<RequestBody = any, ResponseBody = any>
   extends Ctx<RequestBody, ResponseBody> {
   user: ContextUser
   roleId?: string
+  eventEmitter?: ContextEmitter
 }
 
 /**
@@ -49,3 +59,12 @@ export interface UserCtx<RequestBody = any, ResponseBody = any>
 export interface BBContext extends Ctx {
   user?: ContextUser
 }
+
+export interface ContextEmitter {
+  emitRow: (params: {
+    eventName: AutomationEventType
+    appId: string
+    row: Row
+    table?: Table
+  }) => Promise<void>
+}

From 478160c412cf4c008e3b581d7ac7bbf78d4072f3 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 12:28:23 +0100
Subject: [PATCH 20/34] Fix all tests.

---
 packages/backend-core/src/sql/sql.ts          |   4 +-
 .../src/api/routes/tests/search.spec.ts       | 142 ++++++++++--------
 2 files changed, 81 insertions(+), 65 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 883f7a4d5f..d94b817ba4 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -1134,8 +1134,8 @@ class InternalBuilder {
           if (this.client === SqlClient.ORACLE) {
             query = query.orderByRaw(`?? ?? nulls ??`, [
               this.convertClobs(composite),
-              direction,
-              nulls,
+              this.knex.raw(direction),
+              this.knex.raw(nulls as string),
             ])
           } else {
             query = query.orderBy(composite, direction, nulls)
diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index dfee63fa4d..2f3dbe91f6 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -61,6 +61,7 @@ describe.each([
   const isLucene = name === "lucene"
   const isInMemory = name === "in-memory"
   const isInternal = isSqs || isLucene || isInMemory
+  const isOracle = name === DatabaseName.ORACLE
   const isSql = !isInMemory && !isLucene
   const config = setup.getConfig()
 
@@ -155,24 +156,24 @@ describe.each([
 
   describe.each([
     ["table", createTable],
-    // [
-    //   "view",
-    //   async (schema: TableSchema) => {
-    //     const tableId = await createTable(schema)
-    //     const viewId = await createView(
-    //       tableId,
-    //       Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
-    //         const field = schema[fieldName]
-    //         viewSchema[fieldName] = {
-    //           visible: field.visible ?? true,
-    //           readonly: false,
-    //         }
-    //         return viewSchema
-    //       }, {})
-    //     )
-    //     return viewId
-    //   },
-    // ],
+    [
+      "view",
+      async (schema: TableSchema) => {
+        const tableId = await createTable(schema)
+        const viewId = await createView(
+          tableId,
+          Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
+            const field = schema[fieldName]
+            viewSchema[fieldName] = {
+              visible: field.visible ?? true,
+              readonly: false,
+            }
+            return viewSchema
+          }, {})
+        )
+        return viewId
+      },
+    ],
   ])("from %s", (sourceType, createTableOrView) => {
     const isView = sourceType === "view"
 
@@ -3472,54 +3473,69 @@ describe.each([
         })
       })
 
-    describe("SQL injection", () => {
-      const badStrings = [
-        "1; DROP TABLE test;",
-        "1; DELETE FROM test;",
-        "1; UPDATE test SET name = 'foo';",
-        "1; INSERT INTO test (name) VALUES ('foo');",
-        "' OR '1'='1' --",
-        "'; DROP TABLE users; --",
-        "' OR 1=1 --",
-        "' UNION SELECT null, null, null; --",
-        "' AND (SELECT COUNT(*) FROM users) > 0 --",
-        "\"; EXEC xp_cmdshell('dir'); --",
-        "\"' OR 'a'='a",
-        "OR 1=1;",
-        "'; SHUTDOWN --",
-      ]
+    isSql &&
+      describe("SQL injection", () => {
+        const badStrings = [
+          "1; DROP TABLE test;",
+          "1; DELETE FROM test;",
+          "1; UPDATE test SET name = 'foo';",
+          "1; INSERT INTO test (name) VALUES ('foo');",
+          "' OR '1'='1' --",
+          "'; DROP TABLE users; --",
+          "' OR 1=1 --",
+          "' UNION SELECT null, null, null; --",
+          "' AND (SELECT COUNT(*) FROM users) > 0 --",
+          "\"; EXEC xp_cmdshell('dir'); --",
+          "\"' OR 'a'='a",
+          "OR 1=1;",
+          "'; SHUTDOWN --",
+        ]
 
-      describe.only.each(badStrings)("bad string: %s", badString => {
-        it("should not allow SQL injection as a field name", async () => {
-          const tableOrViewId = await createTableOrView({
-            [badString]: {
-              name: badString,
-              type: FieldType.STRING,
-            },
+        describe.each(badStrings)("bad string: %s", badString => {
+          // The SQL that knex generates when you try to use a double quote in a
+          // field name is always invalid and never works, so we skip it for these
+          // tests.
+          const skipFieldNameCheck = isOracle && badString.includes('"')
+
+          !skipFieldNameCheck &&
+            it("should not allow SQL injection as a field name", async () => {
+              const tableOrViewId = await createTableOrView({
+                [badString]: {
+                  name: badString,
+                  type: FieldType.STRING,
+                },
+              })
+
+              await config.api.row.save(tableOrViewId, { [badString]: "foo" })
+
+              const { rows } = await config.api.row.search(
+                tableOrViewId,
+                { query: {} },
+                { status: 200 }
+              )
+
+              expect(rows).toHaveLength(1)
+            })
+
+          it("should not allow SQL injection as a field value", async () => {
+            const tableOrViewId = await createTableOrView({
+              foo: {
+                name: "foo",
+                type: FieldType.STRING,
+              },
+            })
+
+            await config.api.row.save(tableOrViewId, { foo: "foo" })
+
+            const { rows } = await config.api.row.search(
+              tableOrViewId,
+              { query: { equal: { foo: badString } } },
+              { status: 200 }
+            )
+
+            expect(rows).toBeEmpty()
           })
-
-          await config.api.row.search(
-            tableOrViewId,
-            { query: {} },
-            { status: 200 }
-          )
-        })
-
-        it("should not allow SQL injection as a field value", async () => {
-          const tableOrViewId = await createTableOrView({
-            foo: {
-              name: "foo",
-              type: FieldType.STRING,
-            },
-          })
-
-          await config.api.row.search(
-            tableOrViewId,
-            { query: { equal: { foo: badString } } },
-            { status: 200 }
-          )
         })
       })
-    })
   })
 })

From e54bb3fbdc76cbe4252dea99a86669d46ac26344 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 12:33:32 +0100
Subject: [PATCH 21/34] Uncomment view tests.

---
 packages/server/src/api/routes/tests/viewV2.spec.ts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index 0c18307909..dfd4f50bd1 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -35,12 +35,12 @@ import { quotas } from "@budibase/pro"
 import { db, roles, features } from "@budibase/backend-core"
 
 describe.each([
-  // ["lucene", undefined],
-  // ["sqs", undefined],
-  // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
-  // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
-  // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
-  // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
+  ["lucene", undefined],
+  ["sqs", undefined],
+  [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
+  [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
+  [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
+  [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
   [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
 ])("/v2/views (%s)", (name, dsProvider) => {
   const config = setup.getConfig()

From 342d70b3265a559f2d20337471a7ddc2dc4ee9c4 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 13:34:47 +0200
Subject: [PATCH 22/34] Proper typing

---
 .../server/src/api/controllers/row/index.ts   | 46 +++++++++----------
 packages/server/src/db/linkedRows/index.ts    | 11 +----
 packages/server/src/events/BudibaseEmitter.ts | 16 +++++--
 packages/server/src/events/utils.ts           |  6 +--
 packages/types/src/core/events.ts             |  8 ++++
 packages/types/src/core/index.ts              |  1 +
 packages/types/src/sdk/koa.ts                 | 26 +++++++++--
 7 files changed, 68 insertions(+), 46 deletions(-)
 create mode 100644 packages/types/src/core/events.ts

diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts
index d3b1d3d5b2..fe29d46700 100644
--- a/packages/server/src/api/controllers/row/index.ts
+++ b/packages/server/src/api/controllers/row/index.ts
@@ -7,11 +7,11 @@ import * as internal from "./internal"
 import * as external from "./external"
 import { isExternalTableID } from "../../../integrations/utils"
 import {
-  AutomationEventType,
   Ctx,
   DeleteRow,
   DeleteRowRequest,
   DeleteRows,
+  EventType,
   ExportRowsRequest,
   ExportRowsResponse,
   FieldType,
@@ -67,7 +67,7 @@ export async function patch(
     ctx.status = 200
 
     ctx.eventEmitter?.emitRow({
-      eventName: AutomationEventType.ROW_UPDATE,
+      eventName: EventType.ROW_UPDATE,
       appId,
       row,
       table,
@@ -104,14 +104,14 @@ export const save = async (ctx: UserCtx<Row, Row>) => {
         sdk.rows.save(sourceId, ctx.request.body, ctx.user?._id)
       )
   ctx.status = 200
-  ctx.eventEmitter &&
-    ctx.eventEmitter.emitRow({
-      eventName: `row:save`,
-      appId,
-      row,
-      table,
-      user: sdk.users.getUserContextBindings(ctx.user),
-    })
+
+  ctx.eventEmitter?.emitRow({
+    eventName: EventType.ROW_SAVE,
+    appId,
+    row,
+    table,
+    user: sdk.users.getUserContextBindings(ctx.user),
+  })
   ctx.message = `${table.name} saved successfully`
   // prefer squashed for response
   ctx.body = row || squashed
@@ -183,13 +183,12 @@ async function deleteRows(ctx: UserCtx<DeleteRowRequest>) {
   }
 
   for (let row of rows) {
-    ctx.eventEmitter &&
-      ctx.eventEmitter.emitRow({
-        eventName: `row:delete`,
-        appId,
-        row,
-        user: sdk.users.getUserContextBindings(ctx.user),
-      })
+    ctx.eventEmitter?.emitRow({
+      eventName: EventType.ROW_DELETE,
+      appId,
+      row,
+      user: sdk.users.getUserContextBindings(ctx.user),
+    })
     gridSocket?.emitRowDeletion(ctx, row)
   }
   return rows
@@ -204,13 +203,12 @@ async function deleteRow(ctx: UserCtx<DeleteRowRequest>) {
     await quotas.removeRow()
   }
 
-  ctx.eventEmitter &&
-    ctx.eventEmitter.emitRow({
-      eventName: `row:delete`,
-      appId,
-      row: resp.row,
-      user: sdk.users.getUserContextBindings(ctx.user),
-    })
+  ctx.eventEmitter?.emitRow({
+    eventName: EventType.ROW_DELETE,
+    appId,
+    row: resp.row,
+    user: sdk.users.getUserContextBindings(ctx.user),
+  })
   gridSocket?.emitRowDeletion(ctx, resp.row)
 
   return resp
diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts
index fbb83b49a5..73ac695878 100644
--- a/packages/server/src/db/linkedRows/index.ts
+++ b/packages/server/src/db/linkedRows/index.ts
@@ -17,6 +17,7 @@ import {
 import { context, features } from "@budibase/backend-core"
 import {
   ContextUser,
+  EventType,
   FeatureFlag,
   FieldType,
   LinkDocumentValue,
@@ -44,15 +45,7 @@ const INVALID_DISPLAY_COLUMN_TYPE = [
  * This functionality makes sure that when rows with links are created, updated or deleted they are processed
  * correctly - making sure that no stale links are left around and that all links have been made successfully.
  */
-
-export const EventType = {
-  ROW_SAVE: "row:save",
-  ROW_UPDATE: "row:update",
-  ROW_DELETE: "row:delete",
-  TABLE_SAVE: "table:save",
-  TABLE_UPDATED: "table:updated",
-  TABLE_DELETE: "table:delete",
-}
+export { EventType } from "@budibase/types"
 
 function clearRelationshipFields(schema: TableSchema, rows: Row[]) {
   for (let [key, field] of Object.entries(schema)) {
diff --git a/packages/server/src/events/BudibaseEmitter.ts b/packages/server/src/events/BudibaseEmitter.ts
index c8983096d0..9fc5c2f906 100644
--- a/packages/server/src/events/BudibaseEmitter.ts
+++ b/packages/server/src/events/BudibaseEmitter.ts
@@ -1,6 +1,12 @@
 import { EventEmitter } from "events"
 import { rowEmission, tableEmission } from "./utils"
-import { Table, Row, User } from "@budibase/types"
+import {
+  Table,
+  Row,
+  UserBindings,
+  EventType,
+  ContextEmitter,
+} from "@budibase/types"
 
 /**
  * keeping event emitter in one central location as it might be used for things other than
@@ -12,7 +18,7 @@ import { Table, Row, User } from "@budibase/types"
  * Extending the standard emitter to some syntactic sugar and standardisation to the emitted event.
  * This is specifically quite important for template strings used in automations.
  */
-class BudibaseEmitter extends EventEmitter {
+class BudibaseEmitter extends EventEmitter implements ContextEmitter {
   emitRow({
     eventName,
     appId,
@@ -21,17 +27,17 @@ class BudibaseEmitter extends EventEmitter {
     oldRow,
     user,
   }: {
-    eventName: string
+    eventName: EventType.ROW_SAVE | EventType.ROW_DELETE | EventType.ROW_UPDATE
     appId: string
     row: Row
     table?: Table
     oldRow?: Row
-    user: User
+    user: UserBindings
   }) {
     rowEmission({ emitter: this, eventName, appId, row, table, oldRow, user })
   }
 
-  emitTable(eventName: string, appId: string, table?: Table) {
+  emitTable(eventName: EventType, appId: string, table?: Table) {
     tableEmission({ emitter: this, eventName, appId, table })
   }
 
diff --git a/packages/server/src/events/utils.ts b/packages/server/src/events/utils.ts
index 5e4a1bebbf..9cb5eef187 100644
--- a/packages/server/src/events/utils.ts
+++ b/packages/server/src/events/utils.ts
@@ -1,4 +1,4 @@
-import { Table, Row, User } from "@budibase/types"
+import { Table, Row, UserBindings } from "@budibase/types"
 import BudibaseEmitter from "./BudibaseEmitter"
 
 type BBEventOpts = {
@@ -9,7 +9,7 @@ type BBEventOpts = {
   row?: Row
   oldRow?: Row
   metadata?: any
-  user?: User
+  user?: UserBindings
 }
 
 interface BBEventTable extends Table {
@@ -25,7 +25,7 @@ type BBEvent = {
   id?: string
   revision?: string
   metadata?: any
-  user?: User
+  user?: UserBindings
 }
 
 export function rowEmission({
diff --git a/packages/types/src/core/events.ts b/packages/types/src/core/events.ts
new file mode 100644
index 0000000000..3ce948b3b5
--- /dev/null
+++ b/packages/types/src/core/events.ts
@@ -0,0 +1,8 @@
+export const enum EventType {
+  ROW_SAVE = "row:save",
+  ROW_UPDATE = "row:update",
+  ROW_DELETE = "row:delete",
+  TABLE_SAVE = "table:save",
+  TABLE_UPDATED = "table:updated",
+  TABLE_DELETE = "table:delete",
+}
diff --git a/packages/types/src/core/index.ts b/packages/types/src/core/index.ts
index b5cbd7affa..73cc7d35e0 100644
--- a/packages/types/src/core/index.ts
+++ b/packages/types/src/core/index.ts
@@ -1 +1,2 @@
 export * from "./installation"
+export * from "./events"
diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts
index 8add047c0b..f0a81a7886 100644
--- a/packages/types/src/sdk/koa.ts
+++ b/packages/types/src/sdk/koa.ts
@@ -7,10 +7,11 @@ import {
   ConfigType,
   Row,
   Table,
-  AutomationEventType,
+  UserBindings,
 } from "../documents"
 import { FeatureFlag, License } from "../sdk"
 import { Files } from "formidable"
+import { EventType } from "../core"
 
 export interface ContextUser extends Omit<User, "roles"> {
   globalId?: string
@@ -61,10 +62,25 @@ export interface BBContext extends Ctx {
 }
 
 export interface ContextEmitter {
-  emitRow: (params: {
-    eventName: AutomationEventType
+  emitRow(values: {
+    eventName: EventType.ROW_SAVE
     appId: string
     row: Row
-    table?: Table
-  }) => Promise<void>
+    table: Table
+    user: UserBindings
+  }): void
+  emitRow(values: {
+    eventName: EventType.ROW_UPDATE
+    appId: string
+    row: Row
+    table: Table
+    oldRow: Row
+    user: UserBindings
+  }): void
+  emitRow(values: {
+    eventName: EventType.ROW_DELETE
+    appId: string
+    row: Row
+    user: UserBindings
+  }): void
 }

From 637c13c1af339313f99763ff10d1e1c6cb861581 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 13:44:20 +0200
Subject: [PATCH 23/34] Proper typing for emitTable

---
 packages/server/src/api/controllers/table/index.ts | 8 ++++----
 packages/types/src/sdk/koa.ts                      | 5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts
index 2f2f93bffe..77c1f3923a 100644
--- a/packages/server/src/api/controllers/table/index.ts
+++ b/packages/server/src/api/controllers/table/index.ts
@@ -16,6 +16,7 @@ import {
   BulkImportResponse,
   CsvToJsonRequest,
   CsvToJsonResponse,
+  EventType,
   FetchTablesResponse,
   FieldType,
   MigrateRequest,
@@ -129,8 +130,7 @@ export async function save(ctx: UserCtx<SaveTableRequest, SaveTableResponse>) {
   }
   ctx.status = 200
   ctx.message = `Table ${table.name} saved successfully.`
-  ctx.eventEmitter &&
-    ctx.eventEmitter.emitTable(`table:save`, appId, { ...savedTable })
+  ctx.eventEmitter?.emitTable(EventType.TABLE_SAVE, appId, { ...savedTable })
   ctx.body = savedTable
 
   savedTable = await processTable(savedTable)
@@ -143,8 +143,8 @@ export async function destroy(ctx: UserCtx) {
   await sdk.rowActions.deleteAll(tableId)
   const deletedTable = await pickApi({ tableId }).destroy(ctx)
   await events.table.deleted(deletedTable)
-  ctx.eventEmitter &&
-    ctx.eventEmitter.emitTable(`table:delete`, appId, deletedTable)
+
+  ctx.eventEmitter?.emitTable(EventType.TABLE_DELETE, appId, deletedTable)
   ctx.status = 200
   ctx.table = deletedTable
   ctx.body = { message: `Table ${tableId} deleted.` }
diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts
index f0a81a7886..95ea2b652f 100644
--- a/packages/types/src/sdk/koa.ts
+++ b/packages/types/src/sdk/koa.ts
@@ -83,4 +83,9 @@ export interface ContextEmitter {
     row: Row
     user: UserBindings
   }): void
+  emitTable(
+    eventName: EventType.TABLE_SAVE | EventType.TABLE_DELETE,
+    appId: string,
+    table?: Table
+  ): void
 }

From 4dedde5165b9c876480e8077f5bbe63ad55d9ee3 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 13:51:44 +0200
Subject: [PATCH 24/34] Add missing rowaction

---
 .../server/src/events/AutomationEmitter.ts     | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/packages/server/src/events/AutomationEmitter.ts b/packages/server/src/events/AutomationEmitter.ts
index ec1ea7f28c..a95acd0877 100644
--- a/packages/server/src/events/AutomationEmitter.ts
+++ b/packages/server/src/events/AutomationEmitter.ts
@@ -1,7 +1,15 @@
 import { rowEmission, tableEmission } from "./utils"
 import mainEmitter from "./index"
 import env from "../environment"
-import { Table, Row, DocumentType, App, ContextEmitter } from "@budibase/types"
+import {
+  Table,
+  Row,
+  DocumentType,
+  App,
+  ContextEmitter,
+  EventType,
+  UserBindings,
+} from "@budibase/types"
 import { context } from "@budibase/backend-core"
 
 const MAX_AUTOMATIONS_ALLOWED = 5
@@ -36,11 +44,15 @@ class AutomationEmitter implements ContextEmitter {
     appId,
     row,
     table,
+    oldRow,
+    user,
   }: {
-    eventName: string
+    eventName: EventType.ROW_SAVE | EventType.ROW_DELETE | EventType.ROW_UPDATE
     appId: string
     row: Row
     table?: Table
+    oldRow?: Row
+    user: UserBindings
   }) {
     let MAX_AUTOMATION_CHAIN = await this.getMaxAutomationChain()
 
@@ -54,7 +66,9 @@ class AutomationEmitter implements ContextEmitter {
       appId,
       row,
       table,
+      oldRow,
       metadata: this.metadata,
+      user,
     })
   }
 

From f279a1716df4d9ec1aad8211d9a79c1d918854a1 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 15:52:52 +0200
Subject: [PATCH 25/34] Allow overriding the min version where we stop serving
 the power role

---
 packages/backend-core/src/environment.ts    | 2 ++
 packages/backend-core/src/security/roles.ts | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts
index 7d3a9f18f5..4cb0a9c731 100644
--- a/packages/backend-core/src/environment.ts
+++ b/packages/backend-core/src/environment.ts
@@ -223,6 +223,8 @@ const environment = {
   BB_ADMIN_USER_EMAIL: process.env.BB_ADMIN_USER_EMAIL,
   BB_ADMIN_USER_PASSWORD: process.env.BB_ADMIN_USER_PASSWORD,
   OPENAI_API_KEY: process.env.OPENAI_API_KEY,
+  MIN_VERSION_WITHOUT_POWER_ROLE:
+    process.env.MIN_VERSION_WITHOUT_POWER_ROLE || "3.0.0",
 }
 
 export function setEnv(newEnvVars: Partial<typeof environment>): () => void {
diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts
index df7f41e6ce..76c4f08fbe 100644
--- a/packages/backend-core/src/security/roles.ts
+++ b/packages/backend-core/src/security/roles.ts
@@ -19,6 +19,7 @@ import {
 import cloneDeep from "lodash/fp/cloneDeep"
 import { RoleColor, helpers } from "@budibase/shared-core"
 import { uniqBy } from "lodash"
+import { default as env } from "../environment"
 
 export const BUILTIN_ROLE_IDS = {
   ADMIN: "ADMIN",
@@ -545,7 +546,10 @@ async function shouldIncludePowerRole(db: Database) {
     return true
   }
 
-  const isGreaterThan3x = semver.gte(creationVersion, "3.0.0")
+  const isGreaterThan3x = semver.gte(
+    creationVersion,
+    env.MIN_VERSION_WITHOUT_POWER_ROLE
+  )
   return !isGreaterThan3x
 }
 

From 288517df96688b04b57c51d71da1d00fb163e971 Mon Sep 17 00:00:00 2001
From: Adria Navarro <adria@budibase.com>
Date: Thu, 24 Oct 2024 16:08:04 +0200
Subject: [PATCH 26/34] Fix frontend issues due power role missing

---
 .../backend/DataTable/buttons/ManageAccessButton.svelte       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
index 2826d8d986..17c26847b3 100644
--- a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
+++ b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
@@ -34,7 +34,9 @@
   $: buttonLabel = readableRole ? `Access: ${readableRole}` : "Access"
   $: highlight = roleMismatch || selectedRole === Roles.PUBLIC
 
-  $: builtInRoles = builtins.map(roleId => $roles.find(x => x._id === roleId))
+  $: builtInRoles = builtins
+    .map(roleId => $roles.find(x => x._id === roleId))
+    .filter(r => !!r)
   $: customRoles = $roles
     .filter(x => !builtins.includes(x._id))
     .slice()

From c8a783786834e39b6c653dd93ca27d1b76f0efb7 Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Thu, 24 Oct 2024 15:30:05 +0100
Subject: [PATCH 27/34] Skip access API request if unchanged

---
 .../backend/DataTable/buttons/ManageAccessButton.svelte        | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
index 17c26847b3..85227b9140 100644
--- a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
+++ b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte
@@ -102,6 +102,9 @@
   }
 
   const changePermission = async role => {
+    if (role === selectedRole) {
+      return
+    }
     try {
       await permissionsStore.save({
         level: "read",

From 6e6e1368c1395e53e3ff1f0716afc9e9ce04fd91 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 15:32:08 +0100
Subject: [PATCH 28/34] Assert table is not deleted in SQL injection tests.

---
 .../src/api/routes/tests/search.spec.ts       | 77 +++++++++++++------
 1 file changed, 54 insertions(+), 23 deletions(-)

diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 2f3dbe91f6..d69f3f2c38 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -7,6 +7,7 @@ import {
 import {
   context,
   db as dbCore,
+  docIds,
   features,
   MAX_VALID_DATE,
   MIN_VALID_DATE,
@@ -130,14 +131,14 @@ describe.each([
     }
   })
 
-  async function createTable(schema: TableSchema) {
+  async function createTable(schema?: TableSchema) {
     const table = await config.api.table.save(
       tableForDatasource(datasource, { schema })
     )
     return table._id!
   }
 
-  async function createView(tableId: string, schema: ViewV2Schema) {
+  async function createView(tableId: string, schema?: ViewV2Schema) {
     const view = await config.api.viewV2.create({
       tableId: tableId,
       name: generator.guid(),
@@ -154,22 +155,34 @@ describe.each([
     rows = await config.api.row.fetch(tableOrViewId)
   }
 
+  async function getTable(tableOrViewId: string): Promise<Table> {
+    if (docIds.isViewId(tableOrViewId)) {
+      const view = await config.api.viewV2.get(tableOrViewId)
+      return await config.api.table.get(view.tableId)
+    } else {
+      return await config.api.table.get(tableOrViewId)
+    }
+  }
+
   describe.each([
     ["table", createTable],
     [
       "view",
-      async (schema: TableSchema) => {
+      async (schema?: TableSchema) => {
         const tableId = await createTable(schema)
         const viewId = await createView(
           tableId,
-          Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
-            const field = schema[fieldName]
-            viewSchema[fieldName] = {
-              visible: field.visible ?? true,
-              readonly: false,
-            }
-            return viewSchema
-          }, {})
+          Object.keys(schema || {}).reduce<ViewV2Schema>(
+            (viewSchema, fieldName) => {
+              const field = schema![fieldName]
+              viewSchema[fieldName] = {
+                visible: field.visible ?? true,
+                readonly: false,
+              }
+              return viewSchema
+            },
+            {}
+          )
         )
         return viewId
       },
@@ -3476,36 +3489,45 @@ describe.each([
     isSql &&
       describe("SQL injection", () => {
         const badStrings = [
-          "1; DROP TABLE test;",
-          "1; DELETE FROM test;",
-          "1; UPDATE test SET name = 'foo';",
-          "1; INSERT INTO test (name) VALUES ('foo');",
+          "1; DROP TABLE %table_name%;",
+          "1; DELETE FROM %table_name%;",
+          "1; UPDATE %table_name% SET name = 'foo';",
+          "1; INSERT INTO %table_name% (name) VALUES ('foo');",
           "' OR '1'='1' --",
-          "'; DROP TABLE users; --",
+          "'; DROP TABLE %table_name%; --",
           "' OR 1=1 --",
           "' UNION SELECT null, null, null; --",
-          "' AND (SELECT COUNT(*) FROM users) > 0 --",
+          "' AND (SELECT COUNT(*) FROM %table_name%) > 0 --",
           "\"; EXEC xp_cmdshell('dir'); --",
           "\"' OR 'a'='a",
           "OR 1=1;",
           "'; SHUTDOWN --",
         ]
 
-        describe.each(badStrings)("bad string: %s", badString => {
+        describe.each(badStrings)("bad string: %s", badStringTemplate => {
           // The SQL that knex generates when you try to use a double quote in a
           // field name is always invalid and never works, so we skip it for these
           // tests.
-          const skipFieldNameCheck = isOracle && badString.includes('"')
+          const skipFieldNameCheck = isOracle && badStringTemplate.includes('"')
 
           !skipFieldNameCheck &&
             it("should not allow SQL injection as a field name", async () => {
-              const tableOrViewId = await createTableOrView({
-                [badString]: {
-                  name: badString,
-                  type: FieldType.STRING,
+              const tableOrViewId = await createTableOrView()
+              const table = await getTable(tableOrViewId)
+              const badString = badStringTemplate.replace(
+                /%table_name%/g,
+                table.name
+              )
+
+              await config.api.table.save({
+                ...table,
+                schema: {
+                  [badString]: { name: badString, type: FieldType.STRING },
                 },
               })
 
+              expect(await client!.schema.hasTable(table.name)).toBeTrue()
+
               await config.api.row.save(tableOrViewId, { [badString]: "foo" })
 
               const { rows } = await config.api.row.search(
@@ -3515,6 +3537,7 @@ describe.each([
               )
 
               expect(rows).toHaveLength(1)
+              expect(await client!.schema.hasTable(table.name)).toBeTrue()
             })
 
           it("should not allow SQL injection as a field value", async () => {
@@ -3524,6 +3547,13 @@ describe.each([
                 type: FieldType.STRING,
               },
             })
+            const table = await getTable(tableOrViewId)
+            const badString = badStringTemplate.replace(
+              /%table_name%/g,
+              table.name
+            )
+
+            expect(await client!.schema.hasTable(table.name)).toBeTrue()
 
             await config.api.row.save(tableOrViewId, { foo: "foo" })
 
@@ -3534,6 +3564,7 @@ describe.each([
             )
 
             expect(rows).toBeEmpty()
+            expect(await client!.schema.hasTable(table.name)).toBeTrue()
           })
         })
       })

From 977826a0ca344188b75b26bc9c5efd073592f23c Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 15:37:53 +0100
Subject: [PATCH 29/34] Clean up table assertions in SQL injection tests.

---
 .../src/api/routes/tests/search.spec.ts       | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index d69f3f2c38..1021139ed1 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -164,6 +164,14 @@ describe.each([
     }
   }
 
+  async function assertTableExists(name: string) {
+    expect(await client!.schema.hasTable(name)).toBeTrue()
+  }
+
+  async function assertTableNumRows(name: string, numRows: number) {
+    expect(await client!.from(name).count()).toEqual([{ count: `${numRows}` }])
+  }
+
   describe.each([
     ["table", createTable],
     [
@@ -3526,10 +3534,11 @@ describe.each([
                 },
               })
 
-              expect(await client!.schema.hasTable(table.name)).toBeTrue()
-
               await config.api.row.save(tableOrViewId, { [badString]: "foo" })
 
+              await assertTableExists(table.name)
+              await assertTableNumRows(table.name, 1)
+
               const { rows } = await config.api.row.search(
                 tableOrViewId,
                 { query: {} },
@@ -3537,7 +3546,9 @@ describe.each([
               )
 
               expect(rows).toHaveLength(1)
-              expect(await client!.schema.hasTable(table.name)).toBeTrue()
+
+              await assertTableExists(table.name)
+              await assertTableNumRows(table.name, 1)
             })
 
           it("should not allow SQL injection as a field value", async () => {
@@ -3553,7 +3564,8 @@ describe.each([
               table.name
             )
 
-            expect(await client!.schema.hasTable(table.name)).toBeTrue()
+            await assertTableExists(table.name)
+            await assertTableNumRows(table.name, 1)
 
             await config.api.row.save(tableOrViewId, { foo: "foo" })
 
@@ -3564,7 +3576,8 @@ describe.each([
             )
 
             expect(rows).toBeEmpty()
-            expect(await client!.schema.hasTable(table.name)).toBeTrue()
+            await assertTableExists(table.name)
+            await assertTableNumRows(table.name, 1)
           })
         })
       })

From be5564621c0339aa17e52436a511ae232914e7af Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Thu, 24 Oct 2024 16:19:43 +0100
Subject: [PATCH 30/34] Add info display in user side panel to warn builders if
 they haven't published their app

---
 .../src/components/deploy/AppActions.svelte   | 20 +++++++---------
 .../_components/BuilderSidePanel.svelte       | 23 +++++++++++++++----
 .../_components/Component/InfoDisplay.svelte  | 23 +++++++++++--------
 packages/builder/src/stores/builder/index.js  |  2 ++
 .../builder/src/stores/builder/published.js   | 13 +++++++++++
 5 files changed, 56 insertions(+), 25 deletions(-)
 create mode 100644 packages/builder/src/stores/builder/published.js

diff --git a/packages/builder/src/components/deploy/AppActions.svelte b/packages/builder/src/components/deploy/AppActions.svelte
index bb950983a6..9ba46832f4 100644
--- a/packages/builder/src/components/deploy/AppActions.svelte
+++ b/packages/builder/src/components/deploy/AppActions.svelte
@@ -25,6 +25,7 @@
     appStore,
     deploymentStore,
     sortedScreens,
+    appPublished,
   } from "stores/builder"
   import TourWrap from "components/portal/onboarding/TourWrap.svelte"
   import { TOUR_STEP_KEYS } from "components/portal/onboarding/tours.js"
@@ -45,11 +46,6 @@
 
   $: filteredApps = $appsStore.apps.filter(app => app.devId === application)
   $: selectedApp = filteredApps?.length ? filteredApps[0] : null
-  $: latestDeployments = $deploymentStore
-    .filter(deployment => deployment.status === "SUCCESS")
-    .sort((a, b) => a.updatedAt > b.updatedAt)
-  $: isPublished =
-    selectedApp?.status === "published" && latestDeployments?.length > 0
   $: updateAvailable =
     $appStore.upgradableVersion &&
     $appStore.version &&
@@ -117,7 +113,7 @@
   }
 
   const confirmUnpublishApp = async () => {
-    if (!application || !isPublished) {
+    if (!application || !$appPublished) {
       //confirm the app has loaded.
       return
     }
@@ -204,7 +200,7 @@
     >
       <div bind:this={appActionPopoverAnchor}>
         <div class="app-action">
-          <Icon name={isPublished ? "GlobeCheck" : "GlobeStrike"} />
+          <Icon name={$appPublished ? "GlobeCheck" : "GlobeStrike"} />
           <TourWrap stepKeys={[TOUR_STEP_KEYS.BUILDER_APP_PUBLISH]}>
             <span class="publish-open" id="builder-app-publish-button">
               Publish
@@ -219,7 +215,7 @@
       <Popover
         bind:this={appActionPopover}
         align="right"
-        disabled={!isPublished}
+        disabled={!$appPublished}
         anchor={appActionPopoverAnchor}
         offset={35}
         on:close={() => {
@@ -236,13 +232,13 @@
                 class="app-link"
                 on:click={() => {
                   appActionPopover.hide()
-                  if (isPublished) {
+                  if ($appPublished) {
                     viewApp()
                   }
                 }}
               >
                 {$appStore.url}
-                {#if isPublished}
+                {#if $appPublished}
                   <Icon size="S" name="LinkOut" />
                 {/if}
               </span>
@@ -250,7 +246,7 @@
 
             <Body size="S">
               <span class="publish-popover-status">
-                {#if isPublished}
+                {#if $appPublished}
                   <span class="status-text">
                     {lastDeployed}
                   </span>
@@ -279,7 +275,7 @@
               </span>
             </Body>
             <div class="action-buttons">
-              {#if isPublished}
+              {#if $appPublished}
                 <ActionButton
                   quiet
                   icon="Code"
diff --git a/packages/builder/src/pages/builder/app/[application]/_components/BuilderSidePanel.svelte b/packages/builder/src/pages/builder/app/[application]/_components/BuilderSidePanel.svelte
index 035dc5a2ef..687678c08a 100644
--- a/packages/builder/src/pages/builder/app/[application]/_components/BuilderSidePanel.svelte
+++ b/packages/builder/src/pages/builder/app/[application]/_components/BuilderSidePanel.svelte
@@ -14,7 +14,7 @@
     Button,
     FancySelect,
   } from "@budibase/bbui"
-  import { builderStore, appStore, roles } from "stores/builder"
+  import { builderStore, appStore, roles, appPublished } from "stores/builder"
   import {
     groups,
     licensing,
@@ -36,6 +36,7 @@
   import UpgradeModal from "components/common/users/UpgradeModal.svelte"
   import { emailValidator } from "helpers/validation"
   import { fly } from "svelte/transition"
+  import InfoDisplay from "../design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte"
 
   let query = null
   let loaded = false
@@ -608,6 +609,17 @@
     </div>
 
     <div class="body">
+      {#if !$appPublished}
+        <div class="alert">
+          <InfoDisplay
+            icon="AlertCircleFilled"
+            warning
+            title="App unpublished"
+            body="Users won't be able to access your app until you've published it"
+          />
+        </div>
+      {/if}
+
       {#if promptInvite && !userOnboardResponse}
         <Layout gap="S" paddingX="XL">
           <div class="invite-header">
@@ -623,7 +635,7 @@
       {/if}
 
       {#if !promptInvite}
-        <Layout gap="L" noPadding>
+        <Layout gap="M" noPadding>
           {#if filteredInvites?.length}
             <Layout noPadding gap="XS">
               <div class="auth-entity-header">
@@ -926,7 +938,7 @@
   .auth-entity,
   .auth-entity-header {
     display: grid;
-    grid-template-columns: 1fr 180px;
+    grid-template-columns: 1fr 220px;
     align-items: center;
     gap: var(--spacing-xl);
   }
@@ -957,7 +969,7 @@
     overflow-y: auto;
     overflow-x: hidden;
     position: absolute;
-    width: 440px;
+    width: 480px;
     right: 0;
     height: 100%;
     box-shadow: 0 0 40px 10px rgba(0, 0, 0, 0.1);
@@ -1034,4 +1046,7 @@
     gap: var(--spacing-xl);
     padding: var(--spacing-xl) 0;
   }
+  .alert {
+    padding: 0 var(--spacing-xl);
+  }
 </style>
diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
index 88ddf6f9a5..5543a4c7fd 100644
--- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
+++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
@@ -7,9 +7,17 @@
   export let quiet = false
   export let warning = false
   export let error = false
+  export let info = false
 </script>
 
-<div class="info" class:noTitle={!title} class:warning class:error class:quiet>
+<div
+  class="container"
+  class:info
+  class:noTitle={!title}
+  class:warning
+  class:error
+  class:quiet
+>
   {#if title}
     <div class="title">
       <Icon name={icon} />
@@ -27,7 +35,7 @@
 </div>
 
 <style>
-  .info {
+  .container {
     padding: var(--spacing-m) var(--spacing-l);
     background-color: var(--spectrum-global-color-gray-200);
     border-radius: var(--border-radius-s);
@@ -39,6 +47,9 @@
   .error {
     background: rgba(255, 0, 0, 0.2);
   }
+  .info {
+    background: var(--spectrum-global-color-blue-100);
+  }
   .noTitle {
     display: flex;
     align-items: center;
@@ -57,13 +68,7 @@
   }
   .title,
   .icon {
-    color: var(--spectrum-global-color-gray-600);
-  }
-  .info {
-    background-color: var(--background-alt);
-    padding: var(--spacing-m) var(--spacing-l) var(--spacing-m) var(--spacing-l);
-    border-radius: var(--border-radius-s);
-    font-size: 13px;
+    color: var(--spectrum-global-color-gray-700);
   }
   .quiet {
     background: none;
diff --git a/packages/builder/src/stores/builder/index.js b/packages/builder/src/stores/builder/index.js
index 158cd29973..08d87bebf5 100644
--- a/packages/builder/src/stores/builder/index.js
+++ b/packages/builder/src/stores/builder/index.js
@@ -30,6 +30,7 @@ import { queries } from "./queries"
 import { flags } from "./flags"
 import { rowActions } from "./rowActions"
 import componentTreeNodesStore from "./componentTreeNodes"
+import { appPublished } from "./published"
 
 export {
   componentTreeNodesStore,
@@ -65,6 +66,7 @@ export {
   hoverStore,
   snippets,
   rowActions,
+  appPublished,
 }
 
 export const reset = () => {
diff --git a/packages/builder/src/stores/builder/published.js b/packages/builder/src/stores/builder/published.js
new file mode 100644
index 0000000000..6ae9392f40
--- /dev/null
+++ b/packages/builder/src/stores/builder/published.js
@@ -0,0 +1,13 @@
+import { appStore } from "./app"
+import { appsStore } from "stores/portal/apps"
+import { deploymentStore } from "./deployments"
+import { derived } from "svelte/store"
+
+export const appPublished = derived(
+  [appStore, appsStore, deploymentStore],
+  ([$appStore, $appsStore, $deploymentStore]) => {
+    const app = $appsStore.apps.find(app => app.devId === $appStore.appId)
+    const deployments = $deploymentStore.filter(x => x.status === "SUCCESS")
+    return app?.status === "published" && deployments.length > 0
+  }
+)

From 2dec33c12d57d00a7c838d6fcd95d37b3ba30380 Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Thu, 24 Oct 2024 16:27:30 +0100
Subject: [PATCH 31/34] Revert info display changes

---
 .../_components/Component/InfoDisplay.svelte      | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
index 5543a4c7fd..93044cdb9a 100644
--- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
+++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/_components/Component/InfoDisplay.svelte
@@ -7,17 +7,9 @@
   export let quiet = false
   export let warning = false
   export let error = false
-  export let info = false
 </script>
 
-<div
-  class="container"
-  class:info
-  class:noTitle={!title}
-  class:warning
-  class:error
-  class:quiet
->
+<div class="info" class:noTitle={!title} class:warning class:error class:quiet>
   {#if title}
     <div class="title">
       <Icon name={icon} />
@@ -35,7 +27,7 @@
 </div>
 
 <style>
-  .container {
+  .info {
     padding: var(--spacing-m) var(--spacing-l);
     background-color: var(--spectrum-global-color-gray-200);
     border-radius: var(--border-radius-s);
@@ -47,9 +39,6 @@
   .error {
     background: rgba(255, 0, 0, 0.2);
   }
-  .info {
-    background: var(--spectrum-global-color-blue-100);
-  }
   .noTitle {
     display: flex;
     align-items: center;

From 226c8d4f8ec9cb9a3f5f17bb5f088810234b23a5 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 17:33:16 +0100
Subject: [PATCH 32/34] Fix SQL tests.

---
 packages/server/src/integrations/tests/sql.spec.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts
index 5a505fbb40..2b3649161c 100644
--- a/packages/server/src/integrations/tests/sql.spec.ts
+++ b/packages/server/src/integrations/tests/sql.spec.ts
@@ -212,7 +212,7 @@ describe("SQL query builder", () => {
     const filterSet = [`%20%`, `%25%`, `%"john"%`, `%"mary"%`]
     expect(query).toEqual({
       bindings: [...filterSet, limit],
-      sql: `select * from (select * from "test" where COALESCE(LOWER("test"."age"), '') LIKE :1 AND COALESCE(LOWER("test"."age"), '') LIKE :2 and COALESCE(LOWER("test"."name"), '') LIKE :3 AND COALESCE(LOWER("test"."name"), '') LIKE :4 order by "test"."id" asc) where rownum <= :5`,
+      sql: `select * from (select * from "test" where ((COALESCE(LOWER("test"."age"), '') like :1 and COALESCE(LOWER("test"."age"), '') like :2)) and ((COALESCE(LOWER("test"."name"), '') like :3 and COALESCE(LOWER("test"."name"), '') like :4)) order by "test"."id" asc) where rownum <= :5`,
     })
 
     query = new Sql(SqlClient.ORACLE, limit)._query(
@@ -244,7 +244,7 @@ describe("SQL query builder", () => {
 
     expect(query).toEqual({
       bindings: ["John", limit],
-      sql: `select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") = :1) order by "test"."id" asc) where rownum <= :2`,
+      sql: `select * from (select * from "test" where (to_char("test"."name") is not null and to_char("test"."name") = :1) order by "test"."id" asc) where rownum <= :2`,
     })
   })
 
@@ -262,7 +262,7 @@ describe("SQL query builder", () => {
 
     expect(query).toEqual({
       bindings: ["John", limit],
-      sql: `select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") != :1) OR to_char("test"."name") IS NULL order by "test"."id" asc) where rownum <= :2`,
+      sql: `select * from (select * from "test" where (to_char("test"."name") is not null and to_char("test"."name") != :1) or to_char("test"."name") is null order by "test"."id" asc) where rownum <= :2`,
     })
   })
 })

From dd6a0853a488b386a8f1777b88382cc576c01e94 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Thu, 24 Oct 2024 18:05:33 +0100
Subject: [PATCH 33/34] Fix tests (again)

---
 .../src/api/routes/tests/search.spec.ts       | 47 ++++++++++++++-----
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 1021139ed1..9cc1432180 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -164,12 +164,21 @@ describe.each([
     }
   }
 
-  async function assertTableExists(name: string) {
+  async function assertTableExists(nameOrTable: string | Table) {
+    const name =
+      typeof nameOrTable === "string" ? nameOrTable : nameOrTable.name
     expect(await client!.schema.hasTable(name)).toBeTrue()
   }
 
-  async function assertTableNumRows(name: string, numRows: number) {
-    expect(await client!.from(name).count()).toEqual([{ count: `${numRows}` }])
+  async function assertTableNumRows(
+    nameOrTable: string | Table,
+    numRows: number
+  ) {
+    const name =
+      typeof nameOrTable === "string" ? nameOrTable : nameOrTable.name
+    const row = await client!.from(name).count()
+    const count = parseInt(Object.values(row[0])[0] as string)
+    expect(count).toEqual(numRows)
   }
 
   describe.each([
@@ -3495,7 +3504,8 @@ describe.each([
       })
 
     isSql &&
-      describe("SQL injection", () => {
+      !isSqs &&
+      describe.only("SQL injection", () => {
         const badStrings = [
           "1; DROP TABLE %table_name%;",
           "1; DELETE FROM %table_name%;",
@@ -3530,14 +3540,25 @@ describe.each([
               await config.api.table.save({
                 ...table,
                 schema: {
+                  ...table.schema,
                   [badString]: { name: badString, type: FieldType.STRING },
                 },
               })
 
+              if (docIds.isViewId(tableOrViewId)) {
+                const view = await config.api.viewV2.get(tableOrViewId)
+                await config.api.viewV2.update({
+                  ...view,
+                  schema: {
+                    [badString]: { visible: true },
+                  },
+                })
+              }
+
               await config.api.row.save(tableOrViewId, { [badString]: "foo" })
 
-              await assertTableExists(table.name)
-              await assertTableNumRows(table.name, 1)
+              await assertTableExists(table)
+              await assertTableNumRows(table, 1)
 
               const { rows } = await config.api.row.search(
                 tableOrViewId,
@@ -3547,8 +3568,8 @@ describe.each([
 
               expect(rows).toHaveLength(1)
 
-              await assertTableExists(table.name)
-              await assertTableNumRows(table.name, 1)
+              await assertTableExists(table)
+              await assertTableNumRows(table, 1)
             })
 
           it("should not allow SQL injection as a field value", async () => {
@@ -3564,11 +3585,11 @@ describe.each([
               table.name
             )
 
-            await assertTableExists(table.name)
-            await assertTableNumRows(table.name, 1)
-
             await config.api.row.save(tableOrViewId, { foo: "foo" })
 
+            await assertTableExists(table)
+            await assertTableNumRows(table, 1)
+
             const { rows } = await config.api.row.search(
               tableOrViewId,
               { query: { equal: { foo: badString } } },
@@ -3576,8 +3597,8 @@ describe.each([
             )
 
             expect(rows).toBeEmpty()
-            await assertTableExists(table.name)
-            await assertTableNumRows(table.name, 1)
+            await assertTableExists(table)
+            await assertTableNumRows(table, 1)
           })
         })
       })

From 2b1bf4d711e7cb97a02e5284ba2f7413926c8986 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Fri, 25 Oct 2024 10:39:42 +0100
Subject: [PATCH 34/34] Fix lint.

---
 packages/server/src/api/routes/tests/search.spec.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts
index 9cc1432180..d7a7e09e90 100644
--- a/packages/server/src/api/routes/tests/search.spec.ts
+++ b/packages/server/src/api/routes/tests/search.spec.ts
@@ -3505,7 +3505,7 @@ describe.each([
 
     isSql &&
       !isSqs &&
-      describe.only("SQL injection", () => {
+      describe("SQL injection", () => {
         const badStrings = [
           "1; DROP TABLE %table_name%;",
           "1; DELETE FROM %table_name%;",