Integrated search api and update api and created comment logs timelin… #1573
Conversation
📝 WalkthroughWalkthroughThe pull request includes updates to several HTML and JavaScript files within the micro-ui project. Key changes involve updating the stylesheet link for the Changes
Possibly related PRs
Suggested reviewers
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
🧹 Outside diff range comments (3)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint
14-22: Consider consolidating CSS importsThe HTML file includes multiple CSS imports from various versions of
@egovernments/digit-ui-cssand other related packages. This approach may lead to conflicts, increased load times, and maintenance difficulties.Consider the following recommendations:
- Consolidate CSS imports to use a single, consistent version where possible.
- Remove or comment out unused CSS imports to reduce load times and potential conflicts.
- Document the purpose of each CSS import, especially when multiple versions are necessary.
Example refactor:
<!-- Main DIGIT UI CSS --> <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.0.86-campaign/dist/index.css" /> <!-- Component-specific CSS (if needed) --> <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.39/dist/index.css"/> <!-- Module-specific CSS (if needed) --> <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.0.50-microplan/dist/index.css" /> <!-- Removed duplicate and potentially conflicting CSS imports -->health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (1)
Line range hint
78-125: LGTM: Improved boundary validation logicThe changes to the
recursiveParentFindfunction enhance readability and efficiency. The new implementation with a parent-children map is clearer and more maintainable.Consider using a more descriptive variable name instead of
iin theextraParentfilter:- const extraParent = missingParents?.filter((i) => i?.type !== state?.lowestHierarchy); + const extraParent = missingParents?.filter((item) => item?.type !== state?.lowestHierarchy);🧰 Tools
🪛 Biome
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint
92-113: Use thetfunction for translations inDropdowncomponents.In the
Dropdowncomponents, thetprop is currently set to an identity functiont={(label) => label}, meaning labels are not being translated. Replace it with the actual translation functiontto ensure proper localization.Apply this diff to fix the issue:
- t={(label) => label} // Replace with translation function + t={t}This change should be applied to all
Dropdowncomponents where the translation function is required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ 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 (10)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/villageView.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (8 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (13 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
[error] 63-63: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js
[error] 739-739: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 761-761: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 367-367: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 363-363: This let declares a variable that is only assigned once.
'path' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (10)
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/villageView.scss (1)
58-58: Spacing adjustment for.middle-childclassThe margin for the
.middle-childclass has been changed frommargin-top: 1rem;tomargin-bottom: 1.5rem;. This modification will affect the vertical spacing of elements using this class, potentially impacting the overall layout of the village view.Please ensure that this change aligns with the intended design:
- Verify that shifting the margin from top to bottom doesn't negatively impact the layout.
- Confirm that the increased margin (from 1rem to 1.5rem) provides the desired spacing between elements.
- Check the visual appearance of the village view to ensure it meets the updated design requirements.
If you need any assistance in verifying these changes or if further adjustments are needed, please let me know.
health/micro-ui/web/public/index.html (1)
17-17: 🧹 Nitpick (assertive)LGTM! CSS version updated for HCM Workbench module.
The update of the @egovernments/digit-ui-css package from version 1.0.85-campaign to 1.0.86-campaign looks good. This change appears to be related to the inclusion of the HCM Workbench module as mentioned in the comment.
To ensure this change doesn't introduce any unexpected styling issues, please verify the application's appearance after this update. Run the following script to check for any other files that might be affected by this CSS version change:
Consider updating the comment above the link to be more specific:
- <!-- added below css for hcm-workbench module inclusion--> + <!-- Updated CSS version to 1.0.86-campaign for HCM Workbench module inclusion -->This will provide more context for future developers about the nature of this change.
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
99-100: New route added for census serviceThe addition of the "/census-service" route to the proxy configuration is noted. This change allows requests to the census service to be proxied through the application.
To ensure this change is consistent with the application's architecture, please run the following verification:
This script will help identify any related changes or configurations for the census service across the codebase.
✅ Verification successful
Census Service Proxy Configuration Verified
The addition of the
"/census-service"route tosetupProxy.jshas been verified and aligns with its usage across the codebase. Requests to the census service are correctly proxied, ensuring seamless integration with existing components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the census service integration # Test 1: Check if there are any new files or changes related to the census service echo "Checking for census service related files:" fd -e js -e jsx -e ts -e tsx census # Test 2: Verify if there are any imports or uses of the census service in the codebase echo "Checking for census service usage:" rg --type js --type jsx --type ts --type tsx "census.*service" # Test 3: Check for any configuration related to the census service echo "Checking for census service configuration:" rg --type js --type json "census.*service"Length of output: 943
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (5)
35-36: LGTM: Improved code structure and flexibilityThe addition of the
getValueFromPathfunction enhances the flexibility of accessing nested object properties. This change improves the overall structure and readability of the code.
132-143: LGTM: Improved field validation logicThe refactored
areFieldsValidfunction is more concise and efficient. It correctly handles empty arrays and uses a functional approach with thesomemethod to check field validity. This implementation is easier to read and maintain.🧰 Tools
🪛 Biome
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
191-211: LGTM: Added validators for microplan assumptions and data uploadThe addition of
microplanAssumptionsValidatoranduploadDataValidatorfunctions, along with their integration into the validator switch statement, enhances the overall validation process. These changes ensure more comprehensive form validation across different scenarios.
220-223: LGTM: Improved current month retrievalThe modification to the
getCurrentMonthfunction using an array of month names simplifies the process and improves code readability. This approach is more maintainable and less error-prone than using numeric month values.
Line range hint
1-487: Overall: Significant improvements to utility functionsThe changes in this file have greatly enhanced its functionality and maintainability. Key improvements include:
- Refactored validation functions with improved logic and error handling.
- Enhanced boundary data processing with the introduction of materialized paths.
- New utility functions like
epochToDateTimefor better date-time handling.- Improved code structure and readability across multiple functions.
These changes provide better support for the microplan module and should improve the overall performance and maintainability of the codebase.
🧰 Tools
🪛 Biome
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
31-43:⚠️ Potential issueEnsure the
Censusdata is correctly formatted for the APIThe
updatedCensusobject represents a single census entry, but the API endpoint/census-service/_updatemay expect theCensusdata to be an array of census objects. The comment mentions "Sending back the modified Census array," but currently, an object is being sent. Verify that the API expects an array and adjust accordingly.If the API expects an array, wrap
updatedCensusin an array:body: { - Census: updatedCensus, // Sending back the modified Census array + Census: [updatedCensus], // Sending back the modified Census array },Please check the API documentation to confirm the expected request format.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint
105-109: Verify the options provided to the transportation modeDropdown.The options for the transportation mode
Dropdownare hardcoded to only includeOTHER. Is this intentional? If there are additional transportation modes, consider updating the options to reflect all possible choices.
| @@ -19,7 +19,7 @@ | |||
|
|
|||
|
|
|||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.39/dist/index.css"/> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.0.85-campaign/dist/index.css" /> | |||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.0.86-campaign/dist/index.css" /> | |||
There was a problem hiding this comment.
💡 Codebase verification
CSS Update Results in Unreachable Stylesheet
- The updated stylesheet link
https://unpkg.com/@egovernments/digit-ui-css@1.0.86-campaign/dist/index.cssresponds with a 404 error, indicating the CSS file is not accessible. - This issue may lead to missing styles and adversely affect the application's user interface.
Please verify the correct URL for the CSS file or consider reverting the version update to ensure stylesheet accessibility.
🔗 Analysis chain
CSS package version updated
The stylesheet link for @egovernments/digit-ui-css has been updated from version 1.0.85-campaign to 1.0.86-campaign. This change ensures that the application uses the latest version of the specified CSS package.
To ensure this update doesn't introduce any breaking changes or unintended side effects, please run the following verification script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any visual regressions or console errors after updating the CSS version
# Test: Check if the new CSS file is accessible
curl -I https://unpkg.com/@egovernments/digit-ui-css@1.0.86-campaign/dist/index.css
# Test: Search for any TODOs or FIXMEs related to CSS updates
rg -i "todo|fixme" --type html --type css --type js
Length of output: 94644
| "/service-request", | ||
| "/census-service" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Trailing comma added after "/service-request"
The addition of a trailing comma after "/service-request" is a good practice. It improves code maintainability by making future additions to the list easier and resulting in cleaner diffs.
Consider adding trailing commas to other elements in the array for consistency. This can be done in a separate cleanup task if desired.
|
|
||
|
|
||
| const assumptionsFormValidator = (formData) => { | ||
|
|
||
| let requiredFields = [] | ||
| if(formData?.selectedRegistrationProcess && formData?.selectedDistributionProcess){ | ||
| requiredFields = [ | ||
| formData?.selectedRegistrationProcess, | ||
| formData?.selectedDistributionProcess, | ||
| ] | ||
| if (!areFieldsValid(requiredFields)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
| }else if(formData?.selectedRegistrationDistributionMode){ | ||
| requiredFields = [ | ||
| formData?.selectedRegistrationDistributionMode, | ||
| ] | ||
| if (!areFieldsValid(requiredFields)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
| } | ||
| let requiredFields = [] | ||
| if (formData?.selectedRegistrationProcess && formData?.selectedDistributionProcess) { | ||
| requiredFields = [ | ||
| formData?.selectedRegistrationProcess, | ||
| formData?.selectedDistributionProcess, | ||
| ] | ||
| if (!areFieldsValid(requiredFields)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
| } else if (formData?.selectedRegistrationDistributionMode) { | ||
| requiredFields = [ | ||
| formData?.selectedRegistrationDistributionMode, | ||
| ] | ||
| if (!areFieldsValid(requiredFields)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
| } | ||
|
|
||
| const processesAreValid = formData?.selectedRegistrationProcess?.code && formData?.selectedDistributionProcess?.code; | ||
| if (processesAreValid && (formData?.selectedRegistrationProcess.code === formData?.selectedDistributionProcess.code)) { | ||
| return { key: "error", label: "ERROR_REGISTRATION_AND_DISTRIBUTION_ARE_SAME" }; // Customize as needed | ||
| return { key: "error", label: "ERROR_REGISTRATION_AND_DISTRIBUTION_ARE_SAME" }; // Customize as needed | ||
| } | ||
|
|
||
| if (!areFieldsValid(requiredFields)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
|
|
||
| return null; // Return null if all validations pass | ||
| }; | ||
|
|
||
|
|
||
|
|
||
|
|
||
| const microplanAssumptionsValidator = (formData)=>{ | ||
|
|
||
| if(!areFieldsValid(formData.assumptionValues)){ | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
| const microplanAssumptionsValidator = (formData) => { | ||
|
|
||
| return null | ||
| if (!areFieldsValid(formData.assumptionValues)) { | ||
| return { key: "error", label: "ERROR_MANDATORY_FIELDS" }; | ||
| } | ||
|
|
||
| return null |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: Enhanced assumptions form validation
The assumptionsFormValidator function now provides more comprehensive validation for different scenarios, including checks for registration and distribution processes. This improvement enhances the overall robustness of the form validation.
Consider extracting the error messages into constants for better maintainability:
const ERROR_MANDATORY_FIELDS = "ERROR_MANDATORY_FIELDS";
const ERROR_REGISTRATION_AND_DISTRIBUTION_ARE_SAME = "ERROR_REGISTRATION_AND_DISTRIBUTION_ARE_SAME";
// Then use these constants in the return statements
return { key: "error", label: ERROR_MANDATORY_FIELDS };
return { key: "error", label: ERROR_REGISTRATION_AND_DISTRIBUTION_ARE_SAME };| function epochToDateTime(epoch) { | ||
| // Create a new Date object using the epoch time | ||
| const date = new Date(epoch); | ||
| // Extract the date components | ||
| const year = date.getFullYear(); | ||
| const month = String(date.getMonth() + 1).padStart(2, "0"); // Months are 0-based, so add 1 | ||
| const day = String(date.getDate()).padStart(2, "0"); | ||
|
|
||
| // Extract the time components | ||
| let hours = date.getHours(); | ||
| const minutes = String(date.getMinutes()).padStart(2, "0"); | ||
| const seconds = String(date.getSeconds()).padStart(2, "0"); | ||
|
|
||
| // Determine AM/PM and convert to 12-hour format | ||
| const ampm = hours >= 12 ? "PM" : "AM"; | ||
| hours = hours % 12; | ||
| hours = hours ? hours : 12; // the hour '0' should be '12' | ||
| const formattedHours = String(hours).padStart(2, "0"); | ||
|
|
||
| // Format the date and time | ||
| const formattedDate = `${day}/${month}/${year}`; | ||
| const formattedTime = `${formattedHours}:${minutes}:${seconds} ${ampm}`; | ||
|
|
||
| // Return the formatted date and time | ||
| return `${formattedDate} ${formattedTime}`; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: Added epoch to date-time conversion utility
The new epochToDateTime function is a valuable addition, providing a clean way to convert epoch time to a formatted date and time string. It handles both date and time components, including AM/PM formatting, which enhances the utility's versatility.
Consider using template literals for string formatting to improve readability:
- const formattedDate = `${day}/${month}/${year}`;
- const formattedTime = `${formattedHours}:${minutes}:${seconds} ${ampm}`;
- return `${formattedDate} ${formattedTime}`;
+ const formattedDate = `${day}/${month}/${year}`;
+ const formattedTime = `${formattedHours}:${minutes}:${seconds} ${ampm}`;
+ return `${formattedDate} ${formattedTime}`;Committable suggestion was skipped due to low confidence.
| let path = []; | ||
| let currentBoundary = boundary; | ||
|
|
||
| // Build the path from the boundary's parent up to the root | ||
| while (currentBoundary && currentBoundary.parent) { | ||
| currentBoundary = boundaryMap[currentBoundary.parent]; | ||
| if (currentBoundary) { | ||
| path.push(currentBoundary.code); | ||
| } | ||
| } | ||
|
|
||
| // Join the reversed path into a single string | ||
| return path.reverse().join('.'); | ||
| // Join the reversed path into a single string | ||
| return path.reverse().join('.'); | ||
| } | ||
|
|
||
| // First, create a map for easy access to boundaries by code | ||
| const boundaryMap = boundaryData.reduce((map, boundary) => { | ||
| map[boundary.code] = boundary; | ||
| return map; | ||
| map[boundary.code] = boundary; | ||
| return map; | ||
| }, {}); | ||
|
|
||
| // Iterate over all boundaries | ||
| boundaryData.forEach(boundary => { | ||
| const { type, code } = boundary; | ||
| const { type, code } = boundary; | ||
|
|
||
| // Initialize type if not already in the hierarchy | ||
| if (!hierarchy[type]) { | ||
| hierarchy[type] = {}; | ||
| } | ||
| // Initialize type if not already in the hierarchy | ||
| if (!hierarchy[type]) { | ||
| hierarchy[type] = {}; | ||
| } | ||
|
|
||
| // Build the reversed materialized path for this boundary | ||
| const materializedPath = buildMaterializedPath(boundary, boundaryMap); | ||
| // Build the reversed materialized path for this boundary | ||
| const materializedPath = buildMaterializedPath(boundary, boundaryMap); | ||
|
|
||
| // Assign the materialized path to the boundary in the hierarchy | ||
| hierarchy[type][code] = materializedPath; | ||
| // Assign the materialized path to the boundary in the hierarchy | ||
| hierarchy[type][code] = materializedPath; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: Enhanced boundary data hierarchy creation
The refactored createBoundaryDataByHierarchy function now builds a materialized path for each boundary, which can significantly improve performance for hierarchical queries. This approach provides a more structured and efficient way to represent boundary hierarchies.
Consider using optional chaining to simplify the null checks:
- while (currentBoundary && currentBoundary.parent) {
+ while (currentBoundary?.parent) {📝 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.
| let path = []; | |
| let currentBoundary = boundary; | |
| // Build the path from the boundary's parent up to the root | |
| while (currentBoundary && currentBoundary.parent) { | |
| currentBoundary = boundaryMap[currentBoundary.parent]; | |
| if (currentBoundary) { | |
| path.push(currentBoundary.code); | |
| } | |
| } | |
| // Join the reversed path into a single string | |
| return path.reverse().join('.'); | |
| // Join the reversed path into a single string | |
| return path.reverse().join('.'); | |
| } | |
| // First, create a map for easy access to boundaries by code | |
| const boundaryMap = boundaryData.reduce((map, boundary) => { | |
| map[boundary.code] = boundary; | |
| return map; | |
| map[boundary.code] = boundary; | |
| return map; | |
| }, {}); | |
| // Iterate over all boundaries | |
| boundaryData.forEach(boundary => { | |
| const { type, code } = boundary; | |
| const { type, code } = boundary; | |
| // Initialize type if not already in the hierarchy | |
| if (!hierarchy[type]) { | |
| hierarchy[type] = {}; | |
| } | |
| // Initialize type if not already in the hierarchy | |
| if (!hierarchy[type]) { | |
| hierarchy[type] = {}; | |
| } | |
| // Build the reversed materialized path for this boundary | |
| const materializedPath = buildMaterializedPath(boundary, boundaryMap); | |
| // Build the reversed materialized path for this boundary | |
| const materializedPath = buildMaterializedPath(boundary, boundaryMap); | |
| // Assign the materialized path to the boundary in the hierarchy | |
| hierarchy[type][code] = materializedPath; | |
| // Assign the materialized path to the boundary in the hierarchy | |
| hierarchy[type][code] = materializedPath; | |
| let path = []; | |
| let currentBoundary = boundary; | |
| // Build the path from the boundary's parent up to the root | |
| while (currentBoundary?.parent) { | |
| currentBoundary = boundaryMap[currentBoundary.parent]; | |
| if (currentBoundary) { | |
| path.push(currentBoundary.code); | |
| } | |
| } | |
| // Join the reversed path into a single string | |
| return path.reverse().join('.'); | |
| } | |
| // First, create a map for easy access to boundaries by code | |
| const boundaryMap = boundaryData.reduce((map, boundary) => { | |
| map[boundary.code] = boundary; | |
| return map; | |
| }, {}); | |
| // Iterate over all boundaries | |
| boundaryData.forEach(boundary => { | |
| const { type, code } = boundary; | |
| // Initialize type if not already in the hierarchy | |
| if (!hierarchy[type]) { | |
| hierarchy[type] = {}; | |
| } | |
| // Build the reversed materialized path for this boundary | |
| const materializedPath = buildMaterializedPath(boundary, boundaryMap); | |
| // Assign the materialized path to the boundary in the hierarchy | |
| hierarchy[type][code] = materializedPath; |
🧰 Tools
🪛 Biome
[error] 367-367: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 363-363: This let declares a variable that is only assigned once.
'path' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
|
|
||
| <div className="label-pair"> | ||
| <span className="label-heading">{t(`HCM_MICROPLAN_DISCRICT_LABEL`)}</span> | ||
| <span className="label-text">Dist1</span> | ||
| <span className="label-text">{data.boundaryCode}</span> |
There was a problem hiding this comment.
Add null checks when accessing properties of 'data' to prevent runtime errors.
When rendering data from the data object, ensure that properties exist to prevent potential undefined or null dereferences, which can cause runtime errors if the properties are missing. Consider using optional chaining.
For example:
<span className="label-text">{data?.boundaryCode}</span>Update other property accesses accordingly:
- Line 97:
{data?.boundaryCode} - Line 102:
{data?.boundaryCode} - Line 107:
{data?.boundaryCode} - Line 185:
{data?.additionalDetails?.targetPopulation} - Line 195:
{data?.additionalDetails?.confirmedTargetPopulation} - Line 200:
{data?.additionalDetails?.confirmedTotalPopulation} - Line 205:
{data?.assignee}(Ensureassigneeis defined and formatted correctly for display)
Also applies to: 102-102, 107-107, 185-185, 195-195, 200-200, 205-205
🛠️ Refactor suggestion
Refactor repeated 'label-pair' blocks into a reusable component.
The div elements with className "label-pair" are repeated multiple times with similar structure. Consider creating a reusable LabelPair component to reduce code duplication and improve maintainability.
Example:
const LabelPair = ({ headingKey, text }) => (
<div className="label-pair">
<span className="label-heading">{t(headingKey)}</span>
<span className="label-text">{text}</span>
</div>
);Usage:
<LabelPair headingKey="HCM_MICROPLAN_DISCRICT_LABEL" text={data?.boundaryCode} />Also applies to: 102-102, 107-107, 185-185, 190-190, 195-195, 200-200, 205-205
| {timelineSteps.map((step, index) => ( | ||
| <Timeline | ||
| key={index} | ||
| label={step.label} | ||
|
|
||
| subElements={step.subElements} | ||
| variant={step.variant} | ||
| showConnector={step.showConnector} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using unique keys when rendering lists
Using the index as the key in list rendering can lead to issues if the list items change order or if new items are added. If possible, use a unique identifier from the data (e.g., a unique ID from each process instance) as the key for the Timeline components to ensure optimal performance and correctness.
| const [timelineSteps, setTimelineSteps] = useState([]); | ||
|
|
||
| useEffect(() => { | ||
| if (hardCodeData && hardCodeData.ProcessInstances) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify condition with optional chaining
You can use optional chaining to simplify the condition in the if statement.
Apply this minor change:
-if (workflowData && workflowData.ProcessInstances) {
+if (workflowData?.ProcessInstances) {Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 739-739: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| return ( | ||
| <PopUp | ||
| onClose={onClose} | ||
| heading={t(heading)} | ||
| onOverlayClick={onClose} | ||
| children={[ | ||
| <TimelineMolecule key="timeline" initialVisibleCount={4}> | ||
| {timelineSteps.map((step, index) => ( | ||
| <Timeline | ||
| key={index} | ||
| label={step.label} | ||
|
|
||
| subElements={step.subElements} | ||
| variant={step.variant} | ||
| showConnector={step.showConnector} | ||
| /> | ||
| ))} | ||
| </TimelineMolecule> | ||
| ]} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Avoid passing children using a prop
Instead of passing the children via a prop to the PopUp component, nest the TimelineMolecule inside the PopUp component directly. This approach aligns with React best practices and enhances code readability.
Apply this diff to refactor the code:
return (
<PopUp
onClose={onClose}
heading={t(heading)}
onOverlayClick={onClose}
- children={[
- <TimelineMolecule key="timeline" initialVisibleCount={4}>
- {timelineSteps.map((step, index) => (
- <Timeline
- key={index}
- label={step.label}
- subElements={step.subElements}
- variant={step.variant}
- showConnector={step.showConnector}
- />
- ))}
- </TimelineMolecule>
- ]}
>
+ <TimelineMolecule key="timeline" initialVisibleCount={4}>
+ {timelineSteps.map((step, index) => (
+ <Timeline
+ key={index}
+ label={step.label}
+ subElements={step.subElements}
+ variant={step.variant}
+ showConnector={step.showConnector}
+ />
+ ))}
+ </TimelineMolecule>
</PopUp>
);📝 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.
| return ( | |
| <PopUp | |
| onClose={onClose} | |
| heading={t(heading)} | |
| onOverlayClick={onClose} | |
| children={[ | |
| <TimelineMolecule key="timeline" initialVisibleCount={4}> | |
| {timelineSteps.map((step, index) => ( | |
| <Timeline | |
| key={index} | |
| label={step.label} | |
| subElements={step.subElements} | |
| variant={step.variant} | |
| showConnector={step.showConnector} | |
| /> | |
| ))} | |
| </TimelineMolecule> | |
| ]} | |
| /> | |
| ); | |
| return ( | |
| <PopUp | |
| onClose={onClose} | |
| heading={t(heading)} | |
| onOverlayClick={onClose} | |
| > | |
| <TimelineMolecule key="timeline" initialVisibleCount={4}> | |
| {timelineSteps.map((step, index) => ( | |
| <Timeline | |
| key={index} | |
| label={step.label} | |
| subElements={step.subElements} | |
| variant={step.variant} | |
| showConnector={step.showConnector} | |
| /> | |
| ))} | |
| </TimelineMolecule> | |
| </PopUp> | |
| ); |
🧰 Tools
🪛 Biome
[error] 761-761: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
| useEffect(() => { | ||
| if (hardCodeData && hardCodeData.ProcessInstances) { | ||
|
|
||
|
|
||
| // Map API response to timeline steps | ||
| const steps = hardCodeData.ProcessInstances.map((instance, index) => ({ | ||
| label: instance.state.applicationStatus, | ||
| variant: 'completed', | ||
| subElements: [Digit.Utils.microplanv1.epochToDateTime(instance?.auditDetails?.lastModifiedTime), | ||
| instance?.assignes?.length > 0 ? instance.assignes[0]?.name : 'NA', | ||
| t(`${instance.comment}`) | ||
| ], | ||
| showConnector: true | ||
| })); | ||
| setTimelineSteps(steps); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Use fetched data instead of hardcoded data
The component fetches workflow data using useCustomAPIHook, but the fetched data (workflowData) is not used. Instead, the hardcoded data (hardCodeData) is utilized in the useEffect. To ensure the component displays up-to-date and accurate information, consider using workflowData instead of hardCodeData.
Apply this diff to fix the issue:
-useEffect(() => {
- if (hardCodeData && hardCodeData.ProcessInstances) {
+useEffect(() => {
+ if (workflowData && workflowData.ProcessInstances) {
// Map API response to timeline steps
- const steps = hardCodeData.ProcessInstances.map((instance, index) => ({
+ const steps = workflowData.ProcessInstances.map((instance, index) => ({Also, add workflowData to the dependency array of useEffect to update timelineSteps when new data is fetched:
-}, []);
+}, [workflowData]);📝 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.
| useEffect(() => { | |
| if (hardCodeData && hardCodeData.ProcessInstances) { | |
| // Map API response to timeline steps | |
| const steps = hardCodeData.ProcessInstances.map((instance, index) => ({ | |
| label: instance.state.applicationStatus, | |
| variant: 'completed', | |
| subElements: [Digit.Utils.microplanv1.epochToDateTime(instance?.auditDetails?.lastModifiedTime), | |
| instance?.assignes?.length > 0 ? instance.assignes[0]?.name : 'NA', | |
| t(`${instance.comment}`) | |
| ], | |
| showConnector: true | |
| })); | |
| setTimelineSteps(steps); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| if (workflowData && workflowData.ProcessInstances) { | |
| // Map API response to timeline steps | |
| const steps = workflowData.ProcessInstances.map((instance, index) => ({ | |
| label: instance.state.applicationStatus, | |
| variant: 'completed', | |
| subElements: [Digit.Utils.microplanv1.epochToDateTime(instance?.auditDetails?.lastModifiedTime), | |
| instance?.assignes?.length > 0 ? instance.assignes[0]?.name : 'NA', | |
| t(`${instance.comment}`) | |
| ], | |
| showConnector: true | |
| })); | |
| setTimelineSteps(steps); | |
| } | |
| }, [workflowData]); |
🧰 Tools
🪛 Biome
[error] 739-739: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
nipunarora-eGov
dismissed
coderabbitai[bot]’s stale review
aware of hardcoding, will integrate once we have data
…e component
Summary by CodeRabbit
New Features
TimelinePopUpWrappercomponent for displaying workflow timelines.VillageViewcomponent to fetch and display dynamic census data.Bug Fixes
Chores