sboulahtit/orion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: standardize modular architecture patterns

Browse Source 
- Rename module definition variables to follow naming convention:
  - catalog/definition.py: module → catalog_module
  - checkout/definition.py: module → checkout_module
  - cart/definition.py: module → cart_module

- Add router attachment functions for lazy loading:
  - get_catalog_module_with_routers()
  - get_checkout_module_with_routers()
  - get_cart_module_with_routers()

- Move billing exceptions to exceptions.py:
  - Add backwards-compatible aliases (BillingServiceError, etc.)
  - Update billing_service.py to import from exceptions.py

- Standardize VendorEmailSettingsService DI pattern:
  - Change from db in __init__ to db as method parameter
  - Create singleton vendor_email_settings_service instance
  - Update routes and tests to use new pattern

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Samir Boulahtit
2026-02-03 18:40:03 +01:00
parent b769f5a047
commit a76128e016
11 changed files with 209 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
app/modules/billing/exceptions.py
View File

@@ -19,10 +19,13 @@ from app.exceptions.base import (
__all__ = [
# Base billing exception
"BillingException",
"BillingServiceError", # Alias for backwards compatibility
# Subscription exceptions
"SubscriptionNotFoundException",
"NoActiveSubscriptionException",
"NoActiveSubscriptionError", # Alias for backwards compatibility
"SubscriptionNotCancelledException",
"SubscriptionNotCancelledError", # Alias for backwards compatibility
"SubscriptionAlreadyCancelledException",
# Tier exceptions
"TierNotFoundException",
@@ -30,8 +33,10 @@ __all__ = [
"TierLimitExceededException",
# Payment exceptions
"PaymentSystemNotConfiguredException",
"PaymentSystemNotConfiguredError", # Alias for backwards compatibility
"StripeNotConfiguredException",
"StripePriceNotConfiguredException",
"StripePriceNotConfiguredError", # Alias for backwards compatibility
"PaymentFailedException",
# Webhook exceptions
"InvalidWebhookSignatureException",
@@ -57,6 +62,10 @@ class BillingException(BusinessLogicException):
super().__init__(message=message, error_code=error_code, details=details)
# Alias for backwards compatibility with billing_service.py
BillingServiceError = BillingException
# =============================================================================
# Subscription Exceptions
# =============================================================================
@@ -83,6 +92,10 @@ class NoActiveSubscriptionException(BusinessLogicException):
)
# Alias for backwards compatibility with billing_service.py
NoActiveSubscriptionError = NoActiveSubscriptionException
class SubscriptionNotCancelledException(BusinessLogicException):
"""Raised when trying to reactivate a subscription that is not cancelled."""
@@ -93,6 +106,10 @@ class SubscriptionNotCancelledException(BusinessLogicException):
)
# Alias for backwards compatibility with billing_service.py
SubscriptionNotCancelledError = SubscriptionNotCancelledException
class SubscriptionAlreadyCancelledException(BusinessLogicException):
"""Raised when trying to cancel an already cancelled subscription."""
@@ -163,6 +180,10 @@ class PaymentSystemNotConfiguredException(ServiceUnavailableException):
super().__init__(message="Payment system not configured")
# Alias for backwards compatibility with billing_service.py
PaymentSystemNotConfiguredError = PaymentSystemNotConfiguredException
class StripeNotConfiguredException(BillingException):
"""Raised when Stripe is not configured."""
@@ -185,6 +206,10 @@ class StripePriceNotConfiguredException(BusinessLogicException):
self.tier_code = tier_code
# Alias for backwards compatibility with billing_service.py
StripePriceNotConfiguredError = StripePriceNotConfiguredException
class PaymentFailedException(BillingException):
"""Raised when a payment fails."""

2
app/modules/billing/services/__init__.py
View File

@@ -20,6 +20,8 @@ from app.modules.billing.services.admin_subscription_service import (
from app.modules.billing.services.billing_service import (
BillingService,
billing_service,
)
from app.modules.billing.exceptions import (
BillingServiceError,
PaymentSystemNotConfiguredError,
TierNotFoundError,

51
app/modules/billing/services/billing_service.py
View File

@@ -23,54 +23,19 @@ from app.modules.billing.models import (
VendorAddOn,
VendorSubscription,
)
from app.modules.billing.exceptions import (
BillingServiceError,
NoActiveSubscriptionError,
PaymentSystemNotConfiguredError,
StripePriceNotConfiguredError,
SubscriptionNotCancelledError,
TierNotFoundError,
)
from app.modules.tenancy.models import Vendor
logger = logging.getLogger(__name__)
class BillingServiceError(Exception):
"""Base exception for billing service errors."""
pass
class PaymentSystemNotConfiguredError(BillingServiceError):
"""Raised when Stripe is not configured."""
def __init__(self):
super().__init__("Payment system not configured")
class TierNotFoundError(BillingServiceError):
"""Raised when a tier is not found."""
def __init__(self, tier_code: str):
self.tier_code = tier_code
super().__init__(f"Tier '{tier_code}' not found")
class StripePriceNotConfiguredError(BillingServiceError):
"""Raised when Stripe price is not configured for a tier."""
def __init__(self, tier_code: str):
self.tier_code = tier_code
super().__init__(f"Stripe price not configured for tier '{tier_code}'")
class NoActiveSubscriptionError(BillingServiceError):
"""Raised when no active subscription exists."""
def __init__(self):
super().__init__("No active subscription found")
class SubscriptionNotCancelledError(BillingServiceError):
"""Raised when trying to reactivate a non-cancelled subscription."""
def __init__(self):
super().__init__("Subscription is not cancelled")
class BillingService:
"""Service for billing operations."""

29
app/modules/cart/definition.py
View File

@@ -9,7 +9,20 @@ It is session-based and does not require customer authentication.
from app.modules.base import ModuleDefinition, PermissionDefinition
module = ModuleDefinition(
# =============================================================================
# Router Lazy Imports
# =============================================================================
def _get_storefront_router():
"""Lazy import of storefront router to avoid circular imports."""
from app.modules.cart.routes.api.storefront import router
return router
# Cart module definition
cart_module = ModuleDefinition(
code="cart",
name="Shopping Cart",
description="Session-based shopping cart for storefronts",
@@ -42,3 +55,17 @@ module = ModuleDefinition(
# Cart is storefront-only - no admin/vendor menus needed
menu_items={},
)
def get_cart_module_with_routers() -> ModuleDefinition:
"""
Get cart module with routers attached.
This function attaches the routers lazily to avoid circular imports
during module initialization.
"""
cart_module.storefront_router = _get_storefront_router()
return cart_module
__all__ = ["cart_module", "get_cart_module_with_routers"]

38
app/modules/catalog/definition.py
View File

@@ -9,7 +9,28 @@ from app.modules.base import (
)
from app.modules.enums import FrontendType
module = ModuleDefinition(
# =============================================================================
# Router Lazy Imports
# =============================================================================
def _get_admin_router():
"""Lazy import of admin router to avoid circular imports."""
from app.modules.catalog.routes.api.admin import admin_router
return admin_router
def _get_vendor_router():
"""Lazy import of vendor router to avoid circular imports."""
from app.modules.catalog.routes.api.vendor import vendor_router
return vendor_router
# Catalog module definition
catalog_module = ModuleDefinition(
code="catalog",
name="Product Catalog",
description="Product catalog browsing and search for storefronts",
@@ -85,3 +106,18 @@ module = ModuleDefinition(
],
},
)
def get_catalog_module_with_routers() -> ModuleDefinition:
"""
Get catalog module with routers attached.
This function attaches the routers lazily to avoid circular imports
during module initialization.
"""
catalog_module.admin_router = _get_admin_router()
catalog_module.vendor_router = _get_vendor_router()
return catalog_module
__all__ = ["catalog_module", "get_catalog_module_with_routers"]

29
app/modules/checkout/definition.py
View File

@@ -9,7 +9,20 @@ Orchestrates payment processing and order creation.
from app.modules.base import ModuleDefinition, PermissionDefinition
module = ModuleDefinition(
# =============================================================================
# Router Lazy Imports
# =============================================================================
def _get_storefront_router():
"""Lazy import of storefront router to avoid circular imports."""
from app.modules.checkout.routes.api.storefront import router
return router
# Checkout module definition
checkout_module = ModuleDefinition(
code="checkout",
name="Checkout",
description="Checkout and order creation for storefronts",
@@ -42,3 +55,17 @@ module = ModuleDefinition(
# Checkout is storefront-only - no admin/vendor menus needed
menu_items={},
)
def get_checkout_module_with_routers() -> ModuleDefinition:
"""
Get checkout module with routers attached.
This function attaches the routers lazily to avoid circular imports
during module initialization.
"""
checkout_module.storefront_router = _get_storefront_router()
return checkout_module
__all__ = ["checkout_module", "get_checkout_module_with_routers"]

6
app/modules/cms/services/__init__.py
View File

@@ -19,7 +19,8 @@ from app.modules.cms.services.vendor_theme_service import (
)
from app.modules.cms.services.vendor_email_settings_service import (
VendorEmailSettingsService,
get_vendor_email_settings_service,
vendor_email_settings_service,
get_vendor_email_settings_service, # Deprecated: use vendor_email_settings_service
)
__all__ = [
@@ -30,5 +31,6 @@ __all__ = [
"VendorThemeService",
"vendor_theme_service",
"VendorEmailSettingsService",
"get_vendor_email_settings_service",
"vendor_email_settings_service",
"get_vendor_email_settings_service", # Deprecated
]

63
app/modules/cms/services/vendor_email_settings_service.py
View File

@@ -42,24 +42,21 @@ PREMIUM_TIERS = {TierCode.BUSINESS, TierCode.ENTERPRISE}
class VendorEmailSettingsService:
"""Service for managing vendor email settings."""
def __init__(self, db: Session):
self.db = db
# =========================================================================
# READ OPERATIONS
# =========================================================================
def get_settings(self, vendor_id: int) -> VendorEmailSettings | None:
def get_settings(self, db: Session, vendor_id: int) -> VendorEmailSettings | None:
"""Get email settings for a vendor."""
return (
self.db.query(VendorEmailSettings)
db.query(VendorEmailSettings)
.filter(VendorEmailSettings.vendor_id == vendor_id)
.first()
)
def get_settings_or_404(self, vendor_id: int) -> VendorEmailSettings:
def get_settings_or_404(self, db: Session, vendor_id: int) -> VendorEmailSettings:
"""Get email settings or raise 404."""
settings = self.get_settings(vendor_id)
settings = self.get_settings(db, vendor_id)
if not settings:
raise ResourceNotFoundException(
resource_type="vendor_email_settings",
@@ -67,19 +64,19 @@ class VendorEmailSettingsService:
)
return settings
def is_configured(self, vendor_id: int) -> bool:
def is_configured(self, db: Session, vendor_id: int) -> bool:
"""Check if vendor has configured email settings."""
settings = self.get_settings(vendor_id)
settings = self.get_settings(db, vendor_id)
return settings is not None and settings.is_configured
def get_status(self, vendor_id: int) -> dict:
def get_status(self, db: Session, vendor_id: int) -> dict:
"""
Get email configuration status for a vendor.
Returns:
dict with is_configured, is_verified, provider, etc.
"""
settings = self.get_settings(vendor_id)
settings = self.get_settings(db, vendor_id)
if not settings:
return {
"is_configured": False,
@@ -115,6 +112,7 @@ class VendorEmailSettingsService:
def create_or_update(
self,
db: Session,
vendor_id: int,
data: dict,
current_tier: TierCode | None = None,
@@ -123,6 +121,7 @@ class VendorEmailSettingsService:
Create or update vendor email settings.
Args:
db: Database session
vendor_id: Vendor ID
data: Settings data (from_email, from_name, smtp_*, etc.)
current_tier: Vendor's current subscription tier (for premium provider validation)
@@ -143,10 +142,10 @@ class VendorEmailSettingsService:
details={"required_permission": "business_tier"},
)
settings = self.get_settings(vendor_id)
settings = self.get_settings(db, vendor_id)
if not settings:
settings = VendorEmailSettings(vendor_id=vendor_id)
self.db.add(settings)
db.add(settings)
# Update fields
for field in [
@@ -190,36 +189,41 @@ class VendorEmailSettingsService:
settings.is_verified = False
settings.verification_error = None
self.db.flush()
db.flush()
logger.info(f"Updated email settings for vendor {vendor_id}: provider={settings.provider}")
return settings
def delete(self, vendor_id: int) -> None:
def delete(self, db: Session, vendor_id: int) -> None:
"""
Delete email settings for a vendor.
Args:
db: Database session
vendor_id: Vendor ID
Raises:
ResourceNotFoundException: If settings not found
"""
settings = self.get_settings(vendor_id)
settings = self.get_settings(db, vendor_id)
if not settings:
raise ResourceNotFoundException(
resource_type="vendor_email_settings",
identifier=str(vendor_id),
)
self.db.delete(settings)
self.db.flush()
db.delete(settings)
db.flush()
logger.info(f"Deleted email settings for vendor {vendor_id}")
# =========================================================================
# VERIFICATION
# =========================================================================
def verify_settings(self, vendor_id: int, test_email: str) -> dict:
def verify_settings(self, db: Session, vendor_id: int, test_email: str) -> dict:
"""
Verify email settings by sending a test email.
Args:
db: Database session
vendor_id: Vendor ID
test_email: Email address to send test email to
@@ -230,7 +234,7 @@ class VendorEmailSettingsService:
ResourceNotFoundException: If settings not found
ValidationException: If settings incomplete
"""
settings = self.get_settings_or_404(vendor_id)
settings = self.get_settings_or_404(db, vendor_id)
if not settings.is_fully_configured():
raise ValidationException(
@@ -256,7 +260,7 @@ class VendorEmailSettingsService:
# Mark as verified
settings.mark_verified()
self.db.flush()
db.flush()
logger.info(f"Email settings verified for vendor {vendor_id}")
return {
@@ -269,7 +273,7 @@ class VendorEmailSettingsService:
except Exception as e:
error_msg = str(e)
settings.mark_verification_failed(error_msg)
self.db.flush()
db.flush()
logger.warning(f"Email verification failed for vendor {vendor_id}: {error_msg}")
# Return error dict instead of raising - verification failure is not a server error
@@ -476,7 +480,16 @@ class VendorEmailSettingsService:
return providers
# Module-level service factory
# Module-level service instance (singleton pattern)
vendor_email_settings_service = VendorEmailSettingsService()
# Deprecated: Factory function for backwards compatibility
def get_vendor_email_settings_service(db: Session) -> VendorEmailSettingsService:
"""Factory function to get a VendorEmailSettingsService instance."""
return VendorEmailSettingsService(db)
"""
Factory function to get a VendorEmailSettingsService instance.
Deprecated: Use the singleton `vendor_email_settings_service` instead and pass
`db` to individual methods.
"""
return vendor_email_settings_service

21
app/modules/messaging/routes/api/vendor_email_settings.py
View File

@@ -20,7 +20,7 @@ from sqlalchemy.orm import Session
from app.api.deps import get_current_vendor_api
from app.core.database import get_db
from app.modules.cms.services.vendor_email_settings_service import VendorEmailSettingsService
from app.modules.cms.services.vendor_email_settings_service import vendor_email_settings_service
from app.modules.billing.services.subscription_service import subscription_service
from models.schema.auth import UserContext
@@ -137,9 +137,8 @@ def get_email_settings(
Returns settings with sensitive fields masked.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
settings = service.get_settings(vendor_id)
settings = vendor_email_settings_service.get_settings(db, vendor_id)
if not settings:
return EmailSettingsResponse(
configured=False,
@@ -165,8 +164,7 @@ def get_email_status(
Used by frontend to show warning banner if not configured.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
status = service.get_status(vendor_id)
status = vendor_email_settings_service.get_status(db, vendor_id)
return EmailStatusResponse(**status)
@@ -181,13 +179,12 @@ def get_available_providers(
Returns list of providers with availability status.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
# Get vendor's current tier
tier = subscription_service.get_current_tier(db, vendor_id)
return ProvidersResponse(
providers=service.get_available_providers(tier),
providers=vendor_email_settings_service.get_available_providers(tier),
current_tier=tier.value if tier else None,
)
@@ -206,13 +203,13 @@ def update_email_settings(
Raises ValidationException if data is invalid.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
# Get vendor's current tier for validation
tier = subscription_service.get_current_tier(db, vendor_id)
# Service raises appropriate exceptions (API-003 compliance)
settings = service.create_or_update(
settings = vendor_email_settings_service.create_or_update(
db=db,
vendor_id=vendor_id,
data=data.model_dump(exclude_unset=True),
current_tier=tier,
@@ -240,10 +237,9 @@ def verify_email_settings(
Raises ValidationException if verification fails.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
# Service raises appropriate exceptions (API-003 compliance)
result = service.verify_settings(vendor_id, data.test_email)
result = vendor_email_settings_service.verify_settings(db, vendor_id, data.test_email)
db.commit()
return EmailVerifyResponse(
@@ -264,10 +260,9 @@ def delete_email_settings(
Raises ResourceNotFoundException if settings not found.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
# Service raises ResourceNotFoundException if not found (API-003 compliance)
service.delete(vendor_id)
vendor_email_settings_service.delete(db, vendor_id)
db.commit()
return EmailDeleteResponse(