Skip to content

AO3-7231 Upgrade gems and configs to Rails 8.0.x #5515

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.

Sign up for GitHub

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

Open
marcus8448 wants to merge 11 commits into
base: master
Choose a base branch
from
Open

AO3-7231 Upgrade gems and configs to Rails 8.0.x #5515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fecbdd4
AO3-7231 Bump Rails version, run update task
marcus8448 Dec 16, 2025
8fd90f0
AO3-7231 Temporarily use alternative rails-i18n source
marcus8448 Dec 16, 2025
b119a34
AO3-7231 Don't touch owned tag set if it hasn't been persisted yet
marcus8448 Dec 18, 2025
d021bef
AO3-7231 Attempt a test run with 8.0 defaults
marcus8448 Dec 18, 2025
0f97a02
AO3-7231 Set 8.0 defaults
marcus8448 Dec 18, 2025
691cfe8
AO3-7231 Try deleting enqueue_after_transaction_commit?
marcus8448 Dec 18, 2025
579fc0e
AO3-7231 Set enqueue_after_transaction_commit on jobs
marcus8448 Dec 18, 2025
8bf5c17
AO3-7231 Rubocop
marcus8448 Dec 19, 2025
7e33f1e
Merge branch 'master' into AO3-7231
marcus8448 Jan 10, 2026
3c079f5
AO3-7231 Pull from main rails-i18n repository
marcus8448 Jan 10, 2026
c3f8b1c
AO3-7231 Always enable enqueue_after_transaction_commit
marcus8448 Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ gem 'test-unit', '~> 3.2'

gem 'bundler'

gem "rails", "~> 7.2"
gem "rails-i18n"
gem "rails", "~> 8.0.0"
gem "rails-i18n", "~> 8.0", git: "https://github.com/svenfuchs/rails-i18n", ref: "54c1c7c2fdcc311427ec6f1dadd298a60db1ddef"
Copy link
Contributor Author

@marcus8448 marcus8448 Jan 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crashes without an unreleased bugfix. Technically this version targets rails 8.1, but it looks like there are only new keys for 8.1 (none changed/removed) so it shouldn't cause any problems.

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do the rails-i18n upgrade steps in https://github.com/otwcode/otwarchive/blob/master/config/locales/README.md

gem "rack", "~> 2.2"
gem "sprockets", "< 4"

Expand Down Expand Up @@ -107,13 +107,13 @@ gem 'kgio', '2.10.0'
gem "marcel", "1.0.2"

# Library for helping run pt-online-schema-change commands:
gem "departure", "~> 6.8"
gem "departure", "~> 8.0"
Copy link
Contributor Author

@marcus8448 marcus8448 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't see anything of note in the changelog. Note that 8.0.1 seems to have not been published to rubygems (if it does get published I would like to pull it in).


gem "rack-timeout"
gem "puma_worker_killer"

group :test do
gem "rspec-rails", "~> 6.0"
gem "rspec-rails", "~> 8.0"
Copy link
Contributor Author

@marcus8448 marcus8448 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changelog, nothing stood out as well.

gem 'pickle'
gem "shoulda-matchers"
gem "capybara"
Expand Down
145 changes: 76 additions & 69 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ GIT
rails-observers (0.2.0)
activemodel (>= 4.2)

GIT
remote: https://github.com/svenfuchs/rails-i18n
revision: 54c1c7c2fdcc311427ec6f1dadd298a60db1ddef
ref: 54c1c7c2fdcc311427ec6f1dadd298a60db1ddef
specs:
rails-i18n (8.1.0)
i18n (>= 0.7, < 2)
railties (>= 8.0.0, < 9)

GIT
remote: https://github.com/winston/google_visualr
revision: 17b97114a345baadd011e7b442b9a6c91a2b7ab5
Expand All @@ -15,70 +24,69 @@ GEM
remote: https://rubygems.org/
specs:
aaronh-chronic (0.3.9)
actioncable (7.2.2.2)
actionpack (= 7.2.2.2)
activesupport (= 7.2.2.2)
actioncable (8.0.4)
actionpack (= 8.0.4)
activesupport (= 8.0.4)
nio4r (~> 2.0)
websocket-driver (>= 0.6.1)
zeitwerk (~> 2.6)
actionmailbox (7.2.2.2)
actionpack (= 7.2.2.2)
activejob (= 7.2.2.2)
activerecord (= 7.2.2.2)
activestorage (= 7.2.2.2)
activesupport (= 7.2.2.2)
actionmailbox (8.0.4)
actionpack (= 8.0.4)
activejob (= 8.0.4)
activerecord (= 8.0.4)
activestorage (= 8.0.4)
activesupport (= 8.0.4)
mail (>= 2.8.0)
actionmailer (7.2.2.2)
actionpack (= 7.2.2.2)
actionview (= 7.2.2.2)
activejob (= 7.2.2.2)
activesupport (= 7.2.2.2)
actionmailer (8.0.4)
actionpack (= 8.0.4)
actionview (= 8.0.4)
activejob (= 8.0.4)
activesupport (= 8.0.4)
mail (>= 2.8.0)
rails-dom-testing (~> 2.2)
actionpack (7.2.2.2)
actionview (= 7.2.2.2)
activesupport (= 7.2.2.2)
actionpack (8.0.4)
actionview (= 8.0.4)
activesupport (= 8.0.4)
nokogiri (>= 1.8.5)
racc
rack (>= 2.2.4, < 3.2)
rack (>= 2.2.4)
rack-session (>= 1.0.1)
rack-test (>= 0.6.3)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
useragent (~> 0.16)
actionpack-page_caching (1.2.4)
actionpack (>= 4.0.0)
actiontext (7.2.2.2)
actionpack (= 7.2.2.2)
activerecord (= 7.2.2.2)
activestorage (= 7.2.2.2)
activesupport (= 7.2.2.2)
actiontext (8.0.4)
actionpack (= 8.0.4)
activerecord (= 8.0.4)
activestorage (= 8.0.4)
activesupport (= 8.0.4)
globalid (>= 0.6.0)
nokogiri (>= 1.8.5)
actionview (7.2.2.2)
activesupport (= 7.2.2.2)
actionview (8.0.4)
activesupport (= 8.0.4)
builder (~> 3.1)
erubi (~> 1.11)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
active_record_query_trace (1.8.3)
activerecord (>= 6.0.0)
activejob (7.2.2.2)
activesupport (= 7.2.2.2)
activejob (8.0.4)
activesupport (= 8.0.4)
globalid (>= 0.3.6)
activemodel (7.2.2.2)
activesupport (= 7.2.2.2)
activerecord (7.2.2.2)
activemodel (= 7.2.2.2)
activesupport (= 7.2.2.2)
activemodel (8.0.4)
activesupport (= 8.0.4)
activerecord (8.0.4)
activemodel (= 8.0.4)
activesupport (= 8.0.4)
timeout (>= 0.4.0)
activestorage (7.2.2.2)
actionpack (= 7.2.2.2)
activejob (= 7.2.2.2)
activerecord (= 7.2.2.2)
activesupport (= 7.2.2.2)
activestorage (8.0.4)
actionpack (= 8.0.4)
activejob (= 8.0.4)
activerecord (= 8.0.4)
activesupport (= 8.0.4)
marcel (~> 1.0)
activesupport (7.2.2.2)
activesupport (8.0.4)
base64
benchmark (>= 0.3)
bigdecimal
Expand All @@ -90,6 +98,7 @@ GEM
minitest (>= 5.1)
securerandom (>= 0.3)
tzinfo (~> 2.0, >= 2.0.5)
uri (>= 0.13.1)
acts_as_list (0.9.19)
activerecord (>= 3.0)
addressable (2.8.7)
Expand Down Expand Up @@ -220,10 +229,10 @@ GEM
database_cleaner-core (~> 2.0.0)
database_cleaner-core (2.0.1)
date (3.4.1)
departure (6.8.0)
activerecord (>= 6.0.0, < 7.3.0, != 7.0.0)
departure (8.0.0)
activerecord (>= 7.2.0)
mysql2 (>= 0.4.0, < 0.6.0)
railties (>= 6.0.0, < 7.3.0, != 7.0.0)
railties (>= 7.2.0)
devise (4.9.3)
bcrypt (~> 3.0)
orm_adapter (~> 0.1)
Expand Down Expand Up @@ -459,20 +468,20 @@ GEM
rackup (1.0.1)
rack (< 3)
webrick
rails (7.2.2.2)
actioncable (= 7.2.2.2)
actionmailbox (= 7.2.2.2)
actionmailer (= 7.2.2.2)
actionpack (= 7.2.2.2)
actiontext (= 7.2.2.2)
actionview (= 7.2.2.2)
activejob (= 7.2.2.2)
activemodel (= 7.2.2.2)
activerecord (= 7.2.2.2)
activestorage (= 7.2.2.2)
activesupport (= 7.2.2.2)
rails (8.0.4)
actioncable (= 8.0.4)
actionmailbox (= 8.0.4)
actionmailer (= 8.0.4)
actionpack (= 8.0.4)
actiontext (= 8.0.4)
actionview (= 8.0.4)
activejob (= 8.0.4)
activemodel (= 8.0.4)
activerecord (= 8.0.4)
activestorage (= 8.0.4)
activesupport (= 8.0.4)
bundler (>= 1.15.0)
railties (= 7.2.2.2)
railties (= 8.0.4)
rails-controller-testing (1.0.5)
actionpack (>= 5.0.1.rc1)
actionview (>= 5.0.1.rc1)
Expand All @@ -484,16 +493,14 @@ GEM
rails-html-sanitizer (1.6.2)
loofah (~> 2.21)
nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0)
rails-i18n (7.0.9)
i18n (>= 0.7, < 2)
railties (>= 6.0.0, < 8)
railties (7.2.2.2)
actionpack (= 7.2.2.2)
activesupport (= 7.2.2.2)
railties (8.0.4)
actionpack (= 8.0.4)
activesupport (= 8.0.4)
irb (~> 1.13)
rackup (>= 1.0.0)
rake (>= 12.2)
thor (~> 1.0, >= 1.2.2)
tsort (>= 0.2)
zeitwerk (~> 2.6)
rainbow (3.1.1)
raindrops (0.20.1)
Expand Down Expand Up @@ -538,10 +545,10 @@ GEM
rspec-mocks (3.13.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-rails (6.1.5)
actionpack (>= 6.1)
activesupport (>= 6.1)
railties (>= 6.1)
rspec-rails (8.0.2)
actionpack (>= 7.2)
activesupport (>= 7.2)
railties (>= 7.2)
rspec-core (~> 3.13)
rspec-expectations (~> 3.13)
rspec-mocks (~> 3.13)
Expand Down Expand Up @@ -697,7 +704,7 @@ DEPENDENCIES
cucumber-timecop
dalli
database_cleaner
departure (~> 6.8)
departure (~> 8.0)
devise
devise-async
devise-pwned_password
Expand Down Expand Up @@ -739,17 +746,17 @@ DEPENDENCIES
rack-attack
rack-dev-mark (>= 0.7.8)
rack-timeout
rails (~> 7.2)
rails (~> 8.0.0)
rails-controller-testing
rails-i18n
rails-i18n (~> 8.0)!
rails-observers!
redis (< 5.0)
redis-namespace
resque (>= 1.14.0)
resque-scheduler
rest-client (~> 2.1.0)
rollout
rspec-rails (~> 6.0)
rspec-rails (~> 8.0)
rubocop (= 1.22.3)
rubocop-rails (= 2.12.4)
rubocop-rspec (= 2.6.0)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/errors_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class ErrorsController < ApplicationController
%w[403 404 422 500].each do |error_code|
%w[400 403 404 422 500].each do |error_code|
Copy link
Member

@brianjaustin brianjaustin Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to the new version, or just completeness?

Copy link
Contributor Author

@marcus8448 marcus8448 Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More for completeness - the new version just wanted /public/400.html (relevant commit), but when making a copy with the archive styling I took a look at what we did for error 500 and saw this controller. If the handling is redundant considering the static pages then I can just delete it.

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion we should decide on one place to handle it, probably the public file, not here.

Copy link
Member

@brianjaustin brianjaustin Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are multiple error codes here that have been there already, let's do this in a follow-up ticket (something like "standardise HTTP error handling" maybe)

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean for this one specific new error 400, just don't add it to the ErrorsController. The other errors are fine to stay here

define_method error_code.to_sym do
render error_code, status: error_code.to_i, formats: :html
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/tagset_models/tag_set.rb
View file
Open in desktop
Copy link
Contributor Author

@marcus8448 marcus8448 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails 8.0 now saves the parent model (tag set) before the child model (owned tag set), so we can't always touch it here (after_save). In that case the child model (owned tag set) is going to be saved after anyways, so it's fine to just skip touching.

This change looks to be a side effect of rails/rails#49847, but I think this ordering makes more sense so we were probably just depending on buggy behaviour.
Example, if that explanation doesn't make sense: https://gist.github.com/marcus8448/9459bcc0536f2baf0704cbea91fc9d0b

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at your reproduction, did you have a poke around whether any other models are affected by the parent after save/after create being moved around?

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Feel free to just say "no" and we will rely on the automatic tests)

Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def remove_from_set(tags_to_remove)
return unless tags_to_remove.present?
self.set_taggings.where(tag_id: tags_to_remove.map(&:id)).delete_all
remove_tags_from_autocomplete(tags_to_remove)
owned_tag_set&.touch
owned_tag_set.touch if owned_tag_set&.persisted?
end

def add_to_set(tags_to_add)
Expand All @@ -127,7 +127,7 @@ def add_to_set(tags_to_add)
self.set_taggings.create(tag_id: tag.id)
end
add_tags_to_autocomplete(tags_to_add)
owned_tag_set&.touch
owned_tag_set.touch if owned_tag_set&.persisted?
end

# Tags must already exist unless they are being added to an owned tag set
Expand Down
3 changes: 3 additions & 0 deletions app/views/errors/400.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<h2 class="heading">Error 400</h2>
<h3 class="heading">Bad Request</h3>
<p>The server cannot process the request due to a client error.</p>
2 changes: 2 additions & 0 deletions bin/dev
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/usr/bin/env ruby
exec "./bin/rails", "server", *ARGV
13 changes: 5 additions & 8 deletions bin/setup
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
require "fileutils"

APP_ROOT = File.expand_path("..", __dir__)
APP_NAME = "otwarchive"

def system!(*args)
system(*args, exception: true)
Expand All @@ -14,7 +13,6 @@ FileUtils.chdir APP_ROOT do
# Add necessary setup steps to this file.

puts "== Installing dependencies =="
system! "gem install bundler --conservative"
system("bundle check") || system!("bundle install")

# puts "\n== Copying sample files =="
Expand All @@ -28,10 +26,9 @@ FileUtils.chdir APP_ROOT do
puts "\n== Removing old logs and tempfiles =="
system! "bin/rails log:clear tmp:clear"

puts "\n== Restarting application server =="
system! "bin/rails restart"

# puts "\n== Configuring puma-dev =="
# system "ln -nfs #{APP_ROOT} ~/.puma-dev/#{APP_NAME}"
# system "curl -Is https://#{APP_NAME}.test/up | head -n 1"
unless ARGV.include?("--skip-server")
puts "\n== Starting development server =="
STDOUT.flush # flush the output before exec(2) so that it displays
exec "bin/dev"
end
end
5 changes: 1 addition & 4 deletions config/application.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Application < Rails::Application
# These settings can be overridden in specific environments using the files
# in config/environments, which are processed later.

config.load_defaults 7.2
config.load_defaults 8.0
Copy link
Contributor Author

@marcus8448 marcus8448 Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New framework defaults

###
# Specifies whether `to_time` methods preserve the UTC offset of their receivers or preserves the timezone.
# If set to `:zone`, `to_time` methods will use the timezone of their receivers.
# If set to `:offset`, `to_time` methods will use the UTC offset.
# If `false`, `to_time` methods will convert to the local system UTC offset instead.
#++
Rails.application.config.active_support.to_time_preserves_timezone = :zone

I looked at all the uses of to_time and nothing stood out as being timezone (vs offset) dependent. Anything coming into or out of the database is normalized to UTC anyways and duration calculations shouldn't care about the zone.

###
# When both `If-Modified-Since` and `If-None-Match` are provided by the client
# only consider `If-None-Match` as specified by RFC 7232 Section 6.
# If set to `false` both conditions need to be satisfied.
#++
Rails.application.config.action_dispatch.strict_freshness = true

I don't see anything wrong with this.

###
# Set `Regexp.timeout` to `1`s by default to improve security over Regexp Denial-of-Service attacks.
#++
Regexp.timeout = 1

I think 1s should be plenty of time. Do we have any important complex regexps?

Copy link
Contributor

@Bilka2 Bilka2 Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_time: I'm slightly concerned about the use in time_ago_in_words, since it uses Time.now which is not zone aware. Can we switch it to Time.current just for safety? Servers and Rails are in UTC (they weren't always...) so nothing should break, but just in case.

Do we have any important complex regexps?

not sure if complex, but word counting is very important, likely to have very large input and is a regex (ref WordCounter). Maybe test it locally with a very very long chapter and see whether it's an issue?


%w[
app/models/challenge_models
Expand Down Expand Up @@ -105,9 +105,6 @@ class Application < Rails::Application
# Use Resque to run ActiveJobs (including sending delayed mail):
config.active_job.queue_adapter = :resque

# TODO: Remove with Rails 8.0 where this option will be deprecated
config.active_job.enqueue_after_transaction_commit = :always

config.active_model.i18n_customize_full_message = true

config.action_mailer.default_url_options = { host: ArchiveConfig.APP_HOST, protocol: "https" }
Expand Down
Loading
Loading