console recent changes to merged in master

[nitish-egov]

Summary by CodeRabbit

• New Features

  • Introduced a new debugging configuration for enhanced error tracking and debugging capabilities.
  • Added a cache burst functionality for localization after chunk uploads.
  • Enhanced validation for microplan campaigns with additional checks for boundary presence.
  • Improved handling of campaign details and boundary management.
  • Added support for dynamic chunk size configuration during uploads.
  • Implemented Gzip request handling for improved payload processing.
  • Added a new method for processing target calculations, streamlining data handling.
  • Streamlined facility creation and searching configurations.
  • Introduced a new utility for handling Gzip-compressed requests.
  • Added new functions for validating Excel file metadata and enriching template metadata.
  • Added a new TypeScript configuration file for debugging purposes.
  • Added a new column for locale information in the resource details table.

• Bug Fixes

  • Improved error handling in various functions to provide clearer messages and prevent runtime errors.

• Documentation

  • Added comments to the TypeScript configuration file for better clarity on compiler options.

• Refactor

  • Updated several utility functions for better readability and maintainability, particularly in campaign and boundary management.
  • Enhanced middleware to support Gzip content type processing.
  • Simplified data processing logic by modularizing target calculations.
  • Refactored bulk and single data creation logic for improved clarity and maintainability.
  • Removed unnecessary properties from the configuration related to facility microplan.
  • Updated the request middleware to handle Gzip content type.

[coderabbitai]

Walkthrough

The changes in this pull request involve several modifications across various files in the project-factory project. Key updates include changes to the Dockerfile to specify the command for running the application using Yarn, enhancements to the package.json scripts for debugging capabilities, and additions to configuration files to support new properties. New methods and error handling improvements were also introduced in several utility and controller files, enhancing functionality and maintainability. A new TypeScript configuration file was added, and existing configurations were updated for clarity.

Changes

File Path	Change Summary
health-services/project-factory/Dockerfile	Updated command to run application using Yarn in production mode (CMD ["yarn", "prod"]).
health-services/project-factory/package.json	Modified serve and prod scripts to include debugging capabilities based on the DEBUG environment variable.
health-services/project-factory/src/server/config/index.ts	Added new properties to facility and localisation objects, enhancing configuration options.
health-services/project-factory/src/server/controllers/localisationController/localisation.controller.ts	Introduced new method cacheBurst and improved error handling in createLocalisation.
health-services/project-factory/src/server/utils/campaignUtils.ts	Refactored functions for campaign and boundary management, improved error handling, and introduced new utility functions.
health-services/project-factory/src/server/utils/genericUtils.ts	Cosmetic changes for readability and consistency in formatting and logging.
health-services/project-factory/src/server/utils/microplanUtils.ts	Updated validateNationalDuplicacy function to use localizationMap, enhancing phone number validation.
health-services/project-factory/src/server/utils/onGoingCampaignUpdateUtils.ts	Updated modifyProcessedSheetData function to include a new schema parameter for improved data processing.
health-services/project-factory/src/server/utils/request.ts	Enhanced error handling and logging in the httpRequest function.
health-services/project-factory/src/server/utils/transforms/localisationMessageConstructor.ts	Made CHUNK_SIZE dynamic and improved error handling in uploadInChunks.
health-services/project-factory/src/server/validators/campaignValidators.ts	Added import for validateExtraBoundariesForMicroplan and integrated it into immediateValidationForTargetSheet.
health-services/project-factory/src/server/validators/microplanValidators.ts	Updated validateMicroplanFacility error message and added validateExtraBoundariesForMicroplan function for additional validation checks.
health-services/project-factory/tsconfig.debug.json	Introduced new TypeScript configuration file with various compiler options.
health-services/project-factory/tsconfig.json	Added comments to clarify compiler options; no changes to actual configuration values.
health-services/project-factory/src/server/api/campaignApis.ts	Updated performAndSaveResourceActivity to log accurate creation time and refined projectCreate to simplify boundary project mapping logic.
health-services/project-factory/src/server/app.ts	Removed redundant JSON body parser middleware initialization.
health-services/project-factory/src/server/utils/gzipHandler.ts	Introduced a new utility for handling Gzip-compressed requests, including functions for processing and decompressing Gzip data.
health-services/project-factory/src/server/utils/middlewares/requestMiddleware.ts	Updated to handle both application/json and application/gzip content types, integrating Gzip request handling.
health-services/project-factory/src/server/api/genericApis.ts	Integrated processDataForTargetCalculation function to streamline target data processing in getTargetSheetDataAfterCode.
health-services/project-factory/src/server/config/createAndSearch.ts	Removed createBulkDetails and searchDetails from facilityMicroplan, added createDetails property for simplified facility creation logic.
health-services/project-factory/src/server/config/models/createRequestSchema.ts	Removed facilityMicroplan from enum in createRequestSchema, streamlining validation options.
health-services/project-factory/src/server/utils/excelUtils.ts	Added new functions for validating file metadata and enriching template metadata, enhancing Excel file processing.
health-services/project-factory/src/server/utils/campaignMappingUtils.ts	Changed retry variable to be configurable instead of a fixed value.
health-services/project-factory/src/server/config/constants.ts	Introduced a new constant usageColumnStatus for active and inactive statuses.
health-services/project-factory/src/server/utils/microplanIntergration.ts	Updated functions to use usageColumnStatus for standardizing active/inactive status representations.
health-services/project-factory/migration/main/V202422115500__add_locale_generated_resouce.sql	Added new column locale to eg_cm_generated_resource_details table.
health-services/project-factory/src/server/utils/transforms/searchResponseConstructor.ts	Added locale property to generatedResourceTransformer function to include locale information in output.

Possibly related PRs

• Admin Console & microplanning Patch fixes on project factory  #1276: This PR modifies the Dockerfile to explicitly set the command to run the application as CMD ["yarn", "prod"], which is directly related to the changes made in the main PR that also updates the Dockerfile with the same command.
• added changes for performance testing #1236: The modifications to the package.json file, particularly the updates to the prod script that checks the DEBUG environment variable, are relevant as they enhance the control flow for running the application, similar to the changes made in the main PR.
• feat: Implement facility list with inactive by default and toggle act… #1343: This PR introduces a new constant usageColumnStatus which is used in various places to replace hardcoded strings for active and inactive statuses. The main PR also includes changes to the package.json and index.ts files that enhance the handling of statuses, indicating a broader context of improving maintainability and consistency in status management across the codebase.

🐰 In the code, I hop and play,
With scripts and configs, I pave the way.
Yarn runs the show, in production we thrive,
New methods and checks keep our code alive!
Debugging made easy, with just a little tweak,
In the land of code, it's the rabbits we seek! 🐇✨

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 24

🧹 Outside diff range comments (7)

health-services/project-factory/src/server/utils/request.ts (2)

Line range hint 132-196: Enhance error logging for better debugging

While the error handling is comprehensive, the logging can be improved to help with debugging issues.

Consider enhancing the error logging:

     } catch (error: any) {
       const errorResponse = error?.response;
+      const requestDetails = {
+        url: _url,
+        method: _method,
+        params: _params,
+        serviceName: getServiceName(_url)
+      };
       logger.error(
-        "INTER-SERVICE :: FAILURE :: " +
-        getServiceName(_url) +
-        ":: CODE :: " +
-        errorResponse?.status +
-        ":: ERROR :: " +
-        (errorResponse?.data?.Errors?.[0]?.code || error) +
-        ":: DESCRIPTION :: " +
-        errorResponse?.data?.Errors?.[0]?.description
+        JSON.stringify({
+          level: "ERROR",
+          message: "INTER-SERVICE_FAILURE",
+          request: requestDetails,
+          response: {
+            status: errorResponse?.status,
+            error: errorResponse?.data?.Errors?.[0]?.code || error,
+            description: errorResponse?.data?.Errors?.[0]?.description
+          }
+        })
       );

This change:

1. Adds request details to help with debugging
2. Structures logs in a consistent JSON format
3. Makes logs easier to parse and analyze

---

Line range hint 67-67: Improve caching configuration and strategy

The current caching implementation has several areas for improvement:

1. Hard-coded TTL value
2. Non-standardized cache key generation
3. Missing cache invalidation strategy for error cases

Consider these improvements:

1. Move cache configuration to config file:

-  const cacheTTL = 300; // TTL in seconds (5 minutes)
+  const cacheTTL = config.cacheValues.ttl || 300;

2. Standardize cache key generation:

-  const cacheKey = headers && headers.cachekey ? `cache:${headers.cachekey}` : null;
+  const cacheKey = headers?.cachekey ? 
+    `cache:${config.values.env}:${getServiceName(_url)}:${headers.cachekey}` : null;

3. Add cache invalidation for error responses in the catch block:

if (cacheKey && isRedisConnected) {
  await redis.del(cacheKey);
}

Also applies to: 69-69

health-services/project-factory/tsconfig.json (2)

Line range hint 8-9: Enable source maps for improved debugging experience

Given that this PR involves "console recent changes" and debugging capabilities, consider enabling source maps to improve the debugging experience. Currently, source map generation is commented out.

Apply this diff to enable source maps:

    // "declaration": true,                   /* Generates corresponding '.d.ts' file. */
    // "declarationMap": true,                /* Generates a sourcemap for each corresponding '.d.ts' file. */
-   // "sourceMap": true,                     /* Generates corresponding '.map' file. */
+   "sourceMap": true,                        /* Generates corresponding '.map' file. */
    // "outFile": "./",                       /* Concatenate and emit output to single file. */

Also applies to: 53-57

---

Update typeRoots configuration to include local type definitions

The verification revealed that there are local TypeScript declaration files (*.d.ts) in the src/server/models directory that define important interfaces for boundary and MDMS functionality. These files are actively used in the codebase through imports. The current typeRoots configuration that only includes ./node_modules/@types would indeed be too restrictive.

• Add "./src/server/models" to the typeRoots array in tsconfig.json to ensure these local type definitions are properly recognized
• Alternatively, consider using the paths compiler option to map these specific type modules if you want more granular control

🔗 Analysis chain

Line range hint 49-50: Verify module resolution configuration

The typeRoots setting is configured to only look in ./node_modules/@types. This might be too restrictive if you have local type definitions.

Let's check if there are any local type definitions that might be excluded:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for local type definition files
# Look for .d.ts files outside node_modules

echo "Local type definition files:"
fd "\.d\.ts$" --exclude "node_modules"

Length of output: 229

---

Script:

#!/bin/bash
# Let's check the content of these type definition files to understand their usage
echo "Content of Boundary.d.ts:"
cat health-services/project-factory/src/server/models/Boundary.d.ts

echo -e "\nContent of MDMS.d.ts:"
cat health-services/project-factory/src/server/models/MDMS.d.ts

# Also check if these types are being imported anywhere
echo -e "\nUsage of these type definitions:"
rg -l "from ['\"].*/(Boundary|MDMS)['\"]" --type ts

Length of output: 8706

health-services/project-factory/src/server/utils/campaignUtils.ts (3)

Line range hint 932-932: Use let or const instead of var for variable declarations

The var keyword has function scope and can lead to unexpected behavior. It's recommended to use let or const for block-scoped variables. Please replace var with let or const in the following instances.

Apply the following diffs:

- var columns = {};
+ const columns = {};

- var errorData = request.body.sheetErrorDetails;
+ let errorData = request.body.sheetErrorDetails;

- var errorMap: any = {};
+ let errorMap: any = {};

- var newSheetErrorDetails: any = [];
+ let newSheetErrorDetails: any = [];

- var createResourceDetailsIds: any[] = [];
+ const createResourceDetailsIds: any[] = [];

- var parentCampaign = requestBody?.parentCampaign;
+ const parentCampaign = requestBody?.parentCampaign;

Also applies to: 936-936, 957-957, 970-970, 1071-1071, 1322-1322

---

Line range hint 2427-2427: Replace console.log with logger.error for error logging

Using console.log is discouraged in production code. Use the provided logging mechanism for consistent error logging.

Apply the following diffs:

-        console.log(error);
+        logger.error(error);

Also applies to: 2450-2450

---

Line range hint 2458-2461: Handle potential errors when updating the campaign

In the updateCampaignAfterSearch function, ensure that errors returned from updateProjectTypeCampaignService are properly handled to avoid unhandled promise rejections.

Consider adding a try...catch block around the updateProjectTypeCampaignService call:

      try {
          await updateProjectTypeCampaignService(req);
          logger.info("Updated the received campaign object");
      } catch (error: any) {
          logger.error(`Error updating campaign: ${error.message}`);
          // Handle the error appropriately
      }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd9a00f and 3ed33e6.

📒 Files selected for processing (14)

• health-services/project-factory/Dockerfile (1 hunks)
• health-services/project-factory/package.json (1 hunks)
• health-services/project-factory/src/server/config/index.ts (3 hunks)
• health-services/project-factory/src/server/controllers/localisationController/localisation.controller.ts (3 hunks)
• health-services/project-factory/src/server/utils/campaignUtils.ts (4 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (14 hunks)
• health-services/project-factory/src/server/utils/microplanUtils.ts (2 hunks)
• health-services/project-factory/src/server/utils/onGoingCampaignUpdateUtils.ts (2 hunks)
• health-services/project-factory/src/server/utils/request.ts (1 hunks)
• health-services/project-factory/src/server/utils/transforms/localisationMessageConstructor.ts (3 hunks)
• health-services/project-factory/src/server/validators/campaignValidators.ts (2 hunks)
• health-services/project-factory/src/server/validators/microplanValidators.ts (3 hunks)
• health-services/project-factory/tsconfig.debug.json (1 hunks)
• health-services/project-factory/tsconfig.json (1 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/config/index.ts

[error] 100-100: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 911-911: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1313-1313: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/utils/onGoingCampaignUpdateUtils.ts

[error] 82-82: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

health-services/project-factory/src/server/utils/microplanUtils.ts

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

health-services/project-factory/src/server/utils/campaignUtils.ts

[error] 2417-2417: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

health-services/project-factory/tsconfig.debug.json

[error] 4-4: JSON standard does not allow comments.

(parse)

---

[error] 6-6: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 6-6: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 6-6: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 6-6: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 6-6: JSON standard does not allow comments.

(parse)

---

[error] 7-7: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 7-7: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 7-7: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 7-13: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 7-7: JSON standard does not allow comments.

(parse)

---

[error] 14-14: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 14-14: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 14-14: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 14-15: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 14-14: JSON standard does not allow comments.

(parse)

---

[error] 16-16: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 16-16: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 16-16: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 16-25: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 16-16: JSON standard does not allow comments.

(parse)

---

[error] 25-25: JSON standard does not allow comments.

(parse)

---

[error] 26-26: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 26-26: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 26-26: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 26-26: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 26-26: JSON standard does not allow comments.

(parse)

---

[error] 27-27: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 27-27: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 27-27: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 27-27: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 27-27: JSON standard does not allow comments.

(parse)

---

[error] 28-28: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 28-28: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 28-28: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 28-28: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 28-28: JSON standard does not allow comments.

(parse)

---

[error] 29-29: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 29-29: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 29-29: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 29-32: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 29-29: JSON standard does not allow comments.

(parse)

---

[error] 33-33: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 33-33: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 33-33: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 33-34: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 33-33: JSON standard does not allow comments.

(parse)

---

[error] 34-34: JSON standard does not allow comments.

(parse)

---

[error] 35-35: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 35-35: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 35-35: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 35-36: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 35-35: JSON standard does not allow comments.

(parse)

---

[error] 37-37: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 37-37: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 37-37: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 37-39: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 37-37: JSON standard does not allow comments.

(parse)

---

[error] 39-39: JSON standard does not allow comments.

(parse)

---

[error] 40-40: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 40-40: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 40-40: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 40-40: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 40-40: JSON standard does not allow comments.

(parse)

---

[error] 41-41: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 41-41: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 41-41: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 41-43: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 41-41: JSON standard does not allow comments.

(parse)

---

[error] 44-44: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 44-44: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 44-46: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 46-48: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 46-46: JSON standard does not allow comments.

(parse)

---

[error] 49-49: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 49-49: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 49-49: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 49-49: JSON standard does not allow comments.

(parse)

---

[error] 52-52: JSON standard does not allow comments.

(parse)

---

[error] 57-57: JSON standard does not allow comments.

(parse)

---

[error] 61-61: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 61-61: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 61-63: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 63-63: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 64-64: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 64-64: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 64-67: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 67-68: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

🔇 Additional comments (11)

health-services/project-factory/src/server/validators/microplanValidators.ts (1)

273-273: Improved error message clarity

The error message now accurately reflects that the facility capacity validation checks for both empty and zero values.

health-services/project-factory/src/server/validators/campaignValidators.ts (2)

25-25: LGTM!

The import statement is correctly added and properly grouped with related microplan validator imports.

---

1323-1323: LGTM!

The validateExtraBoundariesForMicroplan function is well-integrated into the validation flow, placed at an appropriate point in the sequence of validations.

health-services/project-factory/Dockerfile (1)

Line range hint 1-1: Verify compatibility with Node.js 20

The base image node:20 is relatively recent. Ensure all dependencies and code are compatible with Node.js 20.

Let's check the package.json for any potential compatibility issues:

✅ Verification successful

Node.js 20 upgrade is compatible with the codebase

The upgrade from Node.js 16 to 20 is safe based on the following evidence:

• The package.json dependencies are all compatible with Node.js 20, with many packages explicitly supporting Node.js >=18.0.0
• The codebase already has @types/node v20.11.29 as a dependency
• The LOCALSETUP.md file already recommends Node.js 20
• The TypeScript codebase shows modern patterns compatible with Node.js 20
• Key dependencies like Express, Axios, and Winston are on recent versions that support Node.js 20

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check package.json for engine constraints and dependencies

# Check for engine constraints
echo "Checking engine constraints..."
if [ -f package.json ]; then
    jq '.engines' package.json
fi

# List direct dependencies
echo "Checking dependencies..."
if [ -f package.json ]; then
    jq '.dependencies' package.json
fi

Length of output: 181

---

Script:

#!/bin/bash
# Let's try to find the package.json in the correct directory
echo "Looking for package.json in health-services/project-factory..."
if [ -f "health-services/project-factory/package.json" ]; then
    echo "Found package.json. Checking contents..."
    jq '.engines, .dependencies' health-services/project-factory/package.json
else
    echo "package.json not found in health-services/project-factory"
fi

# Let's also check for any TypeScript files to understand the codebase better
echo -e "\nChecking for TypeScript files..."
fd -e ts -p health-services/project-factory/

# Check for any existing Node.js version references in the codebase
echo -e "\nChecking for Node.js version references..."
rg -i "node.*[0-9]{2}" health-services/project-factory/

Length of output: 32572

health-services/project-factory/tsconfig.json (1)

Line range hint 77-81: Review test file exclusion pattern

The current configuration excludes test files with **/*.test.ts pattern but includes all source files. This might miss test files with different naming patterns (e.g., .spec.ts).

Let's verify if there are any test files that might be unintentionally included:

health-services/project-factory/tsconfig.debug.json (1)

64-67: Verify test file exclusion for debugging

The configuration excludes all test files (**/*.test.ts). Consider if you need to debug test files, in which case you might want to:

1. Remove the test file exclusion
2. Create a separate debug configuration for tests

✅ Verification successful

Test files are needed for debugging

The codebase contains multiple TypeScript files with test-related code (describe/it/test patterns) within the source files themselves. Since these test patterns are integrated into the source files rather than separate .test.ts files, excluding **/*.test.ts won't affect debugging capabilities. The current exclusion pattern is safe to keep.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for test files that might need debugging
fd -e test.ts

Length of output: 15

---

Script:

#!/bin/bash
# Let's try a different approach to find test files
rg -g "*.ts" "^.*test.*\.(ts|tsx)$" --files-only

# Also check for any test-related patterns in the codebase
rg -g "*.ts" -l "(describe|it|test)\s*\(" 

# Check tsconfig patterns in the repository
fd tsconfig

Length of output: 1897

🧰 Tools

🪛 Biome (1.9.4)

[error] 64-64: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 64-64: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 64-67: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

health-services/project-factory/src/server/utils/campaignUtils.ts (1)

1038-1038: Ensure getRootBoundaryCode function handles empty or undefined boundaries

The getRootBoundaryCode function might return an empty string if no root boundary is found. Ensure that this case is properly handled to avoid potential issues downstream.

Run the following script to verify if any calls to getRootBoundaryCode could result in an empty string and handle them appropriately:

health-services/project-factory/src/server/utils/onGoingCampaignUpdateUtils.ts (2)

36-38: LGTM!

The code correctly filters the resources by processedType and maps them to their createResourceId. Implementation is precise and functional.

---

79-79: Ensure all calls to modifyProcessedSheetData are updated

The function signature of modifyProcessedSheetData has been changed. Please verify that all invocations of this function elsewhere in the codebase are updated to match the new parameters (type: any, sheetData: any, schema: any, localizationMap?: any).

Run the following script to find and review all usages of modifyProcessedSheetData:

✅ Verification successful

Let me verify the function calls by checking the actual implementation of these calls to ensure they match the new signature.

---

Let me get more context around these function calls to verify the parameters being passed.

---

All calls to modifyProcessedSheetData are correctly updated

Both calls to modifyProcessedSheetData in the codebase are passing all the required parameters in the correct order:

1. typeWithoutWith as the type parameter
2. processedFacilitySheetData/processedUserSheetData as the sheetData parameter
3. schema as the schema parameter
4. localizationMap as the optional localizationMap parameter

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all calls to `modifyProcessedSheetData` in the codebase.

# Search for the function usage across the repository
rg -A 2 'modifyProcessedSheetData\('

Length of output: 1281

---

Script:

#!/bin/bash
# Get the implementation context of the function calls
ast-grep --pattern 'modifyProcessedSheetData($$$)' 

Length of output: 3009

---

Script:

#!/bin/bash
# Get more context around the function calls in genericUtils.ts
rg -B 2 -A 2 'modifyProcessedSheetData\(' health-services/project-factory/src/server/utils/genericUtils.ts

Length of output: 936

health-services/project-factory/src/server/utils/microplanUtils.ts (2)

Line range hint 96-138: Role validation logic updated correctly

The updates to the validateNationalDuplicacy function enhance the role validation logic and effectively incorporate localization. This improves the clarity of error messages and ensures consistent role handling.

🧰 Tools

🪛 Biome (1.9.4)

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

---

66-66: Verify that all calls to validateNationalDuplicacy are updated

Ensure that all function calls to validateNationalDuplicacy across the codebase have been updated to match the new signature accepting localizationMap instead of phoneNumberKey.

Run the following script to check for any outdated calls to validateNationalDuplicacy still using phoneNumberKey as the third argument:

✅ Verification successful

No outdated calls found - function signature change is consistent

The verification shows that validateNationalDuplicacy is only used once in the codebase, and that single call correctly uses localizationMap as the third argument, matching the function's definition. There are no outdated calls using phoneNumberKey.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for calls to 'validateNationalDuplicacy' with 'phoneNumberKey' as the third argument.

# Find all occurrences where 'validateNationalDuplicacy' is called with 'phoneNumberKey'
rg 'validateNationalDuplicacy\([^,]+,[^,]+,\s*phoneNumberKey\s*\)'

Length of output: 68

---

Script:

#!/bin/bash
# Let's first find all occurrences of validateNationalDuplicacy to see its usage pattern
rg "validateNationalDuplicacy" -A 2

# Also let's look at the function definition to understand the signature change
ast-grep --pattern 'function validateNationalDuplicacy($_) {
  $$$
}'

Length of output: 792

[coderabbitai]

🧹 Nitpick (assertive)

Remove redundant comment

The comment about CMD replacement is redundant as Git history already tracks these changes. Consider removing this line for cleaner code.

-# Replaced by CMD ["yarn", "prod"]

[coderabbitai]

🛠️ Refactor suggestion

Consider adding DEBUG environment variable

According to the summary, the prod script's behavior depends on the DEBUG environment variable, but it's not defined in the Dockerfile.

Add the DEBUG environment variable definition:

 EXPOSE 3000
+
+# Set DEBUG environment variable (default to false for production)
+ENV DEBUG=false
 
 CMD ["yarn", "prod"]

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

⚠️ Potential issue

Critical: Inconsistent error handling for non-successful responses

The new else block treats non-successful HTTP status codes (3xx, 4xx, 5xx) as valid responses, which:

• Breaks the standard error handling pattern where non-200 responses should trigger error handling
• May hide errors from upstream code that expects exceptions for non-200 responses
• Provides less detailed logging compared to the catch block

Consider this alternative implementation that maintains consistent error handling:

-      else{
-        logger.warn(`Error occurred while making request to ${getServiceName(_url)}: with error response ${JSON.stringify(response.data)}`);
-        return sendStatusCode ? { ...response.data, statusCode: responseStatus } : response.data;
-      }
+      else {
+        const error = new Error('Non-successful response status');
+        error.response = response;
+        throw error; // This will be caught by the catch block, ensuring consistent error handling
+      }

This change ensures that:

1. Non-successful responses are properly handled by the catch block
2. Detailed error logging is maintained
3. Upstream code receives consistent error behavior
4. The retry mechanism works for all error cases

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	else{
	logger.warn(`Error occurred while making request to ${getServiceName(_url)}: with error response ${JSON.stringify(response.data)}`);
	return sendStatusCode ? { ...response.data, statusCode: responseStatus } : response.data;
	}
	else {
	const error = new Error('Non-successful response status');
	error.response = response;
	throw error; // This will be caught by the catch block, ensuring consistent error handling
	}

[coderabbitai]

🧹 Nitpick (assertive)

Enable unused parameter checks for debugging

Enable noUnusedParameters to catch potential issues with unused function parameters during debugging.

-        // "noUnusedParameters": true,            /* Report errors on unused parameters. */
+        "noUnusedParameters": true,            /* Report errors on unused parameters. */

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// "noUnusedParameters": true, /* Report errors on unused parameters. */
	"noUnusedParameters": true, /* Report errors on unused parameters. */

🧰 Tools

🪛 Biome (1.9.4)

[error] 35-36: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

[coderabbitai]

💡 Codebase verification

Add custom type definitions path to typeRoots

The project contains custom type definition files in src/server/models/. Update typeRoots to include this path:

"typeRoots": [
    "./node_modules/@types",
    "./src/server/models"
],

🔗 Analysis chain

Verify type definitions location

The current configuration only includes type definitions from node_modules/@types. Verify if there are any custom type definitions in the project that should be included.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any custom type definition files in the project
fd -e d.ts -E "node_modules/*"

Length of output: 156

🧰 Tools

🪛 Biome (1.9.4)

[error] 44-44: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 44-44: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 44-46: End of file expected

Use an array for a sequence of values: [1, 2]

(parse)

---

[error] 46-46: JSON standard does not allow comments.

(parse)

[coderabbitai]

🧹 Nitpick (assertive)

Update comment to reflect dynamic wait time

The comment at line 91 states "wait for 3 second," but the actual wait time is obtained from the configuration and may differ. Update the comment to accurately represent the configurable wait time.

Apply this diff to correct the comment:

- // wait for 3 second
+ // Wait for the configured time after each localisation chunk

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// wait for 3 second
	const waitTime = config.localisation.localizationWaitTimeInBoundaryCreation
	logger.info(`Waiting for ${waitTime / 1000} seconds after each localisation chunk`);
	await new Promise((resolve) => setTimeout(resolve, waitTime));
	// Wait for the configured time after each localisation chunk
	const waitTime = config.localisation.localizationWaitTimeInBoundaryCreation
	logger.info(`Waiting for ${waitTime / 1000} seconds after each localisation chunk`);
	await new Promise((resolve) => setTimeout(resolve, waitTime));

[coderabbitai]

⚠️ Potential issue

Rethrow the original error to preserve stack trace

Using throw new Error(e); replaces the original error's stack trace, making debugging difficult. Instead, rethrow the original error to maintain the stack trace and error message.

Apply this diff to rethrow the original error:

-          throw new Error(e);
+          throw e;

If additional context is needed, wrap the error while preserving the original error:

-          throw new Error(e);
+          throw new Error(`Failed to create localisation: ${e.message}`);
+          // Optionally, include the original error object
+          // throw new Error(`Failed to create localisation: ${e.message}`, { cause: e });

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🛠️ Refactor suggestion

Use Number.parseInt instead of global parseInt

For better consistency and to adhere to modern JavaScript standards, use Number.parseInt instead of the global parseInt function.

Apply this diff to update the code:

-        localizationWaitTimeInBoundaryCreation: parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
-        localizationChunkSizeForBoundaryCreation: parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),
+        localizationWaitTimeInBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
+        localizationChunkSizeForBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	localizationWaitTimeInBoundaryCreation: parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
	localizationChunkSizeForBoundaryCreation: parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),
	localizationWaitTimeInBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
	localizationChunkSizeForBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),

🧰 Tools

🪛 Biome (1.9.4)

[error] 100-100: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

[coderabbitai]

🧹 Nitpick (assertive)

Use template literals and correct the grammatical error in the error message

To enhance readability and maintain consistency, prefer using template literals over string concatenation.

Also, replace 'An user' with 'A user' for grammatical correctness in the error message.

Apply this diff to implement the changes:

- const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+ const errorMessage: any = `A user with ${getLocalizedName(`ROOT_${trimmedRole}`, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can’t be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
	const errorMessage: any = `A user with ${getLocalizedName(`ROOT_${trimmedRole}`, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;

🧰 Tools

🪛 Biome (1.9.4)

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[coderabbitai]

⚠️ Potential issue

Improve type safety in boundary management processing.

The boundary code access assumes array structure without validation.

Add validation:

  if (type == "boundaryManagement") {
    logger.info("Processing data for boundaryManagement type")
    const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
    for (let d of data) {
+     if (!Array.isArray(d) || d.length === 0) {
+       logger.error("Invalid data structure for boundary management");
+       continue;
+     }
      const boundaryCode = d[d.length - 1];
      if (latLongBoundaryMap[boundaryCode]) {
        const [latitude = null, longitude = null] = latLongBoundaryMap[boundaryCode];
        d.push(latitude);
        d.push(longitude);
      }
    }
  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (type == "boundaryManagement") {
	logger.info("Processing data for boundaryManagement type")
	const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
	for (let d of data) {
	const boundaryCode = d[d.length - 1]; // Assume last element is the boundary code
	if (type == "boundaryManagement") {
	logger.info("Processing data for boundaryManagement type")
	const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
	for (let d of data) {
	if (!Array.isArray(d) || d.length === 0) {
	logger.error("Invalid data structure for boundary management");
	continue;
	}
	const boundaryCode = d[d.length - 1]; // Assume last element is the boundary code

🧰 Tools

🪛 Biome (1.9.4)

[error] 1313-1313: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

Actionable comments posted: 5

🔭 Outside diff range comments (1)

health-services/project-factory/src/server/app.ts (1)

Line range hint 77-106: Extract memory monitoring logic

Consider extracting the memory monitoring logic into a separate utility for better maintainability.

Create a new file utils/memoryMonitor.ts:

import * as v8 from 'node:v8';
import { logger } from './logger';

const MEMORY_CHECK_INTERVAL = 5 * 60 * 1000; // 5 minutes
const MEMORY_ALERT_THRESHOLD = 0.8; // 80%

export const startMemoryMonitoring = () => {
  setInterval(() => {
    const stats = v8.getHeapStatistics();
    const usedHeapSize = stats.used_heap_size;
    const heapLimit = stats.heap_size_limit;

    logger.debug(
      JSON.stringify({
        "Heap Usage": {
          used: printMemoryInMB(usedHeapSize),
          limit: printMemoryInMB(heapLimit),
          percentage: ((usedHeapSize / heapLimit) * 100).toFixed(2),
        },
      })
    );

    if (usedHeapSize / heapLimit > MEMORY_ALERT_THRESHOLD) {
      logger.warn("High memory usage detected");
    }
  }, MEMORY_CHECK_INTERVAL);
};

Then update the listen method to use it:

-      setInterval(() => {
-        const stats = v8.getHeapStatistics();
-        // ... monitoring code ...
-      }, 5 * 60 * 1000);
+      startMemoryMonitoring();

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b373ac4 and b69ba14.

📒 Files selected for processing (3)

• health-services/project-factory/src/server/app.ts (1 hunks)
• health-services/project-factory/src/server/utils/gzipHandler.ts (1 hunks)
• health-services/project-factory/src/server/utils/middlewares/requestMiddleware.ts (3 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/gzipHandler.ts

[error] 2-2: A Node.js builtin module should be imported with the node: protocol.

Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.

(lint/style/useNodejsImportProtocol)

🔇 Additional comments (1)

health-services/project-factory/src/server/app.ts (1)

53-53: Remove commented code

Good catch on removing the duplicate bodyParser.json() middleware. Let's clean up by removing the commented line entirely.

-    // this.app.use(bodyParser.json());

[coderabbitai]

🛠️ Refactor suggestion

Update Node.js import to use the node: protocol

For better clarity and explicit Node.js module identification, update the zlib import.

-import * as zlib from "zlib";
+import * as zlib from "node:zlib";

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	import { Request } from "express";
	import * as zlib from "zlib";
	import { Request } from "express";
	import * as zlib from "node:zlib";

🧰 Tools

🪛 Biome (1.9.4)

[error] 2-2: A Node.js builtin module should be imported with the node: protocol.

Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.

(lint/style/useNodejsImportProtocol)

[coderabbitai]

🛠️ Refactor suggestion

Enhance error handling and type safety in decompressGzip

The current implementation could be improved with more specific error handling and type safety.

-const decompressGzip = (gzipBuffer: Buffer): Promise<any> => {
+const decompressGzip = (gzipBuffer: Buffer): Promise<unknown> => {
     return new Promise((resolve, reject) => {
         zlib.gunzip(gzipBuffer, (err, result) => {
-            if (err) return reject(err);
+            if (err) {
+                return reject(new Error(`Gzip decompression failed: ${err.message}`));
+            }
             try {
-                resolve(JSON.parse(result.toString()));
+                const parsed = JSON.parse(result.toString('utf-8'));
+                if (typeof parsed !== 'object' || parsed === null) {
+                    throw new Error('Decompressed content must be a JSON object');
+                }
+                resolve(parsed);
             } catch (parseErr) {
-                reject(new Error("Invalid JSON format in decompressed data"));
+                reject(new Error(`Invalid JSON format: ${parseErr.message}`));
             }
         });
     });
 };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Helper function to decompress Gzip data
	const decompressGzip = (gzipBuffer: Buffer): Promise<any> => {
	return new Promise((resolve, reject) => {
	zlib.gunzip(gzipBuffer, (err, result) => {
	if (err) return reject(err);
	try {
	resolve(JSON.parse(result.toString()));
	} catch (parseErr) {
	reject(new Error("Invalid JSON format in decompressed data"));
	}
	});
	});
	};
	// Helper function to decompress Gzip data
	const decompressGzip = (gzipBuffer: Buffer): Promise<unknown> => {
	return new Promise((resolve, reject) => {
	zlib.gunzip(gzipBuffer, (err, result) => {
	if (err) {
	return reject(new Error(`Gzip decompression failed: ${err.message}`));
	}
	try {
	const parsed = JSON.parse(result.toString('utf-8'));
	if (typeof parsed !== 'object' || parsed === null) {
	throw new Error('Decompressed content must be a JSON object');
	}
	resolve(parsed);
	} catch (parseErr) {
	reject(new Error(`Invalid JSON format: ${parseErr.message}`));
	}
	});
	});
	};

[coderabbitai]

⚠️ Potential issue

Add memory safeguards for buffer collection

The current implementation collects all chunks without size limits, which could lead to memory issues with large requests.

Add a size limit check:

 export const handleGzipRequest = async (req: Request): Promise<void> => {
     const buffers: Buffer[] = [];
+    let totalSize = 0;
+    const MAX_SIZE = 10 * 1024 * 1024; // 10MB limit
 
     // Collect data chunks from the request
     await new Promise<void>((resolve, reject) => {
-        req.on("data", (chunk: any) => buffers.push(chunk));
+        req.on("data", (chunk: Buffer) => {
+            totalSize += chunk.length;
+            if (totalSize > MAX_SIZE) {
+                reject(new Error("Request payload too large"));
+                return;
+            }
+            buffers.push(chunk);
+        });
         req.on("end", resolve);
         req.on("error", reject);
     });

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export const handleGzipRequest = async (req: Request): Promise<void> => {
	const buffers: Buffer[] = [];
	// Collect data chunks from the request
	await new Promise<void>((resolve, reject) => {
	req.on("data", (chunk: any) => buffers.push(chunk));
	req.on("end", resolve);
	req.on("error", reject);
	});
	// Concatenate and decompress the data
	const gzipBuffer = Buffer.concat(buffers);
	try {
	const decompressedData = await decompressGzip(gzipBuffer);
	req.body = decompressedData; // Assign the parsed data to req.body
	} catch (err: any) {
	throw new Error(`Failed to process Gzip data: ${err.message}`);
	}
	};
	export const handleGzipRequest = async (req: Request): Promise<void> => {
	const buffers: Buffer[] = [];
	let totalSize = 0;
	const MAX_SIZE = 10 * 1024 * 1024; // 10MB limit
	// Collect data chunks from the request
	await new Promise<void>((resolve, reject) => {
	req.on("data", (chunk: Buffer) => {
	totalSize += chunk.length;
	if (totalSize > MAX_SIZE) {
	reject(new Error("Request payload too large"));
	return;
	}
	buffers.push(chunk);
	});
	req.on("end", resolve);
	req.on("error", reject);
	});
	// Concatenate and decompress the data
	const gzipBuffer = Buffer.concat(buffers);
	try {
	const decompressedData = await decompressGzip(gzipBuffer);
	req.body = decompressedData; // Assign the parsed data to req.body
	} catch (err: any) {
	throw new Error(`Failed to process Gzip data: ${err.message}`);
	}
	};

[coderabbitai]

🛠️ Refactor suggestion

Improve Gzip request handling

The current implementation assumes content-type exactly matches 'application/gzip'. This should be consistent with the normalized check above.

-    if (contentType === 'application/gzip') {
+    if (normalizedContentType === 'application/gzip') {
       await handleGzipRequest(req);
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (contentType === 'application/gzip') {
	await handleGzipRequest(req);
	}
	if (normalizedContentType === 'application/gzip') {
	await handleGzipRequest(req);
	}

[coderabbitai]

🛠️ Refactor suggestion

Simplify content-type checking logic

The current content-type checking is complex and hard to read. Consider simplifying it.

-    if (!contentType || !contentType.split(';').map(part => part.trim()).includes('application/json') && !contentType.split(';').map(part => part.trim()).includes('application/gzip')) {
-      // If content type is not 'application/json' or 'application/gzip', throw Unsupported Media Type error
-      let e: any = new Error("Unsupported Media Type: Content-Type should be 'application/json' or 'application/gzip'");
-      e = Object.assign(e, { status: 415, code: "UNSUPPORTED_MEDIA_TYPE" });
-      errorResponder(e, req, res, 415)
+    const normalizedContentType = contentType?.split(';')[0].trim();
+    const allowedTypes = ['application/json', 'application/gzip'];
+    
+    if (!normalizedContentType || !allowedTypes.includes(normalizedContentType)) {
+      const error = Object.assign(
+        new Error(`Unsupported Media Type: Content-Type should be one of: ${allowedTypes.join(', ')}`),
+        { status: 415, code: "UNSUPPORTED_MEDIA_TYPE" }
+      );
+      errorResponder(error, req, res, 415);
       return;
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (!contentType || !contentType.split(';').map(part => part.trim()).includes('application/json') && !contentType.split(';').map(part => part.trim()).includes('application/gzip')) {
	// If content type is not 'application/json' or 'application/gzip', throw Unsupported Media Type error
	let e: any = new Error("Unsupported Media Type: Content-Type should be 'application/json' or 'application/gzip'");
	e = Object.assign(e, { status: 415, code: "UNSUPPORTED_MEDIA_TYPE" });
	errorResponder(e, req, res, 415)
	return;
	const normalizedContentType = contentType?.split(';')[0].trim();
	const allowedTypes = ['application/json', 'application/gzip'];
	if (!normalizedContentType || !allowedTypes.includes(normalizedContentType)) {
	const error = Object.assign(
	new Error(`Unsupported Media Type: Content-Type should be one of: ${allowedTypes.join(', ')}`),
	{ status: 415, code: "UNSUPPORTED_MEDIA_TYPE" }
	);
	errorResponder(error, req, res, 415);
	return;
	}

[coderabbitai]

Actionable comments posted: 5

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b69ba14 and ad9a5b8.

📒 Files selected for processing (5)

• health-services/project-factory/src/server/api/campaignApis.ts (0 hunks)
• health-services/project-factory/src/server/api/genericApis.ts (3 hunks)
• health-services/project-factory/src/server/utils/campaignUtils.ts (13 hunks)
• health-services/project-factory/src/server/validators/campaignValidators.ts (5 hunks)
• health-services/project-factory/src/server/validators/microplanValidators.ts (4 hunks)

💤 Files with no reviewable changes (1)

• health-services/project-factory/src/server/api/campaignApis.ts

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/validators/campaignValidators.ts

[error] 97-97: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

health-services/project-factory/src/server/utils/campaignUtils.ts

[error] 1716-1716: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1708-1708: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2366-2366: This let declares a variable that is only assigned once.

'targetNo' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2366-2366: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 2367-2367: This let declares a variable that is only assigned once.

'beneficiaryType' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2411-2411: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

---

[error] 3035-3035: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

🔇 Additional comments (22)

health-services/project-factory/src/server/validators/microplanValidators.ts (4)

1-5: Imports look good
No issues found; the imports for boundary utilities, campaign utilities, and config references are clear and consistent.

---

6-7: Imports of microplan utilities and error handling
The imported utilities appear properly structured and ready for microplan checks.

---

131-131: Refined validation function
The function validateRequiredTargetsForMicroplanCampaigns clearly enforces the numeric constraints for target columns. Looks good.

---

273-273: Clarified empty-or-zero capacity message
Updating the error message to indicate that zero is not allowed ensures better user feedback.

health-services/project-factory/src/server/api/genericApis.ts (3)

7-7: Added import for processDataForTargetCalculation
Importing the new function from campaignUtils clearly indicates modular design.

---

211-211: Usage of extended getJsonData
Passing extra parameters (true,true,sheetName) nicely ensures row numbers and sheet names are embedded for downstream processing.

---

227-227: Delegating target processing to a dedicated function
Excellent approach to keep target calculation logic encapsulated in processDataForTargetCalculation.

health-services/project-factory/src/server/validators/campaignValidators.ts (10)

25-25: New import to handle extra microplan validations
Inclusion of validateExtraBoundariesForMicroplan and other microplan helpers ensures campaign flow remains consistent.

---

144-144: Initialization of columnsToValidate
Declaring this variable with let is valid since it’s reassigned further below.

---

152-154: Loading schema from MDMS
Retrieving columnsNotToBeFreezed and required ensures that the logic for target validation can adapt dynamically.

---

156-156: Populating columnsToValidate
Filtering the non-frozen columns for mandatory targets is a clean approach.

---

158-159: Localized target column names
Using getLocalizedHeaders is consistent with the rest of the codebase.

---

161-161: Special microplan validations
Appropriately calling validateRequiredTargetsForMicroplanCampaigns.

---

165-167: Dual validation approach

1. Standard required columns.
2. Optional columns that contain data.
   This ensures flexible targets in generic campaigns.

---

1332-1332: Sequential tab validations
Initiating district tab checks here is well-placed for microplan flows.

---

1387-1388: Filtering missing tabs
The logic for identifying missing district tabs is straightforward and accurate.

---

1395-1395: Clear error for missing district tabs
Throwing a concise error message improves debugging for large uploads.

health-services/project-factory/src/server/utils/campaignUtils.ts (5)

28-28: Importing searchMDMS
No concerns; it ensures consolidated usage of external queries.

---

962-962: Boundary code retrieval
Assigning boundaryCode via getRootBoundaryCode is consistent with the creation flow.

---

1039-1039: Aligning boundary code assignment in create
Follows the same practice of referencing boundariesCombined for root boundary detection.

---

1115-1115: Conditional boundary code resolution
Ensuring campaign boundary code is determined if not explicitly provided.

---

2411-2411: 🧹 Nitpick (assertive)

Avoid using delete

Deleting properties at runtime can degrade performance. Instead, set it to undefined:

- delete request.body?.boundariesCombined;
+ request.body.boundariesCombined = undefined;

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 2411-2411: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

[coderabbitai]

🧹 Nitpick (assertive)

Use Object.hasOwn() and consider a minor refactor

1. You’re calling Object.prototype.hasOwnProperty.call(dataFromSheet, key) at line 311. Consider using:

- if (Object.prototype.hasOwnProperty.call(dataFromSheet, key)) {
+ if (Object.hasOwn(dataFromSheet, key)) {
    ...
}

This is a recommended modern approach where possible.

2. The rest of the boundary validation logic is solid.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export async function validateExtraBoundariesForMicroplan(request: any, dataFromSheet: any, localizationMap: any) {
	if (await isMicroplanRequest(request)) {
	const campaignBoundariesSet = new Set(request?.body?.campaignBoundaries?.map((boundary: any) => boundary.code));
	for (const key in dataFromSheet) {
	if (key !== getLocalizedName(getBoundaryTabName(), localizationMap) && key !== getLocalizedName(config?.values?.readMeTab, localizationMap)) {
	if (Object.prototype.hasOwnProperty.call(dataFromSheet, key)) {
	const dataArray = (dataFromSheet as { [key: string]: any[] })[key];
	for (const boundaryRow of dataArray) {
	const boundaryCode = boundaryRow[getLocalizedName(getBoundaryColumnName(), localizationMap)];
	if (!campaignBoundariesSet.has(boundaryCode)) {
	throwError("COMMON", 400, "VALIDATION_ERROR", `Some boundaries in uploaded sheet are not present in campaign boundaries. Please upload from downloaded template only.`);
	}
	}
	}
	}
	}
	}
	}
	export async function validateExtraBoundariesForMicroplan(request: any, dataFromSheet: any, localizationMap: any) {
	if (await isMicroplanRequest(request)) {
	const campaignBoundariesSet = new Set(request?.body?.campaignBoundaries?.map((boundary: any) => boundary.code));
	for (const key in dataFromSheet) {
	if (key !== getLocalizedName(getBoundaryTabName(), localizationMap) && key !== getLocalizedName(config?.values?.readMeTab, localizationMap)) {
	if (Object.hasOwn(dataFromSheet, key)) {
	const dataArray = (dataFromSheet as { [key: string]: any[] })[key];
	for (const boundaryRow of dataArray) {
	const boundaryCode = boundaryRow[getLocalizedName(getBoundaryColumnName(), localizationMap)];
	if (!campaignBoundariesSet.has(boundaryCode)) {
	throwError("COMMON", 400, "VALIDATION_ERROR", `Some boundaries in uploaded sheet are not present in campaign boundaries. Please upload from downloaded template only.`);
	}
	}
	}
	}
	}
	}
	}

[coderabbitai]

🧹 Nitpick (assertive)

Switch from let to const

Since rowData is never reassigned:

- let rowData: any = { [codeColumnName]: row[codeColumnName] };
+ const rowData: any = { [codeColumnName]: row[codeColumnName] };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export async function processDataForTargetCalculation(request: any, jsonData: any, codeColumnName: string, localizationMap?: any) {
	// Retrieve targetConfigs from MDMS
	const targetConfigs = await searchMDMS([request?.body?.CampaignDetails?.projectType], "HCM-ADMIN-CONSOLE.targetConfigs", request?.body?.RequestInfo);
	// Process each row of the sheet data
	const resultantData = jsonData.map((row: any) => {
	// Initialize an object to hold row-specific data
	let rowData: any = { [codeColumnName]: row[codeColumnName] };
	export async function processDataForTargetCalculation(request: any, jsonData: any, codeColumnName: string, localizationMap?: any) {
	// Retrieve targetConfigs from MDMS
	const targetConfigs = await searchMDMS([request?.body?.CampaignDetails?.projectType], "HCM-ADMIN-CONSOLE.targetConfigs", request?.body?.RequestInfo);
	// Process each row of the sheet data
	const resultantData = jsonData.map((row: any) => {
	// Initialize an object to hold row-specific data
	const rowData: any = { [codeColumnName]: row[codeColumnName] };

🧰 Tools

🪛 Biome (1.9.4)

[error] 3035-3035: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

🧹 Nitpick (assertive)

Use const and Number.parseInt for clarity

Examples of safe fixes:

- let targets = [];
+ const targets = [];

- let targetNo = parseInt(codesTargetMapping?.[boundaryCode][key]);
+ const targetNo = Number.parseInt(codesTargetMapping?.[boundaryCode][key]);

- let beneficiaryType = key;
+ const beneficiaryType = key;

This helps maintain code consistency and clarity.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const enrichTargetForProject = (project: any, codesTargetMapping: any, boundaryCode: any) => {
	if (codesTargetMapping?.[boundaryCode] && Object.keys(codesTargetMapping[boundaryCode]).length > 0) {
	let targets = [];
	for (const key in codesTargetMapping?.[boundaryCode]) {
	let targetNo = parseInt(codesTargetMapping?.[boundaryCode][key]);
	let beneficiaryType = key;
	targets.push({
	beneficiaryType: beneficiaryType,
	targetNo: targetNo,
	});
	}
	if(targets.length > 0){
	project.targets = targets;
	}
	}
	else{
	logger.info(`No targets found for boundary code ${boundaryCode}`);
	}
	}
	const enrichTargetForProject = (project: any, codesTargetMapping: any, boundaryCode: any) => {
	if (codesTargetMapping?.[boundaryCode] && Object.keys(codesTargetMapping[boundaryCode]).length > 0) {
	const targets = [];
	for (const key in codesTargetMapping?.[boundaryCode]) {
	const targetNo = Number.parseInt(codesTargetMapping?.[boundaryCode][key]);
	const beneficiaryType = key;
	targets.push({
	beneficiaryType: beneficiaryType,
	targetNo: targetNo,
	});
	}
	if(targets.length > 0){
	project.targets = targets;
	}
	}
	else{
	logger.info(`No targets found for boundary code ${boundaryCode}`);
	}
	}

🧰 Tools

🪛 Biome (1.9.4)

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2366-2366: This let declares a variable that is only assigned once.

'targetNo' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2366-2366: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 2367-2367: This let declares a variable that is only assigned once.

'beneficiaryType' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

🧹 Nitpick (assertive)

Use Object.hasOwn() & const where appropriate

1. At line 1716:

- if (childTargetValue.hasOwnProperty(key)) {
+ if (Object.hasOwn(childTargetValue, key)) {

2. At line 1708, totalTargetValue is never reassigned, so use const:

- let totalTargetValue: any = {};
+ const totalTargetValue: any = {};

This keeps code modern and promotes immutability where possible.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Helper function to map individual boundaries
	const mapBoundary = (boundary: any) => {
	if (!boundary.children || boundary.children.length === 0) {
	// If no children, simply return the target value object or default to empty object
	const targetValue = codesTargetMapping[boundary.code];
	return targetValue || {};
	}
	let totalTargetValue = 0;
	for (const child of boundary.children) {
	const childTargetValue = mapBoundary(child);
	totalTargetValue += childTargetValue;
	// Initialize a new object to accumulate total target values from children
	let totalTargetValue: any = {};
	// Iterate through each child and accumulate their target values
	for (const child of boundary.children) {
	const childTargetValue = mapBoundary(child);
	// Accumulate the child target values into the total target value
	for (const key in childTargetValue) {
	if (childTargetValue.hasOwnProperty(key)) {
	// Initialize key in totalTargetValue if it doesn't exist
	totalTargetValue[key] = (totalTargetValue[key] || 0) + childTargetValue[key];
	}
	}
	codesTargetMapping[boundary.code] = totalTargetValue;
	return totalTargetValue;
	};
	}
	// Store the accumulated total target value for the current boundary
	codesTargetMapping[boundary.code] = totalTargetValue;
	return totalTargetValue;
	};
	// Map each boundary response
	for (const boundaryResponse of boundaryResponses) {
	// Helper function to map individual boundaries
	const mapBoundary = (boundary: any) => {
	if (!boundary.children || boundary.children.length === 0) {
	// If no children, simply return the target value object or default to empty object
	const targetValue = codesTargetMapping[boundary.code];
	return targetValue || {};
	}
	// Initialize a new object to accumulate total target values from children
	const totalTargetValue: any = {};
	// Iterate through each child and accumulate their target values
	for (const child of boundary.children) {
	const childTargetValue = mapBoundary(child);
	// Accumulate the child target values into the total target value
	for (const key in childTargetValue) {
	if (Object.hasOwn(childTargetValue, key)) {
	// Initialize key in totalTargetValue if it doesn't exist
	totalTargetValue[key] = (totalTargetValue[key] || 0) + childTargetValue[key];
	}
	}
	}
	// Store the accumulated total target value for the current boundary
	codesTargetMapping[boundary.code] = totalTargetValue;
	return totalTargetValue;
	};
	// Map each boundary response
	for (const boundaryResponse of boundaryResponses) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 1716-1716: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1708-1708: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

Actionable comments posted: 11

🔭 Outside diff range comments (2)

health-services/project-factory/src/server/utils/microplanUtils.ts (2)

Line range hint 104-124: Refactor duplicate validation logic.

The function contains duplicate code blocks for both ROOT_ and non-ROOT_ cases. This can be simplified to improve maintainability.

-      const userRole = user?.role;
-      if (userRole?.startsWith("ROOT_")) {
-        // Trim the role
-        const trimmedRole = userRole.replace("ROOT_", "");
-
-        // Check for duplicates in the roleMap
-        if (roleMap[trimmedRole] && roleMap[trimmedRole]["!sheet#name!"] != user["!sheet#name!"]) {
-          const errorMessage: any = `An user with ${getLocalizedName(trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
-          duplicates.push({ rowNumber: user["!row#number!"], sheetName: user["!sheet#name!"], status: "INVALID", errorDetails: errorMessage });
-        } else {
-          roleMap[trimmedRole] = user;
-        }
-      }
-      else {
-        const trimmedRole = userRole;
-        const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
-        if (roleMap[trimmedRole] && roleMap[trimmedRole]["!sheet#name!"] != user["!sheet#name!"]) {
-          duplicates.push({ rowNumber: user["!row#number!"], sheetName: user["!sheet#name!"], status: "INVALID", errorDetails: errorMessage });
-        } else {
-          roleMap[trimmedRole] = user;
-        }
-      }
+      const userRole = user?.role ?? '';
+      const trimmedRole = userRole.startsWith("ROOT_") ? userRole.replace("ROOT_", "") : userRole;
+      const roleToCheck = userRole.startsWith("ROOT_") ? trimmedRole : `ROOT_${trimmedRole}`;
+      
+      // Check for duplicates in the roleMap
+      if (roleMap[trimmedRole] && roleMap[trimmedRole]["!sheet#name!"] !== user["!sheet#name!"]) {
+        const errorMessage = `A user with ${getLocalizedName(roleToCheck, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+        duplicates.push({
+          rowNumber: user["!row#number!"],
+          sheetName: user["!sheet#name!"],
+          status: "INVALID",
+          errorDetails: errorMessage
+        });
+      } else {
+        roleMap[trimmedRole] = user;
+      }

🧰 Tools

🪛 Biome (1.9.4)

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

---

Line range hint 383-394: Add error handling and type safety for HTTP response.

The function lacks error handling for the HTTP request and type annotations for the response.

+interface PlanConfigurationResponse {
+  PlanConfiguration?: Array<{
+    name: string;
+    [key: string]: any;
+  }>;
+}
+
-export async function planConfigSearch(request: any) {
+export async function planConfigSearch(request: any): Promise<PlanConfigurationResponse> {
   const { tenantId, planConfigurationId } = request.body.MicroplanDetails;
   const searchBody = {
     RequestInfo: request.body.RequestInfo,
     PlanConfigurationSearchCriteria: {
       tenantId: tenantId,
       id: planConfigurationId
     }
   }
 
-  const searchResponse = await httpRequest(config.host.planServiceHost + config.paths.planConfigSearch, searchBody);
+  try {
+    const searchResponse = await httpRequest(
+      `${config.host.planServiceHost}${config.paths.planConfigSearch}`,
+      searchBody
+    );
+    return searchResponse;
+  } catch (error) {
+    throw new Error(`Failed to search plan configuration: ${error.message}`);
+  }
-  return searchResponse;
 }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad9a5b8 and f13f4cf.

📒 Files selected for processing (7)

• health-services/project-factory/src/server/api/campaignApis.ts (9 hunks)
• health-services/project-factory/src/server/api/genericApis.ts (3 hunks)
• health-services/project-factory/src/server/config/createAndSearch.ts (1 hunks)
• health-services/project-factory/src/server/config/index.ts (3 hunks)
• health-services/project-factory/src/server/config/models/createRequestSchema.ts (1 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (14 hunks)
• health-services/project-factory/src/server/utils/microplanUtils.ts (4 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 888-888: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1290-1290: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/config/index.ts

[error] 100-100: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

health-services/project-factory/src/server/api/campaignApis.ts

[error] 1341-1341: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

---

[error] 1261-1261: This let declares a variable that is only assigned once.

'creationTime' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 1262-1262: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'const' instead.

(lint/style/noVar)

health-services/project-factory/src/server/utils/microplanUtils.ts

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

🔇 Additional comments (22)

health-services/project-factory/src/server/config/models/createRequestSchema.ts (1)

7-7: Confirm removal of "facilityMicroplan" references across the codebase.

The removal of "facilityMicroplan" from the type enumeration appears valid. Ensure all references to "facilityMicroplan" have been properly handled or removed across the application to prevent potential mismatches or errors.

Run the following script to search for any remaining references to "facilityMicroplan":

✅ Verification successful

No references to "facilityMicroplan" found in the codebase

The verification process shows that there are no remaining references to "facilityMicroplan" in the codebase. The removal of this value from the enum is safe as it's not being used anywhere else in the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for references to "facilityMicroplan" in code files
rg "facilityMicroplan"

Length of output: 22

health-services/project-factory/src/server/config/createAndSearch.ts (1)

100-102: Ensure consistency with replaced bulk property.

The newly introduced createDetails property replaces the old bulk creation property. Verify that all references to createBulkDetails are removed or migrated accordingly throughout the codebase.

health-services/project-factory/src/server/config/index.ts (4)

42-43: Facility code columns look good.

Defining facilityCodeColumn and facilityType improves clarity for referencing facility fields. Ensure all usage points rely on these new properties for consistency.

---

100-101: Use Number.parseInt instead of global parseInt.

This change has been requested before. Please switch to Number.parseInt to align with modern JavaScript standards.

🧰 Tools

🪛 Biome (1.9.4)

[error] 100-100: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

149-149: Facility creation path fallback verified.

Using a fallback for facilityCreate is appropriate. No further issues noted here.

---

155-155: Cache burst path added.

Defining the cacheBurst ensures requests can invalidate localizations. The usage appears valid.

health-services/project-factory/src/server/api/genericApis.ts (2)

6-7: Modular import strategy.

Introducing processDataForTargetCalculation in the imports fosters better code organization by separating data processing logic.

---

211-211: Appending row metadata to JSON.

Calling getJsonData(sheetData, true, true, sheetName) appends row and sheet metadata, helpful for error tracking. No issues found.

health-services/project-factory/src/server/api/campaignApis.ts (9)

49-49: Improved modular approach for microplan checks.

Importing isMicroplanRequest from microplanUtils clarifies when specialized microplan logic applies, increasing maintainability.

---

304-304: Conditional branch for user resource type.

Filtering logic for user type is consistent and parallels existing user-handling code. Ensure null safety checks if type is missing.

---

1130-1131: Setting address to an empty object.

Explicitly initializing newRequestBody.Facility.address = {} can prevent undefined property issues when the facility creation endpoint expects an address. Validate if the address needs mandatory fields.

---

1132-1132: Facility creation URL usage.

Relying on createAndSearchConfig?.createDetails?.url centralizes facility creation endpoints. No immediate concerns.

---

1234-1239: Initiating bulk creation.

Splitting logic into createBulkData is clear. Ensure you pass all required parameters consistently; a missing argument on line 1236 could be unintentional.

---

1240-1246: Trigger single-entity creation workflow.

Falling back to createSingleData if createDetails is present is logical. Confirm no redundancy if both createBulkDetails and createDetails are defined.

---

1488-1493: Switching to body modification approach.

Calling performAndSaveResourceActivityByChangingBody organizes code for custom transformations before creation. Looks good for specialized flows.

---

1494-1504: Skipping creation on invalid data.

The conditional check to skip calling creation if the resource is 'invalid' is a good safeguard. No issues found.

---

1930-1933: Campaign details object initialization.

Combining query and body fields for tenantId and campaignId ensures more robust retrieval of campaign details from requests.

health-services/project-factory/src/server/utils/genericUtils.ts (5)

581-581: Microplan readMe configuration loading.

Checking isSourceMicroplan to decide which readMe config to load helps keep microplan logic contained. Good approach.

---

584-584: Fallback to standard readMe config.

Falling back to getReadMeConfig if not microplan is logical. No issues here.

---

850-851: Determine resource type from query or body.

Combining query and body is a flexible approach. Validate that fallback logic works if both are provided or neither is provided.

---

863-863: MDMS schema retrieval.

Fetching the MDMS schema for typeWithoutWith is a neat approach. Ensure partial modifications (split('With')) do not cause confusion if future types differ.

---

1059-1059: Attaching error details.

Appending the error details to item.additionalDetails is helpful for debugging. Ensure sensitive data is not leaked in production logs.

[coderabbitai]

🧹 Nitpick (assertive)

Add TypeScript type annotations for function parameters.

The function parameters lack type annotations, which could lead to runtime errors.

-function validateNationalDuplicacy(request: any, userMapping: any, localizationMap: any) {
+interface UserMappingType {
+  [phoneNumber: string]: Array<{
+    role?: string;
+    "!sheet#name!"?: string;
+    "!row#number!"?: number;
+  }>;
+}
+
+function validateNationalDuplicacy(
+  request: { body: { ResourceDetails: { status: string }, sheetErrorDetails?: any[] } },
+  userMapping: UserMappingType,
+  localizationMap: { [key: string]: string }
+) {

[coderabbitai]

🧹 Nitpick (assertive)

Computing target data via utility function.

Delegating to processDataForTargetCalculation centralizes logic for target calculations. Ensure the function handles edge cases (e.g., empty data).

[coderabbitai]

🧹 Nitpick (assertive)

Bulk creation function.

The createBulkData function chunk-splits data creation, handles user creation calls, and delays to avoid concurrency issues. This is well-structured. Consider using const rather than var and let if variables are not reassigned.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1261-1261: This let declares a variable that is only assigned once.

'creationTime' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 1262-1262: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'const' instead.

(lint/style/noVar)

[coderabbitai]

🛠️ Refactor suggestion

Single creation function.

createSingleData handles one-record creation and microplan facility logic. Be mindful of synchronous waits (like 5-second sleeps) if concurrency is needed at scale.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1341-1341: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

[coderabbitai]

🧹 Nitpick (assertive)

Injecting createBulkDetails in request body.

performAndSaveResourceActivityByChangingBody modifies the request body prior to the main creation logic. Consider adding error handling or logging if transformations fail.

[coderabbitai]

🧹 Nitpick (assertive)

Dropdown initialization refetch logic.

Gracefully re-fetching dropdowns if none are found prevents unexpected null references. Consider adding error handling around this fallback.

[coderabbitai]

🧹 Nitpick (assertive)

Hidden columns logging.

While printing hidden columns can help debugging, consider logging structured data (like JSON) for better clarity.

[coderabbitai]

🧹 Nitpick (assertive)

Modifying processed data.

Calling modifyProcessedSheetData allows post-processing. Confirm that the transformations match your updated schema definitions.

[coderabbitai]

🧹 Nitpick (assertive)

Use const instead of let.

The variable declared here is never reassigned. Switching to const clarifies its immutable nature.

- let d of data
+ const d of data

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	for (let d of data) {
	for (const d of data) {

🧰 Tools

🪛 Biome (1.9.4)

[error] 1290-1290: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (1)

health-services/project-factory/src/server/utils/campaignUtils.ts (1)

Line range hint 2196-2206: Consider adding type safety for target data processing.

While the logic is correct, consider adding type definitions for the entry object to improve type safety and code maintainability.

- targetData[key].forEach((entry : any) => {
+ interface TargetEntry {
+   [key: string]: any;
+   "Target at the Selected Boundary level": any;
+   "Parent Target at the Selected Boundary level": Record<string, any>;
+ }
+ targetData[key].forEach((entry: TargetEntry) => {

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f13f4cf and 9df4256.

📒 Files selected for processing (1)

• health-services/project-factory/src/server/utils/campaignUtils.ts (13 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/campaignUtils.ts

[error] 1716-1716: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1708-1708: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2369-2369: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 2425-2425: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

---

[error] 3049-3049: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

🔇 Additional comments (5)

health-services/project-factory/src/server/utils/campaignUtils.ts (5)

962-962: LGTM: Root boundary code assignment is correct.

The code correctly retrieves the root boundary code from the boundariesCombined array.

---

1039-1039: LGTM: Consistent usage of root boundary code retrieval.

The code maintains consistency in how root boundary codes are retrieved and assigned.

---

1115-1115: LGTM: Proper conditional boundary code assignment.

The code correctly handles boundary code assignment based on projectId presence, falling back to root boundary code when needed.

---

2189-2190: LGTM: Proper parameter passing for localization.

The code correctly passes the codeColumnName and localizationMap parameters to support localization in target sheet data retrieval.

---

2425-2425: 🧹 Nitpick (assertive)

Avoid using the delete operator for better performance.

Setting properties to undefined is preferred over using the delete operator.

- delete request.body?.boundariesCombined;
+ request.body.boundariesCombined = undefined;

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 2425-2425: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

[coderabbitai]

🧹 Nitpick (assertive)

Improve code quality with modern JavaScript practices.

Several improvements can be made to align with best practices:

1. Use const for variables that aren't reassigned
2. Use Number.parseInt instead of global parseInt
3. Add proper TypeScript interfaces

+ interface Target {
+   beneficiaryType: string;
+   targetNo: number;
+ }

- const enrichTargetForProject = (project: any, codesTargetMapping: any, boundaryCode: any) => {
+ const enrichTargetForProject = (
+   project: { targets?: Target[] },
+   codesTargetMapping: Record<string, Record<string, string>>,
+   boundaryCode: string
+ ) => {
  if (codesTargetMapping?.[boundaryCode] && Object.keys(codesTargetMapping[boundaryCode]).length > 0) {
-   let targets: any[] = [];
+   const targets: Target[] = [];
    const alreadyPresentTargets = project?.targets || [];

    for (const beneficiaryType in codesTargetMapping[boundaryCode]) {
-     const targetNo = parseInt(codesTargetMapping[boundaryCode][beneficiaryType], 10);
+     const targetNo = Number.parseInt(codesTargetMapping[boundaryCode][beneficiaryType], 10);
      updateOrAddTarget(alreadyPresentTargets, beneficiaryType, targetNo, targets);
    }

    if (targets.length > 0) {
      project.targets = targets;
    }
  }
};

- const updateOrAddTarget = (alreadyPresentTargets: any, beneficiaryType: string, targetNo: number, targets: any[]) => {
+ const updateOrAddTarget = (
+   alreadyPresentTargets: Target[],
+   beneficiaryType: string,
+   targetNo: number,
+   targets: Target[]
+ ) => {

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 Biome (1.9.4)

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2369-2369: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

[coderabbitai]

Actionable comments posted: 8

♻️ Duplicate comments (1)

health-services/project-factory/src/server/validators/microplanValidators.ts (1)

297-297: 🧹 Nitpick (assertive)

Use Object.hasOwn() for property checks

Consider modernizing the property check:

- if (Object.prototype.hasOwnProperty.call(dataFromSheet, key)) {
+ if (Object.hasOwn(dataFromSheet, key)) {

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d832441 and fc6122e.

📒 Files selected for processing (5)

• health-services/project-factory/src/server/config/index.ts (4 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (15 hunks)
• health-services/project-factory/src/server/utils/microplanUtils.ts (4 hunks)
• health-services/project-factory/src/server/validators/campaignValidators.ts (6 hunks)
• health-services/project-factory/src/server/validators/microplanValidators.ts (4 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 887-887: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1289-1289: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/validators/campaignValidators.ts

[error] 97-97: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

health-services/project-factory/src/server/utils/microplanUtils.ts

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

health-services/project-factory/src/server/config/index.ts

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

🔇 Additional comments (23)

health-services/project-factory/src/server/validators/microplanValidators.ts (4)

6-7: LGTM! Clean import additions

The new imports are well-organized and properly aligned with the new functionality.

---

131-131: LGTM! Improved function naming

The rename to validateRequiredTargetsForMicroplanCampaigns better reflects the function's purpose of validating required targets.

---

272-279: LGTM! Improved facility capacity validation

The changes correctly allow for zero capacity facilities while maintaining proper validation bounds and consistent error messaging.

---

292-309: LGTM! Well-implemented boundary validation

The function effectively validates that uploaded boundaries exist within campaign boundaries using efficient Set lookups and proper error handling.

health-services/project-factory/src/server/validators/campaignValidators.ts (2)

97-97: ⚠️ Potential issue

Replace hasOwnProperty with Object.hasOwn()

Using hasOwnProperty directly on objects is discouraged as it can be unsafe. Use Object.hasOwn() instead.

-    boundaryData.some((row: any) => row.hasOwnProperty(column) && row[column] !== null && row[column] !== undefined)
+    boundaryData.some((row: any) => Object.hasOwn(row, column) && row[column] !== null && row[column] !== undefined)

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 97-97: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

1331-1331: Verify microplan boundary validation implementation

Please ensure that the validateExtraBoundariesForMicroplan function properly validates all boundary scenarios for microplan campaigns.

✅ Verification successful

Microplan boundary validation implementation is correct and comprehensive

The implementation in microplanValidators.ts properly validates the boundaries by:

• Checking if the request is a microplan request
• Creating a set of valid campaign boundary codes from the request
• Iterating through all data sheets (excluding boundary and readme tabs)
• Verifying that each boundary code in the uploaded sheets exists in the campaign boundaries
• Throwing a clear validation error if any boundary in the sheet is not present in campaign boundaries

The validation ensures data consistency between the campaign boundaries and the uploaded sheets, preventing mismatches that could cause issues during campaign execution.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check implementation of validateExtraBoundariesForMicroplan

# Search for the function implementation
ast-grep --pattern 'function validateExtraBoundariesForMicroplan($_, $_) {
  $$$
}'

# Search for test coverage
rg -A 5 'validateExtraBoundariesForMicroplan.*test'

Length of output: 139

---

Script:

#!/bin/bash
# Let's try different search patterns to locate the implementation

# Search for the function definition with async keyword
ast-grep --pattern 'async function validateExtraBoundariesForMicroplan($_, $_) {
  $$$
}'

# Search for any occurrence of validateExtraBoundariesForMicroplan
rg -A 10 "validateExtraBoundariesForMicroplan"

# Search for export declaration
ast-grep --pattern 'export async function validateExtraBoundariesForMicroplan($_, $_) {
  $$$
}'

# Search for const declaration
ast-grep --pattern 'const validateExtraBoundariesForMicroplan = async ($_, $_) => {
  $$$
}'

Length of output: 5542

health-services/project-factory/src/server/config/index.ts (3)

101-102: Use Number.parseInt instead of global parseInt.
This duplicates a past review comment, but still remains valid. It helps maintain consistency with current JavaScript standards, which recommend using Number.parseInt.

Apply this diff to update the code:

-    localizationWaitTimeInBoundaryCreation: parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
-    localizationChunkSizeForBoundaryCreation: parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),
+    localizationWaitTimeInBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_WAIT_TIME_IN_BOUNDARY_CREATION || "30000"),
+    localizationChunkSizeForBoundaryCreation: Number.parseInt(process.env.LOCALIZATION_CHUNK_SIZE_FOR_BOUNDARY_CREATION || "2000"),

🧰 Tools

🪛 Biome (1.9.4)

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

155-155: Check usage of the cacheBurst property.

Confirm that wherever cacheBurst is used, it aligns with expectations for cache invalidation or busting. If usage is absent or unclear, consider removing or documenting how it should be utilized.

---

150-150: Verify that all references to facility creation are updated.

You have renamed the path to facilityCreate. Ensure any references elsewhere in the codebase point to the new name and that no references are left to the old facilityBulkCreate.

Use the following shell script to locate references:

✅ Verification successful

Let me search for both the old and new terms, and also check for any facility-related endpoints to ensure we have comprehensive coverage.

---

No issues found with facility creation path references

The search results show that the facilityCreate path is correctly referenced in the codebase and there are no lingering references to facilityBulkCreate. The path is used consistently:

• In project-factory/src/server/config/index.ts: The path is defined as facilityCreate: process.env.EGOV_FACILITY_CREATE_PATH || "facility/v1/_create"
• In project-factory/src/server/config/createAndSearch.ts: The path is correctly referenced using config.paths.facilityCreate

The bulk create endpoint still exists as a separate API endpoint (/facility/v1/bulk/_create) as shown in the API specs and documentation, but it's not related to this configuration change.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
rg -A 3 "facilityBulkCreate"

Length of output: 28

---

Script:

#!/bin/bash
# Search for any facility-related endpoints and paths
rg -i "facility.*create.*path|facility.*/_create" -A 3

# Search for facilityCreate references
rg "facilityCreate" -A 3

Length of output: 9292

health-services/project-factory/src/server/utils/genericUtils.ts (7)

21-21: Ensure effective usage of newly imported functions.
Double-check that all imported members (getReadMeConfigForMicroplan, getRolesForMicroplan, getUserDataFromMicroplanSheet, and isMicroplanRequest) are actually used. If any are unused, consider removing them to keep the code clean.

---

849-850: Verify that 'type' is defined before calling 'includes'.
If request?.query?.type and request?.body?.ResourceDetails?.type are both undefined, type.includes(...) will throw. Safeguard the call in case type is falsy.

---

862-864: Looks good!
The facility sheet processing appears correct and clearly structured.

---

884-885: Validate 'type' existence.
Same concern as in line 849. Ensure type is non-null before invoking type.includes(...).

---

888-888: 🧹 Nitpick (assertive)

Simplify the boolean assignment.
Using a ternary with boolean literals is unnecessary. Instead of const isUpdate = fileUrl ? true : false;, use const isUpdate = !!fileUrl; for clarity.

- const isUpdate = fileUrl ? true : false;
+ const isUpdate = !!fileUrl;

Likely invalid or redundant comment.

---

1286-1291: 🧹 Nitpick (assertive)

Use const instead of let for loop variable.
Since d is not reassigned, using const clarifies its immutability and aligns with best practices.

-for (let d of data) {
+for (const d of data) {

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1289-1289: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

729-735: 🧹 Nitpick (assertive)

Correct the typo and ensure consistent equality checks.

1. The log message uses “refetched drodowns”; consider “refetched dropdowns”.
2. Consider using === instead of == in Object.keys(receivedDropdowns)?.length == 0.

- logger.info("refetched drodowns", JSON.stringify(receivedDropdowns))
+ logger.info("refetched dropdowns", JSON.stringify(receivedDropdowns))

Likely invalid or redundant comment.

health-services/project-factory/src/server/utils/microplanUtils.ts (7)

66-66: The updated function call aligns with the new signature.
This change correctly reflects the new parameter list. Please ensure no references to the old signature remain elsewhere in the code.

---

96-96: Add TypeScript type annotations for function parameters.
Consider adding type interfaces for request, userMapping, and localizationMap to improve clarity and prevent runtime errors.

---

104-107: Logic for handling ROOT_ roles looks good.
This ensures trimming only happens when the role starts with "ROOT_." No issues noted here.

---

111-111: Correct the grammatical article.
Change "An user" to "A user" in the error message to maintain professional, clear language.

---

118-119: Adopt template literals and correct grammatical error.
Please replace:

• "An user" with "A user"
• "ROOT_" + trimmedRole with `ROOT_${trimmedRole}`

🧰 Tools

🪛 Biome (1.9.4)

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

---

383-383: Async conversion is appropriate.
Making planConfigSearch asynchronous is a good move. This ensures proper handling of awaited I/O calls.

---

393-393: Awaiting the HTTP request is correct.
This line properly leverages async/await. No further changes needed.

[coderabbitai]

🧹 Nitpick (assertive)

Add type definitions for better type safety

Consider adding TypeScript interfaces/types for the validation parameters to improve type safety and code maintainability.

interface ValidationRequest {
  body: {
    ResourceDetails: {
      additionalDetails: {
        source: string;
      };
    };
  };
}

interface TargetColumn {
  name: string;
  required: boolean;
}

interface ValidationErrors {
  status: string;
  rowNumber: number;
  errorDetails: string;
  sheetName: string;
}

async function validateTargets(
  request: ValidationRequest, 
  data: any[], 
  errors: ValidationErrors[], 
  localizationMap?: Record<string, string>
) {
  // ... rest of the implementation
}

[coderabbitai]

🧹 Nitpick (assertive)

Enhance error message clarity

The error message could be more descriptive to help users understand and fix the issue.

-            throwError("COMMON", 400, "VALIDATION_ERROR", `District${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} not present in the Target Sheet Uploaded`);
+            throwError("COMMON", 400, "VALIDATION_ERROR", `Required district${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} ${MissingDistricts?.length > 1 ? 'are' : 'is'} missing from the Target Sheet. Please ensure all districts from the campaign boundaries are included.`);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	districtsLocalised?.forEach((tab: any) => {
	if (!tabsOfDistrict?.includes(tab)) {
	MissingDistricts.push(tab);
	}
	});
	}
	if (MissingDistricts.length > 0) {
	throwError("COMMON", 400, "VALIDATION_ERROR", `Districts ${MissingDistricts.join(', ')} not present in the Target Sheet Uploaded`);
	throwError("COMMON", 400, "VALIDATION_ERROR", `District${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} not present in the Target Sheet Uploaded`);
	districtsLocalised?.forEach((tab: any) => {
	if (!tabsOfDistrict?.includes(tab)) {
	MissingDistricts.push(tab);
	}
	});
	}
	if (MissingDistricts.length > 0) {
	throwError("COMMON", 400, "VALIDATION_ERROR", `Required district${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} ${MissingDistricts?.length > 1 ? 'are' : 'is'} missing from the Target Sheet. Please ensure all districts from the campaign boundaries are included.`);

[coderabbitai]

🧹 Nitpick (assertive)

Extract validation logic to reduce code duplication

The validation logic for different error cases can be extracted into a separate function to improve maintainability and reduce duplication.

+function validateTargetValue(target: any, targetColumn: string, rowNumber: number, key: string): any[] {
+    const errors = [];
+    if (typeof target !== 'number') {
+        errors.push({
+            status: "INVALID",
+            rowNumber: rowNumber,
+            errorDetails: `Data in column '${targetColumn}' is not a number.(It can only be positive numbers less than or equal to 1 Million)`,
+            sheetName: key
+        });
+    } else if (target <= 0 || target > 1000000) {
+        errors.push({
+            status: "INVALID",
+            rowNumber: rowNumber,
+            errorDetails: `Data in column '${targetColumn}' is out of range.(It can only be positive numbers less than or equal to 1 Million)`,
+            sheetName: key
+        });
+    } else if (!Number.isInteger(target)) {
+        errors.push({
+            status: "INVALID",
+            rowNumber: rowNumber,
+            errorDetails: `Data in column '${targetColumn}' is not an integer.(It can only be positive numbers less than or equal to 1 Million)`,
+            sheetName: key
+        });
+    }
+    return errors;
+}

 function validateTargetForNormalCampaigns(data: any, errors: any, localizedTargetColumnNames: any, isGenericType: boolean, localizationMap?: { [key: string]: string }) {
     // ... existing code ...
-    if (typeof target !== 'number') {
-        errors.push({
-            status: "INVALID",
-            rowNumber: obj["!row#number!"],
-            errorDetails: `Data in column '${targetColumn}' is not a number.(It can only be positive numbers less than or equal to 1 Million)`,
-            sheetName: key
-        });
-    } else if (target <= 0 || target > 1000000) {
-        errors.push({
-            status: "INVALID",
-            rowNumber: obj["!row#number!"],
-            errorDetails: `Data in column '${targetColumn}' is out of range.(It can only be positive numbers less than or equal to 1 Million)`,
-            sheetName: key
-        });
-    } else if (!Number.isInteger(target)) {
-        errors.push({
-            status: "INVALID",
-            rowNumber: obj["!row#number!"],
-            errorDetails: `Data in column '${targetColumn}' is not an integer.(It can only be positive numbers less than or equal to 1 Million)`,
-            sheetName: key
-        });
-    }
+    const validationErrors = validateTargetValue(target, targetColumn, obj["!row#number!"], key);
+    errors.push(...validationErrors);

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🧹 Nitpick (assertive)

Consider renaming for clarity.

Currently, the field is named boundaryCodeMandatoryForMicroplanFacility, but the environment variable is named BOUNDARY_CODE_HEADER_NAME_FACILITY_MICROPLAN. These names suggest slightly different meanings. Consider renaming this property or the environment variable to maintain clear, consistent naming.

[coderabbitai]

🧹 Nitpick (assertive)

Make the facility code & type configurable.

Both facilityCodeColumn and facilityType are hardcoded. If there’s a need to adapt or extend these values in the future, consider sourcing them from environment variables instead of hardcoding. This ensures flexibility and easier maintenance.

[coderabbitai]

🧹 Nitpick (assertive)

Unify equality checks for consistency.
You’re using both == and === in a single expression. Prefer === for consistency and avoidance of potential pitfalls with type coercion.

[coderabbitai]

🧹 Nitpick (assertive)

Be mindful of potential clutter in logs.
Logging large arrays or objects can clutter logs. Consider logging only relevant details or using debug-level logs when dealing with big data.

[coderabbitai]

Actionable comments posted: 3

🔭 Outside diff range comments (1)

health-services/project-factory/src/server/utils/microplanUtils.ts (1)

Line range hint 387-397: Add error handling and type annotations.

While the async/await usage is correct, the function could benefit from:

1. Error handling for the HTTP request
2. Type annotations for the response

Apply this diff to improve error handling and type safety:

- export async function planConfigSearch(request: any) {
+ interface PlanConfigSearchResponse {
+   PlanConfiguration?: Array<{
+     name: string;
+     [key: string]: any;
+   }>;
+ }
+ 
+ export async function planConfigSearch(request: any): Promise<PlanConfigSearchResponse> {
  const { tenantId, planConfigurationId } = request.body.MicroplanDetails;
  const searchBody = {
    RequestInfo: request.body.RequestInfo,
    PlanConfigurationSearchCriteria: {
      tenantId: tenantId,
      id: planConfigurationId
    }
  }

-  const searchResponse = await httpRequest(config.host.planServiceHost + config.paths.planConfigSearch, searchBody);
-  return searchResponse;
+  try {
+    const searchResponse = await httpRequest(config.host.planServiceHost + config.paths.planConfigSearch, searchBody);
+    return searchResponse;
+  } catch (error) {
+    throwError("COMMON", 500, "PLAN_CONFIG_SEARCH_FAILED", `Failed to search plan configuration: ${error.message}`);
+  }
}

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc6122e and bfccb0a.

📒 Files selected for processing (3)

• health-services/project-factory/src/server/config/index.ts (5 hunks)
• health-services/project-factory/src/server/utils/microplanUtils.ts (5 hunks)
• health-services/project-factory/src/server/validators/microplanValidators.ts (7 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/microplanUtils.ts

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

health-services/project-factory/src/server/config/index.ts

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

🔇 Additional comments (13)

health-services/project-factory/src/server/config/index.ts (6)

34-34: Consider renaming for clarity.

Currently, the field is named boundaryCodeMandatoryForMicroplanFacility, but the environment variable is named BOUNDARY_CODE_HEADER_NAME_FACILITY_MICROPLAN. These names suggest slightly different meanings. Consider renaming this property or the environment variable to maintain clear, consistent naming.

---

43-44: Make the facility code & type configurable.

Both facilityCodeColumn and facilityType are hardcoded. If there’s a need to adapt or extend these values in the future, consider sourcing them from environment variables instead of hardcoding. This ensures flexibility and easier maintenance.

---

101-102: Use Number.parseInt instead of global parseInt.

For better consistency and to adhere to modern JavaScript standards, use Number.parseInt instead of the global parseInt function.

🧰 Tools

🪛 Biome (1.9.4)

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

150-150: Verify references to the renamed path.

The property has been renamed to facilityCreate. Ensure all references, including documentation and function calls, are updated properly to avoid undefined paths.

---

155-155: Confirm usage of the new cache-burst property.

A new cacheBurst property is introduced. Verify that the code references this property where needed (e.g., after updates to localization messages) to effectively control and invalidate cached data.

---

185-186: 🧹 Nitpick (assertive)

Ensure proper usage or validation of the newly added environment variables.

autoRetryIfHttpError and latLongColumns have been introduced to handle retry logic and latitude-longitude column configurations, respectively. Validate any usage or expansions of these properties to confirm they function as intended and cover all necessary edge cases.

Would you like help creating robust validation and error-handling logic for these new environment variables?

health-services/project-factory/src/server/utils/microplanUtils.ts (2)

66-66: LGTM! Consistent with localization refactoring.

The change from phoneNumberKey to localizationMap aligns with the broader effort to support localization throughout the codebase.

---

111-111: Use template literals and correct the grammatical error in the error messages.

The error messages have two issues:

1. Inconsistent use of template literals
2. Incorrect grammar ("An user" should be "A user")

Also applies to: 119-119

health-services/project-factory/src/server/validators/microplanValidators.ts (5)

6-8: LGTM! New imports support enhanced validation capabilities.

The added imports for microplan request validation, error handling, and logging are well-organized and follow TypeScript best practices.

---

132-132: LGTM! Function rename improves clarity.

The rename to validateRequiredTargetsForMicroplanCampaigns better describes the function's purpose of validating required targets.

---

280-286: LGTM! Improved facility capacity validation.

Good improvements:

1. Using logging instead of error for empty capacity
2. Allowing zero as a valid capacity value
3. Clear error messages for invalid values

The validation logic is now more flexible while maintaining data integrity.

---

299-316: 🧹 Nitpick (assertive)

Good boundary validation implementation with room for improvement.

The validation logic is solid, but consider these enhancements:

1. Use Object.hasOwn() instead of Object.prototype.hasOwnProperty.call
2. Provide more specific error messages including the invalid boundary code
3. Consider batch processing for large datasets

- if (Object.prototype.hasOwnProperty.call(dataFromSheet, key)) {
+ if (Object.hasOwn(dataFromSheet, key)) {
     const dataArray = (dataFromSheet as { [key: string]: any[] })[key];
+    const invalidBoundaries = [];
     for (const boundaryRow of dataArray) {
         const boundaryCode = boundaryRow[getLocalizedName(getBoundaryColumnName(), localizationMap)];
         if (!campaignBoundariesSet.has(boundaryCode)) {
-            throwError("COMMON", 400, "VALIDATION_ERROR", `Some boundaries in uploaded sheet are not present in campaign boundaries. Please upload from downloaded template only.`);
+            invalidBoundaries.push(boundaryCode);
         }
     }
+    if (invalidBoundaries.length > 0) {
+        throwError(
+            "COMMON",
+            400,
+            "VALIDATION_ERROR",
+            `The following boundaries are not present in campaign boundaries: ${invalidBoundaries.join(', ')}. Please upload from downloaded template only.`
+        );
+    }
 }

Likely invalid or redundant comment.

---

Line range hint 1-316: Verify test coverage for validation functions.

Please ensure comprehensive test coverage for all validation functions, especially the new validateExtraBoundariesForMicroplan function and the updated facility capacity validation logic.

[coderabbitai]

⚠️ Potential issue

Fix inconsistent role handling logic.

The role handling logic is inconsistent between the if and else blocks:

• The if block removes the "ROOT_" prefix
• The else block adds the "ROOT_" prefix
  This could lead to incorrect role validation.

Apply this diff to fix the logic:

  if (userRole?.startsWith("ROOT_")) {
    const trimmedRole = userRole.replace("ROOT_", "");
    // ... existing code ...
  } else {
-   const trimmedRole = userRole;
-   const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+   const trimmedRole = userRole;
+   const errorMessage: any = `A user with ${getLocalizedName(trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
    // ... existing code ...
  }

Also applies to: 118-119

[coderabbitai]

🧹 Nitpick (assertive)

Consider adding type safety to facility capacity handling.

The facility capacity handling logic looks good, but could benefit from explicit type conversion for numeric values.

Apply this diff to add type safety:

  if(!rowData[facilityCapacityColumn]){
-   rowData[facilityCapacityColumn] = 0
-   element.storageCapacity = 0
+   rowData[facilityCapacityColumn] = 0
+   element.storageCapacity = 0
  }
- else if (rowData[facilityCapacityColumn] >= 0) {
-   element.storageCapacity = rowData[facilityCapacityColumn]
+ else if (Number(rowData[facilityCapacityColumn]) >= 0) {
+   element.storageCapacity = Number(rowData[facilityCapacityColumn])
  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if(!rowData[facilityCapacityColumn]){
	rowData[facilityCapacityColumn] = 0
	element.storageCapacity = 0
	}
	else if (rowData[facilityCapacityColumn] >= 0) {
	if(!rowData[facilityCapacityColumn]){
	rowData[facilityCapacityColumn] = 0
	element.storageCapacity = 0
	}
	else if (Number(rowData[facilityCapacityColumn]) >= 0) {
	element.storageCapacity = Number(rowData[facilityCapacityColumn])
	}

[coderabbitai]

🧹 Nitpick (assertive)

LGTM! Efficient lat/long validation with localization support.

Good use of Set for efficient column lookups. Consider memoizing the localized column names if this function is called frequently, as the mapping operation is performed on every call.

+ // Consider adding at module level:
+ let memoizedLatLongColumns: string[] | null = null;
+ 
  const latLongUnlocalisedColumns = config.values.latLongColumns?.split(',') || [];
- const latLongColumns = latLongUnlocalisedColumns?.map((column: string) => getLocalizedName(column, localizationMap));
+ const latLongColumns = memoizedLatLongColumns ?? 
+   (memoizedLatLongColumns = latLongUnlocalisedColumns?.map(
+     (column: string) => getLocalizedName(column, localizationMap)
+   ));

Also applies to: 202-202

[coderabbitai]

Actionable comments posted: 7

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfccb0a and 5420d8e.

📒 Files selected for processing (4)

• health-services/project-factory/src/server/api/genericApis.ts (4 hunks)
• health-services/project-factory/src/server/config/index.ts (5 hunks)
• health-services/project-factory/src/server/utils/excelUtils.ts (2 hunks)
• health-services/project-factory/src/server/validators/campaignValidators.ts (7 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/validators/campaignValidators.ts

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

health-services/project-factory/src/server/config/index.ts

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

🔇 Additional comments (14)

health-services/project-factory/src/server/validators/campaignValidators.ts (4)

25-31: LGTM! New imports are well organized.

The added imports for microplan validators and file metadata validation are appropriate and align with the new functionality.

---

146-169: LGTM! Enhanced validation logic for target columns.

The changes improve the validation by:

• Separating required and optional target columns
• Adding support for dynamic target templates
• Handling microplan-specific validations

---

1389-1397: 🧹 Nitpick (assertive)

Enhance error message for missing districts

The error message could be more descriptive to help users understand and fix the issue.

- throwError("COMMON", 400, "VALIDATION_ERROR", `District${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} not present in the Target Sheet Uploaded`);
+ throwError("COMMON", 400, "VALIDATION_ERROR", `Required district${MissingDistricts?.length > 1 ? 's' : ''} ${MissingDistricts.join(', ')} ${MissingDistricts?.length > 1 ? 'are' : 'is'} missing from the Target Sheet. Please ensure all districts from the campaign boundaries are included.`);

Likely invalid or redundant comment.

---

98-100: ⚠️ Potential issue

Replace hasOwnProperty with Object.hasOwn()

Using hasOwnProperty directly on objects is discouraged as it can be overridden or tampered with. Use Object.hasOwn() instead for better security.

- boundaryData.some((row: any) => row.hasOwnProperty(column) && row[column] !== null && row[column] !== undefined)
+ boundaryData.some((row: any) => Object.hasOwn(row, column) && row[column] !== null && row[column] !== undefined)

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

health-services/project-factory/src/server/utils/excelUtils.ts (2)

9-9: Good import for locale extraction.

Importing getLocaleFromRequestInfo helps maintain a standardized approach for localizing request data throughout the code.

---

80-123: Robust metadata validation logic.

The metadata validation thoroughly checks for locale & campaign ID. However, ensure the error messages are i18n-friendly if applicable, or at least consistent with the rest of the codebase.

health-services/project-factory/src/server/api/genericApis.ts (2)

227-227: Centralized target calculation logic.

Replacing inline logic with processDataForTargetCalculation is a good approach to keep the code DRY and consistent.

---

6-7: Unused named imports caution.

Multiple utilities are imported from genericUtils and campaignUtils. Ensure all are truly used to avoid clutter.

✅ Verification successful

All imported utilities are actively used in the code

Based on the search results, all imported utilities from both genericUtils and campaignUtils are being used within the genericApis.ts file:

From genericUtils:

• correctParentValues: Used for campaign details
• findMapValue: Used multiple times for element and parent lookups
• getBoundaryRelationshipData: Used to fetch boundary data
• getDataSheetReady: Used twice for data preparation
• getLocalizedHeaders: Used multiple times for sheet names and headers
• sortCampaignDetails: Used for campaign details sorting
• throwError: Used extensively for error handling

From campaignUtils:

• extractCodesFromBoundaryRelationshipResponse: Used for code extraction
• generateFilteredBoundaryData: Used for filtering boundary data
• getConfigurableColumnHeadersBasedOnCampaignType: Used for header configuration
• getFiltersFromCampaignSearchResponse: Used for filter extraction
• getLocalizedName: Used multiple times for localization
• processDataForTargetCalculation: Used for data processing

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Checking usage of newly imported named exports in the code
rg -A 2 "import { correctParentValues.*throwError } from \"../utils/genericUtils\"" 
rg -A 2 "import { extractCodesFromBoundaryRelationshipResponse.*processDataForTargetCalculation } from '../utils/campaignUtils'"

Length of output: 593

---

Script:

#!/bin/bash
# Search for usage of each imported utility from genericUtils
rg "correctParentValues\(" health-services/project-factory/src/server/api/genericApis.ts
rg "findMapValue\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getBoundaryRelationshipData\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getDataSheetReady\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getLocalizedHeaders\(" health-services/project-factory/src/server/api/genericApis.ts
rg "sortCampaignDetails\(" health-services/project-factory/src/server/api/genericApis.ts
rg "throwError\(" health-services/project-factory/src/server/api/genericApis.ts

# Search for usage of each imported utility from campaignUtils
rg "extractCodesFromBoundaryRelationshipResponse\(" health-services/project-factory/src/server/api/genericApis.ts
rg "generateFilteredBoundaryData\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getConfigurableColumnHeadersBasedOnCampaignType\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getFiltersFromCampaignSearchResponse\(" health-services/project-factory/src/server/api/genericApis.ts
rg "getLocalizedName\(" health-services/project-factory/src/server/api/genericApis.ts
rg "processDataForTargetCalculation\(" health-services/project-factory/src/server/api/genericApis.ts

Length of output: 4062

health-services/project-factory/src/server/config/index.ts (6)

34-34: Consider renaming for clarity.

Currently, the field is named boundaryCodeMandatoryForMicroplanFacility, but the environment variable is named BOUNDARY_CODE_HEADER_NAME_FACILITY_MICROPLAN. These names suggest slightly different meanings. Consider renaming this property or the environment variable to maintain clear, consistent naming.

---

43-44: Make the facility code & type configurable.

Both facilityCodeColumn and facilityType are hardcoded. If there’s a need to adapt or extend these values in the future, consider sourcing them from environment variables instead. This approach ensures flexibility and easier maintenance.

---

101-102: Use Number.parseInt instead of global parseInt.

For better consistency and to adhere to modern JavaScript standards, use Number.parseInt instead of the global parseInt function.

🧰 Tools

🪛 Biome (1.9.4)

[error] 101-101: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

150-150: No concerns with renaming the property.

Renaming from facilityBulkCreate to facilityCreate appears logical and aligns with the typical naming convention. Good to go.

---

155-155: Looks good.

The new cacheBurst property is straightforward. No concerns.

---

185-187: Approved for environment-based configurability.

Adding these new environment variable–driven properties (autoRetryIfHttpError, latLongColumns, validateCampaignIdInMetadata) allows for better customization without having to modify code. This approach is good.

[coderabbitai]

🧹 Nitpick (assertive)

Consider extracting validation logic into smaller functions

The validation logic in this function is complex and could benefit from being broken down into smaller, more focused functions for better maintainability and testability.

Consider extracting the validation logic into separate functions:

function validateTargetValue(target: any, targetColumn: string, rowNumber: number, sheetName: string): any[] {
    const errors = [];
    if (typeof target !== 'number') {
        errors.push(createError("INVALID", rowNumber, `Data in column '${targetColumn}' is not a number.`, sheetName));
    } else if (target <= 0 || target > 1000000) {
        errors.push(createError("INVALID", rowNumber, `Data in column '${targetColumn}' is out of range.`, sheetName));
    } else if (!Number.isInteger(target)) {
        errors.push(createError("INVALID", rowNumber, `Data in column '${targetColumn}' is not an integer.`, sheetName));
    }
    return errors;
}

function createError(status: string, rowNumber: number, details: string, sheetName: string) {
    return {
        status,
        rowNumber,
        errorDetails: `${details}(It can only be positive numbers less than or equal to 1 Million)`,
        sheetName
    };
}

🧰 Tools

🪛 Biome (1.9.4)

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

[coderabbitai]

🧹 Nitpick (assertive)

Add a comment explaining the microplan boundary validation logic

Consider adding a comment to explain why microplan facilities require different boundary validation.

+ // Microplan facilities require special boundary validation due to their unique requirements
  if (await isMicroplanRequest(request) && resource?.type == "facility") {
      boundaryColumn = getLocalizedName(config.boundary.boundaryCodeMandatoryForMicroplanFacility, localizationMap);
  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (await isMicroplanRequest(request) && resource?.type == "facility") {
	boundaryColumn = getLocalizedName(config.boundary.boundaryCodeMandatoryForMicroplanFacility, localizationMap);
	// Microplan facilities require special boundary validation due to their unique requirements
	if (await isMicroplanRequest(request) && resource?.type == "facility") {
	boundaryColumn = getLocalizedName(config.boundary.boundaryCodeMandatoryForMicroplanFacility, localizationMap);

[coderabbitai]

🧹 Nitpick (assertive)

Effective metadata enrichment.

Enriching the workbook’s keywords with locale & campaign info is concise and straightforward. If possible, consider a fallback in case the request does not include valid query.campaignId.

[coderabbitai]

🧹 Nitpick (assertive)

Importing enrichTemplateMetaData.

This complements the updated functionality in excelUtils.ts for localizing and stamping metadata. Consider adding fallback logic if the request object lacks keys.

[coderabbitai]

🧹 Nitpick (assertive)

Enhance logging context.

Right now, only a brief comment is provided. Consider logging the actual campaign ID or metadata for better traceability.

 logger.info("Enriching template metadata before file upload.");
+logger.info(`Campaign ID: ${request?.query?.campaignId}, Tenant: ${request?.body?.RequestInfo?.userInfo?.tenantId}`);
 enrichTemplateMetaData(updatedWorkbook, request);

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🛠️ Refactor suggestion

Add consistent error handling.

When the getJsonData call fails or returns invalid data, ensure the code handles it gracefully instead of proceeding.

+ if (!jsonData || jsonData.length === 0) {
+   console.warn("No valid data found in getJsonData.");
+   continue;
+ }

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5420d8e and bd673e7.

📒 Files selected for processing (1)

• health-services/project-factory/src/server/validators/campaignValidators.ts (7 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/validators/campaignValidators.ts

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

🔇 Additional comments (2)

health-services/project-factory/src/server/validators/campaignValidators.ts (2)

113-134: Extract validation logic to reduce code duplication

The validation checks for target in lines 113-134 are repetitive. Consider extracting this logic into a separate function to enhance maintainability and readability.

---

1397-1397: Enhance error message clarity

The error message could be more descriptive to help users understand and fix the issue.

[coderabbitai]

⚠️ Potential issue

Ensure columnsNotToBeFreezed and requiredColumns are defined before use

The variables columnsNotToBeFreezed and requiredColumns might be undefined if schema lacks these properties, leading to runtime errors.

Consider initializing them with default values:

- const columnsNotToBeFreezed = schema?.columnsNotToBeFreezed;
- const requiredColumns = schema?.required;
+ const columnsNotToBeFreezed = schema?.columnsNotToBeFreezed || [];
+ const requiredColumns = schema?.required || [];

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const columnsNotToBeFreezed = schema?.columnsNotToBeFreezed;
	const requiredColumns = schema?.required;
	const columnsNotToBeFreezed = schema?.columnsNotToBeFreezed || [];
	const requiredColumns = schema?.required || [];

[coderabbitai]

🧹 Nitpick (assertive)

Use toLowerCase() instead of toLocaleLowerCase() for consistent string comparison

Using toLowerCase() ensures consistent behavior across different locales when performing case-insensitive comparisons.

Apply this diff:

- .filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLocaleLowerCase() == differentTabsBasedOnLevel.toLowerCase())
+ .filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLowerCase() == differentTabsBasedOnLevel.toLowerCase())

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	.filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLocaleLowerCase() == differentTabsBasedOnLevel.toLowerCase())
	.filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLowerCase() == differentTabsBasedOnLevel.toLowerCase())

[coderabbitai]

🧹 Nitpick (assertive)

Use Object.hasOwn() instead of hasOwnProperty for safer property checks

In line 99, it's recommended to use Object.hasOwn() when checking for property existence to avoid issues when hasOwnProperty is overridden.

Apply this diff:

- boundaryData.some((row: any) => row.hasOwnProperty(column) && row[column] !== null && row[column] !== undefined)
+ boundaryData.some((row: any) => Object.hasOwn(row, column) && row[column] !== null && row[column] !== undefined)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	boundaryData.some((row: any) => row.hasOwnProperty(column) && row[column] !== null && row[column] !== undefined)
	boundaryData.some((row: any) => Object.hasOwn(row, column) && row[column] !== null && row[column] !== undefined)

🧰 Tools

🪛 Biome (1.9.4)

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97b4b32 and b5be48e.

📒 Files selected for processing (2)

• health-services/project-factory/src/server/config/index.ts (6 hunks)
• health-services/project-factory/src/server/utils/campaignMappingUtils.ts (1 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/campaignMappingUtils.ts

[error] 512-512: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'let' instead.

(lint/style/noVar)

health-services/project-factory/src/server/config/index.ts

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 103-103: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

🔇 Additional comments (4)

health-services/project-factory/src/server/config/index.ts (4)

22-22: LGTM! Good improvement to make retry attempts configurable.

The change makes the retry attempts configurable via environment variables, with a sensible default value of 100. This is an improvement over the previous hardcoded value of 75.

---

35-35: Consider renaming for clarity.

Currently, the field is named boundaryCodeMandatoryForMicroplanFacility, but the environment variable is named BOUNDARY_CODE_HEADER_NAME_FACILITY_MICROPLAN. These names suggest slightly different meanings.

---

44-45: Make the facility code & type configurable.

Both facilityCodeColumn and facilityType are hardcoded. If there's a need to adapt or extend these values in the future, consider sourcing them from environment variables instead of hardcoding.

---

102-103: Use Number.parseInt instead of global parseInt

For better consistency and to adhere to modern JavaScript standards, use Number.parseInt instead of the global parseInt function.

🧰 Tools

🪛 Biome (1.9.4)

[error] 102-102: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 103-103: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

[coderabbitai]

🧹 Nitpick (assertive)

Consider using an array for latLongColumns configuration.

The latLongColumns property uses a comma-separated string for multiple column names. Consider using an array configuration for better maintainability and type safety.

Apply this diff to improve the configuration:

-    latLongColumns: process.env.LAT_LONG_SUBSTRINGS || "HCM_ADMIN_CONSOLE_FACILITY_LATITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_FACILITY_LONGITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_TARGET_LAT_OPT,HCM_ADMIN_CONSOLE_TARGET_LONG_OPT",
+    latLongColumns: (process.env.LAT_LONG_SUBSTRINGS || "HCM_ADMIN_CONSOLE_FACILITY_LATITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_FACILITY_LONGITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_TARGET_LAT_OPT,HCM_ADMIN_CONSOLE_TARGET_LONG_OPT").split(","),

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	autoRetryIfHttpError: process.env.AUTO_RETRY_IF_HTTP_ERROR || "socket hang up" /* can be retry if there is any error for which default retry can be set */,
	latLongColumns: process.env.LAT_LONG_SUBSTRINGS || "HCM_ADMIN_CONSOLE_FACILITY_LATITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_FACILITY_LONGITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_TARGET_LAT_OPT,HCM_ADMIN_CONSOLE_TARGET_LONG_OPT",
	validateCampaignIdInMetadata: process.env.VALIDATE_CAMPAIGN_ID_IN_METADATA === "true"
	autoRetryIfHttpError: process.env.AUTO_RETRY_IF_HTTP_ERROR || "socket hang up" /* can be retry if there is any error for which default retry can be set */,
	latLongColumns: (process.env.LAT_LONG_SUBSTRINGS || "HCM_ADMIN_CONSOLE_FACILITY_LATITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_FACILITY_LONGITUDE_OPTIONAL_MICROPLAN,HCM_ADMIN_CONSOLE_TARGET_LAT_OPT,HCM_ADMIN_CONSOLE_TARGET_LONG_OPT").split(","),
	validateCampaignIdInMetadata: process.env.VALIDATE_CAMPAIGN_ID_IN_METADATA === "true"

[coderabbitai]

🛠️ Refactor suggestion

Use let instead of var for block-scoped variable.

Replace var with let to properly scope the retry variable to the block where it's used. This follows modern JavaScript best practices and prevents potential hoisting issues.

Apply this diff to fix the scoping:

-                var retry: any = config?.retryUntilResourceCreationComplete;
+                let retry: any = config?.retryUntilResourceCreationComplete;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	var retry: any = config?.retryUntilResourceCreationComplete;
	let retry: any = config?.retryUntilResourceCreationComplete;

🧰 Tools

🪛 Biome (1.9.4)

[error] 512-512: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'let' instead.

(lint/style/noVar)

[coderabbitai]

Actionable comments posted: 5

🔭 Outside diff range comments (2)

health-services/project-factory/src/server/utils/campaignUtils.ts (2)

Line range hint 2190-2207: Consider using template literals for string concatenation.

Replace string concatenation with template literals for better readability:

- "Boundary target mapping count" + Object.keys(boundaryTargetMapping)?.length
+ `Boundary target mapping count: ${Object.keys(boundaryTargetMapping)?.length}`

---

Line range hint 2258-2290: Add error handling for undefined target mappings.

Consider adding error handling for cases where codesTargetMapping might be undefined:

- const codesTargetMapping = request?.body?.CampaignDetails?.codesTargetMapping;
+ const codesTargetMapping = request?.body?.CampaignDetails?.codesTargetMapping ?? {};
+ if (!codesTargetMapping) {
+   logger.warn('No target mappings found in campaign details');
+ }

Also applies to: 2336-2336

♻️ Duplicate comments (6)

health-services/project-factory/src/server/utils/campaignUtils.ts (2)

1700-1730: 🛠️ Refactor suggestion

Use Object.hasOwn() & const where appropriate.

Replace hasOwnProperty with Object.hasOwn() and use const for immutable variables:

- if (childTargetValue.hasOwnProperty(key)) {
+ if (Object.hasOwn(childTargetValue, key)) {

- let totalTargetValue: any = {};
+ const totalTargetValue: any = {};

🧰 Tools

🪛 Biome (1.9.4)

[error] 1717-1717: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1709-1709: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

2363-2395: 🛠️ Refactor suggestion

Use const and Number.parseInt for clarity.

Replace parseInt with Number.parseInt and use const for variables that aren't reassigned:

- let targets = [];
+ const targets = [];

- let targetNo = parseInt(codesTargetMapping?.[boundaryCode][key]);
+ const targetNo = Number.parseInt(codesTargetMapping?.[boundaryCode][key], 10);

- let beneficiaryType = key;
+ const beneficiaryType = key;

🧰 Tools

🪛 Biome (1.9.4)

[error] 2365-2365: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2370-2370: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

health-services/project-factory/src/server/validators/campaignValidators.ts (2)

93-135: 🧹 Nitpick (assertive)

Extract common validation logic to reduce code duplication

The validation logic for target values is repeated in validateTargetForNormalCampaigns. Refactor this logic into a helper function to enhance maintainability.

Refer to past review comments on lines 93-135 suggesting the extraction of validation code into a separate function.

🧰 Tools

🪛 Biome (1.9.4)

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

1386-1386: 🧹 Nitpick (assertive)

Use toLowerCase() instead of toLocaleLowerCase() for consistency

Using toLocaleLowerCase() may produce inconsistent results across different locales. Replace it with toLowerCase() to ensure consistent behavior.

Apply this diff to make the change:

- .filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLocaleLowerCase() == differentTabsBasedOnLevel.toLowerCase())
+ .filter((data: any) => getLocalizedName(`${request?.body?.ResourceDetails?.hierarchyType}_${data.type}`.toUpperCase(), localizationMap).toLowerCase() == differentTabsBasedOnLevel.toLowerCase())

health-services/project-factory/src/server/utils/microplanUtils.ts (2)

104-107: ⚠️ Potential issue

Fix inconsistent role handling logic and use template literals.

1. The role handling logic is inconsistent:
   • The if block removes the "ROOT_" prefix
   • The else block adds the "ROOT_" prefix
2. String concatenation is used instead of template literals.

Apply this diff to fix both issues:

  if (userRole?.startsWith("ROOT_")) {
    const trimmedRole = userRole.replace("ROOT_", "");
    // ... existing code ...
  } else {
    const trimmedRole = userRole;
-   const errorMessage: any = `An user with ${getLocalizedName("ROOT_" + trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
+   const errorMessage: any = `A user with ${getLocalizedName(trimmedRole, localizationMap)} role can't be assigned to ${getLocalizedName(userRole, localizationMap)} role`;
    // ... existing code ...
  }

Also applies to: 118-119

---

292-296: 🧹 Nitpick (assertive)

Consider adding type safety to facility capacity handling.

The facility capacity handling logic looks good, but could benefit from explicit type conversion for numeric values.

Apply this diff to add type safety:

  if(!rowData[facilityCapacityColumn]){
    rowData[facilityCapacityColumn] = 0
    element.storageCapacity = 0
  }
- else if (rowData[facilityCapacityColumn] >= 0) {
-   element.storageCapacity = rowData[facilityCapacityColumn]
+ else if (Number(rowData[facilityCapacityColumn]) >= 0) {
+   element.storageCapacity = Number(rowData[facilityCapacityColumn])
  }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 396673c and 3fc3a41.

📒 Files selected for processing (9)

• health-services/project-factory/src/server/config/constants.ts (1 hunks)
• health-services/project-factory/src/server/utils/campaignMappingUtils.ts (5 hunks)
• health-services/project-factory/src/server/utils/campaignUtils.ts (18 hunks)
• health-services/project-factory/src/server/utils/excelUtils.ts (3 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (15 hunks)
• health-services/project-factory/src/server/utils/microplanIntergration.ts (4 hunks)
• health-services/project-factory/src/server/utils/microplanUtils.ts (6 hunks)
• health-services/project-factory/src/server/validators/campaignValidators.ts (10 hunks)
• health-services/project-factory/src/server/validators/microplanValidators.ts (7 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/campaignMappingUtils.ts

[error] 512-512: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'let' instead.

(lint/style/noVar)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 888-888: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1290-1290: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/utils/campaignUtils.ts

[error] 1717-1717: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1709-1709: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2365-2365: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2370-2370: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 2426-2426: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

---

[error] 3050-3050: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/utils/microplanUtils.ts

[error] 119-119: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

health-services/project-factory/src/server/validators/campaignValidators.ts

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 728-728: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'const' instead.

(lint/style/noVar)

🔇 Additional comments (24)

health-services/project-factory/src/server/utils/genericUtils.ts (3)

17-17: Clean import organization!

The imports are well-structured and logically grouped, with clear separation between core utilities and feature-specific imports.

Also applies to: 21-21

---

729-736: 🛠️ Refactor suggestion

Add error handling for dropdown initialization.

The dropdown re-fetching logic should include error handling to gracefully handle potential failures:

  let receivedDropdowns = request.body?.dropdowns;
  logger.info("started adding dropdowns in facility", JSON.stringify(receivedDropdowns))

  if (!receivedDropdowns || Object.keys(receivedDropdowns)?.length == 0) {
    logger.info("No dropdowns found");
+   try {
      receivedDropdowns = setDropdownFromSchema(request, schema, localizationMap);
      logger.info("refetched drodowns", JSON.stringify(receivedDropdowns))
+   } catch (error) {
+     logger.error("Failed to fetch dropdowns from schema:", error);
+     throw new Error("Failed to initialize dropdowns");
+   }
  }

Likely invalid or redundant comment.

---

1287-1292: ⚠️ Potential issue

Improve boundary data processing safety.

1. Add array bounds checking before accessing elements
2. Use const instead of let for loop variable (as noted by static analysis)

  if (type == "boundaryManagement") {
    logger.info("Processing data for boundaryManagement type")
    const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
-   for (let d of data) {
+   for (const d of data) {
+     if (!Array.isArray(d) || d.length === 0) {
+       logger.error("Invalid data structure in boundary management");
+       continue;
+     }
      const boundaryCode = d[d.length - 1];

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1290-1290: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/validators/microplanValidators.ts (6)

1-8: LGTM! Well-organized imports.

The new imports are properly organized and align with the added functionality.

---

132-132: LGTM! Descriptive function rename.

The rename from validateTargetsForMicroplanCampaigns to validateRequiredTargetsForMicroplanCampaigns better reflects the function's purpose of validating required targets.

---

195-197: Consider memoizing localized column names.

The previous suggestion about memoizing the localized column names is still valid as the mapping operation is performed on every call.

Also applies to: 202-202

---

280-286: LGTM! Improved facility capacity validation.

Good improvements:

• Using logging instead of error for empty capacity
• Allowing zero capacity with clear validation messages
• Consistent error messaging for numeric range validation

---

299-316: LGTM! Robust boundary validation implementation.

The function effectively validates that all boundaries in the uploaded sheet are present in campaign boundaries.

---

304-304: 🧹 Nitpick (assertive)

Use Object.hasOwn() for modern property check.

Replace Object.prototype.hasOwnProperty.call with the modern Object.hasOwn():

-if (Object.prototype.hasOwnProperty.call(dataFromSheet, key)) {
+if (Object.hasOwn(dataFromSheet, key)) {

Likely invalid or redundant comment.

health-services/project-factory/src/server/utils/campaignUtils.ts (4)

28-28: LGTM! Import additions look good.

The new imports searchMDMS and usageColumnStatus are properly added and align with the new functionality.

Also applies to: 59-59

---

277-277: LGTM! Consistent usage of status constant.

Good use of the imported usageColumnStatus.active constant for setting active status, which improves maintainability.

Also applies to: 304-304

---

963-963: LGTM! Consistent usage of root boundary code retrieval.

The code consistently uses getRootBoundaryCode() with boundariesCombined across different functions, maintaining a uniform approach.

Also applies to: 1040-1040, 1116-1116

---

3042-3066: 🛠️ Refactor suggestion

Improve code quality with modern JavaScript practices.

Several improvements can be made to align with best practices:

- let rowData: any = { [codeColumnName]: row[codeColumnName] };
+ const rowData: any = { [codeColumnName]: row[codeColumnName] };

+ interface TargetData {
+   [codeColumnName: string]: any;
+   'Parent Target at the Selected Boundary level': Record<string, any>;
+   'Target at the Selected Boundary level': Record<string, any>;
+ }

- const resultantData = jsonData.map((row: any) => {
+ const resultantData = jsonData.map((row: Record<string, any>): TargetData => {

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 3050-3050: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/utils/excelUtils.ts (3)

Line range hint 728-729: Replace var with let or const for block scoping

Using var can lead to unexpected behaviors due to function scoping. Replace it with let or const to adhere to best practices.

Apply this diff to address the issue:

- var active = activeColumnName ? data?.[activeColumnName] : usageColumnStatus.active;
+ const active = activeColumnName ? data?.[activeColumnName] : usageColumnStatus.active;

This change also addresses the static analysis hint from Biome regarding the use of var.

---

127-131: ⚠️ Potential issue

Handle missing request parameters in enrichTemplateMetaData

The function assumes that request.body.RequestInfo and request.query.campaignId are always present. Add validation to handle cases where these may be undefined to prevent runtime errors.

Apply this diff to ensure robustness:

 export function enrichTemplateMetaData(updatedWorkbook : any, request : any ){
-  if(request?.body?.RequestInfo && request?.query?.campaignId){
+  if(request?.body?.RequestInfo && request?.query?.campaignId){
     updatedWorkbook.keywords = `${getLocaleFromRequestInfo(request?.body?.RequestInfo)}#${request?.query?.campaignId}`
   }
+  else {
+    throwError(
+      "COMMON",
+      400,
+      "MISSING_DATA",
+      "RequestInfo or campaignId is missing in the request."
+    );
+  }
 }

Likely invalid or redundant comment.

---

377-393: ⚠️ Potential issue

Ensure usageColumnStatus.inactive is defined before use

In the enrichUsageColumnForFacility function, verify that usageColumnStatus.inactive is defined to prevent potential undefined value assignments.

Consider adding a check:

if (usageColumnStatus && usageColumnStatus.inactive) {
  // Proceed to set cell value
  cell.value = usageColumnStatus.inactive;
} else {
  throwError("COMMON", 500, "CONFIG_ERROR", "Usage column status is not defined.");
}

health-services/project-factory/src/server/validators/campaignValidators.ts (2)

728-728: ⚠️ Potential issue

Replace var with const for block scope compliance

On line 728, replace var with const to prevent hoisting issues and ensure proper block scoping.

Apply this diff:

- var active = activeColumnName ? data?.[activeColumnName] : usageColumnStatus.active;
+ const active = activeColumnName ? data?.[activeColumnName] : usageColumnStatus.active;

This change resolves the Biome static analysis hint regarding the use of var.

✅ Verification successful

Replace var with const for better code quality

The variable is not reassigned within its scope and is only used for a condition check. Using const is safe and follows modern JavaScript/TypeScript best practices.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

---

Script:

#!/bin/bash
# Get the context around the var declaration
rg -B 10 -A 10 "var active = activeColumnName" health-services/project-factory/src/server/validators/campaignValidators.ts

# Look for any reassignments of 'active' variable
ast-grep --pattern 'active = $_'

Length of output: 18704

🧰 Tools

🪛 Biome (1.9.4)

[error] 728-728: Use let or const instead of var.

A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
See MDN web docs for more details.
Unsafe fix: Use 'const' instead.

(lint/style/noVar)

---

99-100: ⚠️ Potential issue

Use Object.hasOwn() instead of hasOwnProperty

In validateTargetForNormalCampaigns, using Object.hasOwn() is safer and avoids issues when hasOwnProperty is overridden.

Apply this diff:

 const targetColumnsToValidate = isGenericType ? localizedTargetColumnNames.filter((column: string) =>
-    boundaryData.some((row: any) => row.hasOwnProperty(column) && row[column] !== null && row[column] !== undefined)
+    boundaryData.some((row: any) => Object.hasOwn(row, column) && row[column] !== null && row[column] !== undefined)
 ) : localizedTargetColumnNames;

This change addresses the static analysis warning about not accessing hasOwnProperty directly.

🧰 Tools

🪛 Biome (1.9.4)

[error] 99-99: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

health-services/project-factory/src/server/config/constants.ts (1)

161-164: Well-defined usageColumnStatus constant

Adding the usageColumnStatus constant enhances code clarity and maintainability by centralizing status values.

Ensure all references to "Active" and "Inactive" in the codebase are updated to use this constant.

health-services/project-factory/src/server/utils/microplanIntergration.ts (2)

428-428: LGTM! Good use of constants.

Replacing the hardcoded string with usageColumnStatus.active improves maintainability and consistency.

---

487-491: LGTM! Good use of constants.

Replacing hardcoded strings with usageColumnStatus constants improves maintainability and consistency across the codebase.

Also applies to: 525-525

health-services/project-factory/src/server/utils/microplanUtils.ts (1)

Line range hint 512-512: Use let instead of var for block-scoped variable.

Replace var with let to properly scope the retry variable to the block where it's used. This follows modern JavaScript best practices and prevents potential hoisting issues.

Apply this diff to fix the scoping:

-                var retry: any = config?.retryUntilResourceCreationComplete;
+                let retry: any = config?.retryUntilResourceCreationComplete;

health-services/project-factory/src/server/utils/campaignMappingUtils.ts (2)

192-192: LGTM! Good use of constants.

Replacing the hardcoded string with usageColumnStatus.active improves maintainability and consistency.

---

230-234: LGTM! Good use of constants.

Replacing hardcoded strings with usageColumnStatus constants improves maintainability and consistency across the codebase.

Also applies to: 263-263

[coderabbitai]

🧹 Nitpick (assertive)

Improve condition clarity for facility validation.

The condition active == usageColumnStatus.active || !item?.[uniqueIdentifierColumnName] combines two different concerns. Consider separating them for better readability:

-if (active == usageColumnStatus.active || !item?.[uniqueIdentifierColumnName]) {
+// Always validate if identifier is missing, otherwise only validate active facilities
+if (!item?.[uniqueIdentifierColumnName] || active == usageColumnStatus.active) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const active = activeColumnName ? item[activeColumnName] : usageColumnStatus.active;
	if (active == usageColumnStatus.active || !item?.[uniqueIdentifierColumnName]) {
	enrichErrorForFcailityMicroplan(request, item, errors, localizationMap);
	validateLatLongForFacility(item, errors);
	validateLatLongForFacility(item, errors, localizationMap);
	const active = activeColumnName ? item[activeColumnName] : usageColumnStatus.active;
	// Always validate if identifier is missing, otherwise only validate active facilities
	if (!item?.[uniqueIdentifierColumnName] || active == usageColumnStatus.active) {
	enrichErrorForFcailityMicroplan(request, item, errors, localizationMap);
	validateLatLongForFacility(item, errors, localizationMap);

[coderabbitai]

🧹 Nitpick (assertive)

Organize imports for readability

Consider arranging the import statements alphabetically to enhance readability and maintain consistent code styling.

[coderabbitai]

🛠️ Refactor suggestion

Validate the action parameter in validateFileMetaDataViaFileUrl

The function validateFileMetaDataViaFileUrl checks if action is "validate" but does not handle other possible values. To prevent unexpected behavior, validate the action parameter to ensure it only accepts allowed values.

Apply this diff to enhance validation:

 export async function validateFileMetaDataViaFileUrl(fileUrl: string, expectedLocale: string, expectedCampaignId: string, action: string) {
     if (!fileUrl) {
         throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no file URL was found.");
-    }
-    else if(action === "validate"){
+    }
+    else if(action === "validate"){
         const workbook = await getExcelWorkbookFromFileURL(fileUrl);
         if (!workbook) {
             throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no workbook was found.");
         }
         else {
             validateFileMetadata(workbook, expectedLocale, expectedCampaignId);
         }
     }
+    else {
+        throwError("COMMON", 400, "INVALID_ACTION", `Unsupported action "${action}".`);
+    }
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export async function validateFileMetaDataViaFileUrl(fileUrl: string, expectedLocale: string, expectedCampaignId: string, action: string) {
	if (!fileUrl) {
	throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no file URL was found.");
	}
	else if(action === "validate"){
	const workbook = await getExcelWorkbookFromFileURL(fileUrl);
	if (!workbook) {
	throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no workbook was found.");
	}
	else {
	validateFileMetadata(workbook, expectedLocale, expectedCampaignId);
	}
	}
	}
	export async function validateFileMetaDataViaFileUrl(fileUrl: string, expectedLocale: string, expectedCampaignId: string, action: string) {
	if (!fileUrl) {
	throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no file URL was found.");
	}
	else if(action === "validate"){
	const workbook = await getExcelWorkbookFromFileURL(fileUrl);
	if (!workbook) {
	throwError("COMMON", 400, "VALIDATION_ERROR", "There is an issue while reading the file as no workbook was found.");
	}
	else {
	validateFileMetadata(workbook, expectedLocale, expectedCampaignId);
	}
	}
	else {
	throwError("COMMON", 400, "INVALID_ACTION", `Unsupported action "${action}".`);
	}
	}

[coderabbitai]

🧹 Nitpick (assertive)

Improve error messages in validateFileMetadata for clarity

In the validateFileMetadata function, providing specific details in error messages can help users identify issues more easily. Include the expected and actual values in the error messages.

Apply this diff to enhance error messaging:

 // Validate locale if provided
 if (templateLocale !== expectedLocale) {
     throwError(
         "FILE",
         400,
         "INVALID_TEMPLATE",
-        `The template doesn't have matching locale metadata. Please upload the generated template for the current locale.`
+        `Template locale "${templateLocale}" does not match expected locale "${expectedLocale}". Please upload the correct template.`
     );
 }

 // Validate campaignId if provided
 if (templateCampaignId !== expectedCampaignId && config.values.validateCampaignIdInMetadata) {
     throwError(
         "FILE",
         400,
         "INVALID_TEMPLATE",
-        `The template doesn't have matching campaign metadata. Please upload the generated template for the current campaign only.`
+        `Template campaign ID "${templateCampaignId}" does not match expected campaign ID "${expectedCampaignId}". Please upload the correct template.`
     );
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	export const validateFileMetadata = (workbook: any, expectedLocale: string, expectedCampaignId: string) => {
	// Retrieve and validate keywords from the workbook's custom properties
	const keywords = workbook?.keywords;
	if (!keywords || !keywords.includes("#")) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	"The template doesn't have campaign metadata. Please upload the generated template only."
	);
	}
	const [templateLocale, templateCampaignId] = keywords.split("#");
	// Ensure there are exactly two parts in the metadata
	if (!templateLocale || !templateCampaignId) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	"The template doesn't have valid campaign metadata. Please upload the generated template only."
	);
	}
	// Validate locale if provided
	if (templateLocale !== expectedLocale) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	`The template doesn't have matching locale metadata. Please upload the generated template for the current locale.`
	);
	}
	// Validate campaignId if provided
	if (templateCampaignId !== expectedCampaignId && config.values.validateCampaignIdInMetadata) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	`The template doesn't have matching campaign metadata. Please upload the generated template for the current campaign only.`
	);
	}
	};
	export const validateFileMetadata = (workbook: any, expectedLocale: string, expectedCampaignId: string) => {
	// Retrieve and validate keywords from the workbook's custom properties
	const keywords = workbook?.keywords;
	if (!keywords || !keywords.includes("#")) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	"The template doesn't have campaign metadata. Please upload the generated template only."
	);
	}
	const [templateLocale, templateCampaignId] = keywords.split("#");
	// Ensure there are exactly two parts in the metadata
	if (!templateLocale || !templateCampaignId) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	"The template doesn't have valid campaign metadata. Please upload the generated template only."
	);
	}
	// Validate locale if provided
	if (templateLocale !== expectedLocale) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	`Template locale "${templateLocale}" does not match expected locale "${expectedLocale}". Please upload the correct template.`
	);
	}
	// Validate campaignId if provided
	if (templateCampaignId !== expectedCampaignId && config.values.validateCampaignIdInMetadata) {
	throwError(
	"FILE",
	400,
	"INVALID_TEMPLATE",
	`Template campaign ID "${templateCampaignId}" does not match expected campaign ID "${expectedCampaignId}". Please upload the correct template.`
	);
	}
	};

[coderabbitai]

Actionable comments posted: 10

🔭 Outside diff range comments (1)

health-services/project-factory/src/server/utils/genericUtils.ts (1)

Line range hint 751-805: Improve dropdown handling implementation.

1. Rename handledropdownthings to follow camelCase convention (e.g., handleDropdowns)
2. Extract duplicate validation configuration into a shared constant
3. Consider adding error handling for invalid dropdown values

+ const DROPDOWN_VALIDATION = {
+   type: 'list',
+   showDropDown: true,
+   error: 'Please select a value from the dropdown list.',
+   errorStyle: 'stop',
+   showErrorMessage: true,
+   errorTitle: 'Invalid Entry'
+ };

- async function handledropdownthings(sheet: any, schema: any, localizationMap: any) {
+ async function handleDropdowns(sheet: any, schema: any, localizationMap: any) {
   // ...
   cell.dataValidation = {
-    type: 'list',
-    showDropDown: true,
-    error: 'Please select a value from the dropdown list.',
-    errorStyle: 'stop',
-    showErrorMessage: true,
-    errorTitle: 'Invalid Entry'
+    ...DROPDOWN_VALIDATION,
+    formulae: [`"${dropdowns[key].join(',')}"`]
   };
   // ...
}

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc3a41 and 343393e.

📒 Files selected for processing (3)

• health-services/project-factory/src/server/utils/campaignUtils.ts (18 hunks)
• health-services/project-factory/src/server/utils/excelUtils.ts (4 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (19 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 854-854: This let declares a variable that is only assigned once.

'receivedDropdowns' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 916-916: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1319-1319: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

health-services/project-factory/src/server/utils/campaignUtils.ts

[error] 1717-1717: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1709-1709: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2369-2369: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

---

[error] 2425-2425: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

---

[error] 3049-3049: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

🔇 Additional comments (11)

health-services/project-factory/src/server/utils/excelUtils.ts (3)

66-79: Validate the action parameter.

The function still needs validation for the action parameter to handle invalid values.

---

81-124: Enhance error messages with specific details.

The error messages could be more descriptive by including the expected and actual values.

---

127-131: Consider fallback for missing campaign ID.

The function should handle cases where request.query.campaignId is not available.

health-services/project-factory/src/server/utils/campaignUtils.ts (6)

277-277: Good use of constants!

Replacing hardcoded string values with usageColumnStatus.active improves maintainability and consistency.

Also applies to: 304-304

---

Line range hint 2190-2208: LGTM! Good use of deep equality comparison.

The code correctly identifies target value differences between parent and current levels using _.isEqual for proper object comparison.

---

2425-2425: 🧹 Nitpick (assertive)

Improve performance by avoiding the delete operator

Using the delete operator can impact performance. Setting the property to undefined is more efficient.

Apply this diff:

-    delete request.body?.boundariesCombined;
+    request.body.boundariesCombined = undefined;

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 2425-2425: Avoid the delete operator which can impact performance.

(lint/performance/noDelete)

---

1700-1730: 🛠️ Refactor suggestion

Improve code safety and use modern JavaScript practices

The target value calculation logic is good, but there are a couple of improvements to make:

1. Replace hasOwnProperty with Object.hasOwn() for safer property checks
2. Use const instead of let for totalTargetValue since it's never reassigned

Apply this diff:

-    let totalTargetValue: any = {};
+    const totalTargetValue: any = {};

-      if (childTargetValue.hasOwnProperty(key)) {
+      if (Object.hasOwn(childTargetValue, key)) {

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1717-1717: Do not access Object.prototype method 'hasOwnProperty' from target object.

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

(lint/suspicious/noPrototypeBuiltins)

---

[error] 1709-1709: This let declares a variable that is only assigned once.

'totalTargetValue' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

3041-3065: 🧹 Nitpick (assertive)

Use const for immutable variables

The target data processing logic is good, but can be improved by using const for variables that aren't reassigned.

Apply this diff:

-    let rowData: any = { [codeColumnName]: row[codeColumnName] };
+    const rowData: any = { [codeColumnName]: row[codeColumnName] };

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 3049-3049: This let declares a variable that is only assigned once.

'rowData' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

2362-2394: 🧹 Nitpick (assertive)

Modernize code with ES6+ practices

The target enrichment logic is good, but can be improved with modern JavaScript practices:

1. Use const for variables that aren't reassigned
2. Use Number.parseInt instead of global parseInt

Apply this diff:

-    let targets: any[] = [];
+    const targets: any[] = [];

-      const targetNo = parseInt(codesTargetMapping[boundaryCode][beneficiaryType], 10);
+      const targetNo = Number.parseInt(codesTargetMapping[boundaryCode][beneficiaryType], 10);

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 2364-2364: This let declares a variable that is only assigned once.

'targets' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 2369-2369: Use Number.parseInt instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.

(lint/style/useNumberNamespace)

health-services/project-factory/src/server/utils/genericUtils.ts (2)

820-820: 🧹 Nitpick (assertive)

Improve logging practices.

Consider structuring the log output to avoid cluttering and potential sensitive data exposure:

- logger.info("hiddenColumns", hiddenColumns);
+ logger.info(`Processing ${hiddenColumns?.length || 0} hidden columns`);
+ if (logger.isDebugEnabled()) {
+   logger.debug("Hidden columns:", JSON.stringify(hiddenColumns, null, 2));
+ }

Likely invalid or redundant comment.

---

1316-1321: 🛠️ Refactor suggestion

Improve boundary management data processing.

1. Add input validation for boundary codes
2. Add error handling for missing lat/long data
3. Use const instead of let for loop variable (as flagged by static analysis)

 if (type == "boundaryManagement") {
   logger.info("Processing data for boundaryManagement type")
   const latLongBoundaryMap = await getLatLongMapForBoundaryCodes(request, boundaryCodeList);
-  for (let d of data) {
+  for (const d of data) {
     const boundaryCode = d[d.length - 1];
+    if (!boundaryCode) {
+      logger.warn(`Missing boundary code in data row`);
+      continue;
+    }
     if (latLongBoundaryMap[boundaryCode]) {
       const [latitude = null, longitude = null] = latLongBoundaryMap[boundaryCode];
+      if (latitude === null || longitude === null) {
+        logger.warn(`Missing lat/long data for boundary code: ${boundaryCode}`);
+      }
       d.push(latitude);
       d.push(longitude);
     }
   }
 }

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1319-1319: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

🧹 Nitpick (assertive)

Remove commented out code.

Remove the commented out line as it adds unnecessary noise to the codebase.

-  // sheet.protect('passwordhere', { selectLockedCells: true, selectUnlockedCells: true });

[coderabbitai]

⚠️ Potential issue

Move hardcoded password to configuration.

The hardcoded password "passwordhere" should be moved to a configuration file for better security and maintainability. Consider using environment variables for sensitive data.

+import config from "../config";
+
 function protectSheet(sheet: any) {
   sheet.protect('passwordhere', {
+  sheet.protect(config.values.sheetPassword, {
     selectLockedCells: true,
     selectUnlockedCells: true,
   });
 }

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🧹 Nitpick (assertive)

Move hardcoded strings to constants.

Consider moving the hardcoded string "facility" to a constants file for better maintainability and reusability.

+const FACILITY_CONFIG_TYPE = "facility";
+
 export function enrichUsageColumnForFacility(worksheet: any, localizationMap: any) {
-  const configType = "facility";
+  const configType = FACILITY_CONFIG_TYPE;
   const usageColumn = getLocalizedName(createAndSearch[configType]?.activeColumnName, localizationMap);

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🛠️ Refactor suggestion

Remove commented out code.

The setDropdownFromSchema function is commented out but still referenced in multiple places (lines 893, 918, 1014). Either remove the commented code entirely or restore it if still needed.

[coderabbitai]

🛠️ Refactor suggestion

Standardize equality operators.

Replace loose equality operators with strict equality for consistent type comparison:

- if (type != 'boundaryManagement' && request?.query?.campaignId != 'default' && type != 'boundaryGeometryManagement') {
+ if (type !== 'boundaryManagement' && request?.query?.campaignId !== 'default' && type !== 'boundaryGeometryManagement') {

- else if (type == 'boundaryManagement' || type === 'boundaryGeometryManagement') {
+ else if (type === 'boundaryManagement' || type === 'boundaryGeometryManagement') {

Also applies to: 387-387

[coderabbitai]

🧹 Nitpick (assertive)

Consider caching optimization for localization.

The localization data fetching could benefit from caching to improve performance.

Consider implementing a caching mechanism for frequently accessed localization data to reduce database load and improve response times. This could be achieved using the existing appCache utility or a dedicated localization cache.

[coderabbitai]

🧹 Nitpick (assertive)

Use const for immutable variables.

The variable is never reassigned, use const instead of let:

- let receivedDropdowns = request.body?.dropdowns;
+ const receivedDropdowns = request.body?.dropdowns;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	let receivedDropdowns = request.body?.dropdowns;
	const receivedDropdowns = request.body?.dropdowns;

🧰 Tools

🪛 Biome (1.9.4)

[error] 854-854: This let declares a variable that is only assigned once.

'receivedDropdowns' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

🛠️ Refactor suggestion

Add error handling for file processing.

Add proper error handling for file processing operations:

+ try {
   const modifiedProcessedFacilitySheetData = modifyProcessedSheetData(typeWithoutWith, processedFacilitySheetData, schemaFinal, localizationMap);
   facilitySheetDataFinal = modifiedProcessedFacilitySheetData;
+ } catch (error) {
+   logger.error(`Error processing facility sheet data: ${error.message}`);
+   throwError("COMMON", 500, "FILE_PROCESSING_ERROR", "Failed to process facility sheet data");
+ }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const modifiedProcessedFacilitySheetData = modifyProcessedSheetData(typeWithoutWith, processedFacilitySheetData, schemaFinal, localizationMap);
	facilitySheetDataFinal = modifiedProcessedFacilitySheetData;
	try {
	const modifiedProcessedFacilitySheetData = modifyProcessedSheetData(typeWithoutWith, processedFacilitySheetData, schemaFinal, localizationMap);
	facilitySheetDataFinal = modifiedProcessedFacilitySheetData;
	} catch (error) {
	logger.error(`Error processing facility sheet data: ${error.message}`);
	throwError("COMMON", 500, "FILE_PROCESSING_ERROR", "Failed to process facility sheet data");
	}

[coderabbitai]

🛠️ Refactor suggestion

Improve type safety in request handling.

Add validation and type checking for the extracted type:

- const type = request?.query?.type || request?.body?.ResourceDetails?.type;
+ const type = request?.query?.type || request?.body?.ResourceDetails?.type;
+ if (!type) {
+   throwError("COMMON", 400, "VALIDATION_ERROR", "Type is required");
+ }
+ if (typeof type !== 'string') {
+   throwError("COMMON", 400, "VALIDATION_ERROR", "Type must be a string");
+ }

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

🛠️ Refactor suggestion

Improve error object handling.

Add type checking and validation for error properties to ensure consistent error reporting:

 item.status = generatedResourceStatuses.failed,
 item.additionalDetails = {
   ...item.additionalDetails,
   error: {
-    status: error.status,
-    code: error.code,
-    description: error.description,
-    message: error.message
+    status: error.status || 500,
+    code: error.code || 'UNKNOWN_ERROR',
+    description: error.description || null,
+    message: typeof error.message === 'string' ? error.message : 'Unknown error occurred'
   }
 }

Committable suggestion skipped: line range outside the PR's diff.

[coderabbitai]

Actionable comments posted: 2

🔭 Outside diff range comments (1)

health-services/project-factory/src/server/utils/genericUtils.ts (1)

Line range hint 758-812: Improve dropdown handling with better error handling and cell protection.

The new implementation provides better control over cell protection and validation, but could benefit from error logging.

 async function handledropdownthings(sheet: any, schema: any, localizationMap: any) {
   logger.info(sheet.rowCount)
   const dropdowns = Object.entries(schema.properties)
     .filter(([key, value]: any) => Array.isArray(value.enum) && value.enum.length > 0)
     .reduce((result: any, [key, value]: any) => {
       const newKey: any = getLocalizedName(key, localizationMap);
       result[newKey] = value.enum;
       return result;
     }, {});
   if (dropdowns) {
     logger.info("Dropdowns provided:", dropdowns);
     for (const key of Object.keys(dropdowns)) {
       let dropdownColumnIndex = -1;
+      try {
         if (dropdowns[key]) {
           logger.info(`Processing dropdown key: ${key} with values: ${dropdowns[key]}`);
           // ... rest of the code ...
         }
+      } catch (error) {
+        logger.error(`Error processing dropdown for key ${key}: ${error.message}`);
+      }
     }
   }
 }

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 343393e and d94b380.

📒 Files selected for processing (3)

• health-services/project-factory/migration/main/V202422115500__add_locale_generated_resouce.sql (1 hunks)
• health-services/project-factory/src/server/utils/genericUtils.ts (22 hunks)
• health-services/project-factory/src/server/utils/transforms/searchResponseConstructor.ts (1 hunks)

🧰 Additional context used

🪛 Biome (1.9.4)

health-services/project-factory/src/server/utils/transforms/searchResponseConstructor.ts

[error] 83-83: locale is assigned to itself.

This is where is assigned.

(lint/correctness/noSelfAssign)

health-services/project-factory/src/server/utils/genericUtils.ts

[error] 861-861: This let declares a variable that is only assigned once.

'receivedDropdowns' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

[error] 923-923: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

---

[error] 1326-1326: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

🔇 Additional comments (4)

health-services/project-factory/migration/main/V202422115500__add_locale_generated_resouce.sql (1)

1-2: LGTM! The migration adds locale support correctly.

The migration adds a locale column with appropriate type and length for storing locale codes.

health-services/project-factory/src/server/utils/genericUtils.ts (3)

262-265: LGTM! Search query properly handles locale parameter.

The locale condition is correctly added to the query with proper parameterization.

---

861-861: 🧹 Nitpick (assertive)

Use const for immutable variables.

The variable is never reassigned, use const instead of let.

- let receivedDropdowns = request.body?.dropdowns;
+ const receivedDropdowns = request.body?.dropdowns;

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 861-861: This let declares a variable that is only assigned once.

'receivedDropdowns' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

---

1323-1328: 🧹 Nitpick (assertive)

Use const for loop variable in boundary management processing.

The loop variable is never reassigned, use const instead of let.

- for (let d of data) {
+ for (const d of data) {

Likely invalid or redundant comment.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1326-1326: This let declares a variable that is only assigned once.

'd' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

[coderabbitai]

⚠️ Potential issue

Remove redundant self-assignment of locale property.

The line item.locale = item.locale is a redundant self-assignment that doesn't perform any transformation.

- item.locale = item.locale;

🧰 Tools

🪛 Biome (1.9.4)

[error] 83-83: locale is assigned to itself.

This is where is assigned.

(lint/correctness/noSelfAssign)

[coderabbitai]

⚠️ Potential issue

Add null check for msgId before splitting.

The current implementation might throw an error if msgId is undefined.

- const msgIdRaw = request.body.RequestInfo?.msgId;
- const locale = msgIdRaw?.split('|')[1] || null;
+ const msgIdRaw = request.body.RequestInfo?.msgId;
+ const locale = msgIdRaw ? msgIdRaw.split('|')[1] : null;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const msgIdRaw = request.body.RequestInfo?.msgId;
	const locale = msgIdRaw?.split('|')[1] || null;
	const msgIdRaw = request.body.RequestInfo?.msgId;
	const locale = msgIdRaw ? msgIdRaw.split('|')[1] : null;