From 53b5a45cc810d751fc3cd4d85399f2cbbf249e6c Mon Sep 17 00:00:00 2001 From: Philippe MILINK Date: Fri, 15 Jul 2022 21:30:10 +0200 Subject: [PATCH 1/2] =?UTF-8?q?N'envoie=20pas=20de=20MP=20lors=20de=20la?= =?UTF-8?q?=20d=C3=A9publication=20de=20contenu=20sans=20auteur=20inscrit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #6353 --- zds/tutorialv2/tests/tests_opinion_views.py | 129 +++++++++++------- .../tests/tests_views/tests_published.py | 30 ++++ zds/tutorialv2/views/validations_contents.py | 53 +++---- zds/tutorialv2/views/validations_opinions.py | 38 +++--- 4 files changed, 155 insertions(+), 95 deletions(-) diff --git a/zds/tutorialv2/tests/tests_opinion_views.py b/zds/tutorialv2/tests/tests_opinion_views.py index 140b654250..5cf03d275f 100644 --- a/zds/tutorialv2/tests/tests_opinion_views.py +++ b/zds/tutorialv2/tests/tests_opinion_views.py @@ -9,7 +9,7 @@ from zds.forum.tests.factories import TagFactory from zds.gallery.tests.factories import UserGalleryFactory -from zds.member.tests.factories import ProfileFactory, StaffProfileFactory +from zds.member.tests.factories import ProfileFactory, StaffProfileFactory, UserFactory from zds.tutorialv2.tests.factories import ( PublishableContentFactory, ExtractFactory, @@ -32,6 +32,8 @@ def setUp(self): self.overridden_zds_app["member"]["bot_account"] = ProfileFactory().user.username self.licence = LicenceFactory() + self.anonymous = UserFactory(username=settings.ZDS_APP["member"]["anonymous_account"], password="anything") + self.external = UserFactory(username=settings.ZDS_APP["member"]["external_account"], password="anything") self.user_author = ProfileFactory().user self.user_staff = StaffProfileFactory().user @@ -360,6 +362,25 @@ def test_opinion_unpublication(self, opinions_management): self.assertEqual(PublishedContent.objects.count(), 1) + # unregister author and unpublish + self.client.force_login(self.user_author) + result = self.client.post(reverse("member-unregister"), follow=False) + self.assertEqual(result.status_code, 302) + + self.client.force_login(self.user_staff) + + result = self.client.post( + reverse("validation:unpublish-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), + {"text": text_unpublication, "source": "", "version": opinion_draft.current_version}, + follow=False, + ) + self.assertEqual(result.status_code, 302) + + self.assertEqual(PublishedContent.objects.count(), 0) + + opinion = PublishableContent.objects.get(pk=opinion.pk) + self.assertIsNone(opinion.public_version) + def test_opinion_validation(self): """ Test the validation of PublishableContent where type is OPINION. @@ -545,56 +566,62 @@ def test_ignore_opinion(self): self.assertNotContains(result, opinion.title) def test_permanently_unpublish_opinion(self): - opinion = PublishableContentFactory(type="OPINION") - - opinion.authors.add(self.user_author) - UserGalleryFactory(gallery=opinion.gallery, user=self.user_author, mode="W") - opinion.licence = self.licence - opinion.save() - - opinion_draft = opinion.load_version() - ExtractFactory(container=opinion_draft, db_object=opinion) - ExtractFactory(container=opinion_draft, db_object=opinion) - - self.client.force_login(self.user_author) - - # publish - result = self.client.post( - reverse("validation:publish-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), - {"source": "", "version": opinion_draft.current_version}, - follow=False, - ) - self.assertEqual(result.status_code, 302) - - # login as staff - self.client.force_login(self.user_staff) - - # unpublish opinion - result = self.client.post( - reverse("validation:ignore-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), - { - "operation": "REMOVE_PUB", - }, - follow=False, - ) - self.assertEqual(result.status_code, 200) - - # refresh - opinion = PublishableContent.objects.get(pk=opinion.pk) - - # check that the opinion is not published - self.assertFalse(opinion.in_public()) - - # check that it's impossible to publish the opinion again - result = self.client.get(opinion.get_absolute_url()) - self.assertContains(result, _("Billet modéré")) # front - - result = self.client.post( - reverse("validation:publish-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), - {"source": "", "version": opinion_draft.current_version}, - follow=False, - ) - self.assertEqual(result.status_code, 403) # back + for unregister_author in [False, True]: + with self.subTest(unregister_author): + opinion = PublishableContentFactory(type="OPINION") + + opinion.authors.add(self.user_author) + UserGalleryFactory(gallery=opinion.gallery, user=self.user_author, mode="W") + opinion.licence = self.licence + opinion.save() + + opinion_draft = opinion.load_version() + ExtractFactory(container=opinion_draft, db_object=opinion) + ExtractFactory(container=opinion_draft, db_object=opinion) + + self.client.force_login(self.user_author) + + # publish + result = self.client.post( + reverse("validation:publish-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), + {"source": "", "version": opinion_draft.current_version}, + follow=False, + ) + self.assertEqual(result.status_code, 302) + + if unregister_author: + result = self.client.post(reverse("member-unregister"), follow=False) + self.assertEqual(result.status_code, 302) + + # login as staff + self.client.force_login(self.user_staff) + + # unpublish opinion + result = self.client.post( + reverse("validation:ignore-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), + { + "operation": "REMOVE_PUB", + }, + follow=False, + ) + self.assertEqual(result.status_code, 200) + + # refresh + opinion = PublishableContent.objects.get(pk=opinion.pk) + + # check that the opinion is not published + self.assertFalse(opinion.in_public()) + + # check that it's impossible to publish the opinion again + result = self.client.get(opinion.get_absolute_url()) + self.assertContains(result, _("Billet modéré")) # front + + result = self.client.post( + reverse("validation:publish-opinion", kwargs={"pk": opinion.pk, "slug": opinion.slug}), + {"source": "", "version": opinion_draft.current_version}, + follow=False, + ) + self.assertEqual(result.status_code, 403) # back def test_defenitely_unpublish_alerted_opinion(self): opinion = PublishableContentFactory(type="OPINION") diff --git a/zds/tutorialv2/tests/tests_views/tests_published.py b/zds/tutorialv2/tests/tests_views/tests_published.py index d640ac08b1..d2884e9a59 100644 --- a/zds/tutorialv2/tests/tests_views/tests_published.py +++ b/zds/tutorialv2/tests/tests_views/tests_published.py @@ -62,6 +62,7 @@ def setUp(self): bot = Group(name=overridden_zds_app["member"]["bot_group"]) bot.save() self.external = UserFactory(username=overridden_zds_app["member"]["external_account"], password="anything") + self.anonymous = UserFactory(username=settings.ZDS_APP["member"]["anonymous_account"], password="anything") self.beta_forum = ForumFactory( pk=overridden_zds_app["forum"]["beta_forum_id"], @@ -1364,6 +1365,35 @@ def test_unpublish_with_title_change(self): self.assertEqual(public_count - 1, PublishedContent.objects.count()) self.assertEqual("PENDING", Validation.objects.get(pk=registered_validation.pk).status) + def test_unpublish_unregistered_author(self): + article = PublishedContentFactory(type="ARTICLE", author_list=[self.user_author], licence=self.licence) + registered_validation = Validation( + content=article, + version=article.sha_draft, + status="ACCEPT", + comment_authors="bla", + comment_validator="bla", + date_reserve=datetime.datetime.now(), + date_proposition=datetime.datetime.now(), + date_validation=datetime.datetime.now(), + ) + registered_validation.save() + + self.client.force_login(self.user_author) + result = self.client.post(reverse("member-unregister"), follow=False) + self.assertEqual(result.status_code, 302) + + self.client.force_login(self.user_staff) + public_count = PublishedContent.objects.count() + result = self.client.post( + reverse("validation:revoke", kwargs={"pk": article.pk, "slug": article.public_version.content_public_slug}), + {"text": "This content was bad", "version": article.public_version.sha_public}, + follow=False, + ) + self.assertEqual(302, result.status_code) + self.assertEqual(public_count - 1, PublishedContent.objects.count()) + self.assertEqual("PENDING", Validation.objects.get(pk=registered_validation.pk).status) + def test_unpublish_with_empty_subscription(self): article = PublishedContentFactory(type="ARTICLE", author_list=[self.user_author], licence=self.licence) registered_validation = Validation( diff --git a/zds/tutorialv2/views/validations_contents.py b/zds/tutorialv2/views/validations_contents.py index e66546d5ca..1d356f5559 100644 --- a/zds/tutorialv2/views/validations_contents.py +++ b/zds/tutorialv2/views/validations_contents.py @@ -572,34 +572,35 @@ def form_valid(self, form): self.object.save() # send PM - msg = render_to_string( - "tutorialv2/messages/validation_revoke.md", - { - "content": versioned, - "url": versioned.get_absolute_url() + "?version=" + validation.version, - "admin": self.request.user, - "message_reject": "\n".join(["> " + a for a in form.cleaned_data["text"].split("\n")]), - }, - ) - - bot = get_object_or_404(User, username=settings.ZDS_APP["member"]["bot_account"]) - if not validation.content.validation_private_message: - validation.content.validation_private_message = send_mp( - bot, - validation.content.authors.all(), - self.object.validation_message_title, - validation.content.title, - msg, - send_by_mail=True, - direct=False, - hat=get_hat_from_settings("validation"), - ) - self.object.save() - else: - send_message_mp( - bot, validation.content.validation_private_message, msg, no_notification_for=[self.request.user] + if validation.content.authors.first().username != settings.ZDS_APP["member"]["external_account"]: + msg = render_to_string( + "tutorialv2/messages/validation_revoke.md", + { + "content": versioned, + "url": versioned.get_absolute_url() + "?version=" + validation.version, + "admin": self.request.user, + "message_reject": "\n".join(["> " + a for a in form.cleaned_data["text"].split("\n")]), + }, ) + bot = get_object_or_404(User, username=settings.ZDS_APP["member"]["bot_account"]) + if not validation.content.validation_private_message: + validation.content.validation_private_message = send_mp( + bot, + validation.content.authors.all(), + self.object.validation_message_title, + validation.content.title, + msg, + send_by_mail=True, + direct=False, + hat=get_hat_from_settings("validation"), + ) + self.object.save() + else: + send_message_mp( + bot, validation.content.validation_private_message, msg, no_notification_for=[self.request.user] + ) + messages.success(self.request, _("Le contenu a bien été dépublié.")) self.success_url = self.versioned_object.get_absolute_url() + "?version=" + validation.version signals.validation_management.send( diff --git a/zds/tutorialv2/views/validations_opinions.py b/zds/tutorialv2/views/validations_opinions.py index fea0470600..c8f7df9a68 100644 --- a/zds/tutorialv2/views/validations_opinions.py +++ b/zds/tutorialv2/views/validations_opinions.py @@ -126,7 +126,16 @@ def form_valid(self, form): ) bot = get_object_or_404(User, username=settings.ZDS_APP["member"]["bot_account"]) - if not self.object.validation_private_message: + if self.object.validation_private_message: + send_message_mp( + bot, + self.object.validation_private_message, + msg, + hat=get_hat_from_settings("moderation"), + no_notification_for=[self.request.user], + ) + elif versioned.authors.first().username != settings.ZDS_APP["member"]["external_account"]: + # Send a message only if the author is still registered: self.object.validation_private_message = send_mp( bot, versioned.authors.all(), @@ -138,14 +147,6 @@ def form_valid(self, form): hat=get_hat_from_settings("moderation"), ) self.object.save() - else: - send_message_mp( - bot, - self.object.validation_private_message, - msg, - hat=get_hat_from_settings("moderation"), - no_notification_for=[self.request.user], - ) signals.opinions_management.send( sender=self.__class__, performer=self.request.user, content=self.object, action="unpublish" @@ -220,7 +221,16 @@ def form_valid(self, form): ) bot = get_object_or_404(User, username=settings.ZDS_APP["member"]["bot_account"]) - if not self.object.validation_private_message: + if self.object.validation_private_message: + send_message_mp( + bot, + self.object.validation_private_message, + msg, + hat=get_hat_from_settings("moderation"), + no_notification_for=[self.request.user], + ) + elif versioned.authors.first().username != settings.ZDS_APP["member"]["external_account"]: + # Send a message only if the author is still registered: self.object.validation_private_message = send_mp( bot, versioned.authors.all(), @@ -232,14 +242,6 @@ def form_valid(self, form): hat=get_hat_from_settings("moderation"), ) self.object.save() - else: - send_message_mp( - bot, - self.object.validation_private_message, - msg, - hat=get_hat_from_settings("moderation"), - no_notification_for=[self.request.user], - ) except ValueError: logger.exception("Could not %s the opinion %s", form.cleaned_data["operation"], str(self.object)) return HttpResponse(json.dumps({"result": "FAIL", "reason": str(_("Mauvaise opération"))}), status=400) From 2cb38029cc2f0367a8ccc86b2c0c72f3a8eaa1fa Mon Sep 17 00:00:00 2001 From: Philippe MILINK Date: Sat, 23 Jul 2022 19:10:15 +0200 Subject: [PATCH 2/2] Ne garde *que* les destinataires non-bots des MP de validations --- zds/mp/models.py | 10 ++++ zds/tutorialv2/tests/tests_opinion_views.py | 8 +++ .../tests/tests_views/tests_published.py | 4 ++ zds/tutorialv2/views/validations_contents.py | 7 ++- zds/tutorialv2/views/validations_opinions.py | 57 ++++++++++--------- 5 files changed, 56 insertions(+), 30 deletions(-) diff --git a/zds/mp/models.py b/zds/mp/models.py index afd32149f1..bbfee85f87 100644 --- a/zds/mp/models.py +++ b/zds/mp/models.py @@ -30,6 +30,16 @@ def is_reachable(user): return settings.ZDS_APP["member"]["bot_group"] not in user_group_names +def filter_reachable(users): + """ + Returns a list with only reachable users. + + :param user: a list of users + :return: list of reachable users. + """ + return [u for u in users if is_reachable(u)] + + class PrivateTopic(models.Model): """ Private topic, containing private posts. diff --git a/zds/tutorialv2/tests/tests_opinion_views.py b/zds/tutorialv2/tests/tests_opinion_views.py index 5cf03d275f..20731444b8 100644 --- a/zds/tutorialv2/tests/tests_opinion_views.py +++ b/zds/tutorialv2/tests/tests_opinion_views.py @@ -2,6 +2,7 @@ from unittest.mock import patch from django.conf import settings +from django.contrib.auth.models import Group from django.core.management import call_command from django.urls import reverse from django.test import TestCase @@ -31,9 +32,16 @@ class PublishedContentTests(TutorialTestMixin, TestCase): def setUp(self): self.overridden_zds_app["member"]["bot_account"] = ProfileFactory().user.username + self.bot_group = Group() + self.bot_group.name = settings.ZDS_APP["member"]["bot_group"] + self.bot_group.save() self.licence = LicenceFactory() self.anonymous = UserFactory(username=settings.ZDS_APP["member"]["anonymous_account"], password="anything") + self.anonymous.groups.add(self.bot_group) + self.anonymous.save() self.external = UserFactory(username=settings.ZDS_APP["member"]["external_account"], password="anything") + self.external.groups.add(self.bot_group) + self.external.save() self.user_author = ProfileFactory().user self.user_staff = StaffProfileFactory().user diff --git a/zds/tutorialv2/tests/tests_views/tests_published.py b/zds/tutorialv2/tests/tests_views/tests_published.py index d2884e9a59..cc71a5d098 100644 --- a/zds/tutorialv2/tests/tests_views/tests_published.py +++ b/zds/tutorialv2/tests/tests_views/tests_published.py @@ -62,7 +62,11 @@ def setUp(self): bot = Group(name=overridden_zds_app["member"]["bot_group"]) bot.save() self.external = UserFactory(username=overridden_zds_app["member"]["external_account"], password="anything") + self.external.groups.add(bot) + self.external.save() self.anonymous = UserFactory(username=settings.ZDS_APP["member"]["anonymous_account"], password="anything") + self.anonymous.groups.add(bot) + self.anonymous.save() self.beta_forum = ForumFactory( pk=overridden_zds_app["forum"]["beta_forum_id"], diff --git a/zds/tutorialv2/views/validations_contents.py b/zds/tutorialv2/views/validations_contents.py index 1d356f5559..635becf757 100644 --- a/zds/tutorialv2/views/validations_contents.py +++ b/zds/tutorialv2/views/validations_contents.py @@ -15,7 +15,7 @@ from django.views.generic import ListView, FormView from zds.member.decorator import LoggedWithReadWriteHability -from zds.mp.models import mark_read +from zds.mp.models import mark_read, filter_reachable from zds.tutorialv2 import signals from zds.tutorialv2.forms import ( AskValidationForm, @@ -572,7 +572,8 @@ def form_valid(self, form): self.object.save() # send PM - if validation.content.authors.first().username != settings.ZDS_APP["member"]["external_account"]: + recipients = filter_reachable(validation.content.authors.all()) + if len(recipients) > 0: msg = render_to_string( "tutorialv2/messages/validation_revoke.md", { @@ -587,7 +588,7 @@ def form_valid(self, form): if not validation.content.validation_private_message: validation.content.validation_private_message = send_mp( bot, - validation.content.authors.all(), + recipients, self.object.validation_message_title, validation.content.title, msg, diff --git a/zds/tutorialv2/views/validations_opinions.py b/zds/tutorialv2/views/validations_opinions.py index c8f7df9a68..d59ab77254 100644 --- a/zds/tutorialv2/views/validations_opinions.py +++ b/zds/tutorialv2/views/validations_opinions.py @@ -16,6 +16,8 @@ from zds.gallery.models import Gallery from zds.member.decorator import LoggedWithReadWriteHability +from zds.mp.models import filter_reachable +from zds.mp.utils import send_mp, send_message_mp from zds.tutorialv2 import signals from zds.tutorialv2.forms import ( PublicationForm, @@ -31,7 +33,6 @@ from zds.tutorialv2.utils import clone_repo from zds.tutorialv2.views.validations_contents import logger from zds.utils.models import get_hat_from_settings -from zds.mp.utils import send_mp, send_message_mp class PublishOpinion(LoggedWithReadWriteHability, DoesNotRequireValidationFormViewMixin): @@ -134,19 +135,20 @@ def form_valid(self, form): hat=get_hat_from_settings("moderation"), no_notification_for=[self.request.user], ) - elif versioned.authors.first().username != settings.ZDS_APP["member"]["external_account"]: - # Send a message only if the author is still registered: - self.object.validation_private_message = send_mp( - bot, - versioned.authors.all(), - self.object.validation_message_title, - versioned.title, - msg, - send_by_mail=True, - direct=False, - hat=get_hat_from_settings("moderation"), - ) - self.object.save() + else: + recipients = filter_reachable(versioned.authors.all()) + if len(recipients) > 0: + self.object.validation_private_message = send_mp( + bot, + recipients, + self.object.validation_message_title, + versioned.title, + msg, + send_by_mail=True, + direct=False, + hat=get_hat_from_settings("moderation"), + ) + self.object.save() signals.opinions_management.send( sender=self.__class__, performer=self.request.user, content=self.object, action="unpublish" @@ -229,19 +231,20 @@ def form_valid(self, form): hat=get_hat_from_settings("moderation"), no_notification_for=[self.request.user], ) - elif versioned.authors.first().username != settings.ZDS_APP["member"]["external_account"]: - # Send a message only if the author is still registered: - self.object.validation_private_message = send_mp( - bot, - versioned.authors.all(), - self.object.validation_message_title, - versioned.title, - msg, - send_by_mail=True, - direct=False, - hat=get_hat_from_settings("moderation"), - ) - self.object.save() + else: + recipients = filter_reachable(versioned.authors.all()) + if len(recipients) > 0: + self.object.validation_private_message = send_mp( + bot, + recipients, + self.object.validation_message_title, + versioned.title, + msg, + send_by_mail=True, + direct=False, + hat=get_hat_from_settings("moderation"), + ) + self.object.save() except ValueError: logger.exception("Could not %s the opinion %s", form.cleaned_data["operation"], str(self.object)) return HttpResponse(json.dumps({"result": "FAIL", "reason": str(_("Mauvaise opération"))}), status=400)