Correzione di digest_custom_html per trattarlo come HTML (prima: Sovrascrittura di digest.html.erb)

Sviluppatore
1

TL;DR:

Sto cercando di aggiungere alcune cose al digest. Ero solito sovrascrivere il template, ma ora non posso farlo, per ragioni non chiare.

Poiché sovrascrivere i template è sbagliato, ci sono alcuni punti nel digest che vengono sostituiti con digest_custom_html, ma inserisce il testo, quindi questi necessitano dell’aggiunta di .html_safe. Almeno uno è nel posto sbagliato (ha “above” nel nome, ma in realtà è sotto).

Penso di avere le competenze per un PR su questo, a meno che, essendo così semplice, non sia più facile che lo faccia qualcun altro.

La storia più lunga e dolorosa.

L’ho fatto qui: GitHub - pfaffman/discourse-add-to-summary: Add text to summary before and after title ma sembra che non funzioni più.

Penso che quello che voglio fare, senza modifiche al core, sia sovrascrivere il template digest.html.erb. Ero solito poterlo fare mettendo il file in app/views/user_notifications/digest.html.erb e quel file veniva elaborato invece di quello nel core. Ora sembra non funzionare più.

Ora, abbiamo del fantastico digest_custom_html come questo:

Il lettore attento noterà che gli argomenti popolari iniziano intorno alla riga 78, molto prima della riga 277. Non sono sicuro di quanto tempo ho perso pensando che non stesse succedendo nulla perché stavo guardando nel posto sbagliato. Ma divago.

Sono riuscito a farlo in plugin.rb dopo_initialize.

  require_dependency "user_notifications"
  module ::UserNotificationsHelperOverride
    def digest_custom_html(position_key)
      puts "sto facendo un miglioramento del digest: #{position_key}"
      if position_key == "below_popular_topics"
        puts "sto facendo l'html personalizzato per la posizione above_popular_topics"

        # HTML personalizzato per la posizione degli argomenti popolari
        "<div class='custom-popular-topics'>IL MIO TESTO FANTASTICO</div>"
      else
        puts "sto facendo il super per #{position_key}"
        super
      end
    end
  end

Ed effettivamente aggiunge testo nel punto in cui mi aspetterei che venisse modificato nel template!

Ahimè, non viene trattato come HTML, ma come testo. Quindi. . . .

Ho guardato in all_the_plugins e non ho trovato esempi di nessuno che usasse questo codice.

Sarebbe un PR-welcome se cambiassi le righe come

        <%= digest_custom_html("below_popular_topics") %>

e le sostituissi con

        <%= digest_custom_html("below_popular_topics").html_safe %>

E, già che ci sono, forse assicurarmi che i nomi delle position_key siano più simili alla loro posizione?

1 Mi Piace
2

Ho creato una PR:

2 Mi Piace
3

Ho deciso che digest_custom_html non produce HTML analizzabile è un bug, quindi sto ricategorizzando.

1 Mi Piace
4

@martin, scusa se ti chiamo in causa, ma sei stata l’ultima persona a toccare digest.html.erb (due anni fa). Ti dispiacerebbe dare un’occhiata a questa PR?

1 Mi Piace
5

Questo plugin è stato distribuito e probabilmente me ne dimenticherò molto presto, quindi se qualcuno vuole usare questi campi digest_custom_html come previsto, può aggiungere codice come questo al proprio app.yml per correggere il codice sorgente. Ero troppo pigro per creare un’espressione regolare che li sostituisse tutti, e invece ho fatto solo quello che stavo usando. Modifica secondo le tue esigenze.

Ho creato un template nel plugin che può poi essere incluso in app.yml. Questo lo rende un po’ più facile che armeggiare con un intero blocco di 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 Mi Piace
6

avrebbe senso avere un tag per aiutare ad avvisare il team di aprire PR?

1 Mi Piace
7

Ciao @david. Ci riproverò ancora una volta per vedere se qualcuno è interessato a questa PR. Come descritto sopra; il codice chiaramente non funziona come previsto, ma a nessuno è mai importato. Ho trovato una soluzione alternativa e ho distribuito il codice con una correzione per correggere il template al momento della distribuzione, ma non è una soluzione elegante.

1 Mi Piace
8

Potresti aggiungere .html_safe al risultato della tua sovrascrittura del metodo? Non penso ci sia motivo per cui debba trovarsi nel template erb?

L’obiettivo generale, sia in Rails che in Ember, è posizionare la dicitura “questa stringa è html safe” il più vicino possibile al punto di creazione/generazione, in modo che sia chiarissimo agli sviluppatori che devono assicurarsi che l’HTML sia effettivamente sicuro (cioè, qualsiasi input dell’utente sia escapato).

Quello che stai facendo va bene (finché è testato), ma non è l’uso “previsto” per questi metodi. Se fosse un’API di plugin intenzionale, si troverebbe in plugin/instance.rb con gli altri.

L’uso previsto per questo metodo è inserire markdown nella chiave di traduzione corrispondente:

(Nota anche l’html_safe lì - quella è la stessa tecnica che dovresti usare in qualsiasi sovrascrittura)

2 Mi Piace
9

O.M.G. Apparentemente . . . posso!!! Non mi sarebbe mai venuto in mente che potessi metterlo lì e non tutto in alto nel template dove (almeno nella mia testa) viene renderizzato.

Ora vedo e capisco che digest_custom e che sta chiamando .html_safe, in modo pulito e ordinato, in codice che ho guardato e cercato di capire, ma è confusionario (per me) che stia usando quella sintassi senza parentesi. (O forse non l’avrei mai visto).

Beh, vedo i valori nelle localizzazioni, ma rispetto alla sovrascrittura del template come stavo facendo prima, questa è una soluzione molto meno rischiosa.

Grazie mille per avermi messo sulla strada giusta!

Ecco un’altra cosa che sto facendo che mi sembra sciocca. Il mio digest_custom_html ha bisogno dell’utente per cui viene renderizzato, quindi quello che sto facendo è sovrascrivere digest per impostare @user in modo che il mio digets_custom_html possa avere accesso all’utente per cui è il digest. C’è un modo super facile per evitare di farlo?

after_initialize do
  # Codice che dovrebbe essere eseguito dopo che Rails ha finito di avviarsi

  # 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 Mi Piace
10

Non vedo altro nel template/metodi che utilizzi una variabile user, quindi sì, quello che hai fatto è probabilmente il modo migliore per farlo.

Tuttavia, questo tipo di override non può essere “supportato” e potrebbe interrompersi in qualsiasi momento. Assicurati di avere alcuni test per catturare ciò quando alla fine accadrà.

1 Mi Piace
11

Grazie per la conferma! Apprezzo il tuo tempo.

Capito. Questo è comunque molto meglio che sovrascrivere l’intero template, che era la mia soluzione precedente per un plugin simile.

E, sì, dovrebbero esserci dei test. :wink:

1 Mi Piace
12

Questo argomento è stato chiuso automaticamente 30 giorni dopo l’ultima risposta. Non sono più ammesse nuove risposte.