fix(companion): event type links for org user#5
Conversation
Addresses Cubic AI review feedback (confidence 9/10): The username was hardcoded to 'username' but still used by BasicsTab as a fallback for URL display when bookingUrl is unavailable. This restores the useEffect that fetches the actual username from CalComAPIService.getUsername(). Co-Authored-By: unknown <>
📝 WalkthroughWalkthroughThe pull request introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@companion/app/`(tabs)/(event-types)/event-type-detail.tsx:
- Line 180: The bookingUrl state is declared but never populated, so update
applyEventTypeData to call setBookingUrl with the fetched eventType.bookingUrl
after you obtain the API response; specifically, inside applyEventTypeData
(after you parse/set other basic fields like name/description), call
setBookingUrl(eventType.bookingUrl) so handlePreview and handleCopyLink see the
real URL instead of the empty string.
In `@companion/extension/entrypoints/content.ts`:
- Around line 1125-1131: The copy action currently reconstructs the booking URL
from eventType.users?.[0]?.username and eventType.slug instead of using the
already-computed bookingUrl (so org/custom-domain URLs are lost); update the
clipboard operation in the block where bookingUrl is defined to call
navigator.clipboard.writeText(bookingUrl) (replace the inline template usage) so
the copy honors the exact bookingUrl value used by preview/insert.
In `@companion/services/calcom.ts`:
- Around line 1654-1662: The catch block in getUsername() declares an unused
variable `error` and discards the original failure details; change the catch to
catch (_error) or catch (err) to satisfy the linter and include the original
error context in the thrown error (e.g., throw a new Error like `Failed to get
username: ${err?.message ?? String(err)}` or use the Error `cause` option) so
debugging information from getUserProfile() is preserved while still providing
the descriptive message.
| const [conferencingOptions, setConferencingOptions] = useState<ConferencingOption[]>([]); | ||
| const [conferencingLoading, setConferencingLoading] = useState(false); | ||
| const [eventTypeData, setEventTypeData] = useState<EventType | null>(null); | ||
| const [bookingUrl, setBookingUrl] = useState<string>(""); |
There was a problem hiding this comment.
Critical: bookingUrl state is declared but never populated.
The setBookingUrl setter is never called anywhere in the component. The state remains an empty string (""), causing:
handlePreview()(lines 948-954) to always show "Booking URL not available"handleCopyLink()(lines 956-963) to always show "Booking URL not available"
The applyEventTypeData function fetches eventType from the API (which includes bookingUrl per the type definition), but never calls setBookingUrl(eventType.bookingUrl).
🐛 Proposed fix: Populate bookingUrl from API response
Add the following in applyEventTypeData (around line 459, after setting other basic fields):
if (eventType.hidden !== undefined) setIsHidden(eventType.hidden);
+ if (eventType.bookingUrl) setBookingUrl(eventType.bookingUrl);🧰 Tools
🪛 Biome (2.3.13)
[error] 180-180: This variable setBookingUrl is unused.
Unused variables are often the result of typos, incomplete refactors, or other sources of bugs.
Unsafe fix: If this is intentional, prepend setBookingUrl with an underscore.
(lint/correctness/noUnusedVariables)
🤖 Prompt for AI Agents
In `@companion/app/`(tabs)/(event-types)/event-type-detail.tsx at line 180, The
bookingUrl state is declared but never populated, so update applyEventTypeData
to call setBookingUrl with the fetched eventType.bookingUrl after you obtain the
API response; specifically, inside applyEventTypeData (after you parse/set other
basic fields like name/description), call setBookingUrl(eventType.bookingUrl) so
handlePreview and handleCopyLink see the real URL instead of the empty string.
| const bookingUrl = | ||
| `https://cal.com/${ | ||
| eventType.users?.[0]?.username || "user" | ||
| }/${eventType.slug}`; | ||
| navigator.clipboard | ||
| .writeText(bookingUrl) | ||
| .then(() => { |
There was a problem hiding this comment.
Fix copy link to honor bookingUrl.
The copy action still builds a URL from username/slug, which breaks org/custom-domain event types. Mirror the bookingUrl fallback used by preview/insert.
🔧 Proposed fix
-const bookingUrl =
- `https://cal.com/${eventType.users?.[0]?.username || "user"}/${eventType.slug}`;
+const bookingUrl =
+ eventType.bookingUrl ||
+ `https://cal.com/${eventType.users?.[0]?.username || "user"}/${eventType.slug}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const bookingUrl = | |
| `https://cal.com/${ | |
| eventType.users?.[0]?.username || "user" | |
| }/${eventType.slug}`; | |
| navigator.clipboard | |
| .writeText(bookingUrl) | |
| .then(() => { | |
| const bookingUrl = | |
| eventType.bookingUrl || | |
| `https://cal.com/${ | |
| eventType.users?.[0]?.username || "user" | |
| }/${eventType.slug}`; | |
| navigator.clipboard | |
| .writeText(bookingUrl) | |
| .then(() => { |
🤖 Prompt for AI Agents
In `@companion/extension/entrypoints/content.ts` around lines 1125 - 1131, The
copy action currently reconstructs the booking URL from
eventType.users?.[0]?.username and eventType.slug instead of using the
already-computed bookingUrl (so org/custom-domain URLs are lost); update the
clipboard operation in the block where bookingUrl is defined to call
navigator.clipboard.writeText(bookingUrl) (replace the inline template usage) so
the copy honors the exact bookingUrl value used by preview/insert.
| // Helper to get username | ||
| async function getUsername(): Promise<string> { | ||
| try { | ||
| const profile = await getUserProfile(); | ||
| return profile.username; | ||
| } catch (error) { | ||
| throw new Error("Failed to get username"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Prefix unused error variable and consider preserving error context.
The static analysis tool correctly flags error as unused. Additionally, the generic error message loses debugging context from the original failure.
Proposed fix
async function getUsername(): Promise<string> {
try {
const profile = await getUserProfile();
return profile.username;
- } catch (error) {
- throw new Error("Failed to get username");
+ } catch (_error) {
+ const message = _error instanceof Error ? _error.message : String(_error);
+ throw new Error(`Failed to get username: ${message}`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Helper to get username | |
| async function getUsername(): Promise<string> { | |
| try { | |
| const profile = await getUserProfile(); | |
| return profile.username; | |
| } catch (error) { | |
| throw new Error("Failed to get username"); | |
| } | |
| } | |
| // Helper to get username | |
| async function getUsername(): Promise<string> { | |
| try { | |
| const profile = await getUserProfile(); | |
| return profile.username; | |
| } catch (_error) { | |
| const message = _error instanceof Error ? _error.message : String(_error); | |
| throw new Error(`Failed to get username: ${message}`); | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 1659-1659: This variable error is unused.
Unused variables are often the result of typos, incomplete refactors, or other sources of bugs.
Unsafe fix: If this is intentional, prepend error with an underscore.
(lint/correctness/noUnusedVariables)
🤖 Prompt for AI Agents
In `@companion/services/calcom.ts` around lines 1654 - 1662, The catch block in
getUsername() declares an unused variable `error` and discards the original
failure details; change the catch to catch (_error) or catch (err) to satisfy
the linter and include the original error context in the thrown error (e.g.,
throw a new Error like `Failed to get username: ${err?.message ?? String(err)}`
or use the Error `cause` option) so debugging information from getUserProfile()
is preserved while still providing the descriptive message.
3 participants
Benchmark PR from agentic-review-benchmarks#5
Summary by CodeRabbit
Release Notes