Merge pull request #11274 from diox/discopane-extra-metadata

Add more addon data and text variants of heading/description to discopane API

docs/topics/api/discovery.rst

@@ -39,9 +39,11 @@ Firefox (about:addons).
     :>json int count: The number of results for this query.
     :>json array results: The array containing the results for this query.
     :>json string results[].heading: The heading for this item. May contain some HTML tags.
+    :>json string|null results[].heading_text: The heading for this item. Text-only, content might slightly differ from ``heading`` because of that.
     :>json string|null results[].description: The description for this item, if any. May contain some HTML tags.
+    :>json string|null results[].description_text: The description for this item, if any. Text-only, content might slightly differ from ``description`` because of that.
     :>json boolean results[].is_recommendation: If this item was from the recommendation service, rather than static curated content.
-    :>json object results[].addon: The :ref:`add-on <addon-detail-object>` for this item. Only a subset of fields are present: ``id``, ``current_version`` (with only the ``id``, ``compatibility``, ``is_strict_compatibility_enabled`` and ``files`` fields present), ``guid``, ``icon_url``, ``name``, ``previews``, ``slug``, ``theme_data``, ``type`` and ``url``.
+    :>json object results[].addon: The :ref:`add-on <addon-detail-object>` for this item. Only a subset of fields are present: ``id``, ``authors``, ``average_daily_users``, ``current_version`` (with only the ``id``, ``compatibility``, ``is_strict_compatibility_enabled`` and ``files`` fields present), ``guid``, ``icon_url``, ``name``, ``ratings``, ``previews``, ``slug``, ``theme_data``, ``type`` and ``url``.
 
 
 -----------------

src/olympia/discovery/admin.py

@@ -94,6 +94,9 @@ class DiscoveryItemAdmin(admin.ModelAdmin):
             db_field, request, **kwargs)
 
     def build_preview(self, obj, locale):
+        # FIXME: when disco in Firefox itself lands, change this preview to
+        # match the one Firefox uses.
+        # https://github.com/mozilla/addons-server/issues/11272
         return format_html(
             u'<div class="discovery-preview" data-locale="{}">'
             u'<h2 class="heading">{}</h2>'

src/olympia/discovery/management/commands/extract_disco_strings.py

@@ -29,12 +29,13 @@ class Command(BaseCommand):
 
     def build_output_for_item(self, item):
         output = []
-        heading = item.get('custom_heading')
-        description = item.get('custom_description')
-        if heading:
-            output.append(self.build_output_for_single_value(heading))
-        if description:
-            output.append(self.build_output_for_single_value(description))
+        custom_heading = item.get('custom_heading')
+        custom_description = item.get('custom_description')
+        if custom_heading:
+            output.append(self.build_output_for_single_value(custom_heading))
+        if custom_description:
+            output.append(
+                self.build_output_for_single_value(custom_description))
         return u''.join(output)
 
     def build_output_for_single_value(self, value):

src/olympia/discovery/models.py

@@ -67,48 +67,48 @@ class DiscoveryItem(ModelBase):
         })
         return qs.urlencode()
 
-    @property
-    def heading(self):
-        """
-        Return item heading (translated, including HTML) ready to be returned
-        by the disco pane API.
-        """
+    def _build_heading(self, html=False):
         addon_name = six.text_type(self.custom_addon_name or self.addon.name)
-        authors = u', '.join(
-            author.name for author in self.addon.listed_authors)
-        url = absolutify(self.addon.get_url_path())
 
-        # addons-frontend will add target and rel attributes to the <a> link.
-        # Note: The translated "by" in the middle of both strings is
-        # unfortunate, but the full strings are too opaque/dangerous to be
-        # handled by translators, since they are just HTML and parameters.
-        if self.custom_heading:
-            addon_link = format_html(
-                # The query string should not be encoded twice, so we add it to
-                # the template first, via '%'.
-                u'<a href="{0}?%(query)s">{1} {2} {3}</a>' % {
-                    'query': self.build_querystring()},
-                url, addon_name, ugettext(u'by'), authors)
+        if html:
+            authors = u', '.join(
+                author.name for author in self.addon.listed_authors)
+            url = absolutify(self.addon.get_url_path())
+            # addons-frontend will add target and rel attributes to the <a>
+            # link. Note: The translated "by" in the middle of both strings is
+            # unfortunate, but the full strings are too opaque/dangerous to be
+            # handled by translators, since they are just HTML and parameters.
+            if self.custom_heading:
+                addon_link = format_html(
+                    # The query string should not be encoded twice, so we add
+                    # it to the template first, via '%'.
+                    u'<a href="{0}?%(query)s">{1} {2} {3}</a>' % {
+                        'query': self.build_querystring()},
+                    url, addon_name, ugettext(u'by'), authors)
 
-            value = conditional_escape(ugettext(self.custom_heading)).replace(
-                u'{start_sub_heading}', u'<span>').replace(
-                u'{end_sub_heading}', u'</span>').replace(
-                u'{addon_name}', addon_link)
+                value = conditional_escape(
+                    ugettext(self.custom_heading)).replace(
+                        u'{start_sub_heading}', u'<span>').replace(
+                        u'{end_sub_heading}', u'</span>').replace(
+                        u'{addon_name}', addon_link)
+            else:
+                value = format_html(
+                    # The query string should not be encoded twice, so we add
+                    # it to the template first, via '%'.
+                    u'{0} <span>{1} <a href="{2}?%(query)s">{3}</a></span>' % {
+                        'query': self.build_querystring()},
+                    addon_name, ugettext(u'by'), url, authors)
         else:
-            value = format_html(
-                # The query string should not be encoded twice, so we add it to
-                # the template first, via '%'.
-                u'{0} <span>{1} <a href="{2}?%(query)s">{3}</a></span>' % {
-                    'query': self.build_querystring()},
-                addon_name, ugettext(u'by'), url, authors)
+            if self.custom_heading:
+                value = self.custom_heading.replace(
+                    '{start_sub_heading}', '').replace(
+                    '{end_sub_heading}', '').replace(
+                    '{addon_name}', addon_name)
+            else:
+                value = addon_name
         return value
 
-    @property
-    def description(self):
-        """
-        Return item description (translated, including HTML) ready to be
-        returned by the disco pane API.
-        """
+    def _build_description(self, html=False):
         if self.custom_description:
             value = ugettext(self.custom_description)
         else:
@@ -119,5 +119,44 @@ class DiscoveryItem(ModelBase):
                 value = addon.description
             else:
                 value = u''
-        return format_html(
-            u'<blockquote>{}</blockquote>', value) if value else value
+        if html:
+            return format_html(
+                u'<blockquote>{}</blockquote>', value) if value else value
+        else:
+            return value
+
+    @property
+    def heading(self):
+        """
+        Return item heading (translated, including HTML) ready to be returned
+        by the disco pane API.
+        """
+        return self._build_heading(html=True)
+
+    @property
+    def heading_text(self):
+        """
+        Return item heading (translated, but not including HTML) ready to be
+        returned by the disco pane API.
+
+        It may differ from the HTML version slightly and contain less
+        information, leaving clients the choice to use extra data returned by
+        the API or not.
+        """
+        return self._build_heading(html=False)
+
+    @property
+    def description(self):
+        """
+        Return item description (translated, including HTML) ready to be
+        returned by the disco pane API.
+        """
+        return self._build_description(html=True)
+
+    @property
+    def description_text(self):
+        """
+        Return item description (translated, but not including HTML) ready to
+        be returned by the disco pane API.
+        """
+        return self._build_description(html=False)

src/olympia/discovery/serializers.py

@@ -41,19 +41,23 @@ class DiscoveryAddonSerializer(AddonSerializer):
     current_version = DiscoveryVersionSerializer()
 
     class Meta:
-        fields = ('id', 'current_version', 'guid', 'icon_url', 'name',
-                  'previews', 'slug', 'theme_data', 'type', 'url',)
+        fields = ('id', 'authors', 'average_daily_users', 'current_version',
+                  'guid', 'icon_url', 'name', 'previews', 'ratings', 'slug',
+                  'theme_data', 'type', 'url',)
         model = Addon
 
 
 class DiscoverySerializer(serializers.ModelSerializer):
     heading = serializers.CharField()
     description = serializers.CharField()
+    heading_text = serializers.CharField()
+    description_text = serializers.CharField()
     addon = DiscoveryAddonSerializer()
     is_recommendation = serializers.SerializerMethodField()
 
     class Meta:
-        fields = ('heading', 'description', 'addon', 'is_recommendation')
+        fields = ('heading', 'description', 'heading_text', 'description_text',
+                  'addon', 'is_recommendation')
         model = DiscoveryItem
 
     def get_is_recommendation(self, obj):

src/olympia/discovery/tests/test_models.py

@@ -138,6 +138,42 @@ class TestDiscoveryItem(TestCase):
             u'&lt;script&gt;alert(669)&lt;/script&gt;</a></span>').format(
                 item.build_querystring())
 
+    def test_heading_text(self):
+        addon = addon_factory(slug='somé-slug', name='Sôme Name')
+        user = user_factory(display_name='Fløp')
+        addon.addonuser_set.create(user=user)
+        item = DiscoveryItem.objects.create(addon=addon)
+        assert item.heading_text == 'Sôme Name'
+
+    def test_heading_text_custom_addon_name(self):
+        addon = addon_factory(slug='somé-slug', name='Sôme Name')
+        user = user_factory(display_name='Fløp')
+        addon.addonuser_set.create(user=user)
+        item = DiscoveryItem.objects.create(
+            addon=addon, custom_addon_name='Custôm Name')
+        assert item.heading_text == 'Custôm Name'
+
+    def test_heading_text_custom(self):
+        addon = addon_factory(slug='somé-slug', name=u'Sôme Name')
+        user = user_factory(display_name='Fløp')
+        addon.addonuser_set.create(user=user)
+        item = DiscoveryItem.objects.create(
+            addon=addon,
+            custom_heading=('Fancy Héading {start_sub_heading}with '
+                            '{addon_name}{end_sub_heading}.'))
+        assert item.heading_text == 'Fancy Héading with Sôme Name.'
+
+    def test_heading_text_custom_with_custom_addon_name(self):
+        addon = addon_factory(slug='somé-slug', name='Sôme Name')
+        user = user_factory(display_name='Fløp')
+        addon.addonuser_set.create(user=user)
+        item = DiscoveryItem.objects.create(
+            addon=addon,
+            custom_addon_name='Custôm Name',
+            custom_heading=('Fancy Héading {start_sub_heading}with '
+                            '{addon_name}{end_sub_heading}.'))
+        assert item.heading_text == 'Fancy Héading with Custôm Name.'
+
     def test_description_custom(self):
         addon = addon_factory(summary='Foo', description='Bar')
         item = DiscoveryItem.objects.create(
@@ -185,6 +221,25 @@ class TestDiscoveryItem(TestCase):
             type=amo.ADDON_DICT))
         assert item.description == u''
 
+    def test_description_text_custom(self):
+        addon = addon_factory(summary='Foo', description='Bar')
+        item = DiscoveryItem.objects.create(
+            addon=addon, custom_description='Custôm Desc.')
+        assert item.description_text == 'Custôm Desc.'
+
+    def test_description_text_non_custom_extension(self):
+        addon = addon_factory(summary='')
+        item = DiscoveryItem.objects.create(addon=addon)
+        assert item.description_text == ''
+
+        addon.summary = 'Mÿ Summary'
+        assert item.description_text == 'Mÿ Summary'
+
+    def test_description_text_non_custom_fallback(self):
+        item = DiscoveryItem.objects.create(addon=addon_factory(
+            type=amo.ADDON_DICT))
+        assert item.description_text == ''
+
     @override_settings(DOMAIN='addons.mozilla.org')
     def test_build_querystring(self):
         item = DiscoveryItem.objects.create(addon=addon_factory(

src/olympia/discovery/tests/test_views.py

@@ -61,6 +61,8 @@ class DiscoveryTestMixin(object):
 
         assert result['heading'] == item.heading
         assert result['description'] == item.description
+        assert result['heading_text'] == item.heading_text
+        assert result['description_text'] == item.description_text
 
         self._check_disco_addon_version(
             result['addon']['current_version'], addon.current_version)
@@ -78,6 +80,8 @@ class DiscoveryTestMixin(object):
 
         assert result['heading'] == item.heading
         assert result['description'] == item.description
+        assert result['heading_text'] == item.heading_text
+        assert result['description_text'] == item.description_text
 
 
 class TestDiscoveryViewList(DiscoveryTestMixin, TestCase):