Skip to content

feat: Add project creation, retrieval with search and pagination, bud… - #43

Merged
JahnaviVeera merged 1 commit into
spotmies:masterfrom
JahnaviVeera:Jahnavi_Dev
Mar 6, 2026
Merged

JahnaviVeera merged 1 commit into
spotmies:masterfrom
JahnaviVeera:Jahnavi_Dev

Conversation

@JahnaviVeera

Copy link
Copy Markdown
Collaborator

…get/payment calculations, and user service functionalities.

…get/payment calculations, and user service functionalities.
@JahnaviVeera
JahnaviVeera merged commit 7859ae2 into spotmies:master Mar 6, 2026
1 check passed
@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown

🤖 AI Code Review

🤖 AI Code Review

📌 Summary

  • The changes introduce new fields in getProjectsByCustomerId and enhance the deleteUser function with a transaction for cascading deletions.
  • A test script (test_delete.js) is added to validate the deletion logic.
  • Overall, the changes improve functionality but introduce potential maintainability and performance concerns.

💬 Inline Comments (File-wise)

src/modules/project/project.services.ts

  • Line 709: The addition of startDate and expectedCompletion fields is straightforward. However, ensure these fields are indexed in the database if they are frequently queried or filtered.
    • ✅ Suggestion: Verify database indexing for startDate and expectedCompletion to optimize query performance.

src/modules/user/user.services.ts

  • Line 300: The prisma.$transaction block is well-structured but could become difficult to maintain as the number of related entities grows.

    • ✅ Suggestion: Consider abstracting the deletion logic for related entities into separate helper functions to improve readability and reusability.
  • Line 308: The deletion of project-related entities assumes that all related entities are correctly mapped. If new related entities are added in the future, this logic will need to be updated.

    • ✅ Suggestion: Document this dependency clearly or implement a more dynamic approach (e.g., metadata-driven deletion).
  • Line 328: The deletion of messages for both sender and receiver is correct but could be optimized by combining the OR conditions into a single query.

    • ✅ Suggestion: Use a single query with WHERE userId IN (senderId, receiverId) for better performance.
  • Line 333: The check for user.role === 'supervisor' assumes that the role field is always accurate. If this field is updated elsewhere, it could lead to inconsistencies.

    • ✅ Suggestion: Validate the role field against the supervisor table to ensure consistency before deletion.

test_delete.js

  • Line 5: The test script is useful for validating the deletion logic but lacks cleanup for created test data.

    • ✅ Suggestion: Add cleanup logic to delete the test user and project after the test completes to avoid polluting the database.
  • Line 27: The startDate and expectedCompletion fields are hardcoded. This is fine for testing but could be parameterized for flexibility.

    • ✅ Suggestion: Use variables or configuration files for test data to make the script reusable.

⚠️ High-Risk Issues

  1. Potential Data Integrity Issues in deleteUser:

    • The manual cascading deletion relies on hardcoded entity relationships. If any related entity is missed or new relationships are added in the future, it could lead to orphaned records or data integrity issues.
    • ✅ Suggestion: Use database-level cascading deletes where possible or implement a metadata-driven approach to dynamically identify related entities.
  2. Performance Concerns in deleteUser:

    • The deletion logic involves multiple sequential queries, which could be slow for users with many related records.
    • ✅ Suggestion: Batch deletions or optimize queries to reduce the number of database calls.
  3. Test Script Lacks Isolation:

    • The test script directly interacts with the production database schema, which could lead to unintended side effects if run in a non-test environment.
    • ✅ Suggestion: Use a dedicated test database or mock the Prisma client for testing.

⚠️ High-Risk Issues

  • Potential data integrity issues in deleteUser due to manual cascading deletions.
  • Performance concerns in deleteUser for users with many related records.
  • Test script lacks isolation and cleanup, which could lead to database pollution.

By addressing the above issues, the code can be made more robust, maintainable, and performant. Let me know if you need further clarification or assistance!


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants