docs
This commit is contained in:
1063
docs/ALPHA_PLAN.md
1063
docs/ALPHA_PLAN.md
File diff suppressed because it is too large
Load Diff
82
docs/TESTING_LAYERS.md
Normal file
82
docs/TESTING_LAYERS.md
Normal file
@@ -0,0 +1,82 @@
|
|||||||
|
# Testing layers between unit and integration
|
||||||
|
|
||||||
|
## Unit tests
|
||||||
|
Test a single function or class in isolation.
|
||||||
|
|
||||||
|
- No real dependencies
|
||||||
|
- Everything external is mocked or stubbed
|
||||||
|
- Very fast, very granular
|
||||||
|
- Good for pure logic
|
||||||
|
|
||||||
|
**Goal:** Verify correctness of small, isolated units
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sociable unit tests
|
||||||
|
Units are tested together with their close collaborators.
|
||||||
|
|
||||||
|
- Multiple classes/functions work together
|
||||||
|
- Only external side effects are mocked (DB, network, filesystem)
|
||||||
|
- Internal collaborators are real
|
||||||
|
- Still fast, but more meaningful than isolated units
|
||||||
|
|
||||||
|
**Goal:** Verify local collaboration without infrastructure
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Component / Module tests
|
||||||
|
Test a coherent group of units as one component.
|
||||||
|
|
||||||
|
- Entire module is tested as a black box
|
||||||
|
- Internal structure is irrelevant
|
||||||
|
- Boundaries are mocked, internals are real
|
||||||
|
- No real infrastructure
|
||||||
|
|
||||||
|
**Goal:** Verify that a module behaves correctly as a whole
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Service / Slice tests
|
||||||
|
Test a full use case or service flow.
|
||||||
|
|
||||||
|
- Domain + application logic is real
|
||||||
|
- Adapters are mocked or faked
|
||||||
|
- Represents a real business action
|
||||||
|
- Often maps to one command/query
|
||||||
|
|
||||||
|
**Goal:** Verify business behavior without infrastructure cost
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Contract tests
|
||||||
|
Test the contract between two components or services.
|
||||||
|
|
||||||
|
- Focus on inputs/outputs and schemas
|
||||||
|
- No real integration required
|
||||||
|
- One side is mocked, contract is enforced
|
||||||
|
- Prevents breaking changes
|
||||||
|
|
||||||
|
**Goal:** Verify that boundaries remain compatible
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Integration tests
|
||||||
|
Test real integration with infrastructure.
|
||||||
|
|
||||||
|
- Real database, filesystem, network, etc.
|
||||||
|
- Slower and more expensive
|
||||||
|
- Fewer tests, higher confidence
|
||||||
|
|
||||||
|
**Goal:** Verify that the system works with real dependencies
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## End-to-end (E2E) tests
|
||||||
|
Test the system exactly like a user would.
|
||||||
|
|
||||||
|
- Everything is real
|
||||||
|
- UI → backend → infrastructure
|
||||||
|
- Slow and fragile
|
||||||
|
- Very few tests should exist
|
||||||
|
|
||||||
|
**Goal:** Verify critical user flows
|
||||||
@@ -1,78 +0,0 @@
|
|||||||
# League Actor Model & Permissions (Canonical)
|
|
||||||
|
|
||||||
This document defines the canonical backend actor model and the permission rules for **league admin/owner** operations.
|
|
||||||
|
|
||||||
It is the source of truth for Subtask 0A in [`plans/league-admin-mvp-plan.md`](plans/league-admin-mvp-plan.md:1).
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Session identity (source of truth)
|
|
||||||
|
|
||||||
### What the authenticated session contains
|
|
||||||
- The API authentication layer attaches `request.user.userId` based on the session cookie (`gp_session`).
|
|
||||||
- See [`AuthenticationGuard.canActivate()`](apps/api/src/domain/auth/AuthenticationGuard.ts:16).
|
|
||||||
- The backend uses an async request context (`AsyncLocalStorage`) to make the current request available to services.
|
|
||||||
- See [`requestContextMiddleware()`](adapters/http/RequestContext.ts:24).
|
|
||||||
- Wired globally for the API via [`AppModule.configure()`](apps/api/src/app.module.ts:49).
|
|
||||||
|
|
||||||
### Mapping: `userId` → `driverId`
|
|
||||||
Current canonical mapping (for MVP):
|
|
||||||
- The “actor” is derived from session, and `driverId === userId`.
|
|
||||||
- This is implemented by [`getActorFromRequestContext()`](apps/api/src/domain/auth/getActorFromRequestContext.ts:12).
|
|
||||||
|
|
||||||
Rationale:
|
|
||||||
- The current system uses the session user identity as the same identifier used by racing/league membership repositories (e.g. seeded admin user is `driver-1` in session).
|
|
||||||
- If/when we introduce a real user ↔ driver relationship (1:N), this function becomes the single authoritative mapping point.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Canonical actor model
|
|
||||||
|
|
||||||
The API’s canonical “actor” is:
|
|
||||||
|
|
||||||
```ts
|
|
||||||
type Actor = { userId: string; driverId: string };
|
|
||||||
```
|
|
||||||
|
|
||||||
Returned by [`getActorFromRequestContext()`](apps/api/src/domain/auth/getActorFromRequestContext.ts:12).
|
|
||||||
|
|
||||||
Rules:
|
|
||||||
- All auth/permissions decisions use the actor derived from the authenticated session.
|
|
||||||
- Controllers and services must never use request-body “performer/admin IDs” for authorization decisions.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## League permissions: admin/owner
|
|
||||||
|
|
||||||
### Meaning of “league admin/owner”
|
|
||||||
A driver is authorized as a league admin if:
|
|
||||||
- They have an **active** membership in the league, and
|
|
||||||
- Their membership role is either `owner` or `admin`.
|
|
||||||
|
|
||||||
Authoritative check:
|
|
||||||
- Implemented in the core use case [`GetLeagueAdminPermissionsUseCase.execute()`](core/racing/application/use-cases/GetLeagueAdminPermissionsUseCase.ts:39) by loading membership and validating `status` + `role`.
|
|
||||||
|
|
||||||
### How it is validated server-side
|
|
||||||
Canonical enforcement entrypoint (API layer):
|
|
||||||
- [`requireLeagueAdminOrOwner()`](apps/api/src/domain/league/LeagueAuthorization.ts:15)
|
|
||||||
|
|
||||||
This helper:
|
|
||||||
- Derives the actor from session via [`getActorFromRequestContext()`](apps/api/src/domain/auth/getActorFromRequestContext.ts:12)
|
|
||||||
- Invokes the core use case with `performerDriverId: actor.driverId`
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Contract rule (non-negotiable)
|
|
||||||
|
|
||||||
**No league write operation may accept performer/admin IDs for auth decisions.**
|
|
||||||
|
|
||||||
Concretely:
|
|
||||||
- Request DTOs may still temporarily contain IDs for “target entities” (e.g. `targetDriverId`), but never the acting user/admin/performer ID.
|
|
||||||
- Any endpoint/service that needs “who is performing this” MUST obtain it from session-derived actor, not from request payload, params, or hardcoded values.
|
|
||||||
|
|
||||||
Tests:
|
|
||||||
- Actor derives from session, not payload: [`ActorFromSession`](apps/api/src/domain/auth/ActorFromSession.test.ts:17).
|
|
||||||
- Permission helper uses session-derived actor consistently: [`ActorFromSession`](apps/api/src/domain/auth/ActorFromSession.test.ts:30).
|
|
||||||
- Example application in a league write-like operation (`joinLeague`) ignores payload driverId and uses session actor: [`LeagueService`](apps/api/src/domain/league/LeagueService.test.ts:1).
|
|
||||||
|
|
||||||
---
|
|
||||||
@@ -1,273 +0,0 @@
|
|||||||
# League Standings Calculation Analysis
|
|
||||||
|
|
||||||
## Overview
|
|
||||||
|
|
||||||
This document analyzes the standings calculation logic for the GridPilot leagues feature. The system has two different standings entities:
|
|
||||||
|
|
||||||
1. **Standing** (core/racing/domain/entities/Standing.ts) - League-level standings
|
|
||||||
2. **ChampionshipStanding** (core/racing/domain/entities/championship/ChampionshipStanding.ts) - Season-level standings
|
|
||||||
|
|
||||||
## Architecture Overview
|
|
||||||
|
|
||||||
### Data Flow
|
|
||||||
|
|
||||||
```
|
|
||||||
Race Completion/Result Import
|
|
||||||
↓
|
|
||||||
CompleteRaceUseCase / ImportRaceResultsUseCase
|
|
||||||
↓
|
|
||||||
StandingRepository.recalculate() or StandingRepository.save()
|
|
||||||
↓
|
|
||||||
Standing Entity (League-level)
|
|
||||||
↓
|
|
||||||
GetLeagueStandingsUseCase
|
|
||||||
↓
|
|
||||||
LeagueStandingsPresenter
|
|
||||||
↓
|
|
||||||
API Response
|
|
||||||
```
|
|
||||||
|
|
||||||
### Season-Level Standings Flow
|
|
||||||
|
|
||||||
```
|
|
||||||
Season Completion
|
|
||||||
↓
|
|
||||||
RecalculateChampionshipStandingsUseCase
|
|
||||||
↓
|
|
||||||
ChampionshipStanding Entity (Season-level)
|
|
||||||
↓
|
|
||||||
ChampionshipStandingRepository
|
|
||||||
```
|
|
||||||
|
|
||||||
## Identified Issues
|
|
||||||
|
|
||||||
### Issue 1: InMemoryStandingRepository.recalculate() Doesn't Consider Seasons
|
|
||||||
|
|
||||||
**Location:** `adapters/racing/persistence/inmemory/InMemoryStandingRepository.ts:169-270`
|
|
||||||
|
|
||||||
**Problem:** The `recalculate()` method calculates standings from ALL completed races in a league, not just races from a specific season.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
async recalculate(leagueId: string): Promise<Standing[]> {
|
|
||||||
// ...
|
|
||||||
const races = await this.raceRepository.findCompletedByLeagueId(leagueId);
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:** When standings are recalculated, they include results from all seasons, not just the current/active season. This means:
|
|
||||||
- Historical season results are mixed with current season results
|
|
||||||
- Standings don't accurately reflect a specific season's performance
|
|
||||||
- Drop score policies can't be applied correctly per season
|
|
||||||
|
|
||||||
**Expected Behavior:** Standings should be calculated per season, considering only races from that specific season.
|
|
||||||
|
|
||||||
### Issue 2: CompleteRaceUseCase Uses Hardcoded Points System
|
|
||||||
|
|
||||||
**Location:** `core/racing/application/use-cases/CompleteRaceUseCase.ts:200-203`
|
|
||||||
|
|
||||||
**Problem:** The `updateStandings()` method uses a hardcoded F1 points system instead of the league's configured points system.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
standing = standing.addRaceResult(result.position.toNumber(), {
|
|
||||||
1: 25, 2: 18, 3: 15, 4: 12, 5: 10, 6: 8, 7: 6, 8: 4, 9: 2, 10: 1
|
|
||||||
});
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- All leagues use the same points system regardless of their configuration
|
|
||||||
- Custom points systems are ignored
|
|
||||||
- IndyCar and other racing series points systems are not applied
|
|
||||||
|
|
||||||
**Expected Behavior:** The points system should be retrieved from the league's configuration and applied accordingly.
|
|
||||||
|
|
||||||
### Issue 3: ImportRaceResultsUseCase Recalculates All League Standings
|
|
||||||
|
|
||||||
**Location:** `core/racing/application/use-cases/ImportRaceResultsUseCase.ts:156`
|
|
||||||
|
|
||||||
**Problem:** When importing race results, the method calls `standingRepository.recalculate(league.id.toString())` which recalculates ALL standings for the league.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
await this.standingRepository.recalculate(league.id.toString());
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- Performance issue: Recalculating all standings when only one race is imported
|
|
||||||
- Standings include results from all seasons, not just the current season
|
|
||||||
- Inefficient for large leagues with many races
|
|
||||||
|
|
||||||
**Expected Behavior:** Only recalculate standings for the specific season that the race belongs to.
|
|
||||||
|
|
||||||
### Issue 4: Standing Entity Doesn't Track Season Association
|
|
||||||
|
|
||||||
**Location:** `core/racing/domain/entities/Standing.ts`
|
|
||||||
|
|
||||||
**Problem:** The `Standing` entity only tracks `leagueId` and `driverId`, but not `seasonId`.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
export class Standing extends Entity<string> {
|
|
||||||
readonly leagueId: LeagueId;
|
|
||||||
readonly driverId: DriverId;
|
|
||||||
// No seasonId field
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- Standings are stored per league, not per season
|
|
||||||
- Can't distinguish between standings from different seasons
|
|
||||||
- Can't apply season-specific drop score policies
|
|
||||||
|
|
||||||
**Expected Behavior:** Standing should include a `seasonId` field to track which season the standings belong to.
|
|
||||||
|
|
||||||
### Issue 5: GetLeagueStandingsUseCase Retrieves League-Level Standings
|
|
||||||
|
|
||||||
**Location:** `core/racing/application/use-cases/GetLeagueStandingsUseCase.ts:39`
|
|
||||||
|
|
||||||
**Problem:** The use case retrieves standings from `StandingRepository.findByLeagueId()`, which returns league-level standings.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
const standings = await this.standingRepository.findByLeagueId(input.leagueId);
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- Returns standings that include results from all seasons
|
|
||||||
- Doesn't provide season-specific standings
|
|
||||||
- Can't apply season-specific drop score policies
|
|
||||||
|
|
||||||
**Expected Behavior:** The use case should retrieve season-specific standings (ChampionshipStanding) or accept a seasonId parameter.
|
|
||||||
|
|
||||||
### Issue 6: LeagueStandingsRepository is Not Connected to Standing Calculation
|
|
||||||
|
|
||||||
**Location:** `adapters/racing/persistence/inmemory/InMemoryLeagueStandingsRepository.ts`
|
|
||||||
|
|
||||||
**Problem:** The `LeagueStandingsRepository` interface is defined but not connected to the standing calculation logic.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
export interface LeagueStandingsRepository {
|
|
||||||
getLeagueStandings(leagueId: string): Promise<RawStanding[]>;
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- The repository is not used by any standing calculation use case
|
|
||||||
- It's only used for storing pre-calculated standings
|
|
||||||
- No clear connection between standing calculation and this repository
|
|
||||||
|
|
||||||
**Expected Behavior:** The repository should be used to store and retrieve season-specific standings.
|
|
||||||
|
|
||||||
### Issue 7: LeagueStandingsPresenter Hardcodes Metrics
|
|
||||||
|
|
||||||
**Location:** `apps/api/src/domain/league/presenters/LeagueStandingsPresenter.ts:24-30`
|
|
||||||
|
|
||||||
**Problem:** The presenter hardcodes wins, podiums, and races metrics to 0.
|
|
||||||
|
|
||||||
```typescript
|
|
||||||
wins: 0,
|
|
||||||
podiums: 0,
|
|
||||||
races: 0,
|
|
||||||
```
|
|
||||||
|
|
||||||
**Impact:**
|
|
||||||
- API always returns 0 for these metrics
|
|
||||||
- Users can't see actual win/podium/race counts
|
|
||||||
- TODO comment indicates this is a known issue
|
|
||||||
|
|
||||||
**Expected Behavior:** These metrics should be calculated and returned from the standing entity.
|
|
||||||
|
|
||||||
## Root Cause Analysis
|
|
||||||
|
|
||||||
The core issue is a **design inconsistency** between two different standings systems:
|
|
||||||
|
|
||||||
1. **League-level standings** (`Standing` entity):
|
|
||||||
- Stored per league (not per season)
|
|
||||||
- Updated immediately when races complete
|
|
||||||
- Used by `GetLeagueStandingsUseCase`
|
|
||||||
- Don't support season-specific features
|
|
||||||
|
|
||||||
2. **Season-level standings** (`ChampionshipStanding` entity):
|
|
||||||
- Stored per season
|
|
||||||
- Calculated using `RecalculateChampionshipStandingsUseCase`
|
|
||||||
- Support drop score policies per season
|
|
||||||
- Not used by the main standings API
|
|
||||||
|
|
||||||
The system has **two parallel standings calculation paths** that don't align:
|
|
||||||
- `CompleteRaceUseCase` and `ImportRaceResultsUseCase` update league-level standings
|
|
||||||
- `RecalculateChampionshipStandingsUseCase` calculates season-level standings
|
|
||||||
- `GetLeagueStandingsUseCase` retrieves league-level standings
|
|
||||||
|
|
||||||
## Recommendations
|
|
||||||
|
|
||||||
### Short-term Fixes
|
|
||||||
|
|
||||||
1. **Fix CompleteRaceUseCase to use league's points system**
|
|
||||||
- Retrieve points system from league configuration
|
|
||||||
- Apply correct points based on league settings
|
|
||||||
|
|
||||||
2. **Fix ImportRaceResultsUseCase to recalculate only relevant standings**
|
|
||||||
- Pass seasonId to recalculate method
|
|
||||||
- Only recalculate standings for the specific season
|
|
||||||
|
|
||||||
3. **Add seasonId to Standing entity**
|
|
||||||
- Modify Standing entity to include seasonId
|
|
||||||
- Update all repositories and use cases to handle seasonId
|
|
||||||
|
|
||||||
4. **Fix GetLeagueStandingsUseCase to accept seasonId parameter**
|
|
||||||
- Add seasonId parameter to input
|
|
||||||
- Filter standings by seasonId
|
|
||||||
|
|
||||||
### Long-term Refactoring
|
|
||||||
|
|
||||||
1. **Consolidate standings into single system**
|
|
||||||
- Merge Standing and ChampionshipStanding entities
|
|
||||||
- Use ChampionshipStanding as the primary standings entity
|
|
||||||
- Remove league-level standings
|
|
||||||
|
|
||||||
2. **Update CompleteRaceUseCase to use RecalculateChampionshipStandingsUseCase**
|
|
||||||
- Instead of updating standings immediately, trigger recalculation
|
|
||||||
- Ensure season-specific calculations
|
|
||||||
|
|
||||||
3. **Update ImportRaceResultsUseCase to use RecalculateChampionshipStandingsUseCase**
|
|
||||||
- Instead of calling StandingRepository.recalculate(), call RecalculateChampionshipStandingsUseCase
|
|
||||||
- Ensure season-specific calculations
|
|
||||||
|
|
||||||
4. **Update GetLeagueStandingsUseCase to retrieve ChampionshipStanding**
|
|
||||||
- Change to retrieve season-specific standings
|
|
||||||
- Add seasonId parameter to input
|
|
||||||
|
|
||||||
5. **Remove LeagueStandingsRepository**
|
|
||||||
- This repository is not connected to standing calculation
|
|
||||||
- Use ChampionshipStandingRepository instead
|
|
||||||
|
|
||||||
6. **Fix LeagueStandingsPresenter to return actual metrics**
|
|
||||||
- Calculate wins, podiums, and races from standing entity
|
|
||||||
- Remove hardcoded values
|
|
||||||
|
|
||||||
## Testing Strategy
|
|
||||||
|
|
||||||
### Unit Tests
|
|
||||||
- Test CompleteRaceUseCase with different points systems
|
|
||||||
- Test ImportRaceResultsUseCase with season-specific recalculation
|
|
||||||
- Test GetLeagueStandingsUseCase with seasonId parameter
|
|
||||||
- Test Standing entity with seasonId field
|
|
||||||
|
|
||||||
### Integration Tests
|
|
||||||
- Test complete race flow with season-specific standings
|
|
||||||
- Test import race results flow with season-specific standings
|
|
||||||
- Test standings recalculation for specific season
|
|
||||||
- Test API endpoints with season-specific standings
|
|
||||||
|
|
||||||
### End-to-End Tests
|
|
||||||
- Test league standings API with season parameter
|
|
||||||
- Test standings calculation across multiple seasons
|
|
||||||
- Test drop score policy application per season
|
|
||||||
|
|
||||||
## Conclusion
|
|
||||||
|
|
||||||
The standings calculation system has significant design issues that prevent accurate season-specific standings. The main problems are:
|
|
||||||
|
|
||||||
1. League-level standings don't track seasons
|
|
||||||
2. Points systems are hardcoded in some places
|
|
||||||
3. Standings recalculation includes all seasons
|
|
||||||
4. Two parallel standings systems don't align
|
|
||||||
|
|
||||||
Fixing these issues requires both short-term fixes and long-term refactoring to consolidate the standings system into a single, season-aware design.
|
|
||||||
@@ -1,101 +0,0 @@
|
|||||||
# Standings Calculation Fixes - Summary
|
|
||||||
|
|
||||||
## Overview
|
|
||||||
Fixed multiple issues in the GridPilot leagues feature's standings calculation logic using a TDD approach.
|
|
||||||
|
|
||||||
## Issues Fixed
|
|
||||||
|
|
||||||
### 1. CompleteRaceUseCase - Hardcoded Points System
|
|
||||||
**Problem**: Used hardcoded F1 points (25, 18, 15...) instead of league's configured points system
|
|
||||||
**Impact**: All leagues used same points regardless of configuration (F1, IndyCar, custom)
|
|
||||||
**Fix**: Now uses league's points system from `LeagueScoringConfig`
|
|
||||||
|
|
||||||
### 2. ImportRaceResultsUseCase - Recalculates All League Standings
|
|
||||||
**Problem**: When importing race results, recalculated ALL standings for the league
|
|
||||||
**Impact**: Performance issue + included results from all seasons
|
|
||||||
**Fix**: Now only recalculates standings for the specific season
|
|
||||||
|
|
||||||
### 3. GetLeagueStandingsUseCase - Retrieves League-Level Standings
|
|
||||||
**Problem**: Retrieved standings that included results from all seasons
|
|
||||||
**Impact**: Returns inaccurate standings for any specific season
|
|
||||||
**Fix**: Now accepts `seasonId` parameter and returns season-specific standings
|
|
||||||
|
|
||||||
### 4. LeagueStandingsPresenter - Hardcoded Metrics
|
|
||||||
**Problem**: Hardcoded wins, podiums, and races metrics to 0
|
|
||||||
**Impact**: API always returned 0 for these metrics
|
|
||||||
**Fix**: Now calculates actual metrics from standings data
|
|
||||||
|
|
||||||
## Test Results
|
|
||||||
|
|
||||||
### Core Tests (All Passing)
|
|
||||||
```
|
|
||||||
✓ core/racing/application/use-cases/CompleteRaceUseCase.test.ts (6 tests)
|
|
||||||
✓ core/racing/application/use-cases/ImportRaceResultsUseCase.test.ts (6 tests)
|
|
||||||
✓ core/racing/application/use-cases/GetLeagueStandingsUseCase.test.ts (2 tests)
|
|
||||||
```
|
|
||||||
|
|
||||||
**Total**: 14 tests passed in 307ms
|
|
||||||
|
|
||||||
### Full Test Suite
|
|
||||||
```
|
|
||||||
Test Files: 9 failed, 822 passed, 8 skipped (839)
|
|
||||||
Tests: 9 failed, 4904 passed, 80 skipped (4993)
|
|
||||||
```
|
|
||||||
|
|
||||||
**Note**: The 9 failed tests are pre-existing issues unrelated to standings fixes:
|
|
||||||
- UI test failures in website components
|
|
||||||
- Formatting issues in sponsorship view models
|
|
||||||
- Visibility issues in league config integration tests
|
|
||||||
- Rating issues in team HTTP tests
|
|
||||||
|
|
||||||
## Files Modified
|
|
||||||
|
|
||||||
### Core Domain Layer
|
|
||||||
- `core/racing/application/use-cases/CompleteRaceUseCase.ts`
|
|
||||||
- `core/racing/application/use-cases/ImportRaceResultsUseCase.ts`
|
|
||||||
- `core/racing/application/use-cases/GetLeagueStandingsUseCase.ts`
|
|
||||||
|
|
||||||
### Test Files
|
|
||||||
- `core/racing/application/use-cases/CompleteRaceUseCase.test.ts`
|
|
||||||
- `core/racing/application/use-cases/ImportRaceResultsUseCase.test.ts`
|
|
||||||
- `core/racing/application/use-cases/GetLeagueStandingsUseCase.test.ts`
|
|
||||||
|
|
||||||
### API Layer
|
|
||||||
- `apps/api/src/domain/league/presenters/LeagueStandingsPresenter.ts`
|
|
||||||
- `apps/api/src/domain/race/RaceProviders.ts`
|
|
||||||
|
|
||||||
## Key Improvements
|
|
||||||
|
|
||||||
1. **Accurate Standings**: Standings now correctly reflect season-specific performance
|
|
||||||
2. **Flexible Points Systems**: Leagues can use F1, IndyCar, or custom points systems
|
|
||||||
3. **Better Performance**: Only recalculates relevant standings instead of all standings
|
|
||||||
4. **Complete Metrics**: API now returns accurate win, podium, and race counts
|
|
||||||
5. **Season-Aware**: All standings calculations now consider the specific season
|
|
||||||
|
|
||||||
## Implementation Approach
|
|
||||||
|
|
||||||
Used Test-Driven Development (TDD):
|
|
||||||
1. Wrote failing tests that expected the new behavior
|
|
||||||
2. Implemented fixes to make tests pass
|
|
||||||
3. Verified all tests pass successfully
|
|
||||||
|
|
||||||
## Verification
|
|
||||||
|
|
||||||
All core tests pass successfully:
|
|
||||||
```bash
|
|
||||||
npm test -- --run ./core/racing/application/use-cases/CompleteRaceUseCase.test.ts \
|
|
||||||
./core/racing/application/use-cases/ImportRaceResultsUseCase.test.ts \
|
|
||||||
./core/racing/application/use-cases/GetLeagueStandingsUseCase.test.ts
|
|
||||||
```
|
|
||||||
|
|
||||||
Result: 14 tests passed in 307ms
|
|
||||||
|
|
||||||
## Conclusion
|
|
||||||
|
|
||||||
The fixes successfully address the standings calculation issues. All new tests pass, and the changes are backward compatible. The implementation now correctly handles:
|
|
||||||
- League-specific points systems
|
|
||||||
- Season-specific standings
|
|
||||||
- Accurate metrics calculation
|
|
||||||
- Performance optimization
|
|
||||||
|
|
||||||
The 4 failing tests in the full suite are pre-existing issues unrelated to the standings fixes.
|
|
||||||
@@ -1,161 +0,0 @@
|
|||||||
# League Standings Issues Summary
|
|
||||||
|
|
||||||
## Quick Overview
|
|
||||||
|
|
||||||
The leagues feature's standings calculation has **7 critical issues** that prevent accurate season-specific standings.
|
|
||||||
|
|
||||||
## Issues Found
|
|
||||||
|
|
||||||
### 1. ❌ InMemoryStandingRepository.recalculate() Doesn't Consider Seasons
|
|
||||||
**File:** `adapters/racing/persistence/inmemory/InMemoryStandingRepository.ts:169-270`
|
|
||||||
|
|
||||||
**Problem:** Calculates standings from ALL completed races in a league, not just races from a specific season.
|
|
||||||
|
|
||||||
**Impact:** Standings mix results from all seasons, making them inaccurate for any specific season.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 2. ❌ CompleteRaceUseCase Uses Hardcoded Points System
|
|
||||||
**File:** `core/racing/application/use-cases/CompleteRaceUseCase.ts:200-203`
|
|
||||||
|
|
||||||
**Problem:** Uses hardcoded F1 points system instead of league's configured points system.
|
|
||||||
|
|
||||||
**Impact:** All leagues use same points system regardless of configuration (F1, IndyCar, custom).
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 3. ❌ ImportRaceResultsUseCase Recalculates All League Standings
|
|
||||||
**File:** `core/racing/application/use-cases/ImportRaceResultsUseCase.ts:156`
|
|
||||||
|
|
||||||
**Problem:** Recalculates ALL standings for the league when importing one race.
|
|
||||||
|
|
||||||
**Impact:** Performance issue + includes results from all seasons.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 4. ❌ Standing Entity Doesn't Track Season Association
|
|
||||||
**File:** `core/racing/domain/entities/Standing.ts`
|
|
||||||
|
|
||||||
**Problem:** Standing entity only tracks `leagueId` and `driverId`, no `seasonId`.
|
|
||||||
|
|
||||||
**Impact:** Can't distinguish between standings from different seasons.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 5. ❌ GetLeagueStandingsUseCase Retrieves League-Level Standings
|
|
||||||
**File:** `core/racing/application/use-cases/GetLeagueStandingsUseCase.ts:39`
|
|
||||||
|
|
||||||
**Problem:** Retrieves standings that include results from all seasons.
|
|
||||||
|
|
||||||
**Impact:** Returns inaccurate standings for any specific season.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 6. ❌ LeagueStandingsRepository Not Connected to Standing Calculation
|
|
||||||
**File:** `adapters/racing/persistence/inmemory/InMemoryLeagueStandingsRepository.ts`
|
|
||||||
|
|
||||||
**Problem:** Repository exists but not used by standing calculation logic.
|
|
||||||
|
|
||||||
**Impact:** No clear connection between standing calculation and this repository.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 7. ❌ LeagueStandingsPresenter Hardcodes Metrics
|
|
||||||
**File:** `apps/api/src/domain/league/presenters/LeagueStandingsPresenter.ts:24-30`
|
|
||||||
|
|
||||||
**Problem:** Hardcodes wins, podiums, and races to 0.
|
|
||||||
|
|
||||||
**Impact:** API always returns 0 for these metrics.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Root Cause
|
|
||||||
|
|
||||||
**Design Inconsistency:** Two parallel standings systems that don't align:
|
|
||||||
|
|
||||||
1. **League-level standings** (`Standing` entity):
|
|
||||||
- Updated immediately when races complete
|
|
||||||
- Used by `GetLeagueStandingsUseCase`
|
|
||||||
- Don't support season-specific features
|
|
||||||
|
|
||||||
2. **Season-level standings** (`ChampionshipStanding` entity):
|
|
||||||
- Calculated using `RecalculateChampionshipStandingsUseCase`
|
|
||||||
- Support drop score policies per season
|
|
||||||
- Not used by main standings API
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Recommended Fixes
|
|
||||||
|
|
||||||
### Short-term (Quick Wins)
|
|
||||||
1. ✅ Fix CompleteRaceUseCase to use league's points system
|
|
||||||
2. ✅ Fix ImportRaceResultsUseCase to recalculate only relevant standings
|
|
||||||
3. ✅ Add seasonId to Standing entity
|
|
||||||
4. ✅ Fix GetLeagueStandingsUseCase to accept seasonId parameter
|
|
||||||
|
|
||||||
### Long-term (Refactoring)
|
|
||||||
1. ✅ Consolidate standings into single system (use ChampionshipStanding)
|
|
||||||
2. ✅ Update CompleteRaceUseCase to use RecalculateChampionshipStandingsUseCase
|
|
||||||
3. ✅ Update ImportRaceResultsUseCase to use RecalculateChampionshipStandingsUseCase
|
|
||||||
4. ✅ Update GetLeagueStandingsUseCase to retrieve ChampionshipStanding
|
|
||||||
5. ✅ Remove LeagueStandingsRepository
|
|
||||||
6. ✅ Fix LeagueStandingsPresenter to return actual metrics
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Testing Strategy
|
|
||||||
|
|
||||||
### Unit Tests
|
|
||||||
- Test CompleteRaceUseCase with different points systems
|
|
||||||
- Test ImportRaceResultsUseCase with season-specific recalculation
|
|
||||||
- Test GetLeagueStandingsUseCase with seasonId parameter
|
|
||||||
- Test Standing entity with seasonId field
|
|
||||||
|
|
||||||
### Integration Tests
|
|
||||||
- Test complete race flow with season-specific standings
|
|
||||||
- Test import race results flow with season-specific standings
|
|
||||||
- Test standings recalculation for specific season
|
|
||||||
- Test API endpoints with season-specific standings
|
|
||||||
|
|
||||||
### End-to-End Tests
|
|
||||||
- Test league standings API with season parameter
|
|
||||||
- Test standings calculation across multiple seasons
|
|
||||||
- Test drop score policy application per season
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Files to Modify
|
|
||||||
|
|
||||||
### Core Domain
|
|
||||||
- `core/racing/domain/entities/Standing.ts` - Add seasonId field
|
|
||||||
- `core/racing/domain/repositories/StandingRepository.ts` - Update interface
|
|
||||||
|
|
||||||
### Application Layer
|
|
||||||
- `core/racing/application/use-cases/CompleteRaceUseCase.ts` - Use league points system
|
|
||||||
- `core/racing/application/use-cases/ImportRaceResultsUseCase.ts` - Season-specific recalculation
|
|
||||||
- `core/racing/application/use-cases/GetLeagueStandingsUseCase.ts` - Accept seasonId parameter
|
|
||||||
|
|
||||||
### Persistence Layer
|
|
||||||
- `adapters/racing/persistence/inmemory/InMemoryStandingRepository.ts` - Update recalculate method
|
|
||||||
- `adapters/racing/persistence/inmemory/InMemoryLeagueStandingsRepository.ts` - Remove or refactor
|
|
||||||
|
|
||||||
### API Layer
|
|
||||||
- `apps/api/src/domain/league/presenters/LeagueStandingsPresenter.ts` - Return actual metrics
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Priority
|
|
||||||
|
|
||||||
**HIGH:** Issues 1, 2, 4, 5 (affect accuracy of standings)
|
|
||||||
**MEDIUM:** Issues 3, 6 (affect performance and architecture)
|
|
||||||
**LOW:** Issue 7 (affects API response quality)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Next Steps
|
|
||||||
|
|
||||||
1. Create implementation plan for short-term fixes
|
|
||||||
2. Design long-term refactoring strategy
|
|
||||||
3. Update tests to cover season-specific scenarios
|
|
||||||
4. Implement fixes incrementally
|
|
||||||
5. Verify standings accuracy after each fix
|
|
||||||
Reference in New Issue
Block a user