fix: join multiple username parameter with + in Booker atom instead of ,#23203
fix: join multiple username parameter with + in Booker atom instead of ,#23203ibex088 merged 4 commits intocalcom:mainfrom
Conversation
|
@sahitya-chandra is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe change updates the username query parameter serialization in packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx for the get-public-event API call. When multiple usernames are provided, they are now joined with plus (+) separators instead of commas (,). No other logic in parameter construction, orgId handling, request execution, or response handling was modified. No public/exported API signatures changed. Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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
CodeRabbit Configuration File (
|
github-actions
bot
added
booking-page
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/20/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/20/25)1 label was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/21/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx (3)
27-30: Avoid recomputing username list; compute once and reusePrecompute
usernameListwithuseMemoand reuse it for bothisDynamicand the request param. Reduces duplication and keeps logic consistent.import { useMemo } from "react"; @@ -export const useAtomGetPublicEvent = ({ username, eventSlug, isTeamEvent, teamId, selectedDuration }: Props) => { +export const useAtomGetPublicEvent = ({ username, eventSlug, isTeamEvent, teamId, selectedDuration }: Props) => { @@ - const isDynamic = useMemo(() => { - return getUsernameList(username ?? "").length > 1; - }, [username]); + const usernameList = useMemo(() => getUsernameList(username ?? ""), [username]); + const isDynamic = useMemo(() => usernameList.length > 1, [usernameList]); @@ - const params: Record<string, any> = { + const params: Record<string, any> = { isTeamEvent, teamId, - username: getUsernameList(username ?? "").join("+") + username: usernameList.join("+") };Also applies to: 39-40
42-45: Nit: avoid leaking undefined orgId into paramsIf
organizationIdcan beundefined, the current check may includeorgId=undefined. Guard by type.- if (organizationId !== 0) { - params.orgId = organizationId; - } + if (typeof organizationId === "number" && organizationId !== 0) { + params.orgId = organizationId; + }
33-63: Add a focused test to prevent regressions on '+' delimiterA small test that renders the hook and asserts the outgoing params include
usernamejoined by+will lock this down.Example (paths may need minor adjustments to your test setup):
// packages/platform/atoms/hooks/event-types/public/__tests__/useAtomGetPublicEvent.test.tsx import { renderHook } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import http from "../../../lib/http"; import { useAtomGetPublicEvent } from "../useAtomGetPublicEvent"; jest.mock("../../../lib/http", () => ({ __esModule: true, default: { get: jest.fn().mockResolvedValue({ data: { status: "success", data: { length: 30 } } }) }, })); const wrapper: React.FC<React.PropsWithChildren> = ({ children }) => { const qc = new QueryClient(); return <QueryClientProvider client={qc}>{children}</QueryClientProvider>; }; it("joins usernames with '+' in request params", async () => { const spy = jest.spyOn(http, "get"); renderHook( () => useAtomGetPublicEvent({ username: "alice+bob", eventSlug: "standup", selectedDuration: 45, } as any), { wrapper } ); // wait for first call await new Promise((r) => setTimeout(r, 0)); expect(spy).toHaveBeenCalled(); const [, { params }] = spy.mock.calls[0]; expect(params.username).toBe("alice+bob"); // serializer should encode '+' to %2B on the wire });If you want, I can open a follow-up PR adding this test and wiring any missing providers.
📜 Review details
Configuration used: CodeRabbit UI
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)
packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/hooks/event-types/public/useAtomGetPublicEvent.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/hooks/event-types/public/useAtomGetPublicEvent.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx (2)
39-40: Use '+' to join usernames — aligns with API contract for dynamic group bookingsSwitching to
join("+")fixes the 404s by matching the expectedusername=alice+bobformat.
47-49: Default axios serializer percent-encodes “+” as “%2B”Verified that in packages/platform/atoms/lib/http.ts we create an Axios instance without any custom paramsSerializer. Axios’s built-in buildURL logic uses encodeURIComponent, which encodes “+” to “%2B”. The server’s split on “+” will therefore receive “alice%2Bbob”, decoding back to “alice+bob” correctly. No changes required.
|
@sahitya-chandra Could you please attach a loom video showing that this fix works? |
E2E results are ready! |
5 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist