Bit of refactoring, adding in functionality to remove invalid static formula when the elements that the formula depends on are removed.

packages/server/src/api/controllers/table/bulkFormula.js

@@ -1,15 +1,16 @@
 const { FieldTypes, FormulaTypes } = require("../../../constants")
-const { getAllInternalTables } = require("./utils")
+const { getAllInternalTables, deleteColumns } = require("./utils")
-const { doesContainString } = require("@budibase/string-templates")
+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 })
+}

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
 }

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) => {
+exports.deleteColumns = async (db, table, columnNames) => {
-  columns.forEach(colName => delete table.schema[colName])
+  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
     })
   )

packages/server/src/utilities/rowProcessor/utils.js

@@ -25,6 +25,7 @@ exports.processFormulas = (
     }
     // iterate through rows and process formula
     for (let i = 0; i < rows.length; i++) {
+      if (schema.formula) {
         let row = rows[i]
         let context = contextRows ? contextRows[i] : row
         rows[i] = {
@@ -33,5 +34,6 @@ exports.processFormulas = (
         }
       }
     }
+  }
   return single ? rows[0] : rows
 }

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
 
 /**

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
+ * Same as the doesContainString function, but will check for all the strings
- * JS the JS will be decoded and then the supplied string will be looked for. For example
+ * before confirming it contains.
- * if the template "Hello, your name is {{ related }}" this function would return that true
+ * @param {string} template The template string to search.
- * for the string "related" but not for "name" as it is not within the handlebars statement.
+ * @param {string[]} strings The strings to look for.
- * @param {string} template A template string to search for handlebars instances.
+ * @returns {boolean} Will return true if all strings found in HBS statement.
- * @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) => {
+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])
+}

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
 
 /**