-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix: correct bookingUrl for EU instance in API responses #27046
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
Conversation
For EU deployments where WEBAPP_URL (app.cal.eu) differs from WEBSITE_URL (cal.com), use WEBAPP_URL for non-org booking URLs. This fixes incorrect bookingUrl in API responses for EU instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
supalarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Request to re-use code:
| return options.protocol ? WEBSITE_URL : WEBSITE_URL.replace("https://", "").replace("http://", ""); | ||
| if (!slug) { | ||
| // Use WEBAPP_URL if domains differ (e.g., EU: app.cal.eu vs cal.com) | ||
| const getBaseDomain = (hostname: string) => hostname.split(".").slice(-2).join("."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for const getBaseDomain is copied from getTldPlus1 function (search for it in vs code).
We already have 2 copies of getTldPlus1 - can we define it only once and re-use it in the 2 existing places and here and get rid of copies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review, made following changes:
- created packages/lib/getTldPlus1.ts - new shared utility
- updated getSafeRedirectUrl.ts - removed inline copy, now imports shared function
- updated orgDomains.ts - uses shared getTldPlus1 instead of inline getBaseDomain
Pull request was converted to draft
… function instead of inline copy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
supalarry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
What does this PR do?
Fixes incorrect
bookingUrlfield returned byapi.cal.eu/v2/event-typesfor non-organization users.Problem
https://cal.com/user/eventinstead ofhttps://app.cal.eu/user/eventcal.euredirects to marketing page, not booking pagesapp.cal.euSolution
Updated getOrgFullOrigin() to detect when
WEBSITE_URLandWEBAPP_URLare on different root domains. When they differ (EU case), useWEBAPP_URLinstead.app.cal.comcal.com/user/event✅app.cal.euapp.cal.eu/user/event✅Changes
Type of change
How should this be tested?
yarn test packages/features/ee/organizations/lib/orgDomains.test.ts- all 10 tests passGET api.cal.eu/v2/event-typesreturns correctbookingUrl