Skip to content

Commit

Permalink
Overhaul validation (#109)
Browse files Browse the repository at this point in the history 
* introduce better validation structure

* improve validation with error handling
  • Loading branch information
@mwickett
mwickett authored Jan 20, 2025
1 parent 162479e commit 87bd91b
Show file tree
Hide file tree
Showing 10 changed files with 572 additions and 533 deletions.
49 changes: 36 additions & 13 deletions src/app/games/[id]/scoreDisplay.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { DisplayScores } from "@/lib/gameLogic";
import { useState } from "react";
import { Alert, AlertDescription } from "@/components/ui/alert";
import { AlertCircle } from "lucide-react";
import { validateGameRules, ValidationError } from "@/lib/validation/gameRules";
import { updateRoundScores } from "@/server/mutations";
import { useRouter } from "next/navigation";
import { z } from "zod";
Expand Down Expand Up @@ -46,24 +49,29 @@ export default function ScoreDisplay({
const [editingScores, setEditingScores] = useState<any[]>([]);
const [editingRoundId, setEditingRoundId] = useState<string | null>(null);
const [scoresValid, setScoresValid] = useState(false);
const [error, setError] = useState<string | null>(null);
const [isSaving, setIsSaving] = useState(false);

const validateScores = (scores: any[]) => {
try {
// Basic structure validation
scoresSchema.parse(scores);
const allFieldsTouched = scores.every(
(player) => player.touched.totalCardsPlayed
);
const atLeastOneBlitzed = scores.some(
(player) => player.blitzPileRemaining === 0
);

setScoresValid(allFieldsTouched && atLeastOneBlitzed);
// Game rules validation
validateGameRules(scores);
setScoresValid(true);
setError(null);
} catch (e) {
setScoresValid(false);
if (e instanceof ValidationError) {
setError(e.message);
} else {
setError("Please fill in all fields correctly");
}
}
};

const handleEdit = async (roundIndex: number) => {
setError(null);
try {
// Fetch the actual round scores from the server
const response = await fetch(`/api/rounds/${gameId}/${roundIndex + 1}`);
Expand All @@ -87,7 +95,7 @@ export default function ScoreDisplay({
setEditingRound(roundIndex);
validateScores(roundScores);
} catch (error) {
console.error("Failed to fetch round scores:", error);
setError("Failed to load round scores. Please try again.");
}
};

Expand All @@ -101,6 +109,8 @@ export default function ScoreDisplay({
const handleSave = async () => {
if (!scoresValid || !editingRoundId) return;

setError(null);
setIsSaving(true);
try {
await updateRoundScores(gameId, editingRoundId, editingScores);
setEditingRound(null);
Expand All @@ -109,7 +119,13 @@ export default function ScoreDisplay({
setScoresValid(false);
router.refresh();
} catch (error) {
console.error("Failed to update scores:", error);
if (error instanceof ValidationError) {
setError(error.message);
} else {
setError("Failed to save scores. Please try again.");
}
} finally {
setIsSaving(false);
}
};

Expand All @@ -118,6 +134,7 @@ export default function ScoreDisplay({
field: "blitzPileRemaining" | "totalCardsPlayed",
value: string
) => {
setError(null);
// Allow empty string for better typing experience
if (value === "") {
setEditingScores((prev) =>
Expand Down Expand Up @@ -167,7 +184,13 @@ export default function ScoreDisplay({
};

return (
<div>
<div className="space-y-4">
{error && (
<Alert variant="destructive" className="max-w-md mx-auto">
<AlertCircle className="h-4 w-4" />
<AlertDescription>{error}</AlertDescription>
</Alert>
)}
<Table className="bg-white dark:bg-gray-950 rounded-lg shadow-lg p-6 max-w-md mx-auto my-10">
<TableHeader>
<TableRow>
Expand Down Expand Up @@ -247,11 +270,11 @@ export default function ScoreDisplay({
<div className="space-x-2">
<Button
onClick={handleSave}
disabled={!scoresValid}
disabled={!scoresValid || isSaving}
size="sm"
variant="outline"
>
Save
{isSaving ? "Saving..." : "Save"}
</Button>
<Button
onClick={handleCancel}
Expand Down
46 changes: 25 additions & 21 deletions src/app/games/[id]/scoreEntry.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,26 @@ import React, { useState, useEffect, useCallback } from "react";
import { Label } from "@/components/ui/label";
import { Input } from "@/components/ui/input";
import { Button } from "@/components/ui/button";
import { Alert, AlertDescription } from "@/components/ui/alert";
import { AlertCircle } from "lucide-react";
import { createRoundForGame } from "@/server/mutations";
import { GameWithPlayersAndScores, DisplayScores } from "@/lib/gameLogic";
import { z } from "zod";
import { useRouter } from "next/navigation";
import GameOver from "./GameOver";

// Basic input validation schema for UX
const playerScoreSchema = z.object({
userId: z.string(),
username: z.string(),
roundNumber: z.number().min(1),
blitzPileRemaining: z.number().min(0).max(10),
totalCardsPlayed: z.number().min(0).max(40),
roundNumber: z.number(),
blitzPileRemaining: z.number(),
totalCardsPlayed: z.number(),
touched: z.object({
totalCardsPlayed: z.boolean(),
}),
});

const scoresSchema = z.array(playerScoreSchema);

export default function ScoreEntry({
game,
currentRoundNumber,
Expand All @@ -46,29 +47,18 @@ export default function ScoreEntry({
}))
);
const [scoresValid, setScoresValid] = useState(false);
const [errors, setErrors] = useState<{ [key: string]: string }>({});

const [error, setError] = useState<string | null>(null);
const winner = displayScores.find((score) => score.isWinner);

// Only validate that all fields are filled out
const validateScores = useCallback(() => {
try {
scoresSchema.parse(playerScores);
playerScoreSchema.parse(playerScores[0]); // Validate structure
const allFieldsTouched = playerScores.every(
(player) => player.touched.totalCardsPlayed
);
const atLeastOneBlitzed = playerScores.some(
(player) => player.blitzPileRemaining === 0
);

setScoresValid(allFieldsTouched && atLeastOneBlitzed);
setScoresValid(allFieldsTouched);
} catch (e) {
const validationErrors: { [key: string]: string } = {};
if (e instanceof z.ZodError) {
e.errors.forEach((error) => {
validationErrors[error.path.join(".")] = error.message;
});
}
setErrors(validationErrors);
setScoresValid(false);
}
}, [playerScores]);
Expand All @@ -86,6 +76,7 @@ export default function ScoreEntry({
};

const handleBlitzPileChange = (userId: string, value: string) => {
setError(null); // Clear error on input change
const strippedValue = stripLeadingZeros(value);
const intValue = parseInt(strippedValue, 10);

Expand All @@ -106,6 +97,7 @@ export default function ScoreEntry({
};

const handleTotalCardsChange = (userId: string, value: string) => {
setError(null); // Clear error on input change
const strippedValue = stripLeadingZeros(value);
const intValue = parseInt(strippedValue, 10);

Expand Down Expand Up @@ -163,6 +155,7 @@ export default function ScoreEntry({

const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
setError(null); // Clear any previous errors
try {
await createRoundForGame(game.id, currentRoundNumber, playerScores);
setPlayerScores((prevScores) =>
Expand All @@ -178,7 +171,12 @@ export default function ScoreEntry({
setScoresValid(false);
router.refresh();
} catch (e) {
console.error(e);
if (e instanceof Error) {
setError(e.message);
} else {
console.error(e);
setError("An error occurred while saving scores");
}
}
};

Expand All @@ -187,6 +185,12 @@ export default function ScoreEntry({
className="bg-white dark:bg-gray-950 rounded-lg shadow-lg p-6 max-w-md mx-auto"
onSubmit={handleSubmit}
>
{error && (
<Alert variant="destructive" className="mb-6">
<AlertCircle className="h-4 w-4" />
<AlertDescription>{error}</AlertDescription>
</Alert>
)}
<h1 className="text-2xl font-bold mb-6">
Round {currentRoundNumber} Scores
</h1>
Expand Down
58 changes: 58 additions & 0 deletions src/components/ui/alert.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import * as React from "react";
import { cva, type VariantProps } from "class-variance-authority";
import { cn } from "@/lib/utils";

const alertVariants = cva(
"relative w-full rounded-lg border p-4 [&>svg~*]:pl-7 [&>svg+div]:translate-y-[-3px] [&>svg]:absolute [&>svg]:left-4 [&>svg]:top-4 [&>svg]:text-foreground",
{
variants: {
variant: {
default: "bg-background text-foreground",
destructive:
"border-destructive/50 text-destructive dark:border-destructive [&>svg]:text-destructive",
},
},
defaultVariants: {
variant: "default",
},
}
);

const Alert = React.forwardRef<
HTMLDivElement,
React.HTMLAttributes<HTMLDivElement> & VariantProps<typeof alertVariants>
>(({ className, variant, ...props }, ref) => (
<div
ref={ref}
role="alert"
className={cn(alertVariants({ variant }), className)}
{...props}
/>
));
Alert.displayName = "Alert";

const AlertTitle = React.forwardRef<
HTMLParagraphElement,
React.HTMLAttributes<HTMLHeadingElement>
>(({ className, ...props }, ref) => (
<h5
ref={ref}
className={cn("mb-1 font-medium leading-none tracking-tight", className)}
{...props}
/>
));
AlertTitle.displayName = "AlertTitle";

const AlertDescription = React.forwardRef<
HTMLParagraphElement,
React.HTMLAttributes<HTMLParagraphElement>
>(({ className, ...props }, ref) => (
<div
ref={ref}
className={cn("text-sm [&_p]:leading-relaxed", className)}
{...props}
/>
));
AlertDescription.displayName = "AlertDescription";

export { Alert, AlertTitle, AlertDescription };
Loading

0 comments on commit 87bd91b

Please sign in to comment.