Fix: Pincode autofill in Patient Registration page in public page #10272
Conversation
WalkthroughThe pull request introduces an optional Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. |
Rishith25
changed the title
![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 comments (1)
src/hooks/useOrganization.ts (1)
Line range hint
31-41: Consider cache implications and error handling improvements.Two suggestions for improvement:
- Include
!!authTokenin the query key to prevent cache conflicts between authenticated and unauthenticated requests- Add specific error handling for authentication failures
const { data, isLoading, isError } = useQuery({ - queryKey: ["organization", orgType, name, parentId], + queryKey: ["organization", orgType, name, parentId, !!authToken], queryFn: query(organizationApi.list, { queryParams: { org_type: orgType, parent: parentId, name, }, ...headers, }), enabled: enabled && !!name, + retry: (failureCount, error: any) => { + // Don't retry on auth errors + if (error?.response?.status === 401) return false; + return failureCount < 3; + }, });
🧹 Nitpick comments (2)
src/hooks/useOrganization.ts (1)
24-30: Consider adding token validation and simplifying headers construction.While the current implementation works, consider these improvements:
- Validate the token format/content
- Trim any potential whitespace
- Simplify the headers object structure
- const headers = authToken - ? { - headers: { - Authorization: `Bearer ${authToken}`, - }, - } - : {}; + const headers = authToken?.trim() + ? { headers: { Authorization: `Bearer ${authToken.trim()}` } } + : {}; + + if (headers.headers?.Authorization === 'Bearer ') { + throw new Error('Invalid auth token provided'); + }src/pages/PublicAppointments/PatientRegistration.tsx (1)
234-249: Consider debouncing pincode input.The effect hook correctly manages the autofill states, but consider debouncing the pincode input to prevent unnecessary API calls while the user is typing.
+import { useDebounce } from "@/hooks/useDebounce"; + export function PatientRegistration(props: PatientRegistrationProps) { // ... existing code ... + const debouncedPincode = useDebounce(form.watch("pincode")?.toString() || "", 500); + const { stateOrg, districtOrg } = useStateAndDistrictFromPincode({ - pincode: form.watch("pincode")?.toString() || "", + pincode: debouncedPincode, authToken: tokenData.token, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/hooks/useOrganization.ts(2 hunks)src/hooks/useStateAndDistrictFromPincode.ts(4 hunks)src/pages/PublicAppointments/PatientRegistration.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (10)
src/hooks/useOrganization.ts (3)
14-14: LGTM! Well-typed interface extension.The optional
authTokenparameter is properly typed and integrated into the interface.
22-22: LGTM! Consistent parameter addition.The
authTokenparameter is correctly added to match the interface definition.
Line range hint
1-53: Verify the authentication flow in the patient registration context.Since these changes are part of the pincode autofill feature, please ensure:
- The authentication flow is tested with both valid and invalid tokens
- The pincode autofill functionality works correctly with authenticated requests
- The error handling provides meaningful feedback to users
src/hooks/useStateAndDistrictFromPincode.ts (3)
14-16: LGTM! Interface update follows TypeScript best practices.The optional
authTokenparameter is correctly typed as an optional string.
24-26: LGTM! Function signature update maintains backward compatibility.The optional parameter is correctly destructured, preserving compatibility with existing code.
47-49: Consider error handling for unauthorized requests.While the
authTokenis correctly passed to both organization queries, there's no specific handling for unauthorized responses.Let's verify the error handling in the
useOrganizationhook:Also applies to: 61-63
src/pages/PublicAppointments/PatientRegistration.tsx (4)
4-4: LGTM! Clean import organization and state management.The imports and state declarations are well-organized and follow React best practices.
Also applies to: 10-11, 28-28, 43-43, 86-88
229-232: LGTM! Proper hook integration with authentication.The
useStateAndDistrictFromPincodehook is correctly integrated with the required authentication token.
421-436: LGTM! Accessible feedback implementation.The autofill feedback implementation follows accessibility best practices:
- Uses semantic ARIA attributes
- Provides visual feedback with icon
- Includes proper text messaging
449-456: LGTM! Form state management improvement.The change from direct field manipulation to
form.setValueis a better approach for managing form state.
|
@rithviknishad @Jacobjeevan can you review this PR |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Improvements
Technical Updates