Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#8090] Upgrade to Rails 7.1 #8515

Open
wants to merge 44 commits into
base: develop
Choose a base branch
from
Open

[#8090] Upgrade to Rails 7.1 #8515

wants to merge 44 commits into from

Conversation

gbp
Copy link
Member

@gbp gbp commented Jan 8, 2025

Relevant issue(s)

Depends on #8513
Depends on #8120
Depends on #8514
Fixes #8090

What does this do?

Upgrade to Alaveteli to Rails 7.1

Why was this needed?

Needed to enure Alaveteli to kept up-to-date for security reasons and future development.

gbp added 27 commits January 8, 2025 10:20
Ensure categories have parent associations.

This change enforces parent associations for categories to support
Rails 5.0's `active_record.belongs_to_required_by_default` setting.
Disable parent/child relation validation.

This change is needed so test examples pass after enabling support for
Rails 5.0's `active_record.belongs_to_required_by_default` setting.
Use test factory so the instance has belongs_to associated objects.

This change is needed so this test example passes after enabling support
for Rails 5.0's `active_record.belongs_to_required_by_default` setting.
Configure Comment#user to be optional.

Although we have a DB not null constraint we need to allow user to be
optional as the controller calls `invalid?` and to allow the form to be
rendered prior login.
Configure optional associations.

This change is needed so test examples pass after enabling support for
Rails 5.0's `active_record.belongs_to_required_by_default` setting.
Check errors include message for required belongs_to associations.

This change ensures these test examples will pass after enabling support
for Rails 5.0's `active_record.belongs_to_required_by_default` setting.
In the test environment some `script` test examples look at output
stderr. When doing Rails upgrades we sometimes see deprecation warnings
on boot and these examples will fail.

This change adds a new ENV which can be set to silence these warnings.
Ensures translation objects are correctly build with `globalized_model`
association.

This change is needed so test examples pass after enabling support for
Rails 5.0's `active_record.belongs_to_required_by_default` setting.
Added so we can review these options.
Add examples where we already validated belongs_to associations.

These additional test examples will be useful after we turn on Rails
5.0's `active_record.belongs_to_required_by_default` setting as we will
be able to clean up the `validates presence` calls.
Don't validate this association is present as we already have
validation in place with a custom error message.
These validations are now covered by the relevant belongs_to
association.
This is already called from an before action hook.

Calling it a second time works until we enable the Rails 6.1's framework
default configuration options due to Rails now tracking built associated
objects resulting in two TrackThing instances, but only one being assign
values for required attributes.
So there is better visibility why these have been set without going into
the git commit history.
It appears Rails 6.1's `active_record.has_many_inversing` configuration
option change has introduced a regression in Rails which causes issues
with our code base.

We expect the follow to work, but with this configuration options this
breaks due to how we associated objects via FactoryBot in our test
suite:

  user = User.new
  info_request = InfoRequest.new(user: user)
  comment = Comment.create!(info_request: info_request, user: user)
@gbp gbp force-pushed the 8090-rails-7.1 branch 3 times, most recently from aee7bbf to 55ed43b Compare January 8, 2025 19:24
@gbp gbp force-pushed the 8090-rails-7.1 branch 2 times, most recently from 301346b to f0acf3e Compare January 8, 2025 20:15
Required by Rails 7.1. This has been configured to use MiniMagick. No
changes required to our profile photos processing although we could
switch over to the image_processing pipeline if we chose to.
@gbp gbp force-pushed the 8090-rails-7.1 branch 4 times, most recently from 3b74f70 to ba95bea Compare January 9, 2025 17:01
@gbp gbp marked this pull request as ready for review January 9, 2025 17:28
gbp added 16 commits January 10, 2025 10:59
Fixes:
 NameError: uninitialized constant ActiveRecord::FixtureSet::ClassCache
In Rails 7.1 it will require a default ActiveStorage service to be
defined. This adds a service for the test environment so test examples
don't break when we bump the Rails gem.
When migrating the database under Rails 7.1 this migration breaks due to
a new constraint ensuring enums have a database backed column.

Our `Note#style` was added in a later migration so doesn't exist yet.
Recreating the Note class without the enum defined allows full migration
to succeed.
Ensure we are matching strings with strings. In Rails 7.1 this will
return an `ActionView::OutputBuffer`.
Once `action_controller.raise_on_missing_callback_actions` is enabled we
will see exceptions due to the `signin` action not existing.
Sometimes we need locales to be loaded before the application has been
initialised.

Setting in the in `config.before_configuration` block doesn't persist
correctly and also needs to be called in after initialisation.

This issue became apparent when there is an exception on boot. The error
message might call `to_sentence` [1], which will load a translation and
this then error if `set_locales` hasn't been called.

[1]: https://github.com/rails/rails/blob/d39db5d/activestorage/lib/active_storage/service/registry.rb#L18-L19

See #5382
Changes from running `rails app:update`.
Ensure data can be retrieved via symbol keys. When enabling the Rails
7.1 framework default data will be returned with string keys.
So these helpers are available to files which aren't loaded via
Zeitwerk.
Move ignored lib directories into the `autoload_lib` config.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Rails 7.1.x
1 participant