Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supprimer un contenu ne supprime pas les alertes sur ses commentaires #6520

Open
philippemilink opened this issue Jul 25, 2023 · 1 comment
Labels
C-Back Concerne le back-end Django S-BUG Corrige un problème

Comments

@philippemilink
Copy link
Member

Comment reproduire ?

La liste des étapes qui permet de reproduire le bug :

  1. Signaler un commentaire sur un billet (ça devrait être pareil sur un article ou tutoriel, mais je trouve ça plus simple sur un billet)
  2. Se connecter avec le compte de l'auteur du billet
  3. Dépublier le billet
  4. Supprimer le billet
  5. En tant qu'admin, aller sur la page des alertes : une erreur 500 apparaît, parce que le commentaire associé à l'alerte est nul.

Origine du problème

La suppression du billet supprime en cascade les commentaires qui y ont été postés (ces commentaires sont représentés par la classe ContentReaction, qui hérite de Comment). Par contre, lors de la suppression des commentaires, les alertes associées ne sont pas supprimées, l'attribut comment prend alors la valeur None :

comment = models.ForeignKey(
Comment,
verbose_name="Commentaire",
related_name="alerts_on_this_comment",
db_index=True,
null=True,
blank=True,
on_delete=models.SET_NULL,
)

Je ne sais pas pourquoi c'est le cas. Si c'est juste une erreur, il suffit (a priori) d'utiliser models.CASCADE pour corriger le bug.

@philippemilink philippemilink added S-BUG Corrige un problème C-Back Concerne le back-end Django labels Jul 25, 2023
@Situphen
Copy link
Member

En fait lors du passage à Django 2.1 (#5233) où ont été introduit les on_delete, il y a eu pas mal d'hésitations sur quelle valeur donner à ce paramètre pour certains champs. Certains champs ont donc été mis à SET_NULL par mesure de précaution car c'est moins destructeur que CASCADE (qui comme son nom l'indique peut éventuellement tout supprimer en cascade).

Pour ce champ en particulier, je suppose que c'était soit une précaution à l'époque soit une erreur. Peut-être que @artragis se souvient ? En tous cas, je ne vois pas de contre-indication particulière qui empêcherait de mettre CASCADE ici.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django S-BUG Corrige un problème
Projects
Status: À traiter
Development

No branches or pull requests

2 participants