Allow multiple images for operator shelf (bug 1067438)

docs/api/topics/feed.rst

@@ -1033,6 +1033,7 @@ Operator shelves are represented thusly:
             }
         ],
         "background_image": "http://somecdn.com/someimage.png",
+        "background_image_landing": "http://somecdn.com/some-other-image.png",
         "carrier": "telefonica",
         "description": {
             "en-US": "A description of my collection."
@@ -1051,6 +1052,9 @@ Operator shelves are represented thusly:
     *array* - a list of serializations of the member :ref:`apps <app>`.
 ``background_image``
     *string* - the URL to an image used while displaying the operator shelf.
+``background_image_landing``
+    *string* - the URL to an image used while displaying the operator
+    shelf landing page.
 ``carrier``
     *string* - the slug of the :ref:`carrier <carriers>` the operator shelf
     belongs to.
@@ -1119,6 +1123,8 @@ Create
     :type apps: array
     :param background_image_upload_url: a URL pointing to an image
     :type background_image_upload_url: string
+    :param background_image_landing_upload_url: a URL pointing to an image
+    :type background_image_landing_upload_url: string
     :param carrier: the slug of a :ref:`carrier <carriers>`.
     :type carrier: string
     :param description: a :ref:`translated <overview-translations>` description
@@ -1137,6 +1143,7 @@ Create
         {
             "apps": [19, 1, 44],
             "background_image_upload_url": "http://imgur.com/XXX.jpg",
+            "background_image_landing_upload_url": "http://imgur.com/YYY.jpg",
             "carrier": "telefonica",
             "description": {
                 "en-US": "A list of Telefonica's Favorite apps."
@@ -1171,6 +1178,8 @@ Update
     :type apps: array
     :param background_image_upload_url: a URL pointing to an image
     :type background_image_upload_url: string
+    :param background_image_landing_upload_url: a URL pointing to an image
+    :type background_image_landing_upload_url: string
     :param carrier: the slug of a :ref:`carrier <carriers>`.
     :type carrier: string
     :param description: a :ref:`translated <overview-translations>` description
@@ -1189,6 +1198,7 @@ Update
         {
             "apps": [19, 1, 44],
             "background_image_upload_url": "http://imgur.com/XXX.jpg",
+            "background_image_landing_upload_url": "http://imgur.com/YYY.jpg",
             "carrier": "telefonica",
             "description": {
                 "en-US": "A list of Telefonica's Favorite apps."
@@ -1229,7 +1239,7 @@ Delete
 
 
 Image
-==============
+=====
 
 One-to-one background image or header graphic used to display with the operator
 shelf.

migrations/850-feed-shelf.sql

@@ -0,0 +1 @@
+ALTER TABLE mkt_feed_shelf ADD COLUMN image_landing_hash varchar(8);

mkt/api/v2/urls.py

@@ -28,6 +28,8 @@
 subfeedshelf = SubRouterWithFormat()
 subfeedshelf.register('image', views.FeedShelfImageViewSet,
                       base_name='feed-shelf-image')
+subfeedshelf.register('image_landing', views.FeedShelfLandingImageViewSet,
+                      base_name='feed-shelf-landing-image')
 
 urlpatterns = patterns('',
     url(r'^apps/search/rocketbar/', RocketbarViewV2.as_view(),

mkt/collections/models.py

@@ -72,7 +72,8 @@ def clean_slug(self):
     def get_fallback(cls):
         return cls._meta.get_field('default_language')
 
-    def image_path(self):
+    def image_path(self, suffix=''):
+        # The argument `suffix` isn't used here but is in the feed.
         return os.path.join(settings.COLLECTIONS_ICON_PATH,
                             str(self.pk / 1000),
                             'app_collection_%s.png' % (self.pk,))

mkt/collections/views.py

@@ -252,6 +252,9 @@ class CollectionImageViewSet(CORSMixin, SlugOrIdMixin, MarketplaceView,
                               RestAnonymousAuthentication]
     cors_allowed_methods = ('get', 'put', 'delete')
 
+    hash_field = 'image_hash'
+    image_suffix = ''
+
     # Dummy serializer to keep DRF happy when it's answering to OPTIONS.
     serializer_class = serializers.Serializer
 
@@ -269,7 +272,7 @@ def retrieve(self, request, *args, **kwargs):
         obj = self.get_object()
         if not obj.has_image:
             raise Http404
-        return HttpResponseSendFile(request, obj.image_path(),
+        return HttpResponseSendFile(request, obj.image_path(self.image_suffix),
                                     content_type='image/png')
 
     def update(self, request, *args, **kwargs):
@@ -279,17 +282,17 @@ def update(self, request, *args, **kwargs):
         except ValidationError:
             return Response(status=status.HTTP_400_BAD_REQUEST)
         i = Image.open(img)
-        with storage.open(obj.image_path(), 'wb') as f:
+        with storage.open(obj.image_path(self.image_suffix), 'wb') as f:
             i.save(f, 'png')
         # Store the hash of the original image data sent.
-        obj.update(image_hash=hash_)
+        obj.update(**{self.hash_field: hash_})
 
-        pngcrush_image.delay(obj.image_path())
+        pngcrush_image.delay(obj.image_path(self.image_suffix))
         return Response(status=status.HTTP_204_NO_CONTENT)
 
     def destroy(self, request, *args, **kwargs):
         obj = self.get_object()
         if obj.has_image:
-            storage.delete(obj.image_path())
-            obj.update(image_hash=None)
+            storage.delete(obj.image_path(self.image_suffix))
+            obj.update(**{self.hash_field: None})
         return Response(status=status.HTTP_204_NO_CONTENT)

mkt/feed/indexers.py

@@ -230,6 +230,7 @@ def get_mapping(cls):
                     'carrier': cls.string_not_analyzed(),
                     'created': {'type': 'date', 'format': 'dateOptionalTime'},
                     'image_hash': cls.string_not_analyzed(),
+                    'image_landing_hash': cls.string_not_analyzed(),
                     'item_type': cls.string_not_analyzed(),
                     'region': cls.string_not_analyzed(),
                     'search_names': {'type': 'string',
@@ -255,6 +256,7 @@ def extract_document(cls, pk=None, obj=None):
             'carrier': mkt.carriers.CARRIER_CHOICE_DICT[obj.carrier].slug,
             'created': obj.created,
             'image_hash': obj.image_hash,
+            'image_landing_hash': obj.image_landing_hash,
             'item_type': feed.FEED_TYPE_SHELF,
             'region': mkt.regions.REGIONS_CHOICES_ID_DICT[obj.region].slug,
             'search_names': list(set(string for _, string

mkt/feed/models.py

@@ -153,10 +153,6 @@ class BaseFeedImage(models.Model):
     class Meta:
         abstract = True
 
-    @property
-    def has_image(self):
-        return bool(self.image_hash)
-
 
 class BaseFeedCollectionMembership(amo.models.ModelBase):
     """
@@ -251,10 +247,11 @@ class Meta(BaseFeedCollection.Meta):
     def get_indexer(self):
         return indexers.FeedCollectionIndexer
 
-    def image_path(self):
+    def image_path(self, suffix=''):
         return os.path.join(settings.FEED_COLLECTION_BG_PATH,
                             str(self.pk / 1000),
-                            'feed_collection_%s.png' % (self.pk,))
+                            'feed_collection{suffix}_{pk}.png'.format(
+                                suffix=suffix, pk=self.pk))
 
     def add_app_grouped(self, app, group, order=None):
         """
@@ -312,6 +309,10 @@ class FeedShelf(BaseFeedCollection, BaseFeedImage):
     region = models.PositiveIntegerField(
         choices=mkt.regions.REGIONS_CHOICES_ID)
 
+    # Shelf landing image.
+    image_landing_hash = models.CharField(default=None, max_length=8,
+                                          null=True, blank=True)
+
     membership_class = FeedShelfMembership
     membership_relation = 'feedshelfmembership'
 
@@ -323,10 +324,11 @@ class Meta(BaseFeedCollection.Meta):
     def get_indexer(self):
         return indexers.FeedShelfIndexer
 
-    def image_path(self):
+    def image_path(self, suffix=''):
         return os.path.join(settings.FEED_SHELF_BG_PATH,
                             str(self.pk / 1000),
-                            'feed_shelf_%s.png' % (self.pk,))
+                            'feed_shelf{suffix}_{pk}.png'.format(
+                                suffix=suffix, pk=self.pk))
 
     @property
     def is_published(self):
@@ -372,10 +374,11 @@ def clean(self):
                                   'attribution is defined.')
         super(FeedApp, self).clean()
 
-    def image_path(self):
+    def image_path(self, suffix=''):
         return os.path.join(settings.FEATURED_APP_BG_PATH,
                             str(self.pk / 1000),
-                            'featured_app_%s.png' % (self.pk,))
+                            'featured_app{suffix}_{pk}.png'.format(
+                                suffix=suffix, pk=self.pk))
 
 
 class FeedItem(amo.models.ModelBase):

mkt/feed/serializers.py

@@ -13,7 +13,6 @@
 from mkt.api.serializers import URLSerializerMixin
 from mkt.carriers import CARRIER_CHOICE_DICT
 from mkt.constants.categories import CATEGORY_CHOICES
-from mkt.fireplace.serializers import FireplaceESAppSerializer
 from mkt.regions import REGIONS_CHOICES_ID_DICT
 from mkt.search.serializers import BaseESSerializer
 from mkt.submit.serializers import FeedPreviewESSerializer
@@ -86,20 +85,26 @@ def get_app_count(self, obj):
 
 class FeedImageField(serializers.HyperlinkedRelatedField):
     read_only = True
+    hash_field = 'image_hash'
 
     def get_url(self, obj, view_name, request, format):
-        if obj.has_image:
+        if getattr(obj, self.hash_field, None):
             # Always prefix with STATIC_URL to return images from our CDN.
             prefix = settings.STATIC_URL.strip('/')
             # Always append image_hash so that we can send far-future expires.
-            suffix = '?%s' % obj.image_hash
+            suffix = '?%s' % getattr(obj, self.hash_field)
             url = reverse(view_name, kwargs={'pk': obj.pk}, request=request,
                           format=format)
             return '%s%s%s' % (prefix, url, suffix)
         else:
             return None
 
 
+class FeedLandingImageField(FeedImageField):
+    read_only = True
+    hash_field = 'image_landing_hash'
+
+
 class FeedAppSerializer(ValidateSlugMixin, URLSerializerMixin,
                         serializers.ModelSerializer):
     """
@@ -296,6 +301,9 @@ class FeedShelfSerializer(BaseFeedCollectionSerializer):
     """
     background_image = FeedImageField(
         source='*', view_name='api-v2:feed-shelf-image-detail', format='png')
+    background_image_landing = FeedLandingImageField(
+        source='*', view_name='api-v2:feed-shelf-landing-image-detail',
+        format='png')
     carrier = SlugChoiceField(choices_dict=mkt.carriers.CARRIER_MAP)
     description = TranslationSerializerField(required=False)
     is_published = serializers.BooleanField(source='is_published',
@@ -304,9 +312,9 @@ class FeedShelfSerializer(BaseFeedCollectionSerializer):
     region = SlugChoiceField(choices_dict=mkt.regions.REGION_LOOKUP)
 
     class Meta:
-        fields = ['app_count', 'apps', 'background_image', 'carrier',
-                  'description', 'id', 'is_published', 'name', 'region',
-                  'slug', 'url']
+        fields = ['app_count', 'apps', 'background_image',
+                  'background_image_landing', 'carrier', 'description', 'id',
+                  'is_published', 'name', 'region', 'slug', 'url']
         model = FeedShelf
         url_basename = 'feedshelves'
 
@@ -324,7 +332,8 @@ class Meta(FeedShelfSerializer.Meta):
 
     def fake_object(self, data):
         shelf = self._attach_fields(FeedShelf(), data, (
-            'id', 'carrier', 'image_hash', 'region', 'slug'
+            'id', 'carrier', 'image_hash', 'image_landing_hash', 'region',
+            'slug'
         ))
         shelf = self._attach_translations(shelf, data, (
             'description', 'name'

mkt/feed/tests/test_indexers.py

@@ -123,7 +123,8 @@ def setUp(self):
         self.app_ids = [amo.tests.app_factory().id for app in range(3)]
         self.obj = self.feed_shelf_factory(
             app_ids=self.app_ids, name=self._get_test_l10n(),
-            description=self._get_test_l10n(), image_hash='LOL')
+            description=self._get_test_l10n(), image_hash='LOL',
+            image_landing_hash='ROFL')
         self.indexer = self.obj.get_indexer()()
         self.model = FeedShelf
 
@@ -135,6 +136,7 @@ def test_extract(self):
             mkt.carriers.CARRIER_CHOICE_DICT[self.obj.carrier].slug)
         self._assert_test_l10n(doc['description_translations'])
         eq_(doc['image_hash'], 'LOL')
+        eq_(doc['image_landing_hash'], 'ROFL')
         self._assert_test_l10n(doc['name_translations'])
         eq_(doc['region'],
             mkt.regions.REGIONS_CHOICES_ID_DICT[self.obj.region].slug)

mkt/feed/tests/test_serializers.py

@@ -10,8 +10,7 @@
 import mkt
 import mkt.feed.constants as feed
 from mkt.feed import serializers
-from mkt.feed.constants import (COLLECTION_LISTING, COLLECTION_PROMO,
-                                HOME_NUM_APPS_PROMO_COLL)
+from mkt.feed.constants import COLLECTION_LISTING, COLLECTION_PROMO
 from mkt.feed.models import FeedShelf
 from mkt.feed.tests.test_models import FeedAppMixin, FeedTestMixin
 from mkt.regions import RESTOFWORLD
@@ -67,8 +66,8 @@ def test_deserialize_many(self):
         data = serializers.FeedAppESSerializer(
             [self.data_es, self.data_es], context={
                 'app_map': self.app_map,
-                'request': amo.tests.req_factory_factory('')
-        }, many=True).data
+                'request': amo.tests.req_factory_factory('')},
+            many=True).data
         eq_(data[0]['app']['id'], self.feedapp.app_id)
         eq_(data[1]['description']['en-US'], 'test')
 
@@ -313,14 +312,16 @@ def test_deserialize(self):
         eq_(data['name']['en-US'], 'test')
 
     def test_background_image(self):
-        self.shelf.update(image_hash='LOL')
+        self.shelf.update(image_hash='LOL', image_landing_hash='ROFL')
         self.data_es = self.shelf.get_indexer().extract_document(
             None, obj=self.shelf)
         data = serializers.FeedShelfESSerializer(self.data_es, context={
             'app_map': self.app_map,
             'request': amo.tests.req_factory_factory('')
         }).data
         assert data['background_image'].endswith('image.png?LOL')
+        assert data['background_image_landing'].endswith(
+            'image_landing.png?ROFL')
 
 
 class TestFeedItemSerializer(FeedAppMixin, amo.tests.TestCase):