diff --git a/BackEnd/profiles/tasks.py b/BackEnd/profiles/tasks.py index 26ae6f508..78fe533e8 100644 --- a/BackEnd/profiles/tasks.py +++ b/BackEnd/profiles/tasks.py @@ -1,5 +1,7 @@ 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 @@ -7,19 +9,25 @@ @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() diff --git a/BackEnd/profiles/tests/test_approve_moderation_request.py b/BackEnd/profiles/tests/test_approve_moderation_request.py index e4feda0b7..1ef7226d6 100644 --- a/BackEnd/profiles/tests/test_approve_moderation_request.py +++ b/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() diff --git a/BackEnd/profiles/tests/test_celery_autoapprove.py b/BackEnd/profiles/tests/test_celery_autoapprove.py new file mode 100644 index 000000000..83924e0ac --- /dev/null +++ b/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, + ) diff --git a/BackEnd/profiles/tests/test_reject_moderation_request.py b/BackEnd/profiles/tests/test_reject_moderation_request.py index 5b4ea0839..bd820fbbd 100644 --- a/BackEnd/profiles/tests/test_reject_moderation_request.py +++ b/BackEnd/profiles/tests/test_reject_moderation_request.py @@ -1,3 +1,5 @@ +from unittest.mock import patch, call + from rest_framework import status from rest_framework.test import APITestCase, APIClient @@ -11,6 +13,8 @@ from utils.dump_response import dump # noqa +@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") @@ -27,7 +31,7 @@ def setUp(self) -> None: self.unregistered_user_client = APIClient() - def test_reject_banner_and_logo(self): + def test_reject_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,8 +71,10 @@ def test_reject_banner_and_logo(self): self.assertEqual(self.profile.BLOCKED, self.profile.status) self.assertTrue(self.profile.is_deleted) self.assertFalse(self.user.is_active) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() - def test_reject_banner(self): + def test_reject_banner(self, mock_revoke, mock_schedule): # user updates only banner self.user_client.patch( path="/api/profiles/{profile_id}".format( @@ -104,8 +110,10 @@ def test_reject_banner(self): self.assertEqual(self.profile.BLOCKED, self.profile.status) self.assertTrue(self.profile.is_deleted) self.assertFalse(self.user.is_active) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() - def test_reject_logo(self): + def test_reject_logo(self, mock_revoke, mock_schedule): # user updates logo self.user_client.patch( path="/api/profiles/{profile_id}".format( @@ -141,8 +149,12 @@ def test_reject_logo(self): self.assertEqual(self.profile.BLOCKED, self.profile.status) self.assertTrue(self.profile.is_deleted) self.assertFalse(self.user.is_active) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() - def test_reject_banner_and_logo_processed_request(self): + def test_reject_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( @@ -188,8 +200,12 @@ def test_reject_banner_and_logo_processed_request(self): }, response.json(), ) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() - def test_reject_banner_and_logo_outdated_request(self): + def test_reject_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( @@ -240,8 +256,12 @@ def test_reject_banner_and_logo_outdated_request(self): self.assertNotEqual(self.profile.banner, first_banner) self.assertNotEqual(self.profile.logo, first_logo) self.assertEqual(self.profile.PENDING, self.profile.status) + mock_schedule.assert_has_calls([call(), call()]) + mock_revoke.assert_not_called() - def test_reject_banner_and_logo_wrong_action(self): + def test_reject_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( @@ -270,8 +290,12 @@ def test_reject_banner_and_logo_wrong_action(self): self.assertEqual( {"action": ["Action is not allowed"]}, response.json() ) + mock_schedule.assert_called_once() + mock_revoke.assert_not_called() - def test_reject_banner_and_logo_error_in_signed_id(self): + def test_reject_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( @@ -298,8 +322,12 @@ def test_reject_banner_and_logo_error_in_signed_id(self): self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code) self.assertEqual({"detail": "Not found."}, response.json()) + mock_schedule.assert_called_once() + mock_revoke.assert_not_called() - def test_reject_banner_and_logo_non_existing_profile(self): + def test_reject_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( @@ -326,8 +354,12 @@ def test_reject_banner_and_logo_non_existing_profile(self): self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code) self.assertEqual({"detail": "Not found."}, response.json()) + mock_schedule.assert_called_once() + mock_revoke.assert_not_called() - def test_reject_banner_and_logo_empty_image_fields(self): + def test_reject_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( @@ -358,8 +390,12 @@ def test_reject_banner_and_logo_empty_image_fields(self): }, response.json(), ) + mock_schedule.assert_called_once() + mock_revoke.assert_not_called() - def test_login_blocked_user_due_to_rejected_request(self): + def test_login_blocked_user_due_to_rejected_request( + self, mock_revoke, mock_schedule + ): # user updates both banner and logo self.user_client.patch( path="/api/profiles/{profile_id}".format( @@ -400,8 +436,12 @@ def test_login_blocked_user_due_to_rejected_request(self): {"non_field_errors": ["Profile has been blocked."]}, response.json(), ) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() - def test_register_blocked_user_due_to_rejected_request(self): + def test_register_blocked_user_due_to_rejected_request( + self, mock_revoke, mock_schedule + ): # user updates both banner and logo self.user_client.patch( path="/api/profiles/{profile_id}".format( @@ -449,3 +489,5 @@ def test_register_blocked_user_due_to_rejected_request(self): {"email": ["Email is already registered"]}, response.json(), ) + mock_schedule.assert_called_once() + mock_revoke.assert_called_once() diff --git a/BackEnd/profiles/views.py b/BackEnd/profiles/views.py index 4236a5f77..2c1830e38 100644 --- a/BackEnd/profiles/views.py +++ b/BackEnd/profiles/views.py @@ -25,6 +25,7 @@ from utils.moderation.handle_approved_images import ApprovedImagesDeleter from forum.pagination import ForumPagination +from images.models import ProfileImage from .models import SavedCompany, Profile, Category, Activity, Region from .permissions import ( UserIsProfileOwnerOrReadOnly, @@ -219,18 +220,22 @@ def perform_update(self, serializer): profile = serializer.save() SavedCompany.objects.filter(company=profile).update(is_updated=True) completeness_count(profile) - deletion_checker = ApprovedImagesDeleter(profile) - deletion_checker.handle_potential_deletion() - moderation_manager = ModerationManager(profile) if ( - moderation_manager.check_for_moderation() - or moderation_manager.content_deleted + ProfileImage.BANNER in serializer.validated_data.keys() + or ProfileImage.LOGO in serializer.validated_data.keys() ): - banner = moderation_manager.images["banner"] - logo = moderation_manager.images["logo"] - is_deleted = moderation_manager.content_deleted - send_moderation_email(profile, banner, logo, is_deleted) - moderation_manager.schedule_autoapprove() + deletion_checker = ApprovedImagesDeleter(profile) + deletion_checker.handle_potential_deletion() + moderation_manager = ModerationManager(profile) + if ( + moderation_manager.check_for_moderation() + or moderation_manager.content_deleted + ): + banner = moderation_manager.images["banner"] + logo = moderation_manager.images["logo"] + is_deleted = moderation_manager.content_deleted + send_moderation_email(profile, banner, logo, is_deleted) + moderation_manager.schedule_autoapprove() class ProfileViewCreate(CreateAPIView): diff --git a/BackEnd/utils/moderation/image_moderation.py b/BackEnd/utils/moderation/image_moderation.py index ad7abf531..ab92261f8 100644 --- a/BackEnd/utils/moderation/image_moderation.py +++ b/BackEnd/utils/moderation/image_moderation.py @@ -66,9 +66,10 @@ def check_for_moderation(self): return self.moderation_is_needed def schedule_autoapprove(self): + """Make shure that you called check_for_moderation on your ModerationManager instance, before scheduling autoapprove.""" try: self.revoke_deprecated_autoapprove() - if self.needs_moderation and not self.content_deleted: + if self.moderation_is_needed and not self.content_deleted: banner = self.images.get("banner") logo = self.images.get("logo") banner_uuid = str(banner.uuid) if banner else None diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 61ce28e05..0541c6f43 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -80,7 +80,7 @@ services: networks: - forum_network redis: - image: redis:latest + image: redis:7 container_name: redis ports: - 6379:6379