Skip to content

Commit

Permalink
refactor: suppression du signal post_create au profit d'un appel di…
Browse files Browse the repository at this point in the history
…rect à la méthode `create_post_notifications` (#910)

## Description

🎸 cf titre

## Type de changement

🚧 technique

### Points d'attention

🦺 Bien que déclenché dans la méthode `save` du modèle `Post`, la méthode
n'est appelée que lors de la création d'une nouvelle instance de `Post`.
Pas de risque d'envoi massif de notifications en cas de `save` en masse
sur des instances existantes.
  • Loading branch information
vincentporte committed Feb 13, 2025
1 parent b302af3 commit cac4eac
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 152 deletions.
5 changes: 3 additions & 2 deletions lacommunaute/forum_conversation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
from machina.models.abstract_models import DatedModel
from taggit.managers import TaggableManager

from lacommunaute.forum_conversation.signals import post_create
from lacommunaute.forum_member.shortcuts import get_forum_member_display_name
from lacommunaute.forum_upvote.models import UpVote
from lacommunaute.notification.utils import create_post_notifications
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.models import EmailLastSeen, User

Expand Down Expand Up @@ -155,12 +155,13 @@ def save(self, *args, **kwargs):
created = not self.pk
super().save(*args, **kwargs)
if created:
post_create.send(sender=self.__class__, instance=self)
create_post_notifications(post=self)
EmailLastSeen.objects.seen(
email=self.poster.email if self.poster else self.username, kind=EmailLastSeenKind.POST
)



class CertifiedPost(DatedModel):
topic = models.OneToOneField(
Topic,
Expand Down
4 changes: 0 additions & 4 deletions lacommunaute/forum_conversation/signals.py

This file was deleted.

6 changes: 0 additions & 6 deletions lacommunaute/notification/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,3 @@ class NotificationConfig(AppConfig):
name = "lacommunaute.notification"
verbose_name = _("Notification")
verbose_name_plural = _("Notifications")

def ready(self):
from lacommunaute.forum_conversation.signals import post_create
from lacommunaute.notification.signals import create_post_notifications

post_create.connect(create_post_notifications)
27 changes: 0 additions & 27 deletions lacommunaute/notification/signals.py

This file was deleted.

109 changes: 0 additions & 109 deletions lacommunaute/notification/tests/tests_signals.py

This file was deleted.

109 changes: 107 additions & 2 deletions lacommunaute/notification/tests/tests_utils.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
import pytest
from django.test import TestCase
from faker import Faker

from lacommunaute.forum_conversation.factories import TopicFactory
from lacommunaute.forum.factories import ForumFactory
from lacommunaute.forum_conversation.factories import (
AnonymousPostFactory,
AnonymousTopicFactory,
PostFactory,
TopicFactory,
)
from lacommunaute.notification.enums import EmailSentTrackKind, NotificationDelay, delay_of_notifications
from lacommunaute.notification.factories import EmailSentTrackFactory, NotificationFactory
from lacommunaute.notification.models import EmailSentTrack
from lacommunaute.notification.models import EmailSentTrack, Notification
from lacommunaute.notification.utils import (
collect_new_users_for_onboarding,
get_serialized_messages,
Expand Down Expand Up @@ -73,3 +81,100 @@ def test_post_is_not_topic_head(self, db):
"url": f"{notification.post.topic.get_absolute_url(with_fqdn=True)}?notif={notification.uuid}",
}
]


@pytest.fixture(name="upvoter")
def fixture_upvoter():
return UserFactory()


@pytest.fixture(name="topic_in_forum_with_upvoter")
def fixture_topic_in_forum_with_upvoter(upvoter):
return TopicFactory(forum=ForumFactory(upvoted_by=[upvoter]), with_post=True)


class TestCreatePostNotifications:
def test_pending_topic(self, db, topic_in_forum_with_upvoter, upvoter):
notification = Notification.objects.get()

for key, expected in [
("recipient", upvoter.email),
("post", topic_in_forum_with_upvoter.first_post),
("kind", EmailSentTrackKind.PENDING_TOPIC),
("delay", NotificationDelay.DAY),
]:
assert getattr(notification, key) == expected

def test_first_reply(self, db, topic_in_forum_with_upvoter):
Notification.objects.all().delete()

PostFactory(topic=topic_in_forum_with_upvoter)
notification = Notification.objects.get()

for key, expected in [
("recipient", topic_in_forum_with_upvoter.first_post.poster.email),
("post", topic_in_forum_with_upvoter.last_post),
("kind", EmailSentTrackKind.FIRST_REPLY),
("delay", NotificationDelay.ASAP),
]:
assert getattr(notification, key) == expected

def test_first_reply_on_anonymous_topic(self, db):
topic = AnonymousTopicFactory(with_post=True)
Notification.objects.all().delete()

PostFactory(topic=topic)
notification = Notification.objects.get()

for key, expected in [
("recipient", topic.first_post.username),
("post", topic.last_post),
("kind", EmailSentTrackKind.FIRST_REPLY),
("delay", NotificationDelay.ASAP),
]:
assert getattr(notification, key) == expected

def test_following_replies(self, db, topic_in_forum_with_upvoter):
first_reply_poster = UserFactory()
PostFactory(topic=topic_in_forum_with_upvoter, poster=first_reply_poster)
Notification.objects.all().delete()

PostFactory(topic=topic_in_forum_with_upvoter)

assert Notification.objects.count() == 2

for notification in Notification.objects.all():
assert notification.recipient in [
topic_in_forum_with_upvoter.first_post.poster.email,
first_reply_poster.email,
]
for key, expected in [
("post", topic_in_forum_with_upvoter.last_post),
("kind", EmailSentTrackKind.FOLLOWING_REPLIES),
("delay", NotificationDelay.DAY),
]:
assert getattr(notification, key) == expected

def test_following_replies_on_anonymous_topic(self, db):
topic = AnonymousTopicFactory(with_post=True)
post = AnonymousPostFactory(topic=topic)
Notification.objects.all().delete()

PostFactory(topic=topic)
assert Notification.objects.count() == 2

for notification in Notification.objects.all():
assert notification.recipient in [topic.first_post.username, post.username]
for key, expected in [
("post", topic.last_post),
("kind", EmailSentTrackKind.FOLLOWING_REPLIES),
("delay", NotificationDelay.DAY),
]:
assert getattr(notification, key) == expected

def test_no_notifications_on_unapproved_post(self, db, upvoter):
topic = TopicFactory(forum=ForumFactory(upvoted_by=[upvoter]))

for _ in delay_of_notifications:
PostFactory(topic=topic, approved=False)
assert Notification.objects.count() == 0
28 changes: 26 additions & 2 deletions lacommunaute/notification/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

from django.utils.timezone import now, timedelta

from lacommunaute.notification.enums import EmailSentTrackKind
from lacommunaute.notification.models import EmailSentTrack
from lacommunaute.notification.enums import EmailSentTrackKind, delay_of_notifications
from lacommunaute.notification.models import EmailSentTrack, Notification
from lacommunaute.users.models import User


Expand All @@ -29,3 +29,27 @@ def get_serialized_messages(notifications):
}
for n in notifications
]


def create_post_notifications(post):
"""
When a Post is created, we schedule notifications to users active in the thread
"""
# NOTE: notifications solution depends on the fact that approval is completed at the time of creation
if not post.approved:
return

if post.is_topic_head:
kind = EmailSentTrackKind.PENDING_TOPIC
elif post.is_first_reply:
kind = EmailSentTrackKind.FIRST_REPLY
else:
kind = EmailSentTrackKind.FOLLOWING_REPLIES

delay = delay_of_notifications[kind]

notifications = [
Notification(recipient=email_address, post=post, kind=kind, delay=delay)
for email_address in post.topic.mails_to_notify()
]
Notification.objects.bulk_create(notifications)

0 comments on commit cac4eac

Please sign in to comment.