Le forum évite de remonter les sujets dont le dernier message est masqué #6056

Rowin · 2021-03-02T00:34:22Z

Les fonctions get_last_post et get_last_message tiennent désormais compte de la visibilité des messages pour ne renvoyer que le dernier message visible.
En particulier :

forum.models.Topic.get_last_post ne renvoie plus l'attribute last_message mais effectue une requête pour retourner le dernier post visible
forum.models.Forum.get_last_message tient compte de la visibilité dans la requête qui était déjà effectuée.

Numéro du ticket concerné (optionnel) : #5980

Contrôle qualité

Se connecter comme staff
Masquer le dernier message d'un topic
Sur la vue du forum, le dernier message du topic doit être le message précédent celui qui vient d'être masqué
Démasquer le message masqué précédemment
Sur la vue du forum, le dernier message du topic doit à nouveau être le message démasqué

C'est ma première PR ici (et presque ma première de manière générale), n'hésitez pas à me dire si des choses ne vont pas !

get_last_post et get_last_message tiennent compte des postes masqués

coveralls · 2021-03-02T00:46:40Z

Coverage increased (+0.01%) to 86.712% when pulling 046f4bf on Rowin:feature-lastposthidden into 113eee1 on zestedesavoir:dev.

AmauryCarrade · 2021-03-04T00:23:33Z

Salut, et bienvenue sur le projet ! Merci pour cette contribution qui sera fort utile pour l'ergonomie du forum :) .

Je n'ai pas encore pris le temps de faire une revue extensive, mais je note quelques points auxquels il faudra faire attention. Peut-être que certains sont déjà OK, et alors… tant mieux :p .

last_message était en cache, donc il faut voir que ça ne pose pas de soucis de performances que de le transformer ainsi.
Il faudra vérifier que l'anti-flood (qui accède au dernier message) fonctionne toujours correctement lorsque le dernier message de l'auteur est masqué (pas sûr qu'on ait un test pour ça).
Enfin, mais là encore je n'ai pas encore eu le temps de QA, il serait bien de trier les messages par dernier message visible sur la liste des sujets, comme ça les sujets avec un dernier message masqué ne remontent pas non plus dans la liste elle-même.

Si jamais il y a des soucis avec la redéfinition de last_message(), le plus simple est probablement de faire une propriété last_visible_message() et de l'utiliser là où c'est pertinent (typiquement dans le gabari de la liste des sujets et des forums).

P.-S. Si jamais ça peut t'intéresser, on discute aussi du projet de façon plus directe et informelle sur le serveur Discord non-officiel de ZdS.

AmauryCarrade

Bon, du coup j'ai QA. ❌ NOK, mais c'est un bon début !

Ce qui fonctionne :

tout ce qui est indiqué dans la QA : le bon message est bien affiché sans souci.

Ce qui ne fonctionne pas :

l'anti-flood est effectivement cassé : ici, on ne devrait pas pouvoir poster ;
l'ordre des messages du forum : il serait mieux, je pense, de garder l'ordre chronologique en ignorant les messages masqués.

Faire comme suggéré au précédent commentaire fonctionnerait, je pense, avec une méthode dédiée pour récupérer le dernier message non-masqué, utilisée dans les gabarits des listes de sujets et de forum (car dans la liste des forums, il faudrait aussi afficher le dernier sujet en prenant en compte la date du dernier message non-masqué).

On pourrait envisager de mettre ce message là (non-masqué) en cache également, tout comme le dernier message tout court, ou même à la place, éventuellement, s'il n'y a pas d'usage critique du dernier message tout court.

Ça impliquerait une migration qui enregistrerait le-dit dernier message — n'hésite pas si tu veux t'y coller, on peut aider sans souci si tu débutes avec Django (j'ignore ton niveau exact) :) .

Il faudrait également modifier les vues des listes de sujets afin de trier sur la date du dernier message visible, histoire d'éviter d'avoir des ordres chronologiques étranges et de bien répondre à la demande de #5980.

Enfin, vu que ça ne semble pas te faire peur, ajouter un test pour ce cas limite de l'anti-flood (s'assurer que l'envoi est bien bloqué même si le dernier message posté est masqué) serait pertinent, je pense.

Merci encore pour tout ça !

…plus le titre

…asqués

* Fix 6047 Nouvelle url plus simple pour le profil utilisateur Redirection de l'ancienne url * Ajout des tests * Mise à jour du test vérifiant l'url du profil * Enlever une ligne (commentée) de l'ancien code Co-authored-by: Situphen <[email protected]> * Changement du template pour afficher la bonne url * Changement de l'URL du profile inscrite en dur Co-authored-by: Situphen <[email protected]>

Ajout d'une propriété "update" pour le Topic

Situphen

C'est du très beau travail @Rowin !

Ceci dit étant donné que l'on touche ici aux messages des forums il ne faut vraiment pas faire d'erreurs c'est pourquoi la phase de QA prend du temps ! :)

zds/forum/models.py

zds/utils/templatetags/topics_sort.py

zds/forum/models.py

Utilisation de Post.update et Post.pubdate à la place de Post.update_index_date

…to feature-lastposthidden

AmauryCarrade

✔️ QA OK

Désolé pour le délai, j'ai passé un petit temps sur d'autres choses. Mais j'ai relu le code et testé le comportement et cette fois tout me semble bon : l'ordre est OK, et l'antispam fonctionne correctement. Merci pour cette contribution !

Je ne fusionne pas tout de suite car @Situphen désirait revoir le code lui aussi, mais pour moi c'est ok.

Situphen

Je me rends compte que cette PR n'est vraiment pas triviale, il y a encore quelques changements à effectuer !

Actuellement tu tries l'ordre des sujets dans le gabarit avec le templatetag topics_sort mais ça ne peut pas fonctionner correctement car la liste des sujets est paginée.

Prenons le cas de cinq sujets :

Sujet A dont le dernier message est il y a 10 heures ;
Sujet B dont le dernier message est il y a 11 heures ;
Sujet C dont le dernier message est il y a 12 heures ;
Sujet D dont le dernier message est il y a 13 heures ;
Sujet E dont le dernier message est il y a 14 heures.

Si la pagination est configurée pour n'afficher que 3 sujets par page, les sujets affichés sur la première page sont dans l'ordre Sujet E, Sujet D et Sujet C.

Si le dernier message de Sujet B est édité, les sujets affichés sur la première page sont dans l'ordre Sujet B, Sujet E et Sujet D.

Si ensuite le dernier message de Sujet est masqué, on s'attend à ce que les sujets affichés sur la première page redeviennent dans l'ordre Sujet E, Sujet D et Sujet C. Or ce n'est pas le cas actuellement avec ta PR, les sujets affichés sur la première page sont Sujet E, Sujet D, Sujet B.

Étant donné que la page est paginée, la liste topics dans templates/forum/category/forum.html ne contient que dans l'ordre Sujet B, Sujet E et Sujet D, ce que fait que topics|topics_sort contient dans l'ordre Sujet E, Sujet D, Sujet B.

Si tu veux reproduire cela chez toi, il faut modifier topics_per_page dans zds/abstract_base/zds.py pour modifier le nombre de sujets affichés par page.

Il faudrait donc que tu fasses ce tri avant la pagination. Les requêtes sont :

pour la liste des sujets épinglés

zds-site/zds/forum/views.py

Line 124 in 415038c

Topic.objects.get_all_topics_of_a_forum(self.object.pk, is_sticky=True), context["filter"]
pour le reste des sujets

zds-site/zds/forum/views.py

Line 158 in 415038c

self.queryset = Topic.objects.get_all_topics_of_a_forum(self.object.pk)

Le mieux est donc probablement que tu modifies ceci :

zds-site/zds/forum/managers.py

Lines 93 to 100 in 415038c

    
           def get_all_topics_of_a_forum(self, forum_pk, is_sticky=False): 
        
               return ( 
        
                   self.filter(forum__pk=forum_pk, is_sticky=is_sticky) 
        
                   .order_by("-last_message__pubdate") 
        
                   .select_related("author__profile") 
        
                   .prefetch_related("last_message", "tags") 
        
                   .all() 
        
               )

Il faudra voir si les changements posent des soucis de performance ou pas (notamment regarder si le nombre de requêtes SQL augmente ou pas).

zds/forum/models.py

…nt paginés

Lorsque le dernier message visible était modifié, il était remonté. Ce n'est pas le comportement attendu. Correction de ce comportement.

Rowin · 2021-05-24T22:41:16Z

@Situphen je pense avoir trouvé une solution propre pour gérer ce cas, directement dans le manager et sans rajouter de requêtes en rajoutant une annotation qui contient la date. Ça m'évite d'avoir à gérer une persistance ou a rajouter des requêtes supplémentaires !

Arnaud-D · 2022-06-16T17:18:36Z

@Situphen tu auras le temps de repasser ici ? C'était toi qui avait fait la QA à l'époque et j'ai l'impression que c'est presque bon. Sauf peut-être l'histoire de performance, je ne sais pas si c'est facile à QAiser.

Situphen · 2022-06-17T20:06:56Z

@Situphen tu auras le temps de repasser ici ? C'était toi qui avait fait la QA à l'époque et j'ai l'impression que c'est presque bon. Sauf peut-être l'histoire de performance, je ne sais pas si c'est facile à QAiser.

Je t'avoue qu'en ce moment le développement technique de ZDS ne m'intéresse pas trop, donc il est peu probable que je finisse la QA de cette PR. Libre à toi ou quelqu'un d'autre d'en faire la QA ! Je ne met aucun veto à ce qu'elle soit fusionné si quelqu'un juge que c'est bon :)

philippemilink

Deux petites questions sur le code, et il faudrait régler le conflit sur le fichier zds/utils/tests/test_misc.py.

Une fois que ces modifications seront faites, je testerai, ça me semble bien.

PS : @Rowin puisque cette PR est ouverte depuis longtemps, dis-le si tu n'as pas le temps / l'envie / la motivation / ... de faire les modifications demandées, pour qu'on sache de notre côté comment considérer cette PR :)

philippemilink · 2023-04-16T16:13:42Z

templates/forum/category/forum.html

@@ -86,7 +87,7 @@

    {% if topics %}
        <div class="topic-list navigable-list" itemscope itemtype="http://schema.org/ItemList">
-            {% for topic in topics %}
+            {% for topic in topics|topics_sort %}


Puisque le tri est fait dans get_all_topics_of_a_forum(), on n'a plus besoin de trier ici, non ?

philippemilink · 2023-04-16T16:14:58Z

zds/forum/models.py

+        last_visible_post = self.get_last_visible_post()
+        return last_visible_post.pubdate


self.get_last_visible_post() peut retourner None, dans ce cas, on ne peut pas accéder à l'attribut pubdate et ça va générer une erreur.

Je ne met aucun veto à ce qu'elle soit fusionné si quelqu'un juge que c'est bon :)

Rowin added 2 commits March 2, 2021 01:12

Fix zestedesavoir#5980

8ea40c1

get_last_post et get_last_message tiennent compte des postes masqués

Ajout des tests

3a8256e

AmauryCarrade requested changes Mar 4, 2021

View reviewed changes

AmauryCarrade changed the title ~~Feature lastposthidden~~ Le forum évite de remonter les sujets dont le dernier message est masqué Mar 4, 2021

AmauryCarrade linked an issue Mar 4, 2021 that may be closed by this pull request

Éviter de remonter les sujets du forum dont le dernier message est un spam #5980

Open

entwanne and others added 19 commits March 7, 2021 17:57

Ajout d'un filtre par tag sur les flux RSS/Atom des contenus

55d9662

fixup! Ajout d'un filtre par tag sur les flux RSS/Atom des contenus

39cdb47

fixup! Ajout d'un filtre par tag sur les flux RSS/Atom des contenus

2f2b880

Strip des valeurs des filtres category/subcategory sur les flux RSS/Atom

ea8302e

Relaie les paramTres GET de la page courante aux flux RSS

469ad1a

Correction de l'affichage des membres sur les pages des casquettes

bc02384

Survoler le sous-titre d'une cartouche de sujet du forum ne souligne …

7e7a914

…plus le titre

Rend moins visible un paragraphe

1fd1fd5

On cache le bouton Modifier dans les MPs si quelqu'un a répondu après

3cebb75

Un membre ne peut pas modifier son message s'il est masqué

41e56d0

Cache les boutons Spam potentiel et Réponse utile pour les messages m…

8cb1a6a

…asqués

Fix hauteur body et z-index header

fde6149

Passage à zmarkdown 9.1.4 pour corriger un bug

03e2227

Création d'une méthode last_visible_post

4ed1610

Ajout d'une propriété "update" pour le Topic

Créé un filtre pour trier les topics par date

434e34f

Ajout d'un test de l'anti-spam

bde3301

Ajout d'un test du filtre de tri des topics

59001b6

Merge branch 'dev' into feature-lastposthidden

b20aa9a

AmauryCarrade self-requested a review March 8, 2021 10:31

Situphen self-requested a review April 15, 2021 18:17

Merge branch 'dev' into feature-lastposthidden

8457537

Situphen requested changes Apr 16, 2021

View reviewed changes

zds/forum/models.py Outdated Show resolved Hide resolved

zds/utils/templatetags/topics_sort.py Outdated Show resolved Hide resolved

zds/forum/models.py Outdated Show resolved Hide resolved

Rowin added 4 commits April 27, 2021 22:49

Changement de update pour last_update

1b45c77

Utilisation de Post.update et Post.pubdate à la place de Post.update_index_date

Merge branch 'feature-lastposthidden' of github.com:Rowin/zds-site in…

2dda367

…to feature-lastposthidden

Merge branch 'dev' into feature-lastposthidden

794f1db

Linting

f7d681f

Situphen self-requested a review April 27, 2021 21:06

Rowin and others added 2 commits May 3, 2021 18:42

Merge branch 'dev' into feature-lastposthidden

ff3aea2

Merge branch 'dev' into feature-lastposthidden

267ec15

AmauryCarrade previously approved these changes May 18, 2021

View reviewed changes

Situphen previously requested changes May 23, 2021

View reviewed changes

zds/forum/models.py Outdated Show resolved Hide resolved

Rowin added 2 commits May 25, 2021 00:28

Corrige un problème d'ordre des derniers sujets lorsque les sujets so…

9a9d5b1

…nt paginés

Corrige un problème d'ordre des sujets lorsqu'un message a été modifié

eaa0b01

Lorsque le dernier message visible était modifié, il était remonté. Ce n'est pas le comportement attendu. Correction de ce comportement.

Rowin dismissed AmauryCarrade’s stale review via eaa0b01 May 24, 2021 22:38

Merge branch 'dev' into feature-lastposthidden

814ac8d

Rowin requested a review from Situphen June 11, 2021 12:37

Merge branch 'dev' into feature-lastposthidden

046f4bf

philippemilink requested changes Apr 16, 2023

View reviewed changes

Situphen removed their request for review April 16, 2023 16:46

philippemilink added the S-Zombie Ticket ou PR oubliée label May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Le forum évite de remonter les sujets dont le dernier message est masqué #6056

Le forum évite de remonter les sujets dont le dernier message est masqué #6056

Rowin commented Mar 2, 2021

coveralls commented Mar 2, 2021 •

edited

Loading

AmauryCarrade commented Mar 4, 2021

AmauryCarrade left a comment •

edited

Loading

Situphen left a comment

AmauryCarrade left a comment

Situphen left a comment •

edited

Loading

Rowin commented May 24, 2021

Arnaud-D commented Jun 16, 2022

Situphen commented Jun 17, 2022

philippemilink left a comment

philippemilink Apr 16, 2023

philippemilink Apr 16, 2023

	def get_all_topics_of_a_forum(self, forum_pk, is_sticky=False):
	return (
	self.filter(forum__pk=forum_pk, is_sticky=is_sticky)
	.order_by("-last_message__pubdate")
	.select_related("author__profile")
	.prefetch_related("last_message", "tags")
	.all()
	)

		last_visible_post = self.get_last_visible_post()
		return last_visible_post.pubdate

Le forum évite de remonter les sujets dont le dernier message est masqué #6056

Are you sure you want to change the base?

Le forum évite de remonter les sujets dont le dernier message est masqué #6056

Conversation

Rowin commented Mar 2, 2021

Contrôle qualité

coveralls commented Mar 2, 2021 • edited Loading

AmauryCarrade commented Mar 4, 2021

AmauryCarrade left a comment • edited Loading

Choose a reason for hiding this comment

Situphen left a comment

Choose a reason for hiding this comment

AmauryCarrade left a comment

Choose a reason for hiding this comment

Situphen left a comment • edited Loading

Choose a reason for hiding this comment

Rowin commented May 24, 2021

Arnaud-D commented Jun 16, 2022

Situphen commented Jun 17, 2022

philippemilink left a comment

Choose a reason for hiding this comment

philippemilink Apr 16, 2023

Choose a reason for hiding this comment

philippemilink Apr 16, 2023

Choose a reason for hiding this comment

coveralls commented Mar 2, 2021 •

edited

Loading

AmauryCarrade left a comment •

edited

Loading

Situphen left a comment •

edited

Loading