Drop legacy connection support by tagliala · Pull Request #8883 · activeadmin/activeadmin

tagliala · 2025-12-12T21:47:31Z

Replace all uses of ActiveRecord::Base.connection with
ActiveRecord::Base.with_connection, following Rails guidelines.

Direct connection usage is now dropped for Rails < 7.2, and
attempting to call .connection directly raises in Rails 8.x.
This aligns our codebase with current Rails recommendations and ensures
compatibility going forward.

Tests are updated to handle connection access correctly, running
some cases outside transactional fixtures to validate quoting and
order clause methods.

Close #8881

References:

Rails PR: Add config.active_record.permanent_connection_checkout setting rails/rails#51349
Connection Handling:
https://github.com/rails/rails/blob/v7.2.1/activerecord/lib/active_record/connection_handling.rb#L259

codecov · 2025-12-12T21:50:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.08%. Comparing base (1ef4ad0) to head (29c3596).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #8883   +/-   ##
=======================================
  Coverage   99.08%   99.08%           
=======================================
  Files         139      139           
  Lines        4045     4046    +1     
=======================================
+ Hits         4008     4009    +1     
  Misses         37       37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR addresses the Rails 7.2+ deprecation of ActiveRecord::Base.connection by updating the resource_quoted_column_name method to use the new with_connection API when available, preventing issues when permanent connection checkout is set to :disallowed.

Key Changes

Modified resource_quoted_column_name to use with_connection for Rails 7.2+ with fallback to .connection for older versions
Configured test environment to enforce :disallowed permanent connection checkout on Rails 7.2+
Added tests to verify the fix works correctly without transactional fixtures

Reviewed changes

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

File	Description
`lib/active_admin/resource.rb`	Updated `resource_quoted_column_name` to use Rails 7.2+ `with_connection` API with backward compatibility
`spec/support/rails_template.rb`	Added configuration to disallow permanent connection checkout in Rails 7.2+ test environment
`spec/unit/resource_spec.rb`	Added test with transactional fixture opt-out to validate connection handling
`spec/unit/order_clause_spec.rb`	Added transactional fixture opt-out to test that uses `resource_quoted_column_name`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

spec/unit/resource_spec.rb

spec/unit/order_clause_spec.rb

tagliala · 2026-01-04T23:39:33Z

Converting back to draft because this PR will be affected by #8908

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

spec/support/rails_template.rb

Copilot

Pull request overview

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

spec/support/rails_template.rb

Copilot · 2026-03-06T09:58:59Z

@tagliala I've opened a new pull request, #8958, to work on those changes. Once the pull request is ready, I'll request review from you.

tagliala · 2026-03-06T20:09:08Z

@javierjulio this change should also be good

javierjulio

Thanks! I didn't know what or where uses_transaction was coming from. I suggest adding the following method source link to the comment.

https://github.com/rails/rails/blob/v8.1.2/activerecord/lib/active_record/test_fixtures.rb#L100-L111

javierjulio · 2026-03-06T21:42:50Z

spec/unit/order_clause_spec.rb

+
+    # Prevents automatically wrapping this test in a transaction. Check that we use the proper method to get a connection from the pool to quote the clause.
+    # We don't want to run the test using a leased connection https://github.com/rails/rails/blob/v7.2.3/activerecord/lib/active_record/test_fixtures.rb#L161
+    uses_transaction "#to_sql prepends table name"


Are we sure this is working as intended? If so, all good with the changes.

@copilot why do we need, if we need, uses_transaction here?

@javierjulio I'm not the original author of that change

Good catch, btw.

Maybe this has some effect on 7.1 or previously tested versions

I'm going to remove both those lines, because they do not change how the test is being performed in no versions of rails tested. I did expect a failure by removing those lines, but they didn't fail

When disallowed is set at configuration level, specs don't pass if we are not using with_connection properly

Example:

Failures: 1) ActiveAdmin::Resource#resource_quoted_column_name should return quote argument Failure/Error: resource_class.connection.quote_column_name(column) ActiveRecord::ActiveRecordError: Called deprecated `ActiveRecord::Base.connection` method. Either use `with_connection` or `lease_connection`.

@tagliala ok great, sounds good, thanks again!

Copilot · 2026-03-06T22:16:48Z

@tagliala I've opened a new pull request, #8962, to work on those changes. Once the pull request is ready, I'll request review from you.

Replace all uses of `ActiveRecord::Base.connection` with `ActiveRecord::Base.with_connection`, following Rails guidelines. Direct connection usage is now dropped for Rails < 7.2, and attempting to call .connection directly raises in Rails 8.x. This aligns our codebase with current Rails recommendations and ensures compatibility going forward. Tests are updated to handle connection access correctly, running some cases outside transactional fixtures to validate quoting and order clause methods. Close #8881 References: - Rails PR: rails/rails#51349 - Connection Handling: https://github.com/rails/rails/blob/v7.2.1/activerecord/lib/active_record/connection_handling.rb#L259

…manent_connection_checkout = :disallowed` (#8958) * Initial plan * Fix spec to use lease_connection instead of connection Replace ActiveRecord::Base.connection with ActiveRecord::Base.lease_connection in the in_paginated_batches spec to be compatible with permanent_connection_checkout = :disallowed set in test environment. Co-authored-by: tagliala <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: tagliala <[email protected]>

…specs (#8962) * Initial plan * Remove unnecessary uses_transaction annotations from specs The uses_transaction calls were added alongside with_connection in resource_quoted_column_name, but are not needed because: - with_connection is designed to work correctly within transactional tests - quote_column_name is a pure string operation with no DB writes - No special transaction handling is required for these tests Co-authored-by: tagliala <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: tagliala <[email protected]>

Use with_connection/lease_connection when available (Rails >= 7.2), falling back to connection for older Rails versions. Conditionally set permanent_connection_checkout config only for Rails >= 7.2. This enables the backport to work on the 3-0-stable branch which still supports Rails < 7.2. Co-authored-by: tagliala <[email protected]>

Cherry-pick of e602ef7 applied to 3-0-stable with conflict resolution. Uses with_connection/lease_connection when available (Rails >= 7.2), falling back to connection for older Rails versions. Conditionally sets permanent_connection_checkout config only for Rails >= 7.2. Co-authored-by: tagliala <[email protected]>

Use with_connection/lease_connection when available (Rails >= 7.2), falling back to connection for older Rails versions. Conditionally set permanent_connection_checkout config only for Rails >= 7.2. This enables the backport to work on the 3-0-stable branch which still supports Rails < 7.2. Co-authored-by: tagliala <[email protected]>

Backport PR #8883 with Rails version conditionals Use with_connection/lease_connection when available (Rails >= 7.2), falling back to connection for older Rails versions. Conditionally set permanent_connection_checkout config only for Rails >= 7.2. This enables the backport to work on the 3-0-stable branch which still supports Rails < 7.2. Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: tagliala <[email protected]>

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch 2 times, most recently from 3761303 to 2a83d8d Compare December 13, 2025 10:44

tagliala changed the title ~~Bugfix/8881 permanent connection cherrypick~~ Fix for :disallowed permanent connection checkout Dec 13, 2025

tagliala changed the title ~~Fix for :disallowed permanent connection checkout~~ Fix :disallowed permanent connection checkout Dec 13, 2025

tagliala added the type bug fix label Dec 13, 2025

tagliala requested a review from Copilot December 13, 2025 12:46

Copilot started reviewing on behalf of tagliala December 13, 2025 12:46 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

spec/unit/resource_spec.rb Outdated Show resolved Hide resolved

spec/unit/order_clause_spec.rb Outdated Show resolved Hide resolved

tagliala requested review from javierjulio and mgrunberg December 13, 2025 13:31

tagliala changed the title ~~Fix :disallowed permanent connection checkout~~ Use with_connection for column quoting on Rails 7.2+ Dec 13, 2025

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch 2 times, most recently from a90e6a4 to 565802c Compare December 20, 2025 10:21

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch 2 times, most recently from 6e7a0a2 to dbab39e Compare January 3, 2026 11:00

tagliala marked this pull request as draft January 4, 2026 23:39

javierjulio added the do not merge A PR will have a failed check until this label is removed label Jan 6, 2026

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch 2 times, most recently from a2cc3aa to 9bdd915 Compare January 6, 2026 10:24

tagliala changed the title ~~Use with_connection for column quoting on Rails 7.2+~~ Drop legacy connection support Jan 6, 2026

tagliala marked this pull request as ready for review January 6, 2026 10:27

tagliala removed the do not merge A PR will have a failed check until this label is removed label Jan 6, 2026

tagliala requested a review from Copilot January 6, 2026 10:27

Copilot started reviewing on behalf of tagliala January 6, 2026 10:28 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

spec/support/rails_template.rb Outdated Show resolved Hide resolved

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch 3 times, most recently from 11124da to 3bc360d Compare January 6, 2026 15:29

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from f0684cc to 0d692fc Compare January 28, 2026 09:12

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from 0d692fc to 6c25b4f Compare February 8, 2026 09:54

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from 6c25b4f to 381696c Compare February 17, 2026 18:46

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from c0376a4 to 563ca72 Compare March 5, 2026 08:55

tagliala requested a review from Copilot March 6, 2026 09:24

Copilot started reviewing on behalf of tagliala March 6, 2026 09:25 View session

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from 78c8d98 to e6e6273 Compare March 6, 2026 09:25

Copilot AI reviewed Mar 6, 2026

View reviewed changes

spec/support/rails_template.rb Show resolved Hide resolved

Copilot AI mentioned this pull request Mar 6, 2026

Use lease_connection in in_paginated_batches spec to support permanent_connection_checkout = :disallowed #8958

Merged

mgrunberg approved these changes Mar 6, 2026

View reviewed changes

javierjulio approved these changes Mar 6, 2026

View reviewed changes

tagliala marked this pull request as draft March 6, 2026 22:05

Copilot AI mentioned this pull request Mar 6, 2026

Remove unnecessary uses_transaction from order clause and resource specs #8962

Merged

tagliala marked this pull request as ready for review March 6, 2026 23:12

davidrunger and others added 3 commits March 7, 2026 00:12

tagliala force-pushed the bugfix/8881-permanent-connection-cherrypick branch from b009716 to 29c3596 Compare March 6, 2026 23:12

tagliala merged commit 8fb982f into master Mar 6, 2026
26 checks passed

tagliala deleted the bugfix/8881-permanent-connection-cherrypick branch March 6, 2026 23:25

Copilot AI mentioned this pull request Mar 7, 2026

Backport PR #8883 to 3-0-stable with Rails < 7.2 conditionals #8965

Merged

Uh oh!

Conversation

tagliala commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

tagliala commented Jan 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

tagliala commented Mar 6, 2026

Uh oh!

javierjulio left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javierjulio Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

tagliala Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

tagliala Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

javierjulio Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tagliala commented Dec 12, 2025 •

edited

Loading

codecov bot commented Dec 12, 2025 •

edited

Loading

javierjulio left a comment •

edited

Loading