Conversation
Comment on lines
+720
to
+729
| setValueAs: (value) => { | ||
| if (value === "") { | ||
| return undefined; | ||
| } | ||
| const parsed = Number(value); | ||
| if (Number.isNaN(parsed) || !Number.isInteger(parsed) || parsed < 1) { | ||
| return undefined; | ||
| } | ||
| return parsed; | ||
| }, |
There was a problem hiding this comment.
[Logic] The setValueAs function currently swallows validation errors (like non-integers or values less than 1) by returning undefined, which zod treats as a valid optional value. This means invalid inputs (e.g. "1.5" or "-1") will simply unset the limit rather than showing a validation error.
Simiplifying this to just type coercion allows the Zod schema (z.number().int().min(1)) to handle the validation and provide proper error feedback to the user.
Context for Agents
The `setValueAs` function currently swallows validation errors (like non-integers or values less than 1) by returning `undefined`, which `zod` treats as a valid optional value. This means invalid inputs (e.g. "1.5" or "-1") will simply unset the limit rather than showing a validation error.
Simiplifying this to just type coercion allows the Zod schema (`z.number().int().min(1)`) to handle the validation and provide proper error feedback to the user.
File: apps/web/modules/event-types/components/tabs/advanced/FormBuilder.tsx
Line: 729
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configurable max attendees for
multiemailguest fieldAdds a new optional
maxEntriesfield-level setting (primarily for theguestsmultiemailbooking question) that caps how many additional attendee emails can be entered. The limit is surfaced in the form-builder UI, enforced in the booking UI formultiemailfields, and validated server-side during booking response processing, with new i18n strings for helper text and error messages.Key Changes:
• Extend
baseFieldSchemainpackages/prisma/zod-utils.tswith optionalmaxEntries: z.number().int().min(1).optional()for list-type fields likemultiemail.• Update
TextLikeComponentPropsinpackages/app-store/routing-forms/components/react-awesome-query-builder/widgets.tsxto accept an optionalmaxEntries?: number, enabling typed propagation of this config to text-list components.• Wire
field.maxEntriesthroughComponentForFieldinpackages/features/form-builder/FormBuilderField.tsxintotextListcomponents, somultiemailreceives the limit.• Enhance the
multiemailcomponent inpackages/features/form-builder/Components.tsxto computehasReachedLimitbased onmaxEntries, disable add buttons, guardonClickhandlers, and show helper text usingt("max_attendees_allowed", { count: maxEntries }).• Add a
maxEntriesnumeric input in the booking question editor formultiemailguestsfields inapps/web/modules/event-types/components/tabs/advanced/FormBuilder.tsx, withsetValueAscoercion that rejects non-integer or< 1values and UImin={1}.• Introduce server-side enforcement in
packages/features/bookings/lib/getBookingResponsesSchema.tsso that formultiemailfields, ifbookingField.maxEntriesis set andemails.length > maxEntries, a custom Zod issue is added usingm("max_attendees_allowed", { count: bookingField.maxEntries })before duplicate detection.• Add i18n keys
max_attendeesandmax_attendees_allowedtoapps/web/public/static/locales/en/common.jsonfor the configuration label and limit message.Possible Issues:
• The UI limit for
multiemailuseslistValue.length, which counts even empty rows; the server-side limit uses the parsedemails.length(non-empty, valid emails). This means users may hit the UI cap based on total rows rather than number of non-empty addresses, which may or may not match the desired semantics.• The
maxEntriesconfiguration input is currently displayed only whenformFieldType === "multiemail" && fieldName === "guests"; if additionalmultiemailfields are introduced later, they will not have a UI for settingmaxEntriesunless this condition is broadened.• If non-English locales are missing translations for
max_attendeesormax_attendees_allowed, users in those locales may see fallback keys or inconsistent messaging between frontend and backend errors.• Existing data or API clients that bypass the UI and attempt to set
maxEntriesto0, negative, or non-integer values will be rejected by the Zod schema, which could surface as unexpected validation errors if not accounted for.Review Focus
• Confirm
maxEntriesis added only at the schema level where appropriate (baseFieldSchema) and currently consumed only bymultiemail/guests, with no unintended effect on other field types.• Validate the
multiemailUI logic inpackages/features/form-builder/Components.tsx, particularly thathasReachedLimitsemantics (listValue.lengthincluding empty rows) match product expectations vs counting only non-empty emails.• Check the server-side
multiemailbranch inpackages/features/bookings/lib/getBookingResponsesSchema.tsfor correct ordering: required check, parse, special-case emptyguests, thenmaxEntriesenforcement, then duplicate detection, ensuring no behavior regressions.• Review
maxEntriescoercion inapps/web/modules/event-types/components/tabs/advanced/FormBuilder.tsx(setValueAslogic) to ensure it properly rejects"", non-numeric, non-integer, and< 1inputs to stay consistent with the Zod schema.• Ensure that extending
TextLikeComponentPropsand passingmaxEntries={field.maxEntries}inpackages/features/form-builder/FormBuilderField.tsxis type-safe and does not break existing widget consumers.• Verify that new i18n keys are named consistently and reused correctly across frontend helper text and backend validation messages.
Potential Impact
Functionality: Enables event creators to cap the number of additional guests on
multiemailfields (currently exposed in the UI only for theguestsfield). The booking UI prevents adding more inputs oncemaxEntriesis reached, and submissions exceeding the limit are rejected server-side with a translated error. Existing configurations withoutmaxEntrieskeep prior behavior.Performance: Impact is minimal: the client does an extra array
lengthcheck when rendering buttons, and the server adds an O(1)emails.lengthcheck before existing validation steps.Security: Adds an extra guard against excessively large guest lists on
multiemailfields, slightly reducing risk of abuse via oversized payloads, but does not change auth or authorization behavior.Scalability: No significant scalability changes;
maxEntriesis a scalar field on the booking field schema and the additional checks are constant-time relative to existing per-field validation.Affected Areas
• Booking question form-builder configuration (
apps/web/modules/event-types/components/tabs/advanced/FormBuilder.tsx)• Form-builder runtime components and wiring for
multiemail(packages/features/form-builder/Components.tsx,packages/features/form-builder/FormBuilderField.tsx)• Booking responses validation pipeline, specifically
multiemailhandling (packages/features/bookings/lib/getBookingResponsesSchema.ts)• Shared booking field schema / Zod utilities (
packages/prisma/zod-utils.ts)• Routing-forms widget prop typing (
packages/app-store/routing-forms/components/react-awesome-query-builder/widgets.tsx)• English locale strings for helper and error text (
apps/web/public/static/locales/en/common.json)Code Quality Assessment
packages/features/bookings/lib/getBookingResponsesSchema.ts: The new
maxEntriescheck is added in themultiemailbranch directly after successful parsing and before duplicate detection, with an earlycontinueon violation and consistent use ofm("max_attendees_allowed", ...)for translation-aware messages.packages/features/form-builder/Components.tsx: The
multiemailfactory now normalizesvaluevialistValue, derives a clearhasReachedLimitflag, disables buttons, and guardsonClickhandlers, reducing the risk of accidental over-additions; helper copy is centralized via i18n.packages/features/form-builder/FormBuilderField.tsx: Prop plumbing for
maxEntriesintotextListcomponents is minimal and contained; checks againstcomponentConfig.propsTyperemain readable and the new prop is only passed where relevant.packages/app-store/routing-forms/components/react-awesome-query-builder/widgets.tsx: Adding optional
maxEntries?: numbertoTextLikeComponentPropsis backwards-compatible and does not affect existing widgets, as it is purely an additional optional prop.apps/web/modules/event-types/components/tabs/advanced/FormBuilder.tsx: The
maxEntrieseditor control is gated toformFieldType === "multiemail" && fieldName === "guests"and usessetValueAsto align with the schema (reject non-integers and< 1), though the name-based condition should be kept in sync if theguestsfield name ever changes.apps/web/public/static/locales/en/common.json: New keys
max_attendeesandmax_attendees_allowedfollow existing naming and interpolation patterns (e.g.,{{count}}), aiding reuse across UI and validation.packages/prisma/zod-utils.ts: The
baseFieldSchemaextension formaxEntriesis well-documented with clear JSDoc and usesz.number().int().min(1).optional()to match UI constraints, keeping schema-level validation tight.Best Practices
Validation:
• Enforce critical constraints like
maxEntriesin both the client (disabling add actions) and server (getBookingResponsesSchema) to prevent circumvention.• Keep UI coercion (
setValueAsformaxEntries) consistent with schema-level validation (int().min(1)) to avoid mismatched expectations and opaque save failures.Schema Design:
• Add new configuration such as
maxEntriesat the sharedbaseFieldSchemalayer so it can be reused across multiple UIs and consumers.• Document new schema fields with clear comments describing scope (list fields only, e.g.
multiemail) and semantics.Ux:
• Use i18n keys (
max_attendees,max_attendees_allowed) for helper and error text so users see consistent messaging across editor, booking UI, and validation.• Disable and guard interactive controls (e.g., add buttons in
multiemail) instead of relying solely on validation errors after the fact.Testing Needed
• In the event-type editor, set the
guestsbooking question to typemultiemail, configuremaxEntriesto values like1and3, and verify the booking page disables theadd-guests/add-another-guestbuttons exactly when the number of guest inputs reaches the configured limit.• Attempt to submit a booking with more non-empty guest emails than
maxEntries(e.g., via devtools or direct API call) and confirm server-side validation fails with themax_attendees_allowedmessage, correctly scoped to theguestsfield.• Verify that when
maxEntriesis unset,multiemailbehaves as before: no UI limit on the number of rows and no max-attendees error from the backend.• Test
multiemailedge cases: required vs optionalguests, all inputs empty forguests(should still normalize to an empty array without error), mixed valid/empty entries, and duplicate emails, ensuring the new limit check does not prevent existingduplicate_emailvalidation from firing.• Check that invalid
maxEntriesinput in the editor (empty,0, negative, non-integer, non-numeric) is coerced toundefinedand does not break saving, while valid integers>= 1are persisted.• Confirm the new
max_attendeeslabel andmax_attendees_allowedmessages render correctly in English on both the editor and booking UI, and that other locales have acceptable fallback behavior.This summary was automatically generated by @propel-code-bot