fix(companion): event type links for org user #84
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <>
Code Review by Qodo
1. getUsername() throws generic Error
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1654
to
+1662
| // 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.
1. getusername() throws generic error 📘 Rule violation ⛯ Reliability
• getUsername() catches the underlying failure and throws a generic Error, which violates the requirement to use hierarchical custom errors with typed codes. • The thrown error also drops the original error (no cause), reducing debugging context and making failures harder to diagnose.
Agent prompt
## Issue description
`getUsername()` throws a generic `Error` and drops the original failure cause, violating the custom error hierarchy requirement and reducing debugging context.
## Issue Context
The compliance checklist requires custom errors with typed codes/status and structured context, and robust error handling should preserve the original failure details (e.g., as `cause`) for internal troubleshooting.
## Fix Focus Areas
- companion/services/calcom.ts[1654-1662]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
948
to
954
| const handlePreview = async () => { | ||
| const eventTypeSlug = eventSlug || "preview"; | ||
| let link: string; | ||
| try { | ||
| link = await CalComAPIService.buildEventTypeLink(eventTypeSlug); | ||
| } catch (error) { | ||
| safeLogError("Failed to generate preview link:", error); | ||
| showErrorAlert("Error", "Failed to generate preview link. Please try again."); | ||
| if (!bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available. Please save the event type first."); | ||
| return; | ||
| } | ||
| await openInAppBrowser(link, "event type preview"); | ||
| await openInAppBrowser(bookingUrl, "event type preview"); | ||
| }; |
There was a problem hiding this comment.
2. handlepreview lacks try/catch 📘 Rule violation ⛯ Reliability
• handlePreview() now calls openInAppBrowser(bookingUrl, ...) without any error handling. • If openInAppBrowser() rejects (e.g., invalid URL, platform/browser failure), the promise rejection can surface as an unhandled runtime error instead of showing a user-friendly error message.
Agent prompt
## Issue description
`handlePreview()` awaits `openInAppBrowser()` without any try/catch, risking unhandled promise rejections and missing debugging context.
## Issue Context
Per the robust error handling requirement, failure points should be caught and handled with actionable context and safe internal logging.
## Fix Focus Areas
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[948-954]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
956
to
963
| const handleCopyLink = async () => { | ||
| const eventTypeSlug = eventSlug || "event-link"; | ||
| let link: string; | ||
| try { | ||
| link = await CalComAPIService.buildEventTypeLink(eventTypeSlug); | ||
| } catch (error) { | ||
| safeLogError("Failed to copy link:", error); | ||
| showErrorAlert("Error", "Failed to copy link. Please try again."); | ||
| if (!bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available. Please save the event type first."); | ||
| return; | ||
| } | ||
| await Clipboard.setStringAsync(link); | ||
| await Clipboard.setStringAsync(bookingUrl); | ||
| showSuccessAlert("Success", "Link copied!"); | ||
| }; |
There was a problem hiding this comment.
3. handlecopylink lacks try/catch 📘 Rule violation ⛯ Reliability
• handleCopyLink() now calls Clipboard.setStringAsync(bookingUrl) without guarding against runtime failures. • Clipboard operations can fail (permissions/platform limitations), and without error handling the user may see no meaningful feedback and the app may produce unhandled errors.
Agent prompt
## Issue description
`handleCopyLink()` awaits a clipboard operation without error handling, and then shows success unconditionally.
## Issue Context
Clipboard writes can fail on some platforms; robust error handling requires catching failures and providing actionable context internally while showing safe user-facing messaging.
## Fix Focus Areas
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[956-963]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+11
to
+22
| function getDisplayUrl(bookingUrl?: string, username?: string, slug?: string): string { | ||
| if (bookingUrl) { | ||
| try { | ||
| const url = new URL(bookingUrl); | ||
| // Return domain + pathname (e.g., "i.cal.com/keith/30min") | ||
| return url.hostname + url.pathname; | ||
| } catch { | ||
| // fallback if URL parsing fails | ||
| } | ||
| } | ||
| return username ? `/${username}/${slug}` : `/${slug}`; | ||
| } |
There was a problem hiding this comment.
4. getdisplayurl swallows parse errors 📘 Rule violation ⛯ Reliability
• getDisplayUrl() silently ignores URL parsing failures with an empty catch, making malformed bookingUrl issues hard to diagnose. • This creates a silent failure mode where the UI falls back without any traceability, conflicting with robust edge-case management expectations.
Agent prompt
## Issue description
`getDisplayUrl()` uses an empty `catch` when parsing `bookingUrl`, creating a silent failure mode.
## Issue Context
Robust error handling requires explicit edge-case handling and avoiding swallowed exceptions without any traceability.
## Fix Focus Areas
- companion/components/event-type-list-item/EventTypeListItemParts.tsx[11-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
287
to
297
| const handlePreview = async (eventType: EventType) => { | ||
| if (!eventType.bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available for this event type."); | ||
| return; | ||
| } | ||
| try { | ||
| const link = await CalComAPIService.buildEventTypeLink(eventType.slug); | ||
| // Open in browser | ||
| if (Platform.OS === "web") { | ||
| window.open(link, "_blank"); | ||
| window.open(eventType.bookingUrl, "_blank"); | ||
| } else { | ||
| // For mobile, use in-app browser | ||
| await openInAppBrowser(link, "event type preview"); | ||
| await openInAppBrowser(eventType.bookingUrl, "event type preview"); | ||
| } |
There was a problem hiding this comment.
5. bookingurl opened without validation 📘 Rule violation ⛨ Security
• eventType.bookingUrl (API-provided input) is opened directly via window.open() / openInAppBrowser() without validating scheme/host. • This violates the security-first input validation requirement and can enable unsafe navigation if the value is malformed or attacker-controlled upstream.
Agent prompt
## Issue description
API-provided `bookingUrl` is opened directly without validation, which conflicts with the security-first input validation requirement.
## Issue Context
Values coming from APIs should be treated as external inputs. Before navigating/opening a URL, validate scheme/hostname (and consider safe `window.open` options on web).
## Fix Focus Areas
- companion/app/(tabs)/(event-types)/index.tsx[287-297]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+949
to
962
| if (!bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available. Please save the event type first."); | ||
| return; | ||
| } | ||
| await openInAppBrowser(link, "event type preview"); | ||
| await openInAppBrowser(bookingUrl, "event type preview"); | ||
| }; | ||
|
|
||
| const handleCopyLink = async () => { | ||
| const eventTypeSlug = eventSlug || "event-link"; | ||
| let link: string; | ||
| try { | ||
| link = await CalComAPIService.buildEventTypeLink(eventTypeSlug); | ||
| } catch (error) { | ||
| safeLogError("Failed to copy link:", error); | ||
| showErrorAlert("Error", "Failed to copy link. Please try again."); | ||
| if (!bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available. Please save the event type first."); | ||
| return; | ||
| } | ||
| await Clipboard.setStringAsync(link); | ||
| await Clipboard.setStringAsync(bookingUrl); | ||
| showSuccessAlert("Success", "Link copied!"); |
There was a problem hiding this comment.
6. Bookingurl state never set 🐞 Bug ✓ Correctness
• EventTypeDetail now gates Preview/Copy on a local bookingUrl state, but that state is initialized to an empty string and never updated from fetched event type data. • This makes preview/copy always fail with an error even for existing event types, and also passes an empty bookingUrl into BasicsTab. • The rest of the component fetch path only calls applyEventTypeData(eventType), which does not set bookingUrl, so the state remains empty indefinitely.
Agent prompt
### Issue description
`EventTypeDetail` introduced a `bookingUrl` state that is required for preview/copy, but it is never set after initialization. This breaks preview/copy (always errors) and passes an empty bookingUrl into the Basics tab.
### Issue Context
- `bookingUrl` is optional on `EventType`, so even when present it must be copied into state explicitly.
- The component already fetches the `EventType` and applies it via `applyEventTypeData`, which is the right place to set bookingUrl.
### Fix Focus Areas
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[179-183]
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[451-477]
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[826-840]
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[948-963]
- companion/components/event-type-detail/tabs/BasicsTab.tsx[17-41]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+136
to
157
| if (!eventType.bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available for this event type."); | ||
| return; | ||
| } | ||
| try { | ||
| const link = await CalComAPIService.buildEventTypeLink(eventType.slug); | ||
| await Clipboard.setStringAsync(link); | ||
|
|
||
| await Clipboard.setStringAsync(eventType.bookingUrl); | ||
| showSuccessAlert("Link Copied", "Event type link copied!"); | ||
| } catch { | ||
| showErrorAlert("Error", "Failed to copy link. Please try again."); | ||
| } | ||
| }; | ||
|
|
||
| const _handleShare = async (eventType: EventType) => { | ||
| if (!eventType.bookingUrl) { | ||
| showErrorAlert("Error", "Booking URL not available for this event type."); | ||
| return; | ||
| } | ||
| try { | ||
| const link = await CalComAPIService.buildEventTypeLink(eventType.slug); | ||
| await Share.share({ | ||
| message: `Book a meeting: ${eventType.title}`, | ||
| url: link, | ||
| url: eventType.bookingUrl, | ||
| }); |
There was a problem hiding this comment.
7. Actions require bookingurl 🐞 Bug ✓ Correctness
• EventTypes list screens now refuse to copy/share/preview unless eventType.bookingUrl exists,
showing an error otherwise.
• bookingUrl is explicitly optional in the EventType type and the client service does not derive
a fallback URL, so this can regress functionality whenever the API omits it.
• This creates an inconsistent UX vs other places (e.g., extension code) that still fall back to
constructing https://cal.com/{username}/{slug}.
Agent prompt
### Issue description
Event type list actions (copy/share/preview) now hard-require `eventType.bookingUrl`, but the type marks it optional and the API client does not synthesize it. This can block users from copying/sharing/previewing links.
### Issue Context
Other parts of the codebase (e.g., extension integration) still use `bookingUrl || https://cal.com/{username}/{slug}` patterns, indicating `bookingUrl` may be absent.
### Fix Focus Areas
- companion/app/(tabs)/(event-types)/index.tsx[135-161]
- companion/app/(tabs)/(event-types)/index.ios.tsx[113-139]
- companion/services/types/event-types.types.ts[279-284]
- companion/services/calcom.ts[864-926]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
1122
to
1129
| copyBtn.addEventListener("click", (e) => { | ||
| e.stopPropagation(); | ||
| // Copy to clipboard | ||
| const bookingUrl = `https://cal.com/${ | ||
| eventType.users?.[0]?.username || "user" | ||
| }/${eventType.slug}`; | ||
| const bookingUrl = | ||
| `https://cal.com/${ | ||
| eventType.users?.[0]?.username || "user" | ||
| }/${eventType.slug}`; | ||
| navigator.clipboard |
There was a problem hiding this comment.
8. Copy ignores bookingurl 🐞 Bug ✓ Correctness
• In the extension content script, the Preview button uses eventType.bookingUrl with a fallback, but the Copy button always constructs a https://cal.com/... URL. • When bookingUrl is present (e.g., a different domain like i.cal.com), copy will put the wrong URL on the clipboard while preview opens the correct one. • Other copy/insert paths in the same file already use the correct bookingUrl || fallback behavior, so this appears to be an inconsistent spot.
Agent prompt
### Issue description
One copy button handler in the extension ignores `eventType.bookingUrl`, causing copied links to differ from preview/open behavior when bookingUrl is present.
### Issue Context
The same file already uses the desired fallback pattern in other locations, so align this handler to the existing pattern.
### Fix Focus Areas
- companion/extension/entrypoints/content.ts[1079-1131]
- companion/extension/entrypoints/content.ts[1868-1876]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#706