Merge pull request #706 from Budibase/bug/table-changes-fix

Bug/table changes fix

packages/server/src/api/controllers/model.js

@@ -33,6 +33,7 @@ exports.save = async function(ctx) {
     views: {},
     ...rest,
   }
+  let renameDocs = []
 
   // if the model obj had an _id then it will have been retrieved
   const oldModel = ctx.preExisting
@@ -49,14 +50,11 @@ exports.save = async function(ctx) {
         include_docs: true,
       })
     )
-
-    const docs = records.rows.map(({ doc }) => {
+    renameDocs = records.rows.map(({ doc }) => {
       doc[_rename.updated] = doc[_rename.old]
       delete doc[_rename.old]
       return doc
     })
-
-    await db.bulkDocs(docs)
     delete modelToSave._rename
   }
 
@@ -69,9 +67,6 @@ exports.save = async function(ctx) {
     modelView.schema = modelToSave.schema
   }
 
-  const result = await db.post(modelToSave)
-  modelToSave._rev = result.rev
-
   // update linked records
   await linkRecords.updateLinks({
     instanceId,
@@ -82,6 +77,14 @@ exports.save = async function(ctx) {
     oldModel: oldModel,
   })
 
+  // don't perform any updates until relationships have been
+  // checked by the updateLinks function
+  if (renameDocs.length !== 0) {
+    await db.bulkDocs(renameDocs)
+  }
+  const result = await db.post(modelToSave)
+  modelToSave._rev = result.rev
+
   ctx.eventEmitter &&
     ctx.eventEmitter.emitModel(`model:save`, instanceId, modelToSave)
 
@@ -105,11 +108,8 @@ exports.save = async function(ctx) {
 exports.destroy = async function(ctx) {
   const instanceId = ctx.user.instanceId
   const db = new CouchDB(instanceId)
-
   const modelToDelete = await db.get(ctx.params.modelId)
 
-  await db.remove(modelToDelete)
-
   // Delete all records for that model
   const records = await db.allDocs(
     getRecordParams(ctx.params.modelId, null, {
@@ -117,7 +117,7 @@ exports.destroy = async function(ctx) {
     })
   )
   await db.bulkDocs(
-    records.rows.map(record => ({ _id: record.id, _deleted: true }))
+    records.rows.map(record => ({ ...record.doc, _deleted: true }))
   )
 
   // update linked records
@@ -127,6 +127,9 @@ exports.destroy = async function(ctx) {
     model: modelToDelete,
   })
 
+  // don't remove the table itself until very end
+  await db.remove(modelToDelete)
+
   ctx.eventEmitter &&
     ctx.eventEmitter.emitModel(`model:delete`, instanceId, modelToDelete)
   ctx.status = 200

packages/server/src/db/linkedRecords/LinkController.js

@@ -161,7 +161,7 @@ class LinkController {
           })
         // now add the docs to be deleted to the bulk operation
         operations.push(...toDeleteDocs)
-        // replace this field with a simple entry to denote there are links
+        // remove the field from this row, link doc will be added to record on way out
         delete record[fieldName]
       }
     }
@@ -234,8 +234,16 @@ class LinkController {
     for (let fieldName of Object.keys(schema)) {
       const field = schema[fieldName]
       if (field.type === "link") {
+        // handle this in a separate try catch, want
+        // the put to bubble up as an error, if can't update
+        // table for some reason
+        let linkedModel
+        try {
+          linkedModel = await this._db.get(field.modelId)
+        } catch (err) {
+          continue
+        }
         // create the link field in the other model
-        const linkedModel = await this._db.get(field.modelId)
         linkedModel.schema[field.fieldName] = {
           name: field.fieldName,
           type: "link",

packages/server/src/db/linkedRecords/index.js

@@ -42,6 +42,7 @@ exports.updateLinks = async function({
   model,
   oldModel,
 }) {
+  const baseReturnObj = record == null ? model : record
   if (instanceId == null) {
     throw "Cannot operate without an instance ID."
   }
@@ -50,12 +51,16 @@ exports.updateLinks = async function({
     arguments[0].modelId = model._id
   }
   let linkController = new LinkController(arguments[0])
-  if (
-    !(await linkController.doesModelHaveLinkedFields()) &&
-    (oldModel == null ||
-      !(await linkController.doesModelHaveLinkedFields(oldModel)))
-  ) {
-    return record
+  try {
+    if (
+      !(await linkController.doesModelHaveLinkedFields()) &&
+      (oldModel == null ||
+        !(await linkController.doesModelHaveLinkedFields(oldModel)))
+    ) {
+      return baseReturnObj
+    }
+  } catch (err) {
+    return baseReturnObj
   }
   switch (eventType) {
     case EventType.RECORD_SAVE:

packages/server/src/db/utils.js

@@ -57,21 +57,26 @@ exports.generateModelID = () => {
 
 /**
  * Gets the DB allDocs/query params for retrieving a record.
- * @param {string} modelId The model in which the records have been stored.
+ * @param {string|null} modelId The model in which the records have been stored.
  * @param {string|null} recordId The ID of the record which is being specifically queried for. This can be
  * left null to get all the records in the model.
  * @param {object} otherProps Any other properties to add to the request.
  * @returns {object} Parameters which can then be used with an allDocs request.
  */
-exports.getRecordParams = (modelId, recordId = null, otherProps = {}) => {
+exports.getRecordParams = (
+  modelId = null,
+  recordId = null,
+  otherProps = {}
+) => {
   if (modelId == null) {
-    throw "Cannot build params for records without a model ID"
+    return getDocParams(DocumentTypes.RECORD, null, otherProps)
+  } else {
+    const endOfKey =
+      recordId == null
+        ? `${modelId}${SEPARATOR}`
+        : `${modelId}${SEPARATOR}${recordId}`
+    return getDocParams(DocumentTypes.RECORD, endOfKey, otherProps)
   }
-  const endOfKey =
-    recordId == null
-      ? `${modelId}${SEPARATOR}`
-      : `${modelId}${SEPARATOR}${recordId}`
-  return getDocParams(DocumentTypes.RECORD, endOfKey, otherProps)
 }
 
 /**