From b0af50e6746af6686e2b8f22dc96deab1a4928c6 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 16:17:41 +0200 Subject: [PATCH 1/4] Copy fix --- packages/server/src/sdk/app/views/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index b088051773..2d34a3f5a5 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -77,7 +77,7 @@ async function guardViewSchema( if (!viewSchemaField?.visible) { throw new HTTPError( - `You can't hide "${field.name} because it is a required field."`, + `You can't hide "${field.name}" because it is a required field.`, 400 ) } From 7aa6af6e134a80e8ef863244ca581352a0243b61 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 16:40:09 +0200 Subject: [PATCH 2/4] Add tests to support existing configs --- .../src/api/routes/tests/viewV2.spec.ts | 48 ++++++++++++++++++- packages/server/src/sdk/app/views/index.ts | 11 ++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index c8035bd578..bc48b54b51 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -22,7 +22,7 @@ import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" import merge from "lodash/merge" import { quotas } from "@budibase/pro" -import { roles } from "@budibase/backend-core" +import { db, roles } from "@budibase/backend-core" describe.each([ ["internal", undefined], @@ -840,6 +840,52 @@ describe.each([ }) ) }) + + it("updating schema will only validate modified field", async () => { + let view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + Price: { + visible: true, + }, + Category: { visible: true }, + }, + }) + + // Update the view to an invalid state + const tableToUpdate = await config.api.table.get(table._id!) + ;(tableToUpdate.views![view.name] as ViewV2).schema!.id.visible = false + await db.getDB(config.appId!).put(tableToUpdate) + + view = await config.api.viewV2.get(view.id) + await config.api.viewV2.update({ + ...view, + schema: { + ...view.schema, + Price: { + visible: false, + }, + }, + }) + + expect(await config.api.viewV2.get(view.id)).toEqual( + expect.objectContaining({ + schema: { + id: expect.objectContaining({ + visible: false, + }), + Price: expect.objectContaining({ + visible: false, + }), + Category: expect.objectContaining({ + visible: true, + }), + }, + }) + ) + }) }) describe("delete", () => { diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 2d34a3f5a5..b6ac7b6f6b 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -9,7 +9,7 @@ import { import { HTTPError, db as dbCore } from "@budibase/backend-core" import { features } from "@budibase/pro" import { helpers } from "@budibase/shared-core" -import { cloneDeep } from "lodash" +import { cloneDeep } from "lodash/fp" import * as utils from "../../../db/utils" import { isExternalTableID } from "../../../integrations/utils" @@ -68,12 +68,21 @@ async function guardViewSchema( } } + const existingView = + table?.views && (table.views[view.name] as ViewV2 | undefined) + for (const field of Object.values(table.schema)) { if (!helpers.schema.isRequired(field.constraints)) { continue } const viewSchemaField = viewSchema[field.name] + const existingViewSchema = + existingView?.schema && existingView.schema[field.name] + if (!viewSchemaField && !existingViewSchema?.visible) { + // Supporting existing configs with required columns but hidden in views + continue + } if (!viewSchemaField?.visible) { throw new HTTPError( From 47b77d6744f9f7272196201c4a1f6a6f75dc2f7b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 16:46:50 +0200 Subject: [PATCH 3/4] Run test only with internal tables --- .../src/api/routes/tests/viewV2.spec.ts | 83 ++++++++++--------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index bc48b54b51..375c4c7c77 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -841,51 +841,52 @@ describe.each([ ) }) - it("updating schema will only validate modified field", async () => { - let view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - schema: { - id: { visible: true }, - Price: { - visible: true, - }, - Category: { visible: true }, - }, - }) - - // Update the view to an invalid state - const tableToUpdate = await config.api.table.get(table._id!) - ;(tableToUpdate.views![view.name] as ViewV2).schema!.id.visible = false - await db.getDB(config.appId!).put(tableToUpdate) - - view = await config.api.viewV2.get(view.id) - await config.api.viewV2.update({ - ...view, - schema: { - ...view.schema, - Price: { - visible: false, - }, - }, - }) - - expect(await config.api.viewV2.get(view.id)).toEqual( - expect.objectContaining({ + isInternal && + it("updating schema will only validate modified field", async () => { + let view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), schema: { - id: expect.objectContaining({ - visible: false, - }), - Price: expect.objectContaining({ - visible: false, - }), - Category: expect.objectContaining({ + id: { visible: true }, + Price: { visible: true, - }), + }, + Category: { visible: true }, }, }) - ) - }) + + // Update the view to an invalid state + const tableToUpdate = await config.api.table.get(table._id!) + ;(tableToUpdate.views![view.name] as ViewV2).schema!.id.visible = false + await db.getDB(config.appId!).put(tableToUpdate) + + view = await config.api.viewV2.get(view.id) + await config.api.viewV2.update({ + ...view, + schema: { + ...view.schema, + Price: { + visible: false, + }, + }, + }) + + expect(await config.api.viewV2.get(view.id)).toEqual( + expect.objectContaining({ + schema: { + id: expect.objectContaining({ + visible: false, + }), + Price: expect.objectContaining({ + visible: false, + }), + Category: expect.objectContaining({ + visible: true, + }), + }, + }) + ) + }) }) describe("delete", () => { From ac9f5d5d1ee27367366a9bfbf62e256b45646e17 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 4 Jun 2024 16:50:12 +0200 Subject: [PATCH 4/4] Allow editing old configs --- .../components/grid/controls/ColumnsSettingButton.svelte | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte index 9f7ced013d..ed94a01e56 100644 --- a/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte +++ b/packages/frontend-core/src/components/grid/controls/ColumnsSettingButton.svelte @@ -49,12 +49,15 @@ const requiredTooltip = isRequired && "Required columns must be writable" + const editEnabled = + !isRequired || + columnToPermissionOptions(c) !== PERMISSION_OPTIONS.WRITABLE const options = [ { icon: "Edit", value: PERMISSION_OPTIONS.WRITABLE, - tooltip: requiredTooltip || "Writable", - disabled: isRequired, + tooltip: (!editEnabled && requiredTooltip) || "Writable", + disabled: !editEnabled, }, ] if ($datasource.type === "viewV2") {