From 93ecd44db19d115ad469e92dad70b4571a254ffa Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 18 Jan 2022 17:21:29 +0000 Subject: [PATCH 1/3] Fix for #3721 - deleting invalid relationships if tables have been removed external to budibase - otherwise these could not be removed without deleting the datasource. --- packages/builder/cypress/support/commands.js | 9 ++++---- .../support/queryLevelTransformerFunction.js | 13 ++++++----- .../queryLevelTransformerFunctionWithData.js | 21 +++++++++-------- .../CreateExternalTableModal.svelte | 6 ++++- packages/server/src/integrations/utils.ts | 23 ++++++++++++++----- 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/packages/builder/cypress/support/commands.js b/packages/builder/cypress/support/commands.js index 666025e635..5952f4699c 100644 --- a/packages/builder/cypress/support/commands.js +++ b/packages/builder/cypress/support/commands.js @@ -429,13 +429,14 @@ Cypress.Commands.add("addDatasourceConfig", (datasource, skipFetch) => { // Click to fetch tables if (skipFetch) { cy.get(".spectrum-Dialog-grid").within(() => { - cy.get(".spectrum-Button").contains("Skip table fetch") + cy.get(".spectrum-Button") + .contains("Skip table fetch") .click({ force: true }) }) - } - else { + } else { cy.get(".spectrum-Dialog-grid").within(() => { - cy.get(".spectrum-Button").contains("Save and fetch tables") + cy.get(".spectrum-Button") + .contains("Save and fetch tables") .click({ force: true }) cy.wait(1000) }) diff --git a/packages/builder/cypress/support/queryLevelTransformerFunction.js b/packages/builder/cypress/support/queryLevelTransformerFunction.js index 0c099df1a0..213d2abc20 100644 --- a/packages/builder/cypress/support/queryLevelTransformerFunction.js +++ b/packages/builder/cypress/support/queryLevelTransformerFunction.js @@ -1,12 +1,13 @@ +// eslint-disable-next-line const breweries = data const totals = {} -for (let brewery of breweries) - {const state = brewery.state - if (totals[state] == null) - {totals[state] = 1 - } else - {totals[state]++ +for (let brewery of breweries) { + const state = brewery.state + if (totals[state] == null) { + totals[state] = 1 + } else { + totals[state]++ } } const entries = Object.entries(totals) diff --git a/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js b/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js index f7d9631ef9..3d4f6024d2 100644 --- a/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js +++ b/packages/builder/cypress/support/queryLevelTransformerFunctionWithData.js @@ -1,15 +1,16 @@ +// eslint-disable-next-line const breweries = data const totals = {} -for (let brewery of breweries) - {const state = brewery.state - if (totals[state] == null) - {totals[state] = 1 - } else - {totals[state]++ +for (let brewery of breweries) { + const state = brewery.state + if (totals[state] == null) { + totals[state] = 1 + } else { + totals[state]++ } } -const stateCodes = - {texas: "tx", +const stateCodes = { + texas: "tx", colorado: "co", florida: "fl", iwoa: "ia", @@ -24,7 +25,7 @@ const stateCodes = ohio: "oh", } const entries = Object.entries(totals) -return entries.map(([state, count]) => - {stateCodes[state.toLowerCase()] +return entries.map(([state, count]) => { + stateCodes[state.toLowerCase()] return { state, count, flag: "http://flags.ox3.in/svg/us/${stateCode}.svg" } }) diff --git a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/CreateExternalTableModal.svelte b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/CreateExternalTableModal.svelte index 1d9e246d20..52402a0396 100644 --- a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/CreateExternalTableModal.svelte +++ b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/CreateExternalTableModal.svelte @@ -6,9 +6,12 @@ export let datasource let name = "" + let submitted = false $: valid = name && name.length > 0 && !datasource?.entities[name] $: error = - name && datasource?.entities[name] ? "Table name already in use." : null + !submitted && name && datasource?.entities[name] + ? "Table name already in use." + : null function buildDefaultTable(tableName, datasourceId) { return { @@ -26,6 +29,7 @@ } async function saveTable() { + submitted = true const table = await tables.save(buildDefaultTable(name, datasource._id)) await datasources.fetch() $goto(`../../table/${table._id}`) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 46bec0e33e..df716b1fb9 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -143,11 +143,21 @@ export function isIsoDateString(str: string) { return d.toISOString() === str } +function shouldCopyRelationship(column: { type: string, tableId?: string }, tableIds: [string]) { + return column.type === FieldTypes.LINK && column.tableId && tableIds.includes(column.tableId) +} + +function shouldCopySpecialColumn(column: { type: string }, fetchedColumn: { type: string } | undefined) { + return column.type === FieldTypes.OPTIONS || + ((!fetchedColumn || fetchedColumn.type === FieldTypes.NUMBER) && column.type === FieldTypes.BOOLEAN) +} + // add the existing relationships from the entities if they exist, to prevent them from being overridden function copyExistingPropsOver( tableName: string, table: Table, - entities: { [key: string]: any } + entities: { [key: string]: any }, + tableIds: [string] ) { if (entities && entities[tableName]) { if (entities[tableName].primaryDisplay) { @@ -158,11 +168,10 @@ function copyExistingPropsOver( if (!existingTableSchema.hasOwnProperty(key)) { continue } + const column = existingTableSchema[key] if ( - existingTableSchema[key].type === FieldTypes.LINK || - existingTableSchema[key].type === FieldTypes.OPTIONS || - ((!table.schema[key] || table.schema[key].type === FieldTypes.NUMBER) && - existingTableSchema[key].type === FieldTypes.BOOLEAN) + shouldCopyRelationship(column, tableIds) || + shouldCopySpecialColumn(column, table.schema[key]) ) { table.schema[key] = existingTableSchema[key] } @@ -178,6 +187,8 @@ export function finaliseExternalTables( const invalidColumns = Object.values(InvalidColumns) let finalTables: { [key: string]: any } = {} const errors: { [key: string]: string } = {} + // @ts-ignore + const tableIds: [string] = Object.values(tables).map(table => table._id) for (let [name, table] of Object.entries(tables)) { const schemaFields = Object.keys(table.schema) // make sure every table has a key @@ -189,7 +200,7 @@ export function finaliseExternalTables( continue } // make sure all previous props have been added back - finalTables[name] = copyExistingPropsOver(name, table, entities) + finalTables[name] = copyExistingPropsOver(name, table, entities, tableIds) } // sort the tables by name finalTables = Object.entries(finalTables) From a417aa43c9c608da5e07855a09b27ba87cbbf2c2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 19 Jan 2022 10:24:15 +0000 Subject: [PATCH 2/3] Adding comments to a few SQL table schema building functions to explain their function. --- packages/server/src/integrations/utils.ts | 34 ++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index df716b1fb9..b8c96efffe 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -143,16 +143,41 @@ export function isIsoDateString(str: string) { return d.toISOString() === str } +/** + * This function will determine whether a column is a relationship and whether it + * is currently valid. The reason for the validity check is that tables can be deleted + * outside of Budibase control and if this is the case it will break Budibase relationships. + * The tableIds is a list passed down from the main finalise tables function, which is + * based on the tables that have just been fetched. This will only really be used on subsequent + * fetches to the first one - if the user is periodically refreshing Budibase knowledge of tables. + * @param column The column to check, to see if it is a valid relationship. + * @param tableIds The IDs of the tables which currently exist. + */ function shouldCopyRelationship(column: { type: string, tableId?: string }, tableIds: [string]) { return column.type === FieldTypes.LINK && column.tableId && tableIds.includes(column.tableId) } +/** + * Similar function to the shouldCopyRelationship function, but instead this looks for options and boolean + * types. It is possible to switch a string -> options and a number -> boolean (and vice versus) need to make + * sure that these get copied over when tables are fetched. Also checks whether they are still valid, if a + * column has changed type in the external database then copying it over may not be possible. + * @param column The column to check for options or boolean type. + * @param fetchedColumn The fetched column to check for the type in the external database. + */ function shouldCopySpecialColumn(column: { type: string }, fetchedColumn: { type: string } | undefined) { return column.type === FieldTypes.OPTIONS || ((!fetchedColumn || fetchedColumn.type === FieldTypes.NUMBER) && column.type === FieldTypes.BOOLEAN) } -// add the existing relationships from the entities if they exist, to prevent them from being overridden +/** + * Looks for columns which need to be copied over into the new table definitions, like relationships + * and options types. + * @param tableName The name of the table which is being checked. + * @param table The specific table which is being checked. + * @param entities All the tables that existed before - the old table definitions. + * @param tableIds The IDs of the tables which exist now, to check if anything has been removed. + */ function copyExistingPropsOver( tableName: string, table: Table, @@ -180,6 +205,13 @@ function copyExistingPropsOver( return table } +/** + * Look through the final table definitions to see if anything needs to be + * copied over from the old and if any errors have occurred mark them so + * that the user can be made aware. + * @param tables The list of tables that have been retrieved from the external database. + * @param entities The old list of tables, if there was any to look for definitions in. + */ export function finaliseExternalTables( tables: { [key: string]: any }, entities: { [key: string]: any } From 079ca74f8ad3adf92bc6ae3e599b91a1b09951fd Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 19 Jan 2022 11:53:44 +0000 Subject: [PATCH 3/3] Making the worker tell the UI it is in production when running in Cypress. --- packages/worker/src/api/controllers/system/environment.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/worker/src/api/controllers/system/environment.js b/packages/worker/src/api/controllers/system/environment.js index b897fbd943..4edf1ff8d3 100644 --- a/packages/worker/src/api/controllers/system/environment.js +++ b/packages/worker/src/api/controllers/system/environment.js @@ -6,6 +6,7 @@ exports.fetch = async ctx => { cloud: !env.SELF_HOSTED, accountPortalUrl: env.ACCOUNT_PORTAL_URL, disableAccountPortal: env.DISABLE_ACCOUNT_PORTAL, - isDev: env.isDev(), + // in test need to pretend its in production for the UI (Cypress) + isDev: env.isDev() && !env.isTest(), } }