diff --git a/.architecture-rules.yaml b/.architecture-rules.yaml deleted file mode 100644 index 71cb16e5..00000000 --- a/.architecture-rules.yaml +++ /dev/null @@ -1,1487 +0,0 @@ -# Architecture Rules Configuration -# This file defines the key architectural decisions and patterns that must be followed -# across the application. The validator script uses these rules to check compliance. - -version: "2.0" -project: "letzshop-product-import" -description: "Comprehensive architectural rules for multi-tenant e-commerce platform" - -# ============================================================================ -# CORE ARCHITECTURAL PRINCIPLES -# ============================================================================ - -principles: - - name: "Separation of Concerns" - description: "API endpoints should only handle HTTP concerns. Business logic belongs in services." - - - name: "Layered Architecture" - description: "Routes → Services → Models. Each layer has specific responsibilities." - - - name: "Type Safety" - description: "Use Pydantic models for request/response validation. Use SQLAlchemy models for database." - - - name: "Proper Exception Handling" - description: "Services throw domain exceptions. Global handler converts to HTTP responses. Endpoints do NOT catch or raise HTTPException." - - - name: "Multi-Tenancy" - description: "All queries must be scoped to vendor_id. No cross-vendor data access." - - - name: "Consistent Naming" - description: "API files: plural, Services: singular+service, Models: singular" - -# ============================================================================ -# API ENDPOINT RULES (app/api/v1/**/*.py) -# ============================================================================ - -api_endpoint_rules: - - - id: "API-001" - name: "Endpoint must use Pydantic models for request/response" - severity: "error" - description: | - All API endpoints must use Pydantic models (BaseModel) for request bodies - and response models. Never use raw dicts or SQLAlchemy models directly. - - WHY THIS MATTERS: - - Type safety: Pydantic validates response structure at runtime - - Documentation: OpenAPI/Swagger auto-generates accurate docs - - Contract stability: Schema changes are explicit and reviewable - - IDE support: Consumers get autocomplete and type hints - - Prevents bugs: Field name mismatches caught immediately - - COMMON VIOLATION: Service returns dict, frontend expects different field names. - Example: Service returns {"total_imports": 5} but frontend expects {"total": 5}. - With response_model, this mismatch is caught immediately. - - SCHEMA LOCATION: All response schemas must be defined in models/schema/*.py, - never inline in endpoint files. This ensures schemas are reusable and discoverable. - pattern: - file_pattern: "app/api/v1/**/*.py" - anti_patterns: - - "return dict" - - "-> dict" - - "return db_object" - - "return {" # Returning inline dict literal - example_good: | - # In models/schema/stats.py - class ImportStatsResponse(BaseModel): - total: int - pending: int - completed: int - failed: int - - # In app/api/v1/admin/marketplace.py - @router.get("/stats", response_model=ImportStatsResponse) - def get_import_statistics(db: Session = Depends(get_db)): - return stats_service.get_import_statistics(db) - example_bad: | - # ❌ WRONG: No response_model, returns raw dict - @router.get("/stats") - def get_import_statistics(db: Session = Depends(get_db)): - return stats_service.get_import_statistics(db) # Returns dict - - # ❌ WRONG: Schema defined inline in endpoint file - class MyResponse(BaseModel): # Should be in models/schema/ - ... - - @router.get("/data", response_model=MyResponse) - def get_data(): ... - - - id: "API-002" - name: "Endpoint must NOT contain business logic" - severity: "error" - description: | - API endpoints should only handle HTTP concerns (validation, auth, response formatting). - All business logic must be delegated to service layer. - - Transaction control (db.commit) IS allowed at endpoint level - this is the recommended - pattern for request-scoped transactions. One request = one transaction. - - What's NOT allowed: - - db.add() - creating entities is business logic - - db.query() - complex queries are business logic - - db.delete() - deleting entities is business logic - pattern: - file_pattern: "app/api/v1/**/*.py" - anti_patterns: - - "db.add(" - - "db.delete(" - - "db.query(" - # NOTE: db.commit() is intentionally NOT listed - it's allowed for transaction control - - - id: "API-003" - name: "Endpoint must NOT raise ANY exceptions directly" - severity: "error" - description: | - API endpoints should NOT raise exceptions directly. Endpoints are a thin - orchestration layer that: - 1. Accepts request parameters (validated by Pydantic) - 2. Calls dependencies for authentication/authorization (deps.py raises exceptions) - 3. Calls services for business logic (services raise domain exceptions) - 4. Returns response (formatted by Pydantic) - - Exception raising belongs in: - - Dependencies (app/api/deps.py) - authentication/authorization validation - - Services (app/services/) - business logic validation - - The global exception handler catches all WizamartException subclasses and - converts them to appropriate HTTP responses. - - WRONG (endpoint raises exception): - @router.get("/orders") - def get_orders(current_user: User = Depends(get_current_vendor_api)): - if not hasattr(current_user, "token_vendor_id"): # ❌ Redundant check - raise InvalidTokenException("...") # ❌ Don't raise here - return order_service.get_orders(db, current_user.token_vendor_id) - - RIGHT (dependency guarantees, endpoint trusts): - @router.get("/orders") - def get_orders(current_user: User = Depends(get_current_vendor_api)): - # Dependency guarantees token_vendor_id is present - return order_service.get_orders(db, current_user.token_vendor_id) - pattern: - file_pattern: "app/api/v1/**/*.py" - anti_patterns: - - "raise HTTPException" - - "raise InvalidTokenException" - - "raise InsufficientPermissionsException" - - "if not hasattr\\(current_user.*token_vendor" - exceptions: - - "app/exceptions/handler.py" # Handler is allowed to use HTTPException - - - id: "API-004" - name: "Endpoint must have proper authentication/authorization" - severity: "warning" - description: | - Protected endpoints must use Depends() for authentication. - Use get_current_user, get_current_admin, etc. - - Auto-excluded files: - - */auth.py - Authentication endpoints (login, logout, register) are intentionally public - - Public endpoint markers (place on line before or after decorator): - - # public - Descriptive marker for intentionally unauthenticated endpoints - - # noqa: API-004 - Standard noqa style to suppress warning - - Example: - # public - Stripe webhook receives external callbacks - @router.post("/webhook/stripe") - def stripe_webhook(request: Request): - ... - pattern: - file_pattern: "app/api/v1/**/*.py" - required_if_not_public: - - "Depends(get_current_" - auto_exclude_files: - - "*/auth.py" - public_markers: - - "# public" - - "# noqa: api-004" - - - id: "API-005" - name: "Multi-tenant endpoints must scope queries to vendor_id" - severity: "error" - description: | - All queries in vendor/shop contexts must filter by vendor_id. - Use request.state.vendor_id from middleware. - pattern: - file_pattern: "app/api/v1/vendor/**/*.py" - file_pattern: "app/api/v1/shop/**/*.py" - discouraged_patterns: - - "db.query(.*).all()" # Without vendor filter - -# ============================================================================ -# SERVICE LAYER RULES (app/services/**/*.py) -# ============================================================================ - -service_layer_rules: - - - id: "SVC-001" - name: "Service must NOT raise HTTPException" - severity: "error" - description: | - Services are business logic layer - they should NOT know about HTTP. - Raise domain-specific exceptions instead (ValueError, custom exceptions). - pattern: - file_pattern: "app/services/**/*.py" - anti_patterns: - - "raise HTTPException" - - "from fastapi import HTTPException" - - - id: "SVC-002" - name: "Service must use proper exception handling" - severity: "error" - description: | - Services should raise meaningful domain exceptions, not generic Exception. - Create custom exception classes for business rule violations. - pattern: - file_pattern: "app/services/**/*.py" - discouraged_patterns: - - "raise Exception\\(" - - - id: "SVC-003" - name: "Service methods must accept db session as parameter" - severity: "error" - description: | - Service methods should receive database session as a parameter for testability - and transaction control. Never create session inside service. - pattern: - file_pattern: "app/services/**/*.py" - required_in_method_signature: - - "db: Session" - anti_patterns: - - "SessionLocal()" - - "get_db()" - - - id: "SVC-004" - name: "Service must use Pydantic models for input validation" - severity: "warning" - description: | - Service methods should accept Pydantic models for complex inputs - to ensure type safety and validation. - pattern: - file_pattern: "app/services/**/*.py" - encouraged_patterns: - - "BaseModel" - - - id: "SVC-005" - name: "Service must scope queries to vendor_id in multi-tenant contexts" - severity: "error" - description: | - All database queries must be scoped to vendor_id to prevent cross-tenant data access. - pattern: - file_pattern: "app/services/**/*.py" - check: "vendor_scoping" - - - id: "SVC-006" - name: "Service must NOT call db.commit()" - severity: "warning" - description: | - Services should NOT commit transactions. Transaction control belongs at the - API endpoint level where one request = one transaction. - - This allows: - - Composing multiple service calls in a single transaction - - Clean rollback on any failure - - Easier testing of services in isolation - - The endpoint should call db.commit() after all service operations succeed. - pattern: - file_pattern: "app/services/**/*.py" - anti_patterns: - - "db.commit()" - exceptions: - - "log_service.py" # Audit logs may need immediate commits - - - id: "SVC-007" - name: "Service return types must match API response schemas" - severity: "error" - description: | - When a service method's return value will be used as an API response, - the returned dict keys MUST match the corresponding Pydantic schema fields. - - This prevents the common bug where: - - Service returns {"total_imports": 5, "completed_imports": 3} - - Schema expects {"total": 5, "completed": 3} - - Frontend receives wrong/empty values - - RECOMMENDED PATTERNS: - - 1. Return Pydantic model directly from service: - def get_stats(self, db: Session) -> StatsResponse: - return StatsResponse(total=count, completed=done) - - 2. Return dict with schema-matching keys: - def get_stats(self, db: Session) -> dict: - return {"total": count, "completed": done} # Matches StatsResponse - - 3. Document the expected schema in service docstring: - def get_stats(self, db: Session) -> dict: - \"\"\" - Returns dict compatible with StatsResponse schema. - Keys: total, pending, completed, failed - \"\"\" - - TESTING: Write tests that validate service output against schema: - result = service.get_stats(db) - StatsResponse(**result) # Raises if keys don't match - pattern: - file_pattern: "app/services/**/*.py" - check: "schema_compatibility" - -# ============================================================================ -# MODEL RULES (models/database/*.py, models/schema/*.py) -# ============================================================================ - -model_rules: - - - id: "MDL-001" - name: "Database models must use SQLAlchemy Base" - severity: "error" - description: | - All database models must inherit from SQLAlchemy Base and use proper - column definitions with types and constraints. - pattern: - file_pattern: "models/database/**/*.py" - required_patterns: - - "class.*\\(Base\\):" - - - id: "MDL-002" - name: "Use Pydantic models separately from SQLAlchemy models" - severity: "error" - description: | - Never mix SQLAlchemy and Pydantic in the same model. - SQLAlchemy = database schema, Pydantic = API validation/serialization. - pattern: - file_pattern: "models/**/*.py" - anti_patterns: - - "class.*\\(Base, BaseModel\\):" - - - id: "MDL-003" - name: "Pydantic models must use from_attributes for ORM mode" - severity: "error" - description: | - Pydantic response models must enable from_attributes to work with SQLAlchemy models. - pattern: - file_pattern: "models/schema/**/*.py" - required_in_response_models: - - "from_attributes = True" - - - id: "MDL-004" - name: "Database tables use plural names" - severity: "warning" - description: | - Database table names should be plural lowercase following industry standard - conventions (Rails, Django, Laravel, most ORMs). A table represents a - collection of entities, so plural names are natural: 'users', 'orders', - 'products'. This reads naturally in SQL: SELECT * FROM users WHERE id = 1. - - Examples: - - Good: users, vendors, products, orders, order_items, cart_items - - Bad: user, vendor, product, order - - Junction/join tables use both entity names in plural: - - Good: vendor_users, order_items, product_translations - pattern: - file_pattern: "models/database/**/*.py" - check: "table_naming_plural" - -# ============================================================================ -# EXCEPTION HANDLING RULES -# ============================================================================ - -exception_rules: - - - id: "EXC-001" - name: "Define custom exceptions in exceptions module" - severity: "warning" - description: | - Create domain-specific exceptions in app/exceptions/ for better - error handling and clarity. - pattern: - file_pattern: "app/exceptions/**/*.py" - encouraged_structure: | - class VendorError(Exception): - """Base exception for vendor-related errors""" - pass - - - id: "EXC-002" - name: "Never use bare except" - severity: "error" - description: | - Always specify exception types. Bare except catches everything including - KeyboardInterrupt and SystemExit. - pattern: - file_pattern: "**/*.py" - anti_patterns: - - "except:" - - "except\\s*:" - - - id: "EXC-003" - name: "Log all exceptions with context" - severity: "warning" - description: | - When catching exceptions, log them with context and stack trace. - pattern: - file_pattern: "app/services/**/*.py" - encouraged_patterns: - - "logger.error" - - "exc_info=True" - - - id: "EXC-004" - name: "Domain exceptions must inherit from WizamartException" - severity: "error" - description: | - All custom domain exceptions must inherit from WizamartException (or its - subclasses like ResourceNotFoundException, ValidationException, etc.). - This ensures the global exception handler catches and converts them properly. - pattern: - file_pattern: "app/exceptions/**/*.py" - required_base_class: "WizamartException" - example_good: | - class VendorNotFoundException(ResourceNotFoundException): - def __init__(self, vendor_code: str): - super().__init__(resource_type="Vendor", identifier=vendor_code) - - - id: "EXC-005" - name: "Exception handler must be registered" - severity: "error" - description: | - The global exception handler must be set up in app initialization to - catch WizamartException and convert to HTTP responses. - pattern: - file_pattern: "app/main.py" - required_patterns: - - "setup_exception_handlers" - -# ============================================================================ -# NAMING CONVENTION RULES -# ============================================================================ - -naming_rules: - - - id: "NAM-001" - name: "API files use PLURAL names" - severity: "error" - description: | - API endpoint files should use plural names (vendors.py, products.py) - pattern: - file_pattern: "app/api/v1/**/*.py" - check: "plural_naming" - exceptions: - - "__init__.py" - - "auth.py" - - "health.py" - - - id: "NAM-002" - name: "Service files use SINGULAR + 'service' suffix" - severity: "error" - description: | - Service files should use singular name + _service (vendor_service.py) - pattern: - file_pattern: "app/services/**/*.py" - check: "service_naming" - - - id: "NAM-003" - name: "Model files use SINGULAR names" - severity: "error" - description: | - Both database and schema model files use singular names (product.py) - pattern: - file_pattern: "models/**/*.py" - check: "singular_naming" - - - id: "NAM-004" - name: "Use consistent terminology: vendor not shop" - severity: "warning" - description: | - Use 'vendor' consistently, not 'shop' (except for shop frontend) - pattern: - file_pattern: "app/**/*.py" - discouraged_terms: - - "shop_id" # Use vendor_id - - "shop_service" # Use vendor_service - - - id: "NAM-005" - name: "Use consistent terminology: inventory not stock" - severity: "warning" - description: | - Use 'inventory' consistently, not 'stock' - pattern: - file_pattern: "app/**/*.py" - discouraged_terms: - - "stock_service" # Use inventory_service - -# ============================================================================ -# JAVASCRIPT ARCHITECTURE RULES (Frontend) -# ============================================================================ - -javascript_rules: - - - id: "JS-001" - name: "Use centralized logger, not console" - severity: "error" - description: | - Use window.LogConfig.createLogger() for consistent logging. - Never use console.log, console.error, console.warn directly. - pattern: - file_pattern: "static/**/js/**/*.js" - anti_patterns: - - "console\\.log" - - "console\\.error" - - "console\\.warn" - exceptions: - - "// eslint-disable" - - "console.log('✅" # Bootstrap messages allowed - auto_exclude_files: - - "init-*.js" # Init files run before logger is available - - - id: "JS-002" - name: "Use lowercase apiClient for API calls" - severity: "error" - description: | - Use lowercase 'apiClient' consistently, not 'ApiClient' or 'API_CLIENT' - pattern: - file_pattern: "static/**/js/**/*.js" - anti_patterns: - - "ApiClient\\." - - "API_CLIENT\\." - required_pattern: "apiClient\\." - - - id: "JS-003" - name: "Alpine components must spread ...data()" - severity: "error" - description: | - All Alpine.js components must inherit base layout data using spread operator - pattern: - file_pattern: "static/**/js/**/*.js" - required_in_alpine_components: - - "\\.\\.\\.data\\(\\)" - - - id: "JS-004" - name: "Alpine components must set currentPage" - severity: "error" - description: | - All Alpine.js page components must set a currentPage identifier - pattern: - file_pattern: "static/**/js/**/*.js" - required_in_alpine_components: - - "currentPage:" - - - id: "JS-005" - name: "Initialization methods must include guard" - severity: "error" - description: | - Init methods should prevent duplicate initialization with guard - pattern: - file_pattern: "static/**/js/**/*.js" - recommended_pattern: | - if (window._pageInitialized) return; - window._pageInitialized = true; - - - id: "JS-006" - name: "All async operations must have try/catch with error logging" - severity: "error" - description: | - All API calls and async operations must have error handling - pattern: - file_pattern: "static/**/js/**/*.js" - check: "async_error_handling" - - - id: "JS-007" - name: "Set loading state before async operations" - severity: "warning" - description: | - Loading state should be set before and cleared after async operations - pattern: - file_pattern: "static/**/js/**/*.js" - recommended_pattern: | - loading = true; - try { - // operation - } finally { - loading = false; - } - - - id: "JS-008" - name: "Use apiClient for API calls, not raw fetch()" - severity: "error" - description: | - All API calls must use the apiClient helper instead of raw fetch(). - The apiClient automatically: - - Adds Authorization header with JWT token from cookies - - Sets Content-Type headers - - Handles error responses consistently - - Provides logging integration - - WRONG (raw fetch): - const response = await fetch('/api/v1/admin/products/123'); - - RIGHT (apiClient): - const response = await apiClient.get('/admin/products/123'); - const result = await apiClient.post('/admin/products', data); - await apiClient.delete('/admin/products/123'); - pattern: - file_pattern: "static/**/js/**/*.js" - anti_patterns: - - "fetch\\('/api/" - - 'fetch\\("/api/' - - "fetch\\(`/api/" - exceptions: - - "init-api-client.js" # The apiClient implementation itself - - - id: "JS-009" - name: "Use Utils.showToast() for notifications, not alert() or window.showToast" - severity: "error" - description: | - All user notifications must use Utils.showToast() from static/shared/js/utils.js. - Never use browser alert() dialogs or undefined window.showToast. - - Utils.showToast() provides: - - Consistent styling (Tailwind-based toast in bottom-right corner) - - Automatic fade-out after duration - - Color-coded types (success=green, error=red, warning=yellow, info=blue) - - WRONG (browser dialog): - alert('Product saved successfully'); - alert(errorMessage); - - WRONG (undefined function): - window.showToast('Success', 'success'); - if (window.showToast) { window.showToast(...); } else { alert(...); } - - RIGHT (Utils helper): - Utils.showToast('Product saved successfully', 'success'); - Utils.showToast('Failed to save product', 'error'); - Utils.showToast('Please fill all required fields', 'warning'); - pattern: - file_pattern: "static/**/js/**/*.js" - anti_patterns: - - "alert\\(" - - "window\\.showToast" - exceptions: - - "utils.js" # The Utils implementation itself - -# ============================================================================ -# TEMPLATE RULES (Jinja2) -# ============================================================================ - -template_rules: - - - id: "TPL-001" - name: "Admin templates must extend admin/base.html" - severity: "error" - description: | - All admin templates must extend the base template for consistency. - - Auto-excluded files: - - login.html - Standalone login page (no sidebar/navigation) - - errors/*.html - Error pages extend errors/base.html instead - - test-*.html - Test/development templates - - Standalone template markers (place in first 5 lines): - - {# standalone #} - Mark template as intentionally standalone - - {# noqa: TPL-001 #} - Standard noqa style to suppress error - - - HTML comment style - pattern: - file_pattern: "app/templates/admin/**/*.html" - required_patterns: - - "{% extends ['\"]admin/base\\.html['\"] %}" - auto_exclude_files: - - "login.html" - - "errors/" - - "test-" - standalone_markers: - - "{# standalone #}" - - "{# noqa: tpl-001 #}" - - "" - exceptions: - - "base.html" - - "partials/" - - - id: "TPL-002" - name: "Vendor templates must extend vendor/base.html" - severity: "error" - description: "All vendor templates must extend the base template" - pattern: - file_pattern: "app/templates/vendor/**/*.html" - required_patterns: - - "{% extends ['\"]vendor/base\\.html['\"] %}" - exceptions: - - "base.html" - - "partials/" - - - id: "TPL-003" - name: "Shop templates must extend shop/base.html" - severity: "error" - description: "All shop templates must extend the base template" - pattern: - file_pattern: "app/templates/shop/**/*.html" - required_patterns: - - "{% extends ['\"]shop/base\\.html['\"] %}" - exceptions: - - "base.html" - - "partials/" - - - id: "TPL-004" - name: "Use x-text for dynamic text content (prevents XSS)" - severity: "warning" - description: | - Use x-text directive for dynamic content to prevent XSS vulnerabilities - pattern: - file_pattern: "app/templates/**/*.html" - recommended_pattern: '
' - - - id: "TPL-005" - name: "Use x-html ONLY for safe content" - severity: "error" - description: | - Use x-html only for trusted content like icons, never for user-generated content - pattern: - file_pattern: "app/templates/**/*.html" - safe_usage: - - 'x-html="\\$icon\\(' - - - id: "TPL-006" - name: "Implement loading state for data loads" - severity: "warning" - description: | - All templates that load data should show loading state - pattern: - file_pattern: "app/templates/**/*.html" - recommended_pattern: '