Improvements for fetching data in the dApp #914

kkosiorowska · 2024-12-04T09:32:56Z

Closes AENG-18

This PR improves code for fetching data in the dApp. What has been done

Made each query a separate hook. This approach will allow us to use queries whenever we want. Additionally, the code becomes more clear.
Fixed for checking the status of fetched data, improvement for useIsFetchedWalletData. The previous solution was wrong because useIsFetching returns the number of queries that our application is loading or fetching in the background. We should check the fetch status for all the data we are interested in.
Fixed a bug from AENG-18. The problem was solved by using the @tanstack/react-query package to fetch activities. The data will be reset together at the same time.

UI

Before

Screen.Recording.2024-12-04.at.10.50.42.mov

After

Screen.Recording.2024-12-04.at.10.46.55.mov

Testing

Use React Query dev tools for testing. They help visualize all of the inner workings of React Query.

Screen.Recording.2024-12-04.at.11.11.18.mov

What should be tested before merge

Make sure that the issue described in AENG-18 is no longer exist.
The skeleton of data loading works correctly.

Check if the data is fetched at the start of the app:

TVL
Total Mezo points
Total Acre points

Check if the data is fetched when the wallet is connected:

Bitcoin balance
Bitcoin position
Activities
User Acre points

Check if the data is reset when the wallet is disconnected:

Bitcoin balance
Bitcoin position
Activities
User Acre points

Check if the data is fetched in the time interval. It is currently every 5 minutes:

Checks if the data is updated after the deposit is executed:

Activities
Bitcoin balance

Checks if the data is updated after the withdrawal is executed:

Activities
Bitcoin position

Let's create a dedicated folder for tanstack queries make each query a separate hook. This approach will allow us to use queries whenever we want.Additionally, the code becomes more clear.

The previous solution was wrong because `useIsFetching` returns the number of the queries that your application is loading or fetching in the background. We should check the fetch status for all the data we are interested in.

netlify · 2024-12-04T09:33:15Z

✅ Deploy Preview for acre-dapp ready!

Name	Link
🔨 Latest commit	`b69b1c0`
🔍 Latest deploy log	https://app.netlify.com/sites/acre-dapp/deploys/675ac1f2202a5d00087ca9e5
😎 Deploy Preview	https://deploy-preview-914--acre-dapp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

netlify · 2024-12-04T09:33:16Z

✅ Deploy Preview for acre-dapp-testnet ready!

Name	Link
🔨 Latest commit	`b69b1c0`
🔍 Latest deploy log	https://app.netlify.com/sites/acre-dapp-testnet/deploys/675ac1f2202a5d00087ca9e1
😎 Deploy Preview	https://deploy-preview-914--acre-dapp-testnet.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

linear · 2024-12-04T09:52:07Z

AENG-18 Resets activities state after disconnection

dapp/src/hooks/useTrackActivities.ts

r-czajkowski · 2024-12-05T13:55:09Z

dapp/src/hooks/tanstack-query/useBitcoinPositionQuery.ts

I'm ok with query too but imo it's unnecessary. W/o query we had a bit of abstraction I mean the name did not suggest which lib is used for fetching data. For example, if we decided to replace the @tanstack/react-query library, the word query would probably no longer fit. Just a thought.

r-czajkowski

I tested the code on testnet and it looks like it works as expected 👍. It would be great if Kamil could take a look at it.

`setQueriesData` is a synchronous function that can be used to immediately update cached data of multiple queries by using matching the query key. With the new approach, we can remove activities from redux.

The name should not suggest which lib is used for fetching data.The current solution is more abstract.

kpyszkowski

Summarizing my review:
I wouldn't like to use terms like aggregation or enhancement because it doesn't say anything descriptive.

Respecting your approach of splitting data querying I suggest the following change:

Keep previous hooks structure and names so useStatistics, useAcrePoints etc.
Add new "subcategory" of hooks called queries. From that point we can have hooks such as @/hooks/queries/useStatisticsQuery or @/hooks/queries/useAcrePointsQuery.
The term query sounds quite familiar since we are used to querying paradigm introduced by Tanstack Query library.

Please let me know what do you think.

dapp/src/hooks/useEnhancedStatistics.ts

dapp/src/hooks/useAggregatedAcrePointsData.ts

kkosiorowska · 2024-12-11T08:56:25Z

Add new "subcategory" of hooks called queries. From that point we can have hooks such as @/hooks/queries/useStatisticsQuery or @/hooks/queries/useAcrePointsQuery.
The term query sounds quite familiar since we are used to querying paradigm introduced by Tanstack Query library.

I had a similar opinion. But I share Rafał's opinion here that the name should be more abstract. #914 (comment)

kkosiorowska added 3 commits December 3, 2024 15:52

Use tanstack query to fetch activities

edc5a31

Refactor tanstack queries for dApp

780d6eb

Let's create a dedicated folder for tanstack queries make each query a separate hook. This approach will allow us to use queries whenever we want.Additionally, the code becomes more clear.

Fix for checking the status of fetched data

1d4f022

The previous solution was wrong because `useIsFetching` returns the number of the queries that your application is loading or fetching in the background. We should check the fetch status for all the data we are interested in.

kkosiorowska added the 🎨 dApp dApp label Dec 4, 2024

kkosiorowska self-assigned this Dec 4, 2024

Merge branch 'main' of github.com:thesis/acre into reset-activities

fd4a67a

Add comment: why activities are stored in redux

9ae9218

kkosiorowska marked this pull request as ready for review December 4, 2024 11:16

kkosiorowska requested review from kpyszkowski and r-czajkowski December 4, 2024 11:17

r-czajkowski reviewed Dec 5, 2024

View reviewed changes

r-czajkowski reviewed Dec 6, 2024

View reviewed changes

kkosiorowska added 3 commits December 6, 2024 09:43

Merge branch 'main' of github.com:thesis/acre into reset-activities

77d98cc

Use queryClient.setQueriesData to update activities

c2ccf9b

`setQueriesData` is a synchronous function that can be used to immediately update cached data of multiple queries by using matching the query key. With the new approach, we can remove activities from redux.

Remove query suffix from hooks

bf3b3ab

The name should not suggest which lib is used for fetching data.The current solution is more abstract.

kkosiorowska requested a review from r-czajkowski December 6, 2024 13:40

r-czajkowski previously approved these changes Dec 9, 2024

View reviewed changes

kkosiorowska added 2 commits December 9, 2024 14:34

Merge branch 'main' of github.com:thesis/acre into reset-activities

060d11a

Move logic from wallet selectors to utils for activities

d2128ca

kkosiorowska dismissed r-czajkowski’s stale review via d2128ca December 9, 2024 13:41

kkosiorowska requested a review from r-czajkowski December 9, 2024 13:42

kpyszkowski reviewed Dec 10, 2024

View reviewed changes

dapp/src/hooks/useEnhancedStatistics.ts Outdated Show resolved Hide resolved

dapp/src/hooks/useAggregatedAcrePointsData.ts Show resolved Hide resolved

kkosiorowska added 3 commits December 11, 2024 09:38

Create one hook for statistics

0d5d3e8

Merge branch 'main' of github.com:thesis/acre into reset-activities

fd7e983

Fix failed build

a24af4e

kkosiorowska requested a review from kpyszkowski December 11, 2024 09:01

kpyszkowski approved these changes Dec 11, 2024

View reviewed changes

kpyszkowski enabled auto-merge December 12, 2024 10:44

Merge branch 'main' into reset-activities

b69b1c0

kpyszkowski merged commit f0a59c4 into main Dec 12, 2024
26 of 28 checks passed

kpyszkowski deleted the reset-activities branch December 12, 2024 11:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements for fetching data in the dApp #914

Improvements for fetching data in the dApp #914

kkosiorowska commented Dec 4, 2024 •

edited

Loading

netlify bot commented Dec 4, 2024 •

edited

Loading

netlify bot commented Dec 4, 2024 •

edited

Loading

linear bot commented Dec 4, 2024

r-czajkowski Dec 5, 2024

kkosiorowska Dec 6, 2024

r-czajkowski left a comment

kpyszkowski left a comment •

edited

Loading

kkosiorowska commented Dec 11, 2024

Improvements for fetching data in the dApp #914

Improvements for fetching data in the dApp #914

Conversation

kkosiorowska commented Dec 4, 2024 • edited Loading

UI

Testing

What should be tested before merge

netlify bot commented Dec 4, 2024 • edited Loading

✅ Deploy Preview for acre-dapp ready!

netlify bot commented Dec 4, 2024 • edited Loading

✅ Deploy Preview for acre-dapp-testnet ready!

linear bot commented Dec 4, 2024

r-czajkowski Dec 5, 2024

Choose a reason for hiding this comment

kkosiorowska Dec 6, 2024

Choose a reason for hiding this comment

r-czajkowski left a comment

Choose a reason for hiding this comment

kpyszkowski left a comment • edited Loading

Choose a reason for hiding this comment

kkosiorowska commented Dec 11, 2024

kkosiorowska commented Dec 4, 2024 •

edited

Loading

netlify bot commented Dec 4, 2024 •

edited

Loading

netlify bot commented Dec 4, 2024 •

edited

Loading

kpyszkowski left a comment •

edited

Loading