From 8dbd96a2f9fa347a8f4d6ef8acb18beace5b49bf Mon Sep 17 00:00:00 2001
From: Vijay <vijayraghav22@gmail.com>
Date: Thu, 8 Aug 2024 14:57:58 +0530
Subject: [PATCH] fix: Unable to submit booking form if Attendee Phone is used
 and then booking Fields are saved (#15742)

* update for location identifier

* for typecheck err

* Update packages/features/bookings/lib/handleNewBooking/getBookingData.ts

Co-authored-by: Amit Sharma <74371312+Amit91848@users.noreply.github.com>

* update to use custom label for location, remove usage of hideWhenOneOption, remove string conversion of booking field

* nit

* nit

* nit

* Use the dataStore only instead of modifying bookingFields directly from outside the FormBuilder

* Dont unset the actual label, instead have a different property noLabel for this

---------

Co-authored-by: Amit Sharma <74371312+Amit91848@users.noreply.github.com>
Co-authored-by: Hariom <hariombalhara@gmail.com>
---
 apps/web/components/eventtype/Locations.tsx   | 17 -----
 packages/features/form-builder/Components.tsx | 42 +++++++++---
 .../features/form-builder/FormBuilder.tsx     | 67 ++++++++++---------
 .../form-builder/FormBuilderField.tsx         | 61 +++++++++++------
 4 files changed, 108 insertions(+), 79 deletions(-)

diff --git a/apps/web/components/eventtype/Locations.tsx b/apps/web/components/eventtype/Locations.tsx
index eebf5302e0..0225cc3fa0 100644
--- a/apps/web/components/eventtype/Locations.tsx
+++ b/apps/web/components/eventtype/Locations.tsx
@@ -268,23 +268,6 @@ const Locations: React.FC<LocationsProps> = ({
                         });
                         showToast(t("location_already_exists"), "warning");
                       }
-                      // Whenever location changes, we need to reset the locations item in booking questions list else it overflows
-                      // previously added values resulting in wrong behaviour
-                      const existingBookingFields = getValues("bookingFields");
-                      const findLocation = existingBookingFields.findIndex(
-                        (field) => field.name === "location"
-                      );
-                      if (findLocation >= 0) {
-                        existingBookingFields[findLocation] = {
-                          ...existingBookingFields[findLocation],
-                          type: "radioInput",
-                          label: "",
-                          placeholder: "",
-                        };
-                        setValue("bookingFields", existingBookingFields, {
-                          shouldDirty: true,
-                        });
-                      }
                     }
                   }}
                 />
diff --git a/packages/features/form-builder/Components.tsx b/packages/features/form-builder/Components.tsx
index 021f54325f..b2b430bb4a 100644
--- a/packages/features/form-builder/Components.tsx
+++ b/packages/features/form-builder/Components.tsx
@@ -71,6 +71,7 @@ type Component =
         } & {
           name?: string;
           required?: boolean;
+          translatedDefaultLabel?: string;
         }
       >(
         props: TProps
@@ -386,7 +387,16 @@ export const Components: Record<FieldType, Component> = {
   },
   radioInput: {
     propsType: propsTypes.radioInput,
-    factory: function RadioInputWithLabel({ name, options, optionsInputs, value, setValue, readOnly }) {
+    factory: function RadioInputWithLabel({
+      name,
+      label,
+      options,
+      optionsInputs,
+      value,
+      setValue,
+      readOnly,
+      translatedDefaultLabel,
+    }) {
       useEffect(() => {
         if (!value) {
           setValue({
@@ -398,17 +408,24 @@ export const Components: Record<FieldType, Component> = {
 
       const { t } = useLocale();
 
-      const getCleanLabel = (option: { label: string; value: string }): string | JSX.Element => {
-        if (!option.label) {
+      const didUserProvideLabel = (
+        label: string | undefined,
+        translatedDefaultLabel: string | undefined
+      ): label is string => {
+        return label && translatedDefaultLabel ? translatedDefaultLabel !== label : false;
+      };
+
+      const getCleanLabel = (label: string): string | JSX.Element => {
+        if (!label) {
           return "";
         }
 
-        return option.label.search(/^https?:\/\//) !== -1 ? (
-          <a href={option.label} target="_blank">
-            <span className="underline">{option.label}</span>
+        return label.search(/^https?:\/\//) !== -1 ? (
+          <a href={label} target="_blank">
+            <span className="underline">{label}</span>
           </a>
         ) : (
-          option.label
+          label
         );
       };
 
@@ -434,7 +451,9 @@ export const Components: Record<FieldType, Component> = {
                         }}
                         checked={value?.value === option.value}
                       />
-                      <span className="text-emphasis me-2 ms-2 text-sm">{getCleanLabel(option) ?? ""}</span>
+                      <span className="text-emphasis me-2 ms-2 text-sm">
+                        {getCleanLabel(option.label) ?? ""}
+                      </span>
                       <span>
                         {option.value === "phone" && (
                           <InfoBadge content={t("number_in_international_format")} />
@@ -444,10 +463,13 @@ export const Components: Record<FieldType, Component> = {
                   );
                 })
               ) : (
-                // Show option itself as label because there is just one option
+                // Use the only option itself to determine if the field is required or not.
                 <>
                   <Label className="flex">
-                    {options[0].label}
+                    {/* We still want to show the label of the field if it is changed by the user otherwise the best label would be the option label */}
+                    {getCleanLabel(
+                      didUserProvideLabel(label, translatedDefaultLabel) ? label : options[0].label
+                    )}
                     {!readOnly && optionsInputs[options[0].value]?.required ? (
                       <span className="text-default mb-1 ml-1 text-sm font-medium">*</span>
                     ) : null}
diff --git a/packages/features/form-builder/FormBuilder.tsx b/packages/features/form-builder/FormBuilder.tsx
index 14fbfca75a..8031cb107f 100644
--- a/packages/features/form-builder/FormBuilder.tsx
+++ b/packages/features/form-builder/FormBuilder.tsx
@@ -4,7 +4,6 @@ import type { SubmitHandler, UseFormReturn } from "react-hook-form";
 import { Controller, useFieldArray, useForm, useFormContext } from "react-hook-form";
 import type { z } from "zod";
 
-import { getAndUpdateNormalizedValues } from "@calcom/features/form-builder/FormBuilderField";
 import { classNames } from "@calcom/lib";
 import { useLocale } from "@calcom/lib/hooks/useLocale";
 import { markdownToSafeHTML } from "@calcom/lib/markdownToSafeHTML";
@@ -45,6 +44,13 @@ function getCurrentFieldType(fieldForm: UseFormReturn<RhfFormField>) {
   return fieldTypesConfigMap[fieldForm.watch("type") || "text"];
 }
 
+function isRequiredField(field: RhfFormField) {
+  // 'location' must be shown as required in the UI(but at backend it is not required because not specifying it defaults to CalVideo)
+  // So, handle it in UI only instead of updating bookingFields config in getBookingFields
+  // TODO: FormBuilder must not be made aware of BookingFields, so move this "location" check to outside FormBuilder where the component is used.
+  return field.name === "location" ? true : field.required;
+}
+
 /**
  * It works with a react-hook-form only.
  * `formProp` specifies the name of the property in the react-hook-form that has the fields. This is where fields would be updated.
@@ -119,29 +125,37 @@ export const FormBuilder = function FormBuilder({
         <p className="text-subtle mt-0.5 max-w-[280px] break-words text-sm sm:max-w-[500px]">{description}</p>
         <ul ref={parent} className="border-subtle divide-subtle mt-4 divide-y rounded-md border">
           {fields.map((field, index) => {
-            const options = field.options
-              ? field.options
-              : field.getOptionsAt
-              ? dataStore.options[field.getOptionsAt as keyof typeof dataStore]
-              : [];
+            let options = field.options ?? null;
+            const sources = [...(field.sources || [])];
+            const isRequired = isRequiredField(field);
+            if (!options && field.getOptionsAt) {
+              options = dataStore.options[field.getOptionsAt as keyof typeof dataStore] ?? [];
+              // TODO: The dataStore should itself provide the label directly
+              // This is important because FormBuilder isn't aware of what a location is. It is a generic Form Builder
+              const sourceLabel = field.getOptionsAt === "locations" ? "Location" : field.getOptionsAt;
+              options.forEach((option) => {
+                sources.push({
+                  id: option.value,
+                  label: sourceLabel,
+                  type: "system",
+                });
+              });
+            }
 
-            // Note: We recently started calling getAndUpdateNormalizedValues in the FormBuilder. It was supposed to be called only on booking pages earlier.
-            // Due to this we have to meet some strict requirements like of labelAsSafeHtml.
             if (fieldsThatSupportLabelAsSafeHtml.includes(field.type)) {
               field = { ...field, labelAsSafeHtml: markdownToSafeHTML(field.label ?? "") };
             }
-
-            const { hidden } = getAndUpdateNormalizedValues({ ...field, options }, t);
-            if (field.hideWhenJustOneOption && (hidden || !options?.length)) {
+            const numOptions = options?.length ?? 0;
+            const firstOptionInput =
+              field.optionsInputs?.[options?.[0]?.value as keyof typeof field.optionsInputs];
+            const doesFirstOptionHaveInput = !!firstOptionInput;
+            // If there is only one option and it doesn't have an input required, we don't show the Field for it.
+            // Because booker doesn't see this in UI, there is no point showing it in FormBuilder to configure it.
+            if (field.hideWhenJustOneOption && numOptions <= 1 && !doesFirstOptionHaveInput) {
               return null;
             }
-            let fieldType = fieldTypesConfigMap[field.type];
-            let isRequired = field.required;
-            // For radioInput type, when there's only one option, the type and required takes the first options values
-            if (field.type === "radioInput" && options.length === 1) {
-              fieldType = fieldTypesConfigMap[field.optionsInputs?.[options[0].value].type || field.type];
-              isRequired = field.optionsInputs?.[options[0].value].required || field.required;
-            }
+
+            const fieldType = fieldTypesConfigMap[field.type];
             const isFieldEditableSystemButOptional = field.editable === "system-but-optional";
             const isFieldEditableSystemButHidden = field.editable === "system-but-hidden";
             const isFieldEditableSystem = field.editable === "system";
@@ -151,7 +165,6 @@ export const FormBuilder = function FormBuilder({
             if (!fieldType) {
               throw new Error(`Invalid field type - ${field.type}`);
             }
-            const sources = field.sources || [];
             const groupedBySourceLabel = sources.reduce((groupBy, source) => {
               const item = groupBy[source.label] || [];
               if (source.type === "user" || source.type === "default") {
@@ -244,16 +257,7 @@ export const FormBuilder = function FormBuilder({
                       data-testid="edit-field-action"
                       color="secondary"
                       onClick={() => {
-                        const fieldToEdit = field;
-                        // For radioInput type, when there's only one option, the type and required takes the only first options values
-                        if (fieldToEdit.type === "radioInput" && options.length === 1) {
-                          fieldToEdit.type =
-                            fieldToEdit.optionsInputs?.[options[0].value].type || fieldToEdit.type;
-                          fieldToEdit.required =
-                            fieldToEdit.optionsInputs?.[options[0].value].required || fieldToEdit.required;
-                        }
-
-                        editField(index, fieldToEdit);
+                        editField(index, field);
                       }}>
                       {t("edit")}
                     </Button>
@@ -522,12 +526,13 @@ function FieldEditDialog({
                     <Controller
                       name="required"
                       control={fieldForm.control}
-                      render={({ field: { value, onChange } }) => {
+                      render={({ field: { onChange } }) => {
+                        const isRequired = isRequiredField(fieldForm.getValues());
                         return (
                           <BooleanToggleGroupField
                             data-testid="field-required"
                             disabled={fieldForm.getValues("editable") === "system"}
-                            value={value}
+                            value={isRequired}
                             onValueChange={(val) => {
                               onChange(val);
                             }}
diff --git a/packages/features/form-builder/FormBuilderField.tsx b/packages/features/form-builder/FormBuilderField.tsx
index e3699dd642..a9f2e46fa9 100644
--- a/packages/features/form-builder/FormBuilderField.tsx
+++ b/packages/features/form-builder/FormBuilderField.tsx
@@ -54,7 +54,10 @@ export const FormBuilderField = ({
   const { t } = useLocale();
   const { control, formState } = useFormContext();
 
-  const { hidden, placeholder, label } = getAndUpdateNormalizedValues(field, t);
+  const { hidden, placeholder, label, noLabel, translatedDefaultLabel } = getAndUpdateNormalizedValues(
+    field,
+    t
+  );
 
   return (
     <div data-fob-field-name={field.name} className={classNames(className, hidden ? "hidden" : "")}>
@@ -72,6 +75,8 @@ export const FormBuilderField = ({
                 setValue={(val: unknown) => {
                   onChange(val);
                 }}
+                noLabel={noLabel}
+                translatedDefaultLabel={translatedDefaultLabel}
               />
               <ErrorMessage
                 name="responses"
@@ -120,10 +125,12 @@ const WithLabel = ({
   field,
   children,
   readOnly,
+  noLabel = false,
 }: {
   field: Partial<RhfFormField>;
   readOnly: boolean;
   children: React.ReactNode;
+  noLabel?: boolean;
 }) => {
   const { t } = useLocale();
 
@@ -132,17 +139,21 @@ const WithLabel = ({
       {/* multiemail doesnt show label initially. It is shown on clicking CTA */}
       {/* boolean type doesn't have a label overall, the radio has it's own label */}
       {/* Component itself managing it's label should remove these checks */}
-      {field.type !== "boolean" && field.type !== "multiemail" && field.label && (
-        <div className="mb-2 flex items-center">
-          <Label className="!mb-0 flex">
-            <span>{field.label}</span>
-            <span className="text-emphasis -mb-1 ml-1 text-sm font-medium leading-none">
-              {!readOnly && field.required ? "*" : ""}
-            </span>
-            {field.type === "phone" && <InfoBadge content={t("number_in_international_format")} />}
-          </Label>
-        </div>
-      )}
+      {noLabel
+        ? null
+        : field.type !== "boolean" &&
+          field.type !== "multiemail" &&
+          field.label && (
+            <div className="mb-2 flex items-center">
+              <Label className="!mb-0 flex">
+                <span>{field.label}</span>
+                <span className="text-emphasis -mb-1 ml-1 text-sm font-medium leading-none">
+                  {!readOnly && field.required ? "*" : ""}
+                </span>
+                {field.type === "phone" && <InfoBadge content={t("number_in_international_format")} />}
+              </Label>
+            </div>
+          )}
       {children}
     </div>
   );
@@ -151,7 +162,7 @@ const WithLabel = ({
 /**
  * Ensures that `labels` and `placeholders`, wherever they are, are set properly. If direct values are not set, default values from fieldTypeConfig are used.
  */
-export function getAndUpdateNormalizedValues(field: RhfFormFields[number], t: TFunction) {
+function getAndUpdateNormalizedValues(field: RhfFormFields[number], t: TFunction) {
   let noLabel = false;
   let hidden = !!field.hidden;
   if (field.type === "radioInput") {
@@ -164,6 +175,7 @@ export function getAndUpdateNormalizedValues(field: RhfFormFields[number], t: TF
         throw new Error("radioInput must have optionsInputs");
       }
       if (field.optionsInputs[options[0].value]) {
+        // We don't show the label in this case because the optionInput itself will decide what label to show
         noLabel = true;
       } else {
         // If there's only one option and it doesn't have an input, we don't show the field at all because it's visible in the left side bar
@@ -179,7 +191,8 @@ export function getAndUpdateNormalizedValues(field: RhfFormFields[number], t: TF
     throw new Error(`${field.name}:${field.type} type must have labelAsSafeHtml set`);
   }
 
-  const label = noLabel ? "" : field.labelAsSafeHtml || field.label || t(field.defaultLabel || "");
+  const translatedDefaultLabel = t(field.defaultLabel || "");
+  const label = field.labelAsSafeHtml || field.label || translatedDefaultLabel;
   const placeholder = field.placeholder || t(field.defaultPlaceholder || "");
 
   if (field.variantsConfig?.variants) {
@@ -194,7 +207,7 @@ export function getAndUpdateNormalizedValues(field: RhfFormFields[number], t: TF
     });
   }
 
-  return { hidden, placeholder, label };
+  return { hidden, placeholder, label, noLabel, translatedDefaultLabel };
 }
 
 export const ComponentForField = ({
@@ -202,12 +215,16 @@ export const ComponentForField = ({
   value,
   setValue,
   readOnly,
+  noLabel,
+  translatedDefaultLabel,
 }: {
   field: Omit<RhfFormField, "editable" | "label"> & {
     // Label is optional because radioInput doesn't have a label
     label?: string;
   };
   readOnly: boolean;
+  noLabel?: boolean;
+  translatedDefaultLabel?: string;
 } & ValueProps) => {
   const fieldType = field.type || "text";
   const componentConfig = Components[fieldType];
@@ -227,7 +244,7 @@ export const ComponentForField = ({
   }
   if (componentConfig.propsType === "text") {
     return (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           placeholder={field.placeholder}
           minLength={field.minLength}
@@ -244,7 +261,7 @@ export const ComponentForField = ({
 
   if (componentConfig.propsType === "boolean") {
     return (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           name={field.name}
           label={field.label}
@@ -259,7 +276,7 @@ export const ComponentForField = ({
 
   if (componentConfig.propsType === "textList") {
     return (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           placeholder={field.placeholder}
           name={field.name}
@@ -278,7 +295,7 @@ export const ComponentForField = ({
     }
 
     return (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           readOnly={readOnly}
           value={value as string}
@@ -296,7 +313,7 @@ export const ComponentForField = ({
       throw new Error("Field options is not defined");
     }
     return (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           placeholder={field.placeholder}
           name={field.name}
@@ -320,16 +337,18 @@ export const ComponentForField = ({
     const options = field.options;
 
     return field.options.length ? (
-      <WithLabel field={field} readOnly={readOnly}>
+      <WithLabel field={field} readOnly={readOnly} noLabel={noLabel}>
         <componentConfig.factory
           placeholder={field.placeholder}
           readOnly={readOnly}
           name={field.name}
+          label={field.label}
           value={value as { value: string; optionValue: string }}
           setValue={setValue as (arg: typeof value) => void}
           optionsInputs={field.optionsInputs}
           options={options}
           required={field.required}
+          translatedDefaultLabel={translatedDefaultLabel}
         />
       </WithLabel>
     ) : null;