fix: Allow all URL-safe characters in hidden field Id/ questionId (#2250)

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/HiddenFieldsCard.tsx

@@ -1,11 +1,12 @@
 "use client";
 
+import { validateId } from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/lib/validation";
 import * as Collapsible from "@radix-ui/react-collapsible";
 import { FC, useState } from "react";
-import toast from "react-hot-toast";
+import { toast } from "react-hot-toast";
 
 import { cn } from "@formbricks/lib/cn";
-import { TSurvey, TSurveyHiddenFields, TSurveyQuestions } from "@formbricks/types/surveys";
+import { TSurvey, TSurveyHiddenFields } from "@formbricks/types/surveys";
 import { Button } from "@formbricks/ui/Button";
 import { Input } from "@formbricks/ui/Input";
 import { Label } from "@formbricks/ui/Label";
@@ -117,28 +118,16 @@ const HiddenFieldsCard: FC<HiddenFieldsCardProps> = ({
             className="mt-5"
             onSubmit={(e) => {
               e.preventDefault();
-
-              // Check if the hiddenField is empty or just whitespace
-              if (!hiddenField.trim()) {
-                return toast.error("The field cannot be empty.");
+              const existingQuestionIds = localSurvey.questions.map((question) => question.id);
+              const existingHiddenFieldIds = localSurvey.hiddenFields.fieldIds ?? [];
+              if (validateId("Hidden field", hiddenField, existingQuestionIds, existingHiddenFieldIds)) {
+                updateSurvey({
+                  fieldIds: [...(localSurvey.hiddenFields?.fieldIds || []), hiddenField],
+                  enabled: true,
+                });
+                toast.success("Hidden field added successfully");
+                setHiddenField("");
               }
-
-              const errorMessage = validateHiddenField(
-                // current field
-                hiddenField,
-                // existing fields
-                localSurvey.hiddenFields?.fieldIds || [],
-                // existing questions
-                localSurvey.questions
-              );
-
-              if (errorMessage !== "") return toast.error(errorMessage);
-
-              updateSurvey({
-                fieldIds: [...(localSurvey.hiddenFields?.fieldIds || []), hiddenField],
-                enabled: true,
-              });
-              setHiddenField("");
             }}>
             <Label htmlFor="headline">Hidden Field</Label>
             <div className="mt-2 flex gap-2">
@@ -162,44 +151,3 @@ const HiddenFieldsCard: FC<HiddenFieldsCardProps> = ({
 };
 
 export default HiddenFieldsCard;
-
-const validateHiddenField = (
-  field: string,
-  existingFields: string[],
-  existingQuestions: TSurveyQuestions
-): string => {
-  if (field.trim() === "") {
-    return "Please enter a question";
-  }
-  // no duplicate questions
-  if (existingFields.findIndex((q) => q.toLowerCase() === field.toLowerCase()) !== -1) {
-    return "Question already exists";
-  }
-  // no key words -- userId & suid & existing question ids
-  if (
-    [
-      "userId",
-      "source",
-      "suid",
-      "end",
-      "start",
-      "welcomeCard",
-      "hidden",
-      "verifiedEmail",
-      "multiLanguage",
-    ].includes(field) ||
-    existingQuestions.findIndex((q) => q.id === field) !== -1
-  ) {
-    return "Question not allowed";
-  }
-  // no spaced words --> should be valid query param on url
-  if (field.includes(" ")) {
-    return "Question not allowed, avoid using spaces";
-  }
-  // Check if the parameter contains only alphanumeric characters
-  if (!/^[a-zA-Z0-9]+$/.test(field)) {
-    return "Question not allowed, avoid using special characters";
-  }
-
-  return "";
-};

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/QuestionsView.tsx

@@ -1,6 +1,11 @@
 "use client";
 
 import HiddenFieldsCard from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/HiddenFieldsCard";
+import {
+  isCardValid,
+  validateQuestion,
+  validateSurveyQuestionsInBatch,
+} from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/lib/validation";
 import { createId } from "@paralleldrive/cuid2";
 import { useEffect, useMemo, useState } from "react";
 import { DragDropContext } from "react-beautiful-dnd";
@@ -12,7 +17,6 @@ import { checkForEmptyFallBackValue, extractRecallInfo } from "@formbricks/lib/u
 import { TProduct } from "@formbricks/types/product";
 import { TSurvey, TSurveyQuestion } from "@formbricks/types/surveys";
 
-import { isCardValid, validateQuestion, validateSurveyQuestionsInBatch } from "../lib/validation";
 import AddQuestionButton from "./AddQuestionButton";
 import EditThankYouCard from "./EditThankYouCard";
 import EditWelcomeCard from "./EditWelcomeCard";

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/SurveyMenuBar.tsx

@@ -1,6 +1,10 @@
 "use client";
 
 import SurveyStatusDropdown from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/components/SurveyStatusDropdown";
+import {
+  isCardValid,
+  validateQuestion,
+} from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/lib/validation";
 import { isEqual } from "lodash";
 import { AlertTriangleIcon, ArrowLeftIcon, SettingsIcon } from "lucide-react";
 import { useRouter } from "next/navigation";
@@ -24,7 +28,6 @@ import { Input } from "@formbricks/ui/Input";
 import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@formbricks/ui/Tooltip";
 
 import { updateSurveyAction } from "../actions";
-import { isCardValid, validateQuestion } from "../lib/validation";
 
 interface SurveyMenuBarProps {
   localSurvey: TSurvey;

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/UpdateQuestionId.tsx

@@ -1,5 +1,6 @@
 "use client";
 
+import { validateId } from "@/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/lib/validation";
 import { useState } from "react";
 import toast from "react-hot-toast";
 
@@ -33,38 +34,15 @@ export default function UpdateQuestionId({
     }
 
     const questionIds = localSurvey.questions.map((q) => q.id);
-    if (questionIds.includes(currentValue)) {
-      setIsInputInvalid(true);
-      toast.error("IDs have to be unique per survey.");
-    } else if (currentValue.trim() === "" || currentValue.includes(" ")) {
-      setCurrentValue(prevValue);
-      updateQuestion(questionIdx, { id: prevValue });
-      toast.error("ID should not be empty.");
-      return;
-    } else if (
-      [
-        "userId",
-        "source",
-        "suid",
-        "end",
-        "start",
-        "welcomeCard",
-        "hidden",
-        "verifiedEmail",
-        "multiLanguage",
-      ].includes(currentValue)
-    ) {
-      setCurrentValue(prevValue);
-      updateQuestion(questionIdx, { id: prevValue });
-      toast.error("Reserved words cannot be used as question ID");
-      return;
-    } else {
+    const hiddenFieldIds = localSurvey.hiddenFields.fieldIds ?? [];
+    if (validateId("Question", currentValue, questionIds, hiddenFieldIds)) {
       setIsInputInvalid(false);
       toast.success("Question ID updated.");
+      updateQuestion(questionIdx, { id: currentValue });
+      setPrevValue(currentValue); // after successful update, set current value as previous value
+    } else {
+      setCurrentValue(prevValue);
     }
-
-    updateQuestion(questionIdx, { id: currentValue });
-    setPrevValue(currentValue); // after successful update, set current value as previous value
   };
 
   return (
@@ -77,12 +55,6 @@ export default function UpdateQuestionId({
           value={currentValue}
           onChange={(e) => {
             setCurrentValue(e.target.value);
-            localSurvey.hiddenFields?.fieldIds?.forEach((field) => {
-              if (field === e.target.value) {
-                setIsInputInvalid(true);
-                toast.error("QuestionID can't be equal to hidden fields");
-              }
-            });
           }}
           onBlur={saveAction}
           disabled={!(localSurvey.status === "draft" || question.isDraft)}

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/lib/validation.ts

@@ -1,4 +1,6 @@
 // extend this object in order to add more validation rules
+import { toast } from "react-hot-toast";
+
 import { extractLanguageCodes, getLocalizedValue } from "@formbricks/lib/i18n/utils";
 import {
   TI18nString,
@@ -15,7 +17,10 @@ import {
 } from "@formbricks/types/surveys";
 
 // Utility function to check if label is valid for all required languages
-const isLabelValidForAllLanguages = (label: TI18nString, surveyLanguages: TSurveyLanguage[]): boolean => {
+export const isLabelValidForAllLanguages = (
+  label: TI18nString,
+  surveyLanguages: TSurveyLanguage[]
+): boolean => {
   const filteredLanguages = surveyLanguages.filter((surveyLanguages) => {
     return surveyLanguages.enabled;
   });
@@ -33,7 +38,7 @@ const handleI18nCheckForMultipleChoice = (
 };
 
 // Validation rules
-const validationRules = {
+export const validationRules = {
   openText: (question: TSurveyOpenTextQuestion, languages: TSurveyLanguage[]) => {
     return question.placeholder &&
       getLocalizedValue(question.placeholder, "default").trim() !== "" &&
@@ -84,7 +89,7 @@ const validationRules = {
 };
 
 // Main validation function
-const validateQuestion = (question: TSurveyQuestion, surveyLanguages: TSurveyLanguage[]): boolean => {
+export const validateQuestion = (question: TSurveyQuestion, surveyLanguages: TSurveyLanguage[]): boolean => {
   const specificValidation = validationRules[question.type];
   const defaultValidation = validationRules.defaultValidation;
 
@@ -134,8 +139,6 @@ export const isCardValid = (
   );
 };
 
-export { validateQuestion, isLabelValidForAllLanguages };
-
 export const isValidUrl = (string: string): boolean => {
   try {
     new URL(string);
@@ -144,3 +147,51 @@ export const isValidUrl = (string: string): boolean => {
     return false;
   }
 };
+
+// Function to validate question ID and Hidden field Id
+export const validateId = (
+  type: "Hidden field" | "Question",
+  field: string,
+  existingQuestionIds: string[],
+  existingHiddenFieldIds: string[]
+): boolean => {
+  if (field.trim() === "") {
+    toast.error(`Please enter a ${type} Id.`);
+    return false;
+  }
+
+  const combinedIds = [...existingQuestionIds, ...existingHiddenFieldIds];
+
+  if (combinedIds.findIndex((id) => id.toLowerCase() === field.toLowerCase()) !== -1) {
+    toast.error(`${type} Id already exists in questions or hidden fields.`);
+    return false;
+  }
+
+  const forbiddenIds = [
+    "userId",
+    "source",
+    "suid",
+    "end",
+    "start",
+    "welcomeCard",
+    "hidden",
+    "verifiedEmail",
+    "multiLanguage",
+  ];
+  if (forbiddenIds.includes(field)) {
+    toast.error(`${type} Id not allowed.`);
+    return false;
+  }
+
+  if (field.includes(" ")) {
+    toast.error(`${type} Id not allowed, avoid using spaces.`);
+    return false;
+  }
+
+  if (!/^[a-zA-Z0-9_-]+$/.test(field)) {
+    toast.error(`${type} Id not allowed, use only alphanumeric characters, hyphens, or underscores.`);
+    return false;
+  }
+
+  return true;
+};