Merge branch 'featured'

apps/addons/buttons.py

@@ -39,7 +39,7 @@ def _install_button(context, addon, version=None, show_eula=True,
         template = 'addons/impala/button.html'
     elif mobile:
         template = 'addons/mobile/button.html'
-    else: 
+    else:
         template = 'addons/button.html'
     t = jingo.render_to_string(request, template, c)
     return jinja2.Markup(t)
@@ -129,8 +129,7 @@ def __init__(self, addon, app, lang, version=None, show_eula=True,
                          and not self.lite
                          and not self.self_hosted
                          and not self.is_beta
-                         and addon.is_featured(app, lang)
-                         or addon.is_category_featured(app, lang))
+                         and addon.is_featured(app, lang))
         self.is_persona = addon.type == amo.ADDON_PERSONA
 
         self.accept_eula = addon.has_eula and not show_eula

apps/addons/cron.py

@@ -22,7 +22,7 @@
 from amo.utils import chunked
 from addons import search
 from addons.models import Addon, FrozenAddon, AppSupport
-from addons.utils import ReverseNameLookup
+from addons.utils import ReverseNameLookup, FeaturedManager, CreaturedManager
 from files.models import File
 from stats.models import UpdateCount
 from translations.models import Translation
@@ -445,3 +445,9 @@ def reindex_addons():
     ts = [tasks.index_addons.subtask(args=[chunk])
           for chunk in chunked(sorted(list(ids)), 150)]
     TaskSet(ts).apply_async()
+
+
+@cronjobs.register
+def reset_featured_addons():
+    FeaturedManager.build()
+    CreaturedManager.build()

apps/addons/forms.py

@@ -235,7 +235,7 @@ def __init__(self, *args, **kw):
             # If this add-on is featured for this application, category
             # categories are forbidden.
             form.disabled = (settings.NEW_FEATURES and
-                             self.addon.is_category_featured(app))
+                             self.addon.is_featured(app))
 
     def save(self):
         for f in self.forms:

apps/addons/helpers.py

@@ -21,7 +21,7 @@ def statusflags(context, addon):
     lang = context['LANG']
     if addon.is_unreviewed():
         return 'unreviewed'
-    elif addon.is_featured(app, lang) or addon.is_category_featured(app, lang):
+    elif addon.is_featured(app, lang):
         return 'featuredaddon'
     elif addon.is_selfhosted():
         return 'selfhosted'

apps/addons/models.py

@@ -2,7 +2,6 @@
 import itertools
 import json
 import os
-import random
 import time
 from datetime import datetime, timedelta
 
@@ -25,7 +24,7 @@
 from amo.utils import (send_mail, urlparams, sorted_groupby, JSONEncoder,
                        slugify, to_language)
 from amo.urlresolvers import get_outgoing_url, reverse
-from addons.utils import ReverseNameLookup
+from addons.utils import ReverseNameLookup, FeaturedManager, CreaturedManager
 from files.models import File
 from reviews.models import Review
 from stats.models import AddonShareCountTotal
@@ -75,16 +74,8 @@ def featured(self, app):
         """
         Filter for all featured add-ons for an application in all locales.
         """
-        if settings.NEW_FEATURES:
-            from bandwagon.models import FeaturedCollection
-            ids = FeaturedCollection.objects.addon_ids(app=app)
-            return self.valid().filter(id__in=ids)
-        else:
-            return self.valid().filter(feature__application=app.id)
-
-    def category_featured(self):
-        """Get all category-featured add-ons for ``app`` in all locales."""
-        return self.filter(addoncategory__feature=True)
+        ids = FeaturedManager.featured_ids(app)
+        return amo.models.manual_order(self.valid(), ids, 'addons.id')
 
     def listed(self, app, *status):
         """
@@ -612,11 +603,6 @@ def transformer(addons):
         # Personas need categories for the JSON dump.
         Category.transformer(personas)
 
-        # Store creatured apps on the add-on.
-        creatured = AddonCategory.creatured()
-        for addon in addons:
-            addon._creatured_apps = creatured.get(addon.id, [])
-
         # Attach sharing stats.
         sharing.attach_share_counts(AddonShareCountTotal, 'addon', addon_dict)
 
@@ -739,71 +725,18 @@ def is_unreviewed(self):
     def is_incomplete(self):
         return self.status == amo.STATUS_NULL
 
-    @classmethod
-    def featured(cls, app, lang):
-        # TODO(cvan): Remove this once featured collections are enabled.
-        if settings.NEW_FEATURES:
-            from bandwagon.models import FeaturedCollection
-            ids = FeaturedCollection.objects.addon_ids(app=app, lang=lang)
-            return ids
-        else:
-            # Attach an instance of Feature to Addon so we get persistent
-            # @cached_method caching through the request.
-            if not hasattr(cls, '_feature'):
-                cls._feature = Feature()
-            features = cls._feature.by_app(app)
-            return dict((id, locales) for id, locales in features.items()
-                         if None in locales or lang in locales)
-
     @classmethod
     def featured_random(cls, app, lang):
-        """
-        Sort featured ids for app into those that support lang and
-        those with no locale, then shuffle both groups.
-        """
-        if settings.NEW_FEATURES:
-            from bandwagon.models import FeaturedCollection
-            features = FeaturedCollection.objects.by_locale(app=app, lang=lang)
-            no_locale, specific_locale = [], []
-            for f in features:
-                pk = f['collection__addons']
-                if not pk:
-                    continue
-                if f['locale'] == lang:
-                    specific_locale.append(pk)
-                elif f['locale'] is None:
-                    no_locale.append(pk)
-            random.shuffle(specific_locale)
-            random.shuffle(no_locale)
-            return specific_locale + no_locale
-        else:
-            if not hasattr(cls, '_feature'):
-                cls._feature = Feature()
-            features = cls._feature.by_app(app)
-            no_locale, specific_locale = [], []
-            for id, locales in features.items():
-                if lang in locales:
-                    specific_locale.append(id)
-                elif None in locales:
-                    no_locale.append(id)
-            random.shuffle(no_locale)
-            random.shuffle(specific_locale)
-            return specific_locale + no_locale
+        return FeaturedManager.featured_ids(app, lang)
 
     def is_no_restart(self):
         """Is this a no-restart add-on?"""
         files = self.current_version.all_files
         return files and files[0].no_restart
 
-    def is_featured(self, app, lang):
+    def is_featured(self, app, lang=None):
         """Is add-on globally featured for this app and language?"""
-        return self.id in Addon.featured(app, lang)
-
-    def is_category_featured(self, app, lang=None):
-        """Is add-on featured in any category for this app?"""
-        if lang:
-            return (app.id, lang) in self._creatured_apps
-        return app.id in dict(self._creatured_apps)
+        return self.id in FeaturedManager.featured_ids(app, lang)
 
     def has_full_profile(self):
         """Is developer profile public (completed)?"""
@@ -813,13 +746,6 @@ def has_profile(self):
         """Is developer profile (partially or entirely) completed?"""
         return self.the_reason or self.the_future
 
-    @amo.cached_property(writable=True)
-    def _creatured_apps(self):
-        """Return a list of the apps where this add-on is creatured."""
-        # This exists outside of is_category_featured so we can write the list
-        # in the transformer and avoid repeated .creatured() calls.
-        return AddonCategory.creatured().get(self.id, [])
-
     @amo.cached_property
     def tags_partitioned_by_developer(self):
         """Returns a tuple of developer tags and user tags for this addon."""
@@ -1195,45 +1121,9 @@ def flush_urls(self):
                 '*%s' % self.category.get_url_path(), ]
         return urls
 
-    @classmethod
-    def creatured_ids(cls):
-        """Returns a list of creatured ids."""
-        if settings.NEW_FEATURES:
-            from bandwagon.models import FeaturedCollection
-            return FeaturedCollection.objects.creatured_ids()
-        else:
-            # TODO(cvan): Remove this once new features are enabled.
-            qs = cls.objects.filter(feature=True)
-            f = lambda: list(qs.values_list('addon', 'category',
-                                            'category__application',
-                                            'feature_locales'))
-            return caching.cached_with(qs, f, 'creatured')
-
-    @classmethod
-    def creatured(cls):
-        """Get a dict of {addon: [app,..]} for all creatured add-ons."""
-        rv = {}
-        for addon, cat, catapp, locale in cls.creatured_ids():
-            rv.setdefault(addon, []).append((catapp, locale))
-        return rv
-
     @classmethod
     def creatured_random(cls, category, lang):
-        """
-        Sort creatured ids for category into those that support lang and
-        those with no locale, then shuffle both groups.
-        """
-        no_locale, specific_locale = [], []
-        for addon, cat, catapp, locale in cls.creatured_ids():
-            if cat != category.pk:
-                continue
-            if locale and lang in locale:
-                specific_locale.append(addon)
-            elif not locale:
-                no_locale.append(addon)
-        random.shuffle(no_locale)
-        random.shuffle(specific_locale)
-        return specific_locale + no_locale
+        return CreaturedManager.creatured_ids(category, lang)
 
 
 class AddonRecommendation(models.Model):
@@ -1377,20 +1267,6 @@ def __unicode__(self):
         app = amo.APP_IDS[self.application.id].pretty
         return '%s (%s: %s)' % (self.addon.name, app, self.locale)
 
-    @caching.cached_method
-    def by_app(self, app):
-        """
-        Get a dict of featured add-ons for app.
-
-        Returns: {addon_id: [featured locales]}
-        """
-        qs = Addon.objects.featured(app)
-        vals = qs.extra(select={'locale': 'features.locale'})
-        d = collections.defaultdict(list)
-        for id, locale in vals.values_list('id', 'locale'):
-            d[id].append(locale)
-        return dict(d)
-
 
 class Preview(amo.models.ModelBase):
     addon = models.ForeignKey(Addon, related_name='previews')

apps/addons/tests/test_buttons.py

@@ -24,7 +24,6 @@ class ButtonTest(amo.tests.TestCase):
     def setUp(self):
         self.addon = Mock()
         self.addon.is_featured.return_value = False
-        self.addon.is_category_featured.return_value = False
         self.addon.is_unreviewed.return_value = False
         self.addon.has_eula = False
         self.addon.status = amo.STATUS_PUBLIC
@@ -232,11 +231,6 @@ def test_featured(self):
         eq_(b.install_class, ['featuredaddon'])
         eq_(b.install_text, 'Featured')
 
-    def test_category_featured(self):
-        self.addon.is_category_featured.return_value = True
-        b = self.get_button()
-        assert b.featured
-
     def test_unreviewed(self):
         # Throw featured in there to make sure it's ignored.
         self.addon.is_featured.return_value = True

apps/addons/tests/test_helpers.py

@@ -14,11 +14,6 @@ class TestHelpers(amo.tests.TestCase):
     fixtures = ['addons/featured', 'base/collections', 'base/featured',
                 'bandwagon/featured_collections']
 
-    def setUp(self):
-        # Addon._feature keeps an in-process cache we need to clear.
-        if hasattr(Addon, '_feature'):
-            del Addon._feature
-
     def test_statusflags(self):
         ctx = {'APP': amo.FIREFOX, 'LANG': 'en-US'}
 

apps/addons/tests/test_models.py

@@ -20,10 +20,8 @@
 from amo.signals import _connect, _disconnect
 from addons.models import (Addon, AddonCategory, AddonDependency,
                            AddonRecommendation, AddonType, BlacklistedGuid,
-                           Category, Charity, Feature, FrozenAddon, Persona,
-                           Preview)
+                           Category, Charity, FrozenAddon, Persona, Preview)
 from applications.models import Application, AppVersion
-from bandwagon.models import CollectionAddon, FeaturedCollection
 from devhub.models import ActivityLog
 from files.models import File, Platform
 from files.tests.test_models import TestLanguagePack, UploadTest
@@ -44,14 +42,16 @@ def setUp(self):
 
     @patch.object(settings, 'NEW_FEATURES', False)
     def test_featured(self):
-        featured = Addon.objects.featured(amo.FIREFOX)[0]
-        eq_(featured.id, 1)
-        eq_(Addon.objects.featured(amo.FIREFOX).count(), 5)
+        eq_(Addon.objects.featured(amo.FIREFOX).count(),
+            Addon.objects.valid().filter(feature__application=amo.FIREFOX.id)
+            .count())
 
     @patch.object(settings, 'NEW_FEATURES', True)
     def test_new_featured(self):
-        featured = Addon.objects.featured(amo.FIREFOX)[0]
-        eq_(featured.id, 1001)
+        # TODO: remove this when NEW_FEATURES goes away. It's here because
+        # build() was already called in setUp().
+        from addons.cron import reset_featured_addons
+        reset_featured_addons()
         eq_(Addon.objects.featured(amo.FIREFOX).count(), 6)
 
     def test_listed(self):
@@ -126,7 +126,7 @@ class TestAddonManagerFeatured(amo.tests.TestCase):
     def test_new_featured(self):
         f = Addon.objects.featured(amo.FIREFOX)
         eq_(f.count(), 6)
-        eq_(sorted(f.values_list('id', flat=True)),
+        eq_(sorted(x.id for x in f),
             [1001, 1003, 2464, 3481, 7661, 15679])
         f = Addon.objects.featured(amo.SUNBIRD)
         assert not f.exists()
@@ -151,10 +151,6 @@ class TestAddonModels(amo.tests.TestCase):
 
     def setUp(self):
         TranslationSequence.objects.create(id=99243)
-        # Addon._feature keeps an in-process cache we need to clear.
-        if hasattr(Addon, '_feature'):
-            del Addon._feature
-
         # TODO(andym): use Mock appropriately here.
         self.old_version = amo.FIREFOX.latest_version
         amo.FIREFOX.latest_version = '3.6.15'
@@ -372,49 +368,6 @@ def test_is_featured(self):
         assert a.is_featured(amo.FIREFOX, 'en-US'), (
             'globally featured add-on not recognized')
 
-    @patch.object(settings, 'NEW_FEATURES', False)
-    def test_is_category_featured(self):
-        """Test if an add-on is category featured."""
-        Feature.objects.filter(addon=1001).delete()
-        a = Addon.objects.get(pk=1001)
-        assert not a.is_featured(amo.FIREFOX, 'en-US'), (
-            "Expected add-on should not be in 'en-US' locale")
-
-        assert a.is_category_featured(amo.FIREFOX), (
-            'Expected add-on to be category-featured for the default locale')
-        assert not a.is_category_featured(amo.FIREFOX, 'fr'), (
-            "Expected add-on to not be in 'fr' locale")
-
-        AddonCategory.objects.filter(addon=a).delete()
-        cache.clear()
-        a = Addon.objects.get(pk=1001)
-        assert not a.is_category_featured(amo.FIREFOX), (
-            'Expected add-on to not be category-featured')
-
-    @patch.object(settings, 'NEW_FEATURES', True)
-    def test_new_is_category_featured(self):
-        """Test if an add-on is category featured."""
-        a = Addon.objects.get(pk=1001)
-        assert a.is_category_featured(amo.FIREFOX), (
-            'Expected add-on to have no locale')
-        assert not a.is_category_featured(amo.FIREFOX, 'fr'), (
-            "Expected add-on to not be in 'fr' locale")
-
-        fc = FeaturedCollection.objects.filter(collection__addons=1001)[0]
-        c = CollectionAddon.objects.filter(addon=a,
-                                           collection=fc.collection)[0]
-        c.delete()
-        assert not a.is_featured(amo.FIREFOX, 'en-US'), (
-            "Expected add-on to be in 'en-US' locale")
-        assert a.is_category_featured(amo.FIREFOX), (
-            'Expected add-on to be category-featured for the default locale')
-
-        AddonCategory.objects.filter(addon=a).delete()
-        cache.clear()
-        a = Addon.objects.get(pk=1001)
-        assert not a.is_category_featured(amo.FIREFOX), (
-            'Expected add-on to not be category-featured')
-
     def test_has_full_profile(self):
         """Test if an add-on's developer profile is complete (public)."""
         addon = lambda: Addon.objects.get(pk=3615)