Console settings module updated with proper data #1748
Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several changes across multiple components within the Changes
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 (
|
jagankumar-egov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (7)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/FacilityComponent.js (3)
Line range hint
53-62: Optimize facility data lookup performanceThe current implementation uses a find operation inside a map, resulting in O(n²) complexity. Consider creating a lookup map for better performance.
const updatedProjectFacility = projectFacility?.ProjectFacilities.map(row => { - const facilityData = Facility?.Facilities?.find(facility => facility.id === row.facilityId); + const facilityMap = new Map(Facility?.Facilities?.map(f => [f.id, f]) || []); + const facilityData = facilityMap.get(row.facilityId); return { ...row, storageCapacity: facilityData?.storageCapacity || "NA", name: facilityData?.name || "NA", usage: facilityData?.usage || "NA", address: facilityData?.address || "NA", }; });
Line range hint
45-49: Enhance error handling for API callsThe component should handle API failures gracefully and show appropriate error states to users.
Consider adding error handling:
const { isLoadingFacilty, data: Facility } = Digit.Hooks.useCustomAPIHook(facilityRequestCriteria); +const { error } = Digit.Hooks.useCustomAPIHook(facilityRequestCriteria); + +if (error) { + return <Card> + <div className="error-message"> + {t("FACILITY_FETCH_ERROR")} + <Button onClick={() => window.location.reload()}>{t("RETRY")}</Button> + </div> + </Card>; +}
Line range hint
1-124: Consider splitting component responsibilitiesThe component currently handles multiple concerns: fetching project facilities, fetching facility details, and rendering. Consider:
- Extract data fetching logic into custom hooks
- Create separate components for data and presentation
- Implement proper error boundaries
Example structure:
// useFacilityData.js hook const useFacilityData = (projectId) => { // Project facility fetching logic // Facility details fetching logic return { facilities, loading, error }; }; // FacilityTable.js component const FacilityTable = ({ facilities }) => { // Pure rendering logic }; // FacilityComponent.js const FacilityComponent = ({ projectId }) => { const { facilities, loading, error } = useFacilityData(projectId); // Error and loading handling return <FacilityTable facilities={facilities} />; };health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js (4)
Line range hint
71-82: Show loading states for all API calls.Only the initial loading state is shown. Consider showing loading indicators for subsequent API calls (variant and product details) to improve user experience.
- if (isLoading) { + if (isLoading || VariantLoading) { return <Loader></Loader>; }🧰 Tools
🪛 Biome
[error] 85-85: 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)
Line range hint
156-170: Improve data type handling in table cells.The
getDetailFromProductVariantfunction converts all values to strings, which might not be ideal for formatting certain data types (e.g., dates, numbers, booleans).Consider implementing type-specific formatting:
const getDetailFromProductVariant = (row, key) => { const value = getValue(row, key); if (value === undefined) return "NA"; if (typeof value === 'boolean') return value ? t("YES") : t("NO"); if (typeof value === 'number') return value.toLocaleString(); return value.toString(); };🧰 Tools
🪛 Biome
[error] 85-85: 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)
Line range hint
171-199: Consider implementing pagination.The API supports pagination with offset/limit parameters, but the UI doesn't implement pagination controls. This could be an issue with large datasets.
Consider adding pagination controls using Digit-UI components:
const [pageSize, setPageSize] = useState(10); const [offset, setOffset] = useState(0); // Update API request params params: { tenantId: "mz", offset: offset, limit: pageSize, } // Add pagination controls to the UI <Pagination currentPage={offset / pageSize + 1} totalPages={Math.ceil(totalCount / pageSize)} onPageChange={(page) => setOffset((page - 1) * pageSize)} />🧰 Tools
🪛 Biome
[error] 85-85: 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)
Add error handling and loading states for chained API calls
The component makes three chained API calls using
Digit.Hooks.useCustomAPIHookbut only handles loading state for the first call and lacks error handling for all calls. Add error states to handle API failures gracefully:const { isLoading, isError, error, data: projectResource } = Digit.Hooks.useCustomAPIHook(requestCriteria); const { isLoading: variantLoading, isError: variantError, error: variantErr, data: variantDetails } = Digit.Hooks.useCustomAPIHook(productVariantRequest); const { isLoading: productLoading, isError: productError, error: productErr, data: product } = Digit.Hooks.useCustomAPIHook(productRequest); // In the render method if (isError) return <Card>Error loading resources: {error.message}</Card>; if (variantError) return <Card>Error loading variants: {variantErr.message}</Card>; if (productError) return <Card>Error loading products: {productErr.message}</Card>; if (isLoading || variantLoading || productLoading) return <Loader />;🔗 Analysis chain
Line range hint
46-54: Add error handling for API responses.The component makes multiple chained API calls but lacks error handling. Consider adding error states to handle failed API responses gracefully.
Example implementation:
const { isLoading, isError, error, data: product } = Digit.Hooks.useCustomAPIHook(productRequest); // In the render method if (isError) { return <Card>Error: {error.message}</Card>; }Also applies to: 89-108
Let me generate another verification script to check the API call patterns and error handling in the specific component under review.
This will help us understand how API calls are structured and if there's any existing error handling in the component before making the final assessment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if error handling patterns exist in other components rg -A 2 "isError|error\s*=" health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Length of output: 8932
Script:
#!/bin/bash # Check API calls and error handling in ProjectBeneficiaryComponent.js rg -A 5 "useEffect|Digit\.Hooks|\.post|\.get" health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.jsLength of output: 1622
🧰 Tools
🪛 Biome
[error] 85-85: 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)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (3)
health/micro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/workbench/package.jsonis excluded by!**/*.json
📒 Files selected for processing (6)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Module.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/FacilityComponent.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js(8 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ProjectStaffConfig.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Module.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/FacilityComponent.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ProjectStaffConfig.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js (1)
Pattern **/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Module.js (1)
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#741
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/Module.js:15-15
Timestamp: 2024-10-08T20:11:12.539Z
Learning: When adding a new module code like "campaignmanager" to `moduleCode`, ensure that it is properly handled in all relevant conditional checks and function parameters across the codebase.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js
[error] 85-85: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js
[error] 39-39: 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] 74-74: 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 (4)
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ProjectStaffConfig.js (1)
2-2: Verify role prefix change impact across the system.
The role prefix change from 'WBH_' to 'ACCESSCONTROL_ROLES_ROLES_' might affect role-based access control. While the implementation is sound, we should ensure this change is consistent with the system's role management.
✅ Verification successful
Based on the search results, I can now provide a final response:
Role prefix change is consistent with system-wide access control pattern
The change from 'WBH_' to 'ACCESSCONTROL_ROLES_ROLES_' aligns with the established role naming convention used across multiple modules in the system, including:
- HRMS module (
hrms/src/pages/EmployeeDetails.js,hrms/src/components/pageComponents/jurisdiction.js) - Campaign Manager module (
campaign-manager/src/pages/employee/*.js) - Other HCM modules
The prefix 'ACCESSCONTROL_ROLES_ROLES_' is the standard pattern used for role-based access control throughout the application, making this change a proper alignment with the system's conventions rather than a breaking change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining WBH_ role prefixes that might need updating
rg "WBH_" --type js
# Search for ACCESSCONTROL_ROLES_ROLES_ usage to verify consistency
rg "ACCESSCONTROL_ROLES_ROLES_" --type js
Length of output: 34382
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Module.js (2)
21-26: Verify store implementation compatibility with modulePrefix.
The addition of modulePrefix to the useStore hook parameters suggests changes in the store's module resolution logic. Ensure that the store implementation correctly handles this new parameter.
Let's check the store implementation:
#!/bin/bash
# Search for useStore implementation and usage
echo "Checking useStore implementation..."
ast-grep --pattern 'const useStore = ($params) => {
$$$
}'
echo -e "\nChecking other useStore usages..."
rg "useStore.*modulePrefix" -t js17-18: Verify the impact of moduleCode change across the codebase.
The change from "workbench-hcm" to "campaign-settings" in moduleCode is a breaking change that could affect module resolution and routing. Additionally, the extraction of "hcm" as a separate modulePrefix represents an architectural change in module identification.
Let's verify the usage of these module identifiers:
✅ Verification successful
The moduleCode change appears to be an intentional architectural shift
The change from "workbench-hcm" to "campaign-settings" appears to be an intentional architectural change that is isolated to this module:
- The old
workbench-hcmidentifier is only referenced in package.json files and publish scripts campaign-settingsis only used in this new Module.js implementation- Other modules in the codebase follow similar patterns of defining their own unique moduleCode arrays (e.g., Microplanning, DSS)
The change represents a proper module isolation rather than a breaking change, as the module identifier is scoped to this specific module's context and doesn't appear to be referenced by other modules for dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to both old and new module codes
echo "Checking references to workbench-hcm..."
rg "workbench-hcm" -t js
echo -e "\nChecking references to campaign-settings..."
rg "campaign-settings" -t js
echo -e "\nChecking for potential module resolution patterns..."
rg "moduleCode.*=.*\[.*\]" -t js
Length of output: 357
Script:
#!/bin/bash
# Let's try a more focused search approach
# Search for workbench-hcm references
echo "=== Searching for workbench-hcm references ==="
rg -l "workbench-hcm"
# Search for campaign-settings references
echo -e "\n=== Searching for campaign-settings references ==="
rg -l "campaign-settings"
# Search for moduleCode patterns in JS/TS files
echo -e "\n=== Searching for moduleCode patterns ==="
rg "moduleCode\s*=\s*\[" --type js --type ts -A 1
# Search for module registration patterns
echo -e "\n=== Searching for module registration patterns ==="
rg "Modules\.ComponentRegistryService\.register" --type js --type ts -A 2
Length of output: 2465
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js (1)
135-142: Verify navigation key usage across the codebase.
The changes to navigation keys and codes should be reflected in any components that reference them. Let's verify the usage of these navigation keys and codes.
Also applies to: 147-152
✅ Verification successful
Let me check the actual usage of these navigation codes to ensure they are used consistently.
Navigation keys and codes are consistently used across the codebase
The verification shows that:
- The navigation codes (HCM_*_MAPPED) are only used in two files within the workbench-hcm module
- In ViewProjectConfig.js, they are properly defined as card codes
- In ProjectStaffComponent.js, the code is correctly referenced for conditional logic
- Other files containing "card" references are unrelated to these navigation codes as they belong to different modules (microplan and campaign-manager)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to the navigation keys and codes
echo "Searching for navigation key references..."
rg -l "card[1-5]" --type js
echo "Searching for old/new navigation codes..."
rg -l "(HCM_.*_MAPPED)" --type js
Length of output: 891
Script:
#!/bin/bash
# Check the actual usage of HCM navigation codes in the files
echo "Content of ProjectStaffComponent.js:"
rg -A 5 "HCM_.*_MAPPED" health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js
echo -e "\nContent of ViewProjectConfig.js:"
rg -A 5 "HCM_.*_MAPPED" health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js
Length of output: 1209
...o-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ProjectStaffConfig.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/Module.js
Show resolved
Hide resolved
...ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/FacilityComponent.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/configs/ViewProjectConfig.js
Show resolved
Hide resolved
...ro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js
Outdated
Show resolved
Hide resolved
...eb/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js
Show resolved
Hide resolved
| @@ -36,7 +36,7 @@ const ProjectStaffComponent = (props) => { | |||
| limit: 10, | |||
| }, | |||
| config: { | |||
| enable: data?.horizontalNav?.configNavItems[0].code === "Project Resource" ? true : false, | |||
| enable: props.projectId && data?.horizontalNav?.configNavItems[0].code === "HCM_STAFF_MAPPED" ? true : false, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify API request configurations and add type checking.
- The boolean literals in conditional expressions can be simplified:
- enable: props.projectId && data?.horizontalNav?.configNavItems[0].code === "HCM_STAFF_MAPPED" ? true : false,
+ enable: !!(props.projectId && data?.horizontalNav?.configNavItems[0].code === "HCM_STAFF_MAPPED"),- enable: userIds?.length>0 ? true : false,
+ enable: !!(userIds?.length > 0),- Consider adding type checking for the arrays before accessing their elements:
enable: Array.isArray(data?.horizontalNav?.configNavItems) && data.horizontalNav.configNavItems[0]?.code === "HCM_STAFF_MAPPED"Also applies to: 72-75
🧰 Tools
🪛 Biome
[error] 39-39: 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)
...eb/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectStaffComponent.js
Show resolved
Hide resolved
| setShowResult(data?.Employees[0]); | ||
| } else { | ||
| setShowResult(null); | ||
| setShowToast({ label: "WBH_USER_NOT_FOUND", isError: true }); | ||
| setTimeout(() => setShowToast(null), 5000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and user feedback.
- Consider adding a loading state while the search is in progress
- Add proper error boundary to handle API failures gracefully
- Provide more specific error messages instead of generic "USER_NOT_FOUND"
Example implementation:
onSuccess: async (data) => {
+ setIsLoading(false);
if (data?.Employees && data?.Employees?.length > 0) {
setShowResult(data?.Employees[0]);
} else {
setShowResult(null);
- setShowToast({ label: "WBH_USER_NOT_FOUND", isError: true });
+ setShowToast({
+ label: `WBH_USER_NOT_FOUND_WITH_CODE_${userName}`,
+ isError: true
+ });
setTimeout(() => setShowToast(null), 5000);
}
+ },
+ onError: (error) => {
+ setIsLoading(false);
+ setShowToast({
+ label: error.message || "WBH_SEARCH_ERROR",
+ isError: true
+ });
}Committable suggestion skipped: line range outside the PR's diff.
…nch-hcm/src/components/ProjectBeneficiaryComponent.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update MapView.js * Revert "Update MapView.js" This reverts commit 483b53d. * fixed build issue * updated css verwsion * Fixed some of the module level issues in console settings module * Update health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* audit fixes * css version update * rebased changes * Incremented the package versions of console settings * Setup campaign flow data create issue fixed (#1749) * package version updated due to module caching * Console settings module updated with proper data (#1748) * Update MapView.js * Revert "Update MapView.js" This reverts commit 483b53d. * fixed build issue * updated css verwsion * Fixed some of the module level issues in console settings module * Update health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/src/components/ProjectBeneficiaryComponent.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update Localisation codes on boundary selection (#1750) * rebased changes1 --------- Co-authored-by: Jagankumar <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: ashish-egov <[email protected]>
Choose the appropriate template for your PR:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor