From 8ecf4b023903bd9280a81da515c1d3ede33e8901 Mon Sep 17 00:00:00 2001 From: Lucas Connors Date: Sun, 6 Oct 2019 16:26:45 -0700 Subject: [PATCH] Leaderboard performance low-hanging fruit (#344) * Simplify leaderboard HTML template * Some low-hanging fruit performance improvements and cleanup --- accounts/models.py | 7 +++++ campaign/managers.py | 8 ++++++ campaign/models.py | 4 +++ campaign/views.py | 36 +++++++++++++----------- templates/leaderboard/includes/list.html | 17 ----------- templates/leaderboard/leaderboard.html | 14 +++++++-- 6 files changed, 50 insertions(+), 36 deletions(-) create mode 100644 campaign/managers.py delete mode 100644 templates/leaderboard/includes/list.html diff --git a/accounts/models.py b/accounts/models.py index fb6f909d..b0627ce6 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -117,6 +117,11 @@ def public_profile_url(self): if not self.invest_anonymously: return reverse("public_profile", args=(self.user.username,)) + def get_total_earned(self): + investments = Investment.objects.filter_user_investments(user=self.user) + total_earned = sum(investment.generated_revenue() for investment in investments) + return total_earned + @cache_using_pk def profile_context(self): context = {} @@ -163,6 +168,8 @@ def profile_context(self): generated_revenue_user += investment.generated_revenue() context["artists"][artist.id].total_earned += generated_revenue_user total_earned += generated_revenue_user + + # TODO(lucas): Could refactor to use get_total_earned() instead context["total_earned"] = total_earned # Add percentage of return to context diff --git a/campaign/managers.py b/campaign/managers.py new file mode 100644 index 00000000..46a3f75c --- /dev/null +++ b/campaign/managers.py @@ -0,0 +1,8 @@ +from django.db import models + + +class InvestmentManager(models.Manager): + def filter_user_investments(self, user): + return self.filter( + charge__customer__user=user, charge__paid=True, charge__refunded=False + ) diff --git a/campaign/models.py b/campaign/models.py index d54dc998..2e01d0c5 100644 --- a/campaign/models.py +++ b/campaign/models.py @@ -12,6 +12,8 @@ from pinax.stripe.models import Charge +from campaign.managers import InvestmentManager + class Project(models.Model): @@ -254,6 +256,8 @@ class Investment(models.Model): default=1, help_text="The number of shares an investor made in a transaction" ) + objects = InvestmentManager() + def __str__(self): return "{num_shares} shares in {campaign} to {investor}".format( num_shares=self.num_shares, diff --git a/campaign/views.py b/campaign/views.py index f1dbf37e..d081bb7e 100644 --- a/campaign/views.py +++ b/campaign/views.py @@ -5,36 +5,39 @@ """ from django.core.cache import cache +from django.db.models import OuterRef, Exists from django.views.generic import TemplateView from accounts.models import UserProfile +from campaign.models import Investment class LeaderboardView(TemplateView): template_name = "leaderboard/leaderboard.html" - def investor_context(self, investor, key_to_copy): - context = investor.profile_context() - context["amount"] = context[key_to_copy] - context.update( - { - "name": investor.get_display_name(), - "url": investor.public_profile_url(), - "avatar_url": investor.avatar_url(), - } - ) - return context + @staticmethod + def investor_context(investor): + return { + "name": investor.get_display_name(), + "url": investor.public_profile_url(), + "avatar_url": investor.avatar_url(), + "amount": investor.get_total_earned(), + } # TODO(lucas): Review to improve performance # Warning: top_earned_investors absolutely will not scale, the view is meant # to be run occasionally (once a day) and then have the whole page cached def calculate_leaderboard(self): - # Top earned investors user_profiles = UserProfile.objects.filter(invest_anonymously=False) + investments = Investment.objects.filter_user_investments(user=OuterRef("pk")) + investors = user_profiles.annotate(has_invested=Exists(investments)).filter( + has_invested=True + ) + + # Top earned investors top_earned_investors = [ - self.investor_context(user_profile, "total_earned") - for user_profile in user_profiles + self.investor_context(investor) for investor in investors ] top_earned_investors = list( filter(lambda context: context["amount"] > 0, top_earned_investors) @@ -42,11 +45,10 @@ def calculate_leaderboard(self): top_earned_investors = sorted( top_earned_investors, key=lambda context: context["amount"], reverse=True )[:20] - - return {"top_earned_investors": top_earned_investors} + return top_earned_investors def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) leaderboard = cache.get_or_set("leaderboard", self.calculate_leaderboard) - context.update(leaderboard) + context["top_earned_investors"] = leaderboard return context diff --git a/templates/leaderboard/includes/list.html b/templates/leaderboard/includes/list.html deleted file mode 100644 index 992d5b87..00000000 --- a/templates/leaderboard/includes/list.html +++ /dev/null @@ -1,17 +0,0 @@ -{% load humanize %} -{% load perdiem %} - -
- {% if leaders %} - {% for leader_context in leaders %} - -
- {{ leader_context.name }} -
-

#{{ forloop.counter }}: {{ leader_context.name }}

-

${{ leader_context.amount|notrail_floatformat:2|intcomma }}

-
- - {% endfor %} - {% endif %} -
diff --git a/templates/leaderboard/leaderboard.html b/templates/leaderboard/leaderboard.html index 88de6da2..17d25751 100644 --- a/templates/leaderboard/leaderboard.html +++ b/templates/leaderboard/leaderboard.html @@ -11,7 +11,17 @@ {% block content %}

Top Investors

- {% include "leaderboard/includes/list.html" with name='Investors' leaders=top_earned_investors %} +
+ {% for leader_context in top_earned_investors %} + +
+ {{ leader_context.name }} +
+

#{{ forloop.counter }}: {{ leader_context.name }}

+

${{ leader_context.amount|notrail_floatformat:2|intcomma }}

+
+ + {% endfor %} +
- {% endblock %}