From c0cef18bb20ae8a7bdce165df235b2a6931e7529 Mon Sep 17 00:00:00 2001 From: "Ph. SW." Date: Wed, 23 Oct 2024 08:09:53 +0200 Subject: [PATCH] =?UTF-8?q?Ensemble=20d'am=C3=A9liorations=20sur=20la=20pa?= =?UTF-8?q?ge=20listant=20les=20alertes=20(#6671)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Améliore la zone d'admin pour les alertes * Supprime les alertes concernant les commentaires de contenus supprimés Un billet peut être dépublié puis supprimé par son auteurice, si ce billet avait des commentaires avec des alertes, ces alertes n'étaient pas supprimées, ce qui levait une exception lorsqu'on allait sur la page des alertes. On supprime maintenant les alertes lorsque l'élément qu'elles concernent est supprimé. Problème rapporté par Sentry. * Homogénéise les scopes des alertes en base de données * Précise mieux sur la page des alertes sur quel type d'élément porte l'alerte * N'affiche pas de lien vers un commentaire signalé inaccessible sur la page des alertes S'il y avait une alerte sur un contenu dépublié, l'alerte était toujours listée sur la page des alertes, mais avec un lien vers le commentaire signalé mal formé, puisqu'il était de la forme pages/alertes/?page=1&#p123. --- templates/pages/alerts.html | 12 +++- zds/tutorialv2/tests/factories.py | 1 + .../tests/tests_views/tests_published.py | 58 +++++++++++++++++++ zds/utils/admin.py | 4 +- .../migrations/0028_alert_cascade_delete.py | 39 +++++++++++++ .../migrations/0029_normalize_alert_scopes.py | 44 ++++++++++++++ zds/utils/models.py | 31 ++++++++-- 7 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 zds/utils/migrations/0028_alert_cascade_delete.py create mode 100644 zds/utils/migrations/0029_normalize_alert_scopes.py 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."""