Overhaul of CreateEditRelationship modal

packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte

@@ -10,7 +10,6 @@
   } from "@budibase/bbui"
   import { tables } from "stores/backend"
   import { Helpers } from "@budibase/bbui"
-  import { writable } from "svelte/store"
 
   export let save
   export let datasource
@@ -18,47 +17,95 @@
   export let fromRelationship = {}
   export let toRelationship = {}
   export let close
-  export let selectedFromTable
 
-  let originalFromName = fromRelationship.name,
+  const colNotSet = "Please specify a column name"
-    originalToName = toRelationship.name
+  const relationshipTypes = [
+    {
+      label: "One to Many",
+      value: RelationshipTypes.MANY_TO_ONE,
+    },
+    {
+      label: "Many to Many",
+      value: RelationshipTypes.MANY_TO_MANY,
+    },
+  ]
+
+  let originalFromColumnName = fromRelationship.name,
+    originalToColumnName = toRelationship.name
   let originalFromTable = plusTables.find(
     table => table._id === toRelationship?.tableId
   )
   let originalToTable = plusTables.find(
     table => table._id === fromRelationship?.tableId
   )
-  let fromTable, toTable, through, linkTable, tableOptions
-  let isManyToMany, isManyToOne, relationshipTypes
-  let errors, valid
-  let currentTables = {}
 
-  if (fromRelationship && !fromRelationship.relationshipType) {
+  let tableOptions
-    fromRelationship.relationshipType = RelationshipTypes.MANY_TO_ONE
+  let errors = {}
+  let hasClickedSave = !!fromRelationship.relationshipType
+  let fromPrimary,
+    fromForeign,
+    fromTable,
+    toTable,
+    throughTable,
+    fromColumn,
+    toColumn
+  let fromId, toId, throughId, throughToKey, throughFromKey
+  let isManyToMany, isManyToOne, relationshipType
+
+  $: {
+    if (!fromPrimary) {
+      fromPrimary = fromRelationship.foreignKey
+      fromForeign = toRelationship.foreignKey
+    }
+    if (!fromColumn && !errors.fromColumn) {
+      fromColumn = toRelationship.name
+    }
+    if (!toColumn && !errors.toColumn) {
+      toColumn = fromRelationship.name
+    }
+    if (!fromId) {
+      fromId = toRelationship.tableId
+    }
+    if (!toId) {
+      toId = fromRelationship.tableId
+    }
+    if (!throughId) {
+      throughId = fromRelationship.through
+      throughFromKey = fromRelationship.throughFrom
+      throughToKey = fromRelationship.throughTo
+    }
+    if (!relationshipType) {
+      relationshipType = fromRelationship.relationshipType
+    }
   }
 
-  if (toRelationship && selectedFromTable) {
+  $: tableOptions = plusTables.map(table => ({
-    toRelationship.tableId = selectedFromTable._id
+    label: table.name,
-  }
+    value: table._id,
+  }))
+  $: valid = getErrorCount(errors) === 0 || !hasClickedSave
 
-  function inSchema(table, prop, ogName) {
+  $: isManyToMany = relationshipType === RelationshipTypes.MANY_TO_MANY
-    if (!table || !prop || prop === ogName) {
+  $: isManyToOne = relationshipType === RelationshipTypes.MANY_TO_ONE
-      return false
+  $: fromTable = plusTables.find(table => table._id === fromId)
-    }
+  $: toTable = plusTables.find(table => table._id === toId)
-    const keys = Object.keys(table.schema).map(key => key.toLowerCase())
+  $: throughTable = plusTables.find(table => table._id === throughId)
-    return keys.indexOf(prop.toLowerCase()) !== -1
-  }
 
-  const touched = writable({})
+  $: toRelationship.relationshipType = fromRelationship?.relationshipType
 
-  function invalidThroughTable({ through, throughTo, throughFrom }) {
+  const getErrorCount = errors =>
+    Object.entries(errors)
+      .filter(entry => !!entry[1])
+      .map(entry => entry[0]).length
+
+  function invalidThroughTable() {
     // need to know the foreign key columns to check error
-    if (!through || !throughTo || !throughFrom) {
+    if (!throughId || !throughToKey || !throughFromKey) {
       return false
     }
-    const throughTable = plusTables.find(tbl => tbl._id === through)
+    const throughTbl = plusTables.find(tbl => tbl._id === throughId)
-    const otherColumns = Object.values(throughTable.schema).filter(
+    const otherColumns = Object.values(throughTbl.schema).filter(
-      col => col.name !== throughFrom && col.name !== throughTo
+      col => col.name !== throughFromKey && col.name !== throughToKey
     )
     for (let col of otherColumns) {
       if (col.constraints?.presence && !col.autocolumn) {
@@ -68,145 +115,134 @@
     return false
   }
 
-  function checkForErrors(fromRelate, toRelate) {
+  function validate() {
-    const isMany =
+    const isMany = relationshipType === RelationshipTypes.MANY_TO_MANY
-      fromRelate.relationshipType === RelationshipTypes.MANY_TO_MANY
     const tableNotSet = "Please specify a table"
+    const foreignKeyNotSet = "Please pick a foreign key"
     const errObj = {}
-    if ($touched.from && !fromTable) {
+    if (!relationshipType) {
-      errObj.from = tableNotSet
+      errObj.relationshipType = "Please specify a relationship type"
     }
-    if ($touched.to && !toTable) {
+    if (!fromTable) {
-      errObj.to = tableNotSet
+      errObj.fromTable = tableNotSet
     }
-    if ($touched.through && isMany && !fromRelate.through) {
+    if (!toTable) {
-      errObj.through = tableNotSet
+      errObj.toTable = tableNotSet
     }
-    if ($touched.through && invalidThroughTable(fromRelate)) {
+    if (isMany && !throughTable) {
-      errObj.through = "Ensure non-key columns are nullable or auto-generated"
+      errObj.throughTable = tableNotSet
     }
-    if ($touched.foreign && !isMany && !fromRelate.fieldName) {
+    if (isMany && !throughFromKey) {
-      errObj.foreign = "Please pick the foreign key"
+      errObj.throughFromKey = foreignKeyNotSet
     }
-    const colNotSet = "Please specify a column name"
+    if (isMany && !throughToKey) {
-    if ($touched.fromCol && !toRelate.name) {
+      errObj.throughToKey = foreignKeyNotSet
-      errObj.fromCol = colNotSet
     }
-    if ($touched.toCol && !fromRelate.name) {
+    if (invalidThroughTable()) {
-      errObj.toCol = colNotSet
+      errObj.throughTable =
+        "Ensure non-key columns are nullable or auto-generated"
     }
-    if ($touched.primary && !isMany && !fromPrimary) {
+    if (!isMany && !fromForeign) {
-      errObj.primary = "Please pick the primary key"
+      errObj.fromForeign = foreignKeyNotSet
     }
+    if (!fromColumn) {
+      errObj.fromColumn = colNotSet
+    }
+    if (!toColumn) {
+      errObj.toColumn = colNotSet
+    }
+    if (!isMany && !fromPrimary) {
+      errObj.fromPrimary = "Please pick the primary key"
+    }
+
     // currently don't support relationships back onto the table itself, needs to relate out
     const tableError = "From/to/through tables must be different"
-    if (fromTable && (fromTable === toTable || fromTable === through)) {
+    if (fromTable && (fromTable === toTable || fromTable === throughTable)) {
-      errObj.from = tableError
+      errObj.fromTable = tableError
     }
-    if (toTable && (toTable === fromTable || toTable === through)) {
+    if (toTable && (toTable === fromTable || toTable === throughTable)) {
-      errObj.to = tableError
+      errObj.toTable = tableError
     }
-    if (through && (through === fromTable || through === toTable)) {
+    if (
-      errObj.through = tableError
+      throughTable &&
+      (throughTable === fromTable || throughTable === toTable)
+    ) {
+      errObj.throughTable = tableError
     }
     const colError = "Column name cannot be an existing column"
-    if (inSchema(fromTable, fromRelate.name, originalFromName)) {
+    if (isColumnNameBeingUsed(fromTable, fromColumn, originalFromColumnName)) {
-      errObj.fromCol = colError
+      errObj.fromColumn = colError
     }
-    if (inSchema(toTable, toRelate.name, originalToName)) {
+    if (isColumnNameBeingUsed(toTable, toColumn, originalToColumnName)) {
-      errObj.toCol = colError
+      errObj.toColumn = colError
     }
 
     let fromType, toType
-    if (fromPrimary && fromRelate.fieldName) {
+    if (fromPrimary && fromForeign) {
       fromType = fromTable?.schema[fromPrimary]?.type
-      toType = toTable?.schema[fromRelate.fieldName]?.type
+      toType = toTable?.schema[fromForeign]?.type
     }
     if (fromType && toType && fromType !== toType) {
-      errObj.foreign =
+      errObj.fromForeign =
         "Column type of the foreign key must match the primary key"
     }
+
     errors = errObj
+    return getErrorCount(errors) === 0
   }
 
-  let fromPrimary
+  function isColumnNameBeingUsed(table, columnName, originalName) {
-  $: {
+    if (!table || !columnName || columnName === originalName) {
-    if (!fromPrimary && fromTable) {
+      return false
-      fromPrimary = fromRelationship?.foreignKey
-    }
-  }
-  $: isManyToMany =
-    fromRelationship?.relationshipType === RelationshipTypes.MANY_TO_MANY
-  $: isManyToOne =
-    fromRelationship?.relationshipType === RelationshipTypes.MANY_TO_ONE
-  $: tableOptions = plusTables.map(table => ({
-    label: table.name,
-    value: table._id,
-  }))
-  $: fromTable = plusTables.find(table => table._id === toRelationship?.tableId)
-  $: toTable = plusTables.find(table => table._id === fromRelationship?.tableId)
-  $: through = plusTables.find(table => table._id === fromRelationship?.through)
-  $: checkForErrors(fromRelationship, toRelationship)
-  $: valid =
-    Object.keys(errors).length === 0 &&
-    Object.keys($touched).length !== 0 &&
-    fromTable &&
-    toTable
-
-  $: linkTable = through || toTable
-  $: relationshipTypes = [
-    {
-      label: "Many",
-      value: RelationshipTypes.MANY_TO_MANY,
-    },
-    {
-      label: "One",
-      value: RelationshipTypes.MANY_TO_ONE,
-    },
-  ]
-  $: updateRelationshipType(fromRelationship?.relationshipType)
-  $: tableChanged(fromTable, toTable)
-
-  function updateRelationshipType(fromType) {
-    if (fromType === RelationshipTypes.MANY_TO_MANY) {
-      toRelationship.relationshipType = RelationshipTypes.MANY_TO_MANY
-    } else {
-      toRelationship.relationshipType = RelationshipTypes.MANY_TO_ONE
     }
+    const keys = Object.keys(table.schema).map(key => key.toLowerCase())
+    return keys.indexOf(columnName.toLowerCase()) !== -1
   }
 
   function buildRelationships() {
-    // if any to many only need to check from
-    const manyToMany =
-      fromRelationship.relationshipType === RelationshipTypes.MANY_TO_MANY
-    // main is simply used to know this is the side the user configured it from
     const id = Helpers.uuid()
-    if (!manyToMany) {
+    //Map temporary variables
-      delete fromRelationship.through
-      delete toRelationship.through
-    }
     let relateFrom = {
       ...fromRelationship,
+      tableId: toId,
+      name: toColumn,
+      relationshipType,
+      fieldName: fromForeign,
+      through: throughId,
+      throughFrom: throughFromKey,
+      throughTo: throughToKey,
       type: "link",
       main: true,
       _id: id,
     }
-    let relateTo = {
+    let relateTo = (toRelationship = {
       ...toRelationship,
+      tableId: fromId,
+      name: fromColumn,
+      through: throughId,
       type: "link",
       _id: id,
+    })
+
+    // if any to many only need to check from
+    const manyToMany =
+      relateFrom.relationshipType === RelationshipTypes.MANY_TO_MANY
+
+    if (!manyToMany) {
+      delete relateFrom.through
+      delete relateTo.through
     }
 
     // [0] is because we don't support composite keys for relationships right now
     if (manyToMany) {
       relateFrom = {
         ...relateFrom,
-        through: through._id,
+        through: throughTable._id,
         fieldName: toTable.primary[0],
       }
       relateTo = {
         ...relateTo,
-        through: through._id,
+        through: throughTable._id,
         fieldName: fromTable.primary[0],
         throughFrom: relateFrom.throughTo,
         throughTo: relateFrom.throughFrom,
@@ -235,9 +271,27 @@
     toRelationship = relateTo
   }
 
-  // save the relationship on to the datasource
+  function removeExistingRelationship() {
+    if (originalFromTable && originalFromColumnName) {
+      delete datasource.entities[originalFromTable.name].schema[
+        originalFromColumnName
+      ]
+    }
+    if (originalToTable && originalToColumnName) {
+      delete datasource.entities[originalToTable.name].schema[
+        originalToColumnName
+      ]
+    }
+  }
+
   async function saveRelationship() {
+    hasClickedSave = true
+    if (!validate()) {
+      return false
+    }
     buildRelationships()
+    removeExistingRelationship()
+
     // source of relationship
     datasource.entities[fromTable.name].schema[fromRelationship.name] =
       fromRelationship
@@ -245,61 +299,14 @@
     datasource.entities[toTable.name].schema[toRelationship.name] =
       toRelationship
 
-    // If relationship has been renamed or a different table selected
-    if (
-      originalFromTable?.name &&
-      (originalFromName !== fromRelationship.name ||
-        hasTableChanged(fromTable, toTable))
-    ) {
-      delete datasource.entities[originalFromTable.name].schema[
-        originalFromName
-      ]
-    }
-    if (
-      originalToTable?.name &&
-      (originalToName !== toRelationship.name ||
-        hasTableChanged(fromTable, toTable))
-    ) {
-      delete datasource.entities[originalToTable.name].schema[originalToName]
-    }
-
-    // store the original names so it won't cause an error
-    originalToName = toRelationship.name
-    originalFromName = fromRelationship.name
     await save()
   }
-
   async function deleteRelationship() {
-    delete datasource.entities[originalFromTable.name].schema[originalFromName]
+    removeExistingRelationship()
-    delete datasource.entities[originalToTable.name].schema[originalToName]
     await save()
     await tables.fetch()
     close()
   }
-
-  function hasTableChanged(fromTbl, toTbl) {
-    const areRelationshipsSet =
-      (originalFromName || originalToName) &&
-      originalFromTable?.name === fromTbl?.name &&
-      originalToTable?.name === toTbl?.name
-
-    return (
-      currentTables?.from?._id !== fromTbl?._id ||
-      currentTables?.to?._id !== toTbl?._id ||
-      !areRelationshipsSet
-    )
-  }
-
-  function tableChanged(fromTbl, toTbl) {
-    if (!hasTableChanged(fromTbl, toTbl)) {
-      return
-    }
-    fromRelationship.name = toTbl?.name || ""
-    errors.fromCol = ""
-    toRelationship.name = fromTbl?.name || ""
-    errors.toCol = ""
-    currentTables = { from: fromTbl, to: toTbl }
-  }
 </script>
 
 <ModalContent
@@ -311,7 +318,9 @@
   <Select
     label="Relationship type"
     options={relationshipTypes}
-    bind:value={fromRelationship.relationshipType}
+    bind:value={relationshipType}
+    bind:error={errors.relationshipType}
+    on:change={e => (errors.relationshipType = null)}
   />
   <div class="headings">
     <Detail>Tables</Detail>
@@ -319,68 +328,64 @@
   <Select
     label="Select from table"
     options={tableOptions}
-    disabled={!!selectedFromTable}
+    bind:value={fromId}
-    on:change={() => {
+    bind:error={errors.fromTable}
-      $touched.from = true
+    on:change={e => {
-      $touched.primary = true
+      fromColumn = tableOptions.find(opt => opt.value === e.detail)?.label || ""
+      errors.fromTable = null
+      errors.fromColumn = null
     }}
-    bind:error={errors.from}
-    bind:value={toRelationship.tableId}
   />
   {#if isManyToOne && fromTable}
     <Select
-      label={`Primary Key (${fromTable?.name})`}
+      label={`Primary Key (${fromTable.name})`}
-      options={Object.keys(fromTable?.schema)}
+      options={Object.keys(fromTable.schema)}
-      on:change={() => ($touched.primary = true)}
-      bind:error={errors.primary}
       bind:value={fromPrimary}
+      bind:error={errors.fromPrimary}
+      on:change={e => (errors.fromPrimary = null)}
     />
   {/if}
   <Select
     label={"Select to table"}
     options={tableOptions}
-    on:change={() => {
+    bind:value={toId}
-      $touched.to = true
+    bind:error={errors.toTable}
-      $touched.foreign = true
+    on:change={e => {
+      toColumn = tableOptions.find(opt => opt.value === e.detail)?.label || ""
+      errors.toTable = null
+      errors.toColumn = null
     }}
-    bind:error={errors.to}
-    bind:value={fromRelationship.tableId}
   />
   {#if isManyToMany}
     <Select
       label={"Through"}
       options={tableOptions}
-      on:change={() => {
+      bind:value={throughId}
-        $touched.through = true
+      bind:error={errors.throughTable}
-        $touched.fromForeign = true
-        $touched.toForeign = true
-      }}
-      bind:error={errors.through}
-      bind:value={fromRelationship.through}
     />
-    {#if fromTable && toTable && through}
+    {#if fromTable && toTable && throughTable}
       <Select
         label={`Foreign Key (${fromTable?.name})`}
-        options={Object.keys(through?.schema)}
+        options={Object.keys(throughTable?.schema)}
-        on:change={() => ($touched.fromForeign = true)}
+        bind:value={throughToKey}
-        bind:error={errors.fromForeign}
+        bind:error={errors.throughToKey}
-        bind:value={fromRelationship.throughTo}
+        on:change={e => (errors.throughToKey = null)}
       />
       <Select
         label={`Foreign Key (${toTable?.name})`}
-        options={Object.keys(through?.schema)}
+        options={Object.keys(throughTable?.schema)}
-        on:change={() => ($touched.toForeign = true)}
+        bind:value={throughFromKey}
-        bind:error={errors.toForeign}
+        bind:error={errors.throughFromKey}
-        bind:value={fromRelationship.throughFrom}
+        on:change={e => (errors.throughFromKey = null)}
       />
     {/if}
   {:else if isManyToOne && toTable}
     <Select
       label={`Foreign Key (${toTable?.name})`}
       options={Object.keys(toTable?.schema)}
-      on:change={() => ($touched.foreign = true)}
+      bind:value={fromForeign}
-      bind:error={errors.foreign}
+      bind:error={errors.fromForeign}
-      bind:value={fromRelationship.fieldName}
+      on:change={e => (errors.fromForeign = null)}
     />
   {/if}
   <div class="headings">
@@ -391,21 +396,21 @@
     provide a name for these columns.
   </Body>
   <Input
-    on:blur={() => ($touched.fromCol = true)}
-    on:change={() => ($touched.fromCol = true)}
-    bind:error={errors.fromCol}
     label="From table column"
-    bind:value={toRelationship.name}
+    bind:value={fromColumn}
+    bind:error={errors.fromColumn}
+    on:change={e => {
+      errors.fromColumn = e.detail?.length > 0 ? null : colNotSet
+    }}
   />
   <Input
-    on:blur={() => ($touched.toCol = true)}
-    on:change={() => ($touched.toCol = true)}
-    bind:error={errors.toCol}
     label="To table column"
-    bind:value={fromRelationship.name}
+    bind:value={toColumn}
+    bind:error={errors.toColumn}
+    on:change={e => (errors.toColumn = e.detail?.length > 0 ? null : colNotSet)}
   />
   <div slot="footer">
-    {#if originalFromName != null}
+    {#if originalFromColumnName != null}
       <Button warning text on:click={deleteRelationship}>Delete</Button>
     {/if}
   </div>