Feat/be/locking

[Cemonix]

No description provided.

[Copilot]

Pull Request Overview

This PR refactors the task locking system to use assignment-based concurrency control instead of explicit task locks. It replaces the time-based locking mechanism with auto-assignment features and enhanced task navigation, improving user experience by automatically finding available tasks.

• Replace task locking with auto-assignment when users navigate or access tasks
• Remove TaskLockingService and related lock cleanup infrastructure
• Add new API endpoints for fetching user-assigned or unassigned tasks
• Enhance task navigation with availability counters and automatic fallback logic

Reviewed Changes

Copilot reviewed 32 out of 33 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
server/Server/Services/TaskService.cs	Added auto-assignment logic and task validation with new GetTasksForUserOrUnassignedAsync method
server/Server/Services/TaskNavigationService.cs	Replaced locking with auto-assignment in navigation and added task availability counting
server/Server/Services/TaskLockingService.cs	Completely removed - replaced with assignment-based system
server/Server/Models/Domain/Task.cs	Removed lock-related fields (LockedByUserId, LockedAt, LockExpiresAt)
server/Server/Controllers/TasksController.cs	Enhanced GetTaskById with auto-assignment and access control, added new endpoint
frontend/src/views/project/TasksView.vue	Added logic to show only user-assigned or unassigned tasks for non-managers

Files not reviewed (1)

• server/Server/Data/Migrations/Laberis/20250907153011_RemoveTaskLockingFields.Designer.cs: Language not supported

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The comment mentions 'deferred tasks or completed tasks' but the code only checks for DEFERRED status. Either add the check for completed tasks or update the comment to match the actual implementation.

[Copilot]

The explicit cast to Expression<Func<Models.Domain.Task, bool>> is unnecessary and makes the code harder to read. The compiler can infer the type automatically.

Suggested change

	? (Expression<Func<Models.Domain.Task, bool>>)(t => t.ProjectId == projectId && t.WorkflowStageId == workflowStageId.Value)
	: t => t.ProjectId == projectId;
	? (t => t.ProjectId == projectId && t.WorkflowStageId == workflowStageId.Value)
	: (t => t.ProjectId == projectId);

[Copilot]

Using int.MaxValue as pageSize could lead to memory issues with large datasets. Consider implementing a more efficient approach like using database aggregation functions or processing tasks in batches.

Suggested change

	var allTasks = await _taskRepository.GetAllAsync(filter, pageSize: int.MaxValue);
	var availableTasks = 0;
	var completedTasks = 0;
	var assignedTasks = 0;
	foreach (var task in allTasks)
	{
	// Skip archived and vetoed tasks from counts
	if (task.Status == Models.Domain.Enums.TaskStatus.ARCHIVED ||
	task.Status == Models.Domain.Enums.TaskStatus.VETOED)
	continue;
	if (task.Status == Models.Domain.Enums.TaskStatus.COMPLETED)
	{
	// Count completed tasks by this user
	if (task.AssignedToUserId == userId || task.LastWorkedOnByUserId == userId)
	completedTasks++;
	}
	else if (string.IsNullOrEmpty(task.AssignedToUserId))
	{
	// Unassigned tasks are available
	availableTasks++;
	}
	else if (task.AssignedToUserId != userId)
	{
	// Tasks assigned to other users
	assignedTasks++;
	}
	else
	{
	// Tasks assigned to current user are available to them
	availableTasks++;
	}
	}
	const int batchSize = 1000;
	int page = 1;
	var availableTasks = 0;
	var completedTasks = 0;
	var assignedTasks = 0;
	List<Models.Domain.Task> batch;
	do
	{
	batch = await _taskRepository.GetAllAsync(filter, page: page, pageSize: batchSize);
	foreach (var task in batch)
	{
	// Skip archived and vetoed tasks from counts
	if (task.Status == Models.Domain.Enums.TaskStatus.ARCHIVED ||
	task.Status == Models.Domain.Enums.TaskStatus.VETOED)
	continue;
	if (task.Status == Models.Domain.Enums.TaskStatus.COMPLETED)
	{
	// Count completed tasks by this user
	if (task.AssignedToUserId == userId || task.LastWorkedOnByUserId == userId)
	completedTasks++;
	}
	else if (string.IsNullOrEmpty(task.AssignedToUserId))
	{
	// Unassigned tasks are available
	availableTasks++;
	}
	else if (task.AssignedToUserId != userId)
	{
	// Tasks assigned to other users
	assignedTasks++;
	}
	else
	{
	// Tasks assigned to current user are available to them
	availableTasks++;
	}
	}
	page++;
	} while (batch.Count == batchSize);

[Copilot]

The permission check is incomplete and uses a simplified approach that may not properly validate project-specific permissions. This could lead to unauthorized access if users have MANAGER role in one project but not another.

Suggested change

	private bool HasProjectPermission(string permission)
	{
	// This is a simplified check - in a real implementation, you'd check against the project's permissions
	// For now, we'll use the UPDATE permission as a proxy for MANAGER role
	return User.IsInRole("MANAGER") || User.HasClaim("permission", permission);
	private bool HasProjectPermission(int projectId, string permission)
	{
	// Check for project-specific MANAGER role or permission claim
	// Example claim types: "project:{projectId}:role", "project:{projectId}:permission"
	var projectRoleClaim = User.Claims.FirstOrDefault(c =>
	c.Type == $"project:{projectId}:role" && c.Value == "MANAGER");
	if (projectRoleClaim != null)
	{
	return true;
	}
	var projectPermissionClaim = User.Claims.FirstOrDefault(c =>
	c.Type == $"project:{projectId}:permission" && c.Value == permission);
	return projectPermissionClaim != null;