Issue#252552 Feat: User bulk upload with all entity types#869
Issue#252552 Feat: User bulk upload with all entity types#869Sachintechjoomla wants to merge 23 commits intoELEVATE-Project:developfrom
Conversation
…rties of undefined (reading 'outputFilePath')
WalkthroughAdds safety checks for entity lookup in ARRAY handling; replaces hard-coded root field deletions with dynamic entityField processing in user-invite; extends account creation/login flows (tenant resolution, optional login, token/Redis handling); adds new tenant controller endpoint; enhances user search with status/metaFilters; and adds two GitHub Actions workflows for dev/QA deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TenantCtrl as Tenant Controller
participant AccountSvc as Account Service
participant DB as Database
participant Redis
participant Auth as Token Service
Client->>TenantCtrl: POST /tenant/accountCreate (body, headers)
TenantCtrl->>AccountSvc: accountCreate(req)
AccountSvc->>AccountSvc: resolve tenantDomain (domain | TENANT_CODE_HEADER)
alt tenantDomain resolved
AccountSvc->>DB: create user record
DB-->>AccountSvc: user
opt registerWithLogin == true
AccountSvc->>Auth: generate access & refresh tokens
Auth-->>AccountSvc: tokens
AccountSvc->>Redis: store session/tokenDetail
end
AccountSvc-->>TenantCtrl: result (user [+ tokens])
else tenantDomain missing
AccountSvc-->>TenantCtrl: error
end
TenantCtrl-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 1
🤖 Fix all issues with AI agents
In `@src/helpers/userInvite.js`:
- Around line 352-364: The dynamic entityFields loop currently only assigns a
single _id into row.meta[field]; update the entityFields.forEach handler (the
loop that checks alreadyProcessed and uses externalEntityNameIdMap and
lookupKey) to detect ARRAY-like values (either actual arrays or comma-separated
strings), split them into items, clean each item with the same
whitespace/lowercase logic, build per-item lookupKeys and map to IDs via
externalEntityNameIdMap, and then set row.meta[field] to an array of IDs (or
null/empty array if none); if the value is a single scalar, keep the existing
single-ID assignment behavior. Ensure you reuse the same cleaning logic and
externalEntityNameIdMap lookup so behavior matches professional_subroles
handling.
🧹 Nitpick comments (2)
src/helpers/userInvite.js (2)
366-370: Consider deduplicating fields to delete.If
alreadyProcessedfields are also present inentityFields, you'll iterate over them twice. While harmless (deleting a non-existent key is a no-op), using aSetwould be cleaner.♻️ Optional: Use Set to deduplicate
-const allFieldsToDelete = [...alreadyProcessed, ...entityFields] -allFieldsToDelete.forEach((field) => { - delete row[field] -}) +const allFieldsToDelete = new Set([...alreadyProcessed, ...entityFields]) +allFieldsToDelete.forEach((field) => { + delete row[field] +})
341-370: Consider consolidating entity processing in future refactor.The code now has two processing paths: hardcoded handling (lines 283-339) for 7 specific fields and dynamic handling (lines 352-364) for additional fields. While this works, it increases maintenance burden. Consider consolidating all entity field processing into the dynamic loop in a future iteration.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/generics/utils.jssrc/helpers/userInvite.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:06:32.911Z
Learnt from: nevil-mathew
Repo: ELEVATE-Project/user PR: 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/helpers/userInvite.js
🧬 Code graph analysis (1)
src/helpers/userInvite.js (1)
src/validators/v1/admin.js (1)
field(161-161)
🔇 Additional comments (1)
src/generics/utils.js (1)
1041-1041: LGTM! Good defensive check.The guard prevents a potential
TypeErrorwhenprunedEntities.find()returnsundefinedfor ametaKeythat doesn't match any entity.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…crypting phone and passing correct meta in account search
Issue #000 fix: added new endpoint to create account in teanat and decrypting phone and passing correct meta in account search
Added AWS secret access key exports for deployment.
Updated environment variable export syntax for AWS credentials.
Issue #000 fix: serach Users with one or more meta params
…into Issue#253287
Issue#253287 Feat: User account search API > Add status filter
…into Issue#252552
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/account.js (1)
58-91:⚠️ Potential issue | 🔴 CriticalGuard missing tenant domain/header before dereferencing.
If neither
domainnorTENANT_CODE_HEADERis provided,tenantDomainremains undefined andtenantDomain.tenant_codewill throw (Line 83). Add an explicit guard to return a proper failure response.As per coding guidelines, `src/services/**`: This is core business logic. Please check for correctness, efficiency, and potential edge cases.🛠️ Proposed fix
let tenantDomain if (domain) { tenantDomain = await tenantDomainQueries.findOne({ domain }) if (!tenantDomain) { return notFoundResponse('TENANT_DOMAIN_NOT_FOUND_PING_ADMIN') } } else if (req.headers?.[common.TENANT_CODE_HEADER]) { tenantId = req.headers?.[common.TENANT_CODE_HEADER] tenantDomain = { tenant_code: tenantId } + } else { + return notFoundResponse('TENANT_DOMAIN_NOT_FOUND_PING_ADMIN') }
🤖 Fix all issues with AI agents
In @.github/workflows/brac-dev-deployment.yaml:
- Around line 54-56: The workflow step named "Deploy Stack" currently uses the
SSH action pinned to appleboy/ssh-action@master which risks unexpected upstream
changes; update that usage to a specific stable release tag (e.g., replace
appleboy/ssh-action@master with appleboy/ssh-action@v1.2.4 or the latest pinned
tag) so the "Deploy Stack" step references a fixed, audited release.
In @.github/workflows/brac-qa-deplyment.yaml:
- Around line 55-57: Replace the unpinned GitHub Action reference
"appleboy/ssh-action@master" with a fixed release tag or commit SHA to avoid
supply-chain risk; update the "uses" value in the Deploy Stack to QA step to
"appleboy/ssh-action@v1.2.5" (or a specific commit SHA) so the workflow uses a
stable, immutable version.
In `@src/services/account.js`:
- Around line 1968-1991: The loop currently calls userQueries.getModelName() and
entityTypeQueries.findUserEntityTypesAndEntities(...) per user causing an N+1;
refactor by computing modelName once (call getModelName() before the users
loop), group users by their organization_code (use
user.user_organizations[0].organization_code or guard for missing orgs), then
for each unique org_code call findUserEntityTypesAndEntities(...) once, run
removeDefaultOrgEntityTypes(...) per org (cache prunedEntities keyed by
organization_code or user_organization id), and finally iterate users to call
utils.processDbResponse(user, cachedPrunedEntities[orgKey]) and Object.assign;
also add a fallback when user.user_organizations[0] is absent to avoid crashes.
- Around line 485-550: The code overwrites result when registerWithLogin is
true, dropping result.user and causing later dereferences to fail; update the
logic in the register/login block (referencing result, registerWithLogin,
tokenDetail, userSessionDetails,
userSessionsService.updateUserSessionAndsetRedisData) so that you preserve the
user object when returning tokens—e.g. merge the tokens into the existing result
rather than replacing it (assign access_token and refresh_token onto result or
set result = { user, access_token: ..., refresh_token: ... }) and ensure result
still contains user for downstream uses.
🧹 Nitpick comments (3)
src/services/account.js (3)
325-326: Normalize/validate role titles from the request.
req.body.roles.split(',')can introduce whitespace/empty strings and cause role lookups to fail unexpectedly. Consider trimming and filtering empty values first.As per coding guidelines, `src/services/**`: This is core business logic. Please check for correctness, efficiency, and potential edge cases.♻️ Suggested refinement
- title: { - [Op.in]: req.body.roles ? req.body.roles.split(',') : process.env.DEFAULT_ROLE.split(','), - }, + title: { + [Op.in]: req.body?.roles + ? req.body.roles.split(',').map((r) => r.trim()).filter(Boolean) + : process.env.DEFAULT_ROLE.split(','), + },
384-385: Gate debug logs behind a logger or env flag.
console.logforvalidationData/userModelcan be noisy and may leak internal details in production.
1730-1730: Avoid silently swallowing Redis errors.Consider logging or annotating the intentional ignore to aid troubleshooting.
| - name: Deploy Stack | ||
| uses: appleboy/ssh-action@master | ||
| with: |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
appleboy/ssh-action latest stable release tag version
💡 Result:
The latest stable release tag for appleboy/ssh-action is v1.2.4. [1]
Pin the SSH action to a stable version instead of @master.
Using @master exposes the workflow to unexpected upstream changes and supply-chain risk. Pin to a specific release tag, such as v1.2.4 (the latest stable release).
🤖 Prompt for AI Agents
In @.github/workflows/brac-dev-deployment.yaml around lines 54 - 56, The
workflow step named "Deploy Stack" currently uses the SSH action pinned to
appleboy/ssh-action@master which risks unexpected upstream changes; update that
usage to a specific stable release tag (e.g., replace appleboy/ssh-action@master
with appleboy/ssh-action@v1.2.4 or the latest pinned tag) so the "Deploy Stack"
step references a fixed, audited release.
| - name: Deploy Stack to QA | ||
| uses: appleboy/ssh-action@master | ||
| with: |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable release appleboy/ssh-action GitHub
💡 Result:
Latest stable release of appleboy/ssh-action on GitHub: v1.2.5 (tag v1.2.5), published Jan 28, 2026. [1]
Pin the SSH action to a specific release tag instead of @master.
Using @master introduces supply-chain risk as it can change unexpectedly. Pin to release tag v1.2.5 (the latest stable version) or use a commit SHA.
🤖 Prompt for AI Agents
In @.github/workflows/brac-qa-deplyment.yaml around lines 55 - 57, Replace the
unpinned GitHub Action reference "appleboy/ssh-action@master" with a fixed
release tag or commit SHA to avoid supply-chain risk; update the "uses" value in
the Deploy Stack to QA step to "appleboy/ssh-action@v1.2.5" (or a specific
commit SHA) so the workflow uses a stable, immutable version.
| let tokenDetail = {} | ||
| let result = { user } | ||
| if (registerWithLogin) { | ||
| /** | ||
| * create user session entry and add session_id to token data | ||
| * Entry should be created first, the session_id has to be added to token creation data | ||
| */ | ||
| const userSessionDetails = await userSessionsService.createUserSession( | ||
| user.id, // userid | ||
| '', // refresh token | ||
| '', // Access token | ||
| deviceInfo, | ||
| user.tenant_code | ||
| ) | ||
|
|
||
| /** | ||
| * create user session entry and add session_id to token data | ||
| * Entry should be created first, the session_id has to be added to token creation data | ||
| */ | ||
| const userSessionDetails = await userSessionsService.createUserSession( | ||
| user.id, // userid | ||
| '', // refresh token | ||
| '', // Access token | ||
| deviceInfo, | ||
| user.tenant_code | ||
| ) | ||
| /** | ||
| * Based on user organisation id get user org parent Id value | ||
| * If parent org id is present then set it to tenant of user | ||
| * if not then set user organisation id to tenant | ||
| */ | ||
|
|
||
| /** | ||
| * Based on user organisation id get user org parent Id value | ||
| * If parent org id is present then set it to tenant of user | ||
| * if not then set user organisation id to tenant | ||
| */ | ||
| /* let tenantDetails = await organizationQueries.findOne( | ||
| { id: user.organization_id }, | ||
| { attributes: ['related_orgs'] } | ||
| ) | ||
|
|
||
| /* let tenantDetails = await organizationQueries.findOne( | ||
| { id: user.organization_id }, | ||
| { attributes: ['related_orgs'] } | ||
| ) | ||
| const tenant_id = | ||
| tenantDetails && tenantDetails.parent_id !== null ? tenantDetails.parent_id : user.organization_id */ | ||
|
|
||
| const tenant_id = | ||
| tenantDetails && tenantDetails.parent_id !== null ? tenantDetails.parent_id : user.organization_id */ | ||
| tokenDetail = { | ||
| data: { | ||
| id: user.id, | ||
| name: user.name, | ||
| session_id: userSessionDetails.result.id, | ||
| organization_ids: user.organizations.map((org) => String(org.id)), // Convert to string | ||
| organization_codes: user.organizations.map((org) => String(org.code)), // Convert to string | ||
| tenant_code: tenantDetail.code, | ||
| organizations: user.organizations, | ||
| }, | ||
| } | ||
|
|
||
| const tokenDetail = { | ||
| data: { | ||
| id: user.id, | ||
| name: user.name, | ||
| session_id: userSessionDetails.result.id, | ||
| organization_ids: user.organizations.map((org) => String(org.id)), // Convert to string | ||
| organization_codes: user.organizations.map((org) => String(org.code)), // Convert to string | ||
| tenant_code: tenantDetail.code, | ||
| organizations: user.organizations, | ||
| }, | ||
| const accessToken = utilsHelper.generateToken( | ||
| tokenDetail, | ||
| process.env.ACCESS_TOKEN_SECRET, | ||
| common.accessTokenExpiry | ||
| ) | ||
|
|
||
| const refreshToken = utilsHelper.generateToken( | ||
| tokenDetail, | ||
| process.env.REFRESH_TOKEN_SECRET, | ||
| common.refreshTokenExpiry | ||
| ) | ||
|
|
||
| /** | ||
| * This function call will do below things | ||
| * 1: create redis entry for the session | ||
| * 2: update user-session with token and refresh_token | ||
| */ | ||
| await userSessionsService.updateUserSessionAndsetRedisData( | ||
| userSessionDetails.result.id, | ||
| accessToken, | ||
| refreshToken | ||
| ) | ||
|
|
||
| result = { access_token: accessToken, refresh_token: refreshToken } | ||
| } |
There was a problem hiding this comment.
Preserve user in result when registerWithLogin=true.
result is overwritten with tokens only, so result.user becomes undefined and is later dereferenced (Line 619+). This will throw for the default login‑after‑register path.
🐛 Proposed fix
- let result = { user }
+ let result = { user }
if (registerWithLogin) {
...
- result = { access_token: accessToken, refresh_token: refreshToken }
+ result = { access_token: accessToken, refresh_token: refreshToken, user }
}
...
- result.user = await utils.processDbResponse(result.user, prunedEntities)
+ result.user = await utils.processDbResponse(user, prunedEntities)🤖 Prompt for AI Agents
In `@src/services/account.js` around lines 485 - 550, The code overwrites result
when registerWithLogin is true, dropping result.user and causing later
dereferences to fail; update the logic in the register/login block (referencing
result, registerWithLogin, tokenDetail, userSessionDetails,
userSessionsService.updateUserSessionAndsetRedisData) so that you preserve the
user object when returning tokens—e.g. merge the tokens into the existing result
rather than replacing it (assign access_token and refresh_token onto result or
set result = { user, access_token: ..., refresh_token: ... }) and ensure result
still contains user for downstream uses.
| const defaultOrganizationCode = process.env.DEFAULT_ORGANISATION_CODE | ||
|
|
||
| for (let i = 0; i < users.data.length; i++) { | ||
| let user = users.data[i] | ||
|
|
||
| // Convert Sequelize instance to plain object | ||
| if (user.toJSON) { | ||
| user = user.toJSON() | ||
| users.data[i] = user | ||
| } | ||
|
|
||
| let userOrg = user.user_organizations[0].organization_code | ||
| let validationData = await entityTypeQueries.findUserEntityTypesAndEntities({ | ||
| status: 'ACTIVE', | ||
| organization_code: { | ||
| [Op.in]: [userOrg, defaultOrganizationCode], | ||
| }, | ||
| tenant_code: params.query.tenant_code, | ||
| model_names: { [Op.contains]: [await userQueries.getModelName()] }, | ||
| }) | ||
| const prunedEntities = removeDefaultOrgEntityTypes(validationData, user.user_organizations[0].id) | ||
| const processedUser = await utils.processDbResponse(user, prunedEntities) | ||
| Object.assign(user, processedUser) | ||
| } |
There was a problem hiding this comment.
Avoid per‑user entity queries in the search loop (N+1).
Each user triggers findUserEntityTypesAndEntities (and getModelName) inside the loop, which will scale poorly. Consider precomputing modelName once and caching prunedEntities per organization_code (or batching by unique org codes) before iterating the users.
As per coding guidelines, src/services/**: This is core business logic. Please check for correctness, efficiency, and potential edge cases.
🤖 Prompt for AI Agents
In `@src/services/account.js` around lines 1968 - 1991, The loop currently calls
userQueries.getModelName() and
entityTypeQueries.findUserEntityTypesAndEntities(...) per user causing an N+1;
refactor by computing modelName once (call getModelName() before the users
loop), group users by their organization_code (use
user.user_organizations[0].organization_code or guard for missing orgs), then
for each unique org_code call findUserEntityTypesAndEntities(...) once, run
removeDefaultOrgEntityTypes(...) per org (cache prunedEntities keyed by
organization_code or user_organization id), and finally iterate users to call
utils.processDbResponse(user, cachedPrunedEntities[orgKey]) and Object.assign;
also add a fallback when user.user_organizations[0] is absent to avoid crashes.
3 participants
Summary by CodeRabbit
Bug Fixes
New Features
Improvements