Add support for batch user deactivation by IDs or emails in organizations #790
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe changes introduce support for deactivating multiple users by either IDs or emails within an organization. This involves refactoring validation logic, updating the controller and service layers, and adding new helper and query functions for batch deactivation and session cleanup. Method signatures and validation schemas are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Validator
participant Service
participant UserQueries
participant UserHelper
participant EventBus
Client->>Controller: POST /org-admin/deactivateUser {ids, emails}
Controller->>Validator: Validate request body
Validator-->>Controller: Validation result
Controller->>Service: deactivateUser({ids, emails}, tokenInfo)
Service->>UserQueries: deactivateUserInOrg(filter by ids, org, tenant)
UserQueries-->>Service: [affectedRows, updatedUsersById]
Service->>UserQueries: deactivateUserInOrg(filter by emails, org, tenant)
UserQueries-->>Service: [affectedRows, updatedUsersByEmail]
Service->>UserHelper: removeAllUserSessions(allDeactivatedUserIds, tenant)
UserHelper-->>Service: {success, removedCount}
Service->>EventBus: broadcast deactivateUpcomingSession (userIds)
Service-->>Controller: Success/Failure response
Controller-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/services/org-admin.js (2)
456-456: Remove debug loggingAvoid
console.login services. Use a logger or remove.- console.log(rowsAffected, updatedUsers)
443-456: Minor: reduce duplicated updates and payload
- If an ID and its email are both provided, the second update may re-touch the same rows. Not critical, but you can de-duplicate target IDs upfront or exclude already-updated IDs/emails on the second call.
- After adjusting
deactivateUserInOrgto return IDs (see query comment), you can avoid returning Sequelize instances and simplify to:- const [rowsAffected, updatedUsers] = await userQueries.deactivateUserInOrg(..., true) - updatedByIds.push(...updatedUsers.map((user) => user.id)) + const [rowsAffected, updatedIds] = await userQueries.deactivateUserInOrg(..., true) + updatedByIds.push(...updatedIds)Also applies to: 462-475
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/controllers/v1/org-admin.js(0 hunks)src/database/queries/users.js(1 hunks)src/helpers/userHelper.js(2 hunks)src/services/org-admin.js(2 hunks)src/validators/v1/org-admin.js(1 hunks)
💤 Files with no reviewable changes (1)
- src/controllers/v1/org-admin.js
🧰 Additional context used
📓 Path-based instructions (3)
src/validators/**
⚙️ CodeRabbit Configuration File
Validate all incoming data thoroughly. Check for missing or incomplete validation rules.
Files:
src/validators/v1/org-admin.js
src/database/queries/**
⚙️ CodeRabbit Configuration File
Review database queries for performance. Check for N+1 problems and ensure indexes can be used.
Files:
src/database/queries/users.js
src/services/**
⚙️ CodeRabbit Configuration File
This is core business logic. Please check for correctness, efficiency, and potential edge cases.
Files:
src/services/org-admin.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/database/models/entityType.js:38-38
Timestamp: 2025-07-31T08:43:35.971Z
Learning: The migration for converting tenant_code to a primary key in the EntityType model was already handled in a previous PR, not in the current refactoring PR that focuses on organization codes instead of organization IDs.
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/services/entities.js:18-23
Timestamp: 2025-07-31T08:44:36.982Z
Learning: In the ELEVATE-Project/user codebase, organizationCode and tenantCode parameters passed to service methods always come from req.decodedToken.organization_code and req.decodedToken.tenant_code, which are guaranteed to be present after token validation. Additional validation for these parameters in service methods is unnecessary as the token validation process ensures they are always available.
📚 Learning: 2025-08-06T07:27:08.445Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#783
File: src/database/queries/fileUpload.js:40-59
Timestamp: 2025-08-06T07:27:08.445Z
Learning: In the ELEVATE-Project/user repository, pagination is handled by src/middlewares/pagination.js which validates page and limit parameters and sets req.pageNo and req.pageSize. Service methods like getBulkInvitesFilesList use these pre-validated values when calling database query functions like listUploads. Additional parameter validation in database query functions is redundant since the middleware handles validation upstream.
Applied to files:
src/helpers/userHelper.jssrc/validators/v1/org-admin.js
📚 Learning: 2025-08-06T07:27:08.445Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#783
File: src/database/queries/fileUpload.js:40-59
Timestamp: 2025-08-06T07:27:08.445Z
Learning: In the ELEVATE-Project/user repository, pagination parameters are validated by src/middlewares/pagination.js middleware which sets req.pageNo and req.pageSize. Service methods like getBulkInvitesFilesList use these pre-validated values when calling database query functions. Database query functions like listUploads receive already-validated pagination parameters, so additional validation in the query layer is redundant.
Applied to files:
src/helpers/userHelper.jssrc/validators/v1/org-admin.js
📚 Learning: 2025-08-06T07:28:13.285Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#783
File: src/database/queries/fileUpload.js:40-59
Timestamp: 2025-08-06T07:28:13.285Z
Learning: In the ELEVATE-Project/user repository, pagination parameters (page and limit) are validated in src/middlewares/pagination.js middleware which is applied globally to all routes. The middleware sets req.pageNo and req.pageSize with proper validation, so database query functions like listUploads don't need additional parameter validation.
Applied to files:
src/validators/v1/org-admin.js
📚 Learning: 2025-07-31T08:43:35.971Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/database/models/entityType.js:38-38
Timestamp: 2025-07-31T08:43:35.971Z
Learning: The migration for converting tenant_code to a primary key in the EntityType model was already handled in a previous PR, not in the current refactoring PR that focuses on organization codes instead of organization IDs.
Applied to files:
src/database/queries/users.jssrc/services/org-admin.js
🔇 Additional comments (2)
src/database/queries/users.js (1)
549-571: No additional indexes needed for deactivateUserInOrgThe
user_organizationstable already has the following in place (via 20250506095633-update-relations.js and the PK on creation):
- Primary key on (tenant_code, user_id, organization_code)
- idx_user_organizations_org_tenant on (organization_code, tenant_code)
- idx_user_organizations_user_tenant on (user_id, tenant_code)
- idx_user_organizations_tenant_code on (tenant_code)
The
userstable uses its primary key (id) for lookups; we don’t filter by tenant_code onusersin this query, so no extra index is required.src/services/org-admin.js (1)
404-431: Good docstringThe method-level documentation is clear and accurately reflects behavior and limitations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/validators/v1/org-admin.js (2)
31-31: Apply request-body whitelist and block spoofed tenant codesAdd the whitelist filter to align with other handlers and ensure only allowed keys (emails/ids) are processed. This also prevents clients from injecting tenant_code/organization_code via body; those should come from req.decodedToken per our conventions.
Apply this diff:
deactivateUser: (req) => { + req.body = filterRequestBody(req.body, orgAdmin.deactivateUser) req.checkBody('emails').optional().isArray().withMessage('The "emails" field must be an array, if provided.')Run to verify the whitelist entry and allowed keys:
#!/bin/bash set -euo pipefail # Locate blacklistConfig fd -a blacklistConfig src # Show deactivateUser config context rg -n "deactivateUser" src/constants/blacklistConfig.* -n -A 8 -B 8 || true # Confirm that 'emails' and 'ids' are allowed and tenant keys are NOT allowed rg -n "'emails'|'ids'" src/constants/blacklistConfig.* || true rg -n "tenant(Code|_code)|organization(Code|_code)" src/constants/blacklistConfig.* || true
40-43: Stricter ID validation: enforce positive integers (no floats/negatives)Use isInt({ min: 1 }) instead of isNumeric() and update the error message. Optionally coerce to integers.
- .isNumeric() - .withMessage('Each item in the "ids" array must be a numeric value.') + .isInt({ min: 1 }) + .withMessage('Each item in the "ids" array must be a positive integer.') + .toInt()
🧹 Nitpick comments (1)
src/validators/v1/org-admin.js (1)
31-38: Optional hardening: cap batch sizes and normalize inputPrevent oversized payloads and standardize emails. Pick limits that match ops constraints.
- req.checkBody('emails').optional().isArray().withMessage('The "emails" field must be an array, if provided.') + req.checkBody('emails') + .optional() + .isArray() + .withMessage('The "emails" field must be an array, if provided.') + .custom(arr => arr.length <= 1000) + .withMessage('The "emails" array cannot exceed 1000 items.') @@ - req.checkBody('emails.*') + req.checkBody('emails.*') .optional() - .isEmail() + .isEmail() + .trim() + .normalizeEmail() .withMessage('Each item in the "emails" array must be a valid email address.') @@ - req.checkBody('ids').optional().isArray().withMessage('The "ids" field must be an array, if provided.') + req.checkBody('ids') + .optional() + .isArray() + .withMessage('The "ids" field must be an array, if provided.') + .custom(arr => arr.length <= 1000) + .withMessage('The "ids" array cannot exceed 1000 items.')Note: If your express-validator version lacks normalizeEmail(), keep trim() only.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/database/queries/users.js(1 hunks)src/services/org-admin.js(2 hunks)src/validators/v1/org-admin.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/database/queries/users.js
- src/services/org-admin.js
🧰 Additional context used
📓 Path-based instructions (1)
src/validators/**
⚙️ CodeRabbit Configuration File
Validate all incoming data thoroughly. Check for missing or incomplete validation rules.
Files:
src/validators/v1/org-admin.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/database/models/entityType.js:38-38
Timestamp: 2025-07-31T08:43:35.971Z
Learning: The migration for converting tenant_code to a primary key in the EntityType model was already handled in a previous PR, not in the current refactoring PR that focuses on organization codes instead of organization IDs.
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/services/entities.js:18-23
Timestamp: 2025-07-31T08:44:36.982Z
Learning: In the ELEVATE-Project/user codebase, organizationCode and tenantCode parameters passed to service methods always come from req.decodedToken.organization_code and req.decodedToken.tenant_code, which are guaranteed to be present after token validation. Additional validation for these parameters in service methods is unnecessary as the token validation process ensures they are always available.
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:10-10
Timestamp: 2025-08-08T13:06:32.884Z
Learning: In ELEVATE-Project/user, avoid potential circular dependencies between src/helpers/userHelper.js and src/services/user-sessions by extracting shared session cleanup logic (finding active sessions, deleting Redis keys, ending sessions) into a dedicated utility (e.g., helpers/sessionCleanup.js) that both layers can consume.
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:117-156
Timestamp: 2025-08-08T15:12:44.400Z
Learning: In ELEVATE-Project/user, prefer extracting shared session teardown (find active sessions, delete Redis keys, mark ended_at) into helpers/sessionCleanup.js so both helpers and services use it, preventing upward (helper→service) dependencies and avoiding circular imports.
📚 Learning: 2025-08-06T07:27:08.445Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#783
File: src/database/queries/fileUpload.js:40-59
Timestamp: 2025-08-06T07:27:08.445Z
Learning: In the ELEVATE-Project/user repository, pagination is handled by src/middlewares/pagination.js which validates page and limit parameters and sets req.pageNo and req.pageSize. Service methods like getBulkInvitesFilesList use these pre-validated values when calling database query functions like listUploads. Additional parameter validation in database query functions is redundant since the middleware handles validation upstream.
Applied to files:
src/validators/v1/org-admin.js
📚 Learning: 2025-08-06T07:27:08.445Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#783
File: src/database/queries/fileUpload.js:40-59
Timestamp: 2025-08-06T07:27:08.445Z
Learning: In the ELEVATE-Project/user repository, pagination parameters are validated by src/middlewares/pagination.js middleware which sets req.pageNo and req.pageSize. Service methods like getBulkInvitesFilesList use these pre-validated values when calling database query functions. Database query functions like listUploads receive already-validated pagination parameters, so additional validation in the query layer is redundant.
Applied to files:
src/validators/v1/org-admin.js
🔇 Additional comments (2)
src/validators/v1/org-admin.js (2)
45-54: Good: enforce at least one non-empty identifier arrayThis custom validator correctly blocks requests where both arrays are missing or empty.
45-54: Verify express-validator array argument supportI wasn’t able to locate
express-validatorin package.json or lockfiles—please confirm the installed version supports passing an array toreq.checkBody(). Versions without this signature will skip the custom validator silently. If your version doesn’t acceptreq.checkBody(['field1','field2']), update each multi-field check to a single-field call that inspects all required props.Affected locations:
- src/validators/v1/org-admin.js (lines 45–54)
- src/validators/v1/account.js (lines 93–99 & 191–197)
- src/validators/v1/admin.js (lines 66–72)
Example replacement in org-admin.js:
- req.checkBody(['emails','ids']).custom(() => { + req.checkBody('emails').custom((_, { req }) => { const { emails, ids } = req.body if ((!emails || emails.length === 0) && (!ids || ids.length === 0)) { throw new Error('Provide at least one non-empty "emails" or "ids" array.') } return true })
This was referenced Aug 12, 2025
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(org-admin): update deactivateUser endpoint to support tenant code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes