From 882c390f88be461be0aa78a7e653b9de491a5847 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Wed, 20 Jul 2016 11:58:24 +0200 Subject: [PATCH 1/3] Expose author information in add-on search and detail API --- docs/topics/api/addons.rst | 4 +++ src/olympia/addons/indexers.py | 18 +++++++++++ src/olympia/addons/serializers.py | 32 +++++++++++++++++--- src/olympia/addons/tests/test_indexers.py | 4 ++- src/olympia/addons/tests/test_serializers.py | 23 ++++++++++++-- 5 files changed, 73 insertions(+), 8 deletions(-) diff --git a/docs/topics/api/addons.rst b/docs/topics/api/addons.rst index a1cd2bbf89..4bab478246 100644 --- a/docs/topics/api/addons.rst +++ b/docs/topics/api/addons.rst @@ -68,6 +68,10 @@ This endpoint allows you to fetch a specific add-on by id, slug or guid. .. _addon-detail-object: :>json int id: The add-on id on AMO. + :>json array authors: Array holding information about the authors for this add-on. + :>json int authors[].id: The id for an author. + :>json string authors[].name: The name for an author. + :>json string authors[].url: The link to the profile page for an author. :>json object compatibility: Object detailing the add-on :ref:`add-on application ` and version compatibility. :>json object compatibility[app_name].max: Maximum version of the corresponding app the add-on is compatible with. :>json object compatibility[app_name].min: Minimum version of the corresponding app the add-on is compatible with. diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index a38a930d25..b6e6ec4fd2 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -36,6 +36,18 @@ class AddonIndexer(BaseSearchIndexer): 'app': {'type': 'byte'}, 'appversion': {'properties': {app.id: appver for app in amo.APP_USAGE}}, + 'author': { + 'type': 'object', + 'properties': { + 'id': {'type': 'long', 'index': 'no'}, + 'name': {'type': 'string', }, + 'username': {'type': 'string', 'index': 'no', }, + }, + }, + # FIXME: See issue #3120, the 'authors' property is for + # backwards-compatibility and all code should be switched + # to use 'author.name' instead. We needed a reindex first + # though, which is why the 2 are present at the moment. 'authors': {'type': 'string'}, 'average_daily_users': {'type': 'long'}, 'bayesian_rating': {'type': 'double'}, @@ -176,6 +188,12 @@ class AddonIndexer(BaseSearchIndexer): 'min': min_, 'min_human': min_human, 'max': max_, 'max_human': max_human, } + data['author'] = [{'name': a.name, 'id': a.id, 'username': a.username} + for a in obj.listed_authors] + # FIXME: See issue #3120, the 'authors' property is for + # backwards-compatibility and all code should be switched + # to use 'author.name' instead. We needed a reindex first + # though, which is why the 2 are present at the moment. data['authors'] = [a.name for a in obj.listed_authors] # Quadruple the boost if the add-on is public. if obj.status == amo.STATUS_PUBLIC and 'boost' in data: diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index c47329b51c..8c8d4533a6 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -10,6 +10,7 @@ from olympia.api.serializers import BaseESSerializer from olympia.applications.models import AppVersion from olympia.constants.applications import APPS_ALL from olympia.files.models import File +from olympia.users.models import UserProfile from olympia.versions.models import ApplicationsVersions, Version @@ -22,6 +23,17 @@ class AddonFeatureCompatibilitySerializer(serializers.ModelSerializer): fields = ('e10s', ) +class AddonAuthorSerializer(serializers.ModelSerializer): + url = serializers.SerializerMethodField() + + class Meta: + model = UserProfile + fields = ('name', 'url') + + def get_url(self, obj): + return absolutify(obj.get_url_path()) + + class FileSerializer(serializers.ModelSerializer): url = serializers.SerializerMethodField() platform = ReverseChoiceField(choices=amo.PLATFORM_CHOICES_API.items()) @@ -99,6 +111,7 @@ class VersionSerializer(serializers.ModelSerializer): class AddonSerializer(serializers.ModelSerializer): + authors = AddonAuthorSerializer(many=True, source='listed_authors') current_version = VersionSerializer() description = TranslationSerializerField() edit_url = serializers.SerializerMethodField() @@ -118,11 +131,12 @@ class AddonSerializer(serializers.ModelSerializer): class Meta: model = Addon - fields = ('id', 'current_version', 'default_locale', 'description', - 'edit_url', 'guid', 'homepage', 'icon_url', 'is_listed', - 'name', 'last_updated', 'previews', 'public_stats', - 'review_url', 'slug', 'status', 'summary', 'support_email', - 'support_url', 'tags', 'theme_data', 'type', 'url') + fields = ('id', 'authors', 'current_version', 'default_locale', + 'description', 'edit_url', 'guid', 'homepage', 'icon_url', + 'is_listed', 'name', 'last_updated', 'previews', + 'public_stats', 'review_url', 'slug', 'status', 'summary', + 'support_email', 'support_url', 'tags', 'theme_data', 'type', + 'url') def to_representation(self, obj): data = super(AddonSerializer, self).to_representation(obj) @@ -249,6 +263,14 @@ class ESAddonSerializer(BaseESSerializer, AddonSerializer): obj._current_version.compatible_apps = compatible_apps + data_authors = data.get('author', []) + obj.listed_authors = [ + UserProfile( + id=data_author['id'], display_name=data_author['name'], + username=data_author['username']) + for data_author in data_authors + ] + # We set obj.all_previews to the raw preview data because # ESPreviewSerializer will handle creating the fake Preview object # for us when its to_representation() method is called. diff --git a/src/olympia/addons/tests/test_indexers.py b/src/olympia/addons/tests/test_indexers.py index 539563c42d..df3db03157 100644 --- a/src/olympia/addons/tests/test_indexers.py +++ b/src/olympia/addons/tests/test_indexers.py @@ -44,7 +44,7 @@ class TestAddonIndexer(TestCase): # exist on the model, or it has a different name, or the value we need # to store in ES differs from the one in the db. complex_fields = [ - 'app', 'appversion', 'authors', 'boost', 'category', + 'app', 'appversion', 'author', 'authors', 'boost', 'category', 'current_version', 'description', 'has_theme_rereview', 'has_version', 'name', 'name_sort', 'platforms', 'previews', 'public_stats', 'summary', 'tags', @@ -142,6 +142,8 @@ class TestAddonIndexer(TestCase): 'min_human': '2.0', } } + assert extracted['author'] == [{'name': u'55021 التطب', 'id': 55021, + 'username': '55021'}] assert extracted['authors'] == [u'55021 التطب'] assert extracted['boost'] == self.addon.average_daily_users ** .2 * 4 assert extracted['category'] == [22, 23, 24] # From fixture. diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index 3795358a2a..9addc6d33f 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -6,10 +6,10 @@ from rest_framework.test import APIRequestFactory from olympia import amo from olympia.amo.helpers import absolutify -from olympia.amo.tests import addon_factory, ESTestCase, TestCase +from olympia.amo.tests import addon_factory, ESTestCase, TestCase, user_factory from olympia.amo.urlresolvers import reverse from olympia.addons.indexers import AddonIndexer -from olympia.addons.models import Addon, Persona, Preview +from olympia.addons.models import Addon, AddonUser, Persona, Preview from olympia.addons.serializers import AddonSerializer, ESAddonSerializer from olympia.addons.utils import generate_addon_guid @@ -39,6 +39,16 @@ class AddonSerializerOutputTestMixin(object): support_url=u'https://support.example.org/support/my-addon/', tags=['some_tag', 'some_other_tag'], ) + AddonUser.objects.create(user=user_factory(username='hidden_author'), + addon=self.addon, listed=False) + second_author = user_factory( + username='second_author', display_name=u'Secönd Author') + first_author = user_factory( + username='first_author', display_name=u'First Authôr') + AddonUser.objects.create( + user=second_author, addon=self.addon, position=2) + AddonUser.objects.create( + user=first_author, addon=self.addon, position=1) second_preview = Preview.objects.create( addon=self.addon, position=2, caption={'en-US': u'My câption', 'fr': u'Mön tîtré'}) @@ -76,6 +86,15 @@ class AddonSerializerOutputTestMixin(object): assert result['current_version']['url'] == absolutify( version.get_url_path()) + assert result['authors'] + assert len(result['authors']) == 2 + assert result['authors'][0] == { + 'name': first_author.name, + 'url': absolutify(first_author.get_url_path())} + assert result['authors'][1] == { + 'name': second_author.name, + 'url': absolutify(second_author.get_url_path())} + assert result['edit_url'] == absolutify(self.addon.get_dev_url()) assert result['default_locale'] == self.addon.default_locale assert result['description'] == {'en-US': self.addon.description} From e5d684860923ac9a046cda0fba6218cbc359bce9 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Wed, 20 Jul 2016 12:50:34 +0200 Subject: [PATCH 2/3] Ignore pyflakes F405 (variable defined by star import) for the time being We have many of these and flake8 2.6.x started complaining about them. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index b2f9c994d8..554ba7e057 100644 --- a/setup.cfg +++ b/setup.cfg @@ -11,7 +11,7 @@ norecursedirs = DJANGO_SETTINGS_MODULE = settings_test [flake8] -ignore = F999 +ignore = F999,F405 exclude = services, src/olympia/wsgi.py, From 94ef73585012824d2de0a343678c1fa828e34c35 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Thu, 21 Jul 2016 11:41:42 +0200 Subject: [PATCH 3/3] Rename 'author' to 'listed_authors' in the indexer, it makes more sense --- src/olympia/addons/indexers.py | 32 +++++++++++++---------- src/olympia/addons/serializers.py | 2 +- src/olympia/addons/tests/test_indexers.py | 10 +++---- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index b6e6ec4fd2..67ff04c589 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -36,18 +36,11 @@ class AddonIndexer(BaseSearchIndexer): 'app': {'type': 'byte'}, 'appversion': {'properties': {app.id: appver for app in amo.APP_USAGE}}, - 'author': { - 'type': 'object', - 'properties': { - 'id': {'type': 'long', 'index': 'no'}, - 'name': {'type': 'string', }, - 'username': {'type': 'string', 'index': 'no', }, - }, - }, # FIXME: See issue #3120, the 'authors' property is for # backwards-compatibility and all code should be switched - # to use 'author.name' instead. We needed a reindex first - # though, which is why the 2 are present at the moment. + # to use 'listed_authors.name' instead. We needed a reindex + # first though, which is why the 2 are present at the + # moment. 'authors': {'type': 'string'}, 'average_daily_users': {'type': 'long'}, 'bayesian_rating': {'type': 'double'}, @@ -86,6 +79,14 @@ class AddonIndexer(BaseSearchIndexer): 'is_disabled': {'type': 'boolean'}, 'is_listed': {'type': 'boolean'}, 'last_updated': {'type': 'date'}, + 'listed_authors': { + 'type': 'object', + 'properties': { + 'id': {'type': 'long', 'index': 'no'}, + 'name': {'type': 'string'}, + 'username': {'type': 'string', 'index': 'no'}, + }, + }, 'modified': {'type': 'date', 'index': 'no'}, # Adding word-delimiter to split on camelcase and # punctuation. @@ -188,12 +189,11 @@ class AddonIndexer(BaseSearchIndexer): 'min': min_, 'min_human': min_human, 'max': max_, 'max_human': max_human, } - data['author'] = [{'name': a.name, 'id': a.id, 'username': a.username} - for a in obj.listed_authors] # FIXME: See issue #3120, the 'authors' property is for # backwards-compatibility and all code should be switched - # to use 'author.name' instead. We needed a reindex first - # though, which is why the 2 are present at the moment. + # to use 'listed_authors.name' instead. We needed a reindex + # first though, which is why the 2 are present at the + # moment. data['authors'] = [a.name for a in obj.listed_authors] # Quadruple the boost if the add-on is public. if obj.status == amo.STATUS_PUBLIC and 'boost' in data: @@ -221,6 +221,10 @@ class AddonIndexer(BaseSearchIndexer): obj.current_version.supported_platforms] else: data['has_version'] = None + data['listed_authors'] = [ + {'name': a.name, 'id': a.id, 'username': a.username} + for a in obj.listed_authors + ] # We can use all_previews because the indexing code goes through the # transformer that sets it. diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 8c8d4533a6..872f88eb5f 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -263,7 +263,7 @@ class ESAddonSerializer(BaseESSerializer, AddonSerializer): obj._current_version.compatible_apps = compatible_apps - data_authors = data.get('author', []) + data_authors = data.get('listed_authors', []) obj.listed_authors = [ UserProfile( id=data_author['id'], display_name=data_author['name'], diff --git a/src/olympia/addons/tests/test_indexers.py b/src/olympia/addons/tests/test_indexers.py index df3db03157..33c4502af3 100644 --- a/src/olympia/addons/tests/test_indexers.py +++ b/src/olympia/addons/tests/test_indexers.py @@ -44,10 +44,10 @@ class TestAddonIndexer(TestCase): # exist on the model, or it has a different name, or the value we need # to store in ES differs from the one in the db. complex_fields = [ - 'app', 'appversion', 'author', 'authors', 'boost', 'category', + 'app', 'appversion', 'authors', 'boost', 'category', 'current_version', 'description', 'has_theme_rereview', - 'has_version', 'name', 'name_sort', 'platforms', 'previews', - 'public_stats', 'summary', 'tags', + 'has_version', 'listed_authors', 'name', 'name_sort', 'platforms', + 'previews', 'public_stats', 'summary', 'tags', ] # Fields that need to be present in the mapping, but might be skipped @@ -142,12 +142,12 @@ class TestAddonIndexer(TestCase): 'min_human': '2.0', } } - assert extracted['author'] == [{'name': u'55021 التطب', 'id': 55021, - 'username': '55021'}] assert extracted['authors'] == [u'55021 التطب'] assert extracted['boost'] == self.addon.average_daily_users ** .2 * 4 assert extracted['category'] == [22, 23, 24] # From fixture. assert extracted['has_theme_rereview'] is None + assert extracted['listed_authors'] == [ + {'name': u'55021 التطب', 'id': 55021, 'username': '55021'}] assert extracted['platforms'] == [PLATFORM_ALL.id] assert extracted['tags'] == []