Don't allow empty game names

[tudddorrr]

No description provided.

[claude]

Code Review for PR #605: Don't allow empty game names

I've conducted a comprehensive review of this pull request. Here's my feedback:

✅ Positive Aspects

Code Quality & Best Practices:

• The validation pattern follows the established codebase conventions using the @Validate decorator
• Consistent with other validation implementations found in the codebase (e.g., player-api.service.ts:78, data-export.service.ts)
• Proper use of val.trim().length > 0 to handle whitespace-only strings
• The conditional logic typeof val === 'string' ? val.trim().length > 0 : true correctly allows undefined/null values while validating strings

Security Concerns:

• ✅ No security issues identified
• Input validation prevents empty/whitespace-only names which could cause UI issues
• No risk of injection attacks as this is simple string length validation

Performance Considerations:

• ✅ Minimal performance impact
• Simple string operations (typeof, trim(), length) are very fast
• Validation runs only on PATCH requests, not on every request

🧪 Test Coverage

Strengths:

• Good test case covering the empty string scenario
• Proper error message validation
• Uses the established testing pattern with createOrganisationAndGame() and createUserAndToken()

Suggestions for Improvement:
Consider adding these additional test cases to ensure comprehensive coverage:

1. Whitespace-only strings:
   Test with spaces, tabs, newlines to ensure they're properly rejected

2. Valid name updates:
   Test that valid names still work after adding validation

3. Undefined/null handling:
   Ensure other fields can still be updated when name is not provided

📋 Minor Observations

Consistency with POST endpoint:

• The POST endpoint at game.service.ts:62-64 only validates that name is required but doesn't validate empty strings
• Consider whether the POST endpoint should have the same empty string validation for consistency

Code Style:

• ✅ Follows repository ESLint rules (single quotes, no semicolons, proper indentation)
• ✅ Matches existing validation patterns in the codebase

🎯 Overall Assessment

This is a well-implemented fix that addresses the issue of empty game names effectively. The implementation follows established patterns, has no security concerns, and includes appropriate testing. The validation logic is sound and handles edge cases correctly.

Recommendation: ✅ APPROVE with suggestion to consider the additional test cases mentioned above for even better coverage.

Great work on maintaining consistency with the existing codebase patterns!

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.23%. Comparing base (2cb62c6) to head (34cb00a).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/services/game.service.ts	91.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #605      +/-   ##
===========================================
- Coverage    99.24%   99.23%   -0.01%     
===========================================
  Files          208      208              
  Lines        14527    14539      +12     
  Branches      1299     1300       +1     
===========================================
+ Hits         14417    14428      +11     
- Misses         107      108       +1     
  Partials         3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.