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

Ana/fix rewards updating #3235

Merged
merged 15 commits into from
Dec 3, 2019
Merged

Ana/fix rewards updating #3235

merged 15 commits into from
Dec 3, 2019

Conversation

Bitcoinera
Copy link
Contributor

Closes #3095

Description:

I am getting close to fix this issue: making the rewards update by themselves, without refreshing.
The most significant changes for this are found in the API side. I will now push that branch too.

Still I wonder if the reason why I am getting this error actually lies on the FE:

image

I believe so. I tried in the playground the API with the userRewardsIncremented subscription implemented and it worked quite well.

Thank you! 🚀


For contributor:

  • Added changes entries. Run yarn changelog for a guided process.
  • Reviewed Files changed in the github PR explorer
  • Attach screenshots of the UI components on the PR description (if applicable)
  • Scope of work approved for big PRs

For reviewer:

  • Manually tested the changes on the UI

@codecov
Copy link

codecov bot commented Dec 1, 2019

Codecov Report

Merging #3235 into develop will increase coverage by 0.01%.
The diff coverage is 37.5%.

@@             Coverage Diff             @@
##           develop    #3235      +/-   ##
===========================================
+ Coverage    88.85%   88.86%   +0.01%     
===========================================
  Files          125      125              
  Lines         1911     1913       +2     
  Branches       281      280       -1     
===========================================
+ Hits          1698     1700       +2     
  Misses         206      206              
  Partials         7        7
Impacted Files Coverage Δ
src/components/staking/LiValidator.vue 83.33% <ø> (ø) ⬆️
src/components/common/TmBalance.vue 65% <0%> (-7.23%) ⬇️
src/components/staking/TableValidators.vue 72.41% <33.33%> (-4.51%) ⬇️
src/components/staking/PageValidator.vue 38.63% <66.66%> (+4.59%) ⬆️

@@ -147,6 +146,22 @@ export default {
// query if successful or not as even an unsuccessful tx costs fees
refetchNetworkOnly(this.$apollo.queries.overview)
}
},
userRewardsIncremented: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't this happening every block if the user has delegations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not every block I think, but nearly yes

@Bitcoinera Bitcoinera marked this pull request as ready for review December 2, 2019 18:53
@Bitcoinera
Copy link
Contributor Author

I am only missing the tests now, like I always do! 😅

@Bitcoinera
Copy link
Contributor Author

Can somebody help me out testing the subscriptions? I just noticed they haven't been tested yet throughout the code.

image
🤣 🤣 🤣

Copy link
Collaborator

@faboweb faboweb left a comment

Choose a reason for hiding this comment

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

Why so complicated? There is a TableValidator already does fetch the rewards. Use the blocksubscription there once instead of on every LiValidator?

@faboweb
Copy link
Collaborator

faboweb commented Dec 3, 2019

Can somebody help me out testing the subscriptions? I just noticed they haven't been tested yet throughout the code.

You can just call the functions directly:
wrapper.vm.$apollo.$subscription.blockAdded.update(result)

@Bitcoinera
Copy link
Contributor Author

Why so complicated? There is a TableValidator already does fetch the rewards. Use the blocksubscription there once instead of on every LiValidator?

Fixed! All updating logic now in TableValidators.

@faboweb faboweb merged commit 067d24a into develop Dec 3, 2019
@faboweb faboweb deleted the ana/fix-rewards-updating branch December 3, 2019 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

withdrawing rewards doesn't update available balance until refresh
2 participants