refactor: move letzshop endpoints to marketplace module and add vendor service tests

Move letzshop-related functionality from tenancy to marketplace module:
- Move admin letzshop routes to marketplace/routes/api/admin_letzshop.py
- Move letzshop schemas to marketplace/schemas/letzshop.py
- Remove letzshop code from tenancy module (admin_vendors, vendor_service)
- Update model exports and imports

Add comprehensive unit tests for vendor services:
- test_company_service.py: Company management operations
- test_platform_service.py: Platform management operations
- test_vendor_domain_service.py: Vendor domain operations
- test_vendor_team_service.py: Vendor team management

Update module definitions:
- billing, messaging, payments: Minor definition updates

Add architecture proposals documentation:
- Module dependency redesign session notes
- Decouple modules implementation plan
- Module decoupling proposal

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
2026-02-04 19:25:00 +01:00
parent 37942ae02b
commit 0583dd2cc4
29 changed files with 3643 additions and 650 deletions

View File

@@ -0,0 +1,552 @@
# Implementation Plan: Decouple Core Modules from Optional Modules
## Executive Summary
This plan addresses the remaining architecture violations where core modules have hard dependencies on optional modules. The goal is to ensure the app can run even if optional modules are removed.
**Current Status:**
- ✅ Dashboard statistics (core → analytics) - FIXED via MetricsProvider pattern
- ❌ Tenancy → marketplace, catalog, billing, analytics - **6 violations**
- ❌ CMS → catalog, billing, messaging - **5 violations (1 misplaced service)**
- ❌ Customers → orders - **2 violations**
---
## Part 1: Tenancy Module Violations
### Violation T1: MarketplaceImportJob in models/__init__.py
**Current Code:**
```python
# tenancy/models/__init__.py:22
from app.modules.marketplace.models.marketplace_import_job import MarketplaceImportJob # noqa: F401
```
**Purpose:** SQLAlchemy relationship resolution for `User.marketplace_import_jobs` and `Vendor.marketplace_import_jobs`
**Solution: Remove relationships from core models**
The relationships `User.marketplace_import_jobs` and `Vendor.marketplace_import_jobs` should be defined ONLY in the MarketplaceImportJob model using `backref`, not on the User/Vendor models. This is a one-way relationship that optional modules add to core models.
**Implementation:**
1. Remove the import from `tenancy/models/__init__.py`
2. Remove `marketplace_import_jobs` relationship from User model
3. Remove `marketplace_import_jobs` relationship from Vendor model
4. Ensure MarketplaceImportJob already has the relationship defined with `backref`
5. Access pattern changes: `user.marketplace_import_jobs` → query `MarketplaceImportJob.filter(user_id=user.id)`
**Impact:** Low - This is internal data access, not a public API
---
### Violation T2: MarketplaceImportJob in admin_service.py
**Current Code:**
```python
# tenancy/services/admin_service.py:36,40
from app.modules.marketplace.models import MarketplaceImportJob
from app.modules.marketplace.schemas import MarketplaceImportJobResponse
```
**Used In:**
- `get_marketplace_import_jobs()` - Admin endpoint for listing import jobs
- `get_recent_import_jobs()` - Dashboard recent imports widget
- `_convert_job_to_response()` - Response conversion
**Solution: Move functionality to marketplace module**
These methods belong in the marketplace module, not tenancy. The admin dashboard should call marketplace service methods.
**Implementation:**
1. Create `app/modules/marketplace/services/import_job_service.py` (if not exists)
2. Move `get_marketplace_import_jobs()` logic to marketplace module
3. Move `get_recent_import_jobs()` logic to marketplace module
4. Update admin_service to use lazy imports with try/except:
```python
def get_recent_import_jobs(self, db: Session, limit: int = 10) -> list:
try:
from app.modules.marketplace.services import import_job_service
return import_job_service.get_recent_jobs(db, limit)
except ImportError:
return [] # Marketplace module not installed
```
5. Update admin dashboard route to handle empty list gracefully
**Impact:** Medium - Admin UI shows "No imports" if marketplace disabled
---
### Violation T3: Catalog/Marketplace in vendor_service.py
**Current Code:**
```python
# tenancy/services/vendor_service.py:18-30
from app.modules.catalog.exceptions import ProductAlreadyExistsException
from app.modules.marketplace.exceptions import MarketplaceProductNotFoundException
from app.modules.marketplace.models import MarketplaceProduct
from app.modules.catalog.models import Product
from app.modules.catalog.schemas import ProductCreate
```
**Used In:**
- `add_product_to_catalog()` - Adds marketplace product to vendor catalog
**Solution: Move product management to catalog module**
Product management is catalog functionality, not tenancy functionality. The `add_product_to_catalog()` method should live in the catalog module.
**Implementation:**
1. Create `app/modules/catalog/services/product_catalog_service.py`
2. Move `add_product_to_catalog()` to catalog module
3. Move helper methods `_get_product_by_id_or_raise()` and `_product_in_catalog()`
4. vendor_service delegates to catalog service with lazy import:
```python
def add_product_to_catalog(self, db: Session, vendor_id: int, product_data: dict):
try:
from app.modules.catalog.services import product_catalog_service
return product_catalog_service.add_product(db, vendor_id, product_data)
except ImportError:
raise ModuleNotEnabledException("catalog")
```
**Impact:** Medium - Feature requires both catalog and marketplace modules
---
### Violation T4: TierLimitExceededException in vendor_team_service.py
**Current Code:**
```python
# tenancy/services/vendor_team_service.py:34
from app.modules.billing.exceptions import TierLimitExceededException
# Line 78 (lazy import inside method)
from app.modules.billing.services import subscription_service
subscription_service.check_team_limit(db, vendor.id)
```
**Used In:**
- `invite_team_member()` - Validates team size against subscription tier
**Solution: Protocol-based limit checking**
Define a `TierLimitChecker` protocol in contracts module. Billing implements it, tenancy uses it optionally.
**Implementation:**
1. Add to `app/modules/contracts/billing.py`:
```python
@runtime_checkable
class TierLimitCheckerProtocol(Protocol):
def check_team_limit(self, db: Session, vendor_id: int) -> None:
"""Raises TierLimitExceededException if limit exceeded."""
...
```
2. Add generic exception to tenancy module:
```python
# tenancy/exceptions.py
class TeamSizeLimitExceededException(WizamartException):
"""Team size limit exceeded (billing module provides specific limits)."""
```
3. Update vendor_team_service.py:
```python
def invite_team_member(self, ...):
# Check tier limits if billing module available
try:
from app.modules.billing.services import subscription_service
subscription_service.check_team_limit(db, vendor.id)
except ImportError:
pass # No billing module - no tier limits
except Exception as e:
# Convert billing exception to tenancy exception
if "limit" in str(e).lower():
raise TeamSizeLimitExceededException(str(e))
raise
```
**Impact:** Low - Without billing, team size is unlimited
---
### Violation T5: Analytics/Marketplace in admin_vendors.py
**Current Code:**
```python
# tenancy/routes/api/admin_vendors.py:20,23
from app.modules.analytics.services.stats_service import stats_service
from app.modules.analytics.schemas import VendorStatsResponse
# Lines 348, 399 (lazy imports)
from app.modules.marketplace.services.letzshop_export_service import letzshop_export_service
```
**Used In:**
- `get_vendor_statistics()` - Returns vendor counts
- `export_vendor_products_letzshop()` - CSV export
- `export_vendor_products_letzshop_to_folder()` - Batch export
**Solution A (Analytics):** Use MetricsProvider pattern (already implemented!)
The stats endpoint should use our new `StatsAggregatorService`:
```python
@router.get("/vendors/stats")
def get_vendor_statistics(db: Session = Depends(get_db), ...):
from app.modules.core.services.stats_aggregator import stats_aggregator
metrics = stats_aggregator.get_admin_dashboard_stats(db, platform_id)
# Extract tenancy metrics
tenancy_metrics = metrics.get("tenancy", [])
return _build_vendor_stats_response(tenancy_metrics)
```
**Solution B (Marketplace Export):** Already uses lazy imports - wrap in try/except
```python
@router.get("/vendors/{vendor_id}/export/letzshop")
def export_vendor_products_letzshop(...):
try:
from app.modules.marketplace.services.letzshop_export_service import letzshop_export_service
return letzshop_export_service.export_vendor_products(...)
except ImportError:
raise ModuleNotEnabledException("marketplace")
```
**Impact:** Low - Features gracefully degrade
---
### Violation T6: Analytics in admin_platform_users.py
**Current Code:**
```python
# tenancy/routes/api/admin_platform_users.py:18
from app.modules.analytics.services.stats_service import stats_service
```
**Used In:**
- `get_user_statistics()` - Returns user counts
**Solution:** Same as T5 - use StatsAggregator
```python
@router.get("/users/stats")
def get_user_statistics(db: Session = Depends(get_db), ...):
from app.modules.core.services.stats_aggregator import stats_aggregator
metrics = stats_aggregator.get_admin_dashboard_stats(db, platform_id)
tenancy_metrics = metrics.get("tenancy", [])
return _build_user_stats_response(tenancy_metrics)
```
**Impact:** None - Already have user metrics in tenancy_metrics
---
## Part 2: CMS Module Violations
### Violation C1: ProductMedia in media.py and media_service.py
**Current Code:**
```python
# cms/models/media.py:77-80
product_associations = relationship("ProductMedia", back_populates="media", ...)
# cms/services/media_service.py:31
from app.modules.catalog.models import ProductMedia
```
**Used In:**
- `attach_to_product()` - Creates product-media association
- `detach_from_product()` - Removes product-media association
- `get_media_usage()` - Lists where media is used
**Solution: Move product-media logic to catalog module**
The CMS media service shouldn't know about products. Product-media associations are catalog concerns.
**Implementation:**
1. Create `app/modules/catalog/services/product_media_service.py`
2. Move `attach_to_product()` and `detach_from_product()` to catalog
3. CMS media_service uses lazy delegation:
```python
def attach_to_product(self, db: Session, media_id: int, product_id: int):
try:
from app.modules.catalog.services import product_media_service
return product_media_service.attach_media(db, media_id, product_id)
except ImportError:
raise ModuleNotEnabledException("catalog")
```
4. For `get_media_usage()`, optionally include product associations:
```python
def get_media_usage(self, db: Session, media_id: int) -> dict:
usage = {"pages": [...], "products": []}
try:
from app.modules.catalog.services import product_media_service
usage["products"] = product_media_service.get_media_products(db, media_id)
except ImportError:
pass # No catalog module
return usage
```
**Impact:** Medium - Product-media features require catalog module
---
### Violation C2: TIER_LIMITS in platform.py (Homepage pricing)
**Current Code:**
```python
# cms/routes/pages/platform.py:17
from app.modules.billing.models import TIER_LIMITS, TierCode
```
**Used In:**
- `_get_tiers_data()` - Builds pricing tier display for homepage
- Called by `homepage()` and `content_page()` handlers
**Solution: Use Context Provider pattern**
Billing module should provide tier data via context provider (already supported in module architecture).
**Implementation:**
1. Add context provider to billing module definition:
```python
# billing/definition.py
def _get_platform_context(request, db, platform) -> dict:
from app.modules.billing.models import TIER_LIMITS, TierCode
tiers = [...] # Build tiers data
return {"tiers": tiers, "has_billing": True}
billing_module = ModuleDefinition(
context_providers={
FrontendType.PLATFORM: _get_platform_context,
},
)
```
2. Remove `_get_tiers_data()` from platform.py
3. Template checks `{% if tiers %}` before showing pricing section
4. Homepage gracefully shows "Contact us for pricing" if billing disabled
**Impact:** Low - Pricing section hidden if billing disabled
---
### Violation C3 & C4: MISPLACED SERVICE - vendor_email_settings_service.py
**Current Code:**
```python
# cms/services/vendor_email_settings_service.py:28-33
from app.modules.messaging.models import VendorEmailSettings, EmailProvider, PREMIUM_EMAIL_PROVIDERS
from app.modules.billing.models import VendorSubscription, TierCode
```
**Critical Finding:** This service is in the WRONG MODULE!
The `vendor_email_settings_service.py` is a **messaging** service that manages email provider configuration. It belongs in the messaging module, not CMS.
**Solution: Move service to messaging module**
**Implementation:**
1. Move `cms/services/vendor_email_settings_service.py` → `messaging/services/vendor_email_settings_service.py`
2. Update all imports that reference it (search codebase)
3. For billing tier checks, use lazy import with graceful fallback:
```python
# messaging/services/vendor_email_settings_service.py
def _check_premium_tier(self, db: Session, vendor_id: int, provider: str) -> bool:
if provider not in PREMIUM_EMAIL_PROVIDERS:
return True # Non-premium providers always allowed
try:
from app.modules.billing.services import subscription_service
tier = subscription_service.get_vendor_tier(db, vendor_id)
return tier in {TierCode.BUSINESS, TierCode.ENTERPRISE}
except ImportError:
return True # No billing module - all providers allowed
```
4. Update `cms/services/__init__.py` to remove the export
5. Add backwards-compatibility alias if needed (deprecation warning)
**Impact:** Medium - Requires import path updates
---
### Violation C5: Dead Code - Vendor import
**Current Code:**
```python
# cms/services/vendor_email_settings_service.py:27
from app.modules.tenancy.models import Vendor # UNUSED
```
**Solution:** Remove the unused import when moving the service.
---
## Part 3: Customers Module Violations
### Violation CU1 & CU2: Order imports in customer_service.py
**Current Code:**
```python
# customers/services/customer_service.py:332-350 (lazy import)
from app.modules.orders.models import Order # in get_customer_orders()
# customers/services/customer_service.py:365-396 (lazy import)
from app.modules.orders.models import Order # in get_customer_statistics()
```
**Used In:**
- `get_customer_orders()` - Lists orders for a customer
- `get_customer_statistics()` - Calculates customer LTV metrics
**Solution: Protocol-based customer order service**
Define a `CustomerOrdersProtocol` in contracts. Orders module implements it.
**Implementation:**
1. Add to `app/modules/contracts/orders.py`:
```python
@runtime_checkable
class CustomerOrdersProtocol(Protocol):
def get_customer_orders(
self, db: Session, vendor_id: int, customer_id: int, skip: int, limit: int
) -> tuple[list, int]:
...
def get_customer_statistics(
self, db: Session, vendor_id: int, customer_id: int
) -> dict:
...
```
2. Create `app/modules/orders/services/customer_orders_service.py`:
```python
class CustomerOrdersService:
def get_customer_orders(self, db, vendor_id, customer_id, skip, limit):
from app.modules.orders.models import Order
# ... existing logic
def get_customer_statistics(self, db, vendor_id, customer_id):
from app.modules.orders.models import Order
# ... existing logic
customer_orders_service = CustomerOrdersService()
```
3. Update customer_service.py to use lazy service discovery:
```python
def get_customer_orders(self, db, vendor_id, customer_id, skip=0, limit=50):
try:
from app.modules.orders.services import customer_orders_service
return customer_orders_service.get_customer_orders(db, vendor_id, customer_id, skip, limit)
except ImportError:
return [], 0 # No orders module
def get_customer_statistics(self, db, vendor_id, customer_id):
customer = self.get_customer(db, vendor_id, customer_id)
stats = {
"customer_id": customer_id,
"member_since": customer.created_at,
"is_active": customer.is_active,
"total_orders": 0,
"total_spent": 0.0,
"average_order_value": 0.0,
"last_order_date": None,
}
try:
from app.modules.orders.services import customer_orders_service
order_stats = customer_orders_service.get_customer_statistics(db, vendor_id, customer_id)
stats.update(order_stats)
except ImportError:
pass # No orders module - return base stats
return stats
```
**Impact:** Low - Customer details show "No orders" if orders disabled
---
## Implementation Phases
### Phase 1: Quick Wins (Low Risk)
1. T6: Update admin_platform_users.py to use StatsAggregator
2. T5a: Update admin_vendors.py stats endpoint to use StatsAggregator
3. C5: Remove dead Vendor import
4. T4: Add try/except for billing tier check
### Phase 2: Service Relocation (Medium Risk)
1. C3/C4: Move vendor_email_settings_service to messaging module
2. T2: Move import job methods to marketplace module
3. C1: Move product-media logic to catalog module
### Phase 3: Model Relationship Cleanup (Medium Risk)
1. T1: Remove MarketplaceImportJob relationship from User/Vendor models
2. T3: Move product catalog methods to catalog module
### Phase 4: Protocol-Based Decoupling (Low Risk)
1. CU1/CU2: Create customer orders service in orders module
2. C2: Add billing context provider for pricing tiers
### Phase 5: Testing & Verification
1. Run full test suite with all modules enabled
2. Test with each optional module disabled individually
3. Update architecture validation script
4. Update documentation
---
## Summary Table
| ID | Module | Violation | Solution | Risk | Phase |
|----|--------|-----------|----------|------|-------|
| T1 | tenancy | MarketplaceImportJob in models | Remove relationship from core | Medium | 3 |
| T2 | tenancy | ImportJob in admin_service | Move to marketplace | Medium | 2 |
| T3 | tenancy | Products in vendor_service | Move to catalog | Medium | 3 |
| T4 | tenancy | TierLimit in team_service | Try/except wrapper | Low | 1 |
| T5a | tenancy | Stats in admin_vendors | Use StatsAggregator | Low | 1 |
| T5b | tenancy | Export in admin_vendors | Already lazy - add try/except | Low | 1 |
| T6 | tenancy | Stats in admin_users | Use StatsAggregator | Low | 1 |
| C1 | cms | ProductMedia in media | Move to catalog | Medium | 2 |
| C2 | cms | TIER_LIMITS in platform | Context provider | Low | 4 |
| C3/C4 | cms | MISPLACED email service | Move to messaging | Medium | 2 |
| C5 | cms | Dead Vendor import | Remove | None | 1 |
| CU1/CU2 | customers | Order in customer_service | Protocol + lazy | Low | 4 |
---
## Verification Commands
After implementation, run these to verify:
```bash
# Check no core→optional imports remain
grep -r "from app.modules.analytics" app/modules/core/ app/modules/tenancy/ app/modules/cms/ app/modules/customers/ 2>/dev/null | grep -v "# optional" || echo "Clean!"
grep -r "from app.modules.marketplace" app/modules/core/ app/modules/tenancy/ app/modules/cms/ app/modules/customers/ 2>/dev/null | grep -v "# optional" || echo "Clean!"
grep -r "from app.modules.billing" app/modules/core/ app/modules/tenancy/ app/modules/cms/ app/modules/customers/ 2>/dev/null | grep -v "# optional" || echo "Clean!"
grep -r "from app.modules.catalog" app/modules/core/ app/modules/tenancy/ app/modules/cms/ app/modules/customers/ 2>/dev/null | grep -v "# optional" || echo "Clean!"
grep -r "from app.modules.orders" app/modules/core/ app/modules/tenancy/ app/modules/cms/ app/modules/customers/ 2>/dev/null | grep -v "# optional" || echo "Clean!"
# Run tests with each optional module "disabled" (move temporarily)
# This would be a more advanced test
# Run architecture validator
python scripts/validate_architecture.py
```
---
## Expected Outcome
After all phases complete:
- ✅ App starts successfully even if optional modules are removed
- ✅ Core modules have NO hard imports from optional modules
- ✅ Features gracefully degrade when optional modules disabled
- ✅ All cross-module communication via protocols or lazy imports with try/except