feat: Add custom reason for Out of Office#27950
feat: Add custom reason for Out of Office#27950Shrey-Sutariya wants to merge 6 commits intocalcom:mainfrom
Conversation
|
|
github-actions
bot
added
Low priority
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 33 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonDelete.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonDelete.handler.ts:19">
P2: Non-atomic check-then-delete allows a race where a reason becomes referenced after the check but before deletion, bypassing the “cannot delete if in use” rule. Because the FK uses onDelete: SetNull, the DB won’t prevent deletion.</violation>
</file>
<file name="packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonCreate.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonCreate.handler.ts:17">
P2: Whitespace-only emoji/reason can pass validation and be persisted as empty strings because trimming happens after min-length validation.</violation>
</file>
<file name="apps/web/modules/bookings/components/OutOfOfficeInSlots.tsx">
<violation number="1" location="apps/web/modules/bookings/components/OutOfOfficeInSlots.tsx:42">
P2: OutOfOfficeInSlots now unconditionally calls an authed viewer.ooo.outOfOfficeReasonList query. The router defines this procedure as authedProcedure, so anonymous booking/embed users will receive UNAUTHORIZED errors, which can break or degrade public booking UI. Consider using a public endpoint or gating the query on auth.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| throw new TRPCError({ code: "NOT_FOUND", message: "custom_reason_not_found" }); | ||
| } | ||
|
|
||
| const entryUsingReason = await prisma.outOfOfficeEntry.findFirst({ |
There was a problem hiding this comment.
P2: Non-atomic check-then-delete allows a race where a reason becomes referenced after the check but before deletion, bypassing the “cannot delete if in use” rule. Because the FK uses onDelete: SetNull, the DB won’t prevent deletion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonDelete.handler.ts, line 19:
<comment>Non-atomic check-then-delete allows a race where a reason becomes referenced after the check but before deletion, bypassing the “cannot delete if in use” rule. Because the FK uses onDelete: SetNull, the DB won’t prevent deletion.</comment>
<file context>
@@ -0,0 +1,31 @@
+ throw new TRPCError({ code: "NOT_FOUND", message: "custom_reason_not_found" });
+ }
+
+ const entryUsingReason = await prisma.outOfOfficeEntry.findFirst({
+ where: { reasonId: input.id },
+ });
</file context>
| data: { | ||
| userId: ctx.user.id, | ||
| emoji: input.emoji.trim(), | ||
| reason: input.reason.trim(), |
There was a problem hiding this comment.
P2: Whitespace-only emoji/reason can pass validation and be persisted as empty strings because trimming happens after min-length validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/server/routers/viewer/ooo/outOfOfficeCustomReasonCreate.handler.ts, line 17:
<comment>Whitespace-only emoji/reason can pass validation and be persisted as empty strings because trimming happens after min-length validation.</comment>
<file context>
@@ -0,0 +1,23 @@
+ data: {
+ userId: ctx.user.id,
+ emoji: input.emoji.trim(),
+ reason: input.reason.trim(),
+ enabled: true,
+ },
</file context>
| const router = useRouter(); | ||
|
|
||
| // Check if this is a holiday (no fromUser but has reason) | ||
| const { data: outOfOfficeReasonList } = trpc.viewer.ooo.outOfOfficeReasonList.useQuery(undefined, { |
There was a problem hiding this comment.
P2: OutOfOfficeInSlots now unconditionally calls an authed viewer.ooo.outOfOfficeReasonList query. The router defines this procedure as authedProcedure, so anonymous booking/embed users will receive UNAUTHORIZED errors, which can break or degrade public booking UI. Consider using a public endpoint or gating the query on auth.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/bookings/components/OutOfOfficeInSlots.tsx, line 42:
<comment>OutOfOfficeInSlots now unconditionally calls an authed viewer.ooo.outOfOfficeReasonList query. The router defines this procedure as authedProcedure, so anonymous booking/embed users will receive UNAUTHORIZED errors, which can break or degrade public booking UI. Consider using a public endpoint or gating the query on auth.</comment>
<file context>
@@ -26,17 +28,30 @@ export const OutOfOfficeInSlots = (props: IOutOfOfficeInSlotsProps) => {
const router = useRouter();
- // Check if this is a holiday (no fromUser but has reason)
+ const { data: outOfOfficeReasonList } = trpc.viewer.ooo.outOfOfficeReasonList.useQuery(undefined, {
+ retry: false,
+ });
</file context>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export const outOfOfficeReasonList = async (_opts?: unknown) => { | ||
| const outOfOfficeReasons = await prisma.outOfOfficeReason.findMany({ | ||
| where: { | ||
| enabled: true, |
There was a problem hiding this comment.
🔴 outOfOfficeReasonList leaks all users' custom reasons to every authenticated user
The outOfOfficeReasonList handler returns ALL enabled OutOfOfficeReason rows without filtering by the current user's ID. Since custom reasons now have a non-null userId, every authenticated user sees every other user's custom reasons in their OOO dropdown and reason display.
Root Cause and Impact
The handler at packages/trpc/server/routers/viewer/ooo/outOfOfficeReasons.handler.ts:3-11 queries:
prisma.outOfOfficeReason.findMany({ where: { enabled: true } })This has no userId filter, so it returns system reasons (userId=null) and custom reasons from all users. The correct behavior would be to filter: OR: [{ userId: null }, { userId: ctx.user.id }].
Impact:
- Privacy leak: User A's custom OOO reasons (e.g., "Therapy appointment", "Job interview") are visible to User B.
- UX pollution: Every user's dropdown is cluttered with other users' custom reasons.
- The same unfiltered list is used in
OutOfOfficeInSlots.tsx:42andOutOfOfficeMonthViewDetails.tsx:106for label resolution, so custom reasons from other users will show labels, but this is a minor side-effect compared to the dropdown leak.
(Refers to lines 3-11)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const { data: outOfOfficeReasonList } = trpc.viewer.ooo.outOfOfficeReasonList.useQuery(undefined, { | ||
| retry: false, | ||
| }); |
There was a problem hiding this comment.
🔴 Authenticated tRPC procedure called from public booker page breaks for unauthenticated visitors
Both OutOfOfficeInSlots and OutOfOfficeMonthViewDetails call trpc.viewer.ooo.outOfOfficeReasonList.useQuery(), which is an authedProcedure (see _router.tsx:24). These components are rendered on the public booking page via the Booker component, which is accessible to unauthenticated visitors.
Detailed Explanation
In OutOfOfficeInSlots.tsx:42:
const { data: outOfOfficeReasonList } = trpc.viewer.ooo.outOfOfficeReasonList.useQuery(undefined, {
retry: false,
});And in OutOfOfficeMonthViewDetails.tsx:106:
const { data: outOfOfficeReasonList } = trpc.viewer.ooo.outOfOfficeReasonList.useQuery(undefined, {
retry: false,
});When an unauthenticated booker visits someone's booking page and that person is OOO:
- The component renders and fires an authenticated API request
- The request returns a 401/UNAUTHORIZED error
outOfOfficeReasonListisundefined, sogetReasonLabelfalls back to returning raw reason keys (e.g.,"ooo_reasons_vacation"instead of the translated"Vacation")- Console/network errors appear for every public visitor
Impact: Degraded reason labels on the public booker (raw translation keys displayed) and unnecessary failed API calls for every unauthenticated visitor. For OutOfOfficeMonthViewDetails, this is especially visible since it shows the OOO reason prominently in the month view sidebar.
Prompt for agents
The trpc.viewer.ooo.outOfOfficeReasonList.useQuery() calls in OutOfOfficeInSlots.tsx (line 42) and OutOfOfficeMonthViewDetails.tsx (line 106) use an authedProcedure but are rendered on the public booker page. Fix this by either: (1) creating a new public (unauthenticated) tRPC procedure that returns only system reason labels needed for display, or (2) removing the tRPC calls entirely and instead passing reason labels through the slot data from the server side (the slot already carries the reason string key, and the server could resolve it to a display label before sending). The second approach is more robust and avoids an extra client-side API call.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const entryUsingReason = await prisma.outOfOfficeEntry.findFirst({ | ||
| where: { reasonId: input.id }, | ||
| }); |
There was a problem hiding this comment.
🔴 outOfOfficeCustomReasonDelete checks in-use across ALL users, not scoped to owner
The delete handler checks if a custom reason is in use by querying outOfOfficeEntry.findFirst({ where: { reasonId: input.id } }) without scoping to the current user. This means if User A created a custom reason and User B somehow used it (or a reason ID collision scenario), User A cannot delete their own reason.
Root Cause
In outOfOfficeCustomReasonDelete.handler.ts:19-21:
const entryUsingReason = await prisma.outOfOfficeEntry.findFirst({
where: { reasonId: input.id },
});This queries ALL OOO entries, not just those belonging to the current user. Combined with BUG-0001 (where all users see all custom reasons), if User B selects User A's custom reason for their OOO entry, User A can never delete their own custom reason.
The ownership check at line 12-13 correctly validates that only the reason creator can delete it, but the in-use check should also be scoped to entries owned by the same user, or at minimum clearly documented as intentional cross-user protection.
Impact: Users may be unable to delete their own custom reasons due to other users' OOO entries referencing them (which is possible due to BUG-0001).
Was this helpful? React with 👍 or 👎 to provide feedback.
| {notes && showNotePublicly && ( | ||
| <p className="text-subtle mt-2 max-h-[120px] overflow-y-auto break-words px-2 text-center text-sm italic"> | ||
| {notes} | ||
| {t("note")}:{notes} |
There was a problem hiding this comment.
🟡 Missing space after colon in notes display creates squished text
In OutOfOfficeInSlots.tsx line 96, the notes display uses {t("note")}:{notes} without a space after the colon, producing output like "Note:Some note text" instead of "Note: Some note text".
Comparison with other reason displays
Other similar displays in the same file correctly include a space:
- Line 84:
{t("reason")}: {getReasonLabel(specifiedReason)}(has space) - Line 88:
{t("reason")}: {getReasonLabel(reason)}(has space)
But line 96 is missing the space:
{t("note")}:{notes}| {t("note")}:{notes} | |
| {t("note")}: {notes} |
Was this helpful? React with 👍 or 👎 to provide feedback.
romitg2
changed the title
3 participants
What does this PR do?
Visual Demo (For contributors especially)
Screen.Recording.2026-02-14.at.1.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist