fix: return 404 for unknown rescheduleUid on public and private booking routes (#29399) - #29927
Conversation
…s in availability (calcom#29869)
|
Welcome to Cal.diy, @Samarth1306w! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThe changes return not-found responses for missing reschedule bookings. Cancelled ICS events now increment their sequence number. Calendar availability excludes declined and unanswered invitations for the authenticated attendee. Timezone formatting preserves minutes when removing leading zeroes from single-digit UTC offsets. Tests cover the updated ICS, CalDAV availability, and timezone behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/server/lib/[user]/[type]/getServerSideProps.ts (1)
61-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the changed comment explain why.
At Line 61, the comment repeats the condition and the success behavior. It does not explain why dynamic or matching event types can continue without a redirect. Remove the comment or state the routing reason.
Proposed comment change
- // if no eventTypeId (dynamic) or it matches this eventData - return void (success). + // Keep dynamic bookings and this event type on the current page; other event types require a redirect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/server/lib/`[user]/[type]/getServerSideProps.ts around lines 61 - 62, Update the comment immediately before the conditional in getServerSideProps to explain the routing reason dynamic or matching event types may continue without a redirect, rather than repeating the condition and success behavior; alternatively, remove the comment if that context is already clear from the surrounding code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/d/`[link]/[slug]/getServerSideProps.tsx:
- Around line 108-112: Treat rescheduleUid as invalid when it is missing or an
empty string, rather than relying on truthiness. Update the reschedule handling
in apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx lines 108-112 and
apps/web/server/lib/[user]/[type]/getServerSideProps.ts lines 42-50 so empty
query values return notFound, while valid values continue through
getBookingForReschedule.
In `@packages/lib/CalendarService.test.ts`:
- Around line 700-716: Extend the test “skips CalDAV events with unanswered
(NEEDS-ACTION) or DECLINED invitations for user” with a separate event fixture
whose attendee includes PARTSTAT=DECLINED, then assert that the service returns
no busy period for that event. Keep the existing NEEDS-ACTION fixture and
assertions unchanged.
- Line 702: Update the CalendarService test fixture to construct
TestCalendarService with the desired user email instead of mutating the private
credential field via an unsafe cast. Remove the as any casts from
fetchCalendarObjects and getAvailability(selectedCalendars) fixtures while
preserving their typed mock behavior.
---
Nitpick comments:
In `@apps/web/server/lib/`[user]/[type]/getServerSideProps.ts:
- Around line 61-62: Update the comment immediately before the conditional in
getServerSideProps to explain the routing reason dynamic or matching event types
may continue without a redirect, rather than repeating the condition and success
behavior; alternatively, remove the comment if that context is already clear
from the surrounding code.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a774c43a-39fb-44ea-a500-8fa59464ea46
📒 Files selected for processing (8)
apps/web/lib/d/[link]/[slug]/getServerSideProps.tsxapps/web/server/lib/[user]/[type]/getServerSideProps.tspackages/emails/lib/generateIcsString.test.tspackages/emails/lib/generateIcsString.tspackages/lib/CalendarService.test.tspackages/lib/CalendarService.tspackages/lib/timezone.test.tspackages/lib/timezone.ts
| if (rescheduleUid) { | ||
| booking = await getBookingForReschedule(`${rescheduleUid}`, session?.user?.id); | ||
| if (!booking) { | ||
| return notFound; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
How does Next.js 16.2.3 populate GetServerSidePropsContext.query for a URL containing ?rescheduleUid=?
💡 Result:
In Next.js, including the Pages Router with getServerSideProps, the context.query object populates parameters based on standard URL parsing rules [1]. When a URL contains?rescheduleUid= (with an empty value after the equals sign), the rescheduleUid key will be populated in context.query with an empty string ('') as its value [2]. If the URL is formatted as?rescheduleUid (without the equals sign), behavior can sometimes vary depending on the environment, underlying middleware, or server-side redirects, as frameworks sometimes normalize or interpret these differently [2]. However, in most standard Next.js environments, URLSearchParams—which underpins much of the framework's query parsing—will treat both?rescheduleUid= and?rescheduleUid as resulting in an empty string value for that key [2]. Key points regarding this behavior: 1. Parsing Logic: Next.js relies on standard Node.js/Web API patterns for parsing query strings [2]. Under these standards, a key present in the query string without a value is typically assigned an empty string [2]. 2. Consistent Access: You can access this value in getServerSideProps via context.query.rescheduleUid [1]. 3. Environment Variations: While the core parsing is consistent, issues have been reported in specific production environments (like certain serverless deployments or when using complex middleware) where query parameters might be stripped or behave inconsistently if they are perceived as empty or malformed [3][4][5]. If you find context.query is unexpectedly missing keys, it is often due to external factors like middleware rewrites, ad-blockers, or platform-specific request handling rather than the default behavior of getServerSideProps itself [6][4]. To ensure robust handling, it is recommended to explicitly check for the existence of the parameter or validate the value rather than relying on truthy/falsy checks alone (e.g., check if the property exists in the object or compare against an empty string) [7].
Citations:
- 1: https://nextjs.org/docs/api-reference/data-fetching/get-server-side-props
- 2: Rewrites silently modify query string when "=" is missing vercel/next.js#37028
- 3: [Bug]: Query string not populated for getServerSideProps opennextjs/opennextjs-netlify#1068
- 4: Empty URL search params are swallowed when rewriting via middleware on Vercel vercel/next.js#73722
- 5: Why does searchParams in Next.js 15 server component behave differently locally vs. on Vercel for empty query parameters? vercel/next.js#76004
- 6: Query parameters returning empty string vercel/next.js#34424
- 7: Undefined query param returned as a string vercel/next.js#14813
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate target files"
fd -a 'getServerSideProps\.(ts|tsx)$' . | sed 's#^\./##' | rg 'apps/web/(lib/d/\[link\]/\[slug\]/getServerSideProps\.tsx|server/lib/\[user\]/\[type\]/getServerSideProps\.ts)$' || true
echo
echo "File stats"
wc -l 'apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx' 'apps/web/server/lib/[user]/[type]/getServerSideProps.ts' 2>/dev/null || true
echo
echo "Relevant sections"
sed -n '1,150p' 'apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx' | nl -ba | sed -n '90,125p'
echo "---"
sed -n '1,90p' 'apps/web/server/lib/[user]/[type]/getServerSideProps.ts' | nl -ba | sed -n '35,55p'
echo
echo "Find getBookingForReschedule definition/usages"
rg -n "getBookingForReschedule|rescheduleUid|URLSearchParams|query\.rescheduleUid" apps/web/lib apps/web/server -g '*.ts' -g '*.tsx' || true
echo
echo "Behavioral probe for JS truthiness of possible Next query values"
node - <<'JS'
const values = [undefined, null, "", "abc"];
for (const v of values) {
console.log(JSON.stringify(v), {
hasProperty: Object.prototype.hasOwnProperty.call({rescheduleUid: v}, "rescheduleUid"),
truthy: !!v,
});
}
JSRepository: calcom/cal.diy
Length of output: 522
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Relevant files"
for f in 'apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx' 'apps/web/server/lib/[user]/[type]/getServerSideProps.ts'; do
echo "--- $f"
sed -n '90,125p' "$f"
echo
done
echo "Find getBookingForReschedule and rescheduleUid handling"
rg -n "getBookingForReschedule|rescheduleUid|URLSearchParams|query\.rescheduleUid" apps/web/lib apps/web/server -g '*.ts' -g '*.tsx' || true
echo "Behavioral probe for JavaScript truthiness of possible Next.js query values"
node - <<'JS'
const values = [undefined, null, "", "abc"];
for (const v of values) {
console.log(JSON.stringify(v), {
hasProperty: Object.prototype.hasOwnProperty.call({rescheduleUid: v}, "rescheduleUid"),
truthy: !!v,
emptyStringPresentKey: v === "",
});
}
JSRepository: calcom/cal.diy
Length of output: 7058
Treat a present but empty rescheduleUid as invalid.
?rescheduleUid= or ?rescheduleUid is parsed as rescheduleUid: "", so the current truthiness guards skip the lookup and continue as if no reschedule parameter was provided. Detect missing values separately and return not-found for empty rescheduleUid in both apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx and apps/web/server/lib/[user]/[type]/getServerSideProps.ts.
📍 Affects 2 files
apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx#L108-L112(this comment)apps/web/server/lib/[user]/[type]/getServerSideProps.ts#L42-L50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/d/`[link]/[slug]/getServerSideProps.tsx around lines 108 - 112,
Treat rescheduleUid as invalid when it is missing or an empty string, rather
than relying on truthiness. Update the reschedule handling in
apps/web/lib/d/[link]/[slug]/getServerSideProps.tsx lines 108-112 and
apps/web/server/lib/[user]/[type]/getServerSideProps.ts lines 42-50 so empty
query values return notFound, while valid values continue through
getBookingForReschedule.
| it("skips CalDAV events with unanswered (NEEDS-ACTION) or DECLINED invitations for user", async () => { | ||
| const service = new TestCalendarService(); | ||
| service.credential.user = { email: "user@example.com" } as any; | ||
|
|
||
| const objects = [ | ||
| { | ||
| data: `BEGIN:VCALENDAR | ||
| VERSION:2.0 | ||
| BEGIN:VEVENT | ||
| UID:unanswered-1 | ||
| SUMMARY:Unanswered Invite | ||
| DTSTART:20260729T100000Z | ||
| DTEND:20260729T110000Z | ||
| ORGANIZER;EMAIL=organizer@example.com:mailto:organizer@example.com | ||
| ATTENDEE;EMAIL=user@example.com;X-APPLE-NEEDS-REPLY=TRUE:mailto:user@example.com | ||
| END:VEVENT | ||
| END:VCALENDAR`, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add coverage for PARTSTAT=DECLINED.
The fixture has no PARTSTAT. It tests only the default NEEDS-ACTION branch. Add a separate attendee fixture with PARTSTAT=DECLINED and assert that it produces no busy period.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/CalendarService.test.ts` around lines 700 - 716, Extend the test
“skips CalDAV events with unanswered (NEEDS-ACTION) or DECLINED invitations for
user” with a separate event fixture whose attendee includes PARTSTAT=DECLINED,
then assert that the service returns no busy period for that event. Keep the
existing NEEDS-ACTION fixture and assertions unchanged.
|
|
||
| it("skips CalDAV events with unanswered (NEEDS-ACTION) or DECLINED invitations for user", async () => { | ||
| const service = new TestCalendarService(); | ||
| service.credential.user = { email: "user@example.com" } as any; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'private credential|service\.credential|as any' \
packages/lib/CalendarService.ts \
packages/lib/CalendarService.test.tsRepository: calcom/cal.diy
Length of output: 3139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant CalendarService.ts outline/sections =="
sed -n '370,430p' packages/lib/CalendarService.ts
echo
echo "== TestCalendarService definition/usages =="
rg -n -C 4 'class TestCalendarService|new TestCalendarService|fetchCalendarObjects|getAvailability|selectedCalendars' packages/lib/CalendarService.test.ts
echo
echo "== exact test lines 690-732 =="
sed -n '690,732p' packages/lib/CalendarService.test.tsRepository: calcom/cal.diy
Length of output: 17163
Remove the unsafe fixture casts.
BaseCalendarService.credential is private and is only assigned in the constructor. This cast cannot make later non-contravariant access type-correct; construct TestCalendarService with the desired user email instead. Keep fetchCalendarObjects and getAvailability(selectedCalendars) typed fixtures by removing the as any casts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/CalendarService.test.ts` at line 702, Update the CalendarService
test fixture to construct TestCalendarService with the desired user email
instead of mutating the private credential field via an unsafe cast. Remove the
as any casts from fetchCalendarObjects and getAvailability(selectedCalendars)
fixtures while preserving their typed mock behavior.
Source: Coding guidelines
|
|
2 participants
Fixes #29399
Summary
When an unknown or invalid is supplied in query parameters (e.g. or ), the booking page previously returned a 200 HTTP response and opened a normal booking flow.