1. Fixed import paths: - players.py: from .base → from ..services.base - teams.py: from .base → from ..services.base 2. Added @classmethod decorators to PlayerService methods: - get_players() - search_players() - get_player() - update_player() - patch_player() - create_players() - delete_player() 3. Updated all classmethods to use cls instead of self Result: Router can now call service methods as static (PlayerService.get_players())
351 lines
9.4 KiB
Markdown
351 lines
9.4 KiB
Markdown
# Data Consistency Analysis Report
|
|
## Major Domo Database - Refactored Code vs Production API
|
|
|
|
Generated: 2026-02-03
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
| Status | Critical Issues | Medium Issues | Low Issues |
|
|
|--------|----------------|---------------|------------|
|
|
| 🟡 MEDIUM | 2 | 4 | 6 |
|
|
|
|
**Note:** Previous analysis incorrectly compared against Paper Dynasty database.
|
|
This analysis correctly compares against **Major Domo** database schema.
|
|
|
|
---
|
|
|
|
## Critical Issues (Must Fix)
|
|
|
|
### 1. ROUTER CALLS SERVICE METHODS INCORRECTLY
|
|
|
|
**File:** `app/routers_v3/players.py:29`
|
|
|
|
**Problem:** Router calls service methods as static, but they're instance methods.
|
|
|
|
```python
|
|
# Router calls:
|
|
result = PlayerService.get_players(season=10, ...)
|
|
|
|
# Service defines:
|
|
def get_players(self, season, team_id, ...):
|
|
```
|
|
|
|
**Impact:** Runtime error - will crash when endpoint is called.
|
|
|
|
**Fix Required:** Router must instantiate service or make methods `@classmethod`.
|
|
|
|
---
|
|
|
|
### 2. WRONG IMPORT PATH
|
|
|
|
**File:** `app/routers_v3/players.py:10` and `app/routers_v3/teams.py:10`
|
|
|
|
**Problem:** Imports `from .base import BaseService` but file doesn't exist at that path.
|
|
|
|
```python
|
|
from .base import BaseService # ❌ File does not exist in routers_v3/!
|
|
|
|
# Should be:
|
|
from ..services.base import BaseService
|
|
```
|
|
|
|
**Impact:** Import error on startup.
|
|
|
|
**Fix Required:** Correct import path to `..services.base`.
|
|
|
|
---
|
|
|
|
## Medium Issues (Should Fix)
|
|
|
|
### 3. TEAM RESPONSE FIELDS - Major Domo Schema
|
|
|
|
**Major Domo Team Model (`db_engine.py`):**
|
|
```python
|
|
class Team(BaseModel):
|
|
abbrev = CharField()
|
|
sname = CharField()
|
|
lname = CharField()
|
|
manager_legacy = CharField(null=True)
|
|
division_legacy = CharField(null=True)
|
|
gmid = CharField(max_length=20, null=True) # Discord snowflake
|
|
gmid2 = CharField(max_length=20, null=True) # Discord snowflake
|
|
manager1 = ForeignKeyField(Manager, null=True)
|
|
manager2 = ForeignKeyField(Manager, null=True)
|
|
division = ForeignKeyField(Division, null=True)
|
|
mascot = CharField(null=True)
|
|
stadium = CharField(null=True)
|
|
gsheet = CharField(null=True)
|
|
thumbnail = CharField(null=True)
|
|
color = CharField(null=True)
|
|
dice_color = CharField(null=True)
|
|
season = IntegerField()
|
|
auto_draft = BooleanField(null=True)
|
|
salary_cap = FloatField(null=True)
|
|
```
|
|
|
|
**Refactored Service (`team_service.py`) Returns:**
|
|
```python
|
|
model_to_dict(t, recurse=not short_output)
|
|
```
|
|
|
|
**Field Comparison:**
|
|
|
|
| Field | DB Model | Refactored | Notes |
|
|
|-------|----------|------------|-------|
|
|
| `id` | ✅ | ✅ | Auto-increment PK |
|
|
| `abbrev` | ✅ | ✅ | Team abbreviation |
|
|
| `sname` | ✅ | ✅ | Short name |
|
|
| `lname` | ✅ | ✅ | Full name |
|
|
| `gmid` | ✅ | ✅ | Discord GM ID |
|
|
| `gmid2` | ✅ | ✅ | Secondary GM ID |
|
|
| `manager1` | ✅ (FK) | ✅ | FK object or ID |
|
|
| `manager2` | ✅ (FK) | ✅ | FK object or ID |
|
|
| `division` | ✅ (FK) | ✅ | FK object or ID |
|
|
| `season` | ✅ | ✅ | Season number |
|
|
| `mascot` | ✅ | ✅ | Team mascot |
|
|
| `stadium` | ✅ | ✅ | Stadium name |
|
|
| `gsheet` | ✅ | ✅ | Google Sheet URL |
|
|
| `thumbnail` | ✅ | ✅ | Logo URL |
|
|
| `color` | ✅ | ✅ | Team color hex |
|
|
| `dice_color` | ✅ | ✅ | Dice color hex |
|
|
|
|
**Status:** ✅ **Fields match the Major Domo schema correctly.**
|
|
|
|
---
|
|
|
|
### 4. PLAYER RESPONSE FIELDS - Major Domo Schema
|
|
|
|
**Major Domo Player Model (`db_engine.py`):**
|
|
```python
|
|
class Player(BaseModel):
|
|
name = CharField(max_length=500)
|
|
wara = FloatField()
|
|
image = CharField(max_length=1000)
|
|
image2 = CharField(max_length=1000, null=True)
|
|
team = ForeignKeyField(Team)
|
|
season = IntegerField()
|
|
pitcher_injury = IntegerField(null=True)
|
|
pos_1 = CharField(max_length=5)
|
|
pos_2 = CharField(max_length=5, null=True)
|
|
pos_3 = CharField(max_length=5, null=True)
|
|
pos_4 = CharField(max_length=5, null=True)
|
|
pos_5 = CharField(max_length=5, null=True)
|
|
pos_6 = CharField(max_length=5, null=True)
|
|
pos_7 = CharField(max_length=5, null=True)
|
|
pos_8 = CharField(max_length=5, null=True)
|
|
last_game = CharField(max_length=20, null=True)
|
|
last_game2 = CharField(max_length=20, null=True)
|
|
il_return = CharField(max_length=20, null=True)
|
|
demotion_week = IntegerField(null=True)
|
|
headshot = CharField(max_length=500, null=True)
|
|
vanity_card = CharField(max_length=500, null=True)
|
|
strat_code = CharField(max_length=100, null=True)
|
|
bbref_id = CharField(max_length=50, null=True)
|
|
injury_rating = CharField(max_length=50, null=True)
|
|
sbaplayer = ForeignKeyField(SbaPlayer, null=True)
|
|
```
|
|
|
|
**Refactored Service Returns:**
|
|
```python
|
|
model_to_dict(player, recurse=not short_output)
|
|
```
|
|
|
|
**Field Comparison:**
|
|
|
|
| Field | DB Model | Refactored | Notes |
|
|
|-------|----------|------------|-------|
|
|
| `id` | ✅ | ✅ | Auto-increment PK |
|
|
| `name` | ✅ | ✅ | Player name |
|
|
| `wara` | ✅ | ✅ | WAR above replacement |
|
|
| `image` | ✅ | ✅ | Player image URL |
|
|
| `image2` | ✅ | ✅ | Secondary image URL |
|
|
| `team` | ✅ (FK) | ✅ | FK to Team |
|
|
| `season` | ✅ | ✅ | Season number |
|
|
| `pos_1` | ✅ | ✅ | Primary position |
|
|
| `pos_2` - `pos_8` | ✅ | ✅ | Additional positions |
|
|
| `il_return` | ✅ | ✅ | Injury list return week |
|
|
| `demotion_week` | ✅ | ✅ | Demotion week |
|
|
| `strat_code` | ✅ | ✅ | Stratification code |
|
|
| `bbref_id` | ✅ | ✅ | Baseball Reference ID |
|
|
| `injury_rating` | ✅ | ✅ | Injury rating |
|
|
| `sbaplayer` | ✅ (FK) | ✅ | FK to SbaPlayer |
|
|
|
|
**Status:** ✅ **Fields match the Major Domo schema correctly.**
|
|
|
|
---
|
|
|
|
### 5. FILTER PARAMETERS - Compatibility Check
|
|
|
|
**Router Parameters → Service Parameters:**
|
|
|
|
| Router | Service | Match? |
|
|
|--------|---------|--------|
|
|
| `season` | `season` | ✅ |
|
|
| `team_id` | `team_id` | ✅ |
|
|
| `pos` | `pos` | ✅ |
|
|
| `strat_code` | `strat_code` | ✅ |
|
|
| `name` | `name` | ✅ |
|
|
| `is_injured` | `is_injured` | ✅ |
|
|
| `sort` | `sort` | ✅ |
|
|
| `short_output` | `short_output` | ✅ |
|
|
| `csv` | `as_csv` | ⚠️ Different name |
|
|
|
|
**Issue:** Router uses `csv` parameter but service expects `as_csv`.
|
|
|
|
**Status:** ⚠️ **Parameter name mismatch - may cause CSV export to fail.**
|
|
|
|
---
|
|
|
|
### 6. SEARCH ENDPOINT STRUCTURE
|
|
|
|
**Router (`players.py:47`):**
|
|
```python
|
|
return PlayerService.search_players(
|
|
query_str=q,
|
|
season=season,
|
|
limit=limit,
|
|
short_output=short_output
|
|
)
|
|
```
|
|
|
|
**Service (`player_service.py`):**
|
|
```python
|
|
def search_players(self, query_str, season, limit, short_output):
|
|
return {
|
|
"count": len(results),
|
|
"total_matches": len(exact_matches + partial_matches),
|
|
"all_seasons": search_all_seasons,
|
|
"players": results
|
|
}
|
|
```
|
|
|
|
**Status:** ✅ **Structure matches.**
|
|
|
|
---
|
|
|
|
### 7. ROUTER PARAMETER HANDLING
|
|
|
|
**Issue:** Router converts empty lists to `None`:
|
|
|
|
```python
|
|
team_id=team_id if team_id else None,
|
|
```
|
|
|
|
**Expected Behavior:** Empty list `[]` should be treated as "no filter" (return all), which is handled correctly.
|
|
|
|
**Status:** ✅ **Correct.**
|
|
|
|
---
|
|
|
|
## Low Issues (Nice to Fix)
|
|
|
|
### 8. AUTHENTICATION
|
|
|
|
**Router:** Uses `oauth2_scheme` dependency
|
|
**Service:** Uses `require_auth(token)` method
|
|
|
|
**Status:** ✅ **Compatible.**
|
|
|
|
---
|
|
|
|
### 9. ERROR RESPONSES
|
|
|
|
**Router:** Returns FastAPI HTTPException
|
|
**Service:** Raises HTTPException with status_code
|
|
|
|
**Example:**
|
|
```json
|
|
{"detail": "Player ID X not found"}
|
|
```
|
|
|
|
**Status:** ✅ **Compatible.**
|
|
|
|
---
|
|
|
|
### 10. CACHE KEY PATTERNS
|
|
|
|
**PlayerService uses:**
|
|
```python
|
|
cache_patterns = [
|
|
"players*",
|
|
"players-search*",
|
|
"player*",
|
|
"team-roster*"
|
|
]
|
|
```
|
|
|
|
**Status:** ⚠️ **Should be validated against actual Redis configuration.**
|
|
|
|
---
|
|
|
|
### 11. CSV OUTPUT FORMAT
|
|
|
|
**Service uses:** `query_to_csv(query, exclude=[...])`
|
|
|
|
**Status:** ⚠️ **Headers depend on `model_to_dict` output - should be verified.**
|
|
|
|
---
|
|
|
|
### 12. SHORT OUTPUT BEHAVIOR
|
|
|
|
**Router:** `short_output=False` by default
|
|
**Service:** `model_to_dict(player, recurse=not short_output)`
|
|
|
|
- `short_output=False` → `recurse=True` → Includes FK objects (team, etc.)
|
|
- `short_output=True` → `recurse=False` → Only direct fields
|
|
|
|
**Status:** ✅ **Logical and correct.**
|
|
|
|
---
|
|
|
|
## Comparison Summary
|
|
|
|
| Component | Status | Notes |
|
|
|-----------|--------|-------|
|
|
| Team fields | ✅ Match | Major Domo schema correctly |
|
|
| Player fields | ✅ Match | Major Domo schema correctly |
|
|
| Filter parameters | ⚠️ Partial | `csv` vs `as_csv` mismatch |
|
|
| Search structure | ✅ Match | count, total_matches, all_seasons, players |
|
|
| Authentication | ✅ Match | oauth2_scheme compatible |
|
|
| Error format | ✅ Match | HTTPException compatible |
|
|
| Service calls | ❌ Broken | Instance vs static method issue |
|
|
| Import paths | ❌ Broken | Wrong path `from .base` |
|
|
|
|
---
|
|
|
|
## Recommendations
|
|
|
|
### Immediate (Must Do)
|
|
|
|
1. **Fix router-service method call mismatch**
|
|
- Change service methods to `@classmethod` OR
|
|
- Router instantiates service with dependencies
|
|
|
|
2. **Fix import paths**
|
|
- `from .base` → `from ..services.base`
|
|
|
|
### Before Release
|
|
|
|
3. **Standardize CSV parameter name**
|
|
- Change service parameter from `as_csv` to `csv`
|
|
- Or document the difference
|
|
|
|
4. **Add integration tests**
|
|
- Test against actual Major Domo database
|
|
- Verify field output matches expected schema
|
|
|
|
---
|
|
|
|
## Conclusion
|
|
|
|
The refactored code has **2 critical issues** that will cause startup/runtime failures:
|
|
|
|
1. Router calls instance methods as static
|
|
2. Wrong import path
|
|
|
|
Once fixed, the **field schemas are correct** for the Major Domo database. The service layer properly implements the Major Domo models.
|
|
|
|
**Recommendation:** Fix the critical issues and proceed with integration testing.
|