Conversation
WalkthroughUpdated sendRoleRequestStatusEmail in src/services/org-admin.js to consistently use organizationCode instead of orgCode for organization lookup and the organization_code field in both ACCEPTED and REJECTED branches. No function signatures or exports changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (2)
src/services/org-admin.js (2)
673-686: Email portalURL is set to the tenantDomain object instead of its domain stringtenantDomainQueries.findOne returns an object with a domain attribute, but the template variable portalURL is currently assigned the entire object. This will likely serialize to "[object Object]" or an unexpected structure in emails.
Apply this minimal diff to pass the string domain:
- const tenantDomain = await tenantDomainQueries.findOne( + const tenantDomain = await tenantDomainQueries.findOne( { tenant_code: tenantCode }, { attributes: ['domain'] } ) + const portalURL = tenantDomain?.domain || '' ... - portalURL: tenantDomain, + portalURL,
165-171: Queue retry attempts hardcoded to 1 due to1 || common.NO_OF_ATTEMPTS
1 || common.NO_OF_ATTEMPTSalways evaluates to 1, overriding configured retries and reducing reliability for bulk jobs.Use the configured attempts with a safe fallback:
- attempts: 1 || common.NO_OF_ATTEMPTS, + attempts: Number(common.NO_OF_ATTEMPTS) || 1,
🧹 Nitpick comments (3)
src/services/org-admin.js (3)
631-652: Unify terminology: rename orgCode parameter to organizationCode for clarityThis file uses organization_code/organizationCode almost everywhere else. Renaming the local parameter improves readability and reduces confusion.
-function updateRoleForApprovedRequest(requestDetails, user, tenantCode, orgCode) { +function updateRoleForApprovedRequest(requestDetails, user, tenantCode, organizationCode) { return new Promise(async (resolve, reject) => { try { const newRole = await roleQueries.findOne( { id: requestDetails.role, status: common.ACTIVE_STATUS, tenant_code: tenantCode }, { attributes: ['title', 'id', 'user_type', 'status'] } ) await userOrganizationRoleQueries.create({ tenant_code: tenantCode, user_id: user.id, - organization_code: orgCode, + organization_code: organizationCode, role_id: newRole.id, }) eventBroadcaster('roleChange', { requestBody: { user_id: requestDetails.requester_id, new_roles: [newRole.title], - current_roles: _.map(_.find(user.organizations, { code: orgCode })?.roles || [], 'title'), + current_roles: _.map(_.find(user.organizations, { code: organizationCode })?.roles || [], 'title'), }, })
331-337: Typo in message key 'INAVLID_ORG_ROLE_REQ'The constant looks misspelled; expected 'INVALID_ORG_ROLE_REQ'. If these keys map to i18n resources, changing it may require updating translations.
If it’s safe to correct, apply:
- message: 'INAVLID_ORG_ROLE_REQ', + message: 'INVALID_ORG_ROLE_REQ',
456-459: Avoid logging full updated user objectsconsole.log(rowsAffected, updatedUsers) may log PII (user IDs/emails). Prefer targeted, non-PII logs or a debug logger gated by environment.
- console.log(rowsAffected, updatedUsers) + console.debug('deactivateUser by IDs affected:', rowsAffected)
📜 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
🧬 Code graph analysis (1)
src/services/org-admin.js (1)
src/services/account.js (1)
organizationCode(194-194)
🔇 Additional comments (2)
src/services/org-admin.js (2)
696-702: Consistent use of organizationCode in REJECTED email — LGTMBoth orgName resolution and organization_code now consistently use organizationCode in the REJECTED branch, matching the ACCEPTED branch usage. No further action needed here.
375-381: No further action needed for organization_code availabilityThe authentication middleware (src/middlewares/authenticator.js) always extracts and injects
organization_code(fromdecodedToken.data.organizations[0].code) intoreq.decodedToken.dataalongsidetenant_code, so downstream service calls—including bothupdateRoleForApprovedRequestandsendRoleRequestStatusEmail—will always receive a validtokenInformation.organization_code. Missing or undefined values would already have been caught earlier in the middleware. Consequently, no additional checks are required here and this review comment can be resolved.
2 participants
Summary by CodeRabbit