Annotations and moving of client functions#56
Conversation
📝 WalkthroughWalkthroughThis pull request reorganizes import paths to use a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
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.
| import { DropdownMenuItem } from "../ui/dropdown-menu"; | ||
| import { UserX } from "lucide-react"; | ||
| import { leaveTeamMutationClient as _unusedForNowleaveTeamMutationClientleaveTeamMutationClient } from "@/lib/queries"; | ||
| import { leaveTeamMutationClient as _unusedForNowleaveTeamMutationClientleaveTeamMutationClient } from "@/lib/functions/queries"; |
There was a problem hiding this comment.
Import alias has the original symbol name duplicated — appears to be a concatenation typo.
_unusedForNowleaveTeamMutationClientleaveTeamMutationClient is _unusedForNow + leaveTeamMutationClient (twice). The intended alias was likely _unusedForNowLeaveTeamMutationClient.
✏️ Proposed fix
-import { leaveTeamMutationClient as _unusedForNowleaveTeamMutationClientleaveTeamMutationClient } from "@/lib/functions/queries";
+import { leaveTeamMutationClient as _leaveTeamMutationClient } from "@/lib/functions/queries";📝 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.
| import { leaveTeamMutationClient as _unusedForNowleaveTeamMutationClientleaveTeamMutationClient } from "@/lib/functions/queries"; | |
| import { leaveTeamMutationClient as _leaveTeamMutationClient } from "@/lib/functions/queries"; |
🤖 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 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).
| export function isPublicRoute(pathname: string) { | ||
| return PUBLIC_ROUTES.includes(pathname); | ||
| } |
There was a problem hiding this comment.
isPublicRoute uses exact string match — pathname variants won't be recognized as public.
PUBLIC_ROUTES.includes(pathname) requires a character-perfect match. If pathname ever arrives with a trailing slash (e.g., /sign-in/) or query string appended, it will return false, treating a public page as protected. Consider normalizing before the check or using startsWith for path-prefix matching.
✏️ Proposed stricter normalization
export function isPublicRoute(pathname: string) {
- return PUBLIC_ROUTES.includes(pathname);
+ const normalized = pathname.split("?")[0].replace(/\/$/, "") || "/";
+ return PUBLIC_ROUTES.includes(normalized);
}📝 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.
| export function isPublicRoute(pathname: string) { | |
| return PUBLIC_ROUTES.includes(pathname); | |
| } | |
| export function isPublicRoute(pathname: string) { | |
| const normalized = pathname.split("?")[0].replace(/\/$/, "") || "/"; | |
| return PUBLIC_ROUTES.includes(normalized); | |
| } |
🤖 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 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.
| * @param inviteCode - The invite code for the team to join | ||
| * @returns An object containing the mutation key and mutation function for joining a team. | ||
| */ | ||
| export const joinTeamMutationclient = (inviteCode: string) => |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "joinTeamMutationclient" --type ts --type tsx -C2Repository: acmutsa/Fallback
Length of output: 87
🏁 Script executed:
rg "joinTeamMutationclient" -C2Repository: acmutsa/Fallback
Length of output: 471
Rename joinTeamMutationclient to joinTeamMutationClient for naming consistency.
The exported function has a lowercase c in client, inconsistent with the newly added leaveTeamMutationClient. The function is currently unused in the codebase, making this a safe rename with zero impact.
✏️ Proposed rename
-export const joinTeamMutationclient = (inviteCode: string) =>
+export const joinTeamMutationClient = (inviteCode: string) =>📝 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.
| export const joinTeamMutationclient = (inviteCode: string) => | |
| export const joinTeamMutationClient = (inviteCode: string) => |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/functions/queries.ts` at 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.
1 participant
Satisfies
#53
Summary by CodeRabbit
New Features
Documentation
Chores