Merge pull request #2286 from Budibase/fix/relationships-2167

Some fixes for internal and external relationships

packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/CreateEditRelationship/CreateEditRelationship.svelte

@@ -199,6 +199,9 @@
       delete datasource.entities[toTable.name].schema[originalToName]
     }
 
+    // store the original names so it won't cause an error
+    originalToName = toRelationship.name
+    originalFromName = fromRelationship.name
     await save()
     await tables.fetch()
   }

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

@@ -25,10 +25,10 @@ interface ManyRelationship {
 
 interface RunConfig {
   id: string
-  row: Row
   filters: SearchFilters
   sort: SortJson
   paginate: PaginationJson
+  row: Row
 }
 
 module External {
@@ -89,8 +89,9 @@ module External {
     // build id array
     let idParts = []
     for (let field of primary) {
-      if (row[field]) {
-        idParts.push(row[field])
+      const fieldValue = row[`${table.name}.${field}`]
+      if (fieldValue) {
+        idParts.push(fieldValue)
       }
     }
     if (idParts.length === 0) {
@@ -115,7 +116,11 @@ module External {
     const thisRow: { [key: string]: any } = {}
     // filter the row down to what is actually the row (not joined)
     for (let fieldName of Object.keys(table.schema)) {
-      thisRow[fieldName] = row[fieldName]
+      const value = row[`${table.name}.${fieldName}`]
+      // all responses include "select col as table.col" so that overlaps are handled
+      if (value) {
+        thisRow[fieldName] = value
+      }
     }
     thisRow._id = generateIdForRow(row, table)
     thisRow.tableId = table._id
@@ -191,7 +196,7 @@ module External {
           const isUpdate = !field.through
           const thisKey: string = isUpdate ? "id" : linkTablePrimary
           // @ts-ignore
-          const otherKey: string = isUpdate ? field.foreignKey : tablePrimary
+          const otherKey: string = isUpdate ? field.fieldName : tablePrimary
           row[key].map((relationship: any) => {
             // we don't really support composite keys for relationships, this is why [0] is used
             manyRelationships.push({
@@ -359,7 +364,7 @@ module External {
         }
       }
       if (cache[fullKey] == null) {
-        cache[fullKey] = await makeExternalQuery(this.appId, {
+        const response = await makeExternalQuery(this.appId, {
           endpoint: getEndpoint(tableId, DataSourceOperation.READ),
           filters: {
             equal: {
@@ -367,8 +372,12 @@ module External {
             },
           },
         })
+        // this is the response from knex if no rows found
+        if (!response[0].read) {
+          cache[fullKey] = response
+        }
       }
-      return { rows: cache[fullKey], table }
+      return { rows: cache[fullKey] || [], table }
     }
 
     /**
@@ -418,12 +427,16 @@ module External {
         const { tableName } = breakExternalTableId(tableId)
         const table = this.tables[tableName]
         for (let row of rows) {
-          promises.push(
-            makeExternalQuery(this.appId, {
-              endpoint: getEndpoint(tableId, DataSourceOperation.DELETE),
-              filters: buildFilters(generateIdForRow(row, table), {}, table),
-            })
-          )
+          const filters = buildFilters(generateIdForRow(row, table), {}, table)
+          // safety check, if there are no filters on deletion bad things happen
+          if (Object.keys(filters).length !== 0) {
+            promises.push(
+              makeExternalQuery(this.appId, {
+                endpoint: getEndpoint(tableId, DataSourceOperation.DELETE),
+                filters,
+              })
+            )
+          }
         }
       }
       await Promise.all(promises)
@@ -442,7 +455,7 @@ module External {
           .filter(
             column =>
               column[1].type !== FieldTypes.LINK &&
-              !existing.find((field: string) => field.includes(column[0]))
+              !existing.find((field: string) => field === column[0])
           )
           .map(column => `${table.name}.${column[0]}`)
       }

packages/server/src/api/routes/tests/datasource.spec.js

@@ -82,7 +82,7 @@ describe("/datasources", () => {
             entityId: "users",
           },
           resource: {
-            fields: ["name", "age"],
+            fields: ["users.name", "users.age"],
           },
           filters: {
             string: {
@@ -94,7 +94,7 @@ describe("/datasources", () => {
         .expect(200)
       // this is mock data, can't test it
       expect(res.body).toBeDefined()
-      expect(pg.queryMock).toHaveBeenCalledWith(`select "name", "age" from "users" where "users"."name" like $1 limit $2`, ["John%", 5000])
+      expect(pg.queryMock).toHaveBeenCalledWith(`select "users"."name" as "users.name", "users"."age" as "users.age" from "users" where "users"."name" like $1 limit $2`, ["John%", 5000])
     })
   })
 

packages/server/src/db/linkedRows/LinkController.js

@@ -322,7 +322,7 @@ class LinkController {
     // remove schema from other table
     let linkedTable = await this._db.get(field.tableId)
     delete linkedTable.schema[field.fieldName]
-    this._db.put(linkedTable)
+    await this._db.put(linkedTable)
   }
 
   /**

packages/server/src/integrations/base/sql.ts

@@ -151,7 +151,9 @@ function buildRead(knex: Knex, json: QueryJson, limit: number): KnexQuery {
   }
   // handle select
   if (resource.fields && resource.fields.length > 0) {
-    query = query.select(resource.fields)
+    // select the resources as the format "table.columnName" - this is what is provided
+    // by the resource builder further up
+    query = query.select(resource.fields.map(field => `${field} as ${field}`))
   } else {
     query = query.select("*")
   }

packages/server/src/integrations/mysql.ts

@@ -242,7 +242,7 @@ module MySQLModule {
       const input = this._query(json, { disableReturning: true })
       let row
       // need to manage returning, a feature mySQL can't do
-      if (operation === "awdawd") {
+      if (operation === operation.DELETE) {
         row = this.getReturningRow(json)
       }
       const results = await internalQuery(this.client, input, false)

packages/server/src/integrations/tests/sql.spec.js

@@ -62,12 +62,13 @@ describe("SQL query builder", () => {
   })
 
   it("should test a read with specific columns", () => {
+    const nameProp = `${TABLE_NAME}.name`, ageProp = `${TABLE_NAME}.age`
     const query = sql._query(generateReadJson({
-      fields: ["name", "age"]
+      fields: [nameProp, ageProp]
     }))
     expect(query).toEqual({
       bindings: [limit],
-      sql: `select "name", "age" from "${TABLE_NAME}" limit $1`
+      sql: `select "${TABLE_NAME}"."name" as "${nameProp}", "${TABLE_NAME}"."age" as "${ageProp}" from "${TABLE_NAME}" limit $1`
     })
   })
 

packages/server/src/integrations/utils.ts

@@ -40,8 +40,13 @@ export function breakRowIdField(_id: string): any[] {
   // have to replace on the way back as we swapped out the double quotes
   // when encoding, but JSON can't handle the single quotes
   const decoded: string = decodeURIComponent(_id).replace(/'/g, '"')
-  const parsed = JSON.parse(decoded)
-  return Array.isArray(parsed) ? parsed : [parsed]
+  try {
+    const parsed = JSON.parse(decoded)
+    return Array.isArray(parsed) ? parsed : [parsed]
+  } catch (err) {
+    // wasn't json - likely was handlebars for a many to many
+    return [_id]
+  }
 }
 
 export function convertType(type: string, map: { [key: string]: any }) {