kafka event for tenant #816
Conversation
WalkthroughAdds environment toggles and topics/listener keys for tenant, organization, and user events; adds tenant/org Kafka publishers; introduces DTO event payload builders and a change-detection util; emits structured create/update/domain-change events and renames broadcaster export to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Service as Service (tenant/org/admin/user)
participant Broadcaster as EventBroadcaster (broadcastEvent)
participant KafkaComm as KafkaComm
participant Kafka as Kafka
participant Listener as External Listener
Service->>Service: build event payload (DTO.eventBodyDTO)
Service->>Broadcaster: broadcastEvent(group, payload, isInternal?)
Broadcaster->>Broadcaster: isEventEnabled(group)?
alt enabled
Broadcaster->>KafkaComm: push<Group>EventsToKafka(payload)
KafkaComm->>Kafka: produce to configured topic
Broadcaster->>Listener: POST to configured listener API (if set)
else disabled
Broadcaster-->>Service: skip kafka/listener
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/.env.sample (1)
183-189: Clean up trailing whitespace, quoting, and ensure EOF newline.
- Remove trailing spaces on Lines 183 and 185.
- Drop quotes around values on Lines 187 and 189 for consistency with the rest of the file and to satisfy the linter.
- Add a newline at EOF.
Apply this diff:
-# kafka topic for health check -KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +# kafka topic for health check +KAFKA_HEALTH_CHECK_TOPIC=user-health-check-topic-check #Enable / disable tenant create event -EVENT_ENABLE_TENANT_EVENTS=true +EVENT_ENABLE_TENANT_EVENTS=true #Event Kafka topic for tenant create -EVENT_TENANT_KAFKA_TOPIC='dev.tenantCreate' +EVENT_TENANT_KAFKA_TOPIC=dev.tenantCreate #Event API for tenant create , can be comma separated -EVENT_TENANT_LISTENER_API='http://interface:3567/scp/v1/tenant/add' +EVENT_TENANT_LISTENER_API=http://interface:3567/scp/v1/tenant/add +Additionally, consider reordering keys to satisfy the UnorderedKey hints, but that’s optional for functionality.
src/services/tenant.js (2)
375-376: Avoid mutating the DB response object (minor).Prefer local consts over adding ad-hoc fields to tenantCreateResponse to keep shapes predictable.
Apply this diff:
-tenantCreateResponse.orgId = defaultOrgCreateResponse?.result?.id -tenantCreateResponse.orgCode = defaultOrgCreateResponse?.result?.code +const orgId = defaultOrgCreateResponse?.result?.id +const orgCode = defaultOrgCreateResponse?.result?.codeAnd use orgId/orgCode below when building the event.
408-409: Optionally await broadcast to catch failures.If event delivery matters for auditing/reliability,
await broadcastUserEvent(...)so failures are observable within this flow.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/.env.sample(1 hunks)src/envVariables.js(1 hunks)src/generics/kafka-communication.js(2 hunks)src/helpers/eventBroadcasterMain.js(3 hunks)src/services/tenant.js(3 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/tenant.js
🧬 Code graph analysis (2)
src/helpers/eventBroadcasterMain.js (5)
src/services/org-admin.js (1)
kafkaCommunication(15-15)src/services/organization.js (1)
kafkaCommunication(11-11)src/services/account.js (1)
kafkaCommunication(20-20)src/helpers/userInvite.js (2)
kafkaCommunication(17-17)requestBody(941-945)src/services/userInvite.js (2)
kafkaCommunication(24-24)requestBody(379-383)
src/services/tenant.js (2)
src/services/account.js (8)
require(26-26)require(27-27)require(37-37)require(41-41)UserTransformDTO(38-38)eventBody(604-623)eventBody(1754-1769)common(16-16)src/helpers/userInvite.js (8)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)eventBody(882-896)eventBody(1125-1130)common(8-8)
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 187-187: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 189-189: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 189-189: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
🪛 Biome (2.1.2)
src/helpers/eventBroadcasterMain.js
[error] 14-16: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
🔇 Additional comments (5)
src/generics/kafka-communication.js (2)
28-36: New tenant Kafka producer mirrors existing patterns — OK.Payload shape and error handling are consistent with user-events. Ensure EVENT_TENANT_KAFKA_TOPIC is set (see env validation comment).
60-65: Export added — OK.Exporting pushTenantEventsToKafka is correct and matches the new broadcaster routing.
src/envVariables.js (1)
431-439: EVENT_TENANT_LISTENER_API requirement — OK.Tying the HTTP listener requirement to EVENT_ENABLE_TENANT_EVENTS is appropriate.
src/services/tenant.js (1)
33-34: Imports — OK.Correct modules imported for broadcasting and DTO creation.
src/helpers/eventBroadcasterMain.js (1)
32-34: Enablement flag for tenant API events — OK.Matches the pattern used for other groups.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/organization.js (1)
137-149: Prevent duplicate organization event emission — remove or gate legacy callThe create flow calls eventBroadcasterMain('organizationEvents', eventBody) and then later calls broadcastUserEvent('organizationEvents', eventBodyData) (which itself invokes eventBroadcasterMain + Kafka), resulting in two organizationEvents being emitted. Remove the early legacy event or gate it behind an ENV flag (suggest: EVENT_ENABLE_LEGACY_ORG_EVENTS) so only one emission occurs.
Location: src/services/organization.js — legacy call at ~lines 137–149; new broadcastUserEvent call at ~line 171.
🧹 Nitpick comments (3)
src/.env.sample (1)
183-193: Fix dotenv nits: trailing whitespace, quotes, EOF newline.Removes trailing spaces, unquotes simple values/URLs, and ensures a final newline. Reordering keys can be done later to satisfy UnorderedKey warnings.
-# kafka topic for health check -KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +# kafka topic for health check +KAFKA_HEALTH_CHECK_TOPIC=user-health-check-topic-check #Enable / disable tenant create event -EVENT_ENABLE_TENANT_EVENTS=true +EVENT_ENABLE_TENANT_EVENTS=true #Event Kafka topic for tenant create -EVENT_TENANT_KAFKA_TOPIC='dev.tenantCreate' +EVENT_TENANT_KAFKA_TOPIC=dev.tenantCreate #Event API for tenant create , can be comma separated -EVENT_TENANT_LISTENER_API='http://interface:3567/scp/v1/tenant/add' +EVENT_TENANT_LISTENER_API=http://interface:3567/scp/v1/tenant/add #Enable / disable organization event -EVENT_ENABLE_ORGANIZATION_EVENTS=true +EVENT_ENABLE_ORGANIZATION_EVENTS=true #Event Kafka topic for organization create/update -EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +EVENT_ORGANIZATION_KAFKA_TOPIC=dev.organizationEvent +src/services/organization.js (2)
559-586: Potential double-publish on related_orgs add.You broadcast legacy
updateRelatedOrgsand also emit the new DTO-based update via orgEventEmitter. Confirm consumers won’t receive duplicates; otherwise gate/remove one path.
637-666: Potential double-publish on related_orgs remove.Same duplication risk as addRelatedOrg.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/.env.sample(1 hunks)src/dtos/organizationDTO.js(1 hunks)src/dtos/tenantDTO.js(1 hunks)src/envVariables.js(1 hunks)src/generics/kafka-communication.js(2 hunks)src/generics/utils.js(2 hunks)src/services/admin.js(3 hunks)src/services/organization.js(9 hunks)src/services/tenant.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/envVariables.js
- src/services/tenant.js
🧰 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/admin.jssrc/services/organization.js
🧬 Code graph analysis (5)
src/generics/utils.js (2)
src/database/migrations/20240502174600-add_email_templates.js (1)
updateData(177-177)src/services/organization.js (2)
changes(833-833)_(13-13)
src/services/admin.js (1)
src/services/organization.js (9)
organizationDTO(19-19)require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)eventBodyData(151-169)eventBodyData(845-865)common(2-2)
src/dtos/tenantDTO.js (3)
src/dtos/eventBody.js (1)
disallowedArgs(8-8)src/helpers/userInvite.js (1)
args(1105-1119)src/generics/utils.js (4)
changes(1096-1096)fieldName(1101-1101)oldValue(1099-1099)newValue(1100-1100)
src/dtos/organizationDTO.js (3)
src/dtos/eventBody.js (1)
disallowedArgs(8-8)src/helpers/userInvite.js (1)
args(1105-1119)src/generics/utils.js (5)
changes(1096-1096)result(402-402)fieldName(1101-1101)oldValue(1099-1099)newValue(1100-1100)
src/services/organization.js (4)
src/services/admin.js (9)
require(11-11)require(27-27)require(34-34)require(36-36)require(37-37)eventBodyData(705-720)organizationDTO(43-43)common(15-15)utils(17-17)src/services/account.js (6)
require(26-26)require(27-27)require(37-37)require(41-41)common(16-16)utils(25-25)src/database/queries/organization.js (3)
require(2-2)require(3-3)common(4-4)src/generics/utils.js (2)
common(20-20)changes(1096-1096)
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 187-187: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 189-189: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 191-191: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 191-191: [UnorderedKey] The EVENT_ENABLE_ORGANIZATION_EVENTS key should go before the EVENT_ENABLE_TENANT_EVENTS key
(UnorderedKey)
[warning] 193-193: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 193-193: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 193-193: [UnorderedKey] The EVENT_ORGANIZATION_KAFKA_TOPIC key should go before the EVENT_TENANT_KAFKA_TOPIC key
(UnorderedKey)
🔇 Additional comments (1)
src/services/admin.js (1)
703-723: Clarify: 'delete' event vs soft-deactivate & ensure deleted_at is deliveredsrc/services/admin.js (lines 703–723)
- Emits eventType: 'delete' but the DB change in this block only sets status = INACTIVE (no persisted deleted_at). Confirm contract: if this is a soft-deactivate, change eventType → 'update' and remove deleted/deleted_at from the payload; if it must be a delete, persist deleted_at and set status = DELETED before publishing.
- organizationDTO currently strips deleted_at (per DTO fix). Either have the DTO include deleted_at or stop adding deleted_at here so consumers get a consistent payload.
- Null-safety: guard updatedRows?.[0] (deletedOrgDetails) before dereferencing .id/.created_by.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
src/dtos/tenantDTO.js (2)
23-52: Duplicate code - same issue as organizationDTOThis method has the same implementation as
organizationDTO.eventBodyDTO, creating code duplication.Please refer to the comment in
organizationDTO.jsabout extracting common event body DTO logic.
37-38: Consider preserving undefined valuesSame issue as in organizationDTO - the current logic won't preserve legitimate falsy values.
src/envVariables.js (1)
418-430: Clarify toggle description and Kafka-topic requirement coupling.
- The message says “kafka event” but the flag gates tenant events generally. Update message to avoid confusion.
- Re: tying EVENT_TENANT_KAFKA_TOPIC to EVENT_ENABLE_TENANT_EVENTS (vs EVENT_ENABLE_KAFKA_PUSH) was discussed earlier; risk remains that Kafka is enabled without enforcing the topic.
- message: 'Key to toggle tenant creation kafka event', + message: 'Key to toggle tenant events (HTTP/Kafka)',Confirm broadcaster behavior when EVENT_ENABLE_KAFKA_PUSH=true but EVENT_ENABLE_TENANT_EVENTS=false to ensure no runtime missing-topic errors.
🧹 Nitpick comments (12)
src/.env.sample (5)
183-183: Remove trailing whitespaceLine 183 has trailing whitespace that should be removed.
-KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check'
185-185: Remove trailing whitespaceLine 185 has trailing whitespace that should be removed.
-EVENT_ENABLE_TENANT_EVENTS=true +EVENT_ENABLE_TENANT_EVENTS=true
187-187: Inconsistent quote usage in Kafka topic valuesThe Kafka topic values use single quotes while other string values in the file don't use quotes at all. Consider removing the quotes for consistency.
-EVENT_TENANT_KAFKA_TOPIC='dev.tenantCreate' +EVENT_TENANT_KAFKA_TOPIC=dev.tenantCreate-EVENT_TENANT_LISTENER_API='http://interface:3567/scp/v1/tenant/add' +EVENT_TENANT_LISTENER_API=http://interface:3567/scp/v1/tenant/add-EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +EVENT_ORGANIZATION_KAFKA_TOPIC=dev.organizationEventAlso applies to: 189-189, 193-193
191-191: Remove trailing whitespaceLine 191 has trailing whitespace that should be removed.
-EVENT_ENABLE_ORGANIZATION_EVENTS=true +EVENT_ENABLE_ORGANIZATION_EVENTS=true
193-193: Add blank line at end of fileThe file should end with a blank line for better compatibility with various tools.
EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +src/dtos/organizationDTO.js (2)
14-43: Consider code reuse to avoid duplicationThe
eventBodyDTOmethod is nearly identical to the one intenantDTO.js. This duplication could lead to maintenance issues if the logic needs to change in the future.Consider extracting the common event body DTO logic into a shared utility function that both
organizationDTOandtenantDTOcan use:// In a new file: src/dtos/eventUtils.js function createEventBodyDTO({ entity, eventType, entityId, changedValues = [], args = {} }) { try { if (!entity || !eventType || !entityId) throw new Error('Entity, EventType & EntityId values are mandatory for an Event') const disallowedArgs = ['deleted_at'] disallowedArgs.forEach((key) => { delete args[key] }) const changes = changedValues.reduce((result, obj) => { const { fieldName, oldValue, newValue } = obj if (!result[fieldName]) result[fieldName] = {} if (oldValue) result[fieldName].oldValue = oldValue if (newValue) result[fieldName].newValue = newValue return result }, {}) return { entity, eventType, entityId, changes, ...args, } } catch (error) { console.error(error) return false } } module.exports = { createEventBodyDTO }Then use it in both DTOs:
const { createEventBodyDTO } = require('./eventUtils') static eventBodyDTO(params) { return createEventBodyDTO(params) }
28-29: Consider preserving undefined valuesThe current logic only adds
oldValueandnewValueif they're truthy, which means it won't preserve legitimate falsy values like0,false, or empty strings.-if (oldValue) result[fieldName].oldValue = oldValue -if (newValue) result[fieldName].newValue = newValue +if (oldValue !== undefined) result[fieldName].oldValue = oldValue +if (newValue !== undefined) result[fieldName].newValue = newValuesrc/generics/utils.js (2)
1099-1101: Redundant variable assignmentThe
fieldNamevariable is assigned the same value askeyand could be eliminated.for (let key of Object.keys(updateData)) { let oldValue = oldData[key] let newValue = newData[key] - let fieldName = key // Compare only if key exists in updateData if (!_.isEqual(oldValue, newValue)) { - changes.push({ fieldName, oldValue, newValue }) + changes.push({ fieldName: key, oldValue, newValue }) } }
1095-1109: Consider handling null/undefined inputsThe function assumes
oldData,newData, andupdateDataare always objects. Consider adding defensive checks to prevent runtime errors.function getChanges(oldData, newData, updateData) { let changes = [] + + // Guard against null/undefined inputs + if (!updateData || typeof updateData !== 'object') { + return changes + } + + oldData = oldData || {} + newData = newData || {} for (let key of Object.keys(updateData)) {src/services/admin.js (1)
705-720: Consider adding event emission error handlingThe event emission for organization deletion should be wrapped in a try-catch to prevent failures from affecting the main deactivation flow.
//event Body for org create let deletedOrgDetails = orgRowsAffected.updatedRows?.[0] +if (deletedOrgDetails) { + try { const eventBodyData = organizationDTO.eventBodyDTO({ entity: 'organization', eventType: 'delete', entityId: deletedOrgDetails.id, args: { created_by: deletedOrgDetails.created_by, name: deletedOrgDetails.name, code: deletedOrgDetails.code, deleted_at: deletedOrgDetails?.deleted_at || new Date(), updated_at: deletedOrgDetails?.updated_at || new Date(), status: deletedOrgDetails?.status || common.INACTIVE_STATUS, deleted: true, id: deletedOrgDetails.id, tenant_code: tenantCode, }, }) broadcastUserEvent('organizationEvents', { requestBody: eventBodyData, isInternal: true }) + } catch (eventError) { + console.error('Failed to emit organization deletion event:', eventError) + // Don't throw - continue with the successful deactivation response + } +}src/services/tenant.js (1)
376-378: Minor: avoid mutating the DB response object.
Prefer local consts (orgId/orgCode) over augmenting tenantCreateResponse to reduce accidental coupling.-tenantCreateResponse.orgId = defaultOrgCreateResponse?.result?.id -tenantCreateResponse.orgCode = defaultOrgCreateResponse?.result?.code +const orgId = defaultOrgCreateResponse?.result?.id +const orgCode = defaultOrgCreateResponse?.result?.code…and use orgId/orgCode below.
src/envVariables.js (1)
431-439: Revisit requirement of EVENT_TENANT_LISTENER_API.
If Kafka-only deployments are allowed, making the HTTP listener required when enabling tenant events may overconstrain env setup.Consider a separate HTTP toggle (parity with users), or require the API URL only when HTTP is enabled.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/.env.sample(1 hunks)src/dtos/organizationDTO.js(1 hunks)src/dtos/tenantDTO.js(1 hunks)src/envVariables.js(1 hunks)src/generics/kafka-communication.js(2 hunks)src/generics/utils.js(2 hunks)src/helpers/eventBroadcasterMain.js(3 hunks)src/services/admin.js(3 hunks)src/services/organization.js(9 hunks)src/services/tenant.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/helpers/eventBroadcasterMain.js
- src/generics/kafka-communication.js
🧰 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/tenant.jssrc/services/admin.jssrc/services/organization.js
🧬 Code graph analysis (5)
src/dtos/organizationDTO.js (2)
src/dtos/eventBody.js (1)
disallowedArgs(8-8)src/generics/utils.js (5)
changes(1096-1096)result(402-402)fieldName(1101-1101)oldValue(1099-1099)newValue(1100-1100)
src/dtos/tenantDTO.js (2)
src/dtos/eventBody.js (1)
disallowedArgs(8-8)src/generics/utils.js (4)
changes(1096-1096)fieldName(1101-1101)oldValue(1099-1099)newValue(1100-1100)
src/generics/utils.js (2)
src/database/migrations/20240502174600-add_email_templates.js (1)
updateData(177-177)src/services/organization.js (2)
changes(833-833)_(13-13)
src/services/admin.js (1)
src/services/organization.js (9)
organizationDTO(19-19)require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)eventBodyData(151-169)eventBodyData(845-865)common(2-2)
src/services/organization.js (4)
src/services/admin.js (9)
require(11-11)require(27-27)require(34-34)require(36-36)require(37-37)eventBodyData(705-720)organizationDTO(43-43)common(15-15)utils(17-17)src/database/queries/organization.js (3)
require(2-2)require(3-3)common(4-4)src/generics/utils.js (2)
common(20-20)changes(1096-1096)src/dtos/eventBody.js (1)
changes(11-17)
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 187-187: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 189-189: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 191-191: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 191-191: [UnorderedKey] The EVENT_ENABLE_ORGANIZATION_EVENTS key should go before the EVENT_ENABLE_TENANT_EVENTS key
(UnorderedKey)
[warning] 193-193: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 193-193: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 193-193: [UnorderedKey] The EVENT_ORGANIZATION_KAFKA_TOPIC key should go before the EVENT_TENANT_KAFKA_TOPIC key
(UnorderedKey)
🔇 Additional comments (10)
src/generics/utils.js (1)
1095-1109: LGTM! Clean implementation for change trackingThe
getChangesfunction is well-implemented with proper deep comparison using lodash'sisEqualmethod. It correctly iterates only over the keys inupdateDatato track changes.src/services/admin.js (1)
656-666: Good implementation of event emission for organization deletionThe code correctly captures the updated organization details using the
returning: trueoption and emits the deletion event with appropriate metadata.src/services/tenant.js (2)
33-35: LGTM: imports align with new event flow.
388-407: Align payload shape and await broadcaster.
- Nest meta (avoid spreading) to prevent key collisions and match org event shape.
- ...(tenantCreateResponse.meta || {}), + meta: tenantCreateResponse?.meta || {}, - org_id: tenantCreateResponse.orgId, - org_code: tenantCreateResponse.orgCode, + org_id: orgId ?? tenantCreateResponse.orgId, + org_code: orgCode ?? tenantCreateResponse.orgCode,
- Await the broadcaster (or explicitly catch) to avoid unhandled rejections / dropped events under load.
-broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true }) +await broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true })Confirm TenantDTO.eventBodyDTO expects args.meta (nested) rather than flattened meta keys.
Location: src/services/tenant.js lines 388-407 (also applies to 409-410).
src/services/organization.js (6)
22-22: LGTM: shared broadcaster import matches usage elsewhere.
253-254: LGTM: centralizing update emissions via orgEventEmitter is clean.
559-571: LGTM: related_orgs update with returning:true/RAW for delta calc.
586-589: LGTM: emit after related_orgs append for accurate “after” state.
637-648: LGTM: removal path mirrors add path with returning:true for emitter.
664-667: LGTM: emits removal changes with provided related_orgs delta.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/generics/utils.js (1)
1095-1109: Null-safe keys iteration and consts (optional; aligns with prior discussion).Using default params doesn’t guard against
nullor non-objects. If service-level validation always guarantees an object (as noted), consider this a low-priority hardening; also preferconsthere.-function getChanges(oldData = {}, newData = {}, updateData = {}) { +function getChanges(oldData = {}, newData = {}, updateData = {}) { const changes = [] - for (const key of Object.keys(updateData)) { - let oldValue = oldData[key] - let newValue = newData[key] - let fieldName = key + for (const key of Object.keys(updateData || {})) { + const oldValue = oldData[key] + const newValue = newData[key] + const fieldName = key // Compare only if key exists in updateData if (!_.isEqual(oldValue, newValue)) { changes.push({ fieldName, oldValue, newValue }) } } return changes }src/services/admin.js (1)
668-668: Fix confirmed: correct rowsAffected check
if (!orgUpdateResult || orgUpdateResult.rowsAffected === 0)resolves the prior type/shape bug from earlier revisions.
🧹 Nitpick comments (4)
src/generics/utils.js (2)
1085-1093: Tighten JSDoc wording and param description.Clarify intent and what
updateDatarepresents./** - * Compare object and get the updated value with old value + * Compute per-field changes between oldData and newData, + * restricted to keys present in updateData. * @method * @name getChanges * @param {Object} oldData - Data before update * @param {Object} newData - Data after update - * @param {Object} updateData - reqBody data + * @param {Object} updateData - reqBody subset (keys to inspect) * @returns {Array<{fieldName:string, oldValue:any, newValue:any}>} */
1099-1105: Confirm array ordering semantics; optionally ignore order in equality.If any inspected fields are arrays where order is not meaningful,
_.isEqualwill flag reordering as a change. If order should be ignored, consider a custom comparator.- if (!_.isEqual(oldValue, newValue)) { + const equal = _.isEqualWith(oldValue, newValue, (a, b) => { + if (Array.isArray(a) && Array.isArray(b)) { + return _.isEqual([...a].sort(), [...b].sort()) + } + return undefined // fall back to default comparison + }) + if (!equal) { changes.push({ fieldName, oldValue, newValue }) }src/services/admin.js (2)
703-704: Nit: misleading commentThis block emits a deactivation event, not “org create.”
-//event Body for org create +// Event body for organization deactivation/update
705-721: Emit 'update' (not 'delete') for org soft-deactivateorganization.js emits org changes as eventType: 'update' and organizationDTO accepts empty changedValues; admin.js currently emits eventType: 'delete' when soft-deactivating — inconsistent and risks downstream consumers treating this as a hard delete. Change to an 'update' event, include changedValues ['status','deleted'] and updated_by.
Location: src/services/admin.js (around lines 704–719)
- let deletedOrgDetails = orgUpdateResult.updatedRows?.[0] - const eventBodyData = organizationDTO.eventBodyDTO({ - entity: 'organization', - eventType: 'delete', - entityId: deletedOrgDetails.id, - args: { - created_by: deletedOrgDetails.created_by, - name: deletedOrgDetails.name, - code: deletedOrgDetails.code, - deleted_at: deletedOrgDetails?.deleted_at || new Date(), - updated_at: deletedOrgDetails?.updated_at || new Date(), - status: deletedOrgDetails?.status || common.INACTIVE_STATUS, - deleted: true, - id: deletedOrgDetails.id, - tenant_code: tenantCode, - }, - }) + const updatedOrgDetails = orgUpdateResult.updatedRows?.[0] + const eventBodyData = organizationDTO.eventBodyDTO({ + entity: 'organization', + eventType: 'update', + changedValues: ['status', 'deleted'], + entityId: updatedOrgDetails.id, + args: { + created_by: updatedOrgDetails.created_by, + updated_by: loggedInUserId, + name: updatedOrgDetails.name, + code: updatedOrgDetails.code, + deleted_at: updatedOrgDetails?.deleted_at || new Date(), + updated_at: updatedOrgDetails?.updated_at || new Date(), + status: updatedOrgDetails?.status || common.INACTIVE_STATUS, + deleted: true, + id: updatedOrgDetails.id, + tenant_code: tenantCode, + }, + })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/generics/utils.js(2 hunks)src/services/admin.js(3 hunks)src/services/organization.js(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/organization.js
🧰 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/admin.js
🧠 Learnings (9)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.442Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.442Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.795Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:35:28.237Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.237Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.237Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.237Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:40:34.795Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.795Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.237Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.237Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:01:49.544Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.544Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:02:11.442Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.442Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:02:11.442Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.442Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:03:00.511Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.511Z
Learning: In the ELEVATE-Project/user codebase, the oldValue and newValue properties in changedValues objects passed to tenantDTO.eventBodyDTO are guaranteed to be defined when present, so undefined checks are not necessary according to the data contracts.
Applied to files:
src/generics/utils.js
🧬 Code graph analysis (2)
src/services/admin.js (1)
src/services/organization.js (10)
organizationDTO(19-19)require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)organizationQueries(3-3)common(2-2)eventBodyData(151-169)eventBodyData(845-865)
src/generics/utils.js (1)
src/services/organization.js (2)
changes(833-833)_(13-13)
🔇 Additional comments (4)
src/generics/utils.js (1)
1157-1158: Export addition looks good.
getChangesis correctly added tomodule.exports.src/services/admin.js (3)
43-43: LGTM: organization DTO importImport aligns with usage below.
656-666: Good: normalized update() usage with returning/rawSwitch to
orgUpdateResult+{ returning: true, raw: true }is correct for fetching the updated row.
722-723: LGTM: broadcast routePublishing to
organizationEventswithisInternal: truematches the pattern used in organization service.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/tenant.js (1)
725-731: Fix success message for domain removal.Message says “ADDED” after removal. Update to “REMOVED”.
- message: 'TENANT_DOMAINS_ADDED_SUCCESSFULLY', + message: 'TENANT_DOMAINS_REMOVED_SUCCESSFULLY',
🧹 Nitpick comments (3)
src/.env.sample (2)
183-193: Trim trailing whitespace and add final newline.Fix dotenv-linter warnings by removing trailing spaces on Lines 183, 185, 191 and ensuring the file ends with a newline.
Apply this diff:
-KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' -#Enable / disable tenant create event -EVENT_ENABLE_TENANT_EVENTS=true +#Enable / disable tenant create event +EVENT_ENABLE_TENANT_EVENTS=true #Event Kafka topic for tenant create EVENT_TENANT_KAFKA_TOPIC='dev.tenantEvent' #Event API for tenant create , can be comma separated EVENT_TENANT_LISTENER_API='http://interface:3567/scp/v1/tenant/add' -#Enable / disable organization event -EVENT_ENABLE_ORGANIZATION_EVENTS=true +#Enable / disable organization event +EVENT_ENABLE_ORGANIZATION_EVENTS=true #Event Kafka topic for organization create/update EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +
191-193: Clarify org event toggles to avoid ambiguity.Both EVENT_ENABLE_ORG_EVENTS (Line 118) and EVENT_ENABLE_ORGANIZATION_EVENTS (Line 191) exist. This is confusing for operators. Pick one canonical variable (prefer the newer EVENT_ENABLE_ORGANIZATION_EVENTS) and deprecate the other with a compatibility fallback in src/envVariables.js; update .env.sample and docs accordingly.
src/services/tenant.js (1)
409-409: Consider awaiting event broadcast (or explicitly fire-and-forget).If broadcastUserEvent returns a Promise, not awaiting risks unhandled rejections and lost errors. Either await it or explicitly mark as intentional fire-and-forget.
- broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true }) + await broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true }) + // If you prefer non-blocking: + // void broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/.env.sample(1 hunks)src/services/admin.js(3 hunks)src/services/tenant.js(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/admin.js
🧰 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/tenant.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/tenant.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/.env.sample
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/.env.sample
📚 Learning: 2025-09-12T10:01:49.571Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.571Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/.env.sample
🧬 Code graph analysis (1)
src/services/tenant.js (5)
src/services/admin.js (7)
require(11-11)eventBody(64-81)eventBody(208-219)common(15-15)rowsAffected(772-775)eventBodyData(705-720)utils(17-17)src/services/account.js (8)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)common(16-16)utils(25-25)src/services/organization.js (6)
eventBody(138-146)common(2-2)changes(833-833)eventBodyData(151-169)eventBodyData(845-865)utils(5-5)src/generics/utils.js (2)
common(20-20)changes(1096-1096)src/database/queries/organization.js (3)
rowsAffected(104-104)rowsAffected(143-143)rowsAffected(164-164)
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 187-187: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 189-189: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 191-191: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 191-191: [UnorderedKey] The EVENT_ENABLE_ORGANIZATION_EVENTS key should go before the EVENT_ENABLE_TENANT_EVENTS key
(UnorderedKey)
[warning] 193-193: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 193-193: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 193-193: [UnorderedKey] The EVENT_ORGANIZATION_KAFKA_TOPIC key should go before the EVENT_TENANT_KAFKA_TOPIC key
(UnorderedKey)
🔇 Additional comments (5)
src/services/tenant.js (5)
33-35: LGTM: imports for event broadcasting and DTO.
376-377: LGTM: propagate default org identifiers to tenant response.This enables including org_id/org_code in the create event payload.
558-584: LGTM: domain-add emits precise update event with changes diff.The changes array correctly captures domains old/new values and payload fields look consistent.
698-724: LGTM: domain-remove emits update event with correct diff.
878-901: LGTM: centralized tenantEventEmitter and empty-change handling.getChanges + DTO usage mirrors org flow and will handle empty changedValues.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/services/tenant.js (2)
388-407: Don’t flatten meta; await the emitter.
Flatteningmetainto args risks key collisions and schema drift. Also await the broadcaster to surface failures.const eventBody = TenantDTO.eventBodyDTO({ entity: 'tenant', eventType: 'create', entityId: tenantCreateResponse.code, args: { created_by: tenantCreateResponse.created_by, name: tenantCreateResponse.name, code: tenantCreateResponse.code, created_at: tenantCreateResponse?.created_at || new Date(), updated_at: tenantCreateResponse?.updated_at || new Date(), - ...(tenantCreateResponse.meta || {}), + meta: tenantCreateResponse?.meta || {}, status: tenantCreateResponse?.status || common.ACTIVE_STATUS, deleted: false, org_id: tenantCreateResponse.orgId, org_code: tenantCreateResponse.orgCode, description: tenantCreateResponse.description, logo: tenantCreateResponse.logo, }, }) - - broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true }) + await broadcastUserEvent('tenantEvents', { requestBody: eventBody, isInternal: true })Also applies to: 409-409
458-476: Fix crash when no fields change (destructuring undefined).
If nothing changes,updatedTenantDetailsis undefined and destructuring throws. Emit the event with the original state and a null/empty updated row.- let updatedTenantDetails + let updatedFirstRow // update only if the data is changed if (Object.keys(tenantUpdateBody).length > 0) { tenantUpdateBody.updated_by = userId - updatedTenantDetails = await tenantQueries.update( + const [rowsAffected, updatedRows] = await tenantQueries.update( { code: tenantCode, }, tenantUpdateBody, { returning: true, raw: true, } ) + updatedFirstRow = updatedRows?.[0] } - - const [rowsAffected, updatedRows] = updatedTenantDetails - - await tenantEventEmitter(tenantDetails, updatedRows?.[0], bodyData) + await tenantEventEmitter(tenantDetails, updatedFirstRow, bodyData)src/services/organization.js (1)
151-172: Prevent duplicate create emissions; keep only the new path (awaited).
You still invoke the legacyeventBroadcasterMain('organizationEvents', ...)above while also callingbroadcastUserEvent(...)here. This double-sends create events.
- Remove or feature‑flag the earlier
eventBroadcasterMaincall and drop its now-unused import.- Keep
await broadcastUserEvent(...)(good).
🧹 Nitpick comments (12)
src/dtos/organizationDTO.js (1)
19-23: Avoid mutating args; prevent reserved-key overrides in the response.Delete from a cloned copy to avoid side effects, and place args before reserved fields so args cannot override entity/eventType/entityId/changes.
- const disallowedArgs = ['deleted_at'] - - disallowedArgs.forEach((key) => { - delete args[key] - }) + const disallowedArgs = ['deleted_at'] + const sanitizedArgs = { ...args } + disallowedArgs.forEach((key) => { + delete sanitizedArgs[key] + }) @@ - return { - entity, - eventType, - entityId, - changes, - ...args, - } + return { + ...sanitizedArgs, + entity, + eventType, + entityId, + changes, + }Also applies to: 32-38
src/.env.sample (1)
183-193: Fix trailing whitespace and add newline at EOF; optional: satisfy dotenv-linter ordering.Removes lint noise; no behavior change.
-# kafka topic for health check -KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +# kafka topic for health check +KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' @@ -EVENT_ENABLE_TENANT_EVENTS=true +EVENT_ENABLE_TENANT_EVENTS=true @@ -EVENT_ENABLE_ORGANIZATION_EVENTS=true +EVENT_ENABLE_ORGANIZATION_EVENTS=true @@ -EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +Optional (no diff): consider reordering keys to satisfy UnorderedKey warnings.
src/dtos/tenantDTO.js (1)
28-32: Mirror DTO safety: don’t mutate args; ensure reserved fields can’t be overridden.Keeps behavior identical to org DTO and avoids side effects.
- const disallowedArgs = ['deleted_at'] - - disallowedArgs.forEach((key) => { - delete args[key] - }) + const disallowedArgs = ['deleted_at'] + const sanitizedArgs = { ...args } + disallowedArgs.forEach((key) => { + delete sanitizedArgs[key] + }) @@ - return { - entity, - eventType, - entityId, - changes, - ...args, - } + return { + ...sanitizedArgs, + entity, + eventType, + entityId, + changes, + }Also applies to: 41-47
src/generics/utils.js (1)
1099-1102: Use const for loop-scoped bindings.Minor readability/immutability win.
- let oldValue = oldData[key] - let newValue = newData[key] - let fieldName = key + const oldValue = oldData[key] + const newValue = newData[key] + const fieldName = keysrc/envVariables.js (3)
418-421: Clarify toggle scope in message.This flag is used to gate both API and Kafka tenant events in code; message says “kafka” only. Update to avoid ops confusion.
- message: 'Key to toggle tenant creation kafka event', + message: 'Key to toggle tenant events (API and Kafka)',
440-443: Clarify org toggle scope in message.Align message with usage intent (Kafka and/or future API).
- message: 'Key to toggle organization creation kafka event', + message: 'Key to toggle organization events (Kafka)',
418-452: Normalize event gating toggles or document intentional asymmetry.Found mixed toggles — EVENT_ENABLE_USER_EVENTS, EVENT_ENABLE_KAFKA_PUSH, and EVENT_ENABLE_TENANT_EVENTS (src/helpers/eventBroadcasterMain.js:30–34; src/envVariables.js:395,418). Either consolidate to a consistent naming strategy (e.g., per-entity EVENT_ENABLE__KAFKA_PUSH or a single global EVENT_ENABLE_KAFKA_PUSH) or add inline/docs explaining why the asymmetry is intentional.
src/helpers/eventBroadcasterMain.js (1)
65-79: Await Kafka sends to surface failures to callers.Without await, Promise.allSettled in broadcastUserEvent may report success even if send fails later.
switch (eventGroup) { case 'organizationEvents': - kafkaCommunication.pushOrganizationEventsToKafka(requestBody) + await kafkaCommunication.pushOrganizationEventsToKafka(requestBody) break case 'userEvents': - kafkaCommunication.pushUserEventsToKafka(requestBody) + await kafkaCommunication.pushUserEventsToKafka(requestBody) break case 'tenantEvents': - kafkaCommunication.pushTenantEventsToKafka(requestBody) + await kafkaCommunication.pushTenantEventsToKafka(requestBody) breaksrc/services/tenant.js (3)
558-584: Await tenantEvents broadcast (addDomain).
Ensure failures surface and maintain ordering with DB changes.- broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true })
698-724: Await emitter and fix success message (removeDomain).
- Await the broadcast.
- Response message says “ADDED” for a removal.
- broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) @@ - message: 'TENANT_DOMAINS_ADDED_SUCCESSFULLY', + message: 'TENANT_DOMAINS_REMOVED_SUCCESSFULLY',Also applies to: 725-731
878-901: Await tenantEvents broadcast in tenantEventEmitter.
Keep parity with org flows; avoid silent failures.- broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true })src/services/organization.js (1)
831-867: Await orgEvents broadcast in orgEventEmitter.
Match create flow; avoid silent failures.- broadcastUserEvent('organizationEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserEvent('organizationEvents', { requestBody: eventBodyData, isInternal: true })Given prior agreement to fail fast in org event args, consider removing fallback defaults (e.g.,
|| {}/|| new Date()) in this args block to surface missing data early.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/.env.sample(1 hunks)src/dtos/organizationDTO.js(1 hunks)src/dtos/tenantDTO.js(1 hunks)src/envVariables.js(1 hunks)src/generics/kafka-communication.js(2 hunks)src/generics/utils.js(2 hunks)src/helpers/eventBroadcasterMain.js(3 hunks)src/services/admin.js(3 hunks)src/services/organization.js(9 hunks)src/services/tenant.js(7 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/organization.jssrc/services/tenant.jssrc/services/admin.js
🧠 Learnings (19)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the changedValues parameter passed to tenantDTO.eventBodyDTO is guaranteed to be an array by the calling code, so array validation is not needed.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.js
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.jssrc/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:36:43.529Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:26-29
Timestamp: 2025-09-12T10:36:43.529Z
Learning: In the ELEVATE-Project/user codebase, the changedValues array passed to organizationDTO.eventBodyDTO and tenantDTO.eventBodyDTO always contains a fieldName property when oldValue or newValue are present, so additional fieldName validation is not needed according to the business logic.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the tenantDTO.eventBodyDTO method should always filter out 'deleted_at' from args regardless of event type, as this field should not be allowed in any event payload according to the business rules.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, oldValue and newValue in the changedValues array for organizationDTO.eventBodyDTO are guaranteed to not be undefined by the business logic, and filtering out falsy values (0, false, '') from changes is the intended behavior.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the oldValue and newValue properties in changedValues objects passed to tenantDTO.eventBodyDTO are guaranteed to be defined when present, so undefined checks are not necessary according to the data contracts.
Applied to files:
src/dtos/tenantDTO.jssrc/generics/utils.jssrc/dtos/organizationDTO.jssrc/services/organization.jssrc/services/tenant.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO method intentionally filters out deleted_at from args for all event types (create, update, delete), not just non-delete events. This is by design according to the business requirements.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the changedValues parameter in organizationDTO.eventBodyDTO is guaranteed to always be an array by the calling code, so additional Array.isArray validation is not needed.
Applied to files:
src/dtos/tenantDTO.jssrc/dtos/organizationDTO.jssrc/services/organization.js
📚 Learning: 2025-07-31T08:44:36.982Z
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.
Applied to files:
src/dtos/tenantDTO.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/dtos/tenantDTO.jssrc/.env.samplesrc/helpers/eventBroadcasterMain.jssrc/generics/kafka-communication.jssrc/services/admin.jssrc/envVariables.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/.env.samplesrc/helpers/eventBroadcasterMain.jssrc/generics/kafka-communication.jssrc/services/admin.jssrc/envVariables.js
📚 Learning: 2025-09-12T10:01:49.571Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.571Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/.env.samplesrc/helpers/eventBroadcasterMain.jssrc/generics/kafka-communication.jssrc/services/admin.jssrc/envVariables.js
📚 Learning: 2025-09-11T23:15:19.523Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:14-17
Timestamp: 2025-09-11T23:15:19.523Z
Learning: In the ELEVATE-Project/user codebase, switch cases typically use return statements without break statements when each case returns a value directly. However, when a case has conditional returns, it should include a fallback return statement to prevent unintended fallthrough behavior.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T10:14:22.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-867
Timestamp: 2025-09-12T10:14:22.455Z
Learning: In the ELEVATE-Project/user codebase, the related_orgs field is stored as an array in the database by default, so additional array validation checks are unnecessary when processing related organization data.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/organization.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/organization.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:38:57.594Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:38:57.594Z
Learning: In the ELEVATE-Project/user codebase, when building event payloads in orgEventEmitter function, the preference is to use direct property access without nullish coalescing operators (??) for args object properties. This ensures fail-fast behavior - if expected document properties are missing, the system should throw an error rather than silently falling back to default values, helping to catch data integrity issues early.
Applied to files:
src/services/organization.js
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 187-187: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 189-189: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 189-189: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 191-191: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 191-191: [UnorderedKey] The EVENT_ENABLE_ORGANIZATION_EVENTS key should go before the EVENT_ENABLE_TENANT_EVENTS key
(UnorderedKey)
[warning] 193-193: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 193-193: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 193-193: [UnorderedKey] The EVENT_ORGANIZATION_KAFKA_TOPIC key should go before the EVENT_TENANT_KAFKA_TOPIC key
(UnorderedKey)
🔇 Additional comments (11)
src/generics/kafka-communication.js (2)
28-36: LGTM: tenant Kafka publisher mirrors existing pattern.
38-49: LGTM: org Kafka publisher mirrors existing pattern.src/helpers/eventBroadcasterMain.js (1)
14-18: Endpoints switch: good fallback handling.Returning [] prevents fallthrough; matches project pattern.
src/services/admin.js (2)
656-666: LGTM: use returning+raw and capture update result.Sets status to INACTIVE and records updater; consistent with deactivation.
668-674: Guard fix is correct.rowsAffected check now matches returned shape.
src/services/tenant.js (2)
33-35: LGTM: new imports are appropriate.
376-377: LGTM: carry forward orgId/orgCode from default org.src/services/organization.js (4)
22-22: LGTM: import for unified broadcaster.
252-254: LGTM: unified org update emission via orgEventEmitter.
559-571: LGTM: related_orgs add emits via updatedRows[0] and orgEventEmitter.Also applies to: 586-589
637-648: LGTM: related_orgs remove emits with proper state.Also applies to: 664-666
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/services/admin.js (1)
703-721: Includeupdated_by(and consider minimal parity fields) in org update event.
Addupdated_by: loggedInUserIdfor traceability. Optionally adddeleted: falseandcreated_atfor parity with other org update events.Apply:
- const eventBodyData = organizationDTO.eventBodyDTO({ + const eventBodyData = organizationDTO.eventBodyDTO({ entity: 'organization', eventType: 'update', entityId: updatedOrgDetails.id, args: { - created_by: updatedOrgDetails.created_by, + created_by: updatedOrgDetails.created_by, + updated_by: loggedInUserId, name: updatedOrgDetails.name, code: updatedOrgDetails.code, - updated_at: updatedOrgDetails?.updated_at || new Date(), + updated_at: updatedOrgDetails?.updated_at || new Date(), + // optional parity with other update events: + // created_at: updatedOrgDetails?.created_at || null, + // deleted: false, status: updatedOrgDetails?.status || common.INACTIVE_STATUS, id: updatedOrgDetails.id, tenant_code: tenantCode, }, })Note: per prior learning,
changedValuesis optional; DTO handles empty changes arrays.src/helpers/eventBroadcasterMain.js (2)
68-82: Await Kafka dispatch to surface failures to caller.
Withoutawait, errors won’t be caught byeventBroadcasterKafkaand won’t appear inbroadcastUserEvent’sallSettled.case 'organizationEvents': - kafkaCommunication.pushOrganizationEventsToKafka(requestBody) + await kafkaCommunication.pushOrganizationEventsToKafka(requestBody) break case 'userEvents': - kafkaCommunication.pushUserEventsToKafka(requestBody) + await kafkaCommunication.pushUserEventsToKafka(requestBody) break case 'tenantEvents': - kafkaCommunication.pushTenantEventsToKafka(requestBody) + await kafkaCommunication.pushTenantEventsToKafka(requestBody) break
55-58: Bug: logging endpoint URL from string array.
endPointsis an array of strings;.urlis undefined. Log the string directly.- console.error(`Error for endpoint ${endPoints[index].url}:`, result.reason) + console.error(`Error for endpoint ${endPoints[index]}:`, result.reason)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/helpers/eventBroadcasterMain.js(3 hunks)src/services/admin.js(3 hunks)src/services/tenant.js(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/tenant.js
🧰 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/admin.js
🧠 Learnings (11)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:01:49.571Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.571Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/services/admin.jssrc/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/services/admin.jssrc/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/services/admin.jssrc/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, oldValue and newValue in the changedValues array for organizationDTO.eventBodyDTO are guaranteed to not be undefined by the business logic, and filtering out falsy values (0, false, '') from changes is the intended behavior.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO method intentionally filters out deleted_at from args for all event types (create, update, delete), not just non-delete events. This is by design according to the business requirements.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-11T23:15:19.523Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:14-17
Timestamp: 2025-09-11T23:15:19.523Z
Learning: In the ELEVATE-Project/user codebase, switch cases typically use return statements without break statements when each case returns a value directly. However, when a case has conditional returns, it should include a fallback return statement to prevent unintended fallthrough behavior.
Applied to files:
src/helpers/eventBroadcasterMain.js
🧬 Code graph analysis (2)
src/services/admin.js (1)
src/services/organization.js (10)
organizationDTO(19-19)require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)organizationQueries(3-3)common(2-2)eventBodyData(151-169)eventBodyData(845-865)
src/helpers/eventBroadcasterMain.js (5)
src/services/organization.js (1)
kafkaCommunication(11-11)src/services/org-admin.js (1)
kafkaCommunication(15-15)src/services/account.js (1)
kafkaCommunication(20-20)src/helpers/userInvite.js (1)
kafkaCommunication(17-17)src/services/userInvite.js (1)
kafkaCommunication(24-24)
🔇 Additional comments (6)
src/services/admin.js (3)
43-43: Good: DTO import aligns with new event flow.
Consistent with org events usage elsewhere.
656-666: Update result handling: correct use of{ returning: true, raw: true }.
Switch toorgUpdateResultwith return metadata is appropriate.
668-673: Guard fixed: validaterowsAffected.
The failure path now triggers correctly when no rows updated.src/helpers/eventBroadcasterMain.js (3)
14-18: Good: fallbackreturn []prevents switch fallthrough.
Matches the pattern used fororganizationEvents.
30-31: Enablement gate for tenant events looks good.
Consistent with env-driven toggles.
34-37: Incorrect: env var mismatch claim — both ORG and ORGANIZATION flags existenvVariables.js defines EVENT_ENABLE_ORG_EVENTS (line 188) and EVENT_ENABLE_ORGANIZATION_EVENTS (line 440); src/helpers/eventBroadcasterMain.js uses EVENT_ENABLE_ORG_EVENTS (lines ~27, ~37). The original comment claiming a missing/canonical ORGANIZATION flag is incorrect.
- Decide whether EVENT_ENABLE_ORG_EVENTS is an intended alias; if not, standardize usages to EVENT_ENABLE_ORGANIZATION_EVENTS (e.g., update src/helpers/eventBroadcasterMain.js).
- Confirm whether Kafka gating should be per-group or use the global EVENT_ENABLE_KAFKA_PUSH (userEvents-kafka currently uses the global flag).
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/helpers/userInvite.js (1)
1196-1200: Fix undefined variable newUserCred on failure path.This throws a ReferenceError at runtime.
- isErrorOccured = true - await userInviteQueries.deleteOne(newInvitee.id) - invitee.statusOrUserId = newUserCred + isErrorOccured = true + await userInviteQueries.deleteOne(newInvitee.id) + invitee.statusOrUserId = 'USER_CREATION_FAILED'src/helpers/eventBroadcasterMain.js (1)
56-58: Logging bug: endpoint is a string, not an object.
endPoints[index].urlis undefined. Log the string.- console.error(`Error for endpoint ${endPoints[index].url}:`, result.reason) + console.error(`Error for endpoint ${endPoints[index]}:`, result.reason)src/services/tenant.js (1)
724-731: Wrong success message for removeDomain.Returns “TENANT_DOMAINS_ADDED_SUCCESSFULLY” on removal.
- message: 'TENANT_DOMAINS_ADDED_SUCCESSFULLY', + message: 'TENANT_DOMAINS_REMOVED_SUCCESSFULLY',src/envVariables.js (1)
407-411: UseENABLE_USER_KAFKA_EVENTSin requiredIf
The keyENABLE_USER_KAFKA_PUSHdoesn’t exist; update it toENABLE_USER_KAFKA_EVENTS.requiredIf: { - key: 'ENABLE_USER_KAFKA_PUSH', + key: 'ENABLE_USER_KAFKA_EVENTS', operator: 'EQUALS', value: 'true', },
🧹 Nitpick comments (9)
src/.env.sample (2)
183-191: Fix dotenv lint warnings: trim trailing spaces and add final newline.Remove trailing whitespace on lines 183, 185, 187, 189, 191, 197 and add a newline at EOF to satisfy dotenv-linter.
- KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' +KAFKA_HEALTH_CHECK_TOPIC='user-health-check-topic-check' @@ -EVENT_ENABLE_TENANT_EVENTS=true +EVENT_ENABLE_TENANT_EVENTS=true @@ -ENABLE_TENANT_KAFKA_EVENTS=true +ENABLE_TENANT_KAFKA_EVENTS=true @@ -ENABLE_USER_KAFKA_EVENTS=true +ENABLE_USER_KAFKA_EVENTS=true @@ -ENABLE_ORG_KAFKA_EVENTS=true +ENABLE_ORG_KAFKA_EVENTS=true @@ -EVENT_ENABLE_ORGANIZATION_EVENTS=true +EVENT_ENABLE_ORGANIZATION_EVENTS=true @@ -EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +EVENT_ORGANIZATION_KAFKA_TOPIC='dev.organizationEvent' +Also applies to: 197-199
185-199: Consolidate event and Kafka toggle flags
- Rename
EVENT_ENABLE_ORGANIZATION_EVENTSin.env.sampleto match code’sEVENT_ENABLE_ORG_EVENTS.- Remove redundant Kafka switches: rely on the single
EVENT_ENABLE_KAFKA_PUSHplus per-domainENABLE_<DOMAIN>_KAFKA_EVENTS.src/services/admin.js (1)
703-721: Include updated_by in org deactivation event for auditability.Emit who performed the change.
- let updatedOrgDetails = orgUpdateResult.updatedRows?.[0] + let updatedOrgDetails = orgUpdateResult.updatedRows?.[0] const eventBodyData = organizationDTO.eventBodyDTO({ entity: 'organization', eventType: 'update', entityId: updatedOrgDetails.id, args: { - created_by: updatedOrgDetails.created_by, + created_by: updatedOrgDetails.created_by, + updated_by: loggedInUserId, name: updatedOrgDetails.name, code: updatedOrgDetails.code, updated_at: updatedOrgDetails?.updated_at || new Date(), status: updatedOrgDetails?.status || common.INACTIVE_STATUS, id: updatedOrgDetails.id, tenant_code: tenantCode, }, })src/helpers/eventBroadcasterMain.js (1)
8-10: Fix trimming: filter doesn't trim the values.You’re filtering by truthiness of
url.trim()but not trimming the returned strings. Map to trim, then filter.- if (process.env.EVENT_ORG_LISTENER_URLS) - return process.env.EVENT_ORG_LISTENER_URLS.split(',').filter((url) => url.trim()) + if (process.env.EVENT_ORG_LISTENER_URLS) + return process.env.EVENT_ORG_LISTENER_URLS.split(',').map((url) => url.trim()).filter(Boolean) - if (process.env.EVENT_USER_LISTENER_API) - return process.env.EVENT_USER_LISTENER_API.split(',').filter((url) => url.trim()) + if (process.env.EVENT_USER_LISTENER_API) + return process.env.EVENT_USER_LISTENER_API.split(',').map((url) => url.trim()).filter(Boolean) - if (process.env.EVENT_TENANT_LISTENER_API) - return process.env.EVENT_TENANT_LISTENER_API.split(',').filter((url) => url.trim()) + if (process.env.EVENT_TENANT_LISTENER_API) + return process.env.EVENT_TENANT_LISTENER_API.split(',').map((url) => url.trim()).filter(Boolean)Also applies to: 12-14, 16-18
src/services/tenant.js (4)
409-410: Await the broadcaster for consistency and error surfacing.Aligns with org flow and ensures failures are visible.
- broadcastUserServiceEvent('tenantEvents', { requestBody: eventBody, isInternal: true }) + await broadcastUserServiceEvent('tenantEvents', { requestBody: eventBody, isInternal: true })
565-583: Await tenant update event (domain add).Ensure event sending completes and errors are handled.
- broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true })
705-723: Await tenant update event (domain remove).- broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true })
877-900: Await broadcaster inside tenantEventEmitter.Calling sites await the emitter; without awaiting the internal broadcast, they still return before emit completes.
- broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserServiceEvent('tenantEvents', { requestBody: eventBodyData, isInternal: true })src/services/organization.js (1)
831-867: Await broadcaster in orgEventEmitter.Prevents returning before the event is sent.
- broadcastUserServiceEvent('organizationEvents', { requestBody: eventBodyData, isInternal: true }) + await broadcastUserServiceEvent('organizationEvents', { requestBody: eventBodyData, isInternal: true })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/.env.sample(1 hunks)src/envVariables.js(2 hunks)src/generics/utils.js(2 hunks)src/helpers/eventBroadcasterMain.js(4 hunks)src/helpers/userInvite.js(3 hunks)src/services/account.js(3 hunks)src/services/admin.js(6 hunks)src/services/organization.js(9 hunks)src/services/tenant.js(7 hunks)src/services/user.js(2 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/user.jssrc/services/account.jssrc/services/organization.jssrc/services/tenant.jssrc/services/admin.js
🧠 Learnings (18)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the oldValue and newValue properties in changedValues objects passed to tenantDTO.eventBodyDTO are guaranteed to be defined when present, so undefined checks are not necessary according to the data contracts.
Applied to files:
src/generics/utils.jssrc/services/organization.jssrc/services/tenant.js
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:14:22.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-867
Timestamp: 2025-09-12T10:14:22.455Z
Learning: In the ELEVATE-Project/user codebase, the related_orgs field is stored as an array in the database by default, so additional array validation checks are unnecessary when processing related organization data.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, oldValue and newValue in the changedValues array for organizationDTO.eventBodyDTO are guaranteed to not be undefined by the business logic, and filtering out falsy values (0, false, '') from changes is the intended behavior.
Applied to files:
src/services/organization.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/organization.jssrc/services/tenant.jssrc/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the changedValues parameter in organizationDTO.eventBodyDTO is guaranteed to always be an array by the calling code, so additional Array.isArray validation is not needed.
Applied to files:
src/services/organization.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:36:43.529Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:26-29
Timestamp: 2025-09-12T10:36:43.529Z
Learning: In the ELEVATE-Project/user codebase, the changedValues array passed to organizationDTO.eventBodyDTO and tenantDTO.eventBodyDTO always contains a fieldName property when oldValue or newValue are present, so additional fieldName validation is not needed according to the business logic.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T10:38:57.594Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:38:57.594Z
Learning: In the ELEVATE-Project/user codebase, when building event payloads in orgEventEmitter function, the preference is to use direct property access without nullish coalescing operators (??) for args object properties. This ensures fail-fast behavior - if expected document properties are missing, the system should throw an error rather than silently falling back to default values, helping to catch data integrity issues early.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO method intentionally filters out deleted_at from args for all event types (create, update, delete), not just non-delete events. This is by design according to the business requirements.
Applied to files:
src/services/organization.jssrc/services/admin.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/helpers/eventBroadcasterMain.jssrc/envVariables.jssrc/services/admin.jssrc/.env.sample
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/helpers/eventBroadcasterMain.jssrc/envVariables.jssrc/services/admin.jssrc/.env.sample
📚 Learning: 2025-09-12T10:01:49.571Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.571Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/helpers/eventBroadcasterMain.jssrc/envVariables.jssrc/services/admin.jssrc/.env.sample
📚 Learning: 2025-09-11T23:15:19.523Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:14-17
Timestamp: 2025-09-11T23:15:19.523Z
Learning: In the ELEVATE-Project/user codebase, switch cases typically use return statements without break statements when each case returns a value directly. However, when a case has conditional returns, it should include a fallback return statement to prevent unintended fallthrough behavior.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-08-08T13:06:32.911Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:10-10
Timestamp: 2025-08-08T13:06:32.911Z
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.
Applied to files:
src/services/admin.js
📚 Learning: 2025-08-08T15:12:44.423Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:117-156
Timestamp: 2025-08-08T15:12:44.423Z
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.
Applied to files:
src/services/admin.js
🧬 Code graph analysis (8)
src/services/user.js (3)
src/helpers/userInvite.js (7)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)eventBody(882-896)eventBody(1125-1130)src/services/account.js (6)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)src/services/admin.js (6)
require(11-11)require(27-27)require(34-34)require(36-36)eventBody(64-81)eventBody(208-219)
src/services/account.js (3)
src/helpers/userInvite.js (5)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)src/services/admin.js (4)
require(11-11)require(27-27)require(34-34)require(36-36)src/services/user.js (5)
require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)
src/helpers/userInvite.js (2)
src/services/account.js (6)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)src/services/user.js (6)
require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)eventBody(191-201)
src/generics/utils.js (2)
src/services/organization.js (2)
changes(833-833)_(13-13)src/services/tenant.js (4)
changes(557-563)changes(697-703)changes(879-879)_(30-30)
src/services/organization.js (4)
src/services/account.js (5)
require(26-26)require(27-27)require(37-37)require(41-41)common(16-16)src/services/admin.js (9)
require(11-11)require(27-27)require(34-34)require(36-36)require(37-37)eventBodyData(705-718)organizationDTO(43-43)common(15-15)updatedOrgDetails(704-704)src/database/queries/organization.js (3)
require(2-2)require(3-3)common(4-4)src/dtos/eventBody.js (1)
changes(11-17)
src/helpers/eventBroadcasterMain.js (5)
src/helpers/userInvite.js (2)
kafkaCommunication(17-17)requestBody(941-945)src/services/account.js (1)
kafkaCommunication(20-20)src/services/organization.js (1)
kafkaCommunication(11-11)src/services/org-admin.js (1)
kafkaCommunication(15-15)src/services/userInvite.js (2)
kafkaCommunication(24-24)requestBody(379-383)
src/services/tenant.js (4)
src/helpers/userInvite.js (9)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)eventBody(882-896)eventBody(1125-1130)common(8-8)utils(3-3)src/services/account.js (8)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)common(16-16)utils(25-25)src/services/admin.js (10)
require(11-11)require(27-27)require(34-34)require(36-36)require(37-37)eventBody(64-81)eventBody(208-219)common(15-15)eventBodyData(705-718)utils(17-17)src/services/organization.js (7)
eventBody(138-146)common(2-2)tenantCode(281-281)changes(833-833)eventBodyData(151-169)eventBodyData(845-865)utils(5-5)
src/services/admin.js (2)
src/services/account.js (7)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)common(16-16)src/services/organization.js (10)
require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)organizationDTO(19-19)eventBody(138-146)common(2-2)eventBodyData(151-169)eventBodyData(845-865)
🪛 dotenv-linter (3.3.0)
src/.env.sample
[warning] 183-183: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 183-183: [UnorderedKey] The KAFKA_HEALTH_CHECK_TOPIC key should go before the NOTIFICATION_API_URL key
(UnorderedKey)
[warning] 185-185: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 185-185: [UnorderedKey] The EVENT_ENABLE_TENANT_EVENTS key should go before the EVENT_ENABLE_USER_EVENTS key
(UnorderedKey)
[warning] 187-187: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 187-187: [UnorderedKey] The ENABLE_TENANT_KAFKA_EVENTS key should go before the ENTITY_MANAGEMENT_SERVICE_BASE_URL key
(UnorderedKey)
[warning] 189-189: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 189-189: [UnorderedKey] The ENABLE_USER_KAFKA_EVENTS key should go before the ENTITY_MANAGEMENT_SERVICE_BASE_URL key
(UnorderedKey)
[warning] 191-191: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 191-191: [UnorderedKey] The ENABLE_ORG_KAFKA_EVENTS key should go before the ENABLE_TENANT_KAFKA_EVENTS key
(UnorderedKey)
[warning] 193-193: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 193-193: [UnorderedKey] The EVENT_TENANT_KAFKA_TOPIC key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 195-195: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 195-195: [UnorderedKey] The EVENT_TENANT_LISTENER_API key should go before the EVENT_USER_KAFKA_TOPIC key
(UnorderedKey)
[warning] 197-197: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 197-197: [UnorderedKey] The EVENT_ENABLE_ORGANIZATION_EVENTS key should go before the EVENT_ENABLE_TENANT_EVENTS key
(UnorderedKey)
[warning] 199-199: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 199-199: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 199-199: [UnorderedKey] The EVENT_ORGANIZATION_KAFKA_TOPIC key should go before the EVENT_TENANT_KAFKA_TOPIC key
(UnorderedKey)
🔇 Additional comments (32)
src/services/user.js (2)
26-26: Switch to service-based broadcaster looks good.Import updated to broadcastUserServiceEvent as intended.
203-203: Approve: legacy broadcaster fully removedNo occurrences of
broadcastUserEventremain; event dispatch updated correctly.src/services/account.js (3)
41-41: Updated import aligns with new event routing.Looks correct.
625-626: Create event routed via service broadcaster.Emission matches the new pattern and topic.
1771-1771: Delete event routed via service broadcaster.Consistent with other modules.
src/helpers/userInvite.js (3)
28-28: Import migration to service broadcaster is correct.No issues.
898-899: Bulk-update event emission path updated correctly.Good use of broadcastUserServiceEvent.
1132-1133: Bulk-create event emission path updated correctly.Good.
src/generics/utils.js (1)
1085-1109: Solid rename and JSDoc; utility is clear and minimal.Defaults and deep-equality usage look good.
src/services/admin.js (5)
34-34: Import updated to centralized broadcaster.Good.
43-43: organizationDTO import to build event body is appropriate.Good addition.
83-84: Delete-user event broadcast path migrated.Looks correct.
221-221: Admin create event routed via service broadcaster.No issues.
656-666: Verify organizationQueries.update return structure. Unable to locate its implementation; confirm that with{ returning: true, raw: true }it returns an object{ rowsAffected, updatedRows }so the zero-row check usesrowsAffected.src/services/tenant.js (7)
33-35: Imports look good.Consistent with new broadcaster/DTO usage.
376-378: Good: persist default org identifiers onto tenant response.Used immediately for the create event payload.
388-407: Tenant create event payload looks correct.Schema consistent (meta kept as object; org_id/org_code present).
462-472: Update + safe extraction of first updated row: LGTM.Avoids destructuring undefined and still emits update events when no row changes.
475-476: Good: centralized emitter invocation.Keeps change-detection and emission logic consistent.
557-563: Changes payload for domain add: LGTM.
697-703: Changes payload for domain remove: LGTM.src/services/organization.js (8)
22-23: Import of new broadcaster: LGTM.
137-149: Duplicate create emissions (legacy + new). Remove or gate legacy call.This still fires
eventBroadcasterMain('organizationEvents', ...)and thenbroadcastUserServiceEvent, which internally calls the same broadcaster again, producing duplicates.- // This event will deprecated in future - const eventBody = eventBodyDTO({ - entity: 'organization', - eventType: 'create', - entityId: createdOrganization.id, - args: { - name: bodyData.name, - created_by: loggedInUserId, - }, - }) - eventBroadcasterMain('organizationEvents', { requestBody: eventBody, isInternal: true }) - //ends here + // (legacy emission removed to avoid duplicate events)
150-172: New org create payload + awaited broadcast: LGTM.
252-254: Good: await orgEventEmitter on update.
559-585: Related orgs add: using returning+raw and delta emission: LGTM.
586-589: Good: emit with bodyData.related_orgs context.
637-648: Related orgs remove path: LGTM.
664-666: Good: emit with removal context.src/envVariables.js (3)
399-403: New user Kafka flag: LGTM.
422-447: Tenant event flags and topic gating: LGTM.
448-460: Org Kafka flag + topic requiredIf: LGTM.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/helpers/eventBroadcasterMain.js(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, EVENT_TENANT_KAFKA_TOPIC uses conditional validation (requiredIf) in src/envVariables.js, not default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', the topic becomes required and application startup validation ensures it's present, preventing runtime issues with undefined topics.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T10:02:11.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:28-0
Timestamp: 2025-09-12T10:02:11.455Z
Learning: In the ELEVATE-Project/user codebase, environment variables use conditional validation via requiredIf logic in src/envVariables.js rather than default values. When EVENT_ENABLE_TENANT_EVENTS equals 'true', EVENT_TENANT_KAFKA_TOPIC becomes required and startup validation ensures it's present.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T10:01:49.571Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/generics/kafka-communication.js:38-0
Timestamp: 2025-09-12T10:01:49.571Z
Learning: In the ELEVATE-Project/user codebase, environment variables use a requiredIf validation pattern where EVENT_ORGANIZATION_KAFKA_TOPIC and EVENT_TENANT_KAFKA_TOPIC are optional by default but become required when their respective enable flags (EVENT_ENABLE_ORGANIZATION_EVENTS and EVENT_ENABLE_TENANT_EVENTS) equal 'true'. This prevents undefined topic issues through environment validation rather than runtime guards.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-11T23:15:19.523Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:14-17
Timestamp: 2025-09-11T23:15:19.523Z
Learning: In the ELEVATE-Project/user codebase, switch cases typically use return statements without break statements when each case returns a value directly. However, when a case has conditional returns, it should include a fallback return statement to prevent unintended fallthrough behavior.
Applied to files:
src/helpers/eventBroadcasterMain.js
🧬 Code graph analysis (1)
src/helpers/eventBroadcasterMain.js (5)
src/helpers/userInvite.js (2)
kafkaCommunication(17-17)requestBody(941-945)src/services/organization.js (1)
kafkaCommunication(11-11)src/services/org-admin.js (1)
kafkaCommunication(15-15)src/services/userInvite.js (2)
kafkaCommunication(24-24)requestBody(379-383)src/services/account.js (1)
kafkaCommunication(20-20)
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
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 (1)
src/generics/utils.js (1)
914-926: Fix minutes-to-seconds conversion and preserve unit contextIn src/generics/utils.js, convertExpiryTimeToSeconds currently divides by 60 for “Xm” and the call‐site’s parseInt strips the “m” (e.g. “20m” → 20 seconds), resulting in incorrect TTLs. Replace with:
function convertExpiryTimeToSeconds(expiryTime) { - expiryTime = String(expiryTime) - const match = expiryTime.match(/^(\d+)([m]?)$/) - if (match) { - const value = parseInt(match[1], 10) - const unit = match[2] - if (unit === 'm') { - return Math.floor(value / 60) - } else { - return value - } - } + const str = String(expiryTime).trim() + const match = str.match(/^(\d+)(m)?$/i) + if (!match) return NaN + const value = parseInt(match[1], 10) + return match[2] ? value * 60 : value }Also update the call-site (e.g. in src/generics/utils.js or cloud-services.js) to pass the raw env string instead of parsing it first:
- let expiryInSeconds = parseInt(process.env.SIGNED_URL_EXPIRY_DURATION) || 300 - let updatedExpiryTime = convertExpiryTimeToSeconds(expiryInSeconds) + const rawExpiry = process.env.SIGNED_URL_EXPIRY_DURATION ?? '300' + const updatedExpiryTime = convertExpiryTimeToSeconds(rawExpiry)
🧹 Nitpick comments (3)
src/services/admin.js (2)
83-84: Make event emission explicitly non-blocking (or confirm helper behavior)If
broadcastUserServiceEventreturns a Promise and throws on failure, consider deferring to avoid request-path latency/risks, or confirm the helper already handles this internally.Suggested change:
- broadcastUserServiceEvent('userEvents', { requestBody: eventBody, isInternal: true }) + setImmediate(() => + broadcastUserServiceEvent('userEvents', { requestBody: eventBody, isInternal: true }) + )If the helper already enqueues asynchronously, keep as-is and ignore this. As per coding guidelines.
Also applies to: 221-222
676-686: Good fix on update() result handling; minor clarity refactorUsing
{ returning: true, raw: true }and checkingrowsAffectedis correct. For readability and to prevent shape mix-ups elsewhere, destructure the result and use consistent names.-const orgUpdateResult = await organizationQueries.update( +const orgUpdateResult = await organizationQueries.update( { code: organizationCode, tenant_code: tenantCode }, { status: common.INACTIVE_STATUS, updated_by: loggedInUserId }, { returning: true, raw: true } ) - -if (!orgUpdateResult || orgUpdateResult.rowsAffected === 0) { +const { rowsAffected, updatedRows } = orgUpdateResult || {} +if (!rowsAffected) { return responses.failureResponse({ message: 'ORG_DEACTIVATION_FAILED', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }This also lets you reuse
updatedRowsbelow without repeating the long path. As per coding guidelines.Also applies to: 688-694
src/generics/utils.js (1)
1090-1097: Nit: tighten JSDoc and use constsClarify param meaning and prefer const; also inline fieldName to avoid a temp.
-/** - * Compare object and get the updated value with old value +/** + * Compare objects and return changed fields (with old and new values). * @method * @name extractUpdatedValues * @param {Object} oldData - Data before update * @param {Object} newData - Data after update - * @param {Object} updateData - reqBody data + * @param {Object} updateData - reqBody keys to inspect (fields intended to be updated) * @returns {Array<{fieldName:string, oldValue:any, newValue:any}>} */ function extractUpdatedValues(oldData = {}, newData = {}, updateData = {}) { const changes = [] for (const key of Object.keys(updateData)) { - let oldValue = oldData[key] - let newValue = newData[key] - let fieldName = key + const oldValue = oldData[key] + const newValue = newData[key] // Compare only if key exists in updateData if (!_.isEqual(oldValue, newValue)) { - changes.push({ fieldName, oldValue, newValue }) + changes.push({ fieldName: key, oldValue, newValue }) } } return changes }Also applies to: 1103-1109
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/generics/utils.js(2 hunks)src/helpers/userInvite.js(3 hunks)src/services/admin.js(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/helpers/userInvite.js
🧰 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/admin.js
🧠 Learnings (12)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.009Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.009Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, oldValue and newValue in the changedValues array for organizationDTO.eventBodyDTO are guaranteed to not be undefined by the business logic, and filtering out falsy values (0, false, '') from changes is the intended behavior.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-29T10:51:54.009Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.009Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO method intentionally filters out deleted_at from args for all event types (create, update, delete), not just non-delete events. This is by design according to the business requirements.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the changedValues parameter in organizationDTO.eventBodyDTO is guaranteed to always be an array by the calling code, so additional Array.isArray validation is not needed.
Applied to files:
src/services/admin.js
📚 Learning: 2025-08-08T13:06:32.911Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:10-10
Timestamp: 2025-08-08T13:06:32.911Z
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.
Applied to files:
src/services/admin.js
📚 Learning: 2025-08-08T15:12:44.423Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:117-156
Timestamp: 2025-08-08T15:12:44.423Z
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.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the oldValue and newValue properties in changedValues objects passed to tenantDTO.eventBodyDTO are guaranteed to be defined when present, so undefined checks are not necessary according to the data contracts.
Applied to files:
src/generics/utils.js
🧬 Code graph analysis (1)
src/services/admin.js (2)
src/services/organization.js (10)
require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)organizationDTO(19-19)eventBody(138-146)common(2-2)eventBodyData(151-169)eventBodyData(845-865)src/services/account.js (7)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)common(16-16)
🔇 Additional comments (5)
src/services/admin.js (3)
34-34: Unified broadcaster import looks goodSwitch to
broadcastUserServiceEventaligns with the new event pipeline. No issues spotted. Based on learnings.
43-43: DTO import added correctly
organizationDTOis used later for the org update event payload; import placement is fine.
723-741: Include updated_by and changedValues in deactivation event payloadFor auditability and consumer diffs, add
updated_byand emitchangedValues(at least the status transition). This also matches how org updates are modeled elsewhere.-//Event body for org update (deactivation) -let updatedOrgDetails = orgUpdateResult.updatedRows?.[0] -const eventBodyData = organizationDTO.eventBodyDTO({ +// Event body for org update (deactivation) +const updatedOrgDetails = (orgUpdateResult.updatedRows || updatedRows || [])[0] +const changes = [ + { field: 'status', oldValue: org.status, newValue: updatedOrgDetails?.status } +] +const eventBodyData = organizationDTO.eventBodyDTO({ entity: 'organization', eventType: 'update', entityId: updatedOrgDetails.id, args: { created_by: updatedOrgDetails.created_by, + updated_by: loggedInUserId, name: updatedOrgDetails.name, code: updatedOrgDetails.code, updated_at: updatedOrgDetails?.updated_at || new Date(), status: updatedOrgDetails?.status || common.INACTIVE_STATUS, id: updatedOrgDetails.id, tenant_code: tenantCode, }, + changedValues: changes, })If
organizationDTO.eventBodyDTOalready deriveschangedValuesinternally when omitted, feel free to skip that part—but please still includeupdated_by. Based on learnings.src/generics/utils.js (2)
1161-1161: Export wiring added — OKNew util is correctly exported; no conflicts found.
1089-1113: extractUpdatedValues: implementation is correct and usage is consistent
- Compares only updated keys and returns
{fieldName, oldValue, newValue}with safe defaults; no extra guards required.- Rename from
getChangestoextractUpdatedValuesis applied project-wide; no residual references found.
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/envVariables.js(2 hunks)src/helpers/eventBroadcasterMain.js(4 hunks)src/helpers/userInvite.js(3 hunks)src/services/account.js(3 hunks)src/services/admin.js(6 hunks)src/services/organization.js(9 hunks)src/services/tenant.js(7 hunks)src/services/user.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/user.js
- src/helpers/userInvite.js
🧰 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/tenant.jssrc/services/admin.jssrc/services/organization.jssrc/services/account.js
🧠 Learnings (16)
📓 Common learnings
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.028Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.028Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when using organizationQueries.update with { returning: true, raw: true } options, the method consistently returns a structured result with updatedRows property when successful. The initial check for orgRowsAffected === 0 already handles failed updates, so additional null checking for updatedRows is redundant when the code execution reaches the event emission logic.
Applied to files:
src/services/tenant.jssrc/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:03:00.543Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/tenantDTO.js:23-0
Timestamp: 2025-09-12T11:03:00.543Z
Learning: In the ELEVATE-Project/user codebase, the oldValue and newValue properties in changedValues objects passed to tenantDTO.eventBodyDTO are guaranteed to be defined when present, so undefined checks are not necessary according to the data contracts.
Applied to files:
src/services/tenant.jssrc/services/organization.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In the ELEVATE-Project/user codebase, when organizationQueries.update is called with { returning: true, raw: true }, it returns an object { rowsAffected, updatedRows }, not just a number. The validation logic in admin.js deactivateOrg method checking 'if (orgRowsAffected === 0)' is incorrect and should be 'if (orgRowsAffected.rowsAffected === 0)' to properly validate successful updates.
Applied to files:
src/services/tenant.jssrc/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-12T10:35:28.285Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/admin.js:704-704
Timestamp: 2025-09-12T10:35:28.285Z
Learning: In ELEVATE-Project/user admin.js, there's an inconsistency in organizationQueries.update usage: addOrgAdmin calls it without options (returns number), while deactivateOrg calls it with { returning: true, raw: true } (returns object). The validation logic 'if (orgRowsAffected === 0)' only works for the first case - the second case needs 'if (orgRowsAffected.rowsAffected === 0)' to properly validate updates.
Applied to files:
src/services/tenant.jssrc/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-12T10:40:34.860Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:40:34.860Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO is designed to handle empty changes arrays properly, so organization update events should always be emitted even when changedValues is empty. The DTO handles this scenario correctly and there may be other reasons to emit events beyond just field changes (like metadata updates).
Applied to files:
src/services/tenant.jssrc/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-29T10:51:54.028Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:32-37
Timestamp: 2025-09-29T10:51:54.028Z
Learning: In the ELEVATE-Project/user codebase, the environment variables ENABLE_USER_KAFKA_EVENTS, ENABLE_TENANT_KAFKA_EVENTS, and ENABLE_ORG_KAFKA_EVENTS are properly defined in src/envVariables.js with optional: true validation. These variables control Kafka event publishing for their respective entity types and are correctly referenced in src/helpers/eventBroadcasterMain.js isEventEnabled function.
Applied to files:
src/helpers/eventBroadcasterMain.jssrc/services/admin.jssrc/envVariables.js
📚 Learning: 2025-09-11T23:15:19.523Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/helpers/eventBroadcasterMain.js:14-17
Timestamp: 2025-09-11T23:15:19.523Z
Learning: In the ELEVATE-Project/user codebase, switch cases typically use return statements without break statements when each case returns a value directly. However, when a case has conditional returns, it should include a fallback return statement to prevent unintended fallthrough behavior.
Applied to files:
src/helpers/eventBroadcasterMain.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the organizationDTO.eventBodyDTO method intentionally filters out deleted_at from args for all event types (create, update, delete), not just non-delete events. This is by design according to the business requirements.
Applied to files:
src/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, oldValue and newValue in the changedValues array for organizationDTO.eventBodyDTO are guaranteed to not be undefined by the business logic, and filtering out falsy values (0, false, '') from changes is the intended behavior.
Applied to files:
src/services/admin.jssrc/services/organization.js
📚 Learning: 2025-09-12T11:02:51.008Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:14-43
Timestamp: 2025-09-12T11:02:51.008Z
Learning: In the ELEVATE-Project/user codebase, the changedValues parameter in organizationDTO.eventBodyDTO is guaranteed to always be an array by the calling code, so additional Array.isArray validation is not needed.
Applied to files:
src/services/admin.jssrc/services/organization.js
📚 Learning: 2025-08-08T13:06:32.911Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:10-10
Timestamp: 2025-08-08T13:06:32.911Z
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.
Applied to files:
src/services/admin.js
📚 Learning: 2025-08-08T15:12:44.423Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#790
File: src/helpers/userHelper.js:117-156
Timestamp: 2025-08-08T15:12:44.423Z
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.
Applied to files:
src/services/admin.js
📚 Learning: 2025-09-12T10:14:22.455Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-867
Timestamp: 2025-09-12T10:14:22.455Z
Learning: In the ELEVATE-Project/user codebase, the related_orgs field is stored as an array in the database by default, so additional array validation checks are unnecessary when processing related organization data.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T10:36:43.529Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/dtos/organizationDTO.js:26-29
Timestamp: 2025-09-12T10:36:43.529Z
Learning: In the ELEVATE-Project/user codebase, the changedValues array passed to organizationDTO.eventBodyDTO and tenantDTO.eventBodyDTO always contains a fieldName property when oldValue or newValue are present, so additional fieldName validation is not needed according to the business logic.
Applied to files:
src/services/organization.js
📚 Learning: 2025-09-12T10:38:57.594Z
Learnt from: praveenKDass
PR: ELEVATE-Project/user#816
File: src/services/organization.js:831-0
Timestamp: 2025-09-12T10:38:57.594Z
Learning: In the ELEVATE-Project/user codebase, when building event payloads in orgEventEmitter function, the preference is to use direct property access without nullish coalescing operators (??) for args object properties. This ensures fail-fast behavior - if expected document properties are missing, the system should throw an error rather than silently falling back to default values, helping to catch data integrity issues early.
Applied to files:
src/services/organization.js
🧬 Code graph analysis (5)
src/services/tenant.js (2)
src/helpers/userInvite.js (8)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)eventBody(910-924)eventBody(1153-1158)utils(3-3)src/database/queries/organization.js (3)
rowsAffected(104-104)rowsAffected(143-143)rowsAffected(164-164)
src/helpers/eventBroadcasterMain.js (5)
src/helpers/userInvite.js (1)
kafkaCommunication(17-17)src/services/account.js (1)
kafkaCommunication(20-20)src/services/organization.js (1)
kafkaCommunication(11-11)src/services/org-admin.js (1)
kafkaCommunication(15-15)src/services/userInvite.js (1)
kafkaCommunication(24-24)
src/services/admin.js (2)
src/services/account.js (7)
require(26-26)require(27-27)require(37-37)require(41-41)eventBody(604-623)eventBody(1754-1769)common(16-16)src/services/organization.js (10)
require(12-12)require(14-14)require(15-15)require(18-18)require(22-22)organizationDTO(19-19)eventBody(138-146)common(2-2)eventBodyData(151-169)eventBodyData(845-865)
src/services/organization.js (5)
src/services/account.js (6)
require(26-26)require(27-27)require(37-37)require(41-41)common(16-16)utils(25-25)src/services/admin.js (8)
require(11-11)require(27-27)require(34-34)require(36-36)require(37-37)organizationDTO(43-43)common(15-15)utils(17-17)src/database/queries/organization.js (3)
require(2-2)require(3-3)common(4-4)src/services/tenant.js (5)
common(10-10)changes(557-563)changes(697-703)changes(879-879)utils(29-29)src/dtos/eventBody.js (1)
changes(11-17)
src/services/account.js (3)
src/helpers/userInvite.js (5)
require(18-18)require(23-23)require(27-27)require(28-28)require(29-29)src/services/admin.js (4)
require(11-11)require(27-27)require(34-34)require(36-36)src/services/user.js (5)
require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)
🔇 Additional comments (13)
src/services/account.js (2)
41-41: LGTM! Import updated correctly.The import change from
broadcastUserEventtobroadcastEventaligns with the API rename in the event broadcaster module, maintaining consistency across the codebase.
625-625: LGTM! Function calls updated consistently.Both calls to
broadcastEvent('userEvents', ...)correctly use the renamed function. The event payloads and parameters remain unchanged, ensuring backward compatibility.Also applies to: 1771-1771
src/services/admin.js (2)
34-34: LGTM! Imports updated correctly.The addition of
organizationDTOimport and the change frombroadcastUserEventtobroadcastEventproperly support the enhanced event broadcasting capabilities for both user and organization events.Also applies to: 43-43
83-83: LGTM! User event broadcasts updated consistently.Both user event emissions correctly use the renamed
broadcastEventfunction with the'userEvents'event group. The event payloads remain properly structured.Also applies to: 221-221
src/helpers/eventBroadcasterMain.js (4)
14-18: LGTM! Fallback returns prevent fallthrough.The addition of
return []statements for bothuserEventsandtenantEventscases ensures no unintended fallthrough occurs when the environment variables are not set, consistent with theorganizationEventspattern.
30-37: LGTM! Event enablement gates properly configured.All event groups now have proper enablement checks:
tenantEventscase added for API events- Entity-specific Kafka flags (
EVENT_ENABLE_USER_KAFKA_EVENTS,EVENT_ENABLE_TENANT_KAFKA_EVENTS,EVENT_ENABLE_ORG_KAFKA_EVENTS) correctly gate Kafka publishing for each event typeBased on learnings
68-82: LGTM! Kafka routing properly implemented with await.The switch statement correctly routes events to the appropriate Kafka push functions:
- All three entity types (organization, user, tenant) are handled
- Each Kafka push is properly awaited to prevent silent failures
- Default case logs unknown event groups appropriately
87-109: LGTM! API rename completed successfully.The rename from
broadcastUserEventtobroadcastEventaccurately reflects the function's broader scope, now handling user, organization, and tenant events uniformly. Error logging has been updated consistently.src/services/organization.js (3)
150-172: LGTM: Organization create event properly structured and awaited.The event payload includes all required fields with appropriate fallbacks, and the broadcast is awaited to surface any failures.
253-253: LGTM: Update event emission properly awaited.Correctly passes before/after state and bodyData to the event emitter, with await to catch failures.
586-589: LGTM: Related organization event emissions properly awaited.Both
addRelatedOrgandremoveRelatedOrgcorrectly await the event emitter to ensure errors surface.Also applies to: 664-667
src/services/tenant.js (1)
458-475: LGTM: Tenant update event emission properly awaited.The update flow correctly handles conditional updates and awaits the event emitter to surface failures.
src/envVariables.js (1)
399-460: LGTM: Environment variables follow consistent patterns.The new event control flags and topic/URL variables follow the established
requiredIfvalidation pattern, ensuring topics are required only when their respective event types are enabled. Based on learnings.
Summary by CodeRabbit
New Features
Chores
Utilities