From 6f4e1dd711a8d0e85078b9ebcbb1bae10dbec994 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 11:34:55 +0100 Subject: [PATCH] Respond to PR comment. --- .../src/api/routes/tests/viewV2.spec.ts | 167 +++++++++--------- packages/server/src/sdk/app/tables/getters.ts | 22 +-- packages/server/src/sdk/app/views/external.ts | 21 +-- packages/server/src/sdk/app/views/internal.ts | 21 +-- packages/server/src/sdk/app/views/utils.ts | 10 +- packages/shared-core/src/filters.ts | 2 + packages/shared-core/src/utils.ts | 6 +- 7 files changed, 122 insertions(+), 127 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index a061a370ba..2af11b513b 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -27,13 +27,14 @@ import { ViewV2Schema, ViewV2Type, JsonTypes, - UILogicalOperator, EmptyFilterOption, JsonFieldSubType, UISearchFilter, LegacyFilter, SearchViewRowRequest, ArrayOperator, + UILogicalOperator, + SearchFilters, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -159,11 +160,8 @@ describe.each([ tableId: table._id!, primaryDisplay: "id", queryUI: { - logicalOperator: UILogicalOperator.ALL, - onEmptyFilter: EmptyFilterOption.RETURN_ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -256,6 +254,8 @@ describe.each([ }, }, queryUI: { + logicalOperator: UILogicalOperator.ALL, + onEmptyFilter: EmptyFilterOption.RETURN_ALL, groups: [ { logicalOperator: UILogicalOperator.ALL, @@ -952,29 +952,37 @@ describe.each([ ], }) - expect((await config.api.table.get(tableId)).views).toEqual({ - [view.name]: { - ...view, - query: [ - { operator: "equal", field: "newField", value: "thatValue" }, - ], - // Should also update queryUI because query was not previously set. - queryUI: { - groups: [ - { - logicalOperator: "all", - filters: [ - { - operator: "equal", - field: "newField", - value: "thatValue", - }, - ], - }, - ], + const expected: ViewV2 = { + ...view, + query: [ + { + operator: BasicOperator.EQUAL, + field: "newField", + value: "thatValue", }, - schema: expect.anything(), + ], + // Should also update queryUI because query was not previously set. + queryUI: { + onEmptyFilter: EmptyFilterOption.RETURN_ALL, + logicalOperator: UILogicalOperator.ALL, + groups: [ + { + logicalOperator: UILogicalOperator.ALL, + filters: [ + { + operator: BasicOperator.EQUAL, + field: "newField", + value: "thatValue", + }, + ], + }, + ], }, + schema: expect.anything(), + } + + expect((await config.api.table.get(tableId)).views).toEqual({ + [view.name]: expected, }) }) @@ -1014,38 +1022,42 @@ describe.each([ } await config.api.viewV2.update(updatedData) - expect((await config.api.table.get(tableId)).views).toEqual({ - [view.name]: { - ...updatedData, - // queryUI gets generated from query - queryUI: { - groups: [ - { - logicalOperator: "all", - filters: [ - { - operator: "equal", - field: "newField", - value: "newValue", - }, - ], - }, - ], - }, - schema: { - ...table.schema, - id: expect.objectContaining({ - visible: true, - }), - Category: expect.objectContaining({ - visible: false, - }), - Price: expect.objectContaining({ - visible: true, - readonly: true, - }), - }, + const expected: ViewV2 = { + ...updatedData, + // queryUI gets generated from query + queryUI: { + logicalOperator: UILogicalOperator.ALL, + onEmptyFilter: EmptyFilterOption.RETURN_ALL, + groups: [ + { + logicalOperator: UILogicalOperator.ALL, + filters: [ + { + operator: BasicOperator.EQUAL, + field: "newField", + value: "newValue", + }, + ], + }, + ], }, + schema: { + ...table.schema, + id: expect.objectContaining({ + visible: true, + }), + Category: expect.objectContaining({ + visible: false, + }), + Price: expect.objectContaining({ + visible: true, + readonly: true, + }), + }, + } + + expect((await config.api.table.get(tableId)).views).toEqual({ + [view.name]: expected, }) }) @@ -1283,7 +1295,6 @@ describe.each([ queryUI: { groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -1297,7 +1308,8 @@ describe.each([ }) let updatedView = await config.api.viewV2.get(view.id) - expect(updatedView.query).toEqual({ + let expected: SearchFilters = { + onEmptyFilter: EmptyFilterOption.RETURN_ALL, $and: { conditions: [ { @@ -1311,14 +1323,14 @@ describe.each([ }, ], }, - }) + } + expect(updatedView.query).toEqual(expected) await config.api.viewV2.update({ ...updatedView, queryUI: { groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -1332,7 +1344,8 @@ describe.each([ }) updatedView = await config.api.viewV2.get(view.id) - expect(updatedView.query).toEqual({ + expected = { + onEmptyFilter: EmptyFilterOption.RETURN_ALL, $and: { conditions: [ { @@ -1346,7 +1359,8 @@ describe.each([ }, ], }, - }) + } + expect(updatedView.query).toEqual(expected) }) it("can delete either query and it will get regenerated from queryUI", async () => { @@ -1386,7 +1400,6 @@ describe.each([ queryUI: { groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -1810,7 +1823,9 @@ describe.each([ }) const view = await getDelegate(res) - expect(view.queryUI).toEqual({ + const expected: UISearchFilter = { + onEmptyFilter: EmptyFilterOption.RETURN_ALL, + logicalOperator: UILogicalOperator.ALL, groups: [ { logicalOperator: UILogicalOperator.ALL, @@ -1823,7 +1838,8 @@ describe.each([ ], }, ], - }) + } + expect(view.queryUI).toEqual(expected) }) }) @@ -2669,11 +2685,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -2733,11 +2746,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -3022,11 +3032,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.NOT_EQUAL, @@ -3080,11 +3087,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.NOT_EQUAL, @@ -3175,11 +3179,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -3619,11 +3620,8 @@ describe.each([ name: generator.guid(), type: ViewV2Type.CALCULATION, queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, @@ -3912,11 +3910,8 @@ describe.each([ tableId: table._id!, name: generator.guid(), queryUI: { - onEmptyFilter: EmptyFilterOption.RETURN_ALL, - logicalOperator: UILogicalOperator.ALL, groups: [ { - logicalOperator: UILogicalOperator.ALL, filters: [ { operator: BasicOperator.EQUAL, diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index 270ae7fee0..a8ad606647 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -12,8 +12,6 @@ import { TableResponse, TableSourceType, TableViewsResponse, - View, - ViewV2, FeatureFlag, } from "@budibase/types" import datasources from "../datasources" @@ -21,19 +19,6 @@ import sdk from "../../../sdk" import { ensureQueryUISet } from "../views/utils" import { isV2 } from "../views" -function processView(view: ViewV2 | View) { - if (!isV2(view)) { - return - } - ensureQueryUISet(view) -} - -function processViews(views: (ViewV2 | View)[]) { - for (const view of views) { - processView(view) - } -} - export async function processTable(table: Table): Promise { if (!table) { return table @@ -41,7 +26,12 @@ export async function processTable(table: Table): Promise
{ table = { ...table } if (table.views) { - processViews(Object.values(table.views)) + for (const [key, view] of Object.entries(table.views)) { + if (!isV2(view)) { + continue + } + table.views[key] = ensureQueryUISet(view) + } } if (table._id && isExternalTableID(table._id)) { // Old created external tables via Budibase might have a missing field name breaking some UI such as filters diff --git a/packages/server/src/sdk/app/views/external.ts b/packages/server/src/sdk/app/views/external.ts index 74f2248f58..bee153a910 100644 --- a/packages/server/src/sdk/app/views/external.ts +++ b/packages/server/src/sdk/app/views/external.ts @@ -19,8 +19,7 @@ export async function get(viewId: string): Promise { if (!found) { throw new Error("No view found") } - ensureQueryUISet(found) - return found + return ensureQueryUISet(found) } export async function getEnriched(viewId: string): Promise { @@ -35,22 +34,21 @@ export async function getEnriched(viewId: string): Promise { if (!found) { throw new Error("No view found") } - ensureQueryUISet(found) - return await enrichSchema(found, table.schema) + return await enrichSchema(ensureQueryUISet(found), table.schema) } export async function create( tableId: string, viewRequest: Omit ): Promise { - const view: ViewV2 = { + let view: ViewV2 = { ...viewRequest, id: utils.generateViewID(tableId), version: 2, } - ensureQuerySet(view) - ensureQueryUISet(view) + view = ensureQuerySet(view) + view = ensureQueryUISet(view) const db = context.getAppDB() @@ -62,7 +60,10 @@ export async function create( return view } -export async function update(tableId: string, view: ViewV2): Promise { +export async function update( + tableId: string, + view: Readonly +): Promise { const db = context.getAppDB() const { datasourceId, tableName } = breakExternalTableId(tableId) @@ -80,8 +81,8 @@ export async function update(tableId: string, view: ViewV2): Promise { throw new HTTPError(`Cannot update view type after creation`, 400) } - ensureQuerySet(view) - ensureQueryUISet(view) + view = ensureQuerySet(view) + view = ensureQueryUISet(view) delete views[existingView.name] views[view.name] = view diff --git a/packages/server/src/sdk/app/views/internal.ts b/packages/server/src/sdk/app/views/internal.ts index 77a81a1623..63807bcfd4 100644 --- a/packages/server/src/sdk/app/views/internal.ts +++ b/packages/server/src/sdk/app/views/internal.ts @@ -14,8 +14,7 @@ export async function get(viewId: string): Promise { if (!found) { throw new Error("No view found") } - ensureQueryUISet(found) - return found + return ensureQueryUISet(found) } export async function getEnriched(viewId: string): Promise { @@ -26,22 +25,21 @@ export async function getEnriched(viewId: string): Promise { if (!found) { throw new Error("No view found") } - ensureQueryUISet(found) - return await enrichSchema(found, table.schema) + return await enrichSchema(ensureQueryUISet(found), table.schema) } export async function create( tableId: string, viewRequest: Omit ): Promise { - const view: ViewV2 = { + let view: ViewV2 = { ...viewRequest, id: utils.generateViewID(tableId), version: 2, } - ensureQuerySet(view) - ensureQueryUISet(view) + view = ensureQuerySet(view) + view = ensureQueryUISet(view) const db = context.getAppDB() @@ -53,7 +51,10 @@ export async function create( return view } -export async function update(tableId: string, view: ViewV2): Promise { +export async function update( + tableId: string, + view: Readonly +): Promise { const db = context.getAppDB() const table = await sdk.tables.getTable(tableId) table.views ??= {} @@ -69,8 +70,8 @@ export async function update(tableId: string, view: ViewV2): Promise { throw new HTTPError(`Cannot update view type after creation`, 400) } - ensureQuerySet(view) - ensureQueryUISet(view) + view = ensureQuerySet(view) + view = ensureQueryUISet(view) delete table.views[existingView.name] table.views[view.name] = view diff --git a/packages/server/src/sdk/app/views/utils.ts b/packages/server/src/sdk/app/views/utils.ts index ec942c617a..8ca5fd1098 100644 --- a/packages/server/src/sdk/app/views/utils.ts +++ b/packages/server/src/sdk/app/views/utils.ts @@ -1,13 +1,14 @@ import { ViewV2 } from "@budibase/types" import { utils, dataFilters } from "@budibase/shared-core" -import { isPlainObject } from "lodash" +import { cloneDeep, isPlainObject } from "lodash" import { HTTPError } from "@budibase/backend-core" function isEmptyObject(obj: any) { return obj && isPlainObject(obj) && Object.keys(obj).length === 0 } -export function ensureQueryUISet(view: ViewV2) { +export function ensureQueryUISet(viewArg: Readonly): ViewV2 { + const view = cloneDeep(viewArg) if (!view.queryUI && view.query && !isEmptyObject(view.query)) { if (!Array.isArray(view.query)) { // In practice this should not happen. `view.query`, at the time this code @@ -27,13 +28,16 @@ export function ensureQueryUISet(view: ViewV2) { view.queryUI = utils.processSearchFilters(view.query) } + return view } -export function ensureQuerySet(view: ViewV2) { +export function ensureQuerySet(viewArg: Readonly): ViewV2 { + const view = cloneDeep(viewArg) // We consider queryUI to be the source of truth, so we don't check for the // presence of query here. We will overwrite it regardless of whether it is // present or not. if (view.queryUI && !isEmptyObject(view.queryUI)) { view.query = dataFilters.buildQuery(view.queryUI) } + return view } diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index efdd3b0df2..a5d4e7ca4d 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -475,6 +475,8 @@ export function buildQuery( const query: SearchFilters = {} if (filter.onEmptyFilter) { query.onEmptyFilter = filter.onEmptyFilter + } else { + query.onEmptyFilter = EmptyFilterOption.RETURN_ALL } query[operator] = { diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 0af27d4e81..5db4ead1dc 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -7,6 +7,7 @@ import { ArrayOperator, isLogicalSearchOperator, SearchFilter, + EmptyFilterOption, } from "@budibase/types" import * as Constants from "./constants" import { removeKeyNumbering, splitFiltersArray } from "./filters" @@ -139,10 +140,11 @@ export function isSupportedUserSearch(query: SearchFilters) { export const processSearchFilters = ( filterArray: LegacyFilter[] -): UISearchFilter => { +): Required => { const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray) return { - onEmptyFilter, + logicalOperator: UILogicalOperator.ALL, + onEmptyFilter: onEmptyFilter || EmptyFilterOption.RETURN_ALL, groups: [ { logicalOperator: allOr ? UILogicalOperator.ANY : UILogicalOperator.ALL,