feat: complete LOW priority code quality improvements
Backend: - LOW-002: Add Query validation with max page size limits (100) - LOW-003: Replace magic strings with TaskStatus.is_done flag - LOW-004: Add 'creation' trigger type validation - Add action_executor.py with UpdateFieldAction and AutoAssignAction Frontend: - LOW-005: Replace TypeScript 'any' with 'unknown' + type guards - LOW-006: Add ConfirmModal component with A11Y support - LOW-007: Add ToastContext for user feedback notifications - LOW-009: Add Skeleton components (17 loading states replaced) - LOW-010: Setup Vitest with 21 tests for ConfirmModal and Skeleton Components updated: - App.tsx, ProtectedRoute.tsx, Spaces.tsx, Projects.tsx, Tasks.tsx - ProjectSettings.tsx, AuditPage.tsx, WorkloadPage.tsx, ProjectHealthPage.tsx - Comments.tsx, AttachmentList.tsx, TriggerList.tsx, TaskDetailModal.tsx - NotificationBell.tsx, BlockerDialog.tsx, CalendarView.tsx, WorkloadUserDetail.tsx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -118,6 +118,9 @@ def should_encrypt_file(project: Project, db: Session) -> tuple[bool, Optional[E
|
||||
|
||||
Returns:
|
||||
Tuple of (should_encrypt, encryption_key)
|
||||
|
||||
Raises:
|
||||
HTTPException: If project is confidential but encryption is not available
|
||||
"""
|
||||
# Only encrypt for confidential projects
|
||||
if project.security_level != "confidential":
|
||||
@@ -125,11 +128,14 @@ def should_encrypt_file(project: Project, db: Session) -> tuple[bool, Optional[E
|
||||
|
||||
# Check if encryption is available
|
||||
if not encryption_service.is_encryption_available():
|
||||
logger.warning(
|
||||
logger.error(
|
||||
f"Project {project.id} is confidential but encryption is not configured. "
|
||||
"Files will be stored unencrypted."
|
||||
"Rejecting file upload to maintain security."
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Confidential project requires encryption. Please configure ENCRYPTION_MASTER_KEY environment variable."
|
||||
)
|
||||
return False, None
|
||||
|
||||
# Get active encryption key
|
||||
active_key = db.query(EncryptionKey).filter(
|
||||
@@ -137,11 +143,14 @@ def should_encrypt_file(project: Project, db: Session) -> tuple[bool, Optional[E
|
||||
).first()
|
||||
|
||||
if not active_key:
|
||||
logger.warning(
|
||||
logger.error(
|
||||
f"Project {project.id} is confidential but no active encryption key exists. "
|
||||
"Files will be stored unencrypted. Create a key using /api/admin/encryption-keys/rotate"
|
||||
"Rejecting file upload to maintain security."
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Confidential project requires encryption. Please create an active encryption key first."
|
||||
)
|
||||
return False, None
|
||||
|
||||
return True, active_key
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, status, Query
|
||||
from sqlalchemy.orm import Session
|
||||
from typing import List
|
||||
|
||||
@@ -13,8 +13,8 @@ router = APIRouter()
|
||||
|
||||
@router.get("", response_model=List[DepartmentResponse])
|
||||
async def list_departments(
|
||||
skip: int = 0,
|
||||
limit: int = 100,
|
||||
skip: int = Query(0, ge=0, description="Number of departments to skip"),
|
||||
limit: int = Query(100, ge=1, le=200, description="Max departments to return"),
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(require_permission("users.read")),
|
||||
):
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, status, Query
|
||||
from sqlalchemy.orm import Session
|
||||
from typing import Optional
|
||||
|
||||
@@ -71,8 +71,8 @@ async def generate_weekly_report(
|
||||
|
||||
@router.get("/api/reports/history", response_model=ReportHistoryListResponse)
|
||||
async def list_report_history(
|
||||
limit: int = 10,
|
||||
offset: int = 0,
|
||||
limit: int = Query(10, ge=1, le=100, description="Number of reports to return"),
|
||||
offset: int = Query(0, ge=0, description="Number of reports to skip"),
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(get_current_user),
|
||||
):
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import uuid
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, status, Query
|
||||
from sqlalchemy.orm import Session
|
||||
from typing import Optional
|
||||
|
||||
@@ -11,6 +11,8 @@ from app.schemas.trigger import (
|
||||
)
|
||||
from app.middleware.auth import get_current_user, check_project_access, check_project_edit_access
|
||||
from app.services.trigger_scheduler import TriggerSchedulerService
|
||||
from app.services.trigger_service import TriggerService
|
||||
from app.services.action_executor import ActionValidationError
|
||||
|
||||
router = APIRouter(tags=["triggers"])
|
||||
|
||||
@@ -60,10 +62,11 @@ async def create_trigger(
|
||||
)
|
||||
|
||||
# Validate trigger type
|
||||
if trigger_data.trigger_type not in ["field_change", "schedule"]:
|
||||
valid_trigger_types = ["field_change", "schedule", "creation"]
|
||||
if trigger_data.trigger_type not in valid_trigger_types:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Invalid trigger type. Must be 'field_change' or 'schedule'",
|
||||
detail=f"Invalid trigger type. Must be one of: {', '.join(valid_trigger_types)}",
|
||||
)
|
||||
|
||||
# Validate conditions based on trigger type
|
||||
@@ -111,6 +114,16 @@ async def create_trigger(
|
||||
detail=error_msg or "Invalid cron expression",
|
||||
)
|
||||
|
||||
# Validate actions configuration (FEAT-014, FEAT-015)
|
||||
try:
|
||||
actions_dicts = [a.model_dump(exclude_none=True) for a in trigger_data.actions]
|
||||
TriggerService.validate_actions(actions_dicts, db)
|
||||
except ActionValidationError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=str(e),
|
||||
)
|
||||
|
||||
# Create trigger
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
@@ -119,7 +132,7 @@ async def create_trigger(
|
||||
description=trigger_data.description,
|
||||
trigger_type=trigger_data.trigger_type,
|
||||
conditions=trigger_data.conditions.model_dump(),
|
||||
actions=[a.model_dump() for a in trigger_data.actions],
|
||||
actions=[a.model_dump(exclude_none=True) for a in trigger_data.actions],
|
||||
is_active=trigger_data.is_active,
|
||||
created_by=current_user.id,
|
||||
)
|
||||
@@ -239,7 +252,16 @@ async def update_trigger(
|
||||
)
|
||||
trigger.conditions = trigger_data.conditions.model_dump(exclude_none=True)
|
||||
if trigger_data.actions is not None:
|
||||
trigger.actions = [a.model_dump() for a in trigger_data.actions]
|
||||
# Validate actions configuration (FEAT-014, FEAT-015)
|
||||
try:
|
||||
actions_dicts = [a.model_dump(exclude_none=True) for a in trigger_data.actions]
|
||||
TriggerService.validate_actions(actions_dicts, db)
|
||||
except ActionValidationError as e:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=str(e),
|
||||
)
|
||||
trigger.actions = [a.model_dump(exclude_none=True) for a in trigger_data.actions]
|
||||
if trigger_data.is_active is not None:
|
||||
trigger.is_active = trigger_data.is_active
|
||||
|
||||
@@ -278,8 +300,8 @@ async def delete_trigger(
|
||||
@router.get("/api/triggers/{trigger_id}/logs", response_model=TriggerLogListResponse)
|
||||
async def list_trigger_logs(
|
||||
trigger_id: str,
|
||||
limit: int = 50,
|
||||
offset: int = 0,
|
||||
limit: int = Query(50, ge=1, le=200, description="Number of logs to return"),
|
||||
offset: int = Query(0, ge=0, description="Number of logs to skip"),
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(get_current_user),
|
||||
):
|
||||
|
||||
@@ -50,8 +50,8 @@ async def search_users(
|
||||
|
||||
@router.get("", response_model=List[UserResponse])
|
||||
async def list_users(
|
||||
skip: int = 0,
|
||||
limit: int = 100,
|
||||
skip: int = Query(0, ge=0, description="Number of users to skip"),
|
||||
limit: int = Query(100, ge=1, le=500, description="Max users to return"),
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(require_permission("users.read")),
|
||||
):
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
from pydantic import BaseModel
|
||||
from pydantic import BaseModel, computed_field
|
||||
from typing import Optional, List, Any, Dict
|
||||
from datetime import datetime
|
||||
from decimal import Decimal
|
||||
@@ -79,6 +79,12 @@ class TaskResponse(TaskBase):
|
||||
class Config:
|
||||
from_attributes = True
|
||||
|
||||
# Alias for original_estimate for frontend compatibility
|
||||
@computed_field
|
||||
@property
|
||||
def time_estimate(self) -> Optional[Decimal]:
|
||||
return self.original_estimate
|
||||
|
||||
|
||||
class TaskWithDetails(TaskResponse):
|
||||
assignee_name: Optional[str] = None
|
||||
|
||||
@@ -28,9 +28,23 @@ class TriggerCondition(BaseModel):
|
||||
|
||||
|
||||
class TriggerAction(BaseModel):
|
||||
type: str = Field(default="notify", description="Action type: notify")
|
||||
target: str = Field(default="assignee", description="Target: assignee, creator, project_owner, user:<id>")
|
||||
"""Action configuration for triggers.
|
||||
|
||||
Supported action types:
|
||||
- notify: Send notification (requires target, optional template)
|
||||
- update_field: Update task field (requires field, value)
|
||||
- auto_assign: Auto-assign task (requires strategy, optional user_id for specific_user)
|
||||
"""
|
||||
type: str = Field(..., description="Action type: notify, update_field, auto_assign")
|
||||
# Notify action fields
|
||||
target: Optional[str] = Field(None, description="Target: assignee, creator, project_owner, user:<id>")
|
||||
template: Optional[str] = Field(None, description="Message template with variables")
|
||||
# update_field action fields (FEAT-014)
|
||||
field: Optional[str] = Field(None, description="Field to update: priority, status_id, due_date")
|
||||
value: Optional[Any] = Field(None, description="New value for the field")
|
||||
# auto_assign action fields (FEAT-015)
|
||||
strategy: Optional[str] = Field(None, description="Strategy: round_robin, least_loaded, specific_user")
|
||||
user_id: Optional[str] = Field(None, description="User ID for specific_user strategy")
|
||||
|
||||
|
||||
class TriggerCreate(BaseModel):
|
||||
|
||||
484
backend/app/services/action_executor.py
Normal file
484
backend/app/services/action_executor.py
Normal file
@@ -0,0 +1,484 @@
|
||||
"""Action executor service for automation triggers.
|
||||
|
||||
This module provides the action execution framework for the automation system.
|
||||
It supports extensible action types through a registry pattern.
|
||||
|
||||
FEAT-014: update_field - Update task field values
|
||||
FEAT-015: auto_assign - Automatic task assignment with multiple strategies
|
||||
"""
|
||||
import logging
|
||||
import uuid
|
||||
from abc import ABC, abstractmethod
|
||||
from datetime import datetime, date
|
||||
from decimal import Decimal
|
||||
from typing import Any, Dict, List, Optional, Type
|
||||
|
||||
from sqlalchemy import func
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.models import Task, User, TaskStatus
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class ActionExecutionError(Exception):
|
||||
"""Exception raised when action execution fails."""
|
||||
pass
|
||||
|
||||
|
||||
class ActionValidationError(Exception):
|
||||
"""Exception raised when action config validation fails."""
|
||||
pass
|
||||
|
||||
|
||||
class BaseAction(ABC):
|
||||
"""Base class for all action types."""
|
||||
|
||||
action_type: str = ""
|
||||
|
||||
@abstractmethod
|
||||
def validate_config(self, config: Dict[str, Any], db: Session) -> None:
|
||||
"""Validate action configuration.
|
||||
|
||||
Args:
|
||||
config: Action configuration dict
|
||||
db: Database session for validation queries
|
||||
|
||||
Raises:
|
||||
ActionValidationError: If config is invalid
|
||||
"""
|
||||
pass
|
||||
|
||||
@abstractmethod
|
||||
def execute(
|
||||
self,
|
||||
db: Session,
|
||||
task: Task,
|
||||
config: Dict[str, Any],
|
||||
context: Dict[str, Any],
|
||||
) -> Dict[str, Any]:
|
||||
"""Execute the action.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
task: Target task
|
||||
config: Action configuration
|
||||
context: Execution context (old_values, new_values, current_user, etc.)
|
||||
|
||||
Returns:
|
||||
Dict with execution result details
|
||||
|
||||
Raises:
|
||||
ActionExecutionError: If execution fails
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
class UpdateFieldAction(BaseAction):
|
||||
"""Action to update task field values (FEAT-014).
|
||||
|
||||
Supported fields:
|
||||
- priority: low, medium, high, urgent
|
||||
- status_id: Valid status ID for the task's project
|
||||
- due_date: ISO format date string
|
||||
|
||||
Config format:
|
||||
{
|
||||
"field": "priority",
|
||||
"value": "high"
|
||||
}
|
||||
"""
|
||||
|
||||
action_type = "update_field"
|
||||
|
||||
# Standard fields that can be updated
|
||||
UPDATABLE_FIELDS = {
|
||||
"priority": ["low", "medium", "high", "urgent"],
|
||||
"status_id": None, # Validated dynamically
|
||||
"due_date": None, # Date string validation
|
||||
}
|
||||
|
||||
def validate_config(self, config: Dict[str, Any], db: Session) -> None:
|
||||
"""Validate update_field configuration."""
|
||||
field = config.get("field")
|
||||
value = config.get("value")
|
||||
|
||||
if not field:
|
||||
raise ActionValidationError("Missing required 'field' in update_field config")
|
||||
|
||||
if value is None:
|
||||
raise ActionValidationError("Missing required 'value' in update_field config")
|
||||
|
||||
if field not in self.UPDATABLE_FIELDS:
|
||||
raise ActionValidationError(
|
||||
f"Invalid field '{field}'. Supported fields: {list(self.UPDATABLE_FIELDS.keys())}"
|
||||
)
|
||||
|
||||
# Validate priority values
|
||||
if field == "priority":
|
||||
valid_values = self.UPDATABLE_FIELDS["priority"]
|
||||
if value not in valid_values:
|
||||
raise ActionValidationError(
|
||||
f"Invalid priority value '{value}'. Valid values: {valid_values}"
|
||||
)
|
||||
|
||||
# Validate due_date format
|
||||
if field == "due_date" and value:
|
||||
try:
|
||||
if isinstance(value, str):
|
||||
datetime.fromisoformat(value.replace("Z", "+00:00"))
|
||||
except ValueError:
|
||||
raise ActionValidationError(
|
||||
f"Invalid due_date format '{value}'. Use ISO format (YYYY-MM-DD or YYYY-MM-DDTHH:MM:SS)"
|
||||
)
|
||||
|
||||
def execute(
|
||||
self,
|
||||
db: Session,
|
||||
task: Task,
|
||||
config: Dict[str, Any],
|
||||
context: Dict[str, Any],
|
||||
) -> Dict[str, Any]:
|
||||
"""Execute field update action."""
|
||||
field = config["field"]
|
||||
value = config["value"]
|
||||
old_value = getattr(task, field, None)
|
||||
|
||||
logger.info(
|
||||
f"Executing update_field action: task={task.id}, field={field}, "
|
||||
f"old_value={old_value}, new_value={value}"
|
||||
)
|
||||
|
||||
# Validate status_id exists for this project
|
||||
if field == "status_id" and value:
|
||||
status = db.query(TaskStatus).filter(
|
||||
TaskStatus.id == value,
|
||||
TaskStatus.project_id == task.project_id,
|
||||
).first()
|
||||
if not status:
|
||||
raise ActionExecutionError(
|
||||
f"Status ID '{value}' not found in project {task.project_id}"
|
||||
)
|
||||
|
||||
# Convert due_date string to datetime
|
||||
if field == "due_date" and value:
|
||||
if isinstance(value, str):
|
||||
value = datetime.fromisoformat(value.replace("Z", "+00:00"))
|
||||
|
||||
# Update the field
|
||||
setattr(task, field, value)
|
||||
task.updated_at = datetime.utcnow()
|
||||
|
||||
return {
|
||||
"action_type": self.action_type,
|
||||
"status": "success",
|
||||
"field": field,
|
||||
"old_value": str(old_value) if old_value else None,
|
||||
"new_value": str(value) if value else None,
|
||||
}
|
||||
|
||||
|
||||
class AutoAssignAction(BaseAction):
|
||||
"""Action for automatic task assignment (FEAT-015).
|
||||
|
||||
Strategies:
|
||||
- round_robin: Assign to project members in rotation
|
||||
- least_loaded: Assign to member with lowest workload
|
||||
- specific_user: Assign to a specific user
|
||||
|
||||
Config format:
|
||||
{
|
||||
"strategy": "round_robin" | "least_loaded" | "specific_user",
|
||||
"user_id": "xxx" // Required only for specific_user strategy
|
||||
}
|
||||
"""
|
||||
|
||||
action_type = "auto_assign"
|
||||
|
||||
VALID_STRATEGIES = ["round_robin", "least_loaded", "specific_user"]
|
||||
|
||||
# Class-level state for round-robin tracking per project
|
||||
_round_robin_index: Dict[str, int] = {}
|
||||
|
||||
def validate_config(self, config: Dict[str, Any], db: Session) -> None:
|
||||
"""Validate auto_assign configuration."""
|
||||
strategy = config.get("strategy")
|
||||
|
||||
if not strategy:
|
||||
raise ActionValidationError("Missing required 'strategy' in auto_assign config")
|
||||
|
||||
if strategy not in self.VALID_STRATEGIES:
|
||||
raise ActionValidationError(
|
||||
f"Invalid strategy '{strategy}'. Valid strategies: {self.VALID_STRATEGIES}"
|
||||
)
|
||||
|
||||
if strategy == "specific_user":
|
||||
user_id = config.get("user_id")
|
||||
if not user_id:
|
||||
raise ActionValidationError(
|
||||
"Missing required 'user_id' for specific_user strategy"
|
||||
)
|
||||
# Validate user exists
|
||||
user = db.query(User).filter(
|
||||
User.id == user_id,
|
||||
User.is_active == True,
|
||||
).first()
|
||||
if not user:
|
||||
raise ActionValidationError(f"User '{user_id}' not found or inactive")
|
||||
|
||||
def execute(
|
||||
self,
|
||||
db: Session,
|
||||
task: Task,
|
||||
config: Dict[str, Any],
|
||||
context: Dict[str, Any],
|
||||
) -> Dict[str, Any]:
|
||||
"""Execute auto-assign action."""
|
||||
strategy = config["strategy"]
|
||||
old_assignee_id = task.assignee_id
|
||||
|
||||
logger.info(
|
||||
f"Executing auto_assign action: task={task.id}, strategy={strategy}, "
|
||||
f"old_assignee={old_assignee_id}"
|
||||
)
|
||||
|
||||
if strategy == "specific_user":
|
||||
new_assignee_id = self._assign_specific_user(db, config)
|
||||
elif strategy == "round_robin":
|
||||
new_assignee_id = self._assign_round_robin(db, task)
|
||||
elif strategy == "least_loaded":
|
||||
new_assignee_id = self._assign_least_loaded(db, task)
|
||||
else:
|
||||
raise ActionExecutionError(f"Unknown strategy: {strategy}")
|
||||
|
||||
if new_assignee_id:
|
||||
task.assignee_id = new_assignee_id
|
||||
task.updated_at = datetime.utcnow()
|
||||
|
||||
# Get assignee name for logging
|
||||
assignee = db.query(User).filter(User.id == new_assignee_id).first()
|
||||
assignee_name = assignee.name if assignee else "Unknown"
|
||||
|
||||
logger.info(
|
||||
f"Task {task.id} assigned to user {new_assignee_id} ({assignee_name}) "
|
||||
f"using {strategy} strategy"
|
||||
)
|
||||
|
||||
return {
|
||||
"action_type": self.action_type,
|
||||
"status": "success",
|
||||
"strategy": strategy,
|
||||
"old_assignee_id": old_assignee_id,
|
||||
"new_assignee_id": new_assignee_id,
|
||||
"assignee_name": assignee_name,
|
||||
}
|
||||
else:
|
||||
logger.warning(
|
||||
f"No eligible assignee found for task {task.id} using {strategy} strategy"
|
||||
)
|
||||
return {
|
||||
"action_type": self.action_type,
|
||||
"status": "skipped",
|
||||
"strategy": strategy,
|
||||
"reason": "No eligible assignee found",
|
||||
}
|
||||
|
||||
def _assign_specific_user(self, db: Session, config: Dict[str, Any]) -> Optional[str]:
|
||||
"""Assign to a specific user."""
|
||||
user_id = config["user_id"]
|
||||
user = db.query(User).filter(
|
||||
User.id == user_id,
|
||||
User.is_active == True,
|
||||
).first()
|
||||
return user.id if user else None
|
||||
|
||||
def _assign_round_robin(self, db: Session, task: Task) -> Optional[str]:
|
||||
"""Assign using round-robin strategy among project members.
|
||||
|
||||
Gets all active users in the same department as the project owner,
|
||||
then rotates through them.
|
||||
"""
|
||||
project = task.project
|
||||
if not project:
|
||||
return None
|
||||
|
||||
# Get project members: owner + users in same department
|
||||
members = self._get_project_members(db, project)
|
||||
|
||||
if not members:
|
||||
return None
|
||||
|
||||
# Get or initialize round-robin index for this project
|
||||
project_id = project.id
|
||||
if project_id not in self._round_robin_index:
|
||||
self._round_robin_index[project_id] = 0
|
||||
|
||||
# Get next member in rotation
|
||||
index = self._round_robin_index[project_id] % len(members)
|
||||
selected_user = members[index]
|
||||
|
||||
# Update index for next assignment
|
||||
self._round_robin_index[project_id] = (index + 1) % len(members)
|
||||
|
||||
return selected_user.id
|
||||
|
||||
def _assign_least_loaded(self, db: Session, task: Task) -> Optional[str]:
|
||||
"""Assign to the project member with lowest workload.
|
||||
|
||||
Workload is calculated based on number of incomplete tasks assigned.
|
||||
"""
|
||||
project = task.project
|
||||
if not project:
|
||||
return None
|
||||
|
||||
members = self._get_project_members(db, project)
|
||||
|
||||
if not members:
|
||||
return None
|
||||
|
||||
# Calculate workload for each member (count of incomplete tasks)
|
||||
member_workloads = []
|
||||
for member in members:
|
||||
# Count incomplete tasks (status.is_done = False or no status)
|
||||
incomplete_count = (
|
||||
db.query(func.count(Task.id))
|
||||
.outerjoin(TaskStatus, Task.status_id == TaskStatus.id)
|
||||
.filter(
|
||||
Task.assignee_id == member.id,
|
||||
Task.is_deleted == False,
|
||||
(TaskStatus.is_done == False) | (Task.status_id == None),
|
||||
)
|
||||
.scalar()
|
||||
)
|
||||
member_workloads.append((member, incomplete_count))
|
||||
|
||||
# Sort by workload (ascending) and return the least loaded member
|
||||
member_workloads.sort(key=lambda x: x[1])
|
||||
|
||||
if member_workloads:
|
||||
selected_user, workload = member_workloads[0]
|
||||
logger.debug(
|
||||
f"Least loaded assignment: user={selected_user.id}, "
|
||||
f"current_workload={workload}"
|
||||
)
|
||||
return selected_user.id
|
||||
|
||||
return None
|
||||
|
||||
def _get_project_members(self, db: Session, project) -> List[User]:
|
||||
"""Get all potential assignees for a project.
|
||||
|
||||
Returns active users who are either:
|
||||
- The project owner
|
||||
- In the same department as the project
|
||||
"""
|
||||
owner_id = project.owner_id
|
||||
department_id = project.department_id
|
||||
|
||||
query = db.query(User).filter(User.is_active == True)
|
||||
|
||||
if department_id:
|
||||
# Get users in same department OR project owner
|
||||
query = query.filter(
|
||||
(User.department_id == department_id) | (User.id == owner_id)
|
||||
)
|
||||
else:
|
||||
# No department, just return owner
|
||||
query = query.filter(User.id == owner_id)
|
||||
|
||||
return query.all()
|
||||
|
||||
|
||||
class ActionExecutor:
|
||||
"""Registry and executor for action types.
|
||||
|
||||
Usage:
|
||||
executor = ActionExecutor()
|
||||
result = executor.execute_action(db, task, action_config, context)
|
||||
"""
|
||||
|
||||
_actions: Dict[str, Type[BaseAction]] = {}
|
||||
|
||||
@classmethod
|
||||
def register(cls, action_class: Type[BaseAction]) -> None:
|
||||
"""Register an action type."""
|
||||
cls._actions[action_class.action_type] = action_class
|
||||
logger.debug(f"Registered action type: {action_class.action_type}")
|
||||
|
||||
@classmethod
|
||||
def get_supported_actions(cls) -> List[str]:
|
||||
"""Get list of supported action types."""
|
||||
return list(cls._actions.keys())
|
||||
|
||||
@classmethod
|
||||
def validate_action(cls, action: Dict[str, Any], db: Session) -> None:
|
||||
"""Validate an action configuration.
|
||||
|
||||
Args:
|
||||
action: Action dict with 'type' and other config
|
||||
db: Database session
|
||||
|
||||
Raises:
|
||||
ActionValidationError: If action is invalid
|
||||
"""
|
||||
action_type = action.get("type")
|
||||
|
||||
if not action_type:
|
||||
raise ActionValidationError("Missing action 'type'")
|
||||
|
||||
if action_type not in cls._actions:
|
||||
# Allow unknown actions (like 'notify') to pass through
|
||||
return
|
||||
|
||||
action_class = cls._actions[action_type]
|
||||
action_instance = action_class()
|
||||
config = action.get("config", action) # Support both nested and flat config
|
||||
action_instance.validate_config(config, db)
|
||||
|
||||
@classmethod
|
||||
def execute_action(
|
||||
cls,
|
||||
db: Session,
|
||||
task: Task,
|
||||
action: Dict[str, Any],
|
||||
context: Dict[str, Any],
|
||||
) -> Optional[Dict[str, Any]]:
|
||||
"""Execute a single action.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
task: Target task
|
||||
action: Action dict with 'type' and other config
|
||||
context: Execution context
|
||||
|
||||
Returns:
|
||||
Execution result dict or None if action type not registered
|
||||
|
||||
Raises:
|
||||
ActionExecutionError: If execution fails
|
||||
"""
|
||||
action_type = action.get("type")
|
||||
|
||||
if action_type not in cls._actions:
|
||||
# Not a registered action (might be 'notify' handled elsewhere)
|
||||
return None
|
||||
|
||||
action_class = cls._actions[action_type]
|
||||
action_instance = action_class()
|
||||
|
||||
# Extract config - support both nested and flat config
|
||||
config = action.get("config", action)
|
||||
|
||||
try:
|
||||
result = action_instance.execute(db, task, config, context)
|
||||
return result
|
||||
except ActionExecutionError:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.exception(f"Unexpected error executing {action_type} action")
|
||||
raise ActionExecutionError(f"Failed to execute {action_type}: {str(e)}")
|
||||
|
||||
|
||||
# Register built-in actions
|
||||
ActionExecutor.register(UpdateFieldAction)
|
||||
ActionExecutor.register(AutoAssignAction)
|
||||
@@ -87,16 +87,17 @@ class ReportService:
|
||||
next_week_tasks = []
|
||||
|
||||
for task in all_tasks:
|
||||
status_name = task.status.name.lower() if task.status else ""
|
||||
is_done = status_name in ["done", "completed", "完成"]
|
||||
# Use TaskStatus.is_done flag instead of magic strings
|
||||
is_done = task.status.is_done if task.status else False
|
||||
|
||||
# Check if completed (updated this week)
|
||||
if is_done:
|
||||
if task.updated_at and task.updated_at >= week_start:
|
||||
completed_tasks.append(task)
|
||||
else:
|
||||
# Check if in progress
|
||||
if status_name in ["in progress", "進行中", "doing"]:
|
||||
# Check if task has active status (not done, not blocked)
|
||||
# Tasks without a done status are considered in progress
|
||||
if task.status and not task.status.is_done:
|
||||
in_progress_tasks.append(task)
|
||||
|
||||
# Check if overdue
|
||||
|
||||
@@ -1,9 +1,17 @@
|
||||
import uuid
|
||||
import logging
|
||||
from typing import List, Dict, Any, Optional
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.models import Trigger, TriggerLog, Task, User, Project
|
||||
from app.services.notification_service import NotificationService
|
||||
from app.services.action_executor import (
|
||||
ActionExecutor,
|
||||
ActionExecutionError,
|
||||
ActionValidationError,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class TriggerService:
|
||||
@@ -74,23 +82,78 @@ class TriggerService:
|
||||
old_values: Dict[str, Any],
|
||||
new_values: Dict[str, Any],
|
||||
) -> TriggerLog:
|
||||
"""Execute trigger actions and log the result."""
|
||||
"""Execute trigger actions and log the result.
|
||||
|
||||
Uses a database savepoint to ensure atomicity - if any action fails,
|
||||
all previously executed actions within this trigger are rolled back.
|
||||
"""
|
||||
actions = trigger.actions if isinstance(trigger.actions, list) else [trigger.actions]
|
||||
executed_actions = []
|
||||
error_message = None
|
||||
|
||||
# Build execution context
|
||||
context = {
|
||||
"old_values": old_values,
|
||||
"new_values": new_values,
|
||||
"current_user": current_user,
|
||||
"trigger": trigger,
|
||||
}
|
||||
|
||||
# Use savepoint for transaction atomicity - if any action fails,
|
||||
# all changes made by previous actions will be rolled back
|
||||
savepoint = db.begin_nested()
|
||||
try:
|
||||
for action in actions:
|
||||
action_type = action.get("type")
|
||||
|
||||
# Handle built-in notify action
|
||||
if action_type == "notify":
|
||||
TriggerService._execute_notify_action(db, action, task, current_user, old_values, new_values)
|
||||
executed_actions.append({"type": action_type, "status": "success"})
|
||||
|
||||
# Handle update_field action (FEAT-014)
|
||||
elif action_type == "update_field":
|
||||
result = ActionExecutor.execute_action(db, task, action, context)
|
||||
if result:
|
||||
executed_actions.append(result)
|
||||
logger.info(
|
||||
f"Trigger '{trigger.name}' executed update_field: "
|
||||
f"field={result.get('field')}, new_value={result.get('new_value')}"
|
||||
)
|
||||
|
||||
# Handle auto_assign action (FEAT-015)
|
||||
elif action_type == "auto_assign":
|
||||
result = ActionExecutor.execute_action(db, task, action, context)
|
||||
if result:
|
||||
executed_actions.append(result)
|
||||
logger.info(
|
||||
f"Trigger '{trigger.name}' executed auto_assign: "
|
||||
f"strategy={result.get('strategy')}, assignee={result.get('new_assignee_id')}"
|
||||
)
|
||||
|
||||
# Try to execute via ActionExecutor for extensibility
|
||||
else:
|
||||
result = ActionExecutor.execute_action(db, task, action, context)
|
||||
if result:
|
||||
executed_actions.append(result)
|
||||
|
||||
# All actions succeeded, commit the savepoint
|
||||
savepoint.commit()
|
||||
status = "success"
|
||||
except Exception as e:
|
||||
except ActionExecutionError as e:
|
||||
# Rollback all changes made by previously executed actions
|
||||
savepoint.rollback()
|
||||
status = "failed"
|
||||
error_message = str(e)
|
||||
executed_actions.append({"type": "error", "message": str(e)})
|
||||
logger.error(f"Trigger '{trigger.name}' action execution failed, rolling back: {e}")
|
||||
except Exception as e:
|
||||
# Rollback all changes made by previously executed actions
|
||||
savepoint.rollback()
|
||||
status = "failed"
|
||||
error_message = str(e)
|
||||
executed_actions.append({"type": "error", "message": str(e)})
|
||||
logger.exception(f"Trigger '{trigger.name}' unexpected error, rolling back: {e}")
|
||||
|
||||
log = TriggerLog(
|
||||
id=str(uuid.uuid4()),
|
||||
@@ -198,3 +261,36 @@ class TriggerService:
|
||||
)
|
||||
db.add(log)
|
||||
return log
|
||||
|
||||
@staticmethod
|
||||
def validate_actions(actions: List[Dict[str, Any]], db: Session) -> None:
|
||||
"""Validate trigger actions configuration.
|
||||
|
||||
Args:
|
||||
actions: List of action configurations
|
||||
db: Database session
|
||||
|
||||
Raises:
|
||||
ActionValidationError: If any action is invalid
|
||||
"""
|
||||
valid_action_types = ["notify", "update_field", "auto_assign"]
|
||||
|
||||
for action in actions:
|
||||
action_type = action.get("type")
|
||||
if not action_type:
|
||||
raise ActionValidationError("Missing action 'type'")
|
||||
|
||||
if action_type not in valid_action_types:
|
||||
raise ActionValidationError(
|
||||
f"Invalid action type '{action_type}'. "
|
||||
f"Valid types: {valid_action_types}"
|
||||
)
|
||||
|
||||
# Validate via ActionExecutor for extensible actions
|
||||
if action_type in ["update_field", "auto_assign"]:
|
||||
ActionExecutor.validate_action(action, db)
|
||||
|
||||
@staticmethod
|
||||
def get_supported_action_types() -> List[str]:
|
||||
"""Get list of all supported action types."""
|
||||
return ["notify"] + ActionExecutor.get_supported_actions()
|
||||
|
||||
700
backend/tests/test_action_executor.py
Normal file
700
backend/tests/test_action_executor.py
Normal file
@@ -0,0 +1,700 @@
|
||||
"""Tests for action_executor.py - FEAT-014 and FEAT-015.
|
||||
|
||||
This module tests the action executor framework and the two new action types:
|
||||
- update_field: Update task field values
|
||||
- auto_assign: Automatic task assignment with multiple strategies
|
||||
"""
|
||||
import pytest
|
||||
import uuid
|
||||
from datetime import datetime, timedelta
|
||||
from decimal import Decimal
|
||||
|
||||
from app.models import User, Space, Project, Task, TaskStatus, Trigger, TriggerLog, Department
|
||||
from app.services.action_executor import (
|
||||
ActionExecutor,
|
||||
UpdateFieldAction,
|
||||
AutoAssignAction,
|
||||
ActionExecutionError,
|
||||
ActionValidationError,
|
||||
)
|
||||
from app.services.trigger_service import TriggerService
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_department(db):
|
||||
"""Create a test department."""
|
||||
dept = Department(
|
||||
id=str(uuid.uuid4()),
|
||||
name="Engineering",
|
||||
)
|
||||
db.add(dept)
|
||||
db.commit()
|
||||
return dept
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_user(db, test_department):
|
||||
"""Create a test user."""
|
||||
user = User(
|
||||
id=str(uuid.uuid4()),
|
||||
email="testuser@example.com",
|
||||
name="Test User",
|
||||
department_id=test_department.id,
|
||||
role_id="00000000-0000-0000-0000-000000000003",
|
||||
is_active=True,
|
||||
is_system_admin=False,
|
||||
capacity=Decimal("40.00"),
|
||||
)
|
||||
db.add(user)
|
||||
db.commit()
|
||||
return user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def additional_users(db, test_department):
|
||||
"""Create additional test users for auto-assign tests."""
|
||||
users = []
|
||||
for i in range(3):
|
||||
user = User(
|
||||
id=str(uuid.uuid4()),
|
||||
email=f"user{i}@example.com",
|
||||
name=f"User {i}",
|
||||
department_id=test_department.id,
|
||||
role_id="00000000-0000-0000-0000-000000000003",
|
||||
is_active=True,
|
||||
is_system_admin=False,
|
||||
capacity=Decimal("40.00"),
|
||||
)
|
||||
db.add(user)
|
||||
users.append(user)
|
||||
db.commit()
|
||||
return users
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_space(db, test_user):
|
||||
"""Create a test space."""
|
||||
space = Space(
|
||||
id=str(uuid.uuid4()),
|
||||
name="Test Space",
|
||||
description="Test space for action executor",
|
||||
owner_id=test_user.id,
|
||||
)
|
||||
db.add(space)
|
||||
db.commit()
|
||||
return space
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_project(db, test_space, test_user, test_department):
|
||||
"""Create a test project."""
|
||||
project = Project(
|
||||
id=str(uuid.uuid4()),
|
||||
space_id=test_space.id,
|
||||
title="Test Project",
|
||||
description="Test project for action executor",
|
||||
owner_id=test_user.id,
|
||||
department_id=test_department.id,
|
||||
)
|
||||
db.add(project)
|
||||
db.commit()
|
||||
return project
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_statuses(db, test_project):
|
||||
"""Create test task statuses."""
|
||||
status_todo = TaskStatus(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="To Do",
|
||||
color="#808080",
|
||||
position=0,
|
||||
is_done=False,
|
||||
)
|
||||
status_in_progress = TaskStatus(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="In Progress",
|
||||
color="#0000FF",
|
||||
position=1,
|
||||
is_done=False,
|
||||
)
|
||||
status_done = TaskStatus(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Done",
|
||||
color="#00FF00",
|
||||
position=2,
|
||||
is_done=True,
|
||||
)
|
||||
db.add_all([status_todo, status_in_progress, status_done])
|
||||
db.commit()
|
||||
return status_todo, status_in_progress, status_done
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_task(db, test_project, test_user, test_statuses):
|
||||
"""Create a test task."""
|
||||
task = Task(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
title="Test Task",
|
||||
description="Test task for action executor",
|
||||
status_id=test_statuses[0].id,
|
||||
created_by=test_user.id,
|
||||
assignee_id=test_user.id,
|
||||
priority="medium",
|
||||
)
|
||||
db.add(task)
|
||||
db.commit()
|
||||
db.refresh(task)
|
||||
return task
|
||||
|
||||
|
||||
class TestUpdateFieldAction:
|
||||
"""Tests for FEAT-014: Update Field Action."""
|
||||
|
||||
def test_validate_config_valid_priority(self, db):
|
||||
"""Test validation of valid priority update config."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "priority", "value": "high"}
|
||||
# Should not raise
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_valid_status_id(self, db):
|
||||
"""Test validation of valid status_id update config."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "status_id", "value": "some-uuid"}
|
||||
# Should not raise (status_id validation is deferred to execution)
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_valid_due_date(self, db):
|
||||
"""Test validation of valid due_date update config."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "due_date", "value": "2025-12-31T23:59:59"}
|
||||
# Should not raise
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_missing_field(self, db):
|
||||
"""Test validation fails when field is missing."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"value": "high"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Missing required 'field'" in str(exc.value)
|
||||
|
||||
def test_validate_config_missing_value(self, db):
|
||||
"""Test validation fails when value is missing."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "priority"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Missing required 'value'" in str(exc.value)
|
||||
|
||||
def test_validate_config_invalid_field(self, db):
|
||||
"""Test validation fails for unsupported field."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "invalid_field", "value": "test"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Invalid field" in str(exc.value)
|
||||
|
||||
def test_validate_config_invalid_priority(self, db):
|
||||
"""Test validation fails for invalid priority value."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "priority", "value": "critical"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Invalid priority value" in str(exc.value)
|
||||
|
||||
def test_validate_config_invalid_due_date_format(self, db):
|
||||
"""Test validation fails for invalid due_date format."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "due_date", "value": "not-a-date"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Invalid due_date format" in str(exc.value)
|
||||
|
||||
def test_execute_update_priority(self, db, test_task, test_user):
|
||||
"""Test executing priority update."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "priority", "value": "urgent"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
assert test_task.priority == "medium"
|
||||
|
||||
result = action.execute(db, test_task, config, context)
|
||||
db.commit()
|
||||
|
||||
assert result["status"] == "success"
|
||||
assert result["field"] == "priority"
|
||||
assert result["old_value"] == "medium"
|
||||
assert result["new_value"] == "urgent"
|
||||
assert test_task.priority == "urgent"
|
||||
|
||||
def test_execute_update_status_id(self, db, test_task, test_statuses, test_user):
|
||||
"""Test executing status_id update."""
|
||||
action = UpdateFieldAction()
|
||||
new_status = test_statuses[1] # In Progress
|
||||
config = {"field": "status_id", "value": new_status.id}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = action.execute(db, test_task, config, context)
|
||||
db.commit()
|
||||
|
||||
assert result["status"] == "success"
|
||||
assert result["field"] == "status_id"
|
||||
assert test_task.status_id == new_status.id
|
||||
|
||||
def test_execute_update_status_id_invalid(self, db, test_task, test_user):
|
||||
"""Test executing status_id update with invalid status."""
|
||||
action = UpdateFieldAction()
|
||||
config = {"field": "status_id", "value": "non-existent-status"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
with pytest.raises(ActionExecutionError) as exc:
|
||||
action.execute(db, test_task, config, context)
|
||||
assert "not found in project" in str(exc.value)
|
||||
|
||||
def test_execute_update_due_date(self, db, test_task, test_user):
|
||||
"""Test executing due_date update."""
|
||||
action = UpdateFieldAction()
|
||||
due_date_str = "2025-12-31T23:59:59"
|
||||
config = {"field": "due_date", "value": due_date_str}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = action.execute(db, test_task, config, context)
|
||||
db.commit()
|
||||
|
||||
assert result["status"] == "success"
|
||||
assert result["field"] == "due_date"
|
||||
assert test_task.due_date is not None
|
||||
|
||||
|
||||
class TestAutoAssignAction:
|
||||
"""Tests for FEAT-015: Auto Assign Action."""
|
||||
|
||||
def test_validate_config_valid_round_robin(self, db):
|
||||
"""Test validation of valid round_robin config."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "round_robin"}
|
||||
# Should not raise
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_valid_least_loaded(self, db):
|
||||
"""Test validation of valid least_loaded config."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "least_loaded"}
|
||||
# Should not raise
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_valid_specific_user(self, db, test_user):
|
||||
"""Test validation of valid specific_user config."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "specific_user", "user_id": test_user.id}
|
||||
# Should not raise
|
||||
action.validate_config(config, db)
|
||||
|
||||
def test_validate_config_missing_strategy(self, db):
|
||||
"""Test validation fails when strategy is missing."""
|
||||
action = AutoAssignAction()
|
||||
config = {}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Missing required 'strategy'" in str(exc.value)
|
||||
|
||||
def test_validate_config_invalid_strategy(self, db):
|
||||
"""Test validation fails for invalid strategy."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "invalid_strategy"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Invalid strategy" in str(exc.value)
|
||||
|
||||
def test_validate_config_specific_user_missing_user_id(self, db):
|
||||
"""Test validation fails when user_id is missing for specific_user."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "specific_user"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "Missing required 'user_id'" in str(exc.value)
|
||||
|
||||
def test_validate_config_specific_user_invalid_user_id(self, db):
|
||||
"""Test validation fails for non-existent user."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "specific_user", "user_id": "non-existent-user"}
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
action.validate_config(config, db)
|
||||
assert "not found or inactive" in str(exc.value)
|
||||
|
||||
def test_execute_specific_user(self, db, test_task, test_user, additional_users):
|
||||
"""Test executing specific_user assignment."""
|
||||
action = AutoAssignAction()
|
||||
target_user = additional_users[0]
|
||||
config = {"strategy": "specific_user", "user_id": target_user.id}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = action.execute(db, test_task, config, context)
|
||||
db.commit()
|
||||
|
||||
assert result["status"] == "success"
|
||||
assert result["strategy"] == "specific_user"
|
||||
assert result["new_assignee_id"] == target_user.id
|
||||
assert test_task.assignee_id == target_user.id
|
||||
|
||||
def test_execute_round_robin(self, db, test_task, test_user, additional_users, test_project):
|
||||
"""Test executing round_robin assignment."""
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "round_robin"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
# Clear round robin state for this project
|
||||
AutoAssignAction._round_robin_index.pop(test_project.id, None)
|
||||
|
||||
# First assignment
|
||||
result1 = action.execute(db, test_task, config, context)
|
||||
db.commit()
|
||||
first_assignee = result1["new_assignee_id"]
|
||||
|
||||
assert result1["status"] == "success"
|
||||
assert result1["strategy"] == "round_robin"
|
||||
assert first_assignee is not None
|
||||
|
||||
# Create a new task for second assignment
|
||||
task2 = Task(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
title="Test Task 2",
|
||||
status_id=test_task.status_id,
|
||||
created_by=test_user.id,
|
||||
priority="medium",
|
||||
)
|
||||
db.add(task2)
|
||||
db.commit()
|
||||
db.refresh(task2)
|
||||
|
||||
# Second assignment should get next member in rotation
|
||||
result2 = action.execute(db, task2, config, context)
|
||||
db.commit()
|
||||
second_assignee = result2["new_assignee_id"]
|
||||
|
||||
assert result2["status"] == "success"
|
||||
# Round robin should cycle through members
|
||||
assert second_assignee is not None
|
||||
|
||||
def test_execute_least_loaded(self, db, test_task, test_user, additional_users, test_project, test_statuses):
|
||||
"""Test executing least_loaded assignment."""
|
||||
# Create tasks assigned to different users to create varied workloads
|
||||
for i, user in enumerate(additional_users):
|
||||
for j in range(i + 1): # User 0 gets 1 task, User 1 gets 2, etc.
|
||||
task = Task(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
title=f"Task for user {i} - {j}",
|
||||
status_id=test_statuses[0].id, # To Do (not done)
|
||||
created_by=test_user.id,
|
||||
assignee_id=user.id,
|
||||
priority="medium",
|
||||
)
|
||||
db.add(task)
|
||||
db.commit()
|
||||
|
||||
# Create a new unassigned task
|
||||
new_task = Task(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
title="New Task for Assignment",
|
||||
status_id=test_statuses[0].id,
|
||||
created_by=test_user.id,
|
||||
priority="medium",
|
||||
)
|
||||
db.add(new_task)
|
||||
db.commit()
|
||||
db.refresh(new_task)
|
||||
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "least_loaded"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = action.execute(db, new_task, config, context)
|
||||
db.commit()
|
||||
|
||||
assert result["status"] == "success"
|
||||
assert result["strategy"] == "least_loaded"
|
||||
# Should assign to user with fewest tasks
|
||||
assert result["new_assignee_id"] is not None
|
||||
|
||||
def test_execute_no_eligible_members(self, db, test_user, test_space):
|
||||
"""Test assignment when no eligible members are found."""
|
||||
# Create project without department (only owner as member)
|
||||
project = Project(
|
||||
id=str(uuid.uuid4()),
|
||||
space_id=test_space.id,
|
||||
title="Solo Project",
|
||||
owner_id=test_user.id,
|
||||
department_id=None, # No department
|
||||
)
|
||||
db.add(project)
|
||||
|
||||
status = TaskStatus(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=project.id,
|
||||
name="To Do",
|
||||
color="#808080",
|
||||
position=0,
|
||||
)
|
||||
db.add(status)
|
||||
db.commit()
|
||||
|
||||
task = Task(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=project.id,
|
||||
title="Solo Task",
|
||||
status_id=status.id,
|
||||
created_by=test_user.id,
|
||||
priority="medium",
|
||||
)
|
||||
db.add(task)
|
||||
db.commit()
|
||||
db.refresh(task)
|
||||
|
||||
action = AutoAssignAction()
|
||||
config = {"strategy": "round_robin"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = action.execute(db, task, config, context)
|
||||
|
||||
# Should still work - assigns to owner
|
||||
assert result["status"] == "success"
|
||||
assert task.assignee_id == test_user.id
|
||||
|
||||
|
||||
class TestActionExecutor:
|
||||
"""Tests for ActionExecutor registry and execution."""
|
||||
|
||||
def test_get_supported_actions(self):
|
||||
"""Test getting supported action types."""
|
||||
actions = ActionExecutor.get_supported_actions()
|
||||
assert "update_field" in actions
|
||||
assert "auto_assign" in actions
|
||||
|
||||
def test_validate_action_update_field(self, db):
|
||||
"""Test validating update_field action via executor."""
|
||||
action = {
|
||||
"type": "update_field",
|
||||
"config": {"field": "priority", "value": "high"},
|
||||
}
|
||||
# Should not raise
|
||||
ActionExecutor.validate_action(action, db)
|
||||
|
||||
def test_validate_action_auto_assign(self, db, test_user):
|
||||
"""Test validating auto_assign action via executor."""
|
||||
action = {
|
||||
"type": "auto_assign",
|
||||
"config": {"strategy": "specific_user", "user_id": test_user.id},
|
||||
}
|
||||
# Should not raise
|
||||
ActionExecutor.validate_action(action, db)
|
||||
|
||||
def test_validate_action_unknown_type(self, db):
|
||||
"""Test that unknown action types pass through (for extensibility)."""
|
||||
action = {"type": "unknown_action"}
|
||||
# Should not raise - allows other handlers to process
|
||||
ActionExecutor.validate_action(action, db)
|
||||
|
||||
def test_execute_action_update_field(self, db, test_task, test_user):
|
||||
"""Test executing update_field action via executor."""
|
||||
action = {
|
||||
"type": "update_field",
|
||||
"config": {"field": "priority", "value": "urgent"},
|
||||
}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = ActionExecutor.execute_action(db, test_task, action, context)
|
||||
db.commit()
|
||||
|
||||
assert result is not None
|
||||
assert result["status"] == "success"
|
||||
assert test_task.priority == "urgent"
|
||||
|
||||
def test_execute_action_auto_assign(self, db, test_task, test_user, additional_users):
|
||||
"""Test executing auto_assign action via executor."""
|
||||
target_user = additional_users[1]
|
||||
action = {
|
||||
"type": "auto_assign",
|
||||
"config": {"strategy": "specific_user", "user_id": target_user.id},
|
||||
}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = ActionExecutor.execute_action(db, test_task, action, context)
|
||||
db.commit()
|
||||
|
||||
assert result is not None
|
||||
assert result["status"] == "success"
|
||||
assert test_task.assignee_id == target_user.id
|
||||
|
||||
def test_execute_action_unknown_type_returns_none(self, db, test_task, test_user):
|
||||
"""Test that unknown action types return None."""
|
||||
action = {"type": "unknown_action"}
|
||||
context = {"current_user": test_user}
|
||||
|
||||
result = ActionExecutor.execute_action(db, test_task, action, context)
|
||||
|
||||
assert result is None
|
||||
|
||||
|
||||
class TestTriggerServiceIntegration:
|
||||
"""Tests for TriggerService integration with new actions."""
|
||||
|
||||
def test_validate_actions_update_field(self, db):
|
||||
"""Test TriggerService validates update_field actions."""
|
||||
actions = [{
|
||||
"type": "update_field",
|
||||
"config": {"field": "priority", "value": "high"},
|
||||
}]
|
||||
# Should not raise
|
||||
TriggerService.validate_actions(actions, db)
|
||||
|
||||
def test_validate_actions_auto_assign(self, db, test_user):
|
||||
"""Test TriggerService validates auto_assign actions."""
|
||||
actions = [{
|
||||
"type": "auto_assign",
|
||||
"config": {"strategy": "specific_user", "user_id": test_user.id},
|
||||
}]
|
||||
# Should not raise
|
||||
TriggerService.validate_actions(actions, db)
|
||||
|
||||
def test_validate_actions_invalid_action_type(self, db):
|
||||
"""Test TriggerService rejects invalid action types."""
|
||||
actions = [{"type": "invalid_action"}]
|
||||
with pytest.raises(ActionValidationError) as exc:
|
||||
TriggerService.validate_actions(actions, db)
|
||||
assert "Invalid action type" in str(exc.value)
|
||||
|
||||
def test_get_supported_action_types(self):
|
||||
"""Test getting all supported action types."""
|
||||
types = TriggerService.get_supported_action_types()
|
||||
assert "notify" in types
|
||||
assert "update_field" in types
|
||||
assert "auto_assign" in types
|
||||
|
||||
def test_evaluate_triggers_with_update_field_action(
|
||||
self, db, test_task, test_user, test_project, test_statuses
|
||||
):
|
||||
"""Test trigger evaluation with update_field action."""
|
||||
# Create trigger that updates priority when status changes
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Status Change Priority Trigger",
|
||||
trigger_type="field_change",
|
||||
conditions={
|
||||
"field": "status_id",
|
||||
"operator": "changed_to",
|
||||
"value": test_statuses[1].id, # In Progress
|
||||
},
|
||||
actions=[{
|
||||
"type": "update_field",
|
||||
"config": {"field": "priority", "value": "high"},
|
||||
}],
|
||||
is_active=True,
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(trigger)
|
||||
db.commit()
|
||||
|
||||
old_values = {"status_id": test_statuses[0].id}
|
||||
new_values = {"status_id": test_statuses[1].id}
|
||||
|
||||
logs = TriggerService.evaluate_triggers(db, test_task, old_values, new_values, test_user)
|
||||
db.commit()
|
||||
|
||||
assert len(logs) == 1
|
||||
assert logs[0].status == "success"
|
||||
assert test_task.priority == "high"
|
||||
|
||||
def test_evaluate_triggers_with_auto_assign_action(
|
||||
self, db, test_task, test_user, test_project, test_statuses, additional_users
|
||||
):
|
||||
"""Test trigger evaluation with auto_assign action."""
|
||||
target_user = additional_users[0]
|
||||
|
||||
# Create trigger that auto-assigns when priority changes to urgent
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Priority Auto Assign Trigger",
|
||||
trigger_type="field_change",
|
||||
conditions={
|
||||
"field": "priority",
|
||||
"operator": "changed_to",
|
||||
"value": "urgent",
|
||||
},
|
||||
actions=[{
|
||||
"type": "auto_assign",
|
||||
"config": {"strategy": "specific_user", "user_id": target_user.id},
|
||||
}],
|
||||
is_active=True,
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(trigger)
|
||||
db.commit()
|
||||
|
||||
old_values = {"priority": "medium"}
|
||||
new_values = {"priority": "urgent"}
|
||||
|
||||
logs = TriggerService.evaluate_triggers(db, test_task, old_values, new_values, test_user)
|
||||
db.commit()
|
||||
|
||||
assert len(logs) == 1
|
||||
assert logs[0].status == "success"
|
||||
assert test_task.assignee_id == target_user.id
|
||||
|
||||
def test_evaluate_triggers_with_multiple_actions(
|
||||
self, db, test_task, test_user, test_project, test_statuses, additional_users
|
||||
):
|
||||
"""Test trigger evaluation with multiple actions."""
|
||||
target_user = additional_users[0]
|
||||
|
||||
# Create trigger with both update_field and auto_assign
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Multi Action Trigger",
|
||||
trigger_type="field_change",
|
||||
conditions={
|
||||
"field": "status_id",
|
||||
"operator": "changed_to",
|
||||
"value": test_statuses[1].id,
|
||||
},
|
||||
actions=[
|
||||
{
|
||||
"type": "update_field",
|
||||
"config": {"field": "priority", "value": "urgent"},
|
||||
},
|
||||
{
|
||||
"type": "auto_assign",
|
||||
"config": {"strategy": "specific_user", "user_id": target_user.id},
|
||||
},
|
||||
],
|
||||
is_active=True,
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(trigger)
|
||||
db.commit()
|
||||
|
||||
old_values = {"status_id": test_statuses[0].id}
|
||||
new_values = {"status_id": test_statuses[1].id}
|
||||
|
||||
logs = TriggerService.evaluate_triggers(db, test_task, old_values, new_values, test_user)
|
||||
db.commit()
|
||||
|
||||
assert len(logs) == 1
|
||||
assert logs[0].status == "success"
|
||||
assert test_task.priority == "urgent"
|
||||
assert test_task.assignee_id == target_user.id
|
||||
|
||||
# Check that both actions are logged
|
||||
details = logs[0].details
|
||||
assert len(details["actions_executed"]) == 2
|
||||
@@ -273,3 +273,145 @@ class TestEncryptionKeyValidation:
|
||||
}):
|
||||
settings = Settings()
|
||||
assert settings.ENCRYPTION_MASTER_KEY is None
|
||||
|
||||
|
||||
class TestConfidentialProjectUpload:
|
||||
"""Tests for file upload on confidential projects."""
|
||||
|
||||
@pytest.fixture
|
||||
def test_user(self, db):
|
||||
"""Create a test user."""
|
||||
from app.models import User
|
||||
import uuid as uuid_module
|
||||
|
||||
user = User(
|
||||
id=str(uuid_module.uuid4()),
|
||||
email="testuser_enc@example.com",
|
||||
name="Test User Encryption",
|
||||
role_id="00000000-0000-0000-0000-000000000003",
|
||||
is_active=True,
|
||||
is_system_admin=False,
|
||||
)
|
||||
db.add(user)
|
||||
db.commit()
|
||||
return user
|
||||
|
||||
@pytest.fixture
|
||||
def test_user_token(self, client, mock_redis, test_user):
|
||||
"""Get a token for test user."""
|
||||
from app.core.security import create_access_token, create_token_payload
|
||||
|
||||
token_data = create_token_payload(
|
||||
user_id=test_user.id,
|
||||
email=test_user.email,
|
||||
role="engineer",
|
||||
department_id=None,
|
||||
is_system_admin=False,
|
||||
)
|
||||
token = create_access_token(token_data)
|
||||
mock_redis.setex(f"session:{test_user.id}", 900, token)
|
||||
return token
|
||||
|
||||
@pytest.fixture
|
||||
def test_space(self, db, test_user):
|
||||
"""Create a test space."""
|
||||
from app.models import Space
|
||||
import uuid as uuid_module
|
||||
|
||||
space = Space(
|
||||
id=str(uuid_module.uuid4()),
|
||||
name="Test Space Encryption",
|
||||
description="Test space for encryption tests",
|
||||
owner_id=test_user.id,
|
||||
)
|
||||
db.add(space)
|
||||
db.commit()
|
||||
return space
|
||||
|
||||
@pytest.fixture
|
||||
def confidential_project(self, db, test_space, test_user):
|
||||
"""Create a confidential test project."""
|
||||
from app.models import Project
|
||||
import uuid as uuid_module
|
||||
|
||||
project = Project(
|
||||
id=str(uuid_module.uuid4()),
|
||||
space_id=test_space.id,
|
||||
title="Confidential Project",
|
||||
description="Test confidential project",
|
||||
owner_id=test_user.id,
|
||||
security_level="confidential",
|
||||
)
|
||||
db.add(project)
|
||||
db.commit()
|
||||
return project
|
||||
|
||||
@pytest.fixture
|
||||
def test_task(self, db, confidential_project, test_user):
|
||||
"""Create a test task in confidential project."""
|
||||
from app.models import Task
|
||||
import uuid as uuid_module
|
||||
|
||||
task = Task(
|
||||
id=str(uuid_module.uuid4()),
|
||||
project_id=confidential_project.id,
|
||||
title="Test Task Encryption",
|
||||
description="Test task for encryption tests",
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(task)
|
||||
db.commit()
|
||||
return task
|
||||
|
||||
def test_upload_confidential_project_encryption_unavailable(
|
||||
self, client, test_user_token, test_task, db
|
||||
):
|
||||
"""Test that uploading to confidential project returns 400 when encryption is unavailable."""
|
||||
from io import BytesIO
|
||||
|
||||
# Mock encryption service to return False for is_encryption_available
|
||||
with patch('app.api.attachments.router.encryption_service') as mock_enc_service:
|
||||
mock_enc_service.is_encryption_available.return_value = False
|
||||
|
||||
content = b"Test file content"
|
||||
files = {"file": ("test.pdf", BytesIO(content), "application/pdf")}
|
||||
|
||||
response = client.post(
|
||||
f"/api/tasks/{test_task.id}/attachments",
|
||||
headers={"Authorization": f"Bearer {test_user_token}"},
|
||||
files=files,
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert "ENCRYPTION_MASTER_KEY" in response.json()["detail"]
|
||||
assert "environment variable" in response.json()["detail"]
|
||||
|
||||
def test_upload_confidential_project_no_active_key(
|
||||
self, client, test_user_token, test_task, db
|
||||
):
|
||||
"""Test that uploading to confidential project returns 400 when no active encryption key exists."""
|
||||
from io import BytesIO
|
||||
from app.models import EncryptionKey
|
||||
|
||||
# Make sure no active encryption keys exist
|
||||
db.query(EncryptionKey).filter(EncryptionKey.is_active == True).update(
|
||||
{"is_active": False}
|
||||
)
|
||||
db.commit()
|
||||
|
||||
# Mock encryption service to return True for is_encryption_available
|
||||
with patch('app.api.attachments.router.encryption_service') as mock_enc_service:
|
||||
mock_enc_service.is_encryption_available.return_value = True
|
||||
|
||||
content = b"Test file content"
|
||||
files = {"file": ("test.pdf", BytesIO(content), "application/pdf")}
|
||||
|
||||
response = client.post(
|
||||
f"/api/tasks/{test_task.id}/attachments",
|
||||
headers={"Authorization": f"Bearer {test_user_token}"},
|
||||
files=files,
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert "active encryption key" in response.json()["detail"]
|
||||
assert "create" in response.json()["detail"].lower()
|
||||
|
||||
@@ -375,3 +375,119 @@ class TestTriggerAPI:
|
||||
|
||||
assert response.status_code == 400
|
||||
assert "Invalid operator" in response.json()["detail"]
|
||||
|
||||
|
||||
class TestTriggerActionRollback:
|
||||
"""Tests for trigger action rollback mechanism."""
|
||||
|
||||
def test_multi_action_rollback_on_failure(self, db, test_task, test_project, test_user, test_status):
|
||||
"""Test that when one action fails, all previous actions are rolled back.
|
||||
|
||||
Scenario:
|
||||
1. Create a trigger with 2 actions: update_field (priority) + update_field (invalid status_id)
|
||||
2. The first action should update priority to 'high'
|
||||
3. The second action should fail because of invalid status_id
|
||||
4. The first action's change should be rolled back
|
||||
"""
|
||||
# Record original priority
|
||||
original_priority = test_task.priority
|
||||
|
||||
# Create a trigger with multiple actions where the second one will fail
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Multi-Action Trigger",
|
||||
description="Test rollback on failure",
|
||||
trigger_type="field_change",
|
||||
conditions={
|
||||
"field": "status_id",
|
||||
"operator": "changed_to",
|
||||
"value": test_status[1].id,
|
||||
},
|
||||
actions=[
|
||||
# First action: update priority (should succeed)
|
||||
{
|
||||
"type": "update_field",
|
||||
"field": "priority",
|
||||
"value": "high",
|
||||
},
|
||||
# Second action: update to invalid status (should fail)
|
||||
{
|
||||
"type": "update_field",
|
||||
"field": "status_id",
|
||||
"value": "non-existent-status-id",
|
||||
},
|
||||
],
|
||||
is_active=True,
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(trigger)
|
||||
db.commit()
|
||||
|
||||
old_values = {"status_id": test_status[0].id}
|
||||
new_values = {"status_id": test_status[1].id}
|
||||
|
||||
# Execute trigger - second action should fail
|
||||
logs = TriggerService.evaluate_triggers(db, test_task, old_values, new_values, test_user)
|
||||
db.commit()
|
||||
|
||||
# Verify trigger execution failed
|
||||
assert len(logs) == 1
|
||||
assert logs[0].status == "failed"
|
||||
assert logs[0].error_message is not None
|
||||
assert "not found" in logs[0].error_message.lower()
|
||||
|
||||
# Refresh task from database to get the actual state
|
||||
db.refresh(test_task)
|
||||
|
||||
# Verify that the first action's change was rolled back
|
||||
# Priority should remain unchanged (not 'high')
|
||||
assert test_task.priority == original_priority, (
|
||||
f"Expected priority to be rolled back to '{original_priority}', "
|
||||
f"but got '{test_task.priority}'"
|
||||
)
|
||||
|
||||
def test_all_actions_succeed_committed(self, db, test_task, test_project, test_user, test_status):
|
||||
"""Test that when all actions succeed, changes are committed."""
|
||||
# Create a trigger with multiple successful actions
|
||||
trigger = Trigger(
|
||||
id=str(uuid.uuid4()),
|
||||
project_id=test_project.id,
|
||||
name="Success Trigger",
|
||||
description="Test successful commit",
|
||||
trigger_type="field_change",
|
||||
conditions={
|
||||
"field": "status_id",
|
||||
"operator": "changed_to",
|
||||
"value": test_status[1].id,
|
||||
},
|
||||
actions=[
|
||||
# First action: update priority
|
||||
{
|
||||
"type": "update_field",
|
||||
"field": "priority",
|
||||
"value": "urgent",
|
||||
},
|
||||
],
|
||||
is_active=True,
|
||||
created_by=test_user.id,
|
||||
)
|
||||
db.add(trigger)
|
||||
db.commit()
|
||||
|
||||
old_values = {"status_id": test_status[0].id}
|
||||
new_values = {"status_id": test_status[1].id}
|
||||
|
||||
# Execute trigger
|
||||
logs = TriggerService.evaluate_triggers(db, test_task, old_values, new_values, test_user)
|
||||
db.commit()
|
||||
|
||||
# Verify trigger execution succeeded
|
||||
assert len(logs) == 1
|
||||
assert logs[0].status == "success"
|
||||
|
||||
# Refresh task from database
|
||||
db.refresh(test_task)
|
||||
|
||||
# Verify that the change was committed
|
||||
assert test_task.priority == "urgent"
|
||||
|
||||
Reference in New Issue
Block a user