-
Notifications
You must be signed in to change notification settings - Fork 47
boundary tree implementation #3244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: console
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new boundary viewing page with lazy-loaded hierarchical navigation and an editable node form. Introduces ViewBoundaryV2, a recursive tree with on-demand child fetching, state synchronization, and an UpdateBoundaryForm popup for editing/deleting a node’s parent. Updates routing to include a new boundary/view path. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant ViewBoundaryV2
participant API as Boundary APIs
Note over User,API: Load and explore boundary tree
User->>Router: Navigate to /boundary/view?type=...&hierarchyType=...
Router->>ViewBoundaryV2: Mount component
ViewBoundaryV2->>API: fetchHierarchyDefinition(hierarchyType)
API-->>ViewBoundaryV2: hierarchyDefinition
ViewBoundaryV2->>API: fetchRootBoundary(type per hierarchy)
API-->>ViewBoundaryV2: rootNodes (hasChildren, children=[])
User->>ViewBoundaryV2: Expand node
ViewBoundaryV2->>API: fetchChildren(nextBoundaryType, parentId)
API-->>ViewBoundaryV2: children[]
ViewBoundaryV2-->>ViewBoundaryV2: Update node.children in tree state
sequenceDiagram
autonumber
actor User
participant ViewBoundaryV2
participant PopUp
participant UpdateBoundaryForm
participant API as Boundary/Localization APIs
Note over User,API: Update boundary node (parent/message)
User->>ViewBoundaryV2: Click "Update" on node
ViewBoundaryV2->>PopUp: Open with UpdateBoundaryForm(node, allBoundaries)
PopUp->>UpdateBoundaryForm: Initialize
UpdateBoundaryForm-->>UpdateBoundaryForm: Derive current & candidate parents
User->>UpdateBoundaryForm: Edit code/message and/or select parent
alt Save
UpdateBoundaryForm->>API: optional upsertLocalization if message changed
UpdateBoundaryForm->>API: optional updateBoundaryParent if parent changed
API-->>UpdateBoundaryForm: Responses
UpdateBoundaryForm-->>ViewBoundaryV2: onSave()
UpdateBoundaryForm->>PopUp: onClose()
else Delete
UpdateBoundaryForm->>User: Confirm deletion
User-->>UpdateBoundaryForm: Confirm
UpdateBoundaryForm->>API: updateBoundaryParent(remove parent)
API-->>UpdateBoundaryForm: Response
UpdateBoundaryForm-->>ViewBoundaryV2: onSave()
UpdateBoundaryForm->>PopUp: onClose()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js(1 hunks)health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js(1 hunks)health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/index.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
check
Files:
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/index.jshealth/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.jshealth/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
🧬 Code graph analysis (3)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/index.js (1)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js (1)
ViewBoundaryV2(106-340)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js (2)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js (3)
useTranslation(6-6)handleSave(140-212)UpdateBoundaryForm(5-334)health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/index.js (4)
useTranslation(150-150)ViewBoundaryV2(53-57)location(290-290)tenantId(161-161)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js (1)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js (3)
boundaryType(109-109)handleSave(298-303)hierarchyType(110-110)
🪛 Biome (2.1.2)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
[error] 42-56: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (3)
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/index.js (2)
53-57: LGTM! Lazy loading setup follows established patterns.The ViewBoundaryV2 component is correctly configured with lazy loading and fallback, consistent with other components in this file.
352-352: Route addition looks good.The new boundary/view route is properly integrated and follows the same pattern as existing routes.
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js (1)
214-225: Potential ambiguity in node matching logic.The
findNodeRecursivefunction matches nodes by eithercodeORid. If there's a collision where one node's code matches another node's id, this could return the wrong node.Verify that
codeandidare unique across all nodes and cannot collide. If they can collide, prioritize matching byidonly:const findNodeRecursive = (nodes, target) => { for (const node of nodes) { - if (node.code === target.code || node.id === target.id) { + if (node.id === target.id) { return node; } if (node.children?.length) { const found = findNodeRecursive(node.children, target); if (found) return found; } } return null; };
.../web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
Show resolved
Hide resolved
.../web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
Show resolved
Hide resolved
| // const handleSave = async () => { | ||
| // if (!selectedParent) { | ||
| // alert(t("HCM_BOUNDARY_SELECT_PARENT_ERROR")); | ||
| // return; | ||
| // } | ||
|
|
||
| // try { | ||
| // const localizationPayload = { | ||
| // tenantId: node.tenantId || "dev", | ||
| // messages: [ | ||
| // { | ||
| // code: code, | ||
| // message: msg, // You can customize this message | ||
| // module: `hcm-boundary-${(hierarchyType || "ADMIN").toLowerCase()}`, | ||
| // locale: "en_IN" | ||
| // } | ||
| // ] | ||
| // }; | ||
|
|
||
| // console.log("Calling Localization API with payload:", localizationPayload); | ||
|
|
||
| // const localizationResponse = await Digit.CustomService.getResponse({ | ||
| // url: "/localization/messages/v1/_upsert", | ||
| // body: localizationPayload, | ||
| // }); | ||
|
|
||
| // console.log("Localization API response:", localizationResponse); | ||
|
|
||
| // // Now update the boundary relationship with new parent | ||
| // const payload = { | ||
| // BoundaryRelationship: { | ||
| // tenantId: node.tenantId || "dev", | ||
| // code, | ||
| // boundaryType: node.boundaryType, | ||
| // hierarchyType: hierarchyType || "ADMIN", | ||
| // parent: selectedParent.code, | ||
| // }, | ||
| // }; | ||
|
|
||
| // console.log("Update payload:", payload); | ||
|
|
||
| // const response = await Digit.CustomService.getResponse({ | ||
| // url: "/boundary-service/boundary-relationships/_update", | ||
| // body: payload, | ||
| // }); | ||
|
|
||
| // console.log("Update response:", response); | ||
| // onSave({ ...node, code, parent: selectedParent.code }); | ||
| // onClose(); | ||
| // } catch (error) { | ||
| // console.error("Error updating boundary:", error); | ||
| // alert(t("HCM_BOUNDARY_UPDATE_ERROR") + ": " + error.message); | ||
| // } | ||
| // }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
This large block of commented code should be removed. If needed for reference, it can be retrieved from version control history.
Apply this diff to remove the commented code:
- // const handleSave = async () => {
- // if (!selectedParent) {
- // alert(t("HCM_BOUNDARY_SELECT_PARENT_ERROR"));
- // return;
- // }
-
- // try {
- // const localizationPayload = {
- // tenantId: node.tenantId || "dev",
- // messages: [
- // {
- // code: code,
- // message: msg, // You can customize this message
- // module: `hcm-boundary-${(hierarchyType || "ADMIN").toLowerCase()}`,
- // locale: "en_IN"
- // }
- // ]
- // };
-
- // console.log("Calling Localization API with payload:", localizationPayload);
-
- // const localizationResponse = await Digit.CustomService.getResponse({
- // url: "/localization/messages/v1/_upsert",
- // body: localizationPayload,
- // });
-
- // console.log("Localization API response:", localizationResponse);
-
- // // Now update the boundary relationship with new parent
- // const payload = {
- // BoundaryRelationship: {
- // tenantId: node.tenantId || "dev",
- // code,
- // boundaryType: node.boundaryType,
- // hierarchyType: hierarchyType || "ADMIN",
- // parent: selectedParent.code,
- // },
- // };
-
- // console.log("Update payload:", payload);
-
- // const response = await Digit.CustomService.getResponse({
- // url: "/boundary-service/boundary-relationships/_update",
- // body: payload,
- // });
-
- // console.log("Update response:", response);
- // onSave({ ...node, code, parent: selectedParent.code });
- // onClose();
- // } catch (error) {
- // console.error("Error updating boundary:", error);
- // alert(t("HCM_BOUNDARY_UPDATE_ERROR") + ": " + error.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.
| // const handleSave = async () => { | |
| // if (!selectedParent) { | |
| // alert(t("HCM_BOUNDARY_SELECT_PARENT_ERROR")); | |
| // return; | |
| // } | |
| // try { | |
| // const localizationPayload = { | |
| // tenantId: node.tenantId || "dev", | |
| // messages: [ | |
| // { | |
| // code: code, | |
| // message: msg, // You can customize this message | |
| // module: `hcm-boundary-${(hierarchyType || "ADMIN").toLowerCase()}`, | |
| // locale: "en_IN" | |
| // } | |
| // ] | |
| // }; | |
| // console.log("Calling Localization API with payload:", localizationPayload); | |
| // const localizationResponse = await Digit.CustomService.getResponse({ | |
| // url: "/localization/messages/v1/_upsert", | |
| // body: localizationPayload, | |
| // }); | |
| // console.log("Localization API response:", localizationResponse); | |
| // // Now update the boundary relationship with new parent | |
| // const payload = { | |
| // BoundaryRelationship: { | |
| // tenantId: node.tenantId || "dev", | |
| // code, | |
| // boundaryType: node.boundaryType, | |
| // hierarchyType: hierarchyType || "ADMIN", | |
| // parent: selectedParent.code, | |
| // }, | |
| // }; | |
| // console.log("Update payload:", payload); | |
| // const response = await Digit.CustomService.getResponse({ | |
| // url: "/boundary-service/boundary-relationships/_update", | |
| // body: payload, | |
| // }); | |
| // console.log("Update response:", response); | |
| // onSave({ ...node, code, parent: selectedParent.code }); | |
| // onClose(); | |
| // } catch (error) { | |
| // console.error("Error updating boundary:", error); | |
| // alert(t("HCM_BOUNDARY_UPDATE_ERROR") + ": " + error.message); | |
| // } | |
| // }; |
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
around lines 85 to 138, remove the large block of commented-out handleSave code
(the entire commented section from lines ~85–138) since it is dead code; delete
the commented lines cleanly so the file contains no commented implementation and
leave the current active logic intact, relying on VCS history if this code needs
to be recovered.
| if (selectedParent.code !== currentParent.code) { | ||
| const payload = { | ||
| BoundaryRelationship: { | ||
| tenantId: node.tenantId || "dev", | ||
| code, | ||
| boundaryType: node.boundaryType, | ||
| hierarchyType: hierarchyType || "ADMIN", | ||
| parent: selectedParent.code, | ||
| }, | ||
| }; | ||
|
|
||
| console.log("Calling Update API with payload:", payload); | ||
|
|
||
| updateResponse = await Digit.CustomService.getResponse({ | ||
| url: "/boundary-service/boundary-relationships/_update", | ||
| body: payload, | ||
| }); | ||
|
|
||
| console.log("Update API response:", updateResponse); | ||
| } else { | ||
| console.log("No change in parent, skipping Update API call"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference error.
Line 179 accesses currentParent.code without checking if currentParent is null. If the node is at root level (which is checked at line 78-82), currentParent will remain null, causing a runtime error.
Apply this diff to add a null check:
// ✅ Call boundary update API only if parent changed
- if (selectedParent.code !== currentParent.code) {
+ if (currentParent && selectedParent.code !== currentParent.code) {
const payload = {
BoundaryRelationship: {
tenantId: node.tenantId || "dev",
code,
boundaryType: node.boundaryType,
hierarchyType: hierarchyType || "ADMIN",
parent: selectedParent.code,
},
};
console.log("Calling Update API with payload:", payload);
updateResponse = await Digit.CustomService.getResponse({
url: "/boundary-service/boundary-relationships/_update",
body: payload,
});
console.log("Update API response:", updateResponse);
} else {
console.log("No change in parent, skipping Update API call");
}📝 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 (selectedParent.code !== currentParent.code) { | |
| const payload = { | |
| BoundaryRelationship: { | |
| tenantId: node.tenantId || "dev", | |
| code, | |
| boundaryType: node.boundaryType, | |
| hierarchyType: hierarchyType || "ADMIN", | |
| parent: selectedParent.code, | |
| }, | |
| }; | |
| console.log("Calling Update API with payload:", payload); | |
| updateResponse = await Digit.CustomService.getResponse({ | |
| url: "/boundary-service/boundary-relationships/_update", | |
| body: payload, | |
| }); | |
| console.log("Update API response:", updateResponse); | |
| } else { | |
| console.log("No change in parent, skipping Update API call"); | |
| } | |
| if (currentParent && selectedParent.code !== currentParent.code) { | |
| const payload = { | |
| BoundaryRelationship: { | |
| tenantId: node.tenantId || "dev", | |
| code, | |
| boundaryType: node.boundaryType, | |
| hierarchyType: hierarchyType || "ADMIN", | |
| parent: selectedParent.code, | |
| }, | |
| }; | |
| console.log("Calling Update API with payload:", payload); | |
| updateResponse = await Digit.CustomService.getResponse({ | |
| url: "/boundary-service/boundary-relationships/_update", | |
| body: payload, | |
| }); | |
| console.log("Update API response:", updateResponse); | |
| } else { | |
| console.log("No change in parent, skipping Update API call"); | |
| } |
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
around lines 179 to 200, the code dereferences currentParent.code without
ensuring currentParent is non-null which can throw when the node is root; change
the conditional to guard against null (for example, proceed with update when
currentParent is null or when selectedParent.code !== currentParent.code) and
ensure any other access to currentParent.code in this block is similarly
null-checked so you never read .code from a null object.
| } | ||
|
|
||
| // ✅ Trigger onSave only if something actually changed | ||
| if (msg !== t(node.code) || selectedParent.code !== node.parent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: node.parent property doesn't exist.
Line 203 references node.parent, but based on the data model and the logic in the useEffect (lines 18-83), the node object doesn't have a parent property. The comparison should be with currentParent.code.
Apply this diff to fix the logic:
// ✅ Trigger onSave only if something actually changed
- if (msg !== t(node.code) || selectedParent.code !== node.parent) {
+ if (msg !== t(node.code) || (currentParent && selectedParent.code !== currentParent.code)) {
onSave({ ...node, code, parent: selectedParent.code });
}📝 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 (msg !== t(node.code) || selectedParent.code !== node.parent) { | |
| if (msg !== t(node.code) || (currentParent && selectedParent.code !== currentParent.code)) { |
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
around line 203, the conditional uses node.parent which does not exist; change
the comparison to use currentParent.code instead (i.e., replace
selectedParent.code !== node.parent with selectedParent.code !==
currentParent.code) so the logic correctly compares selected parent against the
current parent code.
| setLoading(true); | ||
| try { | ||
| const childrenData = await onExpand(node); | ||
| console.log("Children fetched for", node.code, ":", childrenData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements.
Multiple console.log statements are present in the component. These should be removed before production or replaced with a proper logging framework.
Also applies to: 169-169, 182-182, 266-266
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
around lines 21, 169, 182 and 266, remove the debug console.log statements and
replace them with either no-op code or calls to the app’s logging utility (e.g.,
logger.debug/info) as appropriate; ensure you import/use the existing logging
framework if available, or wrap the logs with a conditional debug flag so
production builds do not emit console output, and run a quick lint/format pass
to clear any leftover console usage.
| <button | ||
| style={{ | ||
| background: "none", | ||
| border: "1px solid #ccc", | ||
| borderRadius: "4px", | ||
| width: "24px", | ||
| height: "24px", | ||
| cursor: "pointer", | ||
| display: "flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| }} | ||
| onClick={handleToggle} | ||
| disabled={loading} | ||
| > | ||
| {loading ? "..." : expanded ? "-" : "+"} | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add explicit type attribute to button element.
The button is missing an explicit type attribute. Without it, the default type is "submit", which can cause unintended form submissions if this component is placed inside a form.
Apply this diff:
{node.hasChildren && (
<button
+ type="button"
style={{
background: "none",
border: "1px solid #ccc",
borderRadius: "4px",
width: "24px",
height: "24px",
cursor: "pointer",
display: "flex",
alignItems: "center",
justifyContent: "center",
}}
onClick={handleToggle}
disabled={loading}
>
{loading ? "..." : expanded ? "-" : "+"}
</button>
)}📝 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.
| <button | |
| style={{ | |
| background: "none", | |
| border: "1px solid #ccc", | |
| borderRadius: "4px", | |
| width: "24px", | |
| height: "24px", | |
| cursor: "pointer", | |
| display: "flex", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| }} | |
| onClick={handleToggle} | |
| disabled={loading} | |
| > | |
| {loading ? "..." : expanded ? "-" : "+"} | |
| </button> | |
| <button | |
| type="button" | |
| style={{ | |
| background: "none", | |
| border: "1px solid #ccc", | |
| borderRadius: "4px", | |
| width: "24px", | |
| height: "24px", | |
| cursor: "pointer", | |
| display: "flex", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| }} | |
| onClick={handleToggle} | |
| disabled={loading} | |
| > | |
| {loading ? "..." : expanded ? "-" : "+"} | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 42-56: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
around lines 42 to 58, the <button> lacks an explicit type attribute so it
defaults to "submit" and may cause unintended form submissions; update the
element to include type="button" to ensure it does not submit enclosing forms,
keeping all existing props (style, onClick, disabled, children) unchanged.
| const [popupOpen, setPopupOpen] = useState(false); | ||
| const [selectedNode, setSelectedNode] = useState(null); | ||
|
|
||
| const tenantId = window.localStorage.getItem("Employee.tenant-id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add fallback for tenantId from localStorage.
If the localStorage key doesn't exist, tenantId will be null, which could cause API failures. Consider adding a fallback or early return with an error message.
Apply this diff:
- const tenantId = window.localStorage.getItem("Employee.tenant-id")
+ const tenantId = window.localStorage.getItem("Employee.tenant-id") || Digit?.ULBService?.getCurrentTenantId();Alternatively, add validation:
const tenantId = window.localStorage.getItem("Employee.tenant-id")
+
+ if (!tenantId) {
+ setError("Tenant ID not found");
+ setLoading(false);
+ 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.
| const tenantId = window.localStorage.getItem("Employee.tenant-id") | |
| const tenantId = window.localStorage.getItem("Employee.tenant-id") || Digit?.ULBService?.getCurrentTenantId(); |
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
around line 121, reading tenantId from localStorage may return null and break
downstream API calls; add a safe fallback and validation: retrieve the value,
trim/validate it, and if missing either set a sensible default (e.g., empty
string or configured default tenant id) or perform an early return with a clear
error/log message and avoid calling APIs with a null tenantId; update any
callers to handle the validated/guarded tenantId accordingly.
| if (parentNode.boundaryType === "COUNTRY") { | ||
| return boundaries.map((child) => ({ | ||
| ...child, | ||
| hasChildren: hasChildrenInHierarchy(child.boundaryType), | ||
| children: [], | ||
| })); | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify special handling for COUNTRY boundary type.
Line 246 has special logic for boundaryType === "COUNTRY". Ensure this assumption holds for all hierarchy configurations and that COUNTRY is always the root level when present.
🏁 Script executed:
#!/bin/bash
# Search for boundary type definitions and hierarchy configurations
rg -n -C3 'boundaryType.*COUNTRY|COUNTRY.*boundaryType' --type=jsLength of output: 1996
🏁 Script executed:
#!/bin/bash
# Search for hierarchy definitions and boundary type configurations
rg -n -C5 'hierarchyType|hierarchy.*config|boundaryHierarchy' --type=js --type=jsonLength of output: 269577
🏁 Script executed:
#!/bin/bash
# Search for boundary type constants or enums
rg -n -C3 'COUNTRY|STATE|DISTRICT|PROVINCE' --type=js | head -100Length of output: 9694
🏁 Script executed:
#!/bin/bash
# Look for the hasChildrenInHierarchy function definition
ast-grep --pattern 'function hasChildrenInHierarchy($$$) { $$$ }'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Alternative pattern for hasChildrenInHierarchy
rg -n -A10 'hasChildrenInHierarchy.*=|const hasChildrenInHierarchy' --type=jsLength of output: 1736
🏁 Script executed:
#!/bin/bash
# Check the actual boundary hierarchy structure being fetched and if COUNTRY is validated
rg -n -B5 -A10 'boundary-hierarchy-definition/_search' --type=js | head -150Length of output: 17761
🏁 Script executed:
#!/bin/bash
# Look for any validation or checks on COUNTRY being root or required
rg -n -B3 -A3 'parentBoundaryType.*null|null.*parentBoundaryType' --type=js | head -100Length of output: 12634
🏁 Script executed:
#!/bin/bash
# Check the ViewBoundaryV2 file more thoroughly for hierarchy validation
rg -n -B5 -A5 'boundaryHierarchy' health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js | head -100Length of output: 2342
Replace hardcoded "COUNTRY" check with dynamic root boundary type
ViewBoundaryV2 loads boundaryHierarchy from the API and uses the first entry as the root (boundaryHierarchy[0]?.boundaryType). The conditional checks parentNode.boundaryType !== "COUNTRY" (includeParents) and parentNode.boundaryType === "COUNTRY" (return children) at ~lines 238 and 246 assume "COUNTRY" is always the root — this is brittle and will break for hierarchies whose root is different or missing COUNTRY. Replace the hardcoded "COUNTRY" with a dynamic root check (e.g., compare against boundaryHierarchy[0]?.boundaryType or derive parentRoot by finding where parentBoundaryType == null) and update includeParents/child handling accordingly. File: health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js — lines ~238,246.
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
around lines 238 and 246, replace the hardcoded "COUNTRY" root check with a
dynamic root boundary type: compute rootBoundaryType (e.g., const
rootBoundaryType = boundaryHierarchy?.[0]?.boundaryType ||
boundaryHierarchy?.find(h => h.parentBoundaryType == null)?.boundaryType) and
then use rootBoundaryType in the conditional checks (includeParents and the
branch that currently returns children) so the code compares
parentNode.boundaryType to rootBoundaryType rather than the string "COUNTRY";
update both locations to derive rootBoundaryType once and use it for
hasChildren/includeParents logic and when returning mapped children.
| const handleSave = (updatedNode) => { | ||
| console.log("Updated Node:", updatedNode); | ||
| // Refresh the tree or update the specific node | ||
| // You might want to refetch the data here | ||
| handleClose(); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete implementation: Tree refresh not implemented.
The handleSave function contains a TODO comment indicating that tree refresh or node update logic is not implemented. After a successful save, the tree should be refreshed to reflect the changes.
Consider implementing one of these approaches:
- Refetch the entire tree:
const handleSave = async (updatedNode) => {
console.log("Updated Node:", updatedNode);
await fetchRootBoundary(); // Refetch from root
handleClose();
};- Update the specific node in state (more efficient but complex for parent changes)
Would you like me to help implement the tree refresh logic?
🤖 Prompt for AI Agents
In
health/micro-ui/web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/ViewBoundaryV2.js
around lines 298-303, handleSave is a stub that logs the updated node but does
not refresh the tree; change handleSave to perform the refresh after save by
making it async and either (A) call the existing fetchRootBoundary() (or the
function that loads the tree root) and await it before calling handleClose(), or
(B) if you prefer to avoid a full refetch, implement a helper to merge
updatedNode into the current tree state (traverse the tree, replace the matching
node by id, and setState) then call handleClose(); choose option A for
simplicity and reliability.
.../web/packages/modules/campaign-manager/src/pages/employee/boundaryTree/UpdateBoundaryForm.js
Show resolved
Hide resolved
| </div> | ||
| )} | ||
|
|
||
| <div style={{ display: "flex", justifyContent: "flex-end", gap: 10, marginTop: 20 }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove all these inline styles and add necessary tailwind css for it @piyushraj-egov
| const [popupOpen, setPopupOpen] = useState(false); | ||
| const [selectedNode, setSelectedNode] = useState(null); | ||
|
|
||
| const tenantId = window.localStorage.getItem("Employee.tenant-id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you using directly, use from url or hooks
|
|
||
| const boundaries = response?.TenantBoundary?.[0]?.boundary || []; | ||
|
|
||
| if (parentNode.boundaryType === "COUNTRY") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't have any checks like these in core products, as it will vary
Choose the appropriate template for your PR:
Feature/Bugfix Request
JIRA ID
Module
Description
Summary by CodeRabbit