Skip to content
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.

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

Solidus 4.4 / Rails 7.2 compatibility #163

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Solidus 4.4 / Rails 7.2 compatibility #163

wants to merge 8 commits into from

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Dec 17, 2024

Lots of fixes in single commits that make this extension work with Solidus 4.4 and Rails 7.2

@tvdeyen tvdeyen changed the title CI: Lint with Ruby 3.1 Solidus 4.4 / Rails 7.2 compatibility Dec 17, 2024
We install latest Solidus during linting code
and this requires at least Ruby 3.1
By default we test extensions with latest Solidus,
which due to Rails 7.2 support, requires at least
Ruby 3.1
The solidusio/extensions CircleCI Orb sets the RAILS_VERSION
to test the supported Solidus version. We should use that
in order to not install latest Rails (8.0) versions we do
not support yet.
@tvdeyen tvdeyen force-pushed the lint-ruby-3.1 branch 5 times, most recently from ffadb01 to e65f687 Compare December 17, 2024 14:11
Otherwise Rails 8.0 will be used
This controller lives in decorators, while it must
live in lib/controllers/api so it can be conditionally
loaded if api_available?

This worked until now because solidus_support requires
all decorators during start up. In Rails 7.2 it fails,
because it cannot load the :authenticate_user method
anymore.
The Rakefile includes all we need to run the tests. No need for anything different here. Let's reduce confusion.
This gem redefines the host apps ApplicationRecord class,
which is very wrong!

We introduce a namespaced BaseRecord and set the correct
table_name_prefix on that class.
That's how this is supposed to work, apparently.
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.

2 participants