Some checks failed
- Add admin store roles page with merchant→store cascading for superadmin and store-only selection for platform admin - Add permission catalog API with translated labels/descriptions (en/fr/de/lb) - Add permission translations to all 15 module locale files (60 files total) - Add info icon tooltips for permission descriptions in role editor - Add store roles menu item and admin menu item in module definition - Fix store-selector.js URL construction bug when apiEndpoint has query params - Add admin store roles API (CRUD + platform scoping) - Add integration tests for admin store roles and permission catalog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
242 lines
8.4 KiB
Markdown
242 lines
8.4 KiB
Markdown
# Architecture Violations Status
|
|
|
|
**Date:** 2026-01-08
|
|
**Total Violations:** 0 blocking (221 documented/accepted)
|
|
**Status:** ✅ All architecture validation errors resolved
|
|
|
|
## Summary
|
|
|
|
Fixed 18 violations and documented remaining violations as intentional architectural decisions.
|
|
|
|
## Fixed Violations (18)
|
|
|
|
### JavaScript Centralized Logging
|
|
- ✅ `static/admin/js/marketplace.js` - Replaced 18 console.log calls with adminMarketplaceLog
|
|
- ✅ `static/admin/js/store-themes.js` - Replaced 5 console.log calls with storeThemesLog
|
|
- ✅ `static/admin/js/settings.js` - Replaced 1 console.log call with settingsLog
|
|
- ✅ `static/admin/js/imports.js` - Replaced 13 console.log calls with importsLog
|
|
|
|
## Remaining Violations (221)
|
|
|
|
### Category 1: Transaction Control at API Layer (Intentional)
|
|
|
|
**Violation:** API-002 - Database commits in endpoints
|
|
|
|
**Files Affected:**
|
|
- `app/api/v1/admin/merchants.py` (5 occurrences)
|
|
- `app/api/v1/admin/stores.py` (2 occurrences)
|
|
- Other admin endpoints
|
|
|
|
**Architectural Decision:**
|
|
|
|
This is an **intentional and standard pattern** in FastAPI applications:
|
|
|
|
```python
|
|
# Service Layer - Business Logic
|
|
def update_merchant(self, db: Session, merchant_id: int, data: MerchantUpdate):
|
|
merchant = self.get_merchant_by_id(db, merchant_id)
|
|
# ... business logic ...
|
|
db.flush() # Flush to get IDs, but don't commit
|
|
return merchant
|
|
|
|
# API Layer - Transaction Boundary
|
|
@router.put("/merchants/{merchant_id}")
|
|
async def update_merchant_endpoint(merchant_id: int, data: MerchantUpdate, db: Session = Depends(get_db)):
|
|
merchant = merchant_service.update_merchant(db, merchant_id, data)
|
|
db.commit() # ✅ ARCH: Commit at API level for transaction control
|
|
return merchant
|
|
```
|
|
|
|
**Benefits:**
|
|
1. Service methods can call other service methods within same transaction
|
|
2. API layer can rollback entire transaction on error
|
|
3. Allows for complex multi-step operations
|
|
4. Standard practice recommended by FastAPI documentation
|
|
|
|
**Status:** ✅ **ACCEPTED** - Added comments to document intention
|
|
|
|
### Category 2: Raw Dict Responses (Legacy - Low Priority)
|
|
|
|
**Violation:** API-001 - Endpoint returns raw dict instead of Pydantic model
|
|
|
|
**Files Affected:**
|
|
- `app/api/v1/admin/users.py`
|
|
- `app/api/v1/admin/auth.py`
|
|
- `app/api/v1/admin/store_themes.py`
|
|
- `app/api/v1/admin/logs.py`
|
|
- `app/api/v1/admin/notifications.py`
|
|
- Various other endpoints
|
|
|
|
**Reason:** These are legacy endpoints created before Pydantic response models were standardized.
|
|
|
|
**Impact:** Low - endpoints work correctly, just missing type validation/documentation
|
|
|
|
**Refactoring Plan:**
|
|
- ⏰ Low priority - will refactor incrementally
|
|
- 📝 Create Pydantic response models
|
|
- 🔄 Update endpoints to use response_model parameter
|
|
|
|
**Status:** 📝 **DOCUMENTED** - Will fix in future refactoring sprint
|
|
|
|
### Category 3: Service Layer Patterns (Legacy - Medium Priority)
|
|
|
|
**Violations:**
|
|
- SVC-001: Services creating instances of other services
|
|
- SVC-002: Services with unused methods
|
|
- SVC-003: Services storing db session
|
|
|
|
**Reason:** Pre-existing code before architecture rules were established
|
|
|
|
**Refactoring Plan:**
|
|
- ⏰ Medium priority
|
|
- 🔄 Gradual refactoring as we touch each service
|
|
- 📝 Follow dependency injection pattern for new code
|
|
|
|
**Status:** 📝 **DOCUMENTED** - Will fix incrementally
|
|
|
|
### Category 4: Database Queries in Endpoints (Case-by-Case)
|
|
|
|
**Violation:** API-002 - Database queries should be in service layer
|
|
|
|
**Files:**
|
|
- `app/api/v1/admin/stores.py:63`
|
|
- `app/api/v1/admin/store_domains.py:51`
|
|
- `app/api/v1/admin/content_pages.py:188`
|
|
|
|
**Reason:** Simple read queries that don't justify service layer complexity
|
|
|
|
**Decision:**
|
|
- Some simple lookups can stay in endpoints
|
|
- Complex business logic should move to services
|
|
|
|
**Status:** 📝 **REVIEW CASE-BY-CASE**
|
|
|
|
### Category 5: Template Warnings (Low Priority)
|
|
|
|
**Violation:** TMPL-001 - Templates using inline CSS/JS
|
|
|
|
**Reason:** Development speed vs perfect separation
|
|
|
|
**Impact:** Low - doesn't affect functionality
|
|
|
|
**Status:** 📝 **ACCEPTED** - Inline styles OK for admin pages
|
|
|
|
### Category 6: Cross-Module Model Imports (HIGH Priority)
|
|
|
|
**Violation:** MOD-025 - Modules importing and querying models from other modules
|
|
|
|
**Date Added:** 2026-02-26
|
|
|
|
**Total Violations:** ~84 (services and route files, excluding tests and type-hints)
|
|
|
|
**Subcategories:**
|
|
|
|
| Cat | Description | Count | Priority |
|
|
|-----|-------------|-------|----------|
|
|
| 1 | Direct queries on another module's models | ~47 | URGENT |
|
|
| 2 | Creating instances of another module's models | ~15 | URGENT |
|
|
| 3 | Aggregation/count queries across module boundaries | ~11 | URGENT |
|
|
| 4 | Join queries involving another module's models | ~4 | URGENT |
|
|
| 5 | UserContext legacy import path (74 files) | 74 | URGENT |
|
|
|
|
**Top Violating Module Pairs:**
|
|
- `billing → tenancy`: 31 violations
|
|
- `loyalty → tenancy`: 23 violations
|
|
- `marketplace → tenancy`: 18 violations
|
|
- `core → tenancy`: 11 violations
|
|
- `cms → tenancy`: 8 violations
|
|
- `analytics → tenancy/catalog/orders`: 8 violations
|
|
- `inventory → catalog`: 3 violations
|
|
- `marketplace → catalog/orders`: 5 violations
|
|
|
|
**Resolution:** Migrate all cross-module model imports to service calls. See [Cross-Module Migration Plan](cross-module-migration-plan.md).
|
|
|
|
**Status:** :construction: **IN PROGRESS** - Migration plan created, executing per-module
|
|
|
|
### Category 7: Provider Pattern Gaps (MEDIUM Priority — Incremental)
|
|
|
|
**Violation:** Modules with data that should be exposed via providers but aren't
|
|
|
|
**Date Added:** 2026-02-26
|
|
|
|
| Provider | Implementing | Should Add |
|
|
|----------|-------------|------------|
|
|
| MetricsProvider | 8 modules | loyalty, payments, analytics |
|
|
| WidgetProvider | 2 modules (marketplace, tenancy) | orders, billing, catalog, inventory, loyalty |
|
|
| AuditProvider | 1 module (monitoring) | OK — single backend is the design |
|
|
|
|
**Status:** :memo: **PLANNED** - Will add incrementally as we work on each module
|
|
|
|
## Architecture Validation Philosophy
|
|
|
|
### What We Enforce Strictly:
|
|
1. ✅ Centralized logging (no console.log)
|
|
2. ✅ Custom exceptions with proper status codes
|
|
3. ✅ Pydantic models for new endpoints
|
|
4. ✅ Service layer for complex business logic
|
|
|
|
### What We Accept:
|
|
1. ✅ Transaction control at API layer (documented)
|
|
2. ✅ Simple queries in endpoints (case-by-case)
|
|
3. ✅ Legacy code patterns (document and refactor incrementally)
|
|
|
|
### What We're Improving:
|
|
1. 🔄 Gradually adding Pydantic response models
|
|
2. 🔄 Refactoring services to use dependency injection
|
|
3. 🔄 Moving complex queries to service layer
|
|
|
|
## Progress Tracking
|
|
|
|
| Category | Count | Priority | Status |
|
|
|----------|-------|----------|--------|
|
|
| JavaScript console.log | 0 | High | ✅ Fixed |
|
|
| Transaction control (intentional) | ~10 | N/A | ✅ Documented |
|
|
| Raw dict responses | ~40 | Low | 📝 Backlog |
|
|
| Service patterns | ~50 | Medium | 📝 Incremental |
|
|
| Simple queries in endpoints | ~10 | Low | 📝 Case-by-case |
|
|
| Template inline styles | ~110 | Low | ✅ Accepted |
|
|
| **Cross-module model imports** | **~84** | **High** | **🔄 Migrating** |
|
|
| **UserContext legacy path** | **74** | **High** | **🔄 Migrating** |
|
|
| **Provider pattern gaps** | **~8** | **Medium** | **📝 Incremental** |
|
|
|
|
## Validation Command
|
|
|
|
```bash
|
|
python scripts/validate/validate_architecture.py
|
|
```
|
|
|
|
## Next Actions
|
|
|
|
### Immediate (Done)
|
|
- [x] Fix JavaScript console.log violations
|
|
- [x] Document transaction control pattern
|
|
- [x] Add comments to intentional violations
|
|
|
|
### Short Term (Next Sprint)
|
|
- [ ] Move UserContext to tenancy.schemas, update 74 imports (Cat 5)
|
|
- [ ] Add missing service methods to tenancy for cross-module consumers (Cat 1)
|
|
- [ ] Migrate direct model queries to service calls (Cat 1-4)
|
|
- [ ] Create Pydantic response models for top 10 endpoints
|
|
|
|
### Medium Term
|
|
- [ ] Add widget providers to orders, billing, catalog, inventory, loyalty (P5)
|
|
- [ ] Add metrics providers to loyalty, payments (P5)
|
|
- [ ] Refactor 2-3 services to use dependency injection
|
|
- [ ] Move complex queries to service layer
|
|
|
|
### Long Term (Continuous)
|
|
- [ ] Gradually refactor legacy endpoints
|
|
- [ ] Standardize all services
|
|
- [ ] Achieve <50 total violations
|
|
|
|
## Conclusion
|
|
|
|
**Current State:** 221 violations
|
|
- 18 fixed
|
|
- ~120 acceptable (documented reasons)
|
|
- ~80 legacy code (low priority refactoring)
|
|
|
|
**Philosophy:** Enforce strict standards for new code, document and incrementally improve legacy code.
|
|
|
|
**Result:** ✅ Codebase is in good architectural health with clear improvement path.
|