From 2553432ec960656212d951a53420b7eee6eec6ed Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 4 Sep 2024 14:21:25 +0100 Subject: [PATCH 01/10] wip --- .../server/src/integrations/googlesheets.ts | 4 +- .../integrations/tests/googlesheets.spec.ts | 98 +++++++++++-------- 2 files changed, 58 insertions(+), 44 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index f8f5209890..b257a5da9b 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -31,7 +31,7 @@ import { cache, configs, context, HTTPError } from "@budibase/backend-core" import { dataFilters, utils } from "@budibase/shared-core" import { GOOGLE_SHEETS_PRIMARY_KEY } from "../constants" -interface GoogleSheetsConfig { +export interface GoogleSheetsConfig { spreadsheetId: string auth: OAuthClientConfig continueSetupId?: string @@ -157,7 +157,7 @@ const SCHEMA: Integration = { }, } -class GoogleSheetsIntegration implements DatasourcePlus { +export class GoogleSheetsIntegration implements DatasourcePlus { private readonly config: GoogleSheetsConfig private readonly spreadsheetId: string private client: GoogleSpreadsheet = undefined! diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index e9d5290b2c..685345660d 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -2,49 +2,48 @@ import { setEnv as setCoreEnv } from "@budibase/backend-core" import type { GoogleSpreadsheetWorksheet } from "google-spreadsheet" import nock from "nock" -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 } = {} -const sheetsByIndex: GoogleSpreadsheetWorksheet[] = [] -const mockGoogleIntegration = { - useOAuth2Client: jest.fn(), - loadInfo: jest.fn(), - sheetsByTitle, - sheetsByIndex, -} - -GoogleSpreadsheet.mockImplementation(() => mockGoogleIntegration) - import { structures } from "@budibase/backend-core/tests" import TestConfiguration from "../../tests/utilities/TestConfiguration" -import GoogleSheetsIntegration from "../googlesheets" -import { FieldType, Table, TableSchema, TableSourceType } from "@budibase/types" +import { GoogleSheetsConfig, GoogleSheetsIntegration } from "../googlesheets" +import { + Datasource, + FieldType, + SourceName, + Table, + TableSchema, + TableSourceType, +} from "@budibase/types" import { generateDatasourceID } from "../../db/utils" describe("Google Sheets Integration", () => { - let integration: any, - config = new TestConfiguration() - let cleanupEnv: () => void + const config = new TestConfiguration() - beforeAll(() => { + let integration: GoogleSheetsIntegration + let cleanupEnv: () => void + let table: Table + let datasource: Datasource + + const datasourceConfig: GoogleSheetsConfig = { + spreadsheetId: "randomId", + auth: { + appId: "appId", + accessToken: "accessToken", + refreshToken: "refreshToken", + }, + } + + beforeAll(async () => { cleanupEnv = setCoreEnv({ GOOGLE_CLIENT_ID: "test", GOOGLE_CLIENT_SECRET: "test", }) + + datasource = await config.api.datasource.create({ + name: "Test Datasource", + type: "datasource", + source: SourceName.GOOGLE_SHEETS, + config: datasourceConfig, + }) }) afterAll(async () => { @@ -53,17 +52,32 @@ describe("Google Sheets Integration", () => { }) beforeEach(async () => { - integration = new GoogleSheetsIntegration.integration({ - spreadsheetId: "randomId", - auth: { - appId: "appId", - accessToken: "accessToken", - refreshToken: "refreshToken", - }, - }) await config.init() - jest.clearAllMocks() + integration = new GoogleSheetsIntegration(datasourceConfig) + + table = await config.api.table.save({ + name: "Test Table", + type: "table", + sourceId: generateDatasourceID(), + sourceType: TableSourceType.EXTERNAL, + schema: { + name: { + name: "name", + type: FieldType.STRING, + constraints: { + type: "string", + }, + }, + description: { + name: "description", + type: FieldType.STRING, + constraints: { + type: "string", + }, + }, + }, + }) nock.cleanAll() nock("https://www.googleapis.com/").post("/oauth2/v4/token").reply(200, { From 3c58a593f95f93cb047e724d71c5cc95fa0aee78 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 6 Sep 2024 15:03:17 +0100 Subject: [PATCH 02/10] Improve typing around in-memory search. --- .../server/src/integrations/googlesheets.ts | 2 +- .../integrations/tests/googlesheets.spec.ts | 184 ++++-------------- .../integrations/tests/utils/googlesheets.ts | 93 +++++++++ packages/shared-core/src/filters.ts | 53 +++-- 4 files changed, 158 insertions(+), 174 deletions(-) create mode 100644 packages/server/src/integrations/tests/utils/googlesheets.ts diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index b257a5da9b..d8d775c629 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -580,7 +580,7 @@ export class GoogleSheetsIntegration implements DatasourcePlus { let response = [] for (let row of filtered) { response.push( - this.buildRowObject(headerValues, row.toObject(), row._rowNumber) + this.buildRowObject(headerValues, row.toObject(), row["_rowNumber"]) ) } diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 685345660d..80899699bc 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -1,36 +1,22 @@ import { setEnv as setCoreEnv } from "@budibase/backend-core" -import type { GoogleSpreadsheetWorksheet } from "google-spreadsheet" import nock from "nock" -import { structures } from "@budibase/backend-core/tests" import TestConfiguration from "../../tests/utilities/TestConfiguration" -import { GoogleSheetsConfig, GoogleSheetsIntegration } from "../googlesheets" import { Datasource, FieldType, SourceName, - Table, - TableSchema, TableSourceType, } from "@budibase/types" -import { generateDatasourceID } from "../../db/utils" +import { access } from "node:fs" +import { GoogleSheetsMock } from "./utils/googlesheets" describe("Google Sheets Integration", () => { const config = new TestConfiguration() - let integration: GoogleSheetsIntegration let cleanupEnv: () => void - let table: Table let datasource: Datasource - - const datasourceConfig: GoogleSheetsConfig = { - spreadsheetId: "randomId", - auth: { - appId: "appId", - accessToken: "accessToken", - refreshToken: "refreshToken", - }, - } + let mock: GoogleSheetsMock beforeAll(async () => { cleanupEnv = setCoreEnv({ @@ -38,11 +24,20 @@ describe("Google Sheets Integration", () => { GOOGLE_CLIENT_SECRET: "test", }) + await config.init() + datasource = await config.api.datasource.create({ name: "Test Datasource", type: "datasource", source: SourceName.GOOGLE_SHEETS, - config: datasourceConfig, + config: { + spreadsheetId: "randomId", + auth: { + appId: "appId", + accessToken: "accessToken", + refreshToken: "refreshToken", + }, + }, }) }) @@ -52,139 +47,40 @@ describe("Google Sheets Integration", () => { }) beforeEach(async () => { - await config.init() - - integration = new GoogleSheetsIntegration(datasourceConfig) - - table = await config.api.table.save({ - name: "Test Table", - type: "table", - sourceId: generateDatasourceID(), - sourceType: TableSourceType.EXTERNAL, - schema: { - name: { - name: "name", - type: FieldType.STRING, - constraints: { - type: "string", - }, - }, - description: { - name: "description", - type: FieldType.STRING, - constraints: { - type: "string", - }, - }, - }, - }) - nock.cleanAll() - nock("https://www.googleapis.com/").post("/oauth2/v4/token").reply(200, { - grant_type: "client_credentials", - client_id: "your-client-id", - client_secret: "your-client-secret", - }) + mock = GoogleSheetsMock.forDatasource(datasource) + mock.mockAuth() }) - function createBasicTable(name: string, columns: string[]): Table { - return { - type: "table", - name, - sourceId: generateDatasourceID(), - sourceType: TableSourceType.EXTERNAL, - schema: { - ...columns.reduce((p, c) => { - p[c] = { - name: c, + describe("create", () => { + it("creates a new table", async () => { + nock("https://sheets.googleapis.com/", { + reqheaders: { authorization: "Bearer test" }, + }) + .get("/v4/spreadsheets/randomId/") + .reply(200, {}) + + const table = await config.api.table.save({ + name: "Test Table", + type: "table", + sourceId: datasource._id!, + sourceType: TableSourceType.EXTERNAL, + schema: { + name: { + name: "name", 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", () => { - it("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).toHaveBeenCalledTimes(1) - expect(sheet.setHeaderRow).toHaveBeenCalledTimes(1) - expect(sheet.setHeaderRow).toHaveBeenCalledWith(tableColumns) - }) - }) - - it("removing an existing field will remove the header from the google sheet", async () => { - const sheet = 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) - return sheet - }) - expect(sheet.loadHeaderRow).toHaveBeenCalledTimes(1) - expect(sheet.setHeaderRow).toHaveBeenCalledTimes(1) - expect(sheet.setHeaderRow).toHaveBeenCalledWith([ - "name", - "description", - "location", - ]) - }) - }) - - describe("getTableNames", () => { - it("can fetch table names", async () => { - await config.doInContext(structures.uuid(), async () => { - const sheetNames: string[] = [] - for (let i = 0; i < 5; i++) { - const sheet = createSheet({ headerValues: [] }) - sheetsByIndex.push(sheet) - sheetNames.push(sheet.title) - } - - const res = await integration.getTableNames() - - expect(mockGoogleIntegration.loadInfo).toHaveBeenCalledTimes(1) - expect(res).toEqual(sheetNames) - }) - }) - }) - - describe("testConnection", () => { - it("can test successful connections", async () => { - await config.doInContext(structures.uuid(), async () => { - const res = await integration.testConnection() - - expect(mockGoogleIntegration.loadInfo).toHaveBeenCalledTimes(1) - expect(res).toEqual({ connected: true }) + }, + description: { + name: "description", + type: FieldType.STRING, + constraints: { + type: "string", + }, + }, + }, }) }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts new file mode 100644 index 0000000000..f19a50ee76 --- /dev/null +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -0,0 +1,93 @@ +import { Datasource } from "@budibase/types" +import nock from "nock" +import { GoogleSheetsConfig } from "../../googlesheets" + +interface ErrorValue { + type: string + message: string +} + +interface ExtendedValue { + stringValue?: string + numberValue?: number + boolValue?: boolean + formulaValue?: string + errorValue?: ErrorValue +} + +interface CellData { + userEnteredValue: ExtendedValue +} + +interface RowData { + values: CellData[] +} + +interface GridData { + startRow: number + startColumn: number + rowData: RowData[] +} + +interface Sheet { + properties: { + sheetId: string + title: string + } + data: GridData[] +} + +interface Spreadsheet { + spreadsheetId: string + sheets: Sheet[] +} + +export class GoogleSheetsMock { + private config: GoogleSheetsConfig + private sheet: Spreadsheet + + static forDatasource(datasource: Datasource): GoogleSheetsMock { + return new GoogleSheetsMock(datasource.config as GoogleSheetsConfig) + } + + private constructor(config: GoogleSheetsConfig) { + this.config = config + this.sheet = { + spreadsheetId: config.spreadsheetId, + sheets: [], + } + } + + init() { + nock("https://www.googleapis.com/").post("/oauth2/v4/token").reply(200, { + grant_type: "client_credentials", + client_id: "your-client-id", + client_secret: "your-client-secret", + }) + nock("https://oauth2.googleapis.com/") + .post("/token", { + client_id: "test", + client_secret: "test", + grant_type: "refresh_token", + refresh_token: "refreshToken", + }) + .reply(200, { + access_token: "test", + expires_in: 3600, + token_type: "Bearer", + scopes: "https://www.googleapis.com/auth/spreadsheets", + }) + + nock("https://sheets.googleapis.com/", { + reqheaders: { authorization: "Bearer test" }, + }) + .get("/v4/spreadsheets/randomId/") + .reply(200, {}) + + nock("https://sheets.googleapis.com/", { + reqheaders: { authorization: "Bearer test" }, + }) + .post(`/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`) + .reply(200, () => {}) + } +} diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 70fb24b373..bfe8dc1aa1 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -448,10 +448,10 @@ export function fixupFilterArrays(filters: SearchFilters) { return filters } -export const search = ( - docs: Record[], +export function search( + docs: Record[], query: RowSearchParams -): SearchResponse> => { +): SearchResponse> { let result = runQuery(docs, query.query) if (query.sort) { result = sort(result, query.sort, query.sortOrder || SortOrder.ASCENDING) @@ -475,11 +475,11 @@ export const search = ( * from custom doc type e.g. Google Sheets * */ -export const runQuery = ( - docs: Record[], +export function runQuery>( + docs: T[], query: SearchFilters, - findInDoc: Function = deepGet -) => { + findInDoc: (obj: T, key: string) => any = deepGet +): T[] { if (!docs || !Array.isArray(docs)) { return [] } @@ -502,7 +502,7 @@ export const runQuery = ( type: SearchFilterOperator, test: (docValue: any, testValue: any) => boolean ) => - (doc: Record) => { + (doc: T) => { for (const [key, testValue] of Object.entries(query[type] || {})) { const valueToCheck = isLogicalSearchOperator(type) ? doc @@ -749,11 +749,8 @@ export const runQuery = ( } ) - const docMatch = (doc: Record) => { - const filterFunctions: Record< - SearchFilterOperator, - (doc: Record) => boolean - > = { + const docMatch = (doc: T) => { + const filterFunctions: Record boolean> = { string: stringMatch, fuzzy: fuzzyMatch, range: rangeMatch, @@ -797,12 +794,12 @@ export const runQuery = ( * @param sortOrder the sort order ("ascending" or "descending") * @param sortType the type of sort ("string" or "number") */ -export const sort = ( - docs: any[], - sort: string, +export function sort>( + docs: T[], + sort: keyof T, sortOrder: SortOrder, sortType = SortType.STRING -) => { +): T[] { if (!sort || !sortOrder || !sortType) { return docs } @@ -817,19 +814,17 @@ export const sort = ( return parseFloat(x) } - return docs - .slice() - .sort((a: { [x: string]: any }, b: { [x: string]: any }) => { - const colA = parse(a[sort]) - const colB = parse(b[sort]) + return docs.slice().sort((a, b) => { + const colA = parse(a[sort]) + const colB = parse(b[sort]) - const result = colB == null || colA > colB ? 1 : -1 - if (sortOrder.toLowerCase() === "descending") { - return result * -1 - } + const result = colB == null || colA > colB ? 1 : -1 + if (sortOrder.toLowerCase() === "descending") { + return result * -1 + } - return result - }) + return result + }) } /** @@ -838,7 +833,7 @@ export const sort = ( * @param docs the data * @param limit the number of docs to limit to */ -export const limit = (docs: any[], limit: string) => { +export function limit(docs: T[], limit: string): T[] { const numLimit = parseFloat(limit) if (isNaN(numLimit)) { return docs From 0359b20347afbd2aa1d5894db60e2987222cf533 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 6 Sep 2024 16:55:16 +0100 Subject: [PATCH 03/10] First new test passing. --- .../src/api/controllers/table/external.ts | 1 + .../integrations/tests/googlesheets.spec.ts | 14 +- .../integrations/tests/utils/googlesheets.ts | 333 +++++++++++++++++- 3 files changed, 335 insertions(+), 13 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index c3356919c8..fbdb5c0b3a 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -53,6 +53,7 @@ export async function save( builderSocket?.emitDatasourceUpdate(ctx, datasource) return table } catch (err: any) { + throw err if (err instanceof Error) { ctx.throw(400, err.message) } else { diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 80899699bc..21b7128999 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -49,17 +49,11 @@ describe("Google Sheets Integration", () => { beforeEach(async () => { nock.cleanAll() mock = GoogleSheetsMock.forDatasource(datasource) - mock.mockAuth() + mock.init() }) describe("create", () => { it("creates a new table", async () => { - nock("https://sheets.googleapis.com/", { - reqheaders: { authorization: "Bearer test" }, - }) - .get("/v4/spreadsheets/randomId/") - .reply(200, {}) - const table = await config.api.table.save({ name: "Test Table", type: "table", @@ -82,6 +76,12 @@ describe("Google Sheets Integration", () => { }, }, }) + + const cell = mock.getCell(table.name, "A1") + if (!cell) { + throw new Error("Cell not found") + } + expect(cell.userEnteredValue.stringValue).toEqual(table.name) }) }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index f19a50ee76..72c7665679 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -2,6 +2,82 @@ import { Datasource } from "@budibase/types" import nock from "nock" import { GoogleSheetsConfig } from "../../googlesheets" +type Value = string | number | boolean +type Dimension = "ROWS" | "COLUMNS" + +interface Range { + row: number | "ALL" + column: number | "ALL" +} + +interface DimensionProperties { + hiddenByFilter: boolean + hiddenByUser: boolean + pixelSize: number + // developerMetadata: DeveloperMetadata[] + // dataSourceColumnReference: DataSourceColumnReference +} + +interface ValueRange { + range: string + majorDimension: Dimension + values: Value[][] +} + +interface UpdateValuesResponse { + spreadsheetId: string + updatedRange: string + updatedRows: number + updatedColumns: number + updatedCells: number + updatedData: ValueRange +} + +interface AddSheetResponse { + properties: SheetProperties +} + +interface Response { + addSheet?: AddSheetResponse +} + +interface BatchUpdateResponse { + spreadsheetId: string + replies: Response[] + updatedSpreadsheet: Spreadsheet +} + +interface GridProperties { + rowCount: number + columnCount: number + frozenRowCount: number + frozenColumnCount: number + hideGridlines: boolean + rowGroupControlAfter: boolean + columnGroupControlAfter: boolean +} + +interface SheetProperties { + sheetId: number + title: string + gridProperties: GridProperties +} + +interface AddSheetRequest { + properties: SheetProperties +} + +interface Request { + addSheet?: AddSheetRequest +} + +interface BatchUpdateRequest { + requests: Request[] + includeSpreadsheetInResponse: boolean + responseRanges: string[] + responseIncludeGridData: boolean +} + interface ErrorValue { type: string message: string @@ -27,17 +103,21 @@ interface GridData { startRow: number startColumn: number rowData: RowData[] + rowMetadata: DimensionProperties[] + columnMetadata: DimensionProperties[] } interface Sheet { - properties: { - sheetId: string - title: string - } + properties: SheetProperties data: GridData[] } +interface SpreadsheetProperties { + title: string +} + interface Spreadsheet { + properties: SpreadsheetProperties spreadsheetId: string sheets: Sheet[] } @@ -53,6 +133,9 @@ export class GoogleSheetsMock { private constructor(config: GoogleSheetsConfig) { this.config = config this.sheet = { + properties: { + title: "Test Spreadsheet", + }, spreadsheetId: config.spreadsheetId, sheets: [], } @@ -82,12 +165,250 @@ export class GoogleSheetsMock { reqheaders: { authorization: "Bearer test" }, }) .get("/v4/spreadsheets/randomId/") - .reply(200, {}) + .reply(200, () => this.sheet) + .persist() nock("https://sheets.googleapis.com/", { reqheaders: { authorization: "Bearer test" }, }) .post(`/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`) - .reply(200, () => {}) + .reply(200, (uri: string, request: nock.Body): nock.Body => { + const batchUpdateRequest = request as BatchUpdateRequest + const replies: Response[] = [] + + for (const request of batchUpdateRequest.requests) { + if (request.addSheet) { + const properties: SheetProperties = { + title: request.addSheet.properties.title, + sheetId: this.sheet.sheets.length, + gridProperties: { + rowCount: 100, + columnCount: 26, + frozenRowCount: 0, + frozenColumnCount: 0, + hideGridlines: false, + rowGroupControlAfter: false, + columnGroupControlAfter: false, + }, + } + + this.sheet.sheets.push({ + properties, + data: [this.createEmptyGrid(100, 26)], + }) + + replies.push({ addSheet: { properties } }) + } + } + + const response: BatchUpdateResponse = { + spreadsheetId: this.sheet.spreadsheetId, + replies, + updatedSpreadsheet: this.sheet, + } + return response + }) + .persist() + + nock("https://sheets.googleapis.com/", { + reqheaders: { authorization: "Bearer test" }, + }) + .put( + new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`) + ) + .reply(200, (uri, request) => + this.handleValueUpdate(request as ValueRange) + ) + } + + private handleValueUpdate(valueRange: ValueRange): UpdateValuesResponse { + if (valueRange.majorDimension !== "ROWS") { + throw new Error("Only row-major updates are supported") + } + + const { sheet, topLeft, bottomRight } = this.parseA1Notation( + valueRange.range + ) + + if (topLeft.row === "ALL") { + topLeft.row = 0 + } + if (bottomRight.row === "ALL") { + bottomRight.row = sheet.properties.gridProperties.rowCount - 1 + } + if (topLeft.column === "ALL") { + topLeft.column = 0 + } + if (bottomRight.column === "ALL") { + bottomRight.column = sheet.properties.gridProperties.columnCount - 1 + } + + for (let row = topLeft.row; row <= bottomRight.row; row++) { + for ( + let column = topLeft.column; + column <= bottomRight.column; + column++ + ) { + const cell = this.getCellNumericIndexes(sheet, row, column) + if (!cell) { + continue + } + const value = + valueRange.values[row - topLeft.row][column - topLeft.column] + cell.userEnteredValue = this.createValue(value) + } + } + + const response: UpdateValuesResponse = { + spreadsheetId: this.sheet.spreadsheetId, + updatedRange: valueRange.range, + updatedRows: valueRange.values.length, + updatedColumns: valueRange.values[0].length, + updatedCells: valueRange.values.length * valueRange.values[0].length, + updatedData: valueRange, + } + return response + } + + private createValue(from: Value): ExtendedValue { + if (typeof from === "string") { + return { + stringValue: from, + } + } else if (typeof from === "number") { + return { + numberValue: from, + } + } else if (typeof from === "boolean") { + return { + boolValue: from, + } + } else { + throw new Error("Unsupported value type") + } + } + + private createEmptyGrid(numRows: number, numCols: number): GridData { + const rowData: RowData[] = [] + for (let row = 0; row < numRows; row++) { + const cells: CellData[] = [] + for (let col = 0; col < numCols; col++) { + cells.push({ + userEnteredValue: { + stringValue: "", + }, + }) + } + rowData.push({ + values: cells, + }) + } + const rowMetadata: DimensionProperties[] = [] + for (let row = 0; row < numRows; row++) { + rowMetadata.push({ + hiddenByFilter: false, + hiddenByUser: false, + pixelSize: 100, + }) + } + const columnMetadata: DimensionProperties[] = [] + for (let col = 0; col < numCols; col++) { + columnMetadata.push({ + hiddenByFilter: false, + hiddenByUser: false, + pixelSize: 100, + }) + } + + return { + startRow: 0, + startColumn: 0, + rowData, + rowMetadata, + columnMetadata, + } + } + + getCell(sheetName: string, ref: string): CellData | undefined { + const sheet = this.getSheetByName(sheetName) + if (!sheet) { + return undefined + } + const { row, column } = this.parseCell(ref) + if (row === "ALL" || column === "ALL") { + throw new Error("Only single cell references are supported") + } + return this.getCellNumericIndexes(sheet, row, column) + } + + private getCellNumericIndexes( + sheet: Sheet, + row: number, + column: number + ): CellData | undefined { + const data = sheet.data[0] + const rowData = data.rowData[row] + if (!rowData) { + return undefined + } + const cell = rowData.values[column] + if (!cell) { + return undefined + } + return cell + } + + private parseA1Notation(range: string): { + sheet: Sheet + topLeft: Range + bottomRight: Range + } { + let [sheetName, rest] = range.split("!") + const [topLeft, bottomRight] = rest.split(":") + + if (sheetName.startsWith("'") && sheetName.endsWith("'")) { + sheetName = sheetName.slice(1, -1) + } + + const sheet = this.getSheetByName(sheetName) + if (!sheet) { + throw new Error(`Sheet ${sheetName} not found`) + } + + return { + sheet, + topLeft: this.parseCell(topLeft), + bottomRight: this.parseCell(bottomRight), + } + } + + /** + * Parses a cell reference into a row and column. + * @param cell a string of the form A1, B2, etc. + * @returns + */ + private parseCell(cell: string): Range { + const firstChar = cell.slice(0, 1) + if (this.isInteger(firstChar)) { + return { row: parseInt(cell) - 1, column: "ALL" } + } + const column = this.letterToNumber(firstChar) + if (cell.length === 1) { + return { row: "ALL", column } + } + const number = cell.slice(1) + return { row: parseInt(number) - 1, column } + } + + private isInteger(value: string): boolean { + return !isNaN(parseInt(value)) + } + + private letterToNumber(letter: string): number { + return letter.charCodeAt(0) - 65 + } + + private getSheetByName(name: string): Sheet | undefined { + return this.sheet.sheets.find(sheet => sheet.properties.title === name) } } From 1bc84c16335eca43b637a6c0249d549f3e02b9d5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 6 Sep 2024 17:29:56 +0100 Subject: [PATCH 04/10] 2nd test WIP. --- .../integrations/tests/googlesheets.spec.ts | 31 ++- .../integrations/tests/utils/googlesheets.ts | 210 ++++++++++++------ 2 files changed, 174 insertions(+), 67 deletions(-) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 21b7128999..25a3706e10 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -6,9 +6,9 @@ import { Datasource, FieldType, SourceName, + Table, TableSourceType, } from "@budibase/types" -import { access } from "node:fs" import { GoogleSheetsMock } from "./utils/googlesheets" describe("Google Sheets Integration", () => { @@ -84,4 +84,33 @@ describe("Google Sheets Integration", () => { expect(cell.userEnteredValue.stringValue).toEqual(table.name) }) }) + + describe("update", () => { + let table: Table + beforeEach(async () => { + table = await config.api.table.save({ + name: "Test Table", + type: "table", + sourceId: datasource._id!, + sourceType: TableSourceType.EXTERNAL, + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + }, + }) + }) + + it.only("should be able to add a new row", async () => { + await config.api.row.save(table._id!, { + name: "Test Contact", + description: "original description", + }) + }) + }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 72c7665679..242c18988c 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -6,8 +6,8 @@ type Value = string | number | boolean type Dimension = "ROWS" | "COLUMNS" interface Range { - row: number | "ALL" - column: number | "ALL" + row: number + column: number } interface DimensionProperties { @@ -124,7 +124,7 @@ interface Spreadsheet { export class GoogleSheetsMock { private config: GoogleSheetsConfig - private sheet: Spreadsheet + private spreadsheet: Spreadsheet static forDatasource(datasource: Datasource): GoogleSheetsMock { return new GoogleSheetsMock(datasource.config as GoogleSheetsConfig) @@ -132,7 +132,7 @@ export class GoogleSheetsMock { private constructor(config: GoogleSheetsConfig) { this.config = config - this.sheet = { + this.spreadsheet = { properties: { title: "Test Spreadsheet", }, @@ -142,11 +142,14 @@ export class GoogleSheetsMock { } init() { - nock("https://www.googleapis.com/").post("/oauth2/v4/token").reply(200, { - grant_type: "client_credentials", - client_id: "your-client-id", - client_secret: "your-client-secret", - }) + nock("https://www.googleapis.com/") + .post("/oauth2/v4/token") + .reply(200, { + grant_type: "client_credentials", + client_id: "your-client-id", + client_secret: "your-client-secret", + }) + .persist() nock("https://oauth2.googleapis.com/") .post("/token", { client_id: "test", @@ -160,54 +163,22 @@ export class GoogleSheetsMock { token_type: "Bearer", scopes: "https://www.googleapis.com/auth/spreadsheets", }) + .persist() nock("https://sheets.googleapis.com/", { reqheaders: { authorization: "Bearer test" }, }) - .get("/v4/spreadsheets/randomId/") - .reply(200, () => this.sheet) + .get(`/v4/spreadsheets/${this.config.spreadsheetId}/`) + .reply(200, () => this.handleGetSpreadsheet()) .persist() nock("https://sheets.googleapis.com/", { reqheaders: { authorization: "Bearer test" }, }) .post(`/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`) - .reply(200, (uri: string, request: nock.Body): nock.Body => { - const batchUpdateRequest = request as BatchUpdateRequest - const replies: Response[] = [] - - for (const request of batchUpdateRequest.requests) { - if (request.addSheet) { - const properties: SheetProperties = { - title: request.addSheet.properties.title, - sheetId: this.sheet.sheets.length, - gridProperties: { - rowCount: 100, - columnCount: 26, - frozenRowCount: 0, - frozenColumnCount: 0, - hideGridlines: false, - rowGroupControlAfter: false, - columnGroupControlAfter: false, - }, - } - - this.sheet.sheets.push({ - properties, - data: [this.createEmptyGrid(100, 26)], - }) - - replies.push({ addSheet: { properties } }) - } - } - - const response: BatchUpdateResponse = { - spreadsheetId: this.sheet.spreadsheetId, - replies, - updatedSpreadsheet: this.sheet, - } - return response - }) + .reply(200, (_uri, request) => + this.handleBatchUpdate(request as BatchUpdateRequest) + ) .persist() nock("https://sheets.googleapis.com/", { @@ -216,9 +187,94 @@ export class GoogleSheetsMock { .put( new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`) ) - .reply(200, (uri, request) => + .reply(200, (_uri, request) => this.handleValueUpdate(request as ValueRange) ) + .persist() + + nock("https://sheets.googleapis.com/", { + reqheaders: { authorization: "Bearer test" }, + }) + .get( + new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`) + ) + .reply(200, uri => { + const range = uri.split("/").pop() + if (!range) { + throw new Error("No range provided") + } + return this.handleGetValues(decodeURIComponent(range)) + }) + .persist() + } + + private handleGetValues(range: string): ValueRange { + const { sheet, topLeft, bottomRight } = this.parseA1Notation(range) + const valueRange: ValueRange = { + range, + majorDimension: "ROWS", + values: [], + } + + for (let row = topLeft.row; row <= bottomRight.row; row++) { + const values: Value[] = [] + for (let col = topLeft.column; col <= bottomRight.column; col++) { + const cell = this.getCellNumericIndexes(sheet, row, col) + if (!cell) { + throw new Error("Cell not found") + } + values.push(this.unwrapValue(cell.userEnteredValue)) + } + valueRange.values.push(values) + } + + return valueRange + } + + private handleBatchUpdate( + batchUpdateRequest: BatchUpdateRequest + ): BatchUpdateResponse { + const replies: Response[] = [] + + for (const request of batchUpdateRequest.requests) { + if (request.addSheet) { + const response = this.handleAddSheet(request.addSheet) + replies.push({ addSheet: response }) + } + } + + return { + spreadsheetId: this.spreadsheet.spreadsheetId, + replies, + updatedSpreadsheet: this.spreadsheet, + } + } + + private handleAddSheet(request: AddSheetRequest): AddSheetResponse { + const properties: SheetProperties = { + title: request.properties.title, + sheetId: this.spreadsheet.sheets.length, + gridProperties: { + rowCount: 100, + columnCount: 26, + frozenRowCount: 0, + frozenColumnCount: 0, + hideGridlines: false, + rowGroupControlAfter: false, + columnGroupControlAfter: false, + }, + } + + this.spreadsheet.sheets.push({ + properties, + data: [this.createEmptyGrid(100, 26)], + }) + + return { properties } + } + + private handleGetSpreadsheet(): Spreadsheet { + return this.spreadsheet } private handleValueUpdate(valueRange: ValueRange): UpdateValuesResponse { @@ -230,19 +286,6 @@ export class GoogleSheetsMock { valueRange.range ) - if (topLeft.row === "ALL") { - topLeft.row = 0 - } - if (bottomRight.row === "ALL") { - bottomRight.row = sheet.properties.gridProperties.rowCount - 1 - } - if (topLeft.column === "ALL") { - topLeft.column = 0 - } - if (bottomRight.column === "ALL") { - bottomRight.column = sheet.properties.gridProperties.columnCount - 1 - } - for (let row = topLeft.row; row <= bottomRight.row; row++) { for ( let column = topLeft.column; @@ -260,7 +303,7 @@ export class GoogleSheetsMock { } const response: UpdateValuesResponse = { - spreadsheetId: this.sheet.spreadsheetId, + spreadsheetId: this.spreadsheet.spreadsheetId, updatedRange: valueRange.range, updatedRows: valueRange.values.length, updatedColumns: valueRange.values[0].length, @@ -270,6 +313,20 @@ export class GoogleSheetsMock { return response } + private unwrapValue(from: ExtendedValue): Value { + if (from.stringValue !== undefined) { + return from.stringValue + } else if (from.numberValue !== undefined) { + return from.numberValue + } else if (from.boolValue !== undefined) { + return from.boolValue + } else if (from.formulaValue !== undefined) { + return from.formulaValue + } else { + throw new Error("Unsupported value type") + } + } + private createValue(from: Value): ExtendedValue { if (typeof from === "string") { return { @@ -375,10 +432,26 @@ export class GoogleSheetsMock { throw new Error(`Sheet ${sheetName} not found`) } + const parsedTopLeft = this.parseCell(topLeft) + const parsedBottomRight = this.parseCell(bottomRight) + + if (parsedTopLeft.row === "ALL") { + parsedTopLeft.row = 0 + } + if (parsedBottomRight.row === "ALL") { + parsedBottomRight.row = sheet.properties.gridProperties.rowCount - 1 + } + if (parsedTopLeft.column === "ALL") { + parsedTopLeft.column = 0 + } + if (parsedBottomRight.column === "ALL") { + parsedBottomRight.column = sheet.properties.gridProperties.columnCount - 1 + } + return { sheet, - topLeft: this.parseCell(topLeft), - bottomRight: this.parseCell(bottomRight), + topLeft: parsedTopLeft as Range, + bottomRight: parsedBottomRight as Range, } } @@ -387,7 +460,10 @@ export class GoogleSheetsMock { * @param cell a string of the form A1, B2, etc. * @returns */ - private parseCell(cell: string): Range { + private parseCell(cell: string): { + row: number | "ALL" + column: number | "ALL" + } { const firstChar = cell.slice(0, 1) if (this.isInteger(firstChar)) { return { row: parseInt(cell) - 1, column: "ALL" } @@ -409,6 +485,8 @@ export class GoogleSheetsMock { } private getSheetByName(name: string): Sheet | undefined { - return this.sheet.sheets.find(sheet => sheet.properties.title === name) + return this.spreadsheet.sheets.find( + sheet => sheet.properties.title === name + ) } } From 1c5b50773f8e16131733611c0615193e0889b0c8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 9 Sep 2024 12:05:15 +0100 Subject: [PATCH 05/10] Docs to Google Sheets mock. --- .../integrations/tests/utils/googlesheets.ts | 178 ++++++++++++++---- 1 file changed, 144 insertions(+), 34 deletions(-) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 242c18988c..82a274405b 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -2,7 +2,10 @@ import { Datasource } from "@budibase/types" import nock from "nock" import { GoogleSheetsConfig } from "../../googlesheets" +// https://protobuf.dev/reference/protobuf/google.protobuf/#value type Value = string | number | boolean + +// https://developers.google.com/sheets/api/reference/rest/v4/Dimension type Dimension = "ROWS" | "COLUMNS" interface Range { @@ -18,12 +21,14 @@ interface DimensionProperties { // dataSourceColumnReference: DataSourceColumnReference } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values#ValueRange interface ValueRange { range: string majorDimension: Dimension values: Value[][] } +// https://developers.google.com/sheets/api/reference/rest/v4/UpdateValuesResponse interface UpdateValuesResponse { spreadsheetId: string updatedRange: string @@ -33,6 +38,7 @@ interface UpdateValuesResponse { updatedData: ValueRange } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response#AddSheetResponse interface AddSheetResponse { properties: SheetProperties } @@ -41,12 +47,14 @@ interface Response { addSheet?: AddSheetResponse } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response interface BatchUpdateResponse { spreadsheetId: string replies: Response[] updatedSpreadsheet: Spreadsheet } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#GridProperties interface GridProperties { rowCount: number columnCount: number @@ -57,12 +65,14 @@ interface GridProperties { columnGroupControlAfter: boolean } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#SheetProperties interface SheetProperties { sheetId: number title: string gridProperties: GridProperties } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/request#AddSheetRequest interface AddSheetRequest { properties: SheetProperties } @@ -71,6 +81,7 @@ interface Request { addSheet?: AddSheetRequest } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/request interface BatchUpdateRequest { requests: Request[] includeSpreadsheetInResponse: boolean @@ -78,11 +89,13 @@ interface BatchUpdateRequest { responseIncludeGridData: boolean } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/other#ErrorValue interface ErrorValue { type: string message: string } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/other#ExtendedValue interface ExtendedValue { stringValue?: string numberValue?: number @@ -91,14 +104,17 @@ interface ExtendedValue { errorValue?: ErrorValue } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/cells#CellData interface CellData { userEnteredValue: ExtendedValue } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#RowData interface RowData { values: CellData[] } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#GridData interface GridData { startRow: number startColumn: number @@ -107,21 +123,61 @@ interface GridData { columnMetadata: DimensionProperties[] } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#Sheet interface Sheet { properties: SheetProperties data: GridData[] } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets#SpreadsheetProperties interface SpreadsheetProperties { title: string } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets#Spreadsheet interface Spreadsheet { properties: SpreadsheetProperties spreadsheetId: string sheets: Sheet[] } +// https://developers.google.com/sheets/api/reference/rest/v4/ValueInputOption +type ValueInputOption = + | "USER_ENTERED" + | "RAW" + | "INPUT_VALUE_OPTION_UNSPECIFIED" + +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/append#InsertDataOption +type InsertDataOption = "OVERWRITE" | "INSERT_ROWS" + +// https://developers.google.com/sheets/api/reference/rest/v4/ValueRenderOption +type ValueRenderOption = "FORMATTED_VALUE" | "UNFORMATTED_VALUE" | "FORMULA" + +// https://developers.google.com/sheets/api/reference/rest/v4/DateTimeRenderOption +type DateTimeRenderOption = "SERIAL_NUMBER" | "FORMATTED_STRING" + +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/append#query-parameters +interface AppendParams { + valueInputOption?: ValueInputOption + insertDataOption?: InsertDataOption + includeValuesInResponse?: boolean + responseValueRenderOption?: ValueRenderOption + responseDateTimeRenderOption?: DateTimeRenderOption +} + +interface AppendRequest { + range: string + params: AppendParams + body: ValueRange +} + +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/append#response-body +interface AppendResponse { + spreadsheetId: string + tableRange: string + updates: UpdateValuesResponse +} + export class GoogleSheetsMock { private config: GoogleSheetsConfig private spreadsheet: Spreadsheet @@ -141,7 +197,38 @@ export class GoogleSheetsMock { } } - init() { + private route( + method: "get" | "put" | "post", + path: string | RegExp, + handler: (uri: string, request: nock.Body) => nock.Body + ): nock.Scope { + const headers = { reqheaders: { authorization: "Bearer test" } } + const scope = nock("https://sheets.googleapis.com/", headers) + return scope[method](path).reply(200, handler).persist() + } + + private get( + path: string | RegExp, + handler: (uri: string, request: nock.Body) => nock.Body + ): nock.Scope { + return this.route("get", path, handler) + } + + private put( + path: string | RegExp, + handler: (uri: string, request: nock.Body) => nock.Body + ): nock.Scope { + return this.route("put", path, handler) + } + + private post( + path: string | RegExp, + handler: (uri: string, request: nock.Body) => nock.Body + ): nock.Scope { + return this.route("post", path, handler) + } + + private mockAuth() { nock("https://www.googleapis.com/") .post("/oauth2/v4/token") .reply(200, { @@ -164,50 +251,72 @@ export class GoogleSheetsMock { scopes: "https://www.googleapis.com/auth/spreadsheets", }) .persist() + } - nock("https://sheets.googleapis.com/", { - reqheaders: { authorization: "Bearer test" }, - }) - .get(`/v4/spreadsheets/${this.config.spreadsheetId}/`) - .reply(200, () => this.handleGetSpreadsheet()) - .persist() + init() { + this.mockAuth() - nock("https://sheets.googleapis.com/", { - reqheaders: { authorization: "Bearer test" }, - }) - .post(`/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`) - .reply(200, (_uri, request) => - this.handleBatchUpdate(request as BatchUpdateRequest) - ) - .persist() + this.get(`/v4/spreadsheets/${this.config.spreadsheetId}/`, () => + this.handleGetSpreadsheet() + ) - nock("https://sheets.googleapis.com/", { - reqheaders: { authorization: "Bearer test" }, - }) - .put( - new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`) - ) - .reply(200, (_uri, request) => - this.handleValueUpdate(request as ValueRange) - ) - .persist() + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchUpdate + this.post( + `/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`, + (_uri, request) => this.handleBatchUpdate(request as BatchUpdateRequest) + ) - nock("https://sheets.googleapis.com/", { - reqheaders: { authorization: "Bearer test" }, - }) - .get( - new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`) - ) - .reply(200, uri => { + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/update + this.put( + new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`), + (_uri, request) => this.handleValueUpdate(request as ValueRange) + ) + + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/get + this.get( + new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`), + uri => { const range = uri.split("/").pop() if (!range) { throw new Error("No range provided") } return this.handleGetValues(decodeURIComponent(range)) - }) - .persist() + } + ) + + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/append + this.post( + new RegExp( + `/v4/spreadsheets/${this.config.spreadsheetId}/values/.*:append` + ), + (_uri, request) => { + const url = new URL(_uri, "https://sheets.googleapis.com/") + const params: Record = Object.fromEntries( + url.searchParams.entries() + ) + + if (params.includeValuesInResponse === "true") { + params.includeValuesInResponse = true + } else { + params.includeValuesInResponse = false + } + + const range = url.pathname.split("/").pop() + if (!range) { + throw new Error("No range provided") + } + + return this.handleValueAppend({ + range, + params, + body: request as ValueRange, + }) + } + ) } + private handleValueAppend(request: AppendRequest): AppendResponse {} + private handleGetValues(range: string): ValueRange { const { sheet, topLeft, bottomRight } = this.parseA1Notation(range) const valueRange: ValueRange = { @@ -415,6 +524,7 @@ export class GoogleSheetsMock { return cell } + // https://developers.google.com/sheets/api/guides/concepts#cell private parseA1Notation(range: string): { sheet: Sheet topLeft: Range From 1eb8c3409a5cbba0e8de6ead47370f5bfda1a03f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 9 Sep 2024 16:33:35 +0100 Subject: [PATCH 06/10] More progress toward a row save test passing. --- .../server/src/integrations/googlesheets.ts | 17 +- .../integrations/tests/googlesheets.spec.ts | 12 +- .../integrations/tests/utils/googlesheets.ts | 289 ++++++++++++------ 3 files changed, 219 insertions(+), 99 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index d8d775c629..4fc47500eb 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -395,7 +395,12 @@ export class GoogleSheetsIntegration implements DatasourcePlus { sheet, }) case Operation.CREATE_TABLE: - return this.createTable(json?.table?.name) + if (json.table === undefined) { + throw new Error( + "attempted to create a table without specifying the table to create" + ) + } + return this.createTable(json.table) case Operation.UPDATE_TABLE: return this.updateTable(json.table!) case Operation.DELETE_TABLE: @@ -422,13 +427,13 @@ export class GoogleSheetsIntegration implements DatasourcePlus { return rowObject } - private async createTable(name?: string) { - if (!name) { - throw new Error("Must provide name for new sheet.") - } + private async createTable(table: Table) { try { await this.connect() - await this.client.addSheet({ title: name, headerValues: [name] }) + await this.client.addSheet({ + title: table.name, + headerValues: Object.keys(table.schema), + }) } catch (err) { console.error("Error creating new table in google sheets", err) throw err diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 25a3706e10..e31d3e4330 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -49,12 +49,11 @@ describe("Google Sheets Integration", () => { beforeEach(async () => { nock.cleanAll() mock = GoogleSheetsMock.forDatasource(datasource) - mock.init() }) describe("create", () => { it("creates a new table", async () => { - const table = await config.api.table.save({ + await config.api.table.save({ name: "Test Table", type: "table", sourceId: datasource._id!, @@ -77,11 +76,10 @@ describe("Google Sheets Integration", () => { }, }) - const cell = mock.getCell(table.name, "A1") - if (!cell) { - throw new Error("Cell not found") - } - expect(cell.userEnteredValue.stringValue).toEqual(table.name) + expect(mock.cell("A1")).toEqual("name") + expect(mock.cell("B1")).toEqual("description") + expect(mock.cell("A2")).toEqual(null) + expect(mock.cell("B2")).toEqual(null) }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 82a274405b..70481e0e0d 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -3,7 +3,7 @@ import nock from "nock" import { GoogleSheetsConfig } from "../../googlesheets" // https://protobuf.dev/reference/protobuf/google.protobuf/#value -type Value = string | number | boolean +type Value = string | number | boolean | null // https://developers.google.com/sheets/api/reference/rest/v4/Dimension type Dimension = "ROWS" | "COLUMNS" @@ -195,6 +195,9 @@ export class GoogleSheetsMock { spreadsheetId: config.spreadsheetId, sheets: [], } + + this.mockAuth() + this.mockAPI() } private route( @@ -237,6 +240,7 @@ export class GoogleSheetsMock { client_secret: "your-client-secret", }) .persist() + nock("https://oauth2.googleapis.com/") .post("/token", { client_id: "test", @@ -253,9 +257,7 @@ export class GoogleSheetsMock { .persist() } - init() { - this.mockAuth() - + private mockAPI() { this.get(`/v4/spreadsheets/${this.config.spreadsheetId}/`, () => this.handleGetSpreadsheet() ) @@ -280,7 +282,7 @@ export class GoogleSheetsMock { if (!range) { throw new Error("No range provided") } - return this.handleGetValues(decodeURIComponent(range)) + return this.getValueRange(decodeURIComponent(range)) } ) @@ -301,11 +303,17 @@ export class GoogleSheetsMock { params.includeValuesInResponse = false } - const range = url.pathname.split("/").pop() + let range = url.pathname.split("/").pop() if (!range) { throw new Error("No range provided") } + if (range.endsWith(":append")) { + range = range.slice(0, -7) + } + + range = decodeURIComponent(range) + return this.handleValueAppend({ range, params, @@ -315,29 +323,44 @@ export class GoogleSheetsMock { ) } - private handleValueAppend(request: AppendRequest): AppendResponse {} + private handleValueAppend(request: AppendRequest): AppendResponse { + const { range, params, body } = request + const { sheet, bottomRight } = this.parseA1Notation(range) - private handleGetValues(range: string): ValueRange { - const { sheet, topLeft, bottomRight } = this.parseA1Notation(range) - const valueRange: ValueRange = { - range, - majorDimension: "ROWS", - values: [], - } + const newRows = body.values.map(v => this.valuesToRowData(v)) + const toDelete = + params.insertDataOption === "INSERT_ROWS" ? newRows.length : 0 + sheet.data[0].rowData.splice(bottomRight.row + 1, toDelete, ...newRows) + sheet.data[0].rowMetadata.splice(bottomRight.row + 1, toDelete, { + hiddenByUser: false, + hiddenByFilter: false, + pixelSize: 100, + }) - for (let row = topLeft.row; row <= bottomRight.row; row++) { - const values: Value[] = [] - for (let col = topLeft.column; col <= bottomRight.column; col++) { - const cell = this.getCellNumericIndexes(sheet, row, col) - if (!cell) { - throw new Error("Cell not found") - } - values.push(this.unwrapValue(cell.userEnteredValue)) + const updatedRange = this.createA1FromRanges( + sheet, + { + row: bottomRight.row + 1, + column: 0, + }, + { + row: bottomRight.row + newRows.length, + column: 0, } - valueRange.values.push(values) - } + ) - return valueRange + return { + spreadsheetId: this.spreadsheet.spreadsheetId, + tableRange: range, + updates: { + spreadsheetId: this.spreadsheet.spreadsheetId, + updatedRange, + updatedRows: body.values.length, + updatedColumns: body.values[0].length, + updatedCells: body.values.length * body.values[0].length, + updatedData: body, + }, + } } private handleBatchUpdate( @@ -387,29 +410,9 @@ export class GoogleSheetsMock { } private handleValueUpdate(valueRange: ValueRange): UpdateValuesResponse { - if (valueRange.majorDimension !== "ROWS") { - throw new Error("Only row-major updates are supported") - } - - const { sheet, topLeft, bottomRight } = this.parseA1Notation( - valueRange.range - ) - - for (let row = topLeft.row; row <= bottomRight.row; row++) { - for ( - let column = topLeft.column; - column <= bottomRight.column; - column++ - ) { - const cell = this.getCellNumericIndexes(sheet, row, column) - if (!cell) { - continue - } - const value = - valueRange.values[row - topLeft.row][column - topLeft.column] - cell.userEnteredValue = this.createValue(value) - } - } + this.iterateCells(valueRange, (cell, value) => { + cell.userEnteredValue = this.createValue(value) + }) const response: UpdateValuesResponse = { spreadsheetId: this.spreadsheet.spreadsheetId, @@ -422,6 +425,60 @@ export class GoogleSheetsMock { return response } + private iterateCells( + valueRange: ValueRange, + cb: (cell: CellData, value: Value) => void + ) { + if (valueRange.majorDimension !== "ROWS") { + throw new Error("Only row-major updates are supported") + } + + const { sheet, topLeft, bottomRight } = this.parseA1Notation( + valueRange.range + ) + for (let row = topLeft.row; row <= bottomRight.row; row++) { + for (let col = topLeft.column; col <= bottomRight.column; col++) { + const cell = this.getCellNumericIndexes(sheet, row, col) + if (!cell) { + throw new Error("Cell not found") + } + const value = valueRange.values[row - topLeft.row][col - topLeft.column] + cb(cell, value) + } + } + } + + private getValueRange(range: string): ValueRange { + const { sheet, topLeft, bottomRight } = this.parseA1Notation(range) + const valueRange: ValueRange = { + range, + majorDimension: "ROWS", + values: [], + } + + for (let row = topLeft.row; row <= bottomRight.row; row++) { + const values: Value[] = [] + for (let col = topLeft.column; col <= bottomRight.column; col++) { + const cell = this.getCellNumericIndexes(sheet, row, col) + if (!cell) { + throw new Error("Cell not found") + } + values.push(this.unwrapValue(cell.userEnteredValue)) + } + valueRange.values.push(values) + } + + return valueRange + } + + private valuesToRowData(values: Value[]): RowData { + return { + values: values.map(v => { + return { userEnteredValue: this.createValue(v) } + }), + } + } + private unwrapValue(from: ExtendedValue): Value { if (from.stringValue !== undefined) { return from.stringValue @@ -432,12 +489,14 @@ export class GoogleSheetsMock { } else if (from.formulaValue !== undefined) { return from.formulaValue } else { - throw new Error("Unsupported value type") + return null } } private createValue(from: Value): ExtendedValue { - if (typeof from === "string") { + if (from == null) { + return {} + } else if (typeof from === "string") { return { stringValue: from, } @@ -459,15 +518,9 @@ export class GoogleSheetsMock { for (let row = 0; row < numRows; row++) { const cells: CellData[] = [] for (let col = 0; col < numCols; col++) { - cells.push({ - userEnteredValue: { - stringValue: "", - }, - }) + cells.push({ userEnteredValue: this.createValue(null) }) } - rowData.push({ - values: cells, - }) + rowData.push({ values: cells }) } const rowMetadata: DimensionProperties[] = [] for (let row = 0; row < numRows; row++) { @@ -495,16 +548,20 @@ export class GoogleSheetsMock { } } - getCell(sheetName: string, ref: string): CellData | undefined { - const sheet = this.getSheetByName(sheetName) - if (!sheet) { + private cellData(cell: string): CellData | undefined { + const { + sheet, + topLeft: { row, column }, + } = this.parseA1Notation(cell) + return this.getCellNumericIndexes(sheet, row, column) + } + + cell(cell: string): Value | undefined { + const cellData = this.cellData(cell) + if (!cellData) { return undefined } - const { row, column } = this.parseCell(ref) - if (row === "ALL" || column === "ALL") { - throw new Error("Only single cell references are supported") - } - return this.getCellNumericIndexes(sheet, row, column) + return this.unwrapValue(cellData.userEnteredValue) } private getCellNumericIndexes( @@ -525,36 +582,83 @@ export class GoogleSheetsMock { } // https://developers.google.com/sheets/api/guides/concepts#cell + // + // Examples from + // https://code.luasoftware.com/tutorials/google-sheets-api/google-sheets-api-range-parameter-a1-notation + // + // "Sheet1!A1" -> First cell on Row 1 Col 1 + // "Sheet1!A1:C1" -> Col 1-3 (A, B, C) on Row 1 = A1, B1, C1 + // "A1" -> First visible sheet (if sheet name is ommitted) + // "'My Sheet'!A1" -> If sheet name which contain space or start with a bracket. + // "Sheet1" -> All cells in Sheet1. + // "Sheet1!A:A" -> All cells on Col 1. + // "Sheet1!A:B" -> All cells on Col 1 and 2. + // "Sheet1!1:1" -> All cells on Row 1. + // "Sheet1!1:2" -> All cells on Row 1 and 2. + // + // How that translates to our code below, omitting the `sheet` property: + // + // "Sheet1!A1" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 0, column: 0 } } + // "Sheet1!A1:C1" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 0, column: 2 } } + // "A1" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 0, column: 0 } } + // "Sheet1" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 100, column: 25 } } + // -> This is because we default to having a 100x26 grid. + // "Sheet1!A:A" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 99, column: 0 } } + // "Sheet1!A:B" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 99, column: 1 } } + // "Sheet1!1:1" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 0, column: 25 } } + // "Sheet1!1:2" -> { topLeft: { row: 0, column: 0 }, bottomRight: { row: 1, column: 25 } } private parseA1Notation(range: string): { sheet: Sheet topLeft: Range bottomRight: Range } { - let [sheetName, rest] = range.split("!") + let sheet: Sheet + let rest: string + if (!range.includes("!")) { + sheet = this.spreadsheet.sheets[0] + rest = range + } else { + let sheetName = range.split("!")[0] + if (sheetName.startsWith("'") && sheetName.endsWith("'")) { + sheetName = sheetName.slice(1, -1) + } + const foundSheet = this.getSheetByName(sheetName) + if (!foundSheet) { + throw new Error(`Sheet ${sheetName} not found`) + } + sheet = foundSheet + rest = range.split("!")[1] + } + const [topLeft, bottomRight] = rest.split(":") - if (sheetName.startsWith("'") && sheetName.endsWith("'")) { - sheetName = sheetName.slice(1, -1) + const parsedTopLeft = topLeft ? this.parseCell(topLeft) : undefined + let parsedBottomRight = bottomRight + ? this.parseCell(bottomRight) + : undefined + + if (!parsedTopLeft && !parsedBottomRight) { + throw new Error("No range provided") } - const sheet = this.getSheetByName(sheetName) - if (!sheet) { - throw new Error(`Sheet ${sheetName} not found`) + if (!parsedTopLeft) { + throw new Error("No top left cell provided") } - const parsedTopLeft = this.parseCell(topLeft) - const parsedBottomRight = this.parseCell(bottomRight) + if (!parsedBottomRight) { + parsedBottomRight = parsedTopLeft + } - if (parsedTopLeft.row === "ALL") { + if (parsedTopLeft && parsedTopLeft.row === undefined) { parsedTopLeft.row = 0 } - if (parsedBottomRight.row === "ALL") { - parsedBottomRight.row = sheet.properties.gridProperties.rowCount - 1 - } - if (parsedTopLeft.column === "ALL") { + if (parsedTopLeft && parsedTopLeft.column === undefined) { parsedTopLeft.column = 0 } - if (parsedBottomRight.column === "ALL") { + if (parsedBottomRight && parsedBottomRight.row === undefined) { + parsedBottomRight.row = sheet.properties.gridProperties.rowCount - 1 + } + if (parsedBottomRight && parsedBottomRight.column === undefined) { parsedBottomRight.column = sheet.properties.gridProperties.columnCount - 1 } @@ -565,22 +669,31 @@ export class GoogleSheetsMock { } } + private createA1FromRanges(sheet: Sheet, topLeft: Range, bottomRight: Range) { + let title = sheet.properties.title + if (title.includes(" ")) { + title = `'${title}'` + } + const topLeftLetter = this.numberToLetter(topLeft.column) + const bottomRightLetter = this.numberToLetter(bottomRight.column) + const topLeftRow = topLeft.row + 1 + const bottomRightRow = bottomRight.row + 1 + return `${title}!${topLeftLetter}${topLeftRow}:${bottomRightLetter}${bottomRightRow}` + } + /** * Parses a cell reference into a row and column. * @param cell a string of the form A1, B2, etc. * @returns */ - private parseCell(cell: string): { - row: number | "ALL" - column: number | "ALL" - } { + private parseCell(cell: string): Partial { const firstChar = cell.slice(0, 1) if (this.isInteger(firstChar)) { - return { row: parseInt(cell) - 1, column: "ALL" } + return { row: parseInt(cell) - 1 } } const column = this.letterToNumber(firstChar) if (cell.length === 1) { - return { row: "ALL", column } + return { column } } const number = cell.slice(1) return { row: parseInt(number) - 1, column } @@ -594,6 +707,10 @@ export class GoogleSheetsMock { return letter.charCodeAt(0) - 65 } + private numberToLetter(number: number): string { + return String.fromCharCode(number + 65) + } + private getSheetByName(name: string): Sheet | undefined { return this.spreadsheet.sheets.find( sheet => sheet.properties.title === name From 9e9f14d1b744148106318d4a68967bfcd14f9414 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 9 Sep 2024 16:45:15 +0100 Subject: [PATCH 07/10] More comments. --- .../src/integrations/tests/utils/googlesheets.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 70481e0e0d..a690a105fb 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -1,3 +1,12 @@ +// In this file is a mock implementation of the Google Sheets API. It is used +// to test the Google Sheets integration, and it keeps track of a single +// spreadsheet with many sheets. It aims to be a faithful recreation of the +// Google Sheets API, but it is not a perfect recreation. Some fields are +// missing if they aren't relevant to our use of the API. It's possible that +// this will cause problems for future feature development, but the original +// development of these tests involved hitting Google's APIs directly and +// examining the responses. If we couldn't find a good example of something in +// use, it wasn't included. import { Datasource } from "@budibase/types" import nock from "nock" import { GoogleSheetsConfig } from "../../googlesheets" @@ -13,6 +22,7 @@ interface Range { column: number } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#DimensionProperties interface DimensionProperties { hiddenByFilter: boolean hiddenByUser: boolean From 1f405da3c306fd11444ac4d27ca42829282e53ea Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 9 Sep 2024 17:51:32 +0100 Subject: [PATCH 08/10] Add some more tests. --- .../src/api/controllers/table/external.ts | 1 - .../server/src/integrations/googlesheets.ts | 25 ++----- .../integrations/tests/googlesheets.spec.ts | 66 +++++++++++++++++- .../integrations/tests/utils/googlesheets.ts | 69 +++++++++++++++---- 4 files changed, 126 insertions(+), 35 deletions(-) diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index fbdb5c0b3a..c3356919c8 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -53,7 +53,6 @@ export async function save( builderSocket?.emitDatasourceUpdate(ctx, datasource) return table } catch (err: any) { - throw err if (err instanceof Error) { ctx.throw(400, err.message) } else { diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index d6d19e2fd9..960d01ab52 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -378,6 +378,8 @@ export class GoogleSheetsIntegration implements DatasourcePlus { return this.create({ sheet, row: json.body as Row }) case Operation.BULK_CREATE: return this.createBulk({ sheet, rows: json.body as Row[] }) + case Operation.BULK_UPSERT: + return this.createBulk({ sheet, rows: json.body as Row[] }) case Operation.READ: return this.read({ ...json, sheet }) case Operation.UPDATE: @@ -557,32 +559,15 @@ export class GoogleSheetsIntegration implements DatasourcePlus { } else { rows = await sheet.getRows() } - // this is a special case - need to handle the _id, it doesn't exist - // we cannot edit the returned structure from google, it does not have - // setter functions and is immutable, easier to update the filters - // to look for the _rowNumber property rather than rowNumber - if (query.filters?.equal) { - const idFilterKeys = Object.keys(query.filters.equal).filter(filter => - filter.includes(GOOGLE_SHEETS_PRIMARY_KEY) - ) - for (let idFilterKey of idFilterKeys) { - const id = query.filters.equal[idFilterKey] - delete query.filters.equal[idFilterKey] - query.filters.equal[`_${GOOGLE_SHEETS_PRIMARY_KEY}`] = id - } - } if (hasFilters && query.paginate) { rows = rows.slice(offset, offset + limit) } const headerValues = sheet.headerValues - let response = [] - for (let row of rows) { - response.push( - this.buildRowObject(headerValues, row.toObject(), row.rowNumber) - ) - } + let response = rows.map(row => + this.buildRowObject(headerValues, row.toObject(), row.rowNumber) + ) response = dataFilters.runQuery(response, query.filters || {}) if (query.sort) { diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index e31d3e4330..d3044d17eb 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -104,11 +104,73 @@ describe("Google Sheets Integration", () => { }) }) - it.only("should be able to add a new row", async () => { - await config.api.row.save(table._id!, { + it("should be able to add a new row", async () => { + const row = await config.api.row.save(table._id!, { name: "Test Contact", description: "original description", }) + + expect(row.name).toEqual("Test Contact") + expect(row.description).toEqual("original description") + + expect(mock.cell("A2")).toEqual("Test Contact") + expect(mock.cell("B2")).toEqual("original description") + + const row2 = await config.api.row.save(table._id!, { + name: "Test Contact 2", + description: "original description 2", + }) + + expect(row2.name).toEqual("Test Contact 2") + expect(row2.description).toEqual("original description 2") + + // Notable that adding a new row adds it at the top, not the bottom. Not + // entirely sure if this is the intended behaviour or an incorrect + // implementation of the GoogleSheetsMock. + expect(mock.cell("A2")).toEqual("Test Contact 2") + expect(mock.cell("B2")).toEqual("original description 2") + + expect(mock.cell("A3")).toEqual("Test Contact") + expect(mock.cell("B3")).toEqual("original description") + }) + + it("should be able to add multiple rows", async () => { + await config.api.row.bulkImport(table._id!, { + rows: [ + { + name: "Test Contact 1", + description: "original description 1", + }, + { + name: "Test Contact 2", + description: "original description 2", + }, + ], + }) + + expect(mock.cell("A2")).toEqual("Test Contact 1") + expect(mock.cell("B2")).toEqual("original description 1") + expect(mock.cell("A3")).toEqual("Test Contact 2") + expect(mock.cell("B3")).toEqual("original description 2") + }) + + it("should be able to update a row", async () => { + const row = await config.api.row.save(table._id!, { + name: "Test Contact", + description: "original description", + }) + + expect(mock.cell("A2")).toEqual("Test Contact") + expect(mock.cell("B2")).toEqual("original description") + + await config.api.row.save(table._id!, { + ...row, + name: "Test Contact Updated", + description: "original description updated", + }) + + expect(mock.cell("A2")).toEqual("Test Contact Updated") + expect(mock.cell("B2")).toEqual("original description updated") }) }) }) diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index a690a105fb..4851104c8b 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -175,6 +175,20 @@ interface AppendParams { responseDateTimeRenderOption?: DateTimeRenderOption } +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchGet#query-parameters +interface BatchGetParams { + ranges: string[] + majorDimension?: Dimension + valueRenderOption?: ValueRenderOption + dateTimeRenderOption?: DateTimeRenderOption +} + +// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchGet#response-body +interface BatchGetResponse { + spreadsheetId: string + valueRanges: ValueRange[] +} + interface AppendRequest { range: string params: AppendParams @@ -268,39 +282,57 @@ export class GoogleSheetsMock { } private mockAPI() { - this.get(`/v4/spreadsheets/${this.config.spreadsheetId}/`, () => + const spreadsheetId = this.config.spreadsheetId + + this.get(`/v4/spreadsheets/${spreadsheetId}/`, () => this.handleGetSpreadsheet() ) // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchUpdate this.post( - `/v4/spreadsheets/${this.config.spreadsheetId}/:batchUpdate`, + `/v4/spreadsheets/${spreadsheetId}/:batchUpdate`, (_uri, request) => this.handleBatchUpdate(request as BatchUpdateRequest) ) // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/update this.put( - new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`), + new RegExp(`/v4/spreadsheets/${spreadsheetId}/values/.*`), (_uri, request) => this.handleValueUpdate(request as ValueRange) ) - // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/get + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchGet this.get( - new RegExp(`/v4/spreadsheets/${this.config.spreadsheetId}/values/.*`), + new RegExp(`/v4/spreadsheets/${spreadsheetId}/values:batchGet.*`), uri => { - const range = uri.split("/").pop() - if (!range) { - throw new Error("No range provided") + const url = new URL(uri, "https://sheets.googleapis.com/") + const params: BatchGetParams = { + ranges: url.searchParams.getAll("ranges"), + majorDimension: + (url.searchParams.get("majorDimension") as Dimension) || "ROWS", + valueRenderOption: + (url.searchParams.get("valueRenderOption") as ValueRenderOption) || + undefined, + dateTimeRenderOption: + (url.searchParams.get( + "dateTimeRenderOption" + ) as DateTimeRenderOption) || undefined, } - return this.getValueRange(decodeURIComponent(range)) + return this.handleBatchGet(params as unknown as BatchGetParams) } ) + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/get + this.get(new RegExp(`/v4/spreadsheets/${spreadsheetId}/values/.*`), uri => { + const range = uri.split("/").pop() + if (!range) { + throw new Error("No range provided") + } + return this.getValueRange(decodeURIComponent(range)) + }) + // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/append this.post( - new RegExp( - `/v4/spreadsheets/${this.config.spreadsheetId}/values/.*:append` - ), + new RegExp(`/v4/spreadsheets/${spreadsheetId}/values/.*:append`), (_uri, request) => { const url = new URL(_uri, "https://sheets.googleapis.com/") const params: Record = Object.fromEntries( @@ -373,6 +405,19 @@ export class GoogleSheetsMock { } } + private handleBatchGet(params: BatchGetParams): BatchGetResponse { + const { ranges, majorDimension } = params + + if (majorDimension && majorDimension !== "ROWS") { + throw new Error("Only row-major updates are supported") + } + + return { + spreadsheetId: this.spreadsheet.spreadsheetId, + valueRanges: ranges.map(range => this.getValueRange(range)), + } + } + private handleBatchUpdate( batchUpdateRequest: BatchUpdateRequest ): BatchUpdateResponse { From 4ff0dab399440429b14f8886ea37e3ada27c8455 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 10 Sep 2024 10:51:22 +0100 Subject: [PATCH 09/10] Respond to PR feedback. --- .../server/src/integrations/googlesheets.ts | 11 +- .../integrations/tests/utils/googlesheets.ts | 221 +++++++++++------- 2 files changed, 140 insertions(+), 92 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index 960d01ab52..af3212bf60 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -379,6 +379,8 @@ export class GoogleSheetsIntegration implements DatasourcePlus { case Operation.BULK_CREATE: return this.createBulk({ sheet, rows: json.body as Row[] }) case Operation.BULK_UPSERT: + // This is technically not correct because it won't update existing + // rows, but it's better than not having this functionality at all. return this.createBulk({ sheet, rows: json.body as Row[] }) case Operation.READ: return this.read({ ...json, sheet }) @@ -397,14 +399,19 @@ export class GoogleSheetsIntegration implements DatasourcePlus { sheet, }) case Operation.CREATE_TABLE: - if (json.table === undefined) { + if (json.table == null) { throw new Error( "attempted to create a table without specifying the table to create" ) } return this.createTable(json.table) case Operation.UPDATE_TABLE: - return this.updateTable(json.table!) + if (json.table == null) { + throw new Error( + "attempted to create a table without specifying the table to create" + ) + } + return this.updateTable(json.table) case Operation.DELETE_TABLE: return this.deleteTable(json?.table?.name) default: diff --git a/packages/server/src/integrations/tests/utils/googlesheets.ts b/packages/server/src/integrations/tests/utils/googlesheets.ts index 4851104c8b..0f92fea6cd 100644 --- a/packages/server/src/integrations/tests/utils/googlesheets.ts +++ b/packages/server/src/integrations/tests/utils/googlesheets.ts @@ -10,31 +10,71 @@ import { Datasource } from "@budibase/types" import nock from "nock" import { GoogleSheetsConfig } from "../../googlesheets" +import type { + SpreadsheetProperties, + ExtendedValue, + WorksheetDimension, + WorksheetDimensionProperties, + WorksheetProperties, + CellData, + CellBorder, + CellFormat, + CellPadding, + Color, +} from "google-spreadsheet/src/lib/types/sheets-types" + +const BLACK: Color = { red: 0, green: 0, blue: 0 } +const WHITE: Color = { red: 1, green: 1, blue: 1 } +const NO_PADDING: CellPadding = { top: 0, right: 0, bottom: 0, left: 0 } +const DEFAULT_BORDER: CellBorder = { + style: "SOLID", + width: 1, + color: BLACK, + colorStyle: { rgbColor: BLACK }, +} +const DEFAULT_CELL_FORMAT: CellFormat = { + hyperlinkDisplayType: "PLAIN_TEXT", + horizontalAlignment: "LEFT", + verticalAlignment: "BOTTOM", + wrapStrategy: "OVERFLOW_CELL", + textDirection: "LEFT_TO_RIGHT", + textRotation: { angle: 0, vertical: false }, + padding: NO_PADDING, + backgroundColorStyle: { rgbColor: BLACK }, + borders: { + top: DEFAULT_BORDER, + bottom: DEFAULT_BORDER, + left: DEFAULT_BORDER, + right: DEFAULT_BORDER, + }, + numberFormat: { + type: "NUMBER", + pattern: "General", + }, + backgroundColor: WHITE, + textFormat: { + foregroundColor: BLACK, + fontFamily: "Arial", + fontSize: 10, + bold: false, + italic: false, + strikethrough: false, + underline: false, + }, +} // https://protobuf.dev/reference/protobuf/google.protobuf/#value type Value = string | number | boolean | null -// https://developers.google.com/sheets/api/reference/rest/v4/Dimension -type Dimension = "ROWS" | "COLUMNS" - interface Range { row: number column: number } -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#DimensionProperties -interface DimensionProperties { - hiddenByFilter: boolean - hiddenByUser: boolean - pixelSize: number - // developerMetadata: DeveloperMetadata[] - // dataSourceColumnReference: DataSourceColumnReference -} - // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values#ValueRange interface ValueRange { range: string - majorDimension: Dimension + majorDimension: WorksheetDimension values: Value[][] } @@ -50,41 +90,21 @@ interface UpdateValuesResponse { // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response#AddSheetResponse interface AddSheetResponse { - properties: SheetProperties -} - -interface Response { - addSheet?: AddSheetResponse + properties: WorksheetProperties } // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/response interface BatchUpdateResponse { spreadsheetId: string - replies: Response[] + replies: { + addSheet?: AddSheetResponse + }[] updatedSpreadsheet: Spreadsheet } -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#GridProperties -interface GridProperties { - rowCount: number - columnCount: number - frozenRowCount: number - frozenColumnCount: number - hideGridlines: boolean - rowGroupControlAfter: boolean - columnGroupControlAfter: boolean -} - -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#SheetProperties -interface SheetProperties { - sheetId: number - title: string - gridProperties: GridProperties -} - // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/request#AddSheetRequest interface AddSheetRequest { - properties: SheetProperties + properties: WorksheetProperties } interface Request { @@ -99,26 +119,6 @@ interface BatchUpdateRequest { responseIncludeGridData: boolean } -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/other#ErrorValue -interface ErrorValue { - type: string - message: string -} - -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/other#ExtendedValue -interface ExtendedValue { - stringValue?: string - numberValue?: number - boolValue?: boolean - formulaValue?: string - errorValue?: ErrorValue -} - -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/cells#CellData -interface CellData { - userEnteredValue: ExtendedValue -} - // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#RowData interface RowData { values: CellData[] @@ -129,21 +129,16 @@ interface GridData { startRow: number startColumn: number rowData: RowData[] - rowMetadata: DimensionProperties[] - columnMetadata: DimensionProperties[] + rowMetadata: WorksheetDimensionProperties[] + columnMetadata: WorksheetDimensionProperties[] } // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/sheets#Sheet interface Sheet { - properties: SheetProperties + properties: WorksheetProperties data: GridData[] } -// https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets#SpreadsheetProperties -interface SpreadsheetProperties { - title: string -} - // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets#Spreadsheet interface Spreadsheet { properties: SpreadsheetProperties @@ -178,7 +173,7 @@ interface AppendParams { // https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/batchGet#query-parameters interface BatchGetParams { ranges: string[] - majorDimension?: Dimension + majorDimension?: WorksheetDimension valueRenderOption?: ValueRenderOption dateTimeRenderOption?: DateTimeRenderOption } @@ -215,6 +210,12 @@ export class GoogleSheetsMock { this.spreadsheet = { properties: { title: "Test Spreadsheet", + locale: "en_US", + autoRecalc: "ON_CHANGE", + timeZone: "America/New_York", + defaultFormat: {}, + iterativeCalculationSettings: {}, + spreadsheetTheme: {}, }, spreadsheetId: config.spreadsheetId, sheets: [], @@ -308,7 +309,8 @@ export class GoogleSheetsMock { const params: BatchGetParams = { ranges: url.searchParams.getAll("ranges"), majorDimension: - (url.searchParams.get("majorDimension") as Dimension) || "ROWS", + (url.searchParams.get("majorDimension") as WorksheetDimension) || + "ROWS", valueRenderOption: (url.searchParams.get("valueRenderOption") as ValueRenderOption) || undefined, @@ -377,8 +379,11 @@ export class GoogleSheetsMock { hiddenByUser: false, hiddenByFilter: false, pixelSize: 100, + developerMetadata: [], }) + // It's important to give back a correct updated range because the API + // library we use makes use of it to assign the correct row IDs to rows. const updatedRange = this.createA1FromRanges( sheet, { @@ -421,24 +426,31 @@ export class GoogleSheetsMock { private handleBatchUpdate( batchUpdateRequest: BatchUpdateRequest ): BatchUpdateResponse { - const replies: Response[] = [] + const response: BatchUpdateResponse = { + spreadsheetId: this.spreadsheet.spreadsheetId, + replies: [], + updatedSpreadsheet: this.spreadsheet, + } for (const request of batchUpdateRequest.requests) { if (request.addSheet) { - const response = this.handleAddSheet(request.addSheet) - replies.push({ addSheet: response }) + response.replies.push({ + addSheet: this.handleAddSheet(request.addSheet), + }) } } - return { - spreadsheetId: this.spreadsheet.spreadsheetId, - replies, - updatedSpreadsheet: this.spreadsheet, - } + return response } private handleAddSheet(request: AddSheetRequest): AddSheetResponse { - const properties: SheetProperties = { + const properties: Omit = { + index: this.spreadsheet.sheets.length, + hidden: false, + rightToLeft: false, + tabColor: BLACK, + tabColorStyle: { rgbColor: BLACK }, + sheetType: "GRID", title: request.properties.title, sheetId: this.spreadsheet.sheets.length, gridProperties: { @@ -453,11 +465,13 @@ export class GoogleSheetsMock { } this.spreadsheet.sheets.push({ - properties, + properties: properties as WorksheetProperties, data: [this.createEmptyGrid(100, 26)], }) - 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 handleGetSpreadsheet(): Spreadsheet { @@ -518,7 +532,7 @@ export class GoogleSheetsMock { if (!cell) { throw new Error("Cell not found") } - values.push(this.unwrapValue(cell.userEnteredValue)) + values.push(this.cellValue(cell)) } valueRange.values.push(values) } @@ -529,28 +543,32 @@ export class GoogleSheetsMock { private valuesToRowData(values: Value[]): RowData { return { values: values.map(v => { - return { userEnteredValue: this.createValue(v) } + return this.createCellData(v) }), } } private unwrapValue(from: ExtendedValue): Value { - if (from.stringValue !== undefined) { + if ("stringValue" in from) { return from.stringValue - } else if (from.numberValue !== undefined) { + } else if ("numberValue" in from) { return from.numberValue - } else if (from.boolValue !== undefined) { + } else if ("boolValue" in from) { return from.boolValue - } else if (from.formulaValue !== undefined) { + } else if ("formulaValue" in from) { return from.formulaValue } else { return null } } + private cellValue(from: CellData): Value { + return this.unwrapValue(from.userEnteredValue) + } + private createValue(from: Value): ExtendedValue { if (from == null) { - return {} + return {} as ExtendedValue } else if (typeof from === "string") { return { stringValue: from, @@ -568,29 +586,52 @@ export class GoogleSheetsMock { } } + /** + * Because the structure of a CellData is very nested and contains a lot of + * extraneous formatting information, this function abstracts it away and just + * lets you create a cell containing a given value. + * + * When you want to read the value back out, use {@link cellValue}. + * + * @param value value to store in the returned cell + * @returns a CellData containing the given value. Read it back out with + * {@link cellValue} + */ + private createCellData(value: Value): CellData { + return { + userEnteredValue: this.createValue(value), + effectiveValue: this.createValue(value), + formattedValue: value?.toString() || "", + userEnteredFormat: DEFAULT_CELL_FORMAT, + effectiveFormat: DEFAULT_CELL_FORMAT, + } + } + private createEmptyGrid(numRows: number, numCols: number): GridData { const rowData: RowData[] = [] for (let row = 0; row < numRows; row++) { const cells: CellData[] = [] for (let col = 0; col < numCols; col++) { - cells.push({ userEnteredValue: this.createValue(null) }) + cells.push(this.createCellData(null)) } rowData.push({ values: cells }) } - const rowMetadata: DimensionProperties[] = [] + const rowMetadata: WorksheetDimensionProperties[] = [] for (let row = 0; row < numRows; row++) { rowMetadata.push({ hiddenByFilter: false, hiddenByUser: false, pixelSize: 100, + developerMetadata: [], }) } - const columnMetadata: DimensionProperties[] = [] + const columnMetadata: WorksheetDimensionProperties[] = [] for (let col = 0; col < numCols; col++) { columnMetadata.push({ hiddenByFilter: false, hiddenByUser: false, pixelSize: 100, + developerMetadata: [], }) } @@ -616,7 +657,7 @@ export class GoogleSheetsMock { if (!cellData) { return undefined } - return this.unwrapValue(cellData.userEnteredValue) + return this.cellValue(cellData) } private getCellNumericIndexes( From 7438a1d65cdaf888e7d11677d8d3e6e48982902a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 10 Sep 2024 10:59:11 +0100 Subject: [PATCH 10/10] tidy up null checks --- packages/server/src/integrations/googlesheets.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/googlesheets.ts b/packages/server/src/integrations/googlesheets.ts index af3212bf60..0d766ac1ef 100644 --- a/packages/server/src/integrations/googlesheets.ts +++ b/packages/server/src/integrations/googlesheets.ts @@ -399,14 +399,14 @@ export class GoogleSheetsIntegration implements DatasourcePlus { sheet, }) case Operation.CREATE_TABLE: - if (json.table == null) { + if (!json.table) { throw new Error( "attempted to create a table without specifying the table to create" ) } return this.createTable(json.table) case Operation.UPDATE_TABLE: - if (json.table == null) { + if (!json.table) { throw new Error( "attempted to create a table without specifying the table to create" )