[feat] Create game based validation for Admin form #50
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update modifies the form schema validation in an administrative helper module. The changes enforce stricter formatting rules across various fields, requiring trimmed, non-empty strings with specific length constraints. A URL validation is added for the session URL field, and nested validations ensure that arrays (such as players, sets, and matches) contain properly structured objects with required properties. Additionally, new type exports enhance type safety. The Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/app/(routes)/admin/_utils/form-helpers.tsOops! Something went wrong! :( ESLint: 9.17.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tydolla00
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/(routes)/admin/_utils/form-helpers.ts (1)
138-148: Verify player lookup function with new validation.The
findPlayerByGamerTagfunction returns a partial Player object that may have undefined values when no player is found. Consider updating this function to either throw a PlayerNotFoundError (already defined) or return null to better integrate with your enhanced validation.export const findPlayerByGamerTag = (gamerTag: string) => { const player = Object.values(PLAYER_MAPPINGS).find((player) => player.gamerTags.some( (tag) => tag.toLowerCase() === gamerTag.toLowerCase(), ), ); - return { - playerId: player?.playerId, - playerName: player?.playerName, - } as Player; + if (!player) { + throw new PlayerNotFoundError(gamerTag); + } + + return { + playerId: player.playerId, + playerName: player.playerName, + } as Player; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(routes)/admin/_utils/form-helpers.ts(2 hunks)
🔇 Additional comments (6)
src/app/(routes)/admin/_utils/form-helpers.ts (6)
5-6: Good input validation improvements.Adding
.trim()and.min(1)ensures that fields cannot contain only whitespace, improving data quality by requiring meaningful content.
9-12: Appropriate URL validation enhancement.Adding
.url()validation before checking for YouTube-specific requirements is a good practice. This ensures the string is a valid URL format before applying more specific checks.
18-20: Consistent validation approach for readonly fields.Good job applying consistent validation with
.trim().min(1)to readonly fields as well. This ensures backend validation remains robust even though these fields cannot be directly modified by users in the UI.
21-28: Well-structured array validation.The
.nonempty()validation with a clear error message is excellent. This prevents form submission with empty arrays and provides a user-friendly error message explaining what's required.
37-71: Comprehensive nested validation structure.The validation structure for nested objects (setWinners, matchWinners, playerSessions, playerStats) follows a consistent pattern, ensuring that all string fields are trimmed and have minimum length requirements. The error messages for non-empty arrays are clear and descriptive.
77-77: Consider addressing the TODO comment.There's a TODO comment about conditionally applying input validations based on stat name. Since you're enhancing form validation, this might be a good opportunity to implement this feature.
Would you like me to suggest an approach for implementing conditional validation based on stat names?
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/(routes)/admin/_components/form/SetManager.tsx (1)
44-48: Enhanced type safety for set data.Casting arrays to their specific types ensures proper type checking throughout the application.
However, the use of
unknown as Typecasting pattern is generally discouraged. Consider refactoring to initialize with proper types:- matches: [] as unknown as Matches, - setWinners: [] as unknown as SetWinners, + matches: [] as Matches, + setWinners: [] as SetWinners,src/app/(routes)/admin/_components/form/MatchManager.tsx (2)
48-50: Enhanced type safety for match data.Casting arrays to their specific types ensures proper type checking throughout the application.
However, the use of
unknown as Typecasting pattern is generally discouraged. Consider refactoring to initialize with proper types:- matchWinners: [] as unknown as MatchWinners, - playerSessions: playerSessions as unknown as PlayerSessions, + matchWinners: [] as MatchWinners, + playerSessions: playerSessions as PlayerSessions,
104-106: Enhanced type safety for fallback match data.Consistent type casting when no winners are found ensures proper type handling throughout the application.
Similar to above, consider simplifying the type casting:
- matchWinners: [] as unknown as MatchWinners, // Enforce non empty array, + matchWinners: [] as MatchWinners, // Enforce non empty array,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/(routes)/admin/_components/form/EntryCreatorForm.tsx(2 hunks)src/app/(routes)/admin/_components/form/MatchManager.tsx(4 hunks)src/app/(routes)/admin/_components/form/SetManager.tsx(2 hunks)src/app/(routes)/admin/_utils/form-helpers.ts(2 hunks)
🔇 Additional comments (9)
src/app/(routes)/admin/_components/form/EntryCreatorForm.tsx (1)
30-38: Good addition of validation debugging.Adding validation debugging output helps with troubleshooting form issues during development. This ensures that validation errors can be more easily identified and addressed.
src/app/(routes)/admin/_components/form/SetManager.tsx (1)
20-20: Improved type imports for enhanced type safety.Adding specific type imports from form-helpers improves type checking and code clarity.
src/app/(routes)/admin/_components/form/MatchManager.tsx (2)
13-16: Improved type imports for enhanced type safety.Adding specific type imports from form-helpers improves type checking and code clarity.
99-101: Improved type handling for vision-generated match data.Proper type casting for data from external sources enhances type safety and ensures consistency with the rest of the application.
src/app/(routes)/admin/_utils/form-helpers.ts (5)
5-7: Improved validation for basic form fields.Adding string trimming and minimum length requirements ensures that empty or whitespace-only inputs are properly rejected, enhancing data quality.
8-16: Enhanced URL validation for session links.Adding URL validation and trimming ensures that only properly formatted YouTube URLs are accepted.
18-28: Strengthened validation for core form fields.Enforcing trimmed strings with minimum length requirements for required fields and ensuring the players array is non-empty improves data integrity.
33-72: Comprehensive validation for nested arrays and objects.The enhanced validation for setWinners, matches, and playerSessions ensures that all required nested data is properly structured and validated. This prevents incomplete or malformed data from being submitted.
This validation approach aligns perfectly with the PR objective of allowing the form to drive validation and UI.
80-85: Valuable type exports for cross-component consistency.Adding explicit type exports for nested data structures improves code reusability and ensures consistent typing across components.
These types help enforce the validation rules defined in the schema and make it easier to maintain type consistency when working with complex nested data.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/(routes)/admin/_components/form/PlayerAvatar.tsx (1)
58-66: Consider implementing the image loading callback functionality.You've added an
onLoadingStatusChangehandler with a placeholder comment, but there's no actual implementation. Consider either implementing the functionality or removing the comment to avoid confusion.<AvatarImage src={avatarSrc} alt={player.playerName} onLoadingStatusChange={(status) => { if (status === "loaded") { - // Image is ready + // Implement loading completion logic here if needed } }} />src/app/(routes)/admin/_utils/form-helpers.ts (1)
29-104: Comprehensive validation for nested form structures.The validation schema now properly enforces data integrity for complex nested structures like sets, matches, and player sessions. The refine method ensures business logic validation for match scoring.
Consider extracting the complex match validation logic into a separate function for better readability and maintainability.
+ // Validate that winning team has more goals than losing team + const validateMatchScores = (matches: any[]) => { + return matches.every((match, i) => { + let winningTeam = 0; + let losingTeam = 0; + + match.playerSessions.forEach((ps: any) => { + const onWinningTeam = match.matchWinners.some( + (mw: any) => mw.playerId === ps.playerId, + ); + ps.playerStats.forEach((stat: any) => { + if (stat.stat === $Enums.StatName.RL_GOALS) { + if (onWinningTeam) + winningTeam += Number(stat.statValue) || 0; + else losingTeam += Number(stat.statValue) || 0; + } + }); + }); + + if (winningTeam <= losingTeam) + console.warn( + `Error in match ${i + 1}. Winning team should have more goals. Winners: ${winningTeam} Losers: ${losingTeam}`, + ); + return winningTeam > losingTeam; + }); + }; matches: z .array( z.object({ // ... existing validation }), ) .nonempty("At least one match is required") .refine( - (data) => { - // TODO figure out how to pass this as a param. Maybe explicitly define type. - return data.every((match, i) => { - let winningTeam = 0; - let losingTeam = 0; - - match.playerSessions.forEach((ps) => { - const onWinningTeam = match.matchWinners.some( - (mw) => mw.playerId === ps.playerId, - ); - ps.playerStats.forEach((stat) => { - if (stat.stat === $Enums.StatName.RL_GOALS) { - if (onWinningTeam) - winningTeam += Number(stat.statValue) || 0; - else losingTeam += Number(stat.statValue) || 0; - } - }); - }); - if (winningTeam <= losingTeam) - console.warn( - `Error in match ${i + 1}. Winning team should have more goals. Winners: ${winningTeam} Losers: ${winningTeam}`, - ); - return winningTeam > losingTeam; - }); - }, + validateMatchScores, { message: "Winning team should have more goals in each match", }, ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/(routes)/admin/_components/form/EntryCreatorForm.tsx(1 hunks)src/app/(routes)/admin/_components/form/MatchManager.tsx(4 hunks)src/app/(routes)/admin/_components/form/PlayerAvatar.tsx(1 hunks)src/app/(routes)/admin/_components/form/PlayerSessionManager.tsx(1 hunks)src/app/(routes)/admin/_utils/form-helpers.ts(2 hunks)src/app/(routes)/admin/_utils/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/(routes)/admin/_components/form/PlayerSessionManager.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/(routes)/admin/_components/form/EntryCreatorForm.tsx
- src/app/(routes)/admin/_components/form/MatchManager.tsx
🔇 Additional comments (4)
src/app/(routes)/admin/_utils/types.ts (1)
1-1: Good addition of a strongly-typed non-empty array type.This utility type effectively enforces that arrays must contain at least one element, which is useful for form validation and type safety across the application.
src/app/(routes)/admin/_components/form/PlayerAvatar.tsx (1)
55-57: LGTM on the AvatarFallback simplification.Removing the
delayMsprop simplifies the component and follows the principle of keeping components focused on their core responsibilities.src/app/(routes)/admin/_utils/form-helpers.ts (2)
5-28: Great improvement on form field validations.Adding trimming and minimum length requirements ensures data quality and prevents empty submissions. The
.nonempty()validation with a custom error message provides clear feedback to users.
110-115: Good addition of type exports for better type safety.These type exports improve type safety and make it easier to work with complex nested form data structures across components.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/(routes)/admin/_utils/form-helpers.ts (1)
72-101: Consider extracting the goal validation logic into a separate function.The custom refine validation is complex and could be more maintainable if extracted to a named function.
export const formSchema = z.object({ // ... other fields sets: z .array( z.object({ // ... other fields matches: z .array( // ... match object definition ) .nonempty("At least one match is required") - .refine( - (data) => { - // TODO figure out how to pass this as a param. Maybe explicitly define type. - return data.every((match, i) => { - let winningTeam = 0; - let losingTeam = 0; - - match.playerSessions.forEach((ps) => { - const onWinningTeam = match.matchWinners.some( - (mw) => mw.playerId === ps.playerId, - ); - ps.playerStats.forEach((stat) => { - if (stat.stat === $Enums.StatName.RL_GOALS) { - if (onWinningTeam) - winningTeam += Number(stat.statValue) || 0; - else losingTeam += Number(stat.statValue) || 0; - } - }); - }); - if (winningTeam <= losingTeam) - console.warn( - `Error in match ${i + 1}. Winning team should have more goals. Winners: ${winningTeam} Losers: ${losingTeam}`, - ); - return winningTeam > losingTeam; - }); - }, - { - message: "Winning team should have more goals in each match", - }, - ), + .refine(validateWinningTeamGoals, { + message: "Winning team should have more goals in each match", + }), }), ) .nonempty("At least one set is required"), }); +// Helper function to validate that winning team has more goals +const validateWinningTeamGoals = (matches: z.infer<typeof matchesSchema>) => { + return matches.every((match, i) => { + let winningTeam = 0; + let losingTeam = 0; + + match.playerSessions.forEach((ps) => { + const onWinningTeam = match.matchWinners.some( + (mw) => mw.playerId === ps.playerId, + ); + ps.playerStats.forEach((stat) => { + if (stat.stat === $Enums.StatName.RL_GOALS) { + if (onWinningTeam) + winningTeam += Number(stat.statValue) || 0; + else losingTeam += Number(stat.statValue) || 0; + } + }); + }); + if (winningTeam <= losingTeam) + console.warn( + `Error in match ${i + 1}. Winning team should have more goals. Winners: ${winningTeam} Losers: ${losingTeam}`, + ); + return winningTeam > losingTeam; + }); +}; + +// Define match schema separately to use in the validation function +const matchesSchema = z.array( + z.object({ + matchWinners: z.array(z.object({ playerId: z.number(), playerName: z.string() })), + playerSessions: z.array( + z.object({ + playerId: z.number(), + playerSessionName: z.string(), + playerStats: z.array( + z.object({ + statId: z.string(), + stat: z.string(), + statValue: z.string() + }) + ) + }) + ) + }) +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(routes)/admin/_utils/form-helpers.ts(2 hunks)
🔇 Additional comments (7)
src/app/(routes)/admin/_utils/form-helpers.ts (7)
5-20: Improved field validations enhance data integrity.The additions of
.trim()and length constraints across all form fields provide consistent validation and prevent empty or whitespace-only values, which is a good improvement for data quality.
21-28: Well-structured player validation ensures required data.The player array validation with
.nonempty()and the object schema ensures all players have both an ID and a properly formatted name.
37-40: Enhanced set winner validation.Adding trim and min length validation for player names in set winners ensures consistency with the rest of the form.
41-71: Well-structured nested validations for matches.The comprehensive validation structure for matches, match winners, player sessions, and player stats ensures data completeness throughout the form hierarchy.
91-94: Fixed console warning message.The console warning now correctly compares the winning team's score with the losing team's score.
110-115: Excellent type exports improve type safety.These exported types derived from the form schema will enhance type safety when these structures are used in other components.
74-74:❓ Verification inconclusive
Address TODO by introducing typed parameters.
The TODO comment indicates a need to pass parameters more explicitly. Consider implementing this to improve code maintainability.
🏁 Script executed:
#!/bin/bash # Search for pattern to see how similar validations are handled elsewhere in the codebase rg "refine.*\(.*param" --type tsLength of output: 34
Address the TODO by explicitly typing the parameter and ensure consistency with existing patterns.
The search for similar patterns using the
rg "refine.*\(.*param"command returned no output, so we couldn’t automatically infer a standard implementation from the codebase. Please verify manually if there’s a preferred approach elsewhere for handling typed parameters, or update this instance in line 74 ofsrc/app/(routes)/admin/_utils/form-helpers.tsto use explicit type definitions. This change should improve maintainability without deviating from the overall coding practices.
This branch is designed to allow the form to drive the validation and ui for our submission process
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
PlayerSessionManagercomponent.