feat: complete issue fixes and implement remaining features
## Critical Issues (CRIT-001~003) - All Fixed
- JWT secret key validation with pydantic field_validator
- Login audit logging for success/failure attempts
- Frontend API path prefix removal
## High Priority Issues (HIGH-001~008) - All Fixed
- Project soft delete using is_active flag
- Redis session token bytes handling
- Rate limiting with slowapi (5 req/min for login)
- Attachment API permission checks
- Kanban view with drag-and-drop
- Workload heatmap UI (WorkloadPage, WorkloadHeatmap)
- TaskDetailModal integrating Comments/Attachments
- UserSelect component for task assignment
## Medium Priority Issues (MED-001~012) - All Fixed
- MED-001~005: DB commits, N+1 queries, datetime, error format, blocker flag
- MED-006: Project health dashboard (HealthService, ProjectHealthPage)
- MED-007: Capacity update API (PUT /api/users/{id}/capacity)
- MED-008: Schedule triggers (cron parsing, deadline reminders)
- MED-009: Watermark feature (image/PDF watermarking)
- MED-010~012: useEffect deps, DOM operations, PDF export
## New Files
- backend/app/api/health/ - Project health API
- backend/app/services/health_service.py
- backend/app/services/trigger_scheduler.py
- backend/app/services/watermark_service.py
- backend/app/core/rate_limiter.py
- frontend/src/pages/ProjectHealthPage.tsx
- frontend/src/components/ProjectHealthCard.tsx
- frontend/src/components/KanbanBoard.tsx
- frontend/src/components/WorkloadHeatmap.tsx
## Tests
- 113 new tests passing (health: 32, users: 14, triggers: 35, watermark: 32)
## OpenSpec Archives
- add-project-health-dashboard
- add-capacity-update-api
- add-schedule-triggers
- add-watermark-feature
- add-rate-limiting
- enhance-frontend-ux
- add-resource-management-ui
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,38 +1,74 @@
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
from fastapi import APIRouter, Depends, HTTPException, UploadFile, File, Request
|
||||
from fastapi.responses import FileResponse
|
||||
from fastapi.responses import FileResponse, Response
|
||||
from sqlalchemy.orm import Session
|
||||
from typing import Optional
|
||||
|
||||
from app.core.database import get_db
|
||||
from app.middleware.auth import get_current_user
|
||||
from app.models import User, Task, Attachment, AttachmentVersion, AuditAction
|
||||
from app.middleware.auth import get_current_user, check_task_access, check_task_edit_access
|
||||
from app.models import User, Task, Project, Attachment, AttachmentVersion, AuditAction
|
||||
from app.schemas.attachment import (
|
||||
AttachmentResponse, AttachmentListResponse, AttachmentDetailResponse,
|
||||
AttachmentVersionResponse, VersionHistoryResponse
|
||||
)
|
||||
from app.services.file_storage_service import file_storage_service
|
||||
from app.services.audit_service import AuditService
|
||||
from app.services.watermark_service import watermark_service
|
||||
|
||||
router = APIRouter(prefix="/api", tags=["attachments"])
|
||||
|
||||
|
||||
def get_task_or_404(db: Session, task_id: str) -> Task:
|
||||
"""Get task or raise 404."""
|
||||
def get_task_with_access_check(db: Session, task_id: str, current_user: User, require_edit: bool = False) -> Task:
|
||||
"""Get task and verify access permissions."""
|
||||
task = db.query(Task).filter(Task.id == task_id).first()
|
||||
if not task:
|
||||
raise HTTPException(status_code=404, detail="Task not found")
|
||||
|
||||
# Get project for access check
|
||||
project = db.query(Project).filter(Project.id == task.project_id).first()
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
|
||||
# Check access permission
|
||||
if not check_task_access(current_user, task, project):
|
||||
raise HTTPException(status_code=403, detail="Access denied to this task")
|
||||
|
||||
# Check edit permission if required
|
||||
if require_edit and not check_task_edit_access(current_user, task, project):
|
||||
raise HTTPException(status_code=403, detail="Edit access denied to this task")
|
||||
|
||||
return task
|
||||
|
||||
|
||||
def get_attachment_or_404(db: Session, attachment_id: str) -> Attachment:
|
||||
"""Get attachment or raise 404."""
|
||||
def get_attachment_with_access_check(
|
||||
db: Session, attachment_id: str, current_user: User, require_edit: bool = False
|
||||
) -> Attachment:
|
||||
"""Get attachment and verify access permissions."""
|
||||
attachment = db.query(Attachment).filter(
|
||||
Attachment.id == attachment_id,
|
||||
Attachment.is_deleted == False
|
||||
).first()
|
||||
if not attachment:
|
||||
raise HTTPException(status_code=404, detail="Attachment not found")
|
||||
|
||||
# Get task and project for access check
|
||||
task = db.query(Task).filter(Task.id == attachment.task_id).first()
|
||||
if not task:
|
||||
raise HTTPException(status_code=404, detail="Task not found")
|
||||
|
||||
project = db.query(Project).filter(Project.id == task.project_id).first()
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
|
||||
# Check access permission
|
||||
if not check_task_access(current_user, task, project):
|
||||
raise HTTPException(status_code=403, detail="Access denied to this attachment")
|
||||
|
||||
# Check edit permission if required
|
||||
if require_edit and not check_task_edit_access(current_user, task, project):
|
||||
raise HTTPException(status_code=403, detail="Edit access denied to this attachment")
|
||||
|
||||
return attachment
|
||||
|
||||
|
||||
@@ -76,7 +112,7 @@ async def upload_attachment(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Upload a file attachment to a task."""
|
||||
task = get_task_or_404(db, task_id)
|
||||
task = get_task_with_access_check(db, task_id, current_user, require_edit=True)
|
||||
|
||||
# Check if attachment with same filename exists (for versioning in Phase 2)
|
||||
existing = db.query(Attachment).filter(
|
||||
@@ -115,9 +151,6 @@ async def upload_attachment(
|
||||
existing.file_size = file_size
|
||||
existing.updated_at = version.created_at
|
||||
|
||||
db.commit()
|
||||
db.refresh(existing)
|
||||
|
||||
# Audit log
|
||||
AuditService.log_event(
|
||||
db=db,
|
||||
@@ -129,7 +162,9 @@ async def upload_attachment(
|
||||
changes=[{"field": "version", "old_value": new_version - 1, "new_value": new_version}],
|
||||
request_metadata=getattr(request.state, "audit_metadata", None)
|
||||
)
|
||||
|
||||
db.commit()
|
||||
db.refresh(existing)
|
||||
|
||||
return attachment_to_response(existing)
|
||||
|
||||
@@ -175,9 +210,6 @@ async def upload_attachment(
|
||||
)
|
||||
db.add(version)
|
||||
|
||||
db.commit()
|
||||
db.refresh(attachment)
|
||||
|
||||
# Audit log
|
||||
AuditService.log_event(
|
||||
db=db,
|
||||
@@ -189,7 +221,9 @@ async def upload_attachment(
|
||||
changes=[{"field": "filename", "old_value": None, "new_value": attachment.filename}],
|
||||
request_metadata=getattr(request.state, "audit_metadata", None)
|
||||
)
|
||||
|
||||
db.commit()
|
||||
db.refresh(attachment)
|
||||
|
||||
return attachment_to_response(attachment)
|
||||
|
||||
@@ -201,7 +235,7 @@ async def list_task_attachments(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""List all attachments for a task."""
|
||||
task = get_task_or_404(db, task_id)
|
||||
task = get_task_with_access_check(db, task_id, current_user, require_edit=False)
|
||||
|
||||
attachments = db.query(Attachment).filter(
|
||||
Attachment.task_id == task_id,
|
||||
@@ -221,7 +255,7 @@ async def get_attachment(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Get attachment details with version history."""
|
||||
attachment = get_attachment_or_404(db, attachment_id)
|
||||
attachment = get_attachment_with_access_check(db, attachment_id, current_user, require_edit=False)
|
||||
|
||||
versions = db.query(AttachmentVersion).filter(
|
||||
AttachmentVersion.attachment_id == attachment_id
|
||||
@@ -252,8 +286,8 @@ async def download_attachment(
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Download an attachment file."""
|
||||
attachment = get_attachment_or_404(db, attachment_id)
|
||||
"""Download an attachment file with dynamic watermark."""
|
||||
attachment = get_attachment_with_access_check(db, attachment_id, current_user, require_edit=False)
|
||||
|
||||
# Get version to download
|
||||
target_version = version or attachment.current_version
|
||||
@@ -272,6 +306,7 @@ async def download_attachment(
|
||||
raise HTTPException(status_code=404, detail="File not found on disk")
|
||||
|
||||
# Audit log
|
||||
download_time = datetime.now()
|
||||
AuditService.log_event(
|
||||
db=db,
|
||||
event_type="attachment.download",
|
||||
@@ -284,6 +319,63 @@ async def download_attachment(
|
||||
)
|
||||
db.commit()
|
||||
|
||||
# Check if watermark should be applied
|
||||
mime_type = attachment.mime_type or ""
|
||||
if watermark_service.supports_watermark(mime_type):
|
||||
try:
|
||||
# Read the original file
|
||||
with open(file_path, "rb") as f:
|
||||
file_bytes = f.read()
|
||||
|
||||
# Apply watermark based on file type
|
||||
if watermark_service.is_supported_image(mime_type):
|
||||
watermarked_bytes, output_format = watermark_service.add_image_watermark(
|
||||
image_bytes=file_bytes,
|
||||
user_name=current_user.name,
|
||||
employee_id=current_user.employee_id,
|
||||
download_time=download_time
|
||||
)
|
||||
# Update mime type based on output format
|
||||
output_mime_type = f"image/{output_format}"
|
||||
# Update filename extension if format changed
|
||||
original_filename = attachment.original_filename
|
||||
if output_format == "png" and not original_filename.lower().endswith(".png"):
|
||||
original_filename = original_filename.rsplit(".", 1)[0] + ".png"
|
||||
|
||||
return Response(
|
||||
content=watermarked_bytes,
|
||||
media_type=output_mime_type,
|
||||
headers={
|
||||
"Content-Disposition": f'attachment; filename="{original_filename}"'
|
||||
}
|
||||
)
|
||||
|
||||
elif watermark_service.is_supported_pdf(mime_type):
|
||||
watermarked_bytes = watermark_service.add_pdf_watermark(
|
||||
pdf_bytes=file_bytes,
|
||||
user_name=current_user.name,
|
||||
employee_id=current_user.employee_id,
|
||||
download_time=download_time
|
||||
)
|
||||
|
||||
return Response(
|
||||
content=watermarked_bytes,
|
||||
media_type="application/pdf",
|
||||
headers={
|
||||
"Content-Disposition": f'attachment; filename="{attachment.original_filename}"'
|
||||
}
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
# If watermarking fails, log the error but still return the original file
|
||||
# This ensures users can still download files even if watermarking has issues
|
||||
import logging
|
||||
logging.getLogger(__name__).warning(
|
||||
f"Watermarking failed for attachment {attachment_id}: {str(e)}. "
|
||||
"Returning original file."
|
||||
)
|
||||
|
||||
# Return original file without watermark for unsupported types or on error
|
||||
return FileResponse(
|
||||
path=str(file_path),
|
||||
filename=attachment.original_filename,
|
||||
@@ -299,11 +391,10 @@ async def delete_attachment(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Soft delete an attachment."""
|
||||
attachment = get_attachment_or_404(db, attachment_id)
|
||||
attachment = get_attachment_with_access_check(db, attachment_id, current_user, require_edit=True)
|
||||
|
||||
# Soft delete
|
||||
attachment.is_deleted = True
|
||||
db.commit()
|
||||
|
||||
# Audit log
|
||||
AuditService.log_event(
|
||||
@@ -316,9 +407,10 @@ async def delete_attachment(
|
||||
changes=[{"field": "is_deleted", "old_value": False, "new_value": True}],
|
||||
request_metadata=getattr(request.state, "audit_metadata", None)
|
||||
)
|
||||
|
||||
db.commit()
|
||||
|
||||
return {"message": "Attachment deleted", "id": attachment_id}
|
||||
return {"detail": "Attachment deleted", "id": attachment_id}
|
||||
|
||||
|
||||
@router.get("/attachments/{attachment_id}/versions", response_model=VersionHistoryResponse)
|
||||
@@ -328,7 +420,7 @@ async def get_version_history(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Get version history for an attachment."""
|
||||
attachment = get_attachment_or_404(db, attachment_id)
|
||||
attachment = get_attachment_with_access_check(db, attachment_id, current_user, require_edit=False)
|
||||
|
||||
versions = db.query(AttachmentVersion).filter(
|
||||
AttachmentVersion.attachment_id == attachment_id
|
||||
@@ -351,7 +443,7 @@ async def restore_version(
|
||||
current_user: User = Depends(get_current_user)
|
||||
):
|
||||
"""Restore an attachment to a specific version."""
|
||||
attachment = get_attachment_or_404(db, attachment_id)
|
||||
attachment = get_attachment_with_access_check(db, attachment_id, current_user, require_edit=True)
|
||||
|
||||
version_record = db.query(AttachmentVersion).filter(
|
||||
AttachmentVersion.attachment_id == attachment_id,
|
||||
@@ -364,7 +456,6 @@ async def restore_version(
|
||||
old_version = attachment.current_version
|
||||
attachment.current_version = version
|
||||
attachment.file_size = version_record.file_size
|
||||
db.commit()
|
||||
|
||||
# Audit log
|
||||
AuditService.log_event(
|
||||
@@ -377,6 +468,7 @@ async def restore_version(
|
||||
changes=[{"field": "current_version", "old_value": old_version, "new_value": version}],
|
||||
request_metadata=getattr(request.state, "audit_metadata", None)
|
||||
)
|
||||
|
||||
db.commit()
|
||||
|
||||
return {"message": f"Restored to version {version}", "current_version": version}
|
||||
return {"detail": f"Restored to version {version}", "current_version": version}
|
||||
|
||||
Reference in New Issue
Block a user