mtgonline/backend/MIGRATION_AUDIT_REPORT.md

# Database Migration Audit Report
**Project:** mtgonline backend  
**Date:** 2026-07-23  
**Migrations Reviewed:** 5 files in `alembic/versions/`

---

## Migration Chain

| # | File | Revision | Down Revision | Tables Created |
|---|------|----------|---------------|----------------|
| 0 | `000_base_tables.py` | `000` | `None` | `mtgonline_users`, `mtgonline_decklist_files`, `mtgonline_rooms` |
| 1 | `001_initial_user_schema.py` | `001` | `000` | 15 user data tables + re-creates 3 base tables |
| 2 | `002_user_deck_building_tables.py` | `002` | `001` | `user_decks`, `user_deck_cards`, `deck_precedents`, `deck_precedent_cards`, `card_suggestions` |
| 3 | `003_mtgonline_cards_table.py` | `003` | `002` | `mtgonline_cards` |
| 4 | `004_card_import_table.py` | `004` | `003` | `user_card_imports` |

---

## Migration-by-Migration Status

### Migration 000: `000_base_tables.py` — ⚠️ PASS (with warnings)

**Status:** PASS  
**Issues:** None critical.

| Check | Result |
|-------|--------|
| `upgrade()` exists | ✅ |
| `downgrade()` exists | ✅ |
| `mtgonline_users` columns correct | ✅ 20 columns, proper PK, indexes, unique constraints |
| `mtgonline_decklist_files` columns correct | ✅ 11 columns, FK to users, indexes |
| `mtgonline_rooms` columns correct | ✅ 12 columns, FK to users, indexes |
| Foreign keys valid | ✅ All reference `mtgonline_users.id` |
| Indexes created | ✅ `idx_decklist_files_user`, `idx_decklist_files_name`, `idx_rooms_created_by`, `idx_rooms_name` |
| Unique constraints | ✅ `username`, `email` on users |
| Downgrade order correct | ✅ Drops dependent tables first |

---

### Migration 001: `001_initial_user_schema.py` — ❌ FAIL (Critical)

**Status:** FAIL  
**Critical Issue:** Re-creates base tables that migration 000 already created.

#### Critical Issues

| # | Issue | Severity | Details |
|---|-------|----------|---------|
| 1 | **Duplicate table creation** | 🔴 CRITICAL | The `upgrade()` function re-creates `mtgonline_users`, `mtgonline_decklist_files`, and `mtgonline_rooms` — tables that migration 000 already created. This will cause `sqlalchemy.exc.ProgrammingError: relation "mtgonline_users" already exists` when running `alembic upgrade head`. |
| 2 | **Missing `mtgonline_decklist_folders` table** | 🔴 CRITICAL | `user_decks.folder_id` (migration 002) references `mtgonline_decklist_folders.id`, but this table is **never created in any migration**. It only exists in the ORM model (`models.py`). Migration 002 will fail with a FK error. |
| 3 | **Missing `mtgonline_rooms_gametypes` table** | 🟡 WARNING | `RoomGameType` model references `mtgonline_rooms_gametypes` table, never created in any migration. |
| 4 | **Missing `mtgonline_bans` table** | 🟡 WARNING | `Ban` model references `mtgonline_bans` table, never created in any migration. |
| 5 | **Missing `mtgonline_log` table** | 🟡 WARNING | `GameLog` model references `mtgonline_log` table, never created in any migration. |
| 6 | **Missing `mtgonline_audit` table** | 🟡 WARNING | `AuditLog` model references `mtgonline_audit` table, never created in any migration. |

#### Table Creation Analysis (001 upgrade)

All 15 dependent tables are created correctly with valid foreign keys:

| Table | FK References | Valid? |
|-------|--------------|--------|
| `user_sessions` | `mtgonline_users.id` | ✅ |
| `deck_versions` | `mtgonline_decklist_files.id` | ✅ (if base tables exist) |
| `game_replays` | `mtgonline_rooms.id` | ✅ (if base tables exist) |
| `replay_players` | `game_replays.id`, `mtgonline_users.id`, `mtgonline_decklist_files.id` | ✅ |
| `game_outcomes` | `mtgonline_users.id`, `game_replays.game_uuid` | ✅ |
| `user_statistics` | `mtgonline_users.id` (PK) | ✅ |
| `user_card_collection` | `mtgonline_users.id` | ✅ |
| `card_wishlist` | `mtgonline_users.id` | ✅ |
| `user_groups` | `mtgonline_users.id` | ✅ |
| `group_members` | `user_groups.id`, `mtgonline_users.id` | ✅ |
| `group_chat_messages` | `user_groups.id`, `mtgonline_users.id` | ✅ |
| `user_networks` | `mtgonline_users.id` | ✅ |
| `network_members` | `user_networks.id`, `mtgonline_users.id` | ✅ |
| `user_preferences` | `mtgonline_users.id` (PK) | ✅ |
| `user_activity_log` | `mtgonline_users.id` | ✅ |

#### Downgrade Analysis

| Check | Result |
|-------|--------|
| Drop order correct | ✅ (reverse dependency order) |
| Indexes dropped | ⚠️ Not explicitly dropped (but `op.drop_table()` handles this) |
| Base tables dropped | ✅ (at end, after dependents) |

#### Comment Numbering Issue

The `upgrade()` function has inconsistent section numbering:
- "0. Base Tables" → "0.1. Rooms Table" → "1. User Sessions" → "2. Deck Versions" → **"4. Game Replays"** (skips 3)

---

### Migration 002: `002_user_deck_building_tables.py` — ❌ FAIL (Critical)

**Status:** FAIL  
**Critical Issue:** References non-existent `mtgonline_decklist_folders` table.

| Check | Result |
|-------|--------|
| `upgrade()` exists | ✅ |
| `downgrade()` exists | ✅ |
| `user_decks` FK to `mtgonline_decklist_folders.id` | ❌ **Table never created in any migration** |
| `user_deck_cards` FK to `user_decks.id` | ✅ |
| `deck_precedent_cards` FK to `deck_precedents.id` | ✅ |
| `card_suggestions` FK to `user_decks.id` | ✅ |
| Unique constraints | ✅ `uq_deck_card_unique`, `uq_precedent_card_unique`, `uq_suggestion_unique` |
| Indexes created | ✅ |
| Downgrade order correct | ✅ |

#### Additional Issues

| # | Issue | Severity |
|---|-------|----------|
| 7 | `user_deck_cards.card_id` has no FK constraint in migration, but model defines `ForeignKey("mtgonline_cards.id")` | 🟡 WARNING |
| 8 | `card_suggestions.source_card_id` has no FK constraint in migration | 🟡 WARNING |
| 9 | `deck_precedent_cards.card_id` has no FK constraint in migration | 🟡 WARNING |
| 10 | `deck_precedents.created_by` has no FK constraint in migration | 🟡 WARNING |

---

### Migration 003: `003_mtgonline_cards_table.py` — ✅ PASS

**Status:** PASS

| Check | Result |
|-------|--------|
| `upgrade()` exists | ✅ |
| `downgrade()` exists | ✅ |
| `mtgonline_cards` columns correct | ✅ 22 columns |
| Indexes created | ✅ `idx_mtgonline_cards_name`, `idx_mtgonline_cards_set` |
| Foreign keys | None (standalone table) |
| Unique constraints | None |

---

### Migration 004: `004_card_import_table.py` — ⚠️ PASS (with warnings)

**Status:** PASS  
**Issues:** Minor consistency issues.

| Check | Result |
|-------|--------|
| `upgrade()` exists | ✅ |
| `downgrade()` exists | ✅ |
| `user_card_imports` columns correct | ✅ 5 columns |
| FK to `mtgonline_users.id` | ✅ |
| Unique constraint `uq_user_card_imports_user_id` | ✅ |
| Index `idx_user_card_imports_user` | ✅ |
| Primary key definition | ⚠️ `sa.Column('id', sa.Integer(), autoincrement=True, nullable=False)` + `sa.PrimaryKeyConstraint('id')` — redundant but functional |

---

## Cross-Reference: Models vs Migrations

### Tables in Models but NOT in Any Migration (🔴 CRITICAL)

| Model | Table Name | Referenced By |
|-------|-----------|---------------|
| `DecklistFolder` | `mtgonline_decklist_folders` | `DecklistFile.folder_id`, `UserDeck.folder_id` |
| `RoomGameType` | `mtgonline_rooms_gametypes` | `Room.game_types` |
| `Ban` | `mtgonline_bans` | Admin ban records |
| `GameLog` | `mtgonline_log` | Game chat logs |
| `AuditLog` | `mtgonline_audit` | Admin action audit trail |
| `MtgCardMirror` | `mtg_cards_mirror` | Card mirror for deckbuilding |
| `DeckCardLink` | `deck_card_links` | Junction: decks ↔ mirrored cards |
| `CardImportBatch` | `card_import_batches` | Card import batch tracking |
| `UserCardImportRecord` | `user_card_imports_confirmed` | Confirmed import records |

**These 9 tables must be created in migrations before any migration that references them can succeed.**

### Column Type Mismatches (Model vs Migration)

#### `mtgonline_users`

| Column | Migration | Model | Match? |
|--------|-----------|-------|--------|
| `username` | `String(50)` | `String(64)` | ❌ |
| `password_hash` | `String(255)` | `String(128)` | ❌ |
| `salt` | `String(32)` | `String(128)` | ❌ |
| `display_name` | `String(100)` | **Not in model** | ⚠️ |
| `avatar_url` | `String(500)` | **Not in model** | ⚠️ |
| `country` | `String(100)` | `String(2)` | ❌ |
| `real_name` | `String(255)` | `String(128)` | ❌ |
| `avatar_bmp` | `LargeBinary()` | `Text` | ❌ |
| `privlevel` | `Integer()` | `String(50)` | ❌ |
| `is_active` | `Boolean()` | `Boolean()` | ✅ |
| `is_banned` | `Boolean()` | `Boolean()` | ✅ |
| `ban_reason` | `Text()` | `Text()` | ✅ |
| `ban_ends` | `DateTime()` | `DateTime()` | ✅ |
| `vip_status` | `Boolean()` | `Integer()` | ❌ |
| `vip_expiry` | `DateTime()` | `DateTime()` | ✅ |
| `creation_date` | `DateTime()` | `DateTime()` | ✅ |
| `last_login` | `DateTime()` | `DateTime()` | ✅ |

#### `mtgonline_decklist_files`

| Column | Migration | Model | Match? |
|--------|-----------|-------|--------|
| `user_id` | FK column | `owner_id` | ❌ (different name) |
| `name` | `String(255)` | `String(255)` | ✅ |
| `content` | `Text()` | `Text()` (nullable=True) | ⚠️ |
| `description` | `Text()` | **Not in model** | ⚠️ |
| `format` | `String(50), default='standard'` | `String(50), default='native'` | ❌ |
| `is_favorite` | `Boolean()` | **Not in model** | ⚠️ |
| `import_source` | `String(50)` | **Not in model** | ⚠️ |
| `import_confidence` | `Float()` | **Not in model** | ⚠️ |
| `last_played` | `DateTime()` | **Not in model** | ⚠️ |
| **Missing** | — | `folder_id` | ❌ |
| **Missing** | — | `status` | ❌ |

#### `mtgonline_rooms`

| Column | Migration | Model | Match? |
|--------|-----------|-------|--------|
| `name` | `String(100)` | `String(100), unique=True` | ⚠️ (migration missing unique) |
| `description` | `Text()` | `Text()` | ✅ |
| `max_players` | `Integer(), default=8` | **Not in model** | ⚠️ |
| `is_public` | `Boolean()` | **Not in model** | ⚠️ |
| `is_password_protected` | `Boolean()` | `Boolean()` | ✅ |
| `password_hash` | `String(255)` | `String(128)` | ❌ |
| `game_type` | `String(50)` | **Not in model** | ⚠️ |
| `format` | `String(50)` | **Not in model** | ⚠️ |
| `created_by` | `Integer()` | **Not in model** | ⚠️ |

---

## Summary of All Issues

### 🔴 Critical (Must Fix Before Deployment)

| # | Issue | Location | Impact |
|---|-------|----------|--------|
| 1 | **Migration 001 re-creates base tables** | `001_initial_user_schema.py:upgrade()` | `alembic upgrade head` will FAIL — tables already exist from migration 000 |
| 2 | **`mtgonline_decklist_folders` never created** | Missing from all migrations | Migration 002 (`user_decks.folder_id`) will FAIL with FK error |
| 3 | **9 model tables have no migration** | `mtgonline_decklist_folders`, `mtgonline_rooms_gametypes`, `mtgonline_bans`, `mtgonline_log`, `mtgonline_audit`, `mtg_cards_mirror`, `deck_card_links`, `card_import_batches`, `user_card_imports_confirmed` | Any code referencing these tables will fail at runtime |

### 🟡 Warnings (Should Fix)

| # | Issue | Location | Impact |
|---|-------|----------|--------|
| 4 | Column type mismatches (15+ columns) | Migration 000/001 vs models | Schema drift — DB won't match ORM definitions |
| 5 | Column name mismatch (`user_id` vs `owner_id`) | `mtgonline_decklist_files` | ORM won't map correctly |
| 6 | Missing `unique=True` on `mtgonline_rooms.name` | Migration 000 | Model defines it as unique |
| 7 | Missing FK constraints on junction table columns | Migration 002 | `card_id`, `source_card_id`, `created_by` lack FK references |
| 8 | Inconsistent section numbering in migration 001 | `001_initial_user_schema.py` | Code readability |
| 9 | Redundant PK definition in migration 004 | `004_card_import_table.py` | Works but messy |

### ℹ️ Informational

| # | Issue | Location |
|---|-------|----------|
| 10 | Two card table models: `MtgCardMirror` (mtg_cards_mirror) and `MtonlineCard` (mtgonline_cards) | Different tables, different purposes |
| 11 | Migration 001 creates `user_card_collection` without composite unique constraint that model defines | Model has `UniqueConstraint('user_id', 'card_id', 'is_foil', 'is_alt_art')` |
| 12 | `user_card_collection` migration missing composite index `idx_collection_user_card` | Model defines it |

---

## Recommendations

### Immediate (Blockers)

1. **Remove duplicate table creation from migration 001** — Delete the `mtgonline_users`, `mtgonline_decklist_files`, and `mtgonline_rooms` `op.create_table()` calls from `001_initial_user_schema.py`. These are already created by migration 000.

2. **Create migration for `mtgonline_decklist_folders`** — This table is referenced by both `mtgonline_decklist_files.folder_id` (model) and `user_decks.folder_id` (migration 002). Add it before migration 002 runs.

3. **Create migrations for all missing tables** — At minimum: `mtgonline_rooms_gametypes`, `mtgonline_bans`, `mtgonline_log`, `mtgonline_audit`, `mtg_cards_mirror`, `deck_card_links`, `card_import_batches`, `user_card_imports_confirmed`.

### Short-term (Consistency)

4. **Align migration schemas with ORM models** — Fix all column type mismatches and missing columns. The migrations should be the source of truth for the database, and models should match.

5. **Add missing FK constraints in migration 002** — Add `ForeignKey` to `card_id`, `source_card_id`, and `created_by` columns.

6. **Add missing unique constraint on `mtgonline_rooms.name`** — Migration 000 should include `unique=True`.

7. **Add missing constraints to `user_card_collection`** — Migration 001 should include the composite unique constraint and index that the model defines.

### Long-term (Architecture)

8. **Decide on migration strategy** — Either:
   - (a) Remove migration 001's duplicate base tables and keep migration 000 as the single source of base table creation, OR
   - (b) Remove migration 000 entirely and let migration 001 handle all base tables (but this is risky for a production database).

9. **Add Alembic environment script** — Create `alembic/env.py` with `include_object` filter to auto-detect table creation order and prevent circular dependencies.

10. **Consider using `op.create_foreign_key()` explicitly** — Some FK definitions in the migrations use inline `ForeignKey()` which is fine, but explicit `op.create_foreign_key()` calls are more readable and Alembic can better track them for downgrade.