Rails 7.1 support #5359

peterberkenbosch · 2023-08-26T09:23:37Z

Summary

Preparation for supporting Rails 7.1. Very much work in progress, although it looks like the changes needed are minimal so far. The sandbox is running successfully, there are some failing specs that need to be addressed.

Arel::Table removes alias 'table_name' for 'name' activerecord-hackery/ransack#1420
Add support to Circle to run Rails from edge
Add support to Circle to run Rails 7.1
Fix all the specs
Fix or document all deprecation warnings for 7.1
Use Rails 7.1 for installer tests on CircleCI

One of the current RSpec errors are related to this rails/rails#49591 and it looks that this will be fixed in the upcoming Rails 7.1.2 release.

Ransack is pain still. The custom predicates with formatters is failing hard currently:

core/app/models/spree/product.rb:

# The :variants_option_values ransacker filters Spree::Product based on
    # variant option values ids.
    #
    # Usage:
    # Spree::Product.ransack(
    #   variants_option_values_in: [option_value_id1, option_value_id2]
    # ).result
    ransacker :variants_option_values, formatter: proc { |v|
      if OptionValue.exists?(v)
        joins(variants_including_master: :option_values)
          .where(spree_option_values: { id: v })
          .distinct
          .select(:id).arel
      end
    } do |parent|
      parent.table[:id]
    end

1) Spree::Product ransacker :variants_option_values filters products based on option values of their variants
     Failure/Error: result = Spree::Product.ransack(variants_option_values_in: [option_value_1.id]).result
     
     NoMethodError:
       undefined method `value' for #<Arel::SelectManager:0x00000001160b22c0 @ast=#<Ar

Figuring out where this value method call used to be.

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.

codecov · 2023-08-26T09:32:05Z

Codecov Report

Merging #5359 (42fac2a) into main (d095e38) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 42fac2a differs from pull request most recent head bf51cab. Consider uploading reports for the commit bf51cab to get more accurate results

@@           Coverage Diff           @@
##             main    #5359   +/-   ##
=======================================
  Coverage   88.71%   88.71%           
=======================================
  Files         563      563           
  Lines       13896    13896           
=======================================
  Hits        12328    12328           
  Misses       1568     1568

Files Changed	Coverage Δ
core/app/models/spree/preference.rb	`100.00% <100.00%> (ø)`
core/app/models/spree/return_item.rb	`91.40% <100.00%> (ø)`
core/lib/spree/preferences/persistable.rb	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

core/app/models/spree/address.rb

peterberkenbosch · 2023-09-01T08:45:29Z

specs are almost passing. Figuring out why we now get different apostrophe's back from our api..

Took me a few to figure out why this was not matching:

expected: "can't be blank"
            got: "can’t be blank"

' vs ’

peterberkenbosch · 2023-09-01T08:52:06Z

Intentional Rails update:

*   Improve typography of user facing error messages. In English contractions,
    the Unicode APOSTROPHE (`U+0027`) is now RIGHT SINGLE QUOTATION MARK
    (`U+2019`). For example, "can't be blank" is now "can’t be blank".

    *Jon Dufresne*

kennyadsl

Left an initial set of comments; thanks a ton for this great contribution, @peterberkenbosch! ❤️

kennyadsl · 2023-10-06T08:53:19Z

backend/app/controllers/spree/admin/orders_controller.rb

@@ -4,8 +4,8 @@ module Spree
  module Admin
    class OrdersController < Spree::Admin::BaseController
      before_action :initialize_order_events
-      before_action :load_order, only: [:edit, :update, :complete, :advance, :cancel, :resume, :approve, :resend, :unfinalize_adjustments, :finalize_adjustments, :cart, :confirm]


Good finding! Did you check if we also have a (unused) route for this action?

kennyadsl · 2023-10-06T08:54:39Z

.circleci/config.yml

@@ -347,4 +347,7 @@ workflows:
      - test_solidus:
          name: *name
          matrix: { parameters: { rails: ['7.0'], ruby: ['3.2'], database: ['sqlite'], paperclip: [false] } }
+      - test_solidus:


It's a good idea to always test against the latest Rails development branch. We should probably also make this check not mandatory in the GH actions report on PRs.

I'm moving this to a different PR so we can focus on 7.1

core/app/models/spree/shipping_rate.rb

core/lib/solidus_core.rb

kennyadsl · 2023-12-01T18:08:08Z

@peterberkenbosch Hey Peter, did you have any chance to keep up with this? I'd volunteer to continue the work if you want, just let me know please.

peterberkenbosch · 2023-12-04T15:56:46Z

@kennyadsl feel free to pick up. I haven't had the proper focus time to jump on this. Would be nice to have this done.

kennyadsl · 2023-12-06T13:31:44Z

Made another pass to fix other deprecation warnings and errors. As far as I can see the biggest thing left are ransack and previews, which require more time.

core/app/models/spree/order.rb

core/lib/spree/core/engine.rb

core/app/models/spree/order.rb

core/app/models/spree/credit_card.rb

This commit removes the following deprecations: DEPRECATION WARNING: Spree::Order model aliases `bill_address`, but `bill_address` is not an attribute. Starting in Rails 7.2, alias_attribute with non-attribute targets will raise. Use `alias_method :billing_address, :bill_address` or define the method manually. DEPRECATION WARNING: Spree::Order model aliases `ship_address`, but `ship_address` is not an attribute. Starting in Rails 7.2, alias_attribute with non-attribute targets will raise. Use `alias_method :shipping_address, :ship_address` or define the method manually. DEPRECATION WARNING: Spree::CreditCard model aliases `cc_type` and has a method called `cc_type=` defined. Starting in Rails 7.2 `brand=` will not be calling `cc_type=` anymore. You may want to additionally define `brand=` to preserve the current behavior.

Apparently now Rails returns the host without the initial .

…cord::Base DEPRECATION WARNING: SchemaMigration no longer inherits from ActiveRecord::Base. If you want to use the default connection, remove this argument. If you want to use a specific connection, instantiate MigrationContext with the connection's schema migration, for example `MigrationContext.new(path, Dog.connection.schema_migration)`. (called from new at /Users/andrea/nebulab/solidus/core/lib/spree/testing_support/dummy_app/rake_tasks.rb:41)

Due to deprecations, with a previous commit we replaced `alias_attribute` calls for these two attributes with `alias_method`, but the latter doesn't fully cover the previous behavior. When `shipping_address` or `billing_address` are passed as attributes, the error `ActiveModel::UnknownAttributeError` is now raised. For this reason, we're starting to replace them with the right association name. We may want to revisit the possibility to still use `billing_address` and `shipping_address` as providing these aliases doesn't seem to be fully viable anymore.

Starting from Rails 7.1, `ActionView::TestCase#output_buffer` uses `ActionView::OutputBuffer` rather than `ActiveSupport::SafeBuffer`. Due to the slightly different behavior, before comparing it to a string we we need first to convert it. See rails/rails@532e39a

In Rails 7.1 AS will check the global configuration when the macro is used.

Adding the previews path after setting the autoload ensures the file is not autoloaded by Zeitwerk. Co-Authored-By: andrea longhi <andrea@spaghetticode.it>

rails/rails@098b0eb

kennyadsl

Thanks for completing the work here, @elia @spaghetticode!

rainerdema

👏👏👏

tvdeyen · 2023-12-21T11:28:30Z

Great work! Thanks 🙏🏻

peterberkenbosch · 2023-12-21T14:09:28Z

great!! thanks all for pushing this over the finish line! 🎉

github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Aug 26, 2023

github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Aug 26, 2023

peterberkenbosch force-pushed the rails-edge branch from bf51cab to 66034a9 Compare August 26, 2023 10:42

github-actions bot added the changelog:repository Changes to the repository not within any gem label Aug 26, 2023

jarednorman reviewed Aug 28, 2023

View reviewed changes

core/app/models/spree/address.rb Outdated Show resolved Hide resolved

github-actions bot added the changelog:solidus_api Changes to the solidus_api gem label Sep 1, 2023

peterberkenbosch force-pushed the rails-edge branch from ea5b56a to e6ede34 Compare October 6, 2023 06:12

kennyadsl reviewed Oct 6, 2023

View reviewed changes

peterberkenbosch force-pushed the rails-edge branch 2 times, most recently from 0763ba2 to c6a8a68 Compare October 12, 2023 08:41

github-actions bot added the changelog:solidus_admin label Oct 12, 2023

peterberkenbosch force-pushed the rails-edge branch from 72cc736 to d8f6fcc Compare October 12, 2023 09:26

github-actions bot removed the changelog:solidus_api Changes to the solidus_api gem label Oct 12, 2023

kennyadsl force-pushed the rails-edge branch from 016d52b to 537eaf3 Compare December 6, 2023 13:27

spaghetticode reviewed Dec 13, 2023

View reviewed changes

core/app/models/spree/order.rb Outdated Show resolved Hide resolved

spaghetticode reviewed Dec 14, 2023

View reviewed changes

core/lib/spree/core/engine.rb Outdated Show resolved Hide resolved

spaghetticode reviewed Dec 14, 2023

View reviewed changes

core/app/models/spree/order.rb Show resolved Hide resolved

spaghetticode reviewed Dec 14, 2023

View reviewed changes

core/app/models/spree/credit_card.rb Outdated Show resolved Hide resolved

spaghetticode force-pushed the rails-edge branch from 537eaf3 to 5ee2c90 Compare December 15, 2023 16:31

github-actions bot added the changelog:solidus_sample Changes to the solidus_sample gem label Dec 15, 2023

elia changed the title ~~Rails edge (7.1) support~~ Rails 7.1 support Dec 20, 2023

elia self-assigned this Dec 20, 2023

elia force-pushed the rails-edge branch 2 times, most recently from 359448b to 7d91dc7 Compare December 20, 2023 11:25

elia marked this pull request as ready for review December 20, 2023 21:15

elia requested a review from a team as a code owner December 20, 2023 21:15

elia marked this pull request as draft December 20, 2023 21:17

elia marked this pull request as ready for review December 21, 2023 09:59

elia requested a review from rainerdema December 21, 2023 09:59

peterberkenbosch and others added 18 commits December 21, 2023 11:38

Allow Rails 7.1 and up

a4aea38

Use correct renamed behavior module for testing.

bf87f5a

Use up-to-date rspec-rails gem

e683524

Add Rails 7.1 to Circle CI Matrix

eeaf72c

Bump axe-core specs to fix false negative in admin

6e6d66e

Default to Rails 7.1 in development

a240dab

Fix Set-Cookie header domain in tests for Rails 7.1

0a97cc2

Apparently now Rails returns the host without the initial .

Fix the encrypted preference spec by allowing other calls to ENV

532dec4

Setup the active storage configuration

5ef1803

In Rails 7.1 AS will check the global configuration when the macro is used.

Fix preview paths to work with Rails 7.1

85b35ad

Adding the previews path after setting the autoload ensures the file is not autoloaded by Zeitwerk. Co-Authored-By: andrea longhi <andrea@spaghetticode.it>

Don't define the with_values scope for Rails 7.1+

f6047e7

rails/rails@098b0eb

Avoid using deprecated constants when logging VC rendering

95a0b09

Add a note about the value of action_dispatch.show_exceptions

2739f7b

Run the solidus installer on Rails 7.1

66b325f

elia force-pushed the rails-edge branch from 0b7adea to 66b325f Compare December 21, 2023 10:40

kennyadsl reviewed Dec 21, 2023

View reviewed changes

kennyadsl approved these changes Dec 21, 2023

View reviewed changes

rainerdema approved these changes Dec 21, 2023

View reviewed changes

elia merged commit 993fb59 into solidusio:main Dec 21, 2023
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rails 7.1 support #5359

Rails 7.1 support #5359

peterberkenbosch commented Aug 26, 2023 •

edited by elia

Loading

codecov bot commented Aug 26, 2023 •

edited

Loading

peterberkenbosch commented Sep 1, 2023

peterberkenbosch commented Sep 1, 2023

kennyadsl left a comment

kennyadsl Oct 6, 2023

kennyadsl Oct 6, 2023

elia Dec 20, 2023

kennyadsl commented Dec 1, 2023

peterberkenbosch commented Dec 4, 2023

kennyadsl commented Dec 6, 2023

kennyadsl left a comment

rainerdema left a comment

tvdeyen commented Dec 21, 2023

peterberkenbosch commented Dec 21, 2023

Rails 7.1 support #5359

Rails 7.1 support #5359

Conversation

peterberkenbosch commented Aug 26, 2023 • edited by elia Loading

Summary

Checklist

codecov bot commented Aug 26, 2023 • edited Loading

Codecov Report

peterberkenbosch commented Sep 1, 2023

peterberkenbosch commented Sep 1, 2023

kennyadsl left a comment

Choose a reason for hiding this comment

kennyadsl Oct 6, 2023

Choose a reason for hiding this comment

kennyadsl Oct 6, 2023

Choose a reason for hiding this comment

elia Dec 20, 2023

Choose a reason for hiding this comment

kennyadsl commented Dec 1, 2023

peterberkenbosch commented Dec 4, 2023

kennyadsl commented Dec 6, 2023

kennyadsl left a comment

Choose a reason for hiding this comment

rainerdema left a comment

Choose a reason for hiding this comment

tvdeyen commented Dec 21, 2023

peterberkenbosch commented Dec 21, 2023

peterberkenbosch commented Aug 26, 2023 •

edited by elia

Loading

codecov bot commented Aug 26, 2023 •

edited

Loading