Merge pull request #12050 from Budibase/fix/budi-7433-google-sheets-validation-wont-let-you-import-any-sheets-if

Surface errors when importing tables from a datasource, don't surface errors from tables that have been filtered out.

packages/builder/src/components/backend/Datasources/TableImportSelection/index.svelte

@@ -57,7 +57,7 @@
       {#if $store.error}
         <InlineAlert
           type="error"
-          header={$store.error.title}
+          header="Error fetching {tableType}"
           message={$store.error.description}
         />
       {/if}

packages/builder/src/components/backend/Datasources/TableImportSelection/tableSelectionStore.js

@@ -1,6 +1,6 @@
 import { derived, writable, get } from "svelte/store"
 import { keepOpen, notifications } from "@budibase/bbui"
-import { datasources, ImportTableError, tables } from "stores/backend"
+import { datasources, tables } from "stores/backend"
 
 export const createTableSelectionStore = (integration, datasource) => {
   const tableNamesStore = writable([])
@@ -30,12 +30,7 @@ export const createTableSelectionStore = (integration, datasource) => {
       notifications.success(`Tables fetched successfully.`)
       await onComplete()
     } catch (err) {
-      if (err instanceof ImportTableError) {
-        errorStore.set(err)
-      } else {
-        notifications.error("Error fetching tables.")
-      }
-
+      errorStore.set(err)
       return keepOpen
     }
   }

packages/builder/src/stores/backend/datasources.js

@@ -9,15 +9,19 @@ import { API } from "api"
 import { DatasourceFeature } from "@budibase/types"
 import { TableNames } from "constants"
 
-export class ImportTableError extends Error {
-  constructor(message) {
-    super(message)
-    const [title, description] = message.split(" - ")
+class TableImportError extends Error {
+  constructor(errors) {
+    super()
+    this.name = "TableImportError"
+    this.errors = errors
+  }
 
-    this.name = "TableSelectionError"
-    // Capitalize the first character of both the title and description
-    this.title = title[0].toUpperCase() + title.substr(1)
-    this.description = description[0].toUpperCase() + description.substr(1)
+  get description() {
+    let message = ""
+    for (const key in this.errors) {
+      message += `${key}: ${this.errors[key]}\n`
+    }
+    return message
   }
 }
 
@@ -25,7 +29,6 @@ export function createDatasourcesStore() {
   const store = writable({
     list: [],
     selectedDatasourceId: null,
-    schemaError: null,
   })
 
   const derivedStore = derived([store, tables], ([$store, $tables]) => {
@@ -75,18 +78,13 @@ export function createDatasourcesStore() {
     store.update(state => ({
       ...state,
       selectedDatasourceId: id,
-      // Remove any possible schema error
-      schemaError: null,
     }))
   }
 
   const updateDatasource = response => {
-    const { datasource, error } = response
-    if (error) {
-      store.update(state => ({
-        ...state,
-        schemaError: error,
-      }))
+    const { datasource, errors } = response
+    if (errors && Object.keys(errors).length > 0) {
+      throw new TableImportError(errors)
     }
     replaceDatasource(datasource._id, datasource)
     select(datasource._id)
@@ -94,20 +92,11 @@ export function createDatasourcesStore() {
   }
 
   const updateSchema = async (datasource, tablesFilter) => {
-    try {
-      const response = await API.buildDatasourceSchema({
-        datasourceId: datasource?._id,
-        tablesFilter,
-      })
-      updateDatasource(response)
-    } catch (e) {
-      // buildDatasourceSchema call returns user presentable errors with two parts divided with a " - ".
-      if (e.message.split(" - ").length === 2) {
-        throw new ImportTableError(e.message)
-      } else {
-        throw e
-      }
-    }
+    const response = await API.buildDatasourceSchema({
+      datasourceId: datasource?._id,
+      tablesFilter,
+    })
+    updateDatasource(response)
   }
 
   const sourceCount = source => {
@@ -172,12 +161,6 @@ export function createDatasourcesStore() {
     replaceDatasource(datasource._id, null)
   }
 
-  const removeSchemaError = () => {
-    store.update(state => {
-      return { ...state, schemaError: null }
-    })
-  }
-
   const replaceDatasource = (datasourceId, datasource) => {
     if (!datasourceId) {
       return
@@ -230,7 +213,6 @@ export function createDatasourcesStore() {
     create,
     update,
     delete: deleteDatasource,
-    removeSchemaError,
     replaceDatasource,
     getTableNames,
   }

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

@@ -4,7 +4,7 @@ export { views } from "./views"
 export { viewsV2 } from "./viewsV2"
 export { permissions } from "./permissions"
 export { roles } from "./roles"
-export { datasources, ImportTableError } from "./datasources"
+export { datasources } from "./datasources"
 export { integrations } from "./integrations"
 export { sortedIntegrations } from "./sortedIntegrations"
 export { queries } from "./queries"

packages/server/src/api/controllers/datasource.ts

@@ -5,7 +5,6 @@ import {
   getTableParams,
 } from "../../db/utils"
 import { destroy as tableDestroy } from "./table/internal"
-import { BuildSchemaErrors, InvalidColumns } from "../../constants"
 import { getIntegration } from "../../integrations"
 import { invalidateDynamicVariables } from "../../threads/utils"
 import { context, db as dbCore, events } from "@budibase/backend-core"
@@ -14,10 +13,13 @@ import {
   CreateDatasourceResponse,
   Datasource,
   DatasourcePlus,
+  ExternalTable,
   FetchDatasourceInfoRequest,
   FetchDatasourceInfoResponse,
   IntegrationBase,
+  Schema,
   SourceName,
+  Table,
   UpdateDatasourceResponse,
   UserCtx,
   VerifyDatasourceRequest,
@@ -27,23 +29,6 @@ import sdk from "../../sdk"
 import { builderSocket } from "../../websockets"
 import { setupCreationAuth as googleSetupCreationAuth } from "../../integrations/googlesheets"
 
-function getErrorTables(errors: any, errorType: string) {
-  return Object.entries(errors)
-    .filter(entry => entry[1] === errorType)
-    .map(([name]) => name)
-}
-
-function updateError(error: any, newError: any, tables: string[]) {
-  if (!error) {
-    error = ""
-  }
-  if (error.length > 0) {
-    error += "\n"
-  }
-  error += `${newError} ${tables.join(", ")}`
-  return error
-}
-
 async function getConnector(
   datasource: Datasource
 ): Promise<IntegrationBase | DatasourcePlus> {
@@ -71,48 +56,36 @@ async function getAndMergeDatasource(datasource: Datasource) {
   return await sdk.datasources.enrich(enrichedDatasource)
 }
 
-async function buildSchemaHelper(datasource: Datasource) {
+async function buildSchemaHelper(datasource: Datasource): Promise<Schema> {
   const connector = (await getConnector(datasource)) as DatasourcePlus
-  await connector.buildSchema(datasource._id!, datasource.entities!)
-
-  const errors = connector.schemaErrors
-  let error = null
-  if (errors && Object.keys(errors).length > 0) {
-    const noKey = getErrorTables(errors, BuildSchemaErrors.NO_KEY)
-    const invalidCol = getErrorTables(errors, BuildSchemaErrors.INVALID_COLUMN)
-    if (noKey.length) {
-      error = updateError(
-        error,
-        "No primary key constraint found for the following:",
-        noKey
-      )
-    }
-    if (invalidCol.length) {
-      const invalidCols = Object.values(InvalidColumns).join(", ")
-      error = updateError(
-        error,
-        `Cannot use columns ${invalidCols} found in following:`,
-        invalidCol
-      )
-    }
-  }
-  return { tables: connector.tables, error }
+  return await connector.buildSchema(
+    datasource._id!,
+    datasource.entities! as Record<string, ExternalTable>
+  )
 }
 
-async function buildFilteredSchema(datasource: Datasource, filter?: string[]) {
-  let { tables, error } = await buildSchemaHelper(datasource)
-  let finalTables = tables
-  if (filter) {
-    finalTables = {}
-    for (let key in tables) {
-      if (
-        filter.some((filter: any) => filter.toLowerCase() === key.toLowerCase())
-      ) {
-        finalTables[key] = tables[key]
-      }
+async function buildFilteredSchema(
+  datasource: Datasource,
+  filter?: string[]
+): Promise<Schema> {
+  let schema = await buildSchemaHelper(datasource)
+  if (!filter) {
+    return schema
+  }
+
+  let filteredSchema: Schema = { tables: {}, errors: {} }
+  for (let key in schema.tables) {
+    if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) {
+      filteredSchema.tables[key] = schema.tables[key]
     }
   }
-  return { tables: finalTables, error }
+
+  for (let key in schema.errors) {
+    if (filter.some(filter => filter.toLowerCase() === key.toLowerCase())) {
+      filteredSchema.errors[key] = schema.errors[key]
+    }
+  }
+  return filteredSchema
 }
 
 export async function fetch(ctx: UserCtx) {
@@ -156,7 +129,7 @@ export async function buildSchemaFromDb(ctx: UserCtx) {
   const tablesFilter = ctx.request.body.tablesFilter
   const datasource = await sdk.datasources.get(ctx.params.datasourceId)
 
-  const { tables, error } = await buildFilteredSchema(datasource, tablesFilter)
+  const { tables, errors } = await buildFilteredSchema(datasource, tablesFilter)
   datasource.entities = tables
 
   setDefaultDisplayColumns(datasource)
@@ -164,13 +137,11 @@ export async function buildSchemaFromDb(ctx: UserCtx) {
     sdk.tables.populateExternalTableSchemas(datasource)
   )
   datasource._rev = dbResp.rev
-  const cleanedDatasource = await sdk.datasources.removeSecretSingle(datasource)
 
-  const res: any = { datasource: cleanedDatasource }
-  if (error) {
-    res.error = error
+  ctx.body = {
+    datasource: await sdk.datasources.removeSecretSingle(datasource),
+    errors,
   }
-  ctx.body = res
 }
 
 /**
@@ -298,15 +269,12 @@ export async function save(
     type: plus ? DocumentType.DATASOURCE_PLUS : DocumentType.DATASOURCE,
   }
 
-  let schemaError = null
+  let errors: Record<string, string> = {}
   if (fetchSchema) {
-    const { tables, error } = await buildFilteredSchema(
-      datasource,
-      tablesFilter
-    )
-    schemaError = error
-    datasource.entities = tables
+    const schema = await buildFilteredSchema(datasource, tablesFilter)
+    datasource.entities = schema.tables
     setDefaultDisplayColumns(datasource)
+    errors = schema.errors
   }
 
   if (preSaveAction[datasource.source]) {
@@ -327,13 +295,10 @@ export async function save(
     }
   }
 
-  const response: CreateDatasourceResponse = {
+  ctx.body = {
     datasource: await sdk.datasources.removeSecretSingle(datasource),
+    errors,
   }
-  if (schemaError) {
-    response.error = schemaError
-  }
-  ctx.body = response
   builderSocket?.emitDatasourceUpdate(ctx, datasource)
 }
 

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

@@ -37,7 +37,7 @@ describe("/datasources", () => {
         .expect(200)
 
       expect(res.body.datasource.name).toEqual("Test")
-      expect(res.body.errors).toBeUndefined()
+      expect(res.body.errors).toEqual({})
       expect(events.datasource.created).toBeCalledTimes(1)
     })
   })

packages/server/src/constants/index.ts

@@ -159,11 +159,6 @@ export enum InvalidColumns {
   TABLE_ID = "tableId",
 }
 
-export enum BuildSchemaErrors {
-  NO_KEY = "no_key",
-  INVALID_COLUMN = "invalid_column",
-}
-
 export enum AutomationErrors {
   INCORRECT_TYPE = "INCORRECT_TYPE",
   MAX_ITERATIONS = "MAX_ITERATIONS_REACHED",

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

@@ -18,6 +18,7 @@ import _ from "lodash"
 import { generator } from "@budibase/backend-core/tests"
 import { utils } from "@budibase/backend-core"
 import { databaseTestProviders } from "../integrations/tests/utils"
+import { Client } from "pg"
 
 const config = setup.getConfig()!
 
@@ -1055,4 +1056,46 @@ describe("postgres integrations", () => {
       expect(response.body.tableNames.indexOf(primaryName)).not.toBe(-1)
     })
   })
+
+  describe("POST /api/datasources/:datasourceId/schema", () => {
+    let client: Client
+
+    beforeEach(async () => {
+      client = new Client(
+        (await databaseTestProviders.postgres.getDsConfig()).config!
+      )
+      await client.connect()
+    })
+
+    afterEach(async () => {
+      await client.query(`DROP TABLE IF EXISTS "table"`)
+      await client.end()
+    })
+
+    it("recognises when a table has no primary key", async () => {
+      await client.query(`CREATE TABLE "table" (id SERIAL)`)
+
+      const response = await makeRequest(
+        "post",
+        `/api/datasources/${postgresDatasource._id}/schema`
+      )
+
+      expect(response.body.errors).toEqual({
+        table: "Table must have a primary key.",
+      })
+    })
+
+    it("recognises when a table is using a reserved column name", async () => {
+      await client.query(`CREATE TABLE "table" (_id SERIAL PRIMARY KEY) `)
+
+      const response = await makeRequest(
+        "post",
+        `/api/datasources/${postgresDatasource._id}/schema`
+      )
+
+      expect(response.body.errors).toEqual({
+        table: "Table contains invalid columns.",
+      })
+    })
+  })
 })

packages/server/src/integrations/googlesheets.ts

@@ -14,9 +14,14 @@ import {
   SortJson,
   ExternalTable,
   TableRequest,
+  Schema,
 } from "@budibase/types"
 import { OAuth2Client } from "google-auth-library"
-import { buildExternalTableId, finaliseExternalTables } from "./utils"
+import {
+  buildExternalTableId,
+  checkExternalTables,
+  finaliseExternalTables,
+} from "./utils"
 import { GoogleSpreadsheet, GoogleSpreadsheetRow } from "google-spreadsheet"
 import fetch from "node-fetch"
 import { cache, configs, context, HTTPError } from "@budibase/backend-core"
@@ -138,8 +143,6 @@ const SCHEMA: Integration = {
 class GoogleSheetsIntegration implements DatasourcePlus {
   private readonly config: GoogleSheetsConfig
   private client: GoogleSpreadsheet
-  public tables: Record<string, ExternalTable> = {}
-  public schemaErrors: Record<string, string> = {}
 
   constructor(config: GoogleSheetsConfig) {
     this.config = config
@@ -281,19 +284,37 @@ class GoogleSheetsIntegration implements DatasourcePlus {
   async buildSchema(
     datasourceId: string,
     entities: Record<string, ExternalTable>
-  ) {
+  ): Promise<Schema> {
     // not fully configured yet
     if (!this.config.auth) {
-      return
+      return { tables: {}, errors: {} }
     }
     await this.connect()
     const sheets = this.client.sheetsByIndex
     const tables: Record<string, ExternalTable> = {}
+    let errors: Record<string, string> = {}
     await utils.parallelForeach(
       sheets,
       async sheet => {
         // must fetch rows to determine schema
-        await sheet.getRows()
+        try {
+          await sheet.getRows()
+        } catch (err) {
+          // We expect this to always be an Error so if it's not, rethrow it to
+          // make sure we don't fail quietly.
+          if (!(err instanceof Error)) {
+            throw err
+          }
+
+          if (err.message.startsWith("No values in the header row")) {
+            errors[sheet.title] = err.message
+          } else {
+            // If we get an error we don't expect, rethrow to avoid failing
+            // quietly.
+            throw err
+          }
+          return
+        }
 
         const id = buildExternalTableId(datasourceId, sheet.title)
         tables[sheet.title] = this.getTableSchema(
@@ -305,9 +326,9 @@ class GoogleSheetsIntegration implements DatasourcePlus {
       },
       10
     )
-    const final = finaliseExternalTables(tables, entities)
-    this.tables = final.tables
-    this.schemaErrors = final.errors
+    let externalTables = finaliseExternalTables(tables, entities)
+    errors = { ...errors, ...checkExternalTables(externalTables) }
+    return { tables: externalTables, errors }
   }
 
   async query(json: QueryJson) {

packages/server/src/integrations/microsoftSqlServer.ts

@@ -11,6 +11,7 @@ import {
   DatasourceFeature,
   ConnectionInfo,
   SourceName,
+  Schema,
 } from "@budibase/types"
 import {
   getSqlQuery,
@@ -18,6 +19,7 @@ import {
   convertSqlType,
   finaliseExternalTables,
   SqlClient,
+  checkExternalTables,
 } from "./utils"
 import Sql from "./base/sql"
 import { MSSQLTablesResponse, MSSQLColumn } from "./base/types"
@@ -190,8 +192,6 @@ class SqlServerIntegration extends Sql implements DatasourcePlus {
   private readonly config: MSSQLConfig
   private index: number = 0
   private client?: sqlServer.ConnectionPool
-  public tables: Record<string, ExternalTable> = {}
-  public schemaErrors: Record<string, string> = {}
 
   MASTER_TABLES = [
     "spt_fallback_db",
@@ -381,7 +381,7 @@ class SqlServerIntegration extends Sql implements DatasourcePlus {
   async buildSchema(
     datasourceId: string,
     entities: Record<string, ExternalTable>
-  ) {
+  ): Promise<Schema> {
     await this.connect()
     let tableInfo: MSSQLTablesResponse[] = await this.runSQL(this.TABLES_SQL)
     if (tableInfo == null || !Array.isArray(tableInfo)) {
@@ -445,9 +445,12 @@ class SqlServerIntegration extends Sql implements DatasourcePlus {
         schema,
       }
     }
-    const final = finaliseExternalTables(tables, entities)
-    this.tables = final.tables
-    this.schemaErrors = final.errors
+    let externalTables = finaliseExternalTables(tables, entities)
+    let errors = checkExternalTables(externalTables)
+    return {
+      tables: externalTables,
+      errors,
+    }
   }
 
   async queryTableNames() {

packages/server/src/integrations/mysql.ts

@@ -10,6 +10,7 @@ import {
   DatasourceFeature,
   ConnectionInfo,
   SourceName,
+  Schema,
 } from "@budibase/types"
 import {
   getSqlQuery,
@@ -17,6 +18,7 @@ import {
   buildExternalTableId,
   convertSqlType,
   finaliseExternalTables,
+  checkExternalTables,
 } from "./utils"
 import dayjs from "dayjs"
 import { NUMBER_REGEX } from "../utilities"
@@ -140,8 +142,6 @@ export function bindingTypeCoerce(bindings: any[]) {
 class MySQLIntegration extends Sql implements DatasourcePlus {
   private config: MySQLConfig
   private client?: mysql.Connection
-  public tables: Record<string, ExternalTable> = {}
-  public schemaErrors: Record<string, string> = {}
 
   constructor(config: MySQLConfig) {
     super(SqlClient.MY_SQL)
@@ -279,7 +279,7 @@ class MySQLIntegration extends Sql implements DatasourcePlus {
   async buildSchema(
     datasourceId: string,
     entities: Record<string, ExternalTable>
-  ) {
+  ): Promise<Schema> {
     const tables: { [key: string]: ExternalTable } = {}
     await this.connect()
 
@@ -328,9 +328,10 @@ class MySQLIntegration extends Sql implements DatasourcePlus {
     } finally {
       await this.disconnect()
     }
-    const final = finaliseExternalTables(tables, entities)
-    this.tables = final.tables
-    this.schemaErrors = final.errors
+
+    let externalTables = finaliseExternalTables(tables, entities)
+    let errors = checkExternalTables(tables)
+    return { tables: externalTables, errors }
   }
 
   async queryTableNames() {

packages/server/src/integrations/oracle.ts

@@ -9,9 +9,11 @@ import {
   DatasourcePlus,
   DatasourceFeature,
   ConnectionInfo,
+  Schema,
 } from "@budibase/types"
 import {
   buildExternalTableId,
+  checkExternalTables,
   convertSqlType,
   finaliseExternalTables,
   getSqlQuery,
@@ -108,9 +110,6 @@ class OracleIntegration extends Sql implements DatasourcePlus {
   private readonly config: OracleConfig
   private index: number = 1
 
-  public tables: Record<string, ExternalTable> = {}
-  public schemaErrors: Record<string, string> = {}
-
   private readonly COLUMNS_SQL = `
     SELECT
       tabs.table_name,
@@ -265,7 +264,7 @@ class OracleIntegration extends Sql implements DatasourcePlus {
   async buildSchema(
     datasourceId: string,
     entities: Record<string, ExternalTable>
-  ) {
+  ): Promise<Schema> {
     const columnsResponse = await this.internalQuery<OracleColumnsResponse>({
       sql: this.COLUMNS_SQL,
     })
@@ -326,9 +325,9 @@ class OracleIntegration extends Sql implements DatasourcePlus {
         })
     })
 
-    const final = finaliseExternalTables(tables, entities)
-    this.tables = final.tables
-    this.schemaErrors = final.errors
+    let externalTables = finaliseExternalTables(tables, entities)
+    let errors = checkExternalTables(externalTables)
+    return { tables: externalTables, errors }
   }
 
   async getTableNames() {

packages/server/src/integrations/postgres.ts

@@ -10,6 +10,7 @@ import {
   DatasourceFeature,
   ConnectionInfo,
   SourceName,
+  Schema,
 } from "@budibase/types"
 import {
   getSqlQuery,
@@ -17,6 +18,7 @@ import {
   convertSqlType,
   finaliseExternalTables,
   SqlClient,
+  checkExternalTables,
 } from "./utils"
 import Sql from "./base/sql"
 import { PostgresColumn } from "./base/types"
@@ -145,8 +147,6 @@ class PostgresIntegration extends Sql implements DatasourcePlus {
   private readonly config: PostgresConfig
   private index: number = 1
   private open: boolean
-  public tables: Record<string, ExternalTable> = {}
-  public schemaErrors: Record<string, string> = {}
 
   COLUMNS_SQL!: string
 
@@ -274,7 +274,7 @@ class PostgresIntegration extends Sql implements DatasourcePlus {
   async buildSchema(
     datasourceId: string,
     entities: Record<string, ExternalTable>
-  ) {
+  ): Promise<Schema> {
     let tableKeys: { [key: string]: string[] } = {}
     await this.openConnection()
     try {
@@ -342,9 +342,9 @@ class PostgresIntegration extends Sql implements DatasourcePlus {
         }
       }
 
-      const final = finaliseExternalTables(tables, entities)
-      this.tables = final.tables
-      this.schemaErrors = final.errors
+      let finalizedTables = finaliseExternalTables(tables, entities)
+      let errors = checkExternalTables(finalizedTables)
+      return { tables: finalizedTables, errors }
     } catch (err) {
       // @ts-ignore
       throw new Error(err)

packages/server/src/integrations/utils.ts

@@ -4,13 +4,10 @@ import {
   SearchFilters,
   Datasource,
   FieldType,
+  ExternalTable,
 } from "@budibase/types"
 import { DocumentType, SEPARATOR } from "../db/utils"
-import {
-  BuildSchemaErrors,
-  InvalidColumns,
-  NoEmptyFilterStrings,
-} from "../constants"
+import { InvalidColumns, NoEmptyFilterStrings } from "../constants"
 import { helpers } from "@budibase/shared-core"
 
 const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}`
@@ -266,9 +263,9 @@ export function shouldCopySpecialColumn(
 function copyExistingPropsOver(
   tableName: string,
   table: Table,
-  entities: { [key: string]: any },
-  tableIds: [string]
-) {
+  entities: Record<string, Table>,
+  tableIds: string[]
+): Table {
   if (entities && entities[tableName]) {
     if (entities[tableName]?.primaryDisplay) {
       table.primaryDisplay = entities[tableName].primaryDisplay
@@ -295,42 +292,41 @@ function copyExistingPropsOver(
 
 /**
  * Look through the final table definitions to see if anything needs to be
- * copied over from the old and if any errors have occurred mark them so
- * that the user can be made aware.
+ * copied over from the old.
  * @param tables The list of tables that have been retrieved from the external database.
  * @param entities The old list of tables, if there was any to look for definitions in.
  */
 export function finaliseExternalTables(
-  tables: { [key: string]: any },
-  entities: { [key: string]: any }
-) {
-  const invalidColumns = Object.values(InvalidColumns)
-  let finalTables: { [key: string]: any } = {}
-  const errors: { [key: string]: string } = {}
-  // @ts-ignore
-  const tableIds: [string] = Object.values(tables).map(table => table._id)
+  tables: Record<string, ExternalTable>,
+  entities: Record<string, ExternalTable>
+): Record<string, ExternalTable> {
+  let finalTables: Record<string, Table> = {}
+  const tableIds = Object.values(tables).map(table => table._id!)
   for (let [name, table] of Object.entries(tables)) {
-    const schemaFields = Object.keys(table.schema)
-    // make sure every table has a key
-    if (table.primary == null || table.primary.length === 0) {
-      errors[name] = BuildSchemaErrors.NO_KEY
-      continue
-    } else if (
-      schemaFields.find(field =>
-        invalidColumns.includes(field as InvalidColumns)
-      )
-    ) {
-      errors[name] = BuildSchemaErrors.INVALID_COLUMN
-      continue
-    }
-    // make sure all previous props have been added back
     finalTables[name] = copyExistingPropsOver(name, table, entities, tableIds)
   }
-  // sort the tables by name
-  finalTables = Object.entries(finalTables)
+  // sort the tables by name, this is for the UI to display them in alphabetical order
+  return Object.entries(finalTables)
     .sort(([a], [b]) => a.localeCompare(b))
     .reduce((r, [k, v]) => ({ ...r, [k]: v }), {})
-  return { tables: finalTables, errors }
+}
+
+export function checkExternalTables(
+  tables: Record<string, ExternalTable>
+): Record<string, string> {
+  const invalidColumns = Object.values(InvalidColumns) as string[]
+  const errors: Record<string, string> = {}
+  for (let [name, table] of Object.entries(tables)) {
+    if (!table.primary || table.primary.length === 0) {
+      errors[name] = "Table must have a primary key."
+    }
+
+    const schemaFields = Object.keys(table.schema)
+    if (schemaFields.find(f => invalidColumns.includes(f))) {
+      errors[name] = "Table contains invalid columns."
+    }
+  }
+  return errors
 }
 
 /**

packages/types/src/api/web/app/datasource.ts

@@ -2,7 +2,7 @@ import { Datasource } from "../../../documents"
 
 export interface CreateDatasourceResponse {
   datasource: Datasource
-  error?: any
+  errors: Record<string, string>
 }
 
 export interface UpdateDatasourceResponse {

packages/types/src/sdk/datasources.ts

@@ -1,4 +1,4 @@
-import { Table } from "../documents"
+import { ExternalTable, Table } from "../documents"
 
 export const PASSWORD_REPLACEMENT = "--secret-value--"
 
@@ -175,14 +175,19 @@ export interface IntegrationBase {
   }): void
 }
 
-export interface DatasourcePlus extends IntegrationBase {
-  tables: Record<string, Table>
-  schemaErrors: Record<string, string>
+export interface Schema {
+  tables: Record<string, ExternalTable>
+  errors: Record<string, string>
+}
 
+export interface DatasourcePlus extends IntegrationBase {
   // if the datasource supports the use of bindings directly (to protect against SQL injection)
   // this returns the format of the identifier
   getBindingIdentifier(): string
   getStringConcat(parts: string[]): string
-  buildSchema(datasourceId: string, entities: Record<string, Table>): any
+  buildSchema(
+    datasourceId: string,
+    entities: Record<string, ExternalTable>
+  ): Promise<Schema>
   getTableNames(): Promise<string[]>
 }