From 135aff4a31c69b39a85abf6ee1795718f330e1fe Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 Jan 2022 18:22:59 +0000 Subject: [PATCH] Bit of refactoring, adding in functionality to remove invalid static formula when the elements that the formula depends on are removed. --- .../src/api/controllers/table/bulkFormula.js | 81 +++++++++++++++++-- .../src/api/controllers/table/internal.js | 7 +- .../server/src/api/controllers/table/utils.js | 6 +- .../src/utilities/rowProcessor/utils.js | 12 +-- packages/string-templates/src/index.cjs | 1 + packages/string-templates/src/index.js | 35 +++++--- packages/string-templates/src/index.mjs | 1 + 7 files changed, 115 insertions(+), 28 deletions(-) diff --git a/packages/server/src/api/controllers/table/bulkFormula.js b/packages/server/src/api/controllers/table/bulkFormula.js index c1dcaf532b..330751ab54 100644 --- a/packages/server/src/api/controllers/table/bulkFormula.js +++ b/packages/server/src/api/controllers/table/bulkFormula.js @@ -1,15 +1,16 @@ const { FieldTypes, FormulaTypes } = require("../../../constants") -const { getAllInternalTables } = require("./utils") -const { doesContainString } = require("@budibase/string-templates") +const { getAllInternalTables, deleteColumns } = require("./utils") +const { doesContainStrings } = require("@budibase/string-templates") const { cloneDeep } = require("lodash/fp") -const { isEqual } = require("lodash") +const { isEqual, uniq } = require("lodash") /** * This retrieves the formula columns from a table schema that use a specified column name * in the formula. */ -function getFormulaThatUseColumn(table, columnName) { +function getFormulaThatUseColumn(table, columnNames) { let formula = [] + columnNames = Array.isArray(columnNames) ? columnNames : [columnNames] for (let column of Object.values(table.schema)) { // not a static formula, or doesn't contain a relationship if ( @@ -18,7 +19,7 @@ function getFormulaThatUseColumn(table, columnName) { ) { continue } - if (!doesContainString(column.formula, columnName)) { + if (!doesContainStrings(column.formula, columnNames)) { continue } formula.push(column.name) @@ -26,17 +27,78 @@ function getFormulaThatUseColumn(table, columnName) { return formula } +/** + * This functions checks two things: + * 1. when a related table, column or related column is deleted, if any + * tables need to have the formula column removed. + * 2. If a formula has been added, or updated bulk update all the rows + * in the table as per the new formula. + */ + +async function checkRequiredFormulaUpdates(db, table, { oldTable, deletion }) { + // start by retrieving all tables, remove the current table from the list + const tables = (await getAllInternalTables({ db })).filter( + tbl => tbl._id !== table._id + ) + const schemaToUse = oldTable ? oldTable.schema : table.schema + let removedColumns = Object.values(schemaToUse).filter( + column => deletion || !table.schema[column.name] + ) + // remove any formula columns that used related columns + for (let removed of removedColumns) { + let tableToUse = table + // if relationship, get the related table + if (removed.type === FieldTypes.LINK) { + tableToUse = tables.find(table => table._id === removed.tableId) + } + const columnsToDelete = getFormulaThatUseColumn(tableToUse, removed.name) + if (columnsToDelete.length > 0) { + await deleteColumns(db, table, columnsToDelete) + } + // need a special case, where a column has been removed from this table, but was used + // in a different, related tables formula + if (table.relatedFormula) { + for (let relatedTableId of table.relatedFormula) { + const relatedColumns = Object.values(table.schema).filter( + column => column.tableId === relatedTableId + ) + const relatedTable = tables.find(table => table._id === relatedTableId) + // look to see if the column was used in a relationship formula, + // relationships won't be used for this + if ( + relatedTable && + relatedColumns && + removed.type !== FieldTypes.LINK + ) { + let relatedFormulaToRemove = [] + for (let column of relatedColumns) { + relatedFormulaToRemove = relatedFormulaToRemove.concat( + getFormulaThatUseColumn(relatedTable, [ + column.fieldName, + removed.name, + ]) + ) + } + if (relatedFormulaToRemove.length > 0) { + await deleteColumns(db, relatedTable, uniq(relatedFormulaToRemove)) + } + } + } + } + } +} + /** * This function adds a note to related tables that they are * used in a static formula - so that the link controller * can manage hydrating related rows formula fields. This is * specifically only for static formula. */ -exports.updateRelatedFormulaLinksOnTables = async ( +async function updateRelatedFormulaLinksOnTables( db, table, { deletion } = { deletion: false } -) => { +) { // start by retrieving all tables, remove the current table from the list const tables = (await getAllInternalTables({ db })).filter( tbl => tbl._id !== table._id @@ -87,3 +149,8 @@ exports.updateRelatedFormulaLinksOnTables = async ( } } } + +exports.runStaticFormulaChecks = async (db, table, { oldTable, deletion }) => { + await updateRelatedFormulaLinksOnTables(db, table, { deletion }) + await checkRequiredFormulaUpdates(db, table, { oldTable, deletion }) +} diff --git a/packages/server/src/api/controllers/table/internal.js b/packages/server/src/api/controllers/table/internal.js index 88a0a5f442..9a477d25e7 100644 --- a/packages/server/src/api/controllers/table/internal.js +++ b/packages/server/src/api/controllers/table/internal.js @@ -10,7 +10,7 @@ const { } = require("./utils") const usageQuota = require("../../../utilities/usageQuota") const { cleanupAttachments } = require("../../../utilities/rowProcessor") -const { updateRelatedFormulaLinksOnTables } = require("./bulkFormula") +const { runStaticFormulaChecks } = require("./bulkFormula") exports.save = async function (ctx) { const appId = ctx.appId @@ -107,8 +107,7 @@ exports.save = async function (ctx) { tableToSave = await tableSaveFunctions.after(tableToSave) // has to run after, make sure it has _id - await updateRelatedFormulaLinksOnTables(db, tableToSave) - + await runStaticFormulaChecks(db, tableToSave, { oldTable }) return tableToSave } @@ -146,7 +145,7 @@ exports.destroy = async function (ctx) { } // has to run after, make sure it has _id - await updateRelatedFormulaLinksOnTables(db, tableToDelete, { deletion: true }) + await runStaticFormulaChecks(db, tableToDelete, { deletion: true }) await cleanupAttachments(appId, tableToDelete, { rows }) return tableToDelete } diff --git a/packages/server/src/api/controllers/table/utils.js b/packages/server/src/api/controllers/table/utils.js index 16fbeba783..d1a8fd82f7 100644 --- a/packages/server/src/api/controllers/table/utils.js +++ b/packages/server/src/api/controllers/table/utils.js @@ -24,8 +24,8 @@ const viewTemplate = require("../view/viewBuilder") const usageQuota = require("../../../utilities/usageQuota") const { cloneDeep } = require("lodash/fp") -exports.deleteColumn = async (db, table, columns) => { - columns.forEach(colName => delete table.schema[colName]) +exports.deleteColumns = async (db, table, columnNames) => { + columnNames.forEach(colName => delete table.schema[colName]) const rows = await db.allDocs( getRowParams(table._id, null, { include_docs: true, @@ -34,7 +34,7 @@ exports.deleteColumn = async (db, table, columns) => { await db.put(table) return db.bulkDocs( rows.rows.map(({ doc }) => { - columns.forEach(colName => delete doc[colName]) + columnNames.forEach(colName => delete doc[colName]) return doc }) ) diff --git a/packages/server/src/utilities/rowProcessor/utils.js b/packages/server/src/utilities/rowProcessor/utils.js index 30185caca6..95b7828084 100644 --- a/packages/server/src/utilities/rowProcessor/utils.js +++ b/packages/server/src/utilities/rowProcessor/utils.js @@ -25,11 +25,13 @@ exports.processFormulas = ( } // iterate through rows and process formula for (let i = 0; i < rows.length; i++) { - let row = rows[i] - let context = contextRows ? contextRows[i] : row - rows[i] = { - ...row, - [column]: processStringSync(schema.formula, context), + if (schema.formula) { + let row = rows[i] + let context = contextRows ? contextRows[i] : row + rows[i] = { + ...row, + [column]: processStringSync(schema.formula, context), + } } } } diff --git a/packages/string-templates/src/index.cjs b/packages/string-templates/src/index.cjs index e82e8a688d..bc9a410813 100644 --- a/packages/string-templates/src/index.cjs +++ b/packages/string-templates/src/index.cjs @@ -15,6 +15,7 @@ module.exports.processStringSync = templates.processStringSync module.exports.processObjectSync = templates.processObjectSync module.exports.processString = templates.processString module.exports.processObject = templates.processObject +module.exports.doesContainStrings = templates.doesContainStrings module.exports.doesContainString = templates.doesContainString /** diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 514a762e92..3545baa76f 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -224,15 +224,13 @@ module.exports.decodeJSBinding = handlebars => { } /** - * This function looks in the supplied template for handlebars instances, if they contain - * JS the JS will be decoded and then the supplied string will be looked for. For example - * if the template "Hello, your name is {{ related }}" this function would return that true - * for the string "related" but not for "name" as it is not within the handlebars statement. - * @param {string} template A template string to search for handlebars instances. - * @param {string} string The word or sentence to search for. - * @returns {boolean} The this return true if the string is found, false if not. + * Same as the doesContainString function, but will check for all the strings + * before confirming it contains. + * @param {string} template The template string to search. + * @param {string[]} strings The strings to look for. + * @returns {boolean} Will return true if all strings found in HBS statement. */ -module.exports.doesContainString = (template, string) => { +module.exports.doesContainStrings = (template, strings) => { let regexp = new RegExp(FIND_HBS_REGEX) let matches = template.match(regexp) if (matches == null) { @@ -243,9 +241,28 @@ module.exports.doesContainString = (template, string) => { if (exports.isJSBinding(match)) { hbs = exports.decodeJSBinding(match) } - if (hbs.includes(string)) { + let allFound = true + for (let string of strings) { + if (!hbs.includes(string)) { + allFound = false + } + } + if (allFound) { return true } } return false } + +/** + * This function looks in the supplied template for handlebars instances, if they contain + * JS the JS will be decoded and then the supplied string will be looked for. For example + * if the template "Hello, your name is {{ related }}" this function would return that true + * for the string "related" but not for "name" as it is not within the handlebars statement. + * @param {string} template A template string to search for handlebars instances. + * @param {string} string The word or sentence to search for. + * @returns {boolean} The this return true if the string is found, false if not. + */ +module.exports.doesContainString = (template, string) => { + return exports.doesContainStrings(template, [string]) +} diff --git a/packages/string-templates/src/index.mjs b/packages/string-templates/src/index.mjs index 49eb191ab9..a592ae26d5 100644 --- a/packages/string-templates/src/index.mjs +++ b/packages/string-templates/src/index.mjs @@ -15,6 +15,7 @@ export const processStringSync = templates.processStringSync export const processObjectSync = templates.processObjectSync export const processString = templates.processString export const processObject = templates.processObject +export const doesContainStrings = templates.doesContainStrings export const doesContainString = templates.doesContainString /**