Skip to content

fix: booker embed #22898

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

Merged
merged 5 commits into from
Aug 7, 2025
Merged

fix: booker embed #22898

merged 5 commits into from
Aug 7, 2025

Conversation

SomayChauhan
Copy link
Member

@SomayChauhan SomayChauhan commented Aug 5, 2025

What does this PR do?

fixing some weird edge-casses for booker embed

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

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):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

@SomayChauhan SomayChauhan requested review from a team as code owners August 5, 2025 10:01
Copy link
Contributor

coderabbitai bot commented Aug 5, 2025

Walkthrough

The changes span several files and focus on refining type definitions, hook signatures, and request parameter logic. The BookerEmbed component's prop types were updated to allow an optional organizationId for the individual variant. The useMe hook now accepts an optional isEmbed parameter, defaulting to false, and disables its query when in an embedded context. The BaseCalProvider component now passes the isEmbed flag to useMe. In useAtomGetPublicEvent, request parameter construction was refactored to conditionally include orgId only when its value is not zero. The useOAuthClient hook's effect dependencies now include the current HTTP client URL.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd576f5 and b1c6cdf.

📒 Files selected for processing (5)
  • packages/platform/atoms/booker-embed/BookerEmbed.tsx (1 hunks)
  • packages/platform/atoms/cal-provider/BaseCalProvider.tsx (1 hunks)
  • packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx (1 hunks)
  • packages/platform/atoms/hooks/useMe.ts (2 hunks)
  • packages/platform/atoms/hooks/useOAuthClient.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/platform/atoms/hooks/useOAuthClient.ts
  • packages/platform/atoms/hooks/useMe.ts
**/*.{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/useOAuthClient.ts
  • packages/platform/atoms/hooks/useMe.ts
  • packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx
  • packages/platform/atoms/cal-provider/BaseCalProvider.tsx
  • packages/platform/atoms/booker-embed/BookerEmbed.tsx
**/*.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
  • packages/platform/atoms/cal-provider/BaseCalProvider.tsx
  • packages/platform/atoms/booker-embed/BookerEmbed.tsx
🧠 Learnings (3)
📚 Learning: in cal.com's getusereventgroups handler refactor (pr #22618), the membershipcount field for team eve...
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.335Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.

Applied to files:

  • packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
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/platform/atoms/booker-embed/BookerEmbed.tsx
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.

Applied to files:

  • packages/platform/atoms/booker-embed/BookerEmbed.tsx
🧬 Code Graph Analysis (2)
packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx (1)
packages/platform/types/api.ts (1)
  • ApiResponse (33-35)
packages/platform/atoms/cal-provider/BaseCalProvider.tsx (1)
packages/platform/atoms/hooks/useMe.ts (1)
  • useMe (14-30)
🔇 Additional comments (6)
packages/platform/atoms/hooks/useMe.ts (2)

14-14: LGTM - Parameter addition is backward compatible.

The optional isEmbed parameter with a default value maintains backward compatibility while enabling embed-specific behavior.


26-26: All embed contexts are correctly handled in useMe hook

I’ve verified that useMe is invoked with the isEmbed flag in BaseCalProvider (which wraps all embed flows), and in other components it defaults to false but will still be disabled in embed contexts due to an empty authorization header. The enabled: Boolean(http.getAuthorizationHeader()) && !isEmbed logic thus cleanly prevents user‐data fetching in embeds without impacting non‐embed use cases.

No changes required here.

packages/platform/atoms/cal-provider/BaseCalProvider.tsx (1)

63-63: LGTM - Proper integration with updated useMe hook.

The isEmbed parameter is correctly passed to the useMe hook, maintaining consistency with the hook's updated signature and enabling embed-aware behavior.

packages/platform/atoms/hooks/useOAuthClient.ts (1)

58-58: LGTM - Enhanced URL change detection for OAuth client.

Adding http.getUrl() to the dependency array ensures the OAuth client fetch logic re-runs when the HTTP client's base URL changes, which is important for dynamic or embedded environments.

packages/platform/atoms/hooks/event-types/public/useAtomGetPublicEvent.tsx (1)

36-46: LGTM - Improved parameter construction with conditional orgId.

The refactored parameter construction is more explicit and readable. The conditional inclusion of orgId only when organizationId !== 0 is a good optimization that avoids sending unnecessary parameters to the API.

packages/platform/atoms/booker-embed/BookerEmbed.tsx (1)

32-32: LGTM - Type refinement improves flexibility.

Changing organizationId from undefined to number in the individual variant allows optional organization context while maintaining type safety. This aligns well with the conditional parameter handling in useAtomGetPublicEvent where orgId is only included when non-zero.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/booker-embed-2

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@graphite-app graphite-app bot requested a review from a team August 5, 2025 10:01
@keithwillcode keithwillcode added core area: core, team members only platform Anything related to our platform plan labels Aug 5, 2025
Copy link

graphite-app bot commented Aug 5, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (08/05/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (08/06/25)

1 label was added to this PR based on Keith Williams's automation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't call useMe for embeds

Copy link
Member Author

@SomayChauhan SomayChauhan Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes the orgId being sent for non-org members when tryint to access their personal events, @Ryukemeister showed me the other day

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-run useEffect in case the url is not set the first time

Copy link
Member Author

@SomayChauhan SomayChauhan Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow organizationId for user events as well.
This is required because, with the migration to the new /atoms endpoints for the booker atom, organizationId will be necessary in certain cases for user's individual events as well

Copy link
Contributor

@Ryukemeister Ryukemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SomayChauhan can you add a demo video here?

@SomayChauhan
Copy link
Member Author

there's nothing to demo, it works the same

Copy link

vercel bot commented Aug 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Aug 6, 2025 1:00pm
cal-eu ⬜️ Ignored (Inspect) Aug 6, 2025 1:00pm

Copy link
Contributor

github-actions bot commented Aug 6, 2025

E2E results are ready!

@supalarry supalarry merged commit 33aedba into main Aug 7, 2025
81 of 85 checks passed
@supalarry supalarry deleted the fix/booker-embed-2 branch August 7, 2025 08:29
Pallava-Joshi pushed a commit to Pallava-Joshi/cal.com that referenced this pull request Aug 8, 2025
* fix: make organizationId optional number type in BookerEmbed

* feat: dont call useMe for embeds

* useOAuthClient hook should re-run when url is set

* feat: Remove `?orgId=0` from /public event query params

---------

Co-authored-by: Rajiv Sahal <sahalrajiv-extc@atharvacoe.ac.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only platform Anything related to our platform plan ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants