Rename queue page to Set Pre-Draft Queue, remove VORP, fix re-add race condition (#436)
* Rename queue page to Set Pre-Draft Queue, remove VORP, fix re-add race condition - Rename page title, h1, button, error message, and tests from "Pre-Draft Rankings" to "Set Pre-Draft Queue" - Remove VORP values and "Sorted by VORP" labels from the all-players list - Fix bug where removing a player then immediately re-adding would fail with "already in queue": track in-flight removes in a ref and guard handleAdd against firing while a remove is still in-flight https://claude.ai/code/session_01TGAV9A2c72PNkL1ffY7Xeq * Address code review findings on Set Pre-Draft Queue page - Rename component function from PreDraftRankings to SetPreDraftQueue - Guard handleRemove against temp IDs — items added optimistically but not yet written to the DB have no server-side record to delete; skip the API call and just drop them from local state - Surface a toast when handleAdd is blocked by a pending remove instead of silently no-oping - Group both refs before their shared useEffect for readability - Drop justify-between from the All Players desktop heading (sole child) - Update test describe blocks to "Set Pre-Draft Queue Access" https://claude.ai/code/session_01TGAV9A2c72PNkL1ffY7Xeq --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
973e56142f
commit
faecfd8ecc
3 changed files with 30 additions and 22 deletions
|
|
@ -121,7 +121,7 @@ function DraftInfoCardVariant({
|
||||||
</Button>
|
</Button>
|
||||||
) : queueBuilderHref ? (
|
) : queueBuilderHref ? (
|
||||||
<Button asChild variant="outline">
|
<Button asChild variant="outline">
|
||||||
<Link to={queueBuilderHref}>Set Pre-Draft Rankings</Link>
|
<Link to={queueBuilderHref}>Set Pre-Draft Queue</Link>
|
||||||
</Button>
|
</Button>
|
||||||
) : null}
|
) : null}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from "~/components/ui/tabs";
|
||||||
import type { Route } from "./+types/$leagueId.draft-queue.$seasonId";
|
import type { Route } from "./+types/$leagueId.draft-queue.$seasonId";
|
||||||
|
|
||||||
export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors {
|
export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors {
|
||||||
return [{ title: `Pre-Draft Rankings — ${data?.season?.league?.name ?? "League"} - Brackt` }];
|
return [{ title: `Set Pre-Draft Queue — ${data?.season?.league?.name ?? "League"} - Brackt` }];
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function loader(args: Route.LoaderArgs) {
|
export async function loader(args: Route.LoaderArgs) {
|
||||||
|
|
@ -41,7 +41,7 @@ export async function loader(args: Route.LoaderArgs) {
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!userId) {
|
if (!userId) {
|
||||||
throw new Response("You must be logged in to set pre-draft rankings", { status: 401 });
|
throw new Response("You must be logged in to set pre-draft queue", { status: 401 });
|
||||||
}
|
}
|
||||||
|
|
||||||
const userTeam = season.teams.find((t) => t.ownerId === userId);
|
const userTeam = season.teams.find((t) => t.ownerId === userId);
|
||||||
|
|
@ -64,7 +64,7 @@ export async function loader(args: Route.LoaderArgs) {
|
||||||
|
|
||||||
type QueueItem = { id: string; participantId: string };
|
type QueueItem = { id: string; participantId: string };
|
||||||
|
|
||||||
export default function PreDraftRankings() {
|
export default function SetPreDraftQueue() {
|
||||||
const { season, userTeam, availableParticipants, userQueue } =
|
const { season, userTeam, availableParticipants, userQueue } =
|
||||||
useLoaderData<typeof loader>();
|
useLoaderData<typeof loader>();
|
||||||
const { leagueId } = useParams<{ leagueId: string }>();
|
const { leagueId } = useParams<{ leagueId: string }>();
|
||||||
|
|
@ -79,6 +79,9 @@ export default function PreDraftRankings() {
|
||||||
|
|
||||||
// Always-current ref so reorder callback stays stable (no localQueue dependency)
|
// Always-current ref so reorder callback stays stable (no localQueue dependency)
|
||||||
const queueRef = useRef(localQueue);
|
const queueRef = useRef(localQueue);
|
||||||
|
// Track in-flight removes to prevent add racing ahead of a pending remove
|
||||||
|
const pendingRemovesRef = useRef(new Set<string>());
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
queueRef.current = localQueue;
|
queueRef.current = localQueue;
|
||||||
}, [localQueue]);
|
}, [localQueue]);
|
||||||
|
|
@ -91,6 +94,10 @@ export default function PreDraftRankings() {
|
||||||
const handleAdd = useCallback(
|
const handleAdd = useCallback(
|
||||||
async (participantId: string) => {
|
async (participantId: string) => {
|
||||||
if (queuedParticipantIds.has(participantId)) return;
|
if (queuedParticipantIds.has(participantId)) return;
|
||||||
|
if (pendingRemovesRef.current.has(participantId)) {
|
||||||
|
toast("Player is being removed — try again in a moment");
|
||||||
|
return;
|
||||||
|
}
|
||||||
const tempId = `temp-${Date.now()}-${participantId}`;
|
const tempId = `temp-${Date.now()}-${participantId}`;
|
||||||
setLocalQueue((prev) => [...prev, { id: tempId, participantId }]);
|
setLocalQueue((prev) => [...prev, { id: tempId, participantId }]);
|
||||||
|
|
||||||
|
|
@ -118,7 +125,15 @@ export default function PreDraftRankings() {
|
||||||
|
|
||||||
const handleRemove = useCallback(
|
const handleRemove = useCallback(
|
||||||
async (queueId: string) => {
|
async (queueId: string) => {
|
||||||
setLocalQueue((prev) => prev.filter((item) => item.id !== queueId));
|
const item = queueRef.current.find((q) => q.id === queueId);
|
||||||
|
const participantId = item?.participantId;
|
||||||
|
|
||||||
|
setLocalQueue((prev) => prev.filter((q) => q.id !== queueId));
|
||||||
|
|
||||||
|
// Temp IDs haven't been written to the DB yet — nothing to delete server-side.
|
||||||
|
if (queueId.startsWith("temp-")) return;
|
||||||
|
|
||||||
|
if (participantId) pendingRemovesRef.current.add(participantId);
|
||||||
|
|
||||||
const formData = new FormData();
|
const formData = new FormData();
|
||||||
formData.append("queueId", queueId);
|
formData.append("queueId", queueId);
|
||||||
|
|
@ -134,6 +149,8 @@ export default function PreDraftRankings() {
|
||||||
} catch {
|
} catch {
|
||||||
toast.error("Network error — failed to remove from rankings");
|
toast.error("Network error — failed to remove from rankings");
|
||||||
revalidate();
|
revalidate();
|
||||||
|
} finally {
|
||||||
|
if (participantId) pendingRemovesRef.current.delete(participantId);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[userTeam.id, revalidate]
|
[userTeam.id, revalidate]
|
||||||
|
|
@ -194,11 +211,6 @@ export default function PreDraftRankings() {
|
||||||
<p className="font-medium text-sm truncate">{p.name}</p>
|
<p className="font-medium text-sm truncate">{p.name}</p>
|
||||||
<p className="text-xs text-muted-foreground">{p.sport.name}</p>
|
<p className="text-xs text-muted-foreground">{p.sport.name}</p>
|
||||||
</div>
|
</div>
|
||||||
{p.vorpValue !== null && p.vorpValue !== undefined && (
|
|
||||||
<span className="text-xs text-muted-foreground tabular-nums shrink-0">
|
|
||||||
{Number(p.vorpValue).toFixed(1)}
|
|
||||||
</span>
|
|
||||||
)}
|
|
||||||
{inQueue ? (
|
{inQueue ? (
|
||||||
<Button
|
<Button
|
||||||
size="sm"
|
size="sm"
|
||||||
|
|
@ -269,7 +281,7 @@ export default function PreDraftRankings() {
|
||||||
</Link>
|
</Link>
|
||||||
<div className="flex items-center gap-2 mb-1">
|
<div className="flex items-center gap-2 mb-1">
|
||||||
<ListOrdered className="h-6 w-6" />
|
<ListOrdered className="h-6 w-6" />
|
||||||
<h1 className="text-2xl font-bold">Pre-Draft Rankings</h1>
|
<h1 className="text-2xl font-bold">Set Pre-Draft Queue</h1>
|
||||||
</div>
|
</div>
|
||||||
<p className="text-sm text-muted-foreground">
|
<p className="text-sm text-muted-foreground">
|
||||||
Rank the players you want. Your rankings carry into the live draft as your autopick
|
Rank the players you want. Your rankings carry into the live draft as your autopick
|
||||||
|
|
@ -296,9 +308,6 @@ export default function PreDraftRankings() {
|
||||||
</TabsTrigger>
|
</TabsTrigger>
|
||||||
</TabsList>
|
</TabsList>
|
||||||
<TabsContent value="players">
|
<TabsContent value="players">
|
||||||
<div className="flex items-center justify-between mb-3">
|
|
||||||
<span className="text-xs text-muted-foreground">Sorted by VORP</span>
|
|
||||||
</div>
|
|
||||||
{allPlayersPanel}
|
{allPlayersPanel}
|
||||||
</TabsContent>
|
</TabsContent>
|
||||||
<TabsContent value="rankings">{rankingsPanel}</TabsContent>
|
<TabsContent value="rankings">{rankingsPanel}</TabsContent>
|
||||||
|
|
@ -308,9 +317,8 @@ export default function PreDraftRankings() {
|
||||||
{/* Desktop: side-by-side */}
|
{/* Desktop: side-by-side */}
|
||||||
<div className="hidden lg:grid gap-6 lg:grid-cols-3">
|
<div className="hidden lg:grid gap-6 lg:grid-cols-3">
|
||||||
<div className="lg:col-span-2">
|
<div className="lg:col-span-2">
|
||||||
<div className="flex items-center justify-between mb-3">
|
<div className="mb-3">
|
||||||
<h2 className="font-semibold">All Players</h2>
|
<h2 className="font-semibold">All Players</h2>
|
||||||
<span className="text-xs text-muted-foreground">Sorted by VORP</span>
|
|
||||||
</div>
|
</div>
|
||||||
{allPlayersPanel}
|
{allPlayersPanel}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -57,7 +57,7 @@ function checkPreDraftRankingsAccess(opts: {
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!userId) {
|
if (!userId) {
|
||||||
return { status: 401, message: 'You must be logged in to set pre-draft rankings' };
|
return { status: 401, message: 'You must be logged in to set pre-draft queue' };
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!hasTeam) {
|
if (!hasTeam) {
|
||||||
|
|
@ -71,7 +71,7 @@ function checkPreDraftRankingsAccess(opts: {
|
||||||
// Redirect behaviour based on season status
|
// Redirect behaviour based on season status
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
describe('Pre-Draft Rankings Access - Season Status Redirects', () => {
|
describe('Set Pre-Draft Queue Access - Season Status Redirects', () => {
|
||||||
it('redirects to the draft room when status is draft', () => {
|
it('redirects to the draft room when status is draft', () => {
|
||||||
const season = createMockSeason({ status: 'draft' });
|
const season = createMockSeason({ status: 'draft' });
|
||||||
const result = checkPreDraftRankingsAccess({
|
const result = checkPreDraftRankingsAccess({
|
||||||
|
|
@ -121,7 +121,7 @@ describe('Pre-Draft Rankings Access - Season Status Redirects', () => {
|
||||||
// Authentication
|
// Authentication
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
describe('Pre-Draft Rankings Access - Authentication', () => {
|
describe('Set Pre-Draft Queue Access - Authentication', () => {
|
||||||
it('returns 401 when no user is logged in', () => {
|
it('returns 401 when no user is logged in', () => {
|
||||||
const season = createMockSeason({ status: 'pre_draft' });
|
const season = createMockSeason({ status: 'pre_draft' });
|
||||||
const result = checkPreDraftRankingsAccess({
|
const result = checkPreDraftRankingsAccess({
|
||||||
|
|
@ -141,7 +141,7 @@ describe('Pre-Draft Rankings Access - Authentication', () => {
|
||||||
userId: null,
|
userId: null,
|
||||||
hasTeam: false,
|
hasTeam: false,
|
||||||
});
|
});
|
||||||
expect(result).toMatchObject({ message: 'You must be logged in to set pre-draft rankings' });
|
expect(result).toMatchObject({ message: 'You must be logged in to set pre-draft queue' });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -149,7 +149,7 @@ describe('Pre-Draft Rankings Access - Authentication', () => {
|
||||||
// Team membership
|
// Team membership
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
describe('Pre-Draft Rankings Access - Team Membership', () => {
|
describe('Set Pre-Draft Queue Access - Team Membership', () => {
|
||||||
it('returns 403 for a logged-in user with no team in the season', () => {
|
it('returns 403 for a logged-in user with no team in the season', () => {
|
||||||
const season = createMockSeason({ status: 'pre_draft' });
|
const season = createMockSeason({ status: 'pre_draft' });
|
||||||
const result = checkPreDraftRankingsAccess({
|
const result = checkPreDraftRankingsAccess({
|
||||||
|
|
@ -188,7 +188,7 @@ describe('Pre-Draft Rankings Access - Team Membership', () => {
|
||||||
// Redirect targets use the correct leagueId and seasonId
|
// Redirect targets use the correct leagueId and seasonId
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
describe('Pre-Draft Rankings Access - Redirect URL Correctness', () => {
|
describe('Set Pre-Draft Queue Access - Redirect URL Correctness', () => {
|
||||||
it('uses the leagueId from params in the redirect URL', () => {
|
it('uses the leagueId from params in the redirect URL', () => {
|
||||||
const season = createMockSeason({ id: 'season-99', status: 'draft' });
|
const season = createMockSeason({ id: 'season-99', status: 'draft' });
|
||||||
const result = checkPreDraftRankingsAccess({
|
const result = checkPreDraftRankingsAccess({
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue