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

Permet de modifier la casquette à l'édition d'un message #4496

Merged
merged 7 commits into from
Aug 10, 2017
Merged

Permet de modifier la casquette à l'édition d'un message #4496

merged 7 commits into from
Aug 10, 2017

Conversation

gllmc
Copy link
Member

@gllmc gllmc commented Aug 7, 2017

Q R
Type de modification évolution
Ticket(s) (issue(s)) concerné(s) aucun

TODO

  • Les tests

QA

  • Vérifier que l'on peut modifier la casquette lors de l'édition d'un message de forum, d'un sujet, d'un MP ou d'un commentaire d'un contenu.
  • Vérifier que les casquettes disponibles sont celles qu'a l'auteur du message actuellement.
  • Vérifier que lorsqu'on édite comme staff, les casquettes disponibles restent celles de l'auteur du message.
  • Vérifier que restaurer une version depuis l'historique des éditions d'un message entraîne la suppression de la casquette si l'utilisateur ne l'a plus.
  • Code review

@gllmc gllmc added C-Back Concerne le back-end Django C-Front Concerne l'interface du site QA svp labels Aug 7, 2017
@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage decreased (-0.03%) to 88.82% when pulling 286008c on gcodeur:hats-edit into c359b60 on zestedesavoir:dev.

@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage decreased (-0.03%) to 88.82% when pulling e5d1464 on gcodeur:hats-edit into 89bf4f5 on zestedesavoir:dev.

@gllmc gllmc changed the title [WIP] Permet de modifier la casquette à l'édition d'un message Permet de modifier la casquette à l'édition d'un message Aug 9, 2017
@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage decreased (-0.04%) to 88.816% when pulling 9b67247 on gcodeur:hats-edit into 89bf4f5 on zestedesavoir:dev.

Copy link
Member

@pierre-24 pierre-24 left a comment

Choose a reason for hiding this comment

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

Rapport de QA: ok, sauf les commentaires ci-dessous. Également, si on restaure un message, la casquette correspondante n'est pas restaurée (mais je ne sais pas si c'est réellement une feature désirable).

if comment.with_hat:
try:
hat = Hat.objects.get(name=comment.with_hat)
if hat not in comment.author.profile.hats.all():
Copy link
Member

Choose a reason for hiding this comment

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

En toute honneteté, je m'interroge sur l'utilité de ce block if. En effet, je crois que ce qui revenait dans la discussion sur les casquettes (ici, par exemple) était justement qu'elles permetaient de faire comprendre un message à postériori, puisque la casquette y restait même si l'utilisateur n'était plus staff. Là, j'ai l'impression que supprime justement la casquette si l'utilisateur quitte le staff.

if not request.POST.get('hat', None):
return ''
try:
hat = Hat.objects.get(pk=int(request.POST.get('hat')))
assert hat in request.user.profile.hats.all()
if hat not in author.profile.hats.all():
Copy link
Member

Choose a reason for hiding this comment

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

Et là, même chose, non ?

@gllmc
Copy link
Member Author

gllmc commented Aug 9, 2017

@pierre-24: j'avais justement eu une discussion avec @vhf à propos du comportement attendu ici.

En fait, le rôle de la casquette est de certifier que le contenu du message a été écrit par quelqu'un ayant la dite casquette. Du coup, à partir du moment où on édite un message que l'on a posté avec une casquette, on ne peut laisser la casquette au message que si on la possède toujours étant donné que son contenu est modifié.

Pour l'historique des éditions, je n'ai en effet pas enregistré la casquette et j'ai opté pour vérifier si la casquette est toujours possédée par l'utilisateur lors d'une restauration (dans le cas contraire, on la supprime, une restauration étant considérée comme une édition avec l'ancien contenu comme en témoigne la date de dernière édition qui est mise à jour).

Est-ce que cette conception de la chose te paraît plus correcte ?

@pierre-24
Copy link
Member

Ok, j'avais pas vu que c'était uniquement dans le cadre d'éditions. Du coup je viens de retester, QA OK :)

@pierre-24
Copy link
Member

Hop là :)

@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.04%) to 88.854% when pulling 5132960 on gcodeur:hats-edit into 245a825 on zestedesavoir:dev.

@pierre-24 pierre-24 merged commit 7df543d into zestedesavoir:dev Aug 10, 2017
@pierre-24 pierre-24 removed the QA svp label Aug 10, 2017
@pierre-24 pierre-24 added this to the Version de développement milestone Aug 10, 2017
@gllmc gllmc deleted the hats-edit branch August 10, 2017 08:58
sandhose pushed a commit that referenced this pull request Sep 18, 2017
* Permet de modifier la casquette sur le forum

* Permet de modifier la casquette des MP et des commentaires

* Évite les conflits avec l'API

* PEP-8

* Ajout d'un test

* Corrige le test
@A-312 A-312 mentioned this pull request Jan 5, 2019
11 tasks
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 C-Front Concerne l'interface du site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants