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

allows leaderboard to be browsed by product #5505

Merged
merged 3 commits into from
Nov 20, 2019

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Nov 17, 2019

Description

allows leaderboard to be browsed by product

Refers/Fixes

https://gitcoincore.slack.com/archives/CAXQ7PT60/p1573916653291000?thread_ts=1572662544.214300&cid=CAXQ7PT60

Testing

tested locally

lrs.update(active=False)

# save new LR in DB
for key, rankings in ranks.items():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all indentation changes from here on out

@@ -290,6 +290,8 @@ def sum_tip_helper(t, time, index_term, val_usd):

def sum_kudos(kt):
val_usd = kt.value_in_usdt_now
if not kt.kudos_token_cloned_from:
return
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. what does this logic do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a kudos transfer that has a null kudos_token_cloned_from and this logic prevents an exception here.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah nice thanks for the info

@thelostone-mc
Copy link
Member

@owocki looks like we've got conflicts cause of the indentation 😓
would you be able resolve those?

@codecov
Copy link

codecov bot commented Nov 20, 2019

Codecov Report

Merging #5505 into master will decrease coverage by 1.07%.
The diff coverage is 58.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5505      +/-   ##
==========================================
- Coverage   30.24%   29.16%   -1.08%     
==========================================
  Files         247      242       -5     
  Lines       21048    20918     -130     
  Branches     3034     3028       -6     
==========================================
- Hits         6365     6100     -265     
- Misses      14407    14571     +164     
+ Partials      276      247      -29
Impacted Files Coverage Δ
app/marketing/views.py 11.43% <0%> (+0.08%) ⬆️
app/marketing/models.py 63.67% <100%> (+0.61%) ⬆️
...eting/management/commands/assemble_leaderboards.py 64.37% <60.41%> (+0.59%) ⬆️
app/app/context.py 0% <0%> (-77.78%) ⬇️
app/retail/helpers.py 28.57% <0%> (-42.86%) ⬇️
app/retail/templatetags/matches.py 62.5% <0%> (-37.5%) ⬇️
app/quests/views.py 15.31% <0%> (-11.88%) ⬇️
app/dashboard/models.py 50.77% <0%> (-5.42%) ⬇️
app/app/utils.py 21.45% <0%> (-1.29%) ⬇️
app/dashboard/utils.py 42.47% <0%> (-0.68%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cce953d...5eece36. Read the comment docs.

@danlipert danlipert force-pushed the kevin/leaderboard_by_product branch from 23be612 to 9046db2 Compare November 20, 2019 14:48
@danlipert
Copy link
Contributor

@owocki can you take another look at this PR - seems like the tests for the leaderboard are broken. I tried a quick fix but failed

@owocki
Copy link
Contributor Author

owocki commented Nov 20, 2019

yay! fixed. merging

@owocki owocki merged commit c1a7703 into master Nov 20, 2019
@thelostone-mc thelostone-mc deleted the kevin/leaderboard_by_product branch June 27, 2020 00:45
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.

3 participants