Refactor grid row actions to be more explicit and remove extraneous flags

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,
       })
     },
   }

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()
       }
     }

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)
     }
   })
 }