profile speedups (bug 860786)

2013-04-11 19:14:19 -07:00 · 2013-04-11 19:14:19 -07:00 · 5b8abd7eb1
--- a/apps/addons/helpers.py
+++ b/apps/addons/helpers.py
@ -325,9 +325,9 @@ def persona_grid(context, addons):
@jinja2.contextfilter
@register.inclusion_tag('addons/impala/persona_grid.html')
 def impala_persona_grid(context, personas, src=None, pagesize=6, cols=3):
-    pages = chunked(personas, pagesize)
-    columns = 'cols-%d' % cols
-    return new_context(**locals())
+    c = dict(context.items())
+    return dict(pages=chunked(personas, pagesize),
+                columns='cols-%d' % cols, **c)


@register.filter
--- a/apps/addons/models.py
+++ b/apps/addons/models.py
@ -871,7 +871,6 @@ class Addon(amo.models.OnChangeMixin, amo.models.ModelBase):
            return

        addon_dict = dict((a.id, a) for a in addons)
-        non_apps = [a for a in addons if a.type != amo.ADDON_WEBAPP]
        personas = [a for a in addons if a.type == amo.ADDON_PERSONA]
        addons = [a for a in addons if a.type != amo.ADDON_PERSONA]

@ -937,12 +936,6 @@ class Addon(amo.models.OnChangeMixin, amo.models.ModelBase):
                    addon_p.price = price
                    addon_dict[addon_p.addon_id]._premium = addon_p

-        # This isn't cheating, right? I don't want to add `compat` to
-        # market's INSTALLED_APPS.
-        if not settings.MARKETPLACE:
-            # Attach counts for add-on compatibility reports.
-            CompatReport.transformer(non_apps)
-
        return addon_dict

    @property
--- a/apps/addons/templates/addons/impala/persona_grid.html
+++ b/apps/addons/templates/addons/impala/persona_grid.html
@ -1,4 +1,3 @@
-{% cache personas %}
 <ul class="listing-grid personas {{ columns }} c">
 {% for page in pages %}
  {% set first_page = loop.first %}
@ -22,4 +21,3 @@
  </section>
 {% endfor %}
 </ul>
-{% endcache %}
--- a/apps/addons/tests/test_models.py
+++ b/apps/addons/tests/test_models.py
@ -1269,20 +1269,6 @@ class TestAddonModels(amo.tests.TestCase):
        file.update(binary_components=True)
        eq_(addon.binary_components, True)

-    def test_compat_counts_transform_none(self):
-        addon = Addon.objects.get(id=3615)
-        eq_(addon._compat_counts, {'success': 0, 'failure': 0})
-
-    def test_compat_counts_transform_some(self):
-        guid = '{2fa4ed95-0317-4c6a-a74c-5f3e3912c1f9}'
-        CompatReport.objects.create(guid=guid, works_properly=True)
-        CompatReport.objects.create(guid=guid, works_properly=True)
-        CompatReport.objects.create(guid=guid, works_properly=False)
-        CompatReport.objects.create(guid='ballin', works_properly=True)
-        CompatReport.objects.create(guid='ballin', works_properly=False)
-        eq_(Addon.objects.get(id=3615)._compat_counts,
-            {'success': 2, 'failure': 1})
-

 class TestAddonDelete(amo.tests.TestCase):

--- a/apps/compat/models.py
+++ b/apps/compat/models.py
@ -2,6 +2,7 @@ from django.db import models

 import json_field

+import amo
 import amo.models


@ -20,15 +21,15 @@ class CompatReport(amo.models.ModelBase):
    class Meta:
        db_table = 'compatibility_reports'

-    @staticmethod
-    def transformer(addons):
-        qs = CompatReport.uncached
-        for addon in addons:
-            works_ = dict(qs.filter(guid=addon.guid)
-                            .values_list('works_properly')
-                            .annotate(models.Count('id')))
-            addon._compat_counts = {'success': works_.get(True, 0),
-                                    'failure': works_.get(False, 0)}
+    @classmethod
+    def get_counts(self, guid):
+        works = dict(CompatReport.objects.filter(guid=guid)
+                     .values_list('works_properly')
+                     .annotate(models.Count('id')))
+        return {
+            'success': works.get(True, 0),
+            'failure': works.get(False, 0)
+        }


 class AppCompat(amo.models.ModelBase):
--- a/apps/compat/tests.py
+++ b/apps/compat/tests.py
@ -24,6 +24,21 @@ incoming_data = {
 }


+class TestCompatReportModel(amo.tests.TestCase):
+
+    def test_none(self):
+        eq_(CompatReport.get_counts('xxx'), {'success': 0, 'failure': 0})
+
+    def test_some(self):
+        guid = '{2fa4ed95-0317-4c6a-a74c-5f3e3912c1f9}'
+        CompatReport.objects.create(guid=guid, works_properly=True)
+        CompatReport.objects.create(guid=guid, works_properly=True)
+        CompatReport.objects.create(guid=guid, works_properly=False)
+        CompatReport.objects.create(guid='ballin', works_properly=True)
+        CompatReport.objects.create(guid='ballin', works_properly=False)
+        eq_(CompatReport.get_counts(guid), {'success': 2, 'failure': 1})
+
+
 class TestIndex(amo.tests.TestCase):

    # TODO: Test valid version processing here.
--- a/apps/devhub/helpers.py
+++ b/apps/devhub/helpers.py
@ -12,6 +12,7 @@ from amo.urlresolvers import reverse
 from amo.helpers import breadcrumbs, impala_breadcrumbs, page_title
 from access import acl
 from addons.helpers import new_context
+from compat.models import CompatReport


 register.function(acl.check_addon_ownership)
@ -215,3 +216,9 @@ def display_url(url):
    bytes = urllib.unquote(url)
    c = chardet.detect(bytes)
    return bytes.decode(c['encoding'], 'replace')
+
+
+@register.function
+def get_compat_counts(addon):
+    """Get counts for add-on compatibility reports."""
+    return CompatReport.get_counts(addon.guid)
--- a/apps/devhub/templates/devhub/includes/addons_edit_nav.html
+++ b/apps/devhub/templates/devhub/includes/addons_edit_nav.html
@ -36,10 +36,11 @@
        <li class="compat-reports">
          <a href="{{ url('compat.reporter_detail', addon.guid) }}">
            <span>{{ _('Compatibility Reports') }}</span>
-            <b class="failure" title="{{ _('{0} failure reports')|f(addon._compat_counts.failure) }}">
-              {{ addon._compat_counts.failure|numberfmt }}</b>
-            <b class="success" title="{{ _('{0} success reports')|f(addon._compat_counts.success) }}">
-              {{ addon._compat_counts.success|numberfmt }}</b>
+            {% set compat_counts = get_compat_counts(addon) %}
+            <b class="failure" title="{{ _('{0} failure reports')|f(compat_counts.failure) }}">
+              {{ compat_counts.failure|numberfmt }}</b>
+            <b class="success" title="{{ _('{0} success reports')|f(compat_counts.success) }}">
+              {{ compat_counts.success|numberfmt }}</b>
          </a>
        </li>
      {% endif %}
--- a/apps/users/models.py
+++ b/apps/users/models.py
@ -174,12 +174,6 @@ class UserProfile(amo.models.OnChangeMixin, amo.models.ModelBase):
        return self.addons.reviewed().exclude(type=amo.ADDON_WEBAPP).filter(
            addonuser__user=self, addonuser__listed=True)

-    @amo.cached_property
-    def num_addons_listed(self):
-        """Number of public add-ons this user is listed as author of."""
-        return self.addons.reviewed().exclude(type=amo.ADDON_WEBAPP).filter(
-            addonuser__user=self, addonuser__listed=True).count()
-
    @amo.cached_property
    def apps_listed(self):
        """Public apps this user is listed as author of."""
--- a/apps/users/templates/users/profile.html
+++ b/apps/users/templates/users/profile.html
@ -52,8 +52,13 @@
  </section>
 </div>

+{# For anchors. #}
+{% if addons.object_list or personas %}
+  <div id="my-submissions"></div>
+{% endif %}
+
 {% if addons.object_list %}
-  <div id="my-submissions" class="island primary listing c">
+  <div id="my-addons" class="island primary listing c">
    <h2>{{ _("Add-ons I've created") }}</h2>
    <div class="items">
      {{ impala_addon_listing_items(addons.object_list, field=sorting, src='userprofile') }}
@ -63,7 +68,7 @@
 {% endif %}

 {% if personas %}
-  <div id="{{ 'my-themes' if addons.object_list else 'my-submissions' }}" class="island c">
+  <div id="my-themes" class="island c">
    <h2>{{ _("Themes I've created") }}</h2>
    {{ personas|impala_persona_grid(cols=5, pagesize=15) }}
  </div>
--- a/apps/users/tests/test_views.py
+++ b/apps/users/tests/test_views.py
@ -1120,7 +1120,7 @@ class TestProfileSections(amo.tests.TestCase):

        doc = pq(r.content)
        eq_(doc('.num-addons a[href="#my-submissions"]').length, 1)
-        items = doc('#my-submissions .item')
+        items = doc('#my-addons .item')
        eq_(items.length, 2)
        eq_(items('.install[data-addon=3615]').length, 1)
        eq_(items('.install[data-addon=5299]').length, 1)
@ -1135,7 +1135,7 @@ class TestProfileSections(amo.tests.TestCase):
        r = self.client.get(self.url)

        doc = pq(r.content)
-        items = doc('#my-submissions .persona')
+        items = doc('#my-themes .persona')
        eq_(items.length, 1)
        eq_(items('a[href="%s"]' % a.get_url_path()).length, 1)

@ -1192,7 +1192,7 @@ class TestProfileSections(amo.tests.TestCase):
        r = self.client.get(self.url)
        assert len(self.user.addons_listed) <= 10, (
            'This user should have fewer than 10 add-ons.')
-        eq_(pq(r.content)('#my-submissions .paginator').length, 0)
+        eq_(pq(r.content)('#my-addons .paginator').length, 0)

    def test_my_reviews_pagination(self):
        for i in xrange(20):
@ -1200,7 +1200,7 @@ class TestProfileSections(amo.tests.TestCase):
        assert len(self.user.addons_listed) > 10, (
            'This user should have way more than 10 add-ons.')
        r = self.client.get(self.url)
-        eq_(pq(r.content)('#my-submissions .paginator').length, 1)
+        eq_(pq(r.content)('#my-addons .paginator').length, 1)

    def test_my_collections_followed(self):
        coll = Collection.objects.all()[0]
--- a/apps/users/views.py
+++ b/apps/users/views.py
@ -27,7 +27,7 @@ import amo
 import users.notifications as notifications
 from abuse.models import send_abuse_report
 from access.middleware import ACLMiddleware
-from addons.models import Addon
+from addons.models import Addon, AddonUser
 from addons.decorators import addon_view_factory
 from amo import messages
 from amo.decorators import (json_view, login_required, permission_required,
@ -575,18 +575,20 @@ def profile(request, user):
    own_profile = (request.user.is_authenticated() and
                   request.amo_user.id == user.id)

+    addons = []
    personas = []
+    user.num_addons_listed = 0
    if user.is_developer:
-        if webapp:
-            items = user.apps_listed
-        else:
-            items = user.addons_listed.exclude(type=amo.ADDON_PERSONA)
-            personas = (user.addons_listed.filter(type=amo.ADDON_PERSONA)
-                        .order_by('-average_daily_users'))
-        addons = amo.utils.paginate(request,
-                                    items.order_by('-weekly_downloads'))
-    else:
-        addons = []
+        addons = user.addons.reviewed().exclude(type=amo.ADDON_WEBAPP).filter(
+            addonuser__user=user, addonuser__listed=True)
+        user.num_addons_listed = addons.count()
+
+        personas = addons.filter(type=amo.ADDON_PERSONA).order_by(
+            '-average_daily_users')
+
+        addons = addons.exclude(type=amo.ADDON_PERSONA).order_by(
+            '-weekly_downloads')
+        addons = amo.utils.paginate(request, addons, 5)

    def get_addons(reviews):
        if not reviews:
@ -595,6 +597,7 @@ def profile(request, user):
        addons = dict((addon.id, addon) for addon in qs)
        for review in reviews:
            review.addon = addons.get(review.addon_id)
+
    # (Don't show marketplace reviews for AMO (since that would break))
    reviews = list(user.reviews.exclude(addon__type=amo.ADDON_WEBAPP)
                   .transform(get_addons))
--- a/media/css/impala/users.less
+++ b/media/css/impala/users.less
@ -176,7 +176,7 @@ img.icon {
            width: auto;
        }
    }
-    #my-submissions {
+    #my-addons {
        margin: 0 0 15px;
    }
    .secondary {