Feature/warnpop #2025
Feature/warnpop #2025
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications across multiple components in the campaign manager module. Key changes include the restructuring of the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range comments (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js (1)
Line range hint
5-47: Consider enhancing user feedback during confirmation.The component could benefit from additional user feedback mechanisms during the confirmation process.
Consider these improvements:
- Add loading state during confirmation:
-const FinalPopup = ({ showFinalPopUp, setShowFinalPopup, onConfirmClick})=> { +const FinalPopup = ({ showFinalPopUp, setShowFinalPopup, onConfirmClick})=> { + const [isProcessing, setIsProcessing] = useState(false); const { t } = useTranslation(); + + const handleConfirm = async () => { + setIsProcessing(true); + try { + await onConfirmClick(); + setShowFinalPopup(false); + } catch (error) { + // Handle error + } finally { + setIsProcessing(false); + } + }; return ( showFinalPopUp && ( <PopUp // ... other props ... disabled={isProcessing} footerChildren={[ <Button // ... cancel button props ... disabled={isProcessing} />, <Button // ... create button props ... - onClick={() => { - onConfirmClick(); - setShowFinalPopup(false); - }} + onClick={handleConfirm} + disabled={isProcessing} + loading={isProcessing} /> ]} /> ) ); };🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (4)
Line range hint
73-112: Enhance file validation securityThe file validation could be strengthened with additional security measures:
- File size validation
- Content-type verification
- Maximum row limit validation
const handleFileChange = async (event) => { const file = event.target.files[0]; if (file) { + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + if (file.size > MAX_FILE_SIZE) { + setShowToast({ label: t("FILE_TOO_LARGE"), isError: "error" }); + setDisableFile(true); + event.target.value = ""; + return; + } + + // Verify content-type + if (!['application/vnd.ms-excel', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'] + .includes(file.type)) { + setShowToast({ label: t("INVALID_FILE_TYPE"), isError: "error" }); + setDisableFile(true); + event.target.value = ""; + return; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 437-437: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
Line range hint
373-434: Improve polling mechanism robustnessThe polling implementation could be enhanced with:
- Exponential backoff
- Better timeout handling
- Cleanup on component unmount
const pollForStatusCompletion = async (id, typeOfData) => { const maxRetries = 20; let retries = 0; const baseDelay = baseTimeOut?.["HCM-ADMIN-CONSOLE"]?.baseTimeOut?.[0]?.baseTimeOut; const maxTime = baseTimeOut?.["HCM-ADMIN-CONSOLE"]?.baseTimeOut?.[0]?.maxTime; - const pollInterval = Math.max(baseDelay * noOfRows , maxTime); + const getBackoffDelay = (retry) => Math.min( + Math.max(baseDelay * Math.pow(1.5, retry) * noOfRows, maxTime), + 30000 // Max 30 seconds + ); return new Promise((resolve, reject) => { + let timeoutId; + let isUnmounted = false; + const poll = async () => { try { const searchResponse = await Digit.CustomService.getResponse({/*...*/}); if (isUnmounted) return; const status = searchResponse?.ResourceDetails?.[0]?.status; if (status === "completed") { resolve(true); } else if (status === "failed") { reject(new Error(errorCode)); } else { retries++; - setTimeout(poll, pollInterval); + timeoutId = setTimeout(poll, getBackoffDelay(retries)); } } catch (error) { retries++; - setTimeout(poll, pollInterval); + timeoutId = setTimeout(poll, getBackoffDelay(retries)); } }; poll().catch(reject); + + return () => { + isUnmounted = true; + if (timeoutId) clearTimeout(timeoutId); + }; }); };🧰 Tools
🪛 Biome (1.9.4)
[error] 437-437: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
Line range hint
526-531: Standardize error handling and user feedbackConsider implementing a consistent error handling strategy:
- Use InfoCard for validation errors
- Use Toast for async operation results
- Add error boundary for component-level errors
+ <ErrorBoundary fallback={<InfoCard variant="error" label={t("ERROR_BOUNDARY_MESSAGE")} />}> {uiValError && <InfoCard label="Info" text={uiErrorMsg} variant="error" style={{maxWidth:"200rem"}} /> } + </ErrorBoundary>🧰 Tools
🪛 Biome (1.9.4)
[error] 437-437: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
Line range hint
1-694: Consider architectural improvements for better maintainability
- Extract the file handling logic into a custom hook
- Separate the polling logic into a utility function
- Create a dedicated boundary service for API calls
This will improve code organization, reusability, and testing capabilities.
🧰 Tools
🪛 Biome (1.9.4)
[error] 437-437: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryHome.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryHome.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js
[error] 400-400: This let declares a variable that is only assigned once.
'roleTemp' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryHome.js
[error] 86-86: 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] 87-87: 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] 88-88: 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)
🔇 Additional comments (6)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (2)
30-31: LGTM: Well-structured state management additions
The new state variables are appropriately initialized and serve clear purposes:
serviceDefId: Stores the service definition IDupdateDisable: Controls form submission based on modification time
269-269: LGTM: Form disable functionality properly implemented
The isDisabled prop is correctly bound to the updateDisable state, implementing the requirement to prevent updates based on modification time.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js (1)
Line range hint 1-47: Verify component usage and integration.
Since this component handles critical hierarchy creation operations, let's verify its usage across the codebase.
✅ Verification successful
Let me gather more context about the component's integration by checking its usage in BoundaryRelationCreate.js:
Based on the verification results, I can now provide a final response:
FinalPopup component is properly integrated and used as intended.
The verification shows that:
- The component is used exclusively in the boundary hierarchy creation flow within BoundaryRelationCreate.js
- It's properly integrated with confirmation logic and state management
- The translations and UI patterns are consistent with the campaign manager module's design
- The component follows a standard confirmation pattern for critical operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of FinalPopup component and related hierarchy creation logic
# Find all files that import or use FinalPopup
rg -l "FinalPopup" --type js
# Check for any similar confirmation dialogs to ensure consistent UX
ast-grep --pattern 'PopUp type="alert"'
# Look for hierarchy creation related translations
rg "CREATE_BOUNDARY_HIERARCHY|YOU_WON'T_BE_ABLE_TO_UNDO_THIS_STEP_OF_CREATING_HIERARCHY" --type js
Length of output: 979
Script:
#!/bin/bash
# Check how FinalPopup is used in BoundaryRelationCreate
rg -A 10 "FinalPopup" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryRelationCreate.js
# Check for any related boundary creation logic
rg -l "boundary|hierarchy" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/**/*.js
# Look for any related test files
fd "FinalPopup.*test.js|BoundaryRelation.*test.js" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/
Length of output: 18370
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewHierarchy.js (1)
436-436: 🧹 Nitpick (assertive)
Consider improving readability of the translation chain
The translation chain can be made more readable by using template literals.
- <div className="hierarchy-boundary-heading">{t(`HIERARCHY`)} {t(hierarchyType)}</div>
+ <div className="hierarchy-boundary-heading">{t(`HIERARCHY_${hierarchyType}`)}</div>Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryHome.js (2)
1-1: LGTM: Import statement is correctly updated
The InfoCard import is properly added to the existing digit-ui-components imports.
52-52: LGTM: Type variable assignment is well-formatted
The type assignment is properly formatted and maintains the existing logic with config.type fallback.
...web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js
Outdated
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js
Show resolved
Hide resolved
.../web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/BoundaryHome.js
Show resolved
Hide resolved
...b/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js
Show resolved
Hide resolved
...b/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (1)
87-104:⚠️ Potential issueAdd error handling for API response
Similar to previous feedback, consider adding error handling to manage cases where the API response format is unexpected. This ensures robustness against malformed responses.
Apply this diff to include error handling:
select: (res) => { + if (!res?.Services || res.Services.length === 0) { + console.error('Invalid or empty service response'); + return res; + } if (res?.Services?.[0]?.auditDetails) { // Existing code } return res; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (1)
Pattern **/*.js: check
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (4)
30-32: State variables serviceDefId and updateDisable initialized appropriately
The initialization of serviceDefId and updateDisable is correct and will effectively manage the component's state.
93-97: Simplify date comparison logic
As previously suggested, the date comparison can be simplified using the toDateString() method for clarity and conciseness.
Apply this diff to simplify the date comparison:
- const isToday =
- modifiedDate.getDate() === today.getDate() &&
- modifiedDate.getMonth() === today.getMonth() &&
- modifiedDate.getFullYear() === today.getFullYear();
+ const isToday = modifiedDate.toDateString() === today.toDateString();268-268: Confirm that isDisabled prop effectively disables FormComposerV2
Verify that setting isDisabled={updateDisable} correctly disables the form submission when updateDisable is true, and that the FormComposerV2 component handles this prop as expected.
112-113: Verify handling of empty serviceResponseParam in useCustomAPIHook
Ensure that passing an empty object {} to Digit.Hooks.useCustomAPIHook does not trigger unintended API calls or cause errors. It might be more appropriate to conditionally invoke the hook only when serviceResponseParam is set.
Run the following script to confirm that no API calls are made when serviceResponseParam is empty:
✅ Verification successful
Based on the implementation of useCustomAPIHook that we can see, I can now generate the final response:
Passing empty object to useCustomAPIHook is safe and handled appropriately
The implementation shows that the hook safely handles empty parameters:
- It uses object destructuring with default values
config = {} - The API call is made through React Query which safely handles the parameters
- Empty objects for
paramsandbodyare valid and won't cause issues - The hook's implementation ensures proper caching and revalidation behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that useCustomAPIHook does not make API calls with empty parameters.
# Search for instances where useCustomAPIHook is invoked with empty parameters
rg 'useCustomAPIHook\(\s*{}\s*\)'
# Check the implementation of useCustomAPIHook to see how it handles empty parameters
rg -A 5 'function useCustomAPIHook' **/*.js
Length of output: 264
Script:
#!/bin/bash
# Let's try a different approach to find the implementation and usage of useCustomAPIHook
# Search for useCustomAPIHook implementation
ast-grep --pattern 'const useCustomAPIHook = $_'
# Search for any usage of useCustomAPIHook
rg "useCustomAPIHook"
# Search for Digit.Hooks.useCustomAPIHook specifically
rg "Digit\.Hooks\.useCustomAPIHook"
# Let's also check the specific file to understand the context better
cat "health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js"
Length of output: 47138
Script:
#!/bin/bash
# Let's check the implementation of CustomService.getResponse to understand how empty parameters are handled
# First, let's find the CustomService implementation
rg -A 5 "CustomService.*getResponse"
# Also check the useCustomAPIHook implementation more thoroughly
cat "micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useCustomAPIHook.js"
Length of output: 78815
Script:
#!/bin/bash
# Let's try one more time to get the useCustomAPIHook implementation
cat "micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useCustomAPIHook.js"
# Also check if there's any CustomService implementation
rg -A 5 "class CustomService"
Length of output: 1627
...web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js
Show resolved
Hide resolved
suryansh-egov
dismissed stale reviews from coderabbitai[bot] and Bhavya-egov
via
e7cea20
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
New Features
InfoCardcomponent in the BoundaryHome page to display hierarchy information.FinalPopupto provide clearer alert messages.Bug Fixes
Chores