This doesn't really affect us because we don't use `before_committed`
callbacks (and neither do any of our dependencies), so we're using the
new default value.
This is done for performance reasons. Quoting the pull request
introducing this option [1]:
> A config might be overkill, but I wanted to provide an escape hatch
> for any upgraded apps that might be testing the exact value of the
> action_dispatch.parameter_filter header.
Since we don't test the exact value of action_dispatch.parameter_filter,
we can enable this option.
[1] Pull request 46452 in https://github.com/rails/rails
Before this change, every time we saved a record, the association was
validated if we had `belongs_to :something, required: true`. After this
change [1], it's only validated if the `something_id` column is nil (or
`something_type` for polymorphic associations) or if the `something_id`
attribute has changed.
The main difference is that we no longer get validation errors if the
associated record has been deleted. Doesn't affect us much, so we're
going with the new default value.
[1] Pull request 46522 in https://github.com/rails/rails
This is the new default option, and its only dangerous when deploying to
applications with multiple servers. Since this isn't our case, we can
enable it.
The new serializer can decrypt legacy messages using the `marshal`
serializer, so there's no risk of losing data when upgrading. Since we
aren't using applications with several servers, where upgrading some
servers might cause issues on the servers that aren't upgraded yet,
we're enabling the option.
[1] See comments in pull request 42846 in https://github.com/rails/rails
It doesn't really affect us (unless some of our dependencies make this
mistake) because we only use `expires_in/expires_at` once and we do it
correctly, but it might be help us detect this issue if we ever
introduce it in the future.
This option won't even exist in Rails 7.2 [1], and the possibility to
disable it was only added to guarantee safe upgrades in Rails
applications with multiple replicas [2].
Since we don't have applications with multiple replicas, where one
replica could be using Active Job 7.0 and another one could be using
Active Job 7.1 while upgrading, we can enable this options.
[1] Commit 2a761d23d2 in https://github.com/rails/rails
[2] Commit bc1f323338 in https://github.com/rails/rails
The only change between these headers and the ones sent by Rails 7.0
application is that the `"X-Download-Options" => "noopen"` is no longer
sent. Only Internet Explorer used that header, and uploading, previewing
and downloading attachments still works fine on Internet Explorer 11
after this change.
[1] Pull request 43968 in https://github.com/rails/rails
Quoting the Rails configuration guide [1]:
> applications running in :zeitwerk mode do not need require_dependency,
> so models, controllers, jobs, etc. do not need to be in $LOAD_PATH.
> Setting this to false saves Ruby from checking these directories when
> resolving require calls with relative paths, and saves Bootsnap work
> and RAM, since it does not need to build an index for them.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-add-autoload-paths-to-load-path
We were getting a warning since we upgraded to Rails 7.1:
> Rails 7.1 has deprecated the singular fixture_path in favour of an
> array.You should migrate to plural:
We were getting a warning after upgrading to Rails 7.1:
```
DEPRECATION WARNING: ActionDispatch::IllegalStateError is deprecated
without replacement.
```
We were getting a deprecation warning after upgrading to Rails 7.1:
```
DEPRECATION WARNING: Setting action_dispatch.show_exceptions to true is
deprecated. Set to :all instead.
```
So we're passing `:all`.
I've checked whether we can remove the `:show_exceptions` tag completely
now that the new default option is `:rescuable`. The answer is "no" ;).
Removing the `:show_exceptions` tag makes the tests fail.
We were getting the following warning after upgrading to Rails 7.1:
```
Rendered shared/_social_media_meta_tags.html.erb
DEPRECATION WARNING: Passing the class as positional argument is
deprecated and will be removed in Rails 7.2.
Please pass the class as a keyword argument:
serialize :metadata, type: Object
```
We were getting a ton of deprecation warnings:
```
DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor
of `Rails.application.credentials` and will be removed in Rails 7.2
```
However, we don't plan to replace the secrets with credentials in the
foreseeable future because it would affect existing Consul Democracy
installations, so for now we're simply silencing the warnings.
This is an error we've only been able to reproduce on one specific
machine and only when using the development environment.
It looks like Rails 7.1.5.1 uses `ActiveStorage::PreviewImageJob` when
we attach a PDF. However, that raises an IntegrityError because we're
removing the metadata from PDFs. That is, the final PDF is no longer the
same PDF that was uploaded.
This issue wasn't present in the original Rails 7.1.0 release, but was
introduced in Rails 7.1.4 [1] and has already been fixed in Rails 7.2.0
[2].
So we're applying the same solution that was applied in Rails 7.2.0:
disabling automatic previews for PDFs when no variants require them by
changing a method in `ActiveStorage::Attachment`.
[1] See commit 6f729dd39 in https://github.com/rails/rails/
[2] See pull request 51351 in https://github.com/rails/rails/
We're disabling `action_controller.raise_on_missing_callback_actions`
because sometimes we include `before_action :something, only: actions`
in concerns, and we include these concerns in controllers that don't
have all these actions.
Note that Rails 7.1 logs to STDOUT by default [1]; we're re-adding the
condition `if ENV["RAILS_LOG_TO_STDOUT"].present?` because we're still
using files and we're rotating the logs to avoid running out of space.
Also note that the GraphQL controller test (which is actually a request
test, since it's got `type: :request`) that was raising an exception no
longer does it thanks to the new default value for the
`config.action_dispatch.show_exceptions` configuration option. So we're
updating the test accordingly. This option doesn't affect regular
controller tests (without the `type: :request` option), so in other
tests we're still checking exceptions.
[1] Pull request 47138 in https://github.com/rails/rails
We were using a rack browser and testing the
`FeatureFlags::FeatureDisabled` exception was raised. However, we don't
test this exception in any other system tests; we only check it in
controller tests.
So we're using a real browser for consistency and because in Rails 7.1
this test is failing when enabling the new default option
`config.action_dispatch.show_exceptions = :rescuable` in the test
environment.
Apparently, the `ids` method, which originally was implemented as
`pluck(:id)`, sometimes returned duplicate ids [1]. Fixing that issue
generated another issue [2] when combining `.includes` and `order`.
There was an attempt to fix it [3], but it still doesn't fix the case
where the ordering column belongs to an association [4].
This means that, when upgrading to Rails 7.1, calling `.ids` on
`budget.investments.includes(:heading).sort_by_random.for_render`
results in an invalid SQL statement:
```
ActiveRecord::StatementInvalid:
PG::GroupingError: ERROR: column "ids.ordering" must appear in
the GROUP BY clause or be used in an aggregate function
LINE 1: ... = $4 GROUP BY "budget_investments"."id" ORDER BY
ids.orderi...
```
To solve it, we could once again use `.pluck(:id)` instead of `ids`, but
I'm not sure whether there would be a risk to get duplicate IDs in some
cases. We cannot call `.reorder` or `.unscope(:order)` either because we
need the IDs to be ordered randomly.
So we're removing the `includes(:heading)` part when getting the IDs.
Since using `includes` generates a separate query that doesn't affect
the query to get the IDs, removing it makes no difference.
Another option would be to remove the `for_render` call, since we're
including the headings to avoid the N+1 queries problem, but we're doing
so without benchmarks showing that it does actually make a difference.
[1] Issue 46455 in https://github.com/rails/rails
[2] Issue 48080 in https://github.com/rails/rails
[3] Pull request 48101 in https://github.com/rails/rails
[4] Issue 50263 in https://github.com/rails/rails
The "foundation_rails_helper" gem is no longer maintained and is
incompatible with Rails 7.1. To avoid blocking the upgrade, we've vendored
the vendor/foundation_rails_helper/form_builder.rb as a copy of the
original FormBuilder class.
To mantain compatibility with auto_labels and button_class variables, that
are used in the original builder, we are overwriting in the foundation
form builder initializer.
The gem has been removed from the Gemfile and replaced with this vendored
fallback. This workaround is safe to remove once legacy Foundation CSS
support is dropped.
All vendored code retains the original MIT license and attribution.
ros-apartment 3.0.0+ includes official support for connection handling in Rails 7,
so we no longer need to override `ActiveRecord::ConnectionHandling#connected_to`.
References: PR #194 and #243 in ros-apartment
