diff --git a/templates/pages/alerts.html b/templates/pages/alerts.html index bd515d3cca..0fc74ddeb8 100644 --- a/templates/pages/alerts.html +++ b/templates/pages/alerts.html @@ -47,6 +47,7 @@

{% trans "Liste des alertes en cours" %}

{% elif alert.scope == 'PROFILE' %} {{ alert.text }} {% else %} + {# This is an alert about a comment on something (any kind of content or forum post) #} {{ alert.text }} {% endif %} @@ -94,9 +95,16 @@

{% trans "Liste des alertes récemment résolues" %}

{% elif alert.scope == 'PROFILE' %} {{ alert.text }} {% else %} + {# This is an alert about a comment on something (any kind of content or forum post) #} + {% url "member-detail" alert.comment.author.username as url_member_detail %} - {{ alert.text }} par - {{ alert.comment.author.username }} + + {% if alert.is_on_comment_on_unpublished_content %} + {{ alert.text }} + {% else %} + {{ alert.text }} + {% endif %} + par {{ alert.comment.author.username }} {% endif %} diff --git a/zds/tutorialv2/tests/factories.py b/zds/tutorialv2/tests/factories.py index 294dfbe58c..7eba05b258 100644 --- a/zds/tutorialv2/tests/factories.py +++ b/zds/tutorialv2/tests/factories.py @@ -195,6 +195,7 @@ class Meta: ip_address = "192.168.3.1" text = "Bonjour, je me présente, je m'appelle l'homme au texte bidonné" + position = 1 @classmethod def _generate(cls, create, attrs): diff --git a/zds/tutorialv2/tests/tests_views/tests_published.py b/zds/tutorialv2/tests/tests_views/tests_published.py index 9cf9f5689b..3ab70abe64 100644 --- a/zds/tutorialv2/tests/tests_views/tests_published.py +++ b/zds/tutorialv2/tests/tests_views/tests_published.py @@ -15,6 +15,7 @@ from zds.mp.models import PrivateTopic, is_privatetopic_unread from zds.notification.models import ContentReactionAnswerSubscription, Notification from zds.tutorialv2.tests.factories import ( + ContentReactionFactory, PublishableContentFactory, ContainerFactory, ExtractFactory, @@ -2088,3 +2089,60 @@ def test_save_no_redirect(self): result = loads(resp.content.decode("utf-8")) self.assertEqual("ok", result.get("result", None)) self.assertEqual(extract.compute_hash(), result.get("last_hash", None)) + + def test_remove_unpublished_opinion_with_alerted_comments(self): + """Test the page showing alerts with an alerted comment on a removed opinion""" + + alert_page_url = reverse("pages-alerts") + + # 1. Publish opinion + opinion = PublishedContentFactory(type="OPINION", author_list=[self.user_author]) + + # 2. Comment the opinion + random_user = ProfileFactory().user + comment = ContentReactionFactory(related_content=opinion, author=random_user) + + # 3. Create an alert for the comment + self.client.force_login(self.user_staff) + result = self.client.post( + reverse("content:alert-reaction", args=[comment.pk]), + {"signal_text": "No. Try not. Do... or do not. There is no try."}, + follow=False, + ) + self.assertEqual(result.status_code, 302) + + # 4. Display the page listing alerts + resp = self.client.get(alert_page_url) + self.assertEqual(200, resp.status_code) + self.assertContains(resp, comment.get_absolute_url()) # We have a link to the alerted comment + self.assertEqual(len(resp.context["alerts"]), 1) + self.assertEqual(len(resp.context["solved"]), 0) + + # 5. Unpublish the 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) + + # 6. Display the page listing alerts + resp = self.client.get(alert_page_url) + self.assertEqual(200, resp.status_code) + self.assertNotContains(resp, 'href="?page=1#p') # We don't have wrong links + self.assertEqual(len(resp.context["alerts"]), 0) + self.assertEqual(len(resp.context["solved"]), 1) + + # 7. Remove the opinion + self.client.force_login(self.user_author) + result = self.client.post(reverse("content:delete", args=[opinion.pk, opinion.slug]), follow=False) + self.assertEqual(result.status_code, 302) + + # 8. Display the page listing alerts + self.client.force_login(self.user_staff) + resp = self.client.get(alert_page_url) + self.assertEqual(200, resp.status_code) + self.assertEqual(len(resp.context["alerts"]), 0) + self.assertEqual(len(resp.context["solved"]), 0) diff --git a/zds/utils/admin.py b/zds/utils/admin.py index fd6618577d..55b7cf3266 100644 --- a/zds/utils/admin.py +++ b/zds/utils/admin.py @@ -25,9 +25,9 @@ def parent_category(self, obj): class AlertAdmin(admin.ModelAdmin): - list_display = ("author", "text", "solved") + list_display = ("author", "scope", "text", "pubdate", "solved", "solved_date") list_filter = ("scope", "solved") - raw_id_fields = ("author", "comment", "moderator", "privatetopic") + raw_id_fields = ("author", "comment", "moderator", "privatetopic", "profile", "content") ordering = ("-pubdate",) search_fields = ("author__username", "text") diff --git a/zds/utils/migrations/0028_alert_cascade_delete.py b/zds/utils/migrations/0028_alert_cascade_delete.py new file mode 100644 index 0000000000..396afac87d --- /dev/null +++ b/zds/utils/migrations/0028_alert_cascade_delete.py @@ -0,0 +1,39 @@ +# Generated by Django 4.2.16 on 2024-10-20 16:28 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("tutorialv2", "0041_remove_must_reindex"), + ("utils", "0027_remove_update_index_date"), + ] + + operations = [ + migrations.AlterField( + model_name="alert", + name="comment", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="alerts_on_this_comment", + to="utils.comment", + verbose_name="Commentaire", + ), + ), + migrations.AlterField( + model_name="alert", + name="content", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="alerts_on_this_content", + to="tutorialv2.publishablecontent", + verbose_name="Contenu", + ), + ), + ] diff --git a/zds/utils/migrations/0029_normalize_alert_scopes.py b/zds/utils/migrations/0029_normalize_alert_scopes.py new file mode 100644 index 0000000000..315d29906a --- /dev/null +++ b/zds/utils/migrations/0029_normalize_alert_scopes.py @@ -0,0 +1,44 @@ +# Generated by Django 4.2.16 on 2024-10-19 22:55 + +""" +In production, the column `scope` of the table containg the alerts contains +leftovers from older alert management: + +SELECT BINARY scope, COUNT(*) AS nb, MIN(pubdate) AS first_pubdate, MAX(pubdate) AS last_pubdate FROM utils_alert WHERE solved=1 GROUP BY BINARY scope; ++--------------+------+---------------------+---------------------+ +| BINARY scope | nb | first_pubdate | last_pubdate | ++--------------+------+---------------------+---------------------+ +| ARTICLE | 113 | 2017-05-15 11:03:23 | 2024-09-04 10:09:20 | +| Article | 5 | 2017-01-24 21:34:56 | 2017-04-20 15:56:43 | +| CONTENT | 115 | 2017-05-04 12:28:11 | 2024-08-08 08:46:31 | +| FORUM | 3756 | 2016-12-13 19:03:00 | 2024-09-15 22:37:04 | +| OPINION | 202 | 2017-05-21 14:28:24 | 2024-09-04 15:20:13 | +| PROFILE | 1088 | 2019-09-18 22:16:55 | 2024-09-16 17:39:55 | +| TUTORIAL | 392 | 2017-05-21 21:48:11 | 2024-09-12 20:07:01 | +| Tutoriel | 7 | 2016-12-21 22:56:29 | 2017-04-26 12:21:32 | ++--------------+------+---------------------+---------------------+ + +This migration normalizes the scope values of all alerts. +""" + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("utils", "0028_alert_cascade_delete"), + ] + + operations = [ + # These WHERE are actually case *in*sensitive, but it will not change + # the result (just modify more records which don't need it), but + # having a WHERE which is case-sensitive *and* compatible with both + # SQLite and MariaDB seems tricky... + migrations.RunSQL( + ("UPDATE utils_alert SET scope = 'ARTICLE' WHERE scope = 'Article';"), + ), + migrations.RunSQL( + ("UPDATE utils_alert SET scope = 'TUTORIAL' WHERE scope = 'Tutoriel';"), + ), + ] diff --git a/zds/utils/models.py b/zds/utils/models.py index 9a6337fbf2..307e0254c3 100644 --- a/zds/utils/models.py +++ b/zds/utils/models.py @@ -617,7 +617,14 @@ def __str__(self): class Alert(models.Model): - """Alerts on all kinds of Comments and PublishedContents.""" + """Alerts on Profiles, PublishedContents and all kinds of Comments. + + The scope field indicates on which type of element the alert is made: + - PROFILE: the profile of a member + - FORUM: a post on a topic in a forum + - CONTENT: the content (article, opinion or tutorial) itself + - elements of TYPE_CHOICES (ARTICLE, OPINION, TUTORIAL): a comment on a content of this type + """ SCOPE_CHOICES = [ ("PROFILE", _("Profil")), @@ -637,7 +644,7 @@ class Alert(models.Model): db_index=True, null=True, blank=True, - on_delete=models.SET_NULL, + on_delete=models.CASCADE, ) # use of string definition of pk to avoid circular import. profile = models.ForeignKey( @@ -656,7 +663,7 @@ class Alert(models.Model): db_index=True, null=True, blank=True, - on_delete=models.SET_NULL, + on_delete=models.CASCADE, ) scope = models.CharField(max_length=10, choices=SCOPE_CHOICES, db_index=True) text = models.TextField("Texte d'alerte") @@ -681,9 +688,23 @@ class Alert(models.Model): def get_type(self): if self.scope in TYPE_CHOICES_DICT: - return _("Commentaire") + assert self.comment is not None + if self.is_on_comment_on_unpublished_content(): + return _(f"Commentaire sur un {self.SCOPE_CHOICES_DICT[self.scope].lower()} dépublié") + else: + return _(f"Commentaire sur un {self.SCOPE_CHOICES_DICT[self.scope].lower()}") + elif self.scope == "FORUM": + assert self.comment is not None + return _("Message de forum") + elif self.scope == "PROFILE": + assert self.profile is not None + return _("Profil") else: - return self.get_scope_display() + assert self.content is not None + return self.SCOPE_CHOICES_DICT[self.content.type] + + def is_on_comment_on_unpublished_content(self): + return self.scope in TYPE_CHOICES_DICT and not self.get_comment_subclass().related_content.in_public() def is_automated(self): """Returns true if this alert was opened automatically."""