Added warning toast if user click on search without filling, and adde… #1700
Conversation
…d dynamic column for pop inbox table
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant enhancements across several components within the micro-plan module. 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 (
|
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (3)
Line range hint 71-106: Enhance API integration robustness and maintainability.
Consider the following improvements for the data fetching implementation:
- Move API URLs to a configuration file to improve maintainability
- Add error handling and user feedback for failed API calls
- Show loading states during sequential data fetching
Consider creating an API configuration file:
// api-config.js
export const API_ENDPOINTS = {
CENSUS_SEARCH: '/census-service/_search',
EMPLOYEE_SEARCH: '/health-hrms/employees/_search'
};Then update the API calls to use these configurations and add error handling:
const { isLoading, data, error } = Digit.Hooks.useCustomAPIHook({
url: API_ENDPOINTS.CENSUS_SEARCH,
body: {
CensusSearchCriteria: {
tenantId,
source: microplanId,
areaCodes: [boundaryCode]
}
},
config: {
enabled: true,
select: (data) => data?.Census[0],
onError: (error) => {
Digit.Hooks.useToast.error("Failed to fetch census data");
}
}
});Line range hint 147-169: Optimize state management and effect hooks.
The current implementation has several areas for improvement:
- Multiple useEffect hooks could lead to unnecessary re-renders
- Missing cleanup functions in useEffect
- Potential race conditions in state updates
Consider consolidating the state management:
useEffect(() => {
const cleanup = { isMounted: true };
const updateStates = async () => {
if (!workflowData || !data) return;
// Update assignee name
if (employeeData?.user?.name && cleanup.isMounted) {
setAssigneeName(employeeData.user.name);
}
// Update available actions
const selectedState = workflowData.states?.find(
(state) => state.state === data?.status
);
const availableActions = selectedState?.actions?.filter((action) =>
action.roles.some((role) => userRoles.includes(role))
);
if (cleanup.isMounted) {
setAvailableActionsForUser(availableActions || []);
}
};
updateStates();
return () => {
cleanup.isMounted = false;
};
}, [workflowData, data, employeeData, userRoles]);Line range hint 182-277: Enhance accessibility and error handling.
The component needs improvements in accessibility and error handling:
- Add ARIA labels and roles
- Implement keyboard navigation
- Add error boundaries
Consider these improvements:
+import { ErrorBoundary } from '@egovernments/digit-ui-react-components';
+const VillageViewContent = () => {
// ... existing component code ...
return (
<div role="main" aria-label={t('HCM_MICROPLAN_VILLAGE_VIEW_LABEL')}>
<Button
+ aria-label={t('HCM_MICROPLAN_EDIT_POPULATION_BUTTON_LABEL')}
+ onKeyDown={(e) => e.key === 'Enter' && handleEditPopulationClick()}
className="custom-class"
icon="Edit"
label="Edit Confirmed Population"
onClick={handleEditPopulationClick}
/>
// ... rest of the JSX
</div>
);
};
const VillageView = () => (
+ <ErrorBoundary fallback={<div>Something went wrong</div>}>
+ <VillageViewContent />
+ </ErrorBoundary>
);health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Line range hint 516-524: Remove commented-out prop in Toast component.
The error prop is commented out but not used. Consider removing this commented code to maintain cleaner code.
{showToast && (
<Toast style={{ zIndex: 10001 }}
label={showToast.label}
type={showToast.key}
- // error={showToast.key === "error"}
transitionTime={showToast.transitionTime}
onClose={() => setShowToast(null)}
/>
)}📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (5 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (5 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Pattern **/*.js: check
health/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/components/PopInboxTable.js
[error] 990-990: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js
[error] 39-39: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
69-78: LGTM: Empty boundary validation implemented correctly.
The validation logic for empty boundary selection is well-implemented. It provides appropriate user feedback via toast message while preserving the original functionality when boundaries are selected.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
146-146:
Fix typo in translation key
There's a typographical error in the translation key HCM_MICROPLAN_EDIT_POPULATION_SEND_FOR_APPOVAL. It should be HCM_MICROPLAN_EDIT_POPULATION_SEND_FOR_APPROVAL.
Apply the following diff to correct the typo:
label={t(`HCM_MICROPLAN_EDIT_POPULATION_SEND_FOR_APPOVAL`)}
+ label={t(`HCM_MICROPLAN_EDIT_POPULATION_SEND_FOR_APPROVAL`)}Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (2)
1041-1041: Ensure proper handling of assignee data
In the INBOX_ASSIGNEE column, the selector uses props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"). Verify that employeeNameData is always defined and that row.assignee corresponds to a valid key. To avoid potential undefined values, consider providing a default mapping or handling cases where the assignee is not found.
1131-1131: Confirm the data source for DataTable
At line 1131, the data prop for the DataTable component is set to censusData. Ensure that this is the intended data source and that censusData contains the correct and updated data. If the data was previously coming from props.censusData, verify that all references are updated accordingly.
...micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
Show resolved
Hide resolved
...b/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js
Show resolved
Hide resolved
...b/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
Line range hint 1095-1120: Consider passing data through props for flexibility
The data prop in the DataTable component is currently set to the hardcoded censusData. For better flexibility and reusability of the PopInboxTable component, consider passing censusData through props.
Apply this diff:
- data={censusData}
+ data={props.censusData}Ensure that props.censusData is provided when using the PopInboxTable component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
Pattern **/*.js: check
🔇 Additional comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js (1)
23-960: Avoid hardcoding data within the component
The censusData array is still hardcoded within the component. As previously mentioned, this can lead to maintainability issues and unnecessarily increase the bundle size. Consider fetching this data from an API, context, or passing it as props to make the component more dynamic and easier to maintain.
| selector: (row, index) => props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"), | ||
| sortable: true, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistency in optional chaining usage
In the selector for the INBOX_ASSIGNEE column, optional chaining is used inconsistently. Consider updating the code to consistently use optional chaining for better readability and to prevent potential runtime errors.
Apply this diff:
- selector: (row, index) => props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"),
+ selector: (row) => props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"),This change also removes the unused index parameter.
📝 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.
| selector: (row, index) => props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"), | |
| sortable: true, | |
| }, | |
| selector: (row) => props?.employeeNameData?.[row?.assignee] || t("ES_COMMON_NA"), | |
| sortable: true, | |
| }, |
| selector: (row) => { | ||
| const fieldValue = row.additionalFields.find((f) => f.key === field.key)?.value || t("ES_COMMON_NA"); | ||
|
|
||
| // Render a button if editable is true, otherwise render the field value as text | ||
| return row.additionalFields.find((f) => f.key === field.key)?.editable ? ( | ||
| <div style={{ display: 'flex', alignItems: 'center' }}> | ||
| <span style={{ marginRight: '24px' }}>{fieldValue}</span> | ||
| {props.showEditColumn && <Button | ||
| onClick={() => { | ||
| setShowEditVillagePopup(row); | ||
| }} | ||
| variation="secondary" | ||
| icon={"Edit"} | ||
| size="medium" | ||
| />} | ||
| </div> | ||
| ) : ( | ||
| fieldValue | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize by storing the result of the find operation
The function row.additionalFields.find((f) => f.key === field.key) is called multiple times. To improve performance and readability, store its result in a variable to avoid redundant calls.
Apply this diff to refactor:
selector: (row) => {
+ const fieldData = row.additionalFields.find((f) => f.key === field.key);
+ const fieldValue = fieldData?.value || t("ES_COMMON_NA");
- const fieldValue = row.additionalFields.find((f) => f.key === field.key)?.value || t("ES_COMMON_NA");
- return row.additionalFields.find((f) => f.key === field.key)?.editable ? (
+ return fieldData?.editable ? (
<div style={{ display: 'flex', alignItems: 'center' }}>
<span style={{ marginRight: '24px' }}>{fieldValue}</span>
{props.showEditColumn && (
<Button
onClick={() => {
setShowEditVillagePopup(row);
}}
variation="secondary"
icon={"Edit"}
size="medium"
/>
)}
</div>
) : (
fieldValue
);
},📝 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.
| selector: (row) => { | |
| const fieldValue = row.additionalFields.find((f) => f.key === field.key)?.value || t("ES_COMMON_NA"); | |
| // Render a button if editable is true, otherwise render the field value as text | |
| return row.additionalFields.find((f) => f.key === field.key)?.editable ? ( | |
| <div style={{ display: 'flex', alignItems: 'center' }}> | |
| <span style={{ marginRight: '24px' }}>{fieldValue}</span> | |
| {props.showEditColumn && <Button | |
| onClick={() => { | |
| setShowEditVillagePopup(row); | |
| }} | |
| variation="secondary" | |
| icon={"Edit"} | |
| size="medium" | |
| />} | |
| </div> | |
| ) : ( | |
| fieldValue | |
| ); | |
| }, | |
| selector: (row) => { | |
| const fieldData = row.additionalFields.find((f) => f.key === field.key); | |
| const fieldValue = fieldData?.value || t("ES_COMMON_NA"); | |
| return fieldData?.editable ? ( | |
| <div style={{ display: 'flex', alignItems: 'center' }}> | |
| <span style={{ marginRight: '24px' }}>{fieldValue}</span> | |
| {props.showEditColumn && <Button | |
| onClick={() => { | |
| setShowEditVillagePopup(row); | |
| }} | |
| variation="secondary" | |
| icon={"Edit"} | |
| size="medium" | |
| />} | |
| </div> | |
| ) : ( | |
| fieldValue | |
| ); | |
| }, |
…d dynamic column for pop inbox table
Summary by CodeRabbit
Release Notes
New Features
PopInboxTablecomponent with dynamic rendering of table columns based on comprehensive demographic data.EditVillagePopulationPopUpfor better population data management with a unified state structure.VillageViewcomponent, allowing for flexible data representation.Bug Fixes
PopInboxcomponent, enhancing error handling.These updates improve user experience and data management across the application.