Merge pull request #13509 from Budibase/feat/budi-8126

[Fix] Datasource plus schema updates issues

packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte

@@ -13,6 +13,7 @@
     Layout,
     AbsTooltip,
   } from "@budibase/bbui"
+  import { SWITCHABLE_TYPES, ValidColumnNameRegex } from "@budibase/shared-core"
   import { createEventDispatcher, getContext, onMount } from "svelte"
   import { cloneDeep } from "lodash/fp"
   import { tables, datasources } from "stores/builder"
@@ -20,11 +21,6 @@
   import {
     FIELDS,
     RelationshipType,
-    ALLOWABLE_STRING_OPTIONS,
-    ALLOWABLE_NUMBER_OPTIONS,
-    ALLOWABLE_STRING_TYPES,
-    ALLOWABLE_NUMBER_TYPES,
-    SWITCHABLE_TYPES,
     PrettyRelationshipDefinitions,
     DB_TYPE_EXTERNAL,
   } from "constants/backend"
@@ -33,7 +29,6 @@
   import ModalBindableInput from "components/common/bindings/ModalBindableInput.svelte"
   import { getBindings } from "components/backend/DataTable/formula"
   import JSONSchemaModal from "./JSONSchemaModal.svelte"
-  import { ValidColumnNameRegex } from "@budibase/shared-core"
   import { FieldType, FieldSubtype, SourceName } from "@budibase/types"
   import RelationshipSelector from "components/common/RelationshipSelector.svelte"
   import { RowUtils } from "@budibase/frontend-core"
@@ -61,8 +56,8 @@
   let primaryDisplay
   let indexes = [...($tables.selected.indexes || [])]
   let isCreating = undefined
-  let relationshipPart1 = PrettyRelationshipDefinitions.Many
-  let relationshipPart2 = PrettyRelationshipDefinitions.One
+  let relationshipPart1 = PrettyRelationshipDefinitions.MANY
+  let relationshipPart2 = PrettyRelationshipDefinitions.ONE
   let relationshipTableIdPrimary = null
   let relationshipTableIdSecondary = null
   let table = $tables.selected
@@ -175,7 +170,7 @@
   $: typeEnabled =
     !originalName ||
     (originalName &&
-      SWITCHABLE_TYPES.indexOf(editableColumn.type) !== -1 &&
+      SWITCHABLE_TYPES[field.type] &&
       !editableColumn?.autocolumn)
 
   const fieldDefinitions = Object.values(FIELDS).reduce(
@@ -367,16 +362,15 @@
   }
 
   function getAllowedTypes() {
-    if (
-      originalName &&
-      ALLOWABLE_STRING_TYPES.indexOf(editableColumn.type) !== -1
-    ) {
-      return ALLOWABLE_STRING_OPTIONS
-    } else if (
-      originalName &&
-      ALLOWABLE_NUMBER_TYPES.indexOf(editableColumn.type) !== -1
-    ) {
-      return ALLOWABLE_NUMBER_OPTIONS
+    if (originalName) {
+      const possibleTypes = (
+        SWITCHABLE_TYPES[field.type] || [editableColumn.type]
+      ).map(t => t.toLowerCase())
+      return Object.entries(FIELDS)
+        .filter(([fieldType]) =>
+          possibleTypes.includes(fieldType.toLowerCase())
+        )
+        .map(([_, fieldDefinition]) => fieldDefinition)
     }
 
     const isUsers =

packages/builder/src/constants/backend/index.js

@@ -202,26 +202,6 @@ export const PrettyRelationshipDefinitions = {
   ONE: "One row",
 }
 
-export const ALLOWABLE_STRING_OPTIONS = [
-  FIELDS.STRING,
-  FIELDS.OPTIONS,
-  FIELDS.LONGFORM,
-  FIELDS.BARCODEQR,
-]
-export const ALLOWABLE_STRING_TYPES = ALLOWABLE_STRING_OPTIONS.map(
-  opt => opt.type
-)
-
-export const ALLOWABLE_NUMBER_OPTIONS = [FIELDS.NUMBER, FIELDS.BOOLEAN]
-export const ALLOWABLE_NUMBER_TYPES = ALLOWABLE_NUMBER_OPTIONS.map(
-  opt => opt.type
-)
-
-export const SWITCHABLE_TYPES = [
-  ...ALLOWABLE_STRING_TYPES,
-  ...ALLOWABLE_NUMBER_TYPES,
-]
-
 export const BUDIBASE_INTERNAL_DB_ID = INTERNAL_TABLE_SOURCE_ID
 export const DEFAULT_BB_DATASOURCE_ID = "datasource_internal_bb_default"
 export const BUDIBASE_DATASOURCE_TYPE = "budibase"

packages/builder/src/stores/builder/tables.js

@@ -1,8 +1,8 @@
 import { FieldType } from "@budibase/types"
+import { SWITCHABLE_TYPES } from "@budibase/shared-core"
 import { get, writable, derived } from "svelte/store"
 import { cloneDeep } from "lodash/fp"
 import { API } from "api"
-import { SWITCHABLE_TYPES } from "constants/backend"
 
 export function createTablesStore() {
   const store = writable({
@@ -64,7 +64,7 @@ export function createTablesStore() {
       if (
         oldField != null &&
         oldField?.type !== field.type &&
-        SWITCHABLE_TYPES.indexOf(oldField?.type) === -1
+        !SWITCHABLE_TYPES[oldField?.type]?.includes(field.type)
       ) {
         updatedTable.schema[key] = oldField
       }
@@ -148,12 +148,6 @@ export function createTablesStore() {
     if (indexes) {
       draft.indexes = indexes
     }
-    // Add object to indicate if column is being added
-    if (draft.schema[field.name] === undefined) {
-      draft._add = {
-        name: field.name,
-      }
-    }
     draft.schema = {
       ...draft.schema,
       [field.name]: cloneDeep(field),

packages/server/src/api/controllers/table/external.ts

@@ -31,7 +31,6 @@ export async function save(
   renaming?: RenameColumn
 ) {
   const inputs = ctx.request.body
-  const adding = inputs?._add
   // can't do this right now
   delete inputs.rows
   const tableId = ctx.request.body._id
@@ -44,7 +43,7 @@ export async function save(
     const { datasource, table } = await sdk.tables.external.save(
       datasourceId!,
       inputs,
-      { tableId, renaming, adding }
+      { tableId, renaming }
     )
     builderSocket?.emitDatasourceUpdate(ctx, datasource)
     return table

packages/server/src/api/controllers/table/index.ts

@@ -77,11 +77,6 @@ export async function save(ctx: UserCtx<SaveTableRequest, SaveTableResponse>) {
   const renaming = ctx.request.body._rename
 
   const api = pickApi({ table })
-  // do not pass _rename or _add if saving to CouchDB
-  if (api === internal) {
-    delete ctx.request.body._add
-    delete ctx.request.body._rename
-  }
   let savedTable = await api.save(ctx, renaming)
   if (!table._id) {
     savedTable = sdk.tables.enrichViewSchemas(savedTable)

packages/server/src/api/controllers/table/internal.ts

@@ -16,7 +16,7 @@ export async function save(
   ctx: UserCtx<SaveTableRequest, SaveTableResponse>,
   renaming?: RenameColumn
 ) {
-  const { rows, ...rest } = ctx.request.body
+  const { _rename, rows, ...rest } = ctx.request.body
   let tableToSave: Table = {
     _id: generateTableID(),
     ...rest,

packages/server/src/api/routes/tests/table.spec.ts

@@ -219,9 +219,6 @@ describe.each([
 
     it("should add a new column for an internal DB table", async () => {
       const saveTableRequest: SaveTableRequest = {
-        _add: {
-          name: "NEW_COLUMN",
-        },
         ...basicTable(),
       }
 
@@ -235,7 +232,6 @@ describe.each([
         updatedAt: expect.stringMatching(ISO_REGEX_PATTERN),
         views: {},
       }
-      delete expectedResponse._add
       expect(response).toEqual(expectedResponse)
     })
   })

packages/server/src/integration-test/mysql.spec.ts

@@ -16,7 +16,6 @@ import {
   getDatasource,
   rawQuery,
 } from "../integrations/tests/utils"
-import { builderSocket } from "../websockets"
 import { generator } from "@budibase/backend-core/tests"
 // @ts-ignore
 fetch.mockSearch()
@@ -233,72 +232,6 @@ describe("mysql integrations", () => {
   })
 
   describe("POST /api/tables/", () => {
-    const emitDatasourceUpdateMock = jest.fn()
-
-    it("will emit the datasource entity schema with externalType to the front-end when adding a new column", async () => {
-      const addColumnToTable: TableRequest = {
-        type: "table",
-        sourceType: TableSourceType.EXTERNAL,
-        name: uniqueTableName(),
-        sourceId: datasource._id!,
-        primary: ["id"],
-        schema: {
-          id: {
-            type: FieldType.AUTO,
-            name: "id",
-            autocolumn: true,
-          },
-          new_column: {
-            type: FieldType.NUMBER,
-            name: "new_column",
-          },
-        },
-        _add: {
-          name: "new_column",
-        },
-      }
-
-      jest
-        .spyOn(builderSocket!, "emitDatasourceUpdate")
-        .mockImplementation(emitDatasourceUpdateMock)
-
-      await makeRequest("post", "/api/tables/", addColumnToTable)
-
-      const expectedTable: TableRequest = {
-        ...addColumnToTable,
-        schema: {
-          id: {
-            type: FieldType.NUMBER,
-            name: "id",
-            autocolumn: true,
-            constraints: {
-              presence: false,
-            },
-            externalType: "int unsigned",
-          },
-          new_column: {
-            type: FieldType.NUMBER,
-            name: "new_column",
-            autocolumn: false,
-            constraints: {
-              presence: false,
-            },
-            externalType: "float(8,2)",
-          },
-        },
-        created: true,
-        _id: `${datasource._id}__${addColumnToTable.name}`,
-      }
-      delete expectedTable._add
-
-      expect(emitDatasourceUpdateMock).toHaveBeenCalledTimes(1)
-      const emittedDatasource: Datasource =
-        emitDatasourceUpdateMock.mock.calls[0][1]
-      expect(emittedDatasource.entities![expectedTable.name]).toEqual(
-        expectedTable
-      )
-    })
-
     it("will rename a column", async () => {
       await makeRequest("post", "/api/tables/", primaryMySqlTable)
 

packages/server/src/integrations/utils.ts

@@ -7,7 +7,7 @@ import {
 } from "@budibase/types"
 import { DocumentType, SEPARATOR } from "../db/utils"
 import { InvalidColumns, DEFAULT_BB_DATASOURCE_ID } from "../constants"
-import { helpers } from "@budibase/shared-core"
+import { SWITCHABLE_TYPES, helpers } from "@budibase/shared-core"
 import env from "../environment"
 import { Knex } from "knex"
 
@@ -284,8 +284,8 @@ export function isIsoDateString(str: string) {
  * @param column The column to check, to see if it is a valid relationship.
  * @param tableIds The IDs of the tables which currently exist.
  */
-export function shouldCopyRelationship(
-  column: { type: string; tableId?: string },
+function shouldCopyRelationship(
+  column: { type: FieldType.LINK; tableId?: string },
   tableIds: string[]
 ) {
   return (
@@ -303,28 +303,18 @@ export function shouldCopyRelationship(
  * @param column The column to check for options or boolean type.
  * @param fetchedColumn The fetched column to check for the type in the external database.
  */
-export function shouldCopySpecialColumn(
-  column: { type: string },
-  fetchedColumn: { type: string } | undefined
+function shouldCopySpecialColumn(
+  column: { type: FieldType },
+  fetchedColumn: { type: FieldType } | undefined
 ) {
   const isFormula = column.type === FieldType.FORMULA
-  const specialTypes = [
-    FieldType.OPTIONS,
-    FieldType.LONGFORM,
-    FieldType.ARRAY,
-    FieldType.FORMULA,
-    FieldType.BB_REFERENCE,
-  ]
   // column has been deleted, remove - formulas will never exist, always copy
   if (!isFormula && column && !fetchedColumn) {
     return false
   }
   const fetchedIsNumber =
     !fetchedColumn || fetchedColumn.type === FieldType.NUMBER
-  return (
-    specialTypes.indexOf(column.type as FieldType) !== -1 ||
-    (fetchedIsNumber && column.type === FieldType.BOOLEAN)
-  )
+  return fetchedIsNumber && column.type === FieldType.BOOLEAN
 }
 
 /**
@@ -357,11 +347,44 @@ function copyExistingPropsOver(
         continue
       }
       const column = existingTableSchema[key]
+
+      const existingColumnType = column?.type
+      const updatedColumnType = table.schema[key]?.type
+
+      // If the db column type changed to a non-compatible one, we want to re-fetch it
       if (
-        shouldCopyRelationship(column, tableIds) ||
-        shouldCopySpecialColumn(column, table.schema[key])
+        updatedColumnType !== existingColumnType &&
+        !SWITCHABLE_TYPES[updatedColumnType]?.includes(existingColumnType)
       ) {
-        table.schema[key] = existingTableSchema[key]
+        continue
+      }
+
+      if (
+        column.type === FieldType.LINK &&
+        !shouldCopyRelationship(column, tableIds)
+      ) {
+        continue
+      }
+
+      const specialTypes = [
+        FieldType.OPTIONS,
+        FieldType.LONGFORM,
+        FieldType.ARRAY,
+        FieldType.FORMULA,
+        FieldType.BB_REFERENCE,
+      ]
+      if (
+        specialTypes.includes(column.type) &&
+        !shouldCopySpecialColumn(column, table.schema[key])
+      ) {
+        continue
+      }
+
+      table.schema[key] = {
+        ...existingTableSchema[key],
+        externalType:
+          existingTableSchema[key].externalType ||
+          table.schema[key].externalType,
       }
     }
   }

packages/server/src/sdk/app/datasources/datasources.ts

@@ -348,8 +348,7 @@ const preSaveAction: Partial<Record<SourceName, any>> = {
  * Make sure all datasource entities have a display name selected
  */
 export function setDefaultDisplayColumns(datasource: Datasource) {
-  //
-  for (let entity of Object.values(datasource.entities || {})) {
+  for (const entity of Object.values(datasource.entities || {})) {
     if (entity.primaryDisplay) {
       continue
     }

packages/server/src/sdk/app/tables/external/index.ts

@@ -3,7 +3,6 @@ import {
   Operation,
   RelationshipType,
   RenameColumn,
-  AddColumn,
   Table,
   TableRequest,
   ViewV2,
@@ -33,7 +32,7 @@ import * as viewSdk from "../../views"
 export async function save(
   datasourceId: string,
   update: Table,
-  opts?: { tableId?: string; renaming?: RenameColumn; adding?: AddColumn }
+  opts?: { tableId?: string; renaming?: RenameColumn }
 ) {
   let tableToSave: TableRequest = {
     ...update,
@@ -179,14 +178,7 @@ export async function save(
   // remove the rename prop
   delete tableToSave._rename
 
-  // if adding a new column, we need to rebuild the schema for that table to get the 'externalType' of the column
-  if (opts?.adding) {
-    datasource.entities[tableToSave.name] = (
-      await datasourceSdk.buildFilteredSchema(datasource, [tableToSave.name])
-    ).tables[tableToSave.name]
-  } else {
-    datasource.entities[tableToSave.name] = tableToSave
-  }
+  datasource.entities[tableToSave.name] = tableToSave
 
   // store it into couch now for budibase reference
   await db.put(populateExternalTableSchemas(datasource))

packages/shared-core/src/constants/fields.ts

@@ -0,0 +1,33 @@
+import { FieldType } from "@budibase/types"
+
+type SwitchableTypes = Partial<{
+  [K in FieldType]: [K, ...FieldType[]]
+}>
+
+export const SWITCHABLE_TYPES: SwitchableTypes = {
+  [FieldType.STRING]: [
+    FieldType.STRING,
+    FieldType.OPTIONS,
+    FieldType.LONGFORM,
+    FieldType.BARCODEQR,
+  ],
+  [FieldType.OPTIONS]: [
+    FieldType.OPTIONS,
+    FieldType.STRING,
+    FieldType.LONGFORM,
+    FieldType.BARCODEQR,
+  ],
+  [FieldType.LONGFORM]: [
+    FieldType.LONGFORM,
+    FieldType.STRING,
+    FieldType.OPTIONS,
+    FieldType.BARCODEQR,
+  ],
+  [FieldType.BARCODEQR]: [
+    FieldType.BARCODEQR,
+    FieldType.STRING,
+    FieldType.OPTIONS,
+    FieldType.LONGFORM,
+  ],
+  [FieldType.NUMBER]: [FieldType.NUMBER, FieldType.BOOLEAN],
+}

packages/shared-core/src/constants/index.ts

@@ -1,4 +1,5 @@
 export * from "./api"
+export * from "./fields"
 
 export const OperatorOptions = {
   Equals: {

packages/types/src/documents/app/table/table.ts

@@ -1,6 +1,6 @@
 import { Document } from "../../document"
 import { View, ViewV2 } from "../view"
-import { AddColumn, RenameColumn } from "../../../sdk"
+import { RenameColumn } from "../../../sdk"
 import { TableSchema } from "./schema"
 
 export const INTERNAL_TABLE_SOURCE_ID = "bb_internal"
@@ -30,6 +30,5 @@ export interface Table extends Document {
 
 export interface TableRequest extends Table {
   _rename?: RenameColumn
-  _add?: AddColumn
   created?: boolean
 }

packages/types/src/sdk/search.ts

@@ -77,10 +77,6 @@ export interface RenameColumn {
   updated: string
 }
 
-export interface AddColumn {
-  name: string
-}
-
 export interface RelationshipsJson {
   through?: string
   from?: string