-
Notifications
You must be signed in to change notification settings - Fork 19
tenantCode and orgCode added to role change request body #812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe roleChange event payload in src/services/org-admin.js’s updateRoleForApprovedRequest now includes tenant_code and organization_code in addition to user_id, new_roles, and current_roles. Function signatures and control flow remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AdminService as Org Admin Service
participant EventBus as Event Bus
participant Consumers as Downstream Consumers
AdminService->>AdminService: updateRoleForApprovedRequest(...)
note over AdminService: Compute user_id, current_roles, new_roles
AdminService-->>EventBus: Emit roleChange { user_id, current_roles, new_roles, tenant_code, organization_code }
note right of EventBus: New fields: tenant_code, organization_code
EventBus-->>Consumers: Dispatch roleChange payload
Consumers->>Consumers: Handle with tenant/org context
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/services/org-admin.js (7)
325-333: Fix potential TypeError when request not found.requestDetail may be null; accessing requestDetail.status will throw.
Apply:
const requestDetail = await orgRoleReqQueries.requestDetails({ id: requestId, organization_id: tokenInformation.organization_id, tenant_code: tokenInformation.tenant_code, }) - if (requestDetail.status !== common.REQUESTED_STATUS) { + if (!requestDetail) { + return responses.failureResponse({ + message: 'REQUEST_NOT_FOUND', + statusCode: httpStatusCode.bad_request, + responseCode: 'CLIENT_ERROR', + }) + } + + if (requestDetail.status !== common.REQUESTED_STATUS) {
282-286: Prevent tenant/org scoping override via filters.Merging arbitrary filters can let callers override tenant_code/organization_id, risking data leakage.
Apply:
- if (params.body?.filters) { - for (const [key, value] of Object.entries(params.body.filters)) { - filterQuery[key] = value - } - } + if (params.body?.filters) { + const disallowed = new Set(['tenant_code', 'organization_id']) + for (const [key, value] of Object.entries(params.body.filters)) { + if (!disallowed.has(key)) filterQuery[key] = value + } + }
662-665: Promise never settles on error; use reject.In catch, returning error leaves the Promise pending and hangs callers awaiting it.
Apply:
- console.log(error, 'error') - return error + console.log(error, 'error') + return reject(error)
687-687: Fix email template variable: pass domain string, not object.portalURL currently receives the entire tenantDomain object.
Apply:
- portalURL: tenantDomain, + portalURL: tenantDomain?.domain || '',
166-166: Retry attempts logic is inverted.1 || common.NO_OF_ATTEMPTS always yields 1, disabling configured retries.
Apply:
- attempts: 1 || common.NO_OF_ATTEMPTS, + attempts: common.NO_OF_ATTEMPTS || 1,
595-603: Guard against empty/invalid role assignments.findAll returns an array; falsy check won’t catch empty arrays and can wipe roles.
Apply:
- if (!getRoleIds) { + if (!getRoleIds?.length) { return responses.failureResponse({ message: 'INVALID_ROLE_ASSIGNMENTS', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
333-333: Correct typo in error code key
Replace the misspelledINAVLID_ORG_ROLE_REQwithINVALID_ORG_ROLE_REQin both locations to ensure the lookup remains consistent:
- src/services/org-admin.js (line 333)
- locales/en.json (line 118)
🧹 Nitpick comments (2)
src/services/org-admin.js (2)
631-667: Optional: simplify updateRoleForApprovedRequest to async function.Avoid the manual new Promise anti-pattern; let async/await handle it.
I can provide a clean refactor if you want it in this PR.
456-456: Remove debug log.console.log(rowsAffected, updatedUsers) is noisy in production.
Consider removing or guarding behind a debug flag.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/services/org-admin.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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 (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
src/services/org-admin.js (3)
src/controllers/v1/organization.js (2)
tenantCode(214-214)tenantCode(259-259)src/controllers/v1/public.js (1)
tenantCode(7-7)src/helpers/userInvite.js (1)
orgCode(1053-1061)
🔇 Additional comments (1)
src/services/org-admin.js (1)
646-654: Unify roleChange payload and verify consumer handling
- src/services/org-admin.js now emits
tenant_code/organization_code, but src/services/userInvite.js and src/helpers/userInvite.js still useorganization_id; align all producers.- Confirm downstream consumers ignore or correctly process the new
tenant_codeandorganization_codefields.
Summary by CodeRabbit