From 39dff4ab7d0bb09bcff8bc580dd3084f71b3c1f6 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Wed, 4 Feb 2026 21:32:32 +0100 Subject: [PATCH] refactor: fix architecture violations with provider patterns and dependency inversion Major changes: - Add AuditProvider protocol for cross-module audit logging - Move customer order operations to orders module (dependency inversion) - Add customer order metrics via MetricsProvider pattern - Fix missing db parameter in get_admin_context() calls - Move ProductMedia relationship to catalog module (proper ownership) - Add marketplace breakdown stats to marketplace_widgets New files: - contracts/audit.py - AuditProviderProtocol - core/services/audit_aggregator.py - Aggregates audit providers - monitoring/services/audit_provider.py - Monitoring audit implementation - orders/services/customer_order_service.py - Customer order operations - orders/routes/api/vendor_customer_orders.py - Customer order endpoints - catalog/services/product_media_service.py - Product media service - Architecture documentation for patterns Co-Authored-By: Claude Opus 4.5 --- app/modules/analytics/routes/pages/admin.py | 2 +- app/modules/base.py | 43 +++ app/modules/catalog/models/__init__.py | 4 +- app/modules/catalog/models/product.py | 3 + app/modules/catalog/models/product_media.py | 12 +- .../catalog/services/product_media_service.py | 280 ++++++++++++++ app/modules/cms/models/__init__.py | 7 +- app/modules/cms/models/media.py | 20 +- app/modules/cms/services/media_service.py | 149 +------- app/modules/contracts/__init__.py | 7 + app/modules/contracts/audit.py | 168 +++++++++ .../core/routes/api/admin_dashboard.py | 48 ++- app/modules/core/routes/api/admin_settings.py | 18 +- app/modules/core/services/audit_aggregator.py | 216 +++++++++++ app/modules/customers/routes/api/vendor.py | 81 +--- app/modules/customers/schemas/customer.py | 10 +- .../customers/services/customer_service.py | 108 +----- app/modules/marketplace/routes/pages/admin.py | 4 +- .../services/marketplace_widgets.py | 80 +++- app/modules/messaging/routes/pages/admin.py | 2 +- app/modules/monitoring/definition.py | 11 + .../monitoring/services/audit_provider.py | 78 ++++ app/modules/orders/routes/api/vendor.py | 4 + .../routes/api/vendor_customer_orders.py | 163 +++++++++ app/modules/orders/services/__init__.py | 6 + .../orders/services/customer_order_service.py | 122 +++++++ app/modules/orders/services/order_metrics.py | 105 ++++++ .../routes/api/admin_platform_users.py | 64 +++- app/modules/tenancy/routes/pages/admin.py | 24 +- docs/architecture/audit-provider-pattern.md | 265 ++++++++++++++ .../customer-orders-architecture.md | 345 ++++++++++++++++++ docs/architecture/media-architecture.md | 298 +++++++++++++++ .../services/test_customer_order_service.py | 228 ++++++++++++ .../services/test_order_metrics_customer.py | 183 ++++++++++ 34 files changed, 2751 insertions(+), 407 deletions(-) create mode 100644 app/modules/catalog/services/product_media_service.py create mode 100644 app/modules/contracts/audit.py create mode 100644 app/modules/core/services/audit_aggregator.py create mode 100644 app/modules/monitoring/services/audit_provider.py create mode 100644 app/modules/orders/routes/api/vendor_customer_orders.py create mode 100644 app/modules/orders/services/customer_order_service.py create mode 100644 docs/architecture/audit-provider-pattern.md create mode 100644 docs/architecture/customer-orders-architecture.md create mode 100644 docs/architecture/media-architecture.md create mode 100644 tests/unit/services/test_customer_order_service.py create mode 100644 tests/unit/services/test_order_metrics_customer.py diff --git a/app/modules/analytics/routes/pages/admin.py b/app/modules/analytics/routes/pages/admin.py index cd89b84b..30b82f9a 100644 --- a/app/modules/analytics/routes/pages/admin.py +++ b/app/modules/analytics/routes/pages/admin.py @@ -83,5 +83,5 @@ async def admin_code_quality_violation_detail( """ return templates.TemplateResponse( "dev_tools/admin/code-quality-violation-detail.html", - get_admin_context(request, current_user, violation_id=violation_id), + get_admin_context(request, db, current_user, violation_id=violation_id), ) diff --git a/app/modules/base.py b/app/modules/base.py index a25289b0..a330430b 100644 --- a/app/modules/base.py +++ b/app/modules/base.py @@ -44,6 +44,7 @@ if TYPE_CHECKING: from fastapi import APIRouter from pydantic import BaseModel + from app.modules.contracts.audit import AuditProviderProtocol from app.modules.contracts.metrics import MetricsProviderProtocol from app.modules.contracts.widgets import DashboardWidgetProviderProtocol @@ -434,6 +435,26 @@ class ModuleDefinition: # The provider will be discovered by core's WidgetAggregator service. widget_provider: "Callable[[], DashboardWidgetProviderProtocol] | None" = None + # ========================================================================= + # Audit Provider (Module-Driven Audit Logging) + # ========================================================================= + # Callable that returns an AuditProviderProtocol implementation. + # Use a callable (factory function) to enable lazy loading and avoid + # circular imports. Modules can provide audit logging backends. + # + # Example: + # def _get_audit_provider(): + # from app.modules.monitoring.services.audit_provider import audit_provider + # return audit_provider + # + # monitoring_module = ModuleDefinition( + # code="monitoring", + # audit_provider=_get_audit_provider, + # ) + # + # The provider will be discovered by core's AuditAggregator service. + audit_provider: "Callable[[], AuditProviderProtocol] | None" = None + # ========================================================================= # Menu Item Methods (Legacy - uses menu_items dict of IDs) # ========================================================================= @@ -842,6 +863,28 @@ class ModuleDefinition: return None return self.widget_provider() + # ========================================================================= + # Audit Provider Methods + # ========================================================================= + + def has_audit_provider(self) -> bool: + """Check if this module has an audit provider.""" + return self.audit_provider is not None + + def get_audit_provider_instance(self) -> "AuditProviderProtocol | None": + """ + Get the audit provider instance for this module. + + Calls the audit_provider factory function to get the provider. + Returns None if no provider is configured. + + Returns: + AuditProviderProtocol instance, or None + """ + if self.audit_provider is None: + return None + return self.audit_provider() + # ========================================================================= # Magic Methods # ========================================================================= diff --git a/app/modules/catalog/models/__init__.py b/app/modules/catalog/models/__init__.py index 9a5cec68..89c4217c 100644 --- a/app/modules/catalog/models/__init__.py +++ b/app/modules/catalog/models/__init__.py @@ -9,11 +9,11 @@ Usage: """ from app.modules.catalog.models.product import Product -from app.modules.catalog.models.product_translation import ProductTranslation from app.modules.catalog.models.product_media import ProductMedia +from app.modules.catalog.models.product_translation import ProductTranslation __all__ = [ "Product", - "ProductTranslation", "ProductMedia", + "ProductTranslation", ] diff --git a/app/modules/catalog/models/product.py b/app/modules/catalog/models/product.py index 72291601..8338ff35 100644 --- a/app/modules/catalog/models/product.py +++ b/app/modules/catalog/models/product.py @@ -114,6 +114,9 @@ class Product(Base, TimestampMixin): inventory_entries = relationship( "Inventory", back_populates="product", cascade="all, delete-orphan" ) + media_associations = relationship( + "ProductMedia", back_populates="product", cascade="all, delete-orphan" + ) # === CONSTRAINTS & INDEXES === __table_args__ = ( diff --git a/app/modules/catalog/models/product_media.py b/app/modules/catalog/models/product_media.py index 118164cc..7362819f 100644 --- a/app/modules/catalog/models/product_media.py +++ b/app/modules/catalog/models/product_media.py @@ -4,6 +4,11 @@ Product-Media association model. Links media files to products with usage type tracking (main image, gallery, variant images, etc.) + +This model lives in the catalog module because: +- It's an association specific to products +- Catalog owns the relationship between its products and media +- CMS provides generic media storage, catalog defines how it uses media """ from sqlalchemy import ( @@ -25,6 +30,8 @@ class ProductMedia(Base, TimestampMixin): Tracks which media files are used by which products, including the usage type (main image, gallery, variant, etc.) + + This is catalog's association table - CMS doesn't need to know about it. """ __tablename__ = "product_media" @@ -52,8 +59,9 @@ class ProductMedia(Base, TimestampMixin): variant_id = Column(Integer) # Reference to variant if applicable # Relationships - product = relationship("Product") - media = relationship("MediaFile", back_populates="product_associations") + product = relationship("Product", back_populates="media_associations") + # MediaFile is generic and doesn't know about ProductMedia + media = relationship("MediaFile", lazy="joined") __table_args__ = ( UniqueConstraint( diff --git a/app/modules/catalog/services/product_media_service.py b/app/modules/catalog/services/product_media_service.py new file mode 100644 index 00000000..561f589e --- /dev/null +++ b/app/modules/catalog/services/product_media_service.py @@ -0,0 +1,280 @@ +# app/modules/catalog/services/product_media_service.py +""" +Product media service for managing product-media associations. + +This service handles the catalog-specific logic for attaching and +detaching media files to products. It uses the generic MediaFile +from CMS but owns the ProductMedia association. + +This follows the principle that: +- CMS provides generic media storage (agnostic to consumers) +- Catalog defines how it uses media (ProductMedia association) +""" + +import logging + +from sqlalchemy.orm import Session + +from app.modules.catalog.models import Product, ProductMedia +from app.modules.cms.models import MediaFile + +logger = logging.getLogger(__name__) + + +class ProductMediaService: + """Service for product-media association operations.""" + + def attach_media_to_product( + self, + db: Session, + vendor_id: int, + product_id: int, + media_id: int, + usage_type: str = "gallery", + display_order: int = 0, + ) -> ProductMedia | None: + """ + Attach a media file to a product. + + Args: + db: Database session + vendor_id: Vendor ID (for ownership verification) + product_id: Product ID + media_id: Media file ID + usage_type: How the media is used (main_image, gallery, etc.) + display_order: Order for galleries + + Returns: + Created or updated ProductMedia association + + Raises: + ValueError: If product or media doesn't belong to vendor + """ + # Verify product belongs to vendor + product = ( + db.query(Product) + .filter(Product.id == product_id, Product.vendor_id == vendor_id) + .first() + ) + if not product: + raise ValueError(f"Product {product_id} not found for vendor {vendor_id}") + + # Verify media belongs to vendor + media = ( + db.query(MediaFile) + .filter(MediaFile.id == media_id, MediaFile.vendor_id == vendor_id) + .first() + ) + if not media: + raise ValueError(f"Media {media_id} not found for vendor {vendor_id}") + + # Check if already attached with same usage type + existing = ( + db.query(ProductMedia) + .filter( + ProductMedia.product_id == product_id, + ProductMedia.media_id == media_id, + ProductMedia.usage_type == usage_type, + ) + .first() + ) + + if existing: + # Update display order if association exists + existing.display_order = display_order + db.flush() + return existing + + # Create new association + product_media = ProductMedia( + product_id=product_id, + media_id=media_id, + usage_type=usage_type, + display_order=display_order, + ) + + db.add(product_media) + + # Update usage count on media + media.usage_count = (media.usage_count or 0) + 1 + + db.flush() + + logger.info( + f"Attached media {media_id} to product {product_id} as {usage_type}" + ) + + return product_media + + def detach_media_from_product( + self, + db: Session, + vendor_id: int, + product_id: int, + media_id: int, + usage_type: str | None = None, + ) -> int: + """ + Detach a media file from a product. + + Args: + db: Database session + vendor_id: Vendor ID (for ownership verification) + product_id: Product ID + media_id: Media file ID + usage_type: Specific usage type to remove (None = all usages) + + Returns: + Number of associations removed + + Raises: + ValueError: If product doesn't belong to vendor + """ + # Verify product belongs to vendor + product = ( + db.query(Product) + .filter(Product.id == product_id, Product.vendor_id == vendor_id) + .first() + ) + if not product: + raise ValueError(f"Product {product_id} not found for vendor {vendor_id}") + + # Build query + query = db.query(ProductMedia).filter( + ProductMedia.product_id == product_id, + ProductMedia.media_id == media_id, + ) + + if usage_type: + query = query.filter(ProductMedia.usage_type == usage_type) + + deleted_count = query.delete() + + # Update usage count on media + if deleted_count > 0: + media = db.query(MediaFile).filter(MediaFile.id == media_id).first() + if media: + media.usage_count = max(0, (media.usage_count or 0) - deleted_count) + + db.flush() + + logger.info( + f"Detached {deleted_count} media association(s) from product {product_id}" + ) + + return deleted_count + + def get_product_media( + self, + db: Session, + product_id: int, + usage_type: str | None = None, + ) -> list[ProductMedia]: + """ + Get media associations for a product. + + Args: + db: Database session + product_id: Product ID + usage_type: Filter by usage type (None = all) + + Returns: + List of ProductMedia associations + """ + query = ( + db.query(ProductMedia) + .filter(ProductMedia.product_id == product_id) + .order_by(ProductMedia.display_order) + ) + + if usage_type: + query = query.filter(ProductMedia.usage_type == usage_type) + + return query.all() + + def get_media_usage_for_product( + self, + db: Session, + product_id: int, + ) -> dict: + """ + Get all media usage information for a product. + + Args: + db: Database session + product_id: Product ID + + Returns: + Dict with media organized by usage type + """ + associations = self.get_product_media(db, product_id) + + usage_by_type: dict[str, list] = {} + for assoc in associations: + usage_type = assoc.usage_type + if usage_type not in usage_by_type: + usage_by_type[usage_type] = [] + + media = assoc.media + usage_by_type[usage_type].append({ + "association_id": assoc.id, + "media_id": media.id if media else None, + "filename": media.filename if media else None, + "file_url": media.file_url if media else None, + "thumbnail_url": media.thumbnail_url if media else None, + "display_order": assoc.display_order, + }) + + return { + "product_id": product_id, + "media_by_type": usage_by_type, + "total_count": len(associations), + } + + def set_main_image( + self, + db: Session, + vendor_id: int, + product_id: int, + media_id: int, + ) -> ProductMedia | None: + """ + Set the main image for a product. + + Removes any existing main_image association and creates a new one. + + Args: + db: Database session + vendor_id: Vendor ID + product_id: Product ID + media_id: Media file ID to set as main image + + Returns: + The new ProductMedia association + """ + # Remove existing main image + self.detach_media_from_product( + db, vendor_id, product_id, media_id=0, usage_type="main_image" + ) + + # Actually, we need to remove ALL main_image associations, not just for media_id=0 + db.query(ProductMedia).filter( + ProductMedia.product_id == product_id, + ProductMedia.usage_type == "main_image", + ).delete() + + # Attach new main image + return self.attach_media_to_product( + db, + vendor_id=vendor_id, + product_id=product_id, + media_id=media_id, + usage_type="main_image", + display_order=0, + ) + + +# Singleton instance +product_media_service = ProductMediaService() + +__all__ = ["ProductMediaService", "product_media_service"] diff --git a/app/modules/cms/models/__init__.py b/app/modules/cms/models/__init__.py index ba0744c3..6e5513fe 100644 --- a/app/modules/cms/models/__init__.py +++ b/app/modules/cms/models/__init__.py @@ -4,14 +4,15 @@ CMS module database models. This is the canonical location for CMS models including: - ContentPage: CMS pages (marketing, vendor default pages) -- MediaFile: Vendor media library +- MediaFile: Vendor media library (generic, consumer-agnostic) - VendorTheme: Vendor storefront theme configuration Usage: from app.modules.cms.models import ContentPage, MediaFile, VendorTheme -For product-media associations: - from app.modules.catalog.models import ProductMedia +Note: ProductMedia is in the catalog module since it's catalog's association +to media files. CMS provides generic media storage, consumers define their +own associations. """ from app.modules.cms.models.content_page import ContentPage diff --git a/app/modules/cms/models/media.py b/app/modules/cms/models/media.py index 85f8f2dd..5c237b48 100644 --- a/app/modules/cms/models/media.py +++ b/app/modules/cms/models/media.py @@ -1,11 +1,15 @@ # app/modules/cms/models/media.py """ -CORE media file model for vendor media library. +Generic media file model for vendor media library. -This is a CORE framework model used across multiple modules. -MediaFile provides vendor-uploaded media files (images, documents, videos). +This is a consumer-agnostic media storage model. MediaFile provides +vendor-uploaded media files (images, documents, videos) without knowing +what entities will use them. -For product-media associations, use: +Modules that need media (catalog, art-gallery, etc.) define their own +association tables that reference MediaFile. + +For product-media associations: from app.modules.catalog.models import ProductMedia Files are stored in vendor-specific directories: @@ -73,12 +77,8 @@ class MediaFile(Base, TimestampMixin): # Relationships vendor = relationship("Vendor", back_populates="media_files") - # ProductMedia relationship uses string reference to avoid circular import - product_associations = relationship( - "ProductMedia", - back_populates="media", - cascade="all, delete-orphan", - ) + # Note: Consumer-specific associations (ProductMedia, etc.) are defined + # in their respective modules. CMS doesn't know about specific consumers. __table_args__ = ( Index("idx_media_vendor_id", "vendor_id"), diff --git a/app/modules/cms/services/media_service.py b/app/modules/cms/services/media_service.py index eaff9e45..8246b2a0 100644 --- a/app/modules/cms/services/media_service.py +++ b/app/modules/cms/services/media_service.py @@ -16,14 +16,11 @@ import shutil import uuid from datetime import UTC, datetime from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any from sqlalchemy import or_ from sqlalchemy.orm import Session -if TYPE_CHECKING: - from app.modules.catalog.models import ProductMedia - from app.modules.cms.exceptions import ( MediaNotFoundException, MediaUploadException, @@ -420,147 +417,9 @@ class MediaService: return True - def get_media_usage( - self, db: Session, vendor_id: int, media_id: int - ) -> dict: - """ - Get where a media file is being used. - - Returns: - Dict with products and other usage information - """ - media = self.get_media(db, vendor_id, media_id) - - # Get product associations - product_usage = [] - for assoc in media.product_associations: - product = assoc.product - if product: - product_usage.append({ - "product_id": product.id, - "product_name": product.get_title() or f"Product {product.id}", - "usage_type": assoc.usage_type, - }) - - return { - "media_id": media_id, - "products": product_usage, - "other_usage": [], - "total_usage_count": len(product_usage), - } - - def attach_to_product( - self, - db: Session, - vendor_id: int, - media_id: int, - product_id: int, - usage_type: str = "gallery", - display_order: int = 0, - ) -> "ProductMedia | None": - """ - Attach a media file to a product. - - Args: - db: Database session - vendor_id: Vendor ID - media_id: Media file ID - product_id: Product ID - usage_type: How the media is used (main_image, gallery, etc.) - display_order: Order for galleries - - Returns: - Created ProductMedia association, or None if catalog module unavailable - """ - try: - from app.modules.catalog.models import ProductMedia - except ImportError: - logger.warning("Catalog module not available for media-product attachment") - return None - - # Verify media belongs to vendor - media = self.get_media(db, vendor_id, media_id) - - # Check if already attached with same usage type - existing = ( - db.query(ProductMedia) - .filter( - ProductMedia.product_id == product_id, - ProductMedia.media_id == media_id, - ProductMedia.usage_type == usage_type, - ) - .first() - ) - - if existing: - existing.display_order = display_order - db.flush() - return existing - - # Create association - product_media = ProductMedia( - product_id=product_id, - media_id=media_id, - usage_type=usage_type, - display_order=display_order, - ) - - db.add(product_media) - - # Update usage count - media.usage_count = (media.usage_count or 0) + 1 - - db.flush() - - return product_media - - def detach_from_product( - self, - db: Session, - vendor_id: int, - media_id: int, - product_id: int, - usage_type: str | None = None, - ) -> bool: - """ - Detach a media file from a product. - - Args: - db: Database session - vendor_id: Vendor ID - media_id: Media file ID - product_id: Product ID - usage_type: Specific usage type to remove (None = all) - - Returns: - True if detached, False if catalog module unavailable - """ - try: - from app.modules.catalog.models import ProductMedia - except ImportError: - logger.warning("Catalog module not available for media-product detachment") - return False - - # Verify media belongs to vendor - media = self.get_media(db, vendor_id, media_id) - - query = db.query(ProductMedia).filter( - ProductMedia.product_id == product_id, - ProductMedia.media_id == media_id, - ) - - if usage_type: - query = query.filter(ProductMedia.usage_type == usage_type) - - deleted_count = query.delete() - - # Update usage count - if deleted_count > 0: - media.usage_count = max(0, (media.usage_count or 0) - deleted_count) - - db.flush() - - return deleted_count > 0 + # Note: Product-specific methods (attach_to_product, detach_from_product, + # get_media_usage) have been moved to catalog.services.product_media_service + # CMS media_service is now consumer-agnostic. # Create service instance diff --git a/app/modules/contracts/__init__.py b/app/modules/contracts/__init__.py index d4c59cb1..636fd00e 100644 --- a/app/modules/contracts/__init__.py +++ b/app/modules/contracts/__init__.py @@ -45,6 +45,10 @@ Widget Provider Pattern: return [DashboardWidget(key="orders.recent", widget_type="list", ...)] """ +from app.modules.contracts.audit import ( + AuditEvent, + AuditProviderProtocol, +) from app.modules.contracts.base import ServiceProtocol from app.modules.contracts.cms import ContentServiceProtocol from app.modules.contracts.metrics import ( @@ -67,6 +71,9 @@ __all__ = [ "ServiceProtocol", # CMS protocols "ContentServiceProtocol", + # Audit protocols + "AuditEvent", + "AuditProviderProtocol", # Metrics protocols "MetricValue", "MetricsContext", diff --git a/app/modules/contracts/audit.py b/app/modules/contracts/audit.py new file mode 100644 index 00000000..bd51886b --- /dev/null +++ b/app/modules/contracts/audit.py @@ -0,0 +1,168 @@ +# app/modules/contracts/audit.py +""" +Audit provider protocol for cross-module audit logging. + +This module defines the protocol that modules implement to provide audit logging. +The core module's AuditAggregator discovers and uses providers from enabled modules. + +Benefits: +- Audit logging is optional (monitoring module can be disabled) +- Each module can have its own audit implementation +- Core doesn't depend on monitoring module +- Easy to add different audit backends (file, database, external service) + +Usage: + # 1. Implement the protocol in your module + class DatabaseAuditProvider: + @property + def audit_backend(self) -> str: + return "database" + + def log_action(self, db, event: AuditEvent) -> bool: + # Log to database + ... + + # 2. Register in module definition + def _get_audit_provider(): + from app.modules.monitoring.services.audit_provider import audit_provider + return audit_provider + + monitoring_module = ModuleDefinition( + code="monitoring", + audit_provider=_get_audit_provider, + # ... + ) + + # 3. Use via aggregator in core + from app.modules.core.services.audit_aggregator import audit_aggregator + + audit_aggregator.log_action( + db=db, + event=AuditEvent( + admin_user_id=123, + action="create_vendor", + target_type="vendor", + target_id="456", + ) + ) +""" + +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable + +if TYPE_CHECKING: + from sqlalchemy.orm import Session + + +@dataclass +class AuditEvent: + """ + Standard audit event data. + + This is the unit of data passed to audit providers. + It contains all information needed to log an admin action. + + Attributes: + admin_user_id: ID of the admin performing the action + action: Action performed (e.g., "create_vendor", "update_setting") + target_type: Type of target (e.g., "vendor", "user", "setting") + target_id: ID of the target entity (as string) + details: Additional context about the action + ip_address: IP address of the admin (optional) + user_agent: User agent string (optional) + request_id: Request ID for correlation (optional) + + Example: + AuditEvent( + admin_user_id=1, + action="create_setting", + target_type="setting", + target_id="max_vendors", + details={"category": "system", "value_type": "integer"}, + ) + """ + + admin_user_id: int + action: str + target_type: str + target_id: str + details: dict[str, Any] | None = None + ip_address: str | None = None + user_agent: str | None = None + request_id: str | None = None + + +@runtime_checkable +class AuditProviderProtocol(Protocol): + """ + Protocol for modules that provide audit logging. + + Each module can implement this to expose its own audit backend. + The core module's AuditAggregator discovers and uses providers. + + Implementation Notes: + - Providers should be fault-tolerant (don't break operations on failure) + - Return True on success, False on failure + - Log errors internally but don't raise exceptions + - Be mindful of performance (audit logging should be fast) + + Example Implementation: + class DatabaseAuditProvider: + @property + def audit_backend(self) -> str: + return "database" + + def log_action(self, db: Session, event: AuditEvent) -> bool: + try: + from app.modules.tenancy.models import AdminAuditLog + audit_log = AdminAuditLog( + admin_user_id=event.admin_user_id, + action=event.action, + target_type=event.target_type, + target_id=event.target_id, + details=event.details or {}, + ip_address=event.ip_address, + user_agent=event.user_agent, + request_id=event.request_id, + ) + db.add(audit_log) + db.flush() + return True + except Exception: + return False + """ + + @property + def audit_backend(self) -> str: + """ + Backend name for this provider. + + Should be a short, lowercase identifier for the audit backend. + Examples: "database", "file", "elasticsearch", "cloudwatch" + + Returns: + Backend string used for identification + """ + ... + + def log_action(self, db: "Session", event: AuditEvent) -> bool: + """ + Log an audit event. + + Called by the audit aggregator to record admin actions. + Should be fault-tolerant - don't raise exceptions, just return False. + + Args: + db: Database session (may be needed for database backends) + event: The audit event to log + + Returns: + True if logged successfully, False otherwise + """ + ... + + +__all__ = [ + "AuditEvent", + "AuditProviderProtocol", +] diff --git a/app/modules/core/routes/api/admin_dashboard.py b/app/modules/core/routes/api/admin_dashboard.py index f8b92ef5..935aedd9 100644 --- a/app/modules/core/routes/api/admin_dashboard.py +++ b/app/modules/core/routes/api/admin_dashboard.py @@ -20,7 +20,7 @@ from sqlalchemy.orm import Session from app.api.deps import get_current_admin_api from app.core.database import get_db -from app.modules.contracts.widgets import ListWidget +from app.modules.contracts.widgets import BreakdownWidget, ListWidget from app.modules.core.schemas.dashboard import ( AdminDashboardResponse, ImportStatsResponse, @@ -219,29 +219,37 @@ def get_comprehensive_stats( "/stats/marketplace", response_model=list[MarketplaceStatsResponse] ) def get_marketplace_stats( + request: Request, db: Session = Depends(get_db), current_admin: UserContext = Depends(get_current_admin_api), ): - """Get statistics broken down by marketplace (Admin only).""" - # For detailed marketplace breakdown, we still use the analytics service - # as the MetricsProvider pattern is for aggregated stats - try: - from app.modules.analytics.services.stats_service import stats_service + """Get statistics broken down by marketplace (Admin only). - marketplace_stats = stats_service.get_marketplace_breakdown_stats(db=db) - return [ - MarketplaceStatsResponse( - marketplace=stat["marketplace"], - total_products=stat["total_products"], - unique_vendors=stat["unique_vendors"], - unique_brands=stat["unique_brands"], - ) - for stat in marketplace_stats - ] - except ImportError: - # Analytics module not available - logger.warning("Analytics module not available for marketplace breakdown stats") - return [] + Uses the widget_aggregator to get marketplace breakdown data from the + marketplace module's WidgetProvider, avoiding cross-module violations. + """ + platform_id = _get_platform_id(request, current_admin) + + # Get widgets from marketplace module via aggregator + widgets = widget_aggregator.get_admin_dashboard_widgets(db=db, platform_id=platform_id) + + # Extract marketplace breakdown widget + marketplace_widgets = widgets.get("marketplace", []) + for widget in marketplace_widgets: + if widget.key == "marketplace.breakdown" and isinstance(widget.data, BreakdownWidget): + # Transform breakdown items to MarketplaceStatsResponse + return [ + MarketplaceStatsResponse( + marketplace=item.label, + total_products=int(item.value), + unique_vendors=int(item.secondary_value or 0), + unique_brands=0, # Not included in breakdown widget + ) + for item in widget.data.items + ] + + # No breakdown widget found + return [] @admin_dashboard_router.get("/stats/platform", response_model=PlatformStatsResponse) diff --git a/app/modules/core/routes/api/admin_settings.py b/app/modules/core/routes/api/admin_settings.py index eb21f8a2..25b3093c 100644 --- a/app/modules/core/routes/api/admin_settings.py +++ b/app/modules/core/routes/api/admin_settings.py @@ -20,8 +20,8 @@ from app.core.config import settings as app_settings from app.core.database import get_db from app.exceptions import ResourceNotFoundException from app.modules.tenancy.exceptions import ConfirmationRequiredException -from app.modules.monitoring.services.admin_audit_service import admin_audit_service from app.modules.core.services.admin_settings_service import admin_settings_service +from app.modules.core.services.audit_aggregator import audit_aggregator from models.schema.auth import UserContext from app.modules.tenancy.schemas.admin import ( AdminSettingCreate, @@ -116,7 +116,7 @@ def create_setting( ) # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="create_setting", @@ -147,7 +147,7 @@ def update_setting( ) # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="update_setting", @@ -176,7 +176,7 @@ def upsert_setting( ) # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="upsert_setting", @@ -233,7 +233,7 @@ def set_rows_per_page( db=db, setting_data=setting_data, admin_user_id=current_admin.id ) - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="update_setting", @@ -288,7 +288,7 @@ def delete_setting( ) # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="delete_setting", @@ -586,7 +586,7 @@ def update_email_settings( updated_keys.append(field) # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="update_email_settings", @@ -625,7 +625,7 @@ def reset_email_settings( deleted_count += 1 # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="reset_email_settings", @@ -688,7 +688,7 @@ def send_test_email( # Check if email was actually sent (send_raw returns EmailLog, not boolean) if email_log.status == "sent": # Log action - admin_audit_service.log_action( + audit_aggregator.log( db=db, admin_user_id=current_admin.id, action="send_test_email", diff --git a/app/modules/core/services/audit_aggregator.py b/app/modules/core/services/audit_aggregator.py new file mode 100644 index 00000000..7c200bff --- /dev/null +++ b/app/modules/core/services/audit_aggregator.py @@ -0,0 +1,216 @@ +# app/modules/core/services/audit_aggregator.py +""" +Audit aggregator service for collecting audit providers from all modules. + +This service lives in core because audit logging is infrastructure functionality +that should be available across all modules. It discovers AuditProviders from +enabled modules, providing a unified interface for audit logging. + +Benefits: +- Audit logging always works (aggregator is in core) +- Each module can provide its own audit backend +- Optional modules are truly optional (monitoring can be removed without breaking app) +- Easy to add new audit backends (just implement AuditProviderProtocol in your module) + +Usage: + from app.modules.core.services.audit_aggregator import audit_aggregator + from app.modules.contracts.audit import AuditEvent + + # Log an admin action + audit_aggregator.log_action( + db=db, + event=AuditEvent( + admin_user_id=123, + action="create_setting", + target_type="setting", + target_id="max_vendors", + details={"category": "system"}, + ) + ) + + # Or use the convenience method with individual parameters + audit_aggregator.log( + db=db, + admin_user_id=123, + action="create_setting", + target_type="setting", + target_id="max_vendors", + details={"category": "system"}, + ) +""" + +import logging +from typing import TYPE_CHECKING, Any + +from sqlalchemy.orm import Session + +from app.modules.contracts.audit import ( + AuditEvent, + AuditProviderProtocol, +) + +if TYPE_CHECKING: + from app.modules.base import ModuleDefinition + +logger = logging.getLogger(__name__) + + +class AuditAggregatorService: + """ + Aggregates audit providers from all modules. + + This service discovers AuditProviders from enabled modules and provides + a unified interface for audit logging. It handles graceful degradation + when modules are disabled or providers fail. + """ + + def _get_enabled_providers( + self, db: Session + ) -> list[tuple["ModuleDefinition", AuditProviderProtocol]]: + """ + Get audit providers from enabled modules. + + Note: Unlike metrics/widget providers, audit logging doesn't filter + by platform_id - it's a system-wide concern. We still need db to + check if optional modules are enabled. + + Args: + db: Database session + + Returns: + List of (module, provider) tuples for enabled modules with providers + """ + from app.modules.registry import MODULES + + providers: list[tuple[ModuleDefinition, AuditProviderProtocol]] = [] + + for module in MODULES.values(): + # Skip modules without audit providers + if not module.has_audit_provider(): + continue + + # Core and internal modules are always enabled + # For optional modules, we don't strictly check enablement for audit + # because audit logging is system infrastructure + + # Get the provider instance + try: + provider = module.get_audit_provider_instance() + if provider is not None: + providers.append((module, provider)) + except Exception as e: + logger.warning( + f"Failed to get audit provider for module {module.code}: {e}" + ) + + return providers + + def log_action(self, db: Session, event: AuditEvent) -> bool: + """ + Log an audit event using all available providers. + + Called by services/routes to record admin actions. + Uses all available audit providers - if multiple are configured, + all will receive the event (e.g., database + external service). + + Args: + db: Database session + event: The audit event to log + + Returns: + True if at least one provider logged successfully, False otherwise + """ + providers = self._get_enabled_providers(db) + + if not providers: + # No audit providers available - this is acceptable + logger.debug( + f"No audit providers available for action {event.action} " + f"on {event.target_type}:{event.target_id}" + ) + return False + + any_success = False + for module, provider in providers: + try: + success = provider.log_action(db, event) + if success: + any_success = True + logger.debug( + f"Audit logged via {module.code}: {event.action} " + f"on {event.target_type}:{event.target_id}" + ) + else: + logger.warning( + f"Audit provider {module.code} failed to log {event.action}" + ) + except Exception as e: + logger.warning( + f"Failed to log audit via {module.code}: {e}" + ) + # Continue with other providers - graceful degradation + + return any_success + + def log( + self, + db: Session, + admin_user_id: int, + action: str, + target_type: str, + target_id: str, + details: dict[str, Any] | None = None, + ip_address: str | None = None, + user_agent: str | None = None, + request_id: str | None = None, + ) -> bool: + """ + Convenience method to log an audit event with individual parameters. + + This is a shorthand for creating an AuditEvent and calling log_action. + Useful for simple logging calls without explicitly constructing the event. + + Args: + db: Database session + admin_user_id: ID of the admin performing the action + action: Action performed (e.g., "create_vendor", "update_setting") + target_type: Type of target (e.g., "vendor", "user", "setting") + target_id: ID of the target entity (as string) + details: Additional context about the action + ip_address: IP address of the admin (optional) + user_agent: User agent string (optional) + request_id: Request ID for correlation (optional) + + Returns: + True if at least one provider logged successfully, False otherwise + """ + event = AuditEvent( + admin_user_id=admin_user_id, + action=action, + target_type=target_type, + target_id=str(target_id), + details=details, + ip_address=ip_address, + user_agent=user_agent, + request_id=request_id, + ) + return self.log_action(db, event) + + def get_available_backends(self, db: Session) -> list[str]: + """ + Get list of available audit backends. + + Args: + db: Database session + + Returns: + List of backend names from enabled providers + """ + providers = self._get_enabled_providers(db) + return [provider.audit_backend for _, provider in providers] + + +# Singleton instance +audit_aggregator = AuditAggregatorService() + +__all__ = ["AuditAggregatorService", "audit_aggregator"] diff --git a/app/modules/customers/routes/api/vendor.py b/app/modules/customers/routes/api/vendor.py index 3a0fdbb5..7b3ddaa7 100644 --- a/app/modules/customers/routes/api/vendor.py +++ b/app/modules/customers/routes/api/vendor.py @@ -19,9 +19,7 @@ from models.schema.auth import UserContext from app.modules.customers.schemas import ( CustomerDetailResponse, CustomerMessageResponse, - CustomerOrdersResponse, CustomerResponse, - CustomerStatisticsResponse, CustomerUpdate, VendorCustomerListResponse, ) @@ -79,7 +77,9 @@ def get_customer_details( - Get customer by ID - Verify customer belongs to vendor - - Include order statistics + + Note: Order statistics are available via the orders module endpoint: + GET /api/vendor/customers/{customer_id}/order-stats """ # Service will raise CustomerNotFoundException if not found customer = customer_service.get_customer( @@ -88,13 +88,6 @@ def get_customer_details( customer_id=customer_id, ) - # Get statistics - stats = customer_service.get_customer_statistics( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) - return CustomerDetailResponse( id=customer.id, email=customer.email, @@ -104,55 +97,10 @@ def get_customer_details( customer_number=customer.customer_number, is_active=customer.is_active, marketing_consent=customer.marketing_consent, - total_orders=stats["total_orders"], - total_spent=stats["total_spent"], - average_order_value=stats["average_order_value"], - last_order_date=stats["last_order_date"], created_at=customer.created_at, ) -@vendor_router.get("/{customer_id}/orders", response_model=CustomerOrdersResponse) -def get_customer_orders( - customer_id: int, - skip: int = Query(0, ge=0), - limit: int = Query(50, ge=1, le=100), - current_user: UserContext = Depends(get_current_vendor_api), - db: Session = Depends(get_db), -): - """ - Get order history for a specific customer. - - - Get all orders for customer - - Filter by vendor_id - - Return order details - """ - # Service will raise CustomerNotFoundException if not found - orders, total = customer_service.get_customer_orders( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - skip=skip, - limit=limit, - ) - - return CustomerOrdersResponse( - orders=[ - { - "id": o.id, - "order_number": o.order_number, - "status": o.status, - "total": o.total_cents / 100 if o.total_cents else 0, - "created_at": o.created_at, - } - for o in orders - ], - total=total, - skip=skip, - limit=limit, - ) - - @vendor_router.put("/{customer_id}", response_model=CustomerMessageResponse) def update_customer( customer_id: int, @@ -203,26 +151,3 @@ def toggle_customer_status( status = "activated" if customer.is_active else "deactivated" return CustomerMessageResponse(message=f"Customer {status} successfully") - -@vendor_router.get("/{customer_id}/stats", response_model=CustomerStatisticsResponse) -def get_customer_statistics( - customer_id: int, - current_user: UserContext = Depends(get_current_vendor_api), - db: Session = Depends(get_db), -): - """ - Get customer statistics and metrics. - - - Total orders - - Total spent - - Average order value - - Last order date - """ - # Service will raise CustomerNotFoundException if not found - stats = customer_service.get_customer_statistics( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) - - return CustomerStatisticsResponse(**stats) diff --git a/app/modules/customers/schemas/customer.py b/app/modules/customers/schemas/customer.py index 7f7d759e..929191e5 100644 --- a/app/modules/customers/schemas/customer.py +++ b/app/modules/customers/schemas/customer.py @@ -244,7 +244,12 @@ class VendorCustomerListResponse(BaseModel): class CustomerDetailResponse(BaseModel): - """Detailed customer response for vendor management.""" + """Detailed customer response for vendor management. + + Note: Order-related statistics (total_orders, total_spent, last_order_date) + are available via the orders module endpoint: + GET /api/vendor/customers/{customer_id}/order-stats + """ id: int | None = None vendor_id: int | None = None @@ -255,9 +260,6 @@ class CustomerDetailResponse(BaseModel): customer_number: str | None = None marketing_consent: bool | None = None preferred_language: str | None = None - last_order_date: datetime | None = None - total_orders: int | None = None - total_spent: Decimal | None = None is_active: bool | None = None created_at: datetime | None = None updated_at: datetime | None = None diff --git a/app/modules/customers/services/customer_service.py b/app/modules/customers/services/customer_service.py index 52c70f0d..263fd468 100644 --- a/app/modules/customers/services/customer_service.py +++ b/app/modules/customers/services/customer_service.py @@ -305,109 +305,11 @@ class CustomerService: return customers, total - def get_customer_orders( - self, - db: Session, - vendor_id: int, - customer_id: int, - skip: int = 0, - limit: int = 50, - ) -> tuple[list, int]: - """ - Get orders for a specific customer. - - Args: - db: Database session - vendor_id: Vendor ID - customer_id: Customer ID - skip: Pagination offset - limit: Pagination limit - - Returns: - Tuple of (orders, total_count) - - Raises: - CustomerNotFoundException: If customer not found - """ - # Verify customer belongs to vendor - self.get_customer(db, vendor_id, customer_id) - - try: - from app.modules.orders.models import Order - - # Get customer orders - query = ( - db.query(Order) - .filter( - Order.customer_id == customer_id, - Order.vendor_id == vendor_id, - ) - .order_by(Order.created_at.desc()) - ) - - total = query.count() - orders = query.offset(skip).limit(limit).all() - - return orders, total - except ImportError: - # Orders module not available - logger.warning("Orders module not available for customer orders") - return [], 0 - - def get_customer_statistics( - self, db: Session, vendor_id: int, customer_id: int - ) -> dict: - """ - Get detailed statistics for a customer. - - Args: - db: Database session - vendor_id: Vendor ID - customer_id: Customer ID - - Returns: - Dict with customer statistics - """ - customer = self.get_customer(db, vendor_id, customer_id) - - # Get order statistics if orders module is available - try: - from sqlalchemy import func - - from app.modules.orders.models import Order - - order_stats = ( - db.query( - func.count(Order.id).label("total_orders"), - func.sum(Order.total_cents).label("total_spent_cents"), - func.avg(Order.total_cents).label("avg_order_cents"), - func.max(Order.created_at).label("last_order_date"), - ) - .filter(Order.customer_id == customer_id) - .first() - ) - - total_orders = order_stats.total_orders or 0 - total_spent_cents = order_stats.total_spent_cents or 0 - avg_order_cents = order_stats.avg_order_cents or 0 - last_order_date = order_stats.last_order_date - except ImportError: - # Orders module not available - logger.warning("Orders module not available for customer statistics") - total_orders = 0 - total_spent_cents = 0 - avg_order_cents = 0 - last_order_date = None - - return { - "customer_id": customer_id, - "total_orders": total_orders, - "total_spent": total_spent_cents / 100, # Convert to euros - "average_order_value": avg_order_cents / 100 if avg_order_cents else 0.0, - "last_order_date": last_order_date, - "member_since": customer.created_at, - "is_active": customer.is_active, - } + # Note: Customer order methods have been moved to the orders module. + # Use orders.services.customer_order_service for: + # - get_customer_orders() + # Use orders.services.order_metrics.get_customer_order_metrics() for: + # - customer order statistics def toggle_customer_status( self, db: Session, vendor_id: int, customer_id: int diff --git a/app/modules/marketplace/routes/pages/admin.py b/app/modules/marketplace/routes/pages/admin.py index 6d4722b4..84e7c055 100644 --- a/app/modules/marketplace/routes/pages/admin.py +++ b/app/modules/marketplace/routes/pages/admin.py @@ -59,7 +59,7 @@ async def admin_background_tasks_page( """ return templates.TemplateResponse( "marketplace/admin/background-tasks.html", - get_admin_context(request, current_user, flower_url=settings.flower_url), + get_admin_context(request, db, current_user, flower_url=settings.flower_url), ) @@ -127,7 +127,7 @@ async def admin_letzshop_order_detail_page( """ return templates.TemplateResponse( "marketplace/admin/letzshop-order-detail.html", - get_admin_context(request, current_user, order_id=order_id), + get_admin_context(request, db, current_user, order_id=order_id), ) diff --git a/app/modules/marketplace/services/marketplace_widgets.py b/app/modules/marketplace/services/marketplace_widgets.py index f9daab18..9b774031 100644 --- a/app/modules/marketplace/services/marketplace_widgets.py +++ b/app/modules/marketplace/services/marketplace_widgets.py @@ -14,9 +14,11 @@ import logging from sqlalchemy.orm import Session from app.modules.contracts.widgets import ( + BreakdownWidget, DashboardWidget, DashboardWidgetProviderProtocol, ListWidget, + WidgetBreakdownItem, WidgetContext, WidgetListItem, ) @@ -187,7 +189,7 @@ class MarketplaceWidgetProvider: .count() ) - return [ + widgets = [ DashboardWidget( key="marketplace.recent_imports", widget_type="list", @@ -204,6 +206,82 @@ class MarketplaceWidgetProvider: ) ] + # Add marketplace breakdown widget + breakdown_widget = self._get_marketplace_breakdown_widget(db) + if breakdown_widget: + widgets.append(breakdown_widget) + + return widgets + + def _get_marketplace_breakdown_widget( + self, + db: Session, + ) -> DashboardWidget | None: + """ + Get a breakdown widget showing statistics per marketplace. + + Returns: + DashboardWidget with BreakdownWidget data, or None if no data + """ + from sqlalchemy import func + + from app.modules.marketplace.models import MarketplaceProduct + + try: + marketplace_stats = ( + db.query( + MarketplaceProduct.marketplace, + func.count(MarketplaceProduct.id).label("total_products"), + func.count(func.distinct(MarketplaceProduct.vendor_name)).label( + "unique_vendors" + ), + func.count(func.distinct(MarketplaceProduct.brand)).label( + "unique_brands" + ), + ) + .filter(MarketplaceProduct.marketplace.isnot(None)) + .group_by(MarketplaceProduct.marketplace) + .all() + ) + + if not marketplace_stats: + return None + + total_products = sum(stat.total_products for stat in marketplace_stats) + + breakdown_items = [ + WidgetBreakdownItem( + label=stat.marketplace or "Unknown", + value=stat.total_products, + secondary_value=stat.unique_vendors, + percentage=( + round(stat.total_products / total_products * 100, 1) + if total_products > 0 + else 0 + ), + icon="globe", + ) + for stat in marketplace_stats + ] + + return DashboardWidget( + key="marketplace.breakdown", + widget_type="breakdown", + title="Products by Marketplace", + category="marketplace", + data=BreakdownWidget( + items=breakdown_items, + total=total_products, + ), + icon="chart-pie", + description="Product distribution across marketplaces", + order=30, + ) + + except Exception as e: + logger.warning(f"Failed to get marketplace breakdown widget: {e}") + return None + # Singleton instance marketplace_widget_provider = MarketplaceWidgetProvider() diff --git a/app/modules/messaging/routes/pages/admin.py b/app/modules/messaging/routes/pages/admin.py index 5e326ada..7d197477 100644 --- a/app/modules/messaging/routes/pages/admin.py +++ b/app/modules/messaging/routes/pages/admin.py @@ -83,7 +83,7 @@ async def admin_conversation_detail_page( """ return templates.TemplateResponse( "messaging/admin/messages.html", - get_admin_context(request, current_user, conversation_id=conversation_id), + get_admin_context(request, db, current_user, conversation_id=conversation_id), ) diff --git a/app/modules/monitoring/definition.py b/app/modules/monitoring/definition.py index 4ece4d90..6560c7cb 100644 --- a/app/modules/monitoring/definition.py +++ b/app/modules/monitoring/definition.py @@ -17,6 +17,13 @@ def _get_admin_router(): return admin_router +def _get_audit_provider(): + """Lazy import of audit provider to avoid circular imports.""" + from app.modules.monitoring.services.audit_provider import audit_provider + + return audit_provider + + # Monitoring module definition monitoring_module = ModuleDefinition( code="monitoring", @@ -112,6 +119,10 @@ monitoring_module = ModuleDefinition( is_core=False, is_internal=True, # Internal module - admin-only, not customer-facing # ========================================================================= + # Audit Provider + # ========================================================================= + audit_provider=_get_audit_provider, + # ========================================================================= # Self-Contained Module Configuration # ========================================================================= is_self_contained=True, diff --git a/app/modules/monitoring/services/audit_provider.py b/app/modules/monitoring/services/audit_provider.py new file mode 100644 index 00000000..4f4cdfc5 --- /dev/null +++ b/app/modules/monitoring/services/audit_provider.py @@ -0,0 +1,78 @@ +# app/modules/monitoring/services/audit_provider.py +""" +Audit provider implementation for the monitoring module. + +Provides database-backed audit logging using the AdminAuditLog model. +This wraps the existing admin_audit_service functionality in the +AuditProviderProtocol interface. +""" + +import logging +from typing import TYPE_CHECKING + +from sqlalchemy.orm import Session + +from app.modules.contracts.audit import AuditEvent, AuditProviderProtocol +from app.modules.tenancy.models import AdminAuditLog + +if TYPE_CHECKING: + pass + +logger = logging.getLogger(__name__) + + +class DatabaseAuditProvider: + """ + Database-backed audit provider. + + Logs admin actions to the AdminAuditLog table. + This is the default audit backend for the platform. + """ + + @property + def audit_backend(self) -> str: + return "database" + + def log_action(self, db: Session, event: AuditEvent) -> bool: + """ + Log an audit event to the database. + + Args: + db: Database session + event: The audit event to log + + Returns: + True if logged successfully, False otherwise + """ + try: + audit_log = AdminAuditLog( + admin_user_id=event.admin_user_id, + action=event.action, + target_type=event.target_type, + target_id=str(event.target_id), + details=event.details or {}, + ip_address=event.ip_address, + user_agent=event.user_agent, + request_id=event.request_id, + ) + + db.add(audit_log) + db.flush() + + logger.debug( + f"Admin action logged: {event.action} on {event.target_type}:" + f"{event.target_id} by admin {event.admin_user_id}" + ) + + return True + + except Exception as e: + logger.error(f"Failed to log admin action: {str(e)}") + # Don't raise exception - audit logging should not break operations + return False + + +# Singleton instance +audit_provider = DatabaseAuditProvider() + +__all__ = ["DatabaseAuditProvider", "audit_provider"] diff --git a/app/modules/orders/routes/api/vendor.py b/app/modules/orders/routes/api/vendor.py index cb1265d8..de2dbd53 100644 --- a/app/modules/orders/routes/api/vendor.py +++ b/app/modules/orders/routes/api/vendor.py @@ -284,6 +284,9 @@ def ship_order_item( # ============================================================================ # Import sub-routers +from app.modules.orders.routes.api.vendor_customer_orders import ( + vendor_customer_orders_router, +) from app.modules.orders.routes.api.vendor_exceptions import vendor_exceptions_router from app.modules.orders.routes.api.vendor_invoices import vendor_invoices_router @@ -291,3 +294,4 @@ from app.modules.orders.routes.api.vendor_invoices import vendor_invoices_router vendor_router.include_router(_orders_router, tags=["vendor-orders"]) vendor_router.include_router(vendor_exceptions_router, tags=["vendor-order-exceptions"]) vendor_router.include_router(vendor_invoices_router, tags=["vendor-invoices"]) +vendor_router.include_router(vendor_customer_orders_router, tags=["vendor-customer-orders"]) diff --git a/app/modules/orders/routes/api/vendor_customer_orders.py b/app/modules/orders/routes/api/vendor_customer_orders.py new file mode 100644 index 00000000..e0d1e34a --- /dev/null +++ b/app/modules/orders/routes/api/vendor_customer_orders.py @@ -0,0 +1,163 @@ +# app/modules/orders/routes/api/vendor_customer_orders.py +""" +Vendor customer order endpoints. + +These endpoints provide customer-order data, owned by the orders module. +The orders module owns the relationship between customers and orders, +similar to how catalog owns the ProductMedia relationship. + +Vendor Context: Uses token_vendor_id from JWT token. +""" + +import logging +from datetime import datetime + +from fastapi import APIRouter, Depends, Query +from pydantic import BaseModel +from sqlalchemy.orm import Session + +from app.api.deps import get_current_vendor_api, require_module_access +from app.core.database import get_db +from app.modules.enums import FrontendType +from app.modules.orders.services.customer_order_service import customer_order_service +from app.modules.orders.services.order_metrics import order_metrics_provider +from models.schema.auth import UserContext + +logger = logging.getLogger(__name__) + +# Router for customer-order endpoints +vendor_customer_orders_router = APIRouter( + prefix="/customers", + dependencies=[Depends(require_module_access("orders", FrontendType.VENDOR))], +) + + +# ============================================================================ +# Response Models +# ============================================================================ + + +class CustomerOrderItem(BaseModel): + """Order summary for customer order list.""" + + id: int + order_number: str + status: str + total: float + created_at: datetime + + class Config: + from_attributes = True + + +class CustomerOrdersResponse(BaseModel): + """Response for customer orders list.""" + + orders: list[CustomerOrderItem] + total: int + skip: int + limit: int + + +class CustomerOrderStatistic(BaseModel): + """Single statistic value.""" + + key: str + value: int | float | str + label: str + icon: str | None = None + unit: str | None = None + + +class CustomerOrderStatsResponse(BaseModel): + """Response for customer order statistics.""" + + customer_id: int + statistics: list[CustomerOrderStatistic] + + +# ============================================================================ +# Endpoints +# ============================================================================ + + +@vendor_customer_orders_router.get( + "/{customer_id}/orders", + response_model=CustomerOrdersResponse, +) +def get_customer_orders( + customer_id: int, + skip: int = Query(0, ge=0), + limit: int = Query(50, ge=1, le=100), + current_user: UserContext = Depends(get_current_vendor_api), + db: Session = Depends(get_db), +): + """ + Get order history for a specific customer. + + This endpoint is owned by the orders module because: + - Orders module owns the Order model and customer-order relationship + - Customers module is agnostic to what uses customers + + Similar to how catalog owns ProductMedia (product-media relationship) + while CMS provides generic MediaFile storage. + """ + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=current_user.token_vendor_id, + customer_id=customer_id, + skip=skip, + limit=limit, + ) + + return CustomerOrdersResponse( + orders=[ + CustomerOrderItem( + id=o.id, + order_number=o.order_number, + status=o.status, + total=o.total_cents / 100 if o.total_cents else 0, + created_at=o.created_at, + ) + for o in orders + ], + total=total, + skip=skip, + limit=limit, + ) + + +@vendor_customer_orders_router.get( + "/{customer_id}/order-stats", + response_model=CustomerOrderStatsResponse, +) +def get_customer_order_stats( + customer_id: int, + current_user: UserContext = Depends(get_current_vendor_api), + db: Session = Depends(get_db), +): + """ + Get order statistics for a specific customer. + + Uses the MetricsProvider pattern to provide customer-level order metrics. + Returns statistics like total orders, total spent, average order value, etc. + """ + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=current_user.token_vendor_id, + customer_id=customer_id, + ) + + return CustomerOrderStatsResponse( + customer_id=customer_id, + statistics=[ + CustomerOrderStatistic( + key=m.key, + value=m.value, + label=m.label, + icon=m.icon, + unit=m.unit, + ) + for m in metrics + ], + ) diff --git a/app/modules/orders/services/__init__.py b/app/modules/orders/services/__init__.py index abe285ba..ceb5df0e 100644 --- a/app/modules/orders/services/__init__.py +++ b/app/modules/orders/services/__init__.py @@ -25,6 +25,10 @@ from app.modules.orders.services.invoice_pdf_service import ( invoice_pdf_service, InvoicePDFService, ) +from app.modules.orders.services.customer_order_service import ( + customer_order_service, + CustomerOrderService, +) __all__ = [ "order_service", @@ -37,4 +41,6 @@ __all__ = [ "InvoiceService", "invoice_pdf_service", "InvoicePDFService", + "customer_order_service", + "CustomerOrderService", ] diff --git a/app/modules/orders/services/customer_order_service.py b/app/modules/orders/services/customer_order_service.py new file mode 100644 index 00000000..548d0f0d --- /dev/null +++ b/app/modules/orders/services/customer_order_service.py @@ -0,0 +1,122 @@ +# app/modules/orders/services/customer_order_service.py +""" +Customer-order service for managing customer-order relationships. + +This service handles the orders-specific logic for customer order operations. +It follows the principle that: +- Customers module provides generic customer storage (agnostic to consumers) +- Orders module defines how it relates to customers (owns the relationship) + +Similar to how: +- CMS provides generic media storage +- Catalog defines ProductMedia association +""" + +import logging + +from sqlalchemy.orm import Session + +from app.modules.orders.models import Order + +logger = logging.getLogger(__name__) + + +class CustomerOrderService: + """Service for customer-order operations owned by orders module.""" + + def get_customer_orders( + self, + db: Session, + vendor_id: int, + customer_id: int, + skip: int = 0, + limit: int = 50, + ) -> tuple[list[Order], int]: + """ + Get orders for a specific customer. + + Args: + db: Database session + vendor_id: Vendor ID (for ownership verification) + customer_id: Customer ID + skip: Pagination offset + limit: Pagination limit + + Returns: + Tuple of (orders, total_count) + """ + query = ( + db.query(Order) + .filter( + Order.customer_id == customer_id, + Order.vendor_id == vendor_id, + ) + .order_by(Order.created_at.desc()) + ) + + total = query.count() + orders = query.offset(skip).limit(limit).all() + + return orders, total + + def get_recent_orders( + self, + db: Session, + vendor_id: int, + customer_id: int, + limit: int = 5, + ) -> list[Order]: + """ + Get recent orders for a customer. + + Args: + db: Database session + vendor_id: Vendor ID + customer_id: Customer ID + limit: Maximum orders to return + + Returns: + List of recent orders + """ + return ( + db.query(Order) + .filter( + Order.customer_id == customer_id, + Order.vendor_id == vendor_id, + ) + .order_by(Order.created_at.desc()) + .limit(limit) + .all() + ) + + def get_order_count( + self, + db: Session, + vendor_id: int, + customer_id: int, + ) -> int: + """ + Get total order count for a customer. + + Args: + db: Database session + vendor_id: Vendor ID + customer_id: Customer ID + + Returns: + Total order count + """ + return ( + db.query(Order) + .filter( + Order.customer_id == customer_id, + Order.vendor_id == vendor_id, + ) + .count() + ) + + +# Singleton instance +customer_order_service = CustomerOrderService() + +__all__ = ["CustomerOrderService", "customer_order_service"] diff --git a/app/modules/orders/services/order_metrics.py b/app/modules/orders/services/order_metrics.py index a166818c..0f93a97a 100644 --- a/app/modules/orders/services/order_metrics.py +++ b/app/modules/orders/services/order_metrics.py @@ -302,6 +302,111 @@ class OrderMetricsProvider: return [] + def get_customer_order_metrics( + self, + db: Session, + vendor_id: int, + customer_id: int, + context: MetricsContext | None = None, + ) -> list[MetricValue]: + """ + Get order metrics for a specific customer. + + This is an entity-level method (not dashboard-level) that provides + order statistics for a specific customer. Used by customer detail pages. + + Args: + db: Database session + vendor_id: Vendor ID (for ownership verification) + customer_id: Customer ID + context: Optional filtering context + + Returns: + List of MetricValue objects for this customer's order activity + """ + from app.modules.orders.models import Order + + try: + # Base query for customer orders + base_query = db.query(Order).filter( + Order.customer_id == customer_id, + Order.vendor_id == vendor_id, + ) + + # Total orders + total_orders = base_query.count() + + # Revenue stats + revenue_query = db.query( + func.sum(Order.total_amount_cents).label("total_spent_cents"), + func.avg(Order.total_amount_cents).label("avg_order_cents"), + func.max(Order.created_at).label("last_order_date"), + func.min(Order.created_at).label("first_order_date"), + ).filter( + Order.customer_id == customer_id, + Order.vendor_id == vendor_id, + ) + + stats = revenue_query.first() + + total_spent_cents = stats.total_spent_cents or 0 + avg_order_cents = stats.avg_order_cents or 0 + last_order_date = stats.last_order_date + first_order_date = stats.first_order_date + + # Convert cents to currency + total_spent = total_spent_cents / 100 + avg_order_value = avg_order_cents / 100 if avg_order_cents else 0.0 + + return [ + MetricValue( + key="customer.total_orders", + value=total_orders, + label="Total Orders", + category="customer_orders", + icon="shopping-bag", + description="Total orders placed by this customer", + ), + MetricValue( + key="customer.total_spent", + value=round(total_spent, 2), + label="Total Spent", + category="customer_orders", + icon="currency-euro", + unit="EUR", + description="Total amount spent by this customer", + ), + MetricValue( + key="customer.avg_order_value", + value=round(avg_order_value, 2), + label="Avg Order Value", + category="customer_orders", + icon="calculator", + unit="EUR", + description="Average order value for this customer", + ), + MetricValue( + key="customer.last_order_date", + value=last_order_date.isoformat() if last_order_date else "", + label="Last Order", + category="customer_orders", + icon="calendar", + description="Date of most recent order", + ), + MetricValue( + key="customer.first_order_date", + value=first_order_date.isoformat() if first_order_date else "", + label="First Order", + category="customer_orders", + icon="calendar-plus", + description="Date of first order", + ), + ] + except Exception as e: + logger.warning(f"Failed to get customer order metrics: {e}") + return [] + + # Singleton instance order_metrics_provider = OrderMetricsProvider() diff --git a/app/modules/tenancy/routes/api/admin_platform_users.py b/app/modules/tenancy/routes/api/admin_platform_users.py index fadfa754..b7beb7cd 100644 --- a/app/modules/tenancy/routes/api/admin_platform_users.py +++ b/app/modules/tenancy/routes/api/admin_platform_users.py @@ -9,11 +9,12 @@ by the global exception handler. import logging -from fastapi import APIRouter, Body, Depends, Path, Query +from fastapi import APIRouter, Body, Depends, Path, Query, Request from sqlalchemy.orm import Session from app.api.deps import get_current_admin_api from app.core.database import get_db +from app.modules.core.services.stats_aggregator import stats_aggregator from app.modules.tenancy.services.admin_service import admin_service from models.schema.auth import UserContext from models.schema.auth import ( @@ -104,25 +105,60 @@ def create_user( ) +def _get_platform_id(request: Request, current_admin: UserContext) -> int: + """Get platform_id from available sources.""" + # From JWT token + if current_admin.token_platform_id: + return current_admin.token_platform_id + # From request state + platform = getattr(request.state, "platform", None) + if platform: + return platform.id + # First accessible platform + if current_admin.accessible_platform_ids: + return current_admin.accessible_platform_ids[0] + # Fallback + return 1 + + @admin_platform_users_router.get("/stats") def get_user_statistics( + request: Request, db: Session = Depends(get_db), current_admin: UserContext = Depends(get_current_admin_api), ): - """Get user statistics for admin dashboard (Admin only).""" - try: - from app.modules.analytics.services.stats_service import stats_service + """Get user statistics for admin dashboard (Admin only). - return stats_service.get_user_statistics(db) - except ImportError: - # Analytics module not available - return empty stats - logger.warning("Analytics module not available for user statistics") - return { - "total_users": 0, - "active_users": 0, - "inactive_users": 0, - "admin_users": 0, - } + Uses the stats_aggregator to get user metrics from the tenancy module's + MetricsProvider, ensuring no cross-module import violations. + """ + platform_id = _get_platform_id(request, current_admin) + + # Get metrics from stats_aggregator (tenancy module provides user stats) + metrics = stats_aggregator.get_admin_dashboard_stats(db=db, platform_id=platform_id) + + # Extract user stats from tenancy metrics + tenancy_metrics = metrics.get("tenancy", []) + + # Build response from metric values + stats = { + "total_users": 0, + "active_users": 0, + "inactive_users": 0, + "admin_users": 0, + } + + for metric in tenancy_metrics: + if metric.key == "tenancy.total_users": + stats["total_users"] = int(metric.value) + elif metric.key == "tenancy.active_users": + stats["active_users"] = int(metric.value) + elif metric.key == "tenancy.inactive_users": + stats["inactive_users"] = int(metric.value) + elif metric.key == "tenancy.admin_users": + stats["admin_users"] = int(metric.value) + + return stats @admin_platform_users_router.get("/search", response_model=UserSearchResponse) diff --git a/app/modules/tenancy/routes/pages/admin.py b/app/modules/tenancy/routes/pages/admin.py index af2b09f4..a2f8aa55 100644 --- a/app/modules/tenancy/routes/pages/admin.py +++ b/app/modules/tenancy/routes/pages/admin.py @@ -74,7 +74,7 @@ async def admin_company_detail_page( """ return templates.TemplateResponse( "tenancy/admin/company-detail.html", - get_admin_context(request, current_user, company_id=company_id), + get_admin_context(request, db, current_user, company_id=company_id), ) @@ -94,7 +94,7 @@ async def admin_company_edit_page( """ return templates.TemplateResponse( "tenancy/admin/company-edit.html", - get_admin_context(request, current_user, company_id=company_id), + get_admin_context(request, db, current_user, company_id=company_id), ) @@ -149,7 +149,7 @@ async def admin_vendor_detail_page( """ return templates.TemplateResponse( "tenancy/admin/vendor-detail.html", - get_admin_context(request, current_user, vendor_code=vendor_code), + get_admin_context(request, db, current_user, vendor_code=vendor_code), ) @@ -167,7 +167,7 @@ async def admin_vendor_edit_page( """ return templates.TemplateResponse( "tenancy/admin/vendor-edit.html", - get_admin_context(request, current_user, vendor_code=vendor_code), + get_admin_context(request, db, current_user, vendor_code=vendor_code), ) @@ -193,7 +193,7 @@ async def admin_vendor_domains_page( """ return templates.TemplateResponse( "tenancy/admin/vendor-domains.html", - get_admin_context(request, current_user, vendor_code=vendor_code), + get_admin_context(request, db, current_user, vendor_code=vendor_code), ) @@ -239,7 +239,7 @@ async def admin_vendor_theme_page( """ return templates.TemplateResponse( "tenancy/admin/vendor-theme.html", - get_admin_context(request, current_user, vendor_code=vendor_code), + get_admin_context(request, db, current_user, vendor_code=vendor_code), ) @@ -304,7 +304,7 @@ async def admin_user_detail_page( """ return templates.TemplateResponse( "tenancy/admin/admin-user-detail.html", - get_admin_context(request, current_user, user_id=user_id), + get_admin_context(request, db, current_user, user_id=user_id), ) @@ -325,7 +325,7 @@ async def admin_user_edit_page( """ return templates.TemplateResponse( "tenancy/admin/admin-user-edit.html", - get_admin_context(request, current_user, user_id=user_id), + get_admin_context(request, db, current_user, user_id=user_id), ) @@ -410,7 +410,7 @@ async def admin_platform_detail( """ return templates.TemplateResponse( "tenancy/admin/platform-detail.html", - get_admin_context(request, current_user, platform_code=platform_code), + get_admin_context(request, db, current_user, platform_code=platform_code), ) @@ -431,7 +431,7 @@ async def admin_platform_edit( """ return templates.TemplateResponse( "tenancy/admin/platform-edit.html", - get_admin_context(request, current_user, platform_code=platform_code), + get_admin_context(request, db, current_user, platform_code=platform_code), ) @@ -458,7 +458,7 @@ async def admin_platform_menu_config( return templates.TemplateResponse( "tenancy/admin/platform-menu-config.html", - get_admin_context(request, current_user, platform_code=platform_code), + get_admin_context(request, db, current_user, platform_code=platform_code), ) @@ -485,7 +485,7 @@ async def admin_platform_modules( return templates.TemplateResponse( "tenancy/admin/platform-modules.html", - get_admin_context(request, current_user, platform_code=platform_code), + get_admin_context(request, db, current_user, platform_code=platform_code), ) diff --git a/docs/architecture/audit-provider-pattern.md b/docs/architecture/audit-provider-pattern.md new file mode 100644 index 00000000..8d3da7a9 --- /dev/null +++ b/docs/architecture/audit-provider-pattern.md @@ -0,0 +1,265 @@ +# Audit Provider Pattern + +The audit provider pattern enables modules to provide audit logging backends without creating cross-module dependencies. This allows the monitoring module to be truly optional while still providing audit logging when enabled. + +## Overview + +``` +┌─────────────────────────────────────────────────────────────────────┐ +│ Admin Action Request │ +│ (Settings update, User management, etc.) │ +└─────────────────────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────────────┐ +│ AuditAggregatorService │ +│ (app/modules/core/services/audit_aggregator.py) │ +│ │ +│ • Discovers AuditProviders from all registered modules │ +│ • Calls log_action() on all available providers │ +│ • Handles graceful degradation if no providers available │ +└─────────────────────────────────────────────────────────────────────┘ + │ + ┌───────────────┼───────────────┐ + ▼ ▼ ▼ + ┌───────────────┐ ┌───────────────┐ ┌───────────────┐ + │ DatabaseAudit │ │ (Future) │ │ (Future) │ + │ Provider │ │ FileProvider │ │ CloudWatch │ + │ (monitoring) │ │ │ │ │ + └───────────────┘ └───────────────┘ └───────────────┘ + │ + ▼ + ┌───────────────┐ + │AdminAuditLog │ + │ (table) │ + └───────────────┘ +``` + +## Problem Solved + +Before this pattern, core modules had **hard imports** from the monitoring module: + +```python +# BAD: Core module with hard dependency on optional/internal module +from app.modules.monitoring.services.admin_audit_service import admin_audit_service + +def create_setting(...): + result = settings_service.create_setting(...) + admin_audit_service.log_action(...) # Crashes if monitoring disabled! +``` + +This violated the architecture rule: **Core modules cannot depend on optional modules.** + +## Solution: Protocol-Based Audit Logging + +The monitoring module implements `AuditProviderProtocol` and registers it in its `definition.py`. The `AuditAggregatorService` in core discovers and uses audit providers from all modules. + +## Key Components + +### 1. AuditEvent Dataclass + +Standard structure for audit events: + +```python +# app/modules/contracts/audit.py +from dataclasses import dataclass +from typing import Any + +@dataclass +class AuditEvent: + admin_user_id: int # ID of admin performing action + action: str # Action name (e.g., "create_setting") + target_type: str # Target type (e.g., "setting", "user") + target_id: str # Target identifier + details: dict[str, Any] | None = None # Additional context + ip_address: str | None = None # Admin's IP address + user_agent: str | None = None # Browser user agent + request_id: str | None = None # Request correlation ID +``` + +### 2. AuditProviderProtocol + +Protocol that modules implement: + +```python +# app/modules/contracts/audit.py +from typing import Protocol, runtime_checkable + +@runtime_checkable +class AuditProviderProtocol(Protocol): + @property + def audit_backend(self) -> str: + """Backend name (e.g., 'database', 'file', 'cloudwatch').""" + ... + + def log_action(self, db: Session, event: AuditEvent) -> bool: + """Log an audit event. Returns True on success.""" + ... +``` + +### 3. AuditAggregatorService + +The aggregator in core that discovers and uses providers: + +```python +# app/modules/core/services/audit_aggregator.py + +class AuditAggregatorService: + def _get_enabled_providers(self, db: Session): + """Discover audit providers from registered modules.""" + from app.modules.registry import MODULES + + for module in MODULES.values(): + if module.has_audit_provider(): + provider = module.get_audit_provider_instance() + if provider: + yield (module, provider) + + def log_action(self, db: Session, event: AuditEvent) -> bool: + """Log to all available providers.""" + providers = list(self._get_enabled_providers(db)) + + if not providers: + # No providers - acceptable, audit is optional + return False + + any_success = False + for module, provider in providers: + try: + if provider.log_action(db, event): + any_success = True + except Exception as e: + logger.warning(f"Audit provider {module.code} failed: {e}") + + return any_success + + def log(self, db, admin_user_id, action, target_type, target_id, **kwargs): + """Convenience method with individual parameters.""" + event = AuditEvent( + admin_user_id=admin_user_id, + action=action, + target_type=target_type, + target_id=str(target_id), + **kwargs + ) + return self.log_action(db, event) +``` + +## Implementing a Provider + +### Step 1: Create the Provider + +```python +# app/modules/monitoring/services/audit_provider.py + +from app.modules.contracts.audit import AuditEvent, AuditProviderProtocol +from app.modules.tenancy.models import AdminAuditLog + +class DatabaseAuditProvider: + @property + def audit_backend(self) -> str: + return "database" + + def log_action(self, db: Session, event: AuditEvent) -> bool: + try: + audit_log = AdminAuditLog( + admin_user_id=event.admin_user_id, + action=event.action, + target_type=event.target_type, + target_id=event.target_id, + details=event.details or {}, + ip_address=event.ip_address, + user_agent=event.user_agent, + request_id=event.request_id, + ) + db.add(audit_log) + db.flush() + return True + except Exception as e: + logger.error(f"Failed to log audit: {e}") + return False + +audit_provider = DatabaseAuditProvider() +``` + +### Step 2: Register in Module Definition + +```python +# app/modules/monitoring/definition.py + +def _get_audit_provider(): + """Lazy import to avoid circular imports.""" + from app.modules.monitoring.services.audit_provider import audit_provider + return audit_provider + +monitoring_module = ModuleDefinition( + code="monitoring", + name="Platform Monitoring", + audit_provider=_get_audit_provider, # Register the provider + ... +) +``` + +### Step 3: Use via Aggregator + +```python +# app/modules/core/routes/api/admin_settings.py + +from app.modules.core.services.audit_aggregator import audit_aggregator + +@router.post("/settings") +def create_setting(setting_data: SettingCreate, ...): + result = settings_service.create_setting(db, setting_data) + + # Log via aggregator - works even if monitoring is disabled + audit_aggregator.log( + db=db, + admin_user_id=current_admin.id, + action="create_setting", + target_type="setting", + target_id=setting_data.key, + details={"category": setting_data.category}, + ) + + db.commit() + return result +``` + +## Graceful Degradation + +When no audit providers are registered (e.g., monitoring module disabled): + +- `audit_aggregator.log()` returns `False` +- No exceptions are raised +- The operation continues normally +- A debug log message notes the absence of providers + +This ensures core functionality works regardless of whether auditing is available. + +## Multiple Backends + +The pattern supports multiple audit backends simultaneously: + +```python +# All registered providers receive the event +audit_aggregator.log(...) + +# Provider 1: Database (AdminAuditLog table) +# Provider 2: CloudWatch Logs (if aws-monitoring module enabled) +# Provider 3: File-based audit log (if file-audit module enabled) +``` + +## Benefits + +1. **Decoupling**: Core modules don't depend on monitoring +2. **Optional Auditing**: Monitoring can be disabled without breaking the app +3. **Extensibility**: Easy to add new audit backends (file, cloud, SIEM) +4. **Testability**: Can mock audit providers in tests +5. **Fault Tolerance**: One provider failure doesn't affect others + +## Related Documentation + +- [Cross-Module Import Rules](cross-module-import-rules.md) - Import restrictions +- [Metrics Provider Pattern](metrics-provider-pattern.md) - Similar pattern for statistics +- [Widget Provider Pattern](widget-provider-pattern.md) - Similar pattern for dashboard widgets +- [Module System Architecture](module-system.md) - Module structure and auto-discovery diff --git a/docs/architecture/customer-orders-architecture.md b/docs/architecture/customer-orders-architecture.md new file mode 100644 index 00000000..7e352c3d --- /dev/null +++ b/docs/architecture/customer-orders-architecture.md @@ -0,0 +1,345 @@ +# Customer-Orders Architecture + +This document describes the consumer-agnostic customer architecture, following the same pattern as [Media Architecture](media-architecture.md). + +## Overview + +``` +┌─────────────────────────────────────────────────────────────────────┐ +│ CONSUMER MODULES │ +│ │ +│ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │ +│ │ Orders │ │ Loyalty │ │ Future │ │ +│ │ │ │ (future) │ │ Module │ │ +│ │ Order model │ │LoyaltyPoints│ │ XxxCustomer │ │ +│ │ (customer_id)│ │(customer_id)│ │ │ │ +│ └──────┬──────┘ └──────┬──────┘ └──────┬──────┘ │ +│ │ │ │ │ +│ └──────────────────┼──────────────────┘ │ +│ │ │ +│ ▼ │ +│ ┌─────────────────────────────────────────────────────────────┐ │ +│ │ Customers Module │ │ +│ │ │ │ +│ │ Customer (generic, consumer-agnostic storage) │ │ +│ │ CustomerService (CRUD, authentication, profile management) │ │ +│ └─────────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────┘ +``` + +## Design Principles + +### 1. Consumer-Agnostic Customer Storage + +The customers module provides **generic customer storage** without knowing what entities will reference customers: + +- `Customer` stores customer data (email, name, addresses, preferences) +- `CustomerService` handles CRUD, authentication, and profile management +- Customers module has **no knowledge** of orders, loyalty points, or any specific consumers + +### 2. Consumer-Owned Relationships + +Each module that references customers defines its **own relationship**: + +- **Orders**: `Order.customer_id` links orders to customers +- **Future Loyalty**: Would define `LoyaltyPoints.customer_id` +- **Future Subscriptions**: Would define `Subscription.customer_id` + +This follows the principle: **The consumer knows what it needs, the provider doesn't need to know who uses it.** + +### 3. Correct Dependency Direction + +``` +WRONG (Hidden Dependency): + Customers → Orders (customers imports Order model) + +CORRECT: + Orders → Customers (orders references Customer via FK) +``` + +Optional modules (orders) depend on core modules (customers), never the reverse. + +## Key Components + +### Customer Model (Customers Module) + +```python +# app/modules/customers/models/customer.py + +class Customer(Base, TimestampMixin): + """Generic customer - consumer-agnostic.""" + + __tablename__ = "customers" + + id = Column(Integer, primary_key=True) + vendor_id = Column(Integer, ForeignKey("vendors.id"), nullable=False) + + # Authentication + email = Column(String(255), nullable=False) + hashed_password = Column(String(255)) + + # Profile + first_name = Column(String(100)) + last_name = Column(String(100)) + phone = Column(String(50)) + customer_number = Column(String(50), unique=True) + + # Preferences + marketing_consent = Column(Boolean, default=False) + preferred_language = Column(String(10)) + + # Status + is_active = Column(Boolean, default=True) + + # Note: Consumer-specific relationships (orders, loyalty points, etc.) + # are defined in their respective modules. Customers module doesn't + # know about specific consumers. +``` + +### CustomerService (Customers Module) + +The `CustomerService` provides generic operations: + +```python +# app/modules/customers/services/customer_service.py + +class CustomerService: + """Generic customer operations - consumer-agnostic.""" + + def create_customer(self, db, vendor_id, customer_data): + """Create a new customer.""" + ... + + def get_customer(self, db, vendor_id, customer_id): + """Get a customer by ID.""" + ... + + def update_customer(self, db, vendor_id, customer_id, customer_data): + """Update customer profile.""" + ... + + def login_customer(self, db, vendor_id, email, password): + """Authenticate a customer.""" + ... + + # Note: Customer order methods have been moved to the orders module. + # Use orders.services.customer_order_service for order-related operations. +``` + +### Order Model (Orders Module) + +```python +# app/modules/orders/models/order.py + +class Order(Base, TimestampMixin): + """Order with customer reference - orders owns the relationship.""" + + __tablename__ = "orders" + + id = Column(Integer, primary_key=True) + vendor_id = Column(Integer, ForeignKey("vendors.id"), nullable=False) + + # Customer reference - orders module owns this relationship + customer_id = Column(Integer, ForeignKey("customers.id")) + + # Order data + order_number = Column(String(50), unique=True) + status = Column(String(20), default="pending") + total_cents = Column(Integer) + ... + + # Relationship to customer + customer = relationship("Customer", lazy="joined") +``` + +### CustomerOrderService (Orders Module) + +```python +# app/modules/orders/services/customer_order_service.py + +class CustomerOrderService: + """Customer-order operations - owned by orders module.""" + + def get_customer_orders(self, db, vendor_id, customer_id, skip=0, limit=50): + """Get orders for a specific customer.""" + ... + + def get_recent_orders(self, db, vendor_id, customer_id, limit=5): + """Get recent orders for a customer.""" + ... + + def get_order_count(self, db, vendor_id, customer_id): + """Get total order count for a customer.""" + ... +``` + +### Customer Order Metrics (Orders Module) + +Order statistics for customers use the MetricsProvider pattern: + +```python +# app/modules/orders/services/order_metrics.py + +class OrderMetricsProvider: + """Metrics provider including customer-level order metrics.""" + + def get_customer_order_metrics(self, db, vendor_id, customer_id, context=None): + """ + Get order metrics for a specific customer. + + Returns MetricValue objects for: + - total_orders: Total orders placed + - total_spent: Total amount spent + - avg_order_value: Average order value + - last_order_date: Date of most recent order + - first_order_date: Date of first order + """ + ... +``` + +## API Endpoints + +### Customers Module Endpoints + +Customer CRUD operations (no order data): + +``` +GET /api/vendor/customers → List customers +GET /api/vendor/customers/{id} → Customer details (no order stats) +PUT /api/vendor/customers/{id} → Update customer +PUT /api/vendor/customers/{id}/status → Toggle active status +``` + +### Orders Module Endpoints + +Customer order data (owned by orders): + +``` +GET /api/vendor/customers/{id}/orders → Customer's order history +GET /api/vendor/customers/{id}/order-stats → Customer's order statistics +``` + +## Adding Customer References to a New Module + +When creating a module that references customers (e.g., a loyalty module): + +### Step 1: Reference Customer via Foreign Key + +```python +# app/modules/loyalty/models/loyalty_points.py + +class LoyaltyPoints(Base): + """Loyalty points - owned by loyalty module.""" + + __tablename__ = "loyalty_points" + + id = Column(Integer, primary_key=True) + vendor_id = Column(Integer, ForeignKey("vendors.id")) + + # Reference to customer - loyalty module owns this + customer_id = Column(Integer, ForeignKey("customers.id")) + + points_balance = Column(Integer, default=0) + tier = Column(String(20), default="bronze") + + # Relationship + customer = relationship("Customer", lazy="joined") +``` + +### Step 2: Create Your Service + +```python +# app/modules/loyalty/services/customer_loyalty_service.py + +class CustomerLoyaltyService: + """Customer loyalty operations - owned by loyalty module.""" + + def get_customer_points(self, db, vendor_id, customer_id): + """Get loyalty points for a customer.""" + ... + + def add_points(self, db, vendor_id, customer_id, points, reason): + """Add points to customer's balance.""" + ... +``` + +### Step 3: Add Routes in Your Module + +```python +# app/modules/loyalty/routes/api/vendor.py + +@router.get("/customers/{customer_id}/loyalty") +def get_customer_loyalty(customer_id: int, ...): + """Get loyalty information for a customer.""" + return loyalty_service.get_customer_points(db, vendor_id, customer_id) +``` + +## Benefits of This Architecture + +1. **Module Independence**: Orders can be disabled without affecting customers +2. **Extensibility**: New modules easily reference customers +3. **No Hidden Dependencies**: Dependencies flow in one direction +4. **Clean Separation**: Customers handles identity, consumers handle their domain +5. **Testability**: Can test customers without any consumer modules +6. **Single Responsibility**: Each module owns its domain + +## Anti-Patterns to Avoid + +### Don't: Import Consumer Models in Customers + +```python +# BAD - Creates hidden dependency +class CustomerService: + def get_customer_orders(self, db, customer_id): + from app.modules.orders.models import Order # Wrong! + return db.query(Order).filter(Order.customer_id == customer_id).all() +``` + +### Don't: Add Consumer-Specific Fields to Customer + +```python +# BAD - Customer shouldn't know about orders +class Customer(Base): + # These create coupling to orders module + total_orders = Column(Integer) # Wrong approach + last_order_date = Column(DateTime) # Wrong approach +``` + +Instead, query order data from the orders module when needed. + +### Don't: Put Consumer Routes in Customers Module + +```python +# BAD - customers/routes shouldn't serve order data +@router.get("/customers/{id}/orders") +def get_customer_orders(customer_id: int): + from app.modules.orders.models import Order # Wrong! + ... +``` + +## Migration Note + +Previously, the customers module had methods that imported from orders: + +```python +# OLD (removed) +class CustomerService: + def get_customer_orders(self, db, vendor_id, customer_id): + from app.modules.orders.models import Order # Lazy import + ... + + def get_customer_statistics(self, db, vendor_id, customer_id): + from app.modules.orders.models import Order # Lazy import + ... +``` + +These have been moved to the orders module: +- `get_customer_orders()` → `orders.services.customer_order_service` +- `get_customer_statistics()` → `orders.services.order_metrics.get_customer_order_metrics()` + +## Related Documentation + +- [Media Architecture](media-architecture.md) - Similar pattern for media files +- [Module System Architecture](module-system.md) - Module structure and dependencies +- [Cross-Module Import Rules](cross-module-import-rules.md) - Import restrictions +- [Metrics Provider Pattern](metrics-provider-pattern.md) - Provider pattern for statistics diff --git a/docs/architecture/media-architecture.md b/docs/architecture/media-architecture.md new file mode 100644 index 00000000..cd7cca09 --- /dev/null +++ b/docs/architecture/media-architecture.md @@ -0,0 +1,298 @@ +# Media Management Architecture + +This document describes the consumer-agnostic media architecture used in the platform. + +## Overview + +``` +┌─────────────────────────────────────────────────────────────────────┐ +│ CONSUMER MODULES │ +│ │ +│ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐ │ +│ │ Catalog │ │ Art Gallery │ │ Future │ │ +│ │ │ │ (future) │ │ Module │ │ +│ │ ProductMedia│ │ GalleryMedia│ │ XxxMedia │ │ +│ └──────┬──────┘ └──────┬──────┘ └──────┬──────┘ │ +│ │ │ │ │ +│ └──────────────────┼──────────────────┘ │ +│ │ │ +│ ▼ │ +│ ┌─────────────────────────────────────────────────────────────┐ │ +│ │ CMS Module │ │ +│ │ │ │ +│ │ MediaFile (generic, consumer-agnostic storage) │ │ +│ │ MediaService (upload, download, metadata management) │ │ +│ └─────────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────┘ +``` + +## Design Principles + +### 1. Consumer-Agnostic Storage + +The CMS module provides **generic media storage** without knowing what entities will use the media files. This means: + +- `MediaFile` stores file metadata (path, size, type, dimensions) +- `MediaService` handles upload, download, and file operations +- CMS has **no knowledge** of products, galleries, or any specific consumers + +### 2. Consumer-Owned Associations + +Each module that needs media defines its **own association table**: + +- **Catalog**: `ProductMedia` links products to media files +- **Future Art Gallery**: Would define `GalleryMedia` +- **Future Blog**: Would define `PostMedia` + +This follows the principle: **The consumer knows what it needs, the provider doesn't need to know who uses it.** + +### 3. Correct Dependency Direction + +``` +WRONG (Hidden Dependency): + CMS → Catalog (CMS knows about Product) + +CORRECT: + Catalog → CMS (Catalog uses MediaFile) +``` + +Optional modules (catalog) depend on core modules (cms), never the reverse. + +## Key Components + +### MediaFile Model (CMS) + +```python +# app/modules/cms/models/media.py + +class MediaFile(Base, TimestampMixin): + """Generic vendor media file - consumer-agnostic.""" + + __tablename__ = "media_files" + + id = Column(Integer, primary_key=True) + vendor_id = Column(Integer, ForeignKey("vendors.id"), nullable=False) + + # File identification + filename = Column(String(255), nullable=False) # UUID-based + original_filename = Column(String(255)) + file_path = Column(String(500), nullable=False) + + # File properties + media_type = Column(String(20), nullable=False) # image, video, document + mime_type = Column(String(100)) + file_size = Column(Integer) + + # Image/video dimensions + width = Column(Integer) + height = Column(Integer) + + # Metadata + alt_text = Column(String(500)) + description = Column(Text) + folder = Column(String(100), default="general") + + # Usage tracking (updated by consumers) + usage_count = Column(Integer, default=0) + + # Note: Consumer-specific associations (ProductMedia, etc.) are defined + # in their respective modules. CMS doesn't know about specific consumers. +``` + +### MediaService (CMS) + +The `MediaService` provides generic operations: + +```python +# app/modules/cms/services/media_service.py + +class MediaService: + """Generic media operations - consumer-agnostic.""" + + async def upload_file(self, db, vendor_id, file_content, filename, folder="general"): + """Upload a file to vendor's media library.""" + ... + + def get_media(self, db, vendor_id, media_id): + """Get a media file by ID.""" + ... + + def get_media_library(self, db, vendor_id, skip=0, limit=100, **filters): + """List vendor's media files with filtering.""" + ... + + def update_media_metadata(self, db, vendor_id, media_id, **metadata): + """Update file metadata (alt_text, description, etc.).""" + ... + + def delete_media(self, db, vendor_id, media_id): + """Delete a media file.""" + ... +``` + +### ProductMedia Model (Catalog) + +```python +# app/modules/catalog/models/product_media.py + +class ProductMedia(Base, TimestampMixin): + """Product-specific media association - owned by catalog.""" + + __tablename__ = "product_media" + + id = Column(Integer, primary_key=True) + product_id = Column(Integer, ForeignKey("products.id", ondelete="CASCADE")) + media_id = Column(Integer, ForeignKey("media_files.id", ondelete="CASCADE")) + + # Usage type: main_image, gallery, variant, thumbnail, swatch + usage_type = Column(String(50), nullable=False, default="gallery") + display_order = Column(Integer, default=0) + variant_id = Column(Integer) # For variant-specific images + + # Relationships + product = relationship("Product", back_populates="media_associations") + media = relationship("MediaFile", lazy="joined") +``` + +### ProductMediaService (Catalog) + +```python +# app/modules/catalog/services/product_media_service.py + +class ProductMediaService: + """Product-media association operations - catalog-specific.""" + + def attach_media_to_product(self, db, vendor_id, product_id, media_id, + usage_type="gallery", display_order=0): + """Attach a media file to a product.""" + # Verify ownership, create ProductMedia association + ... + + def detach_media_from_product(self, db, vendor_id, product_id, media_id, + usage_type=None): + """Detach media from a product.""" + ... + + def get_product_media(self, db, product_id, usage_type=None): + """Get media associations for a product.""" + ... + + def set_main_image(self, db, vendor_id, product_id, media_id): + """Set the main image for a product.""" + ... +``` + +## Adding Media Support to a New Module + +When creating a new module that needs media (e.g., an art gallery module): + +### Step 1: Create Your Association Model + +```python +# app/modules/art_gallery/models/gallery_media.py + +from sqlalchemy import Column, ForeignKey, Integer, String +from sqlalchemy.orm import relationship +from app.core.database import Base + +class GalleryMedia(Base): + """Gallery-specific media association.""" + + __tablename__ = "gallery_media" + + id = Column(Integer, primary_key=True) + artwork_id = Column(Integer, ForeignKey("artworks.id", ondelete="CASCADE")) + media_id = Column(Integer, ForeignKey("media_files.id", ondelete="CASCADE")) + + # Gallery-specific fields + is_primary = Column(Boolean, default=False) + caption = Column(String(500)) + + # Relationships + artwork = relationship("Artwork", back_populates="media_associations") + media = relationship("MediaFile", lazy="joined") +``` + +### Step 2: Create Your Service + +```python +# app/modules/art_gallery/services/gallery_media_service.py + +from app.modules.cms.models import MediaFile +from app.modules.art_gallery.models import GalleryMedia, Artwork + +class GalleryMediaService: + def attach_media_to_artwork(self, db, artist_id, artwork_id, media_id, **kwargs): + # Verify artwork belongs to artist + # Verify media belongs to artist (vendor_id) + # Create GalleryMedia association + ... +``` + +### Step 3: Use CMS MediaService for File Operations + +```python +from app.modules.cms.services.media_service import media_service + +# Upload a new file +media_file = await media_service.upload_file( + db=db, + vendor_id=artist_id, + file_content=file_bytes, + filename="artwork.jpg", + folder="artworks", +) + +# Then attach it to your entity +gallery_media_service.attach_media_to_artwork( + db=db, + artist_id=artist_id, + artwork_id=artwork.id, + media_id=media_file.id, + is_primary=True, +) +``` + +## Benefits of This Architecture + +1. **Module Independence**: Catalog can be disabled without affecting CMS +2. **Extensibility**: New modules easily add media support +3. **No Hidden Dependencies**: Dependencies flow in one direction +4. **Clean Separation**: CMS handles storage, consumers handle associations +5. **Testability**: Can test CMS without any consumer modules +6. **Single Responsibility**: Each module owns its domain + +## Anti-Patterns to Avoid + +### Don't: Add Consumer References to MediaFile + +```python +# BAD - Creates hidden dependency +class MediaFile(Base): + # CMS now depends on catalog! + product_associations = relationship("ProductMedia", ...) +``` + +### Don't: Put Consumer Logic in MediaService + +```python +# BAD - CMS shouldn't know about products +class MediaService: + def attach_to_product(self, db, product_id, media_id): + from app.modules.catalog.models import ProductMedia # Wrong! +``` + +### Don't: Query Consumer Models from CMS + +```python +# BAD - Hidden dependency +def get_media_usage(self, db, media_id): + from app.modules.catalog.models import ProductMedia # Wrong! + return db.query(ProductMedia).filter(...).all() +``` + +## Related Documentation + +- [Module System Architecture](module-system.md) - Module structure and dependencies +- [Cross-Module Import Rules](cross-module-import-rules.md) - Import restrictions +- [Audit Provider Pattern](audit-provider-pattern.md) - Similar provider pattern for audit logging diff --git a/tests/unit/services/test_customer_order_service.py b/tests/unit/services/test_customer_order_service.py new file mode 100644 index 00000000..1e8d30e8 --- /dev/null +++ b/tests/unit/services/test_customer_order_service.py @@ -0,0 +1,228 @@ +# tests/unit/services/test_customer_order_service.py +""" +Unit tests for CustomerOrderService. + +Tests the orders module's customer-order relationship operations. +This service owns the customer-order relationship (customers module is agnostic). +""" + +from datetime import UTC, datetime + +import pytest + +from app.modules.orders.models import Order +from app.modules.orders.services.customer_order_service import CustomerOrderService + + +@pytest.fixture +def customer_order_service(): + """Create CustomerOrderService instance.""" + return CustomerOrderService() + + +@pytest.fixture +def customer_with_orders(db, test_vendor, test_customer): + """Create a customer with multiple orders.""" + orders = [] + first_name = test_customer.first_name or "Test" + last_name = test_customer.last_name or "Customer" + + for i in range(5): + order = Order( + vendor_id=test_vendor.id, + customer_id=test_customer.id, + order_number=f"ORD-{i:04d}", + status="pending" if i < 2 else "completed", + channel="direct", + order_date=datetime.now(UTC), + subtotal_cents=1000 * (i + 1), + total_amount_cents=1000 * (i + 1), + currency="EUR", + # Customer info + customer_email=test_customer.email, + customer_first_name=first_name, + customer_last_name=last_name, + # Shipping address + ship_first_name=first_name, + ship_last_name=last_name, + ship_address_line_1="123 Test St", + ship_city="Luxembourg", + ship_postal_code="L-1234", + ship_country_iso="LU", + # Billing address + bill_first_name=first_name, + bill_last_name=last_name, + bill_address_line_1="123 Test St", + bill_city="Luxembourg", + bill_postal_code="L-1234", + bill_country_iso="LU", + ) + db.add(order) + orders.append(order) + + db.commit() + for order in orders: + db.refresh(order) + + return test_customer, orders + + +@pytest.mark.unit +class TestCustomerOrderServiceGetOrders: + """Tests for get_customer_orders method.""" + + def test_get_customer_orders_empty( + self, db, customer_order_service, test_vendor, test_customer + ): + """Test getting orders when customer has none.""" + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=test_customer.id, + ) + + assert orders == [] + assert total == 0 + + def test_get_customer_orders_with_data( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test getting orders when customer has orders.""" + customer, _ = customer_with_orders + + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + assert total == 5 + assert len(orders) == 5 + + def test_get_customer_orders_pagination( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test pagination of customer orders.""" + customer, _ = customer_with_orders + + # Get first page + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + skip=0, + limit=2, + ) + + assert total == 5 + assert len(orders) == 2 + + # Get second page + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + skip=2, + limit=2, + ) + + assert total == 5 + assert len(orders) == 2 + + def test_get_customer_orders_ordered_by_date( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test that orders are returned in descending date order.""" + customer, created_orders = customer_with_orders + + orders, _ = customer_order_service.get_customer_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + # Most recent should be first + for i in range(len(orders) - 1): + assert orders[i].created_at >= orders[i + 1].created_at + + def test_get_customer_orders_wrong_vendor( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test that orders from wrong vendor are not returned.""" + customer, _ = customer_with_orders + + # Use non-existent vendor ID + orders, total = customer_order_service.get_customer_orders( + db=db, + vendor_id=99999, + customer_id=customer.id, + ) + + assert orders == [] + assert total == 0 + + +@pytest.mark.unit +class TestCustomerOrderServiceRecentOrders: + """Tests for get_recent_orders method.""" + + def test_get_recent_orders( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test getting recent orders.""" + customer, _ = customer_with_orders + + orders = customer_order_service.get_recent_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + limit=3, + ) + + assert len(orders) == 3 + + def test_get_recent_orders_respects_limit( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test that limit is respected.""" + customer, _ = customer_with_orders + + orders = customer_order_service.get_recent_orders( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + limit=2, + ) + + assert len(orders) == 2 + + +@pytest.mark.unit +class TestCustomerOrderServiceOrderCount: + """Tests for get_order_count method.""" + + def test_get_order_count_zero( + self, db, customer_order_service, test_vendor, test_customer + ): + """Test count when customer has no orders.""" + count = customer_order_service.get_order_count( + db=db, + vendor_id=test_vendor.id, + customer_id=test_customer.id, + ) + + assert count == 0 + + def test_get_order_count_with_orders( + self, db, customer_order_service, test_vendor, customer_with_orders + ): + """Test count when customer has orders.""" + customer, _ = customer_with_orders + + count = customer_order_service.get_order_count( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + assert count == 5 diff --git a/tests/unit/services/test_order_metrics_customer.py b/tests/unit/services/test_order_metrics_customer.py new file mode 100644 index 00000000..14f42aaa --- /dev/null +++ b/tests/unit/services/test_order_metrics_customer.py @@ -0,0 +1,183 @@ +# tests/unit/services/test_order_metrics_customer.py +""" +Unit tests for OrderMetricsProvider customer metrics. + +Tests the get_customer_order_metrics method which provides +customer-level order statistics using the MetricsProvider pattern. +""" + +from datetime import UTC, datetime + +import pytest + +from app.modules.orders.models import Order +from app.modules.orders.services.order_metrics import OrderMetricsProvider + + +@pytest.fixture +def order_metrics_provider(): + """Create OrderMetricsProvider instance.""" + return OrderMetricsProvider() + + +@pytest.fixture +def customer_with_orders(db, test_vendor, test_customer): + """Create a customer with multiple orders for metrics testing.""" + orders = [] + first_name = test_customer.first_name or "Test" + last_name = test_customer.last_name or "Customer" + + for i in range(3): + order = Order( + vendor_id=test_vendor.id, + customer_id=test_customer.id, + order_number=f"METRICS-{i:04d}", + status="completed", + channel="direct", + order_date=datetime.now(UTC), + subtotal_cents=1000 * (i + 1), # 1000, 2000, 3000 + total_amount_cents=1000 * (i + 1), + currency="EUR", + # Customer info + customer_email=test_customer.email, + customer_first_name=first_name, + customer_last_name=last_name, + # Shipping address + ship_first_name=first_name, + ship_last_name=last_name, + ship_address_line_1="123 Test St", + ship_city="Luxembourg", + ship_postal_code="L-1234", + ship_country_iso="LU", + # Billing address + bill_first_name=first_name, + bill_last_name=last_name, + bill_address_line_1="123 Test St", + bill_city="Luxembourg", + bill_postal_code="L-1234", + bill_country_iso="LU", + ) + db.add(order) + orders.append(order) + + db.commit() + for order in orders: + db.refresh(order) + + return test_customer, orders + + +@pytest.mark.unit +class TestOrderMetricsProviderCustomerMetrics: + """Tests for get_customer_order_metrics method.""" + + def test_get_customer_metrics_no_orders( + self, db, order_metrics_provider, test_vendor, test_customer + ): + """Test metrics when customer has no orders.""" + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=test_vendor.id, + customer_id=test_customer.id, + ) + + # Should return metrics even with no orders + assert len(metrics) > 0 + + # Find total_orders metric + total_orders_metric = next( + (m for m in metrics if m.key == "customer.total_orders"), None + ) + assert total_orders_metric is not None + assert total_orders_metric.value == 0 + + def test_get_customer_metrics_with_orders( + self, db, order_metrics_provider, test_vendor, customer_with_orders + ): + """Test metrics when customer has orders.""" + customer, orders = customer_with_orders + + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + # Check total orders + total_orders = next( + (m for m in metrics if m.key == "customer.total_orders"), None + ) + assert total_orders is not None + assert total_orders.value == 3 + + # Check total spent (1000 + 2000 + 3000 = 6000 cents = 60.00) + total_spent = next( + (m for m in metrics if m.key == "customer.total_spent"), None + ) + assert total_spent is not None + assert total_spent.value == 60.0 + + # Check average order value (6000 / 3 = 2000 cents = 20.00) + avg_value = next( + (m for m in metrics if m.key == "customer.avg_order_value"), None + ) + assert avg_value is not None + assert avg_value.value == 20.0 + + def test_get_customer_metrics_has_required_fields( + self, db, order_metrics_provider, test_vendor, customer_with_orders + ): + """Test that all required metric fields are present.""" + customer, _ = customer_with_orders + + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + expected_keys = [ + "customer.total_orders", + "customer.total_spent", + "customer.avg_order_value", + "customer.last_order_date", + "customer.first_order_date", + ] + + metric_keys = [m.key for m in metrics] + for key in expected_keys: + assert key in metric_keys, f"Missing metric: {key}" + + def test_get_customer_metrics_has_labels_and_icons( + self, db, order_metrics_provider, test_vendor, customer_with_orders + ): + """Test that metrics have display metadata.""" + customer, _ = customer_with_orders + + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=test_vendor.id, + customer_id=customer.id, + ) + + for metric in metrics: + assert metric.label, f"Metric {metric.key} missing label" + assert metric.category == "customer_orders" + + def test_get_customer_metrics_wrong_vendor( + self, db, order_metrics_provider, customer_with_orders + ): + """Test metrics with wrong vendor returns zero values.""" + customer, _ = customer_with_orders + + metrics = order_metrics_provider.get_customer_order_metrics( + db=db, + vendor_id=99999, # Non-existent vendor + customer_id=customer.id, + ) + + total_orders = next( + (m for m in metrics if m.key == "customer.total_orders"), None + ) + assert total_orders is not None + assert total_orders.value == 0