Adds query.debounced and updated queries that require debouncing
#9865
Conversation
WalkthroughThis pull request introduces comprehensive improvements to the application's data fetching and internationalization strategies. The primary changes include implementing a debounced query mechanism across multiple components, removing the custom 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. |
Deploying care-fe with
|
| Latest commit: |
6a4d781
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f0dc74df.care-fe.pages.dev |
| Branch Preview URL: | https://rithviknishad-feat-query-deb.care-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Questionnaire/QuestionnaireSearch.tsx (1)
Line range hint
44-56: Remove redundant client-side filtering.The component currently performs filtering twice:
- Server-side via query params (
title: search)- Client-side via
.filter()on the resultsThis is redundant and could lead to inconsistent results.
Remove the client-side filtering since the server is already handling it:
- const filteredQuestionnaires = (questionnaires?.results ?? []).filter( - (item: QuestionnaireDetail) => - item.title.toLowerCase().includes(search.toLowerCase()), - ); + const filteredQuestionnaires = questionnaires?.results ?? [];
🧹 Nitpick comments (7)
src/components/Questionnaire/ValueSetSelect.tsx (1)
51-54: Consider adding minimum character length check for query optimizationWhile the debounced query implementation looks good, consider adding a minimum character length check before triggering the query. The UI already shows a message for a 3-character minimum, but the query will still execute for shorter inputs.
queryFn: query.debounced(routes.valueset.expand, { pathParams: { system }, - body: { count, search: search + searchPostFix }, + body: search.length >= 3 ? { count, search: search + searchPostFix } : null, }),src/pages/Organization/components/OrganizationSelector.tsx (1)
Line range hint
61-70: Consider adding error boundary for API failuresWhile the debounced query implementation is correct, there's no error handling for API failures.
queryFn: query.debounced(organizationApi.list, { queryParams: { parent: selectedLevels[selectedLevels.length - 1]?.id, org_type: "govt", limit: 200, name: searchQuery || undefined, }, ...headers, }), + retry: 2, + onError: (error) => { + console.error('Failed to fetch organizations:', error); + }, enabled: selectedLevels.length > 0,src/pages/FacilityOrganization/FacilityOrganizationView.tsx (1)
Line range hint
43-51: Consider caching strategy for debounced queriesWhile the debounced query implementation is correct, consider adding caching configuration for better performance.
queryFn: query.debounced(routes.facilityOrganization.list, { pathParams: { facilityId, organizationId: id }, queryParams: { parent: id, offset: (page - 1) * limit, limit, name: searchQuery || undefined, }, }), + staleTime: 30000, // Consider data fresh for 30 seconds + cacheTime: 300000, // Keep data in cache for 5 minutessrc/pages/Organization/OrganizationView.tsx (1)
Line range hint
35-41: Consider optimistic updates for better UXWhile the debounced query is implemented correctly, consider adding optimistic updates for a smoother user experience.
queryFn: query.debounced(organizationApi.list, { queryParams: { parent: id, offset: (page - 1) * limit, limit, name: searchQuery || undefined, }, }), + optimisticData: (old) => ({ + ...old, + results: old?.results?.filter(item => + item.name.toLowerCase().includes(searchQuery.toLowerCase()) + ) + }),src/pages/Organization/OrganizationPatients.tsx (1)
Line range hint
37-46: Consider conditional debouncing based on filter type.While the implementation of
query.debouncedis correct, consider that not all filter parameters might need debouncing. For instance, pagination changes could be executed immediately while search inputs could be debounced.You could optimize this by using different query functions based on the type of filter being applied:
- queryFn: query.debounced(organizationApi.listPatients, { + queryFn: (qParams.name || qParams.phone_number) + ? query.debounced(organizationApi.listPatients, { pathParams: { id }, queryParams: { ...(organization?.org_type === "govt" && { organization: id }), page: qParams.page, limit: resultsPerPage, offset: (qParams.page - 1) * resultsPerPage, ...advancedFilter.filter, }, + }) + : query(organizationApi.listPatients, { + // same params + }),src/components/Patient/PatientIndex.tsx (2)
98-105: Consider adding error handling for the patient search queryCurrently, the code does not handle errors from the
useQuery. Incorporating theisErrorstate can enhance user experience by providing feedback when the patient search fails.You can modify the code as follows:
const { data: patientList, isFetching } = useQuery({ queryKey: ["patient-search", facilityId, phoneNumber], queryFn: query.debounced(routes.searchPatient, { body: { phone_number: parsePhoneNumber(phoneNumber) || "", }, }), enabled: !!phoneNumber, + onError: () => { + toast.error(t("error_fetching_patients")); + }, });Alternatively, destructure
isErrorfromuseQueryand update the render logic to display an error message:+ const { data: patientList, isFetching, isError } = useQuery({ ... }); --- - {isFetching || !patientList ? ( + {isError ? ( + <div className="flex items-center justify-center h-[200px]"> + <p className="text-red-500">{t("error_fetching_patients")}</p> + </div> + ) : isFetching || !patientList ? ( // existing loading and no results logic ) : ( // existing results display logic )}
157-157: Update conditional rendering to include error stateIncluding
isErrorin the rendering conditions will provide users with feedback in case of a failed patient search.Modify the conditional rendering as follows:
{ !!phoneNumber && ( <> - {isFetching || !patientList ? ( + {isError ? ( + <div className="flex items-center justify-center h-[200px]"> + <p className="text-red-500">{t("error_fetching_patients")}</p> + </div> + ) : isFetching || !patientList ? ( // existing loading and no results logic ) : ( // existing results display logic )} </> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
public/locale/en.json(10 hunks)src/Utils/request/README.md(1 hunks)src/Utils/request/query.ts(2 hunks)src/components/Common/SearchByMultipleFields.tsx(1 hunks)src/components/Common/UserSelector.tsx(1 hunks)src/components/Patient/PatientIndex.tsx(4 hunks)src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx(5 hunks)src/components/Questionnaire/QuestionnaireSearch.tsx(4 hunks)src/components/Questionnaire/ValueSetSelect.tsx(1 hunks)src/hooks/useDebouncedState.tsx(0 hunks)src/pages/Encounters/EncounterList.tsx(5 hunks)src/pages/Facility/FacilitiesPage.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationView.tsx(4 hunks)src/pages/Organization/OrganizationFacilities.tsx(4 hunks)src/pages/Organization/OrganizationPatients.tsx(4 hunks)src/pages/Organization/OrganizationView.tsx(4 hunks)src/pages/Organization/components/OrganizationSelector.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/useDebouncedState.tsx
🧰 Additional context used
📓 Learnings (1)
src/pages/Organization/components/OrganizationSelector.tsx (1)
Learnt from: i0am0arunava
PR: ohcnetwork/care_fe#9801
File: src/pages/Organization/OrganizationFacilities.tsx:69-75
Timestamp: 2025-01-09T08:56:20.178Z
Learning: When using useDebouncedState with query parameters in TypeScript, ensure the debounced state maintains the same type as the original state (QueryParam). Avoid splitting into separate string states as it breaks type safety.
🔇 Additional comments (21)
src/components/Questionnaire/ValueSetSelect.tsx (1)
47-47: LGTM! State management simplifiedThe change from
useDebouncedStatetouseStateis appropriate since debouncing is now handled at the query level rather than component level.src/pages/Facility/FacilitiesPage.tsx (1)
36-36: LGTM! Good use of debounced query for search functionality.The migration to
query.debouncedis appropriate here as it helps prevent excessive API calls during rapid search input changes.src/components/Common/UserSelector.tsx (1)
46-46: LGTM! Good refactor to use query-level debouncing.The removal of
useDebouncedStatein favor of regularuseStatecombined withquery.debouncedis a good pattern that centralizes the debouncing logic at the query level.Also applies to: 50-50
src/components/Questionnaire/QuestionnaireSearch.tsx (1)
3-3: LGTM! Good addition of i18n support.The integration of react-i18next for internationalization is well implemented.
Also applies to: 36-36, 76-76, 103-103
src/pages/Organization/components/OrganizationSelector.tsx (2)
31-31: LGTM: Clean state management transitionThe transition from
useDebouncedStatetouseStateforsearchQueryis clean and aligns with the new debounced query pattern.
Line range hint
43-49: LGTM: Proper implementation of debounced queryThe implementation correctly uses
query.debouncedfor the API call, which will help prevent excessive requests during rapid user input.src/pages/FacilityOrganization/FacilityOrganizationView.tsx (2)
27-28: LGTM: Clean i18n integrationThe useTranslation hook is properly initialized.
58-58: LGTM: Proper i18n usageTranslation key is correctly implemented for the header text.
src/pages/Organization/OrganizationView.tsx (2)
27-28: LGTM: Clean i18n integrationThe useTranslation hook is properly initialized.
134-135: LGTM: Proper i18n implementation for dynamic contentThe translation implementation correctly handles dynamic content with the searchQuery parameter.
src/pages/Organization/OrganizationFacilities.tsx (3)
31-32: LGTM: Clean i18n integrationThe useTranslation hook is properly initialized.
59-59: LGTM: Consistent i18n implementationTranslation keys are consistently used for both the header and no results message.
Also applies to: 112-112
Line range hint
38-47: Verify debounced query compatibility with useFiltersThe implementation of query.debounced with useFilters needs verification to ensure proper interaction between debouncing and filter updates.
✅ Verification successful
Implementation pattern is consistent with codebase standards
The combination of useFilters with query.debounced is a well-established pattern used successfully across multiple components in the codebase, including FacilitiesPage, EncounterList, and OrganizationPatients. The implementation in OrganizationFacilities.tsx follows the same proven pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of useFilters with debounced queries rg -A 5 "useFilters.*query\.debounced"Length of output: 40
Script:
#!/bin/bash # Search for useFilters hook usage echo "=== useFilters Usage ===" rg "useFilters" -A 3 # Search for query.debounced usage echo -e "\n=== query.debounced Usage ===" rg "query\.debounced" -A 3 # Search for filter-related patterns with debounced echo -e "\n=== Filter Patterns with Debounced ===" rg "filter.*debounced|debounced.*filter" -A 3Length of output: 11971
src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx (2)
Line range hint
57-62: LGTM! Proper implementation of debounced query for organization search.The implementation correctly uses
query.debouncedfor the organization list query, which will help prevent excessive API calls during rapid user input.
114-114: Well-structured internationalization implementation.Good job implementing i18n translations consistently across the component. The translations cover all user-facing strings, making the component fully localizable.
Also applies to: 120-122, 129-131, 155-155, 170-170
src/components/Common/SearchByMultipleFields.tsx (1)
160-163: Verify search behavior after removing manual debouncing.The removal of setTimeout-based debouncing in favor of immediate updates is aligned with using
query.debounced. However, please verify that this change doesn't affect the user experience in components using this common search field.✅ Verification successful
Debouncing implementation change is verified and aligned with codebase standards
The removal of manual setTimeout-based debouncing is correct as the codebase standardizes on
query.debouncedutility with a 500ms default interval for all search-related API calls. This change maintains consistent behavior with other search implementations across the application.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all components using SearchByMultipleFields echo "Components using SearchByMultipleFields:" rg -l "SearchByMultipleFields" --type tsx # Check if these components are using query.debounced echo "\nVerifying debounced query usage in parent components:" rg "query.debounced" -C 5 $(rg -l "SearchByMultipleFields" --type tsx)Length of output: 14101
src/Utils/request/README.md (1)
95-123: Excellent documentation for the new debounced query feature.The documentation is clear, concise, and includes a practical example. The explanation of the debounce interval and the use case for search inputs provides good context for developers.
src/pages/Encounters/EncounterList.tsx (2)
183-183: LGTM! Debounced query implementation aligns with PR objectives.The change from
querytoquery.debouncedwill help prevent excessive API calls during rapid user input, improving performance.
241-241: Verify the translation keys exist in the locale file.The internationalization changes look good, but let's verify that all the required translation keys are present in the locale file.
Also applies to: 264-264, 271-271, 298-298, 319-319
✅ Verification successful
All translation keys are present and properly configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if all the required translation keys exist in the locale file # Define the keys to check keys=("encounters" "search" "search_encounters" "clear_search" "priority") # Read the locale file and check for each key for key in "${keys[@]}"; do echo "Checking key: $key" rg "\"$key\":" public/locale/en.json doneLength of output: 881
Script:
#!/bin/bash # Get the content of the specified lines to check for translation keys rg -A 1 -B 1 --line-number "^.*t\(.*\).*$" src/pages/Encounters/EncounterList.tsxLength of output: 1427
Script:
#!/bin/bash # Check for dynamic translation keys echo "Checking encounter status translations:" rg '"encounter_status__' public/locale/en.json echo -e "\nChecking encounter class translations:" rg '"encounter_class__' public/locale/en.json echo -e "\nChecking encounter priority translations:" rg '"encounter_priority__' public/locale/en.jsonLength of output: 1888
public/locale/en.json (1)
331-331: LGTM! Translation keys are well-structured and consistent.The new translation keys follow the existing patterns and conventions. They are properly organized and their values are descriptive.
Also applies to: 338-338, 558-558, 1175-1176, 1277-1279, 1292-1292, 1305-1305, 1632-1632, 1686-1686, 1688-1688
src/components/Patient/PatientIndex.tsx (1)
98-105: Ensurequery.debouncedintegrates correctly withuseQueryVerify that the custom
query.debouncedfunction works seamlessly withuseQueryand correctly debounces the API calls.To confirm, you can review the implementation of
query.debouncedor run the following script:✅ Verification successful
query.debouncedintegration is correctly implementedThe implementation in PatientIndex.tsx follows the documented pattern and matches the official implementation. The debounced query functionality is properly integrated with useQuery and is working as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check usage of `query.debounced` with `useQuery` # Expected: `query.debounced` should be properly used within `useQuery` without issues. rg 'useQuery.*query\.debounced'Length of output: 33
Script:
#!/bin/bash # First, find the implementation of query.debounced rg -A 5 "query.*debounced.*=" || rg -A 5 "debounced.*=.*function" # Then look for any usage of query.debounced rg "query\.debounced" # Also search for useQuery patterns to understand the integration rg "useQuery.*queryFn:"Length of output: 1835
Script:
#!/bin/bash # Check the implementation and documentation cat src/Utils/request/README.md cat src/Utils/request/query.tsLength of output: 14279
|
👋 Hi, @rithviknishad, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
Line range hint
1-2000: Consider adding error state translations for the new features.While the happy path translations are well covered, consider adding corresponding error state messages for:
- Organization management failures
- Questionnaire operations failures
- Search operation failures
Example additions:
{ + "organization_create_error": "Failed to create organization. Please try again.", + "organization_update_error": "Failed to update organization. Please try again.", + "questionnaire_create_error": "Failed to create questionnaire. Please try again.", + "search_error": "Failed to perform search. Please try again." }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
public/locale/en.json (4)
331-331: LGTM! Organization management translations are well-structured.The added translations for organization management features are clear, consistent, and follow the existing naming conventions.
Also applies to: 1193-1194, 1296-1298, 1324-1324
338-338: LGTM! Questionnaire management translations are clear.The added translations for questionnaire management are user-friendly and maintain consistency with the application's terminology.
Also applies to: 1311-1311
561-561: LGTM! Search functionality translations support the debounced search feature.The added translations for search operations align well with the PR's objective of implementing debounced API calls for search inputs.
Also applies to: 1675-1675
1658-1658: LGTM! Loading state translation follows best practices.The "saving" translation key follows the common UI pattern for indicating loading states during operations.
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
rithviknishad/feat/query-debounce
|
| Run status |
|
| Run duration | 01m 35s |
| Commit |
|
| Committer | Rithvik Nishad |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
6
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Utils/request/README.md (1)
95-123: Consider enhancing the documentation.The documentation clearly explains the usage but could benefit from mentioning the automatic cleanup mechanism through AbortSignal. This would help developers understand why they don't need to handle cleanup manually.
Add this note after line 122:
The debounced query will wait for the specified interval after the last call before executing the request, helping to reduce unnecessary API calls during rapid user input. + +The implementation leverages TanStack Query's built-in AbortSignal, ensuring that when a new query starts, any pending debounced queries are automatically cleaned up. No manual cleanup is required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Utils/request/README.md(1 hunks)src/Utils/request/query.ts(2 hunks)src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx(6 hunks)
🧰 Additional context used
📓 Learnings (1)
src/Utils/request/query.ts (1)
Learnt from: rithviknishad
PR: ohcnetwork/care_fe#9865
File: src/Utils/request/query.ts:86-94
Timestamp: 2025-01-09T17:13:58.516Z
Learning: In the query.debounced implementation, using async sleep with AbortSignal from useQuery is the preferred approach as it leverages React Query's built-in AbortController. When a new query starts, the previous signal gets aborted during sleep, preventing unnecessary API calls without requiring additional cleanup.
🔇 Additional comments (4)
src/Utils/request/query.ts (2)
6-6: LGTM! Effective use of sleep with AbortSignal.The implementation leverages React Query's built-in AbortController, ensuring that when a new query starts, the previous signal gets aborted during sleep, preventing unnecessary API calls.
Also applies to: 106-106
72-109: Well-documented and clean implementation!The debounced query implementation effectively:
- Leverages TanStack Query's AbortSignal for automatic cleanup
- Provides configurable debounce interval with a sensible default
- Maintains type safety through generic parameters
src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx (2)
Line range hint
58-64: LGTM! Proper implementation of debounced search.The implementation correctly:
- Uses query.debounced for organization search
- Includes searchQuery in queryKey for proper cache invalidation
- Enables the query only when needed
4-4: LGTM! Comprehensive internationalization.All user-facing strings have been properly internationalized using the t() function from react-i18next.
Also applies to: 115-225
|
@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! 🙌 |
Docs
References
Debounced Queries
For search inputs or other scenarios requiring debounced API calls, use
query.debounced:The debounced query will wait for the specified interval after the last call before executing the request, helping to reduce unnecessary API calls during rapid user input.
How it works?
useQuerypasses AbortSignal to thequeryFn. When useQuery triggers a second request, the previous queries signal would be cancelled. Since the debounced query waits for a specified duration before firing thefetchcall, the previous inflight query would get cancelled within this time frame when the new request comes up, since the same AbortSignal is passed down to thefetchcall.Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
Internationalization
Performance Improvements
New Features
Localization