fix(companion): event type links for org user#32
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 <>
WalkthroughThis pull request integrates an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![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:
- Around line 180-181: The bookingUrl state is initialized but never populated,
so preview/copy guards fail; update bookingUrl when the event type data is
loaded by calling setBookingUrl with the fetched URL (e.g.,
setBookingUrl(eventType.bookingUrl) or the appropriate property) inside the
data-fetch completion (such as the useEffect or the callback that handles
loadEventType/fetchEventType), and likewise apply the same setBookingUrl update
in the other occurrence mentioned (lines 948-962) so the bookingUrl state
reflects API data and the preview/copy logic can run.
In `@companion/extension/entrypoints/content.ts`:
- Around line 1125-1128: The Gmail copy path constructs bookingUrl from
eventType.users?.[0]?.username instead of using the canonical
eventType.bookingUrl; update the logic where bookingUrl is defined/used (the
bookingUrl variable in entrypoints/content.ts and the Gmail copy button handler)
to prefer eventType.bookingUrl when present and only fall back to
`https://cal.com/${eventType.users?.[0]?.username || "user"}/${eventType.slug}`;
ensure the Gmail copy implementation uses that bookingUrl variable so the copied
link matches preview/insert behavior.
In `@companion/services/calcom.ts`:
- Around line 1654-1662: The catch in getUsername currently declares an unused
error and discards the original error; update the catch to capture the error
(e.g., catch (error)) and preserve its context when rethrowing or logging—for
example include the original error message or use the Error cause option when
throwing a new error from getUsername so you don't lose stack/message
information while removing the unused-variable warning.
🧹 Nitpick comments (3)
companion/components/event-type-detail/tabs/LimitsTab.tsx (1)
127-127: Minor redundancy with className styles.The
flexDirection: "row"andalignItems: "center"in the inline style duplicate theflex-row items-centerTailwind classes already applied. While this doesn't cause functional issues, it adds maintenance overhead. Consider keeping styles in one place.♻️ Optional: Remove redundant inline styles
- style={{ height: 44, flexDirection: "row", alignItems: "center" }} + style={{ height: 44 }}companion/components/event-type-detail/tabs/AdvancedTab.tsx (2)
141-141: Same redundancy as LimitsTab.tsx.The
flexDirection: "row"andalignItems: "center"inline styles duplicate theflex-row items-centerTailwind classes.♻️ Optional: Remove redundant inline styles
- style={{ height, flexDirection: "row", alignItems: "center" }} + style={{ height }}
100-168: Consider extracting sharedSettingRowcomponent.The
SettingRowcomponent in this file is nearly identical to the one inLimitsTab.tsx. Consider extracting it into a shared component to reduce duplication and ensure consistent behavior/styling across tabs.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
companion/app/(tabs)/(availability)/availability-detail.tsxcompanion/app/(tabs)/(bookings)/index.ios.tsxcompanion/app/(tabs)/(bookings)/index.tsxcompanion/app/(tabs)/(event-types)/event-type-detail.tsxcompanion/app/(tabs)/(event-types)/index.ios.tsxcompanion/app/(tabs)/(event-types)/index.tsxcompanion/app/profile-sheet.ios.tsxcompanion/app/profile-sheet.tsxcompanion/components/Header.tsxcompanion/components/event-type-detail/tabs/AdvancedTab.tsxcompanion/components/event-type-detail/tabs/BasicsTab.tsxcompanion/components/event-type-detail/tabs/LimitsTab.tsxcompanion/components/event-type-detail/tabs/RecurringTab.tsxcompanion/components/event-type-list-item/EventTypeListItem.ios.tsxcompanion/components/event-type-list-item/EventTypeListItem.tsxcompanion/components/event-type-list-item/EventTypeListItemParts.tsxcompanion/extension/entrypoints/background/index.tscompanion/extension/entrypoints/content.tscompanion/extension/lib/linkedin.tscompanion/services/calcom.tscompanion/services/types/event-types.types.ts
🧰 Additional context used
🧬 Code graph analysis (6)
companion/components/Header.tsx (1)
companion/components/AppPressable.tsx (1)
AppPressable(48-104)
companion/components/event-type-detail/tabs/AdvancedTab.tsx (1)
packages/features/calendars/weeklyview/types/state.ts (1)
View(7-7)
companion/app/profile-sheet.ios.tsx (1)
packages/features/calendars/weeklyview/types/state.ts (1)
View(7-7)
companion/app/(tabs)/(event-types)/index.tsx (4)
companion/utils/alerts.ts (2)
showErrorAlert(36-51)showSuccessAlert(67-74)companion/services/calcom.ts (1)
EventType(138-138)companion/services/types/event-types.types.ts (1)
EventType(173-284)companion/utils/browser.ts (1)
openInAppBrowser(44-68)
companion/app/(tabs)/(event-types)/index.ios.tsx (4)
companion/utils/alerts.ts (2)
showErrorAlert(36-51)showSuccessAlert(67-74)companion/utils/index.ts (3)
showErrorAlert(14-14)showSuccessAlert(14-14)openInAppBrowser(16-16)companion/services/types/event-types.types.ts (1)
EventType(173-284)companion/hooks/index.ts (1)
EventType(41-41)
companion/app/(tabs)/(event-types)/event-type-detail.tsx (1)
companion/utils/index.ts (2)
showErrorAlert(14-14)openInAppBrowser(16-16)
🪛 Biome (2.3.13)
companion/services/calcom.ts
[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)
companion/app/(tabs)/(event-types)/event-type-detail.tsx
[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)
🔇 Additional comments (33)
companion/extension/entrypoints/background/index.ts (1)
277-283: LGTM! Proper linter suppression with clear justification.The biome-ignore comment appropriately documents why the
anytype assertion is necessary for Safari's Manifest V2browserActionAPI, which isn't present in Chrome's type definitions. The cross-browser fallback pattern is correct.companion/app/(tabs)/(availability)/availability-detail.tsx (1)
91-91: LGTM!The icon color change from
#007AFFto#000000aligns with the broader UI consistency updates across the app.companion/app/profile-sheet.ios.tsx (1)
150-150: LGTM!The bottom border removal from the Profile Header container is consistent with the parallel change in
profile-sheet.tsx, maintaining visual consistency across platforms.companion/app/profile-sheet.tsx (1)
127-127: LGTM!The border removal aligns with the corresponding change in
profile-sheet.ios.tsx, ensuring consistent visual styling.companion/app/(tabs)/(bookings)/index.ios.tsx (1)
65-65: LGTM!Both filter menu label color changes from
#007AFFto#000000are consistent with the UI color token updates seen across the codebase.Also applies to: 126-126
companion/components/event-type-detail/tabs/LimitsTab.tsx (1)
143-150: LGTM!Adding
justifyContent: "center"improves the vertical centering of the Switch component within its container.companion/components/event-type-detail/tabs/AdvancedTab.tsx (1)
157-164: LGTM!The
justifyContent: "center"addition is consistent with LimitsTab.tsx changes.companion/components/event-type-detail/tabs/RecurringTab.tsx (2)
91-93: Minor style redundancy, but LGTM.The inline
flexDirection: "row"andalignItems: "center"styles duplicate what's already applied via theflex-row items-centerclassName. This redundancy is harmless and the layout fix achieves the intended alignment.
107-114: LGTM!Adding
justifyContent: "center"properly centers the Switch vertically within its container.companion/components/Header.tsx (2)
106-114: Style redundancy noted, but consistent with PR-wide changes.The inline
flexDirection: "row"andalignItems: "center"styles duplicate theflex-row items-centerclassName. This pattern appears across multiple files in this PR, suggesting a deliberate approach for explicit styling. The color token updates to black (#000000) for active states are applied consistently.
149-153: LGTM!Color token updates from blue to black for active/selected states are applied consistently across all filter and sort options.
Also applies to: 171-175, 212-217, 234-239, 256-261, 292-297, 313-318, 336-341, 362-368, 388-393
companion/components/event-type-list-item/EventTypeListItem.tsx (1)
50-55: LGTM!Correctly propagates
bookingUrltoEventTypeTitlefor proper URL display and linking, aligning with the PR's goal of using direct booking URLs when available.companion/app/(tabs)/(bookings)/index.tsx (1)
63-81: LGTM!The style updates and color token change to black (
#000000) for the selected state are consistent with the broader PR-wide UI updates in Header.tsx.companion/extension/lib/linkedin.ts (2)
118-119: LGTM!Adding optional
bookingUrlto theEventTypeinterface aligns with the broader PR changes propagating booking URLs across the codebase.
667-672: LGTM!The
buildBookingUrlfunction now correctly preferseventType.bookingUrlwhen available, falling back to the legacy URL construction. This supports organization users who may have different booking URL structures.companion/services/types/event-types.types.ts (1)
282-283: Looks good — optionalbookingUrlkeeps backward compatibility.companion/components/event-type-list-item/EventTypeListItem.ios.tsx (1)
99-104: Good wiring ofbookingUrlinto the title component.companion/app/(tabs)/(event-types)/event-type-detail.tsx (2)
1273-1276: UI alignment tweaks look good.Also applies to: 2340-2350
1430-1430: Prop wiring toBasicsTabis consistent.companion/components/event-type-detail/tabs/BasicsTab.tsx (1)
40-41: Booking URL display + alignment tweaks look solid.Also applies to: 205-210, 257-276
companion/app/(tabs)/(event-types)/index.ios.tsx (1)
1-3: Nice switch tobookingUrlwith clear user-facing guards.Also applies to: 31-31, 113-135, 234-241
companion/app/(tabs)/(event-types)/index.tsx (1)
40-40:bookingUrlflow updates and minor UI tweaks look good.Also applies to: 135-157, 288-297, 452-452, 517-517
companion/components/event-type-list-item/EventTypeListItemParts.tsx (3)
7-22: Clean helper for display URL resolution.Good encapsulation of the fallback logic and parsing behavior.
24-37: EventTypeTitle uses unified display URL.Nice reuse of the helper to keep the title/link text consistent.
54-62: EventTypeLink aligned with bookingUrl display logic.Consistent with the title rendering and avoids duplicate URL construction.
companion/extension/entrypoints/content.ts (8)
827-837: Booking URL added to Gmail event-type shape.Good—this keeps the data model aligned with the new API field.
Also applies to: 868-879, 885-895
1081-1085: Preview button now prefers bookingUrl.This matches the intended behavior and preserves fallback logic.
1291-1300: Insert link now prefers bookingUrl.This aligns the inserted link with the API-provided URL.
1319-1327: Copy helper uses bookingUrl fallback.Consistent with other link paths.
1576-1586: Booking URL added to LinkedIn event-type shape.Good parity with Gmail data shape updates.
Also applies to: 1617-1628, 1633-1644
1829-1832: LinkedIn preview prefers bookingUrl.Matches the new source-of-truth link behavior.
1871-1873: LinkedIn copy link prefers bookingUrl.Consistent with preview/insert behavior.
2036-2044: LinkedIn insert link uses bookingUrl fallback.Good alignment with the new URL sourcing.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const [bookingUrl, setBookingUrl] = useState<string>(""); | ||
| const [saving, setSaving] = useState(false); |
There was a problem hiding this comment.
Populate bookingUrl state or preview/copy will always error.
bookingUrl is initialized but never set from fetched data, so the new guards block preview/copy even when the API provides a URL. This also explains the unused setBookingUrl lint warning.
🐛 Proposed fix
const applyEventTypeData = useCallback((eventType: EventType) => {
setEventTypeData(eventType);
+ setBookingUrl(eventType.bookingUrl ?? "");
// Load basic fieldsAlso applies to: 948-962
🧰 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 around lines 180 -
181, The bookingUrl state is initialized but never populated, so preview/copy
guards fail; update bookingUrl when the event type data is loaded by calling
setBookingUrl with the fetched URL (e.g., setBookingUrl(eventType.bookingUrl) or
the appropriate property) inside the data-fetch completion (such as the
useEffect or the callback that handles loadEventType/fetchEventType), and
likewise apply the same setBookingUrl update in the other occurrence mentioned
(lines 948-962) so the bookingUrl state reflects API data and the preview/copy
logic can run.
| const bookingUrl = | ||
| `https://cal.com/${ | ||
| eventType.users?.[0]?.username || "user" | ||
| }/${eventType.slug}`; |
There was a problem hiding this comment.
Copy link still ignores bookingUrl.
The Gmail copy button should mirror the preview/insert behavior and prefer eventType.bookingUrl. Otherwise org-user links remain wrong in this path.
🔧 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}`; | |
| const bookingUrl = | |
| eventType.bookingUrl || | |
| `https://cal.com/${eventType.users?.[0]?.username || "user"}/${eventType.slug}`; |
🤖 Prompt for AI Agents
In `@companion/extension/entrypoints/content.ts` around lines 1125 - 1128, The
Gmail copy path constructs bookingUrl from eventType.users?.[0]?.username
instead of using the canonical eventType.bookingUrl; update the logic where
bookingUrl is defined/used (the bookingUrl variable in entrypoints/content.ts
and the Gmail copy button handler) to prefer eventType.bookingUrl when present
and only fall back to `https://cal.com/${eventType.users?.[0]?.username ||
"user"}/${eventType.slug}`; ensure the Gmail copy implementation uses that
bookingUrl variable so the copied link matches preview/insert behavior.
| // 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.
Unused error variable and lost error context.
The static analysis correctly flags error as unused. Additionally, the original error details are discarded, which could make debugging harder. Consider preserving the error context.
🛠️ 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 in
getUsername currently declares an unused error and discards the original error;
update the catch to capture the error (e.g., catch (error)) and preserve its
context when rethrowing or logging—for example include the original error
message or use the Error cause option when throwing a new error from getUsername
so you don't lose stack/message information while removing the unused-variable
warning.
2 participants
Benchmark PR from agentic-review-benchmarks#5
Summary by CodeRabbit
Release Notes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.