docs: reorganize migration docs into dedicated subfolder

- Create docs/development/migration/ directory - Move database-migrations.md to migration subfolder - Move svc-006-migration-plan.md to migration subfolder - Update all cross-references in: - mkdocs.yml nav configuration - docs/index.md - docs/architecture/overview.md - docs/backend/overview.md - docs/development/contributing.md - docs/development/troubleshooting.md - docs/getting-started/database-setup.md This separates migration plans from core documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-03 22:16:05 +01:00
parent 804f919f12
commit dd51df7b31
9 changed files with 8 additions and 8 deletions
--- a/docs/development/migration/database-migrations.md
+++ b/docs/development/migration/database-migrations.md
@@ -0,0 +1,328 @@
+# Database Migrations Guide
+
+This guide covers advanced database migration workflows for developers working on schema changes.
+
+## Overview
+
+Our project uses Alembic for database migrations. All schema changes must go through the migration system to ensure:
+- Reproducible deployments
+- Team synchronization
+- Production safety
+- Rollback capability
+
+## Migration Commands Reference
+
+### Creating Migrations
+```bash
+# Auto-generate migration from model changes
+make migrate-create message="add_user_profile_table"
+
+# Create empty migration template for manual changes
+make migrate-create-manual message="add_custom_indexes"
+```
+
+### Applying Migrations
+```bash
+# Apply all pending migrations
+make migrate-up
+
+# Rollback last migration
+make migrate-down
+
+# Rollback to specific revision
+make migrate-down-to revision="abc123"
+```
+
+### Migration Status
+```bash
+# Show current migration status
+make migrate-status
+
+# Show detailed migration history
+alembic history --verbose
+
+# Show specific migration details
+make migrate-show revision="abc123"
+```
+
+### Backup and Safety
+```bash
+# Create database backup before major changes
+make backup-db
+
+# Verify database setup
+make verify-setup
+```
+
+## Development Workflows
+
+### Adding New Database Fields
+
+1. **Modify your SQLAlchemy model**:
+   ```python
+   # In models/database/user.py
+   class User(Base):
+       # ... existing fields
+       profile_image = Column(String, nullable=True)  # NEW FIELD
+   ```
+
+2. **Generate migration**:
+   ```bash
+   make migrate-create message="add_profile_image_to_users"
+   ```
+
+3. **Review generated migration**:
+   ```python
+   # Check alembic/versions/xxx_add_profile_image_to_users.py
+   def upgrade() -> None:
+       op.add_column('users', sa.Column('profile_image', sa.String(), nullable=True))
+
+   def downgrade() -> None:
+       op.drop_column('users', 'profile_image')
+   ```
+
+4. **Apply migration**:
+   ```bash
+   make migrate-up
+   ```
+
+### Adding Database Indexes
+
+1. **Create manual migration**:
+   ```bash
+   make migrate-create-manual message="add_performance_indexes"
+   ```
+
+2. **Edit the migration file**:
+   ```python
+   def upgrade() -> None:
+       # Add indexes for better performance
+       op.create_index('idx_products_marketplace_shop', 'products', ['marketplace', 'shop_name'])
+       op.create_index('idx_users_email_active', 'users', ['email', 'is_active'])
+
+   def downgrade() -> None:
+       op.drop_index('idx_users_email_active', table_name='users')
+       op.drop_index('idx_products_marketplace_shop', table_name='products')
+   ```
+
+3. **Apply migration**:
+   ```bash
+   make migrate-up
+   ```
+
+### Complex Schema Changes
+
+For complex changes that require data transformation:
+
+1. **Create migration with data handling**:
+   ```python
+   def upgrade() -> None:
+       # Create new column
+       op.add_column('products', sa.Column('normalized_price', sa.Numeric(10, 2)))
+       
+       # Migrate data
+       connection = op.get_bind()
+       connection.execute(
+           text("UPDATE products SET normalized_price = CAST(price AS NUMERIC) WHERE price ~ '^[0-9.]+$'")
+       )
+       
+       # Make column non-nullable after data migration
+       op.alter_column('products', 'normalized_price', nullable=False)
+
+   def downgrade() -> None:
+       op.drop_column('products', 'normalized_price')
+   ```
+
+## Production Deployment
+
+### Pre-Deployment Checklist
+- [ ] All migrations tested locally
+- [ ] Database backup created
+- [ ] Migration rollback plan prepared
+- [ ] Team notified of schema changes
+
+### Deployment Process
+```bash
+# 1. Pre-deployment checks
+make pre-deploy-check
+
+# 2. Backup production database
+make backup-db
+
+# 3. Deploy with migrations
+make deploy-prod  # This includes migrate-up
+```
+
+### Rollback Process
+```bash
+# If deployment fails, rollback
+make rollback-prod  # This includes migrate-down
+```
+
+## Best Practices
+
+### Migration Naming
+Use clear, descriptive names:
+```bash
+# Good examples
+make migrate-create message="add_user_profile_table"
+make migrate-create message="remove_deprecated_product_fields"
+make migrate-create message="add_indexes_for_search_performance"
+
+# Avoid vague names
+make migrate-create message="update_database"  # Too vague
+make migrate-create message="fix_stuff"        # Not descriptive
+```
+
+### Safe Schema Changes
+
+**Always Safe**:
+- Adding nullable columns
+- Adding indexes
+- Adding new tables
+- Increasing column size (varchar(50) → varchar(100))
+
+**Potentially Unsafe** (require careful planning):
+- Dropping columns
+- Changing column types
+- Adding non-nullable columns without defaults
+- Renaming tables or columns
+
+**Multi-Step Process for Unsafe Changes**:
+```python
+# Step 1: Add new column
+def upgrade() -> None:
+    op.add_column('users', sa.Column('email_new', sa.String(255)))
+
+# Step 2: Migrate data (separate migration)
+def upgrade() -> None:
+    connection = op.get_bind()
+    connection.execute(text("UPDATE users SET email_new = email"))
+
+# Step 3: Switch columns (separate migration)
+def upgrade() -> None:
+    op.drop_column('users', 'email')
+    op.alter_column('users', 'email_new', new_column_name='email')
+```
+
+### Testing Migrations
+
+1. **Test on copy of production data**:
+   ```bash
+   # Restore production backup to test database
+   # Run migrations on test database
+   # Verify data integrity
+   ```
+
+2. **Test rollback process**:
+   ```bash
+   make migrate-up     # Apply migration
+   # Test application functionality
+   make migrate-down   # Test rollback
+   # Verify rollback worked correctly
+   ```
+
+## Advanced Features
+
+### Environment-Specific Migrations
+
+Use migration context to handle different environments:
+```python
+from alembic import context
+
+def upgrade() -> None:
+    # Only add sample data in development
+    if context.get_x_argument(as_dictionary=True).get('dev_data', False):
+        # Add development sample data
+        pass
+    
+    # Always apply schema changes
+    op.create_table(...)
+```
+
+Run with environment flag:
+```bash
+alembic upgrade head -x dev_data=true
+```
+
+### Data Migrations
+
+For large data transformations, use batch processing:
+```python
+def upgrade() -> None:
+    connection = op.get_bind()
+    
+    # Process in batches to avoid memory issues
+    batch_size = 1000
+    offset = 0
+    
+    while True:
+        result = connection.execute(
+            text(f"SELECT id, old_field FROM products LIMIT {batch_size} OFFSET {offset}")
+        )
+        rows = result.fetchall()
+        
+        if not rows:
+            break
+            
+        for row in rows:
+            # Transform data
+            new_value = transform_function(row.old_field)
+            connection.execute(
+                text("UPDATE products SET new_field = :new_val WHERE id = :id"),
+                {"new_val": new_value, "id": row.id}
+            )
+        
+        offset += batch_size
+```
+
+## Troubleshooting
+
+### Common Issues
+
+**Migration conflicts**:
+```bash
+# When multiple developers create migrations simultaneously
+# Resolve by creating a merge migration
+alembic merge -m "merge migrations" head1 head2
+```
+
+**Failed migration**:
+```bash
+# Check current state
+make migrate-status
+
+# Manually fix database if needed
+# Then mark migration as applied
+alembic stamp head
+```
+
+**Out of sync database**:
+```bash
+# Reset to known good state
+make backup-db
+alembic downgrade base
+make migrate-up
+```
+
+### Recovery Procedures
+
+1. **Database corruption**: Restore from backup, replay migrations
+2. **Failed deployment**: Use rollback process, investigate issue
+3. **Development issues**: Reset local database, pull latest migrations
+
+## Integration with CI/CD
+
+Our deployment pipeline automatically:
+1. Runs migration checks in CI
+2. Creates database backups before deployment
+3. Applies migrations during deployment
+4. Provides rollback capability
+
+Migration failures will halt deployment to prevent data corruption.
+
+## Further Reading
+
+- [Alembic Official Documentation](https://alembic.sqlalchemy.org/)
+- [Database Setup Guide](../getting-started/database-setup-guide.md)
+- [Deployment Guide](../deployment/production.md)
--- a/docs/development/migration/svc-006-migration-plan.md
+++ b/docs/development/migration/svc-006-migration-plan.md
@@ -0,0 +1,266 @@
+# SVC-006 Migration Plan: Move db.commit() from Services to Endpoints
+
+## Overview
+
+**Objective:** Migrate all `db.commit()` calls from service layer to API endpoints to follow the industry-standard transaction control pattern.
+
+**Rule:** SVC-006 - Services must NOT call `db.commit()`
+
+**Principle:** One request = one transaction, with commit controlled at the API endpoint level.
+
+---
+
+## Benefits
+
+| Benefit | Description |
+|---------|-------------|
+| **Composability** | Multiple service calls can be composed in a single transaction |
+| **Clean rollback** | If any operation fails, the entire request rolls back automatically |
+| **Testability** | Services can be tested in isolation without actual commits |
+| **Consistency** | All transactions follow the same pattern |
+
+---
+
+## Migration Pattern
+
+### Before (Anti-pattern)
+```python
+# Service
+def create_vendor(self, db: Session, data: VendorCreate) -> Vendor:
+    vendor = Vendor(**data.model_dump())
+    db.add(vendor)
+    db.commit()      # ❌ Service commits
+    db.refresh(vendor)
+    return vendor
+
+# Endpoint
+def create_vendor_endpoint(...):
+    vendor = vendor_service.create_vendor(db, data)
+    return VendorResponse.model_validate(vendor)
+```
+
+### After (Correct pattern)
+```python
+# Service
+def create_vendor(self, db: Session, data: VendorCreate) -> Vendor:
+    vendor = Vendor(**data.model_dump())
+    db.add(vendor)
+    db.flush()       # ✅ Get ID without committing
+    return vendor
+
+# Endpoint
+def create_vendor_endpoint(...):
+    vendor = vendor_service.create_vendor(db, data)
+    db.commit()      # ✅ ARCH: Commit at API level for transaction control
+    return VendorResponse.model_validate(vendor)
+```
+
+### Key Changes
+
+1. **Replace `db.commit()` with `db.flush()`** in services when you need the ID
+2. **Remove `db.refresh()`** from services (endpoint can do this if needed)
+3. **Remove `db.rollback()`** from service exception handlers (endpoint handles this)
+4. **Add `db.commit()`** to the endpoint after service call(s)
+
+---
+
+## Services to Migrate
+
+### Priority 1: Core Business Services (High Impact)
+| Service | Commits | Complexity | Endpoints to Update |
+|---------|---------|------------|---------------------|
+| `admin_service.py` | 6 | Medium | Multiple admin endpoints |
+| `inventory_service.py` | 9 | High | Inventory management endpoints |
+| `product_service.py` | 3 | Medium | Product CRUD endpoints |
+| `order_service.py` | 2 | Medium | Order processing endpoints |
+
+### Priority 2: Domain Services (Medium Impact)
+| Service | Commits | Complexity | Endpoints to Update |
+|---------|---------|------------|---------------------|
+| `vendor_domain_service.py` | 4 | Medium | Domain management endpoints |
+| `vendor_team_service.py` | 5 | Medium | Team management endpoints |
+| `vendor_theme_service.py` | 3 | Low | Theme endpoints |
+| `customer_service.py` | 4 | Medium | Customer endpoints |
+| `cart_service.py` | 5 | Medium | Cart/checkout endpoints |
+
+### Priority 3: Supporting Services (Lower Impact)
+| Service | Commits | Complexity | Endpoints to Update |
+|---------|---------|------------|---------------------|
+| `content_page_service.py` | 3 | Low | CMS endpoints |
+| `marketplace_product_service.py` | 3 | Low | Marketplace endpoints |
+| `team_service.py` | 2 | Low | Team endpoints |
+| `admin_settings_service.py` | 3 | Low | Settings endpoints |
+| `code_quality_service.py` | 5 | Low | Internal tooling |
+
+### Priority 4: Auth & Audit (Special Handling)
+| Service | Commits | Notes |
+|---------|---------|-------|
+| `auth_service.py` | 1 | Handle carefully - auth flows |
+| `admin_audit_service.py` | 1 | May need immediate commit for audit trail |
+| `marketplace_import_job_service.py` | 1 | Background job - special handling |
+
+### Excluded
+| Service | Commits | Reason |
+|---------|---------|--------|
+| `log_service.py` | 2 | Audit logs require immediate commits |
+
+---
+
+## Completed Migrations
+
+- [x] `vendor_service.py` (6 commits → 0) - Commit: 6bd3af0
+
+---
+
+## Migration Steps (Per Service)
+
+### Step 1: Identify Commits
+```bash
+grep -n "db.commit()" app/services/<service_name>.py
+```
+
+### Step 2: Find Calling Endpoints
+```bash
+grep -rn "<service>_service\.<method>" app/api/ --include="*.py"
+```
+
+### Step 3: Update Service
+For each method with `db.commit()`:
+
+1. **If creating entities:**
+   ```python
+   # Before
+   db.add(entity)
+   db.commit()
+   db.refresh(entity)
+
+   # After
+   db.add(entity)
+   db.flush()  # Get ID without committing
+   ```
+
+2. **If updating entities:**
+   ```python
+   # Before
+   entity.field = value
+   db.commit()
+   db.refresh(entity)
+
+   # After
+   entity.field = value
+   # No commit - endpoint handles it
+   ```
+
+3. **Remove rollback from exception handlers:**
+   ```python
+   # Before
+   except SomeException:
+       db.rollback()
+       raise
+
+   # After
+   except SomeException:
+       raise  # Endpoint handles rollback
+   ```
+
+### Step 4: Update Endpoints
+Add `db.commit()` after service calls:
+```python
+result = some_service.some_method(db, data)
+db.commit()  # ✅ ARCH: Commit at API level for transaction control
+return SomeResponse(...)
+```
+
+### Step 5: Validate
+```bash
+# Check no commits remain in service
+grep -n "db.commit()" app/services/<service_name>.py
+
+# Run architecture validator
+python scripts/validate_architecture.py -o <entity_name>
+
+# Run tests
+pytest tests/test_<service_name>.py -v
+```
+
+### Step 6: Commit
+```bash
+git add app/services/<service>.py app/api/v1/...
+git commit -m "refactor(<entity>): migrate db.commit() from service to endpoints (SVC-006)"
+```
+
+---
+
+## Testing Strategy
+
+### Unit Tests
+- Services should be tested with `db.flush()` (no actual commit)
+- Mock the session or use a test transaction that rolls back
+
+### Integration Tests
+- Test full request flow including commit
+- Verify data persists after endpoint call
+- Verify rollback on exception
+
+### Manual Testing
+After each migration:
+1. Test create operations - verify data persists
+2. Test update operations - verify changes persist
+3. Test error scenarios - verify no partial commits
+
+---
+
+## Rollback Strategy
+
+If issues are found after migration:
+
+1. **Revert the commit:**
+   ```bash
+   git revert <commit_hash>
+   ```
+
+2. **Or fix forward:**
+   - Add `db.commit()` back to service temporarily
+   - Investigate the endpoint that's missing commit
+   - Fix and re-deploy
+
+---
+
+## Progress Tracking
+
+Run this command to check remaining violations:
+```bash
+python scripts/validate_architecture.py 2>&1 | grep "SVC-006" | wc -l
+```
+
+Current status: **60 remaining** (down from 66)
+
+---
+
+## Timeline Recommendation
+
+This migration can be done gradually:
+
+1. **Immediate:** Fix as you touch each service for other changes
+2. **Batch:** Dedicate time to migrate 2-3 services per session
+3. **Priority:** Focus on Priority 1 services first for maximum impact
+
+The SVC-006 rule is set to **warning** (not error), so CI won't fail. This allows gradual migration without blocking deployments.
+
+---
+
+## Commands Reference
+
+```bash
+# Count remaining violations
+python scripts/validate_architecture.py 2>&1 | grep "SVC-006" | wc -l
+
+# List services with commits (sorted by count)
+grep -c "db.commit()" app/services/*.py | grep -v ":0$" | sort -t: -k2 -n
+
+# Validate specific entity
+python scripts/validate_architecture.py -o vendor
+
+# Validate specific file
+python scripts/validate_architecture.py -f app/services/admin_service.py
+```