From ca1b5f04dec5725d40961cbedf3d3dce10d22fc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 05:18:57 +0000 Subject: [PATCH 1/3] refactor(groups): split useUserGroups' {all} boolean into two query modules fetchUserGroups(userId, { all }) flipped both the auth model (admin-gated) and the is_member semantics behind one flag. Splits it into useMyGroupsQuery (always member-scoped, is_member always true) and useAllGroupsQuery (admin view, falling back to the member-only result for non-admins). The Groups page's My/All toggle now renders one of two sibling components instead of threading the boolean into a shared hook. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015AWdzYUiNjQwtASYbWTnaX --- src/api/groups/types.ts | 4 +- .../groups/useAllGroups.integration.test.ts | 51 ++++++ src/api/groups/useAllGroups.ts | 62 ++++++++ src/api/groups/useCreateGroup.ts | 5 +- src/api/groups/useDeleteGroup.ts | 5 +- src/api/groups/useJoinGroup.ts | 5 +- src/api/groups/useLeaveGroup.ts | 5 +- .../groups/useMyGroups.integration.test.ts | 79 ++++++++++ src/api/groups/useMyGroups.ts | 95 ++++++++++++ src/api/groups/useUserGroups.ts | 145 ------------------ .../layout/AppHeader/GroupsIndicator.tsx | 5 +- src/contexts/ActiveScopeContext.tsx | 4 +- .../tabs/VoteTab/AuthedFilteredSetsPanel.tsx | 5 +- src/pages/SetDetails/SetGroupVoting.tsx | 5 +- src/pages/Settings/SettingsPage.tsx | 4 +- src/routes/__root.tsx | 6 +- src/routes/groups/index.tsx | 44 +++++- src/test/integration/fixtures/adminRoles.ts | 12 +- src/test/integration/fixtures/groups.ts | 47 ++++++ 19 files changed, 403 insertions(+), 185 deletions(-) create mode 100644 src/api/groups/useAllGroups.integration.test.ts create mode 100644 src/api/groups/useAllGroups.ts create mode 100644 src/api/groups/useMyGroups.integration.test.ts create mode 100644 src/api/groups/useMyGroups.ts delete mode 100644 src/api/groups/useUserGroups.ts create mode 100644 src/test/integration/fixtures/groups.ts diff --git a/src/api/groups/types.ts b/src/api/groups/types.ts index 98268a938..efa1848e4 100644 --- a/src/api/groups/types.ts +++ b/src/api/groups/types.ts @@ -18,8 +18,8 @@ export type GroupMember = export const groupsKeys = { all: ["groups"] as const, - user: (userId: string, params: unknown = {}) => - [...groupsKeys.all, "user", userId, params] as const, + myGroups: (userId: string) => [...groupsKeys.all, "my", userId] as const, + allGroups: (userId: string) => [...groupsKeys.all, "all", userId] as const, details: () => [...groupsKeys.all, "detail"] as const, detail: (groupId: string) => [...groupsKeys.details(), groupId] as const, bySlug: () => [...groupsKeys.all, "by-slug"] as const, diff --git a/src/api/groups/useAllGroups.integration.test.ts b/src/api/groups/useAllGroups.integration.test.ts new file mode 100644 index 000000000..8fbb98f03 --- /dev/null +++ b/src/api/groups/useAllGroups.integration.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { useQuery } from "@tanstack/react-query"; +import { allGroupsQuery } from "./useAllGroups"; +import { createQueryWrapper } from "@/test/integration/harness"; +import { signInAsTestUser } from "@/test/integration/fixtures/auth"; +import { grantAdminRole } from "@/test/integration/fixtures/adminRoles"; +import { + addGroupMember, + createGroup, +} from "@/test/integration/fixtures/groups"; + +describe("allGroupsQuery", () => { + it("falls back to member-only results for a non-admin caller", async () => { + const userId = await signInAsTestUser(); + const myGroupId = await createGroup(userId); + await addGroupMember(myGroupId, userId); + // A group the caller has no membership row in at all. + const otherGroupId = await createGroup(crypto.randomUUID()); + await addGroupMember(otherGroupId, crypto.randomUUID()); + + const { result } = renderHook(() => useQuery(allGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data?.map((group) => group.id)).toEqual([myGroupId]); + }); + + it("returns every non-archived group for a super-admin caller, with is_member computed per group", async () => { + const userId = await signInAsTestUser(); + await grantAdminRole(userId, "super_admin"); + const myGroupId = await createGroup(userId); + await addGroupMember(myGroupId, userId); + const otherGroupId = await createGroup(crypto.randomUUID()); + await addGroupMember(otherGroupId, crypto.randomUUID()); + + const { result } = renderHook(() => useQuery(allGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + const myGroup = result.current.data?.find((g) => g.id === myGroupId); + const otherGroup = result.current.data?.find((g) => g.id === otherGroupId); + expect(myGroup?.is_member).toBe(true); + expect(otherGroup).toBeDefined(); + expect(otherGroup?.is_member).toBe(false); + }); +}); diff --git a/src/api/groups/useAllGroups.ts b/src/api/groups/useAllGroups.ts new file mode 100644 index 000000000..fc599b354 --- /dev/null +++ b/src/api/groups/useAllGroups.ts @@ -0,0 +1,62 @@ +import { queryOptions, useSuspenseQuery } from "@tanstack/react-query"; +import { supabase } from "@/integrations/supabase/client"; +import type { Group } from "./types"; +import { groupsKeys } from "./types"; +import { attachGroupMeta, fetchMyGroups, getUserGroupIds } from "./useMyGroups"; + +export function allGroupsQuery(userId: string) { + return queryOptions({ + queryKey: groupsKeys.allGroups(userId), + queryFn: () => fetchAllGroups(userId), + }); +} + +export function useAllGroupsQuery(userId: string) { + return useSuspenseQuery(allGroupsQuery(userId)); +} + +/** + * Every non-archived group, for an admin caller. A non-admin caller falls + * back to exactly the member-only result (never an error, never a leaked + * full group list). + */ +async function fetchAllGroups(userId: string): Promise { + const isAdmin = await isUserAdmin(userId); + + if (!isAdmin) { + return fetchMyGroups(userId); + } + + const [userGroupIds, { data: groupsData, error }] = await Promise.all([ + getUserGroupIds(userId), + supabase + .from("groups") + .select("*") + .eq("archived", false) + .order("created_at", { ascending: false }), + ]); + + if (error) { + throw new Error(error.message || "Failed to fetch groups"); + } + + return attachGroupMeta(groupsData || [], userId, { + alwaysMember: false, + memberGroupIds: userGroupIds, + }); +} + +async function isUserAdmin(userId: string): Promise { + const { data: isAdminData, error } = await supabase + .from("admin_roles") + .select("id") + .eq("user_id", userId) + .limit(1); + + if (error) { + console.error("Error checking admin role:", error); + return false; + } + + return isAdminData && isAdminData.length > 0; +} diff --git a/src/api/groups/useCreateGroup.ts b/src/api/groups/useCreateGroup.ts index 2a5f2e9fd..4f16eadd2 100644 --- a/src/api/groups/useCreateGroup.ts +++ b/src/api/groups/useCreateGroup.ts @@ -55,10 +55,7 @@ export function useCreateGroupMutation() { return useMutation({ mutationFn: createGroup, - onSuccess: (_data, variables) => { - queryClient.invalidateQueries({ - queryKey: groupsKeys.user(variables.userId), - }); + onSuccess: () => { queryClient.invalidateQueries({ queryKey: groupsKeys.all, }); diff --git a/src/api/groups/useDeleteGroup.ts b/src/api/groups/useDeleteGroup.ts index c8f0103d8..981517033 100644 --- a/src/api/groups/useDeleteGroup.ts +++ b/src/api/groups/useDeleteGroup.ts @@ -27,11 +27,8 @@ export function useDeleteGroupMutation() { return useMutation({ mutationFn: deleteGroup, - onSuccess: (_data, variables) => { + onSuccess: () => { // Invalidate all group-related queries - queryClient.invalidateQueries({ - queryKey: groupsKeys.user(variables.userId), - }); queryClient.invalidateQueries({ queryKey: groupsKeys.all, }); diff --git a/src/api/groups/useJoinGroup.ts b/src/api/groups/useJoinGroup.ts index f2bc35a94..8c5368b1e 100644 --- a/src/api/groups/useJoinGroup.ts +++ b/src/api/groups/useJoinGroup.ts @@ -28,7 +28,10 @@ export function useJoinGroupMutation() { mutationFn: joinGroup, onSuccess: (_data, variables) => { queryClient.invalidateQueries({ - queryKey: groupsKeys.user(variables.userId), + queryKey: groupsKeys.myGroups(variables.userId), + }); + queryClient.invalidateQueries({ + queryKey: groupsKeys.allGroups(variables.userId), }); toast({ title: "Success", diff --git a/src/api/groups/useLeaveGroup.ts b/src/api/groups/useLeaveGroup.ts index 8059c2811..0ce9740c5 100644 --- a/src/api/groups/useLeaveGroup.ts +++ b/src/api/groups/useLeaveGroup.ts @@ -29,7 +29,10 @@ export function useLeaveGroupMutation() { mutationFn: leaveGroup, onSuccess: (_data, variables) => { queryClient.invalidateQueries({ - queryKey: groupsKeys.user(variables.userId), + queryKey: groupsKeys.myGroups(variables.userId), + }); + queryClient.invalidateQueries({ + queryKey: groupsKeys.allGroups(variables.userId), }); toast({ title: "Success", diff --git a/src/api/groups/useMyGroups.integration.test.ts b/src/api/groups/useMyGroups.integration.test.ts new file mode 100644 index 000000000..0bc8fb07a --- /dev/null +++ b/src/api/groups/useMyGroups.integration.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { useQuery } from "@tanstack/react-query"; +import { myGroupsQuery } from "./useMyGroups"; +import { createQueryWrapper } from "@/test/integration/harness"; +import { signInAsTestUser } from "@/test/integration/fixtures/auth"; +import { + addGroupMember, + createGroup, +} from "@/test/integration/fixtures/groups"; + +describe("myGroupsQuery", () => { + it("returns only the groups the caller is a member of", async () => { + const userId = await signInAsTestUser(); + const myGroupId = await createGroup(userId); + await addGroupMember(myGroupId, userId); + // A group the caller has no membership row in at all. + const otherGroupId = await createGroup(crypto.randomUUID()); + await addGroupMember(otherGroupId, crypto.randomUUID()); + + const { result } = renderHook(() => useQuery(myGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data?.map((group) => group.id)).toEqual([myGroupId]); + }); + + it("marks is_member true and is_creator correctly, without an admin check", async () => { + const userId = await signInAsTestUser(); + const ownGroupId = await createGroup(userId); + await addGroupMember(ownGroupId, userId); + const joinedGroupId = await createGroup(crypto.randomUUID()); + await addGroupMember(joinedGroupId, userId); + + const { result } = renderHook(() => useQuery(myGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + const ownGroup = result.current.data?.find((g) => g.id === ownGroupId); + const joinedGroup = result.current.data?.find( + (g) => g.id === joinedGroupId, + ); + expect(ownGroup).toMatchObject({ is_member: true, is_creator: true }); + expect(joinedGroup).toMatchObject({ is_member: true, is_creator: false }); + }); + + it("reflects every member in member_count, not just the caller", async () => { + const userId = await signInAsTestUser(); + const groupId = await createGroup(userId); + await addGroupMember(groupId, userId); + await addGroupMember(groupId, crypto.randomUUID()); + + const { result } = renderHook(() => useQuery(myGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect( + result.current.data?.find((g) => g.id === groupId)?.member_count, + ).toBe(2); + }); + + it("returns an empty array for a user with no groups", async () => { + const userId = await signInAsTestUser(); + + const { result } = renderHook(() => useQuery(myGroupsQuery(userId)), { + wrapper: createQueryWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toEqual([]); + }); +}); diff --git a/src/api/groups/useMyGroups.ts b/src/api/groups/useMyGroups.ts new file mode 100644 index 000000000..8e0bca2af --- /dev/null +++ b/src/api/groups/useMyGroups.ts @@ -0,0 +1,95 @@ +import { queryOptions, useSuspenseQuery } from "@tanstack/react-query"; +import { supabase } from "@/integrations/supabase/client"; +import type { Group } from "./types"; +import { groupsKeys } from "./types"; + +export function myGroupsQuery(userId: string) { + return queryOptions({ + queryKey: groupsKeys.myGroups(userId), + queryFn: () => fetchMyGroups(userId), + }); +} + +export function useMyGroupsQuery(userId: string) { + return useSuspenseQuery(myGroupsQuery(userId)); +} + +/** Groups the given user is a member of. `is_member` is always `true` — no admin check. */ +export async function fetchMyGroups(userId: string): Promise { + const userGroupIds = await getUserGroupIds(userId); + + if (userGroupIds.length === 0) { + return []; + } + + const { data: groupsData, error } = await supabase + .from("groups") + .select("*") + .eq("archived", false) + .in("id", userGroupIds) + .order("created_at", { ascending: false }); + + if (error) { + throw new Error(error.message || "Failed to fetch groups"); + } + + return attachGroupMeta(groupsData || [], userId, { alwaysMember: true }); +} + +export async function getUserGroupIds(userId: string): Promise { + const { data: userGroups, error } = await supabase + .from("group_members") + .select("group_id") + .eq("user_id", userId); + + if (error) { + console.error("Error fetching user groups:", error); + throw new Error("Failed to fetch user groups"); + } + + return userGroups?.map((ug) => ug.group_id) || []; +} + +export async function attachGroupMeta( + groups: Group[], + userId: string, + { + alwaysMember, + memberGroupIds, + }: { alwaysMember: boolean; memberGroupIds?: string[] }, +): Promise { + const memberCountsByGroupId = await fetchMemberCountsByGroupId( + groups.map((group) => group.id), + ); + + return groups.map((group) => ({ + ...group, + member_count: memberCountsByGroupId.get(group.id) || 0, + is_creator: group.created_by === userId, + is_member: alwaysMember || (memberGroupIds?.includes(group.id) ?? false), + })); +} + +async function fetchMemberCountsByGroupId( + groupIds: string[], +): Promise> { + const memberCountsByGroupId = new Map(); + + if (groupIds.length === 0) { + return memberCountsByGroupId; + } + + const { data: counts, error } = await supabase.rpc("group_member_counts", { + p_group_ids: groupIds, + }); + + if (error) { + throw new Error("Failed to fetch group member counts"); + } + + for (const row of counts || []) { + memberCountsByGroupId.set(row.group_id, row.member_count); + } + + return memberCountsByGroupId; +} diff --git a/src/api/groups/useUserGroups.ts b/src/api/groups/useUserGroups.ts deleted file mode 100644 index 632de3c67..000000000 --- a/src/api/groups/useUserGroups.ts +++ /dev/null @@ -1,145 +0,0 @@ -import { queryOptions } from "@tanstack/react-query"; -import { supabase } from "@/integrations/supabase/client"; -import type { Group } from "./types"; -import { groupsKeys } from "./types"; - -// Check if user is admin -async function isUserAdmin(userId: string): Promise { - const { data: isAdminData, error: isAdminError } = await supabase - .from("admin_roles") - .select("id") - .eq("user_id", userId) - .limit(1); - - if (isAdminError) { - console.error("Error checking admin role:", isAdminError); - return false; - } - - return isAdminData && isAdminData.length > 0; -} - -// Get user's group IDs -async function getUserGroupIds(userId: string): Promise { - const { data: userGroups, error: userGroupsError } = await supabase - .from("group_members") - .select("group_id") - .eq("user_id", userId); - - if (userGroupsError) { - console.error("Error fetching user groups:", userGroupsError); - throw new Error("Failed to fetch user groups"); - } - - return userGroups?.map((ug) => ug.group_id) || []; -} - -// Helper function to add member counts to groups -async function addMemberCounts( - groups: Group[], - userId: string, - userGroupIds: string[], - isUserGroupsOnly: boolean = false, -): Promise { - const groupIds = groups.map((group) => group.id); - const memberCountsByGroupId = await fetchMemberCountsByGroupId(groupIds); - - return groups.map((group) => ({ - ...group, - member_count: memberCountsByGroupId.get(group.id) || 0, - is_creator: group.created_by === userId, - is_member: isUserGroupsOnly ? true : userGroupIds.includes(group.id), - })); -} - -async function fetchMemberCountsByGroupId( - groupIds: string[], -): Promise> { - const memberCountsByGroupId = new Map(); - - if (groupIds.length === 0) { - return memberCountsByGroupId; - } - - const { data: counts, error } = await supabase.rpc("group_member_counts", { - p_group_ids: groupIds, - }); - - if (error) { - throw new Error("Failed to fetch group member counts"); - } - - for (const row of counts || []) { - memberCountsByGroupId.set(row.group_id, row.member_count); - } - - return memberCountsByGroupId; -} - -// Fetch groups from database -async function fetchGroupsFromDb( - shouldFetchAll: boolean, - userGroupIds: string[], -) { - let groupsQuery = supabase - .from("groups") - .select("*") - .eq("archived", false) - .order("created_at", { ascending: false }); - - if (!shouldFetchAll) { - // Fetch only user's groups - if (userGroupIds.length === 0) { - return []; // User has no groups - } - groupsQuery = groupsQuery.in("id", userGroupIds); - } - - const { data: groupsData, error } = await groupsQuery; - - if (error) { - throw new Error(error.message || "Failed to fetch groups"); - } - - return groupsData || []; -} - -// Business logic function -async function fetchUserGroups( - userId: string, - { all = false }: { all?: boolean } = {}, -): Promise { - const userGroupIds = await getUserGroupIds(userId); - - let shouldFetchAllGroups = false; - if (all) { - const isAdmin = await isUserAdmin(userId); - shouldFetchAllGroups = isAdmin; - } - - const groupsData = await fetchGroupsFromDb( - shouldFetchAllGroups, - userGroupIds, - ); - - if (groupsData.length === 0) { - return []; - } - - return addMemberCounts( - groupsData, - userId, - userGroupIds, - !shouldFetchAllGroups, - ); -} - -export function userGroupsQuery( - userId: string, - params: { all?: boolean } = {}, -) { - return queryOptions({ - queryKey: groupsKeys.user(userId, params), - queryFn: () => fetchUserGroups(userId, params), - }); -} diff --git a/src/components/layout/AppHeader/GroupsIndicator.tsx b/src/components/layout/AppHeader/GroupsIndicator.tsx index 42348e6b6..53842d431 100644 --- a/src/components/layout/AppHeader/GroupsIndicator.tsx +++ b/src/components/layout/AppHeader/GroupsIndicator.tsx @@ -1,10 +1,9 @@ import { Suspense } from "react"; import { Link } from "@tanstack/react-router"; import { UserPlus } from "lucide-react"; -import { useSuspenseQuery } from "@tanstack/react-query"; import { Skeleton } from "@/components/ui/skeleton"; import { useAuth } from "@/contexts/AuthContext"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; import { cn } from "@/lib/utils"; import { TooltipButton } from "./TooltipButton"; import { ActiveGroupSwitcher } from "./GroupSwitcher/ActiveGroupSwitcher"; @@ -42,7 +41,7 @@ function GroupsIndicatorContent({ isMobile: boolean; userId: string; }) { - const { data: groups } = useSuspenseQuery(userGroupsQuery(userId)); + const { data: groups } = useMyGroupsQuery(userId); if (groups.length === 0) { return ( diff --git a/src/contexts/ActiveScopeContext.tsx b/src/contexts/ActiveScopeContext.tsx index 418535afa..7d87264b6 100644 --- a/src/contexts/ActiveScopeContext.tsx +++ b/src/contexts/ActiveScopeContext.tsx @@ -2,7 +2,7 @@ import { createContext, useContext, useMemo, useState } from "react"; import type { ReactNode } from "react"; import { useQuery } from "@tanstack/react-query"; import { useAuth } from "@/contexts/AuthContext"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import { resolveActiveGroupId, resolvePinnedScope } from "@/lib/activeGroup"; import type { PinnedScope } from "@/lib/activeGroup"; @@ -66,7 +66,7 @@ function AuthedActiveScopeProvider({ children: ReactNode; }) { const { profile } = useAuth(); - const { data: groups = [] } = useQuery(userGroupsQuery(userId)); + const { data: groups = [] } = useQuery(myGroupsQuery(userId)); const [override, setOverride] = useState(null); const groupIds = useMemo(() => groups.map((group) => group.id), [groups]); diff --git a/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx b/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx index 579664a08..af6c56f3c 100644 --- a/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx +++ b/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx @@ -1,10 +1,9 @@ import { useState } from "react"; -import { useSuspenseQuery } from "@tanstack/react-query"; import { FilterSortControls } from "@/pages/EditionView/tabs/VoteTab/filters/FilterSortControls"; import { GroupScopedSetsPanel } from "@/pages/EditionView/tabs/VoteTab/GroupScopedSetsPanel"; import { EveryoneSetsPanel } from "@/pages/EditionView/tabs/VoteTab/SetsPanelContent"; import { useActiveScope } from "@/contexts/ActiveScopeContext"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; import type { FilteredSetsPanelProps } from "@/pages/EditionView/tabs/VoteTab/FilteredSetsPanel"; import type { BinaryVoteScope, VoteScope } from "@/lib/voteScope"; @@ -12,7 +11,7 @@ export function AuthedFilteredSetsPanel( props: FilteredSetsPanelProps & { userId: string }, ) { const { current, activeGroupId } = useActiveScope(); - const { data: groups } = useSuspenseQuery(userGroupsQuery(props.userId)); + const { data: groups } = useMyGroupsQuery(props.userId); const perspectiveGroupId = current.kind === "group" ? current.groupId : activeGroupId; const perspectiveGroupName = perspectiveGroupId diff --git a/src/pages/SetDetails/SetGroupVoting.tsx b/src/pages/SetDetails/SetGroupVoting.tsx index ce561382f..9909256ec 100644 --- a/src/pages/SetDetails/SetGroupVoting.tsx +++ b/src/pages/SetDetails/SetGroupVoting.tsx @@ -1,9 +1,8 @@ -import { useSuspenseQuery } from "@tanstack/react-query"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { useAuth } from "@/contexts/AuthContext"; import { useActiveScope } from "@/contexts/ActiveScopeContext"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; import { useGroupVotesQuery } from "@/api/voting/useGroupVotes"; import { Users } from "lucide-react"; import { VOTE_CONFIG, VOTES_TYPES, getVoteConfig } from "@/lib/votes/config"; @@ -31,7 +30,7 @@ function SetGroupVotingContent({ artistId: string; userId: string; }) { - const { data: groups } = useSuspenseQuery(userGroupsQuery(userId)); + const { data: groups } = useMyGroupsQuery(userId); const { current } = useActiveScope(); const activeGroupId = current.kind === "group" ? current.groupId : undefined; diff --git a/src/pages/Settings/SettingsPage.tsx b/src/pages/Settings/SettingsPage.tsx index 244be0684..21bb63bf9 100644 --- a/src/pages/Settings/SettingsPage.tsx +++ b/src/pages/Settings/SettingsPage.tsx @@ -3,7 +3,7 @@ import { Link } from "@tanstack/react-router"; import { useSuspenseQuery } from "@tanstack/react-query"; import { TopBar } from "@/components/layout/TopBar"; import { useAuth } from "@/contexts/AuthContext"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; import { profileQuery } from "@/api/auth/useProfile"; import { SignInRequired } from "@/pages/groups/Groups/SignInRequired"; import { ActiveGroupSetting } from "./ActiveGroupSetting"; @@ -33,7 +33,7 @@ export function SettingsPage() { } function SettingsContent({ userId }: { userId: string }) { - const { data: groups } = useSuspenseQuery(userGroupsQuery(userId)); + const { data: groups } = useMyGroupsQuery(userId); const { data: profile } = useSuspenseQuery(profileQuery(userId)); const hasGroups = groups.length > 0; diff --git a/src/routes/__root.tsx b/src/routes/__root.tsx index f792ef160..79de18e23 100644 --- a/src/routes/__root.tsx +++ b/src/routes/__root.tsx @@ -26,7 +26,7 @@ import { z } from "zod"; import type { QueryClient } from "@tanstack/react-query"; import type { User } from "@supabase/supabase-js"; import { supabase } from "@/integrations/supabase/client"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import { pageMeta } from "@/lib/pageHead"; import { useClearStaticTags } from "@/hooks/useClearStaticTags"; @@ -69,9 +69,7 @@ export const Route = createRootRouteWithContext()({ }, loader: async ({ context }) => { if (context.user) { - void context.queryClient.ensureQueryData( - userGroupsQuery(context.user.id, { all: false }), - ); + void context.queryClient.ensureQueryData(myGroupsQuery(context.user.id)); } }, }); diff --git a/src/routes/groups/index.tsx b/src/routes/groups/index.tsx index 92e8b3578..e1b6a1a24 100644 --- a/src/routes/groups/index.tsx +++ b/src/routes/groups/index.tsx @@ -1,9 +1,9 @@ import { createFileRoute } from "@tanstack/react-router"; -import { userGroupsQuery } from "@/api/groups/useUserGroups"; +import { myGroupsQuery, useMyGroupsQuery } from "@/api/groups/useMyGroups"; +import { useAllGroupsQuery } from "@/api/groups/useAllGroups"; import { Suspense, useState } from "react"; import { useNavigate, useRouteContext } from "@tanstack/react-router"; import type { User } from "@supabase/supabase-js"; -import { useSuspenseQuery } from "@tanstack/react-query"; import { useUserPermissionsQuery } from "@/api/auth/useUserPermissions"; import { useDeleteGroupMutation } from "@/api/groups/useDeleteGroup"; import { DeleteGroupDialog } from "@/pages/groups/Groups/DeleteGroupDialog"; @@ -21,9 +21,7 @@ export const Route = createFileRoute("/groups/")({ }), loader: async ({ context }) => { if (context.user) { - void context.queryClient.ensureQueryData( - userGroupsQuery(context.user.id, { all: false }), - ); + void context.queryClient.ensureQueryData(myGroupsQuery(context.user.id)); } }, }); @@ -146,16 +144,46 @@ function GroupsList({ showAllGroups: boolean; onDelete: (id: string, name: string) => void; }) { - const { data: groups } = useSuspenseQuery( - userGroupsQuery(userId, { all: showAllGroups }), + if (showAllGroups) { + return ; + } + return ; +} + +function UserGroupsList({ + userId, + onDelete, +}: { + userId: string; + onDelete: (id: string, name: string) => void; +}) { + const { data: groups } = useMyGroupsQuery(userId); + + return ( + ); +} + +function AllGroupsList({ + userId, + onDelete, +}: { + userId: string; + onDelete: (id: string, name: string) => void; +}) { + const { data: groups } = useAllGroupsQuery(userId); return ( ); } diff --git a/src/test/integration/fixtures/adminRoles.ts b/src/test/integration/fixtures/adminRoles.ts index e239ae62e..c5d1d6baf 100644 --- a/src/test/integration/fixtures/adminRoles.ts +++ b/src/test/integration/fixtures/adminRoles.ts @@ -1,15 +1,21 @@ +import type { Database } from "@/integrations/supabase/types"; import { registerCleanup, testSupabase } from "../harness"; import { SEEDED_USER_ID } from "./constants"; +type AdminRole = Database["public"]["Enums"]["admin_role"]; + /** * Grants a user an `admin_roles` row so both RLS ("Admins can manage X") * and the app's own `is_admin`/`can_edit_artists` RPCs see them as an * admin. Self-cleans. */ -export async function grantAdminRole(userId: string): Promise { +export async function grantAdminRole( + userId: string, + role: AdminRole = "admin", +): Promise { const { error } = await testSupabase.from("admin_roles").insert({ user_id: userId, - role: "admin", + role, created_by: SEEDED_USER_ID, }); if (error) throw error; @@ -19,7 +25,7 @@ export async function grantAdminRole(userId: string): Promise { .from("admin_roles") .delete() .eq("user_id", userId) - .eq("role", "admin"); + .eq("role", role); if (deleteError) throw deleteError; }); } diff --git a/src/test/integration/fixtures/groups.ts b/src/test/integration/fixtures/groups.ts new file mode 100644 index 000000000..04439cf70 --- /dev/null +++ b/src/test/integration/fixtures/groups.ts @@ -0,0 +1,47 @@ +import { registerCleanup, testSupabase } from "../harness"; + +/** Disposable group, created by `createdBy`. Self-cleans (cascades to its members). */ +export async function createGroup(createdBy: string): Promise { + const suffix = crypto.randomUUID(); + + const { data: group, error } = await testSupabase + .from("groups") + .insert({ + name: `Scratch Group ${suffix}`, + slug: `scratch-group-${suffix}`, + created_by: createdBy, + }) + .select("id") + .single(); + if (error) throw error; + + registerCleanup(async () => { + const { error: deleteError } = await testSupabase + .from("groups") + .delete() + .eq("id", group.id); + if (deleteError) throw deleteError; + }); + + return group.id; +} + +/** Adds `userId` as a member of `groupId`. Self-cleans. */ +export async function addGroupMember( + groupId: string, + userId: string, +): Promise { + const { error } = await testSupabase + .from("group_members") + .insert({ group_id: groupId, user_id: userId }); + if (error) throw error; + + registerCleanup(async () => { + const { error: deleteError } = await testSupabase + .from("group_members") + .delete() + .eq("group_id", groupId) + .eq("user_id", userId); + if (deleteError) throw deleteError; + }); +} From 12a68d342a2bd086bfdefcf175c5f4d9bcd4ef73 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 14:48:49 +0000 Subject: [PATCH 2/3] refactor(groups): inline suspense queries, gate all-groups on super admin Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01HcvJ9ZU2F8T5fjAEm4otAW --- src/api/auth/useUserPermissions.ts | 24 +++++++++----- src/api/groups/useAllGroups.ts | 33 ++++++++----------- src/api/groups/useMyGroups.ts | 15 +++------ .../layout/AppHeader/GroupsIndicator.tsx | 5 +-- .../tabs/VoteTab/AuthedFilteredSetsPanel.tsx | 5 +-- src/pages/SetDetails/SetGroupVoting.tsx | 5 +-- src/pages/Settings/SettingsPage.tsx | 4 +-- src/routes/groups/index.tsx | 15 +++++---- 8 files changed, 51 insertions(+), 55 deletions(-) diff --git a/src/api/auth/useUserPermissions.ts b/src/api/auth/useUserPermissions.ts index 8b2f1fe4a..902db3b1d 100644 --- a/src/api/auth/useUserPermissions.ts +++ b/src/api/auth/useUserPermissions.ts @@ -2,10 +2,9 @@ import { queryOptions, useQuery } from "@tanstack/react-query"; import { supabase } from "@/integrations/supabase/client"; import { userPermissionsKeys } from "./types"; -async function checkUserPermissions( - userId: string, - permission: "edit_artists" | "is_admin", -) { +type Permission = "edit_artists" | "is_admin" | "is_super_admin"; + +async function checkUserPermissions(userId: string, permission: Permission) { try { // Use new admin roles system if (permission === "edit_artists") { @@ -26,6 +25,16 @@ async function checkUserPermissions( return false; } return data || false; + } else if (permission === "is_super_admin") { + const { data, error } = await supabase.rpc("has_admin_role", { + check_user_id: userId, + check_role: "super_admin", + }); + if (error) { + console.error("Error checking is_super_admin permission:", error); + return false; + } + return data || false; } return false; @@ -35,10 +44,7 @@ async function checkUserPermissions( } } -export function userPermissionsQuery( - userId: string, - permission: "edit_artists" | "is_admin", -) { +export function userPermissionsQuery(userId: string, permission: Permission) { return queryOptions({ queryKey: userPermissionsKeys.user(userId, permission), queryFn: () => checkUserPermissions(userId, permission), @@ -48,7 +54,7 @@ export function userPermissionsQuery( export function useUserPermissionsQuery( userId: string | undefined, - permission: "edit_artists" | "is_admin", + permission: Permission, ) { return useQuery({ ...userPermissionsQuery(userId!, permission), diff --git a/src/api/groups/useAllGroups.ts b/src/api/groups/useAllGroups.ts index fc599b354..2b6ca67e6 100644 --- a/src/api/groups/useAllGroups.ts +++ b/src/api/groups/useAllGroups.ts @@ -1,4 +1,4 @@ -import { queryOptions, useSuspenseQuery } from "@tanstack/react-query"; +import { queryOptions } from "@tanstack/react-query"; import { supabase } from "@/integrations/supabase/client"; import type { Group } from "./types"; import { groupsKeys } from "./types"; @@ -11,19 +11,16 @@ export function allGroupsQuery(userId: string) { }); } -export function useAllGroupsQuery(userId: string) { - return useSuspenseQuery(allGroupsQuery(userId)); -} - /** - * Every non-archived group, for an admin caller. A non-admin caller falls + * Every non-archived group, for a super-admin caller (the only role `groups` RLS + * lets read them all). Any other caller falls * back to exactly the member-only result (never an error, never a leaked * full group list). */ async function fetchAllGroups(userId: string): Promise { - const isAdmin = await isUserAdmin(userId); + const canViewAll = await isSuperAdmin(userId); - if (!isAdmin) { + if (!canViewAll) { return fetchMyGroups(userId); } @@ -40,23 +37,19 @@ async function fetchAllGroups(userId: string): Promise { throw new Error(error.message || "Failed to fetch groups"); } - return attachGroupMeta(groupsData || [], userId, { - alwaysMember: false, - memberGroupIds: userGroupIds, - }); + return attachGroupMeta(groupsData || [], userId, userGroupIds); } -async function isUserAdmin(userId: string): Promise { - const { data: isAdminData, error } = await supabase - .from("admin_roles") - .select("id") - .eq("user_id", userId) - .limit(1); +async function isSuperAdmin(userId: string): Promise { + const { data, error } = await supabase.rpc("has_admin_role", { + check_user_id: userId, + check_role: "super_admin", + }); if (error) { - console.error("Error checking admin role:", error); + console.error("Error checking super admin role:", error); return false; } - return isAdminData && isAdminData.length > 0; + return data === true; } diff --git a/src/api/groups/useMyGroups.ts b/src/api/groups/useMyGroups.ts index 8e0bca2af..ff790ce01 100644 --- a/src/api/groups/useMyGroups.ts +++ b/src/api/groups/useMyGroups.ts @@ -1,4 +1,4 @@ -import { queryOptions, useSuspenseQuery } from "@tanstack/react-query"; +import { queryOptions } from "@tanstack/react-query"; import { supabase } from "@/integrations/supabase/client"; import type { Group } from "./types"; import { groupsKeys } from "./types"; @@ -10,10 +10,6 @@ export function myGroupsQuery(userId: string) { }); } -export function useMyGroupsQuery(userId: string) { - return useSuspenseQuery(myGroupsQuery(userId)); -} - /** Groups the given user is a member of. `is_member` is always `true` — no admin check. */ export async function fetchMyGroups(userId: string): Promise { const userGroupIds = await getUserGroupIds(userId); @@ -33,7 +29,7 @@ export async function fetchMyGroups(userId: string): Promise { throw new Error(error.message || "Failed to fetch groups"); } - return attachGroupMeta(groupsData || [], userId, { alwaysMember: true }); + return attachGroupMeta(groupsData || [], userId, userGroupIds); } export async function getUserGroupIds(userId: string): Promise { @@ -53,10 +49,7 @@ export async function getUserGroupIds(userId: string): Promise { export async function attachGroupMeta( groups: Group[], userId: string, - { - alwaysMember, - memberGroupIds, - }: { alwaysMember: boolean; memberGroupIds?: string[] }, + memberGroupIds: string[], ): Promise { const memberCountsByGroupId = await fetchMemberCountsByGroupId( groups.map((group) => group.id), @@ -66,7 +59,7 @@ export async function attachGroupMeta( ...group, member_count: memberCountsByGroupId.get(group.id) || 0, is_creator: group.created_by === userId, - is_member: alwaysMember || (memberGroupIds?.includes(group.id) ?? false), + is_member: memberGroupIds.includes(group.id), })); } diff --git a/src/components/layout/AppHeader/GroupsIndicator.tsx b/src/components/layout/AppHeader/GroupsIndicator.tsx index 53842d431..e9a5ab7e0 100644 --- a/src/components/layout/AppHeader/GroupsIndicator.tsx +++ b/src/components/layout/AppHeader/GroupsIndicator.tsx @@ -1,9 +1,10 @@ +import { useSuspenseQuery } from "@tanstack/react-query"; import { Suspense } from "react"; import { Link } from "@tanstack/react-router"; import { UserPlus } from "lucide-react"; import { Skeleton } from "@/components/ui/skeleton"; import { useAuth } from "@/contexts/AuthContext"; -import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import { cn } from "@/lib/utils"; import { TooltipButton } from "./TooltipButton"; import { ActiveGroupSwitcher } from "./GroupSwitcher/ActiveGroupSwitcher"; @@ -41,7 +42,7 @@ function GroupsIndicatorContent({ isMobile: boolean; userId: string; }) { - const { data: groups } = useMyGroupsQuery(userId); + const { data: groups } = useSuspenseQuery(myGroupsQuery(userId)); if (groups.length === 0) { return ( diff --git a/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx b/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx index af6c56f3c..7129cc194 100644 --- a/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx +++ b/src/pages/EditionView/tabs/VoteTab/AuthedFilteredSetsPanel.tsx @@ -1,9 +1,10 @@ +import { useSuspenseQuery } from "@tanstack/react-query"; import { useState } from "react"; import { FilterSortControls } from "@/pages/EditionView/tabs/VoteTab/filters/FilterSortControls"; import { GroupScopedSetsPanel } from "@/pages/EditionView/tabs/VoteTab/GroupScopedSetsPanel"; import { EveryoneSetsPanel } from "@/pages/EditionView/tabs/VoteTab/SetsPanelContent"; import { useActiveScope } from "@/contexts/ActiveScopeContext"; -import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import type { FilteredSetsPanelProps } from "@/pages/EditionView/tabs/VoteTab/FilteredSetsPanel"; import type { BinaryVoteScope, VoteScope } from "@/lib/voteScope"; @@ -11,7 +12,7 @@ export function AuthedFilteredSetsPanel( props: FilteredSetsPanelProps & { userId: string }, ) { const { current, activeGroupId } = useActiveScope(); - const { data: groups } = useMyGroupsQuery(props.userId); + const { data: groups } = useSuspenseQuery(myGroupsQuery(props.userId)); const perspectiveGroupId = current.kind === "group" ? current.groupId : activeGroupId; const perspectiveGroupName = perspectiveGroupId diff --git a/src/pages/SetDetails/SetGroupVoting.tsx b/src/pages/SetDetails/SetGroupVoting.tsx index 9909256ec..c519d97cb 100644 --- a/src/pages/SetDetails/SetGroupVoting.tsx +++ b/src/pages/SetDetails/SetGroupVoting.tsx @@ -1,8 +1,9 @@ +import { useSuspenseQuery } from "@tanstack/react-query"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { useAuth } from "@/contexts/AuthContext"; import { useActiveScope } from "@/contexts/ActiveScopeContext"; -import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import { useGroupVotesQuery } from "@/api/voting/useGroupVotes"; import { Users } from "lucide-react"; import { VOTE_CONFIG, VOTES_TYPES, getVoteConfig } from "@/lib/votes/config"; @@ -30,7 +31,7 @@ function SetGroupVotingContent({ artistId: string; userId: string; }) { - const { data: groups } = useMyGroupsQuery(userId); + const { data: groups } = useSuspenseQuery(myGroupsQuery(userId)); const { current } = useActiveScope(); const activeGroupId = current.kind === "group" ? current.groupId : undefined; diff --git a/src/pages/Settings/SettingsPage.tsx b/src/pages/Settings/SettingsPage.tsx index 21bb63bf9..733e710a6 100644 --- a/src/pages/Settings/SettingsPage.tsx +++ b/src/pages/Settings/SettingsPage.tsx @@ -3,7 +3,7 @@ import { Link } from "@tanstack/react-router"; import { useSuspenseQuery } from "@tanstack/react-query"; import { TopBar } from "@/components/layout/TopBar"; import { useAuth } from "@/contexts/AuthContext"; -import { useMyGroupsQuery } from "@/api/groups/useMyGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; import { profileQuery } from "@/api/auth/useProfile"; import { SignInRequired } from "@/pages/groups/Groups/SignInRequired"; import { ActiveGroupSetting } from "./ActiveGroupSetting"; @@ -33,7 +33,7 @@ export function SettingsPage() { } function SettingsContent({ userId }: { userId: string }) { - const { data: groups } = useMyGroupsQuery(userId); + const { data: groups } = useSuspenseQuery(myGroupsQuery(userId)); const { data: profile } = useSuspenseQuery(profileQuery(userId)); const hasGroups = groups.length > 0; diff --git a/src/routes/groups/index.tsx b/src/routes/groups/index.tsx index e1b6a1a24..33f1c4b5d 100644 --- a/src/routes/groups/index.tsx +++ b/src/routes/groups/index.tsx @@ -1,6 +1,7 @@ +import { useSuspenseQuery } from "@tanstack/react-query"; import { createFileRoute } from "@tanstack/react-router"; -import { myGroupsQuery, useMyGroupsQuery } from "@/api/groups/useMyGroups"; -import { useAllGroupsQuery } from "@/api/groups/useAllGroups"; +import { myGroupsQuery } from "@/api/groups/useMyGroups"; +import { allGroupsQuery } from "@/api/groups/useAllGroups"; import { Suspense, useState } from "react"; import { useNavigate, useRouteContext } from "@tanstack/react-router"; import type { User } from "@supabase/supabase-js"; @@ -40,9 +41,9 @@ function GroupsContent({ user }: { user: User }) { const navigate = useNavigate(); const [showAllGroups, setShowAllGroups] = useState(false); - const { data: isAdmin = false } = useUserPermissionsQuery( + const { data: isSuperAdmin = false } = useUserPermissionsQuery( user.id, - "is_admin", + "is_super_admin", ); const deleteGroupMutation = useDeleteGroupMutation(); const [createDialogOpen, setCreateDialogOpen] = useState(false); @@ -81,7 +82,7 @@ function GroupsContent({ user }: { user: User }) {
setCreateDialogOpen(true)} /> - {isAdmin && ( + {isSuperAdmin && (