feat: Implement payments, daily updates, supervisor, and notification…

[JahnaviVeera]

…s modules with new API endpoints and Prisma schema.

[github-actions]

🤖 AI Code Review

🤖 AI Code Review

📌 Summary

• The changes introduce new features, including referenceId in the Notification model, email redirection for development, and new endpoints for marking and approving construction stages.
• Several improvements were made to the DailyUpdatesServices logic, including stricter validation, better status handling, and progress recalculation.
• Some areas could benefit from enhanced error handling, security checks, and performance optimizations.

---

💬 Inline Comments (File-wise)

check_reference_id.js

• Line 6: The PrismaClient instantiation lacks error handling for connection issues.
  • ✅ Suggestion: Wrap the new PrismaClient() call in a try-catch block to handle potential connection errors gracefully.

prisma/schema.prisma

• Line 209: The referenceId field is added but lacks a unique constraint.
  • ✅ Suggestion: If referenceId is expected to be unique, consider adding @unique to prevent duplicate entries.

src/config/prisma.client.ts

• Line 7: The ssl configuration disables rejectUnauthorized for non-local environments, which can expose the application to man-in-the-middle attacks.
  • ✅ Suggestion: Use a proper certificate validation mechanism instead of disabling rejectUnauthorized.

src/email/emailService.ts

• Line 16: The email redirection logic for development mode hardcodes an email address.
  • ✅ Suggestion: Use an environment variable for the testing email address to avoid hardcoding sensitive information.

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

• Line 1419: The markStageComplete function does not validate the stage parameter against a predefined list of valid stages.
  • ✅ Suggestion: Add validation to ensure stage is one of the allowed values to prevent invalid data from being processed.
• Line 1449: The approveStage function has the same issue with stage validation as markStageComplete.
  • ✅ Suggestion: Implement similar validation here.

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

• Line 613: The restriction logic for approved stages could be optimized by reducing the number of database queries.
  • ✅ Suggestion: Use a single query to check for approved stages instead of multiple conditional checks.
• Line 1304: Automatically approving updates when feedback is added might lead to unintended consequences if the feedback is not explicitly meant as approval.
  • ✅ Suggestion: Consider requiring explicit approval from the customer instead of assuming feedback implies approval.
• Line 1324: The progress recalculation logic assumes a fixed number of stages (6), which might not be future-proof.
  • ✅ Suggestion: Dynamically calculate the total number of stages from the database or configuration.

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

• Line 76: The new routes for marking and approving stages lack rate-limiting or abuse prevention mechanisms.
  • ✅ Suggestion: Add middleware to limit the number of requests per user to prevent abuse.

---

⚠️ High-Risk Issues

1. src/config/prisma.client.ts: Disabling rejectUnauthorized for SSL connections in production environments is a security risk.
   • ✅ Fix: Use a valid SSL certificate and enable rejectUnauthorized to ensure secure connections.
2. src/email/emailService.ts: Hardcoding an email address for development mode can lead to accidental exposure of sensitive information.
   • ✅ Fix: Use an environment variable for the testing email address.
3. src/modules/daily-updates/daily-updates.services.ts: Automatically approving updates based on feedback might lead to data integrity issues.
   • ✅ Fix: Require explicit approval from the customer instead of assuming feedback implies approval.

---

Final Notes

• The changes are well-structured and introduce useful features, but some areas need additional validation and security hardening.
• Addressing the high-risk issues should be prioritized to ensure the application remains secure and reliable.

---

🤖 AI Code Review

📌 Summary

• The code introduces new functionality for marking and approving construction stages, receipt number generation, and notification enhancements.
• Overall, the code is well-structured but has some areas for improvement in terms of maintainability, error handling, and performance.
• A few potential bugs and edge cases were identified, along with suggestions for improvement.

---

💬 Inline Comments (File-wise)

src/modules/notifications/notifications.services.ts

• Line 74: The referenceId parameter addition is a good enhancement for tracking notifications. However, ensure that all existing calls to notifyUser are updated to handle this new parameter to avoid unintended null values.
  • ✅ Suggestion: Audit all usages of notifyUser to ensure compatibility with the new referenceId parameter.

src/modules/payments/payments.controller.ts

• Line 667: The getNextReceiptNumber endpoint is restricted to admin roles. This is appropriate for security, but consider whether supervisors or other roles might also need access.
  • ✅ Suggestion: Confirm role requirements with stakeholders and update the authorizeRoles middleware if necessary.

src/modules/payments/payments.services.ts

• Line 22: The normalizePaymentMethod function could benefit from a more robust mapping mechanism, such as a dictionary, to improve readability and maintainability.

  • ✅ Suggestion: Replace the if statements with a mapping object, e.g., { 'bank_transfer': PaymentMethod.bank_transfer, 'check': PaymentMethod.cheque, ... }.

• Line 637: The generateReceiptNumber function assumes that the customerName will always have at least two characters. This could lead to issues if the name is shorter.

  • ✅ Suggestion: Add a fallback for cases where customerName is less than two characters, e.g., const namePrefix = customerName.substring(0, 2).toUpperCase() || 'XX';.

src/modules/supervisor/supervisor.services.ts

• Line 177: The calculation of project progress assumes there are exactly six stages. If the number of stages changes in the future, this logic will break.
  • ✅ Suggestion: Use a dynamic approach to determine the total number of stages, such as fetching them from a configuration or database.

src/modules/user/user.services.ts

• Line 503: The filter condition for pendingUpdates includes both pending and Approval_Requested statuses. Ensure that this logic aligns with the intended business rules.
  • ✅ Suggestion: Confirm with stakeholders whether Approval_Requested should be treated as pending for customer dashboards.

src/modules/construction/construction.services.ts (Newly added functions)

• Line 20: The normalizeInputStage function has repetitive logic for handling stage names. This could be simplified using a mapping object or a more dynamic approach.

  • ✅ Suggestion: Use a mapping object to handle stage normalization, e.g., { 'Plumbing_&_Electrical': ConstructionStage.Plumbing___Electrical, ... }.

• Line 60: The markStageComplete function assumes that there will always be updates to mark as Approval_Requested. This might not always be true.

  • ✅ Suggestion: Add a fallback mechanism or log a warning when no updates are found for the given stage.

• Line 120: The approveStage function updates project progress based on approved stages. Ensure that the progress calculation logic is consistent across the application.

  • ✅ Suggestion: Extract the progress calculation logic into a reusable utility function to avoid duplication.

---

⚠️ High-Risk Issues

1. Line 637 (generateReceiptNumber): Assumes customerName has at least two characters, which could lead to runtime errors for short names.

   • ✅ Fix: Add a fallback for short names, e.g., const namePrefix = customerName.substring(0, 2).toUpperCase() || 'XX';.

2. Line 60 (markStageComplete): Assumes updates will always exist for the given stage, which might not be true and could lead to inconsistent states.

   • ✅ Fix: Add a fallback mechanism or log a warning when no updates are found.

3. Line 177 (getSupervisorProfile): Hardcodes the number of construction stages as six, which could lead to incorrect progress calculations if the number of stages changes.

   • ✅ Fix: Fetch the total number of stages dynamically from a configuration or database.

---

Final Notes

• The new functionality is well-implemented and aligns with the application's goals. However, there are opportunities to improve maintainability and robustness.
• Addressing the identified issues will enhance the code's reliability and future-proof it against potential changes.

---

🤖 AI Code Review

📌 Summary

• Several files introduce test scripts for Prisma-based database operations.
• Common issues include hardcoded sensitive data, lack of error handling, and potential inefficiencies.
• No high-risk security vulnerabilities, but improvements are recommended for maintainability and best practices.

---

💬 Inline Comments (File-wise)

test_project.js

• Line 4: Hardcoding the projectId directly in the code is not a best practice.
  • ✅ Suggestion: Use environment variables or configuration files to store sensitive or environment-specific data.
• Line 12: The finally block ensures the Prisma client disconnects, which is good. However, consider adding more robust error handling for database operations.
  • ✅ Suggestion: Wrap the findUnique call in a try-catch block to handle potential database errors explicitly.

test_service.js

• Line 2: The .env file path is hardcoded. This could lead to issues in different environments.
  • ✅ Suggestion: Use a configuration management library or environment variables to dynamically determine the .env file path.
• Line 7: Hardcoding projectId, stage, and supervisorId is not ideal.
  • ✅ Suggestion: Pass these values as arguments to the script or load them from a configuration file.
• Line 13: The markStageComplete function call lacks detailed error handling.
  • ✅ Suggestion: Log more context about the error (e.g., input parameters) to aid debugging.

test_supervisor.js

• Line 5: Hardcoding supervisorId directly in the code is not recommended.
  • ✅ Suggestion: Use environment variables or pass it as an argument to the script.
• Line 9: The findMany query could potentially return a large dataset.
  • ✅ Suggestion: Add pagination or limit the number of results returned to avoid performance issues.
• Line 15: Counting completed projects is fine, but consider logging more context if the count is unexpectedly high or low.
  • ✅ Suggestion: Add a conditional log or warning if the count exceeds a certain threshold.

test_supervisor.ts

• Line 5: Similar to test_supervisor.js, hardcoding supervisorId is not ideal.
  • ✅ Suggestion: Use environment variables or script arguments.
• Line 9: The findMany query could return a large dataset.
  • ✅ Suggestion: Implement pagination or a limit to improve performance.
• Line 12: The map function is a good improvement for formatting output compared to test_supervisor.js. However, consider logging only a subset of the data if the dataset is large.
  • ✅ Suggestion: Limit the number of projects logged or log only essential fields.

tmp_check_column.ts

• Line 6: The raw SQL query is vulnerable to SQL injection if user input is ever introduced.
  • ✅ Suggestion: Use parameterized queries with Prisma's $queryRaw to prevent SQL injection risks.
• Line 6: The table name Payment is hardcoded.
  • ✅ Suggestion: Use a variable or configuration to make the script more reusable and maintainable.

---

⚠️ High-Risk Issues

No high-risk issues found.

---

Additional Suggestions

1. Centralize Configuration: Many scripts hardcode sensitive or environment-specific data (e.g., projectId, supervisorId, .env path). Centralizing configuration management would improve maintainability and security.
2. Error Handling: While most scripts include basic error handling, consider adding more detailed logs and context to aid debugging.
3. Performance Optimization: Queries like findMany could return large datasets. Adding pagination or limits would improve performance and prevent potential memory issues.
4. Code Reusability: Several scripts perform similar operations (e.g., querying projects). Consider refactoring shared logic into reusable utility functions or modules.

---