From 5dbe7713841c59c370b1e9b8e469ae00e5f98e64 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 18:53:32 +0000 Subject: [PATCH 1/2] Load the Rails 7.2 framework defaults Bumps `config.load_defaults` to 7.2 and retires config/initializers/new_framework_defaults_7_2.rb, which #907 shipped with every option commented out. Dumped the affected settings before and after rather than reading the release notes. `load_defaults "7.2"` sets exactly five things: yjit false -> true active_job.enqueue_after_transaction_commit :never -> :default active_record.postgresql_adapter_decode_dates false -> true active_record.validate_migration_timestamps false -> true active_storage.web_image_content_types unset -> unset Four of the five are inert here: - `yjit` does nothing until the Ruby bump. The initializer is guarded by `defined?(RubyVM::YJIT.enable)`, and `enable` arrived in Ruby 3.3; on 3.2.3 the constant exists but the method does not. It starts mattering in PR 6. - Active Storage is not loaded -- the require is commented out in config/application.rb -- so `respond_to?(:active_storage)` is false and Rails skips that line entirely. - `postgresql_adapter_decode_dates` only affects queries carrying no type information. The one raw query in the app, `Food.fts`, runs through `find_by_sql`, which casts through the model's attribute types, and selects no date columns in any case. - `validate_migration_timestamps` raises on a migration dated more than a day ahead of now. The newest migration here is from October 2017. That leaves `enqueue_after_transaction_commit`, which is a real change: `:never` enqueued immediately even inside an open transaction, and `:default` asks the adapter. Sidekiq answers true -- both its own adapter and the one still shipped in activejob define it that way, so it does not matter which wins the load order -- meaning a job enqueued inside a transaction now waits for the commit, and a rollback drops it rather than leaving Sidekiq holding a job whose rows were never written. No existing call site changes behaviour. Active Job is reached from exactly three places: `TrackableUsage`'s `after_commit`, which by definition runs with no transaction open; `DataExportSchedulesController#create`; and six `deliver_later` calls, all of them inside Sidekiq workers or rake tasks. None sits in a transaction. Everything else in app/jobs is `perform_async` on a plain Sidekiq worker, which never enters Active Job. So this is protection for code not yet written, which is why it comes with a spec of its own. The suite is unaffected for a reason worth recording: DatabaseCleaner's :transaction strategy opens its wrapping transaction with `joinable: false`, and `ActiveRecord.after_all_transactions_commit` skips non-joinable transactions, so a top-level `perform_later` in a spec still enqueues immediately. Had that wrapper been joinable, every job spec would have been waiting on a commit that never comes. Also settles the deploy-ordering question the upgrade plan flagged for this step: none of the five options touches an on-disk format. `active_support.cache_format_version` is 7.1 before and after -- 7.2 does not change its default, and config/application.rb pins it explicitly anyway -- so there is no old-process/new-process cache incompatibility and this needs no special rollout. Verified: 327 examples, 0 failures; standardrb, erblint and zeitwerk:check clean; boots in development, test and production. The three new specs were checked in both directions -- on `load_defaults 7.1` the two transaction examples fail. Co-Authored-By: Claude Opus 5 (1M context) --- backend/config/application.rb | 2 +- .../new_framework_defaults_7_2.rb | 70 ------------------- .../enqueue_after_transaction_commit_spec.rb | 43 ++++++++++++ 3 files changed, 44 insertions(+), 71 deletions(-) delete mode 100644 backend/config/initializers/new_framework_defaults_7_2.rb create mode 100644 backend/spec/jobs/enqueue_after_transaction_commit_spec.rb diff --git a/backend/config/application.rb b/backend/config/application.rb index 2f848fe1e..90f9d754b 100644 --- a/backend/config/application.rb +++ b/backend/config/application.rb @@ -22,7 +22,7 @@ module Flaredown class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. - config.load_defaults 7.1 + config.load_defaults 7.2 config.add_autoload_paths_to_load_path = false config.active_support.cache_format_version = 7.1 diff --git a/backend/config/initializers/new_framework_defaults_7_2.rb b/backend/config/initializers/new_framework_defaults_7_2.rb deleted file mode 100644 index b549c4a25..000000000 --- a/backend/config/initializers/new_framework_defaults_7_2.rb +++ /dev/null @@ -1,70 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file eases your Rails 7.2 framework defaults upgrade. -# -# Uncomment each configuration one by one to switch to the new default. -# Once your application is ready to run with all new defaults, you can remove -# this file and set the `config.load_defaults` to `7.2`. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. -# https://guides.rubyonrails.org/upgrading_ruby_on_rails.html - -### -# Controls whether Active Job's `#perform_later` and similar methods automatically defer -# the job queuing to after the current Active Record transaction is committed. -# -# Example: -# Topic.transaction do -# topic = Topic.create(...) -# NewTopicNotificationJob.perform_later(topic) -# end -# -# In this example, if the configuration is set to `:never`, the job will -# be enqueued immediately, even though the `Topic` hasn't been committed yet. -# Because of this, if the job is picked up almost immediately, or if the -# transaction doesn't succeed for some reason, the job will fail to find this -# topic in the database. -# -# If `enqueue_after_transaction_commit` is set to `:default`, the queue adapter -# will define the behaviour. -# -# Note: Active Job backends can disable this feature. This is generally done by -# backends that use the same database as Active Record as a queue, hence they -# don't need this feature. -#++ -# Rails.application.config.active_job.enqueue_after_transaction_commit = :default - -### -# Adds image/webp to the list of content types Active Storage considers as an image -# Prevents automatic conversion to a fallback PNG, and assumes clients support WebP, as they support gif, jpeg, and png. -# This is possible due to broad browser support for WebP, but older browsers and email clients may still not support -# WebP. Requires imagemagick/libvips built with WebP support. -#++ -# Rails.application.config.active_storage.web_image_content_types = %w[image/png image/jpeg image/gif image/webp] - -### -# Enable validation of migration timestamps. When set, an ActiveRecord::InvalidMigrationTimestampError -# will be raised if the timestamp prefix for a migration is more than a day ahead of the timestamp -# associated with the current time. This is done to prevent forward-dating of migration files, which can -# impact migration generation and other migration commands. -# -# Applications with existing timestamped migrations that do not adhere to the -# expected format can disable validation by setting this config to `false`. -#++ -# Rails.application.config.active_record.validate_migration_timestamps = true - -### -# Controls whether the PostgresqlAdapter should decode dates automatically with manual queries. -# -# Example: -# ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.select_value("select '2024-01-01'::date") #=> Date -# -# This query used to return a `String`. -#++ -# Rails.application.config.active_record.postgresql_adapter_decode_dates = true - -### -# Enables YJIT as of Ruby 3.3, to bring sizeable performance improvements. If you are -# deploying to a memory constrained environment you may want to set this to `false`. -#++ -# Rails.application.config.yjit = true diff --git a/backend/spec/jobs/enqueue_after_transaction_commit_spec.rb b/backend/spec/jobs/enqueue_after_transaction_commit_spec.rb new file mode 100644 index 000000000..c90baf64b --- /dev/null +++ b/backend/spec/jobs/enqueue_after_transaction_commit_spec.rb @@ -0,0 +1,43 @@ +require "rails_helper" + +# `load_defaults 7.2` sets active_job.enqueue_after_transaction_commit to :default, +# which hands the decision to the queue adapter. Sidekiq keeps its queue in Redis +# rather than the Active Record database, so it takes the abstract adapter's `true`: +# a job enqueued inside a transaction waits for the commit, and a rollback drops it +# instead of leaving Sidekiq holding a job whose rows were never written. +# +# HelloWorldJob is used because it is inert -- nothing else references it, and +# performing it only writes a log line. +describe "enqueuing an Active Job inside an Active Record transaction" do + include ActiveJob::TestHelper + + it "waits for the transaction to commit" do + ActiveRecord::Base.transaction do + HelloWorldJob.perform_later + + expect(enqueued_jobs).to be_empty + end + + expect(enqueued_jobs.size).to eq(1) + end + + it "drops the job when the transaction rolls back" do + ActiveRecord::Base.transaction do + HelloWorldJob.perform_later + + raise ActiveRecord::Rollback + end + + expect(enqueued_jobs).to be_empty + end + + # DatabaseCleaner's :transaction strategy wraps every example in a transaction + # opened with `joinable: false`, and ActiveRecord.after_all_transactions_commit + # skips non-joinable transactions. So an enqueue outside an explicit transaction + # still happens immediately, which is what the rest of the suite assumes. + it "enqueues immediately outside a transaction" do + HelloWorldJob.perform_later + + expect(enqueued_jobs.size).to eq(1) + end +end From 401d446167f271daac6edfd10606d867c46d12da Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 18:53:44 +0000 Subject: [PATCH 2/2] Only inspect :id on Active Record objects in production `config.active_record.attributes_for_inspect` is a 7.2 addition that `load_defaults` does not set; Rails puts it in the generated production.rb instead, and #907 deferred it here on the grounds that it changes behaviour and so belongs with the defaults flip. It is worth taking rather than skipping. `inspect` on an Active Record object renders every column, and that string is how attribute values reach logs and Bugsnag reports -- on this app those values are health data. With `[:id]`, `Profile.new(birth_date: ...).inspect` is `#` instead of a line carrying a date of birth, a sex, a time zone and a screen name. `full_inspect` still prints everything for when you want it. One exception, checked rather than assumed: `User` is unaffected, because Devise::Models::Authenticatable overrides `inspect` and wins the method lookup. Devise's version filters through `serializable_hash`, so credentials were never in that output, but the remaining columns still are. Every other Active Record model in the app takes the new behaviour. Development and test keep the `:all` default, where the whole point of `inspect` is to see the record. Verified: booted RAILS_ENV=production and confirmed `ActiveRecord::Base.attributes_for_inspect == [:id]`, `Profile#inspect` redacted and `Profile#full_inspect` complete. 327 examples, 0 failures; standardrb clean. Co-Authored-By: Claude Opus 5 (1M context) --- backend/config/environments/production.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/backend/config/environments/production.rb b/backend/config/environments/production.rb index 762a2dce0..60c2a6b08 100644 --- a/backend/config/environments/production.rb +++ b/backend/config/environments/production.rb @@ -79,6 +79,11 @@ # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false + # Only use :id for inspections in production. `inspect` on an Active Record + # object otherwise renders every column, which is how attribute values reach + # logs and exception reports -- and on this app those values are health data. + config.active_record.attributes_for_inspect = [:id] + # Enable DNS rebinding protection and other `Host` header attacks. # config.hosts = [ # "example.com", # Allow requests from example.com