Without this change the IdpMetaParser would give an error
in the Devise initializer when starting the application.
I found it annoying to have to connect to the VPN so
I decided to add this condition.
Reviewer, feel free to consider this commit unnecessary
and ask to revert it.
We were having an issue because there was a difference of about 11
seconds between the local times of our machines and the time of the IDP
server. Since right now we can't guarantee the time of these machines is
fully synchronized, for now we're adding a margin of error of one
minute.
Bumps [rubocop](https://github.com/rubocop/rubocop) from 1.71.2 to 1.75.8.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](https://github.com/rubocop/rubocop/compare/v1.71.2...v1.75.8)
---
updated-dependencies:
- dependency-name: rubocop
dependency-version: 1.75.8
dependency-type: direct:development
update-type: version-update:semver-minor
...
Notes:
This commit also includes several style and lint fixes required after
updating RuboCop:
- Removed redundant parentheses now detected by improved
'Style/RedundantParentheses' (1.72 and 1.75.3).
- Replaced ternary expressions with logical OR when the ternary was
returning 'true', as flagged by 'Style/RedundantCondition' (1.73).
- Adjusted block variables to resolve new 'Lint/ShadowingOuterLocalVariable'
offenses (1.75), helping avoid future conflicts during upgrades with
'rails app:updates'
Signed-off-by: dependabot[bot] <support@github.com>
Ahoy 2.0.0 [1] introduced automatic UUID generation for visit_token and
visitor_token. As a result, the custom ensure_uuid method is no longer
needed and can be safely removed from the initializer.
Since we aren't manually generating UUIDs anymore, we no longer need
the uuidtools dependency.
[1] https://github.com/ankane/ahoy/blob/v2.0.0/README.md#token-generation
IMHO, Spring no longer provides benefits in this project and:
- Spring was already disabled in the test environment since commit e4e0cb5d47
- Rails removed Spring as a default installation option in 2021 [1]
[1] PR #42997 from https://github.com/rails/rails/
capistrano3-puma v6.0.0 updated the defaults for puma_access_log and
puma_error_log to use a single file based on puma_env, like:
> set_if_empty :puma_access_log, -> { File.join(shared_path, 'log', "#{fetch(:puma_env)}.log") }
> set_if_empty :puma_error_log, -> { File.join(shared_path, 'log', "#{fetch(:puma_env)}.log") }
However, our installer expect:
- puma_access.log
- puma_error.log
To keep the existing behavior aligned with the installer, we define
the Puma log paths in config/deploy.rb
In capistrano3-puma v6.0.0, the default for 'puma_service_unit_name' changed to:
> "#{application}_puma_#{stage}"
But the installer uses the older convention:
> "puma_#{application}_#{stage}"
To ensure consistency and avoid unit name conflicts when switching between
versions or deploying older branches, we now define the variable explicitly
in config/deploy.rb:
> set :puma_service_unit_name, -> { "puma_#{fetch(:application)}_#{fetch(:stage)}" }
This commit copies the default puma.service.erb template from the
capistrano3-puma gem into lib/capistrano/templates. This allows us to
customize the generated systemd unit file during deploy.
Note that we are also removing the `:puma_conf` variable from `config/deploy.rb`,
as the new ExecStart line in the systemd template (based on capistrano3-puma 6.0.0)
does not rely on a separate Puma config file. The command now directly invokes:
ExecStart=<%= expanded_bundle_command %> exec puma -e <%= fetch(:puma_env) %>
This replaces the older format used in 5.2.0:
ExecStart=<%= expanded_bundle_command %> exec --keep-file-descriptors puma -C <%= fetch(:puma_conf) %>
which required explicitly setting the Puma config path.
In earlier versions of capistrano3-puma, the puma_bind has a default value to:
unix://.../tmp/sockets/puma.sock via set_if_empty in lib/capistrano/puma.rb.
This default was removed in 6.0.0, requiring to explicitly set :puma_bind in deploy.rb.
This caused the following runtime error during deploy:
> Failed to restart consul_puma_staging.service: Unit
> consul_puma_staging.socket has a bad unit file setting.
capistrano3-puma 6.0.0 removed the `puma:systemd:config` and
`puma:systemd:enable` tasks. This commit updates the deploy script to use
the new `puma:install` and `puma:enable` tasks instead.
We're still using YAML to serialize the legislation_annotations ranges
column. I'm not sure whether changing the serializer can have
consequences on existing data, and I'm not sure which serializer we
should provide instead. Quoting the Rails configuration guide [1]:
> Unfortunately there isn't really any suitable defaults available in
> Ruby's standard library. JSON could work as a format, but the json
> gems will cast unsupported types to strings which may lead to bugs.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-active-record-default-column-serializer
I think this doesn't affect us because we use RSpec instead of Rails
test classes. In any case, if it ever affects us, we'll get notified
when a test fails.
As mentioned in the pull request introducing this change [1]:
> FATAL is documented in the Ruby Logger docs as being for "An
> unhandleable error that results in a program crash.", which does not
> really apply to this case since DebugExceptions is handling the error.
So we're using the new default value, which makes more sense.
[1] Pull request 48575 in https://github.com/rails/rails
Since we use a version of Loofah supporting HTML5 since db2d0bb80, the
`Rails::HTML::Sanitizer.best_supported_vendor` method will return the
HTML5 sanitizer. As mentioned in the pull request introducting this
change [1], the libxml2 maintainer wrote:
> it's still a bad idea to use a 20+ years old, unmaintained HTML 4
> parser to sanitize input for the modern web
So we're going with the new default sanitizer.
Note we aren't uncommenting the `action_text.sanitizer_vendor` option
because we don't use Action Text and so it doesn't affect us , and
uncommeting it will raise an error.
Also note we need to change one test because the new sanitizer handles
whitespace slightly differently.
[1] Pull request 48293 in https://github.com/rails/rails
Just like we mentioned in commit 001eee3d6, according to the Rails
configuration guide [1], with this format, Rails serializes cache
entries more efficiently. Most importantly:
> All formats are backward and forward compatible, meaning cache entries
> written in one format can be read when using another format. This
> behavior makes it easy to migrate between formats without invalidating
> the entire cache.
[1] https://guides.rubyonrails.org/v7.1/configuring.html#config-active-support-cache-format-version
It doesn't really affect us because we never use `return`, `break` or
`throw` inside transactions, since it would be confusing exactly because
it wouldn't be 100% clear whether the transaction is committed or not.
So we're using the new default value, which will be the only available
option in Rails 7.2 [1].
[1] Commit eccc6061f4 in https://github.com/rails/rails
This change doesn't affect us, since we don't use `after_commit`
callbacks, and, among our dependencies, AFAIK only the Devise gem uses
them, and it only defines one after_commit callback when creating a
record and another one when updating it, so we're never going to have
more than one callback being executed after a transaction is finished.
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 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
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
Saying that we're supposed to introduce a descriptive title in a field
labelled as "Title" is redundant. Besides, the text of the placeholder
was barely distinguishable, making it harder to fill in the form.
