From 635049ab0f641913022fa39d61bef35cdd203f59 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 14:02:52 +0000 Subject: [PATCH 1/7] Making sure table IDs input are correctly formatted for comparison. --- globalSetup.ts | 8 ++++---- packages/backend-core/src/sql/utils.ts | 8 ++++++++ packages/server/src/api/controllers/row/index.ts | 12 +++++++----- .../server/src/api/controllers/row/utils/utils.ts | 12 +++++++----- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/globalSetup.ts b/globalSetup.ts index ec4f38b388..0b0e276b49 100644 --- a/globalSetup.ts +++ b/globalSetup.ts @@ -4,8 +4,8 @@ import { getContainerRuntimeClient, } from "testcontainers" import { ContainerInfo } from "dockerode" -import path from "path" -import lockfile from "proper-lockfile" +import * as path from "path" +import * as lockfile from "proper-lockfile" import { execSync } from "child_process" interface DockerContext { @@ -29,8 +29,8 @@ function getCurrentDockerContext(): DockerContext { async function getBudibaseContainers() { const client = await getContainerRuntimeClient() - const conatiners = await client.container.list() - return conatiners.filter( + const containers = await client.container.list() + return containers.filter( container => container.Labels["com.budibase"] === "true" && container.Labels["org.testcontainers"] === "true" diff --git a/packages/backend-core/src/sql/utils.ts b/packages/backend-core/src/sql/utils.ts index 1b80ff337d..f1e0c4c5ce 100644 --- a/packages/backend-core/src/sql/utils.ts +++ b/packages/backend-core/src/sql/utils.ts @@ -66,6 +66,14 @@ export function buildExternalTableId(datasourceId: string, tableName: string) { return `${datasourceId}${DOUBLE_SEPARATOR}${tableName}` } +export function checkTableId(tableId: string) { + if (isExternalTableID(tableId) && tableId.includes(" ")) { + return encodeURIComponent(tableId) + } else { + return tableId + } +} + export function breakExternalTableId(tableId: string) { const parts = tableId.split(DOUBLE_SEPARATOR) let datasourceId = parts.shift() diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 0463c0a565..77c05abb95 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -288,19 +288,21 @@ function replaceTableNamesInFilters( for (const key of Object.keys(filter)) { const matches = key.match(`^(?.+)\\.(?.+)`) - const relation = matches?.groups?.["relation"] + // this is the possible table name which we need to check if it needs to be converted + const relatedTableName = matches?.groups?.["relation"] const field = matches?.groups?.["field"] - if (!relation || !field) { + if (!relatedTableName || !field) { continue } - const table = allTables.find(r => r._id === tableId)! - if (Object.values(table.schema).some(f => f.name === relation)) { + const table = allTables.find(r => r._id === tableId) + const isColumnName = !!table?.schema[relatedTableName] + if (!table || isColumnName) { continue } - const matchedTable = allTables.find(t => t.name === relation) + const matchedTable = allTables.find(t => t.name === relatedTableName) const relationship = Object.values(table.schema).find( f => isRelationshipField(f) && f.tableId === matchedTable?._id ) diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 5b60143792..b729d7470d 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -1,6 +1,6 @@ import * as utils from "../../../../db/utils" -import { docIds } from "@budibase/backend-core" +import { docIds, sql } from "@budibase/backend-core" import { Ctx, DatasourcePlusQueryResponse, @@ -65,19 +65,21 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { const { sourceId } = ctx.params if (docIds.isViewId(sourceId)) { return { - tableId: utils.extractViewInfoFromID(sourceId).tableId, + tableId: sql.utils.checkTableId( + utils.extractViewInfoFromID(sourceId).tableId + ), viewId: sourceId, } } - return { tableId: ctx.params.sourceId } + return { tableId: sql.utils.checkTableId(ctx.params.sourceId) } } // now check for old way of specifying table ID if (ctx.params?.tableId) { - return { tableId: ctx.params.tableId } + return { tableId: sql.utils.checkTableId(ctx.params.tableId) } } // check body for a table ID if (ctx.request.body?.tableId) { - return { tableId: ctx.request.body.tableId } + return { tableId: sql.utils.checkTableId(ctx.request.body.tableId) } } throw new Error("Unable to find table ID in request") } From 0da1dcee48ba3cef6dbe2478e0328727a64cc0f1 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 12 Dec 2024 14:48:06 +0000 Subject: [PATCH 2/7] Convert portal admin store to typescript --- packages/builder/src/api.ts | 6 ++--- packages/builder/src/index.d.ts | 8 ------- .../src/stores/portal/{admin.js => admin.ts} | 23 +++++++++++-------- 3 files changed, 16 insertions(+), 21 deletions(-) delete mode 100644 packages/builder/src/index.d.ts rename packages/builder/src/stores/portal/{admin.js => admin.ts} (86%) diff --git a/packages/builder/src/api.ts b/packages/builder/src/api.ts index 5d1a0beaeb..907354499f 100644 --- a/packages/builder/src/api.ts +++ b/packages/builder/src/api.ts @@ -8,7 +8,7 @@ import { get } from "svelte/store" import { auth, navigation } from "./stores/portal" export const API = createAPIClient({ - attachHeaders: (headers: Record) => { + attachHeaders: headers => { // Attach app ID header from store let appId = get(appStore).appId if (appId) { @@ -22,7 +22,7 @@ export const API = createAPIClient({ } }, - onError: (error: any) => { + onError: error => { const { url, message, status, method, handled } = error || {} // Log any errors that we haven't manually handled @@ -45,7 +45,7 @@ export const API = createAPIClient({ } } }, - onMigrationDetected: (appId: string) => { + onMigrationDetected: appId => { const updatingUrl = `/builder/app/updating/${appId}` if (window.location.pathname === updatingUrl) { diff --git a/packages/builder/src/index.d.ts b/packages/builder/src/index.d.ts deleted file mode 100644 index 1cbad4f12c..0000000000 --- a/packages/builder/src/index.d.ts +++ /dev/null @@ -1,8 +0,0 @@ -declare module "api" { - const API: { - getPlugins: () => Promise - createPlugin: (plugin: object) => Promise - uploadPlugin: (plugin: FormData) => Promise - deletePlugin: (id: string) => Promise - } -} diff --git a/packages/builder/src/stores/portal/admin.js b/packages/builder/src/stores/portal/admin.ts similarity index 86% rename from packages/builder/src/stores/portal/admin.js rename to packages/builder/src/stores/portal/admin.ts index 54757fb314..9003be8953 100644 --- a/packages/builder/src/stores/portal/admin.js +++ b/packages/builder/src/stores/portal/admin.ts @@ -2,23 +2,26 @@ import { writable, get } from "svelte/store" import { API } from "api" import { auth } from "stores/portal" import { banner } from "@budibase/bbui" +import { + ConfigChecklistResponse, + GetEnvironmentResponse, + SystemStatusResponse, +} from "@budibase/types" -export const DEFAULT_CONFIG = { +interface PortalAdminStore extends GetEnvironmentResponse { + loaded: boolean + checklist?: ConfigChecklistResponse + status?: SystemStatusResponse +} + +export const DEFAULT_CONFIG: PortalAdminStore = { loaded: false, multiTenancy: false, cloud: false, isDev: false, disableAccountPortal: false, - accountPortalUrl: "", - importComplete: false, - checklist: { - apps: { checked: false }, - smtp: { checked: false }, - adminUser: { checked: false }, - sso: { checked: false }, - }, - maintenance: [], offlineMode: false, + maintenance: [], } export function createAdminStore() { From 512d87ac36de50e1e596c65759200db83b052801 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 12 Dec 2024 14:57:24 +0000 Subject: [PATCH 3/7] Remove redundant default store value --- packages/builder/src/stores/portal/admin.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/builder/src/stores/portal/admin.ts b/packages/builder/src/stores/portal/admin.ts index 9003be8953..2141cf1b9c 100644 --- a/packages/builder/src/stores/portal/admin.ts +++ b/packages/builder/src/stores/portal/admin.ts @@ -14,18 +14,16 @@ interface PortalAdminStore extends GetEnvironmentResponse { status?: SystemStatusResponse } -export const DEFAULT_CONFIG: PortalAdminStore = { - loaded: false, - multiTenancy: false, - cloud: false, - isDev: false, - disableAccountPortal: false, - offlineMode: false, - maintenance: [], -} - export function createAdminStore() { - const admin = writable(DEFAULT_CONFIG) + const admin = writable({ + loaded: false, + multiTenancy: false, + cloud: false, + isDev: false, + disableAccountPortal: false, + offlineMode: false, + maintenance: [], + }) async function init() { await getChecklist() From 3dfdb740acc6d77d40a2a2e913782b9de06e8d12 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 12 Dec 2024 14:58:55 +0000 Subject: [PATCH 4/7] Ensure admin store checklist is optional --- .../automation/AutomationBuilder/FlowChart/ActionModal.svelte | 2 +- packages/builder/src/pages/builder/admin/_layout.svelte | 2 +- .../src/pages/builder/portal/settings/email/index.svelte | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/ActionModal.svelte b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/ActionModal.svelte index 524e70e329..88201aa225 100644 --- a/packages/builder/src/components/automation/AutomationBuilder/FlowChart/ActionModal.svelte +++ b/packages/builder/src/components/automation/AutomationBuilder/FlowChart/ActionModal.svelte @@ -49,7 +49,7 @@ const disabled = () => { return { SEND_EMAIL_SMTP: { - disabled: !$admin.checklist.smtp.checked, + disabled: !$admin.checklist?.smtp?.checked, message: "Please configure SMTP", }, COLLECT: { diff --git a/packages/builder/src/pages/builder/admin/_layout.svelte b/packages/builder/src/pages/builder/admin/_layout.svelte index f03a7b8285..b304d91710 100644 --- a/packages/builder/src/pages/builder/admin/_layout.svelte +++ b/packages/builder/src/pages/builder/admin/_layout.svelte @@ -9,7 +9,7 @@ $: useAccountPortal = cloud && !$admin.disableAccountPortal onMount(() => { - if ($admin?.checklist?.adminUser.checked || useAccountPortal) { + if ($admin?.checklist?.adminUser?.checked || useAccountPortal) { $redirect("../") } else { loaded = true diff --git a/packages/builder/src/pages/builder/portal/settings/email/index.svelte b/packages/builder/src/pages/builder/portal/settings/email/index.svelte index 0b9fb44bac..4d76c4f49c 100644 --- a/packages/builder/src/pages/builder/portal/settings/email/index.svelte +++ b/packages/builder/src/pages/builder/portal/settings/email/index.svelte @@ -177,7 +177,7 @@ From b57aa1675baab7ad4b4d10cfb58e2636b7178867 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 12 Dec 2024 15:00:45 +0000 Subject: [PATCH 5/7] Update tests --- packages/builder/src/stores/portal/admin.test.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/builder/src/stores/portal/admin.test.js b/packages/builder/src/stores/portal/admin.test.js index a3f1e645c3..a4adb4320a 100644 --- a/packages/builder/src/stores/portal/admin.test.js +++ b/packages/builder/src/stores/portal/admin.test.js @@ -1,5 +1,5 @@ import { it, expect, describe, beforeEach, vi } from "vitest" -import { DEFAULT_CONFIG, createAdminStore } from "./admin" +import { createAdminStore } from "./admin" import { writable, get } from "svelte/store" import { API } from "api" @@ -45,11 +45,6 @@ describe("admin store", () => { ctx.returnedStore = createAdminStore() }) - it("inits the writable store with the default config", () => { - expect(writable).toHaveBeenCalledTimes(1) - expect(writable).toHaveBeenCalledWith(DEFAULT_CONFIG) - }) - it("returns the created store", ctx => { expect(ctx.returnedStore).toEqual({ subscribe: expect.toBe(ctx.writableReturn.subscribe), From 7f62f32adc59e911c9d3e5f52492f472c77b36b9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 15:55:39 +0000 Subject: [PATCH 6/7] Adding test case for tables with spaces. --- .../src/api/routes/tests/search.spec.ts | 59 ++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 67f303aac3..e97f48afbe 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -71,18 +71,27 @@ if (descriptions.length) { let tableOrViewId: string let rows: Row[] - async function basicRelationshipTables(type: RelationshipType) { + async function basicRelationshipTables( + type: RelationshipType, + opts?: { + tableName?: string + primaryColumn?: string + otherColumn?: string + } + ) { const relatedTable = await createTable({ - name: { name: "name", type: FieldType.STRING }, + name: { name: opts?.tableName || "name", type: FieldType.STRING }, }) + + const columnName = opts?.primaryColumn || "productCat" + //@ts-ignore - API accepts this structure, will build out rest of definition const tableId = await createTable({ - name: { name: "name", type: FieldType.STRING }, - //@ts-ignore - API accepts this structure, will build out rest of definition - productCat: { + name: { name: opts?.tableName || "name", type: FieldType.STRING }, + [columnName]: { type: FieldType.LINK, relationshipType: type, - name: "productCat", - fieldName: "product", + name: columnName, + fieldName: opts?.otherColumn || "product", tableId: relatedTable, constraints: { type: "array", @@ -2776,6 +2785,42 @@ if (descriptions.length) { }) }) + isSql && + describe("relationship - table with spaces", () => { + let primaryTable: Table, row: Row + + beforeAll(async () => { + const { relatedTable, tableId } = + await basicRelationshipTables( + RelationshipType.ONE_TO_MANY, + { + tableName: "table with spaces", + primaryColumn: "related", + otherColumn: "related", + } + ) + tableOrViewId = tableId + primaryTable = relatedTable + + row = await config.api.row.save(primaryTable._id!, { + name: "foo", + }) + + await config.api.row.save(tableOrViewId, { + name: "foo", + related: [row._id], + }) + }) + + it("should be able to search by table name with spaces", async () => { + await expectQuery({ + equal: { + ["table with spaces.name"]: "foo", + }, + }).toContain([{ name: "foo" }]) + }) + }) + isSql && describe.each([ RelationshipType.MANY_TO_ONE, From f857c36e09813c3224be4afaa5d6809f0612ccbf Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Dec 2024 16:50:21 +0000 Subject: [PATCH 7/7] PR comments. --- packages/backend-core/src/sql/utils.ts | 10 +++------- packages/server/src/api/controllers/row/utils/utils.ts | 10 ++++------ packages/server/src/db/utils.ts | 10 ++++++++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/backend-core/src/sql/utils.ts b/packages/backend-core/src/sql/utils.ts index f1e0c4c5ce..14127a189f 100644 --- a/packages/backend-core/src/sql/utils.ts +++ b/packages/backend-core/src/sql/utils.ts @@ -59,15 +59,11 @@ export function isExternalTable(table: Table) { } export function buildExternalTableId(datasourceId: string, tableName: string) { - // encode spaces - if (tableName.includes(" ")) { - tableName = encodeURIComponent(tableName) - } - return `${datasourceId}${DOUBLE_SEPARATOR}${tableName}` + return `${datasourceId}${DOUBLE_SEPARATOR}${encodeURIComponent(tableName)}` } -export function checkTableId(tableId: string) { - if (isExternalTableID(tableId) && tableId.includes(" ")) { +export function encodeTableId(tableId: string) { + if (isExternalTableID(tableId)) { return encodeURIComponent(tableId) } else { return tableId diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index b729d7470d..baa811fe90 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -65,21 +65,19 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { const { sourceId } = ctx.params if (docIds.isViewId(sourceId)) { return { - tableId: sql.utils.checkTableId( - utils.extractViewInfoFromID(sourceId).tableId - ), + tableId: utils.extractViewInfoFromID(sourceId).tableId, viewId: sourceId, } } - return { tableId: sql.utils.checkTableId(ctx.params.sourceId) } + return { tableId: sql.utils.encodeTableId(ctx.params.sourceId) } } // now check for old way of specifying table ID if (ctx.params?.tableId) { - return { tableId: sql.utils.checkTableId(ctx.params.tableId) } + return { tableId: sql.utils.encodeTableId(ctx.params.tableId) } } // check body for a table ID if (ctx.request.body?.tableId) { - return { tableId: sql.utils.checkTableId(ctx.request.body.tableId) } + return { tableId: sql.utils.encodeTableId(ctx.request.body.tableId) } } throw new Error("Unable to find table ID in request") } diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index 6c1065e847..70c69b3c60 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -1,4 +1,10 @@ -import { context, db as dbCore, docIds, utils } from "@budibase/backend-core" +import { + context, + db as dbCore, + docIds, + utils, + sql, +} from "@budibase/backend-core" import { DatabaseQueryOpts, Datasource, @@ -328,7 +334,7 @@ export function extractViewInfoFromID(viewId: string) { const regex = new RegExp(`^(?.+)${SEPARATOR}([^${SEPARATOR}]+)$`) const res = regex.exec(viewId) return { - tableId: res!.groups!["tableId"], + tableId: sql.utils.encodeTableId(res!.groups!["tableId"]), } }