add/edit mappings #2152
add/edit mappings #2152
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several changes across multiple files in the health micro-UI project. The modifications primarily focus on enhancing the campaign manager module, including updates to CSS styling, new React components for data mapping and upload, and improvements to data validation and configuration. The changes include a new Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (8)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js(10 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/CampaignConfig.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useReadExcelData.js(3 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useReadExcelData.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/CampaignConfig.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js
[error] 28-28: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 52-52: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 66-66: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 67-67: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 119-119: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 144-144: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js
[error] 37-37: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 418-418: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 435-435: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 453-453: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 412-412: 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)
[error] 413-413: 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)
[error] 414-414: 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)
[error] 415-415: 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)
[error] 439-455: 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)
[error] 440-440: 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)
[error] 441-441: 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)
[error] 525-525: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
[error] 522-522: This let declares a variable that is only assigned once.
'validate' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 720-720: 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] 823-823: 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] 691-691: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 794-794: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 880-880: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 889-897: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 898-908: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (7)
health/micro-ui/web/public/index.html (1)
13-13: LGTM! Version update is consistent.The CSS version update matches the change in the other HTML file, maintaining consistency across the application.
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss (2)
277-290: LGTM! Well-structured button layout.The grid layout for footer buttons is well-implemented:
- Equal width columns using
1fr 1fr- Full-width buttons within their containers
- Consistent styling for both primary and secondary buttons
292-295: Verify overflow visibility requirement.Setting
overflow: visibleon the popup children wrap might lead to content overlapping outside the popup boundaries. Please ensure this is the intended behavior and necessary for the UI requirements.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js (1)
783-791:⚠️ Potential issueVerify empty sheet handling.
The commented-out empty sheet validation has been removed. Please verify:
- Is there alternative validation for empty sheets?
- What is the expected behavior when an empty sheet is uploaded?
- Could this lead to potential issues downstream?
Consider either:
- Restoring the empty sheet validation
- Implementing alternative validation
- Documenting why empty sheets are now allowed
+ if (jsonData.length === 0) { + const errorMessage = t("HCM_EMPTY_SHEET"); + setErrorsType((prevErrors) => ({ + ...prevErrors, + [type]: errorMessage, + })); + setIsError(true); + return; + }✅ Verification successful
Empty sheet handling is properly implemented ✅
The commented-out validation was replaced with a better approach: empty sheets are now silently skipped using
if (jsonData.length == 0) continue;at line 608. This is actually a more user-friendly approach for multi-sheet files, preventing unnecessary error messages while maintaining data integrity.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for empty sheet validation patterns and HCM_EMPTY_SHEET usage echo "=== Searching for empty sheet validations ===" rg "jsonData\.length\s*(===|==)\s*0" -A 3 -B 3 echo -e "\n=== Searching for HCM_EMPTY_SHEET usage ===" rg "HCM_EMPTY_SHEET" -A 2 -B 2 echo -e "\n=== Looking for other sheet validation patterns ===" rg "setErrorsType.*type.*empty" -A 3 -B 3 echo -e "\n=== Examining the data processing flow ===" ast-grep --pattern 'const jsonData = $_'Length of output: 7061
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/CampaignConfig.js (3)
286-286: Verify validation type consistency with facility mapping.The
validationType: "facilityWithBoundary"aligns with the component'stype: "facilityWithBoundaryMapping", but there's a slight naming inconsistency between them.✅ Verification successful
The
validationType: "facilityWithBoundary"is correct and consistent with codebase usage.The validation type follows the standard naming convention used throughout the codebase. It's the
type: "facilityWithBoundaryMapping"that should be updated to match this convention.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of these validation types to ensure consistent naming rg -l 'facilityWithBoundary|facilityWithBoundaryMapping' --type jsLength of output: 1402
Script:
#!/bin/bash # Check usage context in relevant files echo "=== UploadData.js ===" rg -A 2 -B 2 'facilityWithBoundary|facilityWithBoundaryMapping' health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js echo -e "\n=== UploadDataMapping.js ===" rg -A 2 -B 2 'facilityWithBoundary|facilityWithBoundaryMapping' health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js echo -e "\n=== schemaConfig.js ===" rg -A 2 -B 2 'facilityWithBoundary|facilityWithBoundaryMapping' health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/schemaConfig.jsLength of output: 7056
339-339: 🛠️ Refactor suggestionVerify validation type consistency with user mapping.
The
validationType: "userWithBoundary"seems inconsistent with the component'stype: "userMapping". This naming mismatch might cause validation issues.Consider aligning the naming conventions. Either:
- type: "userMapping", - validationType: "userWithBoundary" + type: "userWithBoundaryMapping", + validationType: "userWithBoundary"Or:
- type: "userMapping", - validationType: "userWithBoundary" + type: "userMapping", + validationType: "userMapping"
286-286: Verify DataUploadWrapper component handles these validation types.The new
validationTypeproperties are being passed to theDataUploadWrappercomponent. Ensure this component is updated to handle these validation types.Also applies to: 339-339
✅ Verification successful
Validation types are properly handled in the DataUploadWrapper component
The component correctly handles both "facilityWithBoundary" and "userWithBoundary" validation types through a comprehensive validation system that includes schema validation, type checking, and boundary validation with proper error handling and user feedback.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for DataUploadWrapper component implementation echo "Searching for DataUploadWrapper component definition..." fd -e js -e jsx DataUploadWrapper # Search for validation type handling echo "Searching for validation type handling..." rg -A 5 'validationType.*(?:facilityWithBoundary|userWithBoundary)' --type jsLength of output: 2061
Script:
#!/bin/bash # Check DataUploadWrapper implementation echo "Content of DataUploadWrapper.js:" cat "health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DataUploadWrapper.js" # Search for validation-related imports or constants echo -e "\nSearching for validation-related code:" rg -l "validation" --type js --glob "health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/**"Length of output: 5002
Script:
#!/bin/bash # Check UploadDataMappingWrapper implementation echo "Content of UploadDataMappingWrapper.js:" cat "health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMappingWrapper.js" # Check UploadDataMapping implementation echo -e "\nContent of UploadDataMapping.js:" cat "health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js"Length of output: 43413
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].37/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].38/dist/index.css" /> |
There was a problem hiding this comment.
💡 Codebase verification
Invalid package version detected
The specified version 0.2.38 of @egovernments/digit-ui-health-css does not exist in the npm registry. The latest published version is 0.2.1, and the highest available version in the 0.2.x series is 0.2.37. This will cause CSS loading failures.
- health/micro-ui/web/public/index.html
- health/micro-ui/web/micro-ui-internals/example/public/index.html
🔗 Analysis chain
Verify version compatibility and changelog.
The CSS version update from 0.2.36 to 0.2.38 should be reviewed for any breaking changes.
Run this script to check package information and recent versions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check package versions and changelog
curl -s "https://registry.npmjs.org/@egovernments/digit-ui-health-css" | jq -r '.["dist-tags"].latest, .versions | keys[]' | sort -V
Length of output: 187
Script:
#!/bin/bash
# Check package information with corrected jq query
echo "Package information:"
curl -s "https://registry.npmjs.org/@egovernments/digit-ui-health-css" | jq -r '."dist-tags".latest as $latest | "Latest version: \($latest)\nAvailable versions:", (.versions | keys[]) // empty'
echo -e "\nChecking for other occurrences in the codebase:"
rg -l "digit-ui-health-css@" --type html
Length of output: 1052
Script:
#!/bin/bash
echo "Checking version in health/micro-ui/web/public/index.html:"
cat health/micro-ui/web/public/index.html | grep "digit-ui-health-css@"
Length of output: 307
| @@ -266,3 +266,31 @@ | |||
| .app-config-actionBar { | |||
| z-index: 9999; | |||
| } | |||
|
|
|||
| .digit-popup-wrapper.custom-pop-up-mapping { | |||
| width: fit-content !important; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider alternatives to !important.
Using !important for width override suggests potential specificity issues in the CSS cascade. Consider refactoring the selectors to achieve higher specificity without !important.
| @@ -9,6 +9,7 @@ import { InfoCard, PopUp, Toast, Button} from "@egovernments/digit-ui-components | |||
| import { downloadExcelWithCustomName } from "../utils"; | |||
| import { CONSOLE_MDMS_MODULENAME } from "../Module"; | |||
| import TagComponent from "./TagComponent"; | |||
| import { DataType } from "ajv/dist/compile/validate/dataType"; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused import.
The DataType import from ajv is not used anywhere in the code. Consider removing it to avoid unnecessary bundle size increase.
-import { DataType } from "ajv/dist/compile/validate/dataType";| // if (jsonData.length == 0) { | ||
| // const errorMessage = t("HCM_FACILITY_USAGE_VALIDATION"); | ||
| // setErrorsType((prevErrors) => ({ | ||
| // ...prevErrors, | ||
| // [type]: errorMessage, | ||
| // })); | ||
| // setIsError(true); | ||
| // return; | ||
| // } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove commented-out code.
Instead of keeping commented-out code, consider:
- Removing it if it's no longer needed
- Moving it to a separate document if it might be needed later
- Creating a new Git branch if it's a work in progress
- // if (jsonData.length == 0) {
- // const errorMessage = t("HCM_FACILITY_USAGE_VALIDATION");
- // setErrorsType((prevErrors) => ({
- // ...prevErrors,
- // [type]: errorMessage,
- // }));
- // setIsError(true);
- // return;
- // }| if (type === "boundary") { | ||
| if (workbook?.SheetNames.filter(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")).length == 0) { | ||
| const errorMessage = t("HCM_INVALID_BOUNDARY_SHEET"); | ||
| setErrorsType((prevErrors) => ({ | ||
| ...prevErrors, | ||
| [type]: errorMessage, | ||
| })); | ||
| setIsError(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider simplifying the sheet validation.
The boundary sheet validation logic is correct, but it could be more concise.
-if (workbook?.SheetNames.filter(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")).length == 0) {
+const hasBoundarySheets = workbook?.SheetNames.some(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA"));
+if (!hasBoundarySheets) {📝 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.
| if (type === "boundary") { | |
| if (workbook?.SheetNames.filter(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")).length == 0) { | |
| const errorMessage = t("HCM_INVALID_BOUNDARY_SHEET"); | |
| setErrorsType((prevErrors) => ({ | |
| ...prevErrors, | |
| [type]: errorMessage, | |
| })); | |
| setIsError(true); | |
| return; | |
| } | |
| if (type === "boundary") { | |
| const hasBoundarySheet = workbook?.SheetNames.some(sheetName => sheetName === t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")); | |
| if (!hasBoundarySheet) { | |
| const errorMessage = t("HCM_INVALID_BOUNDARY_SHEET"); | |
| setErrorsType((prevErrors) => ({ | |
| ...prevErrors, | |
| [type]: errorMessage, | |
| })); | |
| setIsError(true); | |
| return; | |
| } |
| { | ||
| return ( | ||
| <div style={{ marginBottom: "1rem" }}> | ||
| <label>{t("CHOOSE_BOUNDARY_LEVEL")}</label> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate <label> elements with their corresponding inputs for accessibility
Ensure the <label> is associated with the dropdown by using the htmlFor attribute.
Apply this diff:
-<label>{t("CHOOSE_BOUNDARY_LEVEL")}</label>
+<label htmlFor="boundary-level-dropdown">{t("CHOOSE_BOUNDARY_LEVEL")}</label>And update the <Dropdown> component:
<Dropdown
id="boundary-level-dropdown"
...
/>🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
| t={t} | ||
| /> | ||
| <div style={{height:"1rem"}}></div> | ||
| <label>{t("CHOOSE_BOUNDARY")}</label> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate <label> elements with their corresponding inputs for accessibility
The label should be associated with the MultiSelectDropdown component.
Apply this diff:
-<label>{t("CHOOSE_BOUNDARY")}</label>
+<label htmlFor="boundary-multiselect">{t("CHOOSE_BOUNDARY")}</label>And update the MultiSelectDropdown:
<MultiSelectDropdown
id="boundary-multiselect"
...
/>🧰 Tools
🪛 Biome (1.9.4)
[error] 67-67: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
| return ( | ||
| <div key={column.name} style={{ marginBottom: "1rem" }}> | ||
| <div style={{display:"flex"}}> | ||
| <label>{column.description}</label> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate <label> elements with their corresponding inputs for accessibility
Ensure the label is linked to the input control.
Apply this diff:
-<label>{column.description}</label>
+<label htmlFor={`input-${column.name}`}>{column.description}</label>And update the input:
<TextInput
id={`input-${column.name}`}
...
/>🧰 Tools
🪛 Biome (1.9.4)
[error] 119-119: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
| return ( | ||
| <div key={column.name} style={{ marginBottom: "1rem" }}> | ||
| <div style={{display:"flex"}}> | ||
| <label>{column.description}</label> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate <label> elements with their corresponding inputs for accessibility
Link the label to the dropdown component for better accessibility.
Apply this diff:
-<label>{column.description}</label>
+<label htmlFor={`dropdown-${column.name}`}>{column.description}</label>And update the <Dropdown> component:
<Dropdown
id={`dropdown-${column.name}`}
...
/>🧰 Tools
🪛 Biome (1.9.4)
[error] 144-144: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
| updatedData.forEach((newData, rowIndex) => { | ||
| const cell = targetSheet.getCell(rowIndex + 2, boundaryCodeColumnIndex); | ||
| const statusCell = targetSheet.getCell(rowIndex + 2, statusColumnIndex); | ||
|
|
||
| if (sheetNameToUpdate === "HCM_ADMIN_CONSOLE_USER_LIST") { | ||
| // Update user-specific fields | ||
| const userRoleCell = targetSheet.getCell(rowIndex + 2, userRoleColumnIndex); | ||
| const employmentTypeCell = targetSheet.getCell(rowIndex + 2, employmentTypeColumnIndex); | ||
| const phoneNumberCell = targetSheet.getCell(rowIndex + 2, phoneNumberColumnIndex); | ||
| const userNameCell = targetSheet.getCell(rowIndex + 2, userNameColumnIndex); | ||
|
|
||
| // Update boundary code and status | ||
| cell.value = newData?.[t(schemas?.find((i) => i.description === "Boundary Code (Mandatory)")?.name)]; | ||
| statusCell.value = newData?.[t(schemas?.find((i) => i.description === "User Usage")?.name)]; | ||
|
|
||
| // Update user-specific fields | ||
| userRoleCell.value = newData?.[t(schemas?.find((i) => i.description === "User Role")?.name)]; | ||
| employmentTypeCell.value = newData?.[t(schemas?.find((i) => i.description === "Employement Type")?.name)]; | ||
| phoneNumberCell.value = Number(newData?.[t(schemas?.find((i) => i.description === "Phone Number")?.name)] || 0); | ||
| userNameCell.value = newData?.[t(schemas?.find((i) => i.description === "User Name")?.name)]; | ||
|
|
||
| } else if (sheetNameToUpdate === "HCM_ADMIN_CONSOLE_FACILITIES") { | ||
| // Keep existing facility update logic | ||
| const facilityTypeCell = targetSheet.getCell(rowIndex + 2, facilityTypeColumnIndex); | ||
| const facilityNameCell = targetSheet.getCell(rowIndex + 2, facilityNameColumnIndex); | ||
| const capacityCell = targetSheet.getCell(rowIndex + 2, capacityColumnIndex); | ||
| const facilityStatusCell = targetSheet.getCell(rowIndex + 2, facilityStatusColumnIndex); | ||
|
|
||
| cell.value = newData?.[t(schemas?.find((i) => i.description === "Boundary Code")?.name)]; | ||
| statusCell.value = newData?.[t(schemas?.find((i) => i.description === "Facility usage")?.name)]; | ||
| facilityTypeCell.value = newData?.[t(schemas?.find((i) => i.description === "Facility type")?.name)]; | ||
| facilityNameCell.value = newData?.[t(schemas?.find((i) => i.description === "Facility Name")?.name)]; | ||
| capacityCell.value = Number(newData?.[t(schemas?.find((i) => i.description === "Capacity")?.name)] || 0); | ||
| facilityStatusCell.value = newData?.[t(schemas?.find((i) => i.description === "Facility status")?.name)]; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Validate column indices before accessing them to prevent runtime errors
Ensure that all required column indices (like userRoleColumnIndex, employmentTypeColumnIndex, etc.) are properly initialized before accessing cells at those indices. Accessing a cell with a null index can lead to errors.
Proposed solution:
- Add validation checks for each necessary column index:
if (sheetNameToUpdate === "HCM_ADMIN_CONSOLE_USER_LIST") {
if (userRoleColumnIndex === null) {
console.error('Column "User Role" not found!');
return;
}
if (employmentTypeColumnIndex === null) {
console.error('Column "Employment Type" not found!');
return;
}
// Repeat for other columns...
}This will ensure that if a column is missing, the function will handle it gracefully rather than causing a runtime error.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
15-15:⚠️ Potential issueInvalid package version detected.
The specified version
0.2.38of@egovernments/digit-ui-health-cssdoes not exist in the npm registry. This will cause CSS loading failures.Let's verify the current status:
#!/bin/bash # Description: Check latest available versions of the package echo "Available versions:" curl -s "https://registry.npmjs.org/@egovernments/digit-ui-health-css" | jq -r '.versions | keys[]' | sort -V
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js(5 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js (1)
Pattern **/*.js: check
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js (3)
12-12: Remove unused import.The
DataTypeimport fromajvis not used anywhere in the code. Consider removing it to avoid unnecessary bundle size increase.-import { DataType } from "ajv/dist/compile/validate/dataType";
739-747: Remove commented-out code.Instead of keeping commented-out code, consider:
- Removing it if it's no longer needed
- Moving it to a separate document if it might be needed later
- Creating a new Git branch if it's a work in progress
- // if (jsonData.length == 0) { - // const errorMessage = t("HCM_FACILITY_USAGE_VALIDATION"); - // setErrorsType((prevErrors) => ({ - // ...prevErrors, - // [type]: errorMessage, - // })); - // setIsError(true); - // return; - // } - // if (jsonData.length == 0) { - // const errorMessage = t("HCM_EMPTY_SHEET"); - // setErrorsType((prevErrors) => ({ - // ...prevErrors, - // [type]: errorMessage, - // })); - // setIsError(true); - // return; - // }Also applies to: 784-792
712-721: 🧹 Nitpick (assertive)Simplify the boundary sheet validation logic.
The current validation logic is correct but could be more readable. Consider using a more straightforward approach:
-if (workbook?.SheetNames.filter(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")).length == 0) { +const hasOtherSheets = workbook?.SheetNames.some(sheetName => sheetName !== t("HCM_ADMIN_CONSOLE_BOUNDARY_DATA")); +if (!hasOtherSheets) { const errorMessage = t("HCM_INVALID_BOUNDARY_SHEET"); setErrorsType((prevErrors) => ({ ...prevErrors, [type]: errorMessage, })); setIsError(true); return; }Likely invalid or redundant comment.
| @@ -10,7 +10,7 @@ | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
| <!-- added below css for hcm-workbench module inclusion--> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].37/dist/index.css" /> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].38/dist/index.css" /> | |||
There was a problem hiding this comment.
Production CSS loading will fail due to invalid package version.
The specified version 0.2.38 of @egovernments/digit-ui-health-css does not exist in the npm registry. This is particularly critical as this is the production HTML file.
Recommendations:
- Use the latest published version
0.2.37 - Ensure both HTML files use the same version
- If version
0.2.38contains necessary changes, publish it to npm first
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />📝 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.
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].38/dist/index.css" /> | |
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].37/dist/index.css" /> |
| reject("HCM_FILE_UNAVAILABLE"); | ||
| } | ||
| }; | ||
|
|
||
| reader.readAsArrayBuffer(selectedFile); | ||
| }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unnecessary whitespace.
Remove the extra blank line to maintain consistent code formatting.
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores