Merge pull request #14749 from Budibase/fix/budi-8715-sql-relationshi…

…ps-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",