Исправление digest_custom_html для обработки как HTML (ранее: Переопределение digest.html.erb)

Разработка
1

TL;DR:

Я пытаюсь добавить что-то в дайджест. Раньше я переопределял шаблон, но теперь не могу, по непонятным причинам.

Поскольку переопределение шаблонов — это плохо, в некоторых местах дайджеста используется digest_custom_html, который вставляет текст, поэтому к нему нужно добавлять .html_safe. По крайней мере, один из них находится не на своём месте (в названии есть “above”, но на самом деле он ниже).

Думаю, у меня есть навыки, чтобы сделать PR по этому вопросу, если только из-за его простоты кому-то другому не будет проще это сделать.

Более длинная и болезненная история.

Я делал это здесь: GitHub - pfaffman/discourse-add-to-summary: Add text to summary before and after title · GitHub, но, похоже, это перестало работать.

Я думаю, что хочу сделать, не внося изменений в ядро, — переопределить шаблон digest.html.erb. Раньше я мог сделать это, поместив файл в app/views/user_notifications/digest.html.erb, и этот файл обрабатывался вместо того, что в ядре. Теперь это, похоже, не работает.

Теперь у нас есть несколько крутых digest_custom_html, например:

Внимательный читатель заметит, что популярные темы начинаются примерно на 78-й строке, задолго до 277-й. Не уверен, сколько времени я потратил, думая, что ничего не происходит, потому что искал не в том месте. Но я отвлекся.

Мне удалось сделать это в 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
        "<div class='custom-popular-topics'>MY COOOOOOOL TEXT</div>"
      else
        puts "doing the super for #{position_key}"
        super
      end
    end
  end

И действительно, текст добавляется в то место, где, как я ожидал, он должен измениться в шаблоне!

Но, увы, он обрабатывается не как HTML, а как текст. Так что…

Я просмотрел все плагины и не нашёл примеров использования этого кода кем-либо.

Будет ли приветствоваться PR, если я изменю строки типа

        <%= digest_custom_html("below_popular_topics") %>

и заменю их на

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

И, пока я заодно, возможно, стоит убедиться, что имена position_key более соответствуют их расположению?

2

Я создал PR:

3

Я решил, что digest_custom_html, не генерирующий HTML, который можно распарсить, — это ошибка, поэтому я меняю категорию.

4

@martin, извините, что вынужден вас беспокоить, но вы были последним, кто редактировал digest.html.erb (два года назад). Не могли бы вы взглянуть на этот PR?

5

Этот плагин был развернут, и я, скорее всего, очень скоро о нём забуду. Поэтому, если кто-то захочет использовать поля digest_custom_html по назначению, вы можете добавить подобный код в ваш app.yml, чтобы внести исправления в исходный код. Я был слишком ленив, чтобы написать регулярное выражение для замены всех случаев, и просто сделал замену для того варианта, который использовал сам. Измените код в соответствии с вашими потребностями.

Я создал шаблон в плагине, который затем можно включить в app.yml. Это немного упрощает работу по сравнению с возней с целым блоком 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 '
6

Имеет ли смысл добавить тег, чтобы предупредить команду о наличии открытых PR?

7

Привет, @david. Я попробую ещё раз, чтобы узнать, есть ли кто-то, кто заинтересован в этом PR. Как описано выше: код явно не работает как задумано, но никому это никогда не было важно. Я сделал обходной путь и развернул код с исправлением, чтобы исправить шаблон при развёртывании, но это не самое элегантное решение.

8

Не могли бы вы добавить .html_safe к результату переопределения вашего метода? Мне кажется, нет причин помещать это в шаблон ERB?

Общая цель как в Rails, так и в Ember — размещать метку «эта строка безопасна для HTML» как можно ближе к точке создания/генерации, чтобы разработчикам было предельно ясно, что они должны убедиться в безопасности HTML (т. е. что любой ввод пользователя экранирован).

То, что вы делаете, допустимо (при условии, что это протестировано), но это не «предназначенное» использование этих методов. Если бы это был осознанный плагин API, он был бы размещен в plugin/instance.rb вместе с остальными.

Предназначенное использование этого метода — помещать разметку Markdown в соответствующий ключ перевода:

(Обратите также внимание на html_safe там — это та же техника, которую вам нужно использовать при любом переопределении).

9

О боже! Оказывается . . . Я могу!!! Мне бы и в голову не пришло, что я могу разместить это там, а не в самом шаблоне, где (в моей голове, по крайней мере) оно рендерится.

Теперь я вижу и понимаю, что digest_custom вызывает .html_safe, как ни в чём не бывало, в коде, который я смотрел и пытался понять, но (для меня) сбивает с толку использование этого странного синтаксиса без скобок. (Или, возможно, я просто никогда бы его не заметил).

Что ж, я вижу значения в локалях, но по сравнению с перезаписью шаблона, как я делал раньше, это гораздо менее рискованное решение.

Огромное спасибо за то, что указали мне верный путь!

Вот ещё одна вещь, которую я делаю, и это кажется глупым. Моему digest_custom_html нужен пользователь, для которого он рендерится, поэтому я переопределяю digest, чтобы установить @user, чтобы мой digest_custom_html мог получить доступ к пользователю, для которого предназначен дайджест. Есть ли какой-то супер-простой способ избежать этого?

after_initialize do
  # Код, который должен выполняться после завершения загрузки 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
10

Я не вижу ничего другого в шаблоне/методах, что использует переменную user, так что да, то, что вы сделали, вероятно, лучший способ это сделать.

Но всё же, такие переопределения не могут быть «поддерживаемыми» и могут сломаться в любой момент. Убедитесь, что у вас есть тесты, чтобы отловить это, когда это в конце концов произойдёт.

11

Спасибо за подтверждение! Я ценю ваше время.

Понял. Это всё ещё намного лучше, чем переопределять весь шаблон, что было моим предыдущим решением для похожего плагина.

И да, тесты должны быть. :wink: