Skip to content

Commit

Permalink
feat: enregistrer le dernier evenement connu pour une adresse mail (#894
Browse files Browse the repository at this point in the history
)

## Description

🎸 Creer ou mettre à jour `EmailLastSeen` lors des évènements suivants :
* navigation d'un utilisateur authentifié : dans un `middleware` dedié
* creation d'un `Post`/`Topic` : dans la méthode `save` du modèle `Post`
* click sur une notification email : dans le middleware
`NotificationMiddleware` de suivi des urls dans les mails de
notification


## Type de changement

🎢 Nouvelle fonctionnalité (changement non cassant qui ajoute une
fonctionnalité).
🚧 technique

### Points d'attention

🦺 Les mises à jour de `Post` ne déclenche plus de mise à jour de
`EmailLastSeen` pour éviter la détérioration de données en cas de `save`
massif sur des instances existantes du modèle `Post`
🦺 Creation d'un `DSP`, d'un `UpVote` ou d'un `Event`, les vues sont en
`LoginRequiredOnly`. L'enregistrement du passage de l'utilisateur est
donc déjà enregistré par `MarkAsSeenLoggedUserMiddleware`
🦺 prérequis #891 et #892
  • Loading branch information
vincentporte authored Feb 12, 2025
1 parent fb886e1 commit 9aa316c
Show file tree
Hide file tree
Showing 13 changed files with 152 additions and 8 deletions.
1 change: 1 addition & 0 deletions config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"lacommunaute.utils.middleware.ParkingPageMiddleware",
"lacommunaute.openid_connect.middleware.ProConnectLoginMiddleware",
"lacommunaute.notification.middleware.NotificationMiddleware",
"lacommunaute.users.middleware.MarkAsSeenLoggedUserMiddleware",
]

THIRD_PARTIES_MIDDLEWARE = [
Expand Down
4 changes: 2 additions & 2 deletions lacommunaute/forum/tests/tests_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,12 @@ def test_cannot_submit_post(self, *args):
self.assertEqual(response.status_code, 200)
self.assertNotContains(response, f'id="collapsePost{self.topic.pk}')

def test_queries(self):
def test_numqueries(self):
ContentType.objects.clear_cache()

TopicFactory.create_batch(20, with_post=True)
self.client.force_login(self.user)
with self.assertNumQueries(22):
with self.assertNumQueries(23):
self.client.get(self.url)

def test_certified_post_display(self):
Expand Down
6 changes: 5 additions & 1 deletion lacommunaute/forum_conversation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
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.users.models import User
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.models import EmailLastSeen, User


class TopicQuerySet(models.QuerySet):
Expand Down Expand Up @@ -155,6 +156,9 @@ def save(self, *args, **kwargs):
super().save(*args, **kwargs)
if created:
post_create.send(sender=self.__class__, instance=self)
EmailLastSeen.objects.seen(
email=self.poster.email if self.poster else self.username, kind=EmailLastSeenKind.POST
)


class CertifiedPost(DatedModel):
Expand Down
23 changes: 23 additions & 0 deletions lacommunaute/forum_conversation/tests/tests_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
)
from lacommunaute.forum_conversation.models import Post, Topic
from lacommunaute.forum_member.shortcuts import get_forum_member_display_name
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.factories import UserFactory
from lacommunaute.users.models import EmailLastSeen


@pytest.fixture(name="forum")
Expand All @@ -39,6 +41,27 @@ def test_is_certified(self):
self.assertTrue(topic.last_post.is_certified)


class TestPostModel:
@pytest.mark.parametrize(
"topic", [lambda: TopicFactory(with_post=True), lambda: AnonymousTopicFactory(with_post=True)]
)
def test_email_last_seen_when_create(self, db, topic):
topic = topic()
email_last_seen = EmailLastSeen.objects.get()
assert email_last_seen.last_seen_kind == EmailLastSeenKind.POST

assert email_last_seen.email == topic.poster_email

@pytest.mark.parametrize(
"topic", [lambda: TopicFactory(with_post=True), lambda: AnonymousTopicFactory(with_post=True)]
)
def test_email_last_seen_when_update(self, db, topic):
topic = topic()
EmailLastSeen.objects.all().delete()
topic.first_post.save()
assert EmailLastSeen.objects.count() == 0


class TestTopicManager:
def test_unanswered(self, db, forum):
TopicFactory(forum=forum, posts_count=0)
Expand Down
22 changes: 21 additions & 1 deletion lacommunaute/forum_conversation/tests/tests_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
from lacommunaute.forum_moderation.factories import BlockedDomainNameFactory, BlockedEmailFactory
from lacommunaute.forum_upvote.factories import UpVoteFactory
from lacommunaute.notification.factories import NotificationFactory
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.factories import UserFactory
from lacommunaute.users.models import EmailLastSeen
from lacommunaute.utils.testing import parse_response_to_soup


Expand All @@ -39,6 +41,12 @@
assign_perm = get_class("forum_permission.shortcuts", "assign_perm")


def check_email_last_seen(email):
return EmailLastSeen.objects.filter(
email=email, last_seen_kind__in=[EmailLastSeenKind.POST, EmailLastSeenKind.LOGGED]
).exists()


class TopicCreateViewTest(TestCase):
@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -116,6 +124,8 @@ def test_topic_create_as_anonymous_user(self, *args):
self.assertTrue(topic.approved)
self.assertTrue(topic.first_post.approved)

self.assertTrue(check_email_last_seen(self.post_data["username"]))

def test_topic_create_as_unapproved_anonymous_user(self, *args):
self.post_data["username"] = faker.email()
BlockedEmailFactory(email=self.post_data["username"])
Expand All @@ -133,6 +143,8 @@ def test_topic_create_as_unapproved_anonymous_user(self, *args):
)
self.assertEqual(Topic.objects.count(), 0)

self.assertFalse(check_email_last_seen(self.post_data["username"]))

def test_topic_create_with_nonfr_content(self, *args):
self.client.force_login(self.poster)
self.post_data["content"] = "популярные лучшие песни слушать онлайн"
Expand Down Expand Up @@ -183,6 +195,8 @@ def test_topic_create_as_anonymous_user_with_blocked_domain_name(self, *args):
)
self.assertEqual(Topic.objects.count(), 0)

self.assertFalse(check_email_last_seen(self.post_data["username"]))

def test_topic_create_as_authenticated_user(self, *args):
self.client.force_login(self.poster)

Expand All @@ -196,6 +210,8 @@ def test_topic_create_as_authenticated_user(self, *args):
self.assertTrue(Topic.objects.first().approved)
self.assertTrue(Topic.objects.first().posts.first().approved)

self.assertTrue(check_email_last_seen(self.poster.email))

def test_tags_checkbox_are_displayed(self, *args):
Tag.objects.bulk_create([Tag(name=f"tag_x{i}", slug=f"tag_x{i}") for i in range(2)])
self.client.force_login(self.poster)
Expand Down Expand Up @@ -329,6 +345,7 @@ def test_selected_tags_are_checked(self):

def test_update_by_anonymous_user(self):
topic = AnonymousTopicFactory(with_post=True, forum=self.forum)
EmailLastSeen.objects.all().delete()
session = self.client.session
session["_anonymous_forum_key"] = topic.first_post.anonymous_key
session.save()
Expand Down Expand Up @@ -359,6 +376,9 @@ def test_update_by_anonymous_user(self):
),
)

self.assertFalse(check_email_last_seen("foo@email.com"))
self.assertFalse(check_email_last_seen(topic.first_post.username))

def test_topic_update_with_nonfr_content(self, *args):
self.client.force_login(self.poster)
post_data = {"subject": "s", "content": "популярные лучшие песни слушать онлайн"}
Expand Down Expand Up @@ -737,7 +757,7 @@ def test_numqueries(self):
self.client.force_login(self.poster)

# note vincentporte : to be optimized
with self.assertNumQueries(39):
with self.assertNumQueries(40):
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)

Expand Down
31 changes: 31 additions & 0 deletions lacommunaute/forum_conversation/tests/tests_views_htmx.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import pytest
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from django.test import RequestFactory, TestCase
Expand All @@ -6,6 +7,7 @@
from machina.core.db.models import get_model
from machina.core.loading import get_class

from lacommunaute.forum.factories import ForumFactory
from lacommunaute.forum_conversation.factories import CertifiedPostFactory, PostFactory, TopicFactory
from lacommunaute.forum_conversation.forms import PostForm
from lacommunaute.forum_conversation.models import CertifiedPost, Topic
Expand All @@ -15,7 +17,9 @@
from lacommunaute.forum_moderation.models import BlockedPost
from lacommunaute.forum_upvote.factories import UpVoteFactory
from lacommunaute.notification.factories import NotificationFactory
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.factories import UserFactory
from lacommunaute.users.models import EmailLastSeen


faker = Faker(settings.LANGUAGE_CODE)
Expand Down Expand Up @@ -424,6 +428,33 @@ def test_create_post_with_blocked_domain_name(self):
assert blocked_post.block_reason == BlockedPostReason.BLOCKED_DOMAIN


class TestPostFeedCreateView:
@pytest.mark.parametrize("logged", [True, False])
def test_email_last_seen_is_updated(self, client, db, logged):
topic = TopicFactory(with_post=True, forum=ForumFactory(with_public_perms=True))
url = reverse(
"forum_conversation_extension:post_create",
kwargs={
"forum_pk": topic.forum.pk,
"forum_slug": topic.forum.slug,
"pk": topic.pk,
"slug": topic.slug,
},
)
data = {"content": faker.paragraph(nb_sentences=5)}

if logged:
client.force_login(topic.poster)
else:
data["username"] = "bobby@lapointe.fr"

response = client.post(url, data=data)
assert response.status_code == 200

email = topic.poster.email if logged else data["username"]
assert EmailLastSeen.objects.filter(email=email, last_seen_kind=EmailLastSeenKind.POST).exists()


# vincentporte : not to futur self, rewrite it in pytest style
class CertifiedPostViewTest(TestCase):
@classmethod
Expand Down
2 changes: 1 addition & 1 deletion lacommunaute/forum_upvote/tests/test_upvotelistview.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ def test_numqueries(client, db, django_assert_num_queries):
PostFactory.create_batch(20, topic=TopicFactory(), upvoted_by=[user])

# vincentporte : to be optimized
with django_assert_num_queries(37):
with django_assert_num_queries(38):
client.get(url)
12 changes: 10 additions & 2 deletions lacommunaute/notification/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from django.utils.deprecation import MiddlewareMixin

from lacommunaute.notification.models import Notification
from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.models import EmailLastSeen


class NotificationMiddleware(MiddlewareMixin):
Expand All @@ -15,7 +17,13 @@ def process_request(self, request):

try:
uuid.UUID(notif_uuid, version=4)
try:
notification = Notification.objects.get(uuid=notif_uuid)
except Notification.DoesNotExist:
pass
else:
notification.visited_at = timezone.now()
notification.save()
EmailLastSeen.objects.seen(email=notification.recipient, kind=EmailLastSeenKind.VISITED)
except ValueError:
pass
else:
Notification.objects.filter(uuid=notif_uuid).update(visited_at=timezone.now())
4 changes: 4 additions & 0 deletions lacommunaute/notification/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from lacommunaute.notification.factories import NotificationFactory
from lacommunaute.notification.models import Notification
from lacommunaute.users.models import EmailLastSeen


@pytest.mark.parametrize(
Expand All @@ -25,5 +26,8 @@ def test_notif_param(client, db, notif_param, expected_visited_at):
notification.refresh_from_db()
assert notification.visited_at is not None

email_last_seen = EmailLastSeen.objects.get()
assert email_last_seen.email == notification.recipient

misc_notification.refresh_from_db()
assert misc_notification.visited_at is None
15 changes: 15 additions & 0 deletions lacommunaute/users/middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import logging

from django.utils.deprecation import MiddlewareMixin

from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.models import EmailLastSeen


logger = logging.getLogger(__name__)


class MarkAsSeenLoggedUserMiddleware(MiddlewareMixin):
def process_request(self, request):
if request.user.is_authenticated:
EmailLastSeen.objects.seen(email=request.user.email, kind=EmailLastSeenKind.LOGGED)
7 changes: 6 additions & 1 deletion lacommunaute/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ def seen(self, email, kind):
if kind not in [kind for kind, _ in EmailLastSeenKind.choices]:
raise ValueError(f"Invalid kind: {kind}")

return self.update_or_create(email=email, defaults={"last_seen_at": timezone.now(), "last_seen_kind": kind})
return EmailLastSeen.objects.bulk_create(
[EmailLastSeen(email=email, last_seen_at=timezone.now(), last_seen_kind=kind)],
update_fields=["last_seen_at", "last_seen_kind"],
update_conflicts=True,
unique_fields=["email"],
)


class EmailLastSeen(models.Model):
Expand Down
22 changes: 22 additions & 0 deletions lacommunaute/users/tests/tests_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import pytest

from lacommunaute.users.enums import EmailLastSeenKind
from lacommunaute.users.factories import UserFactory
from lacommunaute.users.models import EmailLastSeen


@pytest.mark.parametrize("logged", [True, False])
def test_mark_as_seen_logged_user_middleware(client, db, logged):
if logged:
user = UserFactory()
client.force_login(user)

response = client.get("/")
assert response.status_code == 200

if logged:
email_last_seen = EmailLastSeen.objects.get()
assert email_last_seen.email == user.email
assert email_last_seen.last_seen_kind == EmailLastSeenKind.LOGGED
else:
assert not EmailLastSeen.objects.exists()
11 changes: 11 additions & 0 deletions lacommunaute/users/tests/tests_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,14 @@ def test_seen_unknown_email(self, db, kind):
email_last_seen = EmailLastSeen.objects.get()
assert email_last_seen.last_seen_kind == kind
assert email_last_seen.last_seen_at is not None

@pytest.mark.parametrize("email_last_seen", [None, lambda: EmailLastSeenFactory()])
def test_numqueries(self, db, django_assert_num_queries, email_last_seen):
if email_last_seen:
email_last_seen = email_last_seen()

email = email_last_seen.email if email_last_seen else EMAIL
kind = email_last_seen.last_seen_kind if email_last_seen else EmailLastSeenKind.POST

with django_assert_num_queries(1):
EmailLastSeen.objects.seen(email=email, kind=kind)

0 comments on commit 9aa316c

Please sign in to comment.