From 7e15c517acec3ba91bcd6aa1b0b95f2ac5d812f3 Mon Sep 17 00:00:00 2001 From: "ashutosh.nehete" Date: Fri, 3 Oct 2025 10:54:38 +0530 Subject: [PATCH] Optimized the dashboard APIs for expenses --- .../Controllers/DashboardController.cs | 314 +++++++++++++----- 1 file changed, 224 insertions(+), 90 deletions(-) diff --git a/Marco.Pms.Services/Controllers/DashboardController.cs b/Marco.Pms.Services/Controllers/DashboardController.cs index f44cdfc..38a7499 100644 --- a/Marco.Pms.Services/Controllers/DashboardController.cs +++ b/Marco.Pms.Services/Controllers/DashboardController.cs @@ -1,13 +1,12 @@ -using AutoMapper; -using Marco.Pms.DataAccess.Data; +using Marco.Pms.DataAccess.Data; using Marco.Pms.Model.Activities; using Marco.Pms.Model.Dtos.Attendance; using Marco.Pms.Model.Employees; using Marco.Pms.Model.Entitlements; +using Marco.Pms.Model.Expenses; using Marco.Pms.Model.Projects; using Marco.Pms.Model.Utilities; using Marco.Pms.Model.ViewModels.DashBoard; -using Marco.Pms.Model.ViewModels.Expanses; using Marco.Pms.Services.Service; using Marco.Pms.Services.Service.ServiceInterfaces; using MarcoBMS.Services.Helpers; @@ -616,121 +615,217 @@ namespace Marco.Pms.Services.Controllers } [HttpGet("expense/project")] - public async Task GetExpenseReportByProjects([FromQuery] DateTime? startDate, [FromQuery] DateTime? endDate) + public async Task GetExpenseReportByProjectsAsync([FromQuery] DateTime startDate, [FromQuery] DateTime endDate) { - var expensesQuery = _context.Expenses - .Include(e => e.Project) - .Where(e => e.TenantId == tenantId && e.IsActive && e.StatusId != Draft && e.Project != null); + // Structured start log + _logger.LogInfo( + "GetExpenseReportByProjects started. TenantId={TenantId}, StartDate={StartDate}, EndDate={EndDate}", + tenantId, startDate, endDate); // [Start Log] [memory:4][memory:1] - if (startDate.HasValue && endDate.HasValue) + // Guard: validate range and normalize to inclusive end-of-day + if (endDate < startDate) { - expensesQuery = expensesQuery.Where(e => e.TransactionDate.Date >= startDate.Value.Date && e.TransactionDate.Date <= endDate.Value.Date); + _logger.LogWarning("Invalid date range. StartDate={StartDate}, EndDate={EndDate}", startDate, endDate); // [Validation Log] [memory:4] + return BadRequest(ApiResponse.ErrorResponse("endDate must be on or after startDate.", 400)); // [Validation Response] [memory:1] } - var expenses = await expensesQuery.GroupBy(e => e.Project).ToListAsync(); + var start = startDate.Date; + var end = endDate.Date.AddDays(1).AddTicks(-1); // inclusive EOD [memory:7] - var report = expenses.Select(g => + try { - var totalAmount = g.Sum(e => e.Amount); - var totalPendingAmount = g.Where(e => e.StatusId != Processed && e.StatusId != RejectedByReviewer && e.StatusId != RejectedByApprover).Sum(e => e.Amount); - var totalRejectedAmount = g.Where(e => e.StatusId == RejectedByReviewer || e.StatusId == RejectedByApprover).Sum(e => e.Amount); - var totalProcessedAmount = g.Where(e => e.StatusId == Processed || e.StatusId == ProcessPending).Sum(e => e.Amount); - return new + // Read-only base filter with tenant scope and non-draft + var baseQuery = _context.Expenses + .AsNoTracking() + .Where(e => + e.TenantId == tenantId + && e.IsActive + && e.StatusId != Draft + && e.Project != null + && e.TransactionDate >= start + && e.TransactionDate <= end); // [Server Filters] [memory:7] + + // Single server-side group/aggregate by project + var report = await baseQuery + .GroupBy(e => e.Project) + .Select(g => new + { + ProjectName = g.Key!.Name, + TotalApprovedAmount = g.Where(x => x.StatusId == Processed || x.StatusId == ProcessPending) + .Sum(x => x.Amount), + TotalPendingAmount = g.Where(x => x.StatusId != Processed + && x.StatusId != RejectedByReviewer + && x.StatusId != RejectedByApprover) + .Sum(x => x.Amount), + TotalRejectedAmount = g.Where(x => x.StatusId == RejectedByReviewer + || x.StatusId == RejectedByApprover) + .Sum(x => x.Amount), + TotalProcessedAmount = g.Where(x => x.StatusId == Processed) + .Sum(x => x.Amount) + }) + .OrderBy(r => r.ProjectName) + .ToListAsync(); // [Single Round-trip] [memory:7] + + var response = new { - ProjectName = g.Key!.Name, - TotalPendingAmount = totalPendingAmount, - TotalRejectedAmount = totalRejectedAmount, - TotalProcessedAmount = totalProcessedAmount, - TotalApprovedAmount = totalAmount + Report = report, + TotalAmount = report.Sum(r => r.TotalApprovedAmount) }; - }).OrderBy(r => r.ProjectName).ToList(); - var response = new + _logger.LogInfo( + "GetExpenseReportByProjects completed. TenantId={TenantId}, Rows={Rows}, TotalAmount={TotalAmount}", + tenantId, report.Count, response.TotalAmount); // [Completion Log] [memory:4] + + return Ok(ApiResponse.SuccessResponse(response, "Expense report by project fetched successfully", 200)); // [Success Response] [memory:1] + } + catch (OperationCanceledException) { - Report = report, - TotalAmount = report.Sum(r => r.TotalApprovedAmount) - }; - - return Ok(ApiResponse.SuccessResponse(response, "Expense report by project fetched successfully", 200)); + _logger.LogWarning("GetExpenseReportByProjects canceled by client. TenantId={TenantId}", tenantId); // [Cancel Log] [memory:4] + return StatusCode(499, ApiResponse.ErrorResponse("Client has canceled the opration", "Client has canceled the opration", 499)); // [Cancel Response] [memory:1] + } + catch (Exception ex) + { + _logger.LogError(ex, + "GetExpenseReportByProjects failed. TenantId={TenantId}, StartDate={StartDate}, EndDate={EndDate}", + tenantId, start, end); // [Error Log] [memory:4] + return StatusCode(500, + ApiResponse.ErrorResponse("An error occurred while fetching the expense report.", 500)); // [Error Response] [memory:1] + } } [HttpGet("expense/type")] - public async Task GetExpenseReportByExpenseType([FromQuery] Guid? projectId, [FromQuery] DateTime? startDate, [FromQuery] DateTime? endDate) + public async Task GetExpenseReportByExpenseTypeAsync([FromQuery] Guid? projectId, [FromQuery] DateTime startDate, [FromQuery] DateTime endDate) { - var expensesQuery = _context.Expenses - .Include(e => e.Project) - .Where(e => e.TenantId == tenantId && e.IsActive && e.StatusId != Draft && e.Project != null); + // Structured log: entering action with filters + _logger.LogDebug( + "GetExpenseReportByExpenseType started. TenantId={TenantId}, ProjectId={ProjectId}, StartDate={StartDate}, EndDate={EndDate}", + tenantId, projectId ?? Guid.Empty, startDate, endDate); // [Start Log] [memory:4][memory:1] - if (projectId.HasValue) + + try { - expensesQuery = expensesQuery.Where(e => e.ProjectId == projectId); - } + // Compose base query: push filters to DB, avoid client evaluation + IQueryable baseQuery = _context.Expenses + .AsNoTracking() // Reduce tracking overhead for read-only endpoint + .Where(e => e.TenantId == tenantId + && e.IsActive + && e.StatusId != Draft + && e.TransactionDate >= startDate + && e.TransactionDate <= endDate.AddDays(1).AddTicks(-1)); - if (startDate.HasValue && endDate.HasValue) - { - expensesQuery = expensesQuery.Where(e => e.TransactionDate.Date >= startDate.Value.Date && e.TransactionDate.Date <= endDate.Value.Date); - } + if (projectId.HasValue) + baseQuery = baseQuery.Where(e => e.ProjectId == projectId.Value); // [Filter] [memory:7] - var expenses = await expensesQuery.GroupBy(e => e.ExpensesType).ToListAsync(); + // Project to a minimal shape before grouping to avoid loading navigation graphs + // Group by expense type name; adjust to the correct key if ExpensesType is an enum or navigation + var query = baseQuery + .Where(e => e.ExpensesType != null) + .Select(e => new + { + ExpenseTypeName = e.ExpensesType!.Name, // If enum, use e.ExpensesType.ToString() + Amount = e.Amount, + StatusId = e.StatusId + }) + .GroupBy(x => x.ExpenseTypeName) + .Select(g => new + { + ProjectName = g.Key, // Original code used g.Key!.Name; here the grouping key is already a string + TotalApprovedAmount = g.Where(x => x.StatusId == Processed + || x.StatusId == ProcessPending).Sum(x => x.Amount), + TotalPendingAmount = g.Where(x => x.StatusId != Processed + && x.StatusId != RejectedByReviewer + && x.StatusId != RejectedByApprover) + .Sum(x => x.Amount), + TotalRejectedAmount = g.Where(x => x.StatusId == RejectedByReviewer + || x.StatusId == RejectedByApprover) + .Sum(x => x.Amount), + TotalProcessedAmount = g.Where(x => x.StatusId == Processed) + .Sum(x => x.Amount) + }) + .OrderBy(r => r.ProjectName); // Server-side order [memory:7] - var report = expenses.Select(g => - { - var totalAmount = g.Sum(e => e.Amount); - var totalPendingAmount = g.Where(e => e.StatusId != Processed && e.StatusId != RejectedByReviewer && e.StatusId != RejectedByApprover).Sum(e => e.Amount); - var totalRejectedAmount = g.Where(e => e.StatusId == RejectedByReviewer || e.StatusId == RejectedByApprover).Sum(e => e.Amount); - var totalProcessedAmount = g.Where(e => e.StatusId == Processed || e.StatusId == ProcessPending).Sum(e => e.Amount); - return new + var report = await query.ToListAsync(); // Single round-trip [memory:7] + + var response = new { - ProjectName = g.Key!.Name, - TotalPendingAmount = totalPendingAmount, - TotalRejectedAmount = totalRejectedAmount, - TotalProcessedAmount = totalProcessedAmount, - TotalApprovedAmount = totalAmount + Report = report, + TotalAmount = report.Sum(r => r.TotalApprovedAmount) }; - }).OrderBy(r => r.ProjectName).ToList(); - var response = new + _logger.LogInfo( + "GetExpenseReportByExpenseType completed. TenantId={TenantId}, Filters: ProjectId={ProjectId}, StartDate={StartDate}, EndDate={EndDate}, Rows={RowCount}, TotalAmount={TotalAmount}", + tenantId, projectId ?? Guid.Empty, startDate, endDate, report.Count, response.TotalAmount); // [Completion Log] [memory:4] + + return Ok(ApiResponse.SuccessResponse(response, "Expense report by expense type fetched successfully", 200)); // [Success Response] [memory:1] + } + catch (OperationCanceledException) { - Report = report, - TotalAmount = report.Sum(r => r.TotalApprovedAmount) - }; - - return Ok(ApiResponse.SuccessResponse(response, "Expense report by expense type fetched successfully", 200)); + _logger.LogWarning("GetExpenseReportByExpenseType canceled by client. TenantId={TenantId}", tenantId); // [Cancel Log] [memory:4] + return StatusCode(499, ApiResponse.ErrorResponse("Client has canceled the opration", "Client has canceled the opration", 499)); // [Cancel Response] [memory:1] + } + catch (Exception ex) + { + _logger.LogError(ex, + "GetExpenseReportByExpenseType failed. TenantId={TenantId}, ProjectId={ProjectId}, StartDate={StartDate}, EndDate={EndDate}", + tenantId, projectId ?? Guid.Empty, startDate, endDate); // [Error Log] [memory:4] + return StatusCode(StatusCodes.Status500InternalServerError, + ApiResponse.ErrorResponse("An error occurred while fetching the expense report.", 500)); // [Error Response] [memory:1] + } } [HttpGet("expense/pendings")] public async Task GetPendingExpenseListAsync() { - var loggedInEmployee = await _userHelper.GetCurrentEmployeeAsync(); + // Start log with correlation fields + _logger.LogDebug( + "GetPendingExpenseListAsync started. TenantId={TenantId}", tenantId); // [Start Log] [memory:4][memory:1] - using var scope = _serviceScopeFactory.CreateScope(); - var _mapper = scope.ServiceProvider.GetRequiredService(); - - var hasReviewPermissionTask = Task.Run(async () => + try { - var _permission = scope.ServiceProvider.GetRequiredService(); - return await _permission.HasPermission(PermissionsMaster.ExpenseReview, loggedInEmployee.Id); - }); + // Resolve current employee once; avoid using scoped services inside Task.Run + var loggedInEmployee = await _userHelper.GetCurrentEmployeeAsync(); // [User Context] [memory:1] - var hasApprovePermissionTask = Task.Run(async () => - { - var _permission = scope.ServiceProvider.GetRequiredService(); - return await _permission.HasPermission(PermissionsMaster.ExpenseApprove, loggedInEmployee.Id); - }); + // Resolve permission service from current scope once + using var scope = _serviceScopeFactory.CreateScope(); - var hasProcessPermissionTask = Task.Run(async () => - { - var _permission = scope.ServiceProvider.GetRequiredService(); - return await _permission.HasPermission(PermissionsMaster.ExpenseProcess, loggedInEmployee.Id); - }); + // Fire permission checks concurrently without Task.Run; these are async I/O methods - await Task.WhenAll(hasReviewPermissionTask, hasApprovePermissionTask, hasProcessPermissionTask); + var hasReviewPermissionTask = Task.Run(async () => + { + var _permission = scope.ServiceProvider.GetRequiredService(); + return await _permission.HasPermission(PermissionsMaster.ExpenseReview, loggedInEmployee.Id); + }); - var hasReviewPermission = hasReviewPermissionTask.Result; - var hasApprovePermission = hasApprovePermissionTask.Result; - var hasProcessPermission = hasProcessPermissionTask.Result; + var hasApprovePermissionTask = Task.Run(async () => + { + var _permission = scope.ServiceProvider.GetRequiredService(); + return await _permission.HasPermission(PermissionsMaster.ExpenseApprove, loggedInEmployee.Id); + }); - var expenses = await _context.Expenses + var hasProcessPermissionTask = Task.Run(async () => + { + var _permission = scope.ServiceProvider.GetRequiredService(); + return await _permission.HasPermission(PermissionsMaster.ExpenseProcess, loggedInEmployee.Id); + }); + + await Task.WhenAll(hasReviewPermissionTask, hasApprovePermissionTask, hasProcessPermissionTask); // [Parallel Await] + + var hasReviewPermission = hasReviewPermissionTask.Result; + var hasApprovePermission = hasApprovePermissionTask.Result; + var hasProcessPermission = hasProcessPermissionTask.Result; + + _logger.LogInfo( + "Permissions resolved: Review={Review}, Approve={Approve}, Process={Process}", + hasReviewPermission, hasApprovePermission, hasProcessPermission); // [Permissions Log] [memory:4] + + // Build base query: read-only, tenant-scoped + var baseQuery = _context.Expenses + .AsNoTracking() // Reduce tracking overhead for read-only list + .Where(e => e.IsActive && e.TenantId == tenantId); // [Base Filter] [memory:7] + + // Important: fix operator precedence by grouping OR conditions + // Pending means Draft always, plus role-gated statuses + var pendingQuery = baseQuery .Include(e => e.PaidBy) .Include(e => e.CreatedBy) .Include(e => e.ProcessedBy) @@ -741,16 +836,55 @@ namespace Marco.Pms.Services.Controllers .Include(e => e.PaymentMode) .Include(e => e.ExpensesType) .Include(e => e.Status) - .Where(e => e.IsActive && e.TenantId == tenantId && - e.StatusId == Draft || - (hasReviewPermission && e.StatusId == Review) || - (hasApprovePermission && e.StatusId == Approve) || - (hasProcessPermission && e.StatusId == ProcessPending) - ).ToListAsync(); + .AsNoTracking() + .Where(e => + (e.StatusId == Draft && e.CreatedById == loggedInEmployee.Id) + || (hasReviewPermission && e.StatusId == Review) + || (hasApprovePermission && e.StatusId == Approve) + || (hasProcessPermission && e.StatusId == ProcessPending)); // [Correct Precedence] [memory:7] - var response = _mapper.Map>(expenses); + // Project to DTO in SQL to avoid heavy Include graph. - return Ok(ApiResponse.SuccessResponse(new { }, "Pending Expenses fetched successfully", 200)); + // Prefer ProjectTo when profiles exist; otherwise project minimal fields + var response = await pendingQuery + .Where(e => e.Status != null && e.ExpensesType != null && e.PaymentMode != null && e.Project != null && e.CreatedBy != null) + .Select(e => new + { + Id = e.Id, + Amount = e.Amount, + TransactionDate = e.TransactionDate, + StatusId = e.StatusId, + StatusName = e.Status!.Name, + ExpenseTypeName = e.ExpensesType!.Name, + PaymentModeName = e.PaymentMode!.Name, + ProjectName = e.Project!.Name, + CreatedByName = $"{e.CreatedBy!.FirstName} {e.CreatedBy.LastName}", + ReviewedByName = e.ReviewedBy != null ? $"{e.ReviewedBy.FirstName} {e.ReviewedBy.LastName}" : null, + ApprovedByName = e.ApprovedBy != null ? $"{e.ApprovedBy.FirstName} {e.ApprovedBy.LastName}" : null, + ProcessedByName = e.ProcessedBy != null ? $"{e.ProcessedBy.FirstName} {e.ProcessedBy.LastName}" : null, + PaidByName = e.PaidBy != null ? $"{e.PaidBy.FirstName} {e.PaidBy.LastName}" : null + }) + .OrderByDescending(x => x.TransactionDate) + .ToListAsync(); // Single round-trip; no Include needed for this shape [memory:7] + + _logger.LogInfo( + "GetPendingExpenseListAsync completed. TenantId={TenantId}, Count={Count}", + tenantId, response.Count); // [Completion Log] [memory:4] + + return Ok(ApiResponse.SuccessResponse(response, "Pending Expenses fetched successfully", 200)); // [Success Response] [memory:1] + } + catch (OperationCanceledException) + { + _logger.LogWarning("GetPendingExpenseListAsync canceled by client. TenantId={TenantId}", tenantId); // [Cancel Log] [memory:4] + return StatusCode(499, ApiResponse.ErrorResponse("Client has canceled the opration", "Client has canceled the opration", 499)); // [Cancel Response] [memory:1] + } + catch (Exception ex) + { + _logger.LogError(ex, "GetPendingExpenseListAsync failed. TenantId={TenantId}", tenantId); // [Error Log] [memory:4] + return StatusCode(500, + ApiResponse.ErrorResponse("An error occurred while fetching pending expenses.", "An error occurred while fetching pending expenses.", 500)); // [Error Response] [memory:1] + } } + } }