Boundary Management ui Info pending changes #1789
Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications across three files related to the campaign manager's functionality. In 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: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useBoundaryHome.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/GeoPode.js(6 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useBoundaryHome.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/GeoPode.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
Pattern **/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/GeoPode.js
[error] 203-203: 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] 209-209: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 211-211: 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] 216-216: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 251-251: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 257-257: 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] 273-273: 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] 323-323: 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] 327-327: 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] 345-345: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 351-351: 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] 206-206: 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] 237-237: 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] 282-290: 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] 291-300: 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] 331-331: 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] 373-381: 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] 382-391: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js
[error] 373-373: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 375-375: 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] 380-380: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 392-392: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 505-505: 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)
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useBoundaryHome.js (2)
Line range hint 1-138: Implementation looks good!
The hook implementation is well-structured with proper error handling, clean data transformation, and efficient caching using React Query. The changes maintain the existing robust implementation while updating the configuration values.
95-101: Verify hierarchy name handling across components.
The hierarchy name handling logic looks correct, but let's verify that other components are properly passing and handling these hierarchy types.
Let's check for consistency in hierarchy name handling:
✅ Verification successful
Based on the search results, I can now generate the final response:
Hierarchy name handling is properly implemented across components
The codebase shows consistent handling of hierarchy types and names:
- Hierarchy types are properly passed through props and configurations
- No hardcoded test values are in use (commented out test values in useBoundaryHome.js)
- Proper fallback patterns are implemented (e.g.
hierarchyType || boundaryConfig?.hierarchy) - Consistent usage of hierarchy type for localization keys and boundary configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for hierarchy type usage patterns in the codebase
echo "Searching for hierarchy type assignments and usage..."
rg -A 2 "hierarchyType[^a-zA-Z]" --type js
echo "Searching for hierarchy name assignments..."
rg -A 2 "hierarchyName[^a-zA-Z]" --type js
# Search for potential hardcoded test values
echo "Checking for potential hardcoded test values..."
rg -A 2 "test008|DEFAULT-BOUNDARY-DEMO" --type js
Length of output: 95308
...cro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useBoundaryHome.js
Outdated
Show resolved
Hide resolved
| <div> | ||
| <div style={{ fontWeight: "600", fontSize: "1.2rem" }}> | ||
| {/* {item?.boundaryType} */} | ||
| {`${t((defaultHierarchyType + "_" + item?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}`} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals instead of string concatenation
String concatenation can be replaced with template literals for better readability and maintainability.
-{`${t((defaultHierarchyType + "_" + item?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}`}
+{t(`${defaultHierarchyType}_${item?.boundaryType}`.toUpperCase().replace(/\s+/g, "_"))}Also applies to: 216-216
🧰 Tools
🪛 Biome
[error] 209-209: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| // setShowLoader(true); | ||
| setShowToast({ label: "HIERARCHY_PLEASE_WAIT", isError: "info", transitionTime:100}); |
There was a problem hiding this comment.
Uncomment loader state management for consistent loading feedback
The loader state management is commented out, which could lead to inconsistent UI feedback during async operations.
-// setShowLoader(true);
+setShowLoader(true);📝 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.
| // setShowLoader(true); | |
| setShowToast({ label: "HIERARCHY_PLEASE_WAIT", isError: "info", transitionTime:100}); | |
| setShowLoader(true); | |
| setShowToast({ label: "HIERARCHY_PLEASE_WAIT", isError: "info", transitionTime:100}); |
...ro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/GeoPode.js
Outdated
Show resolved
Hide resolved
| // setShowLoader(false); | ||
| const errorMessage = error.message === "LEVELS_CANNOT_BE_EMPTY" | ||
| ? t("LEVELS_CANNOT_BE_EMPTY") | ||
| : error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED"); | ||
| ? t("LEVELS_CANNOT_BE_EMPTY") | ||
| : error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED"); | ||
|
|
||
| setShowToast({ label: errorMessage, isError: "error" }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling structure
The error handling could be simplified and made more consistent.
-// setShowLoader(false);
-const errorMessage = error.message === "LEVELS_CANNOT_BE_EMPTY"
- ? t("LEVELS_CANNOT_BE_EMPTY")
- : error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED");
+setShowLoader(false);
+const errorMessage = {
+ "LEVELS_CANNOT_BE_EMPTY": t("LEVELS_CANNOT_BE_EMPTY"),
+ "DEFAULT": error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED")
+}[error.message] || errorMessage.DEFAULT;📝 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.
| // setShowLoader(false); | |
| const errorMessage = error.message === "LEVELS_CANNOT_BE_EMPTY" | |
| ? t("LEVELS_CANNOT_BE_EMPTY") | |
| : error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED"); | |
| ? t("LEVELS_CANNOT_BE_EMPTY") | |
| : error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED"); | |
| setShowToast({ label: errorMessage, isError: "error" }); | |
| setShowLoader(false); | |
| const errorMessage = { | |
| "LEVELS_CANNOT_BE_EMPTY": t("LEVELS_CANNOT_BE_EMPTY"), | |
| "DEFAULT": error?.response?.data?.Errors?.[0]?.message || t("HIERARCHY_CREATION_FAILED") | |
| }[error.message] || errorMessage.DEFAULT; | |
| setShowToast({ label: errorMessage, isError: "error" }); |
...ro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/GeoPode.js
Outdated
Show resolved
Hide resolved
| @@ -367,14 +370,14 @@ const ViewHierarchy = () => { | |||
| <div> | |||
| <div style={{fontWeight:"600", fontSize:"1.2rem"}}> | |||
| {/* {t(hierItem?.boundaryType)} */} | |||
| {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toLowerCase().replace(/\s+/g, "_"))}`} | |||
| {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}`} | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals for better readability
Replace string concatenation with template literals for improved code readability and maintainability.
- {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}`}
+ {t(`${hierarchyType}_${hierItem?.boundaryType}`.toUpperCase().replace(/\s+/g, "_"))}
- {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}-geojson.json`}
+ {t(`${hierarchyType}_${hierItem?.boundaryType}`.toUpperCase().replace(/\s+/g, "_"))}-geojson.json`}Also applies to: 380-380, 391-393
🧰 Tools
🪛 Biome
[error] 373-373: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| @@ -367,14 +370,14 @@ const ViewHierarchy = () => { | |||
| <div> | |||
| <div style={{fontWeight:"600", fontSize:"1.2rem"}}> | |||
| {/* {t(hierItem?.boundaryType)} */} | |||
| {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toLowerCase().replace(/\s+/g, "_"))}`} | |||
| {`${t(( hierarchyType + "_" + hierItem?.boundaryType).toUpperCase().replace(/\s+/g, "_"))}`} | |||
| </div> | |||
| <div style={{height:"1rem"}}></div> | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use self-closing tags for empty elements
For consistency with JSX best practices, use self-closing tags for elements without children.
- <div style={{height:"1rem"}}></div>
+ <div style={{height:"1rem"}} />
- <div style={{height:"2rem"}}></div>
+ <div style={{height:"2rem"}} />Also applies to: 505-505
🧰 Tools
🪛 Biome
[error] 375-375: 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)
| else if ( resFile && resFile?.GeneratedResource?.[0]?.status === "inprogress"){ | ||
| setShowToast({label: "PLEASE_WAIT_AND_RETRY_AFTER_SOME_TIME", isError: "info" }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing the in-progress notification message
The notification for in-progress status is good, but could be more informative by including an estimated wait time or providing a way to check the status later.
- setShowToast({label: "PLEASE_WAIT_AND_RETRY_AFTER_SOME_TIME", isError: "info" });
+ setShowToast({
+ label: "PLEASE_WAIT_AND_RETRY_AFTER_SOME_TIME",
+ subLabel: "ESTIMATED_WAIT_TIME_1_MINUTE",
+ isError: "info"
+ });Committable suggestion skipped: line range outside the PR's diff.
| // console.error("Error while polling:", error); | ||
| retries++; | ||
| setTimeout(poll, pollInterval); |
There was a problem hiding this comment.
Reconsider removing error logging
While adding retry logic is good, completely removing error logging might make it harder to debug issues in production. Consider keeping the logs but with proper error details and logging levels.
- // console.error("Error while polling:", error);
+ console.error("Error during status polling - Attempt ${retries + 1}/${maxRetries}:", {
+ error: error.message,
+ requestId: id,
+ type: typeOfData
+ });Committable suggestion skipped: line range outside the PR's diff.
…gn-manager/src/hooks/useBoundaryHome.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…gn-manager/src/pages/employee/GeoPode.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…gn-manager/src/pages/employee/GeoPode.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
jagankumar-egov
changed the title
Choose the appropriate template for your PR:
Summary by CodeRabbit
New Features
GeoPodecomponent for improved user experience during data processing.ViewHierarchycomponent with toast notifications for in-progress downloads.Bug Fixes
createNewHierarchyfunction to prevent premature exits.Style