set tags property via addon api (#18428)

* set tags property via addon api * Add tests for LazyChoiceField
2021-12-02 14:11:10 +00:00 · 2021-12-02 14:11:10 +00:00 · 115c002892
--- a/docs/topics/api/addons.rst
+++ b/docs/topics/api/addons.rst
@ -319,6 +319,7 @@ This endpoint allows a submission of an upload to create a new add-on and settin
    :<json string slug: The add-on slug.  Valid slugs must only contain letters, numbers (`categories L and N <http://www.unicode.org/reports/tr44/tr44-4.html#GC_Values_Table>`_), ``-``, ``_``, ``~``, and can't be all numeric.
    :<json object|null summary: The add-on summary (See :ref:`translated fields <api-overview-translations>`).
    :<json object|null support_email: The add-on support email (See :ref:`translated fields <api-overview-translations>`).
+    :<json array tags: List containing the text of the tags set on the add-on.
    :<json object version: Object containing the :ref:`version <version-create-request>` to create this addon with.

 ----
@ -353,6 +354,7 @@ This endpoint allows an add-on's AMO metadata to be edited.
    :<json string slug: The add-on slug.  Valid slugs must only contain letters, numbers (`categories L and N <http://www.unicode.org/reports/tr44/tr44-4.html#GC_Values_Table>`_), ``-``, ``_``, ``~``, and can't be all numeric.
    :<json object|null summary: The add-on summary (See :ref:`translated fields <api-overview-translations>`).
    :<json object|null support_email: The add-on support email (See :ref:`translated fields <api-overview-translations>`).
+    :<json array tags: List containing the text of the tags set on the add-on.


 -------------
--- a/docs/topics/api/overview.rst
+++ b/docs/topics/api/overview.rst
@ -428,7 +428,8 @@ These are `v5` specific changes - `v4` changes apply also.
 * 2021-09-23: flattened ``files`` in version detail from an array to a single ``file``. https://github.com/mozilla/addons-server/issues/17839
 * 2021-09-30: removed ``is_webextension`` from file objects (all addons have been webextensions for a while now) in all endpoints. https://github.com/mozilla/addons-server/issues/17658
 * 2021-11-18: added docs for the under-development addon submission & edit apis.
-* 2021-11-25: added `custom_license` to version create/edit endpoints to allow non-predefined licenses to be created and updated. https://github.com/mozilla/addons-server/issues/18034
+* 2021-11-25: added ``custom_license`` to version create/edit endpoints to allow non-predefined licenses to be created and updated. https://github.com/mozilla/addons-server/issues/18034
+* 2021-12-09: enabled setting ``tags`` via addon submission and edit apis. https://github.com/mozilla/addons-server/issues/18268

 .. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/
 .. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/
--- a/src/olympia/addons/models.py
+++ b/src/olympia/addons/models.py
@ -1793,6 +1793,23 @@ class Addon(OnChangeMixin, ModelBase):

        return GitExtractionEntry.objects.filter(addon=self, in_progress=True).exists()

+    @cached_property
+    def tag_list(self):
+        attach_tags([self])
+        return self.tag_list
+
+    def set_tag_list(self, new_tag_list):
+        tag_list_to_add = set(new_tag_list) - set(self.tag_list)
+        tag_list_to_drop = set(self.tag_list) - set(new_tag_list)
+        tags = Tag.objects.filter(tag_text__in=(*tag_list_to_add, *tag_list_to_drop))
+
+        for tag in tags:
+            if tag.tag_text in tag_list_to_add:
+                tag.add_tag(self)
+            elif tag.tag_text in tag_list_to_drop:
+                tag.remove_tag(self)
+        self.tag_list = new_tag_list
+

 dbsignals.pre_save.connect(save_signal, sender=Addon, dispatch_uid='addon_translations')

--- a/src/olympia/addons/serializers.py
+++ b/src/olympia/addons/serializers.py
@ -18,6 +18,7 @@ from olympia.api.fields import (
    EmailTranslationField,
    ESTranslationSerializerField,
    GetTextTranslationSerializerField,
+    LazyChoiceField,
    OutgoingURLField,
    OutgoingURLTranslationField,
    ReverseChoiceField,
@ -38,6 +39,7 @@ from olympia.files.utils import parse_addon
 from olympia.promoted.models import PromotedAddon
 from olympia.search.filters import AddonAppVersionQueryParam
 from olympia.ratings.utils import get_grouped_ratings
+from olympia.tags.models import Tag
 from olympia.users.models import UserProfile
 from olympia.versions.models import (
    ApplicationsVersions,
@ -46,7 +48,7 @@ from olympia.versions.models import (
    VersionPreview,
 )

-from .models import Addon, DeniedSlug, Preview, ReplacementAddon, attach_tags
+from .models import Addon, DeniedSlug, Preview, ReplacementAddon


 class FileSerializer(serializers.ModelSerializer):
@ -797,7 +799,12 @@ class AddonSerializer(serializers.ModelSerializer):
    summary = TranslationSerializerField(required=False, max_length=250)
    support_email = EmailTranslationField(required=False)
    support_url = OutgoingURLTranslationField(required=False)
-    tags = serializers.SerializerMethodField()
+    tags = serializers.ListField(
+        child=LazyChoiceField(choices=Tag.objects.values_list('tag_text', flat=True)),
+        max_length=amo.MAX_TAGS,
+        source='tag_list',
+        required=False,
+    )
    type = ReverseChoiceField(
        choices=list(amo.ADDON_TYPE_CHOICES_API.items()), read_only=True
    )
@ -862,6 +869,7 @@ class AddonSerializer(serializers.ModelSerializer):
            'summary',
            'support_email',
            'support_url',
+            'tags',
            'version',
        )
        read_only_fields = tuple(set(fields) - set(writeable_fields))
@ -894,13 +902,6 @@ class AddonSerializer(serializers.ModelSerializer):
    def get_has_privacy_policy(self, obj):
        return bool(getattr(obj, 'has_privacy_policy', obj.privacy_policy))

-    def get_tags(self, obj):
-        if not hasattr(obj, 'tag_list'):
-            attach_tags([obj])
-        # attach_tags() might not have attached anything to the addon, if it
-        # had no tags.
-        return getattr(obj, 'tag_list', [])
-
    def get_url(self, obj):
        # Use absolutify(get_detail_url()), get_absolute_url() calls
        # get_url_path() which does an extra check on current_version that is
@ -1004,6 +1005,7 @@ class AddonSerializer(serializers.ModelSerializer):
        )
        # Add categories
        addon.set_categories(validated_data.get('all_categories', []))
+        addon.set_tag_list(validated_data.get('tag_list', []))

        self.fields['version'].create(
            {**validated_data.get('version', {}), 'addon': addon}
@ -1028,6 +1030,9 @@ class AddonSerializer(serializers.ModelSerializer):
        if 'all_categories' in validated_data:
            del instance.all_categories  # super.update will have set it.
            instance.set_categories(validated_data['all_categories'])
+        if 'tag_list' in validated_data:
+            del instance.tag_list  # super.update will have set it.
+            instance.set_tag_list(validated_data['tag_list'])
        return instance


@ -1055,6 +1060,7 @@ class ESAddonSerializer(BaseESSerializer, AddonSerializer):
    authors = BaseUserSerializer(many=True, source='listed_authors')
    current_version = ESCurrentVersionSerializer()
    previews = ESPreviewSerializer(many=True, source='current_previews')
+    tags = serializers.ListField(source='tag_list')
    _score = serializers.SerializerMethodField()

    datetime_fields = ('created', 'last_updated', 'modified')
--- a/src/olympia/addons/tests/test_views.py
+++ b/src/olympia/addons/tests/test_views.py
@ -44,6 +44,7 @@ from olympia.constants.promoted import (
 )
 from olympia.files.utils import parse_addon
 from olympia.files.tests.test_models import UploadMixin
+from olympia.tags.models import Tag
 from olympia.users.models import UserProfile
 from olympia.versions.models import ApplicationsVersions, AppVersion, License

@ -1099,6 +1100,40 @@ class TestAddonViewSetCreate(UploadMixin, TestCase):
            'support_url': ['Enter a valid URL.'],
        }

+    def test_set_tags(self):
+        response = self.client.post(
+            self.url,
+            data={**self.minimal_data, 'tags': ['foo', 'bar']},
+        )
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'tags': {
+                0: ['"foo" is not a valid choice.'],
+                1: ['"bar" is not a valid choice.'],
+            }
+        }
+
+        response = self.client.post(
+            self.url,
+            data={
+                **self.minimal_data,
+                'tags': list(Tag.objects.values_list('tag_text', flat=True)),
+            },
+        )
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'tags': ['Ensure this field has no more than 10 elements.'],
+        }
+
+        response = self.client.post(
+            self.url,
+            data={**self.minimal_data, 'tags': ['zoom', 'music']},
+        )
+        assert response.status_code == 201, response.content
+        assert response.data['tags'] == ['zoom', 'music']
+        addon = Addon.objects.get()
+        assert [tag.tag_text for tag in addon.tags.all()] == ['music', 'zoom']
+

 class TestAddonViewSetCreateJWTAuth(TestAddonViewSetCreate):
    client_class = JWTAPITestClient
@ -1375,6 +1410,41 @@ class TestAddonViewSetUpdate(TestCase):
            'support_url': ['Enter a valid URL.'],
        }

+    def test_set_tags(self):
+        response = self.client.patch(
+            self.url,
+            data={'tags': ['foo', 'bar']},
+        )
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'tags': {
+                0: ['"foo" is not a valid choice.'],
+                1: ['"bar" is not a valid choice.'],
+            }
+        }
+
+        response = self.client.patch(
+            self.url,
+            data={'tags': list(Tag.objects.values_list('tag_text', flat=True))},
+        )
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'tags': ['Ensure this field has no more than 10 elements.'],
+        }
+
+        # we're going to keep "zoom", but drop "security"
+        Tag.objects.get(tag_text='zoom').add_tag(self.addon)
+        Tag.objects.get(tag_text='security').add_tag(self.addon)
+
+        response = self.client.patch(
+            self.url,
+            data={'tags': ['zoom', 'music']},
+        )
+        assert response.status_code == 200, response.content
+        assert response.data['tags'] == ['zoom', 'music']
+        self.addon.reload()
+        assert [tag.tag_text for tag in self.addon.tags.all()] == ['music', 'zoom']
+

 class TestAddonViewSetUpdateJWTAuth(TestAddonViewSetUpdate):
    client_class = JWTAPITestClient
--- a/src/olympia/api/fields.py
+++ b/src/olympia/api/fields.py
@ -548,3 +548,27 @@ class FallbackField(fields.Field):
            if rep:
                return rep
        return rep
+
+
+class LazyChoiceField(fields.ChoiceField):
+    """Like ChoiceField but doesn't end up evaluating `choices` in __init__ so suitable
+    for using with queryset."""
+
+    def _get_choices(self):
+        return fields.flatten_choices_dict(self.grouped_choices)
+
+    def _set_choices(self, choices):
+        self._choices = choices
+
+    @property
+    def grouped_choices(self):
+        return fields.to_choices_dict(self._choices)
+
+    @property
+    def choice_strings_to_values(self):
+        # Map the string representation of choices to the underlying value.
+        # Allows us to deal with eg. integer choices while supporting either
+        # integer or string input, but still get the correct datatype out.
+        return {str(key): key for key in self.choices}
+
+    choices = property(_get_choices, _set_choices)
--- a/src/olympia/api/tests/test_fields.py
+++ b/src/olympia/api/tests/test_fields.py
@ -1,3 +1,5 @@
+from collections import OrderedDict
+
 from django.test.utils import override_settings

 import pytest
@ -13,6 +15,7 @@ from olympia.api.fields import (
    ESTranslationSerializerField,
    GetTextTranslationSerializerField,
    GetTextTranslationSerializerFieldFlat,
+    LazyChoiceField,
    FallbackField,
    ReverseChoiceField,
    SlugOrPrimaryKeyRelatedField,
@ -718,3 +721,37 @@ class TestFallbackField(TestCase):
        assert addon.name == 'ho!'  # updated
        assert addon.description is None
        assert addon.summary is None
+
+
+class TestLazyChoiceField(TestCase):
+    def setUp(self) -> None:
+        super().setUp()
+        self.aa = addon_factory(slug='aa')
+        self.bb = addon_factory(slug='bb')
+
+    def test_init_doesnt_evaluate_choices(self):
+        with self.assertNumQueries(0):
+            # No queries for __init__
+            field = LazyChoiceField(choices=Addon.objects.values_list('id', flat=True))
+        # the queryset will be evaluated for this
+        assert field.choices
+        # queryset caching should prevent a further database call though
+        with self.assertNumQueries(0):
+            assert field.choices
+
+    def test_super_functionality(self):
+        """Check the normal functionality of ChoiceField works."""
+        field = LazyChoiceField(choices=Addon.objects.values_list('id', 'slug'))
+
+        assert field.to_representation(self.aa.id) == self.aa.id
+        assert field.to_representation(str(self.aa.id)) == self.aa.id
+        assert field.to_representation(12345) == 12345
+        assert field.to_internal_value(self.bb.id) == self.bb.id
+        assert field.to_internal_value(str(self.bb.id)) == self.bb.id
+        with self.assertRaises(exceptions.ValidationError):
+            # not a valid choice
+            field.to_internal_value(12345)
+        assert field.choice_strings_to_values == OrderedDict(
+            ((str(self.aa.id), self.aa.id), (str(self.bb.id), self.bb.id))
+        )
+        assert field.choices == {self.aa.id: 'aa', self.bb.id: 'bb'}