Skip to content

Upgrade to Rails 7.2 - #655

Open
tungleduyxyz wants to merge 10 commits into
masterfrom
upgrade_rails_7.2
Open

Upgrade to Rails 7.2#655
tungleduyxyz wants to merge 10 commits into
masterfrom
upgrade_rails_7.2

Conversation

@tungleduyxyz

Copy link
Copy Markdown
Contributor

Summary

Upgrade kaui from Rails 7.0 to Rails 7.2.

Changes

  • Gemfile / kaui.gemspec: bump rails to ~> 7.2 and required_ruby_version to >= 3.1.0 (Rails 7.2's new minimum Ruby version)
  • Gemfile: bump activerecord-jdbc-adapter to ~> 72.0 — the 70.x line only supports Rails ~> 7.0 and doesn't implement Rails 7.2's new adapter-loading API (ActiveRecord::ConnectionAdapters.register)
  • CI (.github/workflows/ci.yml): bump JRuby from 9.4.2.0 to 9.4.15.0 (latest 9.4.x patch, Ruby 3.1 compat); switch the JRuby MySQL matrix jobs from database-adapter: mariadb to mysql2 (arjdbc 72.x dropped the mariadb adapter name)
  • test/dummy/config/database.yml: add explicit driver: org.mariadb.jdbc.Driver hint for the mysql2 adapter under JRuby — Rails 7.2 instantiates the adapter class directly (adapter_class.new(config)), so arjdbc's old mariadb_connection helper that used to auto-set the driver is no longer invoked, and without the hint arjdbc defaults to the (unbundled) com.mysql.jdbc.Driver
  • test/dummy/config/application.rb: add sorbet-runtime workaround (T::Configuration.default_checked_level = :never under JRuby) for a js-routes crash under JRuby 10
  • Tests: fix Rails 7.2 removal of the deprecated singular TestFixtures.fixture_path/= API — use plural fixture_paths (Array) in functional_test_helper_nosetup.rb and admin_tenants_controller_test.rb
  • test/dummy/db/schema.rb: regenerate in Rails 7.2 schema format
  • README.md: update dependencies line (JRuby 9.4.15.0)

Verification

  • bundle exec rake test:units → 47 runs, 191 assertions, 0 failures under Rails 7.2.3.2 (CRuby 3.2.2, and JRuby 9.4.15.0 against the MariaDB container)

Notes

  • No other dependency bumps were needed — all other gems (devise 5.0.4, money-rails 3.0.0, sprockets-rails 3.5.2, js-routes 2.4.x, etc.) only require rails/railties >= 7.0.
  • The Kill Bill compatibility table in the README was intentionally left untouched (version numbering is a release decision).

- Bump rails to ~> 7.2 in Gemfile/kaui.gemspec, bump required_ruby_version
  to >= 3.1.0 (Rails 7.2's new minimum)
- Bump activerecord-jdbc-adapter to ~> 72.0 for Rails 7.2 compatibility
- Bump CI JRuby version to 9.4.15.0, update README dependencies accordingly
- Fix Rails 7.2 removal of TestFixtures.fixture_path/= (use fixture_paths)
- Add sorbet-runtime workaround for js-routes crash under JRuby 10
- Regenerate test/dummy/db/schema.rb for Rails 7.2 schema format
- Add explicit driver: org.mariadb.jdbc.Driver hint for the mysql2
  adapter under JRuby (Rails 7.2 instantiates the adapter class
  directly, so arjdbc's old mariadb_connection helper that used to
  auto-set the driver is no longer invoked)
- Allow adapter: mysql2 in the JRuby test block (arjdbc 72.x dropped
  the mariadb adapter name)
- Align local dev default DB password with the docker MariaDB container
arjdbc 72.x dropped the 'mariadb' adapter name, so CI now uses 'mysql2'
with an explicit driver: org.mariadb.jdbc.Driver override. But nothing
was actually loading the driver jar onto the JRuby classpath, causing:

  Java::JavaLang::ClassNotFoundException: org.mariadb.jdbc.Driver

Explicitly require 'jdbc/mariadb' and call Jdbc::MariaDB.load_driver(:require)
before establishing the connection. Note :require must be passed explicitly -
the gem's default :load method does not register the jar with the classloader.
…g backtrace

- app/helpers/kaui/exception_helper.rb: Rails/EnvLocal offense, use Rails.env.local?
- test/functional/kaui/accounts_controller_test.rb: Rails 7.2's redirect_to no
  longer populates a fallback HTML body (previously
  '<html><body>You are being <a href=...>redirected</a>.</body></html>'), so
  parsing @response.body for the account id no longer works. Use the Location
  header instead.
- test/functional/kaui/admin_tenants_controller_test.rb: ActionController::TestCase
  recycles the request env between successive calls in the same test but does
  not clear a stale multipart Content-Type header (see scrub_env! in Rails'
  test_case.rb), so Rack 3.x now raises Rack::Multipart::EmptyContentError on
  the subsequent GET. Explicitly clear the header first.
- lib/kaui/error_handler.rb: TEMPORARY - append exception backtrace to the
  flash error message to diagnose the remaining 2 failures
  (AdminAllowedUsersControllerTest#test_should_get_create/test_should_get_edit)
  via CI output. Will revert once root cause is found.
… env

Diagnostic only - does not change behavior/flow, just logs to help find the
root cause of the humanize NoMethodError affecting admin_allowed_users edit
action tests. Will revert once fixed.
Found via CI fail-fast + random test order surfacing more instances of the
same two Rails 7.2 regressions already fixed once:

- accounts_controller_test.rb#test_should_trigger_invoice: same
  redirect-body-is-now-empty issue as test_should_validate_external_key_if_found.
  Use @response.location directly instead of regex-matching the body.
- admin_allowed_users_controller_test.rb#extract_allowed_id: same issue,
  fall back to parsing response_path (Location header) instead of the body.
- functional_test_helper_nosetup.rb: fix the stale multipart Content-Type bug
  (Rack::Multipart::EmptyContentError) once for all tests instead of
  per-test-file, by clearing CONTENT_TYPE before every #process call. This
  also fixes test_should_get_new_plan_currency which hit the same issue.
- Removed the now-redundant manual fix from
  admin_tenants_controller_test.rb#test_should_download_a_catalog.
Root cause found via the temporary backtrace diagnostic:

app/views/kaui/admin_allowed_users/_form.html.erb passed a blank string
(' ') as the attribute name to f.label, purely for layout spacing. Under
Rails 7.2, ActiveModel::Translation#human_attribute_name has:

  translation = attribute.present? ? attribute.humanize : namespace.humanize

For a blank attribute that doesn't contain a '.', the 'namespace' local
variable is never assigned (it's only set in the dotted-attribute branch),
so this evaluates nil.humanize - a Rails core edge case bug when passing a
blank/whitespace label method name.

Fixed by using label_tag instead of f.label, since this label isn't actually
associated with any model attribute and doesn't need i18n attribute name
translation.

Also reverts the temporary warn-based backtrace diagnostic in
lib/kaui/error_handler.rb added earlier purely for CI debugging - no longer
needed now that the root cause is fixed.
Rails 7.2 quotes String id values in ActiveRecord::RecordNotFound's message
(e.g. 'id'="60" instead of 'id'=60). Update the expected flash message
accordingly in test_should_delete_allowed_user.
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.

1 participant