fix: Routing Forms, Number operators were using string operands (#9182)

* Fix operators for number

* Add number operator unit tests

* Fixes and remove between operators

---------

Co-authored-by: Omar López <zomars@me.com>
This commit is contained in:
Hariom Balhara
2023-06-01 20:29:13 +00:00
committed by GitHub
co-authored by Omar López
parent 94b16fb327
commit 484f603c9e
13 changed files with 314 additions and 90 deletions
@@ -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);
});
@@ -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 }),
},
};
});
@@ -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 {
@@ -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'
@@ -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<string, string | string[]> = {};
const responseValues: Record<string, Response[string]["value"]> = {};
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);
@@ -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;
}
@@ -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<typeof getServerSideProps>) {
@@ -60,12 +61,11 @@ export const getServerSideProps = async function getServerSideProps(
const response: Record<string, Pick<Response[string], "value">> = {};
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 }),
};
});
@@ -187,8 +187,10 @@ function getUrlSearchParamsToForward(response: Response, fields: NonNullable<Pro
// If for some reason, the field isn't there, let's just
return;
}
const valueAsStringOrStringArray =
typeof fieldResponse.value === "number" ? String(fieldResponse.value) : fieldResponse.value;
paramsFromResponse[getFieldIdentifier(foundField) as keyof typeof paramsFromResponse] =
fieldResponse.value;
valueAsStringOrStringArray;
});
// Build query params from current URL. It excludes route params
@@ -6,89 +6,229 @@ afterEach(() => {
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: {
@@ -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;
@@ -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())]),
})
),
});
+1 -1
View File
@@ -14,7 +14,7 @@ export type Response = Record<
// Field ID
string,
{
value: string | string[];
value: number | string | string[];
label: string;
}
>;
+3 -3
View File
@@ -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";