Conversation
branch sync
branch sync
Dev akith
Dev jayasankha
branch sync
…TTP requests documentation
…lEvidenceLA, and SalesEvidenceLA controllers
…remove unused parent_id and parent_type parameters from UploadImages method
Dev jayasankha
Dev jalina
- Added a ProfilePicture property of type byte[] to the User model. - Created a new migration to add the ProfilePicture column to the Users table. - Updated the AppDbContextModelSnapshot to reflect the new ProfilePicture field. - Modified LoginModels to include ProfilePicture in the Login response.
Dev jayasankha
…mprove logging for task overview
Enhance UserTaskController to include detailed task type counts and i…
There was a problem hiding this comment.
Pull Request Overview
This branch update enhances the LA Masterfile functionality by adding sorting and user filtering capabilities, introduces profile picture support for users, updates authorization on several controllers, and changes the ImageData ReportId from string to int for better type consistency.
- Adds sorting and user filtering parameters to all LA Masterfile service and repository methods
- Introduces profile picture support in User model with corresponding upload functionality
- Updates authorization attributes on multiple controllers and modifies ReportId data type
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/LAMasterfileService.cs | Adds sortBy and assignedToUserId parameters to all methods and includes SortBy in response objects |
| services/ILAMasterfileService.cs | Updates interface signatures to include new sorting and filtering parameters |
| repositories/LAMasterfileRepository.cs | Implements sorting and user filtering logic with helper methods |
| repositories/ILAMasterfileRepository.cs | Updates interface to match repository implementation |
| Models/LoginModels.cs | Adds ProfilePicture property to User model |
| Models/LAMasterfileResponse.cs | Adds SortBy and SearchTerm properties for enhanced response data |
| Models/ImageData.cs | Changes ReportId from string to int type |
| Controllers/LAMasterfileController.cs | Updates all endpoints to accept new sorting and filtering query parameters |
| Controllers/UserProfileController.cs | New controller for handling profile picture uploads |
| Controllers/ProfileController.cs | Adds profile picture to user profile response |
| Multiple Controllers | Adds [Authorize] attribute for enhanced security |
Files not reviewed (1)
- Migrations/20250720082954_AddProfilePictureToUser.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
| // Join with UserTasks to filter by assigned user ID for LA tasks | ||
| return query.Where(la => _context.UserTasks | ||
| .Any(ut => ut.UserId == assignedToUserId.Value && | ||
| ut.TaskType == "LA" && | ||
| ut.LandAcquisitionId == la.Id)); |
There was a problem hiding this comment.
The user filtering logic uses Any() in a Where clause which can lead to N+1 query issues and poor performance. Consider using a Join instead of the correlated subquery pattern for better database performance.
| // Join with UserTasks to filter by assigned user ID for LA tasks | |
| return query.Where(la => _context.UserTasks | |
| .Any(ut => ut.UserId == assignedToUserId.Value && | |
| ut.TaskType == "LA" && | |
| ut.LandAcquisitionId == la.Id)); | |
| // Use a Join with UserTasks to filter by assigned user ID for LA tasks | |
| return from la in query | |
| join ut in _context.UserTasks | |
| on la.Id equals ut.LandAcquisitionId | |
| where ut.UserId == assignedToUserId.Value && ut.TaskType == "LA" | |
| select la; |
| var lmAssigned = userTasks.Count(t => t.TaskType == "LM"); | ||
| var lmCompleted = userTasks.Count(t => t.TaskType == "LM" && t.IsCompleted); | ||
| // Debug log: print all userTasks for this user | ||
| Console.WriteLine($"UserTasks for {request.Username}: {string.Join(", ", userTasks.Select(t => $"{t.TaskType}:{t.IsCompleted}"))}"); |
There was a problem hiding this comment.
Debug console output should not be present in production code. Use proper logging frameworks like ILogger instead of Console.WriteLine for debugging purposes.
| Console.WriteLine($"UserTasks for {request.Username}: {string.Join(", ", userTasks.Select(t => $"{t.TaskType}:{t.IsCompleted}"))}"); | |
| _logger.LogInformation("UserTasks for {Username}: {UserTasks}", request.Username, string.Join(", ", userTasks.Select(t => $"{t.TaskType}:{t.IsCompleted}"))); |
| var lmCompleted = userTasks.Count(t => t.TaskType != null && t.TaskType.ToUpper() == "LM" && t.IsCompleted); | ||
|
|
||
| // Debug log: print computed counts | ||
| Console.WriteLine($"LA: {laAssigned}, MR: {mrAssigned}, LM: {lmAssigned}, LA Done: {laCompleted}, MR Done: {mrCompleted}, LM Done: {lmCompleted}"); |
There was a problem hiding this comment.
Debug console output should not be present in production code. Use proper logging frameworks like ILogger instead of Console.WriteLine for debugging purposes.
| Console.WriteLine($"LA: {laAssigned}, MR: {mrAssigned}, LM: {lmAssigned}, LA Done: {laCompleted}, MR Done: {mrCompleted}, LM Done: {lmCompleted}"); | |
| _logger.LogDebug("LA: {LaAssigned}, MR: {MrAssigned}, LM: {LmAssigned}, LA Done: {LaCompleted}, MR Done: {MrCompleted}, LM Done: {LmCompleted}", laAssigned, mrAssigned, lmAssigned, laCompleted, mrCompleted, lmCompleted); |
| } | ||
|
|
||
| [HttpPost("upload-profile-picture")] | ||
| public async Task<IActionResult> UploadProfilePicture([FromForm] ProfilePictureUploadDto dto) |
There was a problem hiding this comment.
The profile picture upload endpoint lacks file validation for size limits, file type restrictions, and malicious content checks. This could allow users to upload large files or potentially harmful content.
|
|
||
| namespace ValuationBackend.Controllers | ||
| { | ||
| public class ProfilePictureUploadDto |
There was a problem hiding this comment.
[nitpick] The DTO class is defined inside the controller file. DTOs should be placed in a separate Models or DTOs folder for better organization and reusability.
No description provided.