Annotations and moving of client functions#56
Merged
christianhelp merged 4 commits intomainfrom Mar 1, 2026
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR moves many utility and query imports into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/shared/LeaveTeamDialog.tsx (2)
22-34:⚠️ Potential issue | 🟡 Minor
roleis declared in the props type but never destructured or used.Callers are required to supply
role(TypeScript will enforce it), but the value is silently discarded. Either destructure and use it, or remove it from the type until it's needed.✏️ Proposed fix
export function LeaveTeamDialog({ teamId: _unusedForNowTeamId, teamName, isPrivate, + role: _role, }: { teamId: string; teamName: string; isPrivate: boolean; role: string; ... })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/shared/LeaveTeamDialog.tsx` around lines 22 - 34, The props type for LeaveTeamDialog declares role but the function signature currently ignores it (teamId: _unusedForNowTeamId, teamName, isPrivate) causing callers to pass a value that's discarded; either remove role from the props type to stop forcing callers to provide it, or include role in the parameter destructuring (e.g., add role to the parameter list) and use it where appropriate (for example to conditionally render owner-specific warnings or behavior in LeaveTeamDialog); update the props object shape accordingly so the type and actual destructuring are consistent.
56-56:⚠️ Potential issue | 🔴 Critical
AlertDialogActionfires no mutation — the "Leave Team" button is a no-op.Both
leaveTeamMutationClientanduseMutationare imported but aliased as_unusedForNow*and are never wired up. Clicking "Leave Team" will close the dialog without calling the API, silently doing nothing to the user's team membership.At minimum, the action should be disabled until the implementation is hooked up, to prevent user confusion:
🛡️ Proposed interim fix to make the incomplete state explicit
-<AlertDialogAction>Leave Team</AlertDialogAction> +<AlertDialogAction disabled>Leave Team</AlertDialogAction>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/shared/LeaveTeamDialog.tsx` at line 56, The "Leave Team" button (<AlertDialogAction>) is currently a no-op because the mutation utilities (leaveTeamMutationClient and useMutation) were aliased as _unusedForNow* and never invoked; either wire the real mutation or disable the action until implemented. Fix by locating AlertDialogAction in LeaveTeamDialog and either (A) hook up the leaveTeamMutationClient/useMutation: call the mutation (e.g., leaveTeamMutationClient.mutate or useMutation(...).mutate) in the button handler, pass the team/user id, and reflect loading/error states on AlertDialogAction, or (B) if the API is not ready, disable AlertDialogAction (set disabled when leaveTeamMutationClient/useMutation is not configured) and show a tooltip or label indicating the action is not yet available; ensure references to leaveTeamMutationClient, useMutation, and AlertDialogAction are used so the compiler/runtime will surface any missing wiring.
🧹 Nitpick comments (1)
apps/web/src/lib/functions/auth.ts (1)
27-38:getSessionandsignOutare pass-through wrappers with no added behavior.Both functions add an indirection layer without extra logic, error transformation, or abstraction.
UserButton.tsxalready callsauthClient.signOut()directly. Consider whether these wrappers are actually needed as part of the public API, or if they'll cause callers to split between two call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/functions/auth.ts` around lines 27 - 38, The getSession and signOut functions are simple pass-through wrappers (authClient.getSession and authClient.signOut) that add no value and risk inconsistent call sites; either remove these wrappers and update callers (e.g., UserButton.tsx and any other usage) to call authClient.getSession()/authClient.signOut() directly, or if a public API surface is desired, implement meaningful behavior inside getSession/signOut (for example, normalize return types, centralize error handling/logging, or add typings and documentation) so the indirection is justified; locate the functions by name (getSession, signOut) in apps/web/src/lib/functions/auth.ts and apply one of the two remediation strategies consistently across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/shared/LeaveTeamDialog.tsx`:
- Line 15: The import alias in the LeaveTeamDialog component is a concatenation
typo: the symbol currently reads
_unusedForNowleaveTeamMutationClientleaveTeamMutationClient; update the import
so the alias is the intended readable name (e.g.,
_unusedForNowLeaveTeamMutationClient) while still importing
leaveTeamMutationClient from "@/lib/functions/queries" so other references to
the alias remain correct (locate the import line that declares
leaveTeamMutationClient and replace the malformed alias with
_unusedForNowLeaveTeamMutationClient).
In `@apps/web/src/lib/functions/auth.ts`:
- Around line 10-12: isPublicRoute currently does an exact match
(PUBLIC_ROUTES.includes(pathname)) so variants like trailing slashes or
query/hash strings will fail; modify isPublicRoute to normalize the incoming
pathname (strip query and hash, collapse trailing slashes) before checking, or
use prefix matching for routes that should match subpaths. Specifically, create
a small normalization step inside isPublicRoute (e.g., remove anything after '?'
or '#', then remove trailing slashes) and then compare against PUBLIC_ROUTES (or
use startsWith against entries intended as prefixes) so '/sign-in/',
'/sign-in?x=1', and '/sign-in#foo' are recognized as public.
In `@apps/web/src/lib/functions/queries.ts`:
- Line 59: Rename the exported function joinTeamMutationclient to
joinTeamMutationClient to match the camelCase convention used by
leaveTeamMutationClient; update the function declaration/export name and any
local references/imports that use joinTeamMutationclient so they reference
joinTeamMutationClient instead, and run a quick repo-wide search to ensure no
remaining references to the old symbol remain.
---
Outside diff comments:
In `@apps/web/src/components/shared/LeaveTeamDialog.tsx`:
- Around line 22-34: The props type for LeaveTeamDialog declares role but the
function signature currently ignores it (teamId: _unusedForNowTeamId, teamName,
isPrivate) causing callers to pass a value that's discarded; either remove role
from the props type to stop forcing callers to provide it, or include role in
the parameter destructuring (e.g., add role to the parameter list) and use it
where appropriate (for example to conditionally render owner-specific warnings
or behavior in LeaveTeamDialog); update the props object shape accordingly so
the type and actual destructuring are consistent.
- Line 56: The "Leave Team" button (<AlertDialogAction>) is currently a no-op
because the mutation utilities (leaveTeamMutationClient and useMutation) were
aliased as _unusedForNow* and never invoked; either wire the real mutation or
disable the action until implemented. Fix by locating AlertDialogAction in
LeaveTeamDialog and either (A) hook up the leaveTeamMutationClient/useMutation:
call the mutation (e.g., leaveTeamMutationClient.mutate or
useMutation(...).mutate) in the button handler, pass the team/user id, and
reflect loading/error states on AlertDialogAction, or (B) if the API is not
ready, disable AlertDialogAction (set disabled when
leaveTeamMutationClient/useMutation is not configured) and show a tooltip or
label indicating the action is not yet available; ensure references to
leaveTeamMutationClient, useMutation, and AlertDialogAction are used so the
compiler/runtime will surface any missing wiring.
---
Nitpick comments:
In `@apps/web/src/lib/functions/auth.ts`:
- Around line 27-38: The getSession and signOut functions are simple
pass-through wrappers (authClient.getSession and authClient.signOut) that add no
value and risk inconsistent call sites; either remove these wrappers and update
callers (e.g., UserButton.tsx and any other usage) to call
authClient.getSession()/authClient.signOut() directly, or if a public API
surface is desired, implement meaningful behavior inside getSession/signOut (for
example, normalize return types, centralize error handling/logging, or add
typings and documentation) so the indirection is justified; locate the functions
by name (getSession, signOut) in apps/web/src/lib/functions/auth.ts and apply
one of the two remediation strategies consistently across the codebase.
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
1 participant
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.
Satisfies
#53
Summary by CodeRabbit
New Features
Documentation
Chores