Add Rails 7 support #4220

peterberkenbosch · 2021-12-10T11:00:32Z

Description

With the release of Rails 7.0.0 we should start preparing Solidus edge to be supported. This PR starts with that.

Default using the latest released rails 7.0 on setup and sandbox creation:

bin/setup
bin/sandbox

There is however still an error showing up with title: TypeError in Spree::Home#index

no implicit conversion of String into Integer

Solving this with:

add mini_magick to sandbox/config/application.rb since the default :vips does not work out of the box with Solidus just yet. (see more details below).

config.active_storage.variant_processor = :mini_magick

If we want to use :vips by default we have to change core/app/models/concerns/spree/active_storage_adapter/attachment.rb#variant:

from:

def variant(style = nil)
  size = style_to_size(style)
  @attachment.variant(
    resize_to_limit: size,
    strip: true
  ).processed
end

to:

def variant(style = nil)
  size = style_to_size(style)
  @attachment.variant(
    resize_to_limit: size.map(&:to_i)
  ).processed
end

The size elements have to be an integer, and strip is not a valid command for :vips

Dependencies that we wait for new release:

ransack
devise
awesome_nested_set tracking PR
font-awesome-rails
rails-i18n
canonical-rails support for Rails 7.0.1 tracking PR
config.active_storage.variant_processor = :mini_magick automation with install generator?
Rails ~> 7.0.0 depends on Ruby >= 2.7.0
Netlify & CircleCI update to Ruby >= 2.7.0

Checklist:

I have followed Pull Request guidelines
I have added a detailed description into each commit message
I have updated Guides and README accordingly to this change (if needed)
I have added tests to cover this change (if needed)
I have attached screenshots to this PR for visual changes (if needed)

jarednorman

Thanks for getting a head start on this @peterberkenbosch!

peterberkenbosch · 2021-12-17T10:57:54Z

Updated this with the last few missing dependencies to run the sandbox.

peterberkenbosch · 2021-12-31T11:34:03Z

Updated with the latest released gems. Wondering if we want to stop supporting Rails 5.2? Also not sure why the added circlci build for postgres_rails70 is not being picked up by circleCI, probably needs someone with more permissions on the CircleCI account to enable or approve that build step?

jarednorman · 2022-01-28T18:18:41Z

Oooh, look at that green check mark.

BenMorganIO · 2022-01-28T20:49:04Z

I know I'm not supposed to leave emoji specific Github comments, but

😎

Thanks Peter!

denys-medynskyi · 2022-02-02T13:41:53Z

hey 👋

Looking forward to Rails 7 🎉

Anything I can do to help?
Do we have an ETA of merging this PR?

bbean86 · 2022-02-02T13:45:01Z

Gemfile

@@ -11,13 +11,18 @@ group :backend, :frontend, :core, :api do
 else
 gem 'rails', ENV['RAILS_VERSION'] || '~> 6.1.0', require: false
 end
+
+ if ENV['RAILS_VERSION'] == '7.0.0'


Does this need to be adjusted to compare against any version later than 7.0.0 since 7.0.1 is already out?

Thank you for opening this PR, by the way, we'd love to bump to Rails 7 ASAP and Solidus is one of the few things preventing that for us.

Yes, this needs to be a bit smarter. Good catch!

jarednorman · 2022-02-02T16:23:51Z

@peterberkenbosch Now that specs are green, should we be taking a more serious look at this PR?

peterberkenbosch · 2022-02-02T16:45:58Z

Yes! Let's do that!

jarednorman

Can you rebase out some of the Gemfile churn as the different versions of things get updated/released? We also don't need stuff like changing the version of devise and then removing it altogether both in the git history.

Left a couple small comments, but generally this all looks good. Thanks for taking this on, Peter. 🙏🏻

jarednorman · 2022-02-02T17:53:23Z

core/lib/spree/core/controller_helpers/current_host.rb

@@ -7,7 +7,7 @@ module CurrentHost
 extend ActiveSupport::Concern

 included do
- if Gem::Requirement.new("> 5.2").satisfied_by?(Rails.gem_version)
+ if Gem::Requirement.new(">= 6").satisfied_by?(Rails.gem_version)


Can you explain why you're doing this?

The remove deprecation warning on Rails 7. Might be better suitable for standalone PR now that I think of it. So we have 1 PR focussed only on adding Rails 7 compatibility.

I think it's pertinent to have it on this PR. Besides, if there's a warning, CI will fail.

ah.. thanks! Let's keep it then.

Hey @peterberkenbosch, what was the deprecation warning?

host is already set or something like that. I'm happy to remove all non-essential changes here actually.

Just wanted to understand if that was coming from Solidus or Rails, because if it's the latter and we will receive warnings, the CI won't fail, it only fails for Solidus deprecations. cc @waiting-for-dev

Ah, it's from Rails, not Solidus.

waiting-for-dev · 2022-02-04T05:10:14Z

Wondering if we want to stop supporting Rails 5.2?

I would say YES! Thoughts? cc @kennyadsl

not sure why the added circlci build for postgres_rails70 is not being picked up by circleCI

@peterberkenbosch, besides defining it, you need to add a new job within workflows -> build -> jobs.

However, what we need to add is a job for postgres_rails61, and update the default Rails version to 7.0 in the Gemfile.

kennyadsl · 2022-02-04T10:20:01Z

For our current policy, we should stop supporting a specific Rails version when it no more receives security patches, and 5.2 appears to be still receiving them, see: https://guides.rubyonrails.org/maintenance_policy.html#severe-security-issues. Also, changing this will probably require a major bump for Solidus, because this would be technically a breaking change, isn't it?

kennyadsl

Thanks 7️⃣ times (at least), @peterberkenbosch! I left some comments, let me know if I'm missing something.

Gemfile

kennyadsl · 2022-02-04T10:26:17Z

bin/sandbox

@@ -53,10 +53,10 @@ fi
 cd ./sandbox
 cat <<RUBY >> Gemfile

+gem 'awesome_nested_set', github: 'peterberkenbosch/awesome_nested_set', branch: 'rails-7-support'


Now that collectiveidea/awesome_nested_set#460 has been merged, why not using:

gem 'awesome_nested_set', github: 'collectiveidea/awesome_nested_set'

as you did in the Gemfile?

kennyadsl · 2022-02-04T10:26:49Z

bin/sandbox

 gem 'solidus', path: '..'
 gem 'solidus_auth_devise', '>= 2.1.0'
-gem 'rails-i18n'
-gem 'solidus_i18n'
+gem 'solidus_i18n', github: 'solidusio/solidus_i18n'


Do we need to release a new version of solidus_i18n with the code currently in master?

I don't think so, revisiting this now. 2.1.1 should have the latest from master now anyway right?

Confirming we don't need to do this anymore. There are a few deprecation warnings showing up, looking into those as well, but it's working as expected right now.

peterberkenbosch · 2022-02-04T12:21:53Z

Thanks! Updating this PR, and will respond back to the questions, suggestions etc.

denys-medynskyi · 2022-02-08T07:53:18Z

hey @peterberkenbosch
Thanks 8️⃣ times.

Do you need any help with this branch getting merged?

peterberkenbosch · 2022-02-08T10:19:07Z

hey @peterberkenbosch Thanks 8️⃣ times.

Do you need any help with this branch getting merged?

Not yet, rebasing all the cruft right now so we have a clean PR to work from.

peterberkenbosch · 2022-02-08T10:55:07Z

Should we track latest Rails 7 release by default on our main (unreleased) branch? We probably have to bump versions here too?

waiting-for-dev · 2022-02-08T11:12:32Z

Should we track latest Rails 7 release by default on our main (unreleased) branch? We probably have to bump versions here too?

I'm not sure I get it. Where exactly should we bump versions?

peterberkenbosch · 2022-02-08T11:20:31Z

@waiting-for-dev I was thinking the Solidus version, but that's not really needed in this PR.

peterberkenbosch · 2022-02-08T11:27:13Z

for Rails 7.0.1 support we need a few more updates on dependencies, tracking those down now.

Rails 7 deprecates Enum#sum in favor of Ruby's Enum#sum, which requires an initial value when elements are not numbers. In this case, as the elements are `Spree::StockQuantities`, we reduce with `:+` so that the first one is taken as the initial value.

Rails 7 deprecates image/jpg mime type, only accepting image/jpeg for JPG images. However, we can keep it as a supported image extension for Solidus applications (`Spree::Config#allowed_image_mime_types`). Host applications will see the deprecation message, and they will be able to act as desired. Besides, we can keep it for the Paperclip adapter.

We create a compatibility module to wrap this kind of internal usage of Rails API

`config.action.view.raise_on_missing_translations` [was deprecated on Rails 6.1](https://github.com/rails/rails/releases/tag/v6.1.0) in favor of `config.i18n.raise_on_missing_translations`. It was [removed on Rails 7.0](https://edgeguides.rubyonrails.org/7_0_release_notes.html#action-view).

The default changed on Rails 7. See: - https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#active-storage-default-variant-processor-changed-to-vips - rails/rails#42744

denys-medynskyi · 2022-04-08T12:41:05Z

Looks like failed test is a false positive 🤔 pr maybe eql shouldn't be used in this test, because as I see it is just different instance...

Failure/Error: expect(shipment.stock_location).not_to eql stock_item.stock_location

  expected: value != #<Spree::StockLocation id: 1998, name: "NY Warehouse", created_at: "2022-04-08 10:41:01", updated_at:..., position: 1, restock_inventory: true, fulfillable: true, code: nil, check_stock_on_transfer: true>
       got: #<Spree::StockLocation id: 1998, name: "NY Warehouse", created_at: "2022-04-08 10:41:01", updated_at:..., position: 1, restock_inventory: true, fulfillable: true, code: nil, check_stock_on_transfer: true>

  (compared using eql?)

  Diff:
  @@ -1,4 +1,4 @@
  -#<Spree::StockLocation:0x0000564a1895da40
  +#<Spree::StockLocation:0x0000564a18960ad8
    id: 1998,
    name: "NY Warehouse",
    created_at: Fri, 08 Apr 2022 10:41:01 UTC +00:00,
./spec/models/spree/order_inventory_spec.rb:181:in `block (6 levels) in <top (required)>'

kennyadsl · 2022-04-08T12:43:46Z

@denys-medynskyi should be a flaky, re-running!

denys-medynskyi · 2022-04-08T12:52:53Z

It is green now 💚

denys-medynskyi · 2022-04-08T13:54:11Z

@kennyadsl can you merge it please 🙏

kennyadsl

👍 Approved.

@denys-medynskyi we need to wait for a review from another core team member before merging.

This commit reverts part of 0cd2431 Even if we now support Rails 7, which only works from Ruby 2.7 onwards, we also still support Rails 5.2, which works from Ruby 2.5. Deprecating Ruby versions is a backward incompatible change, so we'll do that in major releases.

waiting-for-dev · 2022-04-11T03:59:34Z

I added a new commit reverting dropping support for Ruby 2.5 & Ruby 2.6. We still support Rails 5.2, which works from 2.5 onwards. We'll deprecate Ruby & Rails versions on major releases, as that's backward-incompatible changes.

denys-medynskyi · 2022-04-12T07:25:20Z

We are good to merge 🎉

mslinn · 2022-07-17T02:49:30Z

When might this merged PR get packaged into a released gem? Solidus 3.1.7 still uses Rail 6.1.

waiting-for-dev · 2022-07-18T03:24:54Z

When might this merged PR get packaged into a released gem? Solidus 3.1.7 still uses Rail 6.1.

@mslinn, we're wrapping everything up and you can expect a new release in the next couple of weeks 🤞

mslinn · 2022-07-18T14:22:48Z

@waiting-for-dev I am new to this project. Is there anything I should watch out for if I clone master and build the gem locally?

jarednorman · 2022-07-18T16:00:43Z

You can point your Gemfile at the latest code from GitHub, yes. Some stores do that to stay as up to date as possible.

mslinn · 2022-07-26T12:44:09Z

I wrote up a detailed explanation of what I did to build the gems, and created a GitHub discussion to figure out why solidus_core is not loaded when added to a Rails app, but did not get much response. Might someone take a moment to suggest ideas? ... Thanks!

asfour75 · 2023-04-19T19:51:55Z

When might this merged PR get packaged into a released gem? Solidus 3.1.7 still uses Rail 6.1.

@mslinn, we're wrapping everything up and you can expect a new release in the next couple of weeks crossed_fingers

Thank you all for the great effort pushing solidus to suppoer rails 7 as soon as possible ..
It's been almost a year .. Yet I'm still searching for offical announcement .. are we there yet ?

kennyadsl · 2023-04-19T20:59:19Z

Rails 7 support had been merged and released a while ago, maybe in 3.2 if I recall correctly.

BenMorganIO · 2023-04-20T17:18:21Z

@asfour75 Retro Market is on Rails 7 with Solidus 3.3 but we ran 3.2 with 7 as well.

asfour75 · 2023-04-20T20:12:43Z

Rails 7 support had been merged and released a while ago, maybe in 3.2 if I recall correctly.

Based on the offical gihub page https://github.com/solidusio/solidus
it says:

To add Solidus, begin with a Rails 5.2, 6 or 6.1 application and a database configured and created.

so I assumed it only supports up to 6.1
Nevertheless .. I tried it yesterday on rails 7 and it worekd perfectly ..
Thanks for the note .. nice work

asfour75 · 2023-04-20T20:13:11Z

@asfour75 Retro Market is on Rails 7 with Solidus 3.3 but we ran 3.2 with 7 as well.

Yup .. I tried 3.3 on Rails 7 .. worked smoothly ..
Thanks

waiting-for-dev · 2023-04-21T03:10:21Z

Thanks for reporting. The README is currently being fixed at https://github.com/solidusio/solidus/pull/5012/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

jarednorman reviewed Dec 10, 2021

View reviewed changes

peterberkenbosch force-pushed the rails7 branch from f907029 to 16acf54 Compare December 17, 2021 09:57

peterberkenbosch force-pushed the rails7 branch from 3ecea7d to 27f6b21 Compare December 31, 2021 10:23

peterberkenbosch force-pushed the rails7 branch from 0f0b175 to ceead5f Compare December 31, 2021 11:39

peterberkenbosch force-pushed the rails7 branch 2 times, most recently from 0e82a0a to 1f2fb2a Compare January 28, 2022 16:34

bbean86 reviewed Feb 2, 2022

View reviewed changes

jarednorman requested changes Feb 2, 2022

View reviewed changes

kennyadsl reviewed Feb 4, 2022

View reviewed changes

peterberkenbosch force-pushed the rails7 branch from 401cf93 to 059dc35 Compare February 8, 2022 09:51

peterberkenbosch changed the title ~~Allow Rails 7.0.0~~ Allow Rails 7 Feb 8, 2022

peterberkenbosch force-pushed the rails7 branch from a772ca0 to b1f349f Compare February 8, 2022 10:47

peterberkenbosch force-pushed the rails7 branch from 0e58a5c to f2ecc9e Compare February 8, 2022 11:47

peterberkenbosch changed the title ~~Allow Rails 7~~ Allow Rails ~> 7.0.0 Feb 8, 2022

waiting-for-dev added 7 commits April 8, 2022 12:33

Fix deprecation messages on Rails 7 for methods formatting time/date

e04b03b

We create a compatibility module to wrap this kind of internal usage of Rails API

Fix ActiveStorage deprecation messages

f3c473e

Update default image processing library to vips

91cb472

The default changed on Rails 7. See: - https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#active-storage-default-variant-processor-changed-to-vips - rails/rails#42744

Append predicate method name with "?"

7a0471e

peterberkenbosch force-pushed the rails7 branch from 0fc8d7f to 7a0471e Compare April 8, 2022 10:33

kennyadsl approved these changes Apr 8, 2022

View reviewed changes

jarednorman approved these changes Apr 11, 2022

View reviewed changes

kennyadsl merged commit 9fbcce5 into solidusio:master Apr 12, 2022

peterberkenbosch deleted the rails7 branch August 24, 2023 08:45

Add Rails 7 support #4220

Add Rails 7 support #4220

Conversation

peterberkenbosch commented Dec 10, 2021 • edited Loading

jarednorman left a comment

Choose a reason for hiding this comment

peterberkenbosch commented Dec 17, 2021

peterberkenbosch commented Dec 31, 2021

jarednorman commented Jan 28, 2022

BenMorganIO commented Jan 28, 2022 • edited Loading

denys-medynskyi commented Feb 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarednorman commented Feb 2, 2022

peterberkenbosch commented Feb 2, 2022

jarednorman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waiting-for-dev commented Feb 4, 2022

kennyadsl commented Feb 4, 2022

kennyadsl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterberkenbosch commented Feb 4, 2022

denys-medynskyi commented Feb 8, 2022

peterberkenbosch commented Feb 8, 2022

peterberkenbosch commented Feb 8, 2022

waiting-for-dev commented Feb 8, 2022

peterberkenbosch commented Feb 8, 2022

peterberkenbosch commented Feb 8, 2022

denys-medynskyi commented Apr 8, 2022

kennyadsl commented Apr 8, 2022

denys-medynskyi commented Apr 8, 2022

denys-medynskyi commented Apr 8, 2022

kennyadsl left a comment

Choose a reason for hiding this comment

waiting-for-dev commented Apr 11, 2022

denys-medynskyi commented Apr 12, 2022

mslinn commented Jul 17, 2022

waiting-for-dev commented Jul 18, 2022

mslinn commented Jul 18, 2022

jarednorman commented Jul 18, 2022

mslinn commented Jul 26, 2022

asfour75 commented Apr 19, 2023

kennyadsl commented Apr 19, 2023 • edited Loading

BenMorganIO commented Apr 20, 2023 • edited Loading

asfour75 commented Apr 20, 2023

asfour75 commented Apr 20, 2023

waiting-for-dev commented Apr 21, 2023

peterberkenbosch commented Dec 10, 2021 •

edited

Loading

BenMorganIO commented Jan 28, 2022 •

edited

Loading

kennyadsl commented Apr 19, 2023 •

edited

Loading

BenMorganIO commented Apr 20, 2023 •

edited

Loading