Skip to content
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

Replace useTanStack with useQuery #10345

Conversation

AdityaJ2305
Copy link
Contributor

@AdityaJ2305 AdityaJ2305 commented Jan 31, 2025

Proposed Changes

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

  • Refactor
    • Streamlined data retrieval processes for file management, patient file handling, and resource updates by transitioning to the useQuery hook.
    • Improved caching and asynchronous data flows for a more consistent and reliable user experience.
    • Enhancements result in smoother interactions without altering core functionality.

Copy link
Contributor

coderabbitai bot commented Jan 31, 2025

Walkthrough

The changes refactor data fetching across several components by replacing the custom hook useTanStackQueryInstead with the useQuery hook from @tanstack/react-query. Each component now utilizes updated query keys, query functions, and direct data access. In addition, the FileUpload component introduces a queryClient instance to manage query invalidation. The overall behavior regarding when data is fetched remains the same.

Changes

Files Affected Change Summary
src/components/Files/FileBlock.tsx, src/components/Files/FileUpload.tsx Replaced the custom data fetching hook with useQuery, updated query key structures and query functions, and simplified data access. FileUpload now also instantiates a queryClient to allow invalidation of queries for active, archived, and consultation-related summaries.
src/components/Patient/FileUploadPage.tsx, src/components/Resource/ResourceDetailsUpdate.tsx Transitioned from the custom hook to useQuery for patient and resource details fetching. Updated the query key format to include unique identifiers. Added effect hooks in ResourceDetailsUpdate to update state when new data arrives, ensuring the UI reflects the latest fetched information.

Sequence Diagram(s)

sequenceDiagram
    participant C as Component
    participant Q as useQuery Hook
    participant API as API/Query Function
    participant UI as UI Renderer

    C->>Q: Initialize query with queryKey & queryFn
    Q->>API: Execute API call with structured parameters
    API-->>Q: Return fetched data
    Q-->>C: Provide data and loading state
    C->>UI: Render updated information
Loading

Possibly related PRs

  • Util function for cleaner usage of TanStack useQuery #9395: The changes in the main PR are related to the modifications in the data fetching logic using the useQuery hook from the @tanstack/react-query library, which aligns with the utility function introduced in the retrieved PR for cleaner usage of useQuery.
  • Fix: Resource Letter printing issue  #10230: The changes in the main PR and the retrieved PR are related as both involve the transition from the custom hook useTanStackQueryInstead to the useQuery hook from @tanstack/react-query for data fetching, specifically within components that manage resource details.
  • Patient files #9787: The changes in the main PR, which involve refactoring data fetching logic in the FileBlock.tsx and FileUpload.tsx components, are related to the changes in the retrieved PR that also refactor data fetching in the FilesTab component, as both PRs transition from a custom hook to the useQuery hook from @tanstack/react-query and modify how query parameters are structured.

Suggested labels

tested, reviewed

Suggested reviewers

  • rithviknishad
  • Jacobjeevan

Poem

I'm a bunny, hopping in code delight,
Upgraded queries make my heart feel light.
With a twitch of my nose and a happy little hop,
Data flows smoothly nonstop.
Cheers to changes that make our code top-notch! 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 739e706 and 0f9320f.

📒 Files selected for processing (1)
  • src/components/Patient/FileUploadPage.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Patient/FileUploadPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: cypress-run (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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 3875488
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/679fe689337efb00081f293c
😎 Deploy Preview https://deploy-preview-10345--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@AdityaJ2305 AdityaJ2305 marked this pull request as ready for review January 31, 2025 21:59
@AdityaJ2305 AdityaJ2305 requested a review from a team as a code owner January 31, 2025 21:59
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/components/Files/FileUpload.tsx (1)

137-147: Potential optimization of pagination
The logic for offset-based pagination works, but using infinite queries or another approach could simplify caching and reduce repeated fetches.

src/components/Common/UserAutocompleteFormField.tsx (1)

55-68: Conditional disabling logic
The check for empty data upon an unfilled query is helpful, but consider grouping these conditions for readability or adding a short comment for clarity.

src/components/Files/FileBlock.tsx (1)

1-1: LGTM! Well-structured query implementation.

The migration to useQuery follows React Query best practices with a well-organized query key structure. Consider adding error handling for failed queries.

 const { data: fileData } = useQuery({
   queryKey: ["file", { id: file.id, type: fileManager.type, associating_id }],
   queryFn: query(routes.retrieveUpload, {
     queryParams: { file_type: fileManager.type, associating_id },
     pathParams: { id: file.id || "" },
   }),
   enabled: filetype === "AUDIO" && !file.is_archived,
+  retry: false,
+  onError: (error) => {
+    console.error("Failed to fetch file data:", error);
+  },
 });

Also applies to: 16-16, 37-43

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f77a69 and 6e1b180.

📒 Files selected for processing (6)
  • src/CAREUI/misc/PaginatedList.tsx (1 hunks)
  • src/components/Common/UserAutocompleteFormField.tsx (4 hunks)
  • src/components/Files/FileBlock.tsx (4 hunks)
  • src/components/Files/FileUpload.tsx (4 hunks)
  • src/components/Patient/FileUploadPage.tsx (2 hunks)
  • src/components/Resource/ResourceDetailsUpdate.tsx (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: cypress-run (1)
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (19)
src/components/Files/FileUpload.tsx (9)

1-1: Good integration of the react-query library
This import properly sets up the usage of useQuery and useQueryClient.


23-23: Consistent import naming
Using a descriptive import name query to call the underlying request utility is clear and intuitive.


93-93: Using useQueryClient for cache invalidation
This will enable refetch and invalidation across queries for this component. Good approach!


123-135: Refetching multiple queries
Calling invalidateQueries for active, archived, and discharge_summary is a clear, decoupled approach to ensure fresh data. Nicely done!


150-160: Extend the approach used by activeFiles
The structure closely mirrors the active queries usage, ensuring consistency. Good job.


163-177: Conditionally enabled query
Using enabled: type === "consultation" is an efficient way to fetch discharge summaries only when needed. Well done.


180-185: Organized mapping of queries
Grouping queries by tab states keeps the code DRY and easy to manage. Nice approach.


188-188: Single aggregated loading state
Combining all queries' isLoading states might obscure partial loading conditions. Verify if partial loading states need to be distinguished.


194-194: Conditional discharge summary tab
Automatically showing the tab only if results exist is user-friendly and reduces clutter.

src/components/Patient/FileUploadPage.tsx (3)

1-1: UseQuery import
Importing useQuery from @tanstack/react-query is consistent with the query integration approach across the codebase.


7-7: Switching to new 'query' utility
Explicitly importing the query utility is consistent with the rest of the refactor using the same pattern.


17-22: Conditional patient data fetching
Using enabled: !!patientId ensures queries are only fetched when the patientId is valid, preventing unnecessary requests.

src/components/Common/UserAutocompleteFormField.tsx (5)

1-1: Import from react-query
Adopting useQuery aligns with the broader codebase transition to TanStack Query for state management.


14-14: Importing the query utility
Using the same query utility fosters consistency across the application.


37-37: Rename for clarity
Renaming from query to queryParam clarifies its purpose as a variable holding the user's search text.


40-50: useQuery for user list
The queryKey and queryFn structure is well-defined, ensuring the user data is fetched with minimal overhead.


101-102: Synchronize query param and loading
onQuery updates the queryParam, while isLoading accurately reflects query status. Good approach to unify search with the new API.

src/components/Files/FileBlock.tsx (1)

89-89: LGTM! Safe data access with optional chaining.

The use of optional chaining prevents potential runtime errors when accessing the URL.

src/components/Resource/ResourceDetailsUpdate.tsx (1)

1-1: LGTM! Clean import updates.

The imports are correctly updated to support the migration to React Query.

Also applies to: 4-4, 30-30

src/CAREUI/misc/PaginatedList.tsx Outdated Show resolved Hide resolved
src/components/Resource/ResourceDetailsUpdate.tsx Outdated Show resolved Hide resolved
@AdityaJ2305
Copy link
Contributor Author

AdityaJ2305 commented Feb 1, 2025

@rithviknishad @Jacobjeevan this PR is ready for review.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (4)
src/components/Files/FileUpload.tsx (4)

124-136: Consider optimizing query invalidation.

While the current implementation works correctly, you could simplify it using partial key matching.

 const refetchAll = () => {
-  queryClient.invalidateQueries({
-    queryKey: ["viewUpload", "active", type, associatedId],
-  });
-  queryClient.invalidateQueries({
-    queryKey: ["viewUpload", "archived", type, associatedId],
-  });
-  if (type === "consultation") {
-    queryClient.invalidateQueries({
-      queryKey: ["viewUpload", "discharge_summary", associatedId],
-    });
-  }
+  queryClient.invalidateQueries({
+    queryKey: ["viewUpload"],
+    predicate: (query) => 
+      query.queryKey[2] === type || 
+      (type === "consultation" && query.queryKey[2] === "discharge_summary"),
+  });
 };

138-148: Add error handling for the active files query.

Consider handling query errors to improve user experience.

 const { data: activeFiles, isLoading: activeFilesLoading } = useQuery({
   queryKey: ["viewUpload", "active", type, associatedId, offset],
   queryFn: query(routes.viewUpload, {
     queryParams: {
       file_type: type,
       associating_id: associatedId,
       is_archived: false,
       limit: RESULTS_PER_PAGE_LIMIT,
       offset: offset,
     },
   }),
+  onError: (error) => {
+    // Handle error (e.g., show toast notification)
+    console.error("Failed to fetch active files:", error);
+  },
 });

164-178: Enhance type safety for discharge summary query.

Consider adding a type guard for the consultation type check.

+const isConsultationType = (type: string): type is 'consultation' => type === 'consultation';

 const { data: dischargeSummary, isLoading: dischargeSummaryLoading } =
   useQuery({
     queryKey: ["viewUpload", "discharge_summary", associatedId, offset],
     queryFn: query(routes.viewUpload, {
       queryParams: {
         file_type: "discharge_summary",
         associating_id: associatedId,
         is_archived: false,
         limit: RESULTS_PER_PAGE_LIMIT,
         offset: offset,
         silent: true,
       },
     }),
-    enabled: type === "consultation",
+    enabled: isConsultationType(type),
   });

181-186: Consider memoizing the queries object.

To prevent unnecessary re-renders, consider using useMemo for the queries object.

+const queries = useMemo(() => ({
-const queries = {
   UNARCHIVED: { data: activeFiles, isLoading: activeFilesLoading },
   ARCHIVED: { data: archivedFiles, isLoading: archivedFilesLoading },
   DISCHARGE_SUMMARY: {
     data: dischargeSummary,
     isLoading: dischargeSummaryLoading,
   },
-};
+}), [activeFiles, activeFilesLoading, archivedFiles, archivedFilesLoading, dischargeSummary, dischargeSummaryLoading]);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a5f33e and 262126b.

📒 Files selected for processing (1)
  • src/components/Files/FileUpload.tsx (4 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Files/FileUpload.tsx (2)

1-1: LGTM! Clean migration to @tanstack/react-query.

The import changes and query client setup align with the migration requirements.

Also applies to: 24-24, 94-94


151-161: Apply similar error handling to archived files query.

Similar to the active files query, consider adding error handling here.

src/components/Common/UserAutocompleteFormField.tsx Outdated Show resolved Hide resolved
src/components/Common/UserAutocompleteFormField.tsx Outdated Show resolved Hide resolved
src/components/Patient/FileUploadPage.tsx Outdated Show resolved Hide resolved
src/components/Resource/ResourceDetailsUpdate.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
src/components/Patient/FileUploadPage.tsx (1)

17-23: Consider adding type annotations for better type safety.

The query configuration correctly follows the migration guidelines. However, we could enhance type safety.

Consider adding explicit type annotations:

-  const { data: patient } = useQuery({
+  const { data: patient } = useQuery<PatientType>({
     queryKey: ["patient", patientId],
     queryFn: query(routes.getPatient, {
       pathParams: { id: patientId },
     }),
     enabled: !!patientId,
   });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 262126b and 2906b3a.

📒 Files selected for processing (1)
  • src/components/Patient/FileUploadPage.tsx (2 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Patient/FileUploadPage.tsx (2)

1-1: LGTM! Imports are correctly updated for React Query migration.

The imports align with the migration requirements, bringing in both the useQuery hook and the query utility function.

Also applies to: 7-7


18-18: Query key is correctly structured.

Addressing the previous review comment about facilityId: The query key correctly uses only ["patient", patientId] as it's fetching patient data. The facilityId is appropriately used only for navigation and breadcrumbs, not as part of the cache key.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
src/components/Patient/FileUploadPage.tsx (1)

18-24: Consider destructuring isLoading if loading states are needed.

The useQuery implementation follows best practices with proper query key structure and query function. However, if loading states are needed for UI feedback, consider destructuring isLoading from useQuery.

-  const { data: patient } = useQuery<PatientModel>({
+  const { data: patient, isLoading } = useQuery<PatientModel>({
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2906b3a and f0e5da2.

📒 Files selected for processing (1)
  • src/components/Patient/FileUploadPage.tsx (2 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Test
  • GitHub Check: cypress-run (1)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: CodeQL-Build
🔇 Additional comments (2)
src/components/Patient/FileUploadPage.tsx (2)

Line range hint 1-16: LGTM! Import statements and type declarations are well-structured.

The imports correctly reflect the migration from useTanStackQueryInstead to useQuery, and the component props are properly typed.


Line range hint 26-46: LGTM! Component rendering logic is clean and maintainable.

The Page and FileUpload components are properly configured with appropriate props and conditional logic.

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, a minor thing

src/components/Patient/FileUploadPage.tsx Outdated Show resolved Hide resolved
@nihal467
Copy link
Member

nihal467 commented Feb 3, 2025

LGTM

@rithviknishad rithviknishad merged commit 00fbb56 into ohcnetwork:develop Feb 3, 2025
15 checks passed
Copy link

github-actions bot commented Feb 3, 2025

@AdityaJ2305 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! 🙌

@coderabbitai coderabbitai bot mentioned this pull request Feb 3, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed reviewed by a core member tested
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants