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

[ABW-2271] Remove spurious portfolio reloading #829

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

kugel3
Copy link
Contributor

@kugel3 kugel3 commented Oct 9, 2023

Description

The wallet reloads the account portfolios too often. This PR removes most of the unnecessary reloads, and even more cases where an already fetched portfolio is recreated without fetching.

Notes

The next step is probably to move to an architecture where e.g. AccountPortfolioClient emits "events" like portfolioChange(AccountAddress) which we subscribe to, instead of a currentValueSubject of the portfolios. This will make it possible to distinguish between the first reading of an already fetched portfolio, and subsequent updates to the data. This is important because .task is called not just when a view is first created, but every time it comes back into view, causing us to set up subscriptions too often.

How to test

Try everything related to portfolio reloading, making sure that views are automatically reloaded e.g. when

  • Using the faucet
  • Performing a transfer
  • Accepting a remotely initiated transaction
  • etc

PR submission checklist

  • I have tested account to account transfer flow and have confirmed that it works

@kugel3 kugel3 changed the title Remove spurious portfolio reloading [ABW-2271] Remove spurious portfolio reloading Oct 9, 2023
@kugel3 kugel3 force-pushed the ABW-NNNN_Spurious-reloading branch from 725512e to 3ba7ccc Compare October 9, 2023 10:41
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

LGTM

@kugel3 kugel3 force-pushed the ABW-NNNN_Spurious-reloading branch from 3ba7ccc to d8c4c37 Compare October 9, 2023 10:46
@kugel3 kugel3 merged commit 4bd0fe9 into main Oct 9, 2023
5 checks passed
@kugel3 kugel3 deleted the ABW-NNNN_Spurious-reloading branch October 9, 2023 14:21
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants