From e876d14b92673ec75fe942eb17195583a067ef25 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 28 Feb 2024 16:43:41 +0000 Subject: [PATCH 1/4] Ensure unsaved pending changes to rows are applied when changing cell --- .../src/components/grid/stores/rows.js | 51 ++++++++++++++----- .../src/components/grid/stores/ui.js | 10 ++++ .../src/components/grid/stores/validation.js | 22 +++++++- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index 34ba77afa2..f4b0e97942 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -328,25 +328,34 @@ export const createActions = context => { } // Patches a row with some changes - const updateRow = async (rowId, changes, options = { save: true }) => { + const updateRow = async ( + rowId, + changes, + options = { save: true, force: false } + ) => { const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) const index = $rowLookupMap[rowId] const row = $rows[index] - if (index == null || !Object.keys(changes || {}).length) { + if (index == null) { + return + } + if (!options?.force && !Object.keys(changes || {}).length) { return } // Abandon if no changes - let same = true - for (let column of Object.keys(changes)) { - if (row[column] !== changes[column]) { - same = false - break + if (!options?.force) { + let same = true + for (let column of Object.keys(changes)) { + if (row[column] !== changes[column]) { + same = false + break + } + } + if (same) { + return } - } - if (same) { - return } // Immediately update state so that the change is reflected @@ -359,7 +368,7 @@ export const createActions = context => { })) // Stop here if we don't want to persist the change - if (!options?.save) { + if (!options?.save && !options?.force) { return } @@ -508,7 +517,14 @@ export const createActions = context => { } export const initialise = context => { - const { rowChangeCache, inProgressChanges, previousFocusedRowId } = context + const { + rowChangeCache, + inProgressChanges, + previousFocusedRowId, + previousFocusedCellId, + rows, + validation, + } = context // Wipe the row change cache when changing row previousFocusedRowId.subscribe(id => { @@ -519,4 +535,15 @@ export const initialise = context => { }) } }) + + // Ensure any unsaved changes are saved when changing cell + previousFocusedCellId.subscribe(id => { + const rowId = id?.split("-")[0] + const hasErrors = validation.actions.rowHasErrors(rowId) + const hasChanges = Object.keys(get(rowChangeCache)[rowId] || {}).length > 0 + const isSavingChanges = get(inProgressChanges)[rowId] + if (rowId && !hasErrors && hasChanges && !isSavingChanges) { + rows.actions.updateRow(rowId, null, { force: true }) + } + }) } diff --git a/packages/frontend-core/src/components/grid/stores/ui.js b/packages/frontend-core/src/components/grid/stores/ui.js index 129d6614e5..da0558bb5b 100644 --- a/packages/frontend-core/src/components/grid/stores/ui.js +++ b/packages/frontend-core/src/components/grid/stores/ui.js @@ -16,6 +16,7 @@ export const createStores = context => { const hoveredRowId = writable(null) const rowHeight = writable(get(props).fixedRowHeight || DefaultRowHeight) const previousFocusedRowId = writable(null) + const previousFocusedCellId = writable(null) const gridFocused = writable(false) const isDragging = writable(false) const buttonColumnWidth = writable(0) @@ -48,6 +49,7 @@ export const createStores = context => { focusedCellAPI, focusedRowId, previousFocusedRowId, + previousFocusedCellId, hoveredRowId, rowHeight, gridFocused, @@ -129,6 +131,7 @@ export const initialise = context => { const { focusedRowId, previousFocusedRowId, + previousFocusedCellId, rows, focusedCellId, selectedRows, @@ -181,6 +184,13 @@ export const initialise = context => { lastFocusedRowId = id }) + // Remember the last focused cell ID so that we can store the previous one + let lastFocusedCellId = null + focusedCellId.subscribe(id => { + previousFocusedCellId.set(lastFocusedCellId) + lastFocusedCellId = id + }) + // Remove hovered row when a cell is selected focusedCellId.subscribe(cell => { if (cell && get(hoveredRowId)) { diff --git a/packages/frontend-core/src/components/grid/stores/validation.js b/packages/frontend-core/src/components/grid/stores/validation.js index 9c3927f9c9..70db076593 100644 --- a/packages/frontend-core/src/components/grid/stores/validation.js +++ b/packages/frontend-core/src/components/grid/stores/validation.js @@ -1,8 +1,23 @@ -import { writable, get } from "svelte/store" +import { writable, get, derived } from "svelte/store" +// Normally we would break out actions into the explicit "createActions" +// function, but for validation all these actions are pure so can go into +// "createStores" instead to make dependency ordering simpler export const createStores = () => { const validation = writable({}) + // Derive which rows have errors so that we can use that info later + const rowErrorMap = derived(validation, $validation => { + let map = {} + Object.entries($validation).forEach(([key, error]) => { + // Extract row ID from all errored cell IDs + if (error) { + map[key.split("-")[0]] = true + } + }) + return map + }) + const setError = (cellId, error) => { if (!cellId) { return @@ -13,11 +28,16 @@ export const createStores = () => { })) } + const rowHasErrors = rowId => { + return get(rowErrorMap)[rowId] + } + return { validation: { ...validation, actions: { setError, + rowHasErrors, }, }, } From acecea570499d26ddd453157d6abedfc86a43dff Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 29 Feb 2024 10:30:38 +0000 Subject: [PATCH 2/4] Refactor grid row actions to be more explicit and remove extraneous flags --- .../src/components/grid/cells/DataCell.svelte | 4 +- .../grid/overlays/KeyboardManager.svelte | 4 +- .../src/components/grid/stores/rows.js | 100 +++++++++--------- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/packages/frontend-core/src/components/grid/cells/DataCell.svelte b/packages/frontend-core/src/components/grid/cells/DataCell.svelte index cdaf28978a..d8cff26b9d 100644 --- a/packages/frontend-core/src/components/grid/cells/DataCell.svelte +++ b/packages/frontend-core/src/components/grid/cells/DataCell.svelte @@ -59,13 +59,13 @@ isReadonly: () => readonly, getType: () => column.schema.type, getValue: () => row[column.name], - setValue: (value, options = { save: true }) => { + setValue: (value, options = { apply: true }) => { validation.actions.setError(cellId, null) updateValue({ rowId: row._id, column: column.name, value, - save: options?.save, + apply: options?.apply, }) }, } diff --git a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte index 8b0a0f0942..5e3a035d89 100644 --- a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte +++ b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte @@ -217,14 +217,14 @@ const type = $focusedCellAPI.getType() if (type === "number" && keyCodeIsNumber(keyCode)) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(parseInt(key), { save: false }) + $focusedCellAPI.setValue(parseInt(key), { apply: false }) $focusedCellAPI.focus() } else if ( ["string", "barcodeqr", "longform"].includes(type) && (keyCodeIsLetter(keyCode) || keyCodeIsNumber(keyCode)) ) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(key, { save: false }) + $focusedCellAPI.setValue(key, { apply: false }) $focusedCellAPI.focus() } } diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index f4b0e97942..c8d27da2e7 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -3,6 +3,7 @@ import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" +import { isValid } from "@budibase/string-templates" export const createStores = () => { const rows = writable([]) @@ -327,38 +328,31 @@ export const createActions = context => { get(fetch)?.getInitialData() } - // Patches a row with some changes - const updateRow = async ( - rowId, - changes, - options = { save: true, force: false } - ) => { + // Checks if a changeset for a row actually mutates the row or not + const changesAreValid = (row, changes) => { + const columns = Object.keys(changes || {}) + if (!row || !columns.length) { + return false + } + + // Ensure there is at least 1 column that creates a difference + return columns.some(column => row[column] !== changes[column]) + } + + // Patches a row with some changes in local state, and returns whether a + // valid pending change was made or not + const stashRowChanges = (rowId, changes) => { const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) const index = $rowLookupMap[rowId] const row = $rows[index] - if (index == null) { - return - } - if (!options?.force && !Object.keys(changes || {}).length) { - return + + // Check this is a valid change + if (!row || !changesAreValid(row, changes)) { + return false } - // Abandon if no changes - if (!options?.force) { - let same = true - for (let column of Object.keys(changes)) { - if (row[column] !== changes[column]) { - same = false - break - } - } - if (same) { - return - } - } - - // Immediately update state so that the change is reflected + // Add change to cache rowChangeCache.update(state => ({ ...state, [rowId]: { @@ -366,26 +360,30 @@ export const createActions = context => { ...changes, }, })) + return true + } - // Stop here if we don't want to persist the change - if (!options?.save && !options?.force) { + // Saves any pending changes to a row + const applyRowChanges = async rowId => { + const $rows = get(rows) + const $rowLookupMap = get(rowLookupMap) + const index = $rowLookupMap[rowId] + const row = $rows[index] + if (row == null) { return } // Save change try { - inProgressChanges.update(state => ({ - ...state, - [rowId]: true, - })) + // Mark as in progress + inProgressChanges.update(state => ({ ...state, [rowId]: true })) // Update row - const saved = await datasource.actions.updateRow({ - ...cleanRow(row), - ...get(rowChangeCache)[rowId], - }) + const changes = get(rowChangeCache)[rowId] + const newRow = { ...cleanRow(row), ...changes } + const saved = await datasource.actions.updateRow(newRow) - // Update state after a successful change + // Update row state after a successful change if (saved?._id) { rows.update(state => { state[index] = saved @@ -395,6 +393,8 @@ export const createActions = context => { // Handle users table edge case await refreshRow(saved.id) } + + // Wipe row change cache now that we've saved the row rowChangeCache.update(state => { delete state[rowId] return state @@ -402,15 +402,17 @@ export const createActions = context => { } catch (error) { handleValidationError(rowId, error) } - inProgressChanges.update(state => ({ - ...state, - [rowId]: false, - })) + + // Mark as completed + inProgressChanges.update(state => ({ ...state, [rowId]: false })) } // Updates a value of a row - const updateValue = async ({ rowId, column, value, save = true }) => { - return await updateRow(rowId, { [column]: value }, { save }) + const updateValue = async ({ rowId, column, value, apply = true }) => { + const success = stashRowChanges(rowId, { [column]: value }) + if (success && apply) { + await applyRowChanges(rowId) + } } // Deletes an array of rows @@ -420,9 +422,7 @@ export const createActions = context => { } // Actually delete rows - rowsToDelete.forEach(row => { - delete row.__idx - }) + rowsToDelete.forEach(row => delete row.__idx) await datasource.actions.deleteRows(rowsToDelete) // Update state @@ -442,7 +442,7 @@ export const createActions = context => { newRow = newRows[i] // Ensure we have a unique _id. - // This means generating one for non DS+, overriting any that may already + // This means generating one for non DS+, overwriting any that may already // exist as we cannot allow duplicates. if (!$isDatasourcePlus) { newRow._id = Helpers.uuid() @@ -503,7 +503,7 @@ export const createActions = context => { duplicateRow, getRow, updateValue, - updateRow, + applyRowChanges, deleteRows, hasRow, loadNextPage, @@ -537,13 +537,13 @@ export const initialise = context => { }) // Ensure any unsaved changes are saved when changing cell - previousFocusedCellId.subscribe(id => { + previousFocusedCellId.subscribe(async id => { const rowId = id?.split("-")[0] const hasErrors = validation.actions.rowHasErrors(rowId) const hasChanges = Object.keys(get(rowChangeCache)[rowId] || {}).length > 0 const isSavingChanges = get(inProgressChanges)[rowId] if (rowId && !hasErrors && hasChanges && !isSavingChanges) { - rows.actions.updateRow(rowId, null, { force: true }) + await rows.actions.applyRowChanges(rowId) } }) } From bc723c7094db65e19eabbaff012f126a5037c571 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 29 Feb 2024 12:25:21 +0000 Subject: [PATCH 3/4] Lint --- packages/frontend-core/src/components/grid/stores/rows.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index c8d27da2e7..5dc9413ccd 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -3,7 +3,6 @@ import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" -import { isValid } from "@budibase/string-templates" export const createStores = () => { const rows = writable([]) From ee0f0abad25d9e1bcfc499eb2e8b7d4ed3d7b38f Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:55:45 +0000 Subject: [PATCH 4/4] Fix/rename mysql column (#13186) * Rebuild table schema when adding new column to get externalType * Added MySQL integration test suite * Add test for emitting datasource on save new column * Update packages/server/src/integration-test/mysql.spec.ts Co-authored-by: Sam Rose * remove duplicate tests * Use UUID * update account portal * Remove _add for internal save * Internal DB add column unit test * rename column test * update modules * fix tests --------- Co-authored-by: Sam Rose --- packages/account-portal | 2 +- packages/builder/src/stores/builder/tables.js | 6 + packages/pro | 2 +- .../src/api/controllers/table/external.ts | 10 +- .../server/src/api/controllers/table/index.ts | 9 +- .../src/api/controllers/table/internal.ts | 14 +- .../server/src/api/routes/tests/table.spec.ts | 30 ++ .../server/src/integration-test/mysql.spec.ts | 363 ++++++++++++++++++ .../src/sdk/app/tables/external/index.ts | 14 +- .../types/src/documents/app/table/table.ts | 3 +- packages/types/src/sdk/search.ts | 4 + 11 files changed, 440 insertions(+), 17 deletions(-) create mode 100644 packages/server/src/integration-test/mysql.spec.ts diff --git a/packages/account-portal b/packages/account-portal index 19f7a5829f..0c050591c2 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 19f7a5829f4d23cbc694136e45d94482a59a475a +Subproject commit 0c050591c21d3b67dc0c9225d60cc9e2324c8dac diff --git a/packages/builder/src/stores/builder/tables.js b/packages/builder/src/stores/builder/tables.js index 51b8416eda..f86b37ab85 100644 --- a/packages/builder/src/stores/builder/tables.js +++ b/packages/builder/src/stores/builder/tables.js @@ -147,6 +147,12 @@ export function createTablesStore() { if (indexes) { draft.indexes = indexes } + // Add object to indicate if column is being added + if (draft.schema[field.name] === undefined) { + draft._add = { + name: field.name, + } + } draft.schema = { ...draft.schema, [field.name]: cloneDeep(field), diff --git a/packages/pro b/packages/pro index 183b35d3ac..22a278da72 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 183b35d3acd42433dcb2d32bcd89a36abe13afec +Subproject commit 22a278da720d92991dabdcd4cb6c96e7abe29781 diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index f035822068..c85b46a95c 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -6,6 +6,7 @@ import { BulkImportRequest, BulkImportResponse, Operation, + RenameColumn, SaveTableRequest, SaveTableResponse, Table, @@ -25,9 +26,12 @@ function getDatasourceId(table: Table) { return breakExternalTableId(table._id).datasourceId } -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const inputs = ctx.request.body - const renaming = inputs?._rename + const adding = inputs?._add // can't do this right now delete inputs.rows const tableId = ctx.request.body._id @@ -40,7 +44,7 @@ export async function save(ctx: UserCtx) { const { datasource, table } = await sdk.tables.external.save( datasourceId!, inputs, - { tableId, renaming } + { tableId, renaming, adding } ) builderSocket?.emitDatasourceUpdate(ctx, datasource) return table diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 55a896373f..69305c461e 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -74,8 +74,15 @@ export async function save(ctx: UserCtx) { const appId = ctx.appId const table = ctx.request.body const isImport = table.rows + const renaming = ctx.request.body._rename - let savedTable = await pickApi({ table }).save(ctx) + const api = pickApi({ table }) + // do not pass _rename or _add if saving to CouchDB + if (api === internal) { + delete ctx.request.body._add + delete ctx.request.body._rename + } + let savedTable = await api.save(ctx, renaming) if (!table._id) { await events.table.created(savedTable) savedTable = sdk.tables.enrichViewSchemas(savedTable) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 8e90007d88..eb5e4b6c41 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -12,11 +12,12 @@ import { } from "@budibase/types" import sdk from "../../../sdk" -export async function save(ctx: UserCtx) { +export async function save( + ctx: UserCtx, + renaming?: RenameColumn +) { const { rows, ...rest } = ctx.request.body - let tableToSave: Table & { - _rename?: RenameColumn - } = { + let tableToSave: Table = { _id: generateTableID(), ...rest, // Ensure these fields are populated, even if not sent in the request @@ -28,15 +29,12 @@ export async function save(ctx: UserCtx) { tableToSave.views = {} } - const renaming = tableToSave._rename - delete tableToSave._rename - try { const { table } = await sdk.tables.internal.save(tableToSave, { user: ctx.user, rowsToImport: rows, tableId: ctx.request.body._id, - renaming: renaming, + renaming, }) return table diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index 29465145a9..77704a0408 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -26,6 +26,7 @@ import { TableToBuild } from "../../../tests/utilities/TestConfiguration" tk.freeze(mocks.date.MOCK_DATE) const { basicTable } = setup.structures +const ISO_REGEX_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/ describe("/tables", () => { let request = setup.getRequest() @@ -285,6 +286,35 @@ describe("/tables", () => { expect(res.body.schema.roleId).toBeDefined() }) }) + + it("should add a new column for an internal DB table", async () => { + const saveTableRequest: SaveTableRequest = { + _add: { + name: "NEW_COLUMN", + }, + ...basicTable(), + } + + const response = await request + .post(`/api/tables`) + .send(saveTableRequest) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + const expectedResponse = { + ...saveTableRequest, + _rev: expect.stringMatching(/^\d-.+/), + _id: expect.stringMatching(/^ta_.+/), + createdAt: expect.stringMatching(ISO_REGEX_PATTERN), + updatedAt: expect.stringMatching(ISO_REGEX_PATTERN), + views: {}, + } + delete expectedResponse._add + + expect(response.status).toBe(200) + expect(response.body).toEqual(expectedResponse) + }) }) describe("import", () => { diff --git a/packages/server/src/integration-test/mysql.spec.ts b/packages/server/src/integration-test/mysql.spec.ts new file mode 100644 index 0000000000..fac2bfcfeb --- /dev/null +++ b/packages/server/src/integration-test/mysql.spec.ts @@ -0,0 +1,363 @@ +import fetch from "node-fetch" +import { + generateMakeRequest, + MakeRequestResponse, +} from "../api/routes/public/tests/utils" +import { v4 as uuidv4 } from "uuid" +import * as setup from "../api/routes/tests/utilities" +import { + Datasource, + FieldType, + Table, + TableRequest, + TableSourceType, +} from "@budibase/types" +import _ from "lodash" +import { databaseTestProviders } from "../integrations/tests/utils" +import mysql from "mysql2/promise" +import { builderSocket } from "../websockets" +// @ts-ignore +fetch.mockSearch() + +const config = setup.getConfig()! + +jest.unmock("mysql2/promise") +jest.mock("../websockets", () => ({ + clientAppSocket: jest.fn(), + gridAppSocket: jest.fn(), + initialise: jest.fn(), + builderSocket: { + emitTableUpdate: jest.fn(), + emitTableDeletion: jest.fn(), + emitDatasourceUpdate: jest.fn(), + emitDatasourceDeletion: jest.fn(), + emitScreenUpdate: jest.fn(), + emitAppMetadataUpdate: jest.fn(), + emitAppPublish: jest.fn(), + }, +})) + +describe("mysql integrations", () => { + let makeRequest: MakeRequestResponse, + mysqlDatasource: Datasource, + primaryMySqlTable: Table + + beforeAll(async () => { + await config.init() + const apiKey = await config.generateApiKey() + + makeRequest = generateMakeRequest(apiKey, true) + + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterAll(async () => { + await databaseTestProviders.mysql.stop() + }) + + beforeEach(async () => { + primaryMySqlTable = await config.createTable({ + name: uuidv4(), + type: "table", + primary: ["id"], + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + }, + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + value: { + name: "value", + type: FieldType.NUMBER, + }, + }, + sourceId: mysqlDatasource._id, + sourceType: TableSourceType.EXTERNAL, + }) + }) + + afterAll(config.end) + + it("validate table schema", async () => { + const res = await makeRequest( + "get", + `/api/datasources/${mysqlDatasource._id}` + ) + + expect(res.status).toBe(200) + expect(res.body).toEqual({ + config: { + database: "mysql", + host: mysqlDatasource.config!.host, + password: "--secret-value--", + port: mysqlDatasource.config!.port, + user: "root", + }, + plus: true, + source: "MYSQL", + type: "datasource_plus", + _id: expect.any(String), + _rev: expect.any(String), + createdAt: expect.any(String), + updatedAt: expect.any(String), + entities: expect.any(Object), + }) + }) + + describe("POST /api/datasources/verify", () => { + it("should be able to verify the connection", async () => { + await config.api.datasource.verify( + { + datasource: await databaseTestProviders.mysql.datasource(), + }, + { + body: { + connected: true, + }, + } + ) + }) + + it("should state an invalid datasource cannot connect", async () => { + const dbConfig = await databaseTestProviders.mysql.datasource() + await config.api.datasource.verify( + { + datasource: { + ...dbConfig, + config: { + ...dbConfig.config, + password: "wrongpassword", + }, + }, + }, + { + body: { + connected: false, + error: + "Access denied for the specified user. User does not have the necessary privileges or the provided credentials are incorrect. Please verify the credentials, and ensure that the user has appropriate permissions.", + }, + } + ) + }) + }) + + describe("POST /api/datasources/info", () => { + it("should fetch information about mysql datasource", async () => { + const primaryName = primaryMySqlTable.name + const response = await makeRequest("post", "/api/datasources/info", { + datasource: mysqlDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1) + }) + }) + + describe("Integration compatibility with mysql search_path", () => { + let client: mysql.Connection, pathDatasource: Datasource + const database = "test1" + const database2 = "test-2" + + beforeAll(async () => { + const dsConfig = await databaseTestProviders.mysql.datasource() + const dbConfig = dsConfig.config! + + client = await mysql.createConnection(dbConfig) + await client.query(`CREATE DATABASE \`${database}\`;`) + await client.query(`CREATE DATABASE \`${database2}\`;`) + + const pathConfig: any = { + ...dsConfig, + config: { + ...dbConfig, + database, + }, + } + pathDatasource = await config.api.datasource.create(pathConfig) + }) + + afterAll(async () => { + await client.query(`DROP DATABASE \`${database}\`;`) + await client.query(`DROP DATABASE \`${database2}\`;`) + await client.end() + }) + + it("discovers tables from any schema in search path", async () => { + await client.query( + `CREATE TABLE \`${database}\`.table1 (id1 SERIAL PRIMARY KEY);` + ) + const response = await makeRequest("post", "/api/datasources/info", { + datasource: pathDatasource, + }) + expect(response.status).toBe(200) + expect(response.body.tableNames).toBeDefined() + expect(response.body.tableNames).toEqual( + expect.arrayContaining(["table1"]) + ) + }) + + it("does not mix columns from different tables", async () => { + const repeated_table_name = "table_same_name" + await client.query( + `CREATE TABLE \`${database}\`.${repeated_table_name} (id SERIAL PRIMARY KEY, val1 TEXT);` + ) + await client.query( + `CREATE TABLE \`${database2}\`.${repeated_table_name} (id2 SERIAL PRIMARY KEY, val2 TEXT);` + ) + const response = await makeRequest( + "post", + `/api/datasources/${pathDatasource._id}/schema`, + { + tablesFilter: [repeated_table_name], + } + ) + expect(response.status).toBe(200) + expect( + response.body.datasource.entities[repeated_table_name].schema + ).toBeDefined() + const schema = + response.body.datasource.entities[repeated_table_name].schema + expect(Object.keys(schema).sort()).toEqual(["id", "val1"]) + }) + }) + + describe("POST /api/tables/", () => { + let client: mysql.Connection + const emitDatasourceUpdateMock = jest.fn() + + beforeEach(async () => { + client = await mysql.createConnection( + ( + await databaseTestProviders.mysql.datasource() + ).config! + ) + mysqlDatasource = await config.api.datasource.create( + await databaseTestProviders.mysql.datasource() + ) + }) + + afterEach(async () => { + await client.end() + }) + + it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => { + const addColumnToTable: TableRequest = { + type: "table", + sourceType: TableSourceType.EXTERNAL, + name: "table", + sourceId: mysqlDatasource._id!, + primary: ["id"], + schema: { + id: { + type: FieldType.AUTO, + name: "id", + autocolumn: true, + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + }, + }, + _add: { + name: "new_column", + }, + } + + jest + .spyOn(builderSocket!, "emitDatasourceUpdate") + .mockImplementation(emitDatasourceUpdateMock) + + await makeRequest("post", "/api/tables/", addColumnToTable) + + const expectedTable: TableRequest = { + ...addColumnToTable, + schema: { + id: { + type: FieldType.NUMBER, + name: "id", + autocolumn: true, + constraints: { + presence: false, + }, + externalType: "int unsigned", + }, + new_column: { + type: FieldType.NUMBER, + name: "new_column", + autocolumn: false, + constraints: { + presence: false, + }, + externalType: "float(8,2)", + }, + }, + created: true, + _id: `${mysqlDatasource._id}__table`, + } + delete expectedTable._add + + expect(emitDatasourceUpdateMock).toBeCalledTimes(1) + const emittedDatasource: Datasource = + emitDatasourceUpdateMock.mock.calls[0][1] + expect(emittedDatasource.entities!["table"]).toEqual(expectedTable) + }) + + it("will rename a column", async () => { + await makeRequest("post", "/api/tables/", primaryMySqlTable) + + let renameColumnOnTable: TableRequest = { + ...primaryMySqlTable, + schema: { + id: { + name: "id", + type: FieldType.AUTO, + autocolumn: true, + externalType: "unsigned integer", + }, + name: { + name: "name", + type: FieldType.STRING, + externalType: "text", + }, + description: { + name: "description", + type: FieldType.STRING, + externalType: "text", + }, + age: { + name: "age", + type: FieldType.NUMBER, + externalType: "float(8,2)", + }, + }, + } + + const response = await makeRequest( + "post", + "/api/tables/", + renameColumnOnTable + ) + mysqlDatasource = ( + await makeRequest( + "post", + `/api/datasources/${mysqlDatasource._id}/schema` + ) + ).body.datasource + + expect(response.status).toEqual(200) + expect( + Object.keys(mysqlDatasource.entities![primaryMySqlTable.name].schema) + ).toEqual(["id", "name", "description", "age"]) + }) + }) +}) diff --git a/packages/server/src/sdk/app/tables/external/index.ts b/packages/server/src/sdk/app/tables/external/index.ts index 9a2bed0da2..0ace19d00e 100644 --- a/packages/server/src/sdk/app/tables/external/index.ts +++ b/packages/server/src/sdk/app/tables/external/index.ts @@ -3,6 +3,7 @@ import { Operation, RelationshipType, RenameColumn, + AddColumn, Table, TableRequest, ViewV2, @@ -32,7 +33,7 @@ import * as viewSdk from "../../views" export async function save( datasourceId: string, update: Table, - opts?: { tableId?: string; renaming?: RenameColumn } + opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn } ) { let tableToSave: TableRequest = { ...update, @@ -165,8 +166,17 @@ export async function save( // remove the rename prop delete tableToSave._rename + + // if adding a new column, we need to rebuild the schema for that table to get the 'externalType' of the column + if (opts?.adding) { + datasource.entities[tableToSave.name] = ( + await datasourceSdk.buildFilteredSchema(datasource, [tableToSave.name]) + ).tables[tableToSave.name] + } else { + datasource.entities[tableToSave.name] = tableToSave + } + // store it into couch now for budibase reference - datasource.entities[tableToSave.name] = tableToSave await db.put(populateExternalTableSchemas(datasource)) // Since tables are stored inside datasources, we need to notify clients diff --git a/packages/types/src/documents/app/table/table.ts b/packages/types/src/documents/app/table/table.ts index f3b8e6df8d..3b419dd811 100644 --- a/packages/types/src/documents/app/table/table.ts +++ b/packages/types/src/documents/app/table/table.ts @@ -1,6 +1,6 @@ import { Document } from "../../document" import { View, ViewV2 } from "../view" -import { RenameColumn } from "../../../sdk" +import { AddColumn, RenameColumn } from "../../../sdk" import { TableSchema } from "./schema" export const INTERNAL_TABLE_SOURCE_ID = "bb_internal" @@ -29,5 +29,6 @@ export interface Table extends Document { export interface TableRequest extends Table { _rename?: RenameColumn + _add?: AddColumn created?: boolean } diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 67c344d845..7a0ddaed66 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -60,6 +60,10 @@ export interface RenameColumn { updated: string } +export interface AddColumn { + name: string +} + export interface RelationshipsJson { through?: string from?: string