feat: enhance messaging system with improved API and tests
- Refactor messaging API endpoints for admin, shop, and vendor - Add message-specific exceptions (ConversationNotFoundException, etc.) - Enhance messaging service with additional helper methods - Add comprehensive test fixtures for messaging - Add integration tests for admin and vendor messaging APIs - Add unit tests for messaging and attachment services 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -12,12 +12,19 @@ Provides endpoints for:
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, UploadFile
|
||||
from fastapi import APIRouter, Depends, File, Form, Query, UploadFile
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.api.deps import get_current_admin_api
|
||||
from app.core.database import get_db
|
||||
from app.exceptions import (
|
||||
ConversationClosedException,
|
||||
ConversationNotFoundException,
|
||||
InvalidConversationTypeException,
|
||||
InvalidRecipientTypeException,
|
||||
MessageAttachmentException,
|
||||
)
|
||||
from app.services.message_attachment_service import message_attachment_service
|
||||
from app.services.messaging_service import messaging_service
|
||||
from models.database.message import ConversationType, ParticipantType
|
||||
@@ -231,73 +238,45 @@ def get_recipients(
|
||||
current_admin: User = Depends(get_current_admin_api),
|
||||
) -> RecipientListResponse:
|
||||
"""Get list of available recipients for compose modal."""
|
||||
from models.database.customer import Customer
|
||||
from models.database.vendor import VendorUser
|
||||
|
||||
recipients = []
|
||||
|
||||
if recipient_type == ParticipantType.VENDOR:
|
||||
# List vendor users (for admin_vendor conversations)
|
||||
query = (
|
||||
db.query(User, VendorUser)
|
||||
.join(VendorUser, User.id == VendorUser.user_id)
|
||||
.filter(User.is_active == True) # noqa: E712
|
||||
recipient_data, total = messaging_service.get_vendor_recipients(
|
||||
db=db,
|
||||
vendor_id=vendor_id,
|
||||
search=search,
|
||||
skip=skip,
|
||||
limit=limit,
|
||||
)
|
||||
if vendor_id:
|
||||
query = query.filter(VendorUser.vendor_id == vendor_id)
|
||||
if search:
|
||||
search_pattern = f"%{search}%"
|
||||
query = query.filter(
|
||||
(User.username.ilike(search_pattern))
|
||||
| (User.email.ilike(search_pattern))
|
||||
| (User.first_name.ilike(search_pattern))
|
||||
| (User.last_name.ilike(search_pattern))
|
||||
recipients = [
|
||||
RecipientOption(
|
||||
id=r["id"],
|
||||
type=r["type"],
|
||||
name=r["name"],
|
||||
email=r["email"],
|
||||
vendor_id=r["vendor_id"],
|
||||
vendor_name=r.get("vendor_name"),
|
||||
)
|
||||
|
||||
total = query.count()
|
||||
results = query.offset(skip).limit(limit).all()
|
||||
|
||||
for user, vendor_user in results:
|
||||
name = f"{user.first_name or ''} {user.last_name or ''}".strip() or user.username
|
||||
recipients.append(
|
||||
RecipientOption(
|
||||
id=user.id,
|
||||
type=ParticipantType.VENDOR,
|
||||
name=name,
|
||||
email=user.email,
|
||||
vendor_id=vendor_user.vendor_id,
|
||||
vendor_name=vendor_user.vendor.name if vendor_user.vendor else None,
|
||||
)
|
||||
)
|
||||
|
||||
for r in recipient_data
|
||||
]
|
||||
elif recipient_type == ParticipantType.CUSTOMER:
|
||||
# List customers (for admin_customer conversations)
|
||||
query = db.query(Customer).filter(Customer.is_active == True) # noqa: E712
|
||||
if vendor_id:
|
||||
query = query.filter(Customer.vendor_id == vendor_id)
|
||||
if search:
|
||||
search_pattern = f"%{search}%"
|
||||
query = query.filter(
|
||||
(Customer.email.ilike(search_pattern))
|
||||
| (Customer.first_name.ilike(search_pattern))
|
||||
| (Customer.last_name.ilike(search_pattern))
|
||||
)
|
||||
|
||||
total = query.count()
|
||||
results = query.offset(skip).limit(limit).all()
|
||||
|
||||
for customer in results:
|
||||
name = f"{customer.first_name or ''} {customer.last_name or ''}".strip()
|
||||
recipients.append(
|
||||
RecipientOption(
|
||||
id=customer.id,
|
||||
type=ParticipantType.CUSTOMER,
|
||||
name=name or customer.email,
|
||||
email=customer.email,
|
||||
vendor_id=customer.vendor_id,
|
||||
)
|
||||
recipient_data, total = messaging_service.get_customer_recipients(
|
||||
db=db,
|
||||
vendor_id=vendor_id,
|
||||
search=search,
|
||||
skip=skip,
|
||||
limit=limit,
|
||||
)
|
||||
recipients = [
|
||||
RecipientOption(
|
||||
id=r["id"],
|
||||
type=r["type"],
|
||||
name=r["name"],
|
||||
email=r["email"],
|
||||
vendor_id=r["vendor_id"],
|
||||
)
|
||||
for r in recipient_data
|
||||
]
|
||||
else:
|
||||
recipients = []
|
||||
total = 0
|
||||
|
||||
return RecipientListResponse(recipients=recipients, total=total)
|
||||
@@ -320,9 +299,9 @@ def create_conversation(
|
||||
ConversationType.ADMIN_VENDOR,
|
||||
ConversationType.ADMIN_CUSTOMER,
|
||||
]:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Admin can only create admin_vendor or admin_customer conversations",
|
||||
raise InvalidConversationTypeException(
|
||||
message="Admin can only create admin_vendor or admin_customer conversations",
|
||||
allowed_types=["admin_vendor", "admin_customer"],
|
||||
)
|
||||
|
||||
# Validate recipient type matches conversation type
|
||||
@@ -330,17 +309,17 @@ def create_conversation(
|
||||
data.conversation_type == ConversationType.ADMIN_VENDOR
|
||||
and data.recipient_type != ParticipantType.VENDOR
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="admin_vendor conversations require a vendor recipient",
|
||||
raise InvalidRecipientTypeException(
|
||||
conversation_type="admin_vendor",
|
||||
expected_recipient_type="vendor",
|
||||
)
|
||||
if (
|
||||
data.conversation_type == ConversationType.ADMIN_CUSTOMER
|
||||
and data.recipient_type != ParticipantType.CUSTOMER
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="admin_customer conversations require a customer recipient",
|
||||
raise InvalidRecipientTypeException(
|
||||
conversation_type="admin_customer",
|
||||
expected_recipient_type="customer",
|
||||
)
|
||||
|
||||
# Create conversation
|
||||
@@ -460,7 +439,7 @@ def get_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
# Mark as read if requested
|
||||
if mark_read:
|
||||
@@ -498,12 +477,10 @@ async def send_message(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
if conversation.is_closed:
|
||||
raise HTTPException(
|
||||
status_code=400, detail="Cannot send messages to a closed conversation"
|
||||
)
|
||||
raise ConversationClosedException(conversation_id)
|
||||
|
||||
# Process attachments
|
||||
attachments = []
|
||||
@@ -514,7 +491,7 @@ async def send_message(
|
||||
)
|
||||
attachments.append(att_data)
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
raise MessageAttachmentException(str(e))
|
||||
|
||||
# Send message
|
||||
message = messaging_service.send_message(
|
||||
@@ -556,7 +533,7 @@ def close_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
db.commit()
|
||||
logger.info(
|
||||
@@ -585,7 +562,7 @@ def reopen_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
db.commit()
|
||||
logger.info(
|
||||
|
||||
@@ -21,7 +21,12 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.api.deps import get_current_customer_api
|
||||
from app.core.database import get_db
|
||||
from app.exceptions import ConversationNotFoundException, VendorNotFoundException
|
||||
from app.exceptions import (
|
||||
AttachmentNotFoundException,
|
||||
ConversationClosedException,
|
||||
ConversationNotFoundException,
|
||||
VendorNotFoundException,
|
||||
)
|
||||
from app.services.message_attachment_service import message_attachment_service
|
||||
from app.services.messaging_service import messaging_service
|
||||
from models.database.customer import Customer
|
||||
@@ -292,12 +297,7 @@ async def send_message(
|
||||
|
||||
# Check if conversation is closed
|
||||
if conversation.is_closed:
|
||||
from fastapi import HTTPException
|
||||
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Cannot send messages to a closed conversation",
|
||||
)
|
||||
raise ConversationClosedException(conversation_id)
|
||||
|
||||
# Process attachments
|
||||
attachment_data = []
|
||||
@@ -405,7 +405,6 @@ async def download_attachment(
|
||||
|
||||
Validates that customer has access to the conversation.
|
||||
"""
|
||||
from fastapi import HTTPException
|
||||
from fastapi.responses import FileResponse
|
||||
|
||||
vendor = getattr(request.state, "vendor", None)
|
||||
@@ -433,7 +432,7 @@ async def download_attachment(
|
||||
)
|
||||
|
||||
if not attachment:
|
||||
raise HTTPException(status_code=404, detail="Attachment not found")
|
||||
raise AttachmentNotFoundException(attachment_id)
|
||||
|
||||
return FileResponse(
|
||||
path=attachment.file_path,
|
||||
@@ -455,7 +454,6 @@ async def get_attachment_thumbnail(
|
||||
|
||||
Validates that customer has access to the conversation.
|
||||
"""
|
||||
from fastapi import HTTPException
|
||||
from fastapi.responses import FileResponse
|
||||
|
||||
vendor = getattr(request.state, "vendor", None)
|
||||
@@ -483,7 +481,7 @@ async def get_attachment_thumbnail(
|
||||
)
|
||||
|
||||
if not attachment or not attachment.thumbnail_path:
|
||||
raise HTTPException(status_code=404, detail="Thumbnail not found")
|
||||
raise AttachmentNotFoundException(f"{attachment_id}/thumbnail")
|
||||
|
||||
return FileResponse(
|
||||
path=attachment.thumbnail_path,
|
||||
|
||||
86
app/api/v1/vendor/messages.py
vendored
86
app/api/v1/vendor/messages.py
vendored
@@ -14,12 +14,19 @@ Uses get_current_vendor_api dependency which guarantees token_vendor_id is prese
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, UploadFile
|
||||
from fastapi import APIRouter, Depends, File, Form, Query, UploadFile
|
||||
from pydantic import BaseModel
|
||||
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 (
|
||||
ConversationClosedException,
|
||||
ConversationNotFoundException,
|
||||
InvalidConversationTypeException,
|
||||
InvalidRecipientTypeException,
|
||||
MessageAttachmentException,
|
||||
)
|
||||
from app.services.message_attachment_service import message_attachment_service
|
||||
from app.services.messaging_service import messaging_service
|
||||
from models.database.message import ConversationType, ParticipantType
|
||||
@@ -230,41 +237,30 @@ def get_recipients(
|
||||
current_user: User = Depends(get_current_vendor_api),
|
||||
) -> RecipientListResponse:
|
||||
"""Get list of available recipients for compose modal."""
|
||||
from models.database.customer import Customer
|
||||
|
||||
vendor_id = current_user.token_vendor_id
|
||||
recipients = []
|
||||
|
||||
if recipient_type == ParticipantType.CUSTOMER:
|
||||
# List customers for this vendor (for vendor_customer conversations)
|
||||
query = db.query(Customer).filter(
|
||||
Customer.vendor_id == vendor_id,
|
||||
Customer.is_active == True, # noqa: E712
|
||||
recipient_data, total = messaging_service.get_customer_recipients(
|
||||
db=db,
|
||||
vendor_id=vendor_id,
|
||||
search=search,
|
||||
skip=skip,
|
||||
limit=limit,
|
||||
)
|
||||
if search:
|
||||
search_pattern = f"%{search}%"
|
||||
query = query.filter(
|
||||
(Customer.email.ilike(search_pattern))
|
||||
| (Customer.first_name.ilike(search_pattern))
|
||||
| (Customer.last_name.ilike(search_pattern))
|
||||
)
|
||||
|
||||
total = query.count()
|
||||
results = query.offset(skip).limit(limit).all()
|
||||
|
||||
for customer in results:
|
||||
name = f"{customer.first_name or ''} {customer.last_name or ''}".strip()
|
||||
recipients.append(
|
||||
RecipientOption(
|
||||
id=customer.id,
|
||||
type=ParticipantType.CUSTOMER,
|
||||
name=name or customer.email,
|
||||
email=customer.email,
|
||||
vendor_id=customer.vendor_id,
|
||||
)
|
||||
recipients = [
|
||||
RecipientOption(
|
||||
id=r["id"],
|
||||
type=r["type"],
|
||||
name=r["name"],
|
||||
email=r["email"],
|
||||
vendor_id=r["vendor_id"],
|
||||
)
|
||||
for r in recipient_data
|
||||
]
|
||||
else:
|
||||
# Vendors can't start conversations with admins - admins initiate those
|
||||
recipients = []
|
||||
total = 0
|
||||
|
||||
return RecipientListResponse(recipients=recipients, total=total)
|
||||
@@ -286,15 +282,15 @@ def create_conversation(
|
||||
|
||||
# Vendors can only create vendor_customer conversations
|
||||
if data.conversation_type != ConversationType.VENDOR_CUSTOMER:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Vendors can only create vendor_customer conversations",
|
||||
raise InvalidConversationTypeException(
|
||||
message="Vendors can only create vendor_customer conversations",
|
||||
allowed_types=["vendor_customer"],
|
||||
)
|
||||
|
||||
if data.recipient_type != ParticipantType.CUSTOMER:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="vendor_customer conversations require a customer recipient",
|
||||
raise InvalidRecipientTypeException(
|
||||
conversation_type="vendor_customer",
|
||||
expected_recipient_type="customer",
|
||||
)
|
||||
|
||||
# Create conversation
|
||||
@@ -416,11 +412,11 @@ def get_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
# Verify vendor context
|
||||
if conversation.vendor_id and conversation.vendor_id != vendor_id:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
# Mark as read if requested
|
||||
if mark_read:
|
||||
@@ -460,16 +456,14 @@ async def send_message(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
# Verify vendor context
|
||||
if conversation.vendor_id and conversation.vendor_id != vendor_id:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
if conversation.is_closed:
|
||||
raise HTTPException(
|
||||
status_code=400, detail="Cannot send messages to a closed conversation"
|
||||
)
|
||||
raise ConversationClosedException(conversation_id)
|
||||
|
||||
# Process attachments
|
||||
attachments = []
|
||||
@@ -480,7 +474,7 @@ async def send_message(
|
||||
)
|
||||
attachments.append(att_data)
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
raise MessageAttachmentException(str(e))
|
||||
|
||||
# Send message
|
||||
message = messaging_service.send_message(
|
||||
@@ -525,10 +519,10 @@ def close_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
if conversation.vendor_id and conversation.vendor_id != vendor_id:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
conversation = messaging_service.close_conversation(
|
||||
db=db,
|
||||
@@ -567,10 +561,10 @@ def reopen_conversation(
|
||||
)
|
||||
|
||||
if not conversation:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
if conversation.vendor_id and conversation.vendor_id != vendor_id:
|
||||
raise HTTPException(status_code=404, detail="Conversation not found")
|
||||
raise ConversationNotFoundException(str(conversation_id))
|
||||
|
||||
conversation = messaging_service.reopen_conversation(
|
||||
db=db,
|
||||
|
||||
Reference in New Issue
Block a user