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
[DDW-591] Fix delay on loading stake pool list after switching SMASH server #2447
[DDW-591] Fix delay on loading stake pool list after switching SMASH server #2447
Conversation
@daniloprates This looks much better, but there is some odd behavior while loading the stake pools, these are the steps in which it reproduces: 1 - After switching the server, the count goes to 0 and the stake pools are loading. The last two occurrences look wrong to me. Could this be improved? |
…ist-after-switching-smash-servers
@gabriela-ponce I couldn't reproduce it :/ Thanks |
…ist-after-switching-smash-servers
@daniloprates Can you define Testing Checklist, please 🙏 |
Hi @daniloprates. On 17070, not sure why the stake pool keeps on refreshing and the animation never disappear. Is this expected behaviour? Please see video |
@daniloprates I tried to reproduce this issue again and this was the outcome:
|
…ist-after-switching-smash-servers
@gabriela-ponce from these results I couldn't see anything wrong. When you change the SMASH option, the API fetches the new data and, at that time, the store result is "-". Did you notice anything strange, besides that? |
@tomislavhoracek the Scenario 1 @gabriela-ponce added to the Test Scenarios is really the only thing that needs to be tested. The reason for this issue to happen is that, right now, we don't reset the |
Nothing besides that, but to clarify, the "store results: -" happens after the list started loading for the first time, so after switching the SMASH server and showing some items on the list. Hopefully, that's more clear. |
@gabriela-ponce that's fine, as long as the results on screen are correct. There might be a delay in the logs, so they are only helpful to debug something wrong spot in the app itself. Thanks! |
@daniloprates sometimes I get this error, when I switch from moderated to unmoderated vice versa and quickly go to Stake pools view. It seems to be connected to the saving of the newly selected option. If I change the option quickly and go to Stake pools view, the app might not have enough time to save the newly selected option. See screenshot . Tested on 17082 |
@daniloprates I can't reproduce the previous issue, so it looks good for me. |
Hey @miorsufianiohk, thanks for the report. I've tried extensively to reproduce it, but it never happened to me. I've tried quickly changing the SMASH server multiple times, then going to the Stake Pools list. I've even created this function, which allows to quickly toggle between IOHK's and NONE options while in the Stake Pools list screen. You can use it by pasting it in the dev tools console: const toggle = () => {
const currentSmashServerUrl = daedalus.stores.staking.smashServerUrl;
const smashServerUrl = (currentSmashServerUrl === 'direct')
? "https://smash.cardano-mainnet.iohk.io"
: 'direct';
daedalus.actions.staking.selectSmashServerUrl.trigger({ smashServerUrl });
}; Then pasting this multiple times: toggle(); E.G. If you are still getting this error, could you please record a video of it? Also, in which network? |
…ist-after-switching-smash-servers
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.
Good work Danilo.LGTM. I couldnt reproduce the issue Mior had experienced after trying many times across all OS
@gabriela-ponce please re-test this one 🙏 |
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.
I couldn't reproduce the issue. Works as expected 👍
@tomislavhoracek could you give it a go? |
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.
@daniloprates can you remove logs? We can proceed then
This reverts commit 5f7e436.
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.
@daniloprates as always 💯
This PR fixes delay on loading stake pool list after switching the SMASH server.
JIRA ticket
Screenshots
Testing Checklist
Test Scenarios
Scenario 1 - Validate stake pool list updates according to the selected SMASH server
Review Checklist
Basics
feature
/bug
/chore
,release-x.x.x
)yarn test
)yarn dev
)yarn package
/ CI builds)yarn flow:test
)yarn lint
)yarn prettier:check
)yarn manage:translations
produces no changes)yarn storybook
)yarn.lock
file is updatedCode Quality
Testing
After Review
done
column on the YouTrack board