Disentangle Action Text from ApplicationController

This commit allows Action Text to be used without having an
ApplicationController defined.  In doing so, it also fixes Action Text
attachments to render the correct URL host in mailers.

It also avoids allocating an ActionController::Renderer per request.

Fixes #37183.
Fixes #35578.
Fixes #36963.
Closes #38714.

Co-authored-by: Jeremy Daer <jeremydaer@gmail.com>
This commit is contained in:
Jonathan Hefner 2020-09-11 11:43:27 -05:00 committed by Rafael Mendonça França
parent 80f7fd52b2
commit 614e813161
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
20 changed files with 139 additions and 31 deletions

View File

@ -99,6 +99,7 @@ module ActionController
instance.set_response! controller.make_response!(request) instance.set_response! controller.make_response!(request)
instance.render_to_string(*args) instance.render_to_string(*args)
end end
alias_method :render_to_string, :render # :nodoc:
private private
def normalize_keys(defaults, env) def normalize_keys(defaults, env)

View File

@ -10,6 +10,7 @@ module ActionText
mattr_accessor(:scrubber) mattr_accessor(:scrubber)
def render_action_text_content(content) def render_action_text_content(content)
self.prefix_partial_path_with_controller_namespace = false
sanitize_action_text_content(render_action_text_attachments(content)) sanitize_action_text_content(render_action_text_attachments(content))
end end

View File

@ -16,6 +16,7 @@ module ActionText
autoload :Fragment autoload :Fragment
autoload :HtmlConversion autoload :HtmlConversion
autoload :PlainTextConversion autoload :PlainTextConversion
autoload :Rendering
autoload :Serialization autoload :Serialization
autoload :TrixAttachment autoload :TrixAttachment

View File

@ -28,7 +28,7 @@ module ActionText
private private
def trix_attachment_content def trix_attachment_content
if partial_path = attachable.try(:to_trix_content_attachment_partial_path) if partial_path = attachable.try(:to_trix_content_attachment_partial_path)
ActionText::Content.renderer.render(partial: partial_path, object: self, as: model_name.element) ActionText::Content.render(partial: partial_path, object: self, as: model_name.element)
end end
end end
end end

View File

@ -1,12 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
require "active_support/core_ext/module/attribute_accessors_per_thread"
module ActionText module ActionText
class Content class Content
include Serialization include Rendering, Serialization
thread_cattr_accessor :renderer
attr_reader :fragment attr_reader :fragment
@ -88,7 +84,7 @@ module ActionText
end end
def to_rendered_html_with_layout def to_rendered_html_with_layout
renderer.render(partial: "action_text/content/layout", locals: { content: self }) render partial: "action_text/content/layout", locals: { content: self }
end end
def to_s def to_s

View File

@ -37,21 +37,24 @@ module ActionText
end end
initializer "action_text.helper" do initializer "action_text.helper" do
ActiveSupport.on_load(:action_controller_base) do %i[action_controller_base action_mailer].each do |abstract_controller|
ActiveSupport.on_load(abstract_controller) do
helper ActionText::Engine.helpers helper ActionText::Engine.helpers
end end
end end
initializer "action_text.renderer" do |app|
app.executor.to_run { ActionText::Content.renderer = ApplicationController.renderer }
app.executor.to_complete { ActionText::Content.renderer = ApplicationController.renderer }
ActiveSupport.on_load(:action_text_content) do
self.renderer = ApplicationController.renderer
end end
ActiveSupport.on_load(:action_controller_base) do initializer "action_text.renderer" do
before_action { ActionText::Content.renderer = ApplicationController.renderer.new(request.env) } ActiveSupport.on_load(:action_text_content) do
self.renderer = Class.new(ActionController::Base).renderer
end
%i[action_controller_base action_mailer].each do |abstract_controller|
ActiveSupport.on_load(abstract_controller) do
around_action do |controller, action|
ActionText::Content.with_renderer(controller, &action)
end
end
end end
end end

View File

@ -0,0 +1,29 @@
# frozen_string_literal: true
require "active_support/concern"
require "active_support/core_ext/module/attribute_accessors_per_thread"
module ActionText
module Rendering #:nodoc:
extend ActiveSupport::Concern
included do
thread_cattr_accessor :renderer, instance_accessor: false
delegate :render, to: :class
end
class_methods do
def with_renderer(renderer)
previous_renderer = self.renderer
self.renderer = renderer
yield
ensure
self.renderer = previous_renderer
end
def render(*args, &block)
renderer.render_to_string(*args, &block)
end
end
end
end

View File

@ -0,0 +1,5 @@
class Admin::MessagesController < ActionController::Base
def show
@message = Message.find(params[:id])
end
end

View File

@ -1,2 +0,0 @@
class ApplicationController < ActionController::Base
end

View File

@ -1,4 +1,4 @@
class MessagesController < ApplicationController class MessagesController < ActionController::Base
before_action :set_message, only: [:show, :edit, :update, :destroy] before_action :set_message, only: [:show, :edit, :update, :destroy]
# GET /messages # GET /messages

View File

@ -0,0 +1,6 @@
class MessagesMailer < ApplicationMailer
def notification
@message = params[:message]
mail to: params[:recipient], subject: "NEW MESSAGE: #{@message.subject}"
end
end

View File

@ -0,0 +1,16 @@
<dl>
<dt>Subject</dt>
<dd>
<span id="subject"><%= @message.subject %></span>
</dd>
<dt>Content (Plain Text)</dt>
<dd>
<pre id="content-plain"><%= @message.content.to_plain_text %></pre>
</dd>
<dt>Content (HTML)</dt>
<dd>
<div id="content-html"><%= @message.content %></div>
</dd>
</dl>

View File

@ -3,9 +3,6 @@
<head> <head>
<title>Dummy</title> <title>Dummy</title>
<%= csrf_meta_tags %> <%= csrf_meta_tags %>
<%= stylesheet_link_tag 'application', media: 'all' %>
<%= javascript_pack_tag 'application' %>
</head> </head>
<body> <body>

View File

@ -1,13 +1,8 @@
<p id="notice"><%= notice %></p> <p id="notice"><%= notice %></p>
<p> <h1 id="subject"><%= @message.subject %></h1>
<strong>Subject:</strong>
<%= @message.subject %>
</p>
<div class="trix-content"> <div id="content"><%= @message.content %></div>
<%= @message.content.html_safe %>
</div>
<%= link_to 'Edit', edit_message_path(@message) %> | <%= link_to 'Edit', edit_message_path(@message) %> |
<%= link_to 'Back', messages_path %> <%= link_to 'Back', messages_path %>

View File

@ -0,0 +1 @@
<div id="message-content"><%= @message.content %></div>

View File

@ -1,4 +1,7 @@
Rails.application.routes.draw do Rails.application.routes.draw do
resources :messages resources :messages
# For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html
namespace :admin do
resources :messages, only: [:show]
end
end end

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
require "test_helper"
class ActionText::ControllerRenderTest < ActionDispatch::IntegrationTest
test "uses current request environment" do
blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
message = Message.create!(content: ActionText::Content.new.append_attachables(blob))
host! "loocalhoost"
get message_path(message)
assert_select "#content img" do |imgs|
imgs.each { |img| assert_match %r"//loocalhoost/", img["src"] }
end
end
test "resolves partials when controller is namespaced" do
blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
message = Message.create!(content: ActionText::Content.new.append_attachables(blob))
get admin_message_path(message)
assert_select "#content-html .trix-content .attachment--jpg"
end
end

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
require "test_helper"
class ActionText::MailerRenderTest < ActionMailer::TestCase
test "uses default_url_options" do
original_default_url_options = ActionMailer::Base.default_url_options
ActionMailer::Base.default_url_options = { host: "hoost" }
blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
message = Message.new(content: ActionText::Content.new.append_attachables(blob))
MessagesMailer.with(recipient: "test", message: message).notification.deliver_now
assert_select_email do
assert_select "#message-content img" do |imgs|
imgs.each { |img| assert_match %r"//hoost/", img["src"] }
end
end
ensure
ActionMailer::Base.default_url_options = original_default_url_options
end
end

View File

@ -107,6 +107,15 @@ class ActionText::ContentTest < ActiveSupport::TestCase
assert_equal "<blockquote><div>#{attachment_html}</div></blockquote>", content_from_html(html).to_html assert_equal "<blockquote><div>#{attachment_html}</div></blockquote>", content_from_html(html).to_html
end end
test "renders with layout when ApplicationController is not defined" do
html = "<h1>Hello world</h1>"
rendered = content_from_html(html).to_rendered_html_with_layout
assert_includes rendered, html
assert_match %r/\A#{Regexp.escape '<div class="trix-content">'}/, rendered
assert_not defined?(::ApplicationController)
end
private private
def content_from_html(html) def content_from_html(html)
ActionText::Content.new(html).tap do |content| ActionText::Content.new(html).tap do |content|

View File

@ -151,7 +151,7 @@ module ActionView #:nodoc:
# Specify whether rendering within namespaced controllers should prefix # Specify whether rendering within namespaced controllers should prefix
# the partial paths for ActiveModel objects with the namespace. # the partial paths for ActiveModel objects with the namespace.
# (e.g., an Admin::PostsController would render @post using /admin/posts/_post.erb) # (e.g., an Admin::PostsController would render @post using /admin/posts/_post.erb)
cattr_accessor :prefix_partial_path_with_controller_namespace, default: true class_attribute :prefix_partial_path_with_controller_namespace, default: true
# Specify default_formats that can be rendered. # Specify default_formats that can be rendered.
cattr_accessor :default_formats cattr_accessor :default_formats