Merge pull request #11934 from Budibase/chore/relation_deletions

Fix deleting one-to-many relation in SQL

packages/server/src/api/controllers/row/ExternalRequest.ts

@@ -340,10 +340,16 @@ export class ExternalRequest<T extends Operation> {
       // one to many
       if (isOneSide(field)) {
         let id = row[key][0]
-        if (typeof row[key] === "string") {
+        if (id) {
-          id = decodeURIComponent(row[key]).match(/\[(.*?)\]/)?.[1]
+          if (typeof row[key] === "string") {
+            id = decodeURIComponent(row[key]).match(/\[(.*?)\]/)?.[1]
+          }
+          newRow[field.foreignKey || linkTablePrimary] = breakRowIdField(id)[0]
+        } else {
+          // Removing from both new and row, as we don't know if it has already been processed
+          row[field.foreignKey || linkTablePrimary] = null
+          newRow[field.foreignKey || linkTablePrimary] = null
         }
-        newRow[field.foreignKey || linkTablePrimary] = breakRowIdField(id)[0]
       }
       // many to many
       else if (field.through) {
@@ -830,10 +836,7 @@ export class ExternalRequest<T extends Operation> {
     // can't really use response right now
     const response = await getDatasourceAndQuery(json)
     // handle many to many relationships now if we know the ID (could be auto increment)
-    if (
+    if (operation !== Operation.READ) {
-      operation !== Operation.READ &&
-      processed.manyRelationships?.length > 0
-    ) {
       await this.handleManyRelationships(
         table._id || "",
         response[0],

packages/server/src/api/controllers/row/external.ts

@@ -108,13 +108,11 @@ export async function save(ctx: UserCtx) {
     row,
   })
 
-  const responseRow = response as { row: Row }
-
   if (!isEqual(table, updatedTable)) {
     await sdk.tables.saveTable(updatedTable)
   }
 
-  const rowId = responseRow.row._id
+  const rowId = response.row._id
   if (rowId) {
     const row = await sdk.rows.external.getRow(tableId, rowId, {
       relationships: true,

packages/server/src/api/controllers/table/index.ts

@@ -14,7 +14,6 @@ import {
   Table,
   TableResponse,
   UserCtx,
-  Datasource,
 } from "@budibase/types"
 import sdk from "../../../sdk"
 import { jsonFromCsvString } from "../../../utilities/csv"

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

@@ -18,7 +18,6 @@ import {
   SortType,
   StaticQuotaName,
   Table,
-  User,
 } from "@budibase/types"
 import {
   expectAnyExternalColsAttributes,
@@ -1515,9 +1514,82 @@ describe.each([
     })
   })
 
-  describe("bb reference fields", () => {
+  let o2mTable: Table
+  let m2mTable: Table
+  beforeAll(async () => {
+    o2mTable = await config.createTable(
+      { ...generateTableConfig(), name: "o2m" },
+      {
+        skipReassigning: true,
+      }
+    )
+    m2mTable = await config.createTable(
+      { ...generateTableConfig(), name: "m2m" },
+      {
+        skipReassigning: true,
+      }
+    )
+  })
+
+  describe.each([
+    [
+      "relationship fields",
+      () => ({
+        user: {
+          name: "user",
+          relationshipType: RelationshipType.ONE_TO_MANY,
+          type: FieldType.LINK,
+          tableId: o2mTable._id!,
+          fieldName: "fk_o2m",
+        },
+        users: {
+          name: "users",
+          relationshipType: RelationshipType.MANY_TO_MANY,
+          type: FieldType.LINK,
+          tableId: m2mTable._id!,
+          fieldName: "fk_m2m",
+        },
+      }),
+      (tableId: string) =>
+        config.api.row.save(tableId, {
+          name: generator.word(),
+          description: generator.paragraph(),
+          tableId,
+        }),
+      (row: Row) => ({
+        _id: row._id,
+        primaryDisplay: row.name,
+      }),
+    ],
+    [
+      "bb reference fields",
+      () => ({
+        user: {
+          name: "user",
+          relationshipType: RelationshipType.ONE_TO_MANY,
+          type: FieldType.BB_REFERENCE,
+          subtype: FieldTypeSubtypes.BB_REFERENCE.USER,
+        },
+        users: {
+          name: "users",
+          type: FieldType.BB_REFERENCE,
+          subtype: FieldTypeSubtypes.BB_REFERENCE.USER,
+          relationshipType: RelationshipType.MANY_TO_MANY,
+        },
+      }),
+      () => config.createUser(),
+      (row: Row) => ({
+        _id: row._id,
+        email: row.email,
+        firstName: row.firstName,
+        lastName: row.lastName,
+        primaryDisplay: row.email,
+      }),
+    ],
+  ])("links - %s", (__, relSchema, dataGenerator, resultMapper) => {
     let tableId: string
-    let users: User[]
+    let o2mData: Row[]
+    let m2mData: Row[]
 
     beforeAll(async () => {
       const tableConfig = generateTableConfig()
@@ -1532,31 +1604,27 @@ describe.each([
         ...tableConfig,
         schema: {
           ...tableConfig.schema,
-          user: {
+          ...relSchema(),
-            name: "user",
-            type: FieldType.BB_REFERENCE,
-            subtype: FieldTypeSubtypes.BB_REFERENCE.USER,
-            relationshipType: RelationshipType.ONE_TO_MANY,
-          },
-          users: {
-            name: "users",
-            type: FieldType.BB_REFERENCE,
-            subtype: FieldTypeSubtypes.BB_REFERENCE.USER,
-            relationshipType: RelationshipType.MANY_TO_MANY,
-          },
         },
       })
       tableId = table._id!
 
-      users = [
+      o2mData = [
-        await config.createUser(),
+        await dataGenerator(o2mTable._id!),
-        await config.createUser(),
+        await dataGenerator(o2mTable._id!),
-        await config.createUser(),
+        await dataGenerator(o2mTable._id!),
-        await config.createUser(),
+        await dataGenerator(o2mTable._id!),
+      ]
+
+      m2mData = [
+        await dataGenerator(m2mTable._id!),
+        await dataGenerator(m2mTable._id!),
+        await dataGenerator(m2mTable._id!),
+        await dataGenerator(m2mTable._id!),
       ]
     })
 
-    it("can save a row when BB reference fields are empty", async () => {
+    it("can save a row when relationship fields are empty", async () => {
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
@@ -1575,13 +1643,13 @@ describe.each([
       })
     })
 
-    it("can save a row with a single BB reference field", async () => {
+    it("can save a row with a single relationship field", async () => {
-      const user = _.sample(users)!
+      const user = _.sample(o2mData)!
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
         description: generator.name(),
-        user: user,
+        user: [user],
       }
       const row = await config.api.row.save(tableId, rowData)
 
@@ -1589,24 +1657,17 @@ describe.each([
         name: rowData.name,
         description: rowData.description,
         tableId,
-        user: [
+        user: [user].map(u => resultMapper(u)),
-          {
-            _id: user._id,
-            email: user.email,
-            firstName: user.firstName,
-            lastName: user.lastName,
-            primaryDisplay: user.email,
-          },
-        ],
         _id: expect.any(String),
         _rev: expect.any(String),
         id: isInternal ? undefined : expect.any(Number),
         type: isInternal ? "row" : undefined,
+        [`fk_${o2mTable.name}_fk_o2m`]: isInternal ? undefined : user.id,
       })
     })
 
-    it("can save a row with a multiple BB reference field", async () => {
+    it("can save a row with a multiple relationship field", async () => {
-      const selectedUsers = _.sampleSize(users, 2)
+      const selectedUsers = _.sampleSize(m2mData, 2)
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
@@ -1619,13 +1680,7 @@ describe.each([
         name: rowData.name,
         description: rowData.description,
         tableId,
-        users: selectedUsers.map(u => ({
+        users: expect.arrayContaining(selectedUsers.map(u => resultMapper(u))),
-          _id: u._id,
-          email: u.email,
-          firstName: u.firstName,
-          lastName: u.lastName,
-          primaryDisplay: u.email,
-        })),
         _id: expect.any(String),
         _rev: expect.any(String),
         id: isInternal ? undefined : expect.any(Number),
@@ -1633,7 +1688,7 @@ describe.each([
       })
     })
 
-    it("can retrieve rows with no populated BB references", async () => {
+    it("can retrieve rows with no populated relationships", async () => {
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
@@ -1655,14 +1710,15 @@ describe.each([
       })
     })
 
-    it("can retrieve rows with populated BB references", async () => {
+    it("can retrieve rows with populated relationships", async () => {
-      const [user1, user2] = _.sampleSize(users, 2)
+      const user1 = _.sample(o2mData)!
+      const [user2, user3] = _.sampleSize(m2mData, 2)
 
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
         description: generator.name(),
-        users: [user1, user2],
+        users: [user2, user3],
         user: [user1],
       }
       const row = await config.api.row.save(tableId, rowData)
@@ -1672,72 +1728,51 @@ describe.each([
         name: rowData.name,
         description: rowData.description,
         tableId,
-        user: [user1].map(u => ({
+        user: expect.arrayContaining([user1].map(u => resultMapper(u))),
-          _id: u._id,
+        users: expect.arrayContaining([user2, user3].map(u => resultMapper(u))),
-          email: u.email,
-          firstName: u.firstName,
-          lastName: u.lastName,
-          primaryDisplay: u.email,
-        })),
-        users: [user1, user2].map(u => ({
-          _id: u._id,
-          email: u.email,
-          firstName: u.firstName,
-          lastName: u.lastName,
-          primaryDisplay: u.email,
-        })),
         _id: row._id,
         _rev: expect.any(String),
         id: isInternal ? undefined : expect.any(Number),
+        [`fk_${o2mTable.name}_fk_o2m`]: isInternal ? undefined : user1.id,
         ...defaultRowFields,
       })
     })
 
     it("can update an existing populated row", async () => {
-      const [user1, user2, user3] = _.sampleSize(users, 3)
+      const user = _.sample(o2mData)!
+      const [users1, users2, users3] = _.sampleSize(m2mData, 3)
 
       const rowData = {
         ...basicRow(tableId),
         name: generator.name(),
         description: generator.name(),
-        users: [user1, user2],
+        users: [users1, users2],
       }
       const row = await config.api.row.save(tableId, rowData)
 
       const updatedRow = await config.api.row.save(tableId, {
         ...row,
-        user: [user3],
+        user: [user],
-        users: [user3, user2],
+        users: [users3, users1],
       })
       expect(updatedRow).toEqual({
         name: rowData.name,
         description: rowData.description,
         tableId,
-        user: [
+        user: expect.arrayContaining([user].map(u => resultMapper(u))),
-          {
+        users: expect.arrayContaining(
-            _id: user3._id,
+          [users3, users1].map(u => resultMapper(u))
-            email: user3.email,
+        ),
-            firstName: user3.firstName,
-            lastName: user3.lastName,
-            primaryDisplay: user3.email,
-          },
-        ],
-        users: [user3, user2].map(u => ({
-          _id: u._id,
-          email: u.email,
-          firstName: u.firstName,
-          lastName: u.lastName,
-          primaryDisplay: u.email,
-        })),
         _id: row._id,
         _rev: expect.any(String),
         id: isInternal ? undefined : expect.any(Number),
         type: isInternal ? "row" : undefined,
+        [`fk_${o2mTable.name}_fk_o2m`]: isInternal ? undefined : user.id,
       })
     })
 
-    it("can wipe an existing populated BB references in row", async () => {
+    it("can wipe an existing populated relationships in row", async () => {
-      const [user1, user2] = _.sampleSize(users, 2)
+      const [user1, user2] = _.sampleSize(m2mData, 2)
 
       const rowData = {
         ...basicRow(tableId),
@@ -1756,8 +1791,6 @@ describe.each([
         name: rowData.name,
         description: rowData.description,
         tableId,
-        user: isInternal ? null : undefined,
-        users: isInternal ? null : undefined,
         _id: row._id,
         _rev: expect.any(String),
         id: isInternal ? undefined : expect.any(Number),
@@ -1765,34 +1798,35 @@ describe.each([
       })
     })
 
-    it("fetch all will populate the BB references", async () => {
+    it("fetch all will populate the relationships", async () => {
-      const [user1, user2, user3] = _.sampleSize(users, 3)
+      const [user1] = _.sampleSize(o2mData, 1)
+      const [users1, users2, users3] = _.sampleSize(m2mData, 3)
 
       const rows: {
         name: string
         description: string
-        user?: User[]
+        user?: Row[]
-        users?: User[]
+        users?: Row[]
         tableId: string
       }[] = [
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
-          users: [user1, user2],
+          users: [users1, users2],
         },
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
           user: [user1],
-          users: [user1, user3],
+          users: [users1, users3],
         },
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
-          users: [user3],
+          users: [users3],
         },
       ]
 
@@ -1808,57 +1842,50 @@ describe.each([
             name: r.name,
             description: r.description,
             tableId,
-            user: r.user?.map(u => ({
+            user: r.user?.map(u => resultMapper(u)),
-              _id: u._id,
+            users: r.users?.length
-              email: u.email,
+              ? expect.arrayContaining(r.users?.map(u => resultMapper(u)))
-              firstName: u.firstName,
+              : undefined,
-              lastName: u.lastName,
-              primaryDisplay: u.email,
-            })),
-            users: r.users?.map(u => ({
-              _id: u._id,
-              email: u.email,
-              firstName: u.firstName,
-              lastName: u.lastName,
-              primaryDisplay: u.email,
-            })),
             _id: expect.any(String),
             _rev: expect.any(String),
             id: isInternal ? undefined : expect.any(Number),
+            [`fk_${o2mTable.name}_fk_o2m`]:
+              isInternal || !r.user?.length ? undefined : r.user[0].id,
             ...defaultRowFields,
           }))
         )
       )
     })
 
-    it("search all will populate the BB references", async () => {
+    it("search all will populate the relationships", async () => {
-      const [user1, user2, user3] = _.sampleSize(users, 3)
+      const [user1] = _.sampleSize(o2mData, 1)
+      const [users1, users2, users3] = _.sampleSize(m2mData, 3)
 
       const rows: {
         name: string
         description: string
-        user?: User[]
+        user?: Row[]
-        users?: User[]
+        users?: Row[]
         tableId: string
       }[] = [
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
-          users: [user1, user2],
+          users: [users1, users2],
         },
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
           user: [user1],
-          users: [user1, user3],
+          users: [users1, users3],
         },
         {
           ...basicRow(tableId),
           name: generator.name(),
           description: generator.name(),
-          users: [user3],
+          users: [users3],
         },
       ]
 
@@ -1874,23 +1901,15 @@ describe.each([
             name: r.name,
             description: r.description,
             tableId,
-            user: r.user?.map(u => ({
+            user: r.user?.map(u => resultMapper(u)),
-              _id: u._id,
+            users: r.users?.length
-              email: u.email,
+              ? expect.arrayContaining(r.users?.map(u => resultMapper(u)))
-              firstName: u.firstName,
+              : undefined,
-              lastName: u.lastName,
-              primaryDisplay: u.email,
-            })),
-            users: r.users?.map(u => ({
-              _id: u._id,
-              email: u.email,
-              firstName: u.firstName,
-              lastName: u.lastName,
-              primaryDisplay: u.email,
-            })),
             _id: expect.any(String),
             _rev: expect.any(String),
             id: isInternal ? undefined : expect.any(Number),
+            [`fk_${o2mTable.name}_fk_o2m`]:
+              isInternal || !r.user?.length ? undefined : r.user[0].id,
             ...defaultRowFields,
           }))
         ),

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

@@ -48,7 +48,7 @@ export async function processOutputBBReferences(
 ) {
   if (typeof value !== "string") {
     // Already processed or nothing to process
-    return value
+    return value || undefined
   }
 
   const ids = value.split(",").filter(id => !!id)

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

@@ -11,6 +11,7 @@ import {
   processInputBBReferences,
   processOutputBBReferences,
 } from "./bbReferenceProcessor"
+import { isExternalTable } from "../../integrations/utils"
 export * from "./utils"
 
 type AutoColumnProcessingOpts = {
@@ -234,9 +235,6 @@ export async function outputProcessing<T extends Row[] | Row>(
       }
     } else if (column.type == FieldTypes.BB_REFERENCE) {
       for (let row of enriched) {
-        if (row[property] == null) {
-          continue
-        }
         row[property] = await processOutputBBReferences(
           row[property],
           column.subtype as FieldSubtype
@@ -250,6 +248,16 @@ export async function outputProcessing<T extends Row[] | Row>(
       enriched
     )) as Row[]
   }
+  // remove null properties to match internal API
+  if (isExternalTable(table._id!)) {
+    for (let row of enriched) {
+      for (let key of Object.keys(row)) {
+        if (row[key] === null) {
+          delete row[key]
+        }
+      }
+    }
+  }
   return (wasArray ? enriched : enriched[0]) as T
 }
 

packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts

@@ -66,7 +66,7 @@ describe("rowProcessor - outputProcessing", () => {
     )
   })
 
-  it("does not fetch bb references when fields are empty", async () => {
+  it("process output even when the field is not empty", async () => {
     const table: Table = {
       _id: generator.guid(),
       name: "TestTable",
@@ -100,7 +100,7 @@ describe("rowProcessor - outputProcessing", () => {
 
     expect(result).toEqual({ name: "Jack" })
 
-    expect(bbReferenceProcessor.processOutputBBReferences).not.toBeCalled()
+    expect(bbReferenceProcessor.processOutputBBReferences).toBeCalledTimes(1)
   })
 
   it("does not fetch bb references when not in the schema", async () => {