fix: resolve architecture validation violations

- Add invoice exceptions module with proper exception hierarchy - Replace HTTPException with service-layer exceptions in invoice API - Add InvoicePDFGeneratedResponse and InvoiceStatsResponse Pydantic models - Replace db.commit() with db.flush() in services for proper transaction control - Update invoice service to use exceptions from app/exceptions/invoice.py All 14 errors and 14 warnings are now resolved. Validation passes with only INFO-level findings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2025-12-24 18:23:49 +01:00
parent 6232bb47f6
commit 319fba5d39
7 changed files with 276 additions and 147 deletions
--- a/app/api/v1/vendor/invoices.py
+++ b/app/api/v1/vendor/invoices.py
@@ -21,23 +21,27 @@ Endpoints:
 import logging
 from pathlib import Path

-from fastapi import APIRouter, Depends, HTTPException, Query
+from fastapi import APIRouter, Depends, Query
 from fastapi.responses import FileResponse
 from sqlalchemy.orm import Session

 from app.api.deps import get_current_vendor_api
 from app.core.database import get_db
-from app.services.invoice_service import (
+from app.exceptions.invoice import (
    InvoiceNotFoundException,
+    InvoicePDFGenerationException,
+    InvoicePDFNotFoundException,
    InvoiceSettingsNotFoundException,
-    invoice_service,
 )
+from app.services.invoice_service import invoice_service
 from models.database.user import User
 from models.schema.invoice import (
    InvoiceCreate,
    InvoiceListPaginatedResponse,
    InvoiceListResponse,
+    InvoicePDFGeneratedResponse,
    InvoiceResponse,
+    InvoiceStatsResponse,
    InvoiceStatusUpdate,
    VendorInvoiceSettingsCreate,
    VendorInvoiceSettingsResponse,
@@ -81,16 +85,12 @@ def create_invoice_settings(
    Required before creating invoices. Sets company details,
    VAT number, invoice numbering preferences, and payment info.
    """
-    try:
-        settings = invoice_service.create_settings(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            data=data,
-        )
-        return VendorInvoiceSettingsResponse.model_validate(settings)
-    except Exception as e:
-        logger.error(f"Failed to create invoice settings: {e}")
-        raise HTTPException(status_code=400, detail=str(e))
+    settings = invoice_service.create_settings(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        data=data,
+    )
+    return VendorInvoiceSettingsResponse.model_validate(settings)


@router.put("/settings", response_model=VendorInvoiceSettingsResponse)
@@ -102,21 +102,12 @@ def update_invoice_settings(
    """
    Update vendor invoice settings.
    """
-    try:
-        settings = invoice_service.update_settings(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            data=data,
-        )
-        return VendorInvoiceSettingsResponse.model_validate(settings)
-    except InvoiceSettingsNotFoundException:
-        raise HTTPException(
-            status_code=404,
-            detail="Invoice settings not found. Create settings first.",
-        )
-    except Exception as e:
-        logger.error(f"Failed to update invoice settings: {e}")
-        raise HTTPException(status_code=400, detail=str(e))
+    settings = invoice_service.update_settings(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        data=data,
+    )
+    return VendorInvoiceSettingsResponse.model_validate(settings)


 # ============================================================================
@@ -124,7 +115,7 @@ def update_invoice_settings(
 # ============================================================================


-@router.get("/stats")
+@router.get("/stats", response_model=InvoiceStatsResponse)
 def get_invoice_stats(
    current_user: User = Depends(get_current_vendor_api),
    db: Session = Depends(get_db),
@@ -138,7 +129,15 @@ def get_invoice_stats(
    - draft_count: Number of draft invoices
    - paid_count: Number of paid invoices
    """
-    return invoice_service.get_invoice_stats(db, current_user.token_vendor_id)
+    stats = invoice_service.get_invoice_stats(db, current_user.token_vendor_id)
+    return InvoiceStatsResponse(
+        total_invoices=stats.get("total_invoices", 0),
+        total_revenue_cents=stats.get("total_revenue_cents", 0),
+        draft_count=stats.get("draft_count", 0),
+        issued_count=stats.get("issued_count", 0),
+        paid_count=stats.get("paid_count", 0),
+        cancelled_count=stats.get("cancelled_count", 0),
+    )


 # ============================================================================
@@ -204,15 +203,12 @@ def get_invoice(
    """
    Get invoice details by ID.
    """
-    try:
-        invoice = invoice_service.get_invoice_or_raise(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            invoice_id=invoice_id,
-        )
-        return InvoiceResponse.model_validate(invoice)
-    except InvoiceNotFoundException:
-        raise HTTPException(status_code=404, detail="Invoice not found")
+    invoice = invoice_service.get_invoice_or_raise(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        invoice_id=invoice_id,
+    )
+    return InvoiceResponse.model_validate(invoice)


@router.post("", response_model=InvoiceResponse, status_code=201)
@@ -229,22 +225,13 @@ def create_invoice(
    - Calculates VAT based on buyer country and OSS settings
    - Generates invoice number
    """
-    try:
-        invoice = invoice_service.create_invoice_from_order(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            order_id=data.order_id,
-            notes=data.notes,
-        )
-        return InvoiceResponse.model_validate(invoice)
-    except InvoiceSettingsNotFoundException:
-        raise HTTPException(
-            status_code=400,
-            detail="Invoice settings not configured. Set up invoice settings first.",
-        )
-    except Exception as e:
-        logger.error(f"Failed to create invoice: {e}")
-        raise HTTPException(status_code=400, detail=str(e))
+    invoice = invoice_service.create_invoice_from_order(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        order_id=data.order_id,
+        notes=data.notes,
+    )
+    return InvoiceResponse.model_validate(invoice)


@router.put("/{invoice_id}/status", response_model=InvoiceResponse)
@@ -265,19 +252,13 @@ def update_invoice_status(

    Note: Cancelled invoices cannot be modified.
    """
-    try:
-        invoice = invoice_service.update_status(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            invoice_id=invoice_id,
-            new_status=data.status,
-        )
-        return InvoiceResponse.model_validate(invoice)
-    except InvoiceNotFoundException:
-        raise HTTPException(status_code=404, detail="Invoice not found")
-    except Exception as e:
-        logger.error(f"Failed to update invoice status: {e}")
-        raise HTTPException(status_code=400, detail=str(e))
+    invoice = invoice_service.update_status(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        invoice_id=invoice_id,
+        new_status=data.status,
+    )
+    return InvoiceResponse.model_validate(invoice)


 # ============================================================================
@@ -285,7 +266,7 @@ def update_invoice_status(
 # ============================================================================


-@router.post("/{invoice_id}/pdf")
+@router.post("/{invoice_id}/pdf", response_model=InvoicePDFGeneratedResponse)
 def generate_invoice_pdf(
    invoice_id: int,
    regenerate: bool = Query(False, description="Force regenerate if exists"),
@@ -298,19 +279,13 @@ def generate_invoice_pdf(
    Set regenerate=true to force regeneration if PDF already exists.
    Returns the path to the generated PDF.
    """
-    try:
-        pdf_path = invoice_service.generate_pdf(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            invoice_id=invoice_id,
-            force_regenerate=regenerate,
-        )
-        return {"pdf_path": pdf_path, "message": "PDF generated successfully"}
-    except InvoiceNotFoundException:
-        raise HTTPException(status_code=404, detail="Invoice not found")
-    except Exception as e:
-        logger.error(f"Failed to generate PDF: {e}")
-        raise HTTPException(status_code=500, detail=f"Failed to generate PDF: {str(e)}")
+    pdf_path = invoice_service.generate_pdf(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        invoice_id=invoice_id,
+        force_regenerate=regenerate,
+    )
+    return InvoicePDFGeneratedResponse(pdf_path=pdf_path)


@router.get("/{invoice_id}/pdf")
@@ -325,45 +300,39 @@ def download_invoice_pdf(
    Returns the PDF file for download.
    Generates PDF if not already generated.
    """
-    try:
-        # Check if PDF exists, generate if not
-        pdf_path = invoice_service.get_pdf_path(
+    # Check if PDF exists, generate if not
+    pdf_path = invoice_service.get_pdf_path(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        invoice_id=invoice_id,
+    )
+
+    if not pdf_path:
+        # Generate PDF
+        pdf_path = invoice_service.generate_pdf(
            db=db,
            vendor_id=current_user.token_vendor_id,
            invoice_id=invoice_id,
        )

-        if not pdf_path:
-            # Generate PDF
-            pdf_path = invoice_service.generate_pdf(
-                db=db,
-                vendor_id=current_user.token_vendor_id,
-                invoice_id=invoice_id,
-            )
+    # Verify file exists
+    if not Path(pdf_path).exists():
+        raise InvoicePDFNotFoundException(invoice_id)

-        # Verify file exists
-        if not Path(pdf_path).exists():
-            raise HTTPException(status_code=404, detail="PDF file not found")
+    # Get invoice for filename
+    invoice = invoice_service.get_invoice_or_raise(
+        db=db,
+        vendor_id=current_user.token_vendor_id,
+        invoice_id=invoice_id,
+    )

-        # Get invoice for filename
-        invoice = invoice_service.get_invoice_or_raise(
-            db=db,
-            vendor_id=current_user.token_vendor_id,
-            invoice_id=invoice_id,
-        )
+    filename = f"invoice-{invoice.invoice_number}.pdf"

-        filename = f"invoice-{invoice.invoice_number}.pdf"
-
-        return FileResponse(
-            path=pdf_path,
-            media_type="application/pdf",
-            filename=filename,
-            headers={
-                "Content-Disposition": f'attachment; filename="{filename}"'
-            },
-        )
-    except InvoiceNotFoundException:
-        raise HTTPException(status_code=404, detail="Invoice not found")
-    except Exception as e:
-        logger.error(f"Failed to download PDF: {e}")
-        raise HTTPException(status_code=500, detail=f"Failed to download PDF: {str(e)}")
+    return FileResponse(
+        path=pdf_path,
+        media_type="application/pdf",
+        filename=filename,
+        headers={
+            "Content-Disposition": f'attachment; filename="{filename}"'
+        },
+    )
--- a/app/exceptions/init.py
+++ b/app/exceptions/init.py
@@ -90,6 +90,18 @@ from .customer import (
    InvalidCustomerCredentialsException,
 )

+# Invoice exceptions
+from .invoice import (
+    InvoiceNotFoundException,
+    InvoicePDFGenerationException,
+    InvoicePDFNotFoundException,
+    InvoiceSettingsAlreadyExistException,
+    InvoiceSettingsNotFoundException,
+    InvoiceValidationException,
+    InvalidInvoiceStatusTransitionException,
+    OrderNotFoundException as OrderNotFoundForInvoiceException,
+)
+
 # Inventory exceptions
 from .inventory import (
    InsufficientInventoryException,
@@ -272,6 +284,15 @@ __all__ = [
    "TeamValidationException",
    "InvalidInvitationDataException",
    "InvalidInvitationTokenException",
+    # Invoice exceptions
+    "InvoiceNotFoundException",
+    "InvoiceSettingsNotFoundException",
+    "InvoiceSettingsAlreadyExistException",
+    "InvoiceValidationException",
+    "InvoicePDFGenerationException",
+    "InvoicePDFNotFoundException",
+    "InvalidInvoiceStatusTransitionException",
+    "OrderNotFoundForInvoiceException",
    # Inventory exceptions
    "InventoryNotFoundException",
    "InsufficientInventoryException",
--- a/app/exceptions/invoice.py
+++ b/app/exceptions/invoice.py
@@ -0,0 +1,118 @@
+# app/exceptions/invoice.py
+"""
+Invoice-related exceptions.
+
+This module provides exception classes for invoice operations including:
+- Invoice not found errors
+- Invoice settings validation
+- PDF generation errors
+- Invoice status transitions
+"""
+
+from typing import Any
+
+from .base import BusinessLogicException, ResourceNotFoundException, WizamartException
+
+
+class InvoiceNotFoundException(ResourceNotFoundException):
+    """Raised when an invoice is not found."""
+
+    def __init__(self, invoice_id: int | str):
+        super().__init__(
+            resource_type="Invoice",
+            identifier=str(invoice_id),
+            error_code="INVOICE_NOT_FOUND",
+        )
+
+
+class InvoiceSettingsNotFoundException(ResourceNotFoundException):
+    """Raised when invoice settings are not found for a vendor."""
+
+    def __init__(self, vendor_id: int):
+        super().__init__(
+            resource_type="InvoiceSettings",
+            identifier=str(vendor_id),
+            message="Invoice settings not found. Create settings first.",
+            error_code="INVOICE_SETTINGS_NOT_FOUND",
+        )
+
+
+class InvoiceSettingsAlreadyExistException(WizamartException):
+    """Raised when trying to create invoice settings that already exist."""
+
+    def __init__(self, vendor_id: int):
+        super().__init__(
+            message=f"Invoice settings already exist for vendor {vendor_id}",
+            error_code="INVOICE_SETTINGS_ALREADY_EXIST",
+            status_code=409,
+            details={"vendor_id": vendor_id},
+        )
+
+
+class InvoiceValidationException(BusinessLogicException):
+    """Raised when invoice data validation fails."""
+
+    def __init__(self, message: str, details: dict[str, Any] | None = None):
+        super().__init__(
+            message=message,
+            error_code="INVOICE_VALIDATION_ERROR",
+            details=details,
+        )
+
+
+class InvoicePDFGenerationException(WizamartException):
+    """Raised when PDF generation fails."""
+
+    def __init__(self, invoice_id: int, reason: str):
+        super().__init__(
+            message=f"Failed to generate PDF for invoice {invoice_id}: {reason}",
+            error_code="INVOICE_PDF_GENERATION_FAILED",
+            status_code=500,
+            details={"invoice_id": invoice_id, "reason": reason},
+        )
+
+
+class InvoicePDFNotFoundException(ResourceNotFoundException):
+    """Raised when invoice PDF file is not found."""
+
+    def __init__(self, invoice_id: int):
+        super().__init__(
+            resource_type="InvoicePDF",
+            identifier=str(invoice_id),
+            message="PDF file not found. Generate the PDF first.",
+            error_code="INVOICE_PDF_NOT_FOUND",
+        )
+
+
+class InvalidInvoiceStatusTransitionException(BusinessLogicException):
+    """Raised when an invalid invoice status transition is attempted."""
+
+    def __init__(
+        self,
+        current_status: str,
+        new_status: str,
+        reason: str | None = None,
+    ):
+        message = f"Cannot change invoice status from '{current_status}' to '{new_status}'"
+        if reason:
+            message += f": {reason}"
+
+        super().__init__(
+            message=message,
+            error_code="INVALID_INVOICE_STATUS_TRANSITION",
+            details={
+                "current_status": current_status,
+                "new_status": new_status,
+            },
+        )
+
+
+class OrderNotFoundException(ResourceNotFoundException):
+    """Raised when an order for invoice creation is not found."""
+
+    def __init__(self, order_id: int):
+        super().__init__(
+            resource_type="Order",
+            identifier=str(order_id),
+            error_code="ORDER_NOT_FOUND_FOR_INVOICE",
+        )
--- a/app/services/invoice_pdf_service.py
+++ b/app/services/invoice_pdf_service.py
@@ -97,7 +97,7 @@ class InvoicePDFService:
        # Update invoice record with PDF path and timestamp
        invoice.pdf_path = str(pdf_path)
        invoice.pdf_generated_at = datetime.now(UTC)
-        db.commit()
+        db.flush()

        return str(pdf_path)

@@ -151,7 +151,7 @@ class InvoicePDFService:
        # Clear PDF fields
        invoice.pdf_path = None
        invoice.pdf_generated_at = None
-        db.commit()
+        db.flush()

        return True

--- a/app/services/invoice_service.py
+++ b/app/services/invoice_service.py
@@ -25,9 +25,17 @@ from sqlalchemy import and_, func
 from sqlalchemy.orm import Session

 from app.exceptions import (
-    OrderNotFoundException,
    ValidationException,
 )
+from app.exceptions.invoice import (
+    InvoiceNotFoundException,
+    InvoicePDFGenerationException,
+    InvoicePDFNotFoundException,
+    InvoiceSettingsAlreadyExistException,
+    InvoiceSettingsNotFoundException,
+    InvoiceValidationException,
+    OrderNotFoundException,
+)
 from models.database.invoice import (
    Invoice,
    InvoiceStatus,
@@ -89,18 +97,6 @@ LU_VAT_RATES = {
 }


-class InvoiceNotFoundException(Exception):
-    """Raised when invoice not found."""
-
-    pass
-
-
-class InvoiceSettingsNotFoundException(Exception):
-    """Raised when vendor invoice settings not found."""
-
-    pass
-
-
 class InvoiceService:
    """Service for invoice operations."""

@@ -205,9 +201,7 @@ class InvoiceService:
        """Get vendor invoice settings or raise exception."""
        settings = self.get_settings(db, vendor_id)
        if not settings:
-            raise InvoiceSettingsNotFoundException(
-                f"Invoice settings not configured for vendor {vendor_id}"
-            )
+            raise InvoiceSettingsNotFoundException(vendor_id)
        return settings

    def create_settings(
@@ -229,7 +223,7 @@ class InvoiceService:
            **data.model_dump(),
        )
        db.add(settings)
-        db.commit()
+        db.flush()
        db.refresh(settings)

        logger.info(f"Created invoice settings for vendor {vendor_id}")
@@ -249,7 +243,7 @@ class InvoiceService:
            setattr(settings, key, value)

        settings.updated_at = datetime.now(UTC)
-        db.commit()
+        db.flush()
        db.refresh(settings)

        logger.info(f"Updated invoice settings for vendor {vendor_id}")
@@ -278,7 +272,7 @@ class InvoiceService:
            is_vat_registered=bool(vendor.effective_tax_number),
        )
        db.add(settings)
-        db.commit()
+        db.flush()
        db.refresh(settings)

        logger.info(f"Created invoice settings from vendor data for vendor {vendor.id}")
@@ -435,7 +429,7 @@ class InvoiceService:
        )

        db.add(invoice)
-        db.commit()
+        db.flush()
        db.refresh(invoice)

        logger.info(
@@ -465,7 +459,7 @@ class InvoiceService:
        """Get invoice by ID or raise exception."""
        invoice = self.get_invoice(db, vendor_id, invoice_id)
        if not invoice:
-            raise InvoiceNotFoundException(f"Invoice {invoice_id} not found")
+            raise InvoiceNotFoundException(invoice_id)
        return invoice

    def get_invoice_by_number(
@@ -539,7 +533,7 @@ class InvoiceService:

        invoice.status = new_status
        invoice.updated_at = datetime.now(UTC)
-        db.commit()
+        db.flush()
        db.refresh(invoice)

        logger.info(f"Updated invoice {invoice.invoice_number} status to {new_status}")
--- a/app/services/subscription_service.py
+++ b/app/services/subscription_service.py
@@ -163,7 +163,7 @@ class SubscriptionService:
        )

        db.add(subscription)
-        db.commit()
+        db.flush()
        db.refresh(subscription)

        logger.info(
@@ -212,7 +212,7 @@ class SubscriptionService:
        )

        db.add(subscription)
-        db.commit()
+        db.flush()
        db.refresh(subscription)

        logger.info(f"Created subscription for vendor {vendor_id}: {data.tier}")
@@ -232,7 +232,7 @@ class SubscriptionService:
            setattr(subscription, key, value)

        subscription.updated_at = datetime.now(UTC)
-        db.commit()
+        db.flush()
        db.refresh(subscription)

        logger.info(f"Updated subscription for vendor {vendor_id}")
@@ -255,7 +255,7 @@ class SubscriptionService:
        if subscription.status == SubscriptionStatus.TRIAL.value:
            subscription.status = SubscriptionStatus.ACTIVE.value

-        db.commit()
+        db.flush()
        db.refresh(subscription)

        logger.info(f"Upgraded vendor {vendor_id} from {old_tier} to {new_tier}")
@@ -275,7 +275,7 @@ class SubscriptionService:
        subscription.cancellation_reason = reason
        subscription.updated_at = datetime.now(UTC)

-        db.commit()
+        db.flush()
        db.refresh(subscription)

        logger.info(f"Cancelled subscription for vendor {vendor_id}")
@@ -342,13 +342,13 @@ class SubscriptionService:
        """
        subscription = self.get_or_create_subscription(db, vendor_id)
        subscription.increment_order_count()
-        db.commit()
+        db.flush()

    def reset_period_counters(self, db: Session, vendor_id: int) -> None:
        """Reset counters for a new billing period."""
        subscription = self.get_subscription_or_raise(db, vendor_id)
        subscription.reset_period_counters()
-        db.commit()
+        db.flush()
        logger.info(f"Reset period counters for vendor {vendor_id}")

    # =========================================================================
--- a/models/schema/invoice.py
+++ b/models/schema/invoice.py
@@ -281,3 +281,30 @@ class InvoiceListPaginatedResponse(BaseModel):
    page: int
    per_page: int
    pages: int
+
+
+# ============================================================================
+# PDF Response
+# ============================================================================
+
+
+class InvoicePDFGeneratedResponse(BaseModel):
+    """Response for PDF generation."""
+
+    pdf_path: str
+    message: str = "PDF generated successfully"
+
+
+class InvoiceStatsResponse(BaseModel):
+    """Invoice statistics response."""
+
+    total_invoices: int
+    total_revenue_cents: int
+    draft_count: int
+    issued_count: int
+    paid_count: int
+    cancelled_count: int
+
+    @property
+    def total_revenue(self) -> float:
+        return self.total_revenue_cents / 100