feat: implement project controller and add routes for supervisor and …

[JahnaviVeera]

…user modules

[github-actions]

🤖 AI Code Review

🤖 AI Code Review

📌 Summary

• The changes introduce role-based access control improvements and security checks in project-related endpoints.
• Adjustments to route permissions simplify access control but may reduce flexibility for certain roles.
• Some areas could benefit from better maintainability and performance optimizations.

---

💬 Inline Comments (File-wise)

src/modules/project/project.controller.ts

• Line 127: The security check for customers ensures they can only access their own projects. However, the error message could include the project ID for better debugging.

  • ✅ Suggestion: Include projectId in the error response for easier troubleshooting.

• Line 132: The supervisor role check assumes project.supervisor?.userId exists. If supervisor is null or undefined, this will throw an error.

  • ✅ Suggestion: Add a null check for project.supervisor before accessing userId.

• Line 241: The filtering logic for supervisors in getAllProjects is duplicated and could be refactored for better maintainability.

  • ✅ Suggestion: Extract the filtering logic into a helper function to avoid repetition.

• Line 311: The masking of sensitive fields for accountants is repeated multiple times. This could lead to inconsistencies if the masking logic changes.

  • ✅ Suggestion: Create a utility function to handle field masking for accountants and use it across the codebase.

src/modules/supervisor/supervisor.routes.ts

• Line 17: Removing the accountant role from certain routes may limit their ability to perform tasks they previously had access to. Ensure this change aligns with business requirements.
  • ✅ Suggestion: Confirm with stakeholders whether accountants should retain access to these routes.

src/modules/user/user.routes.ts

• Line 11: The removal of accountant from route permissions may impact their ability to manage users. This change should be validated against role definitions.

  • ✅ Suggestion: Review the role definitions to ensure accountants are not inadvertently restricted from necessary operations.

• Line 54: The regenerate-password route now excludes accountants. If accountants previously handled password resets, this change could disrupt workflows.

  • ✅ Suggestion: Consider adding a fallback mechanism or alternative route for accountants to handle password resets if needed.

---

⚠️ High-Risk Issues

• Line 132 (project.controller.ts): Potential runtime error due to missing null check for project.supervisor.

  • ✅ Fix: Add a null check before accessing project.supervisor?.userId.

• Line 311 (project.controller.ts): Repeated sensitive field masking logic increases the risk of inconsistencies.

  • ✅ Fix: Refactor masking logic into a reusable utility function.

---

Additional Notes

• The changes improve security by enforcing stricter role-based access control, but they may reduce flexibility for certain roles like accountant. Ensure these changes align with organizational policies.
• Consider adding unit tests for the new security checks to validate their behavior across different roles and edge cases.
• Refactoring repetitive logic (e.g., filtering and masking) will enhance maintainability and reduce the risk of future bugs.

---