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

feat(CommentsField): create #1044

Merged
merged 4 commits into from Jan 16, 2023
Merged

feat(CommentsField): create #1044

merged 4 commits into from Jan 16, 2023

Conversation

J9rem
Copy link
Contributor

@J9rem J9rem commented Dec 21, 2022

cette PR ajoute le champ comments

ce que ça fait:

  • ajouter le champ comments
  • en suivant les spécifications

Spécifications pour l'amélioration de la gestion des commentaires pour l'extension LMS

Auteur: Jérémy Dufraisse JD

Intentions

Ce document a pour objectif de proposer des spécifications pour améliorer les comportements des commentaires au sein de l'extension LMS

Fonctionnement

  • Les propositions et modifications sont faites par Jérémy.
  • Un historique des changements est indiqué en bas de ce document
  • Les commentaires et propositions d'amélioration se font sur le canal Framateam de LMS (en tagant si possible @j3rem)
  • Des commentaires en vrac peuvent être déposés tout en bas de ce PAD : mais ils seront régulièrement nettoyés lors de leur prise en compte

Spécifications

  • [R01]: le paramètre lms_config['use_yeswiki_comments'] du fichier wakka.config.php ne DEVRAIT plus officiellement être pris en compte

  • [R02] : le nouveau paramètre DEVRAIT être lms_config['lms_comments_handler'] dans le fichier wakka.config.php

  • [R02.1]: le paramètre lms_config['lms_comments_handler'] DOIT être une chaîne de caractères parmi les valeurs possibles:

    • ""
    • "embedded_humhub"
    • "external_humhub"
    • "yeswiki"
    • "discourse"
  • [R02.2] : un post handler UpdateHandler__.php NE DEVRAIT PAS extraire automatiquement la valeur du paramètre lms_config['use_yeswiki_comments'] du fichier wakka.config.php

  • [R02.3] : le paramètre lms_config['lms_comments_handler'] DEVRAIT être accessible dans l'action {{editconfig}}

  • [R03] : le handler YesWiki\Lms\IframeHandler__.php DEVRAIT prendre en compte le nouveau paramètre lms_config['lms_comments_handler'] en remplacement de lms_config['use_yeswiki_comments'] selon

    ['lms_comments_handler'] bf_commentaires pour l'activité Action
    external_humhub true ajouter data-external-comments="0" à <head> et ajouter le javascript (*)
    external_humhub false ne rien faire
    embedded_humhub true ne rien faire (laisser le/la dev. ajuster le template fiche-x... dédié)
    embedded_humhub false ne rien faire
    yeswiki peu importe ne rien faire
    discourse peu importe ne rien faire
    "" peu importe ne rien faire

    (*): javascript https://gitlab.com/cuzy/humhub-modules-external-websites ou for-external-website-embedded-in-humhub.js version modifiée du dépôt

  • [R03.1] : il DEVRAIT y avoir un fichier docs/user/en/README.md compatible de la doc embarquée à partir de YesWiki 4.3 et y indiquant pour embedded_humhub le lien vers la doc https://gitlab.com/cuzy/humhub-modules-external-websites/-/tree/master/docs et un exemple de code js à rajouter dans le template fiche-x.tpl.html

  • [R04] : l'ensemble des modifications DEVRAIT considérer que la version de YesWiki est a minima doyphore v4.2.4

  • [R05] : un nouveau champ bazar DEVRAIT être créé pour le choix d'activer ou non les commentaires

  • [R05.1] : ce champ DOIT hériter de SelectField.php

  • [R05.2] : ce champ DOIT avoir un propertyName de type bf_commentaires, configurable par le contructeur de formulaire

  • [R05.3] : ce champ DEVRAIT afficher un message d'information si lms_config['lms_comments_handler'] est parmi external_humhub,embedded_humhub ou discourse pour indiquer que les commentaires ne sont pas affichés par YesWiki

  • [R05.4] : le message d'aide du champ pour le choix d'afficher les commentaires DEVRAIT proposer de l'aide ou un lien vers l'aide pour savoir régler les paramètres

  • [R05.5] : le message d'information PEUT être:

    • en gris et italique
    • external_humhub : Les commentaires sont gérés par la plateforme sociale HumHub (Humhub intégré par YesWiki)
    • external_humhub : Les commentaires sont gérés par la plateforme sociale HumHub (YesWiki intégré par Humhub)
    • discourse : Les commentaires sont gérés par la plateforme discourse : mais ça n'est pas encore fonctionnel !
    • autre : pas de message d'information

    JD : voir [R05.11] pour le message d'avertissement

  • [R05.6] : le champ DOIT être visible dans le constructeur graphique de formulaire

  • [R05.7] : lors de l'enregistrement et de l'affichage de la fiche, le champ DOIT mettre à jour les paramètres d'affichage de commentaires de la fiche dans YesWiki en fonction de

['lms_comments_handler'] bf_commentaires pour l'activité acl comments
external_humhub peu importe comments-closed
embedded_humhub peu importe comments-closed
discourse peu importe comments-closed
yeswiki true suivre le tableau ci-dessous
yeswiki false comments-closed
"" true identique à yeswiki + true
"" false identique à yeswiki + false

Adrien : rester avec un champ commentaire oui / non permet de garder une UX simple pour les formateurs formatrices, en création de fiches activités

valeur par défaut du champ CommentsField droits appliqués si commentaires fermés droits appliqués par CommentsField si ouvert
vide comments-closed +
+ comments-closed +
% comments-closed %
@GroupeName comments-closed @GroupeName
UserName comments-closed UserName
comments-closed comments-closed +

cette valeur DOIT prendre le dessus sur le comportement du champ acls et des droits par défaut

  • ce tableau PEUT être fournit dans docs/user/en/README.md signalé en [R03.1] ou dans la doc du coeur si le champ est dans le coeur
  • [R05.8] : si l'activité possède déjà un réglage pour l'affichage des commentaires, ne pas le changer s'il est différent de comments-closed, vide ou +
  • [R05.9] : le post handler UpdateHander__.php DOIT remplacer le contenu du formulaire activité pour pointer vers ce nouveau champ:
    • modifier le formulaire par défaut Activité pour utiliser le champ CommentsField en remplacement de listeListeOuinonLmsbf_commentaires (exemple :comments***bf_comentaires*** *** ***)
    • mettre à jour le formulaire existant du wiki pour remplacer la ligne listeListeOuinonLmsbf_commentaires si elle est existante en préservant le label (texte affiché) et la valeur par défaut
  • [R05.10] : ce champ PEUT se nommer CommentsField (nom de fichier CommentsField.php)
  • [R05.11] : le nouveau champ bazar DEVRAIT afficher un message d'avertissement si les commentaires ne sont pas activés sur le YesWiki si lms_config['lms_comments_handler'] == "yeswiki" et comments_activated == false
  • [R05.12] : le post handler UpdateHander__.php DOIT remplacer le contenu de toutes les fiches pour renommer la clé listeListeOuinonLmsbf_commentaires en bf_commentaires
  • [R05.13]: le nouveau champ CommentsField PEUT être intégré dans le coeur de YesWiki en faisant attention à adapter le template de rendu de fiche de LMS
  • [R05.14] : le champ CommentsField DOIT être supprimé dès qu'il est disponible dans une version officielle de YesWiki

Historique des changements

  • 2022-10-14 : JD : création
  • 2022-10-21 : JD
    • [R02] : activity_comments => lms_comments_handler
    • [R02.1] : format 'activity_comments' => ['enabled' => true,'external_comments' => true] => chaîne de caractères parmi "","embedded_humhub","external_humhub","yeswiki","discourse"
    • [R02.2] : DEVRAIT extraire le paramètre lms_config['use_yeswiki_comments'] du fichier wakka.config.php et le remplacer par => NE DEVRAIT PAS extraire automatiquement la valeur du paramètre lms_config['use_yeswiki_comments'] du fichier wakka.config.php
    • [R02.3] : activity_comments => lms_comments_handler
    • [R03] : activity_comments => lms_comments_handler et adaptation du tableau et listeListeOuinonLmsbf_commentaires => bf_commentaires
    • [R05.2] : ce champ DOIT avoir un propertyName identique à celui de SelectField.php pour ne pas avoir à remodifier toutes les fichers => ce champ DOIT avoir un propertyName de type bf_commentaires, configurable par le contructeur de formulaire
    • [R05.3] :
      • activity_comments => lms_comments_handler
      • nouvelle condition pour le message d'avertissement si outil de commentaire externes
      • avertissement => information
    • [R05.5]: changement du message d'information
    • [R05.7] : activity_comments => lms_comments_handler + adaptation du tableau et listeListeOuinonLmsbf_commentaires => bf_commentaires
    • [R05.10] : lmsactivitycommentsselect => CommentsField
    • ajout [R05.11] : message d'avertissement si les commentaires ne sont pas activés sur le YesWiki
    • ajout [R05.12] : UpdateHander__.php : listeListeOuinonLmsbf_commentaires => bf_commentaires
    • ajout [R05.13] : champ CommentsField dans le coeur
  • 2022-10-27 JD+Adrien:
    • [R03] : pour embedded_humhub ajouter le javascript (*) => ne rien faire
    • [R03.1] ajout : créer un fichier docs/user/en/README.md
    • [R05.7] : ajout de la précision en faisant attention au comportement du champ acls s'il est présent
    • [R05.9] : ajout de précisions sur la procédure de updateHandler
    • [R05.14] ajout : le champ CommentsField DOIT être supprimé

Commentaires en vrac

  • 2022-10-21 MrFlos:

    field CommentsField qui remplacerait la liste oui/non d'activation des commentaires pour les activités. En mode saisie il permettrait de choisir d'activer les commentaires ou pas en proposant les groupes ayant droit comme pour les acls comments yeswiki classiques (et en sauvant, pour yeswiki: il applique les droits a la fiche, pour humhub : il ferait rien (je croies)), en mode rendu, il pourrait checker les droits acls pour savoir s'il doit afficher les commentaires, et si ok, mettre le js humhub et pour yeswiki laisser le templates twig gérer l'affichage en 2 ou 3 colonnes en fonction des droits d’accès des commentaires.
    CommentField pourrait être une Facade qui implémente CommentFieldHumhub

  • 2022-10-21 Adrien:

    je note dans ce qui m'intéresserait de discuter avec vous :

    • la sélection de groupe mentionnée par flo pour les commentaires
    • les tests spécifiques qui prévoient que le coeur n'est pas à jour
    • [R05.13]: ajouter ? "mettre à jour le rendu classique d'une fiche bazar" ?
  • Adrien (2022-10-21) : pour "embedded-humhub", je ne sais pas ce que tu en penses @mrflos mais ça me paraît compliqué vu qu'il faudrait rajouter 6 paramètres supplémentaires pour pouvoir inclure les bonnes infos dans le code introduit au niveau du template de fiche LMS Activité... et de toute façon j'ai l'impression que l'utilisateur voudra customiser le rendu de la page comme notamment l'emplacement des commentaires... donc est-ce bien utile ?

attention cette PR pointe vers la branche coms-improvement vu que c'est très lié

@mrflos
Copy link
Contributor

mrflos commented Dec 21, 2022

hello, pour ma part je ne suis pas super chaud à créer encore un field pour cela.

  • C'est un champ qu'on ne pourra pas utiliser plusieurs fois dans un formulaire.
  • Le nombre de champs présents dans bazar ne fait que croître ça devient inutilisable ou du moins incompréhensible.
  • ca génère des conflits avec les choix fait pour les acls des fiches

C'est pour cela que j'ai mis bazar dans les priorités de la version yeswiki 4.5, en particulier le fait de convertir la table nature en page avec du json, car cela permettrait de plus facilement mettre des options avancées pour les formulaires et donc par exemple de sortir les acls,les themes des fiches, la création d'un user, les titres automatiques, comme des options apparaissant dans l'espace de saisie/modif du formulaire, et pas comme des champs à ajouter au formulaire, ce qui serait à mon avis bien plus compréhensible, l'éditeur de formulaire servant à gérer que les champs apparaissant vraiment.

Après vous avez peut être un plan pour utiliser cela rapidement, donc au point où on en est, je ne suis pas contre le fait de rajouter ce champs, mais en validant avec vous que ce comportement changera en version 4.5.

@J9rem J9rem changed the base branch from coms-improvement to doryphore-dev December 22, 2022 10:23
@J9rem
Copy link
Contributor Author

J9rem commented Dec 22, 2022

@mrflos je comprends ta position et effectivement l'état actuel de la partie des champs bazar ne permet pas de rendre lisible les ajouts.

L'intérêt de la PR est de proposer un champ bazar pour le module LMS, car il y a son usage, mais de le proposer au coeur car son code est générique (non spécifique à lms)

Je préfère laisser le soin à @acheype de décider entre

  1. intégrer le champ comments dans le coeur pour doryphore 4.4 même s'il n'y a pas de garantie de son maintien en doryphore 4.5
  2. ou intégrer le champ comments dans l'extension lms (une fois la décision prise, il me sera facile de déplacer le code dans la PR LMS feat(commentsfield): use from core yeswiki-extension-lms#50)

mais en validant avec vous que ce comportement changera en version 4.5

Je continue à ne pas vouloir me prononcer pour ce qui est prévu dans 4.5, c'est-à-dire que je ne suis ni en train de valider, ni approuver, ni désapprouver, ni consentir, ...
Je ne fais que des propositions concernant l'état actuel de YesWiki pour la version 4.4

@mrflos
Copy link
Contributor

mrflos commented Dec 22, 2022

Ok @acheype dis nous ce qui a ta faveur!

@acheype
Copy link
Contributor

acheype commented Dec 26, 2022

On a en effet pas mal parlé de ce champ avec @J9rem lors des spécs.

Ce champ CommentsField a son intérêt en terme d'usage pour l'utilisateur : cela lui permet d'avoir simplement une simple case à cocher pour définir s'il veut un commentaire ou non sur une Activité LMS.
Pour le Lms c'est important de garder cette simplicité (c'est le même principe pour activer ou non les réactions sur une fiche), j'imagine mal lui dire de renseigner d'abord sa fiche puis d'aller cliquer en bas pour ouvrir les commentaires et enfin d'aller sélectionner entre Utilisateur connecté, Groupe admin, ... (cette procédure peut être adaptée à un admin qui édite les pages de son wiki, mais c'est trop compliqué pour un simple utilisateur).

Pour le coeur de YesWiki, J'imagine que c'est intéressant aussi de pouvoir laisser un utilisateur renseigner dans la fiche qu'il a saisie, s'il accepte d'avoir des commentaires sur sa fiche. Pour moi, c'est une fonctionnalité qui a sa place dans le coeur.

Après je suis pleinement d'accord avec toi @mrflos, le jour où la table nature est transformée en fiche json, et qu'on pourra facilement ajouter des options aux formulaires, plutôt que des champs, c'est clair qu'on pourra transformer ce champ (ainsi que son homologue pour les réactions) en options. Le code étant déjà là, ça ne devrait pas être trop compliqué.

Bref, je suis pour qu'on intègre CommentsField au cœur.

@acheype
Copy link
Contributor

acheype commented Dec 26, 2022

En regardant de plus près la PR, j'ai remarqué qu'il y avait la valeur par défaut défini pour CommentsField qui va outrepassé les droits des commentaires d' acls... Et c'est bien ce qui est écrit dans les specs...
J'avais plutôt pour idée que cette valeur puisse être récupérée du champ acls (et si non définie, utiliser par défaut +).

@J9rem, est-ce que c'est parce que qu'on ne peut pas accéder aux autres champs depuis un champ ? Si oui, j'imagine que quand on passera à des options de formulaire, on pourra alors se baser sur la valeur d'acls...

@J9rem
Copy link
Contributor Author

J9rem commented Jan 2, 2023

@mrflos @acheype , merci pour le partage de vos réflexions.

A vous lire, je me demande si ça ne serait pas mieux de modifier le champ acls pour qu'il puisse avoir les options actuellement proposé dans commentsfield ?

Avantage:

  • pas de nouveau champ mais usage d'un champ existant
  • le champ acls reste un champ spécial de type "option de formulaire qui ne peut être présent qu'une seule fois"
  • pas de conflit entre acls et comments vu que tout sera dans le même champ

Inconvénient:

  • ajout d'options/paramètres supplémentaires pour le champ acls, concertation à avoir

La rétrocompatibilité pour le champ acls sera possible à 100 %.

Qu'en dites-vous ?

J'attends votre validation à tous les deux avant de coder dans ce sens. Je passe la PR en draft

@J9rem J9rem marked this pull request as draft January 2, 2023 15:20
@mrflos
Copy link
Contributor

mrflos commented Jan 2, 2023

Yes, ca parait un bon compromis si c'est pas trop compliqué à mettre cela dans le champ acls

@acheype
Copy link
Contributor

acheype commented Jan 2, 2023

Pareil, je valide. Très bonne idée !
Peut-être juste rendre optionnelle le fait de pouvoir sélectionner à l'édition de chaque fiche s'il on veut des commentaires...

@J9rem
Copy link
Contributor Author

J9rem commented Jan 2, 2023

euh oui @acheype , j'étais resté flou mais c'était l'intention.

Je vais faire sous 15 jours un commit sur la présente branche, et vous me direz si le format du champ acls vous convient

@J9rem J9rem force-pushed the feat/comments-field branch 2 times, most recently from 5e5fbff to 2ae293b Compare January 4, 2023 10:08
@J9rem J9rem marked this pull request as ready for review January 4, 2023 13:35
@J9rem
Copy link
Contributor Author

J9rem commented Jan 4, 2023

ça y est le champ acls a été modifié afin de ne pas rajouter le champ comments.

Copy link
Contributor

@mrflos mrflos left a comment

Choose a reason for hiding this comment

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

Hello @J9rem merci pour cette proposition, c'est super!
Je propose juste 2 reformulations pour que ce soit plus compréhensible.

@@ -278,6 +278,15 @@
'BAZ_CLOSE_THIS_WINDOW' => 'Fermer cette fen&ecirc;tre',
'BAZ_BOOKMARKLET_LABEL' => 'Créer une fiche %{form}',

// fields/CommentsField.php
'BAZ_ACTIVATE_COMMENTS' => 'Activer les commentaires ?',
Copy link
Contributor

Choose a reason for hiding this comment

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

Peut être préciser 'Activer les commentaires sur cette fiche'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bonne idée, j'ai poussé la modification sur la ranche avant fusion

Copy link
Contributor

Choose a reason for hiding this comment

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

Slt,
Désolé de ne pas avoir réagi plus tôt. Pour celui-ci précisément, je ne vois pas l'intérêt d'écrire « sur cette fiche ». Ca me paraît logique vu qu'on est en train d'éditer la fiche en question.
Et vu qu'on essaie de privilégier quand on peut des labels de champ courts, et que dans le LMS on a déjà l'autre champ « Activer les réactions ? », je suis plus pour remettre « Activer les commentaires ? » qu'uniformiser en proposant « Activer les réactions pour cette fiche ? ».
J'attends votre réponse et si vous êtes d'accord, je peux m'occuper du fix pour éviter que tu ais à remodifer Jerem.
Pour l'autre message par contre, c'était une bonne idée !

Copy link
Contributor

Choose a reason for hiding this comment

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

Oui, je peux vivre avec, même si je trouve que rajouter "sur la fiche" est plus explicite c'est en effet plus long donc ya des avantage/inconvénients.
Ca me va donc de faire marche arrière si vous préférez, juste un petit point de vigilance à ne pas voir les PR du coeur à travers le prisme du lms seulement, il faut que le label soit clair aussi dans un autre contexte que les activités LMS (ce qui ici me semble le cas)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Qu'en résulte-t-il de votre décision ?

  1. remplacer Activer les commentaires pour cette fiche ? par Activer les commentaires ?
  2. ou remplacer Activer les réactions ? par Activer les réactions pour cette fiche ?

Dès que vous avez décidé, j'applique la modification.

Copy link
Contributor

Choose a reason for hiding this comment

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

J'ai déjà fait le commit et le merge tout à l'heure @J9rem ;)

L'important pour moi était surtout d'uniformiser avec les réactions qui sont maintenant également dans le cœur.

Je n'avais plus en tête que les intitulés des champs étaient au dessus (et non sur le côté), donc il n'y finalement pas de soucis à rajouter « sur cette fiche » et vu que ce semble être ta préférence @mrflos, je suis aller dans ce sens en modifiant le message d'édition des réactions (proposition 2 du commentaire ci-dessus).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

toi aussi tu es aussi rapide que l'éclair @acheype , j'avais pas encore vu ton commit

@@ -215,6 +215,14 @@
'BAZ_FORM_EDIT_BOOKMARKLET_TEXT_LABEL' => "Texte affiché",
'BAZ_FORM_EDIT_BOOKMARKLET_TEXT_VALUE' => "Glisser-déposer le bouton dans votre barre de favoris",

'BAZ_ACTIVATE_COMMENTS' => 'Activer les commentaires ?',
'BAZ_ACTIVATE_COMMENTS_HINT' => 'Droits mis à jour lors de l\'enregistrement de la fiche',
'BAZ_FORM_EDIT_COMMENTS_FIELD_DEFAULT_ACTIVATION_LABEL' => 'Activation des commentaires par défaut',
Copy link
Contributor

Choose a reason for hiding this comment

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

Peut être plutôt 'Choix par défaut pour l'activation des commentaires'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bonne idée, j'ai poussé la modification sur la ranche avant fusion

@J9rem J9rem merged commit 3b85874 into doryphore-dev Jan 16, 2023
@J9rem J9rem deleted the feat/comments-field branch January 16, 2023 09:20
@acheype
Copy link
Contributor

acheype commented Jan 17, 2023

@J9rem J'ai trouvé un léger bug :

  • j'édite une fiche, je mets "Oui" à "Activer les commentaires ?"
  • je clique sur "Fermer les commentaires" en bas de la fiche à la consultation
  • lorsque j'ouvre à nouveau la fiche en édition, "Activer les commentaires ?" est à "Oui"

Pourtant j'ai bien l'acl "comments-closed" sur la fiche...

@J9rem
Copy link
Contributor Author

J9rem commented Jan 17, 2023

C'est pas un bug, c'est le comportement qui a été spécifié.

@J9rem
Copy link
Contributor Author

J9rem commented Jan 17, 2023

mais @acheype je reste d'accord avec toi que c'est un comportement bizarre : corrigé par b9937ac

@mrflos
Copy link
Contributor

mrflos commented Jan 17, 2023

merci pour tous ces micro changements / améliorations / échanges, c'est vraiment top!

@acheype
Copy link
Contributor

acheype commented Jan 17, 2023

Merci c'est nickel maintenant !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants