Fix: Breadcrumbs redirection to patient edit consultation form #8985
Conversation
- Added Consultation breadcrumb with URI for updating consultation - Updated backUrl for consultation update Fixes ohcnetwork#8765
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…o issues/8759/breadcrumbs-consultation
Co-authored-by: Rithvik Nishad <[email protected]>
WalkthroughThe changes in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 (
|
Co-authored-by: Rithvik Nishad <[email protected]>
|
|
|
@shauryag2002 what is the status on the PR ? |
I am little busy past few days, resolving requested changes today! |
github-actions
bot
added
needs-triage
question
…I for consultation
…o issues/8759/breadcrumbs-consultation
![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 (1)
src/components/Facility/ConsultationDetails/index.tsx (1)
Based on the search results, I can now provide a conclusive response about the backUrl behavior:
The backUrl="/patients" is inconsistent with the navigation pattern in the codebase
The search results show that similar patient-related views follow a more specific navigation pattern:
- Patient home uses:
backUrl={facilityId ?/facility/${facilityId}/patients: "/patients"}- Other patient-related views consistently navigate back to facility-specific paths
The current implementation should be updated to:
backUrl={`/facility/${facilityId}/patients`}This change would:
- Maintain consistency with the rest of the codebase's navigation patterns
- Keep users within the context of the current facility
- Provide a better user experience by returning to the facility-specific patient list
🔗 Analysis chain
Line range hint
209-224: Verify the backUrl behavior in PageTitleWhile the consultation breadcrumb changes look good, let's verify that the backUrl="/patients" is the intended behavior. Should users be redirected to the general patients list, or would it be more appropriate to return to the specific patient's details?
Let's check for consistency in backUrl usage across similar views:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other PageTitle components with backUrl props to verify consistency rg -A 2 'backUrl=' --type tsxLength of output: 57
Script:
#!/bin/bash # Search for PageTitle components with backUrl props using a more general pattern rg -A 2 'backUrl.*=' --type ts --type tsx --type jsxLength of output: 82
Script:
#!/bin/bash # Search for PageTitle component usage patterns rg -A 2 '<PageTitle' --type ts --type tsx --type jsxLength of output: 82
Script:
#!/bin/bash # Find all files that might contain PageTitle component fd -e tsx -e ts -e jsx PageTitleLength of output: 70
Script:
#!/bin/bash # Search for navigation patterns in consultation related files rg -A 2 'consultation.*navigation|navigation.*consultation' --type ts --type tsxLength of output: 110
Script:
#!/bin/bash # Let's examine the PageTitle component implementation cat src/components/Common/PageTitle.tsxLength of output: 2088
Script:
#!/bin/bash # Search for navigation patterns without file type restrictions rg -A 2 'backUrl.*=|<PageTitle'Length of output: 16116
Script:
#!/bin/bash # Search for consultation-related files fd consultationLength of output: 1684
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Facility/ConsultationDetails/index.tsx(1 hunks)src/components/Patient/PatientConsentRecords.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientConsentRecords.tsx
🔇 Additional comments (1)
src/components/Facility/ConsultationDetails/index.tsx (1)
214-217: LGTM! The breadcrumb navigation fix looks correct.
The addition of the consultation breadcrumb with the proper update URI addresses the navigation issue described in #8759. The implementation correctly redirects users to the consultation update form instead of creating a new consultation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/components/Facility/ConsultationDetails/index.tsx (4)
195-197: Improve template literal formattingThe tab button styling logic is correct, but the formatting could be improved for better readability.
Apply this formatting fix:
const tabButtonClasses = (selected: boolean) => `capitalize min-w-max-content cursor-pointer font-bold whitespace-nowrap ${ selected === true ? "border-primary-500 hover:border-secondary-300 text-primary-600 border-b-2" : "text-secondary-700 hover:text-secondary-700" }`;🧰 Tools
🪛 GitHub Check: lint
[failure] 195-195:
Insert⏎······
[failure] 196-196:
Insert··
[failure] 197-197:
Insert··
Line range hint
386-397: Extract camera feed visibility logic into a separate functionThe current implementation has multiple nested conditions that could be simplified for better maintainability.
Consider extracting the visibility logic into a dedicated function:
+ const shouldShowFeedTab = () => { + return ( + isCameraAttached && + !consultationData?.discharge_date && + consultationData?.current_bed?.bed_object?.id && + CameraFeedPermittedUserTypes.includes(authUser.user_type) + ); + }; if (p === "FEED") { - if ( - isCameraAttached === false || // No camera attached - consultationData?.discharge_date || // Discharged - !consultationData?.current_bed?.bed_object?.id || // Not admitted to bed - !CameraFeedPermittedUserTypes.includes( - authUser.user_type, - ) - ) - return null; // Hide feed tab + if (!shouldShowFeedTab()) return null; }🧰 Tools
🪛 GitHub Check: lint
[failure] 195-195:
Insert⏎······
[failure] 196-196:
Insert··
[failure] 197-197:
Insert··
279-280: Fix indentation in navigation pathThe navigation logic is correct, but the indentation should be consistent.
Apply this formatting fix:
? navigate( - `/facility/${facilityId}/patient/${patientId}/consultation/${consultationId}/notes`, + `/facility/${facilityId}/patient/${patientId}/consultation/${consultationId}/notes`, )🧰 Tools
🪛 GitHub Check: lint
[failure] 279-279:
Insert··
[failure] 280-280:
Replace····················with······················
316-323: Simplify date display logicThe date display logic could be more concise while maintaining readability.
Consider this simplified version:
- <div className="text-3xl font-bold"> - {relativeTime( - consultationData.discharge_date - ? consultationData.discharge_date - : consultationData.encounter_date, - )} - </div> + <div className="text-3xl font-bold"> + {relativeTime(consultationData.discharge_date ?? consultationData.encounter_date)} + </div>🧰 Tools
🪛 GitHub Check: lint
[failure] 316-316:
Delete··
[failure] 317-317:
Delete··
[failure] 318-318:
Delete··
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Facility/ConsultationDetails/index.tsx(4 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/components/Facility/ConsultationDetails/index.tsx
[failure] 195-195:
Insert ⏎······
[failure] 196-196:
Insert ··
[failure] 197-197:
Insert ··
[failure] 224-224:
Insert ··
[failure] 225-225:
Insert ··
[failure] 279-279:
Insert ··
[failure] 280-280:
Replace ···················· with ······················
[failure] 316-316:
Delete ··
[failure] 317-317:
Delete ··
[failure] 318-318:
Delete ··
🔇 Additional comments (1)
src/components/Facility/ConsultationDetails/index.tsx (1)
216-219: LGTM: Breadcrumb navigation fix implemented correctly
The addition of the consultation breadcrumb with the correct update URI should resolve the redirection issue described in #8759.
Let's verify the breadcrumb navigation paths:
✅ Verification successful
Breadcrumb navigation path is consistent with router configuration
The verification confirms that:
- The consultation update path
/facility/${facilityId}/patient/${patientId}/consultation/${consultationId}/updateis correctly defined inConsultationRoutes.tsx - The same path pattern is consistently used across other components that need to link to the consultation update form
- No conflicting or inconsistent navigation paths were found
The breadcrumb implementation aligns with the established routing structure and will correctly redirect users to the consultation update form, fixing issue #8759.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistency of consultation update paths across the codebase
# Search for other consultation update path patterns
rg -l "consultation/\$\{[^}]+\}/update"
# Search for potential conflicting navigation paths
rg "consultation.*update" --type tsx
Length of output: 107
Script:
#!/bin/bash
# Search for consultation update paths with proper file type
rg -t ts -t tsx "consultation.*update"
# Search for router configurations and navigation paths
rg -t ts -t tsx "consultation.*route|path.*consultation"
# Look for breadcrumb configurations
rg -t ts -t tsx "breadcrumb.*consultation|consultation.*breadcrumb"
Length of output: 253
Script:
#!/bin/bash
# Search for consultation paths and breadcrumbs without file type restrictions
rg "consultation.*update"
# Find all TypeScript/React files that might contain consultation routes
fd "\.(tsx?|jsx?)$" | xargs rg "consultation.*route|path.*consultation"
# Look for breadcrumb configurations
fd "\.(tsx?|jsx?)$" | xargs rg "breadcrumb.*consultation|consultation.*breadcrumb"
Length of output: 17286
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Facility/ConsultationDetails/index.tsx (2)
424-424: Consider extracting FEED tab visibility logic to improve readability.The current implementation has multiple nested conditions that could be simplified for better maintainability.
Consider extracting the visibility logic to a custom hook or helper function:
+const useFeedTabVisibility = ( + isCameraAttached: boolean, + consultationData: ConsultationModel, + userType: string +) => { + return ( + isCameraAttached && + !consultationData?.discharge_date && + consultationData?.current_bed?.bed_object?.id && + CameraFeedPermittedUserTypes.includes(userType) + ); +}; export const ConsultationDetails = (props: any) => { // ... existing code ... + const showFeedTab = useFeedTabVisibility( + isCameraAttached, + consultationData, + authUser.user_type + ); // Replace the existing condition with: if (p === "FEED" && !showFeedTab) return null;🧰 Tools
🪛 GitHub Check: lint
[failure] 424-424:
Insert⏎
424-424: Add newline at end of file.Add a newline character at the end of the file to comply with coding standards.
}; +🧰 Tools
🪛 GitHub Check: lint
[failure] 424-424:
Insert⏎
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Facility/ConsultationDetails/index.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/components/Facility/ConsultationDetails/index.tsx
[failure] 424-424:
Insert ⏎
🔇 Additional comments (1)
src/components/Facility/ConsultationDetails/index.tsx (1)
217-220: LGTM! Verify the consultation update route.
The breadcrumb implementation looks correct and aligns with the PR objective to fix navigation issues.
Let's verify the consultation update route exists and handles updates correctly:
✅ Verification successful
Based on the search results, I can now generate the final response as we have found the route definition in src/Routers/routes/ConsultationRoutes.tsx:
The consultation update route exists and is properly configured
The breadcrumb URI /facility/${facilityId}/patient/${patientId}/consultation/${id}/update matches the defined route in src/Routers/routes/ConsultationRoutes.tsx:
"/facility/:facilityId/patient/:patientId/consultation/:id/update": ({
facilityId,
patientId,
id,🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consultation update route exists in the router configuration
# Expected: Find route definition for consultation update
# Search for route definition
rg -A 3 "consultation/:consultationId/update"
Length of output: 45
Script:
#!/bin/bash
# Let's try different approaches to find the route definition
# 1. Search for consultation route patterns more broadly
rg -A 3 "consultation.*update"
# 2. Look for router configuration files
fd "router|routes" --type f
# 3. Search for consultation route components
rg -A 3 "path.*consultation"
Length of output: 47976
bodhish
added
needs testing
and removed
question
|
LGTM |
|
👋 Hi, @shauryag2002, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…o issues/8759/breadcrumbs-consultation
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
ConfirmDialogto dynamically display patient code status for consent records.ConsultationDetailscomponent for direct navigation to the consultation update page.Improvements
backUrl.