Project factory tenant id fix #1779
Conversation
ritikegov
requested review from
jagankumar-egov,
kavi-egov and
sathishp-eGov
as code owners
WalkthroughThis PR refactors tenant ID handling across the project-factory service to be more explicit, passing tenantId as an optional parameter through validation and API layers, removing hardcoded defaults from configuration, and adding a GitHub Actions workflow trigger branch. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Rationale: Moderate complexity due to tenant ID threading across multiple API and validation layers, signature changes requiring call-site verification, and a potentially breaking configuration change (removed fallback default). Changes are cohesive in intent but heterogeneous in scope, requiring separate reasoning for config behavior, API signatures, and validator logic. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
health-services/project-factory/src/server/api/campaignApis.ts (1)
1502-1524: Don’t publish Kafka messages with undefined tenantId; remove fallback and fail fastdefaultTenantId can now be undefined; publishing with
tenantId || config.app.defaultTenantIdrisks an empty tenant and cross-tenant leakage. Resolve to a single effective tenantId and enforce presence.Apply:
- const tenantId = request?.body?.ResourceDetails?.tenantId; + const tenantId = + request?.body?.ResourceDetails?.tenantId ?? + request?.query?.tenantId ?? + config.app.defaultTenantId; + if (!tenantId) { + logger.error("TenantId missing while publishing Kafka messages"); + throwError("COMMON", 400, "VALIDATION_ERROR", "tenantId is required"); + } ... - await produceModifiedMessages( - persistMessage, - config?.kafka?.KAFKA_UPDATE_RESOURCE_DETAILS_TOPIC, - request?.body?.ResourceDetails?.tenantId - ); + await produceModifiedMessages( + persistMessage, + config?.kafka?.KAFKA_UPDATE_RESOURCE_DETAILS_TOPIC, + tenantId + ); ... - tenantId || config.app.defaultTenantId + tenantIdAdditionally, align other paths to avoid hardcoded tenants (e.g., Line 777 sets
tenantId: 'mz'in getEmployeesBasedOnUserName). Replace with the request’s tenantId.Also applies to: 1539-1543
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/buildProjectFactory.yml(1 hunks)health-services/project-factory/src/server/api/campaignApis.ts(2 hunks)health-services/project-factory/src/server/api/healthApis.ts(1 hunks)health-services/project-factory/src/server/config/index.ts(1 hunks)health-services/project-factory/src/server/validators/campaignValidators.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
health-services/project-factory/src/server/validators/campaignValidators.ts (1)
health-services/project-factory/src/server/api/healthApis.ts (1)
fetchProductVariants(7-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/buildProjectFactory.yml (1)
8-8: Trigger addition looks fineBranch added to push triggers; no issues.
| export async function fetchProductVariants(pvarIds: string[] , tenantId?: string) { | ||
| const CHUNK_SIZE = 100; | ||
| const allProductVariants: any[] = []; | ||
| const params: any = { limit: CHUNK_SIZE, offset: 0, tenantId: defaultRequestInfo?.RequestInfo?.userInfo?.tenantId }; | ||
| const params: any = { limit: CHUNK_SIZE, offset: 0, tenantId: tenantId }; | ||
|
|
There was a problem hiding this comment.
Enforce tenantId; avoid sending undefined in query params
Passing tenantId: undefined can generate invalid requests. Fail fast if tenantId is missing.
-export async function fetchProductVariants(pvarIds: string[] , tenantId?: string) {
+export async function fetchProductVariants(pvarIds: string[] , tenantId?: string) {
const CHUNK_SIZE = 100;
const allProductVariants: any[] = [];
- const params: any = { limit: CHUNK_SIZE, offset: 0, tenantId: tenantId };
+ if (!tenantId) {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "tenantId is required to fetch product variants");
+ }
+ const params: any = { limit: CHUNK_SIZE, offset: 0, tenantId };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In health-services/project-factory/src/server/api/healthApis.ts around lines 7
to 11, the function accepts tenantId as optional and then includes tenantId:
undefined in query params; enforce tenantId presence by making the parameter
required in the function signature (remove the optional ?) and/or add an early
runtime check (if tenantId is missing throw a clear Error like "tenantId is
required") so you never send undefined in params, and update any callers to pass
a valid tenantId.
| logLevel: process.env.APP_LOG_LEVEL || "debug", | ||
| debugLogCharLimit: process.env.APP_MAX_DEBUG_CHAR ? Number(process.env.APP_MAX_DEBUG_CHAR) : 1000, | ||
| defaultTenantId: process.env.DEFAULT_TENANT_ID || "mz", | ||
| defaultTenantId: process.env.DEFAULT_TENANT_ID , |
There was a problem hiding this comment.
🧩 Analysis chain
Audit usages of defaultTenantId; it may now be undefined
Since there’s no fallback, ensure no code depends on a non-empty default. Prefer explicit tenantId everywhere; otherwise, add boot-time validation or remove fallbacks.
Run to find usages relying on defaultTenantId:
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=ts --type=tsx --type=js --type=jsx '\bdefaultTenantId\b'Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n -C2 'defaultTenantId'Length of output: 1556
🏁 Script executed:
#!/bin/bash
# Get context around coreApis.ts:15
echo "=== coreApis.ts context ==="
sed -n '1,30p' health-services/project-factory/src/server/api/coreApis.ts
echo -e "\n=== campaignApis.ts context around 1542 ==="
sed -n '1535,1550p' health-services/project-factory/src/server/api/campaignApis.tsLength of output: 1601
🏁 Script executed:
#!/bin/bash
# Check if there's any git history showing what changed on line 97
echo "=== Checking git log for config/index.ts ==="
cd health-services/project-factory && git log -p --follow -S 'defaultTenantId' -- src/server/config/index.ts 2>/dev/null | head -50
echo -e "\n=== Searching for DEFAULT_TENANT_ID in .env or docs ==="
find . -name ".env*" -o -name "*.md" | xargs grep -l "DEFAULT_TENANT_ID" 2>/dev/null || echo "No env file references found"
echo -e "\n=== Checking produceModifiedMessages signature ==="
rg -n -A5 'function produceModifiedMessages|const produceModifiedMessages|export.*produceModifiedMessages' | head -30Length of output: 1033
Enforce a non-empty DEFAULT_TENANT_ID or restore a fallback
- coreApis.ts (src/server/api/coreApis.ts:15):
defaultRequestInfo.RequestInfo.userInfo.tenantIdmay now be undefined, breaking MDMS calls. - campaignApis.ts (src/server/api/campaignApis.ts:1542):
tenantId || config.app.defaultTenantIdcan pass undefined toproduceModifiedMessages, causingtenantId.includes()to throw.
Add a boot-time assertion thatprocess.env.DEFAULT_TENANT_IDis set or reintroduce a safe default.
🤖 Prompt for AI Agents
In health-services/project-factory/src/server/config/index.ts around line 97,
the export for defaultTenantId currently assigns process.env.DEFAULT_TENANT_ID
which may be undefined at runtime; add a boot-time assertion that
DEFAULT_TENANT_ID is present and non-empty (throw a clear error and exit during
startup if missing) or explicitly set a safe fallback value before exporting
(e.g., a validated constant like 'default' or read from a secure config source)
so downstream code never receives undefined and functions like
tenantId.includes() are safe.
| const tenantId = request?.body?.CampaignDetails?.tenantId; | ||
| if(tenantId === undefined){ | ||
| logger.error("tenantId is undefined"); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Require tenantId for product-variant validation; validate at both levels
Logging when tenantId is undefined but continuing leads to invalid downstream calls. Enforce presence and propagate explicitly.
- const tenantId = request?.body?.CampaignDetails?.tenantId;
- if(tenantId === undefined){
- logger.error("tenantId is undefined");
- }
+ const tenantId = request?.body?.CampaignDetails?.tenantId;
+ if (!tenantId) {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "tenantId is required to validate product variants");
+ }
...
- await validatePvarIds(pvarIds as string[] ,tenantId);
+ await validatePvarIds(pvarIds as string[], tenantId);Also guard inside validatePvarIds:
-async function validatePvarIds(pvarIds: string[] , tenantId?: string) {
+async function validatePvarIds(pvarIds: string[] , tenantId?: string) {
+ if (!tenantId) {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "tenantId is required to validate product variants");
+ }
if (!pvarIds?.length || pvarIds.some((id:any) => !id)) {
throwError("COMMON", 400, "VALIDATION_ERROR", "productVariantId is required in every delivery rule's resources");
}
- const allProductVariants = await fetchProductVariants(pvarIds ,tenantId);
+ const allProductVariants = await fetchProductVariants(pvarIds, tenantId);Check for any other call sites that still use the old signature:
Also applies to: 1281-1283, 1285-1293
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=ts --type=tsx '\bvalidatePvarIds\s*\('Length of output: 27
1 participant
Summary by CodeRabbit
Bug Fixes
Chores