From 484f603c9ef38ad7c2aa4dc9cb0fc2b7a8992ded Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Fri, 2 Jun 2023 01:59:13 +0530 Subject: [PATCH] fix: Routing Forms, Number operators were using string operands (#9182) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix operators for number * Add number operator unit tests * Fixes and remove between operators --------- Co-authored-by: Omar López --- .../routing-forms/api/responses/[formId].ts | 3 +- .../components/FormInputFields.tsx | 5 +- .../routing-forms/jsonLogicToPrisma.ts | 66 ++++- .../lib/getQueryBuilderConfig.ts | 6 + .../routing-forms/lib/processRoute.tsx | 4 +- .../routing-forms/lib/transformResponse.ts | 12 + .../pages/router/[...appPages].tsx | 10 +- .../pages/routing-link/[...appPages].tsx | 4 +- .../test/lib/jsonLogicToPrisma.test.ts | 272 +++++++++++++----- .../routing-forms/trpc/report.handler.ts | 12 +- .../routing-forms/trpc/response.schema.ts | 2 +- .../app-store/routing-forms/types/types.d.ts | 2 +- packages/types/AppGetServerSideProps.d.ts | 6 +- 13 files changed, 314 insertions(+), 90 deletions(-) create mode 100644 packages/app-store/routing-forms/lib/transformResponse.ts diff --git a/packages/app-store/routing-forms/api/responses/[formId].ts b/packages/app-store/routing-forms/api/responses/[formId].ts index 0480804d49..040623ac56 100644 --- a/packages/app-store/routing-forms/api/responses/[formId].ts +++ b/packages/app-store/routing-forms/api/responses/[formId].ts @@ -39,7 +39,8 @@ async function* getResponses( if (value instanceof Array) { serializedValue = value.map((val) => escapeCsvText(val)).join(" | "); } else { - serializedValue = escapeCsvText(value); + // value can be a number as well for type Number field + serializedValue = escapeCsvText(String(value)); } csvCells.push(serializedValue); }); diff --git a/packages/app-store/routing-forms/components/FormInputFields.tsx b/packages/app-store/routing-forms/components/FormInputFields.tsx index 12d00188b7..4a15f6550f 100644 --- a/packages/app-store/routing-forms/components/FormInputFields.tsx +++ b/packages/app-store/routing-forms/components/FormInputFields.tsx @@ -4,6 +4,7 @@ import type { Dispatch, SetStateAction } from "react"; import getFieldIdentifier from "../lib/getFieldIdentifier"; import { getQueryBuilderConfig } from "../lib/getQueryBuilderConfig"; import isRouterLinkedField from "../lib/isRouterLinkedField"; +import transformResponse from "../lib/transformResponse"; import type { SerializableForm, Response } from "../types/types"; type Props = { @@ -54,14 +55,14 @@ export default function FormInputFields(props: Props) { required={!!field.required} listValues={options} data-testid={`form-field-${getFieldIdentifier(field)}`} - setValue={(value) => { + setValue={(value: number | string | string[]) => { setResponse((response) => { response = response || {}; return { ...response, [field.id]: { label: field.label, - value, + value: transformResponse({ field, value }), }, }; }); diff --git a/packages/app-store/routing-forms/jsonLogicToPrisma.ts b/packages/app-store/routing-forms/jsonLogicToPrisma.ts index b170cf45c0..2d2b966b6e 100644 --- a/packages/app-store/routing-forms/jsonLogicToPrisma.ts +++ b/packages/app-store/routing-forms/jsonLogicToPrisma.ts @@ -43,6 +43,22 @@ const OPERATOR_MAP = { operator: "NOT.equals", secondaryOperand: "", }, + ">": { + operator: "gt", + secondaryOperand: null, + }, + ">=": { + operator: "gte", + secondaryOperand: null, + }, + "<": { + operator: "lt", + secondaryOperand: null, + }, + "<=": { + operator: "lte", + secondaryOperand: null, + }, all: { operator: "array_contains", secondaryOperand: null, @@ -58,6 +74,7 @@ const GROUP_OPERATOR_MAP = { "!": "NOT", } as const; +const NumberOperators = [">", ">=", "<", "<="]; const convertSingleQueryToPrismaWhereClause = ( operatorName: keyof typeof OPERATOR_MAP, logicData: LogicData, @@ -71,13 +88,56 @@ const convertSingleQueryToPrismaWhereClause = ( logicData[operatorName] instanceof Array ? logicData[operatorName] : [logicData[operatorName]]; const mainOperand = operatorName !== "in" ? operands[0].var : operands[1].var; + let secondaryOperand = staticSecondaryOperand || (operatorName !== "in" ? operands[1] : operands[0]) || ""; if (operatorName === "all") { secondaryOperand = secondaryOperand.in[1]; } - const prismaWhere = { - response: { path: [mainOperand, "value"], [`${prismaOperator}`]: secondaryOperand }, - }; + + const isNumberOperator = NumberOperators.includes(operatorName); + const secondaryOperandAsNumber = typeof secondaryOperand === "string" ? Number(secondaryOperand) : null; + + let prismaWhere; + if (secondaryOperandAsNumber) { + // We know that it's number operator so Prisma should query number + // Note that if we get string values in DB(e.g. '100'), those values can't be filtered with number operators. + if (isNumberOperator) { + prismaWhere = { + response: { + path: [mainOperand, "value"], + [`${prismaOperator}`]: secondaryOperandAsNumber, + }, + }; + } else { + // We know that it's not number operator but the input field might have been a number and thus stored value in DB as number. + // Also, even for input type=number we might accidentally get string value(e.g. '100'). So, let reporting do it's best job with both number and string. + prismaWhere = { + OR: [ + { + response: { + path: [mainOperand, "value"], + // Query as string e.g. equals '100' + [`${prismaOperator}`]: secondaryOperand, + }, + }, + { + response: { + path: [mainOperand, "value"], + // Query as number e.g. equals 100 + [`${prismaOperator}`]: secondaryOperandAsNumber, + }, + }, + ], + }; + } + } else { + prismaWhere = { + response: { + path: [mainOperand, "value"], + [`${prismaOperator}`]: secondaryOperand, + }, + }; + } if (isNegation) { return { diff --git a/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts b/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts index 46af4cd1c9..89b41a4325 100644 --- a/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts +++ b/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts @@ -50,8 +50,14 @@ export function getQueryBuilderConfig(form: RoutingForm, forReporting = false) { const initialConfigCopy = { ...InitialConfig, operators: { ...InitialConfig.operators } }; if (forReporting) { + // Empty and Not empty doesn't work well with JSON querying in prisma. Try to implement these when we desperately need these operators. delete initialConfigCopy.operators.is_empty; delete initialConfigCopy.operators.is_not_empty; + + // Between and Not between aren't directly supported by prisma. So, we need to update jsonLogicToPrisma to generate gte and lte query for between. It can be implemented later. + delete initialConfigCopy.operators.between; + delete initialConfigCopy.operators.not_between; + initialConfigCopy.operators.__calReporting = true; } // You need to provide your own config. See below 'Config format' diff --git a/packages/app-store/routing-forms/lib/processRoute.tsx b/packages/app-store/routing-forms/lib/processRoute.tsx index 6565da7af1..d1c20a12d4 100644 --- a/packages/app-store/routing-forms/lib/processRoute.tsx +++ b/packages/app-store/routing-forms/lib/processRoute.tsx @@ -50,13 +50,13 @@ export function processRoute({ const jsonLogicQuery = QbUtils.jsonLogicFormat(state.tree, state.config); const logic = jsonLogicQuery.logic; let result = false; - const responseValues: Record = {}; + const responseValues: Record = {}; for (const [uuid, { value }] of Object.entries(response)) { responseValues[uuid] = value; } if (logic) { - // Leave the logs for easy debugging of routing form logic test. + // Leave the logs for debugging of routing form logic test in production console.log("Checking logic with response", logic, responseValues); // eslint-disable-next-line @typescript-eslint/no-explicit-any result = jsonLogic.apply(logic as any, responseValues); diff --git a/packages/app-store/routing-forms/lib/transformResponse.ts b/packages/app-store/routing-forms/lib/transformResponse.ts new file mode 100644 index 0000000000..e87ad5b12c --- /dev/null +++ b/packages/app-store/routing-forms/lib/transformResponse.ts @@ -0,0 +1,12 @@ +import type { Field, Response } from "../types/types"; + +export default function transformResponse({ + field, + value, +}: { + field: Field; + value: Response[string]["value"]; +}) { + // type="number" still gives value as a string but we need to store that as number so that number operators can work. + return field.type === "number" && typeof value === "string" ? Number(value) : value; +} diff --git a/packages/app-store/routing-forms/pages/router/[...appPages].tsx b/packages/app-store/routing-forms/pages/router/[...appPages].tsx index b9cc2c5516..306c246d37 100644 --- a/packages/app-store/routing-forms/pages/router/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/router/[...appPages].tsx @@ -7,6 +7,7 @@ import type { inferSSRProps } from "@calcom/types/inferSSRProps"; import getFieldIdentifier from "../../lib/getFieldIdentifier"; import { getSerializableForm } from "../../lib/getSerializableForm"; import { processRoute } from "../../lib/processRoute"; +import transformResponse from "../../lib/transformResponse"; import type { Response } from "../../types/types"; export default function Router({ form, message }: inferSSRProps) { @@ -60,12 +61,11 @@ export const getServerSideProps = async function getServerSideProps( const response: Record> = {}; serializableForm.fields?.forEach((field) => { - const rawFieldResponse = fieldsResponses[getFieldIdentifier(field)] || ""; - console.log(field); - const fieldResponse = - field.type === "multiselect" ? rawFieldResponse.split(",").map((r) => r.trim()) : rawFieldResponse; + const fieldResponse = fieldsResponses[getFieldIdentifier(field)] || ""; + const value = + field.type === "multiselect" ? fieldResponse.split(",").map((r) => r.trim()) : fieldResponse; response[field.id] = { - value: fieldResponse, + value: transformResponse({ field, value }), }; }); diff --git a/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx b/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx index dc082b7c3e..c63a6f61b4 100644 --- a/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx @@ -187,8 +187,10 @@ function getUrlSearchParamsToForward(response: Response, fields: NonNullable { vi.resetAllMocks(); }); -describe("jsonLogicToPrisma - Single Query", () => { - it("should support Short 'Equals' operator", () => { - const prismaWhere = jsonLogicToPrisma({ - logic: { and: [{ "==": [{ var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }, "A"] }] }, - }); - expect(prismaWhere).toEqual({ - AND: [ - { - response: { - path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], - equals: "A", - }, - }, - ], - }); - }); - - it("should support Short 'Not Equals' operator", () => { - const prismaWhere = jsonLogicToPrisma({ - logic: { and: [{ "!=": [{ var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }, "abc"] }] }, - }); - expect(prismaWhere).toEqual({ - AND: [ - { - NOT: { +describe("jsonLogicToPrisma(Reporting)", () => { + describe("Text Operand", () => { + it("should support 'Equals' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { and: [{ "==": [{ var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }, "A"] }] }, + }); + expect(prismaWhere).toEqual({ + AND: [ + { response: { path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], - equals: "abc", + equals: "A", }, }, - }, - ], + ], + }); }); - }); - it("should support Short 'Contains' operator", () => { - const prismaWhere = jsonLogicToPrisma({ - logic: { and: [{ in: ["A", { var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }] }] }, - }); - expect(prismaWhere).toEqual({ - AND: [ - { - response: { - path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], - string_contains: "A", + it("should support 'Not Equals' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { and: [{ "!=": [{ var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }, "abc"] }] }, + }); + expect(prismaWhere).toEqual({ + AND: [ + { + NOT: { + response: { + path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], + equals: "abc", + }, + }, }, - }, - ], + ], + }); }); - }); - it("should support Short 'Not Contains' operator", () => { - const prismaWhere = jsonLogicToPrisma({ - logic: { and: [{ "!": { in: ["a", { var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }] } }] }, - }); - expect(prismaWhere).toEqual({ - AND: [ - { - NOT: { + it("should support 'Contains' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { and: [{ in: ["A", { var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }] }] }, + }); + expect(prismaWhere).toEqual({ + AND: [ + { response: { path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], - string_contains: "a", + string_contains: "A", }, }, - }, - ], + ], + }); + }); + + it("should support 'Not Contains' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { and: [{ "!": { in: ["a", { var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }] } }] }, + }); + expect(prismaWhere).toEqual({ + AND: [ + { + NOT: { + response: { + path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], + string_contains: "a", + }, + }, + }, + ], + }); + }); + + describe("Number Type", () => { + it("should support 'greater than' operator", () => { + let prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { + ">": [ + { + var: "a0d113a8-8e40-49b7-87b1-7f4ab57d226f", + }, + // Giving a string here to test that it is converted to a number + "100", + ], + }, + ], + }, + }); + + expect(prismaWhere).toEqual({ + AND: [{ response: { path: ["a0d113a8-8e40-49b7-87b1-7f4ab57d226f", "value"], gt: 100 } }], + }); + + prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { + ">": [ + { + var: "a0d113a8-8e40-49b7-87b1-7f4ab57d226f", + }, + // A number would also work + 100, + ], + }, + ], + }, + }); + + expect(prismaWhere).toEqual({ + AND: [{ response: { path: ["a0d113a8-8e40-49b7-87b1-7f4ab57d226f", "value"], gt: 100 } }], + }); + }); + it("should support 'greater than or equal to' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { + ">=": [ + { + var: "a0d113a8-8e40-49b7-87b1-7f4ab57d226f", + }, + // Giving a string here to test that it is converted to a number + "100", + ], + }, + ], + }, + }); + + expect(prismaWhere).toEqual({ + AND: [{ response: { path: ["a0d113a8-8e40-49b7-87b1-7f4ab57d226f", "value"], gte: 100 } }], + }); + }); + it("should support 'less than' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { + "<": [ + { + var: "a0d113a8-8e40-49b7-87b1-7f4ab57d226f", + }, + // Giving a string here to test that it is converted to a number + "100", + ], + }, + ], + }, + }); + + expect(prismaWhere).toEqual({ + AND: [{ response: { path: ["a0d113a8-8e40-49b7-87b1-7f4ab57d226f", "value"], lt: 100 } }], + }); + }); + it("should support 'less than or equal to' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { + "<=": [ + { + var: "a0d113a8-8e40-49b7-87b1-7f4ab57d226f", + }, + // Giving a string here to test that it is converted to a number + "100", + ], + }, + ], + }, + }); + + expect(prismaWhere).toEqual({ + AND: [{ response: { path: ["a0d113a8-8e40-49b7-87b1-7f4ab57d226f", "value"], lte: 100 } }], + }); + }); + it("'Equals' operator should query with string as well as number", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { and: [{ "==": [{ var: "505d3c3c-aa71-4220-93a9-6fd1e1087939" }, "1"] }] }, + }); + + expect(prismaWhere).toEqual({ + AND: [ + { + OR: [ + { + response: { + path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], + equals: "1", + }, + }, + { + response: { + path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"], + equals: 1, + }, + }, + ], + }, + ], + }); + }); }); }); - it("should support 'MultiSelect' 'Equals' operator", () => { - const prismaWhere = jsonLogicToPrisma({ - logic: { - and: [{ all: [{ var: "267c7817-81a5-4bef-9d5b-d0faa4cd0d71" }, { in: [{ var: "" }, ["C", "D"]] }] }], - }, - }); - expect(prismaWhere).toEqual({ - AND: [ - { response: { path: ["267c7817-81a5-4bef-9d5b-d0faa4cd0d71", "value"], array_contains: ["C", "D"] } }, - ], + describe("MultiSelect", () => { + it("should support 'Equals' operator", () => { + const prismaWhere = jsonLogicToPrisma({ + logic: { + and: [ + { all: [{ var: "267c7817-81a5-4bef-9d5b-d0faa4cd0d71" }, { in: [{ var: "" }, ["C", "D"]] }] }, + ], + }, + }); + expect(prismaWhere).toEqual({ + AND: [ + { + response: { path: ["267c7817-81a5-4bef-9d5b-d0faa4cd0d71", "value"], array_contains: ["C", "D"] }, + }, + ], + }); }); }); -}); -describe("jsonLogicToPrisma - Single Query", () => { + it("should support where All Match ['Equals', 'Equals'] operator", () => { const prismaWhere = jsonLogicToPrisma({ logic: { @@ -116,6 +256,7 @@ describe("jsonLogicToPrisma - Single Query", () => { ], }); }); + it("should support where Any Match ['Equals', 'Equals'] operator", () => { const prismaWhere = jsonLogicToPrisma({ logic: { @@ -143,6 +284,7 @@ describe("jsonLogicToPrisma - Single Query", () => { ], }); }); + it("should support where None Match ['Equals', 'Equals'] operator", () => { const prismaWhere = jsonLogicToPrisma({ logic: { diff --git a/packages/app-store/routing-forms/trpc/report.handler.ts b/packages/app-store/routing-forms/trpc/report.handler.ts index 365c20d9da..5b35e0e478 100644 --- a/packages/app-store/routing-forms/trpc/report.handler.ts +++ b/packages/app-store/routing-forms/trpc/report.handler.ts @@ -52,9 +52,9 @@ export const reportHandler = async ({ ctx: { prisma }, input }: ReportHandlerOpt }); const fields = serializedForm?.fields || []; const headers = fields.map((f) => f.label + (f.deleted ? "(Deleted)" : "")); - const responses: string[][] = []; + const responses: (string | number)[][] = []; rows.forEach((r) => { - const rowResponses: string[] = []; + const rowResponses: (string | number)[] = []; responses.push(rowResponses); fields.forEach((field) => { if (!r.response) { @@ -62,13 +62,13 @@ export const reportHandler = async ({ ctx: { prisma }, input }: ReportHandlerOpt } const response = r.response as Response; const value = response[field.id]?.value || ""; - let stringValue = ""; + let transformedValue; if (value instanceof Array) { - stringValue = value.join(", "); + transformedValue = value.join(", "); } else { - stringValue = value; + transformedValue = value; } - rowResponses.push(stringValue); + rowResponses.push(transformedValue); }); }); const areThereNoResultsOrLessThanAskedFor = !rows.length || rows.length < take; diff --git a/packages/app-store/routing-forms/trpc/response.schema.ts b/packages/app-store/routing-forms/trpc/response.schema.ts index 9020de630a..b0f4a5a8d0 100644 --- a/packages/app-store/routing-forms/trpc/response.schema.ts +++ b/packages/app-store/routing-forms/trpc/response.schema.ts @@ -6,7 +6,7 @@ export const ZResponseInputSchema = z.object({ response: z.record( z.object({ label: z.string(), - value: z.union([z.string(), z.array(z.string())]), + value: z.union([z.string(), z.number(), z.array(z.string())]), }) ), }); diff --git a/packages/app-store/routing-forms/types/types.d.ts b/packages/app-store/routing-forms/types/types.d.ts index 30ace46329..d093ac88fa 100644 --- a/packages/app-store/routing-forms/types/types.d.ts +++ b/packages/app-store/routing-forms/types/types.d.ts @@ -14,7 +14,7 @@ export type Response = Record< // Field ID string, { - value: string | string[]; + value: number | string | string[]; label: string; } >; diff --git a/packages/types/AppGetServerSideProps.d.ts b/packages/types/AppGetServerSideProps.d.ts index 0a31b856d0..3ad1b2eea6 100644 --- a/packages/types/AppGetServerSideProps.d.ts +++ b/packages/types/AppGetServerSideProps.d.ts @@ -1,7 +1,7 @@ -import { GetServerSidePropsContext, GetServerSidePropsResult } from "next"; -import { CalendsoSessionUser } from "next-auth"; +import type { GetServerSidePropsContext, GetServerSidePropsResult } from "next"; +import type { CalendsoSessionUser } from "next-auth"; -import prisma from "@calcom/prisma"; +import type prisma from "@calcom/prisma"; import type { ssrInit } from "@server/lib/ssr";