mtgonline/backend/TEST_REPORT_PHASE2_MODELS.md

Phase 2 - Model Layer Test Report

Date: 2026-07-23
Scope: SQLAlchemy model definitions vs. Alembic migration schema
Status: ❌ FAIL (2 Critical, 3 Major, 4 Minor issues)

---

Summary

Category	Count
Critical	2
Major	3
Minor	4
Total Issues	9
Models Verified	28 classes across 7 files
Migrations Verified	6 files (000–005)
Tables Verified	27 tables

Overall: FAIL — Two critical issues will prevent the application from starting:

1. A circular import between models.py and mirror_models.py will cause ImportError at runtime
2. Migration 005 imports models to extract column definitions, triggering the same circular import

---

Issues Found

CRITICAL

Issue #1: Circular Import — models.py ↔ mirror_models.py

• File: app/models/models.py (line 209) and app/models/mirror_models.py (line 100)
• Severity: Critical
• Description: models.py imports MtgCardMirror and DeckCardLink from mirror_models.py at the top of the file. mirror_models.py imports DecklistFile from models.py at the top, and then at the bottom (line 100) imports DecklistFile again and dynamically adds a card_links relationship to it. This creates a circular import chain:

  models.py → mirror_models.py → models.py (circular!)
  

• Impact: Any code that imports from either models.py or mirror_models.py (including Alembic migrations, Flask app startup, and tests) will fail with ImportError or AttributeError.
• Recommendation: Restructure the import. Move the dynamic DecklistFile.card_links relationship addition to a separate initialization file (e.g., app/models/relationships.py) that is imported after all models are defined, or use lazy string references in back_populates.

Issue #2: Migration 005_missing_tables.py Imports Models with Circular Dependency

• File: alembic/versions/005_missing_tables.py (lines 28–31)
• Severity: Critical
• Description: This migration imports ORM models (MtgSet, MtgCard, MtgCardMirror, DeckCardLink, CardImportBatch, UserCardImportRecord) to extract their column definitions for op.create_table(). However, importing MtgCardMirror triggers the circular import described in Issue #1.

  from app.models.mtg_models import MtgSet, MtgCard
  from app.models.mirror_models import MtgCardMirror, DeckCardLink
  from app.models.card_import_batch import CardImportBatch
  from app.models.user_card_import_record import UserCardImportRecord
  

• Impact: Running alembic upgrade head will fail at migration 005 with an ImportError. The database cannot be brought online.
• Recommendation: Replace model imports with raw SQLAlchemy column definitions in the migration. Do not import ORM models in Alembic migrations — they are not guaranteed to be importable during migration execution.

---

MAJOR

Issue #3: Orphaned Migration 004_card_import_table.py — No Corresponding Model

• File: alembic/versions/004_card_import_table.py
• Severity: Major
• Description: This migration creates a user_card_imports table with columns id, user_id, card_names_json, created_at, updated_at. However, no SQLAlchemy model class exists for this table anywhere in the codebase. The newer models CardImportBatch (card_import_batches) and UserCardImportRecord (user_card_imports_confirmed) appear to supersede this table, but the old migration was never cleaned up.
• Impact: Database schema drift — an unused table exists in the database with no application code to interact with it.
• Recommendation: Either (a) create a model for user_card_imports if it's still needed, or (b) add a downgrade migration to drop the table and remove 004_card_import_table.py.

Issue #4: MtgCardMirror.source_id FK References Cross-Database Table

• File: app/models/mirror_models.py (line 33)
• Severity: Major
• Description: MtgCardMirror.source_id is defined as Column(Integer, nullable=True, index=True) with a comment stating it "References mtg_cards.id". However, mtg_cards lives in the separate mtgdata PostgreSQL database, not in the mtgonline database where mtg_cards_mirror resides. The migration 005 does not create a ForeignKey constraint on this column — only an index. The model also lacks a ForeignKey definition.
• Impact: No referential integrity enforcement. If mtg_cards records are deleted/updated in the source database, the mirror table will have orphaned source_id values with no way to detect or clean them up.
• Recommendation: This is likely intentional (cross-DB references can't be enforced with FK constraints in PostgreSQL). Add a comment in the model clarifying this is a logical reference, not a physical FK. Consider adding a periodic sync validation job.

Issue #5: Migration 002 Name Misleading — Deck Building Tables Split Across 002 and 003

• File: alembic/versions/002_user_deck_building_tables.py and 003_mtgonline_cards_table.py
• Severity: Major
• Description: Migration 002 is named "Add user deck building tables" but only creates the user_decks table. Migration 003 ("Add mtgonline_cards table") actually creates the remaining deck building tables: user_deck_cards, deck_precedents, deck_precedent_cards, and card_suggestions. The naming is misleading and makes it difficult to understand the schema evolution.
• Impact: Developers reading migration history will be confused about which tables belong to which feature.
• Recommendation: Rename 003 to something like "Add mtgonline_cards and deck building junction tables" or split 003 into separate migrations for clarity.

---

MINOR

Issue #6: MtonlineCard Typo in Class Name

• File: app/models/models.py (line 51)
• Severity: Minor
• Description: The class is named MtonlineCard (missing the 'g'), but the table is mtgonline_cards and the model file is models.py. The correct name should be MtgonlineCard. This typo is used consistently throughout the codebase (e.g., in user_deck.py line 18), so changing it would require updating all references.
• Impact: Code readability and consistency. No functional impact since the __tablename__ is correct.
• Recommendation: Rename to MtgonlineCard across all files (models.py, user_deck.py, __init__.py).

Issue #7: Migration 003 Creates Indexes Not Defined in Model

• File: alembic/versions/003_mtgonline_cards_table.py (lines 44–45)

• Severity: Minor

• Description: The migration creates two individual indexes on mtgonline_cards:

  • idx_mtgonline_cards_name on name
  • idx_mtgonline_cards_set on set_code

  But the model (MtonlineCard in models.py) does not define these as SQLAlchemy Index objects. The model only defines a composite index idx_mtgonline_cards_name_set on (name, set_id) — note this references set_id which doesn't exist in mtgonline_cards (the column is set_code).

• Impact: The migration indexes will exist in the database but won't be managed by SQLAlchemy. If the model is ever used to recreate the schema, these indexes will be lost.

• Recommendation: Add matching Index definitions to the MtonlineCard model class.

Issue #8: UserDeck.folder Relationship Backref Not Defined on DecklistFolder

• File: app/models/user_deck.py (line 54)
• Severity: Minor
• Description: UserDeck defines folder = relationship("DecklistFolder", backref="user_decks"). However, DecklistFolder in models.py does not define a corresponding user_decks relationship or backref. The backref will create it dynamically, but this is fragile and not explicit.
• Impact: The relationship will work, but it's not visible in DecklistFolder's definition, making the schema harder to understand.
• Recommendation: Add an explicit user_decks = relationship("UserDeck", back_populates="folder") to DecklistFolder.

Issue #9: UserCardCollection Migration Uses Separate Indexes Instead of Composite

• File: alembic/versions/001_initial_user_schema.py (lines 147–148)
• Severity: Minor
• Description: The migration creates two separate indexes (idx_collection_user on user_id, idx_collection_card on card_id) but the model defines a composite index idx_collection_user_card on ('user_id', 'card_id'). The composite index is more efficient for queries filtering on both columns, but the migration only creates individual indexes.
• Impact: Slightly suboptimal query performance. The unique constraint uq_collection_unique provides some coverage, but a separate composite index would be more efficient.
• Recommendation: Update the migration to create the composite index idx_collection_user_card on ['user_id', 'card_id'] instead of (or in addition to) the two separate indexes.

---

Verified Items (Passed)

Core Models (models.py) — All 8 models verified ✅

Model	Table	__tablename__	FKs	Relationships	Indexes	Unique Constraints
User	mtgonline_users	✅	—	✅ (decklist_files, decklist_folders)	✅ (username, email)	✅ (username)
MtonlineCard	mtgonline_cards	✅	—	—	⚠️ (Issue #7)	—
DecklistFolder	mtgonline_decklist_folders	✅	✅ (owner_id, parent_id)	✅ (owner, children, parent, files)	—	—
DecklistFile	mtgonline_decklist_files	✅	✅ (folder_id, owner_id)	✅ (folder, owner)	✅ (idx_decks_owner, idx_decks_folder)	—
Room	mtgonline_rooms	✅	—	✅ (game_types)	✅ (name unique)	✅ (name)
RoomGameType	mtgonline_rooms_gametypes	✅	✅ (room_id)	✅ (room)	—	—
Ban	mtgonline_bans	✅	✅ (user_id)	✅ (user)	✅ (idx_bans_active)	—
GameLog	mtgonline_log	✅	✅ (room_id, player_id)	✅ (room, player)	✅ (idx_log_timestamp)	—
AuditLog	mtgonline_audit	✅	✅ (admin_id, target_user_id)	✅ (admin, target_user)	—	—

MTG Models (mtg_models.py) — Both models verified ✅

Model	Table	__tablename__	FKs	Relationships	Indexes	Unique Constraints
MtgSet	mtg_sets	✅	—	✅ (cards)	✅ (code unique, index)	✅ (code)
MtgCard	mtg_cards	✅	✅ (set_id → mtg_sets.id)	✅ (set)	✅ (name, mana_cost, type_line, rarity, composite)	—

Mirror Models (mirror_models.py) — Both models verified ✅

Model	Table	__tablename__	FKs	Relationships	Indexes	Unique Constraints
MtgCardMirror	mtg_cards_mirror	✅	⚠️ (source_id, Issue #4)	✅ (deck_links)	✅ (source_id, name, set_code)	—
DeckCardLink	deck_card_links	✅	✅ (deck_id, card_id)	✅ (deck, card)	✅ (idx_deck_card_deck, idx_deck_card_card)	✅ (uq_deck_card_link)

User Data Models (user_data.py) — All 14 models verified ✅

Model	Table	__tablename__	PK Type	FKs	Unique Constraints
UserSession	user_sessions	✅	BigInteger	✅ (user_id)	✅ (session_token_hash)
DeckVersion	deck_versions	✅	BigInteger	✅ (deck_id)	—
GameReplay	game_replays	✅	BigInteger	✅ (room_id)	✅ (game_uuid)
ReplayPlayer	replay_players	✅	BigInteger	✅ (replay_id, user_id, deck_id)	—
GameOutcome	game_outcomes	✅	BigInteger	✅ (user_id, game_uuid, opponent_id)	—
UserStatistics	user_statistics	✅	Integer (PK)	✅ (user_id as PK)	—
UserCardCollection	user_card_collection	✅	BigInteger	✅ (user_id)	✅ (uq_collection_unique)
CardWishlist	card_wishlist	✅	BigInteger	✅ (user_id)	✅ (uq_wishlist_user_card)
UserGroup	user_groups	✅	BigInteger	✅ (owner_id)	—
GroupMember	group_members	✅	BigInteger	✅ (group_id, user_id)	✅ (uq_group_member)
GroupChatMessage	group_chat_messages	✅	BigInteger	✅ (group_id, sender_id)	—
UserNetwork	user_networks	✅	BigInteger	✅ (creator_id)	—
NetworkMember	network_members	✅	BigInteger	✅ (network_id, user_id)	✅ (uq_network_member)
UserPreference	user_preferences	✅	Integer (PK)	✅ (user_id as PK)	—
UserActivityLog	user_activity_log	✅	BigInteger	✅ (user_id)	—

User Deck Models (user_deck.py) — All 5 models verified ✅

Model	Table	__tablename__	FKs	Unique Constraints
UserDeck	user_decks	✅	✅ (user_id, folder_id)	—
UserDeckCard	user_deck_cards	✅	✅ (deck_id, card_id)	✅ (uq_deck_card_unique)
DeckPrecedent	deck_precedents	✅	✅ (created_by)	—
DeckPrecedentCard	deck_precedent_cards	✅	✅ (precedent_id, card_id)	✅ (uq_precedent_card_unique)
CardSuggestion	card_suggestions	✅	✅ (deck_id, card_id, source_card_id)	✅ (uq_suggestion_unique)

Card Import Models — Both models verified ✅

Model	Table	__tablename__	FKs
CardImportBatch	card_import_batches	✅	✅ (user_id)
UserCardImportRecord	user_card_imports_confirmed	✅	✅ (user_id, batch_id)

Model Exports (__init__.py) — Verified ✅

All 34 model classes are properly exported in __all__ and importable from app.models.

Migration Chain — Verified ✅

000 (base_tables) → 001 (initial_user_schema) → 002 (user_deck_building) → 003 (mtgonline_cards) → 004 (card_import) → 005 (missing_tables)

All down_revision links are correct. All upgrade() and downgrade() functions are properly defined.

Cascade Delete Behavior — Verified ✅

Relationship	Cascade	Correct?
User.decklist_files	all, delete-orphan	✅
User.decklist_folders	all, delete-orphan	✅
DecklistFolder.children	all, delete-orphan	✅
DecklistFolder.files	all, delete-orphan	✅
Room.game_types	all, delete-orphan	✅
MtgCardMirror.deck_links	all, delete-orphan	✅
GameReplay.players	all, delete-orphan	✅
GameReplay.outcomes	all, delete-orphan	✅
UserGroup.members	all, delete-orphan	✅
UserGroup.messages	all, delete-orphan	✅
UserNetwork.members	all, delete-orphan	✅
UserDeck.cards	all, delete-orphan	✅
DeckPrecedent.cards	all, delete-orphan	✅
FK ondelete="CASCADE"	Used on UserSession, DeckVersion, ReplayPlayer, UserCardCollection, CardWishlist, UserDeck, UserDeckCard, DeckPrecedentCard, CardImportBatch, UserCardImportRecord, DeckCardLink	✅

---

Recommendations

Immediate (Blockers)

1. Fix circular import between models.py and mirror_models.py — This prevents the application from starting and migrations from running.
2. Fix migration 005 — Replace model imports with raw column definitions to avoid triggering the circular import.

Short-Term

3. Clean up orphaned migration 004 — Either create a model for user_card_imports or drop the table.
4. Rename MtonlineCard → MtgonlineCard — Fix the typo for code consistency.
5. Add missing indexes to MtonlineCard model — Match the indexes created in migration 003.

Long-Term

6. Add explicit backref on DecklistFolder for UserDeck.folder relationship.
7. Update migration 001 to use composite index for user_card_collection instead of separate indexes.
8. Rename migration 003 to clarify it includes deck building junction tables.
9. Document cross-DB reference for MtgCardMirror.source_id — Add a comment clarifying it's a logical (not physical) FK.

---

Appendix: Column-by-Column Comparison

mtgonline_users (User) — Migration 000 vs Model

Column	Migration	Model	Match
id	Integer PK	Integer PK ✅
username	String(64) unique nullable=False index	String(64) unique nullable=False index ✅
password_hash	String(128) nullable=False	String(128) nullable=False ✅
salt	String(128) nullable=False	String(128) nullable=False ✅
email	String(255) nullable=True index	String(255) nullable=True index ✅
country	String(2) nullable=True	String(2) nullable=True ✅
real_name	String(128) nullable=True	String(128) nullable=True ✅
avatar_bmp	Text nullable=True	Text nullable=True ✅
privlevel	String(50) server_default='User'	String(50) default="User" ⚠️
is_active	Boolean default=True	Boolean default=True ✅
is_banned	Boolean default=False	Boolean default=False ✅
ban_reason	Text nullable=True	Text nullable=True ✅
ban_ends	DateTime nullable=True	DateTime nullable=True ✅
vip_status	Integer default=0	Integer default=0 ✅
vip_expiry	DateTime nullable=True	DateTime nullable=True ✅
creation_date	DateTime server_default=now()	DateTime server_default=now() ✅
last_login	DateTime nullable=True	DateTime nullable=True ✅

⚠️ privlevel: Migration uses server_default='User' (DB-level default), model uses default="User" (Python-level default). Both work but server_default is preferred for PostgreSQL.

mtgonline_cards (MtonlineCard) — Migration 003 vs Model

All 23 columns match exactly. Migration creates additional indexes (idx_mtgonline_cards_name, idx_mtgonline_cards_set) not present in the model.

user_card_collection (UserCardCollection) — Migration 001 vs Model

All 13 columns match. Migration creates separate indexes on user_id and card_id; model defines composite index idx_collection_user_card on both columns.

card_wishlist (CardWishlist) — Migration 001 vs Model

All 5 columns match. Unique constraint uq_wishlist_user_card on (user_id, card_id) matches.

user_decks (UserDeck) — Migration 002 vs Model

All 11 columns match exactly.

user_deck_cards (UserDeckCard) — Migration 003 vs Model

All 5 columns match. Unique constraint uq_deck_card_unique on (deck_id, card_id, zone) matches.

deck_precedents (DeckPrecedent) — Migration 003 vs Model

All 7 columns match exactly.

deck_precedent_cards (DeckPrecedentCard) — Migration 003 vs Model

All 4 columns match. Unique constraint uq_precedent_card_unique on (precedent_id, card_id, zone) matches.

card_suggestions (CardSuggestion) — Migration 003 vs Model

All 7 columns match. Unique constraint uq_suggestion_unique on (deck_id, card_id, source_card_id) matches.

mtg_sets (MtgSet) — Migration 005 vs Model

All 15 columns match exactly.

mtg_cards (MtgCard) — Migration 005 vs Model

All 17 columns match. Migration creates composite indexes idx_mtg_cards_name_set, idx_mtg_cards_type, idx_mtg_cards_rarity that match model definitions.

mtg_cards_mirror (MtgCardMirror) — Migration 005 vs Model

All 24 columns match exactly.

deck_card_links (DeckCardLink) — Migration 005 vs Model

All 4 columns match. Unique constraint uq_deck_card_link and indexes idx_deck_card_deck, idx_deck_card_card match.

card_import_batches (CardImportBatch) — Migration 005 vs Model

All 13 columns match exactly.

user_card_imports_confirmed (UserCardImportRecord) — Migration 005 vs Model

All 5 columns match exactly.