Merge pull request #14749 from Budibase/fix/budi-8715-sql-relationships-many-side

Fix SQL removing related row from many side

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

@@ -325,7 +325,7 @@ class InternalBuilder {
     return input
   }
 
-  private parseBody(body: any) {
+  private parseBody(body: Record<string, any>) {
     for (let [key, value] of Object.entries(body)) {
       const { column } = this.splitter.run(key)
       const schema = this.table.schema[column]
@@ -1259,6 +1259,10 @@ class InternalBuilder {
 
   create(opts: QueryOptions): Knex.QueryBuilder {
     const { body } = this.query
+    if (!body) {
+      throw new Error("Cannot create without row body")
+    }
+
     let query = this.qualifiedKnex({ alias: false })
     const parsedBody = this.parseBody(body)
 
@@ -1417,6 +1421,9 @@ class InternalBuilder {
 
   update(opts: QueryOptions): Knex.QueryBuilder {
     const { body, filters } = this.query
+    if (!body) {
+      throw new Error("Cannot update without row body")
+    }
     let query = this.qualifiedKnex()
     const parsedBody = this.parseBody(body)
     query = this.addFilters(query, filters)

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

@@ -4,6 +4,7 @@ import {
   AutoFieldSubType,
   AutoReason,
   Datasource,
+  DatasourcePlusQueryResponse,
   FieldSchema,
   FieldType,
   FilterType,
@@ -269,18 +270,13 @@ export class ExternalRequest<T extends Operation> {
     }
   }
 
-  private async removeManyToManyRelationships(
-    rowId: string,
-    table: Table,
-    colName: string
-  ) {
+  private async removeManyToManyRelationships(rowId: string, table: Table) {
     const tableId = table._id!
     const filters = this.prepareFilters(rowId, {}, table)
     // safety check, if there are no filters on deletion bad things happen
     if (Object.keys(filters).length !== 0) {
       return getDatasourceAndQuery({
         endpoint: getEndpoint(tableId, Operation.DELETE),
-        body: { [colName]: null },
         filters,
         meta: {
           table,
@@ -291,13 +287,18 @@ export class ExternalRequest<T extends Operation> {
     }
   }
 
-  private async removeOneToManyRelationships(rowId: string, table: Table) {
+  private async removeOneToManyRelationships(
+    rowId: string,
+    table: Table,
+    colName: string
+  ) {
     const tableId = table._id!
     const filters = this.prepareFilters(rowId, {}, table)
     // safety check, if there are no filters on deletion bad things happen
     if (Object.keys(filters).length !== 0) {
       return getDatasourceAndQuery({
         endpoint: getEndpoint(tableId, Operation.UPDATE),
+        body: { [colName]: null },
         filters,
         meta: {
           table,
@@ -557,8 +558,9 @@ export class ExternalRequest<T extends Operation> {
           return matchesPrimaryLink
         }
 
-        const matchesSecondayLink = row[linkSecondary] === body?.[linkSecondary]
-        return matchesPrimaryLink && matchesSecondayLink
+        const matchesSecondaryLink =
+          row[linkSecondary] === body?.[linkSecondary]
+        return matchesPrimaryLink && matchesSecondaryLink
       }
 
       const existingRelationship = rows.find((row: { [key: string]: any }) =>
@@ -595,8 +597,8 @@ export class ExternalRequest<T extends Operation> {
       for (let row of rows) {
         const rowId = generateIdForRow(row, table)
         const promise: Promise<any> = isMany
-          ? this.removeManyToManyRelationships(rowId, table, colName)
-          : this.removeOneToManyRelationships(rowId, table)
+          ? this.removeManyToManyRelationships(rowId, table)
+          : this.removeOneToManyRelationships(rowId, table, colName)
         if (promise) {
           promises.push(promise)
         }
@@ -619,12 +621,12 @@ export class ExternalRequest<T extends Operation> {
         rows.map(row => {
           const rowId = generateIdForRow(row, table)
           return isMany
-            ? this.removeManyToManyRelationships(
+            ? this.removeManyToManyRelationships(rowId, table)
+            : this.removeOneToManyRelationships(
                 rowId,
                 table,
                 relationshipColumn.fieldName
               )
-            : this.removeOneToManyRelationships(rowId, table)
         })
       )
     }
@@ -669,6 +671,7 @@ export class ExternalRequest<T extends Operation> {
       config.includeSqlRelationships === IncludeRelationship.INCLUDE
 
     // clean up row on ingress using schema
+    const unprocessedRow = config.row
     const processed = this.inputProcessing(row, table)
     row = processed.row
     let manyRelationships = processed.manyRelationships
@@ -743,9 +746,20 @@ export class ExternalRequest<T extends Operation> {
 
     // aliasing can be disabled fully if desired
     const aliasing = new sdk.rows.AliasTables(Object.keys(this.tables))
-    let response = env.SQL_ALIASING_DISABLE
-      ? await getDatasourceAndQuery(json)
-      : await aliasing.queryWithAliasing(json, makeExternalQuery)
+    let response: DatasourcePlusQueryResponse
+    // there's a chance after input processing nothing needs updated, so pass over the call
+    // we might still need to perform other operations like updating the foreign keys on other rows
+    if (
+      this.operation === Operation.UPDATE &&
+      Object.keys(row || {}).length === 0 &&
+      unprocessedRow
+    ) {
+      response = [unprocessedRow]
+    } else {
+      response = env.SQL_ALIASING_DISABLE
+        ? await getDatasourceAndQuery(json)
+        : await aliasing.queryWithAliasing(json, makeExternalQuery)
+    }
 
     // if it's a counting operation there will be no more processing, just return the number
     if (this.operation === Operation.COUNT) {

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

@@ -1114,6 +1114,33 @@ describe.each([
       expect(getResp.user2[0]._id).toEqual(user2._id)
     })
 
+    it("should be able to remove a relationship from many side", async () => {
+      const row = await config.api.row.save(otherTable._id!, {
+        name: "test",
+        description: "test",
+      })
+      const row2 = await config.api.row.save(otherTable._id!, {
+        name: "test",
+        description: "test",
+      })
+      const { _id } = await config.api.row.save(table._id!, {
+        relationship: [{ _id: row._id }, { _id: row2._id }],
+      })
+      const relatedRow = await config.api.row.get(table._id!, _id!, {
+        status: 200,
+      })
+      expect(relatedRow.relationship.length).toEqual(2)
+      await config.api.row.save(table._id!, {
+        ...relatedRow,
+        relationship: [{ _id: row._id }],
+      })
+      const afterRelatedRow = await config.api.row.get(table._id!, _id!, {
+        status: 200,
+      })
+      expect(afterRelatedRow.relationship.length).toEqual(1)
+      expect(afterRelatedRow.relationship[0]._id).toEqual(row._id)
+    })
+
     it("should be able to update relationships when both columns are same name", async () => {
       let row = await config.api.row.save(table._id!, {
         name: "test",