Refactor eula_policy action write path to be cleaner & safer (#22586)

* Refactor eula_policy action write path to be cleaner & safer

https://www.django-rest-framework.org/api-guide/viewsets/#routing-additional-http-methods-for-extra-actions

---------

Co-authored-by: Andrew Williamson <awilliamson@mozilla.com>

Author: 2 people

src/olympia/addons/tests/test_views.py

@@ -3015,7 +3015,7 @@ def test_developer_version_serializer_used_for_authors(self):
         # not logged in
         assert 'source' not in self.client.get(self.url).data
 
-        user = UserProfile.objects.create(username='user')
+        user = user_factory()
         self.client.login_api(user)
 
         # logged in but not an author
@@ -5022,7 +5022,7 @@ def test_developer_version_serializer_used_for_authors(self):
         assert 'source' not in self.client.get(self.url).data['results'][0]
         assert 'source' not in self.client.get(self.url).data['results'][1]
 
-        user = UserProfile.objects.create(username='user')
+        user = user_factory()
         self.client.login_api(user)
 
         # logged in but not an author
@@ -5045,6 +5045,7 @@ def setUp(self):
             guid=generate_addon_guid(), name='My Addôn', slug='my-addon'
         )
         self.url = reverse_ns('addon-eula-policy', kwargs={'pk': self.addon.pk})
+        self.user = user_factory(read_dev_agreement=self.days_ago(1))
 
     def test_url(self):
         self.detail_url = reverse_ns('addon-detail', kwargs={'pk': self.addon.pk})
@@ -5076,8 +5077,7 @@ def test_eula_and_policy(self):
         assert data['privacy_policy'] == {'en-US': 'My Prïvacy, My Policy'}
 
     def test_update_non_author(self):
-        user = UserProfile.objects.create(username='user')
-        self.client.login_api(user)
+        self.client.login_api(self.user)
         response = self.client.patch(
             self.url,
             {
@@ -5136,9 +5136,8 @@ def test_update_anonymous(self):
         assert response.status_code == 401
 
     def test_update(self):
-        user = UserProfile.objects.create(username='user')
-        AddonUser.objects.create(user=user, addon=self.addon)
-        self.client.login_api(user)
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.client.login_api(self.user)
         assert not ActivityLog.objects.filter(
             action=amo.LOG.EDIT_PROPERTIES.id
         ).exists()
@@ -5178,9 +5177,8 @@ def test_update(self):
         ].details == ['eula', 'privacy_policy']
 
     def test_update_put(self):
-        user = UserProfile.objects.create(username='user')
-        AddonUser.objects.create(user=user, addon=self.addon)
-        self.client.login_api(user)
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.client.login_api(self.user)
         response = self.client.put(
             self.url,
             {
@@ -5195,12 +5193,73 @@ def test_update_put(self):
         )
         assert response.status_code == 405
 
+    def test_update_author_not_read_dev_agreement(self):
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.user.update(read_dev_agreement=None)
+        self.client.login_api(self.user)
+        response = self.client.patch(
+            self.url,
+            {
+                'eula': {
+                    'en-US': 'My Updated Add-on EULA in English',
+                    'fr': 'Mes Conditions générales d’utilisation',
+                },
+                'privacy_policy': {
+                    'en-US': 'My privacy policy',
+                },
+            },
+        )
+        assert response.status_code == 403
+        self.addon.reload()
+        assert not self.addon.eula
+        assert not self.addon.privacy_policy
+
+    def test_update_reviewer_not_author(self):
+        self.grant_permission(self.user, 'Addons:Review')
+        self.client.login_api(self.user)
+        response = self.client.patch(
+            self.url,
+            {
+                'eula': {
+                    'en-US': 'My Updated Add-on EULA in English',
+                    'fr': 'Mes Conditions générales d’utilisation',
+                },
+                'privacy_policy': {
+                    'en-US': 'My privacy policy',
+                },
+            },
+        )
+        assert response.status_code == 403
+        self.addon.reload()
+        assert not self.addon.eula
+        assert not self.addon.privacy_policy
+
+    def test_update_disabled(self):
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.client.login_api(self.user)
+        self.addon.update(status=amo.STATUS_DISABLED)
+        response = self.client.patch(
+            self.url,
+            {
+                'eula': {
+                    'en-US': 'My Updated Add-on EULA in English',
+                    'fr': 'Mes Conditions générales d’utilisation',
+                },
+                'privacy_policy': {
+                    'en-US': 'My privacy policy',
+                },
+            },
+        )
+        assert response.status_code == 403
+        self.addon.reload()
+        assert not self.addon.eula
+        assert not self.addon.privacy_policy
+
     def test_update_something_else(self):
         assert self.addon.summary
         original_summary = self.addon.summary
-        user = UserProfile.objects.create(username='user')
-        AddonUser.objects.create(user=user, addon=self.addon)
-        self.client.login_api(user)
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.client.login_api(self.user)
         response = self.client.patch(
             self.url,
             {'summary': 'attempting to change the summary via wrong endpoint'},
@@ -5212,10 +5271,9 @@ def test_update_something_else(self):
         assert self.addon.summary == original_summary
 
     def test_update_on_theme(self):
-        user = UserProfile.objects.create(username='user')
         self.addon.update(type=amo.ADDON_STATICTHEME)
-        AddonUser.objects.create(user=user, addon=self.addon)
-        self.client.login_api(user)
+        AddonUser.objects.create(user=self.user, addon=self.addon)
+        self.client.login_api(self.user)
         response = self.client.patch(
             self.url,
             {

src/olympia/addons/views.py

@@ -19,7 +19,7 @@
     RetrieveModelMixin,
     UpdateModelMixin,
 )
-from rest_framework.permissions import SAFE_METHODS, IsAuthenticated
+from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 from rest_framework.settings import api_settings
 from rest_framework.views import APIView
@@ -346,20 +346,18 @@ def get_georestrictions(self):
 
     @action(
         detail=True,
-        methods=['get', 'patch'],
         serializer_class=AddonEulaPolicySerializer,
         # For this action, developers use the same serializer - it only
         # contains eula/privacy policy.
         serializer_class_for_developers=AddonEulaPolicySerializer,
     )
     def eula_policy(self, request, pk=None):
-        kwargs = {}
-        if request.method in SAFE_METHODS:
-            method = self.retrieve
-        else:
-            kwargs['partial'] = True
-            method = self.update
-        return method(request, **kwargs)
+        return self.retrieve(request)
+
+    @eula_policy.mapping.patch
+    def update_eula_policy(self, request, pk=None):
+        self.permission_classes = self.write_permission_classes
+        return self.update(request, partial=True)
 
     @action(detail=True)
     def delete_confirm(self, request, *args, **kwargs):