-
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
RELEASE/HCMPRE-1923 : Microplanning 0.2 release #2134
base: master
Are you sure you want to change the base?
Conversation
* Revert "Bugfix/hcmpre 1583 (#2021)" This reverts commit 201a0ce. * BUGFIX/HCMPRE-1634 Revert ing"updated core version (#2020)" * BUGFIX/HCMPRE-1583 fixes --------- Co-authored-by: Swathi-eGov <[email protected]>
FEATURE/HCMPRE-1634: Fixed all the version updates
#2047) BUGFIX/HCMPRE-1505 : Updated Core Version To Fix Validation Issue on Profile Update
* changes to createUpdatePlan * Changes to UI, createUpdatePlan * Putting the Hierarchy near the name * Loc change * Resolved comments * Null check for Hierarchy level * Changes * Indirectly Mutating planObject * changes * changes --------- Co-authored-by: Abishek <[email protected]>
…2092) * FEATURE/HCMPRE-1787 : Untagging all the current users if boundary is changed during setup * FEATUTE/HCMPRE-1787 : Added different error cases
…ntegration and apply filter (#2091) * Adding filters * Working Filter Search * InboxFilter Changes * removing console.log * Css package update * changes * changes * changes * changes * changes * A lot of changes * changes * changes * changes * uniform payload structure
* Adding filters * Working Filter Search * InboxFilter Changes * removing console.log * Css package update * changes * changes * changes * changes * changes * A lot of changes * changes * changes * changes * uniform payload structure * Adding Facility Dropdown * code replacing name attribute * Loader changes * comments resolved * Loader change * Id to id
* for clickng on save and next * remove console
* updated plan inbox * Update createUpdatePlanProject.js * formatted createupdateplanproject
…full flow (#2111) handled the case where capacity is 0 and tested full flow
* PopInbox and facilityId search in planInbox * Localization * Chages to CSS and removing filter * Changes to facility filter and security Question * css package update * facilityId removed * Resolved comments, made payload to facilityId * component package update
#2122) * FEATURE/HCMPRE-1704 : Removing current validation from assumptions and making sure the flow works as expected( for mixed strat, regis and dist can be same) * FEATURE/HCMPRE-1704 : Review updates
Warning Rate limit exceeded@Swathi-eGov has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request encompasses multiple changes across various files in the health microplan UI project. The modifications primarily focus on updating CSS stylesheets, enhancing component functionality, improving state management, and refining user interface interactions. Key areas of change include updating stylesheet versions, modifying component logic in files like Changes
Sequence DiagramsequenceDiagram
participant User
participant UI
participant APIService
participant StateManager
User->>UI: Interact with Microplan Components
UI->>StateManager: Update Component State
StateManager->>APIService: Fetch/Update Configuration
APIService-->>StateManager: Return Configuration Data
StateManager-->>UI: Update UI with New State
UI->>User: Render Updated Interface
Possibly related PRs
Suggested reviewers
Poem
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 (
|
* useEffect updated * CountAssignToAll issue in planInbox
ae7e2b8
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: 19
🔭 Outside diff range comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (3)
Line range hint
71-306
: Consider breaking down the large useEffect hook into smaller, focused functions.The useEffect hook handling file validation is quite lengthy (200+ lines) and handles multiple responsibilities. Consider extracting the validation logic into separate utility functions for better maintainability and testability.
Example structure:
+ const validateUploadedFile = async (file, tenantId) => { + // Extract validation logic here + } + + const handleValidationSuccess = async (result, fileName) => { + // Extract success handling logic here + } + + const handleValidationError = async (error, processedFileStore) => { + // Extract error handling logic here + } useEffect(async () => { const fetchData = async () => { if (!errorsType[type] && uploadedFile?.length > 0 && !isSuccess) { - // Current implementation + try { + const validationResult = await validateUploadedFile(uploadedFile, tenantId); + if (validationResult.success) { + await handleValidationSuccess(validationResult, fileName); + } else { + await handleValidationError(validationResult.error, validationResult.processedFileStore); + } + } catch (error) { + console.error(error); + } } }; await fetchData(); }, [errorsType]);
Line range hint
308-366
: Enhance file upload security checks.The file upload validation could be strengthened by adding:
- File type validation
- File size limits
- Content-type verification
const onBulkUploadSubmit = async (file) => { try { if (file.length > 1) { setShowToast({ key: "error", label: t("HCM_ERROR_MORE_THAN_ONE_FILE") }); return; } + // Add file type validation + const allowedTypes = ['application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', 'application/vnd.ms-excel']; + if (!allowedTypes.includes(file[0]?.type)) { + setShowToast({ key: "error", label: t("INVALID_FILE_TYPE") }); + return; + } + + // Add file size validation (e.g., 5MB limit) + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB in bytes + if (file[0]?.size > MAX_FILE_SIZE) { + setShowToast({ key: "error", label: t("FILE_SIZE_EXCEEDED") }); + return; + } setFileName(file?.[0]?.name);
Line range hint
368-445
: Centralize error handling logic.The error handling is scattered across multiple functions. Consider creating a centralized error handling utility to maintain consistency in error messages and handling patterns.
// Create a utility for centralized error handling const handleError = (error, defaultMessage) => { if (error?.response?.data?.Errors?.[0]) { const { message, description } = error.response.data.Errors[0]; return description ? `${message}: ${description}` : message; } return defaultMessage; }; // Usage in your functions } catch (error) { const errorMessage = handleError(error, t("HCM_ERROR_FILE_UPLOAD_FAILED")); setShowToast({ key: "error", label: errorMessage }); }health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Line range hint
503-515
: Remove duplicate effect hook.This effect hook is identical to the one at lines 123-135. Having duplicate hooks watching the same dependency could lead to maintenance issues.
Consolidate the duplicate hooks into a single useEffect:
- useEffect(() => { - if (selectedFilter?.status === "VALIDATED") { - setActiveLink({ code: "", name: "" }); - setShowTab(false); - } else { - if (!showTab) { - setActiveLink({ - code: "ASSIGNED_TO_ME", - name: "ASSIGNED_TO_ME", - }); - setShowTab(true); - } - } - }, [selectedFilter?.status]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (7)
health/micro-ui/web/core/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/example/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/packages/css/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.json
is excluded by!**/*.json
health/micro-ui/web/microplan/package.json
is excluded by!**/*.json
health/micro-ui/web/package.json
is excluded by!**/*.json
health/micro-ui/web/workbench/package.json
is excluded by!**/*.json
📒 Files selected for processing (18)
health/micro-ui/web/micro-ui-internals/example/public/index.html
(1 hunks)health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsForm.js
(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js
(19 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
(21 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js
(14 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js
(1 hunks)health/micro-ui/web/public/index.html
(1 hunks)
👮 Files not reviewed due to content moderation or server errors (6)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsForm.js
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
🧰 Additional context used
📓 Path-based instructions (15)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/utils/utilities.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsForm.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.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/example/src/UICustomizations.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundarySelection.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Pattern **/*.js
: check
📓 Learnings (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1670
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessMgmtTableWrapper.js:156-160
Timestamp: 2024-11-12T10:40:11.591Z
Learning: In the `UserAccessMgmtTableWrapper` component, since the `ShowMoreWrapper` is rendered within the cell, additional checks or row-specific state management are not required.
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (1)
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-11-10T20:01:49.000Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1847
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js:308-314
Timestamp: 2024-11-18T04:35:51.535Z
Learning: In `PlanInbox.js`, the variable `planWithCensus?.StatusCount[selectedFilter]` is always defined in the given context, so null checks are unnecessary.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
[error] 111-111: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js
[error] 272-272: 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/example/src/UICustomizations.js
[error] 802-802: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 803-803: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 804-804: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 812-825: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 827-839: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 892-892: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 903-903: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 914-914: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 925-925: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 829-829: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
[error] 375-375: 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/microplan/src/hooks/services/createUpdatePlanProject.js
[error] 468-474: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 476-485: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 488-488: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 490-490: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 491-491: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 708-708: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 732-737: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 751-761: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 758-758: Avoid the delete operator which can impact performance.
(lint/performance/noDelete)
[error] 918-943: This case is falling through to the next case.
Add a break
or return
statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
[error] 74-74: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 77-77: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 73-73: This let declares a variable that is only assigned once.
'key' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 125-125: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
[error] 220-223: 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] 217-217: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🔇 Additional comments (13)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js (1)
413-413
: LGTM! Localization key updated.The message key change aligns with the standardized localization pattern used in the microplanning module.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js (1)
128-129
: LGTM! Good use of dynamic configuration.The addition of
additionalCustomization
andprojectType
properties enables flexible capacity display based on the project type.health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
585-589
: LGTM! Robust null check and fallback.The capacity display implementation properly handles undefined/null values and provides a translated fallback.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (4)
36-36
: LGTM! Improved state initialization.The change from
null
to a structured object with default values improves type safety and provides better state management.
174-179
: LGTM! Well-structured API query parameters.The search criteria is properly constructed with:
- Consistent null checks before adding filter properties
- Correct usage of spread operator for optional parameters
- Clear mapping of filter values to API parameters
Also applies to: 209-216
457-461
: LGTM! Well-implemented count update logic.The effect hook correctly updates the count states when new data is received.
865-865
: LGTM! Enhanced UI components.The changes improve the user experience by:
- Adding KPI visualization with proper props
- Implementing consistent styling for the no-results state based on filter status
Also applies to: 1020-1022
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
14-15
: Verify compatibility with updated CSS versions.The CSS dependencies have been updated to newer versions. Please ensure these versions are stable and compatible with the current implementation.
Run the following script to check if the new versions exist and are publicly available:
health/micro-ui/web/public/index.html (1)
11-11
: Verify the stability of the updated CSS package versions.The significant version jump from 0.2.6 to 0.2.37 for digit-ui-health-css warrants verification.
Let's verify the existence and stability of these versions:
Also applies to: 13-13
✅ Verification successful
✓ CSS package versions are valid and stable
Both
@egovernments/[email protected]
and@egovernments/[email protected]
are confirmed to exist in the NPM registry and have no deprecation notices.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the specified versions exist and are the latest stable versions # Check NPM registry for digit-ui-components-css echo "Checking @egovernments/digit-ui-components-css versions..." curl -s https://registry.npmjs.org/@egovernments/digit-ui-components-css | jq -r '.versions | keys[]' | grep "0.0.2-beta.44" || echo "Version 0.0.2-beta.44 not found" # Check NPM registry for digit-ui-health-css echo "Checking @egovernments/digit-ui-health-css versions..." curl -s https://registry.npmjs.org/@egovernments/digit-ui-health-css | jq -r '.versions | keys[]' | grep "0.2.37" || echo "Version 0.2.37 not found" # Check for any deprecation notices echo "Checking for deprecation notices..." curl -s https://registry.npmjs.org/@egovernments/digit-ui-health-css | jq -r '.versions["0.2.37"].deprecated // "No deprecation notice"' curl -s https://registry.npmjs.org/@egovernments/digit-ui-components-css | jq -r '.versions["0.0.2-beta.44"].deprecated // "No deprecation notice"'Length of output: 919
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (2)
263-266
: LGTM! Checkbox alignment fix.The positioning adjustments for checkboxes improve the visual alignment within the search component.
536-541
: LGTM! Well-structured dropdown spacing.The
gap-between-dropdowns
class follows flexbox best practices for consistent vertical spacing.health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (2)
1303-1307
: LGTM! Good handling of zero capacity case.The implementation correctly handles both zero and non-zero capacity values, with appropriate fallback to "NA".
800-866
:⚠️ Potential issueAddress security concern with hardcoded dummy file ID and improve code organization.
- The hardcoded dummy file ID should be removed and replaced with proper error handling.
- The download and action select handlers should be moved outside the switch case for better maintainability.
Apply this diff to improve the code:
case "ACTIONS": - // TODO : Replace dummy file id with real file id when API is ready - const dummyFile = "c22a7676-d5d7-49b6-bcdb-83e9519f58df" - const microplanFileId = row?.campaignDetails?.additionalDetails?.microplanFileId || dummyFile; + const microplanFileId = row?.campaignDetails?.additionalDetails?.microplanFileId; let options = []; if (row?.status == "DRAFT") { options = [{ code: "1", name: "MP_ACTIONS_EDIT_SETUP" }]; } else { options = [{ code: "1", name: "MP_ACTIONS_VIEW_SUMMARY" }]; }🧰 Tools
🪛 Biome (1.9.4)
[error] 802-802: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 803-803: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 804-804: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 812-825: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 827-839: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 829-829: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/FacilityMappingConfig.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...th/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js
Show resolved
Hide resolved
.../micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js
Show resolved
Hide resolved
.../micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js
Show resolved
Hide resolved
.../micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js
Show resolved
Hide resolved
470f718
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
♻️ Duplicate comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (2)
540-543
: 🛠️ Refactor suggestionImprove filter reset logic.
The current implementation only sets the status from activeFilter. Consider explicitly resetting all other filter values to null for better clarity and consistency.
- setSelectedFilter((prev)=>({ - status:Object.entries(activeFilter)?.[0]?.[0] - })); + setSelectedFilter({ + status: Object.entries(activeFilter)?.[0]?.[0], + onRoadCondition: null, + terrain: null, + securityQ1: null, + securityQ2: null, + facilityId: [] + });
378-378
: 🧹 Nitpick (assertive)Simplify boolean expression.
The boolean literal in the conditional expression can be simplified.
- enabled: selectedFilter?.status ? true : false, + enabled: Boolean(selectedFilter?.status),🧰 Tools
🪛 Biome (1.9.4)
[error] 378-378: 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
Plan: Pro
📒 Files selected for processing (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
(21 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Pattern **/*.js
: check
📓 Learnings (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1847
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js:308-314
Timestamp: 2024-11-18T04:35:51.535Z
Learning: In `PlanInbox.js`, the variable `planWithCensus?.StatusCount[selectedFilter]` is always defined in the given context, so null checks are unnecessary.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
[error] 378-378: 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/microplan/src/components/InboxFilterWrapper.js
[error] 74-74: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 77-77: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 73-73: This let declares a variable that is only assigned once.
'key' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 140-140: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
[error] 235-238: 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] 232-232: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🔇 Additional comments (11)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (5)
36-36
: LGTM! Improved state initialization.The change from null to an object with default values is a good practice that makes the code more predictable and reduces the need for null checks.
174-181
: LGTM! Enhanced filter capabilities.The addition of new filter conditions (onRoadCondition, terrain, security questions, and facilityId) improves the search functionality.
863-865
: LGTM! Enhanced user context display.The addition of hierarchyLevel to the user role display improves user context and understanding of their permissions.
814-820
: LGTM! Clear workflow state management.The button state logic clearly maps workflow states to actions, making the code more maintainable.
868-868
: Verify KPI component integration.The GenericKpiFromDSS component has been added with multiple props. Please ensure that all required props are being passed correctly.
✅ Verification successful
✓ KPI component integration verified successfully
All required props are being passed correctly to the GenericKpiFromDSS component, and the implementation is consistent with other usages in the codebase. The component properly handles these props for both data fetching and rendering.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check GenericKpiFromDSS component usage and props ast-grep --pattern 'GenericKpiFromDSS module="MICROPLAN" $$$'Length of output: 63
Script:
#!/bin/bash # Find component definition and usages echo "=== Component Definition ===" ast-grep --pattern 'const GenericKpiFromDSS = $_' echo -e "\n=== Component Imports ===" rg "import.*GenericKpiFromDSS" -l echo -e "\n=== All Component Usages ===" rg "<GenericKpiFromDSS" -A 2 echo -e "\n=== Prop Types or Type Definitions ===" fd "GenericKpiFromDSS" --type fLength of output: 9991
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (6)
9-10
: Seamless context integration
Good use ofuseMyContext()
here to retrieve global state. This approach helps maintain a clean and modular structure.
14-15
: Well-structured default state
InitializingfilterValues
with clear, descriptive keys promotes readability and helps prevent null or undefined errors.
18-29
: Effective default selection logic
The reduction logic buildingdefaultSelectedOptions
is thorough and clearly handles distinct cases forfacilityId
vs. others.
32-43
: Proper synchronization with props
TheuseEffect
watch onprops.defaultValue
andt
ensures local state remains in sync when defaults or translations change. Nicely done.
185-188
: Verify alignment with retrieved learnings
The retrieved learnings mention always using"name"
foroptionsKey
. However, here it's set to"code"
. Confirm if this is intentional. If the values are indeed prepared to match"code"
, consider documenting this deviation from the default guideline.
235-238
: Add a unique key for iterable child
When mapping overstate.securityQuestions
, each returned element requires a uniquekey
prop. This ensures proper identity tracking in React.return ( <LabelFieldPair + key={item.id} vertical style={{ paddingBottom: isLastElement ? "1rem" : "0" }} >
✅ Verification successful
Add a unique key for iterable child
The review comment is correct. TheLabelFieldPair
component is rendered withinstate.securityQuestions.map()
without a key prop. Addingkey={item.id}
is the appropriate fix as each security question has a unique id.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find the component implementation containing LabelFieldPair ast-grep --pattern 'return ( $$$ <LabelFieldPair $$$ > $$$ </LabelFieldPair> $$$ )' # Also search for any map operations returning LabelFieldPair ast-grep --pattern '$$.map($$ => { $$$ <LabelFieldPair $$$ > $$$ </LabelFieldPair> $$$ })'Length of output: 214
Script:
#!/bin/bash # First verify the file exists fd "InboxFilterWrapper.js" --type f # Then examine its content cat health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.jsLength of output: 9433
🧰 Tools
🪛 Biome (1.9.4)
[error] 235-238: 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)
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
7f49c38
PR for features in Microplanning 0.2 version
Summary:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style Updates
Performance