From 9fcd06b015bfd320a2ccaa7fd767cf5df5065ffd Mon Sep 17 00:00:00 2001 From: aj-read <58521077+aj-read@users.noreply.github.com> Date: Sun, 16 Aug 2026 13:49:08 +0100 Subject: [PATCH 1/3] Add 'Overview' tab to game navigation --- packages/web/src/Router.tsx | 8 + .../DrawProposalNotificationBadge.tsx | 29 ++ .../src/components/GameDetailLayout.test.tsx | 101 ++++++ .../web/src/components/GameDetailLayout.tsx | 59 +++- .../src/components/GameDropdownMenu.test.tsx | 15 +- .../web/src/components/GameDropdownMenu.tsx | 12 +- .../src/components/PlayerInfoContent.test.tsx | 294 +++++++++++++++++- .../web/src/components/PlayerInfoContent.tsx | 292 +++++++++++++++-- packages/web/src/mocks/handlers.test.ts | 53 ++++ packages/web/src/mocks/handlers.ts | 30 +- .../GameDetail/DrawProposalsScreen.tsx | 2 +- .../GameDetail/GameInfoScreen.test.tsx | 44 +++ .../src/screens/GameDetail/GameInfoScreen.tsx | 6 +- .../web/src/screens/GameDetail/MapScreen.tsx | 5 - .../screens/GameDetail/OrdersScreen.test.tsx | 10 +- .../src/screens/GameDetail/OrdersScreen.tsx | 81 +---- .../GameDetail/OverviewScreen.test.tsx | 63 ++++ .../src/screens/GameDetail/OverviewScreen.tsx | 85 +++++ .../screens/GameDetail/PlayerInfoScreen.tsx | 38 +-- .../GameDetail/PlayerProfileScreen.tsx | 2 +- .../screens/GameDetail/ProposeDrawScreen.tsx | 2 +- packages/web/src/screens/GameDetail/index.ts | 4 + 22 files changed, 1070 insertions(+), 165 deletions(-) create mode 100644 packages/web/src/components/DrawProposalNotificationBadge.tsx create mode 100644 packages/web/src/components/GameDetailLayout.test.tsx create mode 100644 packages/web/src/mocks/handlers.test.ts create mode 100644 packages/web/src/screens/GameDetail/GameInfoScreen.test.tsx create mode 100644 packages/web/src/screens/GameDetail/OverviewScreen.test.tsx create mode 100644 packages/web/src/screens/GameDetail/OverviewScreen.tsx diff --git a/packages/web/src/Router.tsx b/packages/web/src/Router.tsx index 84ac90a4c..24c6453d6 100644 --- a/packages/web/src/Router.tsx +++ b/packages/web/src/Router.tsx @@ -219,6 +219,14 @@ export const createAuthenticatedRoutes = ( element: , children: [ { index: true, element: }, + { + path: "overview", + element: ( + }> + + + ), + }, { path: "orders", element: ( diff --git a/packages/web/src/components/DrawProposalNotificationBadge.tsx b/packages/web/src/components/DrawProposalNotificationBadge.tsx new file mode 100644 index 000000000..86cdfbfa3 --- /dev/null +++ b/packages/web/src/components/DrawProposalNotificationBadge.tsx @@ -0,0 +1,29 @@ +import React from "react"; + +import type { DrawProposal } from "@/api/generated/endpoints"; +import { Badge } from "@/components/ui/badge"; + +export const getDrawProposalNotificationCount = ( + proposals: readonly DrawProposal[] | undefined +) => + proposals?.filter( + proposal => + proposal.status === "pending" && + proposal.myVote !== null && + proposal.myVote.accepted === null + ).length ?? 0; + +export const DrawProposalNotificationBadge: React.FC<{ count: number }> = ({ + count, +}) => { + if (count === 0) return null; + + return ( + + {count} + + ); +}; diff --git a/packages/web/src/components/GameDetailLayout.test.tsx b/packages/web/src/components/GameDetailLayout.test.tsx new file mode 100644 index 000000000..e60cf296e --- /dev/null +++ b/packages/web/src/components/GameDetailLayout.test.tsx @@ -0,0 +1,101 @@ +import { render, screen } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router"; +import { describe, expect, it, vi } from "vitest"; + +import { GameDetailLayout } from "./GameDetailLayout"; + +const mockGameData = vi.fn(); +const mockDrawProposalsData = vi.fn(); + +vi.mock("@/api/generated/endpoints", () => ({ + useGameRetrieve: () => ({ data: mockGameData() }), + useGamesDrawProposalsList: () => ({ data: mockDrawProposalsData() }), +})); + +vi.mock("@/components/Navigation", () => ({ + Navigation: ({ + items, + }: { + items: Array<{ label: string; badge?: string; isActive?: boolean }>; + }) => ( +
+ {items.map(item => ( +
+ ))} +
+ ), +})); + +vi.mock("@/components/GameMap", () => ({ GameMap: () => null })); +vi.mock("@/components/OfflineBanner", () => ({ OfflineBanner: () => null })); +vi.mock("@/components/SafeAreaView", () => ({ + SafeAreaView: ({ children }: { children: React.ReactNode }) => <>{children}, +})); +vi.mock("@/components/ui/sidebar", () => ({ + SidebarProvider: ({ children }: { children: React.ReactNode }) => <>{children}, + Sidebar: ({ children }: { children: React.ReactNode }) => <>{children}, + SidebarHeader: ({ children }: { children: React.ReactNode }) => <>{children}, + SidebarContent: ({ children }: { children: React.ReactNode }) => <>{children}, + SidebarInset: ({ children }: { children: React.ReactNode }) => <>{children}, +})); + +const renderLayout = ( + initialEntry = "/game/g1/phase/1/overview", + child =
Overview
+) => + render( + + + {child}} + /> + + + ); + +describe("GameDetailLayout draw-proposal notification", () => { + it("adds a notification dot to each Overview navigation item", () => { + mockGameData.mockReturnValue({ + status: "active", + sandbox: false, + members: [], + totalUnreadMessageCount: 0, + }); + mockDrawProposalsData.mockReturnValue([ + { + id: 1, + status: "pending", + myVote: { included: true, accepted: null }, + }, + ]); + + renderLayout(); + + expect(screen.getAllByTestId("nav-Overview")).toHaveLength(2); + for (const overviewItem of screen.getAllByTestId("nav-Overview")) { + expect(overviewItem).toHaveAttribute("data-badge", "•"); + } + }); + + it("keeps Overview active throughout the Game Info flow", () => { + mockGameData.mockReturnValue({ + status: "active", + sandbox: false, + members: [], + totalUnreadMessageCount: 0, + }); + mockDrawProposalsData.mockReturnValue([]); + + renderLayout("/game/g1/phase/1/game-info",
Game Info
); + + for (const overviewItem of screen.getAllByTestId("nav-Overview")) { + expect(overviewItem).toHaveAttribute("data-active", "true"); + } + }); +}); diff --git a/packages/web/src/components/GameDetailLayout.tsx b/packages/web/src/components/GameDetailLayout.tsx index 51ce86f3e..69dd97925 100644 --- a/packages/web/src/components/GameDetailLayout.tsx +++ b/packages/web/src/components/GameDetailLayout.tsx @@ -1,7 +1,13 @@ import React, { useMemo } from "react"; import { useLocation, useNavigate, useSearchParams } from "react-router"; import { useRequiredParams } from "@/hooks"; -import { ArrowLeft, Map, Gavel, MessageCircle } from "lucide-react"; +import { + ArrowLeft, + Trophy, + Map, + Gavel, + MessageCircle, +} from "lucide-react"; import { cn } from "@/lib/utils"; import { Sidebar, @@ -15,9 +21,18 @@ import { Navigation } from "@/components/Navigation"; import { GameMap } from "@/components/GameMap"; import { SafeAreaView } from "@/components/SafeAreaView"; import { OfflineBanner } from "@/components/OfflineBanner"; -import { useGameRetrieve } from "@/api/generated/endpoints"; +import { + useGameRetrieve, + useGamesDrawProposalsList, +} from "@/api/generated/endpoints"; +import { getDrawProposalNotificationCount } from "@/components/DrawProposalNotificationBadge"; const navigationItems = [ + { + label: "Overview", + icon: Trophy, + path: "/game/:gameId/phase/:phaseId/overview", + }, { label: "Map", icon: Map, path: "/game/:gameId/phase/:phaseId" }, { label: "Orders", icon: Gavel, path: "/game/:gameId/phase/:phaseId/orders" }, { label: "Chat", icon: MessageCircle, path: "/game/:gameId/phase/:phaseId/chat" }, @@ -45,6 +60,17 @@ const GameDetailLayout: React.FC = ({ query.state.data?.status === "active" ? 5000 : false, }, }); + const isStartedGame = + game?.status === "active" || + game?.status === "completed" || + game?.status === "abandoned"; + const canShowDrawProposals = + !!game && !game.sandbox && isStartedGame; + const { data: drawProposals } = useGamesDrawProposalsList(gameId, { + query: { enabled: canShowDrawProposals }, + }); + const drawProposalNotificationCount = + getDrawProposalNotificationCount(drawProposals); const [searchParams] = useSearchParams(); @@ -55,11 +81,18 @@ const GameDetailLayout: React.FC = ({ const searchParamsStr = searchParams.toString(); const chatBasePath = `/game/${gameId}/phase/${phaseId}/chat`; const isInChatChannel = location.pathname.startsWith(chatBasePath + "/"); + const overviewFlowPaths = [ + `/game/${gameId}/phase/${phaseId}/game-info`, + `/game/${gameId}/phase/${phaseId}/draw-proposals`, + `/game/${gameId}/phase/${phaseId}/propose-draw`, + `/game/${gameId}/phase/${phaseId}/player/`, + ]; return items.map(item => { const basePath = item.path .replace(":gameId", gameId) .replace(":phaseId", phaseId); const badge = + (item.label === "Overview" && drawProposalNotificationCount > 0) || (item.label === "Chat" && game?.totalUnreadMessageCount && game.totalUnreadMessageCount > 0) || @@ -77,9 +110,14 @@ const GameDetailLayout: React.FC = ({ } else { path = searchParamsStr ? `${basePath}?${searchParamsStr}` : basePath; } - const isActive = item.label === "Chat" - ? location.pathname === chatBasePath || location.pathname.startsWith(chatBasePath + "/") - : location.pathname === basePath; + const isActive = + item.label === "Chat" + ? location.pathname === chatBasePath || + location.pathname.startsWith(chatBasePath + "/") + : item.label === "Overview" + ? location.pathname === basePath || + overviewFlowPaths.some(path => location.pathname.startsWith(path)) + : location.pathname === basePath; return { ...item, path, @@ -87,7 +125,16 @@ const GameDetailLayout: React.FC = ({ badge, }; }); - }, [gameId, phaseId, searchParams, location.pathname, game?.totalUnreadMessageCount, game?.sandbox, game?.members]); + }, [ + gameId, + phaseId, + searchParams, + location.pathname, + game?.totalUnreadMessageCount, + game?.sandbox, + game?.members, + drawProposalNotificationCount, + ]); // Filter out Map for desktop sidebar since map is already visible in right // panel. Unlike the bottom nav, the sidebar Chat icon should return to the diff --git a/packages/web/src/components/GameDropdownMenu.test.tsx b/packages/web/src/components/GameDropdownMenu.test.tsx index 5055ad4e3..34b73c748 100644 --- a/packages/web/src/components/GameDropdownMenu.test.tsx +++ b/packages/web/src/components/GameDropdownMenu.test.tsx @@ -21,7 +21,10 @@ if (!Element.prototype.scrollIntoView) { Element.prototype.scrollIntoView = () => {}; } -const renderMenu = (game: React.ComponentProps["game"]) => { +const renderMenu = ( + game: React.ComponentProps["game"], + includePlayerInfo = true +) => { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); @@ -31,7 +34,7 @@ const renderMenu = (game: React.ComponentProps["game"]) {}} - onNavigateToPlayerInfo={() => {}} + onNavigateToPlayerInfo={includePlayerInfo ? () => {} : undefined} /> @@ -43,6 +46,14 @@ const openMenu = async () => { }; describe("GameDropdownMenu", () => { + it("hides Player info when the current screen has an Overview tab", async () => { + renderMenu(mockActiveGames[0], false); + await openMenu(); + expect( + screen.queryByRole("menuitem", { name: /player info/i }) + ).not.toBeInTheDocument(); + }); + it("shows 'Clone to sandbox' for an active, non-sandbox game", async () => { renderMenu(mockActiveGames[0]); await openMenu(); diff --git a/packages/web/src/components/GameDropdownMenu.tsx b/packages/web/src/components/GameDropdownMenu.tsx index 37ac3f92a..e70e7a7a1 100644 --- a/packages/web/src/components/GameDropdownMenu.tsx +++ b/packages/web/src/components/GameDropdownMenu.tsx @@ -67,7 +67,7 @@ interface GameDropdownMenuProps { | "status" >; onNavigateToGameInfo: () => void; - onNavigateToPlayerInfo: () => void; + onNavigateToPlayerInfo?: () => void; } export function GameDropdownMenu({ @@ -208,10 +208,12 @@ export function GameDropdownMenu({ Game info - - - Player info - + {onNavigateToPlayerInfo && ( + + + Player info + + )} { diff --git a/packages/web/src/components/PlayerInfoContent.test.tsx b/packages/web/src/components/PlayerInfoContent.test.tsx index 9a97951f1..f0922d025 100644 --- a/packages/web/src/components/PlayerInfoContent.test.tsx +++ b/packages/web/src/components/PlayerInfoContent.test.tsx @@ -11,6 +11,13 @@ const mockVariantsData = vi.fn(); const mockCurrentPhaseData = vi.fn(); const mockUserProfileData = vi.fn(); const mockKickMutateAsync = vi.fn(); +const mockChannelsData = vi.fn(); +const mockCreateChannelMutateAsync = vi.fn(); +const mockIsMobile = vi.fn(); + +vi.mock("@/hooks/use-mobile", () => ({ + useIsMobile: () => mockIsMobile(), +})); vi.mock("@/api/generated/endpoints", () => ({ useGameRetrieveSuspense: () => ({ data: mockGameData() }), @@ -22,8 +29,17 @@ vi.mock("@/api/generated/endpoints", () => ({ mutateAsync: mockKickMutateAsync, isPending: false, }), + useGamesChannelsList: () => ({ + data: mockChannelsData(), + isLoading: false, + }), + useGamesChannelsCreateCreate: () => ({ + mutateAsync: mockCreateChannelMutateAsync, + isPending: false, + }), getGameRetrieveQueryKey: () => ["game"], getGameAddableUserListQueryKey: () => ["addable-user"], + getGamesChannelsListQueryKey: () => ["channels"], })); vi.mock("@/components/NationFlag", () => ({ @@ -41,12 +57,20 @@ vi.mock("@/components/AddBotSheet", () => ({ open ?
: null, })); -const renderPlayerInfo = () => +const renderPlayerInfo = (initialEntry = "/game/game-1") => render( - + } /> + } + /> + } + /> @@ -79,8 +103,10 @@ describe("PlayerInfoContent", () => { beforeEach(() => { vi.clearAllMocks(); mockVariantsData.mockReturnValue([classicalVariant]); - mockCurrentPhaseData.mockReturnValue({ supplyCenters: [] }); + mockCurrentPhaseData.mockReturnValue({ supplyCenters: [], units: [] }); mockUserProfileData.mockReturnValue({ canCreateBotGames: true }); + mockChannelsData.mockReturnValue([]); + mockIsMobile.mockReturnValue(false); }); it("shows the civil disorder badge for members in civil disorder", () => { @@ -117,6 +143,268 @@ describe("PlayerInfoContent", () => { expect(screen.queryByText("Civil Disorder")).not.toBeInTheDocument(); }); + it("shows unit and supply-center counts from the current phase", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [{ ...baseMember }], + }); + mockCurrentPhaseData.mockReturnValue({ + units: [ + { nation: { name: "England" } }, + { nation: { name: "England" } }, + { nation: { name: "France" } }, + ], + supplyCenters: [ + { nation: { name: "England" } }, + { nation: { name: "England" } }, + { nation: { name: "England" } }, + ], + }); + + renderPlayerInfo(); + + expect(screen.getByText("2 units")).toBeInTheDocument(); + expect(screen.getByText("3 centers")).toBeInTheDocument(); + }); + + it("uses the nation as the in-game title and puts player details in a popover", async () => { + const user = userEvent.setup(); + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + sandbox: false, + nmrExtensionsAllowed: 2, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember, commitment: "high", nmrExtensionsRemaining: 1 }, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + + expect(screen.getByText("England")).toBeInTheDocument(); + expect(screen.queryByText("Alice")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("Commitment: High")).not.toBeInTheDocument(); + expect( + screen.queryByText("1 extension remaining") + ).not.toBeInTheDocument(); + + await user.click( + screen.getByRole("button", { name: "View England player details" }) + ); + + expect(screen.getByText("Alice")).toBeInTheDocument(); + expect(screen.getByLabelText("Commitment: High")).toBeInTheDocument(); + expect(screen.getByText("1 extension remaining")).toBeInTheDocument(); + expect(screen.queryByText("Player")).not.toBeInTheDocument(); + }); + + it("shows eliminated as the player's status", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [{ ...baseMember, eliminated: true, civilDisorder: true }], + }); + + renderPlayerInfo(); + + expect(screen.getByText("Eliminated")).toBeInTheDocument(); + expect(screen.queryByText("Civil Disorder")).not.toBeInTheDocument(); + }); + + it("dims and moves eliminated and civil-disorder powers to the bottom", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember, nation: "England", eliminated: true }, + { + ...baseMember, + id: 2, + name: "Bob", + nation: "France", + isCurrentUser: false, + }, + { + ...baseMember, + id: 3, + name: "Carol", + nation: "Germany", + isCurrentUser: false, + civilDisorder: true, + }, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + + expect( + screen + .getAllByRole("button", { name: /View .* player details/ }) + .map(button => button.getAttribute("aria-label")) + ).toEqual([ + "View France player details", + "View England player details", + "View Germany player details", + ]); + expect(screen.getByText("France").closest(".gap-4")).not.toHaveClass( + "opacity-[0.7]" + ); + expect(screen.getByText("England").closest(".gap-4")).toHaveClass( + "opacity-[0.7]" + ); + expect(screen.getByText("Germany").closest(".gap-4")).toHaveClass( + "opacity-[0.7]" + ); + }); + + it("moves the winner to the top when the game is completed", () => { + const winner = { + ...baseMember, + id: 3, + name: "Carol", + nation: "Germany", + isCurrentUser: false, + eliminated: true, + }; + + mockGameData.mockReturnValue({ + variantId: "classical", + status: "completed", + nmrExtensionsAllowed: 0, + victory: { + id: 1, + type: "solo", + winningPhaseId: 1, + members: [winner], + }, + phases: [{ id: 1, status: "completed" }], + members: [ + { ...baseMember, nation: "England", eliminated: true }, + { + ...baseMember, + id: 2, + name: "Bob", + nation: "France", + isCurrentUser: false, + }, + winner, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + + expect( + screen + .getAllByRole("button", { name: /View .* player details/ }) + .map(button => button.getAttribute("aria-label")) + ).toEqual([ + "View Germany player details", + "View France player details", + "View England player details", + ]); + }); + + it("opens an existing one-to-one chat with another human player", async () => { + const user = userEvent.setup(); + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + pressType: "regular", + sandbox: false, + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember }, + { ...baseMember, id: 2, name: "Bob", isCurrentUser: false }, + ], + }); + mockChannelsData.mockReturnValue([ + { id: 42, private: true, memberIds: [1, 2] }, + ]); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + await user.click(screen.getByLabelText("Message Bob")); + + expect(await screen.findByTestId("channel-screen")).toBeInTheDocument(); + expect(mockCreateChannelMutateAsync).not.toHaveBeenCalled(); + }); + + it("marks a player's chat shortcut when their direct channel is unread", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + pressType: "regular", + sandbox: false, + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember }, + { ...baseMember, id: 2, name: "Bob", isCurrentUser: false }, + ], + }); + mockChannelsData.mockReturnValue([ + { + id: 42, + private: true, + memberIds: [1, 2], + unreadMessageCount: 2, + }, + ]); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + + expect( + screen.getByRole("button", { + name: "Message Bob, 2 unread messages", + }) + ).toBeInTheDocument(); + }); + + it("creates a one-to-one chat when one does not exist", async () => { + const user = userEvent.setup(); + mockCreateChannelMutateAsync.mockResolvedValue({ + id: 43, + private: true, + memberIds: [1, 2], + }); + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + pressType: "regular", + sandbox: false, + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember }, + { ...baseMember, id: 2, name: "Bob", isCurrentUser: false }, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + await user.click(screen.getByLabelText("Message Bob")); + + expect(mockCreateChannelMutateAsync).toHaveBeenCalledWith({ + gameId: "game-1", + data: { memberIds: [2] }, + }); + expect(await screen.findByTestId("channel-screen")).toBeInTheDocument(); + }); + it("shows the game master above the players when one is set", () => { mockGameData.mockReturnValue({ variantId: "classical", diff --git a/packages/web/src/components/PlayerInfoContent.tsx b/packages/web/src/components/PlayerInfoContent.tsx index c80410926..78ff0ed31 100644 --- a/packages/web/src/components/PlayerInfoContent.tsx +++ b/packages/web/src/components/PlayerInfoContent.tsx @@ -1,6 +1,16 @@ import React, { useState } from "react"; -import { Bot, Shield, Star, Trophy, UserPlus, X } from "lucide-react"; -import { Link, useParams } from "react-router"; +import { + Bot, + MessageCircle, + Shield, + Star, + Swords, + Trophy, + User, + UserPlus, + X, +} from "lucide-react"; +import { Link, useNavigate, useParams } from "react-router"; import { toast } from "sonner"; import { useQueryClient } from "@tanstack/react-query"; @@ -14,28 +24,41 @@ import { Button } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; import { Badge } from "@/components/ui/badge"; import { ScreenCard, ScreenCardContent } from "@/components/ui/screen-card"; +import { + Popover, + PopoverContent, + PopoverTrigger, +} from "@/components/ui/popover"; import { useGameRetrieveSuspense, useGamePhaseRetrieve, useGameKickDestroy, useUserRetrieveSuspense, getGameAddableUserListQueryKey, + getGamesChannelsListQueryKey, getGameRetrieveQueryKey, + useGamesChannelsCreateCreate, + useGamesChannelsList, + type Channel, Member, } from "@/api/generated/endpoints"; import { useGameVariant } from "@/hooks/useGameVariant"; import { getCurrentPhaseId } from "@/util"; import { useRequiredParams } from "@/hooks"; +import { useIsMobile } from "@/hooks/use-mobile"; export const PlayerInfoContent: React.FC = () => { const { gameId } = useRequiredParams<{ gameId: string }>(); const { phaseId } = useParams<{ phaseId: string }>(); + const navigate = useNavigate(); + const isMobile = useIsMobile(); const { data: game } = useGameRetrieveSuspense(gameId); const variant = useGameVariant(game); const { data: userProfile } = useUserRetrieveSuspense(); const queryClient = useQueryClient(); const kickMutation = useGameKickDestroy(); + const createChannelMutation = useGamesChannelsCreateCreate(); const [addBotOpen, setAddBotOpen] = useState(false); @@ -46,6 +69,29 @@ export const PlayerInfoContent: React.FC = () => { { query: { enabled: !!currentPhaseId } } ); + const currentMember = game.members.find(member => member.isCurrentUser); + const isNoPressActiveGame = + game.pressType === "no_press" && + game.status !== "completed" && + game.status !== "abandoned"; + const canUseChat = + !!phaseId && !!currentMember && !game.sandbox && !isNoPressActiveGame; + const channelsQuery = useGamesChannelsList(gameId, { + query: { enabled: canUseChat }, + }); + + const getDirectChannel = (member: Member) => + channelsQuery.data?.find(channel => { + const memberIds = channel.memberIds ?? []; + return ( + channel.private && + memberIds.length === 2 && + !!currentMember && + memberIds.includes(currentMember.id) && + memberIds.includes(member.id) + ); + }); + const getSupplyCenterCount = (member: Member) => { if (!currentPhase) return undefined; return currentPhase.supplyCenters.filter( @@ -53,6 +99,12 @@ export const PlayerInfoContent: React.FC = () => { ).length; }; + const getUnitCount = (member: Member) => { + if (!currentPhase) return undefined; + return currentPhase.units.filter(unit => unit.nation.name === member.nation) + .length; + }; + const winnerIds = game.victory?.members?.map(m => m.id) || []; const isPending = game.status === "pending"; @@ -64,6 +116,18 @@ export const PlayerInfoContent: React.FC = () => { : 0; const canAddBots = isPending && game.canManage && userProfile.canCreateBotGames; + const sortedMembers = [...game.members].sort((a, b) => { + if (game.status === "completed") { + const winnerOrder = + Number(winnerIds.includes(b.id)) - Number(winnerIds.includes(a.id)); + if (winnerOrder !== 0) return winnerOrder; + } + + return ( + Number(!!a.eliminated || !!a.civilDisorder) - + Number(!!b.eliminated || !!b.civilDisorder) + ); + }); const handleRemoveBot = async (member: Member) => { try { @@ -82,6 +146,33 @@ export const PlayerInfoContent: React.FC = () => { } }; + const handleOpenChat = async (member: Member) => { + if (!phaseId || !currentMember) return; + + const existingChannel = getDirectChannel(member); + + if (existingChannel) { + navigate( + `/game/${gameId}/phase/${phaseId}/chat/channel/${existingChannel.id}` + ); + return; + } + + try { + const channel = await createChannelMutation.mutateAsync({ + gameId, + data: { memberIds: [member.id] }, + }); + queryClient.setQueryData( + getGamesChannelsListQueryKey(gameId), + old => [...(old ?? []), channel] + ); + navigate(`/game/${gameId}/phase/${phaseId}/chat/channel/${channel.id}`); + } catch { + toast.error("Failed to open chat"); + } + }; + return ( <> @@ -110,34 +201,62 @@ export const PlayerInfoContent: React.FC = () => {
)} - {game.members.map(member => { + {sortedMembers.map(member => { const supplyCenterCount = getSupplyCenterCount(member); + const unitCount = getUnitCount(member); const isWinner = winnerIds.includes(member.id); + const flagUrl = variant + ? findNationFlagUrl(variant.nations, member.nation) + : null; + const nationColor = variant + ? findNationColor(variant.nations, member.nation) + : null; + const showChatShortcut = + canUseChat && + !member.isCurrentUser && + !member.isBot && + !member.kicked; + const directChannel = showChatShortcut + ? getDirectChannel(member) + : undefined; + const unreadMessageCount = directChannel?.unreadMessageCount ?? 0; + const showNationFocusedLayout = !!phaseId && !!member.nation; return (
- {member.nation && variant && ( - - )} + {member.nation && + variant && + (flagUrl ? ( + + ) : ( + + ))}
- {member.userId ? ( + {showNationFocusedLayout ? ( + {member.nation} + ) : member.userId ? ( {member.name} @@ -151,9 +270,11 @@ export const PlayerInfoContent: React.FC = () => { Bot )} - {!member.isBot && member.commitment && ( - - )} + {!showNationFocusedLayout && + !member.isBot && + member.commitment && ( + + )} {member.isGameCreator && ( @@ -166,33 +287,140 @@ export const PlayerInfoContent: React.FC = () => { {game.victory?.type === "solo" ? "Winner" : "Draw"} )} - {member.civilDisorder && } + {member.eliminated ? ( + Eliminated + ) : ( + member.civilDisorder && + )}
{member.nation && (
- {member.nation} + {!showNationFocusedLayout && ( + <> + {member.nation} + + + )} + + + {unitCount !== undefined ? ( + {unitCount} units + ) : ( + + )} + {supplyCenterCount !== undefined ? ( - {supplyCenterCount} + {supplyCenterCount} centers ) : ( - + )} - {game.nmrExtensionsAllowed > 0 && ( - <> - - {member.nmrExtensionsRemaining} ext. remaining - - )}
)}
+ {showNationFocusedLayout && ( +
+ {showChatShortcut && ( + + )} + + + + + + +
+
+ {member.userId ? ( + + {member.name} + + ) : ( +

+ {member.name} +

+ )} + {!member.isBot && member.commitment && ( + + )} +
+ {game.nmrExtensionsAllowed > 0 && ( +

+ {member.nmrExtensionsRemaining}{" "} + {member.nmrExtensionsRemaining === 1 + ? "extension" + : "extensions"}{" "} + remaining +

+ )} +
+
+
+
+ )} + + {!showNationFocusedLayout && showChatShortcut && ( + + )} + {isPending && game.canManage && member.isBot && ( , +})); + +describe("GameInfoScreen", () => { + it("returns to Overview when closed", async () => { + const user = userEvent.setup(); + render( + + + } + /> + Overview screen
} + /> + + + ); + + await user.click( + screen.getByRole("button", { name: "Close Game Info" }) + ); + + expect(screen.getByText("Overview screen")).toBeInTheDocument(); + }); +}); diff --git a/packages/web/src/screens/GameDetail/GameInfoScreen.tsx b/packages/web/src/screens/GameDetail/GameInfoScreen.tsx index 555881fa1..116957dbb 100644 --- a/packages/web/src/screens/GameDetail/GameInfoScreen.tsx +++ b/packages/web/src/screens/GameDetail/GameInfoScreen.tsx @@ -13,14 +13,16 @@ const GameInfoScreen: React.FC = () => { }>(); const handleNavigateToPlayerInfo = () => { - navigate(`/game/${gameId}/phase/${phaseId}/player-info`); + navigate(`/game/${gameId}/phase/${phaseId}/overview`); }; return (
navigate(`/game/${gameId}/phase/${phaseId}`)} + onNavigateBack={() => + navigate(`/game/${gameId}/phase/${phaseId}/overview`) + } variant="secondary" />
diff --git a/packages/web/src/screens/GameDetail/MapScreen.tsx b/packages/web/src/screens/GameDetail/MapScreen.tsx index eaea6a93f..342664876 100644 --- a/packages/web/src/screens/GameDetail/MapScreen.tsx +++ b/packages/web/src/screens/GameDetail/MapScreen.tsx @@ -22,10 +22,6 @@ const MapScreen: React.FC = () => { navigate(`/game/${gameId}/phase/${phaseId}/game-info`); }; - const handleNavigateToPlayerInfo = () => { - navigate(`/game/${gameId}/phase/${phaseId}/player-info`); - }; - return (
{
} diff --git a/packages/web/src/screens/GameDetail/OrdersScreen.test.tsx b/packages/web/src/screens/GameDetail/OrdersScreen.test.tsx index 29edc83bf..91135c164 100644 --- a/packages/web/src/screens/GameDetail/OrdersScreen.test.tsx +++ b/packages/web/src/screens/GameDetail/OrdersScreen.test.tsx @@ -52,7 +52,9 @@ vi.mock("@/components/NationFlag", () => ({ })); vi.mock("@/components/PhaseSelect", () => ({ PhaseSelect: () => null })); vi.mock("@/components/PhaseGuidance", () => ({ PhaseGuidance: () => null })); -vi.mock("@/components/GameDropdownMenu", () => ({ GameDropdownMenu: () => null })); +vi.mock("@/components/GameDropdownMenu", () => ({ + GameDropdownMenu: () => , +})); const baseMember = (overrides = {}) => ({ id: 1, @@ -219,6 +221,12 @@ describe("OrdersScreen confirm orders button", () => { renderOrdersScreen(); expect(screen.getByRole("button", { name: /confirm orders/i })).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /draw proposals/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /game menu/i }) + ).not.toBeInTheDocument(); }); }); diff --git a/packages/web/src/screens/GameDetail/OrdersScreen.tsx b/packages/web/src/screens/GameDetail/OrdersScreen.tsx index 697862783..e65e749cb 100644 --- a/packages/web/src/screens/GameDetail/OrdersScreen.tsx +++ b/packages/web/src/screens/GameDetail/OrdersScreen.tsx @@ -10,13 +10,11 @@ import { SearchX, Star, UserX, - Handshake, Eye, } from "lucide-react"; import { toast } from "sonner"; import { QueryErrorBoundary } from "@/components/QueryErrorBoundary"; -import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Separator } from "@/components/ui/separator"; import { @@ -37,7 +35,6 @@ import { import { Notice } from "@/components/Notice"; import { NationFlag, findNationFlagUrl, findNationColor } from "@/components/NationFlag"; import { NationBadge } from "@/components/NationBadge"; -import { GameDropdownMenu } from "@/components/GameDropdownMenu"; import { GameDetailAppBar } from "./AppBar"; import { Panel } from "@/components/Panel"; import { PhaseSelect } from "@/components/PhaseSelect"; @@ -54,7 +51,6 @@ import { useGameConfirmPhasePartialUpdate, useGameResolvePhaseCreate, useGameRetrieveSuspense, - useGamesDrawProposalsListSuspense, useGameRecoverFromCivilDisorderCreate, getGameRetrieveQueryKey, getGameOrdersListQueryKey, @@ -136,26 +132,6 @@ const buildNationGroups = ( }); }; -const DrawProposalsBadge: React.FC<{ gameId: string; currentMemberId?: number }> = ({ - gameId, - currentMemberId, -}) => { - const { data: proposals } = useGamesDrawProposalsListSuspense(gameId); - if (currentMemberId === undefined) return null; - const count = proposals.filter( - p => p.status === "pending" && p.myVote !== null && p.myVote.accepted === null - ).length; - if (count === 0) return null; - return ( - - {count} - - ); -}; - const OrdersScreen: React.FC = () => { const navigate = useNavigate(); const queryClient = useQueryClient(); @@ -265,14 +241,6 @@ const OrdersScreen: React.FC = () => { } }; - const handleNavigateToGameInfo = () => { - navigate(`/game/${gameId}/phase/${phaseId}/game-info`); - }; - - const handleNavigateToPlayerInfo = () => { - navigate(`/game/${gameId}/phase/${phaseId}/player-info`); - }; - const nationGroups = buildNationGroups( isActivePhase, safePhaseStates, @@ -282,16 +250,6 @@ const OrdersScreen: React.FC = () => { ); const hasContent = nationGroups.length > 0; - const isStartedGame = - game.status === "active" || - game.status === "completed" || - game.status === "abandoned"; - const showDrawProposalsButton = !game.sandbox && isStartedGame; - - const handleNavigateToDrawProposals = () => { - navigate(`/game/${gameId}/phase/${phaseId}/draw-proposals`); - }; - const rightFooterButton = (() => { if (!canModifyOrders) return null; if (game.sandbox) @@ -343,18 +301,11 @@ const OrdersScreen: React.FC = () => {
-
- - - - -
- +
+ + + +
} onNavigateBack={() => navigate("/")} @@ -478,27 +429,9 @@ const OrdersScreen: React.FC = () => { )} - {!isCurrentMemberInCivilDisorder && (rightFooterButton || showDrawProposalsButton) && ( + {!isCurrentMemberInCivilDisorder && rightFooterButton && ( -
-
- {showDrawProposalsButton && ( - - )} -
+
{rightFooterButton}
diff --git a/packages/web/src/screens/GameDetail/OverviewScreen.test.tsx b/packages/web/src/screens/GameDetail/OverviewScreen.test.tsx new file mode 100644 index 000000000..b9c666bff --- /dev/null +++ b/packages/web/src/screens/GameDetail/OverviewScreen.test.tsx @@ -0,0 +1,63 @@ +import { render, screen, within } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router"; +import { describe, expect, it, vi } from "vitest"; + +import { OverviewScreen } from "./OverviewScreen"; + +const mockGameData = vi.fn(); +const mockDrawProposalsData = vi.fn(); + +vi.mock("@/api/generated/endpoints", () => ({ + useGameRetrieveSuspense: () => ({ data: mockGameData() }), + useGamesDrawProposalsList: () => ({ data: mockDrawProposalsData() }), +})); + +vi.mock("@/components/PlayerInfoContent", () => ({ + PlayerInfoContent: () =>
Players
, +})); + +vi.mock("@/components/GameDropdownMenu", () => ({ + GameDropdownMenu: () => , +})); + +vi.mock("./AppBar", () => ({ + GameDetailAppBar: ({ rightButton }: { rightButton?: React.ReactNode }) => ( +
{rightButton}
+ ), +})); + +const renderOverview = () => + render( + + + } + /> + + + ); + +describe("OverviewScreen draw proposals", () => { + it("shows the active-proposal notification badge on the button", () => { + mockGameData.mockReturnValue({ + status: "active", + sandbox: false, + }); + mockDrawProposalsData.mockReturnValue([ + { + id: 1, + status: "pending", + myVote: { included: true, accepted: null }, + }, + ]); + + renderOverview(); + + const link = screen.getByRole("link", { name: /draw proposals/i }); + expect(within(link).getByText("1")).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /game menu/i }) + ).toBeInTheDocument(); + }); +}); diff --git a/packages/web/src/screens/GameDetail/OverviewScreen.tsx b/packages/web/src/screens/GameDetail/OverviewScreen.tsx new file mode 100644 index 000000000..b0df60064 --- /dev/null +++ b/packages/web/src/screens/GameDetail/OverviewScreen.tsx @@ -0,0 +1,85 @@ +import React, { Suspense } from "react"; +import { Handshake } from "lucide-react"; +import { Link, useNavigate } from "react-router"; + +import { + useGameRetrieveSuspense, + useGamesDrawProposalsList, +} from "@/api/generated/endpoints"; +import { + DrawProposalNotificationBadge, + getDrawProposalNotificationCount, +} from "@/components/DrawProposalNotificationBadge"; +import { GameDropdownMenu } from "@/components/GameDropdownMenu"; +import { Panel } from "@/components/Panel"; +import { PlayerInfoContent } from "@/components/PlayerInfoContent"; +import { QueryErrorBoundary } from "@/components/QueryErrorBoundary"; +import { Button } from "@/components/ui/button"; +import { useRequiredParams } from "@/hooks"; +import { GameDetailAppBar } from "./AppBar"; + +const OverviewScreen: React.FC = () => { + const navigate = useNavigate(); + const { gameId, phaseId } = useRequiredParams<{ + gameId: string; + phaseId: string; + }>(); + const { data: game } = useGameRetrieveSuspense(gameId); + + const isStartedGame = + game.status === "active" || + game.status === "completed" || + game.status === "abandoned"; + const showDrawProposals = !game.sandbox && isStartedGame; + const { data: drawProposals } = useGamesDrawProposalsList(gameId, { + query: { enabled: showDrawProposals }, + }); + const drawProposalNotificationCount = + getDrawProposalNotificationCount(drawProposals); + + return ( +
+ + navigate(`/game/${gameId}/phase/${phaseId}/game-info`) + } + /> + } + /> +
+ + + + + {showDrawProposals && ( + + + + )} + +
+
+ ); +}; + +const OverviewScreenSuspense: React.FC = () => ( + + }> + + + +); + +export { OverviewScreenSuspense as OverviewScreen }; diff --git a/packages/web/src/screens/GameDetail/PlayerInfoScreen.tsx b/packages/web/src/screens/GameDetail/PlayerInfoScreen.tsx index d2522b46b..4fd552bdc 100644 --- a/packages/web/src/screens/GameDetail/PlayerInfoScreen.tsx +++ b/packages/web/src/screens/GameDetail/PlayerInfoScreen.tsx @@ -1,39 +1,23 @@ -import React, { Suspense } from "react"; -import { useNavigate } from "react-router"; -import { GameDetailAppBar } from "./AppBar"; -import { Panel } from "@/components/Panel"; -import { PlayerInfoContent } from "@/components/PlayerInfoContent"; +import React from "react"; +import { Navigate, useLocation } from "react-router"; import { useRequiredParams } from "@/hooks"; const PlayerInfoScreen: React.FC = () => { - const navigate = useNavigate(); + const location = useLocation(); const { gameId, phaseId } = useRequiredParams<{ gameId: string; phaseId: string; }>(); return ( -
- navigate(`/game/${gameId}/phase/${phaseId}`)} - variant="secondary" - /> -
- - - - - -
-
+ ); }; -const PlayerInfoScreenSuspense: React.FC = () => ( -
}> - - -); - -export { PlayerInfoScreenSuspense as PlayerInfoScreen }; +export { PlayerInfoScreen }; diff --git a/packages/web/src/screens/GameDetail/PlayerProfileScreen.tsx b/packages/web/src/screens/GameDetail/PlayerProfileScreen.tsx index 8943295bc..b8e23a803 100644 --- a/packages/web/src/screens/GameDetail/PlayerProfileScreen.tsx +++ b/packages/web/src/screens/GameDetail/PlayerProfileScreen.tsx @@ -18,7 +18,7 @@ const PlayerProfileScreen: React.FC = () => { - navigate(`/game/${gameId}/phase/${phaseId}/player-info`) + navigate(`/game/${gameId}/phase/${phaseId}/overview`) } variant="secondary" /> diff --git a/packages/web/src/screens/GameDetail/ProposeDrawScreen.tsx b/packages/web/src/screens/GameDetail/ProposeDrawScreen.tsx index 50938961f..9fe6f2e0a 100644 --- a/packages/web/src/screens/GameDetail/ProposeDrawScreen.tsx +++ b/packages/web/src/screens/GameDetail/ProposeDrawScreen.tsx @@ -61,7 +61,7 @@ const ProposeDrawScreen: React.FC = () => { }; const handleBack = () => { - navigate(`/game/${gameId}/phase/${phaseId}/orders`); + navigate(`/game/${gameId}/phase/${phaseId}/draw-proposals`); }; const isSubmitting = createProposalMutation.isPending; diff --git a/packages/web/src/screens/GameDetail/index.ts b/packages/web/src/screens/GameDetail/index.ts index 7a490f6b8..138c951b3 100644 --- a/packages/web/src/screens/GameDetail/index.ts +++ b/packages/web/src/screens/GameDetail/index.ts @@ -3,6 +3,10 @@ import { lazyScreen } from "../../utils/lazyScreen"; export const GameDetail = { MapScreen: lazyScreen(() => import("./MapScreen"), "MapScreen"), OrdersScreen: lazyScreen(() => import("./OrdersScreen"), "OrdersScreen"), + OverviewScreen: lazyScreen( + () => import("./OverviewScreen"), + "OverviewScreen" + ), ChannelListScreen: lazyScreen( () => import("./ChannelListScreen"), "ChannelListScreen" From 6a2674915f43d5fea1fe86cdab02b5af3b3d642c Mon Sep 17 00:00:00 2001 From: aj-read <58521077+aj-read@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:43:05 +0100 Subject: [PATCH 2/3] Removed duplicate Player Info menu on Overview tab --- .../src/components/PlayerInfoContent.test.tsx | 132 +++++++++--------- .../web/src/components/PlayerInfoContent.tsx | 54 ++++--- 2 files changed, 97 insertions(+), 89 deletions(-) diff --git a/packages/web/src/components/PlayerInfoContent.test.tsx b/packages/web/src/components/PlayerInfoContent.test.tsx index d796449f4..534b27c8f 100644 --- a/packages/web/src/components/PlayerInfoContent.test.tsx +++ b/packages/web/src/components/PlayerInfoContent.test.tsx @@ -214,6 +214,7 @@ describe("PlayerInfoContent", () => { renderPlayerInfo("/game/game-1/phase/1/overview"); expect(screen.getByText("England")).toBeInTheDocument(); + expect(screen.getByText("you")).toBeInTheDocument(); expect(screen.queryByText("Alice")).not.toBeInTheDocument(); expect(screen.queryByLabelText("Commitment: High")).not.toBeInTheDocument(); expect(screen.queryByText("1 extension remaining")).not.toBeInTheDocument(); @@ -293,6 +294,58 @@ describe("PlayerInfoContent", () => { ); }); + it("orders powers by supply-center count and preserves ordinary order for ties", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember, nation: "England" }, + { + ...baseMember, + id: 2, + name: "Bob", + nation: "France", + isCurrentUser: false, + }, + { + ...baseMember, + id: 3, + name: "Carol", + nation: "Germany", + isCurrentUser: false, + }, + ], + }); + mockCurrentPhaseData.mockReturnValue({ + units: [], + supplyCenters: [ + { nation: { name: "England" } }, + { nation: { name: "England" } }, + { nation: { name: "France" } }, + { nation: { name: "France" } }, + { nation: { name: "France" } }, + { nation: { name: "France" } }, + { nation: { name: "Germany" } }, + { nation: { name: "Germany" } }, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + + expect( + screen + .getAllByRole("button", { name: /View .* player details/ }) + .map(button => button.getAttribute("aria-label")) + ).toEqual([ + "View France player details", + "View England player details", + "View Germany player details", + ]); + }); + it("moves the winner to the top when the game is completed", () => { const winner = { ...baseMember, @@ -617,87 +670,36 @@ describe("PlayerInfoContent", () => { }); }); - it("does not offer Remove Player to non-admins", async () => { - const user = userEvent.setup(); + it("does not show player options when no management action is available", () => { mockGameData.mockReturnValue({ variantId: "classical", - status: "pending", + status: "active", canManage: false, nmrExtensionsAllowed: 0, victory: null, - phases: [], + phases: [{ id: 1, status: "active" }], members: [ - { ...baseMember, nation: null }, + { ...baseMember }, { ...baseMember, id: 2, - name: "The Dealmaker", + userId: 77, + name: "Bob", isCurrentUser: false, - isBot: true, - nation: null, + nation: "France", removable: true, }, ], }); - renderPlayerInfo(); + renderPlayerInfo("/game/game-1/phase/1/overview"); - await user.click(screen.getByLabelText("Options for The Dealmaker")); expect( - screen.getByRole("menuitem", { name: "View Profile" }) - ).toBeInTheDocument(); - expect( - screen.queryByRole("menuitem", { name: "Remove Player" }) + screen.queryByLabelText("Options for Bob") ).not.toBeInTheDocument(); - }); - - it("navigates to the profile from the player menu", async () => { - const user = userEvent.setup(); - mockGameData.mockReturnValue({ - variantId: "classical", - status: "active", - nmrExtensionsAllowed: 0, - victory: null, - phases: [{ id: 1, status: "active" }], - members: [ - { ...baseMember, id: 1, name: "Alice" }, - { ...baseMember, id: 2, userId: 77, name: "Bob", isCurrentUser: false }, - ], - }); - - renderPlayerInfo(); - - await user.click(screen.getByLabelText("Options for Bob")); - const item = screen.getByRole("menuitem", { name: "View Profile" }); - expect(item).not.toHaveAttribute("aria-disabled", "true"); - }); - - it("disables View Profile for a member with no profile to link to", async () => { - const user = userEvent.setup(); - mockGameData.mockReturnValue({ - variantId: "classical", - status: "active", - nmrExtensionsAllowed: 0, - victory: null, - phases: [{ id: 1, status: "active" }], - members: [ - { ...baseMember, id: 1, name: "Alice" }, - { - ...baseMember, - id: 2, - userId: null, - name: "Anonymous", - isCurrentUser: false, - }, - ], - }); - - renderPlayerInfo(); - - await user.click(screen.getByLabelText("Options for Anonymous")); expect( - screen.getByRole("menuitem", { name: "View Profile" }) - ).toHaveAttribute("aria-disabled", "true"); + screen.getByRole("button", { name: "View France player details" }) + ).toBeInTheDocument(); }); it("shows the removed badge for a kicked member", () => { @@ -810,8 +812,7 @@ describe("PlayerInfoContent", () => { ).not.toBeInTheDocument(); }); - it("does not offer Remove Player for a member who has not missed orders", async () => { - const user = userEvent.setup(); + it("does not show player options for a member who cannot be removed", () => { mockGameData.mockReturnValue({ variantId: "classical", status: "active", @@ -833,9 +834,8 @@ describe("PlayerInfoContent", () => { renderPlayerInfo(); - await user.click(screen.getByLabelText("Options for Bob")); expect( - screen.queryByRole("menuitem", { name: "Remove Player" }) + screen.queryByLabelText("Options for Bob") ).not.toBeInTheDocument(); }); diff --git a/packages/web/src/components/PlayerInfoContent.tsx b/packages/web/src/components/PlayerInfoContent.tsx index 5218641e1..1292e55e7 100644 --- a/packages/web/src/components/PlayerInfoContent.tsx +++ b/packages/web/src/components/PlayerInfoContent.tsx @@ -28,6 +28,7 @@ import { findNationFlagUrl, findNationColor, } from "@/components/NationFlag"; +import { NationBadge } from "@/components/NationBadge"; import { NationSeatFlag, getNationSeatLabel } from "@/components/NationSeat"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { @@ -149,6 +150,10 @@ export const PlayerInfoContent: React.FC = () => { const canAddBots = isPending && game.canManage && userProfile.canCreateBotGames; const sortedMembers = [...game.members].sort((a, b) => { + const supplyCenterOrder = + (getSupplyCenterCount(b) ?? 0) - (getSupplyCenterCount(a) ?? 0); + if (supplyCenterOrder !== 0) return supplyCenterOrder; + if (game.status === "completed") { const winnerOrder = Number(winnerIds.includes(b.id)) - Number(winnerIds.includes(a.id)); @@ -303,7 +308,17 @@ export const PlayerInfoContent: React.FC = () => {
{showNationFocusedLayout ? ( - {member.nation} + <> + {member.nation} + {member.isCurrentUser && variant && ( + + you + + )} + ) : member.userId ? ( { )} - - - - - - navigate(profilePath(member))} - > - - View Profile - - {canRemove(member) && ( + {canRemove(member) && ( + + + + + setMemberToRemove(member)} > Remove Player - )} - - + + + )}
); From dcf58ae096fb1c810d6857bc8cb38eae3f9971a3 Mon Sep 17 00:00:00 2001 From: aj-read <58521077+aj-read@users.noreply.github.com> Date: Sat, 29 Aug 2026 09:22:32 +0100 Subject: [PATCH 3/3] Addressed code review findings: chat channel fix so that memberIDs are used properly. --- packages/web/src/api/generated/endpoints.ts | 20 +++-- .../src/components/PlayerInfoContent.test.tsx | 77 ++++++++++++++++++- .../web/src/components/PlayerInfoContent.tsx | 44 +++++++---- service/channel/serializers.py | 11 +++ service/channel/tests/test_channel.py | 7 ++ service/channel/views.py | 6 +- service/openapi-schema.yaml | 14 +++- 7 files changed, 152 insertions(+), 27 deletions(-) diff --git a/packages/web/src/api/generated/endpoints.ts b/packages/web/src/api/generated/endpoints.ts index d06c4eebb..5297ddc07 100644 --- a/packages/web/src/api/generated/endpoints.ts +++ b/packages/web/src/api/generated/endpoints.ts @@ -106,6 +106,10 @@ export interface Channel { readonly private: boolean; readonly messages: readonly ChannelMessage[]; readonly unreadMessageCount: number; + readonly memberIds: readonly number[]; +} + +export interface ChannelCreate { memberIds: number[]; } @@ -7225,14 +7229,14 @@ method that returns the game object. Also adds game to the serializer context. */ export const gamesChannelsCreateCreate = ( gameId: string, - channel: NonReadonly, + channelCreate: ChannelCreate, signal?: AbortSignal ) => { return customInstance({ url: `/games/${gameId}/channels/create/`, method: "POST", headers: { "Content-Type": "application/json" }, - data: channel, + data: channelCreate, signal, }); }; @@ -7244,13 +7248,13 @@ export const getGamesChannelsCreateCreateMutationOptions = < mutation?: UseMutationOptions< Awaited>, TError, - { gameId: string; data: NonReadonly }, + { gameId: string; data: ChannelCreate }, TContext >; }): UseMutationOptions< Awaited>, TError, - { gameId: string; data: NonReadonly }, + { gameId: string; data: ChannelCreate }, TContext > => { const mutationKey = ["gamesChannelsCreateCreate"]; @@ -7264,7 +7268,7 @@ export const getGamesChannelsCreateCreateMutationOptions = < const mutationFn: MutationFunction< Awaited>, - { gameId: string; data: NonReadonly } + { gameId: string; data: ChannelCreate } > = props => { const { gameId, data } = props ?? {}; @@ -7277,7 +7281,7 @@ export const getGamesChannelsCreateCreateMutationOptions = < export type GamesChannelsCreateCreateMutationResult = NonNullable< Awaited> >; -export type GamesChannelsCreateCreateMutationBody = NonReadonly; +export type GamesChannelsCreateCreateMutationBody = ChannelCreate; export type GamesChannelsCreateCreateMutationError = unknown; export const useGamesChannelsCreateCreate = < @@ -7288,7 +7292,7 @@ export const useGamesChannelsCreateCreate = < mutation?: UseMutationOptions< Awaited>, TError, - { gameId: string; data: NonReadonly }, + { gameId: string; data: ChannelCreate }, TContext >; }, @@ -7296,7 +7300,7 @@ export const useGamesChannelsCreateCreate = < ): UseMutationResult< Awaited>, TError, - { gameId: string; data: NonReadonly }, + { gameId: string; data: ChannelCreate }, TContext > => { return useMutation( diff --git a/packages/web/src/components/PlayerInfoContent.test.tsx b/packages/web/src/components/PlayerInfoContent.test.tsx index 534b27c8f..0e907a0c8 100644 --- a/packages/web/src/components/PlayerInfoContent.test.tsx +++ b/packages/web/src/components/PlayerInfoContent.test.tsx @@ -21,6 +21,7 @@ const mockCurrentPhaseData = vi.fn(); const mockUserProfileData = vi.fn(); const mockKickMutateAsync = vi.fn(); const mockChannelsData = vi.fn(); +const mockChannelsRefetch = vi.fn(); const mockCreateChannelMutateAsync = vi.fn(); const mockIsMobile = vi.fn(); @@ -41,6 +42,7 @@ vi.mock("@/api/generated/endpoints", () => ({ useGamesChannelsList: () => ({ data: mockChannelsData(), isLoading: false, + refetch: mockChannelsRefetch, }), useGamesChannelsCreateCreate: () => ({ mutateAsync: mockCreateChannelMutateAsync, @@ -132,6 +134,7 @@ describe("PlayerInfoContent", () => { mockCurrentPhaseData.mockReturnValue({ supplyCenters: [], units: [] }); mockUserProfileData.mockReturnValue({ canCreateBotGames: true }); mockChannelsData.mockReturnValue([]); + mockChannelsRefetch.mockResolvedValue({ data: [] }); mockIsMobile.mockReturnValue(false); }); @@ -197,6 +200,26 @@ describe("PlayerInfoContent", () => { expect(screen.getByText("3 centers")).toBeInTheDocument(); }); + it("uses singular unit and supply-center labels", () => { + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [{ ...baseMember }], + }); + mockCurrentPhaseData.mockReturnValue({ + units: [{ nation: { name: "England" } }], + supplyCenters: [{ nation: { name: "England" } }], + }); + + renderPlayerInfo(); + + expect(screen.getByText("1 unit")).toBeInTheDocument(); + expect(screen.getByText("1 center")).toBeInTheDocument(); + }); + it("uses the nation as the in-game title and puts player details in a popover", async () => { const user = userEvent.setup(); mockGameData.mockReturnValue({ @@ -210,7 +233,6 @@ describe("PlayerInfoContent", () => { { ...baseMember, commitment: "high", nmrExtensionsRemaining: 1 }, ], }); - renderPlayerInfo("/game/game-1/phase/1/overview"); expect(screen.getByText("England")).toBeInTheDocument(); @@ -245,7 +267,7 @@ describe("PlayerInfoContent", () => { expect(screen.queryByText("Civil Disorder")).not.toBeInTheDocument(); }); - it("dims and moves eliminated and civil-disorder powers to the bottom", () => { + it("dims and moves eliminated, civil-disorder, and removed powers to the bottom", () => { mockGameData.mockReturnValue({ variantId: "classical", status: "active", @@ -269,6 +291,24 @@ describe("PlayerInfoContent", () => { isCurrentUser: false, civilDisorder: true, }, + { + ...baseMember, + id: 4, + name: "Dave", + nation: "Turkey", + isCurrentUser: false, + kicked: true, + }, + ], + }); + mockCurrentPhaseData.mockReturnValue({ + units: [], + supplyCenters: [ + { nation: { name: "France" } }, + { nation: { name: "Turkey" } }, + { nation: { name: "Turkey" } }, + { nation: { name: "Turkey" } }, + { nation: { name: "Turkey" } }, ], }); @@ -280,6 +320,7 @@ describe("PlayerInfoContent", () => { .map(button => button.getAttribute("aria-label")) ).toEqual([ "View France player details", + "View Turkey player details", "View England player details", "View Germany player details", ]); @@ -292,6 +333,9 @@ describe("PlayerInfoContent", () => { expect(screen.getByText("Germany").closest(".gap-4")).toHaveClass( "opacity-[0.7]" ); + expect(screen.getByText("Turkey").closest(".gap-4")).toHaveClass( + "opacity-[0.7]" + ); }); it("orders powers by supply-center count and preserves ordinary order for ties", () => { @@ -486,6 +530,35 @@ describe("PlayerInfoContent", () => { ); }); + it("opens a channel created by another client when creation reports a duplicate", async () => { + const user = userEvent.setup(); + mockCreateChannelMutateAsync.mockRejectedValue(new Error("Duplicate channel")); + mockChannelsRefetch.mockResolvedValue({ + data: [{ id: 44, private: true, memberIds: [1, 2] }], + }); + mockGameData.mockReturnValue({ + variantId: "classical", + status: "active", + pressType: "regular", + sandbox: false, + nmrExtensionsAllowed: 0, + victory: null, + phases: [{ id: 1, status: "active" }], + members: [ + { ...baseMember }, + { ...baseMember, id: 2, name: "Bob", isCurrentUser: false }, + ], + }); + + renderPlayerInfo("/game/game-1/phase/1/overview"); + await user.click(screen.getByLabelText("Message Bob")); + + expect(mockChannelsRefetch).toHaveBeenCalled(); + expect(mockNavigate).toHaveBeenCalledWith( + "/game/game-1/phase/1/chat/channel/44" + ); + }); + it("shows the game master above the players when one is set", () => { mockGameData.mockReturnValue({ variantId: "classical", diff --git a/packages/web/src/components/PlayerInfoContent.tsx b/packages/web/src/components/PlayerInfoContent.tsx index 1292e55e7..0e396abd2 100644 --- a/packages/web/src/components/PlayerInfoContent.tsx +++ b/packages/web/src/components/PlayerInfoContent.tsx @@ -109,8 +109,11 @@ export const PlayerInfoContent: React.FC = () => { query: { enabled: canUseChat }, }); - const getDirectChannel = (member: Member) => - channelsQuery.data?.find(channel => { + const getDirectChannel = ( + member: Member, + channels = channelsQuery.data + ) => + channels?.find(channel => { const memberIds = channel.memberIds ?? []; return ( channel.private && @@ -149,20 +152,21 @@ export const PlayerInfoContent: React.FC = () => { : 0; const canAddBots = isPending && game.canManage && userProfile.canCreateBotGames; + const isInactivePower = (member: Member) => + !!member.eliminated || !!member.civilDisorder || !!member.kicked; const sortedMembers = [...game.members].sort((a, b) => { - const supplyCenterOrder = - (getSupplyCenterCount(b) ?? 0) - (getSupplyCenterCount(a) ?? 0); - if (supplyCenterOrder !== 0) return supplyCenterOrder; - if (game.status === "completed") { const winnerOrder = Number(winnerIds.includes(b.id)) - Number(winnerIds.includes(a.id)); if (winnerOrder !== 0) return winnerOrder; } + const inactiveOrder = + Number(isInactivePower(a)) - Number(isInactivePower(b)); + if (inactiveOrder !== 0) return inactiveOrder; + return ( - Number(!!a.eliminated || !!a.civilDisorder) - - Number(!!b.eliminated || !!b.civilDisorder) + (getSupplyCenterCount(b) ?? 0) - (getSupplyCenterCount(a) ?? 0) ); }); @@ -217,6 +221,17 @@ export const PlayerInfoContent: React.FC = () => { ); navigate(`/game/${gameId}/phase/${phaseId}/chat/channel/${channel.id}`); } catch { + const refreshedChannels = await channelsQuery.refetch(); + const existingChannel = getDirectChannel( + member, + refreshedChannels.data + ); + if (existingChannel) { + navigate( + `/game/${gameId}/phase/${phaseId}/chat/channel/${existingChannel.id}` + ); + return; + } toast.error("Failed to open chat"); } }; @@ -275,9 +290,7 @@ export const PlayerInfoContent: React.FC = () => {
{isPending && member.isCurrentUser && variant ? ( @@ -372,7 +385,9 @@ export const PlayerInfoContent: React.FC = () => { {unitCount !== undefined ? ( - {unitCount} units + + {unitCount} {unitCount === 1 ? "unit" : "units"} + ) : ( )} @@ -381,7 +396,10 @@ export const PlayerInfoContent: React.FC = () => { {supplyCenterCount !== undefined ? ( - {supplyCenterCount} centers + + {supplyCenterCount}{" "} + {supplyCenterCount === 1 ? "center" : "centers"} + ) : ( )} diff --git a/service/channel/serializers.py b/service/channel/serializers.py index 593afa320..1577b5d78 100644 --- a/service/channel/serializers.py +++ b/service/channel/serializers.py @@ -2,6 +2,7 @@ from django.apps import apps from django.conf import settings from django.utils import timezone +from drf_spectacular.utils import extend_schema_field from .models import Channel, ChannelMessage, ChannelMember from nation.serializers import NationSerializer @@ -49,7 +50,14 @@ class ChannelSerializer(serializers.Serializer): private = serializers.BooleanField(read_only=True) messages = ChannelMessageSerializer(many=True, read_only=True) unread_message_count = serializers.IntegerField(read_only=True, default=0) + member_ids = serializers.SerializerMethodField() + @extend_schema_field(serializers.ListField(child=serializers.IntegerField())) + def get_member_ids(self, channel): + return [member.id for member in channel.members.all()] + + +class ChannelCreateSerializer(serializers.Serializer): member_ids = serializers.ListField(child=serializers.IntegerField(), required=True, write_only=True) def validate_member_ids(self, value): @@ -75,6 +83,9 @@ def create(self, validated_data): game = self.context["game"] return Channel.objects.create_from_member_ids(request.user, validated_data["member_ids"], game) + def to_representation(self, instance): + return ChannelSerializer(instance, context=self.context).data + class ChannelMarkReadSerializer(serializers.Serializer): def create(self, validated_data): diff --git a/service/channel/tests/test_channel.py b/service/channel/tests/test_channel.py index 55bce97c0..f8aa1828b 100644 --- a/service/channel/tests/test_channel.py +++ b/service/channel/tests/test_channel.py @@ -34,6 +34,10 @@ def test_create_channel_success( assert response.status_code == status.HTTP_201_CREATED assert "id" in response.data assert response.data["name"] == "England, France" + assert set(response.data["member_ids"]) == { + active_game_with_phase_state.members.exclude(id=other_member.id).get().id, + other_member.id, + } @pytest.mark.django_db def test_create_channel_name_longer_than_250_characters( @@ -137,6 +141,9 @@ def test_list_channels_as_member(self, authenticated_client, active_game_with_ch assert "Public Channel" in channel_names assert "Private Non-Member" not in channel_names + private_channel = next(channel for channel in response.data if channel["name"] == "Private Member") + assert private_channel["member_ids"] == [active_game_with_channels.members.first().id] + @pytest.mark.django_db def test_list_channels_as_non_member( self, authenticated_client_for_tertiary_user, active_game_with_channels, classical_france_nation diff --git a/service/channel/views.py b/service/channel/views.py index 6e275ece3..a5151dcce 100644 --- a/service/channel/views.py +++ b/service/channel/views.py @@ -1,15 +1,17 @@ from rest_framework import permissions, generics, status from rest_framework.response import Response +from drf_spectacular.utils import extend_schema from common.permissions import IsActiveOrCompletedGame, IsGameMember, IsChannelMember, IsNotKickedGameMember, IsNotSandboxGame, IsNotNoPressActiveGame from .models import Channel -from .serializers import ChannelSerializer, ChannelMessageSerializer, ChannelMarkReadSerializer +from .serializers import ChannelSerializer, ChannelCreateSerializer, ChannelMessageSerializer, ChannelMarkReadSerializer from common.views import SelectedGameMixin, SelectedChannelMixin, CurrentGameMemberMixin +@extend_schema(request=ChannelCreateSerializer, responses={201: ChannelSerializer}) class ChannelCreateView(SelectedGameMixin, CurrentGameMemberMixin, generics.CreateAPIView): permission_classes = [permissions.IsAuthenticated, IsActiveOrCompletedGame, IsNotKickedGameMember, IsNotSandboxGame, IsNotNoPressActiveGame] - serializer_class = ChannelSerializer + serializer_class = ChannelCreateSerializer class ChannelMessageCreateView(SelectedGameMixin, SelectedChannelMixin, CurrentGameMemberMixin, generics.CreateAPIView): diff --git a/service/openapi-schema.yaml b/service/openapi-schema.yaml index 8707236f4..7cd93bab2 100644 --- a/service/openapi-schema.yaml +++ b/service/openapi-schema.yaml @@ -1316,7 +1316,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Channel' + $ref: '#/components/schemas/ChannelCreate' required: true security: - jwtAuth: [] @@ -1927,7 +1927,7 @@ components: type: array items: type: integer - writeOnly: true + readOnly: true required: - id - memberIds @@ -1935,6 +1935,16 @@ components: - name - private - unreadMessageCount + ChannelCreate: + type: object + properties: + memberIds: + type: array + items: + type: integer + writeOnly: true + required: + - memberIds ChannelMember: type: object properties: