Merge pull request #14610 from Budibase/budi-8637-googlesheets-issues-automations-row-actions-and-filtering-1

`fetchSchema` tests for Google Sheets integration

packages/server/package.json

@@ -80,7 +80,7 @@
     "dotenv": "8.2.0",
     "form-data": "4.0.0",
     "global-agent": "3.0.0",
-    "google-spreadsheet": "npm:@budibase/google-spreadsheet@4.1.3",
+    "google-spreadsheet": "npm:@budibase/google-spreadsheet@4.1.5",
     "ioredis": "5.3.2",
     "isolated-vm": "^4.7.2",
     "jimp": "0.22.12",

packages/server/src/integrations/googlesheets.ts

@@ -330,15 +330,16 @@ export class GoogleSheetsIntegration implements DatasourcePlus {
       return { tables: {}, errors: {} }
     }
     await this.connect()
+
     const sheets = this.client.sheetsByIndex
     const tables: Record<string, Table> = {}
     let errors: Record<string, string> = {}
+
     await utils.parallelForeach(
       sheets,
       async sheet => {
-        // must fetch rows to determine schema
         try {
-          await sheet.getRows()
+          await sheet.getRows({ limit: 1 })
         } catch (err) {
           // We expect this to always be an Error so if it's not, rethrow it to
           // make sure we don't fail quietly.
@@ -346,16 +347,25 @@ export class GoogleSheetsIntegration implements DatasourcePlus {
             throw err
           }
 
-          if (err.message.startsWith("No values in the header row")) {
+          if (
-            errors[sheet.title] = err.message
+            err.message.startsWith("No values in the header row") ||
-          } else {
+            err.message.startsWith("All your header cells are blank")
+          ) {
+            errors[
+              sheet.title
+            ] = `Failed to find a header row in sheet "${sheet.title}", is the first row blank?`
+            return
+          }
+
           // If we get an error we don't expect, rethrow to avoid failing
           // quietly.
           throw err
         }
-          return
+      },
-        }
+      10
+    )
 
+    for (const sheet of sheets) {
       const id = buildExternalTableId(datasourceId, sheet.title)
       tables[sheet.title] = this.getTableSchema(
         sheet.title,
@@ -363,9 +373,8 @@ export class GoogleSheetsIntegration implements DatasourcePlus {
         datasourceId,
         id
       )
-      },
+    }
-      10
+
-    )
     let externalTables = finaliseExternalTables(tables, entities)
     errors = { ...errors, ...checkExternalTables(externalTables) }
     return { tables: externalTables, errors }

packages/server/src/integrations/tests/googlesheets.spec.ts

@@ -244,6 +244,20 @@ describe("Google Sheets Integration", () => {
         expect.arrayContaining(Array.from({ length: 248 }, (_, i) => `${i}`))
       )
     })
+
+    it("can export rows", async () => {
+      const resp = await config.api.row.exportRows(table._id!, {})
+      const parsed = JSON.parse(resp)
+      expect(parsed.length).toEqual(2)
+      expect(parsed[0]).toMatchObject({
+        name: "Test Contact 1",
+        description: "original description 1",
+      })
+      expect(parsed[1]).toMatchObject({
+        name: "Test Contact 2",
+        description: "original description 2",
+      })
+    })
   })
 
   describe("update", () => {
@@ -491,4 +505,97 @@ describe("Google Sheets Integration", () => {
       expect(emptyRows.length).toEqual(0)
     })
   })
+
+  describe("fetch schema", () => {
+    it("should fail to import a completely blank sheet", async () => {
+      mock.createSheet({ title: "Sheet1" })
+      await config.api.datasource.fetchSchema(
+        {
+          datasourceId: datasource._id!,
+          tablesFilter: ["Sheet1"],
+        },
+        {
+          status: 200,
+          body: {
+            errors: {
+              Sheet1:
+                'Failed to find a header row in sheet "Sheet1", is the first row blank?',
+            },
+          },
+        }
+      )
+    })
+
+    it("should fail to import multiple sheets with blank headers", async () => {
+      mock.createSheet({ title: "Sheet1" })
+      mock.createSheet({ title: "Sheet2" })
+
+      await config.api.datasource.fetchSchema(
+        {
+          datasourceId: datasource!._id!,
+          tablesFilter: ["Sheet1", "Sheet2"],
+        },
+        {
+          status: 200,
+          body: {
+            errors: {
+              Sheet1:
+                'Failed to find a header row in sheet "Sheet1", is the first row blank?',
+              Sheet2:
+                'Failed to find a header row in sheet "Sheet2", is the first row blank?',
+            },
+          },
+        }
+      )
+    })
+
+    it("should only fail the sheet with missing headers", async () => {
+      mock.createSheet({ title: "Sheet1" })
+      mock.createSheet({ title: "Sheet2" })
+      mock.createSheet({ title: "Sheet3" })
+
+      mock.set("Sheet1!A1", "name")
+      mock.set("Sheet1!B1", "dob")
+      mock.set("Sheet2!A1", "name")
+      mock.set("Sheet2!B1", "dob")
+
+      await config.api.datasource.fetchSchema(
+        {
+          datasourceId: datasource!._id!,
+          tablesFilter: ["Sheet1", "Sheet2", "Sheet3"],
+        },
+        {
+          status: 200,
+          body: {
+            errors: {
+              Sheet3:
+                'Failed to find a header row in sheet "Sheet3", is the first row blank?',
+            },
+          },
+        }
+      )
+    })
+
+    it("should only succeed if sheet with missing headers is not being imported", async () => {
+      mock.createSheet({ title: "Sheet1" })
+      mock.createSheet({ title: "Sheet2" })
+      mock.createSheet({ title: "Sheet3" })
+
+      mock.set("Sheet1!A1", "name")
+      mock.set("Sheet1!B1", "dob")
+      mock.set("Sheet2!A1", "name")
+      mock.set("Sheet2!B1", "dob")
+
+      await config.api.datasource.fetchSchema(
+        {
+          datasourceId: datasource!._id!,
+          tablesFilter: ["Sheet1", "Sheet2"],
+        },
+        {
+          status: 200,
+          body: { errors: {} },
+        }
+      )
+    })
+  })
 })

packages/server/src/integrations/tests/utils/googlesheets.ts

@@ -22,6 +22,7 @@ import type {
   CellPadding,
   Color,
   GridRange,
+  DataSourceSheetProperties,
 } from "google-spreadsheet/src/lib/types/sheets-types"
 
 const BLACK: Color = { red: 0, green: 0, blue: 0 }
@@ -91,7 +92,7 @@ interface UpdateValuesResponse {
 
 // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/request#AddSheetRequest
 interface AddSheetRequest {
-  properties: WorksheetProperties
+  properties: Partial<WorksheetProperties>
 }
 
 // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response#AddSheetResponse
@@ -236,6 +237,38 @@ export class GoogleSheetsMock {
     this.mockAPI()
   }
 
+  public cell(cell: string): Value | undefined {
+    const cellData = this.cellData(cell)
+    if (!cellData) {
+      return undefined
+    }
+    return this.cellValue(cellData)
+  }
+
+  public set(cell: string, value: Value): void {
+    const cellData = this.cellData(cell)
+    if (!cellData) {
+      throw new Error(`Cell ${cell} not found`)
+    }
+    cellData.userEnteredValue = this.createValue(value)
+  }
+
+  public sheet(name: string | number): Sheet | undefined {
+    if (typeof name === "number") {
+      return this.getSheetById(name)
+    }
+    return this.getSheetByName(name)
+  }
+
+  public createSheet(opts: Partial<WorksheetProperties>): Sheet {
+    const properties = this.defaultWorksheetProperties(opts)
+    if (this.getSheetByName(properties.title)) {
+      throw new Error(`Sheet ${properties.title} already exists`)
+    }
+    const resp = this.handleAddSheet({ properties })
+    return this.getSheetById(resp.properties.sheetId)!
+  }
+
   private route(
     method: "get" | "put" | "post",
     path: string | RegExp,
@@ -462,35 +495,39 @@ export class GoogleSheetsMock {
     return response
   }
 
-  private handleAddSheet(request: AddSheetRequest): AddSheetResponse {
+  private defaultWorksheetProperties(
-    const properties: Omit<WorksheetProperties, "dataSourceSheetProperties"> = {
+    opts: Partial<WorksheetProperties>
+  ): WorksheetProperties {
+    return {
       index: this.spreadsheet.sheets.length,
       hidden: false,
       rightToLeft: false,
       tabColor: BLACK,
       tabColorStyle: { rgbColor: BLACK },
       sheetType: "GRID",
-      title: request.properties.title,
+      title: "Sheet",
       sheetId: this.spreadsheet.sheets.length,
       gridProperties: {
         rowCount: 100,
         columnCount: 26,
-        frozenRowCount: 0,
-        frozenColumnCount: 0,
-        hideGridlines: false,
-        rowGroupControlAfter: false,
-        columnGroupControlAfter: false,
       },
+      dataSourceSheetProperties: {} as DataSourceSheetProperties,
+      ...opts,
+    }
   }
 
+  private handleAddSheet(request: AddSheetRequest): AddSheetResponse {
+    const properties = this.defaultWorksheetProperties(request.properties)
     this.spreadsheet.sheets.push({
-      properties: properties as WorksheetProperties,
+      properties,
-      data: [this.createEmptyGrid(100, 26)],
+      data: [
+        this.createEmptyGrid(
+          properties.gridProperties.rowCount,
+          properties.gridProperties.columnCount
+        ),
+      ],
     })
-
+    return { properties }
-    // dataSourceSheetProperties is only returned by the API if the sheet type is
-    // DATA_SOURCE, which we aren't using, so sadly we need to cast here.
-    return { properties: properties as WorksheetProperties }
   }
 
   private handleDeleteRange(request: DeleteRangeRequest) {
@@ -767,21 +804,6 @@ export class GoogleSheetsMock {
     return this.getCellNumericIndexes(sheetId, startRowIndex, startColumnIndex)
   }
 
-  public cell(cell: string): Value | undefined {
-    const cellData = this.cellData(cell)
-    if (!cellData) {
-      return undefined
-    }
-    return this.cellValue(cellData)
-  }
-
-  public sheet(name: string | number): Sheet | undefined {
-    if (typeof name === "number") {
-      return this.getSheetById(name)
-    }
-    return this.getSheetByName(name)
-  }
-
   private getCellNumericIndexes(
     sheet: Sheet | number,
     row: number,

yarn.lock

@@ -2053,6 +2053,44 @@
   resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39"
   integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==
 
+"@budibase/backend-core@2.32.6":
+  version "0.0.0"
+  dependencies:
+    "@budibase/nano" "10.1.5"
+    "@budibase/pouchdb-replication-stream" "1.2.11"
+    "@budibase/shared-core" "0.0.0"
+    "@budibase/types" "0.0.0"
+    aws-cloudfront-sign "3.0.2"
+    aws-sdk "2.1030.0"
+    bcrypt "5.1.0"
+    bcryptjs "2.4.3"
+    bull "4.10.1"
+    correlation-id "4.0.0"
+    dd-trace "5.2.0"
+    dotenv "16.0.1"
+    ioredis "5.3.2"
+    joi "17.6.0"
+    jsonwebtoken "9.0.2"
+    knex "2.4.2"
+    koa-passport "^6.0.0"
+    koa-pino-logger "4.0.0"
+    lodash "4.17.21"
+    node-fetch "2.6.7"
+    passport-google-oauth "2.0.0"
+    passport-local "1.0.0"
+    passport-oauth2-refresh "^2.1.0"
+    pino "8.11.0"
+    pino-http "8.3.3"
+    posthog-node "4.0.1"
+    pouchdb "7.3.0"
+    pouchdb-find "7.2.2"
+    redlock "4.2.0"
+    rotating-file-stream "3.1.0"
+    sanitize-s3-objectkey "0.0.1"
+    semver "^7.5.4"
+    tar-fs "2.1.1"
+    uuid "^8.3.2"
+
 "@budibase/handlebars-helpers@^0.13.2":
   version "0.13.2"
   resolved "https://registry.yarnpkg.com/@budibase/handlebars-helpers/-/handlebars-helpers-0.13.2.tgz#73ab51c464e91fd955b429017648e0257060db77"
@@ -2095,6 +2133,45 @@
     pouchdb-promise "^6.0.4"
     through2 "^2.0.0"
 
+"@budibase/pro@npm:@budibase/pro@latest":
+  version "2.32.6"
+  resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-2.32.6.tgz#02ddef737ee8f52dafd8fab8f8f277dfc89cd33f"
+  integrity sha512-+XEv4JtMvUKZWyllcw+iFOh44zxsoJLmUdShu4bAjj5zXWgElF6LjFpK51IrQzM6xKfQxn7N2vmxu7175u5dDQ==
+  dependencies:
+    "@budibase/backend-core" "2.32.6"
+    "@budibase/shared-core" "2.32.6"
+    "@budibase/string-templates" "2.32.6"
+    "@budibase/types" "2.32.6"
+    "@koa/router" "8.0.8"
+    bull "4.10.1"
+    dd-trace "5.2.0"
+    joi "17.6.0"
+    jsonwebtoken "9.0.2"
+    lru-cache "^7.14.1"
+    memorystream "^0.3.1"
+    node-fetch "2.6.7"
+    scim-patch "^0.8.1"
+    scim2-parse-filter "^0.2.8"
+
+"@budibase/shared-core@2.32.6":
+  version "0.0.0"
+  dependencies:
+    "@budibase/types" "0.0.0"
+    cron-validate "1.4.5"
+
+"@budibase/string-templates@2.32.6":
+  version "0.0.0"
+  dependencies:
+    "@budibase/handlebars-helpers" "^0.13.2"
+    dayjs "^1.10.8"
+    handlebars "^4.7.8"
+    lodash.clonedeep "^4.5.0"
+
+"@budibase/types@2.32.6":
+  version "0.0.0"
+  dependencies:
+    scim-patch "^0.8.1"
+
 "@bull-board/api@5.10.2":
   version "5.10.2"
   resolved "https://registry.yarnpkg.com/@bull-board/api/-/api-5.10.2.tgz#ae8ff6918b23897bf879a6ead3683f964374c4b3"
@@ -12277,10 +12354,10 @@ google-p12-pem@^4.0.0:
   dependencies:
     node-forge "^1.3.1"
 
-"google-spreadsheet@npm:@budibase/google-spreadsheet@4.1.3":
+"google-spreadsheet@npm:@budibase/google-spreadsheet@4.1.5":
-  version "4.1.3"
+  version "4.1.5"
-  resolved "https://registry.yarnpkg.com/@budibase/google-spreadsheet/-/google-spreadsheet-4.1.3.tgz#bcee7bd9d90f82c54b16a9aca963b87aceb050ad"
+  resolved "https://registry.yarnpkg.com/@budibase/google-spreadsheet/-/google-spreadsheet-4.1.5.tgz#c89ffcbfcb1a3538e910d9275f73efc1d7deb85f"
-  integrity sha512-03VX3/K5NXIh6+XAIDZgcHPmR76xwd8vIDL7RedMpvM2IcXK0Iq/KU7FmLY0t/mKqORAGC7+0rajd0jLFezC4w==
+  integrity sha512-t1uBjuRSkNLnZ89DYtYQ2GW33xVU84qOyOPbGi+M0w7cAJofs95PwlBLhVol6Pv5VbeL0I1J7M4XyVqp0nSZtQ==
   dependencies:
     axios "^1.4.0"
     lodash "^4.17.21"