Respond to PR comment.

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({
+        const expected: ViewV2 = {
-          [view.name]: {
+          ...view,
-            ...view,
+          query: [
-            query: [
+            {
-              { operator: "equal", field: "newField", value: "thatValue" },
+              operator: BasicOperator.EQUAL,
-            ],
+              field: "newField",
-            // Should also update queryUI because query was not previously set.
+              value: "thatValue",
-            queryUI: {
-              groups: [
-                {
-                  logicalOperator: "all",
-                  filters: [
-                    {
-                      operator: "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({
+        const expected: ViewV2 = {
-          [view.name]: {
+          ...updatedData,
-            ...updatedData,
+          // queryUI gets generated from query
-            // queryUI gets generated from query
+          queryUI: {
-            queryUI: {
+            logicalOperator: UILogicalOperator.ALL,
-              groups: [
+            onEmptyFilter: EmptyFilterOption.RETURN_ALL,
-                {
+            groups: [
-                  logicalOperator: "all",
+              {
-                  filters: [
+                logicalOperator: UILogicalOperator.ALL,
-                    {
+                filters: [
-                      operator: "equal",
+                  {
-                      field: "newField",
+                    operator: BasicOperator.EQUAL,
-                      value: "newValue",
+                    field: "newField",
-                    },
+                    value: "newValue",
-                  ],
+                  },
-                },
+                ],
-              ],
+              },
-            },
+            ],
-            schema: {
-              ...table.schema,
-              id: expect.objectContaining({
-                visible: true,
-              }),
-              Category: expect.objectContaining({
-                visible: false,
-              }),
-              Price: expect.objectContaining({
-                visible: true,
-                readonly: true,
-              }),
-            },
           },
+          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,

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<Table> {
   if (!table) {
     return table
@@ -41,7 +26,12 @@ export async function processTable(table: Table): Promise<Table> {
 
   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

packages/server/src/sdk/app/views/external.ts

@@ -19,8 +19,7 @@ export async function get(viewId: string): Promise<ViewV2> {
   if (!found) {
     throw new Error("No view found")
   }
-  ensureQueryUISet(found)
+  return ensureQueryUISet(found)
-  return found
 }
 
 export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
@@ -35,22 +34,21 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
   if (!found) {
     throw new Error("No view found")
   }
-  ensureQueryUISet(found)
+  return await enrichSchema(ensureQueryUISet(found), table.schema)
-  return await enrichSchema(found, table.schema)
 }
 
 export async function create(
   tableId: string,
   viewRequest: Omit<ViewV2, "id" | "version">
 ): Promise<ViewV2> {
-  const view: ViewV2 = {
+  let view: ViewV2 = {
     ...viewRequest,
     id: utils.generateViewID(tableId),
     version: 2,
   }
 
-  ensureQuerySet(view)
+  view = ensureQuerySet(view)
-  ensureQueryUISet(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<ViewV2> {
+export async function update(
+  tableId: string,
+  view: Readonly<ViewV2>
+): Promise<ViewV2> {
   const db = context.getAppDB()
 
   const { datasourceId, tableName } = breakExternalTableId(tableId)
@@ -80,8 +81,8 @@ export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
-  ensureQuerySet(view)
+  view = ensureQuerySet(view)
-  ensureQueryUISet(view)
+  view = ensureQueryUISet(view)
 
   delete views[existingView.name]
   views[view.name] = view

packages/server/src/sdk/app/views/internal.ts

@@ -14,8 +14,7 @@ export async function get(viewId: string): Promise<ViewV2> {
   if (!found) {
     throw new Error("No view found")
   }
-  ensureQueryUISet(found)
+  return ensureQueryUISet(found)
-  return found
 }
 
 export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
@@ -26,22 +25,21 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
   if (!found) {
     throw new Error("No view found")
   }
-  ensureQueryUISet(found)
+  return await enrichSchema(ensureQueryUISet(found), table.schema)
-  return await enrichSchema(found, table.schema)
 }
 
 export async function create(
   tableId: string,
   viewRequest: Omit<ViewV2, "id" | "version">
 ): Promise<ViewV2> {
-  const view: ViewV2 = {
+  let view: ViewV2 = {
     ...viewRequest,
     id: utils.generateViewID(tableId),
     version: 2,
   }
 
-  ensureQuerySet(view)
+  view = ensureQuerySet(view)
-  ensureQueryUISet(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<ViewV2> {
+export async function update(
+  tableId: string,
+  view: Readonly<ViewV2>
+): Promise<ViewV2> {
   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<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
-  ensureQuerySet(view)
+  view = ensureQuerySet(view)
-  ensureQueryUISet(view)
+  view = ensureQueryUISet(view)
 
   delete table.views[existingView.name]
   table.views[view.name] = view

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>): ViewV2 {
+  const view = cloneDeep<ViewV2>(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>): ViewV2 {
+  const view = cloneDeep<ViewV2>(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
 }

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] = {

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<UISearchFilter> => {
   const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray)
   return {
-    onEmptyFilter,
+    logicalOperator: UILogicalOperator.ALL,
+    onEmptyFilter: onEmptyFilter || EmptyFilterOption.RETURN_ALL,
     groups: [
       {
         logicalOperator: allOr ? UILogicalOperator.ANY : UILogicalOperator.ALL,