From 915c7d177d5bd96d3ee869aacb58581f4df21fde Mon Sep 17 00:00:00 2001 From: ChantyTaguan Date: Tue, 28 Dec 2021 14:45:00 +0100 Subject: [PATCH] Supprime notifications forum inaccessible (#6196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Supprime code inutile Redondant avec notification/receivers.py remove_group_subscription_on_quitting_groups * Complète les tests * Supprime les notifications caduques qui trainent * Ajout d'un forum réservé au staff dans les fixtures --- fixtures/forums.yaml | 8 ++ zds/member/views.py | 9 -- ...n_notifications_new_topic_forums_groups.py | 26 +++++ zds/notification/tests/tests_tricky.py | 102 +++++++++++++++++- 4 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 zds/notification/migrations/0017_clean_notifications_new_topic_forums_groups.py diff --git a/fixtures/forums.yaml b/fixtures/forums.yaml index 69c5e589a1..a49fcf641e 100644 --- a/fixtures/forums.yaml +++ b/fixtures/forums.yaml @@ -106,3 +106,11 @@ subtitle: Discutez de tout ! category: 4 slug: discussions-generales +- model: forum.Forum + pk: 13 + fields: + title: Staff only + subtitle: Réservé à l'équipe ! + category: 4 + slug: staff-only + groups: [1] diff --git a/zds/member/views.py b/zds/member/views.py index bd237803fe..8f142e4dd0 100644 --- a/zds/member/views.py +++ b/zds/member/views.py @@ -1405,16 +1405,7 @@ def settings_promote(request, user_pk): request, _("{0} n'appartient maintenant plus au groupe {1}.").format(user.username, group.name), ) - topics_followed = TopicAnswerSubscription.objects.get_objects_followed_by(user) - for topic in topics_followed: - if isinstance(topic, Topic) and group in topic.forum.groups.all(): - TopicAnswerSubscription.objects.toggle_follow(topic, user) else: - for group in usergroups: - topics_followed = TopicAnswerSubscription.objects.get_objects_followed_by(user) - for topic in topics_followed: - if isinstance(topic, Topic) and group in topic.forum.groups.all(): - TopicAnswerSubscription.objects.toggle_follow(topic, user) user.groups.clear() messages.warning(request, _("{0} n'appartient (plus ?) à aucun groupe.").format(user.username)) diff --git a/zds/notification/migrations/0017_clean_notifications_new_topic_forums_groups.py b/zds/notification/migrations/0017_clean_notifications_new_topic_forums_groups.py new file mode 100644 index 0000000000..76267b56f4 --- /dev/null +++ b/zds/notification/migrations/0017_clean_notifications_new_topic_forums_groups.py @@ -0,0 +1,26 @@ +from django.db import migrations + +from zds.notification.models import NewTopicSubscription +from zds.forum.models import Topic, Forum + + +def cleanup(apps, *_): + for forum in Forum.objects.filter(groups__isnull=False).all(): + for subscription in NewTopicSubscription.objects.get_subscriptions(forum): + if not forum.can_read(subscription.user): + subscription.is_active = False + if subscription.last_notification: + subscription.last_notification.is_read = True + subscription.last_notification.save() + subscription.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("notification", "0016_auto_20190114_1301"), + ] + + operations = [ + migrations.RunPython(cleanup), + ] diff --git a/zds/notification/tests/tests_tricky.py b/zds/notification/tests/tests_tricky.py index e07f2e4779..15642c246d 100644 --- a/zds/notification/tests/tests_tricky.py +++ b/zds/notification/tests/tests_tricky.py @@ -5,13 +5,15 @@ from django.core import mail from django.test import TestCase from django.test.utils import override_settings +from django.contrib.contenttypes.models import ContentType -from zds.forum.factories import ForumCategoryFactory, ForumFactory +from zds.forum.factories import ForumCategoryFactory, ForumFactory, TopicFactory, PostFactory from zds.forum.models import Topic from zds.gallery.factories import UserGalleryFactory from zds.member.factories import StaffProfileFactory, ProfileFactory from zds.notification.models import ( NewTopicSubscription, + TopicAnswerSubscription, Notification, NewPublicationSubscription, ContentReactionAnswerSubscription, @@ -287,7 +289,7 @@ def test_no_dead_ping_notif_on_moving_to_private_forum(self): self.assertEqual(subscription.last_notification, Notification.objects.filter(sender=self.user2).first()) self.assertTrue(subscription.last_notification.is_read, "As forum is not reachable, notification is read") - def test_no_more_notif_on_losing_all_groups(self): + def test_no_more_new_topic_notif_on_losing_all_groups(self): NewTopicSubscription.objects.get_or_create_active(self.to_be_changed_staff, self.forum12) self.client.force_login(self.staff) self.client.post( @@ -302,13 +304,18 @@ def test_no_more_notif_on_losing_all_groups(self): ) subscription = NewTopicSubscription.objects.get_existing(self.to_be_changed_staff, self.forum12, True) self.assertIsNotNone(subscription, "There must be an active subscription for now") + self.assertIsNotNone(subscription.last_notification, "There must be a notification.") + self.assertFalse(subscription.last_notification.is_read, "The notification has not been read yet") + self.to_be_changed_staff.groups.clear() self.to_be_changed_staff.save() + subscription = NewTopicSubscription.objects.get_existing(self.to_be_changed_staff, self.forum12, False) - self.assertIsNotNone(subscription, "There must be an active subscription for now") + self.assertIsNotNone(subscription, "The subscription should now be inactive") self.assertFalse(subscription.is_active) + self.assertTrue(subscription.last_notification.is_read, "As forum is not reachable, notification is read") - def test_no_more_notif_on_losing_one_group(self): + def test_no_more_new_topic_notif_on_losing_one_group(self): NewTopicSubscription.objects.get_or_create_active(self.to_be_changed_staff, self.forum12) self.client.force_login(self.staff) self.client.post( @@ -323,11 +330,98 @@ def test_no_more_notif_on_losing_one_group(self): ) subscription = NewTopicSubscription.objects.get_existing(self.to_be_changed_staff, self.forum12, True) self.assertIsNotNone(subscription, "There must be an active subscription for now") + self.assertIsNotNone(subscription.last_notification, "There must be a notification.") + self.assertFalse(subscription.last_notification.is_read, "The notification has not been read yet") + self.to_be_changed_staff.groups.remove(list(self.to_be_changed_staff.groups.all())[0]) self.to_be_changed_staff.save() + subscription = NewTopicSubscription.objects.get_existing(self.to_be_changed_staff, self.forum12, False) + self.assertIsNotNone(subscription, "There must be an inactive subscription now") + self.assertFalse(subscription.is_active) + self.assertTrue(subscription.last_notification.is_read, "As forum is not reachable, notification is read") + + def test_no_more_topic_answer_notif_on_losing_all_groups(self): + self.client.force_login(self.to_be_changed_staff) + self.client.post( + reverse("topic-new") + f"?forum={self.forum12.pk}", + { + "title": "Super sujet", + "subtitle": "Pour tester les notifs", + "text": "En tout cas l'un abonnement", + "tags": "", + }, + follow=False, + ) + topic = Topic.objects.filter(title="Super sujet").first() + + self.client.force_login(self.staff) + self.client.post( + reverse("post-new") + f"?sujet={topic.pk}", + { + "last_post": topic.last_message.pk, + "text": "C'est tout simplement l'histoire de la ville de Paris que je voudrais vous conter ", + }, + follow=False, + ) + + subscription = TopicAnswerSubscription.objects.get_existing( + content_object=topic, user=self.to_be_changed_staff, is_active=True + ) + self.assertIsNotNone(subscription, "There must be an active subscription for now") + self.assertIsNotNone(subscription.last_notification, "There must be a notification.") + self.assertFalse(subscription.last_notification.is_read, "The notification has not been read yet") + + self.to_be_changed_staff.groups.clear() + self.to_be_changed_staff.save() + + subscription = TopicAnswerSubscription.objects.get_existing( + content_object=topic, user=self.to_be_changed_staff, is_active=False + ) + self.assertIsNotNone(subscription, "The subscription must now be inactive") + self.assertFalse(subscription.is_active) + self.assertTrue(subscription.last_notification.is_read, "As forum is not reachable, notification is read") + + def test_no_more_topic_answer_notif_on_losing_one_group(self): + self.client.force_login(self.to_be_changed_staff) + self.client.post( + reverse("topic-new") + f"?forum={self.forum12.pk}", + { + "title": "Super sujet", + "subtitle": "Pour tester les notifs", + "text": "En tout cas l'un abonnement", + "tags": "", + }, + follow=False, + ) + topic = Topic.objects.filter(title="Super sujet").first() + + self.client.force_login(self.staff) + self.client.post( + reverse("post-new") + f"?sujet={topic.pk}", + { + "last_post": topic.last_message.pk, + "text": "C'est tout simplement l'histoire de la ville de Paris que je voudrais vous conter ", + }, + follow=False, + ) + + subscription = TopicAnswerSubscription.objects.get_existing( + content_object=topic, user=self.to_be_changed_staff, is_active=True + ) self.assertIsNotNone(subscription, "There must be an active subscription for now") + self.assertIsNotNone(subscription.last_notification, "There must be a notification.") + self.assertFalse(subscription.last_notification.is_read, "The notification has not been read yet") + + self.to_be_changed_staff.groups.remove(list(self.to_be_changed_staff.groups.all())[0]) + self.to_be_changed_staff.save() + + subscription = TopicAnswerSubscription.objects.get_existing( + content_object=topic, user=self.to_be_changed_staff, is_active=False + ) + self.assertIsNotNone(subscription, "The subscription must now be inactive") self.assertFalse(subscription.is_active) + self.assertTrue(subscription.last_notification.is_read, "As forum is not reachable, notification is read") @override_for_contents()