Merge pull request #3477 from Budibase/fix/table-col-deletion

Table/column deletion checks and disabling fetched external table deletion

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

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}

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

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
@@ -179,6 +180,7 @@
 
   function hideDeleteDialog() {
     confirmDeleteDialog.hide()
+    deleteColName = ""
     deletion = false
   }
 
@@ -408,9 +410,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>

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,18 +38,26 @@
 
   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)
     }
   }
 
+  function hideDeleteDialog() {
+    deleteTableName = ""
+  }
+
   async function save() {
     await tables.save(table)
     notifications.success("Table renamed successfully")
@@ -67,7 +77,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}>
@@ -90,11 +102,15 @@
   bind:this={confirmDeleteDialog}
   okText="Delete Table"
   onOk={deleteTable}
+  onCancel={hideDeleteDialog}
   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 +118,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>

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}

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

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 => {

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)

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

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

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

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