## Summary
This PR bumps RuboCop Performance to 1.16.0 and suppresses the following new offenses:
```console
% bundle exec rubocop
(snip)
Offenses:
actionpack/lib/action_dispatch/routing/mapper.rb:309:16:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /#/.match?(to)
^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/routing/mapper.rb:1643:18:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /#/.match?(to)
^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/routing/route_set.rb:887:67:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
path = Journey::Router::Utils.normalize_path(path) unless %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/assertions/routing.rb:86:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(expected_path)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/assertions/routing.rb:205:14:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/integration.rb:235:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actiontext/bin/webpack:18:6:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 150))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
actiontext/bin/webpack-dev-server:18:6:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 150))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb:120:64:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
elsif column.type == :uuid && value.is_a?(String) && /\(\)/.match?(value)
^^^^^^^^^^^^^^^^^^^^
railties/lib/rails/commands/secrets/secrets_command.rb:28:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /secrets\.yml\.enc/.match?(error.message)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3088 files inspected, 10 offenses detected, 10 offenses autocorrectable
```
## Additional Information
This behavior change is based on:
https://github.com/rubocop/rubocop-performance/pull/332
* Logging to a file doesn't make sense in production
You're going to run out of space, and it doesn't play well with containers. Either you log to STDOUT, and let your container setup aggregate the logs, or you'll be switching to syslogger or whatever. You won't be logging to a file in production any more.
* Remove from Dockerfile too
* Did not mean to change this default
But we should make it easy to see how to change it.
* Restore what we had
There is a new [Trix.js][] release, bumping the version from `1.3.1` to
`2.0.4`.
Applications that consume Action Text through the `@rails/actiontext`
npm package can manage their own Trix dependency version.
Similarly, Importmaps users can `pin "trix", to: "..."` to a CDN serving
`2.0.4`.
For the sake of applications that depend on Trix through the
`action_text` gem, this commit updates the pre-bundled JS and CSS code.
These changes were generated by executing the following:
```sh
cd actiontext
yarn build
cp ../node_modules/trix/dist/trix.css app/assets/stylesheets/trix.css
cp ../node_modules/trix/dist/trix.esm.js app/assets/javascripts/trix.js
```
[Trix.js]: https://trix-editor.org
[v2.0.4]: https://github.com/basecamp/trix/releases/tag/v2.0.4
of webpacker.
Include the js and css in the application.html.erb which now shows the
rich text editor on the page. This allows the integration tests to run.
Fixes https://github.com/rails/rails/issues/46804
Running `bin/rails action_text:install` appends `import "trix"` to the `application.js` file without inserting a newline.
In my app this resulted in the following:
```js
// import statements....
import Rails from "@rails/ujs"
Rails.start()import "trix"
import "@rails/actiontext"
```
I assume this bug occurred because my `application.js` was listed at some point and removed an extra line, which was not expected by this generator.
I recommend prepending the string to be inserted with a `\n` just to be safe.
This represents a +2x performance optimization when the replacement
logic is based on some condition, and it returns the same unchanged
node when it wants to skip it:
```ruby
html = <<~HTML
<div>
#{'<p>ignore me</p>' * 1000}
#{'<p>replace me</p>' * 1000}
</div>
HTML
content = content_from_html(html)
replacement_example = -> do
content.fragment.replace("p") do |node|
if node.text =~ /replace me/
"<p>replace me</p>"
else
node
end
end
end
current_implementation = -> do
class ActionText::Fragment
def replace(selector)
update do |source|
source.css(selector).each do |node|
replacement_node = yield(node)
node.replace(replacement_node.to_s)
end
end
end
end
replacement_example.call
end
new_implementation = -> do
class ActionText::Fragment
def replace(selector)
update do |source|
source.css(selector).each do |node|
replacement_node = yield(node)
node.replace(replacement_node.to_s) if node != replacement_node
end
end
end
end
replacement_example.call
end
Benchmark.ips do |x|
x.report "Current implementation", ¤t_implementation
x.report "New implementation", &new_implementation
x.compare!
end
```
Results:
```
Warming up --------------------------------------
Current implementation
2.000 i/100ms
New implementation 5.000 i/100ms
Calculating -------------------------------------
Current implementation
32.484 (±30.8%) i/s - 134.000 in 5.036419s
New implementation 74.878 (±38.7%) i/s - 250.000 in 5.052168s
Comparison:
New implementation: 74.9 i/s
Current implementation: 32.5 i/s - 2.31x (± 0.00) slower
```
When a host is not specified for an `ActionController::Renderer`'s env,
the host and related options will now be derived from the routes'
`default_url_options` and `ActionDispatch::Http::URL.secure_protocol`.
For example, with:
```ruby
Rails.application.default_url_options = { host: "rubyonrails.org" }
Rails.application.config.force_ssl = true
```
Before:
```ruby
ApplicationController.renderer.render inline: "<%= blog_url %>"
# => "http://example.org/blog"
```
After:
```ruby
ApplicationController.renderer.render inline: "<%= blog_url %>"
# => "https://rubyonrails.org/blog"
```
As a consequence, Action Text attachment URLs rendered in a background
job (a la Turbo Streams) will now use `Rails.application.default_url_options`.
Fixes#41795.
Fixeshotwired/turbo-rails#54.
Fixeshotwired/turbo-rails#155.
* Use storage/ instead of db/ for sqlite3 db files
db/ should be for configuration only, not data. This will make it easier to mount a single volume into a container for testing, development, and even sqlite3 in production.
When System Tests call `fill_in_rich_text_area`, they interact with
`<trix-editor>` elements by changing the contents programmatically.
This is unlike how end-users will interact with the element. Overhauling
the test helper to more accurately reflect Real World usage would
require a sizable effort.
With that being said, leaving the `<trix-editor>` with focus after
populating its contents is a minor change that makes it a more genuine
recreation.
Since engine initializers run later in the process, we need to run this
initializer earlier than the default.
This ensures they're all registered before the environments are loaded.
Because they are CamelCase, RDoc will automatically link these
references. Automatic links have monospace formatting, whereas, in some
cases, manual links do not.
This commit adds `ActionText.deprecator`, and adds it to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
Expand the `has_rich_text` signature to accept a `strict_loading:`
value. Forward that value along to the `has_one` declaration made under
the hood. When omitted, `strict_loading:` will be set to the value of
the `strict_loading_by_default` class attribute (false by default).
Commit 37d1429ab1 introduced the DummyERB to avoid loading the environment when
running `rake -T`.
The DummyCompiler simply replaced all output from `<%=` with a fixed string and
removed everything else. This worked okay when it was used for YAML values.
When using `<%=` within a YAML key, it caused an error in the YAML parser,
making it impossible to use ERB as you would expect. For example a
`database.yml` file containing the following should be possible:
development:
<% 5.times do |i| %>
shard_<%= i %>:
database: db/development_shard_<%= i %>.sqlite3
adapter: sqlite3
<% end %>
Instead of using a broken ERB compiler we can temporarily use a
`Rails.application.config` that does not raise an error when configurations are
accessed which have not been set as described in #35468.
This change removes the `DummyCompiler` and uses the standard `ERB::Compiler`.
It introduces the `DummyConfig` which delegates all known configurations to the
real `Rails::Application::Configuration` instance and returns a dummy string for
everything else. This restores the full ERB capabilities without compromising on
speed when generating the rake tasks for multiple databases.
Deprecates `config.active_record.suppress_multiple_database_warning`.
This commit implementes `rake test:isolated` for Action Text.
While many of modules have `rake test:isolated`, Action Text does not.
This could find #46113 in advance.
- Usage
```
cd actiontext
bundle exec rake test:isolated
```
- It detects #46113 by reverting the merge commit via #46119
```
$ git revert -m 1 4e7f876
$ bundle exec rake test:isolated
/home/yahonda/.rbenv/versions/3.1.2/bin/ruby -w -Ilib -Itest test/integration/controller_render_test.rb
... snip ...
E
Error:
ActionText::ControllerRenderTest#test_renders_as_HTML_when_the_request_format_is_not_HTML:
NoMethodError: undefined method `render_to_string' for nil:NilClass
(renderer || default_renderer).render_to_string(*args, &block)
^^^^^^^^^^^^^^^^^
test/integration/controller_render_test.rb:19:in `block in <class:ControllerRenderTest>'
bin/test test/integration/controller_render_test.rb:17
```
To run isolated tests at Rails CI,
https://github.com/rails/buildkite-config also needs updated.
Follow-up to #45144.
This ensures that a renderer is always available for Action Text, even
when `ActionController::Base` was not previously loaded.
Fixes#46113.
As with #45144, this still avoids loading `ActionController::Base`
unnecessarily when rendering mail after Action Text has been loaded.
**Before:**
```
$ bin/rails r 'Benchmark.memory { |x| x.report("load"){ MyBlankMailer.blank_email.body } }'
Calculating -------------------------------------
load 4.466M memsize ( 1.205M retained)
29.202k objects ( 11.943k retained)
50.000 strings ( 50.000 retained)
```
**After:**
```
$ bin/rails r 'Benchmark.memory { |x| x.report("load"){ MyBlankMailer.blank_email.body } }'
Calculating -------------------------------------
load 4.462M memsize ( 1.205M retained)
29.141k objects ( 11.940k retained)
50.000 strings ( 50.000 retained)
```
Co-authored-by: Christopher Louvet <cl@nonplaces.com>
this makes it possible for an application to embed markup in a document
that would otherwise be undesirable or expensive to process. For example,
an incoming email may include a complicated bit of DOM in a quote, and
while you don't want to have to process and rewrite it, you also don't want
to discard it; the content attribute of ContentAttachment allows you to
make that work.
Since Puma 5.0 (puma/puma@05936689c8),
Puma will automatically set `workers` to `ENV["WEB_CONCURRENCY"] || 0`.
Additionally, if `ENV["WEB_CONCURRENCY"]` > 1, Puma will automatically
set `preload_app`.
This can lead to confusing scenarios for users who are unaware of this
behavior and have customized `config/puma.rb`. For example, if a user
uncomments the `workers` and `preload_app!` directives, it is clear that
Puma will preload the app, and the number of workers can be configured
by setting `ENV["WEB_CONCURRENCY"]`. If the user sets
`ENV["WEB_CONCURRENCY"]` > 1, but then changes their mind and removes
the `workers` or `preload_app!` directives *without* clearing
`ENV["WEB_CONCURRENCY"]`, Puma will still preload the app and launch
`ENV["WEB_CONCURRENCY"]` number of workers. Similarly, if a user
uncomments *only* the `workers` directive and sets
`ENV["WEB_CONCURRENCY"]` > 1, Puma will preload the app even though the
`preload_app!` directive is still commented out.
To avoid such scenarios, this commit removes the commented-out `workers`
and `preload_app!` directives from the default `config/puma.rb`.
Also, to improve discoverability of available configuration options,
this commit adds a link to the Puma DSL documentation at the top of the
file.
Every time I write `config.cache_classes` I have to pause for a moment to make
sure I get it right. It makes you think.
On the other hand, if you read `config.enable_reloading = true`, does the
application reload? You do not need to spend 1 cycle of brain CPU to nod.
RDoc will automatically format and link API references as long as they
are not already marked up as inline code.
This commit removes markup from various API references so that those
references will link to the relevant API docs.
Prior to this commit, `ActionText::Content#inspect` called `#to_s` and
truncated the result to 25 characters. However, `#to_s` calls
`#to_rendered_html_with_layout` which, by default, renders the same
first 25 characters for every instance.
For example, with models such as:
```ruby
class Message < ActiveRecord::Base
has_rich_text :content
end
Message.create(content: "first message")
Message.create(content: "second message")
Message.create(content: "third message")
```
The output of `#inspect` is indistinguishable:
```irb
irb> Message.all.map { |message| message.content.body }
Rendered .../actiontext/app/views/action_text/contents/_content.html.erb within layouts/action_text/contents/_content (Duration: 2.4ms | Allocations: 881)
Rendered .../actiontext/app/views/action_text/contents/_content.html.erb within layouts/action_text/contents/_content (Duration: 0.7ms | Allocations: 257)
Rendered .../actiontext/app/views/action_text/contents/_content.html.erb within layouts/action_text/contents/_content (Duration: 0.6ms | Allocations: 257)
=> [#<ActionText::Content "<div class=\"trix-conte...">,
#<ActionText::Content "<div class=\"trix-conte...">,
#<ActionText::Content "<div class=\"trix-conte...">]
```
This commit changes `#inspect` to call `#to_html`, which does not render
the Action Text layout. So the output of `#inspect` will be:
```irb
irb> Message.all.map { |message| message.content.body }
=> [#<ActionText::Content "first message">,
#<ActionText::Content "second message">,
#<ActionText::Content "third message">]
```
* Revert "Pass service_name param to DirectUploadsController"
This reverts commit 193289dbbe.
* Revert "Multi-service direct uploads in Action Text attachment uploads"
This reverts commit 0b69ad4de6.
When a Rails app is generated with the --css option and the
action_text:install task is run, the Trix editor will not work as
expected. This occurs because using cssbundling-rails does not create an
application.css file, which would normally automatically require
actiontext.css.
Adding a step to append an import statement to the base CSS or SCSS
file, when one can be detected, solves the issue. In the case a base CSS
or SCSS file can't be detected, we output a warning to import the
necessary CSS file.
* Switch to a single controller option for choosing JavaScript approach
* Remove remnants of webpacker specific work within Rails
* No longer used
* Missing space
* Raise if unknown option is passed
* Style
* Use latest versions
* Make channels setup generic to all node setups
* Make Action Text installer work with any node package manager
* Explaining variables are not useless
* Rubocop pleasing
* Don't rely on Rails.root
Tests don't like it!
* Rubocopping
* Assume importmap
* No longer relevant
* Another cop
* Style
* Correct installation notice
* Add dependencies for action cable when adding a channel
* Fix paths to be relative to generator
* Just go straight to yarn, forget about binstub
* Fix tests
* Fixup installer, only yarn once
* Test generically with run
* Style
* Fix reference and reversibility
* Style
* Fix test
* Test pinning dependencies
* Remove extra space
* Add more tests
* Use latest dependencies
* Relegated this to controllers
* Refactor ChannelGenerator + more tests
Use a uniform level of abstraction
* No benefit to having actiontext css as scss
* Update test
* Update docs
* No more css assets to be generated
New world, new CSS frameworks, new needs.
* SCSS is becoming optional
* Remove Sass as a default-on setting
But continue to make it easy to add.
* Update docs
* No longer used
* Update tests
* Update docs
* Update docs
* No longer used
* No longer by default
* Fix tests
* Promote Tailwind CSS as an alternative to Sass
* Fix test and copy task
* Update railties/lib/rails/generators/rails/app/templates/Gemfile.tt
Co-authored-by: Kevin Newton <kddnewton@gmail.com>
Co-authored-by: Kevin Newton <kddnewton@gmail.com>
* Turbolinks is being replaced with Hotwire
* Make --webpack opt-in
* Don't use specific webpacker installers any more in preparation for next Webpacker
* Update railties/lib/rails/app_updater.rb
Co-authored-by: Alex Ghiculescu <alex@tanda.co>
* Trailing whitespace
* Convert to Turbo data attribute for tracking
* Default is no webpack, no hotwire
* Swap out turbolinks references for hotwire
* Drop explicit return
* Only generate package.json if using webpack
* Only create package.json in webpack mode
* Only create app/javascript in webpack mode
* Generate correct style/js links based on js mode
* Fix tests from changed output format
Not sure why these are showing up in this PR, though.
* Rubocopping
* Stick with webpack for the test app for now
* Adjust tests
* Replace minitest-reporters with minitest-ci (#43016)
minitest-reporters is used to create junit xml reports on CI.
But when it loads before rails minitest plugin makes
`Rails::TestUnitReporter` not being added as a reporter.
minitest-ci is now only loaded at ci and does not interferes with
rails minitest plugins. And keeps junit reports workings
* Too heavy handed to actually run bundle
Just like we don't auto-migrate
* Pin js frameworks in importmap
Instead of having importmap preconfigure it.
* Match updated app/javascript path
* No need for the explaining comment
* Fixes test cases for replace webpack with importmapped Hotwire as default js (#42999)
* Fix rubocop issues
* Fix more railities test cases
* Fix plugin generator railties shared test cases
* Fix Action Text install generator asset pipeline spec
* They're modules, not files
* Let dev use the latest release as well
So we don't have to replace unexisting dev releases with latest release
* Make Webpack responsible for generating all the JS files it needs
Webpacker 6 has already moved from app/javascript to app/packs.
* Don't add rails/ujs by default any longer
All the ajax/form functionality has been superseded by Turbo. The rest lives in a weird inbetween land we need to address through other means.
* Use new importmap location
* Switch to using turbo-rails and stimulus-rails directly
The hotwire-rails gem does not offer enough value for its indirection
* Use latest Webpacker
* Prevent version resolution requests from getting swallowed
* Use ESM syntax for imports
* Move management of yarn, package.json, etc to Webpacker 6
* Update for Webpacker 6
* Move bin/setup addition to Webpacker as well
* Remove dead tests
* Bump to Webpacker 6.0.0.rc.2
* No longer relevant given the new default is no webpacker
* Rely on Webpacker 6
* No longer relevant
* No longer relevant
* Make cable channel generator work for both webpacker and importmap setups
* Fix tests
* For tests testing importmap way
* Use Webpacker 6 dummy
* RuboCopping
* One more bump to fix webpack-dev-server
* Another bump. Hopefully the last one!
* Also enough to not want turbo tracking on
* Fix tests
* Latest
* Fix tests
* Fix more tests
* Fix tests
Co-authored-by: Alex Ghiculescu <alex@tanda.co>
Co-authored-by: André Luis Leal Cardoso Junior <andrehjr@gmail.com>
Co-authored-by: Abhay Nikam <nikam.abhay1@gmail.com>
Co-authored-by: Guillermo Iguaran <guilleiguaran@gmail.com>
* Stop trying to configure listen by default on compatible platforms
Modern computers with SSDs don't see much/any benefit from having an evented file update watcher. Remove complexity by taking this spinning-drive concession out.
* Actually need listen for testing the opt-in
* Test no longer relevant
* Move to ESM and drop Babel transpiling
* Make the activestorage JS directly available through the asset pipeline as a precompiled asset
* Use example with direct attachment
* Explain that direct reference is possible
* Active Storage JS is a module
* Retain umd asset for backwards compatibility, add ESM file in addition
* Explain how to use activestorage.esm with importmap
* Use untranspiled activestorage inclusion
* Don't repeat terser options
* Action Text JS should be available via the asset pipeline too
* Main was a module anyway, no need to reference that twice
* Fix rollup references
* Precompile action text JS for asset pipeline
* No JavaScript dependencies needed with the asset pipeline
* Stub Webpacker::Engine to trigger webpack path for testing
* Extract asset paths
* Exercise asset pipeline path
* Terser doesn't do anything useful on this small package
* Make trix directly available to the asset pipeline
* Indirect doesn't carry its worth
* Reminder for development about keeping things in sync for the asset pipeline
* Ensure this isn't turned into undefined while mirroring
* Mirror Trix CSS for asset pipeline
* Add the needed JS include tag automatically under the asset pipeline
* Please RuboCop
* Keep the peer dependency
Even though we also need it explicitly as a dev dependency in order to generate the mirror output for trix.
* Fix test
* Add CHANGELOG entry
Similar to: #42378
Tried adding test cases for the changes but
migration file would always use id as primary_key_type
and reference type as foreign_key_type. Did not find
any good way to assert the changes.
Tested locally and following is the schema
generated for Action Text migration if
`primary_key_type: :uuid`
```
create_table "action_text_rich_texts", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.string "name", null: false
t.text "body"
t.string "record_type", null: false
t.uuid "record_id", null: false
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["record_type", "record_id", "name"], name: "index_action_text_rich_texts_uniqueness", unique: true
end
```
Terser is more up to date with modern javascript features, and the
uglifier gem repository recommends using it for minifying ES6+.
Followup for 955041b
Affected gems:
* actionmailbox
* activestorage
* actiontext
This fixes the following warning:
DEPRECATION WARNING: Using legacy connection handling is deprecated. Please set
`legacy_connection_handling` to `false` in your application.
The new connection handling does not support `connection_handlers`
getter and setter.
Read more about how to migrate at: https://guides.rubyonrails.org/active_record_multiple_databases.html#migrate-to-the-new-connection-handling
(called from require at ~/.gem/ruby/3.1.0/gems/zeitwerk-2.4.2/lib/zeitwerk/kernel.rb:34)
Currently when you make a new Rails app, we generate a lot of initializers. For new users, I think we should try and include as few as possible - the less files, the less daunting a new app is. And for upgrades I'd like to [continue to simplify the update process](https://github.com/rails/rails/pull/41083), in this case by not bringing back initializers you have probably already dismissed or modified.
In this PR I'm proposing we remove two initializers: `application_controller_renderer.rb` and `cookies_serializer.rb`:
**`application_controller_renderer.rb`**. This configures [`ActionController::Renderer`](https://api.rubyonrails.org/classes/ActionController/Renderer.html), for rendering views outside of controller actions. I don't think this is something most Rails apps will need (certainly not on day 1); users can configure this feature when they need it.
**`cookies_serializer.rb`**. This was added for [Rails 4.1](https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#cookies-serializer). The behaviour is:
- For new apps, the initializer says `:json`.
- For upgraded apps that don't have the initializer, it is added with value `:marshal`.
- If there's no initializer, the [default value](c9a89a4067/actionpack/lib/action_dispatch/middleware/cookies.rb (L589)) is `:marshal`.
Since nobody should be upgrading direct from Rails 4.0 to Rails 7.0, we can simplify this by using new framework defaults. So the behavior will now be:
- For new apps, `config.load_defaults("7.0")` sets the value to `:json`.
- The `new_framework_defaults_7_0.rb` file explains this, and suggests using `:hybrid` to be upgrade to JSON cookies.
- No changes to [the code](c9a89a4067/actionpack/lib/action_dispatch/middleware/cookies.rb (L589)); the default value is `:marshal` if you don't set one.
So if you were not setting a `cookies_serializer` previously and you want to keep using `:marshal`, you'll need to explicitly set this before using `config.load_defaults("7.0")`, otherwise it will switch to `:json`. The upside of this is you won't get the `cookies_serializer.rb` file created for you every time you upgrade.
Both methods are defined in multiple parts of the framework. It would
be useful to put them in a proper place, so that repetition is
avoided.
I chose the implementation from `ActiveRecord` because it's a bit more
complete with the `SQLCounter` class, and also because other parts
depend on it.
Trix's `<trix-editor>` doesn't support the [form][] property like
`<textarea>` or other form fields.
For example, consider the following HTML and event listener:
```html
<form action="/articles" method="post">
<textarea name="content"></textarea>
<button type="submit">Save</button>
</form>
<script>
addEventListener("keydown", ({ key, metaKey, target }) => {
if (target.form && key == "Enter" && (metaKey || ctrlKey)) {
form.requestSubmit()
}
})
</script>
```
The `target` (an instance of `HTMLTextAreaElement` relies on the
[HTMLTextAreaElement.form][] property for access to its associated
`<form>`. While it's usually equivalent to `target.closest("form")`,
that isn't always the case. Declaring a `[form]` attribute with another
`<form>` element's `[id]` value can associate a field to a `<form>` that
is _not an ancestor_. That means that the event listener from above
would continue to work with this HTML:
```html
<textarea name="content" form="new_article"></textarea>
<!-- elsewhere -->
<form id="new_article" action="/articles" method="post">
<button type="submit">Save</button>
</form>
```
Unfortunately, if the `<textarea>` element were replaced with a
`<trix-editor>`, the event listener's reliance on accessing the form as
a property would break, since the `<trix-editor>` custom element doesn't
declare that property. There is currently a pull request
([basecamp/trix#899][]) to add support for accessing the `form` as a
property of the `<trix-editor>` element.
The [feedback][] provided on that pull request suggests that we
implement the `form` property by delegating to the `<input
type="hidden">` element. Currently, `<input type="hidden">` elements
constructed by Action Text helpers cannot declare the `[form]`
attribute.
This commit adds support by special-casing the `options[:form]` key
within `ActionText::TagHelper#rich_text_area_tag`.
[form]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLTextAreaElement#properties
[basecamp/trix#899]: https://github.com/basecamp/trix/pull/899#discussion_r618543357
[feedback]: https://github.com/basecamp/trix/pull/899#discussion_r618543357
The test cases where failing because the test
`test_converts_Trix-formatted_attachments_with_custom_tag_name` set a
custom tag_name to `arbitrary-tag`. This test would also set the
ActionText::AttachmentGallery::ATTACHMENT_SELECTOR private constant
to `arbitrary-tag`.
Other test cases had proper ActionText::Attachment.tag_name set to
`action-text-attachment` but the constant once defined would not
reset.
This PR attempts to fix the issue by converting the
ActionText::AttachmentGallery::{ATTACHMENT_}SELECTOR to class methods
Fixes#41782
The helpers of Action Text are added to a couple of non-reloadable
base classes:
initializer "action_text.helper" do
%i[action_controller_base action_mailer].each do |abstract_controller|
ActiveSupport.on_load(abstract_controller) do
helper ActionText::Engine.helpers
end
end
end
Therefore, it does not make sense that they are reloadable themselves.
For the same price, we can also make the models non-reloadable, thus
saving parent applications from the unnecessary work of reloading this
engine.
We did this for turbo-rails as well.
* Improve ActionText::FixtureSet documentation
Support for Action Text attachments in fixtures was added by [76b33aa][] and
released as part of [6.1.1][], but has not yet been documented.
This commit documents the `ActionText::FixtureSet` for the API
documentation, and mentions it in the Rails Guides pages.
[76b33aa]: 76b33aa3d1
[6.1.1]: https://github.com/rails/rails/releases/tag/v6.1.1
* Fix indention of comments
Co-authored-by: David Heinemeier Hansson <david@loudthinking.com>
Extensible layout
---
Expose how we render the HTML _surrounding_ rich text content as an
extensible `layouts/action_text/contents/_content.html.erb` template to
encourage user-land customizations, while retaining private API control
over how the rich text itself is rendered by moving the
`#render_action_text_content` helper invocation to the
`action_text/contents/_content.html.erb` partial.
Extensible Attachable `#to_attachable_partial_path`
---
When an application declares a canonical partial for a record, there is
no way to override which partial is used when transformed to Rich Text.
For example, a default `Person < ApplicationRecord` instance returns
`"people/person"` from calls to `#to_partial_path`, resulting in the
`app/views/people/_person.html.erb` partial being rendered.
Prior to this change, when encountering an `<action-text-attachment
sgid="...">` element, ActionText retrieved the corresponding
`Attachable` instance (usually an `ActiveRecord::Base` instance) and
transformed it to rich text HTML by rendering the partial that
corresponds to its `#to_partial_path`.
This proposed change instead invokes
`Attachable#to_attachable_partial_path`. By default,
`#to_attachable_partial_path` is an alias for `#to_partial_path`.
Guides
---
Extend the `guides/action_text_overview` document to
describe how to customize these templates, and to better illustrate how
ActionText::Attachable instances are rendered into HTML.
Because `ActionText::Content.renderer` is implemented as a
`thread_cattr_accessor`, any default value set in the main thread will
be inaccessible from other threads. Therefore, use a `cattr_accessor`
to store the default renderer, and fall back to it when `renderer` has
not been set by e.g. `with_renderer`.
Fixes#40757.
`form_with` would generate a remote form by default.
This confused users because they were forced to handle remote requests.
All new 6.1 applications will generate non-remote forms by default.
When upgrading a 6.0 application you can enable remote forms by default by
setting `config.action_view.form_with_generates_remote_forms` to `true`.
Since #40222, Action Text HTML is rendered in the context of the current
request. This causes the Action Text template format to default to the
request format, which prevents the template from being resolved when the
request format is not `:html` (e.g. `:json`). Therefore, override the
template format to always be `:html`.
Fixes#40695.
This commit adds tests for the Action Text install generator. It also
includes a few changes in and around `generators_test_helper.rb` to make
writing similar tests easier in the future.
Closes#39317.
Co-authored-by: Abhay Nikam <nikam.abhay1@gmail.com>
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>
`Minitest.plugin_rails_init` sets `Minitest.backtrace_filter` to
`Rails.backtrace_cleaner` right before tests are run, overwriting the
value set in test_helper.rb.
`Rails.backtrace_cleaner` silences backtrace lines that do not start
with `Rails.root` followed by e.g. "lib/" or "test/". Thus when
`Rails.root` is a subdirectory of the project directory -- for example,
when testing a plugin that has a dummy app -- all lines of the backtrace
are silenced.
This commit adds a fallback such that when all backtrace lines are
silenced, the original `Minitest.backtrace_filter` is used instead.
Additionally, this commit refactors and expands existing test coverage.
Permit applications to hack in custom DB config for ActionText::RichText until AT has first-class multi-DB support:
ActiveSupport.on_load(:action_text_record) do
connects_to reading: :action_text_replica, writing: :action_text_primary
end
Matt Swanson