Add cacheHelper, remove Redis, tenant/org scoping, cache invalidation #824
Conversation
…d update user-role service to use the corrected constant
…type read in user login process
…t and eviction methods
…e, update, and delete operations
…nization in constants
…idate cache on role updates and deletions
…paces in admin service
…cache invalidation for user organization deletions
… cache management
…ity types and profiles
…er organization ID
…update cache invalidation methods
WalkthroughAdds a centralized, namespaced cache helper and integrates tenant/ and organization-scoped caching and eviction across controllers, services, and DB queries; removes the old Redis wrapper; renames an English language constant and adds CACHE_CONFIG; updates many method signatures to accept tenant/organization context; and bumps a cache dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as Controller (v1)
participant Service as Service (User/Org/Account/etc.)
participant Cache as cacheHelper
participant Redis as Redis
participant DB as Database
Client->>Controller: HTTP request (includes tenant_code/org_code)
Controller->>Service: call(..., tenantCode, organizationCode)
Service->>Cache: getOrSet({tenantCode, orgId, ns, id, fetchFn})
alt Cache hit
Cache-->>Service: cached value
else Cache miss
Cache->>Redis: GET
alt Redis miss
Service->>DB: query(...)
DB-->>Service: data
Service->>Cache: set(key, data, ttl)
Cache->>Redis: SET EX ttl
Cache-->>Service: ack
else Redis hit
Redis-->>Cache: value
Cache-->>Service: value
end
end
Service-->>Controller: response
Controller-->>Client: response
sequenceDiagram
autonumber
participant Controller as Org Controller
participant OrgSvc as OrganizationsHelper
participant DB as DB (org queries)
participant Cache as cacheHelper
note over OrgSvc,DB: tenant-scoped validation and updates
Controller->>OrgSvc: addRelatedOrg(id, relatedIds, tenantCode)
OrgSvc->>DB: find related orgs where tenant_code = tenantCode
DB-->>OrgSvc: validated orgs
OrgSvc->>DB: update related_orgs (where tenant_code = tenantCode)
DB-->>OrgSvc: update result
par Evict caches
OrgSvc->>Cache: evictNamespace({tenantCode, orgId, ns: organization})
OrgSvc->>Cache: evictNamespace({tenantCode, orgId, ns: profile})
and Evict tenant patterns if default org
OrgSvc->>Cache: evictTenantByPattern(tenantCode, {...})
end
OrgSvc-->>Controller: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (23)
src/locales/en.json (1)
140-141: Consistency: “organisations” vs “organizations”.The new key uses “organisations” (British), while many messages use “Organization”. Align spelling across keys/messages to avoid mixed UX.
src/database/queries/userOrganization.js (1)
109-119: Avoid mutating caller-provided options; build a local queryOptions.Mutating
optionsand deletingorganizationAttributescan cause side effects at call sites. Prefer a local copy.Apply:
- if (options?.organizationAttributes?.length > 0) { - options.include = [ + if (options?.organizationAttributes?.length > 0) { + const queryOptions = { ...options } + queryOptions.include = [ { model: Organization, attributes: options.organizationAttributes, as: 'organization', where: { tenant_code: filter.tenant_code }, }, ] - delete options.organizationAttributes - } - return await UserOrganization.findAll({ + delete queryOptions.organizationAttributes + return await UserOrganization.findAll({ + where: filter, + ...queryOptions, + raw: true, + }) + } + return await UserOrganization.findAll({ where: filter, ...options, raw: true, })Also confirm
raw: truewithincludeis intended (Sequelize flattens results).src/constants/common.js (1)
116-128: Clarify TTL units and immutability of CACHE_CONFIG.Confirm elevate-node-cache v2 expects TTLs in seconds (values look like seconds). Consider freezing to prevent runtime mutation.
Apply (optional):
- CACHE_CONFIG: { + CACHE_CONFIG: Object.freeze({ enableCache: true, enableTracking: true, shards: 32, versionDefault: 0, namespaces: { profile: { name: 'profile', enabled: true, defaultTtl: 3600, useInternal: false }, entity_types: { name: 'entity_types', enabled: true, defaultTtl: 86400, useInternal: false }, tenant: { name: 'tenant', enabled: true, defaultTtl: 21600, useInternal: false }, branding: { name: 'branding', enabled: true, defaultTtl: 43200, useInternal: false }, organization: { name: 'organization', enabled: true, defaultTtl: 21600, useInternal: false }, }, - }, + }),src/services/tenant.js (3)
442-445: Good: targeted tenant cache eviction after update.Evicting by tenant pattern post-update keeps cache coherent; swallowing errors avoids blocking writes. Consider debug logging on eviction failures for observability.
526-533: Good: namespace-wide eviction after domain additions.Appropriate breadth; domains affect tenant resolution. Same note about optional debug logs on failures.
645-659: Fix success message for removeDomain flow.Response uses TENANT_DOMAINS_ADDED_SUCCESSFULLY after removal. Use a “removed”/“updated” message to avoid confusion.
Apply:
- message: 'TENANT_DOMAINS_ADDED_SUCCESSFULLY', + message: 'TENANT_DOMAINS_REMOVED_SUCCESSFULLY',If the locale key doesn’t exist, add it to
src/locales/en.jsonor reuse a neutral “updated” message.Want me to add the locale entry and wire it up?
src/helpers/userHelper.js (1)
102-116: Parallelize cache deletions for better latency.You’re deleting per-org keys sequentially. Use Promise.all to parallelize.
Apply:
- const ns = common.CACHE_CONFIG.namespaces.profile.name - for (const orgId of orgCodes) { - const fullKey = await cacheClient.buildKey({ - tenantCode: user.tenant_code, - orgId, - ns, - id: userId, - }) - await cacheClient.del(fullKey) - } + const ns = common.CACHE_CONFIG.namespaces.profile.name + await Promise.all( + orgCodes.map(async (orgId) => { + const fullKey = await cacheClient.buildKey({ + tenantCode: user.tenant_code, + orgId, + ns, + id: userId, + }) + return cacheClient.del(fullKey) + }) + )src/services/admin.js (1)
604-618: Don’t let cache failures break role assignment—wrap eviction in try/catch.Evictions should be best-effort. Wrap both evictNamespace and del.
- await cacheClient.evictNamespace({ - tenantCode, - orgId: organization.code, - ns: common.CACHE_CONFIG.namespaces.organization.name, - patternSuffix: '*', - }) - - const cacheKey = cacheClient.namespacedKey({ - tenantCode, - orgId: organization.code, - ns: common.CACHE_CONFIG.namespaces.profile.name, - id: user.id, - }) - await cacheClient.del(cacheKey) + try { + await cacheClient.evictNamespace({ + tenantCode, + orgId: organization.code, + ns: common.CACHE_CONFIG.namespaces.organization.name, + patternSuffix: '*', + }) + const cacheKey = cacheClient.namespacedKey({ + tenantCode, + orgId: organization.code, + ns: common.CACHE_CONFIG.namespaces.profile.name, + id: user.id, + }) + await cacheClient.del(cacheKey) + } catch (err) { + console.error('Cache eviction failed in addOrgAdmin:', err) + }src/services/account.js (4)
560-566: Best-effort eviction: swallow errors.Avoid failing user creation on eviction errors.
- await cacheClient.evictNamespace({ - tenantCode: tenantDetail.code, - orgId: organization.code, - ns: common.CACHE_CONFIG.namespaces.organization.name, - patternSuffix: '*', - }) + try { + await cacheClient.evictNamespace({ + tenantCode: tenantDetail.code, + orgId: organization.code, + ns: common.CACHE_CONFIG.namespaces.organization.name, + patternSuffix: '*', + }) + } catch (err) { + console.error('Cache eviction failed (create):', err) + }
769-776: Cast ALLOWED_ACTIVE_SESSIONS to number.Ensure numeric comparison to avoid string coercion edge cases.
- if (activeSessionCount >= process.env.ALLOWED_ACTIVE_SESSIONS) { + if (activeSessionCount >= Number(process.env.ALLOWED_ACTIVE_SESSIONS)) {
842-856: Guard fetchFn to return a stable array for caching.Ensure prunedEntities isn’t null/undefined if DB returns nothing.
- fetchFn: async () => { - const raw = await entityTypeQueries.findUserEntityTypesAndEntities({ + fetchFn: async () => { + const raw = await entityTypeQueries.findUserEntityTypesAndEntities({ status: 'ACTIVE', organization_code: { [Op.in]: orgCodes }, tenant_code: tenantDetail.code, model_names: { [Op.contains]: [modelName] }, }) - return removeDefaultOrgEntityTypes(raw, user.organizations[0].id) + return removeDefaultOrgEntityTypes(raw || [], user.organizations[0].id) },
2043-2051: Don’t let profile cache deletion break password changes.Wrap namespacedKey/del in try/catch.
- const ns = common.CACHE_CONFIG.namespaces.profile.name - const cacheKey = cacheClient.namespacedKey({ - tenantCode, - orgId: organizationCode, - ns, - id: userId, - }) - await cacheClient.del(cacheKey) + try { + const ns = common.CACHE_CONFIG.namespaces.profile.name + const cacheKey = cacheClient.namespacedKey({ + tenantCode, + orgId: organizationCode, + ns, + id: userId, + }) + await cacheClient.del(cacheKey) + } catch (err) { + console.error('Profile cache delete failed (changePassword):', err) + }src/services/user.js (2)
310-317: Optional: Prefer namespacedKey for consistency.
You already use namespacedKey elsewhere; consider using it here instead of buildKey to keep a single pattern.Apply this diff:
- const ns = common.CACHE_CONFIG.namespaces.profile.name - const fullKey = await cacheClient.buildKey({ - tenantCode, - orgId: organizationCode, - ns, - id, - }) + const ns = common.CACHE_CONFIG.namespaces.profile.name + const fullKey = cacheClient.namespacedKey({ tenantCode, orgId: organizationCode, ns, id })
369-384: Scope the entity-types cache by the user’s org (not requester org).
If organizationCode ≠ user’s org, you’ll fragment cache keys and miss invalidations. Use userOrg for orgId.Apply this diff:
- const prunedEntities = await cacheClient.getOrSet({ - tenantCode, - orgId: organizationCode, + const prunedEntities = await cacheClient.getOrSet({ + tenantCode, + orgId: userOrg,src/services/user-role.js (1)
94-112: DRY: Extract cache-eviction into a helper.
Both update and delete evict the same namespaces; centralize to reduce duplication and mistakes.Apply this refactor within this file:
+async function evictOrgAndProfileNamespaces(tenantCode, orgCode) { + try { + await cacheClient.evictNamespace({ tenantCode, orgId: orgCode, ns: common.CACHE_CONFIG.namespaces.organization.name }) + } catch (e) { + console.error(e) + } + try { + await cacheClient.evictNamespace({ tenantCode, orgId: orgCode, ns: common.CACHE_CONFIG.namespaces.profile.name }) + } catch (e) { + console.error(e) + } +}Then replace both blocks with:
- await cacheClient - .evictNamespace({ - tenantCode, - orgId: userOrganizationCode, - ns: common.CACHE_CONFIG.namespaces.organization.name, - }) - .catch((error) => { - console.error(error) - }) - await cacheClient - .evictNamespace({ - tenantCode, - orgId: userOrganizationCode, - ns: common.CACHE_CONFIG.namespaces.profile.name, - }) - .catch((error) => { - console.error(error) - }) + await evictOrgAndProfileNamespaces(tenantCode, userOrganizationCode)Also applies to: 161-178
src/generics/cacheHelper.js (8)
169-172: Short‑circuit getOrSet when cache is globally disabledAvoid unnecessary key builds and set attempts.
async function getOrSet({ key, tenantCode, ttl = undefined, fetchFn, orgId, ns, id, useInternal = undefined }) { + if (!ENABLE_CACHE) return await fetchFn() if (!namespaceEnabled(ns)) return await fetchFn()
241-253: Chunk SCAN deletions to avoid large-argument calls to DEL/UNLINKCalling with 1000+ spread args can hit engine limits; delete in smaller batches.
- if (keys.length) { - try { - if (unlink && typeof redis.unlink === 'function') await redis.unlink(...keys) - else await redis.del(...keys) - } catch (e) { - for (const k of keys) { - try { - if (unlink && typeof redis.unlink === 'function') await redis.unlink(k) - else await redis.del(k) - } catch (__) {} - } - } - } + if (keys.length) { + const maxArgs = 500 + for (let i = 0; i < keys.length; i += maxArgs) { + const slice = keys.slice(i, i + maxArgs) + try { + if (unlink && typeof redis.unlink === 'function') await redis.unlink(...slice) + else await redis.del(...slice) + } catch (e) { + for (const k of slice) { + try { + if (unlink && typeof redis.unlink === 'function') await redis.unlink(k) + else await redis.del(k) + } catch (__) {} + } + } + } + }
217-221: Allow namespace eviction even if the namespace is disabledOperators should be able to clear leftovers regardless of current enablement.
async function evictNamespace({ tenantCode, orgId = null, ns, patternSuffix = '*' } = {}) { if (!tenantCode || !ns) return - if (!namespaceEnabled(ns)) return const base = orgId ? `tenant:${tenantCode}:org:${orgId}` : `tenant:${tenantCode}` const pattern = `${base}:${ns}:${patternSuffix}` await scanAndDelete(pattern) }
77-90: buildKey is synchronous; drop async/await for clarity and tiny overhead reductionNo awaits inside; callers safely await a plain value anyway.
-async function buildKey({ tenantCode, orgId, ns, id, key }) { +function buildKey({ tenantCode, orgId, ns, id, key }) { @@ - const fullKey = - ns || id - ? await buildKey({ tenantCode, orgId, ns: ns || 'ns', id: id || key }) - : await buildKey({ tenantCode, key }) + const fullKey = + ns || id + ? buildKey({ tenantCode, orgId, ns: ns || 'ns', id: id || key }) + : buildKey({ tenantCode, key }) @@ - const fullKey = await buildKey({ tenantCode, orgId, ns, id }) + const fullKey = buildKey({ tenantCode, orgId, ns, id }) @@ - const fullKey = await buildKey({ tenantCode, orgId, ns, id }) + const fullKey = buildKey({ tenantCode, orgId, ns, id })Also applies to: 174-179, 195-197, 206-208
103-104: Use console.error with message first for better log readabilityMinor logging polish.
- console.log(err, 'error in getting native redis client') + console.error('error in getting native redis client', err)
72-74: Avoid filtering out numeric 0 in key constructionfilter(Boolean) drops 0; prefer explicit null/undefined check.
-return [base, ns, id].filter(Boolean).join(':') +return [base, ns, id].filter(v => v !== undefined && v !== null).join(':') @@ -const final = [base, effNs, id || key].filter(Boolean).join(':') +const final = [base, effNs, id || key].filter(v => v !== undefined && v !== null).join(':') @@ -const final = [base, key].filter(Boolean).join(':') +const final = [base, key].filter(v => v !== undefined && v !== null).join(':')Also applies to: 83-89
138-144: TTL parity for InternalCache (if supported)If InternalCache.setKey supports TTL, pass it to keep expiry consistent with Redis.
Can you confirm the signature of InternalCache.setKey in elevate-node-cache? If it accepts (key, value, ttlSeconds), mirror the TTL like Redis. If not supported, consider adding per-namespace in-memory eviction or skipping in-memory for TTL’d namespaces.
92-96: Remove unused shardOf, md5 import, and SHARD_RETENTION_DAYS (or export if intended).Declared in src/generics/cacheHelper.js (SHARD_RETENTION_DAYS ≈ line 20; shardOf ≈ lines 92–96) but not referenced or exported — delete to reduce footprint or export/use where needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/configs/redis.jsis excluded by!src/configs/**
📒 Files selected for processing (20)
src/constants/common.js(2 hunks)src/controllers/v1/organization.js(3 hunks)src/controllers/v1/user-role.js(2 hunks)src/controllers/v1/user.js(1 hunks)src/database/queries/organization.js(3 hunks)src/database/queries/userOrganization.js(1 hunks)src/generics/cacheHelper.js(1 hunks)src/generics/redis-communication.js(0 hunks)src/helpers/userHelper.js(3 hunks)src/locales/en.json(1 hunks)src/package.json(1 hunks)src/services/account.js(10 hunks)src/services/admin.js(3 hunks)src/services/entities.js(4 hunks)src/services/entityType.js(6 hunks)src/services/organization.js(6 hunks)src/services/public.js(3 hunks)src/services/tenant.js(4 hunks)src/services/user-role.js(6 hunks)src/services/user.js(4 hunks)
💤 Files with no reviewable changes (1)
- src/generics/redis-communication.js
🧰 Additional context used
📓 Path-based instructions (3)
src/database/queries/**
⚙️ CodeRabbit configuration file
Review database queries for performance. Check for N+1 problems and ensure indexes can be used.
Files:
src/database/queries/userOrganization.jssrc/database/queries/organization.js
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/entityType.jssrc/services/user-role.jssrc/services/entities.jssrc/services/account.jssrc/services/admin.jssrc/services/tenant.jssrc/services/public.jssrc/services/user.js
src/controllers/**
⚙️ CodeRabbit configuration file
These are API controllers. Focus on request/response handling, security (auth middleware usage), and proper status codes.
Files:
src/controllers/v1/user-role.jssrc/controllers/v1/user.jssrc/controllers/v1/organization.js
🧠 Learnings (8)
📚 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/database/queries/userOrganization.jssrc/controllers/v1/user-role.jssrc/helpers/userHelper.jssrc/database/queries/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 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/database/queries/userOrganization.jssrc/services/organization.jssrc/helpers/userHelper.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/database/queries/userOrganization.jssrc/helpers/userHelper.jssrc/services/admin.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/services/organization.jssrc/controllers/v1/user.jssrc/database/queries/organization.jssrc/controllers/v1/organization.js
📚 Learning: 2025-07-31T08:43:35.971Z
Learnt from: nevil-mathew
PR: ELEVATE-Project/user#776
File: src/database/models/entityType.js:38-38
Timestamp: 2025-07-31T08:43:35.971Z
Learning: The migration for converting tenant_code to a primary key in the EntityType model was already handled in a previous PR, not in the current refactoring PR that focuses on organization codes instead of organization IDs.
Applied to files:
src/services/organization.jssrc/database/queries/organization.jssrc/controllers/v1/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/helpers/userHelper.jssrc/services/user.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/helpers/userHelper.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: When session management is shared across helpers and services in ELEVATE-Project/user, prefer extracting the primitives (query active sessions, delete Redis keys, mark ended) into a lower-level generics utility to avoid helper↔service circular dependencies and keep Redis key handling consistent.
Applied to files:
src/helpers/userHelper.js
🧬 Code graph analysis (8)
src/services/user-role.js (3)
src/services/user.js (7)
require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)userOrganizationCode(478-478)common(10-10)src/generics/cacheHelper.js (2)
require(3-3)common(5-5)src/controllers/v1/user-role.js (1)
updateRole(60-66)
src/services/entities.js (2)
src/generics/cacheHelper.js (2)
common(5-5)require(3-3)src/services/tenant.js (3)
common(10-10)require(32-32)cacheClient(33-33)
src/generics/cacheHelper.js (1)
src/services/user.js (10)
require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)common(10-10)ns(146-146)ns(310-310)ns(591-591)fullKey(311-316)
src/helpers/userHelper.js (3)
src/services/user.js (16)
cacheClient(28-28)require(17-17)require(19-19)require(20-20)require(24-24)require(26-26)user(50-53)user(339-339)user(462-462)user(512-512)user(547-547)ns(146-146)ns(310-310)ns(591-591)common(10-10)fullKey(311-316)src/generics/cacheHelper.js (5)
require(3-3)common(5-5)fullKey(174-177)fullKey(195-195)fullKey(206-206)src/database/queries/userOrganization.js (1)
require(3-3)
src/services/tenant.js (1)
src/generics/cacheHelper.js (2)
require(3-3)common(5-5)
src/database/queries/organization.js (2)
src/controllers/v1/organization.js (8)
result(124-124)result(143-143)result(159-163)result(180-185)result(194-198)result(206-210)result(251-255)result(296-300)src/database/models/organization.js (1)
Organization(3-67)
src/services/public.js (2)
src/services/tenant.js (5)
require(32-32)common(10-10)tenantQueries(11-11)responses(31-31)httpStatusCode(9-9)src/generics/cacheHelper.js (2)
require(3-3)common(5-5)
src/services/user.js (3)
src/generics/utils.js (3)
removeDefaultOrgEntityTypes(835-842)value(892-892)value(914-914)src/generics/cacheHelper.js (4)
fullKey(174-177)fullKey(195-195)fullKey(206-206)value(182-182)src/controllers/v1/user.js (2)
userDetails(48-54)userDetails(85-85)
🔇 Additional comments (26)
src/controllers/v1/user.js (1)
52-54: Signature/order OK — no changes required.
Controller passes tenant_code then organization_code; src/services/user.js defines read(id, header = null, language, tenantCode, organizationCode) so the argument order matches.src/controllers/v1/user-role.js (1)
64-66: No action required — roleService.update/delete signatures match controller.src/services/user-role.js defines:
- update(id, bodyData, userOrganizationId, userOrganizationCode, tenantCode)
- delete(id, userOrganizationId, userOrganizationCode, tenantCode)
src/constants/common.js (1)
104-104: Potential breaking change — add deprecated alias for ENGLISH_LANGUGE_CODESearch returned no matches in the repo; absence of evidence isn’t proof. Add a temporary deprecated alias to avoid regressions.
File: src/constants/common.js:104
ENGLISH_LANGUAGE_CODE: 'en', +// Deprecated alias; remove after downstreams migrate. +ENGLISH_LANGUGE_CODE: 'en',src/package.json (1)
46-46: Confirm elevate-node-cache v2 compatibility and API usage.Dependency is in src/package.json:46 and imports exist in src/generics/cacheHelper.js and src/generics/utils.js (const { RedisCache, InternalCache } = require('elevate-node-cache')). Code uses RedisCache.getKey / setKey(key, value, ttlSeconds) / deleteKey / native(), InternalCache.getKey / setKey / delKey, and higher-level helpers (getOrSet, evictNamespace, evictTenantByPattern) across services. Verify v2 preserves these method names/semantics and that setKey's TTL units haven’t changed (seconds vs ms); update call-sites if any API/behavior changed.
src/services/entities.js (1)
55-57: LGTM: Post‑mutation cache invalidation placement is correct.Invalidate after successful create/update/delete and rely on the internal try/catch. Good.
Please ensure DEFAULT_ORGANISATION_CODE is set consistently across environments, as it gates tenant-wide evictions.
Also applies to: 115-117, 197-199
src/helpers/userHelper.js (1)
73-79: Good capture of org codes pre-deletion for targeted cache eviction.Fetching and deduping org codes before removing mappings ensures accurate cache key computation.
src/services/admin.js (1)
40-40: LGTM: cache client import.Consistent with new caching strategy.
src/database/queries/organization.js (2)
116-133: LGTM: tenant-scoped whereClause prevents duplicates and enforces tenancy.Approach with Op.contains/null guard is correct and index-friendly on id/tenant_code.
Ensure a GIN index exists on related_orgs if large datasets depend on the @> operator for performance.
152-170: Incorrect — array_remove is correct here (single value removal).Callers pass the single org id as the value to remove and an array of target org ids as the WHERE ids (src/controllers/v1/organization.js:204–208; src/services/organization.js:637,686–688), so the current update removes that one value from each target's related_orgs — no change required.
Likely an incorrect or invalid review comment.
src/services/public.js (4)
15-15: Import of cache client looks correct and consistent with PR direction.
37-42: LGTM: Tenant-scoped getOrSet for tenant details.
Keying by tenant namespace with id=code is appropriate.
49-58: LGTM: Tenant- and org-scoped getOrSet for organization details.
Filter includes tenant_code, which is correct.
62-62: Verify status code change to 200 OK.
This endpoint previously returned 201; confirm clients/tests don’t rely on 201.src/services/user.js (3)
28-29: Import of cache client aligns with new cache layer.
146-154: LGTM: Evict profile cache on update.
Namespaced key and del are used correctly.
591-599: LGTM: Evict profile cache on language preference change.src/services/entityType.js (5)
50-52: LGTM: Post-create cache invalidation.
92-94: LGTM: Post-update cache invalidation.
112-138: LGTM: Cached readAllSystemEntityTypes with pruning.
157-184: LGTM: Cached readUserEntityTypes result shape is consistent.
13-39: Invalidate caches on entity-type mutations — pattern check OK.
evictNamespace and evictTenantByPattern produce patterns that match buildKey output (tenant:tenantCode:org:orgId:ns:id and tenant:tenantCode:ns:id), so the patternSuffixorg:*:<namespace>:*will correctly match entity-type/profile keys.src/services/user-role.js (2)
82-82: Signatures extended with org/tenant context — LGTM.
Aligns with controller changes.Also applies to: 148-148
244-244: LGTM: Language constant fixed to ENGLISH_LANGUAGE_CODE.src/services/organization.js (3)
196-214: LGTM: Evict organization and profile namespaces on update.
525-559: LGTM: Tenant-scoped validation and normalization for addRelatedOrg.
Not-found path is clear and tenant-safe.
590-617: Confirmed: appendRelatedOrg/removeRelatedOrg return { rowsAffected, updatedRows } when called with returning: trueBoth functions in src/database/queries/organization.js destructure Sequelize's update result and return { rowsAffected, updatedRows } when options.returning is truthy — using updatedRows in the service is valid.
…and implement cache eviction logic
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
Summary by CodeRabbit
New Features
Performance
Bug Fixes
Localization
Chores