From fb3032f9d11d827d1ee6891c21a6c5e772d56f91 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 1 Feb 2023 19:09:36 +0000 Subject: [PATCH 1/8] Re-working the error handling for the SQL relationship modal, as well as adding some better defaults for the majority of the options to make the UI a bit easier to use. --- .../Datasources/CreateEditRelationship.svelte | 263 ++++++++---------- .../backend/Datasources/relationshipErrors.js | 86 ++++++ 2 files changed, 208 insertions(+), 141 deletions(-) create mode 100644 packages/builder/src/components/backend/Datasources/relationshipErrors.js diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index e43437d756..68b51ef3a1 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -10,17 +10,17 @@ } from "@budibase/bbui" import { tables } from "stores/backend" import { Helpers } from "@budibase/bbui" + import { RelationshipErrorChecker } from "./relationshipErrors" + import { onMount } from "svelte" export let save export let datasource export let plusTables = [] export let fromRelationship = {} export let toRelationship = {} + export let selectedFromTable export let close - const colNotSet = "Please specify a column name" - const relationshipAlreadyExists = - "A relationship between these tables already exists." const relationshipTypes = [ { label: "One to Many", @@ -42,8 +42,11 @@ ) let tableOptions + let errorChecker = new RelationshipErrorChecker( + invalidThroughTable, + relationshipExists + ) let errors = {} - let hasClickedSave = !!fromRelationship.relationshipType let fromPrimary, fromForeign, fromTable, @@ -51,12 +54,19 @@ throughTable, fromColumn, toColumn - let fromId, toId, throughId, throughToKey, throughFromKey + let fromId = selectedFromTable?._id, + toId, + throughId, + throughToKey, + throughFromKey let isManyToMany, isManyToOne, relationshipType + let hasValidated = false $: { if (!fromPrimary) { fromPrimary = fromRelationship.foreignKey + } + if (!fromForeign) { fromForeign = toRelationship.foreignKey } if (!fromColumn && !errors.fromColumn) { @@ -77,7 +87,8 @@ throughToKey = fromRelationship.throughTo } if (!relationshipType) { - relationshipType = fromRelationship.relationshipType + relationshipType = + fromRelationship.relationshipType || RelationshipTypes.MANY_TO_ONE } } @@ -85,7 +96,7 @@ label: table.name, value: table._id, })) - $: valid = getErrorCount(errors) === 0 || !hasClickedSave + $: valid = getErrorCount(errors) === 0 && allRequiredAttributesSet() $: isManyToMany = relationshipType === RelationshipTypes.MANY_TO_MANY $: isManyToOne = relationshipType === RelationshipTypes.MANY_TO_ONE @@ -95,10 +106,20 @@ $: toRelationship.relationshipType = fromRelationship?.relationshipType - const getErrorCount = errors => - Object.entries(errors) + function getErrorCount(errors) { + return Object.entries(errors) .filter(entry => !!entry[1]) .map(entry => entry[0]).length + } + + function allRequiredAttributesSet() { + const base = fromTable && toTable && fromColumn && toColumn + if (isManyToOne) { + return base && fromPrimary && fromForeign + } else { + return base && throughTable && throughFromKey && throughToKey + } + } function invalidThroughTable() { // need to know the foreign key columns to check error @@ -118,93 +139,48 @@ } function validate() { - const isMany = relationshipType === RelationshipTypes.MANY_TO_MANY - const tableNotSet = "Please specify a table" - const foreignKeyNotSet = "Please pick a foreign key" + if (!allRequiredAttributesSet() && !hasValidated) { + return + } + hasValidated = true + errorChecker.setType(relationshipType) const errObj = {} - if (!relationshipType) { - errObj.relationshipType = "Please specify a relationship type" - } - if (!fromTable) { - errObj.fromTable = tableNotSet - } - if (!toTable) { - errObj.toTable = tableNotSet - } - if (isMany && !throughTable) { - errObj.throughTable = tableNotSet - } - if (isMany && !throughFromKey) { - errObj.throughFromKey = foreignKeyNotSet - } - if (isMany && !throughToKey) { - errObj.throughToKey = foreignKeyNotSet - } - if (invalidThroughTable()) { - errObj.throughTable = - "Ensure non-key columns are nullable or auto-generated" - } - if (!isMany && !fromForeign) { - errObj.fromForeign = foreignKeyNotSet - } - if (!fromColumn) { - errObj.fromColumn = colNotSet - } - if (!toColumn) { - errObj.toColumn = colNotSet - } - if (!isMany && !fromPrimary) { - errObj.fromPrimary = "Please pick the primary key" - } - if (isMany && relationshipExists()) { - errObj.fromTable = relationshipAlreadyExists - errObj.toTable = relationshipAlreadyExists - } - + errObj.relationshipType = errorChecker.relationshipTypeSet(relationshipType) + errObj.fromTable = errorChecker.tableSet(fromTable) + errObj.toTable = errorChecker.tableSet(toTable) + errObj.throughTable = errorChecker.throughTableSet(throughTable) + errObj.throughFromKey = errorChecker.manyForeignKeySet(throughFromKey) + errObj.throughToKey = errorChecker.manyForeignKeySet(throughToKey) + errObj.throughTable = errorChecker.throughIsNullable() + errObj.fromForeign = errorChecker.foreignKeySet(fromForeign) + errObj.fromPrimary = errorChecker.foreignKeySet(fromPrimary) + errObj.fromTable = errorChecker.doesRelationshipExists() + errObj.toTable = errorChecker.doesRelationshipExists() // 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 === throughTable)) { - errObj.fromTable = tableError - } - if (toTable && (toTable === fromTable || toTable === throughTable)) { - errObj.toTable = tableError - } - if ( - throughTable && - (throughTable === fromTable || throughTable === toTable) - ) { - errObj.throughTable = tableError - } - const colError = "Column name cannot be an existing column" - if (isColumnNameBeingUsed(toTable, fromColumn, originalFromColumnName)) { - errObj.fromColumn = colError - } - if (isColumnNameBeingUsed(fromTable, toColumn, originalToColumnName)) { - errObj.toColumn = colError - } - - let fromType, toType - if (fromPrimary && fromForeign) { - fromType = fromTable?.schema[fromPrimary]?.type - toType = toTable?.schema[fromForeign]?.type - } - if (fromType && toType && fromType !== toType) { - errObj.fromForeign = - "Column type of the foreign key must match the primary key" - } - + errObj.fromTable = errorChecker.differentTables(fromId, toId, throughId) + errObj.toTable = errorChecker.differentTables(toId, fromId, throughId) + errObj.throughTable = errorChecker.differentTables(throughId, fromId, toId) + errObj.fromColumn = errorChecker.columnBeingUsed( + toTable, + fromColumn, + originalFromColumnName + ) + errObj.toColumn = errorChecker.columnBeingUsed( + fromTable, + toColumn, + originalToColumnName + ) + errObj.fromForeign = errorChecker.typeMismatch( + fromTable, + toTable, + fromPrimary, + fromForeign + ) + console.log(errObj) errors = errObj return getErrorCount(errors) === 0 } - function isColumnNameBeingUsed(table, columnName, originalName) { - if (!table || !columnName || columnName === originalName) { - return false - } - const keys = Object.keys(table.schema).map(key => key.toLowerCase()) - return keys.indexOf(columnName.toLowerCase()) !== -1 - } - function buildRelationships() { const id = Helpers.uuid() //Map temporary variables @@ -320,7 +296,6 @@ } async function saveRelationship() { - hasClickedSave = true if (!validate()) { return false } @@ -342,6 +317,20 @@ await tables.fetch() close() } + + function changed(fn) { + if (fn && typeof fn === "function") { + fn() + } + validate() + } + + onMount(() => { + if (selectedFromTable) { + fromColumn = selectedFromTable.name + fromPrimary = selectedFromTable?.primary[0] || null + } + }) (errors.relationshipType = null)} + on:change={changed} />
Tables
- + changed(() => { + const table = plusTables.find(tbl => tbl._id === e.detail) + fromColumn = table?.name || "" + fromPrimary = table?.primary?.[0] + })} + /> + {/if} {#if isManyToOne && fromTable} { - toColumn = tableOptions.find(opt => opt.value === e.detail)?.label || "" - if (errors.toTable === relationshipAlreadyExists) { - errors.fromColumn = null - } - errors.toTable = null - errors.toColumn = null - errors.fromTable = null - errors.throughTable = null - }} + on:change={e => + changed(() => { + const table = plusTables.find(tbl => tbl._id === e.detail) + toColumn = table.name || "" + fromForeign = null + })} /> {#if isManyToMany} { - if (throughFromKey === e.detail) { - throughFromKey = null - } - errors.throughToKey = null - }} + on:change={e => + changed(() => { + if (throughFromKey === e.detail) { + throughFromKey = null + } + })} /> (errors.toColumn = e.detail?.length > 0 ? null : colNotSet)} + on:change={changed} />
{#if originalFromColumnName != null} diff --git a/packages/builder/src/components/backend/Datasources/relationshipErrors.js b/packages/builder/src/components/backend/Datasources/relationshipErrors.js new file mode 100644 index 0000000000..71fb634ada --- /dev/null +++ b/packages/builder/src/components/backend/Datasources/relationshipErrors.js @@ -0,0 +1,86 @@ +import { RelationshipTypes } from "constants/backend" + +export const typeMismatch = + "Column type of the foreign key must match the primary key" +export const columnCantExist = "Column name cannot be an existing column" +export const mustBeDifferentTables = "From/to/through tables must be different" +export const primaryKeyNotSet = "Please pick the primary key" +export const throughNotNullable = + "Ensure non-key columns are nullable or auto-generated" +export const noRelationshipType = "Please specify a relationship type" +export const tableNotSet = "Please specify a table" +export const foreignKeyNotSet = "Please pick a foreign key" +export const relationshipAlreadyExists = + "A relationship between these tables already exists" + +function isColumnNameBeingUsed(table, columnName, originalName) { + if (!table || !columnName || columnName === originalName) { + return false + } + const keys = Object.keys(table.schema).map(key => key.toLowerCase()) + return keys.indexOf(columnName.toLowerCase()) !== -1 +} + +export class RelationshipErrorChecker { + constructor(invalidThroughTableFn, relationshipExistsFn) { + this.invalidThroughTable = invalidThroughTableFn + this.relationshipExists = relationshipExistsFn + } + + setType(type) { + this.type = type + } + + isMany() { + return this.type === RelationshipTypes.MANY_TO_MANY + } + + relationshipTypeSet(type) { + return !type ? noRelationshipType : null + } + + tableSet(table) { + return !table ? tableNotSet : null + } + + throughTableSet(table) { + return this.isMany() && !table ? tableNotSet : null + } + + manyForeignKeySet(key) { + return this.isMany() && !key ? foreignKeyNotSet : null + } + + foreignKeySet(key) { + return !this.isMany() && !key ? foreignKeyNotSet : null + } + + throughIsNullable() { + return this.invalidThroughTable() ? throughNotNullable : null + } + + doesRelationshipExists() { + return this.isMany() && this.relationshipExists() + ? relationshipAlreadyExists + : null + } + + differentTables(table1, table2, table3) { + // currently don't support relationships back onto the table itself, needs to relate out + const error = table1 && (table1 === table2 || (table3 && table1 === table3)) + return error ? mustBeDifferentTables : null + } + + columnBeingUsed(table, column, ogName) { + return isColumnNameBeingUsed(table, column, ogName) ? columnCantExist : null + } + + typeMismatch(fromTable, toTable, primary, foreign) { + let fromType, toType + if (primary && foreign) { + fromType = fromTable?.schema[primary]?.type + toType = toTable?.schema[foreign]?.type + } + return fromType && toType && fromType !== toType ? typeMismatch : null + } +} From 70279e959c04595f775ecf948a32dda7d84f2152 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 1 Feb 2023 19:10:41 +0000 Subject: [PATCH 2/8] Removing console log. --- .../components/backend/Datasources/CreateEditRelationship.svelte | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 68b51ef3a1..1901053564 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -176,7 +176,6 @@ fromPrimary, fromForeign ) - console.log(errObj) errors = errObj return getErrorCount(errors) === 0 } From fc3e9be7531f612e5dca2709204e4194a87f7f1b Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 1 Feb 2023 19:26:26 +0000 Subject: [PATCH 3/8] Some final updates, clean up some code that could be causing reactive issues. --- .../Datasources/CreateEditRelationship.svelte | 136 ++++++++---------- .../backend/Datasources/relationshipErrors.js | 23 +-- 2 files changed, 72 insertions(+), 87 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 1901053564..57ac41a093 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -62,65 +62,18 @@ let isManyToMany, isManyToOne, relationshipType let hasValidated = false - $: { - if (!fromPrimary) { - fromPrimary = fromRelationship.foreignKey - } - if (!fromForeign) { - 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 || RelationshipTypes.MANY_TO_ONE - } - } - $: tableOptions = plusTables.map(table => ({ label: table.name, value: table._id, })) $: valid = getErrorCount(errors) === 0 && allRequiredAttributesSet() - $: isManyToMany = relationshipType === RelationshipTypes.MANY_TO_MANY $: isManyToOne = relationshipType === RelationshipTypes.MANY_TO_ONE $: fromTable = plusTables.find(table => table._id === fromId) $: toTable = plusTables.find(table => table._id === toId) $: throughTable = plusTables.find(table => table._id === throughId) - $: toRelationship.relationshipType = fromRelationship?.relationshipType - function getErrorCount(errors) { - return Object.entries(errors) - .filter(entry => !!entry[1]) - .map(entry => entry[0]).length - } - - function allRequiredAttributesSet() { - const base = fromTable && toTable && fromColumn && toColumn - if (isManyToOne) { - return base && fromPrimary && fromForeign - } else { - return base && throughTable && throughFromKey && throughToKey - } - } - function invalidThroughTable() { // need to know the foreign key columns to check error if (!throughId || !throughToKey || !throughFromKey) { @@ -137,6 +90,49 @@ } return false } + function relationshipExists() { + if ( + originalFromTable && + originalToTable && + originalFromTable === fromTable && + originalToTable === toTable + ) { + return false + } + let fromThroughLinks = Object.values( + datasource.entities[fromTable.name].schema + ).filter(value => value.through) + let toThroughLinks = Object.values( + datasource.entities[toTable.name].schema + ).filter(value => value.through) + + const matchAgainstUserInput = (fromTableId, toTableId) => + (fromTableId === fromId && toTableId === toId) || + (fromTableId === toId && toTableId === fromId) + + return !!fromThroughLinks.find(from => + toThroughLinks.find( + to => + from.through === to.through && + matchAgainstUserInput(from.tableId, to.tableId) + ) + ) + } + + function getErrorCount(errors) { + return Object.entries(errors) + .filter(entry => !!entry[1]) + .map(entry => entry[0]).length + } + + function allRequiredAttributesSet() { + const base = fromTable && toTable && fromColumn && toColumn + if (isManyToOne) { + return base && fromPrimary && fromForeign + } else { + return base && throughTable && throughFromKey && throughToKey + } + } function validate() { if (!allRequiredAttributesSet() && !hasValidated) { @@ -153,7 +149,7 @@ errObj.throughToKey = errorChecker.manyForeignKeySet(throughToKey) errObj.throughTable = errorChecker.throughIsNullable() errObj.fromForeign = errorChecker.foreignKeySet(fromForeign) - errObj.fromPrimary = errorChecker.foreignKeySet(fromPrimary) + errObj.fromPrimary = errorChecker.primaryKeySet(fromPrimary) errObj.fromTable = errorChecker.doesRelationshipExists() errObj.toTable = errorChecker.doesRelationshipExists() // currently don't support relationships back onto the table itself, needs to relate out @@ -252,35 +248,6 @@ toRelationship = relateTo } - function relationshipExists() { - if ( - originalFromTable && - originalToTable && - originalFromTable === fromTable && - originalToTable === toTable - ) { - return false - } - let fromThroughLinks = Object.values( - datasource.entities[fromTable.name].schema - ).filter(value => value.through) - let toThroughLinks = Object.values( - datasource.entities[toTable.name].schema - ).filter(value => value.through) - - const matchAgainstUserInput = (fromTableId, toTableId) => - (fromTableId === fromId && toTableId === toId) || - (fromTableId === toId && toTableId === fromId) - - return !!fromThroughLinks.find(from => - toThroughLinks.find( - to => - from.through === to.through && - matchAgainstUserInput(from.tableId, to.tableId) - ) - ) - } - function removeExistingRelationship() { if (originalFromTable && originalFromColumnName) { delete datasource.entities[originalFromTable.name].schema[ @@ -325,6 +292,21 @@ } onMount(() => { + if (fromRelationship) { + fromPrimary = fromRelationship.foreignKey + toId = fromRelationship.tableId + throughId = fromRelationship.through + throughFromKey = fromRelationship.throughFrom + throughToKey = fromRelationship.throughTo + toColumn = fromRelationship.name + } + if (toRelationship) { + fromForeign = toRelationship.foreignKey + fromId = toRelationship.tableId + fromColumn = toRelationship.name + } + relationshipType = + fromRelationship.relationshipType || RelationshipTypes.MANY_TO_ONE if (selectedFromTable) { fromColumn = selectedFromTable.name fromPrimary = selectedFromTable?.primary[0] || null diff --git a/packages/builder/src/components/backend/Datasources/relationshipErrors.js b/packages/builder/src/components/backend/Datasources/relationshipErrors.js index 71fb634ada..cdb7805506 100644 --- a/packages/builder/src/components/backend/Datasources/relationshipErrors.js +++ b/packages/builder/src/components/backend/Datasources/relationshipErrors.js @@ -1,16 +1,15 @@ import { RelationshipTypes } from "constants/backend" -export const typeMismatch = - "Column type of the foreign key must match the primary key" -export const columnCantExist = "Column name cannot be an existing column" -export const mustBeDifferentTables = "From/to/through tables must be different" -export const primaryKeyNotSet = "Please pick the primary key" -export const throughNotNullable = +const typeMismatch = "Column type of the foreign key must match the primary key" +const columnCantExist = "Column name cannot be an existing column" +const mustBeDifferentTables = "From/to/through tables must be different" +const primaryKeyNotSet = "Please pick the primary key" +const throughNotNullable = "Ensure non-key columns are nullable or auto-generated" -export const noRelationshipType = "Please specify a relationship type" -export const tableNotSet = "Please specify a table" -export const foreignKeyNotSet = "Please pick a foreign key" -export const relationshipAlreadyExists = +const noRelationshipType = "Please specify a relationship type" +const tableNotSet = "Please specify a table" +const foreignKeyNotSet = "Please pick a foreign key" +const relationshipAlreadyExists = "A relationship between these tables already exists" function isColumnNameBeingUsed(table, columnName, originalName) { @@ -55,6 +54,10 @@ export class RelationshipErrorChecker { return !this.isMany() && !key ? foreignKeyNotSet : null } + primaryKeySet(key) { + return !this.isMany() && !key ? primaryKeyNotSet : null + } + throughIsNullable() { return this.invalidThroughTable() ? throughNotNullable : null } From f2a819d45b3a05b8d150203ee0f7494abb787ec5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 2 Feb 2023 14:14:06 +0000 Subject: [PATCH 4/8] Adding many to many arrow to make reading relationships easier. --- .../TableIntegrationMenu/PlusConfigForm.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte index b510cc0967..ab5b3ccee0 100644 --- a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte +++ b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte @@ -82,7 +82,7 @@ let displayString if (throughTableName) { - displayString = `${fromTableName} through ${throughTableName} → ${toTableName}` + displayString = `${fromTableName} ↔ ${toTableName}` } else { displayString = `${fromTableName} → ${toTableName}` } From 382920938d2033ec940ebfa428cbd188b0e82809 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 2 Feb 2023 16:19:50 +0000 Subject: [PATCH 5/8] PR comments. --- .../Datasources/CreateEditRelationship.svelte | 77 +++++++++---------- .../backend/Datasources/relationshipErrors.js | 4 +- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 57ac41a093..64a6057a7c 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -120,14 +120,12 @@ } function getErrorCount(errors) { - return Object.entries(errors) - .filter(entry => !!entry[1]) - .map(entry => entry[0]).length + return Object.entries(errors).filter(entry => !!entry[1]).length } function allRequiredAttributesSet() { const base = fromTable && toTable && fromColumn && toColumn - if (isManyToOne) { + if (relationshipType === RelationshipTypes.MANY_TO_ONE) { return base && fromPrimary && fromForeign } else { return base && throughTable && throughFromKey && throughToKey @@ -140,39 +138,37 @@ } hasValidated = true errorChecker.setType(relationshipType) - const errObj = {} - errObj.relationshipType = errorChecker.relationshipTypeSet(relationshipType) - errObj.fromTable = errorChecker.tableSet(fromTable) - errObj.toTable = errorChecker.tableSet(toTable) - errObj.throughTable = errorChecker.throughTableSet(throughTable) - errObj.throughFromKey = errorChecker.manyForeignKeySet(throughFromKey) - errObj.throughToKey = errorChecker.manyForeignKeySet(throughToKey) - errObj.throughTable = errorChecker.throughIsNullable() - errObj.fromForeign = errorChecker.foreignKeySet(fromForeign) - errObj.fromPrimary = errorChecker.primaryKeySet(fromPrimary) - errObj.fromTable = errorChecker.doesRelationshipExists() - errObj.toTable = errorChecker.doesRelationshipExists() - // currently don't support relationships back onto the table itself, needs to relate out - errObj.fromTable = errorChecker.differentTables(fromId, toId, throughId) - errObj.toTable = errorChecker.differentTables(toId, fromId, throughId) - errObj.throughTable = errorChecker.differentTables(throughId, fromId, toId) - errObj.fromColumn = errorChecker.columnBeingUsed( - toTable, - fromColumn, - originalFromColumnName - ) - errObj.toColumn = errorChecker.columnBeingUsed( - fromTable, - toColumn, - originalToColumnName - ) - errObj.fromForeign = errorChecker.typeMismatch( - fromTable, - toTable, - fromPrimary, - fromForeign - ) - errors = errObj + errors = { + relationshipType: errorChecker.relationshipTypeSet(relationshipType), + fromTable: + errorChecker.tableSet(fromTable) || + errorChecker.doesRelationshipExists() || + errorChecker.differentTables(fromId, toId, throughId), + toTable: + errorChecker.tableSet(toTable) || + errorChecker.doesRelationshipExists() || + errorChecker.differentTables(toId, fromId, throughId), + throughTable: + errorChecker.throughTableSet(throughTable) || + errorChecker.throughIsNullable() || + errorChecker.differentTables(throughId, fromId, toId), + throughFromKey: errorChecker.manyForeignKeySet(throughFromKey), + throughToKey: errorChecker.manyForeignKeySet(throughToKey), + fromForeign: + errorChecker.foreignKeySet(fromForeign) || + errorChecker.typeMismatch(fromTable, toTable, fromPrimary, fromForeign), + fromPrimary: errorChecker.primaryKeySet(fromPrimary), + fromColumn: errorChecker.columnBeingUsed( + toTable, + fromColumn, + originalFromColumnName + ), + toColumn: errorChecker.columnBeingUsed( + fromTable, + toColumn, + originalToColumnName + ), + } return getErrorCount(errors) === 0 } @@ -285,7 +281,7 @@ } function changed(fn) { - if (fn && typeof fn === "function") { + if (typeof fn === "function") { fn() } validate() @@ -325,7 +321,10 @@ options={relationshipTypes} bind:value={relationshipType} bind:error={errors.relationshipType} - on:change={changed} + on:change={() => + changed(() => { + hasValidated = false + })} />
Tables diff --git a/packages/builder/src/components/backend/Datasources/relationshipErrors.js b/packages/builder/src/components/backend/Datasources/relationshipErrors.js index cdb7805506..55353f7423 100644 --- a/packages/builder/src/components/backend/Datasources/relationshipErrors.js +++ b/packages/builder/src/components/backend/Datasources/relationshipErrors.js @@ -1,7 +1,7 @@ import { RelationshipTypes } from "constants/backend" const typeMismatch = "Column type of the foreign key must match the primary key" -const columnCantExist = "Column name cannot be an existing column" +const columnBeingUsed = "Column name cannot be an existing column" const mustBeDifferentTables = "From/to/through tables must be different" const primaryKeyNotSet = "Please pick the primary key" const throughNotNullable = @@ -75,7 +75,7 @@ export class RelationshipErrorChecker { } columnBeingUsed(table, column, ogName) { - return isColumnNameBeingUsed(table, column, ogName) ? columnCantExist : null + return isColumnNameBeingUsed(table, column, ogName) ? columnBeingUsed : null } typeMismatch(fromTable, toTable, primary, foreign) { From 6fd0188761e1c6ebf4819098e03abf2dd2fbd192 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 2 Feb 2023 16:59:12 +0000 Subject: [PATCH 6/8] Updating reactivity to fix issues with occasionally incorrect errors. --- .../Datasources/CreateEditRelationship.svelte | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 64a6057a7c..419b6400ef 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -47,13 +47,7 @@ relationshipExists ) let errors = {} - let fromPrimary, - fromForeign, - fromTable, - toTable, - throughTable, - fromColumn, - toColumn + let fromPrimary, fromForeign, fromColumn, toColumn let fromId = selectedFromTable?._id, toId, throughId, @@ -69,11 +63,20 @@ $: valid = getErrorCount(errors) === 0 && allRequiredAttributesSet() $: isManyToMany = relationshipType === RelationshipTypes.MANY_TO_MANY $: isManyToOne = relationshipType === RelationshipTypes.MANY_TO_ONE - $: fromTable = plusTables.find(table => table._id === fromId) - $: toTable = plusTables.find(table => table._id === toId) - $: throughTable = plusTables.find(table => table._id === throughId) $: toRelationship.relationshipType = fromRelationship?.relationshipType + function getFromTable() { + return plusTables.find(table => table._id === fromId) + } + + function getToTable() { + return plusTables.find(table => table._id === toId) + } + + function getThroughTable() { + return plusTables.find(table => table._id === throughId) + } + function invalidThroughTable() { // need to know the foreign key columns to check error if (!throughId || !throughToKey || !throughFromKey) { @@ -94,16 +97,16 @@ if ( originalFromTable && originalToTable && - originalFromTable === fromTable && - originalToTable === toTable + originalFromTable === getFromTable() && + originalToTable === getToTable() ) { return false } let fromThroughLinks = Object.values( - datasource.entities[fromTable.name].schema + datasource.entities[getFromTable().name].schema ).filter(value => value.through) let toThroughLinks = Object.values( - datasource.entities[toTable.name].schema + datasource.entities[getToTable().name].schema ).filter(value => value.through) const matchAgainstUserInput = (fromTableId, toTableId) => @@ -124,11 +127,11 @@ } function allRequiredAttributesSet() { - const base = fromTable && toTable && fromColumn && toColumn + const base = getFromTable() && getToTable() && fromColumn && toColumn if (relationshipType === RelationshipTypes.MANY_TO_ONE) { return base && fromPrimary && fromForeign } else { - return base && throughTable && throughFromKey && throughToKey + return base && getThroughTable() && throughFromKey && throughToKey } } @@ -138,6 +141,9 @@ } hasValidated = true errorChecker.setType(relationshipType) + const fromTable = getFromTable(), + toTable = getToTable(), + throughTable = getThroughTable() errors = { relationshipType: errorChecker.relationshipTypeSet(relationshipType), fromTable: @@ -210,13 +216,13 @@ if (manyToMany) { relateFrom = { ...relateFrom, - through: throughTable._id, - fieldName: toTable.primary[0], + through: getThroughTable()._id, + fieldName: getToTable().primary[0], } relateTo = { ...relateTo, - through: throughTable._id, - fieldName: fromTable.primary[0], + through: getThroughTable()._id, + fieldName: getFromTable().primary[0], throughFrom: relateFrom.throughTo, throughTo: relateFrom.throughFrom, } @@ -265,10 +271,10 @@ removeExistingRelationship() // source of relationship - datasource.entities[fromTable.name].schema[fromRelationship.name] = + datasource.entities[getFromTable().name].schema[fromRelationship.name] = fromRelationship // save other side of relationship in the other schema - datasource.entities[toTable.name].schema[toRelationship.name] = + datasource.entities[getToTable().name].schema[toRelationship.name] = toRelationship await save() @@ -343,10 +349,10 @@ })} /> {/if} - {#if isManyToOne && fromTable} + {#if isManyToOne && fromId} @@ -390,8 +396,8 @@ })} /> Date: Thu, 2 Feb 2023 17:25:02 +0000 Subject: [PATCH 7/8] Some final fixes based on comments, adding foreign key type checking for through tables. --- .../Datasources/CreateEditRelationship.svelte | 76 ++++++++++--------- .../backend/Datasources/relationshipErrors.js | 24 ++++-- 2 files changed, 60 insertions(+), 40 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 419b6400ef..854e35d6dc 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -65,16 +65,8 @@ $: isManyToOne = relationshipType === RelationshipTypes.MANY_TO_ONE $: toRelationship.relationshipType = fromRelationship?.relationshipType - function getFromTable() { - return plusTables.find(table => table._id === fromId) - } - - function getToTable() { - return plusTables.find(table => table._id === toId) - } - - function getThroughTable() { - return plusTables.find(table => table._id === throughId) + function getTable(id) { + return plusTables.find(table => table._id === id) } function invalidThroughTable() { @@ -97,16 +89,16 @@ if ( originalFromTable && originalToTable && - originalFromTable === getFromTable() && - originalToTable === getToTable() + originalFromTable === getTable(fromId) && + originalToTable === getTable(toId) ) { return false } let fromThroughLinks = Object.values( - datasource.entities[getFromTable().name].schema + datasource.entities[getTable(fromId).name].schema ).filter(value => value.through) let toThroughLinks = Object.values( - datasource.entities[getToTable().name].schema + datasource.entities[getTable(toId).name].schema ).filter(value => value.through) const matchAgainstUserInput = (fromTableId, toTableId) => @@ -127,11 +119,11 @@ } function allRequiredAttributesSet() { - const base = getFromTable() && getToTable() && fromColumn && toColumn + const base = getTable(fromId) && getTable(toId) && fromColumn && toColumn if (relationshipType === RelationshipTypes.MANY_TO_ONE) { return base && fromPrimary && fromForeign } else { - return base && getThroughTable() && throughFromKey && throughToKey + return base && getTable(throughId) && throughFromKey && throughToKey } } @@ -141,9 +133,9 @@ } hasValidated = true errorChecker.setType(relationshipType) - const fromTable = getFromTable(), - toTable = getToTable(), - throughTable = getThroughTable() + const fromTable = getTable(fromId), + toTable = getTable(toId), + throughTable = getTable(throughId) errors = { relationshipType: errorChecker.relationshipTypeSet(relationshipType), fromTable: @@ -158,8 +150,22 @@ errorChecker.throughTableSet(throughTable) || errorChecker.throughIsNullable() || errorChecker.differentTables(throughId, fromId, toId), - throughFromKey: errorChecker.manyForeignKeySet(throughFromKey), - throughToKey: errorChecker.manyForeignKeySet(throughToKey), + throughFromKey: + errorChecker.manyForeignKeySet(throughFromKey) || + errorChecker.manyTypeMismatch( + fromTable, + throughTable, + fromTable.primary[0], + throughFromKey + ), + throughToKey: + errorChecker.manyForeignKeySet(throughToKey) || + errorChecker.manyTypeMismatch( + toTable, + throughTable, + toTable.primary[0], + throughToKey + ), fromForeign: errorChecker.foreignKeySet(fromForeign) || errorChecker.typeMismatch(fromTable, toTable, fromPrimary, fromForeign), @@ -216,13 +222,13 @@ if (manyToMany) { relateFrom = { ...relateFrom, - through: getThroughTable()._id, - fieldName: getToTable().primary[0], + through: getTable(throughId)._id, + fieldName: getTable(toId).primary[0], } relateTo = { ...relateTo, - through: getThroughTable()._id, - fieldName: getFromTable().primary[0], + through: getTable(throughId)._id, + fieldName: getTable(fromId).primary[0], throughFrom: relateFrom.throughTo, throughTo: relateFrom.throughFrom, } @@ -271,10 +277,10 @@ removeExistingRelationship() // source of relationship - datasource.entities[getFromTable().name].schema[fromRelationship.name] = + datasource.entities[getTable(fromId).name].schema[fromRelationship.name] = fromRelationship // save other side of relationship in the other schema - datasource.entities[getToTable().name].schema[toRelationship.name] = + datasource.entities[getTable(toId).name].schema[toRelationship.name] = toRelationship await save() @@ -351,8 +357,8 @@ {/if} {#if isManyToOne && fromId} @@ -396,8 +402,8 @@ })} /> Date: Thu, 2 Feb 2023 17:37:41 +0000 Subject: [PATCH 8/8] Final fix - making sure relationships can be built from table UI. --- .../backend/Datasources/CreateEditRelationship.svelte | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte index 854e35d6dc..4a3c4f6c60 100644 --- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte +++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte @@ -48,11 +48,7 @@ ) let errors = {} let fromPrimary, fromForeign, fromColumn, toColumn - let fromId = selectedFromTable?._id, - toId, - throughId, - throughToKey, - throughFromKey + let fromId, toId, throughId, throughToKey, throughFromKey let isManyToMany, isManyToOne, relationshipType let hasValidated = false @@ -316,6 +312,7 @@ relationshipType = fromRelationship.relationshipType || RelationshipTypes.MANY_TO_ONE if (selectedFromTable) { + fromId = selectedFromTable._id fromColumn = selectedFromTable.name fromPrimary = selectedFromTable?.primary[0] || null }