#755 improve automoderation and tests

BackEnd/profiles/tasks.py

@@ -1,25 +1,33 @@
 from celery import shared_task
+from django.db import transaction
 
+from administration.models import AutoapproveTask
 from .models import Profile
 from images.models import ProfileImage
 from utils.completeness_counter import completeness_count
 
 
 @shared_task
 def celery_autoapprove(profile_id, banner_uuid, logo_uuid):
-    profile = Profile.objects.get(pk=profile_id)
-    if banner_uuid:
-        banner = ProfileImage.objects.get(pk=banner_uuid)
-        banner.is_approved = True
-        profile.banner_approved = banner
-        banner.save()
+    with transaction.atomic():
+        profile = Profile.objects.get(pk=profile_id)
+        if banner_uuid:
+            banner = ProfileImage.objects.get(pk=banner_uuid)
+            banner.is_approved = True
+            profile.banner_approved = banner
+            banner.save()
 
-    if logo_uuid:
-        logo = ProfileImage.objects.get(pk=logo_uuid)
-        logo.is_approved = True
-        profile.logo_approved = logo
-        logo.save()
+        if logo_uuid:
+            logo = ProfileImage.objects.get(pk=logo_uuid)
+            logo.is_approved = True
+            profile.logo_approved = logo
+            logo.save()
 
-    profile.status = profile.AUTOAPPROVED
-    profile.save()
-    completeness_count(profile)
+        profile.status = profile.AUTOAPPROVED
+        profile.save()
+        completeness_count(profile)
+        deprecated_record = AutoapproveTask.objects.filter(
+            profile=profile
+        ).first()
+        if deprecated_record:
+            deprecated_record.delete()

BackEnd/profiles/tests/test_approve_moderation_request.py

@@ -1,4 +1,4 @@
-from unittest.mock import patch
+from unittest.mock import patch, call
 
 from rest_framework import status
 from rest_framework.test import APITestCase, APIClient
@@ -14,6 +14,7 @@
 
 
 @patch("profiles.views.ModerationManager.schedule_autoapprove")
+@patch("profiles.views.ModerationManager.revoke_deprecated_autoapprove")
 class TestProfileModeration(APITestCase):
     def setUp(self) -> None:
         self.banner = ProfileimageFactory(image_type="banner")
@@ -28,7 +29,7 @@ def setUp(self) -> None:
 
         self.moderator_client = APIClient()
 
-    def test_approve_banner_and_logo(self, mock_manager):
+    def test_approve_banner_and_logo(self, mock_revoke, mock_schedule):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -67,9 +68,10 @@ def test_approve_banner_and_logo(self, mock_manager):
         self.assertEqual(self.profile.banner_approved, self.profile.banner)
         self.assertEqual(self.profile.logo_approved, self.profile.logo)
         self.assertEqual(self.profile.APPROVED, self.profile.status)
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_called_once()
 
-    def test_approve_banner(self, mock_manager):
+    def test_approve_banner(self, mock_revoke, mock_schedule):
         # user updates only banner
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -103,9 +105,10 @@ def test_approve_banner(self, mock_manager):
         self.assertTrue(self.banner.is_approved)
         self.assertEqual(self.profile.banner_approved, self.profile.banner)
         self.assertEqual(self.profile.APPROVED, self.profile.status)
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_called_once()
 
-    def test_approve_logo(self, mock_manager):
+    def test_approve_logo(self, mock_revoke, mock_schedule):
         # user updates logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -139,9 +142,12 @@ def test_approve_logo(self, mock_manager):
         self.assertTrue(self.logo.is_approved)
         self.assertEqual(self.profile.logo_approved, self.profile.logo)
         self.assertEqual(self.profile.APPROVED, self.profile.status)
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_called_once()
 
-    def test_approve_banner_and_logo_processed_request(self, mock_manager):
+    def test_approve_banner_and_logo_processed_request(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -187,9 +193,12 @@ def test_approve_banner_and_logo_processed_request(self, mock_manager):
             },
             response.json(),
         )
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_called_once()
 
-    def test_approve_banner_and_logo_outdated_request(self, mock_manager):
+    def test_approve_banner_and_logo_outdated_request(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -242,9 +251,12 @@ def test_approve_banner_and_logo_outdated_request(self, mock_manager):
         self.assertNotEqual(self.profile.banner, first_banner)
         self.assertNotEqual(self.profile.logo, first_logo)
         self.assertEqual(self.profile.PENDING, self.profile.status)
-        mock_manager.assert_called()
+        mock_schedule.assert_has_calls([call(), call()])
+        mock_revoke.assert_not_called()
 
-    def test_approve_banner_and_logo_wrong_action(self, mock_manager):
+    def test_approve_banner_and_logo_wrong_action(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -273,9 +285,12 @@ def test_approve_banner_and_logo_wrong_action(self, mock_manager):
         self.assertEqual(
             {"action": ["Action is not allowed"]}, response.json()
         )
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_not_called()
 
-    def test_approve_banner_and_logo_error_in_signed_id(self, mock_manager):
+    def test_approve_banner_and_logo_error_in_signed_id(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -302,9 +317,12 @@ def test_approve_banner_and_logo_error_in_signed_id(self, mock_manager):
 
         self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
         self.assertEqual({"detail": "Not found."}, response.json())
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_not_called()
 
-    def test_approve_banner_and_logo_non_existing_profile(self, mock_manager):
+    def test_approve_banner_and_logo_non_existing_profile(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -331,9 +349,12 @@ def test_approve_banner_and_logo_non_existing_profile(self, mock_manager):
 
         self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
         self.assertEqual({"detail": "Not found."}, response.json())
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_not_called()
 
-    def test_approve_banner_and_logo_empty_image_fields(self, mock_manager):
+    def test_approve_banner_and_logo_empty_image_fields(
+        self, mock_revoke, mock_schedule
+    ):
         # user updates both banner and logo
         self.user_client.patch(
             path="/api/profiles/{profile_id}".format(
@@ -365,4 +386,5 @@ def test_approve_banner_and_logo_empty_image_fields(self, mock_manager):
             },
             response.json(),
         )
-        mock_manager.assert_called_once()
+        mock_schedule.assert_called_once()
+        mock_revoke.assert_not_called()

BackEnd/profiles/tests/test_celery_autoapprove.py

@@ -0,0 +1,86 @@
+from unittest.mock import patch
+
+from rest_framework.test import APITestCase
+
+from authentication.factories import UserFactory
+from profiles.factories import (
+    ProfileCompanyFactory,
+)
+from administration.models import AutoModeration
+from images.factories import ProfileimageFactory
+from utils.unittest_helper import AnyInt
+
+
+@patch("utils.moderation.image_moderation.celery_autoapprove.apply_async")
+class TestCelery(APITestCase):
+    def setUp(self) -> None:
+        self.banner = ProfileimageFactory(image_type="banner")
+        self.logo = ProfileimageFactory(image_type="logo")
+        self.user = UserFactory()
+        self.profile = ProfileCompanyFactory.create(person=self.user)
+        self.client.force_authenticate(self.user)
+
+    def test_autoapprove_banner_and_logo(self, mock_task):
+        mock_task.return_value.id = "sometaskid"
+        self.client.patch(
+            path="/api/profiles/{profile_id}".format(
+                profile_id=self.profile.id
+            ),
+            data={
+                "banner": self.banner.uuid,
+                "logo": self.logo.uuid,
+            },
+        )
+        mock_task.assert_called_with(
+            (self.profile.id, self.banner.uuid, self.logo.uuid),
+            countdown=AnyInt(),
+        )
+
+    def test_autoapprove_only_banner(self, mock_task):
+        mock_task.return_value.id = "sometaskid"
+        self.client.patch(
+            path="/api/profiles/{profile_id}".format(
+                profile_id=self.profile.id
+            ),
+            data={
+                "banner": self.banner.uuid,
+            },
+        )
+        mock_task.assert_called_with(
+            (self.profile.id, self.banner.uuid, None), countdown=AnyInt()
+        )
+
+    def test_autoapprove_only_logo(self, mock_task):
+        mock_task.return_value.id = "sometaskid"
+        self.client.patch(
+            path="/api/profiles/{profile_id}".format(
+                profile_id=self.profile.id
+            ),
+            data={
+                "logo": self.logo.uuid,
+            },
+        )
+        mock_task.assert_called_with(
+            (self.profile.id, None, self.logo.uuid), countdown=AnyInt()
+        )
+
+    def test_change_autoapprove_time(self, mock_task):
+        test_value = 5
+        autapprove_delay = AutoModeration.get_auto_moderation_hours()
+        autapprove_delay.auto_moderation_hours = test_value
+        autapprove_delay.save()
+
+        mock_task.return_value.id = "sometaskid"
+        self.client.patch(
+            path="/api/profiles/{profile_id}".format(
+                profile_id=self.profile.id
+            ),
+            data={
+                "banner": self.banner.uuid,
+                "logo": self.logo.uuid,
+            },
+        )
+        mock_task.assert_called_with(
+            (self.profile.id, self.banner.uuid, self.logo.uuid),
+            countdown=test_value * 60 * 60,
+        )