Skip to content

Commit

Permalink
Ensemble d'améliorations sur la page listant les alertes (#6671)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
philippemilink authored Oct 23, 2024
1 parent 1a6e95c commit c0cef18
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 9 deletions.
12 changes: 10 additions & 2 deletions templates/pages/alerts.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ <h1>{% trans "Liste des alertes en cours" %}</h1>
{% elif alert.scope == 'PROFILE' %}
<a href="{{ alert.profile.get_absolute_url }}">{{ alert.text }}</a>
{% else %}
{# This is an alert about a comment on something (any kind of content or forum post) #}
<a href="{{ alert.get_comment_subclass.get_absolute_url }}">{{ alert.text }}</a>
{% endif %}
</td>
Expand Down Expand Up @@ -94,9 +95,16 @@ <h1>{% trans "Liste des alertes récemment résolues" %}</h1>
{% elif alert.scope == 'PROFILE' %}
<a href="{{ alert.profile.get_absolute_url }}">{{ alert.text }}</a>
{% 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 %}
<a href="{{ alert.get_comment_subclass.get_absolute_url }}">{{ alert.text }}</a> par
<a href="{{ url_member_detail }}">{{ alert.comment.author.username }}</a>

{% if alert.is_on_comment_on_unpublished_content %}
<em>{{ alert.text }}</em>
{% else %}
<a href="{{ alert.get_comment_subclass.get_absolute_url }}">{{ alert.text }}</a>
{% endif %}
par <a href="{{ url_member_detail }}">{{ alert.comment.author.username }}</a>
{% endif %}
</td>
<td>
Expand Down
1 change: 1 addition & 0 deletions zds/tutorialv2/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
58 changes: 58 additions & 0 deletions zds/tutorialv2/tests/tests_views/tests_published.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
4 changes: 2 additions & 2 deletions zds/utils/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
39 changes: 39 additions & 0 deletions zds/utils/migrations/0028_alert_cascade_delete.py
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
44 changes: 44 additions & 0 deletions zds/utils/migrations/0029_normalize_alert_scopes.py
Original file line number Diff line number Diff line change
@@ -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';"),
),
]
31 changes: 26 additions & 5 deletions zds/utils/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand All @@ -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(
Expand All @@ -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")
Expand All @@ -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."""
Expand Down

0 comments on commit c0cef18

Please sign in to comment.