fix(companion): event type links for org user#5
Open
tomerqodo wants to merge 7 commits intoqodo_full_base_fixcompanion_event_type_links_for_org_user_pr5from
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 <>
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
+1655
to
+1661
| 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 any failure from getUserProfile() and throws a plain Error with a generic message, losing the original cause and any structured context. • This violates the requirement for hierarchical, typed custom errors with context, and will make downstream error handling/debugging inconsistent. • It also prevents callers from distinguishing auth/network failures vs. unexpected data issues.
Agent prompt
## Issue description
`getUsername()` throws a generic `Error("Failed to get username")` and drops the original exception/context, violating the requirement for hierarchical custom errors with typed codes and structured context.
## Issue Context
This function wraps `getUserProfile()` and is likely used in multiple user-facing flows; without typed codes/context (and ideally `cause`), downstream error handling and debugging becomes unreliable.
## 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
+257
to
+276
| {(() => { | ||
| // Parse bookingUrl to get domain prefix (e.g., "i.cal.com/" or "cal.com/username/") | ||
| if (props.bookingUrl) { | ||
| try { | ||
| const url = new URL(props.bookingUrl); | ||
| // Get path without the last segment (slug) | ||
| const pathParts = url.pathname.split("/").filter(Boolean); | ||
| pathParts.pop(); // Remove slug | ||
| // Compute prefix outside try/catch for React Compiler | ||
| let prefix = "/"; | ||
| if (pathParts.length > 0) { | ||
| prefix = `/${pathParts.join("/")}/`; | ||
| } | ||
| return `${url.protocol}//${url.hostname}${prefix}`; | ||
| } catch { | ||
| // fallback | ||
| } | ||
| } | ||
| return `cal.com/${props.username}/`; | ||
| })()} |
There was a problem hiding this comment.
2. Basicstab url parse ignores errors 📘 Rule violation ⛯ Reliability
• The inline booking URL prefix computation in BasicsTab silently ignores URL parsing errors
(catch {}), which can mask bad bookingUrl values.
• This is a silent failure at a known failure point (new URL(...)), reducing the ability to debug
edge cases in production.
• Prefer explicit validation and/or internal logging (redacting as needed) when parsing fails, then
fall back deterministically.
Agent prompt
## Issue description
`BasicsTab` parses `props.bookingUrl` with `new URL(...)` but swallows parsing failures without any logging/context, which is a silent failure.
## Issue Context
`bookingUrl` is likely API-provided; parsing failures should be diagnosable (internally) and handled explicitly.
## Fix Focus Areas
- companion/components/event-type-detail/tabs/BasicsTab.tsx[257-276]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+949
to
+953
| 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.
3. Detail link always missing 🐞 Bug ✓ Correctness
• EventTypeDetail now gates Preview/Copy on a local bookingUrl state, but that state is initialized to "" and never set from fetched EventType data. • This causes Preview and Copy Link to always show “Booking URL not available…” even when the event type has a valid slug/username. • Users lose core functionality (preview/copy) in the event type detail screen.
Agent prompt
### Issue description
`EventTypeDetail` uses a local `bookingUrl` state for preview/copy, but it is never set from the fetched event type. This makes preview/copy always fail.
### Issue Context
`CalComAPIService.getEventTypeById(...)` returns an `EventType` object which can contain `bookingUrl` (optional). Even when `bookingUrl` is missing, the screen already has enough info (username + slug) to build a fallback URL.
### Fix Focus Areas
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[174-183]
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[451-470]
- companion/app/(tabs)/(event-types)/event-type-detail.tsx[948-963]
### Suggested approach
- Prefer removing the separate `bookingUrl` state entirely and compute it via `useMemo` from `eventTypeData?.bookingUrl` with a fallback `https://cal.com/${username}/${eventSlug}` when possible.
- If keeping state, set it in `applyEventTypeData` (e.g., `setBookingUrl(eventType.bookingUrl ?? "")`) and/or add an effect to recompute when `username`/`eventSlug` change.
ⓘ 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.
4. Extension copies wrong url 🐞 Bug ✓ Correctness
• In the content script, Preview now uses eventType.bookingUrl || fallback, but the Copy button in
the same UI still copies only the hardcoded https://cal.com/{username}/{slug}.
• This creates an inconsistent UX (Preview opens one URL, Copy copies another) and breaks copy for
users on custom booking domains.
• A later section in the same file already uses the correct bookingUrl || fallback for copy,
suggesting this is an accidental missed update.
Agent prompt
### Issue description
The extension content script’s first event type UI block opens `eventType.bookingUrl` for Preview but copies a hardcoded `https://cal.com/...` URL for Copy. This breaks copying for custom domains and is inconsistent within the same UI.
### Issue Context
A later section in the same file already uses `eventType.bookingUrl || fallback` for copy, so the fix should align the first copy handler.
### Fix Focus Areas
- companion/extension/entrypoints/content.ts[1079-1087]
- companion/extension/entrypoints/content.ts[1122-1131]
### Suggested approach
Change the first copy handler’s `bookingUrl` assignment to:
```ts
const bookingUrl = eventType.bookingUrl || `https://cal.com/${eventType.users?.[0]?.username || "user"}/${eventType.slug}`;
```
so both Preview and Copy use the same source of truth.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+136
to
142
| 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!"); |
There was a problem hiding this comment.
5. Link actions too strict 🐞 Bug ⛯ Reliability
• EventTypes list screens (iOS and cross-platform) now hard-require eventType.bookingUrl and show an error when it’s missing. • bookingUrl is explicitly typed as optional, and other parts of the codebase handle its absence via fallback URL construction. • If the API omits bookingUrl in some responses, share/copy/preview will fail even though the URL can be derived from username + slug.
Agent prompt
### Issue description
EventTypes screens now require `eventType.bookingUrl`, but the field is optional. If it’s missing in API responses, users can’t share/copy/preview even though the URL can be derived.
### Issue Context
Other parts of the code (e.g., extension LinkedIn integration) treat `bookingUrl` as optional and build a fallback from username + slug.
### Fix Focus Areas
- companion/app/(tabs)/(event-types)/index.tsx[135-161]
- companion/app/(tabs)/(event-types)/index.tsx[287-301]
- companion/app/(tabs)/(event-types)/index.ios.tsx[113-139]
### Suggested approach
Option A (UI-level fallback):
- Replace the early `if (!eventType.bookingUrl) return` with:
- `const url = eventType.bookingUrl ?? derivedUrlFromUsernameAndSlug;`
- Only error if neither is possible.
Option B (data-level normalization):
- In the service layer or hook that returns event types, ensure `bookingUrl` is always populated (compute fallback once) so the UI can treat it as required.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 agentic-review-benchmarks#5