-
Notifications
You must be signed in to change notification settings - Fork 531
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
Remove pin-code autofill feature #10291
Conversation
WalkthroughThis pull request involves the removal of pincode-related functionality across multiple files. The changes primarily eliminate the Changes
Suggested Labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (2)
✨ Finishing Touches
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. |
Deploying care-fe with Cloudflare Pages
|
CARE Run #4531
Run Properties:
|
Project |
CARE
|
Branch Review |
rithviknishad/fix/remove-pincode-autofill
|
Run status |
Passed #4531
|
Run duration | 06m 09s |
Commit |
fb769098a3: Remove pin-code autofill feature
|
Committer | Rithvik Nishad |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
10
|
View all changes introduced in this branch ↗︎ |
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: 0
🧹 Nitpick comments (2)
src/components/Facility/FacilityForm.tsx (2)
52-55
: Clean up unused interface property.The
organizationId
prop is defined in theFacilityProps
interface but is not used in the component. Consider removing it to maintain a clean interface.interface FacilityProps { - organizationId?: string; facilityId?: string; onSubmitSuccess?: () => void; }
Line range hint
290-305
: Consider adding pattern validation for pincode input.While the maxLength validation is good, consider adding pattern validation to ensure only numeric input:
<Input data-cy="facility-pincode" placeholder="Enter pincode" maxLength={6} + pattern="[0-9]*" + inputMode="numeric" {...field} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Utils/utils.ts
(0 hunks)src/components/Facility/FacilityForm.tsx
(1 hunks)src/components/Patient/PatientRegistration.tsx
(0 hunks)src/hooks/useStateAndDistrictFromPincode.ts
(0 hunks)
💤 Files with no reviewable changes (3)
- src/Utils/utils.ts
- src/hooks/useStateAndDistrictFromPincode.ts
- src/components/Patient/PatientRegistration.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/components/Facility/FacilityForm.tsx (3)
Line range hint
60-75
: LGTM! Pincode validation maintained.The form schema correctly maintains pincode validation while removing the auto-population functionality, aligning with the PR objective.
Line range hint
130-138
: LGTM! Clean submission logic.The form submission logic is well-structured and properly handles both creation and updates.
Line range hint
1-450
: Verify complete removal of autofill functionality.Let's verify that all autofill-related code has been completely removed from the codebase.
✅ Verification successful
Autofill functionality has been successfully removed
The verification confirms that all pincode autofill functionality has been completely removed from the active codebase. The only remaining traces are inactive elements (commented code and translation strings) that don't affect the application's behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to pincode autofill functionality echo "Searching for potential remaining autofill references..." rg -i "autofill|useStateAndDistrictFromPincode|getPincodeDetails" --type ts --type tsx echo "Checking for any TODO comments related to pincode..." rg -i "todo.*pincode|fixme.*pincode" --type ts --type tsxLength of output: 442
Script:
#!/bin/bash # Search without file type restrictions echo "Searching for autofill related code..." rg -i "autofill|useStateAndDistrictFromPincode|getPincodeDetails" echo "Checking for pincode-related API endpoints..." rg -i "pincode.*api|api.*pincode" echo "Checking for TODO comments..." rg -i "todo.*pincode|fixme.*pincode" # Check for any utility functions related to pincode echo "Checking for pincode utility functions..." ast-grep --pattern 'function $_(pincode$_) { $$$ }'Length of output: 1494
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: 1
🧹 Nitpick comments (1)
src/pages/PublicAppointments/PatientSelect.tsx (1)
178-178
: Update function name reference.Update the function call to match the corrected name
getPatientDoBorAge
.- {getPatienDoBorAge(patient)} + {getPatientDoBorAge(patient)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/Providers/PatientUserProvider.tsx
(3 hunks)src/Utils/permissions.ts
(1 hunks)src/Utils/request/api.tsx
(3 hunks)src/Utils/utils.ts
(2 hunks)src/common/constants.tsx
(0 hunks)src/components/Common/FacilitySelect.tsx
(1 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx
(0 hunks)src/components/Users/UserListAndCard.tsx
(2 hunks)src/components/Users/models.tsx
(0 hunks)src/components/ui/sidebar/patient-nav.tsx
(2 hunks)src/pages/Appointments/AppointmentDetail.tsx
(3 hunks)src/pages/Encounters/EncounterShow.tsx
(0 hunks)src/pages/Facility/hooks/useFacilityFilters.ts
(0 hunks)src/pages/Patient/Utils.tsx
(0 hunks)src/pages/PublicAppointments/PatientRegistration.tsx
(2 hunks)src/pages/PublicAppointments/PatientSelect.tsx
(4 hunks)src/types/emr/patient.ts
(0 hunks)src/types/scheduling/schedule.ts
(2 hunks)
💤 Files with no reviewable changes (7)
- src/types/emr/patient.ts
- src/components/Users/models.tsx
- src/components/Patient/PatientDetailsTab/Demography.tsx
- src/pages/Facility/hooks/useFacilityFilters.ts
- src/pages/Patient/Utils.tsx
- src/pages/Encounters/EncounterShow.tsx
- src/common/constants.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Users/UserListAndCard.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9080
File: src/components/Users/UserListAndCard.tsx:306-312
Timestamp: 2024-11-22T10:44:19.282Z
Learning: The `getDistrict` function in `src/components/Users/UserListAndCard.tsx` returns a JSX element styled specifically for the card view and should not be reused in the list view.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (14)
src/Utils/permissions.ts (1)
27-27
: LGTM! Function logic simplified.The simplified permission check aligns with the removal of location-based checks, making the code more maintainable.
src/components/Common/FacilitySelect.tsx (1)
101-101
: LGTM! Simplified facility label display.Removed district information from facility labels as part of removing location-based features.
src/pages/Appointments/AppointmentDetail.tsx (2)
57-57
: LGTM! Added import for new utility function.Added import for
stringifyGeoOrganization
to handle address formatting.
335-335
: LGTM! Improved address formatting.Replaced manual address concatenation with the
stringifyGeoOrganization
utility function, which provides a more maintainable way to display hierarchical location data.src/Utils/utils.ts (1)
276-286
: LGTM! Well-implemented utility function.The
stringifyGeoOrganization
function:
- Cleanly handles the organization hierarchy traversal
- Safely handles undefined cases
- Uses clear variable names
- Provides a consistent format for location strings
src/components/ui/sidebar/patient-nav.tsx (1)
9-9
: LGTM! Type migration looks good.The change from
AppointmentPatient
toPatient
type is consistent with the codebase-wide refactoring.Also applies to: 18-18
src/types/scheduling/schedule.ts (1)
4-4
: LGTM! Type updates are consistent.The migration from
AppointmentPatient
toPatient
type in theAppointment
interface maintains consistency with the broader refactoring effort.Also applies to: 143-143
src/components/Users/UserListAndCard.tsx (1)
Line range hint
131-194
: LGTM! Component simplification improves maintainability.The removal of the district column and related props aligns with the PR objective of removing location-related features. The component is now more focused and maintainable.
src/pages/PublicAppointments/PatientRegistration.tsx (2)
36-37
: LGTM! Import changes align with the new patient type system.The changes correctly update the imports to use the new
Patient
type while retaining theAppointmentPatientRegister
type for registration.
175-175
: LGTM! Type update in mutation callback.The mutation callback correctly uses the new
Patient
type, maintaining consistency with the API response type.src/Utils/request/api.tsx (1)
625-625
: LGTM! API route return types updated consistently.The return types for both
getPatient
andcreatePatient
routes have been correctly updated to use the newPatient
type, maintaining consistency across the API layer.Also applies to: 636-636
src/Providers/PatientUserProvider.tsx (3)
10-10
: LGTM! Context type definitions updated correctly.The type definitions have been properly updated to use the new
Patient
type throughout the context interface.Also applies to: 13-15
28-29
: LGTM! State initialization aligned with new types.The state hooks have been correctly updated to use the new
Patient
type for both the patients array and selected patient.
46-46
: LGTM! Local storage handling updated.The local storage parsing has been properly typed to use the new
Patient
type.
LGTM , verify at public page, facility, user and patient creation |
@rithviknishad 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
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
Removed Features
Code Cleanup
Impact