This clarifies that the boolean interpretation (1) is due to YAML rather
than I18n, (2) is case insensitive, and (3) affects both keys and
values.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Background
----------
During integration tests, it is desirable for the application to respond
as closely as possible to the way it would in production. This improves
confidence that the application behavior acts as it should.
In Rails tests, one major mismatch between the test and production
environments is that exceptions raised during an HTTP request (e.g.
`ActiveRecord::RecordNotFound`) are re-raised within the test rather
than rescued and then converted to a 404 response.
Setting `config.action_dispatch.show_exceptions` to `true` will make the
test environment act like production, however, when an unexpected
internal server error occurs, the test will be left with a opaque 500
response rather than presenting a useful stack trace. This makes
debugging more difficult.
This leaves the developer with choosing between higher quality
integration tests or an improved debugging experience on a failure.
I propose that we can achieve both.
Solution
--------
Change the configuration option `config.action_dispatch.show_exceptions`
from a boolean to one of 3 values: `:all`, `:rescuable`, `:none`. The
values `:all` and `:none` behaves the same as the previous `true` and
`false` respectively. What was previously `true` (now `:all`) continues
to be the default for non-test environments.
The new `:rescuable` value is the new default for the test environment.
It will show exceptions in the response only for rescuable exceptions as
defined by `ActionDispatch::ExceptionWrapper.rescue_responses`. In the
event of an unexpected internal server error, the exception that caused
the error will still be raised within the test so as to provide a useful
stack trace and a good debugging experience.
`assert_queries` in Action Text's test helpers can detect queries from
a previous test if background jobs are allowed to run during its
execution.
Changing the queue_adapter to the test adapter for all tests using the
helper ensure no jobs can run during its execution.
Both `ActionText::Record` and `ActionText::RichText` have dedicated `ActiveSupport` load hooks. This adds an
additional hook for `ActionText::EncryptedRichText`, so that external libraries have a similarly simple way
to run code after the subclass is loaded.
Currently when opening the main framework pages there is no introduction
to the framework. Instead we only see a whole lot of modules and the
`gem_version` and `version` methods.
By including the READMEs using the `:include:` directive each frameworks
has a nice introduction.
For markdown READMEs we need to add the :markup: directive.
[ci-skip]
Co-authored-by: zzak <zzakscott@gmail.com>
Before this commit, using ActionText and calling `#as_json` on a
non-persisted `ActiveStorage::Blob` raised an error.
This is because `ActionText::Attachable` is included in
`ActiveStorage::Blob` and overrides the `#as_json` method to
expose a global signed id. However, a global signed id can only
be generated on a persisted instance.
This commit fixes the issue by making sure the blob is persisted
before exposing the global signed id.
* Remove Copyright years
* Basecamp is now 37signals... again
Co-authored-by: David Heinemeier Hansson <dhh@hey.com>
---------
Co-authored-by: David Heinemeier Hansson <dhh@hey.com>
YAML has quite a bit of footguns, as such it's desirable
to be able to substitute it for something else or even
simply to force users to define a serializer explictly for
every serialized columns.
## 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.
Justin Perkins