vynte-infra/calendar
2
0
Fork 0
Code Issues Pull Requests Actions 1.8k Packages Projects Releases Wiki Activity

fix: Unable to submit booking form if Attendee Phone is used and then booking Fields are saved (#15742)

Browse Source 
* 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>
This commit is contained in:
Vijay
2024-08-08 14:57:58 +05:30
committed by GitHub
co-authored by Amit Sharma Hariom
parent 3b186b19e6
commit 8dbd96a2f9
4 changed files with 108 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/components/eventtype/Locations.tsx
-17
View File
@@ -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,
});
}
}
}}
/>
packages/features/form-builder/Components.tsx
+32 -10
View File
@@ -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}
packages/features/form-builder/FormBuilder.tsx
+36 -31
View File
@@ -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);
}}
packages/features/form-builder/FormBuilderField.tsx
+40 -21
View File
@@ -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;