-
Notifications
You must be signed in to change notification settings - Fork 540
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
Date handling in DOB section #10415
base: develop
Are you sure you want to change the base?
Date handling in DOB section #10415
Conversation
WalkthroughThis PR introduces multiple refactors across components, including modifications in PatientRegistration to add an Changes
Sequence Diagram(s)sequenceDiagram
participant User as End User
participant PL as PatientLogin Component
participant Req as request()
participant OTP_API as OTP API Endpoint
participant Toast as Toast Notifier
User->>PL: Enter phone number & request OTP
PL->>Req: Call sendOTP (with phone number)
Req->>OTP_API: POST phone_number for OTP
OTP_API-->>Req: Response (success/failure)
Req-->>PL: Return OTP send result
alt Success
PL->>User: Prompt to enter OTP
User->>PL: Inputs OTP
PL->>Req: Call verifyOTP (with phone_number, otp)
Req->>OTP_API: POST otp verification request
OTP_API-->>Req: Verification response
Req-->>PL: Return verification result
PL->>User: Proceed on verified login
else Failure
Req-->>PL: Error response
PL->>Toast: Display error message
end
sequenceDiagram
participant Provider as AuthUserProvider
participant Timer as setInterval
participant Req as request()
participant API as Token Refresh API
participant LS as Local Storage
Provider->>Timer: Initialize periodic refresh
Timer->>Provider: Trigger updateRefreshToken
Provider->>Req: Call refresh token request
Req->>API: Send refresh token data
API-->>Req: Return new tokens
Req-->>Provider: Provide refreshed tokens
Provider->>LS: Update tokens in localStorage
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Tip 🌐 Web search-backed reviews and chat
✨ 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. |
@Jacobjeevan Can you please take a look? |
Pushed some changes 👍 |
I have tested and I think it is ready for review. |
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.
changes unrelated to the linked issues present. did u forget to link additional issues or was it unintended?
There is a different PR opened for one issue ( #10413 ) and I did not notice that it is not reviewed. So those changes are merged into this. |
Either update the linked issues in this PR's main body comment and close the other one, or revert those changes from this PR. |
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: 8
🔭 Outside diff range comments (2)
src/components/Patient/allergy/list.tsx (1)
127-135
: Remove debug logging from production code.The
useEffect
hook contains console logging that should be removed before deploying to production.- useEffect(() => { - console.log( - "Allergy Note:", - allergy.note, - isLongNote, - displayNote, - note.length, - ); - }, [allergy.note, isLongNote, displayNote, note.length]);src/components/Common/FilePreviewDialog.tsx (1)
244-253
: Consider consistent zoom behavior across file types.The component maintains zoom functionality for images but removes it for PDFs. This creates an inconsistent user experience across different file types.
Consider either:
- Restoring zoom functionality for PDFs
- Using a consistent approach for both file types
Example implementation for PDF zoom:
<PDFViewer url={fileUrl} onDocumentLoadSuccess={(numPages: number) => { setPage(1); setNumPages(numPages); }} pageNumber={page} + scale={file_state.zoom * 0.25} />
🧹 Nitpick comments (3)
src/hooks/useFileUpload.tsx (1)
107-121
: Consider extracting compression options to constants.The image compression logic is now simpler, but the compression options could be extracted to named constants for better maintainability.
+const IMAGE_COMPRESSION_OPTIONS = { + initialQuality: 0.6, + alwaysKeepResolution: true, +}; selectedFiles.forEach((file) => { const ext: string = file.name.split(".")[1]; if (ExtImage.includes(ext)) { - const options = { - initialQuality: 0.6, - alwaysKeepResolution: true, - }; - imageCompression(file, options).then((compressedFile: File) => { + imageCompression(file, IMAGE_COMPRESSION_OPTIONS).then((compressedFile: File) => {src/components/Files/FilesTab.tsx (1)
579-585
: Consider adding more descriptive ARIA labels.While the dialog has basic ARIA attributes, consider adding more descriptive labels for better accessibility.
<Dialog open={open} onOpenChange={onOpenChange} - aria-labelledby="file-upload-dialog" + aria-labelledby="file-upload-dialog-title" + aria-describedby="file-upload-dialog-description" > <DialogContent className="mb-8 rounded-lg p-5 max-w-fit" - aria-describedby="file-upload" + role="dialog" >src/components/Common/PDFViewer.tsx (1)
24-24
: Consider making the height prop configurable.The hardcoded height of 650 pixels might not be optimal for all screen sizes and use cases.
- <Page pageNumber={props.pageNumber} height={650} /> + <Page + pageNumber={props.pageNumber} + height={props.height ?? 650} + />Also update the props interface:
props: Readonly<{ url: string; pageNumber: number; height?: number; onDocumentLoadSuccess: (numPages: number) => void; }>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (28)
cypress/e2e/patient_spec/patient_details.cy.ts
(1 hunks)cypress/fixtures/users.json
(0 hunks)package.json
(2 hunks)public/locale/en.json
(2 hunks)public/locale/hi.json
(2 hunks)public/locale/kn.json
(2 hunks)public/locale/ml.json
(2 hunks)public/locale/ta.json
(2 hunks)src/CAREUI/misc/PrintPreview.tsx
(1 hunks)src/Providers/AuthUserProvider.tsx
(4 hunks)src/Utils/request/api.tsx
(1 hunks)src/components/Auth/Login.tsx
(5 hunks)src/components/Auth/ResetPassword.tsx
(3 hunks)src/components/Common/FilePreviewDialog.tsx
(1 hunks)src/components/Common/PDFViewer.tsx
(1 hunks)src/components/Facility/FacilityHome.tsx
(4 hunks)src/components/Files/FilesTab.tsx
(4 hunks)src/components/Patient/allergy/list.tsx
(1 hunks)src/components/Resource/ResourceCreate.tsx
(6 hunks)src/components/Users/UserAvatar.tsx
(5 hunks)src/components/Users/UserSummary.tsx
(3 hunks)src/hooks/useAuthUser.ts
(1 hunks)src/hooks/useFileUpload.tsx
(3 hunks)src/pages/Appointments/AppointmentDetail.tsx
(3 hunks)src/pages/Appointments/AppointmentsPage.tsx
(1 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx
(0 hunks)src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx
(1 hunks)src/pages/PublicAppointments/auth/PatientLogin.tsx
(6 hunks)
💤 Files with no reviewable changes (2)
- cypress/fixtures/users.json
- src/pages/Encounters/tabs/EncounterUpdatesTab.tsx
🧰 Additional context used
📓 Learnings (2)
cypress/e2e/patient_spec/patient_details.cy.ts (1)
Learnt from: nihal467
PR: ohcnetwork/care_fe#9285
File: cypress/e2e/patient_spec/PatientDoctorNotes.cy.ts:17-17
Timestamp: 2024-12-04T07:06:49.613Z
Learning: In `PatientDoctorNotes.cy.ts`, the login methods `loginManuallyAsNurse()` and `loginByRole("nurse")` serve different purposes and approaches. Refactoring `loginManuallyAsNurse()` to use `loginByRole()` is not necessary in this context.
src/components/Users/UserSummary.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9080
File: src/components/Users/UserSummary.tsx:53-71
Timestamp: 2024-11-22T12:04:39.044Z
Learning: In the `UserSummaryTab` component (`src/components/Users/UserSummary.tsx`), when there is an error during user deletion, the delete dialog is intentionally closed to prevent the user from immediately trying to delete the user again.
🔇 Additional comments (51)
src/components/Patient/allergy/list.tsx (1)
71-76
: LGTM! Query key has been simplified.The removal of
encounterStatus
from the query key array simplifies the caching strategy while maintaining the essential parameters.src/pages/Appointments/AppointmentsPage.tsx (1)
978-978
: LGTM! Fixed width wrapper improves UI consistency.The addition of a fixed-width wrapper prevents layout shifts and ensures consistent dropdown width across all instances.
src/pages/Appointments/AppointmentDetail.tsx (3)
164-164
: LGTM: Simplified padding for consistent layout.The removal of responsive padding makes the appointment token card's layout more consistent across screen sizes.
170-170
: LGTM: Improved button container and action section spacing.The button container and action section spacing changes maintain good visual hierarchy.
Also applies to: 193-193
219-219
: LGTM: Simplified AppointmentDetails layout.The container padding and title styling changes maintain clean visual presentation.
Also applies to: 223-223
cypress/e2e/patient_spec/patient_details.cy.ts (1)
11-11
: Verify the updated login credentials across test files.The change from "devdoctor" to "doctor" aligns with the removal of obsolete user entries.
Let's verify the credentials and their usage:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the existence of "doctor" credentials and check for any remaining "devdoctor" references # Check if "doctor" credentials exist in users.json echo "Checking for doctor credentials..." rg -l '"doctor"' cypress/fixtures/users.json # Check for any remaining "devdoctor" references that might need updating echo "Checking for remaining devdoctor references..." rg -l "devdoctor" "cypress/e2e/**/*.{ts,js}"Length of output: 366
Verified the updated login credentials and absence of deprecated "devdoctor" references.
- The "doctor" credential exists in cypress/fixtures/users.json.
- A subsequent search across the cypress/e2e directory confirms no usage of "devdoctor" remains.
src/CAREUI/misc/PrintPreview.tsx (2)
27-27
: Verify the impact of reduced default scale on mobile devices.The default scale has been significantly reduced from 1 to 0.44, which might affect readability on mobile devices. While the md breakpoint maintains the original scale, smaller screens will see content at less than half size.
Please test the component on various mobile devices to ensure content remains readable and usable at 0.44 scale.
27-32
: Verify if these layout changes are within PR scope.These layout modifications to the PrintPreview component appear unrelated to the PR's stated objective of handling dates in the DOB section. Consider whether these changes should be part of a separate PR focused on layout improvements.
src/hooks/useFileUpload.tsx (2)
54-54
: LGTM! Simplified handleFileUpload signature.The removal of the
combineToPDF
parameter aligns with the removal of PDF generation functionality.
251-291
: LGTM! Improved upload logic.The upload logic is now more focused and maintainable with better error handling.
src/components/Files/FilesTab.tsx (3)
156-156
: LGTM! Simplified callback.The onUpload callback is now more concise while maintaining the same functionality.
593-641
: LGTM! Improved file list UI.The new file list UI is more user-friendly with clear actions and feedback. The error handling is well-integrated into the design.
1-1
: Verify the scope of changes.The changes in these files focus on file upload functionality, which seems unrelated to the PR's stated objective of updating date handling in the DOB section. This aligns with the comment in the PR that mentions unrelated changes were inadvertently merged.
Please confirm if these changes should be part of a separate PR focused on file upload improvements.
src/components/Common/PDFViewer.tsx (1)
5-29
: Verify impact of removing zoom functionality.The removal of scale/zoom functionality might affect users who need to view detailed information in PDFs. Please ensure this aligns with user experience requirements.
src/components/Auth/ResetPassword.tsx (2)
73-88
: Simplify password reset logic with direct requestsThe refactoring in the
handleSubmit
function (lines 73-88) replaces theuseMutation
hook with direct use of therequest
utility, streamlining the password reset process. Error handling is managed effectively, and the navigation upon success or failure is appropriate.
91-105
: Efficient token validation usinguseEffect
The changes in lines 91-105 move token validation into a
useEffect
hook, making the code more straightforward by directly validating the reset token upon component mount. Redirecting to/invalid-reset
when the token is invalid ensures proper user flow.src/hooks/useAuthUser.ts (1)
5-10
: EnhancesignIn
method with detailed return typeBy introducing the
SignInReturnType
type alias and updating thesignIn
method's return type (lines 5-10), error handling is improved. This allowssignIn
to return detailed information about the request result, which can enhance the authentication flow and error management.src/components/Users/UserAvatar.tsx (4)
2-2
: LGTM! Import changes look good.The imports have been updated to remove unused imports and add the new
request
function.Also applies to: 19-19
30-37
: LGTM! Query configuration looks good.The
pathParams
object is now properly typed with explicit property name, improving code clarity.
56-56
: LGTM! Avatar upload handling looks good.The query invalidation has been simplified to only invalidate the necessary query key.
Also applies to: 68-79
68-79
: LGTM! Avatar deletion handling looks good.The implementation has been simplified by using the direct
request
function while maintaining the same functionality.src/components/Users/UserSummary.tsx (2)
31-31
: LGTM! Import changes look good.The
request
function has been imported for direct API calls.
41-56
: LGTM! User deletion mutation looks good.The implementation has been updated to use the
request
function while maintaining the same functionality.src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx (1)
158-172
: LGTM! Layout improvements look good.The layout has been improved by:
- Using flex layout for better responsiveness
- Grouping related information together
- Maintaining consistent spacing and alignment
src/pages/PublicAppointments/auth/PatientLogin.tsx (4)
9-9
: LGTM! Import changes look good.Added necessary imports for toast notifications and HTTP error types.
Also applies to: 36-37
78-94
: LGTM! OTP sending implementation looks good.The implementation has been improved with:
- Direct request function usage
- Proper error handling with user feedback
- Silent option to suppress default error handling
105-141
: LGTM! OTP verification implementation looks good.The implementation has been improved with:
- Better type safety for function parameters
- Proper error handling with user feedback
- Clear error messages from the API response
248-248
: LGTM! Resend OTP implementation looks good.The implementation has been simplified while maintaining the same functionality.
src/components/Facility/FacilityHome.tsx (4)
37-37
: LGTM!Import of the new
request
utility for standardized API calls.
109-111
: LGTM!Clean extraction of the close handler into a named function.
113-125
: LGTM!Clean extraction of the delete handler into a named function with proper error handling and user feedback.
156-169
: LGTM!Clean refactor of cover image deletion to use the new
request
function with proper error handling and cache invalidation.src/components/Resource/ResourceCreate.tsx (5)
2-2
: LGTM!Clean import updates to support the refactoring:
- Removed
useMutation
as it's no longer needed- Added
useState
for loading state- Added
request
utility- Updated type import
Also applies to: 4-4, 48-48, 50-50
60-60
: LGTM!Added loading state management.
107-141
: LGTM!Clean refactor of the submit handler with proper:
- Loading state management
- Error handling using try-catch
- Success/error toasts
- Navigation on success
153-155
: LGTM!Added loading state check before rendering.
417-419
: LGTM!Updated submit button to show loading state.
src/Utils/request/api.tsx (1)
625-627
: LGTM!Enhanced error handling by updating the
loginByOtp
response type to support both success and error cases.src/components/Auth/Login.tsx (4)
38-38
: LGTM!Import of the new
request
utility for standardized API calls.
74-74
: LGTM!Simplified auth context usage by removing unused
isAuthenticating
.
113-119
: LGTM!Clean refactor of OTP sending to use the new
request
function with proper error handling.
306-307
: LGTM!Updated loading state to use mutation states directly.
package.json (2)
3-3
: Version Downgrade Verification:
The package version has been correctly downgraded to"2.9.0"
, aligning with the PR objectives. Please ensure that this version change is compatible with all consuming modules and that any tests affected by this downgrade pass as expected.
182-183
: Optional Dependencies Version Update:
The versions for@rollup/rollup-linux-arm64-gnu
and@rollup/rollup-linux-x64-gnu
have been updated from"4.34.4"
to"4.32.1"
. This change is consistent with the dependency adjustments outlined in the PR objectives. Verify that these version changes do not introduce build or runtime issues.public/locale/hi.json (1)
265-265
: Localization Key Consolidation:
A new key"deleted_successfully": "{{name}} सफलतापूर्वक हटा दिया गया"
has been introduced, replacing the older"entity_deleted_successfully"
entry. This consolidation streamlines the deletion confirmation messages and appears consistent with similar changes in other localization files.public/locale/kn.json (1)
267-267
: Rename Localization Key for Deletion Success Message
The key has been renamed from"entity_deleted_successfully"
to"deleted_successfully"
, which promotes consistency across localization files. Please ensure that all application references to the old key are updated accordingly in case the UI or code still expects"entity_deleted_successfully"
.public/locale/ta.json (1)
266-266
: Key Consolidation for Deletion Success MessageThe new
"deleted_successfully": "{{name}} வெற்றிகரமாக நீக்கப்பட்டது"
key has been added to consolidate deletion feedback messages, replacing the older key"entity_deleted_successfully"
. Please ensure that all UI components or code references have been updated accordingly to use this new key for consistency across locales.public/locale/ml.json (1)
689-689
: LGTM! The translation for "deleted_successfully" is accurate and consistent.The translation "{{name}} വിജയകരമായി ഇല്ലാതാക്കി" correctly conveys "{{name}} successfully deleted" in Malayalam.
public/locale/en.json (3)
691-693
: Consistent Deletion Message Key Added
The new key"deleted_successfully": "{{name}} deleted successfully"
has been introduced (line 692), which is consistent with unifying deletion feedback messages across locales. Please make sure that all references previously using the removed"entity_deleted_successfully"
key are now updated appropriately.
949-951
: Improved OTP Error Message
The updated key"error_sending_otp": "Error while sending OTP, Please try again later"
(line 950) provides clearer feedback for OTP failures. Ensure that this error message is also synchronized with other language files for uniform user experience.
1-2331
: Verification of Removed Keys
It appears that keys such as"combine_files_pdf"
and"entity_deleted_successfully"
have been removed from this file, likely as part of streamlining the localization strings. Please verify that any UI components or modules that referenced these keys now use the new"deleted_successfully"
key (or other appropriate alternatives) to avoid any missing text errors.
src/CAREUI/misc/PrintPreview.tsx
Outdated
const { t } = useTranslation(); | ||
|
||
return ( | ||
<Page title={props.title}> | ||
<div className="mx-auto my-8 xl:w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden"> | ||
<div className="mx-auto my-8 w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden"> |
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.
🛠️ Refactor suggestion
Fixed width may cause layout issues on smaller screens.
Removing the xl:
prefix from w-[50rem]
makes the width fixed across all screen sizes. This could cause horizontal scrolling on mobile devices where the viewport width is less than 50rem (800px).
Consider making the width responsive:
- <div className="mx-auto my-8 w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden">
+ <div className="mx-auto my-8 w-full max-w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="mx-auto my-8 w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden"> | |
<div className="mx-auto my-8 w-full max-w-[50rem] border rounded-xl border-gray-200 shadow-2xl overflow-hidden"> |
@@ -218,7 +214,7 @@ const FilePreviewDialog = (props: FilePreviewProps) => { | |||
</a> | |||
</Button> | |||
)} | |||
<Button variant="outline" type="button" onClick={handleClose}> | |||
<Button variant="outline" type="button" onClick={onClose}> |
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.
Restore state reset functionality.
Replacing handleClose
with direct onClose
call skips important state reset logic. This could lead to stale state when the dialog is reopened.
- <Button variant="outline" type="button" onClick={onClose}>
+ <Button variant="outline" type="button" onClick={handleClose}>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Button variant="outline" type="button" onClick={onClose}> | |
<Button variant="outline" type="button" onClick={handleClose}> |
src/Providers/AuthUserProvider.tsx
Outdated
const updateRefreshToken = async (silent = false) => { | ||
const refresh = localStorage.getItem(LocalStorageKeys.refreshToken); | ||
|
||
if (!refresh) { | ||
return; | ||
} | ||
|
||
const { res, data } = await request(routes.token_refresh, { | ||
body: { refresh }, | ||
silent, | ||
}); | ||
|
||
if (res?.status !== 200 || !data) { | ||
localStorage.removeItem(LocalStorageKeys.accessToken); | ||
localStorage.removeItem(LocalStorageKeys.refreshToken); | ||
return; | ||
} | ||
|
||
localStorage.setItem(LocalStorageKeys.accessToken, data.access); | ||
localStorage.setItem(LocalStorageKeys.refreshToken, data.refresh); | ||
}; |
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.
🛠️ Refactor suggestion
Handle token refresh failure appropriately
In the updateRefreshToken
function (lines 166-186), when the token refresh fails (e.g., res?.status !== 200
or !data
), the tokens are removed from localStorage
, but the application doesn't log out the user or update the state accordingly. This might leave the app in an inconsistent state.
Consider invoking signOut()
or updating the application state to ensure the user is properly logged out when the token refresh fails.
src/Providers/AuthUserProvider.tsx
Outdated
useEffect(() => { | ||
if (tokenRefreshQuery.isError) { | ||
localStorage.removeItem(LocalStorageKeys.accessToken); | ||
localStorage.removeItem(LocalStorageKeys.refreshToken); | ||
if (!user) { | ||
return; | ||
} | ||
|
||
if (tokenRefreshQuery.data) { | ||
const { access, refresh } = tokenRefreshQuery.data; | ||
localStorage.setItem(LocalStorageKeys.accessToken, access); | ||
localStorage.setItem(LocalStorageKeys.refreshToken, refresh); | ||
} | ||
}, [tokenRefreshQuery.data, tokenRefreshQuery.isError]); | ||
|
||
const { mutateAsync: signIn, isPending: isAuthenticating } = useMutation({ | ||
mutationFn: mutate(routes.login), | ||
onSuccess: (data: JwtTokenObtainPair) => { | ||
setAccessToken(data.access); | ||
localStorage.setItem(LocalStorageKeys.accessToken, data.access); | ||
localStorage.setItem(LocalStorageKeys.refreshToken, data.refresh); | ||
queryClient.invalidateQueries({ queryKey: ["currentUser"] }); | ||
|
||
if (location.pathname === "/" || location.pathname === "/login") { | ||
navigate(getRedirectOr("/")); | ||
updateRefreshToken(true); | ||
setInterval( | ||
() => updateRefreshToken(), | ||
careConfig.auth.tokenRefreshInterval, | ||
); | ||
}, [user]); |
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.
Prevent memory leaks by clearing intervals in useEffect
The useEffect
hook starting at line 46 sets up a setInterval
to call updateRefreshToken
, but it doesn't clear the interval when the component unmounts or when the user
dependency changes. This can lead to memory leaks or multiple intervals running concurrently.
Apply this diff to fix the issue:
useEffect(() => {
if (!user) {
return;
}
updateRefreshToken(true);
- setInterval(
+ const intervalId = setInterval(
() => updateRefreshToken(),
careConfig.auth.tokenRefreshInterval,
);
+ return () => clearInterval(intervalId);
}, [user]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
if (tokenRefreshQuery.isError) { | |
localStorage.removeItem(LocalStorageKeys.accessToken); | |
localStorage.removeItem(LocalStorageKeys.refreshToken); | |
if (!user) { | |
return; | |
} | |
if (tokenRefreshQuery.data) { | |
const { access, refresh } = tokenRefreshQuery.data; | |
localStorage.setItem(LocalStorageKeys.accessToken, access); | |
localStorage.setItem(LocalStorageKeys.refreshToken, refresh); | |
} | |
}, [tokenRefreshQuery.data, tokenRefreshQuery.isError]); | |
const { mutateAsync: signIn, isPending: isAuthenticating } = useMutation({ | |
mutationFn: mutate(routes.login), | |
onSuccess: (data: JwtTokenObtainPair) => { | |
setAccessToken(data.access); | |
localStorage.setItem(LocalStorageKeys.accessToken, data.access); | |
localStorage.setItem(LocalStorageKeys.refreshToken, data.refresh); | |
queryClient.invalidateQueries({ queryKey: ["currentUser"] }); | |
if (location.pathname === "/" || location.pathname === "/login") { | |
navigate(getRedirectOr("/")); | |
updateRefreshToken(true); | |
setInterval( | |
() => updateRefreshToken(), | |
careConfig.auth.tokenRefreshInterval, | |
); | |
}, [user]); | |
useEffect(() => { | |
if (!user) { | |
return; | |
} | |
updateRefreshToken(true); | |
const intervalId = setInterval( | |
() => updateRefreshToken(), | |
careConfig.auth.tokenRefreshInterval, | |
); | |
return () => clearInterval(intervalId); | |
}, [user]); |
src/components/Users/UserSummary.tsx
Outdated
onSuccess: () => { | ||
toast.success(t("user_deleted_successfully")); | ||
setShowDeleteDialog(false); | ||
setshowDeleteDialog(false); |
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.
🛠️ Refactor suggestion
Update state setter function name in all occurrences.
Update all occurrences of setshowDeleteDialog
to follow the camelCase naming convention.
- setshowDeleteDialog(false);
+ setShowDeleteDialog(false);
- setshowDeleteDialog(false);
+ setShowDeleteDialog(false);
- setshowDeleteDialog(false);
+ setShowDeleteDialog(false);
- onClick={() => setshowDeleteDialog(true)}
+ onClick={() => setShowDeleteDialog(true)}
Also applies to: 53-53, 97-97, 188-188
src/components/Users/UserSummary.tsx
Outdated
import EditUserSheet from "@/pages/Organization/components/EditUserSheet"; | ||
import { UserBase } from "@/types/user/user"; | ||
|
||
export default function UserSummaryTab({ userData }: { userData?: UserBase }) { | ||
const { t } = useTranslation(); | ||
const [showDeleteDialog, setShowDeleteDialog] = useState(false); | ||
const [showDeleteDialog, setshowDeleteDialog] = useState(false); |
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.
🛠️ Refactor suggestion
Follow React state naming conventions.
The state setter function setshowDeleteDialog
should follow camelCase naming convention with the first letter of each subsequent word capitalized.
- const [showDeleteDialog, setshowDeleteDialog] = useState(false);
+ const [showDeleteDialog, setShowDeleteDialog] = useState(false);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const [showDeleteDialog, setshowDeleteDialog] = useState(false); | |
const [showDeleteDialog, setShowDeleteDialog] = useState(false); |
public/locale/ml.json
Outdated
"entered_in_error": "തെറ്റായി നൽകി", | ||
"entered_in_error": "പിശകിൽ പ്രവേശിച്ചു", |
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.
Remove duplicate key "entered_in_error"
There are two entries for the key "entered_in_error" with different translations. Remove one of them to maintain consistency.
Apply this diff to remove the duplicate:
- "entered_in_error": "തെറ്റായി നൽകി",
- "entered_in_error": "പിശകിൽ പ്രവേശിച്ചു",
+ "entered_in_error": "തെറ്റായി നൽകി"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"entered_in_error": "തെറ്റായി നൽകി", | |
"entered_in_error": "പിശകിൽ പ്രവേശിച്ചു", | |
"entered_in_error": "തെറ്റായി നൽകി" |
public/locale/ml.json
Outdated
"error_sending_otp": "OTP അയയ്ക്കുമ്പോൾ പിശക്, ദയവായി പിന്നീട് വീണ്ടും ശ്രമിക്കുക", | ||
"error_updating_encounter": "എൻകൗണ്ടർ അപ്ഡേറ്റ് ചെയ്യുന്നതിൽ പിശക്", |
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.
Remove duplicate key "error_sending_otp"
The key "error_sending_otp" appears twice in the file with identical translations. Remove one of them.
Apply this diff to remove the duplicate:
- "error_sending_otp": "OTP അയയ്ക്കുമ്പോൾ പിശക്, ദയവായി പിന്നീട് വീണ്ടും ശ്രമിക്കുക",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"error_sending_otp": "OTP അയയ്ക്കുമ്പോൾ പിശക്, ദയവായി പിന്നീട് വീണ്ടും ശ്രമിക്കുക", | |
"error_updating_encounter": "എൻകൗണ്ടർ അപ്ഡേറ്റ് ചെയ്യുന്നതിൽ പിശക്", | |
"error_updating_encounter": "എൻകൗണ്ടർ അപ്ഡേറ്റ് ചെയ്യുന്നതിൽ പിശക്", |
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
Proposed Changes
Update the date-fields by using onBlur event handlers, remove auto focus which was one of the reason to create the form with wrong values.
Screen.Recording.2025-02-06.at.1.10.59.PM.mov
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor
Style
Chores