chore(rails): upgrade to rails 8.1

[adi-herwana-nus]

Summary

Bumps Rails 8.0.5.1 → 8.1.3.1 and moves config.load_defaults to 8.1. Ruby stays on 3.3.5
(3.4 is a separate PR).

Taking the framework defaults in the same PR is deliberate: it surfaces any behaviour change on
staging now, rather than deferring it into a later deploy that would have to be debugged on its own.
All seven of the 8.1 defaults were audited against the codebase first — see §4.

All four application-code items flagged in the pre-bump audit landed as verified no-ops. The work
that actually turned up here was found during the bump, by the test suite and by a full CHANGELOG
sweep — not by the release-notes guide.

---

2. calculated_attributes — the one genuine trap

The checklist had this filed as a routine ref bump ("verify it dispatches 8.1 patches"). It was two
separate problems.

2a. The old pin is a hard boot failure on 8.1, not a misbehaviour

The gem requires its patch file by exact Rails version:

require "calculated_attributes/rails_#{ActiveRecord::VERSION::MAJOR}_#{ActiveRecord::VERSION::MINOR}_patches"

v1.1.1 ships rails_7_0/7_1/7_2/8_0_patches.rb and no 8.1 file → LoadError at boot. Its gemspec
(activerecord >= 7.0.0, < 9) resolves happily, so Bundler gives no warning. This would have surfaced
only as a dead deploy.

2b. v1.2.0 — the ref we must move to — carries a regression that breaks us twice

Upstream commit 9cfea61 ("Support .count on relations that have previously had calculated
applied") adds an override of ActiveRecord::Relation#calculate placed after a bare private:

1. Wrong visibility → infinite recursion. calculate is public API, and ActiveRecord::Querying
   delegates it via delegate :calculate, ..., to: :all. Demoted to private, the delegation cannot
   dispatch, so the call falls to Relation#method_missing, which delegates back to the class:
   Querying#calculate → all.calculate → method_missing → … ⇒ SystemStackError. This broke ~30 specs.

2. Clearing select_values corrupts the relation. The override does self.select_values = []
   in place. Any count on the relation therefore wipes the projections for subsequent use —
   the query that actually failed was not the count but the record load after it, which came back as
   SELECT course_groups.* with an ORDER BY still naming a calculated alias ⇒ PG::UndefinedColumn.

No clean ref exists: the regression (9cfea61, 2026-05-14) predates the 8.1-support commit
(464fd11, 2026-05-29), so every ref carrying rails_8_1_patches.rb also carries the bug.

2c. Fix — config/initializers/calculated_attributes_patch.rb

Removes the override so stock ActiveRecord::Calculations#calculate is used again. Guarded on
source_location, so a future gem version that drops or relocates the override degrades this to a
no-op rather than silently removing a fixed implementation.

With this patch, v1.2.0 is behaviourally identical to the v1.1.1 running in production.

Stock calculate was verified to handle every aggregate shape on a calculated relation — plain
count, limit/offset count, distinct count, count(:id), sum, and count-then-to_a
(the sequence the override corrupted). Plain count even emits a clean SELECT COUNT(*) with no
calculated subqueries, so there is no performance cost either.

Only two production call sites aggregate over a calculated relation — System Admin → Courses and
its per-instance twin, both .count then .map(&:id). Both verified. They are precisely what the
override would have broken: the in-place wipe would have left the following load without projections,
degrading the page to a silent N+1.

⚠️ Worth reporting upstream — the stray private looks unintentional and is not 8.1-specific.

2d. Decoupling the leaderboards from select_values

Independently of the patch, the four leaderboard scopes now order by the underlying SQL expression
rather than by the aliased column, via ApplicationRecord.calculated_expression:

# before
all.calculated(:average_achievement_count).order('average_achievement_count DESC')
# after
all.calculated(:average_achievement_count).order(calculated_expression(:average_achievement_count).desc)

The expression is derived from the same lambda that defines the projection, so the two cannot drift.

• It costs nothing. PostgreSQL emits a byte-identical plan either way —
  Sort Key: ((SubPlan 1)) DESC reuses the same SubPlan as the select list (verified with EXPLAIN).
• It genuinely decouples. The leaderboard specs pass with the patch disabled, i.e. with the
  gem's broken override active.

This means a future gem version that fixes only the visibility bug would leave us with a slow path
(values no longer selected ⇒ per-record lazy re-query) rather than a broken one.

---

3. Application-code changes forced by 8.1

3a. head now raises DoubleRenderError — and it exposed a real bug

Rails 8.1 adds one line to ActionController::Metal#head:

raise ::AbstractController::DoubleRenderError if response_body

On 8.0, head after a render silently overwrote the response; on 8.1 it raises.

Eight controller methods call both super and head. The seven destroy actions are safe — their
super is Course::Assessment::Question::Controller#destroy, which only calls
flag_assessment_not_synced_with_koditsu and never renders. The exception is
User::RegistrationsController#create, whose super is Devise's create, rendering via
respond_with.

The fix hoists the enrol_course validation above super, which is independently correct: a
non-local return out of a transaction block commits it (Rails 7.1+), so the previous ordering
created the user account and then returned 404/403, leaving an orphaned registration. The specs only
ever asserted status codes, so no expectation changed.

3b. Controller-test param encoding now preserves nil

nil.to_query("key") returns key instead of key= (ActiveSupport CHANGELOG: "preventing round
tripping with Rack::Utils.parse_nested_query"). A spec passing explanation: nil therefore reaches
the controller as nil rather than "", hitting the NOT NULL constraint on
course_assessment_question_rubric_based_response_criterions.explanation.

Spec artifact, not a product bug — verified against the client types: on the question side
QuestionRubricGradeData.explanation is a plain string; only the answer side allows null. Real
clients never send null here. Spec now sends '', which is what the form posts.

---

4. Framework defaults

config.load_defaults is moved to 8.1 in this PR, so any behaviour change surfaces on staging now
rather than being deferred into a later, separately-debugged deploy.

bin/rails app:update was not run wholesale — it rewrites bin/, config/boot.rb and every
environment file for no benefit here. Its one artefact,
config/initializers/new_framework_defaults_8_1.rb, is deliberately not committed: every one of
its options ships commented out, so the file is pure comments and adds no behaviour.

The complete 8.1 delta — seven settings, not six

⚠️ The generated defaults file is not the full picture. load_defaults "8.1" sets seven
things; the template documents only six. Read the source, not the template:

railties-8.1.3.1/lib/rails/application/configuration.rb:345

when "8.1"
  load_defaults "8.0"
  self.yjit = !Rails.env.local?          # <- absent from new_framework_defaults_8_1.rb
  action_controller.escape_json_responses = false
  action_controller.action_on_path_relative_redirect = :raise
  active_record.raise_on_missing_required_finder_order_columns = true
  active_support.escape_js_separators_in_json = false
  action_view.render_tracker = :ruby
  action_view.remove_hidden_field_autocomplete = true

Setting	Effect on us
yjit = !Rails.env.local?	No production change. load_defaults 7.2 already set yjit = true unconditionally (configuration.rb:325), so production has had YJIT all along; 8.1 only narrows it, switching it off in dev/test — which is what Rails intends, since reloading and mocking make YJIT unhelpful there. Moot locally anyway: this Ruby build has no RubyVM::YJIT.
escape_json_responses = false	No-op — see below (Yajl).
escape_js_separators_in_json = false	No-op — see below (Yajl).
action_on_path_relative_redirect = :raise	Clear. All 23 controller redirect_to sites audited: 22 use named path helpers (always /…); the only dynamic one, attachment_references_controller.rb:24, redirects to FileUploader#url, a presigned https://… S3 URL (or a /uploads/… path) and already passes allow_other_host: true. Nothing path-relative. (The redirect_to calls under app/jobs/ are TrackableJob's own method, not the controller one.)
raise_on_missing_required_finder_order_columns = true	Adopted for free, and already verified safe. We had decided not to opt in; load_defaults 8.1 includes it. The audit found all 207 concrete models have order columns, so it upgrades an unreachable deprecation into an unreachable error.
render_tracker = :ruby	Irrelevant — no fragment caching in any view, so there are no cache digests to track.
remove_hidden_field_autocomplete = true	Cosmetic, tiny surface. Drops autocomplete="off" from helper-generated hidden inputs. Only 8 server-rendered form views exist (simple_form_for, all under assessment question bundles/groups); everything else is the React SPA.

The two JSON-escaping options are already no-ops here — because of Yajl

An earlier draft of this PR warned that flipping load_defaults 8.1 would stop the JSON renderer
escaping HTML entities and U+2028/9, and called for a security review. That warning was wrong, and
the reason is worth recording.

config/application.rb:11 does require 'yajl/json_gem', which replaces the JSON gem's encoder with
Yajl's C implementation. That bypasses ActiveSupport's escaping entirely:

ActiveSupport::JSON.encode({text: "<b>&</b>"})  =>  {"text":"<b>&</b>"}
                    to_json (Yajl)              =>  {"text":"<b>&</b>"}

Both response paths go through Yajl, so neither is affected by the flag:

• render json: (315 sites) — the renderer's escape: false option is simply ignored by Yajl;
• jbuilder (389 templates) — Jbuilder#target! calls plain @attributes.to_json, without options.

Verified end-to-end through the real controller renderer: ApplicationController.render(json: …)
produces byte-identical output with escape_json_responses set to true and to false, and the
body carries a literal U+2028 (bytes E2 80 A8) today, not an escape sequence — so
escape_js_separators_in_json is inert as well.

There is nothing to test for these two options, on staging or anywhere. The app has been emitting
unescaped JSON since Yajl was adopted. That is safe here because of the architecture, not because of
Rails: this is an API + React SPA, JSON is only ever fetched and JSON.parsed, and there is no
server-rendered JSON embedded in <script> tags and no JSONP endpoint (Rails preserves escaping
when a callback is present anyway). Should anyone later add server-rendered inline JSON, escaping
must be handled at that call site — the framework default will not do it.

We are not adopting raise_on_missing_required_finder_order_columns: every table already has a
primary key by convention, so it guards against something that should not arise.

---

5. Full CHANGELOG sweep

Why it was needed. The pre-bump audit read the release-notes guide. That guide does not mention
the head → DoubleRenderError change at all — its Action Pack section has only Removals,
Deprecations and Notable changes, and this shipped with no deprecation cycle. Per-framework CHANGELOGs
are a strict superset of the guide. Do the CHANGELOG sweep first on the 8.2 hop.

Extracted the complete 8.0 → 8.1 delta from all 12 component CHANGELOGs — 353 entries. Read every
headline for activerecord (103), activesupport (63) and actionpack (47); keyword-filtered the
rest.

Empirical cross-check: across ~5,000 examples the only deprecation the app emits is a Devise
routing one. Nothing else trips an 8.1 path.

Verified clear against the codebase

Change	Verification
Route → non-existing controller now returns 500, not 404	All 685 routed controllers resolve
update_all with DISTINCT/WITH/WITH RECURSIVE deprecated	23 call sites, zero deprecations emitted
:class_name invalid on polymorphic belongs_to	No such declaration
Minimum PostgreSQL raised to 9.5	PG 16.8
Enumerable#sole returns the full tuple	Unused
Underscore-prefixed controller methods become action methods	None defined
Blob autosave change; :azure service removed	Active Storage unused (CarrierWave + S3)

Watch after deploy (no action taken)

• remote_ip no longer ignores X-Forwarded-For entries carrying port info, and link-local ranges
  were added to the default proxy list. Feeds current_sign_in_ip, ActionCable and lograge.
• Rollbar volume — execution wrapping now reports all exceptions, including Exception.
• schema.rb columns are now sorted alphabetically — the next migration will produce a large,
  noisy schema diff. Land that reordering on its own or it will bury a real change.

Known deprecations, deliberately out of scope

• Devise 4.9.4 emits four routing deprecations per boot from devise_for
  (resource received a hash argument …). Devise's internals, not our routes. Removed in 8.2; Devise
  5.0.4 exists and devise-multi_email allows it, but a major Devise bump is its own PR with its own
  auth risk.
• Rack 3.2.7 deprecates :unprocessable_entity → :unprocessable_content (19 usages). Removal is
  "a future Rack", not 8.1/8.2 — a mechanical rename for its own PR.

---

6. Verification

Automated. Run in halves against a freshly reset test DB:

• spec/models spec/services spec/libraries — 3291 examples, 0 failures
• spec/controllers spec/jobs spec/mailers spec/notifiers spec/helpers spec/uploaders spec/components —
  1703 examples; after the fixes above, the only remaining failures are two order-dependent flakes
  (below). With seed 12345, one.
• Leaderboard-specific after the ordering change: leaderboards_controller + spec/helpers —
  114 examples, 0 failures; course/group + course_user — 93 examples, 0 failures, both
  with and without the calculated_attributes patch.
• RuboCop clean on all changed files.

Honest caveats on the local numbers. Two gaps, both deferred to CI by choice:

1. The full-suite run after the leaderboard change was interrupted by a process teardown before
   finishing. Targeted coverage of the changed code is green as listed above.
2. The figures above were all gathered under load_defaults 8.0. The suite has not been run
   locally with load_defaults 8.1 — the flip was audited setting-by-setting against the codebase
   (§4) rather than by a local run.

The authoritative full-suite signal for this branch is therefore CI, not local.

Two pre-existing flakes, neither caused by this PR:

1. koditsu/submissions_concern_spec.rb:84 — fails identically on 8.0; passes in isolation.

2. ai_generated_post_service_spec.rb (:123, :185) — retrieves its just-created record via
   Course::Discussion::Post.last. The model has default_scope { ordered_by_created_at.with_creator },
   so .last means max created_at, not highest id. Meanwhile
   spec/models/concerns/acts_as_contract_spec.rb:67 does
   travel_to(2.minutes.from_now) { post.save! } and, because the suite commits, that future-dated row
   wins .last for every spec running afterwards. Reproduced deterministically on a clean DB:

   Order	Result
   ai_generated alone	12 examples, 0 failures
   polluter first	2 failures
   polluter second	19 examples, 0 failures

   Fix: Have the spec query .last scoped to the individual submission question instead of globally.

Manual, on staging. Leaderboards verified working, including the tiebreaker — courses where the
whole visible ranking is decided by it (metric constant, timestamp strictly ascending down the page).
Other exploratory testing surfaced no issues.

---

7. Not in this PR

• Ruby 3.3.5 → 3.4.x — separate commit, by request.
• config.load_defaults 8.1 — now included in this PR (see §4).
• Devise 5.x and the :unprocessable_content rename — own PRs.
• A guard spec for enqueue_after_transaction_commit — written and mutation-verified during the
  audit but held back; its value is pinning behaviour against the 8.2 default flip.

Sources

• Rails 8.1 release notes · Upgrading Ruby on Rails
• Per-framework CHANGELOGs, actionpack/activerecord/activesupport 8.1.3.1 (the authoritative source — see §5)
• aha-app/calculated_attributes — refs ecaf6c9 (v1.1.1), 9cfea61 (regression), 464fd11 (8.1 support), 992fdd9 (v1.2.0)

[Copilot]

Pull request overview

Upgrades the app from Rails 8.0.5.1 to 8.1.3.1 (including config.load_defaults 8.1) and applies compatibility fixes needed to keep the app booting and tests stable—most notably around the calculated_attributes gem and a Rails 8.1 head double-render behavior change.

Changes:

• Bump Rails to ~> 8.1.0 and update lockfile to Rails 8.1.3.1, with config.load_defaults 8.1.
• Update calculated_attributes to a Rails-8.1-compatible ref and add an initializer to neutralize the upstream Relation#calculate regression; update leaderboard ordering to use the underlying calculated SQL expression.
• Adjust affected controller/spec behavior uncovered during the bump (Devise registration flow ordering, controller spec params, and a flaky .last lookup in a service spec).

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Gemfile	Updates Rails dependency to ~> 8.1.0 and bumps calculated_attributes ref with explanatory notes.
Gemfile.lock	Locks Rails 8.1.3.1 and updated dependency graph (including calculated_attributes 1.2.0).
config/application.rb	Moves config.load_defaults from 8.0 to 8.1.
config/initializers/calculated_attributes_patch.rb	Removes the gem’s problematic Relation#calculate override on Rails boot.
app/models/application_record.rb	Adds calculated_expression helper to order by the calculated SQL expression instead of alias.
app/models/course/group.rb	Updates leaderboard ordering scopes to use calculated_expression(...).
app/models/course_user.rb	Updates leaderboard ordering scopes to use calculated_expression(...).
app/controllers/user/registrations_controller.rb	Reorders enrol-course validation before Devise super to avoid Rails 8.1 double-render errors and transaction pitfalls.
spec/controllers/course/assessment/question/rubric_based_responses_controller_spec.rb	Avoids passing nil where Rails 8.1 param encoding now preserves it.
spec/services/course/assessment/answer/ai_generated_post_service_spec.rb	Avoids global .last by scoping the lookup to the relevant association.

Suppressed comments (1)

app/controllers/user/registrations_controller.rb:70

• Since enrol_course was already loaded/validated, prefer using it directly when creating the enrol request (rather than re-reading invitation_params[:enrol_course_id]) and use resource for clarity. This keeps the creation consistent with the earlier validation and avoids accidental drift if the params change.

          user: @user,
          course_id: invitation_params[:enrol_course_id],
          creator: @user,
          updater: @user

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

app/controllers/user/registrations_controller.rb:72

• Inside the transaction, Course::EnrolRequest.create! uses @user and invitation_params[:enrol_course_id] even though the validated resource and enrol_course are already available. Using the validated objects avoids relying on controller instance variable conventions and ensures the enrol request always points at the course that was checked/published/enrollable.

      if resource.persisted? && enrol_course
        @enrol_request = Course::EnrolRequest.create!(
          user: @user,
          course_id: invitation_params[:enrol_course_id],
          creator: @user,
          updater: @user
        )