-
Notifications
You must be signed in to change notification settings - Fork 0
Add guest management functionality to existing bookings #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: guest-management-base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ import { | |
| Tooltip, | ||
| } from "@calcom/ui"; | ||
|
|
||
| import { AddGuestsDialog } from "@components/dialog/AddGuestsDialog"; | ||
| import { ChargeCardDialog } from "@components/dialog/ChargeCardDialog"; | ||
| import { EditLocationDialog } from "@components/dialog/EditLocationDialog"; | ||
| import { ReassignDialog } from "@components/dialog/ReassignDialog"; | ||
|
|
@@ -189,6 +190,14 @@ function BookingListItem(booking: BookingItemProps) { | |
| }, | ||
| icon: "map-pin" as const, | ||
| }, | ||
| { | ||
| id: "add_members", | ||
| label: t("additional_guests"), | ||
| onClick: () => { | ||
| setIsOpenAddGuestsDialog(true); | ||
| }, | ||
| icon: "user-plus" as const, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Module Boundary / UI Availability Guard (confidence: 84%) The 'Add Guests' action is added unconditionally to the booking actions array. This means it will appear for past, cancelled, and rejected bookings. The intent specification requires that the 'Add Guests' option should only show for upcoming/active bookings. Other actions in this component are conditionally added based on booking status. Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Booking State Guard - UI (confidence: 100%) The 'Add Guests' action is unconditionally added to the booking actions list for all bookings, regardless of booking status. This means cancelled, rejected, and past bookings will show the 'Add Guests' option, which is a UX issue and could lead to data integrity problems if the backend also lacks proper guards. Evidence:
Agent: architecture |
||
| }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 CRITICAL — Conditional Rendering (confidence: 100%) The 'Add Guests' action is unconditionally added to the booking actions array for all bookings, regardless of booking status. This means the option appears for cancelled, rejected, and past bookings. The acceptance criteria explicitly states: 'BookingListItem only shows the Add Guests option for bookings in an appropriate state (e.g., upcoming, confirmed — not cancelled or past)'. Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/booking/BookingListItem.tsx
+++ b/apps/web/components/booking/BookingListItem.tsx
@@ -190,14 +190,16 @@ function BookingListItem(booking: BookingItemProps) {
},
icon: "map-pin" as const,
},
- {
- id: "add_members",
- label: t("additional_guests"),
- onClick: () => {
- setIsOpenAddGuestsDialog(true);
- },
- icon: "user-plus" as const,
- },
];
+ if (
+ booking.status !== BookingStatus.CANCELLED &&
+ booking.status !== BookingStatus.REJECTED &&
+ booking.status !== BookingStatus.AWAITING_HOST &&
+ booking.status !== BookingStatus.PENDING &&
+ !isPast
+ ) {
+ editBookingActions.push({
+ id: "add_members",
+ label: t("additional_guests"),
+ onClick: () => {
+ setIsOpenAddGuestsDialog(true);
+ },
+ icon: "user-plus" as const,
+ });
+ }
+
if (booking.eventType.schedulingType === SchedulingType.ROUND_ROBIN) {🤖 Grapple PR auto-fix • critical • Review this diff before applying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — UI - Missing Conditional Rendering (confidence: 100%) The 'Add Guests' action is unconditionally added to the actions array for all bookings. It should only appear for active/upcoming bookings where the user has permission, similar to how other actions are conditionally rendered. Cancelled, rejected, and past bookings should not show this option. Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — UI Missing Guard on Action Visibility (confidence: 100%) The 'Add Guests' action is unconditionally added to the actions array for all bookings, regardless of booking status (cancelled, rejected, past). The acceptance criteria states the action should 'only be surfaced for bookings where it is appropriate'. Other actions in this file are conditionally shown based on booking state, but this one is always present. Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/booking/BookingListItem.tsx
+++ b/apps/web/components/booking/BookingListItem.tsx
@@ -190,13 +190,17 @@ function BookingListItem(booking: BookingItemProps) {
},
icon: "map-pin" as const,
},
- {
- id: "add_members",
- label: t("additional_guests"),
- onClick: () => {
- setIsOpenAddGuestsDialog(true);
- },
- icon: "user-plus" as const,
- },
];
+ const isPast = booking.startTime < new Date();
+ const isActiveBooking =
+ booking.status !== BookingStatus.CANCELLED &&
+ booking.status !== BookingStatus.REJECTED &&
+ !isPast;
+
+ if (isActiveBooking) {
+ editBookingActions.push({
+ id: "add_members",
+ label: t("additional_guests"),
+ onClick: () => {
+ setIsOpenAddGuestsDialog(true);
+ },
+ icon: "user-plus" as const,
+ });
+ }
+
if (booking.eventType.schedulingType === SchedulingType.ROUND_ROBIN) {🤖 Grapple PR auto-fix • major • Review this diff before applying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/booking/BookingListItem.tsx
+++ b/apps/web/components/booking/BookingListItem.tsx
@@ -189,13 +189,15 @@ function BookingListItem(booking: BookingItemProps) {
},
icon: "map-pin" as const,
},
- {
- id: "add_members",
- label: t("additional_guests"),
- onClick: () => {
- setIsOpenAddGuestsDialog(true);
- },
- icon: "user-plus" as const,
- },
];
+ if (booking.listingStatus === "upcoming") {
+ editBookingActions.push({
+ id: "add_members",
+ label: t("additional_guests"),
+ onClick: () => {
+ setIsOpenAddGuestsDialog(true);
+ },
+ icon: "user-plus" as const,
+ });
+ }
+
if (booking.eventType.schedulingType === SchedulingType.ROUND_ROBIN) {🤖 Grapple PR auto-fix • major • Review this diff before applying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Missing Action Visibility Control (confidence: 93%) The 'Add Guests' action is added to the Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/booking/BookingListItem.tsx
+++ b/apps/web/components/booking/BookingListItem.tsx
@@ -183,19 +183,25 @@ function BookingListItem(booking: BookingItemProps) {
},
},
- {
- id: "change_location",
- label: t("edit_location"),
- onClick: () => {
- setIsOpenLocationDialog(true);
- },
- icon: "map-pin" as const,
- },
- {
- id: "add_members",
- label: t("additional_guests"),
- onClick: () => {
- setIsOpenAddGuestsDialog(true);
- },
- icon: "user-plus" as const,
- },
];
+
+ const isBookingModifiable =
+ booking.status === BookingStatus.ACCEPTED || booking.status === BookingStatus.PENDING;
+
+ if (isBookingModifiable) {
+ editBookingActions.push({
+ id: "change_location",
+ label: t("edit_location"),
+ onClick: () => {
+ setIsOpenLocationDialog(true);
+ },
+ icon: "map-pin" as const,
+ });
+ editBookingActions.push({
+ id: "add_members",
+ label: t("additional_guests"),
+ onClick: () => {
+ setIsOpenAddGuestsDialog(true);
+ },
+ icon: "user-plus" as const,
+ });
+ }
if (booking.eventType.schedulingType === SchedulingType.ROUND_ROBIN) {🤖 Grapple PR auto-fix • minor • Review this diff before applying |
||
| ]; | ||
|
|
||
| if (booking.eventType.schedulingType === SchedulingType.ROUND_ROBIN) { | ||
|
|
@@ -256,6 +265,7 @@ function BookingListItem(booking: BookingItemProps) { | |
| const [isOpenRescheduleDialog, setIsOpenRescheduleDialog] = useState(false); | ||
| const [isOpenReassignDialog, setIsOpenReassignDialog] = useState(false); | ||
| const [isOpenSetLocationDialog, setIsOpenLocationDialog] = useState(false); | ||
| const [isOpenAddGuestsDialog, setIsOpenAddGuestsDialog] = useState(false); | ||
| const setLocationMutation = trpc.viewer.bookings.editLocation.useMutation({ | ||
| onSuccess: () => { | ||
| showToast(t("location_updated"), "success"); | ||
|
|
@@ -344,6 +354,11 @@ function BookingListItem(booking: BookingItemProps) { | |
| setShowLocationModal={setIsOpenLocationDialog} | ||
| teamId={booking.eventType?.team?.id} | ||
| /> | ||
| <AddGuestsDialog | ||
| isOpenDialog={isOpenAddGuestsDialog} | ||
| setIsOpenDialog={setIsOpenAddGuestsDialog} | ||
| bookingId={booking.id} | ||
| /> | ||
| {booking.paid && booking.payment[0] && ( | ||
| <ChargeCardDialog | ||
| isOpenDialog={chargeCardDialogIsOpen} | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,107 @@ | ||||||||||||||||
| import type { Dispatch, SetStateAction } from "react"; | ||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||
| import { z } from "zod"; | ||||||||||||||||
|
|
||||||||||||||||
| import { useLocale } from "@calcom/lib/hooks/useLocale"; | ||||||||||||||||
| import { trpc } from "@calcom/trpc/react"; | ||||||||||||||||
| import { | ||||||||||||||||
| Button, | ||||||||||||||||
| Dialog, | ||||||||||||||||
| DialogContent, | ||||||||||||||||
| DialogFooter, | ||||||||||||||||
| DialogHeader, | ||||||||||||||||
| MultiEmail, | ||||||||||||||||
| Icon, | ||||||||||||||||
| showToast, | ||||||||||||||||
| } from "@calcom/ui"; | ||||||||||||||||
|
|
||||||||||||||||
| interface IAddGuestsDialog { | ||||||||||||||||
| isOpenDialog: boolean; | ||||||||||||||||
| setIsOpenDialog: Dispatch<SetStateAction<boolean>>; | ||||||||||||||||
| bookingId: number; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| export const AddGuestsDialog = (props: IAddGuestsDialog) => { | ||||||||||||||||
| const { t } = useLocale(); | ||||||||||||||||
| const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { | ||||||||||||||||
| const uniqueEmails = new Set(emails); | ||||||||||||||||
| return uniqueEmails.size === emails.length; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Code organization (confidence: 91%) Validation schema is defined inside component render scope, causing it to be recreated on every render. Should be defined outside component. Evidence:
Agent: style |
||||||||||||||||
| }); | ||||||||||||||||
| const { isOpenDialog, setIsOpenDialog, bookingId } = props; | ||||||||||||||||
| const utils = trpc.useUtils(); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-side Validation Redundancy (confidence: 80%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Pattern Violation (confidence: 83%) The Zod schema Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/dialog/AddGuestsDialog.tsx
+++ b/apps/web/components/dialog/AddGuestsDialog.tsx
@@ -1,6 +1,7 @@
import type { Dispatch, SetStateAction } from "react";
import { useState } from "react";
import { z } from "zod";
+
import { useLocale } from "@calcom/lib/hooks/useLocale";
import { trpc } from "@calcom/trpc/react";
import {
@@ -17,15 +18,15 @@ import {
showToast,
} from "@calcom/ui";
+// Defined at module level to avoid re-instantiation on every render.
+// Validates that all guest emails are valid and unique.
+const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => {
+ const uniqueEmails = new Set(emails);
+ return uniqueEmails.size === emails.length;
+});
+
interface IAddGuestsDialog {
isOpenDialog: boolean;
setIsOpenDialog: Dispatch<SetStateAction<boolean>>;
bookingId: number;
}
export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const { t } = useLocale();
- const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => {
- const uniqueEmails = new Set(emails);
- return uniqueEmails.size === emails.length;
- });
const { isOpenDialog, setIsOpenDialog, bookingId } = props;🤖 Grapple PR auto-fix • minor • Review this diff before applying |
||||||||||||||||
| const [multiEmailValue, setMultiEmailValue] = useState<string[]>([""]); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-side Validation Mismatch (confidence: 100%) The client-side Zod schema ( Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/dialog/AddGuestsDialog.tsx
+++ b/apps/web/components/dialog/AddGuestsDialog.tsx
@@ -1,6 +1,5 @@
import type { Dispatch, SetStateAction } from "react";
import { useState } from "react";
-import { z } from "zod";
import { useLocale } from "@calcom/lib/hooks/useLocale";
import { trpc } from "@calcom/trpc/react";
@@ -14,6 +13,13 @@ import {
showToast,
} from "@calcom/ui";
+import { z } from "zod";
+
+// Defined at module scope so it is not recreated on every render.
+// Adds client-side uniqueness validation for better UX before the network call.
+const ZAddGuestsClientSchema = z
+ .array(z.string().email())
+ .min(1)
+ .refine((emails) => new Set(emails).size === emails.length, {
+ message: "emails_must_be_unique_valid",
+ });
+
interface IAddGuestsDialog {
isOpenDialog: boolean;
setIsOpenDialog: Dispatch<SetStateAction<boolean>>;
@@ -23,11 +29,6 @@ interface IAddGuestsDialog {
export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const { t } = useLocale();
- const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => {
- const uniqueEmails = new Set(emails);
- return uniqueEmails.size === emails.length;
- });
- const { isOpenDialog, setIsOpenDialog, bookingId } = props;
+ const { isOpenDialog, setIsOpenDialog, bookingId } = props;
const utils = trpc.useUtils();
const [multiEmailValue, setMultiEmailValue] = useState<string[]>([""]);
const [isInvalidEmail, setIsInvalidEmail] = useState(false);
@@ -48,10 +49,10 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const handleAdd = () => {
- if (multiEmailValue.length === 0) {
- return;
- }
- const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+ // ZAddGuestsClientSchema already enforces min(1), so the explicit
+ // length-0 guard is covered by safeParse below.
+ const validationResult = ZAddGuestsClientSchema.safeParse(
+ multiEmailValue.filter((email) => email.trim() !== "")
+ );
if (validationResult.success) {
addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
} else {🤖 Grapple PR auto-fix • minor • Review this diff before applying |
||||||||||||||||
| const [isInvalidEmail, setIsInvalidEmail] = useState(false); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-side Validation (confidence: 80%) The MultiEmail state is initialized with Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Client-Side Validation Bypass (confidence: 100%) The multiEmailValue state is initialized with Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/dialog/AddGuestsDialog.tsx
+++ b/apps/web/components/dialog/AddGuestsDialog.tsx
@@ -35,7 +35,7 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({
onSuccess: async () => {
showToast(t("guests_added"), "success");
setIsOpenDialog(false);
- setMultiEmailValue([""]);
+ setMultiEmailValue([]);
utils.viewer.bookings.invalidate();
},
onError: (err) => {
@@ -46,8 +46,10 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const handleAdd = () => {
- if (multiEmailValue.length === 0) {
+ // Filter out empty string entries before validation — the MultiEmail component
+ // initializes with [""] so a plain length check will never be falsy on first render.
+ const filteredEmails = multiEmailValue.filter((email) => email.trim() !== "");
+ if (filteredEmails.length === 0) {
return;
}
- const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+ const validationResult = ZAddGuestsInputSchema.safeParse(filteredEmails);
if (validationResult.success) {
- addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
+ addGuestsMutation.mutate({ bookingId, guests: filteredEmails });
} else {
setIsInvalidEmail(true);
}
@@ -78,7 +80,7 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
<Button
onClick={() => {
- setMultiEmailValue([""]);
+ setMultiEmailValue([]);
setIsInvalidEmail(false);
setIsOpenDialog(false);
}}🤖 Grapple PR auto-fix • major • Review this diff before applying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-Side Validation (confidence: 89%) The multiEmailValue is initialized with Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 89% |
||||||||||||||||
|
|
||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Error State Management (confidence: 93%) The Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 93% |
||||||||||||||||
| const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({ | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Email Validation (confidence: 100%) The initial state of Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • major • confidence: 100% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Documentation (confidence: 88%) Missing error message i18n key 'unable_to_add_guests' fallback is problematic. The error message construction attempts to translate error.message but should validate it exists. Evidence:
Agent: style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-Side Validation Mismatch (confidence: 93%) The initial state of Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡
--- a/apps/web/components/dialog/AddGuestsDialog.tsx
+++ b/apps/web/components/dialog/AddGuestsDialog.tsx
@@ -49,9 +49,12 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
const handleAdd = () => {
- if (multiEmailValue.length === 0) {
+ // Filter out empty strings before validation; initial state is [""] so
+ // length === 0 check never triggers without this normalization.
+ const nonEmptyEmails = multiEmailValue.filter((email) => email.trim() !== "");
+ if (nonEmptyEmails.length === 0) {
return;
}
- const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+ const validationResult = ZAddGuestsInputSchema.safeParse(nonEmptyEmails);
if (validationResult.success) {
- addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
+ addGuestsMutation.mutate({ bookingId, guests: nonEmptyEmails });
} else {
setIsInvalidEmail(true);
@@ -90,7 +93,10 @@ export const AddGuestsDialog = (props: IAddGuestsDialog) => {
<Button data-testid="add_members" loading={addGuestsMutation.isPending} onClick={handleAdd}>
+ <Button
+ data-testid="add_members"
+ loading={addGuestsMutation.isPending}
+ onClick={handleAdd}
+ disabled={multiEmailValue.filter((email) => email.trim() !== "").length === 0}>
{t("add")}
</Button>🤖 Grapple PR auto-fix • minor • Review this diff before applying |
||||||||||||||||
| onSuccess: async () => { | ||||||||||||||||
| showToast(t("guests_added"), "success"); | ||||||||||||||||
| setIsOpenDialog(false); | ||||||||||||||||
| setMultiEmailValue([""]); | ||||||||||||||||
| utils.viewer.bookings.invalidate(); | ||||||||||||||||
| }, | ||||||||||||||||
| onError: (err) => { | ||||||||||||||||
| const message = `${err.data?.code}: ${t(err.message)}`; | ||||||||||||||||
| showToast(message || t("unable_to_add_guests"), "error"); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Error messages (confidence: 81%) Error message construction concatenates err.data?.code and err.message without null checks. If either is undefined, the message will be malformed (e.g., 'undefined: undefined'). Evidence:
Agent: style |
||||||||||||||||
| }, | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Data Exposure (confidence: 76%) tRPC error messages from the server are passed directly to i18n translation and displayed to the user. The Evidence:
Agent: security |
||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| const handleAdd = () => { | ||||||||||||||||
| if (multiEmailValue.length === 0) { | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
| const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue); | ||||||||||||||||
| if (validationResult.success) { | ||||||||||||||||
| addGuestsMutation.mutate({ bookingId, guests: multiEmailValue }); | ||||||||||||||||
| } else { | ||||||||||||||||
| setIsInvalidEmail(true); | ||||||||||||||||
| } | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| return ( | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Client-side Validation (confidence: 98%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 98% |
||||||||||||||||
| <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}> | ||||||||||||||||
| <DialogContent enableOverflow> | ||||||||||||||||
| <div className="flex flex-row space-x-3"> | ||||||||||||||||
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | ||||||||||||||||
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div className="w-full pt-1"> | ||||||||||||||||
| <DialogHeader title={t("additional_guests")} /> | ||||||||||||||||
| <MultiEmail | ||||||||||||||||
| label={t("add_emails")} | ||||||||||||||||
| value={multiEmailValue} | ||||||||||||||||
| readOnly={false} | ||||||||||||||||
| setValue={setMultiEmailValue} | ||||||||||||||||
| /> | ||||||||||||||||
|
|
||||||||||||||||
| {isInvalidEmail && ( | ||||||||||||||||
| <div className="my-4 flex text-sm text-red-700"> | ||||||||||||||||
| <div className="flex-shrink-0"> | ||||||||||||||||
| <Icon name="triangle-alert" className="h-5 w-5" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div className="ml-3"> | ||||||||||||||||
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | ||||||||||||||||
| </div> | ||||||||||||||||
| </div> | ||||||||||||||||
| )} | ||||||||||||||||
|
|
||||||||||||||||
| <DialogFooter> | ||||||||||||||||
| <Button | ||||||||||||||||
| onClick={() => { | ||||||||||||||||
| setMultiEmailValue([""]); | ||||||||||||||||
| setIsInvalidEmail(false); | ||||||||||||||||
| setIsOpenDialog(false); | ||||||||||||||||
| }} | ||||||||||||||||
| type="button" | ||||||||||||||||
| color="secondary"> | ||||||||||||||||
| {t("cancel")} | ||||||||||||||||
| </Button> | ||||||||||||||||
| <Button data-testid="add_members" loading={addGuestsMutation.isPending} onClick={handleAdd}> | ||||||||||||||||
| {t("add")} | ||||||||||||||||
| </Button> | ||||||||||||||||
| </DialogFooter> | ||||||||||||||||
| </div> | ||||||||||||||||
| </div> | ||||||||||||||||
| </DialogContent> | ||||||||||||||||
| </Dialog> | ||||||||||||||||
| ); | ||||||||||||||||
| }; | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ import type { EmailVerifyLink } from "./templates/account-verify-email"; | |||||||||||||
| import AccountVerifyEmail from "./templates/account-verify-email"; | ||||||||||||||
| import type { OrganizationNotification } from "./templates/admin-organization-notification"; | ||||||||||||||
| import AdminOrganizationNotification from "./templates/admin-organization-notification"; | ||||||||||||||
| import AttendeeAddGuestsEmail from "./templates/attendee-add-guests-email"; | ||||||||||||||
| import AttendeeAwaitingPaymentEmail from "./templates/attendee-awaiting-payment-email"; | ||||||||||||||
| import AttendeeCancelledEmail from "./templates/attendee-cancelled-email"; | ||||||||||||||
| import AttendeeCancelledSeatEmail from "./templates/attendee-cancelled-seat-email"; | ||||||||||||||
|
|
@@ -48,6 +49,7 @@ import type { OrganizationCreation } from "./templates/organization-creation-ema | |||||||||||||
| import OrganizationCreationEmail from "./templates/organization-creation-email"; | ||||||||||||||
| import type { OrganizationEmailVerify } from "./templates/organization-email-verification"; | ||||||||||||||
| import OrganizationEmailVerification from "./templates/organization-email-verification"; | ||||||||||||||
| import OrganizerAddGuestsEmail from "./templates/organizer-add-guests-email"; | ||||||||||||||
| import OrganizerAttendeeCancelledSeatEmail from "./templates/organizer-attendee-cancelled-seat-email"; | ||||||||||||||
| import OrganizerCancelledEmail from "./templates/organizer-cancelled-email"; | ||||||||||||||
| import OrganizerDailyVideoDownloadRecordingEmail from "./templates/organizer-daily-video-download-recording-email"; | ||||||||||||||
|
|
@@ -520,6 +522,32 @@ export const sendLocationChangeEmails = async ( | |||||||||||||
|
|
||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Dependency - Unused Parameter (confidence: 98%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 98% |
||||||||||||||
| await Promise.all(emailsToSend); | ||||||||||||||
| }; | ||||||||||||||
| export const sendAddGuestsEmails = async (calEvent: CalendarEvent, newGuests: string[]) => { | ||||||||||||||
| const calendarEvent = formatCalEvent(calEvent); | ||||||||||||||
|
|
||||||||||||||
| const emailsToSend: Promise<unknown>[] = []; | ||||||||||||||
| emailsToSend.push(sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent }))); | ||||||||||||||
|
|
||||||||||||||
| if (calendarEvent.team?.members) { | ||||||||||||||
| for (const teamMember of calendarEvent.team.members) { | ||||||||||||||
| emailsToSend.push( | ||||||||||||||
| sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent, teamMember })) | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| emailsToSend.push( | ||||||||||||||
| ...calendarEvent.attendees.map((attendee) => { | ||||||||||||||
| if (newGuests.includes(attendee.email)) { | ||||||||||||||
| return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee)); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Email Template Mismatch (confidence: 100%) For new guests, the code sends Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 100% |
||||||||||||||
| } else { | ||||||||||||||
| return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee)); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Email Logic Error (confidence: 100%) The sendAddGuestsEmails function compares Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • major • confidence: 100% |
||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| await Promise.all(emailsToSend); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Code organization (confidence: 85%) The sendAddGuestsEmails function has inconsistent email dispatch logic. It sends OrganizerAddGuestsEmail to the organizer and team members, but then sends either AttendeeScheduledEmail or AttendeeAddGuestsEmail to attendees based on whether they are in the newGuests list. This logic appears inverted—new guests should receive AttendeeScheduledEmail, while existing attendees should receive AttendeeAddGuestsEmail. Evidence:
Agent: style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Code organization (confidence: 72%) In sendAddGuestsEmails function, the logic for sending AttendeeAddGuestsEmail vs AttendeeScheduledEmail is based on whether the attendee is in the newGuests array. However, newly added guests should always receive AttendeeScheduledEmail (initial invitation), not AttendeeAddGuestsEmail (notification of additional guests). The current logic is inverted. Evidence:
Agent: style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 72% |
||||||||||||||
| }; | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Data Flow (confidence: 100%) The Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Email Logic (confidence: 100%) The Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 100% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 100% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Code organization (confidence: 100%) The sendAddGuestsEmails function sends AttendeeScheduledEmail to newly added guests instead of AttendeeAddGuestsEmail. This contradicts the expected behavior and email template purpose. Evidence:
Agent: style |
||||||||||||||
| export const sendFeedbackEmail = async (feedback: Feedback) => { | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Data Flow - Email Deduplication Mismatch (confidence: 100%) The Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Cross-module Consistency (confidence: 87%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Email Dispatch Logic (confidence: 100%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • major • confidence: 100% |
||||||||||||||
| await sendEmail(() => new FeedbackEmail(feedback)); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Email Dispatch Logic (confidence: 100%) The Evidence:
Agent: architecture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 100% |
||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { AttendeeScheduledEmail } from "./AttendeeScheduledEmail"; | ||
|
|
||
| export const AttendeeAddGuestsEmail = (props: React.ComponentProps<typeof AttendeeScheduledEmail>) => ( | ||
| <AttendeeScheduledEmail | ||
| title="new_guests_added" | ||
| headerType="calendarCircle" | ||
| subject="guests_added_event_type_subject" | ||
| {...props} | ||
| /> | ||
| ); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Unused Import/Parameter (confidence: 84%) The React component Evidence:
Agent: architecture |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { OrganizerScheduledEmail } from "./OrganizerScheduledEmail"; | ||
|
|
||
| export const OrganizerAddGuestsEmail = (props: React.ComponentProps<typeof OrganizerScheduledEmail>) => ( | ||
| <OrganizerScheduledEmail | ||
| title="new_guests_added" | ||
| headerType="calendarCircle" | ||
| subject="guests_added_event_type_subject" | ||
| callToAction={null} | ||
| {...props} | ||
| /> | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { renderEmail } from "../"; | ||
| import generateIcsString from "../lib/generateIcsString"; | ||
| import AttendeeScheduledEmail from "./attendee-scheduled-email"; | ||
|
|
||
| export default class AttendeeAddGuestsEmail extends AttendeeScheduledEmail { | ||
| protected async getNodeMailerPayload(): Promise<Record<string, unknown>> { | ||
| return { | ||
| icalEvent: { | ||
| filename: "event.ics", | ||
| content: generateIcsString({ | ||
| event: this.calEvent, | ||
| title: this.t("new_guests_added"), | ||
| subtitle: this.t("emailed_you_and_any_other_attendees"), | ||
| role: "attendee", | ||
| status: "CONFIRMED", | ||
| }), | ||
| method: "REQUEST", | ||
| }, | ||
| to: `${this.attendee.name} <${this.attendee.email}>`, | ||
| from: `${this.calEvent.organizer.name} <${this.getMailerOptions().from}>`, | ||
| replyTo: this.calEvent.organizer.email, | ||
| subject: `${this.t("guests_added_event_type_subject", { | ||
| eventType: this.calEvent.type, | ||
| name: this.calEvent.team?.name || this.calEvent.organizer.name, | ||
| date: this.getFormattedDate(), | ||
| })}`, | ||
| html: await renderEmail("AttendeeAddGuestsEmail", { | ||
| calEvent: this.calEvent, | ||
| attendee: this.attendee, | ||
| }), | ||
| text: this.getTextBody("new_guests_added"), | ||
| }; | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Code organization (confidence: 76%) New email template class duplicates node-mailer payload logic from parent AttendeeScheduledEmail. Only subtitle changes (new_guests_added vs emailed_you_and_any_other_attendees). Consider if this warrants a full class or if a simpler composition pattern would reduce duplication. Evidence:
Agent: style |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||
| import { APP_NAME } from "@calcom/lib/constants"; | ||||||
|
|
||||||
| import { renderEmail } from "../"; | ||||||
| import generateIcsString from "../lib/generateIcsString"; | ||||||
| import OrganizerScheduledEmail from "./organizer-scheduled-email"; | ||||||
|
|
||||||
| export default class OrganizerAddGuestsEmail extends OrganizerScheduledEmail { | ||||||
| protected async getNodeMailerPayload(): Promise<Record<string, unknown>> { | ||||||
| const toAddresses = [this.teamMember?.email || this.calEvent.organizer.email]; | ||||||
|
|
||||||
| return { | ||||||
| icalEvent: { | ||||||
| filename: "event.ics", | ||||||
| content: generateIcsString({ | ||||||
| event: this.calEvent, | ||||||
| title: this.t("new_guests_added"), | ||||||
| subtitle: this.t("emailed_you_and_any_other_attendees"), | ||||||
| role: "organizer", | ||||||
| status: "CONFIRMED", | ||||||
| }), | ||||||
| method: "REQUEST", | ||||||
| }, | ||||||
| from: `${APP_NAME} <${this.getMailerOptions().from}>`, | ||||||
| to: toAddresses.join(","), | ||||||
| replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)], | ||||||
| subject: `${this.t("guests_added_event_type_subject", { | ||||||
| eventType: this.calEvent.type, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MINOR — Information Disclosure — Attendee PII in ReplyTo Header (confidence: 93%) The organizer notification email sets Evidence:
Agent: security There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • minor • confidence: 93% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Defensive Coding (confidence: 81%) The subject line references Evidence:
Agent: logic |
||||||
| name: this.calEvent.attendees[0].name, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 MAJOR — Missing Null Safety (confidence: 100%) The organizer email subject accesses Evidence:
Agent: logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅
Suggested change
🤖 Grapple PR auto-fix • major • confidence: 100% |
||||||
| date: this.getFormattedDate(), | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 INFO — Code patterns (confidence: 93%) Email subject name field uses first attendee instead of organizer name. Subject should consistently reference who the event is with. Evidence:
Agent: style |
||||||
| })}`, | ||||||
| html: await renderEmail("OrganizerAddGuestsEmail", { | ||||||
| attendee: this.calEvent.organizer, | ||||||
| calEvent: this.calEvent, | ||||||
| }), | ||||||
| text: this.getTextBody("new_guests_added"), | ||||||
| }; | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 CRITICAL — Business Logic - Missing UI Filtering (confidence: 92%)
The 'Add Guests' action is added unconditionally to the booking action list without checking if the booking is upcoming/active. The acceptance criteria explicitly require: 'The BookingListItem should only show the Add Guests option for bookings that are upcoming/active (not cancelled or past).' The action is added in the same list as other always-visible actions, with no conditional guard.
Evidence:
Agent: logic