-
Notifications
You must be signed in to change notification settings - Fork 5
afix: improve team invite acceptance reliability #233
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
base: master
Are you sure you want to change the base?
Conversation
- Add user-friendly error messages based on error type - Allow manual retry for failed invite acceptance attempts - Add detailed logging for debugging invite flow - Show retry hint for retryable errors only 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds expanded accept-invite flow in the team invite route: logs lifecycle events, introduces success state with timed redirect, invalidates queries on success, and implements multi-branch error handling and structured error display. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Invite Page UI
participant SVC as Accept Invite Handler
participant API as Backend API
participant QC as Query Cache
participant NAV as Router
User->>UI: Click "Accept Invitation"
UI->>SVC: acceptInvite(inviteId)
Note right of SVC: Log: starting accept with inviteId
SVC->>API: POST /invites/{inviteId}/accept
alt Success
API-->>SVC: 200 OK
SVC->>QC: Invalidate team/user queries
SVC-->>UI: setSuccess(true)
Note right of UI: Show success message
UI->>NAV: Redirect after short delay
else Error
API-->>SVC: Error (401/already member/invalid/expired/network/other)
SVC-->>UI: Map error -> message
Note right of UI: Show structured error + conditional retry hint
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
Deploying maple with
|
| Latest commit: |
8facb6c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b68ee7a4.maple-ca8.pages.dev |
| Branch Preview URL: | https://fix-team-invite-acceptance.maple-ca8.pages.dev |
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.
Greptile Summary
This PR improves the team invite acceptance flow by implementing comprehensive error handling and user experience enhancements in the team invite route component. The changes focus on replacing generic error messages with specific, actionable feedback based on different error scenarios.
The implementation categorizes errors using string matching on error messages to distinguish between:
- Session expiration (401/unauthorized errors) - prompts user to refresh
- Already a member scenarios - informs user they're already part of the team
- Invalid or expired invitation links - indicates the link is no longer valid
- Network connectivity issues - suggests checking connection
- Generic fallback handling for other error types
The PR also adds intelligent retry guidance by conditionally showing retry hints only for errors that are actually retryable (excluding "already a member" and "no longer valid" scenarios). Additionally, detailed console logging has been added throughout the invite acceptance flow to improve debugging capabilities for this critical user onboarding process.
This change integrates well with the existing React component structure, utilizing the existing error state management while enhancing the user feedback mechanism. The error categorization approach allows the frontend to provide meaningful responses without requiring backend API changes.
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it only improves error handling and user messaging
- Score reflects solid implementation of user experience improvements with defensive error handling patterns
- Pay close attention to the string matching logic in the error categorization section to ensure all expected error formats are covered
1 file reviewed, no comments
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/routes/team.invite.$inviteId.tsx (2)
31-42: Fix: query errors are shown as “Invalid or Expired” instead of a retryable errorWhen
checkTeamInvitethrows (network/5xx/init failure),inviteDatais undefined and the UI falls into the “Invalid or Expired” branch. HandleisErrorexplicitly and offer a retry.Apply this diff:
@@ - const { data: inviteData, isLoading: checkingInvite } = useQuery<CheckInviteResponse>({ + const { + data: inviteData, + isLoading: checkingInvite, + isError: inviteLoadError, + error: inviteError, + refetch: refetchInvite + } = useQuery<CheckInviteResponse>({ @@ }); @@ - // Invalid or expired invite - if (!inviteData?.valid) { + // Error while checking invite (e.g., network/server) + if (inviteLoadError) { + return ( + <> + <TopNav /> + <FullPageMain> + <div className="flex items-center justify-center min-h-[60vh]"> + <Card className="w-full max-w-md"> + <CardHeader> + <div className="flex items-center justify-center mb-4"> + <div className="rounded-full bg-destructive/10 p-3"> + <AlertCircle className="h-8 w-8 text-destructive" /> + </div> + </div> + <CardTitle className="text-center">Unable to verify invitation</CardTitle> + <CardDescription className="text-center"> + {(inviteError as Error)?.message || "A network or server error occurred."} + </CardDescription> + </CardHeader> + <CardContent> + <Button onClick={() => refetchInvite()} className="w-full"> + Try Again + </Button> + </CardContent> + </Card> + </div> + </FullPageMain> + </> + ); + } + + // Invalid or expired invite + if (!inviteData?.valid) {Also applies to: 133-162
23-26: Capture HTTP status in API errors for deterministic retry logic
acceptTeamInvite throws a plain Error with only a message, soe.statuswill always be undefined and status-based retry checks won’t work. Define a custom error class (e.g. ApiError extends Error) or attachstatusto the Error infrontend/src/billing/billingApi.tsbefore eachthrow, then update the catch block infrontend/src/routes/team.invite.$inviteId.tsxto usee.status(and/ore.code) for computingretryable.
🧹 Nitpick comments (5)
frontend/src/routes/team.invite.$inviteId.tsx (5)
79-82: Add cleanup for the redirect timeout to prevent leaks on unmountStore the timeout id and clear it on unmount. Also prefer
replace: trueon the post-join redirect.Apply this diff:
@@ -import { useState, useEffect } from "react"; +import { useState, useEffect, useRef } from "react"; @@ const [success, setSuccess] = useState(false); + const redirectTimeoutRef = useRef<number | null>(null); @@ - setTimeout(() => { - navigate({ to: "/" }); - }, 2000); + redirectTimeoutRef.current = window.setTimeout(() => { + navigate({ to: "/", replace: true }); + }, 2000);Additionally, add this cleanup effect (near other hooks):
useEffect(() => { return () => { if (redirectTimeoutRef.current) { window.clearTimeout(redirectTimeoutRef.current); } }; }, []);Also applies to: 1-1, 25-26
47-53: Use history replace on redirectsPrevents users from navigating “back” into transient invite pages.
Apply this diff:
@@ - navigate({ - to: "/signup", - search: { - next: `/team/invite/${inviteId}` - } - }); + navigate({ + to: "/signup", + search: { next: `/team/invite/${inviteId}` }, + replace: true + }); @@ - navigate({ to: "/" }); + navigate({ to: "/", replace: true });Also applies to: 80-81
73-75: Also invalidate the invite query on successKeeps local cache consistent if the user navigates back before redirect fires.
Apply this diff:
await queryClient.invalidateQueries({ queryKey: ["teamStatus"] }); await queryClient.invalidateQueries({ queryKey: ["billingStatus"] }); + await queryClient.invalidateQueries({ queryKey: ["teamInvite", inviteId] });
58-58: Line-length and copy nitsA few strings likely exceed the 100-char limit; consider extracting to named constants to satisfy formatting. Also, minor copy tweaks are optional (“This invitation link is no longer valid. It may have expired or already been used.” could be split over two sentences in code as a constant).
Also applies to: 149-151, 180-181
180-181: Optional: normalize API fields to camelCase at the edge
team_nameandinvited_by_nameviolate local camelCase conventions. Consider mapping the response to{ teamName, invitedByName }via a DTO or Zod transform and updating references. Not blocking.Also applies to: 213-224
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/routes/team.invite.$inviteId.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use 2-space indentation, double quotes, and a 100-character line limit for formatting
Use camelCase for variable and function names
Use try/catch with specific error types for error handling
Files:
frontend/src/routes/team.invite.$inviteId.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript typing and avoid
anywhen possible
Files:
frontend/src/routes/team.invite.$inviteId.tsx
🧬 Code graph analysis (1)
frontend/src/routes/team.invite.$inviteId.tsx (2)
frontend/src/billing/billingService.ts (1)
getBillingService(203-208)frontend/src/components/ui/alert.tsx (1)
AlertDescription(49-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-ios
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: Cloudflare Pages
| setError(null); | ||
|
|
||
| try { | ||
| console.log("[TeamInvite] Attempting to accept invite:", inviteId); |
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.
Do not log the invite token; treat it as sensitive
Invite IDs often function as bearer tokens. Avoid printing them to console.
Apply this diff:
- console.log("[TeamInvite] Attempting to accept invite:", inviteId);
+ console.log("[TeamInvite] Attempting to accept invite");
@@
- console.error("[TeamInvite] Failed to accept invite:", err);
+ console.error("[TeamInvite] Failed to accept invite", err);Also applies to: 83-83
🤖 Prompt for AI Agents
In frontend/src/routes/team.invite.$inviteId.tsx around lines 66 and 83, the
code logs the inviteId to the console which exposes a sensitive token; remove or
redact any console.log or other debug statements that print inviteId and instead
log non-sensitive context (e.g., "Attempting to accept invite" without the
token) or use conditional/local-only debugging that never prints the token in
production; ensure both occurrences at lines 66 and 83 are updated accordingly.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes