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

fix: resolve email settings architecture violations and add tests/docs

Browse Source 
- Fix API-002 in admin/settings.py: use service layer for DB delete
- Fix API-001/API-003 in vendor/email_settings.py: add Pydantic response
  models, remove HTTPException raises
- Fix SVC-002/SVC-006 in vendor_email_settings_service.py: use domain
  exceptions, change db.commit() to db.flush()
- Add unit tests for VendorEmailSettingsService
- Add integration tests for vendor and admin email settings APIs
- Add user guide (docs/guides/email-settings.md)
- Add developer guide (docs/implementation/email-settings.md)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Samir Boulahtit
2026-01-05 22:38:10 +01:00
parent 36603178c3
commit 84a523cd7b
9 changed files with 1765 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/api/v1/admin/settings.py
View File

@@ -610,7 +610,8 @@ def reset_email_settings(
for key in EMAIL_SETTING_KEYS:
setting = admin_settings_service.get_setting_by_key(db, key)
if setting:
db.delete(setting)
# Use service method for deletion (API-002 compliance)
admin_settings_service.delete_setting(db, key, current_admin.id)
deleted_count += 1
# Log action

167
app/api/v1/vendor/email_settings.py vendored
View File

@@ -14,13 +14,12 @@ Vendor Context: Uses token_vendor_id from JWT token (authenticated vendor API pa
import logging
from fastapi import APIRouter, Depends, HTTPException
from fastapi import APIRouter, Depends
from pydantic import BaseModel, EmailStr, Field
from sqlalchemy.orm import Session
from app.api.deps import get_current_vendor_api
from app.core.database import get_db
from app.exceptions import NotFoundError, ValidationError, AuthorizationError
from app.services.vendor_email_settings_service import VendorEmailSettingsService
from app.services.subscription_service import subscription_service
from models.database.user import User
@@ -76,16 +75,62 @@ class VerifyEmailRequest(BaseModel):
test_email: EmailStr = Field(..., description="Email address to send test email to")
# Response models for API-001 compliance
class EmailSettingsResponse(BaseModel):
"""Response for email settings."""
configured: bool
verified: bool | None = None
settings: dict | None = None
message: str | None = None
class EmailStatusResponse(BaseModel):
"""Response for email status check."""
is_configured: bool
is_verified: bool
class ProvidersResponse(BaseModel):
"""Response for available providers."""
providers: list[dict]
current_tier: str | None
class EmailUpdateResponse(BaseModel):
"""Response for email settings update."""
success: bool
message: str
settings: dict
class EmailVerifyResponse(BaseModel):
"""Response for email verification."""
success: bool
message: str
class EmailDeleteResponse(BaseModel):
"""Response for email settings deletion."""
success: bool
message: str
# =============================================================================
# ENDPOINTS
# =============================================================================
@router.get("")
@router.get("", response_model=EmailSettingsResponse)
def get_email_settings(
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> EmailSettingsResponse:
"""
Get current email settings for the vendor.
@@ -96,24 +141,24 @@ def get_email_settings(
settings = service.get_settings(vendor_id)
if not settings:
return {
"configured": False,
"settings": None,
"message": "Email settings not configured. Configure SMTP to send emails to customers.",
}
return EmailSettingsResponse(
configured=False,
settings=None,
message="Email settings not configured. Configure SMTP to send emails to customers.",
)
return {
"configured": settings.is_configured,
"verified": settings.is_verified,
"settings": settings.to_dict(),
}
return EmailSettingsResponse(
configured=settings.is_configured,
verified=settings.is_verified,
settings=settings.to_dict(),
)
@router.get("/status")
@router.get("/status", response_model=EmailStatusResponse)
def get_email_status(
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> EmailStatusResponse:
"""
Get email configuration status.
@@ -121,14 +166,15 @@ def get_email_status(
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
return service.get_status(vendor_id)
status = service.get_status(vendor_id)
return EmailStatusResponse(**status)
@router.get("/providers")
@router.get("/providers", response_model=ProvidersResponse)
def get_available_providers(
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> ProvidersResponse:
"""
Get available email providers for current tier.
@@ -140,22 +186,24 @@ def get_available_providers(
# Get vendor's current tier
tier = subscription_service.get_current_tier(db, vendor_id)
return {
"providers": service.get_available_providers(tier),
"current_tier": tier.value if tier else None,
}
return ProvidersResponse(
providers=service.get_available_providers(tier),
current_tier=tier.value if tier else None,
)
@router.put("")
@router.put("", response_model=EmailUpdateResponse)
def update_email_settings(
data: EmailSettingsUpdate,
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> EmailUpdateResponse:
"""
Create or update email settings.
Premium providers (SendGrid, Mailgun, SES) require Business+ tier.
Raises AuthorizationException if tier is insufficient.
Raises ValidationException if data is invalid.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
@@ -163,63 +211,66 @@ def update_email_settings(
# Get vendor's current tier for validation
tier = subscription_service.get_current_tier(db, vendor_id)
try:
settings = service.create_or_update(
vendor_id=vendor_id,
data=data.model_dump(exclude_unset=True),
current_tier=tier,
)
return {
"success": True,
"message": "Email settings updated successfully",
"settings": settings.to_dict(),
}
except AuthorizationError as e:
raise HTTPException(status_code=403, detail=str(e))
except ValidationError as e:
raise HTTPException(status_code=400, detail=str(e))
# Service raises appropriate exceptions (API-003 compliance)
settings = service.create_or_update(
vendor_id=vendor_id,
data=data.model_dump(exclude_unset=True),
current_tier=tier,
)
db.commit()
return EmailUpdateResponse(
success=True,
message="Email settings updated successfully",
settings=settings.to_dict(),
)
@router.post("/verify")
@router.post("/verify", response_model=EmailVerifyResponse)
def verify_email_settings(
data: VerifyEmailRequest,
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> EmailVerifyResponse:
"""
Verify email settings by sending a test email.
Sends a test email to the provided address and updates verification status.
Raises ResourceNotFoundException if settings not configured.
Raises ValidationException if verification fails.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
try:
result = service.verify_settings(vendor_id, data.test_email)
if result["success"]:
return result
else:
raise HTTPException(status_code=400, detail=result["message"])
except NotFoundError as e:
raise HTTPException(status_code=404, detail=str(e))
except ValidationError as e:
raise HTTPException(status_code=400, detail=str(e))
# Service raises appropriate exceptions (API-003 compliance)
result = service.verify_settings(vendor_id, data.test_email)
db.commit()
return EmailVerifyResponse(
success=result["success"],
message=result["message"],
)
@router.delete("")
@router.delete("", response_model=EmailDeleteResponse)
def delete_email_settings(
current_user: User = Depends(get_current_vendor_api),
db: Session = Depends(get_db),
):
) -> EmailDeleteResponse:
"""
Delete email settings.
Warning: This will disable email sending for the vendor.
Raises ResourceNotFoundException if settings not found.
"""
vendor_id = current_user.token_vendor_id
service = VendorEmailSettingsService(db)
if service.delete(vendor_id):
return {"success": True, "message": "Email settings deleted"}
else:
raise HTTPException(status_code=404, detail="Email settings not found")
# Service raises ResourceNotFoundException if not found (API-003 compliance)
service.delete(vendor_id)
db.commit()
return EmailDeleteResponse(
success=True,
message="Email settings deleted",
)

91
app/services/vendor_email_settings_service.py
View File

@@ -18,7 +18,12 @@ from email.mime.text import MIMEText
from sqlalchemy.orm import Session
from app.exceptions import NotFoundError, ValidationError, AuthorizationError
from app.exceptions import (
AuthorizationException,
ResourceNotFoundException,
ValidationException,
ExternalServiceException,
)
from models.database import (
Vendor,
VendorEmailSettings,
@@ -57,9 +62,9 @@ class VendorEmailSettingsService:
"""Get email settings or raise 404."""
settings = self.get_settings(vendor_id)
if not settings:
raise NotFoundError(
f"Email settings not found for vendor {vendor_id}. "
"Configure email settings to send emails."
raise ResourceNotFoundException(
resource_type="vendor_email_settings",
identifier=str(vendor_id),
)
return settings
@@ -125,14 +130,18 @@ class VendorEmailSettingsService:
Returns:
Updated VendorEmailSettings
Raises:
AuthorizationException: If trying to use premium provider without required tier
"""
# Validate premium provider access
provider = data.get("provider", "smtp")
if provider in [p.value for p in PREMIUM_EMAIL_PROVIDERS]:
if current_tier not in PREMIUM_TIERS:
raise AuthorizationError(
f"Provider '{provider}' requires Business or Enterprise tier. "
"Upgrade your plan to use advanced email providers."
raise AuthorizationException(
message=f"Provider '{provider}' requires Business or Enterprise tier. "
"Upgrade your plan to use advanced email providers.",
required_permission="business_tier",
)
settings = self.get_settings(vendor_id)
@@ -182,21 +191,26 @@ class VendorEmailSettingsService:
settings.is_verified = False
settings.verification_error = None
self.db.commit()
self.db.refresh(settings)
self.db.flush()
logger.info(f"Updated email settings for vendor {vendor_id}: provider={settings.provider}")
return settings
def delete(self, vendor_id: int) -> bool:
"""Delete email settings for a vendor."""
def delete(self, vendor_id: int) -> None:
"""
Delete email settings for a vendor.
Raises:
ResourceNotFoundException: If settings not found
"""
settings = self.get_settings(vendor_id)
if settings:
self.db.delete(settings)
self.db.commit()
logger.info(f"Deleted email settings for vendor {vendor_id}")
return True
return False
if not settings:
raise ResourceNotFoundException(
resource_type="vendor_email_settings",
identifier=str(vendor_id),
)
self.db.delete(settings)
self.db.flush()
logger.info(f"Deleted email settings for vendor {vendor_id}")
# =========================================================================
# VERIFICATION
@@ -212,11 +226,18 @@ class VendorEmailSettingsService:
Returns:
dict with success status and message
Raises:
ResourceNotFoundException: If settings not found
ValidationException: If settings incomplete
"""
settings = self.get_settings_or_404(vendor_id)
if not settings.is_fully_configured():
raise ValidationError("Email settings incomplete. Configure all required fields first.")
raise ValidationException(
message="Email settings incomplete. Configure all required fields first.",
field="settings",
)
try:
# Send test email based on provider
@@ -229,11 +250,14 @@ class VendorEmailSettingsService:
elif settings.provider == EmailProvider.SES.value:
self._send_ses_test(settings, test_email)
else:
raise ValidationError(f"Unknown provider: {settings.provider}")
raise ValidationException(
message=f"Unknown provider: {settings.provider}",
field="provider",
)
# Mark as verified
settings.mark_verified()
self.db.commit()
self.db.flush()
logger.info(f"Email settings verified for vendor {vendor_id}")
return {
@@ -241,12 +265,15 @@ class VendorEmailSettingsService:
"message": f"Test email sent successfully to {test_email}",
}
except (ValidationException, ExternalServiceException):
raise # Re-raise domain exceptions
except Exception as e:
error_msg = str(e)
settings.mark_verification_failed(error_msg)
self.db.commit()
self.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
return {
"success": False,
"message": f"Failed to send test email: {error_msg}",
@@ -304,7 +331,10 @@ class VendorEmailSettingsService:
from sendgrid import SendGridAPIClient
from sendgrid.helpers.mail import Mail
except ImportError:
raise ValidationError("SendGrid library not installed. Contact support.")
raise ExternalServiceException(
service_name="SendGrid",
message="SendGrid library not installed. Contact support.",
)
message = Mail(
from_email=(settings.from_email, settings.from_name),
@@ -327,7 +357,10 @@ class VendorEmailSettingsService:
response = sg.send(message)
if response.status_code >= 400:
raise Exception(f"SendGrid error: {response.status_code}")
raise ExternalServiceException(
service_name="SendGrid",
message=f"SendGrid error: HTTP {response.status_code}",
)
def _send_mailgun_test(self, settings: VendorEmailSettings, to_email: str) -> None:
"""Send test email via Mailgun."""
@@ -356,14 +389,20 @@ class VendorEmailSettingsService:
)
if response.status_code >= 400:
raise Exception(f"Mailgun error: {response.text}")
raise ExternalServiceException(
service_name="Mailgun",
message=f"Mailgun error: {response.text}",
)
def _send_ses_test(self, settings: VendorEmailSettings, to_email: str) -> None:
"""Send test email via Amazon SES."""
try:
import boto3
except ImportError:
raise ValidationError("boto3 library not installed. Contact support.")
raise ExternalServiceException(
service_name="Amazon SES",
message="boto3 library not installed. Contact support.",
)
client = boto3.client(
"ses",