From c5bac36c533f33a1e7a78a0e2f30e4e369f6f54c Mon Sep 17 00:00:00 2001 From: chamikaJ Date: Wed, 11 Jun 2025 12:28:25 +0530 Subject: [PATCH] feat(project-finance): enhance task cost tracking and UI updates - Added `actual_cost_from_logs` to task data structure for improved cost tracking. - Updated SQL queries in ProjectFinanceController to ensure accurate fixed cost updates and task hierarchy recalculations. - Enhanced the project finance slice to optimize task hierarchy recalculations, ensuring accurate financial data representation. - Modified FinanceTable component to reflect changes in cost calculations, preventing double counting and improving UI responsiveness. --- .../controllers/project-finance-controller.ts | 13 +- .../projects/finance/project-finance.slice.ts | 108 +++++++++------- .../finance-table/finance-table-wrapper.tsx | 4 +- .../finance-table/finance-table.tsx | 122 +++++++++--------- .../types/project/project-finance.types.ts | 1 + 5 files changed, 135 insertions(+), 113 deletions(-) diff --git a/worklenz-backend/src/controllers/project-finance-controller.ts b/worklenz-backend/src/controllers/project-finance-controller.ts index b5cfda45..4199078a 100644 --- a/worklenz-backend/src/controllers/project-finance-controller.ts +++ b/worklenz-backend/src/controllers/project-finance-controller.ts @@ -360,6 +360,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { Number(task.total_time_logged_seconds) || 0 ), estimated_cost: Number(task.estimated_cost) || 0, + actual_cost_from_logs: Number(task.actual_cost_from_logs) || 0, fixed_cost: Number(task.fixed_cost) || 0, total_budget: Number(task.total_budget) || 0, total_actual: Number(task.total_actual) || 0, @@ -426,14 +427,15 @@ export default class ProjectfinanceController extends WorklenzControllerBase { .send(new ServerResponse(false, null, "Cannot update fixed cost for parent tasks. Fixed cost is calculated from subtasks.")); } - const q = ` + // Update only the specific subtask's fixed cost + const updateQuery = ` UPDATE tasks SET fixed_cost = $1, updated_at = NOW() WHERE id = $2 RETURNING id, name, fixed_cost; `; - const result = await db.query(q, [fixed_cost, taskId]); + const result = await db.query(updateQuery, [fixed_cost, taskId]); if (result.rows.length === 0) { return res @@ -441,7 +443,10 @@ export default class ProjectfinanceController extends WorklenzControllerBase { .send(new ServerResponse(false, null, "Task not found")); } - return res.status(200).send(new ServerResponse(true, result.rows[0])); + return res.status(200).send(new ServerResponse(true, { + updated_task: result.rows[0], + message: "Fixed cost updated successfully." + })); } @HandleExceptions() @@ -839,6 +844,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { Number(task.total_time_logged_seconds) || 0 ), estimated_cost: Number(task.estimated_cost) || 0, + actual_cost_from_logs: Number(task.actual_cost_from_logs) || 0, fixed_cost: Number(task.fixed_cost) || 0, total_budget: Number(task.total_budget) || 0, total_actual: Number(task.total_actual) || 0, @@ -1161,6 +1167,7 @@ export default class ProjectfinanceController extends WorklenzControllerBase { Number(task.total_time_logged_seconds) || 0 ), estimated_cost: Number(task.estimated_cost) || 0, + actual_cost_from_logs: Number(task.actual_cost_from_logs) || 0, fixed_cost: Number(task.fixed_cost) || 0, total_budget: Number(task.total_budget) || 0, total_actual: Number(task.total_actual) || 0, 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 fdb57197..564379df 100644 --- a/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts +++ b/worklenz-frontend/src/features/projects/finance/project-finance.slice.ts @@ -78,52 +78,65 @@ const hasTaskChanged = (oldTask: IProjectFinanceTask, newTask: IProjectFinanceTa // Optimized recursive calculation for task hierarchy with memoization const recalculateTaskHierarchy = (tasks: IProjectFinanceTask[]): IProjectFinanceTask[] => { return tasks.map(task => { - const cacheKey = generateTaskCacheKey(task); - const cached = taskCalculationCache.get(cacheKey); - - // If task has subtasks, first recalculate all subtasks recursively + // If task has loaded subtasks, recalculate from subtasks if (task.sub_tasks && task.sub_tasks.length > 0) { const updatedSubTasks = recalculateTaskHierarchy(task.sub_tasks); - // Calculate parent task totals from subtasks + // Calculate totals from subtasks only (for time and costs from logs) const subtaskTotals = updatedSubTasks.reduce((acc, subtask) => ({ estimated_cost: acc.estimated_cost + (subtask.estimated_cost || 0), fixed_cost: acc.fixed_cost + (subtask.fixed_cost || 0), - total_actual: acc.total_actual + (subtask.total_actual || 0), + actual_cost_from_logs: acc.actual_cost_from_logs + (subtask.actual_cost_from_logs || 0), estimated_seconds: acc.estimated_seconds + (subtask.estimated_seconds || 0), total_time_logged_seconds: acc.total_time_logged_seconds + (subtask.total_time_logged_seconds || 0) }), { estimated_cost: 0, fixed_cost: 0, - total_actual: 0, + actual_cost_from_logs: 0, estimated_seconds: 0, total_time_logged_seconds: 0 }); + // For parent tasks with loaded subtasks: use ONLY the subtask totals + // The parent's original values were backend-aggregated, now we use frontend subtask aggregation + const totalFixedCost = subtaskTotals.fixed_cost; // Only subtask fixed costs + const totalEstimatedCost = subtaskTotals.estimated_cost; // Only subtask estimated costs + const totalActualCostFromLogs = subtaskTotals.actual_cost_from_logs; // Only subtask logged costs + const totalActual = totalActualCostFromLogs + totalFixedCost; + // Update parent task with aggregated values const updatedTask = { ...task, sub_tasks: updatedSubTasks, - estimated_cost: subtaskTotals.estimated_cost, - fixed_cost: subtaskTotals.fixed_cost, - total_actual: subtaskTotals.total_actual, + estimated_cost: totalEstimatedCost, + fixed_cost: totalFixedCost, + actual_cost_from_logs: totalActualCostFromLogs, + total_actual: totalActual, estimated_seconds: subtaskTotals.estimated_seconds, total_time_logged_seconds: subtaskTotals.total_time_logged_seconds, - total_budget: subtaskTotals.estimated_cost + subtaskTotals.fixed_cost, - variance: subtaskTotals.total_actual - (subtaskTotals.estimated_cost + subtaskTotals.fixed_cost) + total_budget: totalEstimatedCost + totalFixedCost, + variance: totalActual - (totalEstimatedCost + totalFixedCost) }; - // Cache the result - taskCalculationCache.set(cacheKey, { - task: { ...task }, - result: updatedTask, - timestamp: Date.now() - }); - return updatedTask; } + // For parent tasks without loaded subtasks, trust backend-calculated values + if (task.sub_tasks_count > 0 && (!task.sub_tasks || task.sub_tasks.length === 0)) { + // Parent task with unloaded subtasks - backend has already calculated aggregated values + const { totalBudget, totalActual, variance } = calculateTaskCosts(task); + return { + ...task, + total_budget: totalBudget, + total_actual: totalActual, + variance: variance + }; + } + // For leaf tasks, check cache first + const cacheKey = generateTaskCacheKey(task); + const cached = taskCalculationCache.get(cacheKey); + if (cached && !hasTaskChanged(cached.task, task)) { return { ...cached.result, ...task }; // Merge with current task to preserve other properties } @@ -137,7 +150,7 @@ const recalculateTaskHierarchy = (tasks: IProjectFinanceTask[]): IProjectFinance variance: variance }; - // Cache the result + // Cache the result only for leaf tasks taskCalculationCache.set(cacheKey, { task: { ...task }, result: updatedTask, @@ -340,7 +353,12 @@ export const projectFinancesSlice = createSlice({ }) .addCase(fetchProjectFinances.fulfilled, (state, action) => { state.loading = false; - state.taskGroups = action.payload.groups; + // Apply hierarchy recalculation to ensure parent tasks show correct aggregated values + const recalculatedGroups = action.payload.groups.map(group => ({ + ...group, + tasks: recalculateTaskHierarchy(group.tasks) + })); + state.taskGroups = recalculatedGroups; state.projectRateCards = action.payload.project_rate_cards; state.project = action.payload.project; // Clear cache when fresh data is loaded @@ -369,16 +387,20 @@ export const projectFinancesSlice = createSlice({ }); }; - // Update groups while preserving expansion state + // Update groups while preserving expansion state and applying hierarchy recalculation const updatedTaskGroups = action.payload.groups.map(newGroup => { const existingGroup = state.taskGroups.find(g => g.group_id === newGroup.group_id); if (existingGroup) { + const tasksWithExpansion = preserveExpansionState(existingGroup.tasks, newGroup.tasks); return { ...newGroup, - tasks: preserveExpansionState(existingGroup.tasks, newGroup.tasks) + tasks: recalculateTaskHierarchy(tasksWithExpansion) }; } - return newGroup; + return { + ...newGroup, + tasks: recalculateTaskHierarchy(newGroup.tasks) + }; }); // Update data without changing loading state for silent refresh @@ -393,30 +415,20 @@ export const projectFinancesSlice = createSlice({ const group = state.taskGroups.find(g => g.group_id === groupId); if (group) { - // 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; - // Recalculate financial values immediately for UI responsiveness - const totalBudget = (task.estimated_cost || 0) + fixedCost; - const totalActual = task.total_actual || 0; - const variance = totalActual - totalBudget; - - task.total_budget = totalBudget; - task.variance = variance; - return true; - } - - // Search in subtasks recursively - if (task.sub_tasks && findAndUpdateTask(task.sub_tasks, targetId)) { - return true; - } - } - return false; - }; + // Update the specific task's fixed cost and recalculate the entire hierarchy + const result = updateTaskAndRecalculateHierarchy( + group.tasks, + taskId, + (task) => ({ + ...task, + fixed_cost: fixedCost + }) + ); - findAndUpdateTask(group.tasks, taskId); + if (result.updated) { + group.tasks = result.tasks; + clearCalculationCache(); + } } }) .addCase(fetchSubTasks.fulfilled, (state, action) => { @@ -447,6 +459,8 @@ export const projectFinancesSlice = createSlice({ // Find the parent task in any group and add the subtasks for (const group of state.taskGroups) { if (findAndUpdateTask(group.tasks, parentTaskId)) { + // Recalculate the hierarchy after adding subtasks to ensure parent values are correct + group.tasks = recalculateTaskHierarchy(group.tasks); break; } } diff --git a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table-wrapper.tsx b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table-wrapper.tsx index 475a761b..0fc13c13 100644 --- a/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table-wrapper.tsx +++ b/worklenz-frontend/src/pages/projects/projectView/finance/finance-tab/finance-table/finance-table-wrapper.tsx @@ -73,7 +73,7 @@ const FinanceTableWrapper: React.FC = ({ activeTablesL // Parent task - use its aggregated values which already include subtask totals return { hours: acc.hours + (task.estimated_seconds || 0), - cost: acc.cost + ((task.total_actual || 0) - (task.fixed_cost || 0)), + cost: acc.cost + (task.actual_cost_from_logs || 0), fixedCost: acc.fixedCost + (task.fixed_cost || 0), totalBudget: acc.totalBudget + (task.total_budget || 0), totalActual: acc.totalActual + (task.total_actual || 0), @@ -85,7 +85,7 @@ const FinanceTableWrapper: React.FC = ({ activeTablesL // Leaf task - use its individual values return { hours: acc.hours + (task.estimated_seconds || 0), - cost: acc.cost + ((task.total_actual || 0) - (task.fixed_cost || 0)), + cost: acc.cost + (task.actual_cost_from_logs || 0), fixedCost: acc.fixedCost + (task.fixed_cost || 0), totalBudget: acc.totalBudget + (task.total_budget || 0), totalActual: acc.totalActual + (task.total_actual || 0), 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 67098181..46ecd9b1 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 @@ -48,8 +48,6 @@ const FinanceTable = ({ // Get the latest task groups from Redux store const taskGroups = useAppSelector((state) => state.projectFinances.taskGroups); - const activeGroup = useAppSelector((state) => state.projectFinances.activeGroup); - const billableFilter = useAppSelector((state) => state.projectFinances.billableFilter); // Auth and permissions const auth = useAuthService(); @@ -71,7 +69,7 @@ const FinanceTable = ({ useEffect(() => { const handleClickOutside = (event: MouseEvent) => { if (selectedTask && !(event.target as Element)?.closest('.fixed-cost-input')) { - // Save current value before closing + // Save current value before closing if it has changed if (editingFixedCostValue !== null) { immediateSaveFixedCost(editingFixedCostValue, selectedTask.id); } else { @@ -85,7 +83,7 @@ const FinanceTable = ({ return () => { document.removeEventListener('mousedown', handleClickOutside); }; - }, [selectedTask, editingFixedCostValue]); + }, [selectedTask, editingFixedCostValue, tasks]); // Cleanup timeout on unmount useEffect(() => { @@ -169,9 +167,11 @@ const FinanceTable = ({ try { // Update the task fixed cost - this will automatically trigger hierarchical recalculation + // The Redux slice handles parent task updates through recalculateTaskHierarchy await dispatch(updateTaskFixedCostAsync({ taskId, groupId: table.group_id, fixedCost })).unwrap(); - // No need for manual parent calculations or API refetch - the Redux slice handles it efficiently + setSelectedTask(null); + setEditingFixedCostValue(null); } catch (error) { console.error('Failed to update fixed cost:', error); } @@ -211,7 +211,24 @@ const FinanceTable = ({ // Set new timeout saveTimeoutRef.current = setTimeout(() => { - if (value !== null) { + // Find the current task to check if value actually changed + const findTask = (tasks: IProjectFinanceTask[], id: string): IProjectFinanceTask | null => { + for (const task of tasks) { + if (task.id === id) return task; + if (task.sub_tasks) { + const found = findTask(task.sub_tasks, id); + if (found) return found; + } + } + return null; + }; + + const currentTask = findTask(tasks, taskId); + const currentFixedCost = currentTask?.fixed_cost || 0; + const newFixedCost = value || 0; + + // Only save if the value actually changed + if (newFixedCost !== currentFixedCost && value !== null) { handleFixedCostChange(value, taskId); setSelectedTask(null); setEditingFixedCostValue(null); @@ -227,11 +244,30 @@ const FinanceTable = ({ saveTimeoutRef.current = null; } - if (value !== null) { + // Find the current task to check if value actually changed + const findTask = (tasks: IProjectFinanceTask[], id: string): IProjectFinanceTask | null => { + for (const task of tasks) { + if (task.id === id) return task; + if (task.sub_tasks) { + const found = findTask(task.sub_tasks, id); + if (found) return found; + } + } + return null; + }; + + const currentTask = findTask(tasks, taskId); + const currentFixedCost = currentTask?.fixed_cost || 0; + const newFixedCost = value || 0; + + // Only save if the value actually changed + if (newFixedCost !== currentFixedCost && value !== null) { handleFixedCostChange(value, taskId); + } else { + // Just close the editor without saving + setSelectedTask(null); + setEditingFixedCostValue(null); } - setSelectedTask(null); - setEditingFixedCostValue(null); }; // Calculate indentation based on nesting level @@ -453,7 +489,7 @@ const FinanceTable = ({ case FinanceTableColumnKeys.TOTAL_ACTUAL: return {formatNumber(task.total_actual)}; case FinanceTableColumnKeys.COST: - return {formatNumber((task.total_actual || 0) - (task.fixed_cost || 0))}; + return {formatNumber(task.actual_cost_from_logs || 0)}; default: return null; } @@ -489,7 +525,7 @@ const FinanceTable = ({ // Calculate totals for the current table // Optimized calculation that avoids double counting in nested hierarchies const totals = useMemo(() => { - const calculateTaskTotalsFlat = (taskList: IProjectFinanceTask[]): any => { + const calculateTaskTotalsRecursive = (taskList: IProjectFinanceTask[]): any => { let totals = { hours: 0, total_time_logged: 0, @@ -502,24 +538,24 @@ const FinanceTable = ({ }; for (const task of taskList) { - // For parent tasks with subtasks, only count the aggregated values (no double counting) - // For leaf tasks, count their individual values if (task.sub_tasks && task.sub_tasks.length > 0) { - // Parent task - use its aggregated values which already include subtask totals - totals.hours += task.estimated_seconds || 0; - totals.total_time_logged += task.total_time_logged_seconds || 0; - totals.estimated_cost += task.estimated_cost || 0; - totals.actual_cost_from_logs += (task.total_actual || 0) - (task.fixed_cost || 0); - totals.fixed_cost += task.fixed_cost || 0; - totals.total_budget += task.total_budget || 0; - totals.total_actual += task.total_actual || 0; - totals.variance += task.variance || 0; + // Parent task with loaded subtasks - only count the subtasks recursively + // This completely avoids the parent's aggregated values to prevent double counting + const subtaskTotals = calculateTaskTotalsRecursive(task.sub_tasks); + totals.hours += subtaskTotals.hours; + totals.total_time_logged += subtaskTotals.total_time_logged; + totals.estimated_cost += subtaskTotals.estimated_cost; + totals.actual_cost_from_logs += subtaskTotals.actual_cost_from_logs; + totals.fixed_cost += subtaskTotals.fixed_cost; + totals.total_budget += subtaskTotals.total_budget; + totals.total_actual += subtaskTotals.total_actual; + totals.variance += subtaskTotals.variance; } else { - // Leaf task - use its individual values + // Leaf task or parent task without loaded subtasks - use its values directly totals.hours += task.estimated_seconds || 0; totals.total_time_logged += task.total_time_logged_seconds || 0; totals.estimated_cost += task.estimated_cost || 0; - totals.actual_cost_from_logs += (task.total_actual || 0) - (task.fixed_cost || 0); + totals.actual_cost_from_logs += task.actual_cost_from_logs || 0; totals.fixed_cost += task.fixed_cost || 0; totals.total_budget += task.total_budget || 0; totals.total_actual += task.total_actual || 0; @@ -530,7 +566,7 @@ const FinanceTable = ({ return totals; }; - return calculateTaskTotalsFlat(tasks); + return calculateTaskTotalsRecursive(tasks); }, [tasks]); // Format the totals for display @@ -602,42 +638,6 @@ const FinanceTable = ({ {/* task rows with recursive hierarchy */} {!isCollapse && flattenedTasks} - - {/* Group totals row */} - {!isCollapse && tasks.length > 0 && ( - - {financeTableColumns.map((col) => ( - - {col.key === FinanceTableColumnKeys.TASK ? ( - - Group Total - - ) : col.key === FinanceTableColumnKeys.MEMBERS ? null : ( - renderFinancialTableHeaderContent(col.key) - )} - - ))} - - )} ); }; diff --git a/worklenz-frontend/src/types/project/project-finance.types.ts b/worklenz-frontend/src/types/project/project-finance.types.ts index f52fea18..554c2465 100644 --- a/worklenz-frontend/src/types/project/project-finance.types.ts +++ b/worklenz-frontend/src/types/project/project-finance.types.ts @@ -34,6 +34,7 @@ export interface IProjectFinanceTask { total_time_logged_seconds: number; total_time_logged: string; // Formatted time string like "4h 30m 12s" estimated_cost: number; + actual_cost_from_logs: number; members: IProjectFinanceMember[]; billable: boolean; fixed_cost: number;