feat: Introduce new modules for project management, user, quotations,…

[JahnaviVeera]

… daily updates, documents, and notifications, including an accountant role with restricted project financial data access.

[github-actions]

🤖 AI Code Review

🤖 AI Code Review

📌 Summary

• The changes introduce the accountant role across multiple modules, expanding its permissions.
• The migration script adds the accountant value to the users_role_enum type.
• Some improvements to null handling and mapping logic in user.services.ts were made.
• No critical security vulnerabilities were identified, but there are areas for improvement in maintainability and clarity.

---

💬 Inline Comments (File-wise)

prisma/migrations/20260317000000_add_accountant_to_user_role/migration.sql

• Line 2: The migration adds a new value to the users_role_enum. While this is correct, consider documenting the implications of this change (e.g., how existing users will be affected or if any default role assignments need updates).
  • ✅ Suggestion: Add a comment or documentation about whether this migration requires any follow-up actions, such as updating existing user roles.

src/modules/daily-updates/daily-updates.routes.ts

• Lines 65-66: Adding the accountant role to approval/rejection routes is logical, but ensure this aligns with business requirements. Accountants typically handle financial data, so their involvement in these actions should be validated.
  • ✅ Suggestion: Confirm with stakeholders that accountant should have these permissions.

src/modules/documents/documents.routes.ts

• Multiple Lines: The accountant role is added to several document-related routes. While this is consistent, ensure that accountants should indeed have access to all these endpoints, especially sensitive ones like deleteDocument.
  • ✅ Suggestion: Perform a role-based access control (RBAC) review to ensure the accountant role is not over-permissioned.

src/modules/notifications/notifications.routes.ts

• Line 12: Adding accountant to the global middleware for notifications is fine, but ensure this role needs access to all notifications.
  • ✅ Suggestion: If accountant should only access specific notifications, consider filtering notifications based on role.

src/modules/project/project.controller.ts

• Lines 554-565: Masking the totalBudget field for accountant users is a good security measure. However, the implementation modifies the original projects array directly, which could lead to unintended side effects.
  • ✅ Suggestion: Use map to create a new array instead of mutating the original:

    const maskedProjects = projects.map((p: any) => ({
        ...p,
        totalBudget: "••••••"
    }));

src/modules/quotations/quotations.routes.ts

• Multiple Lines: The accountant role is added to quotation-related routes. Ensure that accountants should have permissions for actions like deleteQuotation or resendQuotation.
  • ✅ Suggestion: Review the business logic to confirm that these permissions align with the role's responsibilities.

src/modules/user/user.services.ts

• Lines 296-308: The null handling for timezone is a good addition, but the logic could be simplified for readability.

  • ✅ Suggestion: Combine the null check and mapping logic for better clarity:

    if (updatedUserData.timezone?.trim()) {
        const timeZoneMap = { /* mapping */ };
        dataToUpdate.timezone = timeZoneMap[updatedUserData.timezone] || updatedUserData.timezone;
    } else {
        dataToUpdate.timezone = null;
    }

• Lines 313-325: Similar to the timezone logic, the currency handling could be simplified.

  • ✅ Suggestion: Apply the same pattern as above for consistency.

• Lines 548-550: The spread operator in getClosedCustomersList is used to include all fields, but it may unintentionally expose sensitive data.

  • ✅ Suggestion: Explicitly define which fields to include in the response to avoid accidental data leaks:

    return {
        projectId: latestProject?.projectId || null,
        projectName: latestProject?.projectName || "No Project Assigned",
        createdAt: latestProject?.createdAt || user.createdAt,
        customer: {
            userId: user.userId,
            userName: user.userName,
            email: user.email,
            contact: user.contact
        }
    };

---

⚠️ High-Risk Issues

No high-risk issues found. However, ensure that the expanded permissions for the accountant role are thoroughly reviewed to avoid over-permissioning, which could lead to unauthorized access to sensitive data or actions.

---

This review highlights areas for improvement in maintainability and clarity while ensuring the changes align with business requirements. Let me know if further clarification is needed!

---