From 672469526e95181280543d646fa302c86fc79fee Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 7 Oct 2024 16:33:14 +0100 Subject: [PATCH] Mark calculation views explicitly instead of figuring it out implicitly. --- .../src/api/controllers/view/viewsV2.ts | 2 + .../src/api/routes/tests/viewV2.spec.ts | 64 ++++++++++++++++++- packages/server/src/sdk/app/views/external.ts | 3 + packages/server/src/sdk/app/views/index.ts | 7 ++ packages/server/src/sdk/app/views/internal.ts | 4 ++ packages/shared-core/src/helpers/views.ts | 5 ++ packages/types/src/documents/app/view.ts | 5 ++ 7 files changed, 88 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index c98940ed88..c60ab497cd 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -127,6 +127,7 @@ export async function create(ctx: Ctx) { const parsedView: Omit, "id" | "version"> = { name: view.name, + type: view.type, tableId: view.tableId, query: view.query, queryUI: view.queryUI, @@ -163,6 +164,7 @@ export async function update(ctx: Ctx) { const parsedView: RequiredKeys = { id: view.id, name: view.name, + type: view.type, version: view.version, tableId: view.tableId, query: view.query, diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 5238b9b752..0a8dd2ac70 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -26,6 +26,7 @@ import { BBReferenceFieldSubType, NumericCalculationFieldMetadata, ViewV2Schema, + ViewV2Type, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -155,7 +156,7 @@ describe.each([ }) it("can persist views with all fields", async () => { - const newView: Required> = { + const newView: Required> = { name: generator.name(), tableId: table._id!, primaryDisplay: "id", @@ -549,6 +550,7 @@ describe.each([ let view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { sum: { visible: true, @@ -571,6 +573,29 @@ describe.each([ expect(sum.calculationType).toEqual(CalculationType.SUM) expect(sum.field).toEqual("Price") }) + + it("cannot create a view with calculation fields unless it has the right type", async () => { + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "Price", + }, + }, + }, + { + status: 400, + body: { + message: + "Calculation fields are not allowed in non-calculation views", + }, + } + ) + }) }) describe("update", () => { @@ -615,7 +640,9 @@ describe.each([ it("can update all fields", async () => { const tableId = table._id! - const updatedData: Required> = { + const updatedData: Required< + Omit + > = { version: view.version, id: view.id, tableId, @@ -830,6 +857,32 @@ describe.each([ ) }) + it("cannot update view type after creation", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + Price: { + visible: true, + }, + }, + }) + + await config.api.viewV2.update( + { + ...view, + type: ViewV2Type.CALCULATION, + }, + { + status: 400, + body: { + message: "Cannot update view type after creation", + }, + } + ) + }) + isInternal && it("updating schema will only validate modified field", async () => { let view = await config.api.viewV2.create({ @@ -902,6 +955,7 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { country: { visible: true, @@ -2639,6 +2693,7 @@ describe.each([ it("should be able to search by calculations", async () => { const view = await config.api.viewV2.create({ tableId: table._id!, + type: ViewV2Type.CALCULATION, name: generator.guid(), schema: { "Quantity Sum": { @@ -2673,6 +2728,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { quantity: { visible: true, @@ -2711,6 +2767,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { aggregate: { visible: true, @@ -2771,6 +2828,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { count: { visible: true, @@ -2805,6 +2863,7 @@ describe.each([ { tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { count: { visible: true, @@ -2853,6 +2912,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { sum: { visible: true, diff --git a/packages/server/src/sdk/app/views/external.ts b/packages/server/src/sdk/app/views/external.ts index 3afd7e9bf9..b69ac0b9eb 100644 --- a/packages/server/src/sdk/app/views/external.ts +++ b/packages/server/src/sdk/app/views/external.ts @@ -70,6 +70,9 @@ export async function update(tableId: string, view: ViewV2): Promise { if (!existingView || !existingView.name) { throw new HTTPError(`View ${view.id} not found in table ${tableId}`, 404) } + if (isV2(existingView) && existingView.type !== view.type) { + throw new HTTPError(`Cannot update view type after creation`, 400) + } delete views[existingView.name] views[view.name] = view diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 73785edd98..e217f16400 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -111,6 +111,13 @@ async function guardViewSchema( if (helpers.views.isCalculationView(view)) { await guardCalculationViewSchema(table, view) + } else { + if (helpers.views.hasCalculationFields(view)) { + throw new HTTPError( + "Calculation fields are not allowed in non-calculation views", + 400 + ) + } } await checkReadonlyFields(table, view) diff --git a/packages/server/src/sdk/app/views/internal.ts b/packages/server/src/sdk/app/views/internal.ts index 19a9f6ab14..96b41bffe8 100644 --- a/packages/server/src/sdk/app/views/internal.ts +++ b/packages/server/src/sdk/app/views/internal.ts @@ -59,6 +59,10 @@ export async function update(tableId: string, view: ViewV2): Promise { throw new HTTPError(`View ${view.id} not found in table ${tableId}`, 404) } + if (isV2(existingView) && existingView.type !== view.type) { + throw new HTTPError(`Cannot update view type after creation`, 400) + } + delete table.views[existingView.name] table.views[view.name] = view await db.put(table) diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts index f41c66adc8..0113375adf 100644 --- a/packages/shared-core/src/helpers/views.ts +++ b/packages/shared-core/src/helpers/views.ts @@ -3,6 +3,7 @@ import { ViewCalculationFieldMetadata, ViewFieldMetadata, ViewV2, + ViewV2Type, } from "@budibase/types" import { pickBy } from "lodash" @@ -21,6 +22,10 @@ export function isBasicViewField( type UnsavedViewV2 = Omit export function isCalculationView(view: UnsavedViewV2) { + return view.type === ViewV2Type.CALCULATION +} + +export function hasCalculationFields(view: UnsavedViewV2) { return Object.values(view.schema || {}).some(isCalculationField) } diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index c58852ecea..02c9b23e38 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -79,10 +79,15 @@ export enum CalculationType { MAX = "max", } +export enum ViewV2Type { + CALCULATION = "calculation", +} + export interface ViewV2 { version: 2 id: string name: string + type?: ViewV2Type primaryDisplay?: string tableId: string query?: LegacyFilter[] | SearchFilters