Fix: Reflect Updated Service Notes Immediately in Asset Details without page reload #9226
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Assets/AssetServiceEditModal.tsx (2)
58-60: Consider adding error handling for refetch operationWhile the placement of
refetch()is correct, consider handling potential failures in the refetch operation to ensure robust error recovery.props.handleUpdate(); props.handleClose(); - props.refetch(); + try { + await props.refetch(); + } catch (error) { + Notification.Error({ + msg: "Failed to refresh service details. Please reload the page.", + }); + }
27-27: Implementation effectively addresses the immediate update requirementThe changes successfully implement the immediate refresh functionality while maintaining code clarity and separation of concerns. The solution is clean and maintainable.
Consider implementing optimistic updates in the future to improve perceived performance. This would involve updating the UI immediately while the API request is in progress, then reverting if the request fails.
Also applies to: 59-59
src/components/Assets/AssetManage.tsx (1)
602-602: Consider consolidating refresh handlersThe addition of the
refetchprop correctly implements immediate updates. However, we now have two separate refresh calls:handleUpdatecallingserviceRefetchand the newrefetch. Consider consolidating these into a single handler for better maintainability.- handleUpdate={() => serviceRefetch()} - refetch={refetch} + handleUpdate={() => { + serviceRefetch(); + refetch(); + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Assets/AssetManage.tsx(2 hunks)src/components/Assets/AssetServiceEditModal.tsx(2 hunks)
🔇 Additional comments (3)
src/components/Assets/AssetServiceEditModal.tsx (2)
27-27: LGTM: Prop addition aligns with requirements
The addition of the refetch prop is well-typed and properly positioned to support immediate data refresh functionality.
27-27: Verify parent component implementation of refetch
Let's ensure the parent component properly implements the refetch functionality.
Also applies to: 59-59
✅ Verification successful
Parent component properly implements refetch functionality
The parent component AssetManage.tsx correctly implements the refetch functionality:
- It obtains
refetchfrom theuseQueryhook for asset data - Passes it to
AssetServiceEditModalas a prop - The refetch is used to refresh asset data after service updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components that render AssetServiceEditModal
ast-grep --pattern 'const $_ = () => {
$$$
return (
$$$
<AssetServiceEditModal
$$$
refetch={$_}
$$$
/>
$$$
)
}'
# Search for useQuery implementations that provide refetch
rg -A 5 'useQuery.*\{.*refetch.*\}'
Length of output: 191
Script:
#!/bin/bash
# Search for components that render AssetServiceEditModal with more flexible pattern
ast-grep --pattern '<AssetServiceEditModal'
# Search for files importing AssetServiceEditModal
rg -l "import.*AssetServiceEditModal"
# Search for useQuery with refetch in the importing files
rg --type typescript --type tsx "const.*useQuery.*\{.*refetch"
Length of output: 223
Script:
#!/bin/bash
# Check the content of the parent component
cat src/components/Assets/AssetManage.tsx
# Search for useQuery usage without file type restriction
rg "const.*useQuery.*\{.*refetch"
Length of output: 22110
src/components/Assets/AssetManage.tsx (1)
72-76: LGTM! Correctly destructured refetch from useQuery
The addition of refetch to the useQuery hook destructuring is well-implemented and necessary for enabling immediate data refresh functionality.
|
@rithviknishad Done the changes |
|
@Rishith25 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Before
After
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Improvements
ShiftDetailscomponent's button properties without altering existing functionality.Bug Fixes