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

Améliore l'historique d'édition sur le forum #5189

Merged
merged 3 commits into from
Nov 21, 2019

Conversation

A-312
Copy link
Contributor

@A-312 A-312 commented Dec 20, 2018

Fix : #5116

image

Peut-être modifier : "Historique des éditions du message" par autres choses ?

Q/A : Vérifier que les boutons/auteurs correspondent au bonne ligne.

@A-312
Copy link
Contributor Author

A-312 commented Dec 20, 2018

NB : Je me suis compliqué la vie.

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 3cca550 on A-312:fix5116 into cfa85a4 on zestedesavoir:dev.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 3cca550 on A-312:fix5116 into cfa85a4 on zestedesavoir:dev.

@coveralls
Copy link

coveralls commented Dec 20, 2018

Coverage Status

Coverage decreased (-6.03%) to 66.211% when pulling 9ac01e2 on A-312:fix5116 into d6bbb78 on zestedesavoir:dev.

@A-312 A-312 force-pushed the fix5116 branch 2 times, most recently from 3a14da0 to 61b916d Compare December 20, 2018 19:05
@A-312
Copy link
Contributor Author

A-312 commented Dec 20, 2018

Je vais finir par faire une PR pour colorer travis. x)

@A-312

This comment has been minimized.

@gllmc
Copy link
Member

gllmc commented Dec 28, 2018

Salut ! Je ne suis pas sûr d'avoir bien compris le but de cette PR. Pour confirmation, s'agit-il de décaler la colonne "version avant édition" d'un cran afin de faire coïncider la date d'une version et son contenu ?

@A-312
Copy link
Contributor Author

A-312 commented Dec 28, 2018

Salut ! Je ne suis pas sûr d'avoir bien compris le but de cette PR. Pour confirmation, s'agit-il de décaler la colonne "version avant édition" d'un cran afin de faire coïncider la date d'une version et son contenu ?

C'est bien ça pour que ça soit plus évident. (Comme tu peux le lire dans l'issue, si tu avais vu l'explication?)

@gllmc
Copy link
Member

gllmc commented Dec 28, 2018

En effet, j'ai oublié de regarder le ticket associé, désolé !

Du coup, ça me semble être une très bonne idée. J'essaye de faire la QA demain.

Copy link
Member

@gllmc gllmc left a comment

Choose a reason for hiding this comment

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

Ça m'a l'air de marcher très bien ! Quelques petites corrections, mais rien de grave.

templates/pages/edit_detail.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
@A-312 A-312 force-pushed the fix5116 branch 3 times, most recently from face015 to f725281 Compare January 10, 2019 15:41
@A-312
Copy link
Contributor Author

A-312 commented Jan 10, 2019

@gcodeur Je n'ai pas su mettre tes changements en local pour git squash tes commits en un seul. :/

Copy link
Member

@gllmc gllmc left a comment

Choose a reason for hiding this comment

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

Encore une chose à corriger et c'est tout bon !

@gcodeur Je n'ai pas su mettre tes changements en local pour git squash tes commits en un seul. :/

Euh, je ne comprends pas vraiment ce que tu veux dire. Lors de ma première revue, j'ai utilisé le système de suggestions de GitHub (qui l'interface m'a proposé, mais que je ne maîtrise pas vraiment 😅 ). Par contre, j'ai l'impression que ta PR est très bien actuellement. Il y a un seul commit avec toutes les modifications suggérées, non ?

templates/pages/edit_detail.html Show resolved Hide resolved
@A-312
Copy link
Contributor Author

A-312 commented Jan 19, 2019

Corrigé

Copy link
Member

@artragis artragis left a comment

Choose a reason for hiding this comment

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

Je fais confiance à gcodeur pour le code général là il ne manque que la doc.

zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
@A-312
Copy link
Contributor Author

A-312 commented Jan 20, 2019

Je dois traduire en anglais je suppose ?

Cette fonction permet d'attribuer l’élément sélectionné, suivant ou précédent de {list} à une variable {ouput_name}.

sibling {list} {dir} [as {ouput_name}]
  • {list} : Variable/Point d'entrée.
  • {dir} : next/prev ou une valeur numérique représentant l'index de l'élément voulu dans {list}.
  • {ouput_name} : Optionnel. S'il n'est pas défini, il prend la valeur de :
    - current si {dir} est numérique ;
    - next ou prev si {dir} vaut next/prev.

Pour trouver la position actuelle dans le tableau et ainsi procéder aux instructions next/prev, le code va vérifier dans l'ordre du code :

  • Si {dir} est numérique, la fonction utilise cette valeur comme position finale ;
  • Si vous avez défini la variable siblingStep avant d'appeler la fonction, cette valeur est utilisée comme position d'entrée avant d’appliquer -1 ou +1 en fonction de la valeur de {dir} (-> next/prev) ;
  • Usage recommandé : Automatiquement dans une boucle for grâce à la variable forloop.counter0 ;
  • Si aucun des deux cas précédent est rempli :
    - avec {dir} == next on obtient le deuxième élément (index = 1) ;
    - avec {dir} == prev on obtient l'avant dernier élément (index = len(list) - 1).

Si l'élément existe, l'élément est renvoyé sinon une liste vide est retourné '{}'.

Ces deux lignes font donc la même chose :

{% sibling edits 0 as current %}
{% sibling edits 0 %}

@A-312
Copy link
Contributor Author

A-312 commented Jan 29, 2019

dans https://github.comA/zestedesavoir/zds-site/tree/dev/doc/source/utils ? Je ne trouve pas le fichier correspondant pour les templatetags.

@artragis
Copy link
Member

@A-312
Copy link
Contributor Author

A-312 commented Apr 19, 2019

@artragis fait

@artragis
Copy link
Member

ok, je check ça tout à l'heure, merci !)

zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
templates/pages/comment_edits_history.html Outdated Show resolved Hide resolved
@A-312
Copy link
Contributor Author

A-312 commented Aug 22, 2019

La PR est plus vieille que ça, enfin je l'espère 😬

Copy link
Contributor

@gustavi gustavi left a comment

Choose a reason for hiding this comment

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

Niveau code c'est très bien mis à part des remarques sur le détail. J'essaye de trouver un peu de temps demain midi pour QA si personne ne s'en ai chargé.

zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
zds/utils/templatetags/sibling.py Outdated Show resolved Hide resolved
@gustavi gustavi requested a review from artragis August 25, 2019 21:14
@artragis
Copy link
Member

QA KO pour un détail : les boutons sont mal alignés
image

@artragis artragis added C-Front Concerne l'interface du site S-Évolution Ajoute de nouvelles fonctionnalités labels Sep 11, 2019
@A-312
Copy link
Contributor Author

A-312 commented Sep 11, 2019

Et :
image

Pour les boutons, il faut voir pour avoir une colonne plus grande et surement enlever le float, s'il y en a un.

@firm1
Copy link
Contributor

firm1 commented Nov 4, 2019

Hello @A-312 on en est ou sur cette PR ? Tu pourrait la terminer et/ou faire un rebase, que je puisse la QA dans la foulée ?

@A-312
Copy link
Contributor Author

A-312 commented Nov 5, 2019

Elle était prête

@A-312
Copy link
Contributor Author

A-312 commented Nov 5, 2019

D'ailleurs elle a déjà été Q/A dans cet état et elle était passé, j'ai rollback/ignoré certain commentaires de code review qui faisait planter le code.

@firm1
Copy link
Contributor

firm1 commented Nov 5, 2019

D'ailleurs elle a déjà été Q/A dans cet état et elle était passé,

Le dernier commentaire de QA disait KO. Et je ne vois pas d'autre commentaire qui dise que la QA est passée.

@A-312
Copy link
Contributor Author

A-312 commented Nov 5, 2019

Je l'ai remis à un état antérieur

@firm1
Copy link
Contributor

firm1 commented Nov 6, 2019

Rapport de QA

Le code fonctionne, mais je suis un peu embêté par en relisant ton code, tu t'es un peu compliqué la vie.

Je t'ai fais une PR avec mes suggestions et ça évite de créer un tag.

@firm1
Copy link
Contributor

firm1 commented Nov 9, 2019

Du coup @A-312 tu as eu le temps de jeter un coup d'oeil pour merger ma PR sur ta branche ?

optimisation du template
@firm1
Copy link
Contributor

firm1 commented Nov 12, 2019

Puisque je suis intervenu sur cette PR, je ne peux plus la QA :(

@artragis
Copy link
Member

Ok pour moi

@artragis artragis merged commit 5622518 into zestedesavoir:dev Nov 21, 2019
@A-312 A-312 deleted the fix5116 branch April 12, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Front Concerne l'interface du site S-Évolution Ajoute de nouvelles fonctionnalités
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Améliorer l'historique d'édition sur le forum
6 participants