From 1767650337d5feb0c7cff3a4ef072cec26143605 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 2 May 2024 16:51:48 +0100 Subject: [PATCH 1/7] processInputBBReference vs processInputBBReferences --- .../rowProcessor/bbReferenceProcessor.ts | 160 +++++++++--------- .../src/utilities/rowProcessor/index.ts | 9 +- .../tests/bbReferenceProcessor.spec.ts | 140 ++++++++------- 3 files changed, 160 insertions(+), 149 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts index e5edc65edd..f26c29a0cd 100644 --- a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts +++ b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts @@ -10,99 +10,93 @@ import { InvalidBBRefError } from "./errors" const ROW_PREFIX = DocumentType.ROW + SEPARATOR -export function processInputBBReferences( +export async function processInputBBReference( value: string | { _id: string }, - type: FieldType.BB_REFERENCE_SINGLE -): Promise -export function processInputBBReferences( - value: string | string[] | { _id: string } | { _id: string }[], - type: FieldType.BB_REFERENCE, - subtype: BBReferenceFieldSubType -): Promise + subtype: BBReferenceFieldSubType.USER +): Promise { + if (value && Array.isArray(value)) { + throw "BB_REFERENCE_SINGLE cannot be an array" + } + let id = typeof value === "string" ? value : value?._id -export async function processInputBBReferences( - value: string | string[] | { _id: string } | { _id: string }[], - type: FieldType.BB_REFERENCE | FieldType.BB_REFERENCE_SINGLE, - subtype?: BBReferenceFieldSubType -): Promise { - switch (type) { - case FieldType.BB_REFERENCE: { - let referenceIds: string[] = [] + if (!id) { + return null + } - if (Array.isArray(value)) { - referenceIds.push( - ...value.map(idOrDoc => - typeof idOrDoc === "string" ? idOrDoc : idOrDoc._id - ) - ) - } else if (typeof value !== "string") { - referenceIds.push(value._id) - } else { - referenceIds.push( - ...value - .split(",") - .filter(x => x) - .map((id: string) => id.trim()) - ) + switch (subtype) { + case BBReferenceFieldSubType.USER: { + if (id.startsWith(ROW_PREFIX)) { + id = dbCore.getGlobalIDFromUserMetadataID(id) } - // make sure all reference IDs are correct global user IDs - // they may be user metadata references (start with row prefix) - // and these need to be converted to global IDs - referenceIds = referenceIds.map(id => { - if (id?.startsWith(ROW_PREFIX)) { - return dbCore.getGlobalIDFromUserMetadataID(id) - } else { - return id + try { + await cache.user.getUser(id) + return id + } catch (e: any) { + if (e.statusCode === 404) { + throw new InvalidBBRefError(id, BBReferenceFieldSubType.USER) } - }) - - switch (subtype) { - case undefined: - throw "Subtype must be defined" - case BBReferenceFieldSubType.USER: - case BBReferenceFieldSubType.USERS: { - const { notFoundIds } = await cache.user.getUsers(referenceIds) - - if (notFoundIds?.length) { - throw new InvalidBBRefError( - notFoundIds[0], - BBReferenceFieldSubType.USER - ) - } - - if (!referenceIds?.length) { - return null - } - - if (subtype === BBReferenceFieldSubType.USERS) { - return referenceIds - } - - return referenceIds.join(",") - } - default: - throw utils.unreachable(subtype) + throw e } } - case FieldType.BB_REFERENCE_SINGLE: { - if (value && Array.isArray(value)) { - throw "BB_REFERENCE_SINGLE cannot be an array" - } - - const id = typeof value === "string" ? value : value._id - - const user = await cache.user.getUser(id) - - if (!user) { - throw new InvalidBBRefError(id, BBReferenceFieldSubType.USER) - } - - return user._id! - } default: - throw utils.unreachable(type) + throw utils.unreachable(subtype) + } +} +export async function processInputBBReferences( + value: string | string[] | { _id: string }[], + subtype: BBReferenceFieldSubType +): Promise { + if (!value || !value[0]) { + return null + } + + let referenceIds + if (typeof value === "string") { + referenceIds = value + .split(",") + .map(u => u.trim()) + .filter(u => !!u) + } else { + referenceIds = value.map(idOrDoc => + typeof idOrDoc === "string" ? idOrDoc : idOrDoc._id + ) + } + + // make sure all reference IDs are correct global user IDs + // they may be user metadata references (start with row prefix) + // and these need to be converted to global IDs + referenceIds = referenceIds.map(id => { + if (id?.startsWith(ROW_PREFIX)) { + return dbCore.getGlobalIDFromUserMetadataID(id) + } else { + return id + } + }) + + switch (subtype) { + case undefined: + throw "Subtype must be defined" + case BBReferenceFieldSubType.USER: + case BBReferenceFieldSubType.USERS: { + const { notFoundIds } = await cache.user.getUsers(referenceIds) + + if (notFoundIds?.length) { + throw new InvalidBBRefError( + notFoundIds[0], + BBReferenceFieldSubType.USER + ) + } + + if (!referenceIds?.length) { + return null + } + + return referenceIds + } + default: + throw utils.unreachable(subtype) } } diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 499ea0e4c5..8c8ac6e27d 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -12,6 +12,7 @@ import { } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { + processInputBBReference, processInputBBReferences, processOutputBBReferences, } from "./bbReferenceProcessor" @@ -161,13 +162,9 @@ export async function inputProcessing( delete clonedRow[key].url } } else if (field.type === FieldType.BB_REFERENCE && value) { - clonedRow[key] = await processInputBBReferences( - value, - field.type, - field.subtype - ) + clonedRow[key] = await processInputBBReferences(value, field.subtype) } else if (field.type === FieldType.BB_REFERENCE_SINGLE && value) { - clonedRow[key] = await processInputBBReferences(value, field.type) + clonedRow[key] = await processInputBBReference(value, field.subtype) } } diff --git a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts index 609dc9ffc0..a3c5cf8911 100644 --- a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts @@ -2,6 +2,7 @@ import _ from "lodash" import * as backendCore from "@budibase/backend-core" import { BBReferenceFieldSubType, FieldType, User } from "@budibase/types" import { + processInputBBReference, processInputBBReferences, processOutputBBReferences, } from "../bbReferenceProcessor" @@ -22,6 +23,7 @@ jest.mock("@budibase/backend-core", (): typeof backendCore => { ...actual.cache, user: { ...actual.cache.user, + getUser: jest.fn(actual.cache.user.getUser), getUsers: jest.fn(actual.cache.user.getUsers), }, }, @@ -31,9 +33,6 @@ jest.mock("@budibase/backend-core", (): typeof backendCore => { const config = new DBTestConfiguration() describe("bbReferenceProcessor", () => { - const cacheGetUsersSpy = backendCore.cache.user - .getUsers as jest.MockedFunction - const users: User[] = [] beforeAll(async () => { const userCount = 10 @@ -56,21 +55,81 @@ describe("bbReferenceProcessor", () => { jest.clearAllMocks() }) - describe("processInputBBReferences", () => { + describe("processInputBBReference", () => { describe("subtype user", () => { + const cacheGetUserSpy = backendCore.cache.user + .getUser as jest.MockedFunction + it("validate valid string id", async () => { const user = _.sample(users) const userId = user!._id! const result = await config.doInTenant(() => - processInputBBReferences( - userId, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReference(userId, BBReferenceFieldSubType.USER) ) expect(result).toEqual(userId) + expect(cacheGetUserSpy).toHaveBeenCalledTimes(1) + expect(cacheGetUserSpy).toHaveBeenCalledWith(userId) + }) + + it("throws an error given an invalid id", async () => { + const userId = generator.guid() + + await expect( + config.doInTenant(() => + processInputBBReference(userId, BBReferenceFieldSubType.USER) + ) + ).rejects.toThrow( + new InvalidBBRefError(userId, BBReferenceFieldSubType.USER) + ) + }) + + it("validate valid user object", async () => { + const userId = _.sample(users)!._id! + + const result = await config.doInTenant(() => + processInputBBReference({ _id: userId }, BBReferenceFieldSubType.USER) + ) + + expect(result).toEqual(userId) + expect(cacheGetUserSpy).toHaveBeenCalledTimes(1) + expect(cacheGetUserSpy).toHaveBeenCalledWith(userId) + }) + + it("empty strings will return null", async () => { + const result = await config.doInTenant(() => + processInputBBReference("", BBReferenceFieldSubType.USER) + ) + + expect(result).toEqual(null) + }) + + it("should convert user medata IDs to global IDs", async () => { + const userId = _.sample(users)!._id! + const userMetadataId = backendCore.db.generateUserMetadataID(userId) + const result = await config.doInTenant(() => + processInputBBReference(userMetadataId, BBReferenceFieldSubType.USER) + ) + expect(result).toBe(userId) + }) + }) + }) + + describe("processInputBBReferences", () => { + describe("subtype user", () => { + const cacheGetUsersSpy = backendCore.cache.user + .getUsers as jest.MockedFunction + + it("validate valid string id", async () => { + const user = _.sample(users) + const userId = user!._id! + + const result = await config.doInTenant(() => + processInputBBReferences(userId, BBReferenceFieldSubType.USER) + ) + + expect(result).toEqual([userId]) expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) expect(cacheGetUsersSpy).toHaveBeenCalledWith([userId]) }) @@ -80,11 +139,7 @@ describe("bbReferenceProcessor", () => { await expect( config.doInTenant(() => - processInputBBReferences( - userId, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences(userId, BBReferenceFieldSubType.USER) ) ).rejects.toThrow( new InvalidBBRefError(userId, BBReferenceFieldSubType.USER) @@ -98,14 +153,10 @@ describe("bbReferenceProcessor", () => { const userIdCsv = userIds.join(" , ") const result = await config.doInTenant(() => - processInputBBReferences( - userIdCsv, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences(userIdCsv, BBReferenceFieldSubType.USER) ) - expect(result).toEqual(userIds.join(",")) + expect(result).toEqual(userIds) expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) expect(cacheGetUsersSpy).toHaveBeenCalledWith(userIds) }) @@ -122,45 +173,22 @@ describe("bbReferenceProcessor", () => { await expect( config.doInTenant(() => - processInputBBReferences( - userIdCsv, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences(userIdCsv, BBReferenceFieldSubType.USER) ) ).rejects.toThrow( new InvalidBBRefError(wrongId, BBReferenceFieldSubType.USER) ) }) - it("validate valid user object", async () => { - const userId = _.sample(users)!._id! - - const result = await config.doInTenant(() => - processInputBBReferences( - { _id: userId }, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) - ) - - expect(result).toEqual(userId) - expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) - expect(cacheGetUsersSpy).toHaveBeenCalledWith([userId]) - }) - it("validate valid user object array", async () => { - const userIds = _.sampleSize(users, 3).map(x => x._id!) + const inputUsers = _.sampleSize(users, 3).map(u => ({ _id: u._id! })) + const userIds = inputUsers.map(u => u._id) const result = await config.doInTenant(() => - processInputBBReferences( - userIds, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences(inputUsers, BBReferenceFieldSubType.USER) ) - expect(result).toEqual(userIds.join(",")) + expect(result).toEqual(userIds) expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) expect(cacheGetUsersSpy).toHaveBeenCalledWith(userIds) }) @@ -169,7 +197,7 @@ describe("bbReferenceProcessor", () => { const result = await config.doInTenant(() => processInputBBReferences( "", - FieldType.BB_REFERENCE, + BBReferenceFieldSubType.USER ) ) @@ -179,11 +207,7 @@ describe("bbReferenceProcessor", () => { it("empty arrays will return null", async () => { const result = await config.doInTenant(() => - processInputBBReferences( - [], - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences([], BBReferenceFieldSubType.USER) ) expect(result).toEqual(null) @@ -193,13 +217,9 @@ describe("bbReferenceProcessor", () => { const userId = _.sample(users)!._id! const userMetadataId = backendCore.db.generateUserMetadataID(userId) const result = await config.doInTenant(() => - processInputBBReferences( - userMetadataId, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processInputBBReferences(userMetadataId, BBReferenceFieldSubType.USER) ) - expect(result).toBe(userId) + expect(result).toEqual([userId]) }) }) }) From 2c5e9ff78470e04d7f8e877811c674c197a62a2e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 09:31:24 +0200 Subject: [PATCH 2/7] processOutputBBReference vs processOutputBBReferences --- .../rowProcessor/bbReferenceProcessor.ts | 99 +++++++++---------- .../tests/bbReferenceProcessor.spec.ts | 95 +++++++++++++++--- 2 files changed, 129 insertions(+), 65 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts index f26c29a0cd..ad6291dbc7 100644 --- a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts +++ b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts @@ -1,7 +1,6 @@ import { cache, db as dbCore } from "@budibase/backend-core" import { utils } from "@budibase/shared-core" import { - FieldType, BBReferenceFieldSubType, DocumentType, SEPARATOR, @@ -104,67 +103,30 @@ interface UserReferenceInfo { _id: string primaryDisplay: string email: string - firstName: string - lastName: string + firstName?: string + lastName?: string } -export function processOutputBBReferences( +export async function processOutputBBReference( value: string, - type: FieldType.BB_REFERENCE_SINGLE -): Promise -export function processOutputBBReferences( - value: string, - type: FieldType.BB_REFERENCE, - subtype: BBReferenceFieldSubType -): Promise - -export async function processOutputBBReferences( - value: string | string[], - type: FieldType.BB_REFERENCE | FieldType.BB_REFERENCE_SINGLE, - subtype?: BBReferenceFieldSubType -) { + subtype: BBReferenceFieldSubType.USER +): Promise { if (value === null || value === undefined) { // Already processed or nothing to process return value || undefined } - switch (type) { - case FieldType.BB_REFERENCE: { - const ids = - typeof value === "string" ? value.split(",").filter(id => !!id) : value - - switch (subtype) { - case undefined: - throw "Subtype must be defined" - case BBReferenceFieldSubType.USER: - case BBReferenceFieldSubType.USERS: { - const { users } = await cache.user.getUsers(ids) - if (!users.length) { - return undefined - } - - return users.map(u => ({ - _id: u._id, - primaryDisplay: u.email, - email: u.email, - firstName: u.firstName, - lastName: u.lastName, - })) - } - default: - throw utils.unreachable(subtype) - } - } - case FieldType.BB_REFERENCE_SINGLE: { - if (!value) { - return undefined - } + if (!value) { + return undefined + } + switch (subtype) { + case BBReferenceFieldSubType.USER: let user try { user = await cache.user.getUser(value as string) } catch (err: any) { - if (err.code !== 404) { + if (err.statusCode !== 404) { throw err } } @@ -173,15 +135,46 @@ export async function processOutputBBReferences( } return { - _id: user._id, + _id: user._id!, primaryDisplay: user.email, email: user.email, firstName: user.firstName, lastName: user.lastName, } - } - default: - throw utils.unreachable(type) + throw utils.unreachable(subtype) + } +} + +export async function processOutputBBReferences( + value: string, + subtype: BBReferenceFieldSubType +): Promise { + if (value === null || value === undefined) { + // Already processed or nothing to process + return value || undefined + } + + const ids = + typeof value === "string" ? value.split(",").filter(id => !!id) : value + + switch (subtype) { + case BBReferenceFieldSubType.USER: + case BBReferenceFieldSubType.USERS: { + const { users } = await cache.user.getUsers(ids) + if (!users.length) { + return undefined + } + + return users.map(u => ({ + _id: u._id!, + primaryDisplay: u.email, + email: u.email, + firstName: u.firstName, + lastName: u.lastName, + })) + } + default: + throw utils.unreachable(subtype) } } diff --git a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts index a3c5cf8911..2825c9755d 100644 --- a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts @@ -4,6 +4,7 @@ import { BBReferenceFieldSubType, FieldType, User } from "@budibase/types" import { processInputBBReference, processInputBBReferences, + processOutputBBReference, processOutputBBReferences, } from "../bbReferenceProcessor" import { @@ -33,6 +34,12 @@ jest.mock("@budibase/backend-core", (): typeof backendCore => { const config = new DBTestConfiguration() describe("bbReferenceProcessor", () => { + const cacheGetUserSpy = backendCore.cache.user.getUser as jest.MockedFunction< + typeof backendCore.cache.user.getUser + > + const cacheGetUsersSpy = backendCore.cache.user + .getUsers as jest.MockedFunction + const users: User[] = [] beforeAll(async () => { const userCount = 10 @@ -57,9 +64,6 @@ describe("bbReferenceProcessor", () => { describe("processInputBBReference", () => { describe("subtype user", () => { - const cacheGetUserSpy = backendCore.cache.user - .getUser as jest.MockedFunction - it("validate valid string id", async () => { const user = _.sample(users) const userId = user!._id! @@ -118,9 +122,6 @@ describe("bbReferenceProcessor", () => { describe("processInputBBReferences", () => { describe("subtype user", () => { - const cacheGetUsersSpy = backendCore.cache.user - .getUsers as jest.MockedFunction - it("validate valid string id", async () => { const user = _.sample(users) const userId = user!._id! @@ -224,6 +225,41 @@ describe("bbReferenceProcessor", () => { }) }) + describe("processOutputBBReference", () => { + describe("subtype user", () => { + it("fetches user given a valid string id", async () => { + const user = _.sample(users)! + const userId = user._id! + + const result = await config.doInTenant(() => + processOutputBBReference(userId, BBReferenceFieldSubType.USER) + ) + + expect(result).toEqual({ + _id: user._id, + primaryDisplay: user.email, + email: user.email, + firstName: user.firstName, + lastName: user.lastName, + }) + expect(cacheGetUserSpy).toHaveBeenCalledTimes(1) + expect(cacheGetUserSpy).toHaveBeenCalledWith(userId) + }) + + it("returns undefined given an unexisting user", async () => { + const userId = generator.guid() + + const result = await config.doInTenant(() => + processOutputBBReference(userId, BBReferenceFieldSubType.USER) + ) + + expect(result).toBeUndefined() + expect(cacheGetUserSpy).toHaveBeenCalledTimes(1) + expect(cacheGetUserSpy).toHaveBeenCalledWith(userId) + }) + }) + }) + describe("processOutputBBReferences", () => { describe("subtype user", () => { it("fetches user given a valid string id", async () => { @@ -231,11 +267,7 @@ describe("bbReferenceProcessor", () => { const userId = user._id! const result = await config.doInTenant(() => - processOutputBBReferences( - userId, - FieldType.BB_REFERENCE, - BBReferenceFieldSubType.USER - ) + processOutputBBReferences(userId, BBReferenceFieldSubType.USER) ) expect(result).toEqual([ @@ -259,7 +291,6 @@ describe("bbReferenceProcessor", () => { const result = await config.doInTenant(() => processOutputBBReferences( [userId1, userId2].join(","), - FieldType.BB_REFERENCE, BBReferenceFieldSubType.USER ) ) @@ -279,6 +310,46 @@ describe("bbReferenceProcessor", () => { expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) expect(cacheGetUsersSpy).toHaveBeenCalledWith([userId1, userId2]) }) + + it("trims unexisting users user given a valid string id csv", async () => { + const [user1, user2] = _.sampleSize(users, 2) + const userId1 = user1._id! + const userId2 = user2._id! + + const unexistingUserId1 = generator.guid() + const unexistingUserId2 = generator.guid() + + const input = [ + unexistingUserId1, + userId1, + unexistingUserId2, + userId2, + ].join(",") + + const result = await config.doInTenant(() => + processOutputBBReferences(input, BBReferenceFieldSubType.USER) + ) + + expect(result).toHaveLength(2) + expect(result).toEqual( + expect.arrayContaining( + [user1, user2].map(u => ({ + _id: u._id, + primaryDisplay: u.email, + email: u.email, + firstName: u.firstName, + lastName: u.lastName, + })) + ) + ) + expect(cacheGetUsersSpy).toHaveBeenCalledTimes(1) + expect(cacheGetUsersSpy).toHaveBeenCalledWith([ + unexistingUserId1, + userId1, + unexistingUserId2, + userId2, + ]) + }) }) }) }) From d259bdbf7b2610ed734794db44f9bea06f93a1c2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 09:34:42 +0200 Subject: [PATCH 3/7] Fix --- packages/server/src/utilities/rowProcessor/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 8c8ac6e27d..47aa777852 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -250,7 +250,6 @@ export async function outputProcessing( for (let row of enriched) { row[property] = await processOutputBBReferences( row[property], - column.type, column.subtype ) } @@ -261,7 +260,7 @@ export async function outputProcessing( for (let row of enriched) { row[property] = await processOutputBBReferences( row[property], - column.type + column.subtype ) } } From b3ff417844c3f181ada9567329f3a8c64cb0b2b3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 13:04:44 +0200 Subject: [PATCH 4/7] Lint --- .../server/src/utilities/rowProcessor/bbReferenceProcessor.ts | 3 ++- .../utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts index ad6291dbc7..325b3b37ba 100644 --- a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts +++ b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts @@ -121,7 +121,7 @@ export async function processOutputBBReference( } switch (subtype) { - case BBReferenceFieldSubType.USER: + case BBReferenceFieldSubType.USER: { let user try { user = await cache.user.getUser(value as string) @@ -141,6 +141,7 @@ export async function processOutputBBReference( firstName: user.firstName, lastName: user.lastName, } + } default: throw utils.unreachable(subtype) } diff --git a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts index 2825c9755d..c5a7b7fc84 100644 --- a/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/bbReferenceProcessor.spec.ts @@ -1,6 +1,6 @@ import _ from "lodash" import * as backendCore from "@budibase/backend-core" -import { BBReferenceFieldSubType, FieldType, User } from "@budibase/types" +import { BBReferenceFieldSubType, User } from "@budibase/types" import { processInputBBReference, processInputBBReferences, From ab647d1f0f982d97ba6699b5a51753ee151d3976 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 16:10:36 +0200 Subject: [PATCH 5/7] Fix fetching BB_reference arrays --- packages/server/src/api/controllers/row/utils/basic.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/utils/basic.ts b/packages/server/src/api/controllers/row/utils/basic.ts index 6255e13c1c..02f92e15e9 100644 --- a/packages/server/src/api/controllers/row/utils/basic.ts +++ b/packages/server/src/api/controllers/row/utils/basic.ts @@ -104,7 +104,10 @@ export function basicProcessing({ export function fixArrayTypes(row: Row, table: Table) { for (let [fieldName, schema] of Object.entries(table.schema)) { - if (schema.type === FieldType.ARRAY && typeof row[fieldName] === "string") { + if ( + [FieldType.ARRAY, FieldType.BB_REFERENCE].includes(schema.type) && + typeof row[fieldName] === "string" + ) { try { row[fieldName] = JSON.parse(row[fieldName]) } catch (err) { From d91292f532234a6affb2fe31206bcad532f5dae8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 16:21:35 +0200 Subject: [PATCH 6/7] Handle null or empty on processor --- .../rowProcessor/bbReferenceProcessor.ts | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts index 325b3b37ba..d69fe73052 100644 --- a/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts +++ b/packages/server/src/utilities/rowProcessor/bbReferenceProcessor.ts @@ -108,14 +108,9 @@ interface UserReferenceInfo { } export async function processOutputBBReference( - value: string, + value: string | null | undefined, subtype: BBReferenceFieldSubType.USER ): Promise { - if (value === null || value === undefined) { - // Already processed or nothing to process - return value || undefined - } - if (!value) { return undefined } @@ -148,14 +143,12 @@ export async function processOutputBBReference( } export async function processOutputBBReferences( - value: string, + value: string | null | undefined, subtype: BBReferenceFieldSubType ): Promise { - if (value === null || value === undefined) { - // Already processed or nothing to process - return value || undefined + if (!value) { + return undefined } - const ids = typeof value === "string" ? value.split(",").filter(id => !!id) : value From 23d6c0dc3abe8577eb529c45edf02f8400fd35ca Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 3 May 2024 16:35:20 +0200 Subject: [PATCH 7/7] Fix tests --- .../src/utilities/rowProcessor/index.ts | 3 +- .../tests/inputProcessing.spec.ts | 66 +++++++++++++++++-- .../tests/outputProcessing.spec.ts | 60 +++++++++++++++-- 3 files changed, 119 insertions(+), 10 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 47aa777852..1badc438b0 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -14,6 +14,7 @@ import { cloneDeep } from "lodash/fp" import { processInputBBReference, processInputBBReferences, + processOutputBBReference, processOutputBBReferences, } from "./bbReferenceProcessor" import { isExternalTableID } from "../../integrations/utils" @@ -258,7 +259,7 @@ export async function outputProcessing( column.type == FieldType.BB_REFERENCE_SINGLE ) { for (let row of enriched) { - row[property] = await processOutputBBReferences( + row[property] = await processOutputBBReference( row[property], column.subtype ) diff --git a/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts b/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts index 223340497e..b1928b696b 100644 --- a/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts @@ -10,7 +10,9 @@ import { import * as bbReferenceProcessor from "../bbReferenceProcessor" jest.mock("../bbReferenceProcessor", (): typeof bbReferenceProcessor => ({ + processInputBBReference: jest.fn(), processInputBBReferences: jest.fn(), + processOutputBBReference: jest.fn(), processOutputBBReferences: jest.fn(), })) @@ -19,7 +21,64 @@ describe("rowProcessor - inputProcessing", () => { jest.resetAllMocks() }) - it("processes BB references if on the schema and it's populated", async () => { + const processInputBBReferenceMock = + bbReferenceProcessor.processInputBBReference as jest.Mock + const processInputBBReferencesMock = + bbReferenceProcessor.processInputBBReferences as jest.Mock + + it("processes single BB references if on the schema and it's populated", async () => { + const userId = generator.guid() + + const table: Table = { + _id: generator.guid(), + name: "TestTable", + type: "table", + sourceId: INTERNAL_TABLE_SOURCE_ID, + sourceType: TableSourceType.INTERNAL, + schema: { + name: { + type: FieldType.STRING, + name: "name", + constraints: { + presence: true, + type: "string", + }, + }, + user: { + type: FieldType.BB_REFERENCE_SINGLE, + subtype: BBReferenceFieldSubType.USER, + name: "user", + constraints: { + presence: true, + type: "string", + }, + }, + }, + } + + const newRow = { + name: "Jack", + user: "123", + } + + const user = structures.users.user() + + processInputBBReferenceMock.mockResolvedValue(user) + + const { row } = await inputProcessing(userId, table, newRow) + + expect(bbReferenceProcessor.processInputBBReference).toHaveBeenCalledTimes( + 1 + ) + expect(bbReferenceProcessor.processInputBBReference).toHaveBeenCalledWith( + "123", + "user" + ) + + expect(row).toEqual({ ...newRow, user }) + }) + + it("processes multiple BB references if on the schema and it's populated", async () => { const userId = generator.guid() const table: Table = { @@ -56,9 +115,7 @@ describe("rowProcessor - inputProcessing", () => { const user = structures.users.user() - ;( - bbReferenceProcessor.processInputBBReferences as jest.Mock - ).mockResolvedValue(user) + processInputBBReferencesMock.mockResolvedValue(user) const { row } = await inputProcessing(userId, table, newRow) @@ -67,7 +124,6 @@ describe("rowProcessor - inputProcessing", () => { ) expect(bbReferenceProcessor.processInputBBReferences).toHaveBeenCalledWith( "123", - "bb_reference", "user" ) diff --git a/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts b/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts index 526460b350..8a19b6349b 100644 --- a/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts @@ -11,7 +11,9 @@ import { generator, structures } from "@budibase/backend-core/tests" import * as bbReferenceProcessor from "../bbReferenceProcessor" jest.mock("../bbReferenceProcessor", (): typeof bbReferenceProcessor => ({ + processInputBBReference: jest.fn(), processInputBBReferences: jest.fn(), + processOutputBBReference: jest.fn(), processOutputBBReferences: jest.fn(), })) @@ -20,10 +22,12 @@ describe("rowProcessor - outputProcessing", () => { jest.resetAllMocks() }) + const processOutputBBReferenceMock = + bbReferenceProcessor.processOutputBBReference as jest.Mock const processOutputBBReferencesMock = bbReferenceProcessor.processOutputBBReferences as jest.Mock - it("fetches bb user references given a populated field", async () => { + it("fetches single user references given a populated field", async () => { const table: Table = { _id: generator.guid(), name: "TestTable", @@ -40,7 +44,7 @@ describe("rowProcessor - outputProcessing", () => { }, }, user: { - type: FieldType.BB_REFERENCE, + type: FieldType.BB_REFERENCE_SINGLE, subtype: BBReferenceFieldSubType.USER, name: "user", constraints: { @@ -57,18 +61,66 @@ describe("rowProcessor - outputProcessing", () => { } const user = structures.users.user() - processOutputBBReferencesMock.mockResolvedValue(user) + processOutputBBReferenceMock.mockResolvedValue(user) const result = await outputProcessing(table, row, { squash: false }) expect(result).toEqual({ name: "Jack", user }) + expect(bbReferenceProcessor.processOutputBBReference).toHaveBeenCalledTimes( + 1 + ) + expect(bbReferenceProcessor.processOutputBBReference).toHaveBeenCalledWith( + "123", + BBReferenceFieldSubType.USER + ) + }) + + it("fetches users references given a populated field", async () => { + const table: Table = { + _id: generator.guid(), + name: "TestTable", + type: "table", + sourceId: INTERNAL_TABLE_SOURCE_ID, + sourceType: TableSourceType.INTERNAL, + schema: { + name: { + type: FieldType.STRING, + name: "name", + constraints: { + presence: true, + type: "string", + }, + }, + users: { + type: FieldType.BB_REFERENCE, + subtype: BBReferenceFieldSubType.USER, + name: "users", + constraints: { + presence: false, + type: "string", + }, + }, + }, + } + + const row = { + name: "Jack", + users: "123", + } + + const users = [structures.users.user()] + processOutputBBReferencesMock.mockResolvedValue(users) + + const result = await outputProcessing(table, row, { squash: false }) + + expect(result).toEqual({ name: "Jack", users }) + expect( bbReferenceProcessor.processOutputBBReferences ).toHaveBeenCalledTimes(1) expect(bbReferenceProcessor.processOutputBBReferences).toHaveBeenCalledWith( "123", - FieldType.BB_REFERENCE, BBReferenceFieldSubType.USER ) })