From 7b81f59eba49ff0474667017eb5fb7bb46ccc36a Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Tue, 6 Jan 2026 22:40:10 +0100 Subject: [PATCH] fix: resolve architecture validation errors in media and customers APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add proper media exceptions (MediaNotFoundException, MediaUploadException, etc.) - Update media service to use exceptions from app/exceptions/media - Remove direct HTTPException raises from vendor/media.py and vendor/customers.py - Move db.query from customers API to service layer (get_customer_orders) - Fix pagination macro call in vendor/media.html template - Update media.js: add parent init call, PlatformSettings, apiClient.postFormData - Add try/catch error handling to media.js init method 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- app/api/v1/vendor/customers.py | 187 +++++++++++++------------------ app/api/v1/vendor/media.py | 183 +++++++++++++----------------- app/exceptions/media.py | 102 ++++++++++++++++- app/services/customer_service.py | 44 ++++++++ app/services/media_service.py | 32 +++--- app/templates/vendor/media.html | 2 +- static/vendor/js/media.js | 95 ++++++++++++++-- 7 files changed, 397 insertions(+), 248 deletions(-) diff --git a/app/api/v1/vendor/customers.py b/app/api/v1/vendor/customers.py index 8252d1b4..958d6651 100644 --- a/app/api/v1/vendor/customers.py +++ b/app/api/v1/vendor/customers.py @@ -8,14 +8,12 @@ The get_current_vendor_api dependency guarantees token_vendor_id is present. import logging -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.api.deps import get_current_vendor_api from app.core.database import get_db -from app.exceptions.customer import CustomerNotFoundException from app.services.customer_service import customer_service -from models.database.order import Order from models.database.user import User from models.schema.customer import ( CustomerDetailResponse, @@ -78,38 +76,35 @@ def get_customer_details( - Verify customer belongs to vendor - Include order statistics """ - try: - customer = customer_service.get_customer( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) + # Service will raise CustomerNotFoundException if not found + customer = customer_service.get_customer( + db=db, + vendor_id=current_user.token_vendor_id, + 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, - ) + # 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, - first_name=customer.first_name, - last_name=customer.last_name, - phone=customer.phone, - 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, - ) - - except CustomerNotFoundException: - raise HTTPException(status_code=404, detail="Customer not found") + return CustomerDetailResponse( + id=customer.id, + email=customer.email, + first_name=customer.first_name, + last_name=customer.last_name, + phone=customer.phone, + 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, + ) @router.get("/{customer_id}/orders", response_model=CustomerOrdersResponse) @@ -127,45 +122,30 @@ def get_customer_orders( - Filter by vendor_id - Return order details """ - try: - # Verify customer belongs to vendor - customer_service.get_customer( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) + # 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, + ) - # Get customer orders - query = ( - db.query(Order) - .filter( - Order.customer_id == customer_id, - Order.vendor_id == current_user.token_vendor_id, - ) - .order_by(Order.created_at.desc()) - ) - - total = query.count() - orders = query.offset(skip).limit(limit).all() - - 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, - ) - - except CustomerNotFoundException: - raise HTTPException(status_code=404, detail="Customer not found") + 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, + ) @router.put("/{customer_id}", response_model=CustomerMessageResponse) @@ -181,23 +161,17 @@ def update_customer( - Update customer details - Verify customer belongs to vendor """ - try: - customer_service.update_customer( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - customer_data=customer_data, - ) + # Service will raise CustomerNotFoundException if not found + customer_service.update_customer( + db=db, + vendor_id=current_user.token_vendor_id, + customer_id=customer_id, + customer_data=customer_data, + ) - db.commit() + db.commit() - return CustomerMessageResponse(message="Customer updated successfully") - - except CustomerNotFoundException: - raise HTTPException(status_code=404, detail="Customer not found") - except Exception as e: - db.rollback() - raise HTTPException(status_code=400, detail=str(e)) + return CustomerMessageResponse(message="Customer updated successfully") @router.put("/{customer_id}/status", response_model=CustomerMessageResponse) @@ -212,23 +186,17 @@ def toggle_customer_status( - Toggle customer is_active status - Verify customer belongs to vendor """ - try: - customer = customer_service.toggle_customer_status( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) + # Service will raise CustomerNotFoundException if not found + customer = customer_service.toggle_customer_status( + db=db, + vendor_id=current_user.token_vendor_id, + customer_id=customer_id, + ) - db.commit() + db.commit() - status = "activated" if customer.is_active else "deactivated" - return CustomerMessageResponse(message=f"Customer {status} successfully") - - except CustomerNotFoundException: - raise HTTPException(status_code=404, detail="Customer not found") - except Exception as e: - db.rollback() - raise HTTPException(status_code=400, detail=str(e)) + status = "activated" if customer.is_active else "deactivated" + return CustomerMessageResponse(message=f"Customer {status} successfully") @router.get("/{customer_id}/stats", response_model=CustomerStatisticsResponse) @@ -245,14 +213,11 @@ def get_customer_statistics( - Average order value - Last order date """ - try: - stats = customer_service.get_customer_statistics( - db=db, - vendor_id=current_user.token_vendor_id, - customer_id=customer_id, - ) + # 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) - - except CustomerNotFoundException: - raise HTTPException(status_code=404, detail="Customer not found") + return CustomerStatisticsResponse(**stats) diff --git a/app/api/v1/vendor/media.py b/app/api/v1/vendor/media.py index 43e56e92..e778c2f6 100644 --- a/app/api/v1/vendor/media.py +++ b/app/api/v1/vendor/media.py @@ -8,12 +8,13 @@ The get_current_vendor_api dependency guarantees token_vendor_id is present. import logging -from fastapi import APIRouter, Depends, File, HTTPException, Query, UploadFile +from fastapi import APIRouter, Depends, File, Query, UploadFile from sqlalchemy.orm import Session from app.api.deps import get_current_vendor_api from app.core.database import get_db -from app.services.media_service import MediaNotFoundException, media_service +from app.exceptions.media import MediaOptimizationException +from app.services.media_service import media_service from models.database.user import User from models.schema.media import ( MediaDetailResponse, @@ -86,35 +87,29 @@ async def upload_media( - Save metadata to database - Return file URL """ - try: - # Read file content - file_content = await file.read() + # Read file content + file_content = await file.read() - # Upload using service - media_file = await media_service.upload_file( - db=db, - vendor_id=current_user.token_vendor_id, - file_content=file_content, - filename=file.filename or "unnamed", - folder=folder or "general", - ) + # Upload using service (exceptions will propagate to handler) + media_file = await media_service.upload_file( + db=db, + vendor_id=current_user.token_vendor_id, + file_content=file_content, + filename=file.filename or "unnamed", + folder=folder or "general", + ) - db.commit() + db.commit() - return MediaUploadResponse( - id=media_file.id, - file_url=media_file.file_url, - thumbnail_url=media_file.thumbnail_url, - filename=media_file.original_filename, - file_size=media_file.file_size, - media_type=media_file.media_type, - message="File uploaded successfully", - ) - - except Exception as e: - db.rollback() - logger.error(f"Failed to upload media: {e}") - raise HTTPException(status_code=400, detail=str(e)) + return MediaUploadResponse( + id=media_file.id, + file_url=media_file.file_url, + thumbnail_url=media_file.thumbnail_url, + filename=media_file.original_filename, + file_size=media_file.file_size, + media_type=media_file.media_type, + message="File uploaded successfully", + ) @router.post("/upload/multiple", response_model=MultipleUploadResponse) @@ -185,17 +180,14 @@ def get_media_details( - Return file URL - Return basic info """ - try: - media = media_service.get_media( - db=db, - vendor_id=current_user.token_vendor_id, - media_id=media_id, - ) + # Service will raise MediaNotFoundException if not found + media = media_service.get_media( + db=db, + vendor_id=current_user.token_vendor_id, + media_id=media_id, + ) - return MediaDetailResponse.model_validate(media) - - except MediaNotFoundException: - raise HTTPException(status_code=404, detail="Media file not found") + return MediaDetailResponse.model_validate(media) @router.put("/{media_id}", response_model=MediaDetailResponse) @@ -213,27 +205,21 @@ def update_media_metadata( - Update description - Move to different folder """ - try: - media = media_service.update_media_metadata( - db=db, - vendor_id=current_user.token_vendor_id, - media_id=media_id, - filename=metadata.filename, - alt_text=metadata.alt_text, - description=metadata.description, - folder=metadata.folder, - metadata=metadata.metadata, - ) + # Service will raise MediaNotFoundException if not found + media = media_service.update_media_metadata( + db=db, + vendor_id=current_user.token_vendor_id, + media_id=media_id, + filename=metadata.filename, + alt_text=metadata.alt_text, + description=metadata.description, + folder=metadata.folder, + metadata=metadata.metadata, + ) - db.commit() + db.commit() - return MediaDetailResponse.model_validate(media) - - except MediaNotFoundException: - raise HTTPException(status_code=404, detail="Media file not found") - except Exception as e: - db.rollback() - raise HTTPException(status_code=400, detail=str(e)) + return MediaDetailResponse.model_validate(media) @router.delete("/{media_id}", response_model=MediaDetailResponse) @@ -250,22 +236,16 @@ def delete_media( - Delete database record - Return success/error """ - try: - media_service.delete_media( - db=db, - vendor_id=current_user.token_vendor_id, - media_id=media_id, - ) + # Service will raise MediaNotFoundException if not found + media_service.delete_media( + db=db, + vendor_id=current_user.token_vendor_id, + media_id=media_id, + ) - db.commit() + db.commit() - return MediaDetailResponse(message="Media file deleted successfully") - - except MediaNotFoundException: - raise HTTPException(status_code=404, detail="Media file not found") - except Exception as e: - db.rollback() - raise HTTPException(status_code=400, detail=str(e)) + return MediaDetailResponse(message="Media file deleted successfully") @router.get("/{media_id}/usage", response_model=MediaUsageResponse) @@ -280,17 +260,14 @@ def get_media_usage( - Check products using this media - Return list of usage """ - try: - usage = media_service.get_media_usage( - db=db, - vendor_id=current_user.token_vendor_id, - media_id=media_id, - ) + # Service will raise MediaNotFoundException if not found + usage = media_service.get_media_usage( + db=db, + vendor_id=current_user.token_vendor_id, + media_id=media_id, + ) - return MediaUsageResponse(**usage) - - except MediaNotFoundException: - raise HTTPException(status_code=404, detail="Media file not found") + return MediaUsageResponse(**usage) @router.post("/optimize/{media_id}", response_model=OptimizationResultResponse) @@ -304,30 +281,24 @@ def optimize_media( Note: Image optimization requires PIL/Pillow to be installed. """ - try: - media = media_service.get_media( - db=db, - vendor_id=current_user.token_vendor_id, - media_id=media_id, - ) + # Service will raise MediaNotFoundException if not found + media = media_service.get_media( + db=db, + vendor_id=current_user.token_vendor_id, + media_id=media_id, + ) - if media.media_type != "image": - raise HTTPException( - status_code=400, - detail="Only images can be optimized" - ) + if media.media_type != "image": + raise MediaOptimizationException("Only images can be optimized") - # For now, return current state - optimization is done on upload - return OptimizationResultResponse( - media_id=media_id, - original_size=media.file_size, - optimized_size=media.optimized_size or media.file_size, - savings_percent=0.0 if not media.optimized_size else - round((1 - media.optimized_size / media.file_size) * 100, 1), - optimized_url=media.file_url, - message="Image optimization applied on upload" if media.is_optimized - else "Image not yet optimized", - ) - - except MediaNotFoundException: - raise HTTPException(status_code=404, detail="Media file not found") + # For now, return current state - optimization is done on upload + return OptimizationResultResponse( + media_id=media_id, + original_size=media.file_size, + optimized_size=media.optimized_size or media.file_size, + savings_percent=0.0 if not media.optimized_size else + round((1 - media.optimized_size / media.file_size) * 100, 1), + optimized_url=media.file_url, + message="Image optimization applied on upload" if media.is_optimized + else "Image not yet optimized", + ) diff --git a/app/exceptions/media.py b/app/exceptions/media.py index d7663627..55635187 100644 --- a/app/exceptions/media.py +++ b/app/exceptions/media.py @@ -1 +1,101 @@ -# Media/file management exceptions +# app/exceptions/media.py +""" +Media/file management exceptions. +""" + +from typing import Any + +from .base import ( + BusinessLogicException, + ResourceNotFoundException, + ValidationException, +) + + +class MediaNotFoundException(ResourceNotFoundException): + """Raised when a media file is not found.""" + + def __init__(self, media_id: int | str): + super().__init__( + resource_type="MediaFile", + identifier=str(media_id), + message=f"Media file '{media_id}' not found", + error_code="MEDIA_NOT_FOUND", + ) + + +class MediaUploadException(BusinessLogicException): + """Raised when media upload fails.""" + + def __init__(self, message: str, details: dict[str, Any] | None = None): + super().__init__( + message=message, + error_code="MEDIA_UPLOAD_FAILED", + details=details, + ) + + +class MediaValidationException(ValidationException): + """Raised when media validation fails (file type, size, etc.).""" + + def __init__( + self, + message: str = "Media validation failed", + field: str | None = None, + details: dict[str, Any] | None = None, + ): + super().__init__(message=message, field=field, details=details) + self.error_code = "MEDIA_VALIDATION_FAILED" + + +class UnsupportedMediaTypeException(ValidationException): + """Raised when media file type is not supported.""" + + def __init__(self, file_type: str, allowed_types: list[str] | None = None): + details = {"file_type": file_type} + if allowed_types: + details["allowed_types"] = allowed_types + + super().__init__( + message=f"Unsupported media type: {file_type}", + field="file", + details=details, + ) + self.error_code = "UNSUPPORTED_MEDIA_TYPE" + + +class MediaFileTooLargeException(ValidationException): + """Raised when media file exceeds size limit.""" + + def __init__(self, file_size: int, max_size: int, media_type: str = "file"): + super().__init__( + message=f"File size ({file_size} bytes) exceeds maximum allowed ({max_size} bytes) for {media_type}", + field="file", + details={ + "file_size": file_size, + "max_size": max_size, + "media_type": media_type, + }, + ) + self.error_code = "MEDIA_FILE_TOO_LARGE" + + +class MediaOptimizationException(BusinessLogicException): + """Raised when media optimization fails.""" + + def __init__(self, message: str = "Only images can be optimized"): + super().__init__( + message=message, + error_code="MEDIA_OPTIMIZATION_FAILED", + ) + + +class MediaDeleteException(BusinessLogicException): + """Raised when media deletion fails.""" + + def __init__(self, message: str, details: dict[str, Any] | None = None): + super().__init__( + message=message, + error_code="MEDIA_DELETE_FAILED", + details=details, + ) diff --git a/app/services/customer_service.py b/app/services/customer_service.py index ce13ce20..0bda80b7 100644 --- a/app/services/customer_service.py +++ b/app/services/customer_service.py @@ -310,6 +310,50 @@ 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 + """ + from models.database.order import Order + + # Verify customer belongs to vendor + self.get_customer(db, vendor_id, customer_id) + + # 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 + def get_customer_statistics( self, db: Session, vendor_id: int, customer_id: int ) -> dict: diff --git a/app/services/media_service.py b/app/services/media_service.py index be72ebf9..6b766ac7 100644 --- a/app/services/media_service.py +++ b/app/services/media_service.py @@ -20,7 +20,13 @@ from pathlib import Path from sqlalchemy import or_ from sqlalchemy.orm import Session -from app.exceptions import ValidationException +from app.exceptions.media import ( + MediaNotFoundException, + MediaUploadException, + MediaValidationException, + UnsupportedMediaTypeException, + MediaFileTooLargeException, +) from models.database.media import MediaFile, ProductMedia logger = logging.getLogger(__name__) @@ -63,14 +69,6 @@ MAX_FILE_SIZES = { THUMBNAIL_SIZE = (200, 200) -class MediaNotFoundException(Exception): - """Raised when media file is not found.""" - - def __init__(self, media_id: int): - self.media_id = media_id - super().__init__(f"Media file {media_id} not found") - - class MediaService: """Service for vendor media library operations.""" @@ -105,26 +103,24 @@ class MediaService: Tuple of (extension, media_type) Raises: - ValidationException: If file is invalid + MediaValidationException: If file is invalid + UnsupportedMediaTypeException: If file type is not supported + MediaFileTooLargeException: If file exceeds size limit """ ext = self._get_file_extension(filename) if not ext: - raise ValidationException("File must have an extension") + raise MediaValidationException("File must have an extension", field="file") media_type = self._get_media_type(ext) if not media_type: - allowed = ", ".join(sorted(ALLOWED_EXTENSIONS.keys())) - raise ValidationException( - f"File type '{ext}' not allowed. Allowed types: {allowed}" + raise UnsupportedMediaTypeException( + ext, allowed_types=list(ALLOWED_EXTENSIONS.keys()) ) max_size = MAX_FILE_SIZES.get(media_type, 10 * 1024 * 1024) if file_size > max_size: - max_mb = max_size / (1024 * 1024) - raise ValidationException( - f"File too large. Maximum size for {media_type} is {max_mb:.0f} MB" - ) + raise MediaFileTooLargeException(file_size, max_size, media_type) return ext, media_type diff --git a/app/templates/vendor/media.html b/app/templates/vendor/media.html index 8085716b..b40b426b 100644 --- a/app/templates/vendor/media.html +++ b/app/templates/vendor/media.html @@ -199,7 +199,7 @@
- {{ pagination('pagination', 'loadMedia') }} + {{ pagination('pagination.pages > 1') }}
diff --git a/static/vendor/js/media.js b/static/vendor/js/media.js index 55c004df..06aa79e1 100644 --- a/static/vendor/js/media.js +++ b/static/vendor/js/media.js @@ -48,6 +48,60 @@ function vendorMedia() { pages: 0 }, + // Computed pagination properties required by pagination macro + get startIndex() { + if (this.pagination.total === 0) return 0; + return (this.pagination.page - 1) * this.pagination.per_page + 1; + }, + + get endIndex() { + return Math.min(this.pagination.page * this.pagination.per_page, this.pagination.total); + }, + + get totalPages() { + return this.pagination.pages; + }, + + get pageNumbers() { + const pages = []; + const total = this.pagination.pages; + const current = this.pagination.page; + + if (total <= 7) { + for (let i = 1; i <= total; i++) pages.push(i); + } else { + pages.push(1); + if (current > 3) pages.push('...'); + for (let i = Math.max(2, current - 1); i <= Math.min(total - 1, current + 1); i++) { + pages.push(i); + } + if (current < total - 2) pages.push('...'); + pages.push(total); + } + return pages; + }, + + previousPage() { + if (this.pagination.page > 1) { + this.pagination.page--; + this.loadMedia(); + } + }, + + nextPage() { + if (this.pagination.page < this.pagination.pages) { + this.pagination.page++; + this.loadMedia(); + } + }, + + goToPage(pageNum) { + if (pageNum !== '...' && pageNum >= 1 && pageNum <= this.pagination.pages) { + this.pagination.page = pageNum; + this.loadMedia(); + } + }, + // Modal states showUploadModal: false, showDetailModal: false, @@ -65,8 +119,30 @@ function vendorMedia() { uploadingFiles: [], async init() { + // Guard against duplicate initialization + if (window._vendorMediaInitialized) return; + window._vendorMediaInitialized = true; + vendorMediaLog.info('Initializing media library...'); - await this.loadMedia(); + + try { + // IMPORTANT: Call parent init first to set vendorCode from URL + const parentInit = data().init; + if (parentInit) { + await parentInit.call(this); + } + + // Initialize pagination per_page from PlatformSettings + if (window.PlatformSettings) { + this.pagination.per_page = await window.PlatformSettings.getRowsPerPage(); + } + + await this.loadMedia(); + } catch (err) { + vendorMediaLog.error('Failed to initialize media library:', err); + this.error = err.message || 'Failed to initialize media library'; + this.loading = false; + } }, async loadMedia() { @@ -231,22 +307,19 @@ function vendorMedia() { const formData = new FormData(); formData.append('file', file); - const response = await fetch(`/api/v1/vendor/media/upload?folder=${this.uploadFolder}`, { - method: 'POST', - headers: { - 'Authorization': `Bearer ${localStorage.getItem('vendor_token')}` - }, - body: formData - }); + // Use apiClient.postFormData for automatic auth handling + const response = await apiClient.postFormData( + `/vendor/media/upload?folder=${this.uploadFolder}`, + formData + ); if (response.ok) { uploadItem.status = 'success'; vendorMediaLog.info(`Uploaded: ${file.name}`); } else { - const errorData = await response.json(); uploadItem.status = 'error'; - uploadItem.error = errorData.detail || 'Upload failed'; - vendorMediaLog.error(`Upload failed for ${file.name}:`, errorData); + uploadItem.error = response.message || 'Upload failed'; + vendorMediaLog.error(`Upload failed for ${file.name}:`, response); } } catch (err) { uploadItem.status = 'error';