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.

Sign up for GitHub

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

Jump to bottom

fetch the last not empty coin balance records #2538

Merged
merged 17 commits into from
Aug 14, 2019
Merged

fetch the last not empty coin balance records #2538

merged 17 commits into from
Aug 14, 2019

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented Aug 9, 2019 •
edited
Loading

fixes #2535

Motivation

We were fetching coin balance records for coin balance chart in the period of 90 days. This PR fetches last 90 not empty token balance records

Changelog

  • fetch the last not empty coin balance records

@ayrat555 ayrat555 changed the title fetch the last not empty coin balance records [WIP] fetch the last not empty coin balance records Aug 9, 2019
@coveralls
Copy link

coveralls commented Aug 9, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 65e5c15e-5cc7-4140-9086-7245e916a629

  • 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 79.15%
Totals Coverage Status
Change from base Build ca5b6dbe-704e-4998-8bb9-5dbc0e0756fc: -0.2%
Covered Lines: 5197
Relevant Lines: 6566
💛 - Coveralls

@ayrat555 ayrat555 changed the title [WIP] fetch the last not empty coin balance records fetch the last not empty coin balance records Aug 9, 2019
vbaranov
vbaranov requested changes Aug 9, 2019
View reviewed changes
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@ayrat555

/address/0x9aee56142bbfa9427e103ee13a702178bbf36362/coin_balances

Screenshot 2019-08-09 at 19 08 28

Because of the wrong order of data

Screenshot 2019-08-09 at 19 07 20

apps/block_scout_web/assets/js/lib/coin_balance_history_chart.js Outdated Show resolved Hide resolved
pasqu4le
pasqu4le reviewed Aug 9, 2019
View reviewed changes
Copy link
Contributor

@pasqu4le pasqu4le left a comment

Choose a reason for hiding this comment

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

A couple of questions

apps/explorer/lib/explorer/chain.ex Outdated Show resolved Hide resolved
apps/explorer/lib/explorer/chain/address/coin_balance.ex Show resolved Hide resolved
@ayrat555
Copy link
Contributor Author

@vbaranov I fixed the chart

@vbaranov vbaranov self-requested a review August 12, 2019 14:42
vbaranov
vbaranov requested changes Aug 12, 2019
View reviewed changes
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@ayrat555

Duplicates:

Screenshot 2019-08-12 at 17 42 21

@vbaranov
Copy link
Member

@ayrat555

/address/0xebd37b256563e30c6f9289a8e2702f0852880833/coin_balances

Screenshot 2019-08-12 at 17 59 51

@ayrat555
Copy link
Contributor Author

@vbaranov I fixed it. the issue was in chart steps

@ayrat555
Copy link
Contributor Author

@vbaranov the issue with duplicate entries is caused by duplicate entries in the DB. Can you please create an issue for it? I'll resolve it in future PRs. I think the issue with the chart is completely fixed by this PR

@vbaranov vbaranov mentioned this pull request Aug 13, 2019
Closed
@vbaranov vbaranov self-requested a review August 13, 2019 17:16
@vbaranov vbaranov requested a review from pasqu4le August 13, 2019 17:17
@vbaranov
Copy link
Member

@ayrat555 tests are broken

@ayrat555
Copy link
Contributor Author

@vbaranov the test is fixed

@ayrat555 ayrat555 merged commit 6a9aa65 into master Aug 14, 2019
@ayrat555 ayrat555 deleted the ab-fetch-last-coin-balance-records branch August 14, 2019 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pasqu4le pasqu4le pasqu4le approved these changes

@saneery saneery saneery approved these changes

@vbaranov vbaranov vbaranov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mismatch in token balance update stats
5 participants
@ayrat555 @coveralls @vbaranov @saneery @pasqu4le