-
Notifications
You must be signed in to change notification settings - Fork 22
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
added hierarchy level for village and enable the edit button based on… #1671
Conversation
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to two primary components: 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 (
|
...micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
Outdated
Show resolved
Hide resolved
...micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
Show resolved
Hide resolved
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: 4
🧹 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (2)
Line range hint
171-176
: Remove commented code and unused state.The row selection logic has been simplified, but there's some cleanup needed:
- Remove the commented code block
- Remove the unused
isIntermediate
state and related logic since it's no longer being usedApply these changes:
- const [isIntermediate, setIsIntermediate] = useState(false); const handleRowSelect = (event) => { - // if(!event?.allSelected && event?.selectedCount >0){ - // setIsIntermediate(true); - // } props?.onRowSelect(event); }; const selectProps = { hideLabel: true, - isIntermediate: isIntermediate, mainClassName: "data-table-select-checkbox", };
Line range hint
1-249
: Consider component decomposition and type safety improvements.The component handles multiple responsibilities and could benefit from:
- Breaking down into smaller, focused components (e.g., separate components for timeline popup, edit popup)
- Adding TypeScript for better type safety and improved maintainability
- Using a custom hook for pagination logic
Consider creating:
useTablePagination
hook for pagination logicTimelinePopup
andEditVillagePopup
as separate components- TypeScript interfaces for props and data structures
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
Pattern
**/*.js
: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
Pattern
**/*.js
: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
[error] 73-73: 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] 115-115: 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)
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
109-109
:⚠️ Potential issueAdd safety checks for URL parameters.
While the URL construction is simplified, it's vulnerable to potential issues:
- Add null checks for
url?.microplanId
andurl?.campaignId
- Sanitize the
boundaryCode
parameter- Validate
window.contextPath
Consider applying this safer implementation:
- history.push(`/${window.contextPath}/employee/microplan/village-view?microplanId=${url?.microplanId}&boundaryCode=${row.boundaryCode}&campaignId=${url?.campaignId}`) + const contextPath = window.contextPath || ''; + const params = new URLSearchParams({ + microplanId: url?.microplanId || '', + boundaryCode: row.boundaryCode || '', + campaignId: url?.campaignId || '' + }); + history.push(`/${contextPath}/employee/microplan/village-view?${params.toString()}`);health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
259-259
: 🧹 Nitpick (assertive)Localize the button label for internationalization
The button label "Edit Confirmed Population" is hard-coded in English. To support internationalization, wrap the label with the
t()
function for translation.Apply this diff:
- label="Edit Confirmed Population" + label={t(`HCM_MICROPLAN_EDIT_CONFIRMED_POPULATION_LABEL`)}Ensure that the translation key
HCM_MICROPLAN_EDIT_CONFIRMED_POPULATION_LABEL
is added to your localization files.Likely invalid or redundant comment.
… role
Summary by CodeRabbit
New Features
PopInboxTable
for simplified URL construction.VillageView
component with dynamic management of campaign-related data and user interactions.Bug Fixes
PopInboxTable
.Documentation