Merge branch 'v3-ui' into error-on-bulkimporting-relationship-fields

lerna.json

@@ -1,6 +1,6 @@
 {
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
-  "version": "2.33.12",
+  "version": "2.33.13",
   "npmClient": "yarn",
   "packages": [
     "packages/*",

packages/backend-core/src/sql/sql.ts

@@ -529,7 +529,7 @@ class InternalBuilder {
         if (!matchesTableName) {
           updatedKey = filterKey.replace(
             new RegExp(`^${relationship.column}.`),
-            `${aliases![relationship.tableName]}.`
+            `${aliases?.[relationship.tableName] || relationship.tableName}.`
           )
         } else {
           updatedKey = filterKey
@@ -1091,7 +1091,14 @@ class InternalBuilder {
             )
           }
         } else {
-          query = query.count(`* as ${aggregation.name}`)
+          if (this.client === SqlClient.ORACLE) {
+            const field = this.convertClobs(`${tableName}.${aggregation.field}`)
+            query = query.select(
+              this.knex.raw(`COUNT(??) as ??`, [field, aggregation.name])
+            )
+          } else {
+            query = query.count(`${aggregation.field} as ${aggregation.name}`)
+          }
         }
       } else {
         const fieldSchema = this.getFieldSchema(aggregation.field)
@@ -1579,7 +1586,7 @@ class InternalBuilder {
     query = this.addFilters(query, filters, { relationship: true })
 
     // handle relationships with a CTE for all others
-    if (relationships?.length) {
+    if (relationships?.length && aggregations.length === 0) {
       const mainTable =
         this.query.tableAliases?.[this.query.endpoint.entityId] ||
         this.query.endpoint.entityId
@@ -1594,10 +1601,8 @@ class InternalBuilder {
       // add JSON aggregations attached to the CTE
       return this.addJsonRelationships(cte, tableName, relationships)
     }
-    // no relationships found - return query
-    else {
-      return query
-    }
+
+    return query
   }
 
   update(opts: QueryOptions): Knex.QueryBuilder {

packages/builder/src/pages/builder/portal/users/users/index.svelte

@@ -406,7 +406,7 @@
     allowSelectRows={!readonly}
     {customRenderers}
     loading={!$fetch.loaded || !groupsLoaded}
-    defaultSortColumn={"__selectable"}
+    defaultSortColumn={"access"}
   />
 
   <div class="pagination">

packages/server/nodemon.json

@@ -8,6 +8,6 @@
     "../string-templates"
   ],
   "ext": "js,ts,json,svelte",
-  "ignore": ["**/*.spec.ts", "**/*.spec.js", "../*/dist/**/*"],
+  "ignore": ["**/*.spec.ts", "**/*.spec.js", "../*/dist/**/*", "client/**/*", "builder/**/*"],
   "exec": "yarn build && node --no-node-snapshot ./dist/index.js"
 }

packages/server/src/api/controllers/row/ExternalRequest.ts

@@ -682,7 +682,19 @@ export class ExternalRequest<T extends Operation> {
     filters = this.prepareFilters(id, filters || {}, table)
     const relationships = buildExternalRelationships(table, this.tables)
 
+    let aggregations: Aggregation[] = []
+    if (sdk.views.isView(this.source)) {
+      const calculationFields = helpers.views.calculationFields(this.source)
+      for (const [key, field] of Object.entries(calculationFields)) {
+        aggregations.push({
+          ...field,
+          name: key,
+        })
+      }
+    }
+
     const incRelationships =
+      aggregations.length === 0 &&
       config.includeSqlRelationships === IncludeRelationship.INCLUDE
 
     // clean up row on ingress using schema
@@ -709,17 +721,6 @@ export class ExternalRequest<T extends Operation> {
       throw "Deletion must be filtered"
     }
 
-    let aggregations: Aggregation[] = []
-    if (sdk.views.isView(this.source)) {
-      const calculationFields = helpers.views.calculationFields(this.source)
-      for (const [key, field] of Object.entries(calculationFields)) {
-        aggregations.push({
-          ...field,
-          name: key,
-        })
-      }
-    }
-
     let json: QueryJson = {
       endpoint: {
         datasourceId: this.datasource._id!,

packages/server/src/api/controllers/static/index.ts

@@ -145,6 +145,16 @@ const requiresMigration = async (ctx: Ctx) => {
 }
 
 export const serveApp = async function (ctx: UserCtx) {
+  if (ctx.url.includes("apple-touch-icon.png")) {
+    ctx.redirect("/builder/bblogo.png")
+    return
+  }
+  // no app ID found, cannot serve - return message instead
+  if (!context.getAppId()) {
+    ctx.body = "No content found - requires app ID"
+    return
+  }
+
   const needMigrations = await requiresMigration(ctx)
 
   const bbHeaderEmbed =

packages/server/src/api/controllers/view/viewsV2.ts

@@ -12,6 +12,8 @@ import {
   RelationSchemaField,
   ViewFieldMetadata,
   CalculationType,
+  CountDistinctCalculationFieldMetadata,
+  CountCalculationFieldMetadata,
 } from "@budibase/types"
 import { builderSocket, gridSocket } from "../../../websockets"
 import { helpers } from "@budibase/shared-core"
@@ -22,27 +24,31 @@ function stripUnknownFields(
   if (helpers.views.isCalculationField(field)) {
     if (field.calculationType === CalculationType.COUNT) {
       if ("distinct" in field && field.distinct) {
-        return {
+        const strippedField: RequiredKeys<CountDistinctCalculationFieldMetadata> =
+          {
+            order: field.order,
+            width: field.width,
+            visible: field.visible,
+            readonly: field.readonly,
+            icon: field.icon,
+            distinct: field.distinct,
+            calculationType: field.calculationType,
+            field: field.field,
+            columns: field.columns,
+          }
+        return strippedField
+      } else {
+        const strippedField: RequiredKeys<CountCalculationFieldMetadata> = {
           order: field.order,
           width: field.width,
           visible: field.visible,
           readonly: field.readonly,
           icon: field.icon,
-          distinct: field.distinct,
           calculationType: field.calculationType,
           field: field.field,
           columns: field.columns,
         }
-      } else {
-        return {
-          order: field.order,
-          width: field.width,
-          visible: field.visible,
-          readonly: field.readonly,
-          icon: field.icon,
-          calculationType: field.calculationType,
-          columns: field.columns,
-        }
+        return strippedField
       }
     }
     const strippedField: RequiredKeys<ViewCalculationFieldMetadata> = {

packages/server/src/api/routes/tests/static.spec.ts

@@ -152,4 +152,22 @@ describe("/static", () => {
       expect(res.body.builderPreview).toBe(true)
     })
   })
+
+  describe("/", () => {
+    it("should move permanently from base call (public call)", async () => {
+      const res = await request.get(`/`)
+      expect(res.status).toEqual(301)
+      expect(res.text).toEqual(
+        `Redirecting to <a href="/builder">/builder</a>.`
+      )
+    })
+
+    it("should not error when trying to get 'apple-touch-icon.png' (public call)", async () => {
+      const res = await request.get(`/apple-touch-icon.png`)
+      expect(res.status).toEqual(302)
+      expect(res.text).toEqual(
+        `Redirecting to <a href="/builder/bblogo.png">/builder/bblogo.png</a>.`
+      )
+    })
+  })
 })

packages/server/src/api/routes/tests/viewV2.spec.ts

@@ -693,6 +693,12 @@ describe.each([
                 calculationType: CalculationType.COUNT,
                 field: "Price",
               },
+              countDistinct: {
+                visible: true,
+                calculationType: CalculationType.COUNT,
+                distinct: true,
+                field: "Price",
+              },
               min: {
                 visible: true,
                 calculationType: CalculationType.MIN,
@@ -708,11 +714,6 @@ describe.each([
                 calculationType: CalculationType.AVG,
                 field: "Price",
               },
-              sum2: {
-                visible: true,
-                calculationType: CalculationType.SUM,
-                field: "Price",
-              },
             },
           },
           {
@@ -763,10 +764,12 @@ describe.each([
               count: {
                 visible: true,
                 calculationType: CalculationType.COUNT,
+                field: "Price",
               },
               count2: {
                 visible: true,
                 calculationType: CalculationType.COUNT,
+                field: "Price",
               },
             },
           },
@@ -774,7 +777,7 @@ describe.each([
             status: 400,
             body: {
               message:
-                'Duplicate calculation on field "*", calculation type "count"',
+                'Duplicate calculation on field "Price", calculation type "count"',
             },
           }
         )
@@ -805,7 +808,7 @@ describe.each([
             status: 400,
             body: {
               message:
-                'Duplicate calculation on field "Price", calculation type "count"',
+                'Duplicate calculation on field "Price", calculation type "count distinct"',
             },
           }
         )
@@ -820,6 +823,7 @@ describe.each([
             count: {
               visible: true,
               calculationType: CalculationType.COUNT,
+              field: "Price",
             },
             count2: {
               visible: true,
@@ -831,6 +835,26 @@ describe.each([
         })
       })
 
+      it("does not confuse counts on different fields in the duplicate check", async () => {
+        await config.api.viewV2.create({
+          tableId: table._id!,
+          name: generator.guid(),
+          type: ViewV2Type.CALCULATION,
+          schema: {
+            count: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+              field: "Price",
+            },
+            count2: {
+              visible: true,
+              calculationType: CalculationType.COUNT,
+              field: "Category",
+            },
+          },
+        })
+      })
+
       !isLucene &&
         it("does not get confused when a calculation field shadows a basic one", async () => {
           const table = await config.api.table.save(
@@ -1607,6 +1631,7 @@ describe.each([
             view.schema!.count = {
               visible: true,
               calculationType: CalculationType.COUNT,
+              field: "age",
             }
             await config.api.viewV2.update(view)
 
@@ -3659,6 +3684,153 @@ describe.each([
               )
             })
 
+            it("should be able to filter on relationships", async () => {
+              const companies = await config.api.table.save(
+                saveTableRequest({
+                  schema: {
+                    name: {
+                      name: "name",
+                      type: FieldType.STRING,
+                    },
+                  },
+                })
+              )
+
+              const employees = await config.api.table.save(
+                saveTableRequest({
+                  schema: {
+                    age: {
+                      type: FieldType.NUMBER,
+                      name: "age",
+                    },
+                    name: {
+                      type: FieldType.STRING,
+                      name: "name",
+                    },
+                    company: {
+                      type: FieldType.LINK,
+                      name: "company",
+                      tableId: companies._id!,
+                      relationshipType: RelationshipType.ONE_TO_MANY,
+                      fieldName: "company",
+                    },
+                  },
+                })
+              )
+
+              const view = await config.api.viewV2.create({
+                tableId: employees._id!,
+                name: generator.guid(),
+                type: ViewV2Type.CALCULATION,
+                queryUI: {
+                  groups: [
+                    {
+                      filters: [
+                        {
+                          operator: BasicOperator.EQUAL,
+                          field: "company.name",
+                          value: "Aperture Science Laboratories",
+                        },
+                      ],
+                    },
+                  ],
+                },
+                schema: {
+                  sum: {
+                    visible: true,
+                    calculationType: CalculationType.SUM,
+                    field: "age",
+                  },
+                },
+              })
+
+              const apertureScience = await config.api.row.save(
+                companies._id!,
+                {
+                  name: "Aperture Science Laboratories",
+                }
+              )
+
+              const blackMesa = await config.api.row.save(companies._id!, {
+                name: "Black Mesa",
+              })
+
+              await Promise.all([
+                config.api.row.save(employees._id!, {
+                  name: "Alice",
+                  age: 25,
+                  company: apertureScience._id,
+                }),
+                config.api.row.save(employees._id!, {
+                  name: "Bob",
+                  age: 30,
+                  company: apertureScience._id,
+                }),
+                config.api.row.save(employees._id!, {
+                  name: "Charly",
+                  age: 27,
+                  company: blackMesa._id,
+                }),
+                config.api.row.save(employees._id!, {
+                  name: "Danny",
+                  age: 15,
+                  company: blackMesa._id,
+                }),
+              ])
+
+              const { rows } = await config.api.viewV2.search(view.id, {
+                query: {},
+              })
+
+              expect(rows).toHaveLength(1)
+              expect(rows[0].sum).toEqual(55)
+            })
+
+            it("should be able to count non-numeric fields", async () => {
+              const table = await config.api.table.save(
+                saveTableRequest({
+                  schema: {
+                    firstName: {
+                      type: FieldType.STRING,
+                      name: "firstName",
+                    },
+                    lastName: {
+                      type: FieldType.STRING,
+                      name: "lastName",
+                    },
+                  },
+                })
+              )
+
+              const view = await config.api.viewV2.create({
+                tableId: table._id!,
+                name: generator.guid(),
+                type: ViewV2Type.CALCULATION,
+                schema: {
+                  count: {
+                    visible: true,
+                    calculationType: CalculationType.COUNT,
+                    field: "firstName",
+                  },
+                },
+              })
+
+              await config.api.row.bulkImport(table._id!, {
+                rows: [
+                  { firstName: "Jane", lastName: "Smith" },
+                  { firstName: "Jane", lastName: "Doe" },
+                  { firstName: "Alice", lastName: "Smith" },
+                ],
+              })
+
+              const { rows } = await config.api.viewV2.search(view.id, {
+                query: {},
+              })
+
+              expect(rows).toHaveLength(1)
+              expect(rows[0].count).toEqual(3)
+            })
+
             it("should be able to filter rows on the view itself", async () => {
               const table = await config.api.table.save(
                 saveTableRequest({

packages/server/src/sdk/app/applications/applications.ts

@@ -25,10 +25,12 @@ export async function fetch(status: AppStatus, user: ContextUser) {
   const all = status === AppStatus.ALL
   let apps = (await dbCore.getAllApps({ dev, all })) as App[]
 
-  const enrichedUser = await groups.enrichUserRolesFromGroups({
+  // need to type this correctly - add roles back in to convert from ContextUser to User
+  const completeUser: User = {
     ...user,
-    roles: user.roles || {},
-  })
+    roles: user?.roles || {},
+  }
+  const enrichedUser = await groups.enrichUserRolesFromGroups(completeUser)
   apps = filterAppList(enrichedUser, apps)
 
   const appIds = apps

packages/server/src/sdk/app/rows/search/internal/sqs.ts

@@ -351,6 +351,7 @@ export async function search(
           aggregations.push({
             name: key,
             calculationType: field.calculationType,
+            field: mapToUserColumn(field.field),
           })
         }
       } else {

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

@@ -61,12 +61,68 @@ export function isView(view: any): view is ViewV2 {
   return view.id && docIds.isViewId(view.id) && view.version === 2
 }
 
+function guardDuplicateCalculationFields(view: Omit<ViewV2, "id" | "version">) {
+  const seen: Record<string, Record<CalculationType, boolean>> = {}
+  const calculationFields = helpers.views.calculationFields(view)
+  for (const name of Object.keys(calculationFields)) {
+    const schema = calculationFields[name]
+    const isCount = schema.calculationType === CalculationType.COUNT
+    const isDistinct = "distinct" in schema
+
+    if (isCount && isDistinct) {
+      // We do these separately.
+      continue
+    }
+
+    if (seen[schema.field]?.[schema.calculationType]) {
+      throw new HTTPError(
+        `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType}"`,
+        400
+      )
+    }
+    seen[schema.field] ??= {} as Record<CalculationType, boolean>
+    seen[schema.field][schema.calculationType] = true
+  }
+}
+
+function guardDuplicateCountDistinctFields(
+  view: Omit<ViewV2, "id" | "version">
+) {
+  const seen: Record<string, Record<CalculationType, boolean>> = {}
+  const calculationFields = helpers.views.calculationFields(view)
+  for (const name of Object.keys(calculationFields)) {
+    const schema = calculationFields[name]
+    const isCount = schema.calculationType === CalculationType.COUNT
+    if (!isCount) {
+      continue
+    }
+
+    const isDistinct = "distinct" in schema
+    if (!isDistinct) {
+      // We do these separately.
+      continue
+    }
+
+    if (seen[schema.field]?.[schema.calculationType]) {
+      throw new HTTPError(
+        `Duplicate calculation on field "${schema.field}", calculation type "${schema.calculationType} distinct"`,
+        400
+      )
+    }
+    seen[schema.field] ??= {} as Record<CalculationType, boolean>
+    seen[schema.field][schema.calculationType] = true
+  }
+}
+
 async function guardCalculationViewSchema(
   table: Table,
   view: Omit<ViewV2, "id" | "version">
 ) {
   const calculationFields = helpers.views.calculationFields(view)
 
+  guardDuplicateCalculationFields(view)
+  guardDuplicateCountDistinctFields(view)
+
   if (Object.keys(calculationFields).length > 5) {
     throw new HTTPError(
       "Calculation views can only have a maximum of 5 fields",
@@ -74,29 +130,8 @@ async function guardCalculationViewSchema(
     )
   }
 
-  const seen: Record<string, Record<CalculationType, boolean>> = {}
-
   for (const name of Object.keys(calculationFields)) {
     const schema = calculationFields[name]
-    const isCount = schema.calculationType === CalculationType.COUNT
-    const isDistinct = isCount && "distinct" in schema && schema.distinct
-
-    const field = isCount && !isDistinct ? "*" : schema.field
-    if (seen[field]?.[schema.calculationType]) {
-      throw new HTTPError(
-        `Duplicate calculation on field "${field}", calculation type "${schema.calculationType}"`,
-        400
-      )
-    }
-    seen[field] ??= {} as Record<CalculationType, boolean>
-    seen[field][schema.calculationType] = true
-
-    // Count fields that aren't distinct don't need to reference another field,
-    // so we don't validate it.
-    if (isCount && !isDistinct) {
-      continue
-    }
-
     if (!schema.field) {
       throw new HTTPError(
         `Calculation field "${name}" is missing a "field" property`,
@@ -112,6 +147,7 @@ async function guardCalculationViewSchema(
       )
     }
 
+    const isCount = schema.calculationType === CalculationType.COUNT
     if (!isCount && !isNumeric(targetSchema.type)) {
       throw new HTTPError(
         `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`,

packages/types/src/documents/app/view.ts

@@ -54,12 +54,12 @@ export interface NumericCalculationFieldMetadata
 
 export interface CountCalculationFieldMetadata extends BasicViewFieldMetadata {
   calculationType: CalculationType.COUNT
+  field: string
 }
 
 export interface CountDistinctCalculationFieldMetadata
   extends CountCalculationFieldMetadata {
   distinct: true
-  field: string
 }
 
 export type ViewCalculationFieldMetadata =

packages/types/src/sdk/row.ts

@@ -18,11 +18,11 @@ export interface NumericAggregation extends BaseAggregation {
 
 export interface CountAggregation extends BaseAggregation {
   calculationType: CalculationType.COUNT
+  field: string
 }
 
 export interface CountDistinctAggregation extends CountAggregation {
   distinct: true
-  field: string
 }
 
 export type Aggregation =

packages/worker/src/api/controllers/global/users.ts

@@ -312,16 +312,21 @@ export const tenantUserLookup = async (ctx: any) => {
  * So the account holder may not be found until further pagination has occurred
  */
 export const accountHolderLookup = async (ctx: Ctx) => {
-  const users = await userSdk.core.getAllUsers()
-  const response = await userSdk.core.getExistingAccounts(
-    users.map(u => u.email)
-  )
-  const holder = response[0]
-  if (!holder) {
-    return
+  try {
+    const users = await userSdk.core.getAllUsers()
+    const response = await userSdk.core.getExistingAccounts(
+      users.map(u => u.email)
+    )
+    const holder = response[0]
+    if (!holder) {
+      ctx.body = null
+      return
+    }
+    holder._id = users.find(u => u.email === holder.email)?._id
+    ctx.body = holder
+  } catch (e) {
+    ctx.body = null
   }
-  holder._id = users.find(u => u.email === holder.email)?._id
-  ctx.body = holder
 }
 
 /*