From cf76cf7aaeb2d1513ac0f40a65ed73bc99b21d9d Mon Sep 17 00:00:00 2001 From: Dan <57096365+danrfletcher@users.noreply.github.com> Date: Thu, 22 Aug 2024 14:38:46 +0100 Subject: [PATCH] feat: Multiple Options Paste and Routing Form improvements related to Select fields (#15706) * Add handlePaste function and integrate it with options fields * Prevents function running if user does not paste a list, allows list items to be pasted without deleting following items * Subs useState for rhf useWatch to fix issues with multiple form fields * Sets options array as optional property in zod schema * Assigns default value to options array to fix possibly undefined type errors * Sets placedholder property as optional * maps selectText to options on mount * Removes comma & semicolon delimiting for pasted lists (new line only) & defines regex as a constant * Sets default value of placeholder to empty string, provides default fallback for component text field if placeholder is omitted. * Fix type error on placeholder * Extracts handlePaste to service function & tests thoroughly * Some improvements and tests arent needed * More fixes * Fix reporting as well * TS fixes * Do transforms to and fro from label * Add unit tests * Unit test reporting * Test Improvements * Self review feedback addressed --------- Co-authored-by: Hariom Balhara Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com> --- apps/web/playwright/fixtures/routingForms.ts | 4 + apps/web/playwright/fixtures/users.ts | 49 +++- apps/web/playwright/webhook.e2e.ts | 37 ++- .../routing-forms/api/responses/[formId].ts | 4 +- .../components/FormInputFields.tsx | 20 +- .../routing-forms/components/SingleForm.tsx | 4 +- .../lib/getQueryBuilderConfig.ts | 10 +- .../routing-forms/lib/getSerializableForm.ts | 5 +- .../routing-forms/lib/processRoute.tsx | 6 +- .../routing-forms/lib/selectOptions.ts | 51 +++++ .../routing-forms/lib/substituteVariables.ts | 10 +- .../lib/transformResponse.test.ts | 144 ++++++++++++ .../routing-forms/lib/transformResponse.ts | 81 ++++++- .../pages/form-edit/[...appPages].tsx | 170 +++++++++----- .../pages/router/getServerSideProps.ts | 8 +- .../pages/routing-link/[...appPages].tsx | 73 ++---- .../getUrlSearchParamsToForward.test.ts | 216 ++++++++++++++++++ .../getUrlSearchParamsToForward.ts | 87 +++++++ .../playwright/tests/basic.e2e.ts | 65 +++++- .../trpc/buildResponsesForReporting.test.ts | 158 +++++++++++++ .../routing-forms/trpc/report.handler.ts | 102 +++++++-- .../routing-forms/trpc/response.handler.ts | 4 +- .../app-store/routing-forms/trpc/utils.ts | 97 ++++++-- .../app-store/routing-forms/types/types.d.ts | 6 +- packages/app-store/routing-forms/zod.ts | 14 ++ packages/features/shell/Shell.tsx | 5 +- packages/prisma/seed-app-store.ts | 19 +- 27 files changed, 1221 insertions(+), 228 deletions(-) create mode 100644 packages/app-store/routing-forms/lib/selectOptions.ts create mode 100644 packages/app-store/routing-forms/lib/transformResponse.test.ts create mode 100644 packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.test.ts create mode 100644 packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.ts create mode 100644 packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts diff --git a/apps/web/playwright/fixtures/routingForms.ts b/apps/web/playwright/fixtures/routingForms.ts index 751503e315..aff3d4bb76 100644 --- a/apps/web/playwright/fixtures/routingForms.ts +++ b/apps/web/playwright/fixtures/routingForms.ts @@ -33,6 +33,10 @@ export const createRoutingFormsFixture = () => { label: string; identifier?: string; required: boolean; + options?: { + label: string; + id: string | null; + }[]; }[]; }) { return await prisma.app_RoutingForms_Form.create({ diff --git a/apps/web/playwright/fixtures/users.ts b/apps/web/playwright/fixtures/users.ts index 4c1e0a2d44..8cde024acf 100644 --- a/apps/web/playwright/fixtures/users.ts +++ b/apps/web/playwright/fixtures/users.ts @@ -301,6 +301,10 @@ export const createUsersFixture = ( } if (scenario.seedRoutingForms) { + const option2Uuid = "d1302635-9f12-17b1-9153-c3a854649182"; + const option1Uuid = "d1292635-9f12-17b1-9153-c3a854649182"; + const multiSelectLegacyFieldUuid = "d4292635-9f12-17b1-9153-c3a854649182"; + const multiSelectFieldUuid = "d9892635-9f12-17b1-9153-c3a854649182"; await prisma.app_RoutingForms_Form.create({ data: { routes: [ @@ -366,6 +370,26 @@ export const createUsersFixture = ( }, { id: "aa8ba8b9-0123-4456-b89a-b182623406d8", + action: { type: "customPageMessage", value: "Multiselect(Legacy) chosen" }, + queryValue: { + id: "aa8ba8b9-0123-4456-b89a-b182623406d8", + type: "group", + children1: { + "b98a8abb-cdef-4012-b456-718262343d27": { + type: "rule", + properties: { + field: multiSelectLegacyFieldUuid, + value: [["Option-2"]], + operator: "multiselect_equals", + valueSrc: ["value"], + valueType: ["multiselect"], + }, + }, + }, + }, + }, + { + id: "bb9ea8b9-0123-4456-b89a-b182623406d8", action: { type: "customPageMessage", value: "Multiselect chosen" }, queryValue: { id: "aa8ba8b9-0123-4456-b89a-b182623406d8", @@ -374,8 +398,8 @@ export const createUsersFixture = ( "b98a8abb-cdef-4012-b456-718262343d27": { type: "rule", properties: { - field: "d4292635-9f12-17b1-9153-c3a854649182", - value: [["Option-2"]], + field: multiSelectFieldUuid, + value: [[option2Uuid]], operator: "multiselect_equals", valueSrc: ["value"], valueType: ["multiselect"], @@ -399,13 +423,30 @@ export const createUsersFixture = ( required: true, }, { - id: "d4292635-9f12-17b1-9153-c3a854649182", + id: multiSelectLegacyFieldUuid, type: "multiselect", - label: "Multi Select", + label: "Multi Select(with Legacy `selectText`)", identifier: "multi", selectText: "Option-1\nOption-2", required: false, }, + { + id: multiSelectFieldUuid, + type: "multiselect", + label: "Multi Select", + identifier: "multi-new-format", + options: [ + { + id: option1Uuid, + label: "Option-1", + }, + { + id: option2Uuid, + label: "Option-2", + }, + ], + required: false, + }, ], user: { connect: { diff --git a/apps/web/playwright/webhook.e2e.ts b/apps/web/playwright/webhook.e2e.ts index 11dad203e9..ed758e8338 100644 --- a/apps/web/playwright/webhook.e2e.ts +++ b/apps/web/playwright/webhook.e2e.ts @@ -647,6 +647,7 @@ test.describe("FORM_SUBMITTED", async () => { responses: { name: { value: "John Doe", + response: "John Doe", }, }, }, @@ -673,14 +674,33 @@ test.describe("FORM_SUBMITTED", async () => { identifier: "name", required: true, }, + { + type: "multiselect", + label: "Multi Select", + identifier: "multi", + required: true, + options: [ + { + label: "Option-1", + id: "1", + }, + { + label: "Option-2", + id: "2", + }, + ], + }, ], }); await page.waitForLoadState("networkidle"); await gotoRoutingLink({ page, formId: form.id }); - const fieldName = "name"; - await page.fill(`[data-testid="form-field-${fieldName}"]`, "John Doe"); + const textFieldIdentifier = "name"; + const multiSelectFieldIdentifier = "multi"; + await page.fill(`[data-testid="form-field-${textFieldIdentifier}"]`, "John Doe"); + await page.click(`[data-testid="form-field-${multiSelectFieldIdentifier}"]`); // Open dropdown + await page.click("text=Option-2"); // Select option page.click('button[type="submit"]'); await webhookReceiver.waitForRequestCount(1); @@ -699,10 +719,23 @@ test.describe("FORM_SUBMITTED", async () => { responses: { name: { value: "John Doe", + response: "John Doe", + }, + multi: { + value: ["Option-2"], + response: [ + { + id: "2", + label: "Option-2", + }, + ], }, }, }, + /* Legacy fields Start */ name: "John Doe", + multi: ["Option-2"], + /* Legacy fields End */ }); webhookReceiver.close(); diff --git a/packages/app-store/routing-forms/api/responses/[formId].ts b/packages/app-store/routing-forms/api/responses/[formId].ts index 19e2a8fa5e..66e4e17653 100644 --- a/packages/app-store/routing-forms/api/responses/[formId].ts +++ b/packages/app-store/routing-forms/api/responses/[formId].ts @@ -6,7 +6,7 @@ import { entityPrismaWhereClause, canEditEntity } from "@calcom/lib/entityPermis import prisma from "@calcom/prisma"; import { getSerializableForm } from "../../lib/getSerializableForm"; -import type { Response, SerializableForm } from "../../types/types"; +import type { FormResponse, SerializableForm } from "../../types/types"; function escapeCsvText(str: string) { return str.replace(/,/, "%2C"); @@ -32,7 +32,7 @@ async function* getResponses( ) { const csv: string[] = []; responses.forEach((response) => { - const fieldResponses = response.response as Response; + const fieldResponses = response.response as FormResponse; const csvCells: string[] = []; headerFields.forEach((headerField) => { const fieldResponse = fieldResponses[headerField.id]; diff --git a/packages/app-store/routing-forms/components/FormInputFields.tsx b/packages/app-store/routing-forms/components/FormInputFields.tsx index ad87020c33..552a52438e 100644 --- a/packages/app-store/routing-forms/components/FormInputFields.tsx +++ b/packages/app-store/routing-forms/components/FormInputFields.tsx @@ -4,13 +4,14 @@ 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"; +import { getUIOptionsForSelect } from "../lib/selectOptions"; +import { getFieldResponseForJsonLogic } from "../lib/transformResponse"; +import type { SerializableForm, FormResponse } from "../types/types"; type Props = { form: SerializableForm; - response: Response; - setResponse: Dispatch>; + response: FormResponse; + setResponse: Dispatch>; }; export default function FormInputFields(props: Props) { @@ -31,14 +32,7 @@ export default function FormInputFields(props: Props) { } const Component = widget.factory; - const optionValues = field.selectText?.trim().split("\n"); - const options = optionValues?.map((value) => { - const title = value; - return { - value, - title, - }; - }); + const options = getUIOptionsForSelect(field); return (
@@ -62,7 +56,7 @@ export default function FormInputFields(props: Props) { ...response, [field.id]: { label: field.label, - value: transformResponse({ field, value }), + value: getFieldResponseForJsonLogic({ field, value }), }, }; }); diff --git a/packages/app-store/routing-forms/components/SingleForm.tsx b/packages/app-store/routing-forms/components/SingleForm.tsx index 6a7a884ebb..8dca9b44a1 100644 --- a/packages/app-store/routing-forms/components/SingleForm.tsx +++ b/packages/app-store/routing-forms/components/SingleForm.tsx @@ -36,7 +36,7 @@ import { getAbsoluteEventTypeRedirectUrl } from "../getEventTypeRedirectUrl"; import { RoutingPages } from "../lib/RoutingPages"; import { isFallbackRoute } from "../lib/isFallbackRoute"; import { processRoute } from "../lib/processRoute"; -import type { Response, Route, SerializableForm } from "../types/types"; +import type { FormResponse, Route, SerializableForm } from "../types/types"; import { FormAction, FormActionsDropdown, FormActionsProvider } from "./FormActions"; import FormInputFields from "./FormInputFields"; import RoutingNavBar from "./RoutingNavBar"; @@ -238,7 +238,7 @@ function SingleForm({ form, appUrl, Page, enrichedWithUserProfileForm }: SingleF const { t } = useLocale(); const [isTestPreviewOpen, setIsTestPreviewOpen] = useState(false); - const [response, setResponse] = useState({}); + const [response, setResponse] = useState({}); const [decidedAction, setDecidedAction] = useState(null); const [skipFirstUpdate, setSkipFirstUpdate] = useState(true); const [eventTypeUrl, setEventTypeUrl] = useState(""); diff --git a/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts b/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts index 055e43df85..bcfeaac19d 100644 --- a/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts +++ b/packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts @@ -1,6 +1,7 @@ import { FieldTypes } from "../pages/form-edit/[...appPages]"; import type { QueryBuilderUpdatedConfig, RoutingForm } from "../types/types"; import { InitialConfig } from "./InitialConfig"; +import { getUIOptionsForSelect } from "./selectOptions"; export function getQueryBuilderConfig(form: RoutingForm, forReporting = false) { const fields: Record< @@ -22,14 +23,7 @@ export function getQueryBuilderConfig(form: RoutingForm, forReporting = false) { field = field.routerField; } if (FieldTypes.map((f) => f.value).includes(field.type)) { - const optionValues = field.selectText?.trim().split("\n"); - const options = optionValues?.map((value) => { - const title = value; - return { - value, - title, - }; - }); + const options = getUIOptionsForSelect(field); const widget = InitialConfig.widgets[field.type]; const widgetType = widget.type; diff --git a/packages/app-store/routing-forms/lib/getSerializableForm.ts b/packages/app-store/routing-forms/lib/getSerializableForm.ts index f13531a2e8..7679dd8678 100644 --- a/packages/app-store/routing-forms/lib/getSerializableForm.ts +++ b/packages/app-store/routing-forms/lib/getSerializableForm.ts @@ -10,6 +10,7 @@ import { zodFields, zodRoutes } from "../zod"; import getConnectedForms from "./getConnectedForms"; import isRouter from "./isRouter"; import isRouterLinkedField from "./isRouterLinkedField"; +import { getFieldWithOptions } from "./selectOptions"; /** * Doesn't have deleted fields by default @@ -57,7 +58,9 @@ export async function getSerializableForm({ name: f.name, description: f.description, })); - const finalFields = fields; + + const finalFields = fields.map((field) => getFieldWithOptions(field)); + let teamMembers: SerializableFormTeamMembers[] = []; if (form.teamId) { teamMembers = await prisma.user.findMany({ diff --git a/packages/app-store/routing-forms/lib/processRoute.tsx b/packages/app-store/routing-forms/lib/processRoute.tsx index 662bc81480..4ea8fb4e85 100644 --- a/packages/app-store/routing-forms/lib/processRoute.tsx +++ b/packages/app-store/routing-forms/lib/processRoute.tsx @@ -4,7 +4,7 @@ import type { App_RoutingForms_Form } from "@prisma/client"; import { Utils as QbUtils } from "react-awesome-query-builder"; import type { z } from "zod"; -import type { Response, Route, SerializableForm } from "../types/types"; +import type { FormResponse, Route, SerializableForm } from "../types/types"; import type { zodNonRouterRoute } from "../zod"; import { getQueryBuilderConfig } from "./getQueryBuilderConfig"; import { isFallbackRoute } from "./isFallbackRoute"; @@ -16,7 +16,7 @@ export function processRoute({ response, }: { form: SerializableForm; - response: Record>; + response: Record>; }) { const queryBuilderConfig = getQueryBuilderConfig(form); @@ -52,7 +52,7 @@ 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; } diff --git a/packages/app-store/routing-forms/lib/selectOptions.ts b/packages/app-store/routing-forms/lib/selectOptions.ts new file mode 100644 index 0000000000..39613d8da4 --- /dev/null +++ b/packages/app-store/routing-forms/lib/selectOptions.ts @@ -0,0 +1,51 @@ +/** + * @fileoverview + * + * This file holds the utilities to build the options to render in the select field and it could be loaded on client side as well. + */ +import type { z } from "zod"; + +import type { zodFieldView } from "../zod"; + +type Field = z.infer; + +const buildOptionsFromLegacySelectText = ({ legacySelectText }: { legacySelectText: string }) => { + return legacySelectText + .trim() + .split("\n") + .map((fieldValue) => ({ + label: fieldValue, + id: null, + })); +}; + +export const getFieldWithOptions = (field: T) => { + const legacySelectText = field.selectText; + if (field.options) { + return { + ...field, + options: field.options, + }; + } else if (legacySelectText) { + const options = buildOptionsFromLegacySelectText({ legacySelectText }); + return { + ...field, + options, + }; + } + return { + ...field, + }; +}; + +export function getUIOptionsForSelect(field: Field) { + return getFieldWithOptions(field).options?.map((option) => { + // We prefer option.id as that doesn't change when we change the option text/label. + // Fallback to option.label for fields saved in DB in old format which didn't have `options` + const value = option.id ?? option.label; + return { + value, + title: option.label, + }; + }); +} diff --git a/packages/app-store/routing-forms/lib/substituteVariables.ts b/packages/app-store/routing-forms/lib/substituteVariables.ts index b352df7db2..aebcae47f7 100644 --- a/packages/app-store/routing-forms/lib/substituteVariables.ts +++ b/packages/app-store/routing-forms/lib/substituteVariables.ts @@ -1,11 +1,11 @@ import slugify from "@calcom/lib/slugify"; -import type { Response, Route, Field } from "../types/types"; +import type { FormResponse, Route, Field } from "../types/types"; import getFieldIdentifier from "./getFieldIdentifier"; export const substituteVariables = ( routeValue: Route["action"]["value"], - response: Response, + response: FormResponse, fields: Field[] ) => { const regex = /\{([^\}]+)\}/g; @@ -15,7 +15,11 @@ export const substituteVariables = ( variables.forEach((variable) => { for (const key in response) { - const identifier = getFieldIdentifier(fields.find((field) => field.id === key)); + const field = fields.find((field) => field.id === key) + if (!field) { + continue; + } + const identifier = getFieldIdentifier(field); if (identifier.toLowerCase() === variable.toLowerCase()) { eventTypeUrl = eventTypeUrl.replace(`{${variable}}`, slugify(response[key].value.toString() || "")); } diff --git a/packages/app-store/routing-forms/lib/transformResponse.test.ts b/packages/app-store/routing-forms/lib/transformResponse.test.ts new file mode 100644 index 0000000000..1c7b8eca9a --- /dev/null +++ b/packages/app-store/routing-forms/lib/transformResponse.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect } from "vitest"; + +import { getFieldResponseForJsonLogic } from "./transformResponse"; + +describe("getFieldResponseForJsonLogic", () => { + it("should return an empty string if value is undefined", () => { + const field = { type: "text", options: undefined }; + const value = undefined; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toBe(""); + }); + + it("should transform value for a number type field to number", () => { + const field = { type: "number", options: undefined }; + const value = "123"; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toBe(123); + }); + + describe("multiselect", () => { + describe("non-legacy options", () => { + it("should return option ids for the field if the value is an array of option ids", () => { + const field = { + type: "multiselect", + options: [ + { id: "1", label: "Option 1" }, + { id: "2", label: "Option 2" }, + ], + }; + const value = ["1", "2"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["1", "2"]); + }); + + it("should return ids if matching labels are provided in value", () => { + const field = { + type: "multiselect", + options: [ + { id: "1", label: "Option 1" }, + { id: "2", label: "Option 2" }, + ], + }; + const value = ["Option 1", "Option 2"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["1", "2"]); + }); + + it("should return the value as it is if it doesn't match any of the labels or option ids", () => { + const field = { + type: "multiselect", + options: [ + { id: "1", label: "Option 1" }, + { id: "2", label: "Option 2" }, + ], + }; + const value = ["Option 10", "Option 11"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["Option 10", "Option 11"]); + }); + }); + + describe("legacy options", () => { + it("should return what is given for a field that has legacy options but the values don't match labels", () => { + const field = { + type: "multiselect", + options: [ + { id: null, label: "Option 1" }, + { id: null, label: "Option 2" }, + ], + }; + const value = ["1", "2"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["1", "2"]); + }); + + it("should return matching labels for a field that has legacy options", () => { + const field = { + type: "multiselect", + options: [ + { id: null, label: "Option 1" }, + { id: null, label: "Option 2" }, + ], + }; + const value = ["Option 1", "Option 2"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["Option 1", "Option 2"]); + }); + + it("should handle multiselect field with legacy options", () => { + const field = { + type: "multiselect", + options: [ + { label: "Option 1", id: null }, + { label: "Option 2", id: null }, + ], + }; + const value = ["Option 1", "Option 2"]; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toEqual(["Option 1", "Option 2"]); + }); + }); + }); + + describe("select", () => { + it("should handle select field with options", () => { + const field = { + type: "select", + options: [ + { id: "1", label: "Option 1" }, + { id: "2", label: "Option 2" }, + ], + }; + const value = "1"; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toBe("1"); + }); + + it("should handle select field with legacy options", () => { + const field = { + type: "select", + options: [ + { label: "Option 1", id: null }, + { label: "Option 2", id: null }, + ], + }; + const value = "Option 1"; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toBe("Option 1"); + }); + + it("should handle select field with number as string", () => { + const field = { + type: "select", + options: [ + { id: "1", label: "Option 1" }, + { id: "2", label: "Option 2" }, + ], + }; + const value = 1; + const result = getFieldResponseForJsonLogic({ field, value }); + expect(result).toBe("1"); + }); + }); +}); diff --git a/packages/app-store/routing-forms/lib/transformResponse.ts b/packages/app-store/routing-forms/lib/transformResponse.ts index f7e3d41152..641878cf1a 100644 --- a/packages/app-store/routing-forms/lib/transformResponse.ts +++ b/packages/app-store/routing-forms/lib/transformResponse.ts @@ -1,11 +1,59 @@ -import type { Field, Response } from "../types/types"; +import type { Field, FormResponse } from "../types/types"; -export default function transformResponse({ +/** + * It takes care of correctly transforming the input to label or id depending on various cases + * - It allows us to prefill with ID or Label + * - It allows backward compatibility with legacy routes(with labels for options) + */ +function transformSelectValue({ + field, + idOrLabel, +}: { + field: Pick; + idOrLabel: string; +}) { + idOrLabel = idOrLabel.trim(); + const options = field.options; + if (!options) { + return idOrLabel; + } + const areOptionsInLegacyFormat = !!options.find((option) => !option.id); + // Because for legacy saved options, routes must have labels in them instead of ids + const shouldUseLabelAsValue = areOptionsInLegacyFormat; + const foundOptionById = options.find((option) => option.id === idOrLabel); + if (foundOptionById) { + if (shouldUseLabelAsValue) { + return foundOptionById.label; + } else { + // If shouldUseLabelAsValue is false, then we must use id as value + // Because shouldUseLabelAsValue is false, id must be set already + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return foundOptionById.id!; + } + } else { + // No option was found that matches ID + // So check if the label is provided + const foundOptionByLabel = options.find((option) => { + return option.label === idOrLabel; + }); + if (foundOptionByLabel) { + if (!shouldUseLabelAsValue) { + // If shouldUseLabelAsValue is false, then we must use id as value + // Because shouldUseLabelAsValue is false, id must be set already + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return foundOptionByLabel.id!; + } + } + } + return idOrLabel; +} + +export function getFieldResponseForJsonLogic({ field, value, }: { - field: Field; - value: Response[string]["value"] | undefined; + field: Pick; + value: FormResponse[string]["value"] | undefined; }) { if (!value) { return ""; @@ -18,13 +66,24 @@ export default function transformResponse({ return value; } if (field.type === "multiselect") { - if (value instanceof Array) { - return value; - } - return value - .toString() - .split(",") - .map((v) => v.trim()); + // Could be option id(i.e. a UUIDv4) or option label for ease of prefilling + let valueOrLabelArray = value instanceof Array ? value : value.toString().split(","); + + valueOrLabelArray = valueOrLabelArray.map((idOrLabel) => { + return transformSelectValue({ field, idOrLabel }); + }); + + return valueOrLabelArray; + } + + if (field.type === "select") { + const valueAsStringOrStringArray = typeof value === "number" ? String(value) : value; + const valueAsString = + valueAsStringOrStringArray instanceof Array + ? valueAsStringOrStringArray[0] + : valueAsStringOrStringArray; + + return transformSelectValue({ field, idOrLabel: valueAsString }); } return value; } diff --git a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx index c54b3db6e4..facba432d5 100644 --- a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx @@ -1,7 +1,7 @@ "use client"; import { useAutoAnimate } from "@formkit/auto-animate/react"; -import { useEffect, useState } from "react"; +import type { ClipboardEvent } from "react"; import type { UseFormReturn } from "react-hook-form"; import { Controller, useFieldArray, useWatch } from "react-hook-form"; import { v4 as uuidv4 } from "uuid"; @@ -29,8 +29,26 @@ import SingleForm, { } from "../../components/SingleForm"; export { getServerSideProps }; +type SelectOption = { label: string; id: string | null }; type HookForm = UseFormReturn; -type SelectOption = { placeholder: string; value: string; id: string }; + +const appendArray = ({ + target, + arrayToAppend, + appendAt, +}: { + arrayToAppend: T[]; + target: T[]; + appendAt: number; +}) => { + // Avoid mutating the original array + const copyOfTarget = [...target]; + const numItemsToRemove = arrayToAppend.length; + copyOfTarget.splice(appendAt, numItemsToRemove, ...arrayToAppend); + return copyOfTarget; +}; + +const PASTE_OPTIONS_SEPARATOR_REGEX = /\n+/; export const FieldTypes = [ { @@ -90,65 +108,51 @@ function Field({ }) { const { t } = useLocale(); const [animationRef] = useAutoAnimate(); + const watchedOptions = + useWatch({ + control: hookForm.control, + name: `${hookFieldNamespace}.options`, + defaultValue: [ + { label: "", id: uuidv4() }, + { label: "", id: uuidv4() }, + { label: "", id: uuidv4() }, + { label: "", id: uuidv4() }, + ], + }) || []; - const [options, setOptions] = useState([ - { placeholder: "< 10", value: "", id: uuidv4() }, - { placeholder: "10-100", value: "", id: uuidv4() }, - { placeholder: "100-500", value: "", id: uuidv4() }, - { placeholder: "> 500", value: "", id: uuidv4() }, - ]); - - const handleRemoveOptions = (index: number) => { - const updatedOptions = options.filter((_, i) => i !== index); - setOptions(updatedOptions); - updateSelectText(updatedOptions); + const setOptions = (updatedOptions: SelectOption[]) => { + hookForm.setValue(`${hookFieldNamespace}.options`, updatedOptions); }; - const handleAddOptions = () => { - setOptions((prevState) => [ - ...prevState, + const handleRemoveOptions = (index: number) => { + const updatedOptions = watchedOptions.filter((_, i) => i !== index); + // We can't let the user remove the last option + if (updatedOptions.length === 0) { + return; + } + setOptions(updatedOptions); + }; + + const addOption = () => { + setOptions([ + ...watchedOptions, { - placeholder: "New Option", - value: "", + label: "", id: uuidv4(), }, ]); }; - useEffect(() => { - const originalValues = hookForm.getValues(`${hookFieldNamespace}.selectText`); - if (originalValues) { - const values: SelectOption[] = originalValues.split("\n").map((fieldValue) => ({ - value: fieldValue, - placeholder: "", - id: uuidv4(), - })); - setOptions(values); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - const router = hookForm.getValues(`${hookFieldNamespace}.router`); const routerField = hookForm.getValues(`${hookFieldNamespace}.routerField`); - const handleChange = (e: React.ChangeEvent, optionIndex: number) => { - const updatedOptions = options.map((opt, index) => ({ + const updateLabelAtIndex = ({ label, optionIndex }: { label: string; optionIndex: number }) => { + const updatedOptions = watchedOptions.map((opt, index) => ({ ...opt, - ...(index === optionIndex ? { value: e.target.value } : {}), + ...(index === optionIndex ? { label } : {}), })); setOptions(updatedOptions); - updateSelectText(updatedOptions); - }; - - const updateSelectText = (updatedOptions: SelectOption[]) => { - hookForm.setValue( - `${hookFieldNamespace}.selectText`, - updatedOptions - .filter((opt) => opt.value) - .map((opt) => opt.value) - .join("\n") - ); }; const label = useWatch({ @@ -162,18 +166,49 @@ function Field({ }); function move(index: number, increment: 1 | -1) { - const newList = [...options]; + const newList = [...watchedOptions]; - const type = options[index]; - const tmp = options[index + increment]; + const type = watchedOptions[index]; + const tmp = watchedOptions[index + increment]; if (tmp) { newList[index] = tmp; newList[index + increment] = type; } setOptions(newList); - updateSelectText(newList); } + const handlePasteInOptionAtIndex = ({ + event, + optionIndex, + }: { + event: ClipboardEvent; + optionIndex: number; + }) => { + const paste = event.clipboardData.getData("text"); + // The value being pasted could be a list of options + const optionsBeingPasted = paste + .split(PASTE_OPTIONS_SEPARATOR_REGEX) + .map((optionLabel) => optionLabel.trim()) + .filter((optionLabel) => optionLabel) + .map((optionLabel) => ({ label: optionLabel.trim(), id: uuidv4() })); + if (optionsBeingPasted.length === 1) { + // If there is only one option, we would just let that option be pasted + return; + } + + // Don't allow pasting that value, as we would update the options through state update + event.preventDefault(); + + const updatedOptions = appendArray({ + target: watchedOptions, + arrayToAppend: optionsBeingPasted, + appendAt: optionIndex, + }); + setOptions(updatedOptions); + }; + + const optionsPlaceholders = ["< 10", "10 - 100", "100 - 500", "> 500"]; + return (
    - {options.map((field, index) => ( -
  • + {watchedOptions.map((option, index) => ( +
  • + handlePasteInOptionAtIndex({ event, optionIndex: index }) + }>
    - {options.length && index !== 0 ? ( + {watchedOptions.length && index !== 0 ? ( ) : null} - {options.length && index !== options.length - 1 ? ( + {watchedOptions.length && index !== watchedOptions.length - 1 ? ( + watchedOptions.length > 1 ? ( + + ) : null } />
    @@ -312,7 +356,7 @@ function Field({ type="button" StartIcon="plus" color="secondary" - onClick={handleAddOptions}> + onClick={addOption}> Add an option
diff --git a/packages/app-store/routing-forms/pages/router/getServerSideProps.ts b/packages/app-store/routing-forms/pages/router/getServerSideProps.ts index a11dc45266..f8288c9bef 100644 --- a/packages/app-store/routing-forms/pages/router/getServerSideProps.ts +++ b/packages/app-store/routing-forms/pages/router/getServerSideProps.ts @@ -12,8 +12,8 @@ import getFieldIdentifier from "../../lib/getFieldIdentifier"; import { getSerializableForm } from "../../lib/getSerializableForm"; import { processRoute } from "../../lib/processRoute"; import { substituteVariables } from "../../lib/substituteVariables"; -import transformResponse from "../../lib/transformResponse"; -import type { Response } from "../../types/types"; +import { getFieldResponseForJsonLogic } from "../../lib/transformResponse"; +import type { FormResponse } from "../../types/types"; import { isAuthorizedToViewTheForm } from "../routing-link/getServerSideProps"; const log = logger.getSubLogger({ prefix: ["[routing-forms]", "[router]"] }); @@ -96,7 +96,7 @@ export const getServerSideProps = async function getServerSideProps( form: enrichFormWithMigrationData(formWithUserProfile), }); - const response: Response = {}; + const response: FormResponse = {}; if (!serializableForm.fields) { throw new Error("Form has no fields"); } @@ -105,7 +105,7 @@ export const getServerSideProps = async function getServerSideProps( response[field.id] = { label: field.label, - value: transformResponse({ field, value: fieldResponse }), + value: getFieldResponseForJsonLogic({ field, value: fieldResponse }), }; }); 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 e69ffd0d56..a607639014 100644 --- a/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx @@ -23,9 +23,10 @@ import { getAbsoluteEventTypeRedirectUrl } from "../../getEventTypeRedirectUrl"; import getFieldIdentifier from "../../lib/getFieldIdentifier"; import { processRoute } from "../../lib/processRoute"; import { substituteVariables } from "../../lib/substituteVariables"; -import transformResponse from "../../lib/transformResponse"; -import type { NonRouterRoute, Response } from "../../types/types"; +import { getFieldResponseForJsonLogic } from "../../lib/transformResponse"; +import type { NonRouterRoute, FormResponse } from "../../types/types"; import { getServerSideProps } from "./getServerSideProps"; +import { getUrlSearchParamsToForward } from "./getUrlSearchParamsToForward"; type Props = inferSSRProps; const useBrandColors = ({ @@ -59,10 +60,13 @@ function RoutingForm({ form, profile, ...restProps }: Props) { // - like a network error // - or he abandoned booking flow in between const formFillerId = formFillerIdRef.current; - const decidedActionWithFormResponseRef = useRef<{ action: NonRouterRoute["action"]; response: Response }>(); + const decidedActionWithFormResponseRef = useRef<{ + action: NonRouterRoute["action"]; + response: FormResponse; + }>(); const router = useRouter(); - const onSubmit = (response: Response) => { + const onSubmit = (response: FormResponse) => { const decidedAction = processRoute({ form, response }); if (!decidedAction) { @@ -97,7 +101,11 @@ function RoutingForm({ form, profile, ...restProps }: Props) { if (!fields) { throw new Error("Routing Form fields must exist here"); } - const allURLSearchParams = getUrlSearchParamsToForward(decidedActionWithFormResponse.response, fields); + const allURLSearchParams = getUrlSearchParamsToForward({ + formResponse: decidedActionWithFormResponse.response, + fields, + searchParams: new URLSearchParams(window.location.search), + }); const decidedAction = decidedActionWithFormResponse.action; sdkActionManager?.fire("routed", { actionType: decidedAction.type, @@ -191,55 +199,6 @@ function RoutingForm({ form, profile, ...restProps }: Props) { ); } -function getUrlSearchParamsToForward(response: Response, fields: NonNullable) { - type Params = Record; - const paramsFromResponse: Params = {}; - const paramsFromCurrentUrl: Params = {}; - - // Build query params from response - Object.entries(response).forEach(([key, fieldResponse]) => { - const foundField = fields.find((f) => f.id === key); - if (!foundField) { - // 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] = - valueAsStringOrStringArray; - }); - - // Build query params from current URL. It excludes route params - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - for (const [name, value] of new URLSearchParams(window.location.search).entries()) { - const target = paramsFromCurrentUrl[name]; - if (target instanceof Array) { - target.push(value); - } else { - paramsFromCurrentUrl[name] = [value]; - } - } - - const allQueryParams: Params = { - ...paramsFromCurrentUrl, - // In case of conflict b/w paramsFromResponse and paramsFromCurrentUrl, paramsFromResponse should win as the booker probably improved upon the prefilled value. - ...paramsFromResponse, - }; - - const allQueryURLSearchParams = new URLSearchParams(); - - // Make serializable URLSearchParams instance - Object.entries(allQueryParams).forEach(([param, value]) => { - const valueArray = value instanceof Array ? value : [value]; - valueArray.forEach((v) => { - allQueryURLSearchParams.append(param, v); - }); - }); - - return allQueryURLSearchParams; -} - export default function RoutingLink(props: inferSSRProps) { return ; } @@ -250,7 +209,7 @@ export { getServerSideProps }; const usePrefilledResponse = (form: Props["form"]) => { const searchParams = useCompatSearchParams(); - const prefillResponse: Response = {}; + const prefillResponse: FormResponse = {}; // Prefill the form from query params form.fields?.forEach((field) => { @@ -259,10 +218,10 @@ const usePrefilledResponse = (form: Props["form"]) => { const value = valuesFromQuery.length > 1 ? valuesFromQuery : valuesFromQuery[0]; prefillResponse[field.id] = { - value: transformResponse({ field, value }), + value: getFieldResponseForJsonLogic({ field, value }), label: field.label, }; }); - const [response, setResponse] = useState(prefillResponse); + const [response, setResponse] = useState(prefillResponse); return [response, setResponse] as const; }; diff --git a/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.test.ts b/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.test.ts new file mode 100644 index 0000000000..789cf959f5 --- /dev/null +++ b/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.test.ts @@ -0,0 +1,216 @@ +import { v4 as uuidv4 } from "uuid"; +import { describe, it, expect } from "vitest"; + +import { getUrlSearchParamsToForward } from "./getUrlSearchParamsToForward"; + +function fromEntriesWithDuplicateKeys(entries: IterableIterator<[string, string]> | null) { + const result: Record = {}; + + if (entries === null) { + return result; + } + + // Consider setting atleast ES2015 as target + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + for (const [key, value] of entries) { + if (result.hasOwnProperty(key)) { + let currentValue = result[key]; + if (!Array.isArray(currentValue)) { + currentValue = [currentValue]; + } + currentValue.push(value); + result[key] = currentValue; + } else { + result[key] = value; + } + } + return result; +} + +describe("getUrlSearchParamsToForward", () => { + it("should build query params from response correctly when identifier is present in fields", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const formResponse = { + [field1Id]: { value: "value1", label: "Field 1" }, + [field2Id]: { value: ["option1", "option2"], label: "Field 2" }, + }; + + const fields = [ + { id: field1Id, identifier: "f1", type: "text", label: "Field 1" }, + { + id: field2Id, + identifier: "f2", + label: "Field 2", + type: "multiselect", + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + ], + }, + ]; + + const searchParams = new URLSearchParams("?query1=value1&query2=value2"); + const expectedParams = { + f1: "value1", + f2: ["Option 1", "Option 2"], + query1: "value1", + query2: "value2", + }; + + const result = getUrlSearchParamsToForward({ + formResponse, + fields, + searchParams, + }); + expect(fromEntriesWithDuplicateKeys(result.entries())).toEqual(expectedParams); + }); + + it("should build query params from response correctly when identifier is not present in fields. Should fallback to label", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const formResponse = { + [field1Id]: { value: "value1" }, + [field2Id]: { value: "value2" }, + }; + + const fields = [ + { id: field1Id, label: "Field 1", type: "text" }, + { id: field2Id, label: "Field 2", type: "text" }, + ]; + + const searchParams = new URLSearchParams("?query1=value1&query2=value2"); + + const expectedParams = { + "Field 1": "value1", + "Field 2": "value2", + query1: "value1", + query2: "value2", + }; + + const result = getUrlSearchParamsToForward({ + formResponse, + fields, + searchParams, + }); + expect(fromEntriesWithDuplicateKeys(result.entries())).toEqual(expectedParams); + }); + + it("should handle select fields correctly when options have id set", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const formResponse = { + [field1Id]: { value: "Option 1" }, + [field2Id]: { value: ["Option 1", "Option 2"] }, + }; + + const fields = [ + { + id: field1Id, + label: "Field 1", + type: "select", + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + ], + }, + { + id: field2Id, + label: "Field 2", + type: "multiselect", + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + { id: "option3", label: "Option 3" }, + ], + }, + ]; + + const searchParams = new URLSearchParams("?query1=value1&query2=value2"); + + const expectedParams = { + "Field 1": "Option 1", + "Field 2": ["Option 1", "Option 2"], + query1: "value1", + query2: "value2", + }; + + const result = getUrlSearchParamsToForward({ + formResponse, + fields, + searchParams, + }); + expect(fromEntriesWithDuplicateKeys(result.entries())).toEqual(expectedParams); + }); + + it("should handle select fields correctly when options have no id set(Legacy options)", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const formResponse = { + [field1Id]: { value: "Option 1" }, + [field2Id]: { value: ["Option 1", "Option 2"] }, + }; + + const fields = [ + { + id: field1Id, + label: "Field 1", + type: "select", + options: [ + { id: null, label: "Option 1" }, + { id: null, label: "Option 2" }, + ], + }, + { + id: field2Id, + label: "Field 2", + type: "multiselect", + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + { id: "option3", label: "Option 3" }, + ], + }, + ]; + + const searchParams = new URLSearchParams("?query1=value1&query2=value2"); + + const expectedParams = { + "Field 1": "Option 1", + "Field 2": ["Option 1", "Option 2"], + query1: "value1", + query2: "value2", + }; + + const result = getUrlSearchParamsToForward({ + formResponse, + fields, + searchParams, + }); + expect(fromEntriesWithDuplicateKeys(result.entries())).toEqual(expectedParams); + }); + + it("should handle number values correctly", () => { + const field1Id = uuidv4(); + const response = { + [field1Id]: { value: 123 }, + }; + + const fields = [{ id: field1Id, label: "Field 1", type: "number" }]; + + const searchParams = new URLSearchParams("?query1=value1&query2=value2"); + const expectedParams = { + "Field 1": "123", + query1: "value1", + query2: "value2", + }; + + const result = getUrlSearchParamsToForward({ + formResponse: response, + fields, + searchParams, + }); + expect(fromEntriesWithDuplicateKeys(result.entries())).toEqual(expectedParams); + }); +}); diff --git a/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.ts b/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.ts new file mode 100644 index 0000000000..281a637fcc --- /dev/null +++ b/packages/app-store/routing-forms/pages/routing-link/getUrlSearchParamsToForward.ts @@ -0,0 +1,87 @@ +import type { inferSSRProps } from "@calcom/types/inferSSRProps"; + +import getFieldIdentifier from "../../lib/getFieldIdentifier"; +import type { getServerSideProps } from "./getServerSideProps"; + +type Props = inferSSRProps; +export function getUrlSearchParamsToForward({ + formResponse, + fields, + searchParams, +}: { + formResponse: Record< + string, + { + value: number | string | string[]; + } + >; + fields: Pick< + NonNullable[number], + "id" | "type" | "options" | "identifier" | "label" + >[]; + searchParams: URLSearchParams; +}) { + type Params = Record; + const paramsFromResponse: Params = {}; + const paramsFromCurrentUrl: Params = {}; + + // Build query params from response + Object.entries(formResponse).forEach(([key, fieldResponse]) => { + const foundField = fields.find((f) => f.id === key); + if (!foundField) { + // If for some reason, the field isn't there, let's just + return; + } + let valueAsStringOrStringArray = + typeof fieldResponse.value === "number" ? String(fieldResponse.value) : fieldResponse.value; + if (foundField.type === "select" || foundField.type === "multiselect") { + const options = foundField.options || []; + let arr = + valueAsStringOrStringArray instanceof Array + ? valueAsStringOrStringArray + : [valueAsStringOrStringArray]; + arr = arr.map((idOrLabel) => { + const foundOptionById = options.find((option) => { + return option.id === idOrLabel; + }); + if (foundOptionById) { + return foundOptionById.label; + } + return idOrLabel; + }); + valueAsStringOrStringArray = foundField.type === "select" ? arr[0] : arr; + } + paramsFromResponse[getFieldIdentifier(foundField) as keyof typeof paramsFromResponse] = + valueAsStringOrStringArray; + }); + + // Build query params from current URL. It excludes route params + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + for (const [name, value] of searchParams.entries()) { + const target = paramsFromCurrentUrl[name]; + if (target instanceof Array) { + target.push(value); + } else { + paramsFromCurrentUrl[name] = [value]; + } + } + + const allQueryParams: Params = { + ...paramsFromCurrentUrl, + // In case of conflict b/w paramsFromResponse and paramsFromCurrentUrl, paramsFromResponse should win as the booker probably improved upon the prefilled value. + ...paramsFromResponse, + }; + + const allQueryURLSearchParams = new URLSearchParams(); + + // Make serializable URLSearchParams instance + Object.entries(allQueryParams).forEach(([param, value]) => { + const valueArray = value instanceof Array ? value : [value]; + valueArray.forEach((v) => { + allQueryURLSearchParams.append(param, v); + }); + }); + + return allQueryURLSearchParams; +} diff --git a/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts b/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts index 2c4211f752..7cef783ec3 100644 --- a/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts +++ b/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts @@ -17,6 +17,11 @@ function todo(title: string) { test.skip(title, () => {}); } +const Identifiers = { + multi: "multi", + multiNewFormat: "multi-new-format", +}; + test.describe("Routing Forms", () => { test.describe("Zero State Routing Forms", () => { test("should be able to add a new form and view it", async ({ page }) => { @@ -254,11 +259,11 @@ test.describe("Routing Forms", () => { responses.push(rowResponses); } - expect(headers).toEqual(["Test field", "Multi Select"]); + expect(headers).toEqual(["Test field", "Multi Select(with Legacy `selectText`)", "Multi Select"]); expect(responses).toEqual([ - ["event-routing", ""], - ["external-redirect", ""], - ["custom-page", ""], + ["event-routing", "", ""], + ["external-redirect", "", ""], + ["custom-page", "", ""], ]); await page.goto(`apps/routing-forms/route-builder/${routingForm.id}`); @@ -281,19 +286,21 @@ test.describe("Routing Forms", () => { }); const csvRows = csv.trim().split("\n"); const csvHeaderRow = csvRows[0]; - expect(csvHeaderRow).toEqual("Test field,Multi Select,Submission Time"); + expect(csvHeaderRow).toEqual( + "Test field,Multi Select(with Legacy `selectText`),Multi Select,Submission Time" + ); const firstResponseCells = csvRows[1].split(","); const secondResponseCells = csvRows[2].split(","); const thirdResponseCells = csvRows[3].split(","); - expect(firstResponseCells.slice(0, -1).join(",")).toEqual("event-routing,"); + expect(firstResponseCells.slice(0, -1).join(",")).toEqual("event-routing,,"); expect(new Date(firstResponseCells.at(-1) as string).getDay()).toEqual(new Date().getDay()); - expect(secondResponseCells.slice(0, -1).join(",")).toEqual("external-redirect,"); + expect(secondResponseCells.slice(0, -1).join(",")).toEqual("external-redirect,,"); expect(new Date(secondResponseCells.at(-1) as string).getDay()).toEqual(new Date().getDay()); - expect(thirdResponseCells.slice(0, -1).join(",")).toEqual("custom-page,"); + expect(thirdResponseCells.slice(0, -1).join(",")).toEqual("custom-page,,"); expect(new Date(thirdResponseCells.at(-1) as string).getDay()).toEqual(new Date().getDay()); }); @@ -318,8 +325,13 @@ test.describe("Routing Forms", () => { await page.goto(`/router?form=${routingForm.id}&Test field=custom-page`); await expect(page.locator("text=Custom Page Result")).toBeVisible(); - await page.goto(`/router?form=${routingForm.id}&Test field=doesntmatter&multi=Option-2`); - await expect(page.locator("text=Multiselect chosen")).toBeVisible(); + await page.goto(`/router?form=${routingForm.id}&Test field=doesntmatter&${Identifiers.multi}=Option-2`); + await expect(page.locator("text=Multiselect(Legacy) chosen")).toBeVisible({ timeout: 10000 }); + + await page.goto( + `/router?form=${routingForm.id}&Test field=doesntmatter&${Identifiers.multiNewFormat}=d1302635-9f12-17b1-9153-c3a854649182` + ); + await expect(page.locator("text=Multiselect chosen")).toBeVisible({ timeout: 10000 }); }); test("Routing Link should validate fields", async ({ page, users }) => { @@ -364,8 +376,34 @@ test.describe("Routing Forms", () => { route = await page.locator('[data-testid="test-routing-result"]').innerText(); expect(routingType).toBe("External Redirect"); expect(route).toBe("https://google.com"); + await page.click('[data-testid="dialog-rejection"]'); + + // Multiselect(Legacy) + await page.click('[data-testid="test-preview"]'); + await page.fill('[data-testid="form-field-Test field"]', "doesntmatter"); + await page.click(`[data-testid="form-field-${Identifiers.multi}"]`); // Open dropdown + await page.click("text=Option-2"); // Select option + await page.click('[data-testid="test-routing"]'); + routingType = await page.locator('[data-testid="test-routing-result-type"]').innerText(); + route = await page.locator('[data-testid="test-routing-result"]').innerText(); + expect(routingType).toBe("Custom Page"); + expect(route).toBe("Multiselect(Legacy) chosen"); + await page.click('[data-testid="dialog-rejection"]'); + + // Multiselect + await page.click('[data-testid="test-preview"]'); + await page.fill('[data-testid="form-field-Test field"]', "doesntmatter"); + await page.click(`[data-testid="form-field-${Identifiers.multiNewFormat}"]`); // Open dropdown + await page.click("text=Option-2"); // Select option + await page.click('[data-testid="test-routing"]'); + routingType = await page.locator('[data-testid="test-routing-result-type"]').innerText(); + route = await page.locator('[data-testid="test-routing-result"]').innerText(); + expect(routingType).toBe("Custom Page"); + expect(route).toBe("Multiselect chosen"); + await page.click('[data-testid="dialog-rejection"]'); //fallback route + await page.click('[data-testid="test-preview"]'); await page.fill('[data-testid="form-field-Test field"]', "fallback"); await page.click('[data-testid="test-routing"]'); routingType = await page.locator('[data-testid="test-routing-result-type"]').innerText(); @@ -452,8 +490,11 @@ async function addAllTypesOfFieldsAndSaveForm( identifier = "firstField"; } - if (fieldTypeLabel === "MultiSelect" || fieldTypeLabel === "Select") { - await page.fill(`[name="fields.${nth}.selectText"]`, "123\n456\n789"); + if (fieldTypeLabel === "Multiple Selection" || fieldTypeLabel === "Single Selection") { + await page.fill(`[data-testid="fields.${nth}.options.0-input"]`, "123"); + await page.fill(`[data-testid="fields.${nth}.options.1-input"]`, "456"); + await page.fill(`[data-testid="fields.${nth}.options.2-input"]`, "789"); + await page.fill(`[data-testid="fields.${nth}.options.3-input"]`, "10-11-12"); } await page.fill(`[name="fields.${nth}.label"]`, label); diff --git a/packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts b/packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts new file mode 100644 index 0000000000..fb107cb868 --- /dev/null +++ b/packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts @@ -0,0 +1,158 @@ +import { v4 as uuidv4 } from "uuid"; +import { describe, it, expect } from "vitest"; + +import { buildResponsesForReporting } from "./report.handler"; + +describe("buildResponsesForReporting", () => { + it("for fields with options, it should return the labels of the options", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + + const responsesFromDb = [ + { + [field1Id]: { value: "value1" }, + [field2Id]: { value: ["option1", "option2"] }, + }, + ]; + + const fields = [ + { id: field1Id, label: "Field 1" }, + { + id: field2Id, + label: "Field 2", + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + ], + }, + ]; + + const expectedResponses = [["value1", "Option 1, Option 2"]]; + + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(["Field 1", "Field 2"]); + }); + + it("for fields with options having null id, it should return what DB provided(as that would be labels only).", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const responsesFromDb = [ + { + [field1Id]: { value: "value1" }, + [field2Id]: { value: ["Option 1", "Option 2"] }, + }, + ]; + + const fields = [ + { id: field1Id, label: "Field 1" }, + { + id: field2Id, + label: "Field 2", + options: [ + { id: null, label: "Option 1" }, + { id: null, label: "Option 2" }, + ], + }, + ]; + + const expectedResponses = [["value1", "Option 1, Option 2"]]; + + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(["Field 1", "Field 2"]); + }); + + it("for fields with no options but having array value, it should return the value as is", () => { + const field2Id = uuidv4(); + const responsesFromDb = [ + { + [field2Id]: { value: ["Option 1", "Option 2"] }, + }, + ]; + + const fields = [ + { + id: field2Id, + label: "Field 2", + }, + ]; + + const expectedResponses = [["Option 1, Option 2"]]; + + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(["Field 2"]); + }); + + it("should correctly handle numeric responses converting them to strings", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const responsesFromDb = [ + { + [field1Id]: { value: 1 }, + [field2Id]: { value: [1, 2] }, + }, + ]; + + const fields = [ + { id: field1Id, label: "Field 1" }, + { + id: field2Id, + label: "Field 2", + options: [ + { id: null, label: "Option 1" }, + { id: null, label: "Option 2" }, + ], + }, + ]; + const expectedResponses = [["1", "1, 2"]]; + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(["Field 1", "Field 2"]); + }); + + it("should handle empty responses", () => { + const field1Id = uuidv4(); + const responsesFromDb = [{}]; + + const fields = [{ id: field1Id, label: "Field 1" }]; + + const expectedResponses = [[""]]; + + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(["Field 1"]); + }); + + it("should show correct header for deleted fields", () => { + const field1Id = uuidv4(); + const field2Id = uuidv4(); + const responsesFromDb = [ + { + [field1Id]: { value: "value1" }, + [field2Id]: { value: ["Option 1", "Option 2"] }, + }, + ]; + + const fields = [ + { id: field1Id, label: "Field 1" }, + { + id: field2Id, + label: "Field 2", + deleted: true, + options: [ + { id: "option1", label: "Option 1" }, + { id: "option2", label: "Option 2" }, + ], + }, + ]; + + const expectedResponses = [["value1", "Option 1, Option 2"]]; + const expectedHeaders = ["Field 1", "Field 2(Deleted)"]; + + const { responses, headers } = buildResponsesForReporting({ responsesFromDb, fields }); + expect(responses).toEqual(expectedResponses); + expect(headers).toEqual(expectedHeaders); + }); +}); diff --git a/packages/app-store/routing-forms/trpc/report.handler.ts b/packages/app-store/routing-forms/trpc/report.handler.ts index cfb34167ac..ac13e1b924 100644 --- a/packages/app-store/routing-forms/trpc/report.handler.ts +++ b/packages/app-store/routing-forms/trpc/report.handler.ts @@ -1,10 +1,14 @@ +import type { z } from "zod"; + import logger from "@calcom/lib/logger"; import type { PrismaClient } from "@calcom/prisma"; +import type { App_RoutingForms_FormResponse } from "@calcom/prisma/client"; import { TRPCError } from "@calcom/trpc/server"; import { jsonLogicToPrisma } from "../jsonLogicToPrisma"; import { getSerializableForm } from "../lib/getSerializableForm"; -import type { Response } from "../types/types"; +import type { FormResponse } from "../types/types"; +import type { zodNonRouterField, zodFieldView } from "../zod"; import type { TReportInputSchema } from "./report.schema"; interface ReportHandlerOptions { @@ -13,6 +17,45 @@ interface ReportHandlerOptions { }; input: TReportInputSchema; } + +type Field = z.infer; + +function ensureStringOrStringArray(value: string | number | (string | number)[]): string | string[] { + if (typeof value === "string") { + return value; + } else if (value instanceof Array) { + return value.map((v) => v.toString()); + } + return [value.toString()]; +} + +function getLabelsFromOptionIds({ + options, + optionIds, +}: { + options: NonNullable; + optionIds: string | string[]; +}) { + if (optionIds instanceof Array) { + const labels = optionIds.map((optionId) => { + const foundOption = options.find((option) => option.id === optionId); + // It would mean that the optionId is actually a label which is why it isn't matching any option id. + // Fallback to optionId(i.e. label) which was the case with legacy options + if (!foundOption) { + return optionId; + } + return foundOption.label; + }); + return labels; + } else { + const foundOption = options.find((option) => option.id === optionIds); + if (!foundOption) { + return [optionIds]; + } + return [foundOption.label]; + } +} + export const reportHandler = async ({ ctx: { prisma }, input }: ReportHandlerOptions) => { // Can be any prisma `where` clause // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -49,27 +92,13 @@ export const reportHandler = async ({ ctx: { prisma }, input }: ReportHandlerOpt take, skip, }); + const fields = serializedForm?.fields || []; - const headers = fields.map((f) => f.label + (f.deleted ? "(Deleted)" : "")); - const responses: (string | number)[][] = []; - rows.forEach((r) => { - const rowResponses: (string | number)[] = []; - responses.push(rowResponses); - fields.forEach((field) => { - if (!r.response) { - return; - } - const response = r.response as Response; - const value = response[field.id]?.value || ""; - let transformedValue; - if (value instanceof Array) { - transformedValue = value.join(", "); - } else { - transformedValue = value; - } - rowResponses.push(transformedValue); - }); + const { responses, headers } = buildResponsesForReporting({ + responsesFromDb: rows.map((r) => r.response), + fields, }); + const areThereNoResultsOrLessThanAskedFor = !rows.length || rows.length < take; return { headers, @@ -79,3 +108,36 @@ export const reportHandler = async ({ ctx: { prisma }, input }: ReportHandlerOpt }; export default reportHandler; + +export function buildResponsesForReporting({ + responsesFromDb, + fields, +}: { + responsesFromDb: App_RoutingForms_FormResponse["response"][]; + fields: Pick, "id" | "options" | "label" | "deleted">[]; +}) { + const headers = fields.map((f) => f.label + (f.deleted ? "(Deleted)" : "")); + const responses: string[][] = []; + responsesFromDb.forEach((r) => { + const rowResponses: string[] = []; + responses.push(rowResponses); + fields.forEach((field) => { + if (!r) { + return; + } + const response = r as FormResponse; + const value = response[field.id]?.value || ""; + if (field.options) { + const optionIds = ensureStringOrStringArray(value); + const labels = getLabelsFromOptionIds({ options: field.options, optionIds }); + rowResponses.push(labels.join(", ")); + } else { + const arrayOfValues = value instanceof Array ? value : [value]; + const transformedValue = arrayOfValues.join(", "); + rowResponses.push(transformedValue); + } + }); + }); + + return { responses, headers }; +} diff --git a/packages/app-store/routing-forms/trpc/response.handler.ts b/packages/app-store/routing-forms/trpc/response.handler.ts index 5472129236..d030126500 100644 --- a/packages/app-store/routing-forms/trpc/response.handler.ts +++ b/packages/app-store/routing-forms/trpc/response.handler.ts @@ -6,7 +6,7 @@ import { RoutingFormSettings } from "@calcom/prisma/zod-utils"; import { TRPCError } from "@calcom/trpc/server"; import { getSerializableForm } from "../lib/getSerializableForm"; -import type { Response } from "../types/types"; +import type { FormResponse } from "../types/types"; import type { TResponseInputSchema } from "./response.schema"; import { onFormSubmission } from "./utils"; @@ -117,7 +117,7 @@ export const responseHandler = async ({ ctx, input }: ResponseHandlerOptions) => await onFormSubmission( { ...serializableFormWithFields, userWithEmails }, - dbFormResponse.response as Response + dbFormResponse.response as FormResponse ); return dbFormResponse; } catch (e) { diff --git a/packages/app-store/routing-forms/trpc/utils.ts b/packages/app-store/routing-forms/trpc/utils.ts index bd825e54ce..a1b2df8a8f 100644 --- a/packages/app-store/routing-forms/trpc/utils.ts +++ b/packages/app-store/routing-forms/trpc/utils.ts @@ -8,30 +8,97 @@ import { WebhookTriggerEvents } from "@calcom/prisma/client"; import type { Ensure } from "@calcom/types/utils"; import type { OrderedResponses } from "../types/types"; -import type { Response, SerializableForm } from "../types/types"; +import type { FormResponse, SerializableForm } from "../types/types"; + +type Field = NonNullable["fields"]>[number]; + +function isOptionsField(field: Pick) { + return (field.type === "select" || field.type === "multiselect") && field.options; +} + +function getFieldResponse({ + field, + fieldResponseValue, +}: { + fieldResponseValue: FormResponse[keyof FormResponse]["value"]; + field: Pick; +}) { + if (!isOptionsField(field)) { + return { + value: fieldResponseValue, + response: fieldResponseValue, + }; + } + + if (!field.options) { + return { + value: fieldResponseValue, + response: fieldResponseValue, + }; + } + + const valueArray = fieldResponseValue instanceof Array ? fieldResponseValue : [fieldResponseValue]; + const chosenOptions = valueArray.map((idOrLabel) => { + const foundOptionById = field.options?.find((option) => { + return option.id === idOrLabel; + }); + if (foundOptionById) { + return { + label: foundOptionById.label, + id: foundOptionById.id, + }; + } else { + return { + label: idOrLabel.toString(), + id: null, + }; + } + }); + return { + // value is a legacy prop that is just sending the labels which can change + value: chosenOptions.map((option) => option.label), + // response is new prop that is sending the label along with id(which doesn't change) + response: chosenOptions, + }; +} + +type SelectFieldWebhookResponse = string | number | string[] | { label: string; id: string | null }; +type FORM_SUBMITTED_WEBHOOK_RESPONSES = Record< + string, + { + /** + * Deprecates `value` prop as it now has both the id(that doesn't change) and the label(that can change but is human friendly) + */ + response: number | string | string[] | SelectFieldWebhookResponse | SelectFieldWebhookResponse[]; + /** + * @deprecated Use `response` instead + */ + value: FormResponse[keyof FormResponse]["value"]; + } +>; export async function onFormSubmission( form: Ensure< SerializableForm & { user: Pick; userWithEmails?: string[] }, "fields" >, - response: Response + response: FormResponse ) { - const fieldResponsesByName: Record< - string, - { - value: Response[keyof Response]["value"]; - } - > = {}; + const fieldResponsesByIdentifier: FORM_SUBMITTED_WEBHOOK_RESPONSES = {}; for (const [fieldId, fieldResponse] of Object.entries(response)) { + const field = form.fields.find((f) => f.id === fieldId); + if (!field) { + throw new Error(`Field with id ${fieldId} not found`); + } // Use the label lowercased as the key to identify a field. const key = form.fields.find((f) => f.id === fieldId)?.identifier || - (fieldResponse.label as keyof typeof fieldResponsesByName); - fieldResponsesByName[key] = { - value: fieldResponse.value, - }; + (fieldResponse.label as keyof typeof fieldResponsesByIdentifier); + fieldResponsesByIdentifier[key] = getFieldResponse({ + fieldResponseValue: fieldResponse.value, + field, + }); } const { userId, teamId } = getWebhookTargetEntity(form); @@ -57,14 +124,14 @@ export async function onFormSubmission( formId: form.id, formName: form.name, teamId: form.teamId, - responses: fieldResponsesByName, + responses: fieldResponsesByIdentifier, }, rootData: { // Send responses unwrapped at root level for backwards compatibility - ...Object.entries(fieldResponsesByName).reduce((acc, [key, value]) => { + ...Object.entries(fieldResponsesByIdentifier).reduce((acc, [key, value]) => { acc[key] = value.value; return acc; - }, {} as Record), + }, {} as Record), }, }).catch((e) => { console.error(`Error executing routing form webhook`, webhook, e); diff --git a/packages/app-store/routing-forms/types/types.d.ts b/packages/app-store/routing-forms/types/types.d.ts index cc8694803a..5fc63e9e4c 100644 --- a/packages/app-store/routing-forms/types/types.d.ts +++ b/packages/app-store/routing-forms/types/types.d.ts @@ -10,7 +10,7 @@ export type RoutingForm = SerializableForm; export type QueryBuilderUpdatedConfig = typeof QueryBuilderInitialConfig & { fields: Config["fields"] }; -export type Response = Record< +export type FormResponse = Record< // Field ID string, { @@ -20,7 +20,7 @@ export type Response = Record< >; export type Fields = z.infer; -export type Field = Fields[number]; +export type Field = NonNullable[number]; export type Routes = z.infer; export type Route = Routes[0]; export type NonRouterRoute = z.infer; @@ -55,4 +55,4 @@ export type SerializableRoute = }) | GlobalRoute; -export type OrderedResponses = Response[string][]; +export type OrderedResponses = FormResponse[string][]; diff --git a/packages/app-store/routing-forms/zod.ts b/packages/app-store/routing-forms/zod.ts index 4aaba39070..738412940c 100644 --- a/packages/app-store/routing-forms/zod.ts +++ b/packages/app-store/routing-forms/zod.ts @@ -6,9 +6,23 @@ export const zodNonRouterField = z.object({ identifier: z.string().optional(), placeholder: z.string().optional(), type: z.string(), + /** + * @deprecated in favour of `options` + */ selectText: z.string().optional(), required: z.boolean().optional(), deleted: z.boolean().optional(), + options: z + .array( + z.object({ + label: z.string(), + // To keep backwards compatibility with the options generated from legacy selectText, we allow saving null as id + // It helps in differentiating whether the routing logic should consider the option.label as value or option.id as value. + // This is important for legacy routes which has option.label saved in conditions and it must keep matching with the value of the option + id: z.string().or(z.null()), + }) + ) + .optional(), }); export const zodRouterField = zodNonRouterField.extend({ diff --git a/packages/features/shell/Shell.tsx b/packages/features/shell/Shell.tsx index 4dcb01520b..1d78879fb7 100644 --- a/packages/features/shell/Shell.tsx +++ b/packages/features/shell/Shell.tsx @@ -214,7 +214,8 @@ const useBanners = () => { const Layout = (props: LayoutProps) => { const banners = useBanners(); - + const pathname = usePathname(); + const isFullPageWithoutSidebar = pathname.startsWith("/apps/routing-forms/reporting/"); const { data: user } = trpc.viewer.me.useQuery(); const { boot } = useIntercom(); const pageTitle = typeof props.heading === "string" && !props.title ? props.heading : props.title; @@ -253,7 +254,7 @@ const Layout = (props: LayoutProps) => {
- {banners && !props.isPlatformUser && ( + {banners && !props.isPlatformUser && !isFullPageWithoutSidebar && (
{Object.keys(banners).map((key) => { if (key === "teamUpgradeBanner") { diff --git a/packages/prisma/seed-app-store.ts b/packages/prisma/seed-app-store.ts index eaf642a04e..bcee5f3155 100644 --- a/packages/prisma/seed-app-store.ts +++ b/packages/prisma/seed-app-store.ts @@ -135,11 +135,28 @@ async function seedAppData() { { id: "d4292635-9f12-17b1-9153-c3a854649182", type: "multiselect", - label: "Multi Select", + label: "Multi Select(with legacy `selectText`)", identifier: "multi", selectText: "Option-1\nOption-2", required: false, }, + { + id: "d4292635-9f12-17b1-9153-c3a854649182", + type: "multiselect", + label: "Multi Select", + identifier: "multi", + options: [ + { + id: "d1234635-9f12-17b1-9153-c3a854649182", + label: "Option-1", + }, + { + id: "d1235635-9f12-17b1-9153-c3a854649182", + label: "Option-2", + }, + ], + required: false, + }, ], user: { connect: {