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

Corrige un bug lié aux demandes de mise en avant #6626

Merged
merged 3 commits into from
Jul 21, 2024

Conversation

Situphen
Copy link
Member

@Situphen Situphen commented Jul 17, 2024

  • Reproduit un bug liée aux demandes de mise en avant
    • Le test échoue sans la correction et réussit avec la correction, donc il reproduit bien le bug du ticket.
  • Corrige un bug lié aux demandes de mise en avant
    • La condition isinstance(q.content_object, Topic) or not q.content_object.is_obsolete exclue les contenus obsolètes de la liste des mises en avant mais elle génère une AttributeError quand q.content_object est nul pour les contenus. On pourrait simplement ajouter la condition q.content_object is not None pour former q.content_object and (isinstance(q.content_object, Topic) or not q.content_object.is_obsolete) ou q.content_object and not (isinstance(q.content_object, PublishableContent) and q.content_object.is_obsolete) mais j'ai opté pour q.content_object and not getattr(q.content_object, "is_obsolete", False) qui est plus concis.
    • J'en ai profité pour remplacer .all() par .iterator() tant qu'à faire.

QA : Github Actions + une reproduction locale du bug si possible

@coveralls
Copy link

coveralls commented Jul 17, 2024

Coverage Status

coverage: 88.873% (+0.02%) from 88.854%
when pulling 7ff0b7f on Situphen:en-avant-guingamp
into ab676f6 on zestedesavoir:dev.

@philippemilink
Copy link
Member

Plutôt que de juste exclure les FeaturedRequesteds qui n'ont plus de content_object existant en base de données, il faudrait qu'au moment de la suppression du content_object cible, ça entraîne en cascade la suppression des éventuels objets FeaturedRequested liés.

@Situphen
Copy link
Member Author

Situphen commented Jul 19, 2024

J'ai modifié les tests pour tester la suppression d'un contenu et d'un sujet du forum. J'ai annulé la modification de la vue qui liste les demandes de mise en avant. J'ai utilisé le signal pre_delete pour supprimer les demandes de mise en avant lors de la suppression d'un contenu ou d'un sujet du forum.

@philippemilink
Copy link
Member

Ça fonctionne, mais je remarque un défaut qui persiste :

  1. proposer la mise en une d'un contenu
  2. en tant que staff, dépublier ce contenu
  3. sur la page des demandes de mises en une, le contenu apparaît toujours, mais le lien vers le contenu est faux (c'est le lien vers la page actuelle)

Je ne sais pas quelle est la meilleure solution à implémenter dans cas :

  • conserver les demandes en une de contenus plus publiés mais toujours existants, peut-être le contenu sera à nouveau publié dans le futur et alors les demandes de mise en une seront toujours pertinentes ? (je viens de tester, si on republie, le lien dans la demande de mise en une redevient correct) Dans ce cas, il faudrait garder le titre mais enlever le lien, en indiquant que ce contenu n'est plus publié
  • supprimer les demandes de mise en une lors de la dépublication d'un contenu ?

@Situphen
Copy link
Member Author

Je ne suis pas sûr non plus de savoir ce qui est le mieux. J'ai l'impression qu'il aurait fallu initialement se baser sur l'objet PublishedContent et non pas PublishableContent car mettre en une un contenu non publié n'a pas vraiment d'intérêt. J'aurais donc tendance à supprimer la demande si le contenu est dépublié, d'autant plus qu'il me parait improbable qu'un contenu soit dépublié pour être de nouveau publié peu de temps après (enfin pour les tutoriels et articles, c'est probablement différent pour les billets). À noter que le même genre de problématique peut se poser avec les sujets du forum qui sont mis à la corbeille ou d'une manière générale dans des forums privés.

J'ai ajouté un commit à moindre coût pour supprimer les demandes de mise en avant lors de la dépublication des contenus. Je ne pense pas qu'il faille aller plus loin que ça, c'est déjà pas mal non ?

Copy link
Member

@philippemilink philippemilink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA OK ✔️

@philippemilink philippemilink merged commit b7e0f4c into zestedesavoir:dev Jul 21, 2024
12 checks passed
@Situphen Situphen deleted the en-avant-guingamp branch July 21, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants