fix: booker-atom-401-error-rescheduleUid#23495
fix: booker-atom-401-error-rescheduleUid#23495hemantmm wants to merge 11 commits intocalcom:mainfrom
Conversation
|
@hemantmm is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR standardizes handling of rescheduleUid and bookingUid across the Booker feature: it sanitizes these values in the store and initialization paths, normalizes them when read from URL and wrapper props, tightens checks in hooks and query enablement to reject the literal strings "null" and "", and updates downstream logic (isRescheduling, view selection, success event routing, and redirect params). A documentation note was added advising not to pass "null" or empty strings for these props. No public API signatures were changed. Assessment against linked issues
Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/platform/atoms/hooks/bookings/useGetBookingForReschedule.ts (1)
46-49: Don’t swallow errors in queryFn catch; rethrow to set isErrorWithout rethrow, React Query treats failures as success with
undefined.- .catch((err) => { - props.onError?.(err); - }); + .catch((err) => { + props.onError?.(err); + throw err; + });packages/platform/atoms/booker/BookerPlatformWrapper.tsx (1)
298-306: Sanitize allrescheduleUidprops in BookerPlatformWrapper
Wrapprops.rescheduleUidto exclude"","null", ornullbefore passing to hooks/APIs to prevent invalid values.Apply this pattern at:
- line 181 (
rescheduleUid: props.rescheduleUid ?? null)- line 197 (
rescheduleUid: props.rescheduleUid ?? null)- line 305 (
rescheduleUid: props.rescheduleUid)- rescheduleUid: props.rescheduleUid, + rescheduleUid: + props.rescheduleUid && props.rescheduleUid !== "null" && props.rescheduleUid !== "" + ? props.rescheduleUid + : undefined,packages/features/bookings/Booker/store.ts (2)
376-386: Bug: reschedule check uses unsanitized input; use the sanitized store value.The condition still reads the raw
rescheduleUidarg. If a caller passes the literal "null", this block will incorrectly execute. Use the post-set, sanitized value from the store.- if (rescheduleUid && bookingData) { + if (get().rescheduleUid && bookingData) { set({ selectedTimeslot: null }); const originalBookingLength = dayjs(bookingData?.endTime).diff( dayjs(bookingData?.startTime), "minutes" ); set({ selectedDuration: originalBookingLength }); if (!isPlatform || allowUpdatingUrlParams) { updateQueryParam("duration", originalBookingLength ?? ""); } }
282-286: Remove “null” literals from URL params; omit the param when absent
- packages/features/bookings/Booker/store.ts: replace
updateQueryParam("bookingUid", seatedEventData.bookingUid ?? "null")
with a conditional that callsremoveQueryParam("bookingUid")whenbookingUidis missing.- apps/web/lib/reschedule/[uid]/getServerSideProps.ts: eliminate the
eventType.seatsPerTimeSlot ? "&bookingUid=null" : ""branch and only append&bookingUid=<id>if a realbookingUidexists.packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
347-357: Sanitize UID coming from server errors before writing to the store.This path bypasses initialization and can reintroduce the literal "null" or empty strings into state.
- if (error.message === ErrorCode.BookerLimitExceededReschedule && error.data?.rescheduleUid) { - useBookerStore.setState({ - rescheduleUid: error.data?.rescheduleUid, - }); + if (error.message === ErrorCode.BookerLimitExceededReschedule) { + const nextUid = + error.data?.rescheduleUid && + error.data.rescheduleUid !== "null" && + error.data.rescheduleUid !== "" + ? error.data.rescheduleUid + : null; + useBookerStore.setState({ rescheduleUid: nextUid }); }
🧹 Nitpick comments (7)
packages/platform/atoms/hooks/bookings/useGetBookingForReschedule.ts (1)
50-51: Consolidate UID validation and reuse sanitized value for key/path/enabledAvoid duplicating checks and ensure cache keys exclude invalid UIDs.
Apply this minimal change here:
- enabled: isInit && !!props?.uid && props.uid !== "null" && props.uid !== "", + enabled: isInit && Boolean(validUid),Outside this hunk, define and reuse a sanitized UID:
// right after isInit const validUid = props.uid && props.uid !== "null" && props.uid.trim() !== "" ? props.uid : undefined; const pathname = validUid ? `/${V2_ENDPOINTS.bookings}/${validUid}/reschedule` : "";And update:
- queryKey: [QUERY_KEY, validUid]
- Only call http.get when validUid is defined.
packages/platform/atoms/booker/BookerPlatformWrapper.tsx (2)
135-141: Compute once, reuse sanitized UID for reschedule fetchPrecompute a single
uidForRescheduleand pass it here; this also enables reuse in cleanup.- useGetBookingForReschedule({ - uid: - props.rescheduleUid && props.rescheduleUid !== "null" && props.rescheduleUid !== "" - ? props.rescheduleUid - : props.bookingUid && props.bookingUid !== "null" && props.bookingUid !== "" - ? props.bookingUid - : "", + // above this call (outside this hunk), add: + // const uidForReschedule = + // props.rescheduleUid && props.rescheduleUid !== "null" && props.rescheduleUid !== "" + // ? props.rescheduleUid + // : props.bookingUid && props.bookingUid !== "null" && props.bookingUid !== "" + // ? props.bookingUid + // : ""; + useGetBookingForReschedule({ + uid: uidForReschedule, onSuccess: (data) => { setBookingData(data); }, });
491-497: Cleanup should remove the actual query key used (handles fallback to bookingUid)Currently only removes by
props.rescheduleUid, leaving possible stale cache when falling back tobookingUid.- queryClient.removeQueries({ - queryKey: [BOOKING_RESCHEDULE_KEY, props.rescheduleUid], - exact: true, - }); + queryClient.removeQueries({ + queryKey: [BOOKING_RESCHEDULE_KEY, uidForReschedule], + exact: true, + });packages/features/bookings/Booker/components/hooks/useBookingForm.ts (1)
50-52: De-duplicate UID validation via a shared helperMultiple files replicate the same checks; prefer
isValidUid(uid).- view: - rescheduleUid && rescheduleUid !== "null" && rescheduleUid !== "" ? "reschedule" : "booking", + view: isValidUid(rescheduleUid) ? "reschedule" : "booking",Outside this hunk, add once per module (or import from a shared util):
const isValidUid = (uid?: string | null) => typeof uid === "string" && uid !== "null" && uid.trim() !== "";packages/platform/atoms/booker/BookerWebWrapper.tsx (2)
62-68: UseuseSearchParams(already available) and trim; avoid reparsingwindow.location.searchSimplifies SSR checks and keeps logic consistent.
- const rescheduleUid = - typeof window !== "undefined" - ? (() => { - const val = new URLSearchParams(window.location.search).get("rescheduleUid"); - return val && val !== "null" && val !== "" ? val : null; - })() - : null; + const rescheduleUid = useMemo(() => { + const val = searchParams?.get("rescheduleUid"); + return val && val !== "null" && val.trim() !== "" ? val : null; + }, [searchParams]);
71-77: Same simplification forbookingUid; preferuseSearchParams+ trimKeeps behavior consistent and reduces duplication.
- const bookingUid = - typeof window !== "undefined" - ? (() => { - const val = new URLSearchParams(window.location.search).get("bookingUid"); - return val && val !== "null" && val !== "" ? val : null; - })() - : null; + const bookingUid = useMemo(() => { + const val = searchParams?.get("bookingUid"); + return val && val !== "null" && val.trim() !== "" ? val : null; + }, [searchParams]);packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
156-156: Simplify and rely on store-level sanitization.
rescheduleUid !== "null"is redundant after store sanitization.- const isRescheduling = !!rescheduleUid && rescheduleUid !== "null" && !!bookingData; + const isRescheduling = Boolean(rescheduleUid && bookingData);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
docs/platform/atoms/booker.mdx(1 hunks)packages/features/bookings/Booker/components/hooks/useBookingForm.ts(1 hunks)packages/features/bookings/Booker/components/hooks/useBookings.ts(1 hunks)packages/features/bookings/Booker/store.ts(1 hunks)packages/platform/atoms/booker/BookerPlatformWrapper.tsx(1 hunks)packages/platform/atoms/booker/BookerWebWrapper.tsx(1 hunks)packages/platform/atoms/hooks/bookings/useGetBookingForReschedule.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/platform/atoms/booker/BookerPlatformWrapper.tsxpackages/platform/atoms/booker/BookerWebWrapper.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/platform/atoms/booker/BookerPlatformWrapper.tsxpackages/features/bookings/Booker/store.tspackages/features/bookings/Booker/components/hooks/useBookingForm.tspackages/platform/atoms/hooks/bookings/useGetBookingForReschedule.tspackages/platform/atoms/booker/BookerWebWrapper.tsxpackages/features/bookings/Booker/components/hooks/useBookings.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/platform/atoms/booker/BookerPlatformWrapper.tsxpackages/features/bookings/Booker/store.tspackages/features/bookings/Booker/components/hooks/useBookingForm.tspackages/platform/atoms/hooks/bookings/useGetBookingForReschedule.tspackages/platform/atoms/booker/BookerWebWrapper.tsxpackages/features/bookings/Booker/components/hooks/useBookings.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/Booker/store.tspackages/features/bookings/Booker/components/hooks/useBookingForm.tspackages/platform/atoms/hooks/bookings/useGetBookingForReschedule.tspackages/features/bookings/Booker/components/hooks/useBookings.ts
🧠 Learnings (3)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/Booker/store.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/features/bookings/Booker/components/hooks/useBookingForm.ts
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.
Applied to files:
packages/platform/atoms/hooks/bookings/useGetBookingForReschedule.ts
🪛 LanguageTool
docs/platform/atoms/booker.mdx
[grammar] ~134-~134: There might be a mistake here.
Context: ... | > Note: > When using rescheduleUid or `bookingUi...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...e prop or pass null. Passing "null" or empty string may result in a 401 Unauth...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/features/bookings/Booker/components/hooks/useBookingForm.ts (1)
50-52: LGTM — stricter check avoids accidental “reschedule” modeThe additional guards on
"null"/""are correct and align with the PR’s objective.packages/features/bookings/Booker/store.ts (1)
341-345: Sanitization of rescheduleUid/bookingUid looks good.Guarding against "null" and "" aligns with the PR goal and prevents accidental auth errors.
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
324-331: Former time inclusion only when rescheduling — LGTM.Conditionally setting
formerTimebased onisReschedulingmatches the new contract.Also applies to: 429-431
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/platform/atoms/booker.mdx (1)
134-136: LGTM—clarifies invalid values and outcomes.This addresses the earlier wording suggestion and clearly warns about
"null"/empty strings causing 401s.
🧹 Nitpick comments (1)
docs/platform/atoms/booker.mdx (1)
134-136: Wording fix: avoid mixed contraction style.Use "do not" (no contraction) for consistency with the preceding sentence.
-> When using `rescheduleUid` or `bookingUid` props, do **not** pass the string `"null"` or an empty string. If you don’t have a valid value, omit the prop or pass `null` (without quotes). Passing `"null"` or an empty string may result in a 401 Unauthorized error from the API. +> When using `rescheduleUid` or `bookingUid` props, do **not** pass the string `"null"` or an empty string. If you do not have a valid value, omit the prop or pass `null` (without quotes). Passing `"null"` or an empty string may result in a 401 Unauthorized error from the API.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/platform/atoms/booker.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/platform/atoms/booker.mdx
[grammar] ~134-~134: There might be a mistake here.
Context: ... | > Note: > When using rescheduleUid or `bookingUi...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
dosubot
bot
added
the
bookings
|
@anikdhabal, I have attached a video. Can you have a look at it. |
|
cc @supalarry |
|
Thank you for your PR Hemant! We appreciate your contribution. I am not sure about the solution because:
I am closing this issue for now because I asked original issue owner to provide more details. |
4 participants
closes: #20949
What does this PR do?
rescheduleUid#20949These changes ensure that the rescheduleUiD is only considered valid if they are non-empty and not the string "null". It prevents passing invalid or empty UIDs to the backend API, which would otherwise result in 401 Unauthorized errors.
The update applies this check throughout the Booker Atom's hooks, components, and store initialization, and adds a documentation note to warn developers not to pass "null" or empty strings for these props. This results in more robust handling of booking rescheduling and avoids unnecessary or invalid API calls.
Video:
Before:
Screen.Recording.2025-09-04.at.18.52.13.mov
After:
Screen.Recording.2025-09-04.at.18.50.22.mov
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist