Ensure organisation games are loaded where needed

[tudddorrr]

No description provided.

[claude]

Code Review - PR #607: Ensure organisation games are loaded where needed

Overview

This PR addresses a data loading issue by ensuring organisation.games are populated where needed across the authentication and user registration flows. The changes are focused and well-targeted.

✅ Positives

Code Quality & Best Practices:

• Clean, focused changes that follow existing patterns
• Good use of MikroORM's isInitialized() check to avoid redundant queries
• Consistent population strategy across related endpoints
• Proper separation of concerns - caching strategy preserved while fixing data loading

Performance Considerations:

• Smart approach in getUserFromToken.ts:21-23 - populating after cache lookup prevents circular references in cached data
• Cache expiration increased to 600 seconds (10 minutes) which is reasonable for user/game data
• Conditional population using isInitialized() prevents unnecessary database calls

Test Coverage:

• Comprehensive test updates across all affected endpoints
• Tests now properly verify organisation.games presence
• Good use of createOrganisationAndGame() utility in register test for more realistic scenarios
• Fixed inconsistent assertion methods (toBe → toEqual)

🔍 Areas of Concern

Potential Issues:

1. Cache Invalidation: The longer cache duration (600s) in getUserFromToken.ts:17 and policy.ts:44 means stale data could persist longer if games are added/removed from organizations
2. N+1 Query Risk: While the current approach is safe, consider if multiple user tokens are processed simultaneously - each might trigger separate populate calls

Security:

• No security concerns identified - changes don't expose sensitive data or create vulnerabilities

🛠 Minor Improvements

Code Style:

// src/lib/auth/getUserFromToken.ts:21-23
// Consider extracting the magic number
const GAMES_POPULATION_CHECK = !user.organisation.games.isInitialized()
if (GAMES_POPULATION_CHECK) {
  await em.populate(user, ['organisation.games'])
}

Performance Optimization:
Consider batching population if this function is called frequently:

// Alternative approach for high-frequency scenarios
const usersToPopulate = users.filter(user => !user.organisation.games.isInitialized())
if (usersToPopulate.length > 0) {
  await em.populate(usersToPopulate, ['organisation.games'])
}

📋 Recommendations

1. Monitor Performance: Keep an eye on database query patterns after deployment to ensure the populate calls don't impact performance
2. Cache Strategy: Consider implementing cache invalidation when games are added/removed from organizations
3. Documentation: Consider adding a comment explaining the post-cache population strategy in getUserFromToken.ts

🎯 Verdict

✅ Approved - This is a solid fix that addresses the data loading issue without introducing significant risks. The approach is clean, well-tested, and follows established patterns in the codebase. The performance considerations are well-handled, and the test coverage is thorough.

The changes correctly ensure that organisation.games are available where needed while maintaining the existing caching strategy and performance characteristics.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.22%. Comparing base (03d70f8) to head (890a80c).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #607   +/-   ##
========================================
  Coverage    99.22%   99.22%           
========================================
  Files          208      208           
  Lines        14466    14473    +7     
  Branches      1298     1302    +4     
========================================
+ Hits         14354    14361    +7     
  Misses         109      109           
  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.