diff --git a/apps/addons/helpers.py b/apps/addons/helpers.py index fa6be87e18..ca736436fe 100644 --- a/apps/addons/helpers.py +++ b/apps/addons/helpers.py @@ -1,3 +1,5 @@ +import random + import jinja2 from jingo import register, env @@ -101,6 +103,17 @@ def tags_box(context, addon, tags=None): return c +@register.filter +@jinja2.contextfilter +def shuffle_boundary(context, addons, boundary=None): + if not boundary: + return addons + split = addons[:boundary], addons[boundary:] + for s in split: + random.shuffle(s) + return split[0] + split[1] + + @register.inclusion_tag('addons/listing/items.html') @jinja2.contextfunction def addon_listing_items(context, addons, show_date=False, src=None, @@ -110,8 +123,10 @@ def addon_listing_items(context, addons, show_date=False, src=None, @register.inclusion_tag('addons/listing/items_compact.html') @jinja2.contextfunction -def addon_listing_items_compact(context, addons, show_date=False, - src=None): +def addon_listing_items_compact(context, addons, boundary=0, + show_date=False, src=None): + if show_date == 'featured' and boundary: + addons = shuffle_boundary(context, list(addons), boundary) return new_context(**locals()) diff --git a/apps/addons/models.py b/apps/addons/models.py index 6dd1c2fac8..3b082a3253 100644 --- a/apps/addons/models.py +++ b/apps/addons/models.py @@ -63,15 +63,50 @@ class AddonManager(amo.models.ManagerBase): """Get valid, enabled add-ons only""" return self.filter(self.valid_q(amo.LISTED_STATUSES)) - def featured(self, app): + def featured(self, app, by_locale=False): """ - Filter for all featured add-ons for an application in all locales. + Filter for all featured add-ons for an application. Specify + by_locale to have it filtered by the current locale. """ - return self.valid().filter(feature__application=app.id) + qs = Q(feature__application=app.id) + if by_locale: + locales = [] + for locale in by_locale(): + locale = to_language(locale) if locale else locale + locales.append(Q(feature__locale=locale)) - def category_featured(self): - """Get all category-featured add-ons for ``app`` in all locales.""" - return self.filter(addoncategory__feature=True) + qs = qs & Q(feature__isnull=False) & reduce(operator.or_, locales) + return (self.valid().filter(qs).order_by('-feature__locale')) + + return self.valid().filter(qs) + + def category_featured(self, category=None, by_locale=False): + """ + Filter for all featured addons for a category. Specify + by_locale to have it filtered by the current locale. + """ + qs = Q(addoncategory__feature=True) + + if by_locale: + locales = [] + for locale in by_locale(): + prefix = 'addoncategory__feature_locales' + if locale: + locale = to_language(locale) + prefix += '__contains' + else: + # Sadly looks like theres '' and NULL in the column, + # hopefully the new admin tools will clean this out. + locales.append(Q(**{prefix: ''})) + locales.append(Q(**{prefix: locale})) + + qs = (qs & Q(addoncategory__category=category) & + reduce(operator.or_, locales)) + return self.filter(qs).order_by('-addoncategory__feature_locales') + + if category: + qs = qs & Q(addoncategory__category=category) + return self.filter(qs) def listed(self, app, *status): """ diff --git a/apps/addons/templates/addons/home.html b/apps/addons/templates/addons/home.html index 04056a75fe..20fc444e5c 100644 --- a/apps/addons/templates/addons/home.html +++ b/apps/addons/templates/addons/home.html @@ -80,8 +80,8 @@
{% for key, addons in addon_sets.items() %}
- {{ addon_listing_items_compact(addons, show_date=key, - src='homepagebrowse') }} + {{ addon_listing_items_compact(addons, boundary=boundary, + show_date=key, src='homepagebrowse') }}
diff --git a/apps/addons/views.py b/apps/addons/views.py index e0c7b46e32..832445a423 100644 --- a/apps/addons/views.py +++ b/apps/addons/views.py @@ -256,7 +256,8 @@ class HomepageFilter(BaseFilter): def filter_featured(self): # It's ok to cache this for a while...it'll expire eventually. - return Addon.objects.featured(self.request.APP).order_by('?') + return Addon.objects.featured(self.request.APP, + by_locale=amo.LOCALE_ALL) def home(request): @@ -264,6 +265,8 @@ def home(request): base = Addon.objects.listed(request.APP).exclude(type=amo.ADDON_PERSONA) filter = HomepageFilter(request, base, key='browse', default='featured') addon_sets = dict((key, qs[:4]) for key, qs in filter.all().items()) + boundary = (Addon.objects.featured(request.APP, + by_locale=amo.LOCALE_CURRENT).count()) # Collections. q = Collection.objects.filter(listed=True, application=request.APP.id) @@ -284,7 +287,8 @@ def home(request): return jingo.render(request, 'addons/home.html', {'downloads': downloads, 'top_tags': top_tags, 'filter': filter, 'addon_sets': addon_sets, - 'collections': collections, 'promobox': promobox}) + 'collections': collections, 'promobox': promobox, + 'boundary': boundary}) @dec.mobilized(home) diff --git a/apps/amo/__init__.py b/apps/amo/__init__.py index cdbfc0501d..4ca6cff97b 100644 --- a/apps/amo/__init__.py +++ b/apps/amo/__init__.py @@ -10,6 +10,7 @@ from tower import ugettext_lazy as _ from licenses import license_text from .log import LOG, LOG_BY_ID, LOG_KEEP, log +from django.utils.translation import trans_real as translation logger_log = commonware.log.getLogger('z.amo') @@ -211,6 +212,9 @@ ADDON_ICON_SIZES = [32, 48, 64] # Preview upload sizes [thumb, full] ADDON_PREVIEW_SIZES = [(200, 150), (700, 525)] +LOCALE_CURRENT = lambda: (translation.get_language(),) +LOCALE_ALL = lambda: (None, translation.get_language()) + # These types don't maintain app compatibility in the db. Instead, we look at # APP.types and APP_TYPE_SUPPORT to figure out where they are compatible. NO_COMPAT = (ADDON_SEARCH, ADDON_PERSONA) diff --git a/apps/browse/templates/browse/category_landing.html b/apps/browse/templates/browse/category_landing.html index c7f4049b57..1aadf466dd 100644 --- a/apps/browse/templates/browse/category_landing.html +++ b/apps/browse/templates/browse/category_landing.html @@ -28,7 +28,7 @@ {% cache addons %} {# The featured add-ons for a category should be small, so we get them all with the if. Shuffling here to take advantage of the cached fragment. #} - {% for addon in (addons|list|shuffle)[:3] %} + {% for addon in (addons|list|shuffle_boundary(boundary))[:3] %}

  • diff --git a/apps/browse/templates/browse/creatured.html b/apps/browse/templates/browse/creatured.html index 1a5ac3569e..56e998f7f9 100644 --- a/apps/browse/templates/browse/creatured.html +++ b/apps/browse/templates/browse/creatured.html @@ -14,7 +14,7 @@
    {% if addons %} - {{ addon_listing_items(addons, src='creatured') }} + {{ addon_listing_items(addons|list|shuffle_boundary(boundary), src='creatured') }} {% else %}

    {{ _('No featured add-ons in {0}.')|f(category.name) }}

    {% endif %} diff --git a/apps/browse/templates/browse/featured.html b/apps/browse/templates/browse/featured.html index fa70cb2a26..12dcedaf42 100644 --- a/apps/browse/templates/browse/featured.html +++ b/apps/browse/templates/browse/featured.html @@ -28,7 +28,7 @@ {% cache addons %}
    - {{ addon_listing_items(addons|list|shuffle, src="recommended") }} + {{ addon_listing_items(addons|list|shuffle_boundary(boundary), src="recommended") }}
    {% endcache %} diff --git a/apps/browse/templates/browse/personas/category_landing.html b/apps/browse/templates/browse/personas/category_landing.html index 5412961ebc..71e47389b9 100644 --- a/apps/browse/templates/browse/personas/category_landing.html +++ b/apps/browse/templates/browse/personas/category_landing.html @@ -27,11 +27,9 @@
    {% set addons = addon_sets[special][:16] %}
      - {% cache addons %} {% for addon in addons %}
    • {{ persona_preview(addon.persona, size='small') }}
      • {% endfor %} - {% endcache %}
    {{ breadcrumbs([(None, _('Personas'))]) }}

    {{ _('Personas') }}

    @@ -44,13 +42,13 @@
    - {% set addons = featured[:6] %} + {% set addons = featured|list|shuffle_boundary(boundary) %} {% if addons %}

    {{ _('Featured Personas') }}

      {% cache addons %} - {% for addon in addons %} + {% for addon in addons[:6] %}
    • {{ persona_preview(addon.persona, size='small') }}
      • {% endfor %} {% endcache %} diff --git a/apps/browse/tests.py b/apps/browse/tests.py index b39977d471..2a374da99f 100644 --- a/apps/browse/tests.py +++ b/apps/browse/tests.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import datetime from datetime import timedelta +import re from urlparse import urlparse from django import http @@ -252,6 +253,173 @@ class TestCategoryPages(test_utils.TestCase): eq_(len(doc('.item')), 1) +class TestFeaturedLocale(test_utils.TestCase): + fixtures = ('base/apps', 'base/category', 'base/addon_3615', + 'base/featured', 'addons/featured', 'browse/nameless-addon') + + def setUp(self): + self.addon = Addon.objects.get(pk=3615) + self.persona = Addon.objects.get(pk=15679) + self.extension = Addon.objects.get(pk=2464) + self.category = Category.objects.get(slug='bookmarks') + + self.url = reverse('browse.creatured', args=['bookmarks']) + + def change_addoncategory(self, addon, locale='es-ES'): + ac = addon.addoncategory_set.all()[0] + ac.feature_locales = locale + ac.save() + + def change_addon(self, addon, locale='es-ES'): + feature = addon.feature_set.all()[0] + feature.locale = locale + feature.save() + + def list_featured(self, content): + # Not sure we want to get into testing randomness + # between multiple executions of a page, but if this is a quick + # way to print out the results and check yourself that they + # are changing. + doc = pq(content) + ass = doc('.featured-inner .item a') + rx = re.compile('/(en-US|es-ES)/firefox/addon/(\d+)/$') + for a in ass: + mtch = rx.match(a.attrib['href']) + if mtch: + print mtch.group(2) + + def test_featured(self): + addons = (Addon.objects.featured(app=amo.FIREFOX, + by_locale=amo.LOCALE_ALL)) + eq_(addons.count(), 6) + + def test_category_featured(self): + cat = Category.objects.get(id=22) + addons = (Addon.objects.category_featured(category=cat, + by_locale=amo.LOCALE_ALL)) + eq_(addons.count(), 2) + + def test_creatured_locale_en_US(self): + res = self.client.get(self.url) + assert self.addon in res.context['addons'] + + def test_creatured_locale_nones(self): + self.change_addoncategory(self.addon, '') + res = self.client.get(self.url) + assert self.addon in res.context['addons'] + + self.change_addoncategory(self.addon, None) + res = self.client.get(self.url) + assert self.addon in res.context['addons'] + + def test_creatured_locale_many(self): + self.change_addoncategory(self.addon, 'en-US,es-ES') + res = self.client.get(self.url) + assert self.addon in res.context['addons'] + + res = self.client.get(self.url.replace('en-US', 'es-ES')) + assert self.addon in res.context['addons'] + + def test_creatured_locale_not_en_US(self): + self.change_addoncategory(self.addon, 'es-ES') + res = self.client.get(self.url) + assert self.addon not in res.context['addons'] + + def test_creatured_locale_es_ES(self): + res = self.client.get(self.url.replace('en-US', 'es-ES')) + assert self.addon in res.context['addons'] + + def test_featured_locale_en_US(self): + res = self.client.get(reverse('browse.featured')) + assert self.extension in res.context['addons'] + + def test_featured_locale_es_ES(self): + addon = self.extension + self.change_addon(addon, 'es-ES') + url = reverse('browse.featured') + res = self.client.get(url) + assert addon not in res.context['addons'] + + res = self.client.get(url.replace('en-US', 'es-ES')) + assert addon in res.context['addons'] + + def test_featured_extensions_no_category_en_US(self): + addon = self.extension + res = self.client.get(reverse('browse.extensions')) + assert addon in res.context['addons'].object_list + + def test_featured_extensions_with_category_es_ES(self): + addon = self.addon + res = self.client.get(reverse('browse.extensions', args=['bookmarks'])) + assert addon in res.context['filter'].all()['featured'] + + self.change_addoncategory(addon, 'es-ES') + res = self.client.get(reverse('browse.extensions', args=['bookmarks'])) + assert addon not in res.context['filter'].all()['featured'] + + def test_featured_persona_no_category_en_US(self): + addon = self.persona + url = reverse('browse.personas') + res = self.client.get(url) + assert addon in res.context['featured'] + + self.change_addon(addon, 'es-ES') + res = self.client.get(url) + assert addon not in res.context['featured'] + + res = self.client.get(url.replace('en-US', 'es-ES')) + assert addon in res.context['featured'] + + def test_featured_persona_category_en_US(self): + addon = self.persona + category = Category.objects.get(id=22) + category.update(type=amo.ADDON_PERSONA) + + addon.addoncategory_set.create(category=category, feature=True) + url = reverse('browse.personas', args=[category.slug]) + res = self.client.get(url) + assert addon in res.context['featured'] + + self.change_addoncategory(addon, 'es-ES') + res = self.client.get(url) + assert addon not in res.context['featured'] + + res = self.client.get(url.replace('en-US', 'es-ES')) + assert addon in res.context['featured'] + + def test_homepage(self): + addon = self.persona + url = reverse('home') + res = self.client.get(url) + assert addon in res.context['filter'].filter_featured() + + self.change_addon(addon, 'es-ES') + res = self.client.get(url) + assert addon not in res.context['filter'].filter_featured() + + res = self.client.get(url.replace('en-US', 'es-ES')) + assert addon in res.context['filter'].filter_featured() + + def test_homepage_order(self): + another = Addon.objects.get(id=1003) + self.change_addon(another, 'en-US') + url = reverse('home') + res = self.client.get(url) + items = res.context['filter'].filter_featured() + # The order should be random within those boundaries. + eq_([1003, 3481], sorted([i.pk for i in items[0:2]])) + eq_([1001, 2464, 7661, 15679], sorted([i.pk for i in items[2:]])) + + res = self.client.get(url.replace('en-US', 'es-ES')) + items = res.context['filter'].filter_featured() + eq_([1001, 2464, 7661, 15679], sorted([i.pk for i in items])) + + self.change_addon(another, 'es-ES') + items = res.context['filter'].filter_featured() + assert items[0].pk == another.pk + eq_([1001, 2464, 7661, 15679], sorted([i.pk for i in items[1:]])) + + class TestListingByStatus(test_utils.TestCase): fixtures = ['base/apps', 'base/addon_3615'] @@ -609,7 +777,6 @@ class TestFeaturedPage(test_utils.TestCase): def test_featured_addons(self): """Make sure that only featured add-ons are shown""" - response = self.client.get(reverse('browse.featured')) eq_([1001, 1003], sorted(a.id for a in response.context['addons'])) diff --git a/apps/browse/views.py b/apps/browse/views.py index d27b488a5c..b3a673b479 100644 --- a/apps/browse/views.py +++ b/apps/browse/views.py @@ -178,8 +178,8 @@ class CategoryLandingFilter(BaseFilter): default) def filter_featured(self): - return Addon.objects.filter(addoncategory__feature=True, - addoncategory__category=self.category) + return Addon.objects.category_featured(category=self.category, + by_locale=amo.LOCALE_ALL) def category_landing(request, category): @@ -187,8 +187,11 @@ def category_landing(request, category): .filter(categories__id=category.id)) filter = CategoryLandingFilter(request, base, category, key='browse', default='featured') + boundary = (Addon.objects.category_featured(category=category, + by_locale=amo.LOCALE_CURRENT).count()) return jingo.render(request, 'browse/category_landing.html', {'category': category, 'filter': filter, + 'boundary': boundary, 'search_cat': '%s,0' % category.type}) @@ -196,10 +199,14 @@ def creatured(request, category): TYPE = amo.ADDON_EXTENSION q = Category.objects.filter(application=request.APP.id, type=TYPE) category = get_object_or_404(q, slug=category) - addons = Addon.objects.public().filter(addoncategory__feature=True, - addoncategory__category=category) + addons = (Addon.objects.public() & + Addon.objects.category_featured(category=category, + by_locale=amo.LOCALE_ALL)) + boundary = (Addon.objects.category_featured(category=category, + by_locale=amo.LOCALE_CURRENT).count()) return jingo.render(request, 'browse/creatured.html', - {'addons': addons, 'category': category}) + {'addons': addons, 'category': category, + 'boundary': boundary}) class PersonasFilter(BaseFilter): @@ -259,13 +266,24 @@ def personas(request, category=None): template = 'category_landing.html' addons = amo.utils.paginate(request, filter.qs, 30, count=count) + boundary = 0 + if category: + featured = base & Addon.objects.category_featured(category=category, + by_locale=amo.LOCALE_ALL) + boundary = (Addon.objects.category_featured(category=category, + by_locale=amo.LOCALE_CURRENT).count()) + else: + featured = base & Addon.objects.featured(request.APP, + by_locale=amo.LOCALE_ALL) + boundary = (Addon.objects.featured(request.APP, + by_locale=amo.LOCALE_CURRENT).count()) - featured = base & Addon.objects.featured(request.APP) is_homepage = category is None and 'sort' not in request.GET return jingo.render(request, 'browse/personas/' + template, {'categories': categories, 'category': category, 'filter': filter, 'addons': addons, - 'featured': featured, 'is_homepage': is_homepage, + 'featured': featured, + 'boundary': boundary, 'is_homepage': is_homepage, 'search_cat': 'personas'}) @@ -381,5 +399,8 @@ def search_tools(request, category=None): def featured(request, category=None): - addons = Addon.objects.featured(request.APP) - return jingo.render(request, 'browse/featured.html', {'addons': addons}) + count = (Addon.objects.featured(request.APP, by_locale=amo.LOCALE_CURRENT) + .count()) + addons = Addon.objects.featured(request.APP, by_locale=amo.LOCALE_ALL) + return jingo.render(request, 'browse/featured.html', + {'addons': addons, 'boundary': count})