Allow Rails 7.2 #5843

tvdeyen · 2024-08-29T10:17:01Z

Summary

Allows to update host apps to Rails 7.2.

The only change is that we need to raise the minimum version of Ruby to 3.1, but since it is EOL anyway
it should not be a big deal for anyone.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

MadelineCollier

How should we handle the error we get with Ransack 4.2 and above? It doesn't look like there has been any movement on the PR to fix the issue.

tvdeyen · 2024-09-11T19:01:48Z

How should we handle the error we get with Ransack 4.2 and above? It doesn't look like there has been any movement on the PR to fix the issue.

That's a very good question. I need to read on the issue first in order to make an opinion. Since I use Ransack 4.2 with great success in several other apps and engines I am not sure why this is an issue in Solidus at all.

tvdeyen · 2024-09-11T20:36:38Z

I looked into the problem and I am sure I can replace the custom ransacker with normal ransack. PR upcoming maybe tomorrow

MadelineCollier

Nice, thanks for addressing the root cause ❤️

tvdeyen · 2024-09-12T08:42:27Z

Nice, thanks for addressing the root cause ❤️

Should be fixed by #5853

forkata

Great work here @tvdeyen! We may be blocked by the solidus_starter_frontend that is causing issues bundling due to cannonical_rails 😞 I believe there hasn't been a release that includes this PR yet. There is an open issue for it here.

.circleci/config.yml

tvdeyen · 2024-10-16T14:51:12Z

Great work here @tvdeyen! We may be blocked by the solidus_starter_frontend that is causing issues bundling due to cannonical_rails 😞 I believe there hasn't been a release that includes this PR yet. There is an open issue for it here.

Thanks Chris, I really need to come back to this some day.

I can try to ping Denis if he is willing to release version of canonical_rails, but honestly I would like to get rid of it.

Canonical tags are rarely necessary nowadays and probably causing more problems to your SEO if not done right (and I have been bitten by this gem's wrong canonical tags it generates). Maybe I find some time to get rid of it in the starter frontend.

forkata · 2024-10-16T18:01:26Z

@tvdeyen I completely agree with you on the unnecessary dependency on canonical_rails. I would be happy to support with trying to remove that dependency from the starter frontend if you don't have time to tackle that?

tvdeyen · 2024-10-18T12:51:30Z

@tvdeyen I completely agree with you on the unnecessary dependency on canonical_rails. I would be happy to support with trying to remove that dependency from the starter frontend if you don't have time to tackle that?

@forkata sure, that would be really helpful!

jumph4x · 2024-10-23T21:13:09Z

Apologies for the delays on this. Getting it handled right now.

Though I understand it might get dropped as a dep. No matter, releasing a patch bump is a good idea: https://rubygems.org/gems/canonical-rails/versions/0.2.16

codecov · 2024-10-29T17:38:39Z

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.58%. Comparing base (f4c1cee) to head (e0b78d0).
Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
.../solidus/install/app_templates/frontend/starter.rb	0.00%	1 Missing ⚠️
.../lib/spree/testing_support/dummy_app/migrations.rb	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5843      +/-   ##
==========================================
+ Coverage   89.31%   89.58%   +0.27%     
==========================================
  Files         712      783      +71     
  Lines       16187    17989    +1802     
==========================================
+ Hits        14457    16116    +1659     
- Misses       1730     1873     +143

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

tvdeyen · 2024-10-30T09:39:35Z

Apologies for the delays on this. Getting it handled right now.

Though I understand it might get dropped as a dep. No matter, releasing a patch bump is a good idea: https://rubygems.org/gems/canonical-rails/versions/0.2.16

Thanks, Denis! No worries.

tvdeyen · 2024-10-30T09:50:17Z

@solidusio/core-team this is now ready for review.

We still need a merge ofhttps://github.com/solidusio/solidus_starter_frontend/pull/374 that allows Rails 7.2 in order to make the solidus-installer tests pass in this branch. This is kind of a chicken egg situation. The PR in solidus_starter_frontend runs with this branch to make it's tests green and vice versa. I still think we should merge the solidus_starter_frontend PR first so we can revert back to main in the starter frontend app template and this change in the solidus-starter PR

WDYT?

tvdeyen · 2024-10-30T10:17:38Z

I extracted the Ruby 3.1 bump into a separate PR for better visibility

We need at least v3.7 of awesome_nested_set for Rails 7.2 support

Rails 7.2 removed migration_context from ActiveRecord::Base.connection

Without that ActiveRecord 7.2 expects a DB table for it.

This installs the master branch of canonical-rails. Necessary for Rails 7.2 compatibility.

We updated to Rails 7.2 and need to update the cache version.

Rails 8.0 removes the option to remove keyword arguments for defingin enums.

How has this ever worked this way?

This is a flaky test, so give Capybara it bit more time to find the button.

tvdeyen self-assigned this Aug 29, 2024

tvdeyen force-pushed the rails-7.2 branch from 2cc3f7f to e4199b0 Compare September 6, 2024 16:34

MadelineCollier reviewed Sep 11, 2024

View reviewed changes

MadelineCollier reviewed Sep 12, 2024

View reviewed changes

tvdeyen force-pushed the rails-7.2 branch 3 times, most recently from 6ac936c to a5637df Compare September 12, 2024 09:38

tvdeyen added this to the 4.4 milestone Oct 9, 2024

forkata reviewed Oct 16, 2024

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

tvdeyen force-pushed the rails-7.2 branch from 64e09b7 to c39935b Compare October 16, 2024 14:55

tvdeyen force-pushed the rails-7.2 branch 3 times, most recently from cd00f02 to 0294c66 Compare October 18, 2024 19:23

tvdeyen force-pushed the rails-7.2 branch from 0294c66 to 0a9b182 Compare October 29, 2024 14:42

tvdeyen mentioned this pull request Oct 29, 2024

Do not use solidus_frontend to test extension test app generator #5888

Merged

tvdeyen force-pushed the rails-7.2 branch from 8c88405 to 8924fa5 Compare October 29, 2024 17:17

tvdeyen force-pushed the rails-7.2 branch from 8924fa5 to 3c79b76 Compare October 30, 2024 09:04

tvdeyen marked this pull request as ready for review October 30, 2024 09:42

tvdeyen requested a review from a team as a code owner October 30, 2024 09:42

This was referenced Oct 30, 2024

Add Rails 7.2 support solidusio/solidus_starter_frontend#374

Open

Bump minimum Ruby version to 3.1 #5891

Merged

tvdeyen force-pushed the rails-7.2 branch from 7ce7fd0 to 0250100 Compare October 30, 2024 10:19

jarednorman approved these changes Oct 31, 2024

View reviewed changes

tvdeyen added 9 commits October 31, 2024 22:38

Allow Rails 7.2

83f76a4

We need at least v3.7 of awesome_nested_set for Rails 7.2 support

Fix pending migration check for Rails 7.2

36db12a

Rails 7.2 removed migration_context from ActiveRecord::Base.connection

Mark Dummy App's ApplicationRecord as abstract class

977ac53

Without that ActiveRecord 7.2 expects a DB table for it.

Add Rails 7.2 to circle ci test matrix

a6e3a0a

Use rails-7.2 branch of solidus_starter_frontend

bf1de64

This installs the master branch of canonical-rails. Necessary for Rails 7.2 compatibility.

CI: Bump solidus-installer cache key

aab4eea

We updated to Rails 7.2 and need to update the cache version.

Use positional arguments for defining enums

f761108

Rails 8.0 removes the option to remove keyword arguments for defingin enums.

Fix enum definition in TaxRate.rb

b4c03f2

How has this ever worked this way?

Test: Give "Receive" button more time to render

e0b78d0

This is a flaky test, so give Capybara it bit more time to find the button.

tvdeyen force-pushed the rails-7.2 branch from c5ea0e6 to e0b78d0 Compare October 31, 2024 21:38

github-actions bot removed changelog:solidus_api Changes to the solidus_api gem changelog:solidus_sample Changes to the solidus_sample gem changelog:solidus Changes to the solidus meta-gem changelog:solidus_admin labels Oct 31, 2024

tvdeyen merged commit 78e5f28 into solidusio:main Nov 1, 2024
15 of 16 checks passed

tvdeyen deleted the rails-7.2 branch November 1, 2024 10:03

kennyadsl mentioned this pull request Nov 6, 2024

Incompatible versions gems #5898

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Rails 7.2 #5843

Allow Rails 7.2 #5843

tvdeyen commented Aug 29, 2024 •

edited

Loading

MadelineCollier left a comment

tvdeyen commented Sep 11, 2024

tvdeyen commented Sep 11, 2024 •

edited

Loading

MadelineCollier left a comment

tvdeyen commented Sep 12, 2024

forkata left a comment

tvdeyen commented Oct 16, 2024

forkata commented Oct 16, 2024

tvdeyen commented Oct 18, 2024

jumph4x commented Oct 23, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading

tvdeyen commented Oct 30, 2024

tvdeyen commented Oct 30, 2024

tvdeyen commented Oct 30, 2024

Allow Rails 7.2 #5843

Allow Rails 7.2 #5843

Conversation

tvdeyen commented Aug 29, 2024 • edited Loading

Summary

Checklist

MadelineCollier left a comment

Choose a reason for hiding this comment

tvdeyen commented Sep 11, 2024

tvdeyen commented Sep 11, 2024 • edited Loading

MadelineCollier left a comment

Choose a reason for hiding this comment

tvdeyen commented Sep 12, 2024

forkata left a comment

Choose a reason for hiding this comment

tvdeyen commented Oct 16, 2024

forkata commented Oct 16, 2024

tvdeyen commented Oct 18, 2024

jumph4x commented Oct 23, 2024 • edited Loading

codecov bot commented Oct 29, 2024 • edited Loading

Codecov Report

tvdeyen commented Oct 30, 2024

tvdeyen commented Oct 30, 2024

tvdeyen commented Oct 30, 2024

tvdeyen commented Aug 29, 2024 •

edited

Loading

tvdeyen commented Sep 11, 2024 •

edited

Loading

jumph4x commented Oct 23, 2024 •

edited

Loading

codecov bot commented Oct 29, 2024 •

edited

Loading