From 72bf66926d59483a4ded2300862d16010148219e Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Mon, 22 Nov 2021 15:26:24 +0000
Subject: [PATCH 1/2] Updating UI to have a table/column deletion check which
 makes sure the user understands what they are doing, also disabling external
 table deletion.

---
 packages/bbui/src/Form/Core/TextField.svelte  |  2 +
 packages/bbui/src/Form/Input.svelte           |  2 +
 .../cypress/integration/createTable.spec.js   |  2 +
 .../DataTable/modals/CreateEditColumn.svelte  | 16 ++++++-
 .../popovers/EditTablePopover.svelte          | 47 +++++++++++++------
 .../pages/builder/portal/apps/index.svelte    |  6 ++-
 packages/builder/src/stores/backend/tables.js | 11 +++--
 .../src/api/controllers/table/external.js     |  9 +++-
 8 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/packages/bbui/src/Form/Core/TextField.svelte b/packages/bbui/src/Form/Core/TextField.svelte
index 37989a291e..85c71338f7 100644
--- a/packages/bbui/src/Form/Core/TextField.svelte
+++ b/packages/bbui/src/Form/Core/TextField.svelte
@@ -11,6 +11,7 @@
   export let readonly = false
   export let updateOnChange = true
   export let quiet = false
+  export let dataCy
 
   const dispatch = createEventDispatcher()
   let focus = false
@@ -78,6 +79,7 @@
     {disabled}
     {readonly}
     {id}
+    data-cy={dataCy}
     value={value || ""}
     placeholder={placeholder || ""}
     on:click
diff --git a/packages/bbui/src/Form/Input.svelte b/packages/bbui/src/Form/Input.svelte
index d1dbaff50b..1660b67799 100644
--- a/packages/bbui/src/Form/Input.svelte
+++ b/packages/bbui/src/Form/Input.svelte
@@ -13,6 +13,7 @@
   export let error = null
   export let updateOnChange = true
   export let quiet = false
+  export let dataCy
 
   const dispatch = createEventDispatcher()
   const onChange = e => {
@@ -23,6 +24,7 @@
 
 <Field {label} {labelPosition} {error}>
   <TextField
+    {dataCy}
     {updateOnChange}
     {error}
     {disabled}
diff --git a/packages/builder/cypress/integration/createTable.spec.js b/packages/builder/cypress/integration/createTable.spec.js
index 96a1bd75aa..20ec919b57 100644
--- a/packages/builder/cypress/integration/createTable.spec.js
+++ b/packages/builder/cypress/integration/createTable.spec.js
@@ -53,6 +53,7 @@ context("Create a Table", () => {
     cy.get(".spectrum-Table-editIcon > use").click()
     cy.contains("Delete").click()
     cy.wait(50)
+    cy.get(`[data-cy="delete-column-confirm"]`).type("nameupdated")
     cy.contains("Delete Column").click()
     cy.contains("nameupdated").should("not.exist")
   })
@@ -66,6 +67,7 @@ context("Create a Table", () => {
         cy.get(".actions .spectrum-Icon").click({ force: true })
       })
     cy.get(".spectrum-Menu > :nth-child(2)").click()
+    cy.get(`[data-cy="delete-table-confirm"]`).type("dog")
     cy.contains("Delete Table").click()
     cy.contains("dog").should("not.exist")
   })
diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
index 83ed741b75..86ad625c2c 100644
--- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
+++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
@@ -62,6 +62,7 @@
   let indexes = [...($tables.selected.indexes || [])]
   let confirmDeleteDialog
   let deletion
+  let deleteColName
 
   $: checkConstraints(field)
   $: required = !!field?.constraints?.presence || primaryDisplay
@@ -408,9 +409,20 @@
 </ModalContent>
 <ConfirmDialog
   bind:this={confirmDeleteDialog}
-  body={`Are you sure you wish to delete this column? Your data will be deleted and this action cannot be undone.`}
   okText="Delete Column"
   onOk={deleteColumn}
   onCancel={hideDeleteDialog}
   title="Confirm Deletion"
-/>
+  disabled={deleteColName !== field.name}
+>
+  <p>
+    Are you sure you wish to delete the column <b>{field.name}?</b>
+    Your data will be deleted and this action cannot be undone - enter the column
+    name to confirm.
+  </p>
+  <Input
+    dataCy="delete-column-confirm"
+    bind:value={deleteColName}
+    placeholder={field.name}
+  />
+</ConfirmDialog>
diff --git a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
index 04094b881a..0c0f8efe4f 100644
--- a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
+++ b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
@@ -21,8 +21,10 @@
   let originalName = table.name
   let templateScreens
   let willBeDeleted
+  let deleteTableName
 
   $: external = table?.type === "external"
+  $: allowDeletion = !external || table?.created
 
   function showDeleteModal() {
     templateScreens = $allScreens.filter(
@@ -36,15 +38,19 @@
 
   async function deleteTable() {
     const wasSelectedTable = $tables.selected
-    await tables.delete(table)
-    store.actions.screens.delete(templateScreens)
-    await tables.fetch()
-    notifications.success("Table deleted")
-    if (table.type === "external") {
-      await datasources.fetch()
-    }
-    if (wasSelectedTable && wasSelectedTable._id === table._id) {
-      $goto("./table")
+    try {
+      await tables.delete(table)
+      await store.actions.screens.delete(templateScreens)
+      await tables.fetch()
+      if (table.type === "external") {
+        await datasources.fetch()
+      }
+      notifications.success("Table deleted")
+      if (wasSelectedTable && wasSelectedTable._id === table._id) {
+        $goto("./table")
+      }
+    } catch (err) {
+      notifications.error(err)
     }
   }
 
@@ -67,7 +73,9 @@
     <Icon s hoverable name="MoreSmallList" />
   </div>
   <MenuItem icon="Edit" on:click={editorModal.show}>Edit</MenuItem>
-  <MenuItem icon="Delete" on:click={showDeleteModal}>Delete</MenuItem>
+  {#if allowDeletion}
+    <MenuItem icon="Delete" on:click={showDeleteModal}>Delete</MenuItem>
+  {/if}
 </ActionMenu>
 
 <Modal bind:this={editorModal}>
@@ -91,10 +99,13 @@
   okText="Delete Table"
   onOk={deleteTable}
   title="Confirm Deletion"
+  disabled={deleteTableName !== table.name}
 >
-  Are you sure you wish to delete the table
-  <i>{table.name}?</i>
-  The following will also be deleted:
+  <p>
+    Are you sure you wish to delete the table
+    <b>{table.name}?</b>
+    The following will also be deleted:
+  </p>
   <b>
     <div class="delete-items">
       {#each willBeDeleted as item}
@@ -102,7 +113,15 @@
       {/each}
     </div>
   </b>
-  This action cannot be undone.
+  <p>
+    This action cannot be undone - to continue please enter the table name below
+    to confirm.
+  </p>
+  <Input
+    bind:value={deleteTableName}
+    placeholder={table.name}
+    dataCy="delete-table-confirm"
+  />
 </ConfirmDialog>
 
 <style>
diff --git a/packages/builder/src/pages/builder/portal/apps/index.svelte b/packages/builder/src/pages/builder/portal/apps/index.svelte
index c238356915..da0d977a42 100644
--- a/packages/builder/src/pages/builder/portal/apps/index.svelte
+++ b/packages/builder/src/pages/builder/portal/apps/index.svelte
@@ -305,7 +305,11 @@
   Are you sure you want to delete the app <b>{selectedApp?.name}</b>?
 
   <p>Please enter the app name below to confirm.</p>
-  <Input bind:value={appName} data-cy="delete-app-confirmation" />
+  <Input
+    bind:value={appName}
+    data-cy="delete-app-confirmation"
+    placeholder={selectedApp?.name}
+  />
 </ConfirmDialog>
 <ConfirmDialog
   bind:this={unpublishModal}
diff --git a/packages/builder/src/stores/backend/tables.js b/packages/builder/src/stores/backend/tables.js
index 781746fed4..728b4e3d6d 100644
--- a/packages/builder/src/stores/backend/tables.js
+++ b/packages/builder/src/stores/backend/tables.js
@@ -1,5 +1,5 @@
-import { writable, get } from "svelte/store"
-import { views, queries, datasources } from "./"
+import { get, writable } from "svelte/store"
+import { datasources, queries, views } from "./"
 import { cloneDeep } from "lodash/fp"
 import api from "builderStore/api"
 import { SWITCHABLE_TYPES } from "../../constants/backend"
@@ -97,7 +97,12 @@ export function createTablesStore() {
       })
     },
     delete: async table => {
-      await api.delete(`/api/tables/${table._id}/${table._rev}`)
+      const response = await api.delete(
+        `/api/tables/${table._id}/${table._rev}`
+      )
+      if (response.status !== 200) {
+        throw (await response.json()).message
+      }
       update(state => ({
         ...state,
         list: state.list.filter(existing => existing._id !== table._id),
diff --git a/packages/server/src/api/controllers/table/external.js b/packages/server/src/api/controllers/table/external.js
index 8e5d390f4e..2453ca7a37 100644
--- a/packages/server/src/api/controllers/table/external.js
+++ b/packages/server/src/api/controllers/table/external.js
@@ -161,9 +161,13 @@ function isRelationshipSetup(column) {
 exports.save = async function (ctx) {
   const appId = ctx.appId
   const table = ctx.request.body
-  // can't do this
+  // can't do this right now
   delete table.dataImport
   const datasourceId = getDatasourceId(ctx.request.body)
+  // table doesn't exist already, note that it is created
+  if (!table._id) {
+    table.created = true
+  }
   let tableToSave = {
     type: "table",
     _id: buildExternalTableId(datasourceId, table.name),
@@ -265,6 +269,9 @@ exports.save = async function (ctx) {
 exports.destroy = async function (ctx) {
   const appId = ctx.appId
   const tableToDelete = await getTable(appId, ctx.params.tableId)
+  if (!tableToDelete || !tableToDelete.created) {
+    ctx.throw(400, "Cannot delete tables which weren't created in Budibase.")
+  }
   const datasourceId = getDatasourceId(tableToDelete)
 
   const db = new CouchDB(appId)

From 9dfa8c76514a2f6b7e224e2546803a9fab96d117 Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Mon, 22 Nov 2021 17:42:41 +0000
Subject: [PATCH 2/2] Fixing issue from review, values weren't wiped when
 delete modals closed, also adding a fix for threading to disable it in
 development as node-ts was causing memory leaks on low memory systems
 (doesn't apply to production built version).

---
 .../DataTable/modals/CreateEditColumn.svelte      |  1 +
 .../popovers/EditTablePopover.svelte              |  5 +++++
 packages/server/scripts/dev/manage.js             |  1 +
 packages/server/src/environment.js                | 15 +++++++++++++++
 packages/server/src/threads/automation.js         |  4 +++-
 packages/server/src/threads/index.js              | 12 +++++++++---
 packages/server/src/threads/query.js              |  3 +++
 7 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
index 86ad625c2c..841e781cf2 100644
--- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
+++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte
@@ -180,6 +180,7 @@
 
   function hideDeleteDialog() {
     confirmDeleteDialog.hide()
+    deleteColName = ""
     deletion = false
   }
 
diff --git a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
index 0c0f8efe4f..6d8c5811ec 100644
--- a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
+++ b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte
@@ -54,6 +54,10 @@
     }
   }
 
+  function hideDeleteDialog() {
+    deleteTableName = ""
+  }
+
   async function save() {
     await tables.save(table)
     notifications.success("Table renamed successfully")
@@ -98,6 +102,7 @@
   bind:this={confirmDeleteDialog}
   okText="Delete Table"
   onOk={deleteTable}
+  onCancel={hideDeleteDialog}
   title="Confirm Deletion"
   disabled={deleteTableName !== table.name}
 >
diff --git a/packages/server/scripts/dev/manage.js b/packages/server/scripts/dev/manage.js
index bd91056f84..f3938ad8ee 100644
--- a/packages/server/scripts/dev/manage.js
+++ b/packages/server/scripts/dev/manage.js
@@ -50,6 +50,7 @@ async function init() {
       SELF_HOSTED: 1,
       DISABLE_ACCOUNT_PORTAL: "",
       MULTI_TENANCY: "",
+      DISABLE_THREADING: 1,
     }
     let envFile = ""
     Object.keys(envFileJson).forEach(key => {
diff --git a/packages/server/src/environment.js b/packages/server/src/environment.js
index 4044bb3787..925fede086 100644
--- a/packages/server/src/environment.js
+++ b/packages/server/src/environment.js
@@ -23,6 +23,8 @@ if (!LOADED && isDev() && !isTest()) {
   LOADED = true
 }
 
+let inThread = false
+
 module.exports = {
   // important
   PORT: process.env.PORT,
@@ -62,6 +64,7 @@ module.exports = {
   USERID_API_KEY: process.env.USERID_API_KEY,
   DEPLOYMENT_CREDENTIALS_URL: process.env.DEPLOYMENT_CREDENTIALS_URL,
   ALLOW_DEV_AUTOMATIONS: process.env.ALLOW_DEV_AUTOMATIONS,
+  DISABLE_THREADING: process.env.DISABLE_THREADING,
   _set(key, value) {
     process.env[key] = value
     module.exports[key] = value
@@ -72,6 +75,18 @@ module.exports = {
   isProd: () => {
     return !isDev()
   },
+  // used to check if already in a thread, don't thread further
+  setInThread: () => {
+    inThread = true
+  },
+  isInThread: () => {
+    return inThread
+  },
+}
+
+// threading can cause memory issues with node-ts in development
+if (isDev() && module.exports.DISABLE_THREADING == null) {
+  module.exports._set("DISABLE_THREADING", "1")
 }
 
 // clean up any environment variable edge cases
diff --git a/packages/server/src/threads/automation.js b/packages/server/src/threads/automation.js
index 1cd751e895..11ee28dbe8 100644
--- a/packages/server/src/threads/automation.js
+++ b/packages/server/src/threads/automation.js
@@ -1,3 +1,6 @@
+// when thread starts, make sure it is recorded
+const env = require("../environment")
+env.setInThread()
 const actions = require("../automations/actions")
 const automationUtils = require("../automations/automationUtils")
 const AutomationEmitter = require("../events/AutomationEmitter")
@@ -6,7 +9,6 @@ const { DEFAULT_TENANT_ID } = require("@budibase/auth").constants
 const CouchDB = require("../db")
 const { DocumentTypes, isDevAppID } = require("../db/utils")
 const { doInTenant } = require("@budibase/auth/tenancy")
-const env = require("../environment")
 const usage = require("../utilities/usageQuota")
 const { definitions: triggerDefs } = require("../automations/triggerInfo")
 
diff --git a/packages/server/src/threads/index.js b/packages/server/src/threads/index.js
index e24b45bbfa..94571c31d1 100644
--- a/packages/server/src/threads/index.js
+++ b/packages/server/src/threads/index.js
@@ -24,10 +24,16 @@ function typeToFile(type) {
 class Thread {
   constructor(type, opts = { timeoutMs: null, count: 1 }) {
     this.type = type
-    if (!env.isTest()) {
+    this.count = opts.count ? opts.count : 1
+    this.disableThreading =
+      env.isTest() ||
+      env.DISABLE_THREADING ||
+      this.count === 0 ||
+      env.isInThread()
+    if (!this.disableThreading) {
       const workerOpts = {
         autoStart: true,
-        maxConcurrentWorkers: opts.count ? opts.count : 1,
+        maxConcurrentWorkers: this.count,
       }
       if (opts.timeoutMs) {
         workerOpts.maxCallTime = opts.timeoutMs
@@ -40,7 +46,7 @@ class Thread {
     return new Promise((resolve, reject) => {
       let fncToCall
       // if in test then don't use threading
-      if (env.isTest()) {
+      if (this.disableThreading) {
         fncToCall = require(typeToFile(this.type))
       } else {
         fncToCall = this.workers
diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js
index ca8bf2161c..6b0d021e94 100644
--- a/packages/server/src/threads/query.js
+++ b/packages/server/src/threads/query.js
@@ -1,3 +1,6 @@
+// when thread starts, make sure it is recorded
+const env = require("../environment")
+env.setInThread()
 const ScriptRunner = require("../utilities/scriptRunner")
 const { integrations } = require("../integrations")