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
This commit is contained in:
parent
14b9a15a24
commit
303c049b68
2 changed files with 16 additions and 7 deletions
|
|
@ -64,7 +64,7 @@ export async function loader(args: Route.LoaderArgs) {
|
|||
|
||||
type QueueItem = { id: string; participantId: string };
|
||||
|
||||
export default function PreDraftRankings() {
|
||||
export default function SetPreDraftQueue() {
|
||||
const { season, userTeam, availableParticipants, userQueue } =
|
||||
useLoaderData<typeof loader>();
|
||||
const { leagueId } = useParams<{ leagueId: string }>();
|
||||
|
|
@ -81,6 +81,7 @@ export default function PreDraftRankings() {
|
|||
const queueRef = useRef(localQueue);
|
||||
// Track in-flight removes to prevent add racing ahead of a pending remove
|
||||
const pendingRemovesRef = useRef(new Set<string>());
|
||||
|
||||
useEffect(() => {
|
||||
queueRef.current = localQueue;
|
||||
}, [localQueue]);
|
||||
|
|
@ -92,7 +93,11 @@ export default function PreDraftRankings() {
|
|||
|
||||
const handleAdd = useCallback(
|
||||
async (participantId: string) => {
|
||||
if (queuedParticipantIds.has(participantId) || pendingRemovesRef.current.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}`;
|
||||
setLocalQueue((prev) => [...prev, { id: tempId, participantId }]);
|
||||
|
||||
|
|
@ -124,6 +129,10 @@ export default function PreDraftRankings() {
|
|||
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();
|
||||
|
|
@ -308,7 +317,7 @@ export default function PreDraftRankings() {
|
|||
{/* Desktop: side-by-side */}
|
||||
<div className="hidden lg:grid gap-6 lg:grid-cols-3">
|
||||
<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>
|
||||
</div>
|
||||
{allPlayersPanel}
|
||||
|
|
|
|||
|
|
@ -71,7 +71,7 @@ function checkPreDraftRankingsAccess(opts: {
|
|||
// 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', () => {
|
||||
const season = createMockSeason({ status: 'draft' });
|
||||
const result = checkPreDraftRankingsAccess({
|
||||
|
|
@ -121,7 +121,7 @@ describe('Pre-Draft Rankings Access - Season Status Redirects', () => {
|
|||
// Authentication
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Pre-Draft Rankings Access - Authentication', () => {
|
||||
describe('Set Pre-Draft Queue Access - Authentication', () => {
|
||||
it('returns 401 when no user is logged in', () => {
|
||||
const season = createMockSeason({ status: 'pre_draft' });
|
||||
const result = checkPreDraftRankingsAccess({
|
||||
|
|
@ -149,7 +149,7 @@ describe('Pre-Draft Rankings Access - Authentication', () => {
|
|||
// 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', () => {
|
||||
const season = createMockSeason({ status: 'pre_draft' });
|
||||
const result = checkPreDraftRankingsAccess({
|
||||
|
|
@ -188,7 +188,7 @@ describe('Pre-Draft Rankings Access - Team Membership', () => {
|
|||
// 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', () => {
|
||||
const season = createMockSeason({ id: 'season-99', status: 'draft' });
|
||||
const result = checkPreDraftRankingsAccess({
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue