zds-site icon indicating copy to clipboard operation
zds-site copied to clipboard

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

Open Rowin opened this issue 4 years ago • 6 comments

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 !

Rowin avatar Mar 02 '21 00:03 Rowin

Coverage Status

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

coveralls avatar Mar 02 '21 00:03 coveralls

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 .

  1. last_message était en cache, donc il faut voir que ça ne pose pas de soucis de performances que de le transformer ainsi.
  2. 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).
  3. 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 avatar Mar 04 '21 00:03 AmauryCarrade

@AmauryCarrade juste pour info, au cas où tu l'aurai manqué, à priori c'est bon pour moi (ou en tout cas, suffisamment pour que tu repasses un coup de QA je pense)

Rowin avatar Apr 08 '21 22:04 Rowin

@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 !

Rowin avatar May 24 '21 22:05 Rowin

@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.

Arnaud-D avatar Jun 16 '22 17:06 Arnaud-D

@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 :)

Situphen avatar Jun 17 '22 20:06 Situphen