Correction de digest_custom_html pour qu'il soit traité comme du HTML (était : surcharge de digest.html.erb)

Dev
1

TL;DR :

J’essaie d’ajouter des éléments au résumé. J’avais l’habitude de remplacer le modèle, mais je ne peux plus le faire, pour des raisons obscures.

Parce que remplacer les modèles est mauvais, il y a certains endroits dans le résumé qui sont remplacés par digest_custom_html, mais cela insère du texte, donc ceux-ci ont besoin de .html_safe ajouté. Au moins un est au mauvais endroit (a “above” dans le nom, mais est en réalité en dessous).

Je pense avoir les compétences pour une PR sur ce sujet, sauf que, parce que c’est si simple, il pourrait être plus facile pour quelqu’un d’autre de le faire.

L’histoire plus longue et plus douloureuse.

Je l’ai fait ici : GitHub - pfaffman/discourse-add-to-summary: Add text to summary before and after title mais cela semble ne plus fonctionner.

Je pense que ce que je veux faire, sans modifier le cœur, est de remplacer le modèle digest.html.erb. J’avais l’habitude de pouvoir le faire en le plaçant dans app/views/user_notifications/digest.html.erb et ce fichier serait traité à la place de celui du cœur. Cela ne semble plus fonctionner.

Maintenant, nous avons du code cool digest_custom_html comme ceci :

Le lecteur averti remarquera que les sujets populaires commencent vers la ligne 78, bien avant la ligne 277. Je ne suis pas sûr du temps que j’ai perdu à penser que rien ne se passait parce que je regardais au mauvais endroit. Mais je digresse.

J’ai réussi à faire ceci dans plugin.rb after_initialize.

  require_dependency "user_notifications"
  module ::UserNotificationsHelperOverride
    def digest_custom_html(position_key)
      puts "doing improved the digest: #{position_key}"
      if position_key == "below_popular_topics"
        puts "doing the custom html for above_popular_topics"

        # Custom HTML for the popular topics position
        "\u003cdiv class='custom-popular-topics'\u003eMY COOOOOOOL TEXT\u003c/div\u003e"
      else
        puts "doing the super for #{position_key}"
        super
      end
    end
  end

Et cela ajoute bien du texte à l’endroit où je m’attendrais à ce qu’il soit modifié dans le modèle !

Hélas, ce n’est pas traité comme du HTML, mais comme du texte. Donc. . . .

J’ai regardé dans all_the_plugins et je n’ai trouvé aucun exemple de quelqu’un utilisant ce code.

Serait-ce un PR-welcome si je changeais les lignes comme

        \u003c%= digest_custom_html("below_popular_topics") %\u003e

et les remplacer par

        \u003c%= digest_custom_html("below_popular_topics").html_safe %\u003e

Et, pendant que j’y suis, peut-être m’assurer que les noms des position_key sont plus proches de leur position ?

1 « J'aime »
2

J’ai créé une PR :

2 « J'aime »
3

J’ai décidé que digest_custom_html ne produisant pas de HTML analysable est un bug, je le donc reclassifie.

1 « J'aime »
4

@martin, désolé de te pointer du doigt, mais tu es la dernière personne à avoir touché à digest.html.erb (il y a deux ans). Pourrais-tu jeter un œil à cette PR ?

1 « J'aime »
5

Ce plugin a été déployé et je l’oublierai probablement très bientôt, donc si quelqu’un veut utiliser ces champs digest_custom_html comme prévu, vous pouvez ajouter du code comme celui-ci à votre app.yml pour patcher la source. J’étais trop paresseux pour créer une regex qui les remplace toutes, et j’ai plutôt fait celle que j’utilisais. Modifiez selon vos besoins.

J’ai créé un modèle dans le plugin qui peut ensuite être inclus dans le app.yml. Cela le rend un peu plus facile que de s’embêter avec un bloc entier de yaml.

hooks:
  after_code:
    - replace:
        filename: "/var/www/discourse/app/views/user_notifications/digest.html.erb"
        from: 'digest_custom_html("above_footer") '
        to: 'digest_custom_html("above_footer").html_safe '
1 « J'aime »
6

Est-ce que ce serait pertinent d’avoir une étiquette pour aider à avertir l’équipe d’ouvrir des PR (Pull Requests) ?

1 « J'aime »
7

Salut @david. Je vais essayer une dernière fois pour voir si quelqu’un est intéressé par cette PR. Comme décrit ci-dessus ; le code ne fonctionne clairement pas comme prévu, mais personne ne s’en est jamais soucié. J’ai trouvé une solution de contournement et déployé le code avec une correction pour patcher le template au déploiement, mais ce n’est pas une solution élégante.

1 « J'aime »
8

Pourriez-vous ajouter .html_safe au résultat de votre remplacement de méthode ? Je ne pense pas qu’il y ait de raison qu’il doive être dans le modèle erb ?

L’objectif général, à la fois dans Rails et dans Ember, est de placer la mention « cette chaîne est html safe » aussi près que possible du point d’auteur/génération, afin qu’il soit très clair pour les développeurs qu’ils doivent s’assurer que le HTML est effectivement sûr (c’est-à-dire que toute entrée utilisateur est échappée)

Ce que vous faites est acceptable (tant qu’il est testé), mais ce n’est pas l’utilisation « prévue » pour ces méthodes. S’il s’agissait d’une API de plugin intentionnelle, elle se trouverait dans plugin/instance.rb avec les autres.

L’utilisation prévue de cette méthode est de mettre du markdown dans la clé de traduction correspondante :

(Notez également le html_safe ici - c’est la même technique que vous devriez utiliser dans n’importe quel remplacement)

2 « J'aime »
9

O.M.G. Apparemment . . . Je peux!!! Il ne me serait jamais venu à l’esprit que je pourrais le mettre là et pas tout en haut du modèle où (dans ma tête, du moins) il est rendu.

Je vois et comprends maintenant que digest_custom et qu’il appelle .html_safe, joliment comme il faut, dans un code que j’ai regardé et essayé de comprendre, mais c’est déroutant (pour moi) qu’il utilise cette syntaxe funky sans parenthèses. (Ou peut-être que je ne l’aurais jamais vu).

Eh bien, je vois les valeurs dans les locales, mais comparé à la surcharge du modèle comme je le faisais auparavant, c’est une solution beaucoup moins risquée.

Merci infiniment de m’avoir mis sur la bonne voie !

Voici une autre chose que je fais et qui me semble stupide. Mon digest_custom_html a besoin de l’utilisateur pour lequel il est rendu, donc ce que je fais, c’est de remplacer digest pour définir @user afin que mon digets_custom_html puisse avoir accès à l’utilisateur pour lequel le digest est destiné. Y a-t-il un moyen super simple d’éviter cela ?

after_initialize do
  # Code qui doit s'exécuter après le démarrage complet de Rails

  # require_relative \"lib/discourse_add_jobs_to_digest/user_notifications_helper_override\"
  require_relative \"lib/discourse_add_jobs_to_digest/engine\"
  require_relative \"lib/discourse_add_jobs_to_digest/job_api\"

  require_dependency \"user_notifications\"
  module ::UserNotificationsHelperOverride
    def digest_custom_html(position_key)
      if position_key == \"above_footer\"
        DiscourseAddJobsToDigest::JobApi.get_jobs_html(@user).html_safe
      else
        super
      end
    end
  end

  UserNotificationsHelper.prepend(::UserNotificationsHelperOverride)

  module ::UserNotificationsOverride
    def digest(user, opts = {})
      @user = user
      super
    end
  end
  UserNotifications.prepend(::UserNotificationsOverride)
end
2 « J'aime »
10

Je ne vois rien d’autre dans le template/les méthodes qui utilise une variable user, donc oui, ce que vous avez fait est probablement la meilleure façon de procéder.

Cependant, ce type de substitutions ne peut pas être « pris en charge » et peut casser à tout moment. Assurez-vous d’avoir des tests pour détecter cela lorsqu’il se produira éventuellement.

1 « J'aime »
11

Merci pour la confirmation ! J’apprécie votre temps.

Compris. C’est toujours bien mieux que de remplacer tout le modèle, ce qui était ma solution précédente pour un plugin similaire.

Et oui, il devrait y avoir des tests. :wink:

1 « J'aime »
12

Ce sujet a été automatiquement fermé 30 jours après la dernière réponse. Les nouvelles réponses ne sont plus autorisées.