From 509fcc8f64b052af551ad3ea84f7eabca3bb4741 Mon Sep 17 00:00:00 2001 From: chamikaJ Date: Mon, 9 Jun 2025 11:24:49 +0530 Subject: [PATCH] refactor(project-finance): improve task cost calculations and UI hierarchy - Updated SQL queries in the ProjectFinanceController to exclude parent tasks from descendant cost calculations, ensuring accurate financial data aggregation. - Refactored the project finance slice to implement recursive task updates for fixed costs, estimated costs, and time logged, enhancing task management efficiency. - Enhanced the FinanceTable component to visually represent task hierarchy with improved indentation and hover effects, improving user experience and clarity. - Added responsive styles for nested tasks and adjusted task name styling for better readability across different levels. --- .../controllers/project-finance-controller.ts | 20 +- .../projects/finance/project-finance.slice.ts | 168 ++++++++++++---- .../finance-table/finance-table.css | 64 +++++- .../finance-table/finance-table.tsx | 186 +++++++++++------- 4 files changed, 316 insertions(+), 122 deletions(-) diff --git a/worklenz-backend/src/controllers/project-finance-controller.ts b/worklenz-backend/src/controllers/project-finance-controller.ts index bd78153e..a5bb5e39 100644 --- a/worklenz-backend/src/controllers/project-finance-controller.ts +++ b/worklenz-backend/src/controllers/project-finance-controller.ts @@ -176,12 +176,12 @@ export default class ProjectfinanceController extends WorklenzControllerBase { tc.billable, tc.fixed_cost, tc.sub_tasks_count, - -- For parent tasks, sum values from all descendants including self + -- For parent tasks, sum values from descendants only (exclude parent task itself) CASE WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.estimated_seconds) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.estimated_seconds END as estimated_seconds, @@ -189,7 +189,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.total_time_logged_seconds) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.total_time_logged_seconds END as total_time_logged_seconds, @@ -197,7 +197,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.estimated_cost) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.estimated_cost END as estimated_cost, @@ -205,7 +205,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.actual_cost_from_logs) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.actual_cost_from_logs END as actual_cost_from_logs @@ -860,12 +860,12 @@ export default class ProjectfinanceController extends WorklenzControllerBase { tc.billable, tc.fixed_cost, tc.sub_tasks_count, - -- For parent tasks, sum values from all descendants including self + -- For parent tasks, sum values from descendants only (exclude parent task itself) CASE WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.estimated_seconds) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.estimated_seconds END as estimated_seconds, @@ -873,7 +873,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.total_time_logged_seconds) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.total_time_logged_seconds END as total_time_logged_seconds, @@ -881,7 +881,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.estimated_cost) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.estimated_cost END as estimated_cost, @@ -889,7 +889,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { WHEN tc.level = 0 AND tc.sub_tasks_count > 0 THEN ( SELECT SUM(sub_tc.actual_cost_from_logs) FROM task_costs sub_tc - WHERE sub_tc.root_id = tc.id + WHERE sub_tc.root_id = tc.id AND sub_tc.id != tc.id ) ELSE tc.actual_cost_from_logs END as actual_cost_from_logs diff --git a/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts b/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts index c74a6d4b..42581a93 100644 --- a/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts +++ b/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts @@ -122,53 +122,109 @@ export const projectFinancesSlice = createSlice({ updateTaskFixedCost: (state, action: PayloadAction<{ taskId: string; groupId: string; fixedCost: number }>) => { const { taskId, groupId, fixedCost } = action.payload; const group = state.taskGroups.find(g => g.group_id === groupId); + if (group) { - const task = group.tasks.find(t => t.id === taskId); - if (task) { - task.fixed_cost = fixedCost; - // Don't recalculate here - let the backend handle it and we'll refresh - } + // Recursive function to find and update a task in the hierarchy + const findAndUpdateTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + task.fixed_cost = fixedCost; + // Don't recalculate here - let the backend handle it and we'll refresh + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + + findAndUpdateTask(group.tasks, taskId); } }, updateTaskEstimatedCost: (state, action: PayloadAction<{ taskId: string; groupId: string; estimatedCost: number }>) => { const { taskId, groupId, estimatedCost } = action.payload; const group = state.taskGroups.find(g => g.group_id === groupId); + if (group) { - const task = group.tasks.find(t => t.id === taskId); - if (task) { - task.estimated_cost = estimatedCost; - // Recalculate task costs after updating estimated cost - const { totalBudget, totalActual, variance } = calculateTaskCosts(task); - task.total_budget = totalBudget; - task.total_actual = totalActual; - task.variance = variance; - } + // Recursive function to find and update a task in the hierarchy + const findAndUpdateTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + task.estimated_cost = estimatedCost; + // Recalculate task costs after updating estimated cost + const { totalBudget, totalActual, variance } = calculateTaskCosts(task); + task.total_budget = totalBudget; + task.total_actual = totalActual; + task.variance = variance; + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + + findAndUpdateTask(group.tasks, taskId); } }, updateTaskTimeLogged: (state, action: PayloadAction<{ taskId: string; groupId: string; timeLoggedSeconds: number; timeLoggedString: string }>) => { const { taskId, groupId, timeLoggedSeconds, timeLoggedString } = action.payload; const group = state.taskGroups.find(g => g.group_id === groupId); + if (group) { - const task = group.tasks.find(t => t.id === taskId); - if (task) { - task.total_time_logged_seconds = timeLoggedSeconds; - task.total_time_logged = timeLoggedString; - // Recalculate task costs after updating time logged - const { totalBudget, totalActual, variance } = calculateTaskCosts(task); - task.total_budget = totalBudget; - task.total_actual = totalActual; - task.variance = variance; - } + // Recursive function to find and update a task in the hierarchy + const findAndUpdateTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + task.total_time_logged_seconds = timeLoggedSeconds; + task.total_time_logged = timeLoggedString; + // Recalculate task costs after updating time logged + const { totalBudget, totalActual, variance } = calculateTaskCosts(task); + task.total_budget = totalBudget; + task.total_actual = totalActual; + task.variance = variance; + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + + findAndUpdateTask(group.tasks, taskId); } }, toggleTaskExpansion: (state, action: PayloadAction<{ taskId: string; groupId: string }>) => { const { taskId, groupId } = action.payload; const group = state.taskGroups.find(g => g.group_id === groupId); + if (group) { - const task = group.tasks.find(t => t.id === taskId); - if (task) { - task.show_sub_tasks = !task.show_sub_tasks; - } + // Recursive function to find and toggle a task in the hierarchy + const findAndToggleTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + task.show_sub_tasks = !task.show_sub_tasks; + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndToggleTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + + findAndToggleTask(group.tasks, taskId); } }, updateProjectFinanceCurrency: (state, action: PayloadAction) => { @@ -200,26 +256,56 @@ export const projectFinancesSlice = createSlice({ .addCase(updateTaskFixedCostAsync.fulfilled, (state, action) => { const { taskId, groupId, fixedCost } = action.payload; const group = state.taskGroups.find(g => g.group_id === groupId); + if (group) { - const task = group.tasks.find(t => t.id === taskId); - if (task) { - task.fixed_cost = fixedCost; - // Don't recalculate here - trigger a refresh instead for accuracy - } + // Recursive function to find and update a task in the hierarchy + const findAndUpdateTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + task.fixed_cost = fixedCost; + // Don't recalculate here - trigger a refresh instead for accuracy + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + + findAndUpdateTask(group.tasks, taskId); } }) .addCase(fetchSubTasks.fulfilled, (state, action) => { const { parentTaskId, subTasks } = action.payload; + + // Recursive function to find and update a task in the hierarchy + const findAndUpdateTask = (tasks: IProjectFinanceTask[], targetId: string): boolean => { + for (const task of tasks) { + if (task.id === targetId) { + // Found the parent task, add subtasks + task.sub_tasks = subTasks.map(subTask => ({ + ...subTask, + is_sub_task: true, + parent_task_id: targetId + })); + task.show_sub_tasks = true; + return true; + } + + // Search in subtasks recursively + if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { + return true; + } + } + return false; + }; + // Find the parent task in any group and add the subtasks for (const group of state.taskGroups) { - const parentTask = group.tasks.find(t => t.id === parentTaskId); - if (parentTask) { - parentTask.sub_tasks = subTasks.map(subTask => ({ - ...subTask, - is_sub_task: true, - parent_task_id: parentTaskId - })); - parentTask.show_sub_tasks = true; + if (findAndUpdateTask(group.tasks, parentTaskId)) { break; } } diff --git a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.css b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.css index 09389058..e6f6b544 100644 --- a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.css +++ b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.css @@ -1 +1,63 @@ -/* Finance Table Styles */ \ No newline at end of file +/* Finance Table Styles */ + +/* Enhanced hierarchy visual indicators */ +.finance-table-task-row { + transition: all 0.2s ease-in-out; + border-bottom: 1px solid rgba(0, 0, 0, 0.06); +} + +.dark .finance-table-task-row { + border-bottom: 1px solid rgba(255, 255, 255, 0.08); +} + +/* Hover effect is now handled by inline styles in the component for consistency */ + +/* Nested task styling */ +.finance-table-nested-task { + /* No visual connectors, just clean indentation */ +} + +/* Expand/collapse button styling */ +.finance-table-expand-btn { + transition: all 0.2s ease-in-out; + border-radius: 2px; + padding: 2px; +} + +.finance-table-expand-btn:hover { + background: rgba(0, 0, 0, 0.05); + transform: scale(1.1); +} + +.dark .finance-table-expand-btn:hover { + background: rgba(255, 255, 255, 0.05); +} + +/* Task name styling for different levels */ +.finance-table-task-name { + transition: all 0.2s ease-in-out; +} + +.finance-table-task-name:hover { + color: #40a9ff !important; +} + +/* Fixed cost input styling */ +.fixed-cost-input { + border-radius: 4px; +} + +.fixed-cost-input:focus { + box-shadow: 0 0 0 2px rgba(24, 144, 255, 0.2); +} + +/* Responsive adjustments for nested content */ +@media (max-width: 768px) { + .finance-table-nested-task { + padding-left: 12px; + } + + .finance-table-task-name { + font-size: 12px !important; + } +} \ No newline at end of file diff --git a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.tsx b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.tsx index 8c8b74b6..968956a0 100644 --- a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.tsx +++ b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table.tsx @@ -1,5 +1,5 @@ import { Flex, InputNumber, Skeleton, Tooltip, Typography } from 'antd'; -import { useEffect, useMemo, useState, useRef } from 'react'; +import React, { useEffect, useMemo, useState, useRef } from 'react'; import { useAppSelector } from '@/hooks/useAppSelector'; import { DollarCircleOutlined, @@ -24,6 +24,8 @@ import { useParams } from 'react-router-dom'; import { useAuthService } from '@/hooks/useAuth'; import { canEditFixedCost } from '@/utils/finance-permissions'; import './finance-table.css'; +import { fetchPhasesByProjectId } from '@/features/projects/singleProject/phase/phases.slice'; +import { fetchPriorities } from '@/features/taskAttributes/taskPrioritySlice'; type FinanceTableProps = { table: IProjectFinanceGroup; @@ -41,6 +43,7 @@ const FinanceTable = ({ const [selectedTask, setSelectedTask] = useState(null); const [editingFixedCostValue, setEditingFixedCostValue] = useState(null); const [tasks, setTasks] = useState(table.tasks); + const [hoveredTaskId, setHoveredTaskId] = useState(null); const saveTimeoutRef = useRef(null); const dispatch = useAppDispatch(); @@ -159,8 +162,10 @@ const FinanceTable = ({ if (!taskId || !projectId) return; dispatch(setSelectedTaskId(taskId)); - dispatch(setShowTaskDrawer(true)); + dispatch(fetchPhasesByProjectId(projectId)); + dispatch(fetchPriorities()); dispatch(fetchTask({ taskId, projectId })); + dispatch(setShowTaskDrawer(true)); }; // Handle task expansion/collapse @@ -208,35 +213,112 @@ const FinanceTable = ({ setEditingFixedCostValue(null); }; - const renderFinancialTableColumnContent = (columnKey: FinanceTableColumnKeys, task: IProjectFinanceTask) => { + // Calculate indentation based on nesting level + const getTaskIndentation = (level: number) => level * 32; // 32px per level for better visibility + + // Recursive function to render task hierarchy + const renderTaskHierarchy = (task: IProjectFinanceTask, level: number = 0): React.ReactElement[] => { + const elements: React.ReactElement[] = []; + + // Add the current task + const isHovered = hoveredTaskId === task.id; + const rowIndex = elements.length; + const defaultBg = rowIndex % 2 === 0 ? themeWiseColor('#fafafa', '#232323', themeMode) : themeWiseColor('#ffffff', '#181818', themeMode); + const hoverBg = themeMode === 'dark' ? 'rgba(64, 169, 255, 0.08)' : 'rgba(24, 144, 255, 0.04)'; + + elements.push( + 0 ? 'finance-table-nested-task' : ''} ${themeMode === 'dark' ? 'dark' : ''}`} + onMouseEnter={() => setHoveredTaskId(task.id)} + onMouseLeave={() => setHoveredTaskId(null)} + > + {financeTableColumns.map((col) => ( + e.stopPropagation() + : undefined + } + > + {renderFinancialTableColumnContent(col.key, task, level)} + + ))} + + ); + + // Add subtasks recursively if they are expanded and loaded + if (task.show_sub_tasks && task.sub_tasks) { + task.sub_tasks.forEach(subTask => { + elements.push(...renderTaskHierarchy(subTask, level + 1)); + }); + } + + return elements; + }; + + const renderFinancialTableColumnContent = (columnKey: FinanceTableColumnKeys, task: IProjectFinanceTask, level: number = 0) => { switch (columnKey) { case FinanceTableColumnKeys.TASK: return ( - - {/* Indentation for subtasks */} - {task.is_sub_task &&
} + {/* Expand/collapse icon for parent tasks */} {task.sub_tasks_count > 0 && (
{ e.stopPropagation(); handleTaskExpansion(task); }} > - {task.show_sub_tasks ? : } + {task.show_sub_tasks ? : }
)} + {/* Spacer for tasks without subtasks to align with those that have expand icons */} + {task.sub_tasks_count === 0 && level > 0 && ( +
+ )} + {/* Task name */} 0 ? 140 : 160), + maxWidth: Math.max(100, 200 - getTaskIndentation(level) - (task.sub_tasks_count > 0 ? 26 : 18)), cursor: 'pointer', - color: '#1890ff' + color: '#1890ff', + fontSize: Math.max(12, 14 - level * 0.3), // Slightly smaller font for deeper levels + opacity: Math.max(0.85, 1 - level * 0.03), // Slightly faded for deeper levels + fontWeight: level > 0 ? 400 : 500 // Slightly lighter weight for nested tasks }} onClick={(e) => { e.stopPropagation(); @@ -251,7 +333,7 @@ const FinanceTable = ({ > {task.name} - {task.billable && } + {task.billable && } ); @@ -277,11 +359,11 @@ const FinanceTable = ({
); case FinanceTableColumnKeys.HOURS: - return {task.estimated_hours}; + return {task.estimated_hours}; case FinanceTableColumnKeys.TOTAL_TIME_LOGGED: - return {task.total_time_logged}; + return {task.total_time_logged}; case FinanceTableColumnKeys.ESTIMATED_COST: - return {formatNumber(task.estimated_cost)}; + return {formatNumber(task.estimated_cost)}; case FinanceTableColumnKeys.FIXED_COST: return selectedTask?.id === task.id && hasEditPermission ? ( `${value}`.replace(/\B(?=(\d{3})+(?!\d))/g, ',')} parser={(value) => Number(value!.replace(/\$\s?|(,*)/g, ''))} min={0} @@ -313,7 +395,8 @@ const FinanceTable = ({ cursor: hasEditPermission ? 'pointer' : 'default', width: '100%', display: 'block', - opacity: hasEditPermission ? 1 : 0.7 + opacity: hasEditPermission ? 1 : 0.7, + fontSize: Math.max(12, 14 - level * 0.5) }} onClick={hasEditPermission ? (e) => { e.stopPropagation(); @@ -328,7 +411,8 @@ const FinanceTable = ({ return ( 0 ? '#FF0000' : '#6DC376' + color: task.variance > 0 ? '#FF0000' : '#6DC376', + fontSize: Math.max(12, 14 - level * 0.5) }} > {task.variance < 0 ? '+' + formatNumber(Math.abs(task.variance)) : @@ -337,11 +421,11 @@ const FinanceTable = ({ ); case FinanceTableColumnKeys.TOTAL_BUDGET: - return {formatNumber(task.total_budget)}; + return {formatNumber(task.total_budget)}; case FinanceTableColumnKeys.TOTAL_ACTUAL: - return {formatNumber(task.total_actual)}; + return {formatNumber(task.total_actual)}; case FinanceTableColumnKeys.COST: - return {formatNumber((task.total_actual || 0) - (task.fixed_cost || 0))}; + return {formatNumber((task.total_actual || 0) - (task.fixed_cost || 0))}; default: return null; } @@ -363,32 +447,29 @@ const FinanceTable = ({ return parts.join(' '); }; - // Flatten tasks to include subtasks for rendering + // Generate flattened task list with all nested levels const flattenedTasks = useMemo(() => { - const flattened: IProjectFinanceTask[] = []; + const flattened: React.ReactElement[] = []; tasks.forEach(task => { - // Add the parent task - flattened.push(task); - - // Add subtasks if they are expanded and loaded - if (task.show_sub_tasks && task.sub_tasks) { - task.sub_tasks.forEach(subTask => { - flattened.push(subTask); - }); - } + flattened.push(...renderTaskHierarchy(task, 0)); }); return flattened; - }, [tasks]); + }, [tasks, selectedTask, editingFixedCostValue, hasEditPermission, themeMode, hoveredTaskId]); - // Calculate totals for the current table (only count parent tasks to avoid double counting) + // Calculate totals for the current table + // Since the backend already aggregates subtask values into parent tasks, + // we only need to sum the parent tasks (tasks without is_sub_task flag) const totals = useMemo(() => { return tasks.reduce( (acc, task) => { // Calculate actual cost from logs (total_actual - fixed_cost) const actualCostFromLogs = (task.total_actual || 0) - (task.fixed_cost || 0); + // The backend already handles aggregation for parent tasks with subtasks + // Parent tasks contain the sum of their subtasks' values + // So we can safely sum all parent tasks (which are the tasks in this array) return { hours: acc.hours + (task.estimated_seconds || 0), total_time_logged: acc.total_time_logged + (task.total_time_logged_seconds || 0), @@ -480,43 +561,8 @@ const FinanceTable = ({ )} - {/* task rows */} - {!isCollapse && flattenedTasks.map((task, idx) => ( - e.currentTarget.style.background = themeWiseColor('#f0f0f0', '#333', themeMode)} - onMouseLeave={e => e.currentTarget.style.background = idx % 2 === 0 ? themeWiseColor('#fafafa', '#232323', themeMode) : themeWiseColor('#ffffff', '#181818', themeMode)} - > - {financeTableColumns.map((col) => ( - e.stopPropagation() - : undefined - } - > - {renderFinancialTableColumnContent(col.key, task)} - - ))} - - ))} + {/* task rows with recursive hierarchy */} + {!isCollapse && flattenedTasks} ); };