From b231311a879a0f4eff787318ebb6322028911afd Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 27 Feb 2023 13:17:05 +0100 Subject: [PATCH 1/5] Don't hide errors on the frontend --- .../backend/DataTable/modals/CreateEditColumn.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index d7225a6645..352f094507 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -192,13 +192,13 @@ editableColumn.name = originalName } - function deleteColumn() { + async function deleteColumn() { try { editableColumn.name = deleteColName if (editableColumn.name === $tables.selected.primaryDisplay) { notifications.error("You cannot delete the display column") } else { - tables.deleteField(editableColumn) + await tables.deleteField(editableColumn) notifications.success(`Column ${editableColumn.name} deleted.`) confirmDeleteDialog.hide() hide() From 30fde61d4d571d82a216acf46888cef42f1e8cc6 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 27 Feb 2023 13:33:19 +0100 Subject: [PATCH 2/5] Handle deletes --- packages/server/src/integrations/googlesheets.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index d3caf0b944..587616adfc 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -308,10 +308,17 @@ class GoogleSheetsIntegration implements DatasourcePlus { } await sheet.setHeaderRow(headers) } else { - let newField = Object.keys(table.schema).find( + const updatedHeaderValues = [...sheet.headerValues] + + const newField = Object.keys(table.schema).find( key => !sheet.headerValues.includes(key) ) - await sheet.setHeaderRow([...sheet.headerValues, newField]) + + if (newField) { + updatedHeaderValues.push(newField) + } + + await sheet.setHeaderRow(updatedHeaderValues) } } catch (err) { console.error("Error updating table in google sheets", err) From abe06a127ad3c7fff5bd77775e4a576aa4e9bdf4 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 27 Feb 2023 17:25:26 +0100 Subject: [PATCH 3/5] Types --- .../server/src/integrations/googlesheets.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 587616adfc..30633c9203 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -13,7 +13,7 @@ import { DataSourceOperation, FieldTypes } from "../constants" import { GoogleSpreadsheet } from "google-spreadsheet" import env from "../environment" import { tenancy, db as dbCore, constants } from "@budibase/backend-core" -const fetch = require("node-fetch") +import fetch from "node-fetch" interface GoogleSheetsConfig { spreadsheetId: string @@ -112,7 +112,7 @@ const SCHEMA: Integration = { class GoogleSheetsIntegration implements DatasourcePlus { private readonly config: GoogleSheetsConfig - private client: any + private client: GoogleSpreadsheet public tables: Record = {} public schemaErrors: Record = {} @@ -211,7 +211,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async buildSchema(datasourceId: string) { await this.connect() - const sheets = await this.client.sheetsByIndex + const sheets = this.client.sheetsByIndex const tables: Record = {} for (let sheet of sheets) { // must fetch rows to determine schema @@ -294,7 +294,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async updateTable(table?: any) { try { await this.connect() - const sheet = await this.client.sheetsByTitle[table.name] + const sheet = this.client.sheetsByTitle[table.name] await sheet.loadHeaderRow() if (table._rename) { @@ -329,7 +329,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async deleteTable(sheet: any) { try { await this.connect() - const sheetToDelete = await this.client.sheetsByTitle[sheet] + const sheetToDelete = this.client.sheetsByTitle[sheet] return await sheetToDelete.delete() } catch (err) { console.error("Error deleting table in google sheets", err) @@ -340,7 +340,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async create(query: { sheet: string; row: any }) { try { await this.connect() - const sheet = await this.client.sheetsByTitle[query.sheet] + const sheet = this.client.sheetsByTitle[query.sheet] const rowToInsert = typeof query.row === "string" ? JSON.parse(query.row) : query.row const row = await sheet.addRow(rowToInsert) @@ -356,7 +356,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async read(query: { sheet: string }) { try { await this.connect() - const sheet = await this.client.sheetsByTitle[query.sheet] + const sheet = this.client.sheetsByTitle[query.sheet] const rows = await sheet.getRows() const headerValues = sheet.headerValues const response = [] @@ -375,7 +375,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async update(query: { sheet: string; rowIndex: number; row: any }) { try { await this.connect() - const sheet = await this.client.sheetsByTitle[query.sheet] + const sheet = this.client.sheetsByTitle[query.sheet] const rows = await sheet.getRows() const row = rows[query.rowIndex] if (row) { @@ -399,7 +399,7 @@ class GoogleSheetsIntegration implements DatasourcePlus { async delete(query: { sheet: string; rowIndex: number }) { await this.connect() - const sheet = await this.client.sheetsByTitle[query.sheet] + const sheet = this.client.sheetsByTitle[query.sheet] const rows = await sheet.getRows() const row = rows[query.rowIndex] if (row) { From f85ea527bd6e23915b7b4458eabed6ed485ba7a7 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 27 Feb 2023 17:25:48 +0100 Subject: [PATCH 4/5] Add tests for adding/removing columns in a google spreadsheet --- packages/server/__mocks__/node-fetch.ts | 3 + .../integrations/tests/googlesheets.spec.ts | 119 ++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 packages/server/src/integrations/tests/googlesheets.spec.ts diff --git a/packages/server/__mocks__/node-fetch.ts b/packages/server/__mocks__/node-fetch.ts index 78071dc8af..fdf44d173d 100644 --- a/packages/server/__mocks__/node-fetch.ts +++ b/packages/server/__mocks__/node-fetch.ts @@ -172,6 +172,9 @@ module FetchMock { ), ok: true, }) + } else if (url === "https://www.googleapis.com/oauth2/v4/token") { + // any valid response + return json({}) } else if (url.includes("failonce.com")) { failCount++ if (failCount === 1) { diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts new file mode 100644 index 0000000000..186388740a --- /dev/null +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -0,0 +1,119 @@ +import type { GoogleSpreadsheetWorksheet } from "google-spreadsheet" + +jest.mock("google-auth-library") +const { OAuth2Client } = require("google-auth-library") + +const setCredentialsMock = jest.fn() +const getAccessTokenMock = jest.fn() + +OAuth2Client.mockImplementation(() => { + return { + setCredentials: setCredentialsMock, + getAccessToken: getAccessTokenMock, + } +}) + +jest.mock("google-spreadsheet") +const { GoogleSpreadsheet } = require("google-spreadsheet") + +const sheetsByTitle: { [title: string]: GoogleSpreadsheetWorksheet } = {} + +GoogleSpreadsheet.mockImplementation(() => { + return { + useOAuth2Client: jest.fn(), + loadInfo: jest.fn(), + sheetsByTitle, + } +}) + +import { structures } from "@budibase/backend-core/tests" +import TestConfiguration from "../../tests/utilities/TestConfiguration" +import GoogleSheetsIntegration from "../googlesheets" +import { FieldType, Table, TableSchema } from "../../../../types/src/documents" + +describe("Google Sheets Integration", () => { + let integration: any, + config = new TestConfiguration() + + beforeEach(async () => { + integration = new GoogleSheetsIntegration.integration({ + spreadsheetId: "randomId", + auth: { + appId: "appId", + accessToken: "accessToken", + refreshToken: "refreshToken", + }, + }) + await config.init() + }) + + function createBasicTable(name: string, columns: string[]): Table { + return { + name, + schema: { + ...columns.reduce((p, c) => { + p[c] = { + name: c, + type: FieldType.STRING, + constraints: { + type: "string", + }, + } + return p + }, {} as TableSchema), + }, + } + } + + function createSheet({ + headerValues, + }: { + headerValues: string[] + }): GoogleSpreadsheetWorksheet { + return { + // to ignore the unmapped fields + ...({} as any), + loadHeaderRow: jest.fn(), + headerValues, + setHeaderRow: jest.fn(), + } + } + + describe("update table", () => { + test("adding a new field will be adding a new header row", async () => { + await config.doInContext(structures.uuid(), async () => { + const tableColumns = ["name", "description", "new field"] + const table = createBasicTable(structures.uuid(), tableColumns) + + const sheet = createSheet({ headerValues: ["name", "description"] }) + sheetsByTitle[table.name] = sheet + await integration.updateTable(table) + + expect(sheet.loadHeaderRow).toBeCalledTimes(1) + expect(sheet.setHeaderRow).toBeCalledTimes(1) + expect(sheet.setHeaderRow).toBeCalledWith(tableColumns) + }) + }) + + test("removing an existing field will not remove the data from the spreadsheet", async () => { + await config.doInContext(structures.uuid(), async () => { + const tableColumns = ["name"] + const table = createBasicTable(structures.uuid(), tableColumns) + + const sheet = createSheet({ + headerValues: ["name", "description", "location"], + }) + sheetsByTitle[table.name] = sheet + await integration.updateTable(table) + + expect(sheet.loadHeaderRow).toBeCalledTimes(1) + expect(sheet.setHeaderRow).toBeCalledTimes(1) + expect(sheet.setHeaderRow).toBeCalledWith([ + "name", + "description", + "location", + ]) + }) + }) + }) +}) From f017f0d54e35035559e8d7adb8897eb5ff18f670 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 27 Feb 2023 17:41:42 +0100 Subject: [PATCH 5/5] Catch issue on test --- packages/server/src/integrations/tests/googlesheets.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 186388740a..1e28be33c6 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -113,6 +113,9 @@ describe("Google Sheets Integration", () => { "description", "location", ]) + + // No undefineds are sent + expect((sheet.setHeaderRow as any).mock.calls[0][0]).toHaveLength(3) }) }) })