packages/app-store/routing-forms/api/responses/[formId].ts (1)

21-29: Restrict Prisma selection to only required fields to reduce payload.

Per repo guidelines, select only what you need. For the CSV loop below, only response and createdAt are used from FormResponse rows.

Apply this diff:

   while (
     (responses = await prisma.app_RoutingForms_FormResponse.findMany({
       where: {
         formId,
       },
       take: take,
       skip: skip,
+      select: {
+        response: true,
+        createdAt: true,
+      },
     })) &&

packages/app-store/routing-forms/lib/substituteVariables.ts (1)

26-41: Precompute maps to avoid quadratic scans and repeated work.

The nested loop (variables × response × fields.find) is O(V × R × F) and repeatedly computes identifiers/human-readable values. Pre-index fields by id and precompute identifier→value once from response, then substitute. This improves readability and performance.

Apply this refactor:

   variables.forEach((variable) => {
-    for (const key in response) {
-      const field = fields.find((field) => field.id === key);
-      if (!field) {
-        continue;
-      }
-      const identifier = getFieldIdentifier(field);
-      if (identifier.toLowerCase() === variable.toLowerCase()) {
-        const humanReadableValues = getHumanReadableFieldResponseValue({
-          field,
-          value: response[key].value,
-        });
-        // ['abc', 'def'] ----toString---> 'abc,def' ----slugify---> 'abc-def'
-        const valueToSubstitute = slugify(humanReadableValues.toString());
-        eventTypeUrl = eventTypeUrl.replace(`{${variable}}`, valueToSubstitute);
-      }
-    }
+    // Build maps once outside the variable loop for efficiency
+    // Note: hoist these outside variables.forEach if many variables are expected
+    const idToField = new Map(fields.map((f) => [f.id, f]));
+    const identifierToValue: Record<string, string> = {};
+    for (const key in response) {
+      const field = idToField.get(key);
+      if (!field) continue;
+      const identifier = getFieldIdentifier(field).toLowerCase();
+      const humanReadableValues = getHumanReadableFieldResponseValue({
+        field,
+        value: response[key].value,
+      });
+      identifierToValue[identifier] = slugify(humanReadableValues.toString());
+    }
+
+    const valueToSubstitute = identifierToValue[variable.toLowerCase()];
+    if (valueToSubstitute !== undefined) {
+      eventTypeUrl = eventTypeUrl.replaceAll(`{${variable}}`, valueToSubstitute);
+    }
   });

packages/app-store/routing-forms/components/_components/TestForm.tsx (1)

149-151: Be explicit about eventTypeId gate to avoid truthiness pitfalls.

The intent is to skip the mutation for custom event type (eventTypeId = 0) and only run when a valid, positive ID exists. Make the check explicit for readability.

-    if (supportsTeamMembersMatchingLogic && route.action.eventTypeId) {
+    if (
+      supportsTeamMembersMatchingLogic &&
+      typeof route.action.eventTypeId === "number" &&
+      route.action.eventTypeId > 0
+    ) {

packages/lib/server/service/routingForm/responseData/getHumanReadableFieldResponseValue.ts (4)

11-18: Prefer Array.isArray and clarify the scalar-number behavior

Use Array.isArray for array detection. Also, the JSDoc implies numbers become strings, but here numbers become a single-element array of strings. Clarify that to avoid confusion for downstream consumers.

Apply this diff:

 function ensureStringOrStringArray(value: string | number | (string | number)[]): string | string[] {
   if (typeof value === "string") {
     return value;
-  } else if (value instanceof Array) {
+  } else if (Array.isArray(value)) {
     return value.map((v) => v.toString());
   }
   return [value.toString()];
 }

And update the JSDoc above to reflect the array-wrapping for non-string scalars:

 /**
- * Ensures a value is either a string or string array.
- * Converts numbers to strings.
+ * Normalizes a response value to a string or string[].
+ * Numbers are converted to strings; non-string scalars are wrapped in a single-element array
+ * to preserve downstream join/CSV semantics.
  *
  * @param value - The value to normalize
- * @returns String or string array
+ * @returns string | string[]
  */

---

29-36: Annotate return type and use Array.isArray

Make the return type explicit (string[]) and use Array.isArray for consistency and cross-realm safety.

Apply this diff:

 function getLabelsFromOptionIds({
   options,
   optionIds,
 }: {
   options: NonNullable<Field["options"]>;
   optionIds: string | string[];
-}) {
-  if (optionIds instanceof Array) {
+}): string[] {
+  if (Array.isArray(optionIds)) {
     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;

Optional note: If options can be large and this runs in hot paths, consider precomputing a Map from id to label to reduce repeated linear scans.

---

56-64: JSDoc return type mismatch

The comment says “Array of human-readable values” but the function can return string or string[]. Update the doc to avoid confusion.

Apply this diff:

 /**
  * Get the human-readable value for a field response.
  * For select/multiselect/radio fields, this returns the label of the selected option(s).
- * For other fields, it returns the value as-is.
+ * For other fields, it returns the normalized value (string or string[]).
  *
  * @param field - The field definition with options
  * @param value - The response value (typically an option ID for select fields)
- * @returns Array of human-readable values
+ * @returns string | string[]
  */

---

72-79: Consider normalizing to a single return shape (string[])

Returning a union (string | string[]) forces callers to normalize again. Given the downstream usage relies on array join semantics (toString), you could simplify by always returning string[] (e.g., wrap single strings into an array). This would reduce branching in consumers.

Happy to provide a small refactor if you want to adopt a consistent string[] return shape.

packages/app-store/routing-forms/lib/substituteVariables.test.ts (1)

219-229: Add tests for radio fields and legacy label fallbacks

Two high-signal additions:

• Radio-type field behaves like select.
• Legacy responses where value holds a label instead of an ID should still substitute correctly (you already support this in the utility).

Apply this diff to extend coverage:

   it("should handle textarea fields like text fields", () => {
     const fields = [
       { id: "field1", identifier: "description", label: "Description", type: "textarea" as const },
     ];
     const routeValue = "/ticket/{description}";
     const response = createFormResponse("field1", "Bug Report Summary", "Description");

     const result = substituteVariables(routeValue, response, fields);
     expect(result).toBe("/ticket/bug-report-summary");
   });
+
+  it("should substitute radio field option labels", () => {
+    const fields: Field[] = [
+      {
+        id: "fieldRadio",
+        identifier: "choice",
+        label: "Choice",
+        type: "radio" as const,
+        options: [
+          { id: "opt-1", label: "First Option" },
+          { id: "opt-2", label: "Second Option" },
+        ],
+      },
+    ];
+    const routeValue = "/pick/{choice}";
+    const response = createFormResponse("fieldRadio", "opt-1", "Choice");
+    const result = substituteVariables(routeValue, response, fields);
+    expect(result).toBe("/pick/first-option");
+  });
+
+  it("should fallback to labels when option IDs are not found (legacy)", () => {
+    const fields = [
+      createSelectField("field1", "country", "Country", [{ id: "ind-001", label: "India" }]),
+    ];
+    const routeValue = "/team/{country}/meeting";
+    // Simulate legacy value already being the label instead of the ID
+    const response = createFormResponse("field1", "India", "Country");
+    const result = substituteVariables(routeValue, response, fields);
+    expect(result).toBe("/team/india/meeting");
+  });
 });

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.