From 16eaa52f5c83ff1f4e64655fd848bbfdc94e9168 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Mon, 26 Aug 2019 17:44:23 +0200 Subject: [PATCH 01/14] Add Rails 6 to CircleCI --- .circleci/config.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index f3714dceb6b..1e43d632152 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -121,6 +121,24 @@ jobs: - setup - test + postgres_rails60: + executor: postgres + parallelism: *parallelism + environment: + RAILS_VERSION: '~> 6.0.0' + steps: + - setup + - test + + mysql_rails60: + executor: mysql + parallelism: *parallelism + environment: + RAILS_VERSION: '~> 6.0.0' + steps: + - setup + - test + workflows: build: jobs: @@ -132,6 +150,8 @@ workflows: - mysql - postgres_rails51 - mysql_rails51 + - postgres_rails60 + - mysql_rails60 - stoplight/push: project: solidus/solidus-api git_token: $STOPLIGHT_GIT_TOKEN From f8ccaeaaf54008d4f98a686035be839d718c9981 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Thu, 20 Jun 2019 17:13:16 +0200 Subject: [PATCH 02/14] Allow Rails 6 --- core/solidus_core.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index 12fd56a8d15..697a84667b9 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |s| actionmailer actionpack actionview activejob activemodel activerecord activesupport railties ].each do |rails_dep| - s.add_dependency rails_dep, ['>= 5.1', '< 5.3.x'] + s.add_dependency rails_dep, ['>= 5.1', '< 7.0.x'] end s.add_dependency 'activemerchant', '~> 1.66' From 220ea02019739ad308885e3e5435190ac7d49197 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Mon, 26 Aug 2019 17:47:21 +0200 Subject: [PATCH 03/14] Bump rspec-rails to 4.0.0.beta2 for Rails 6 support --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 8c3042592ca..344c2df4986 100644 --- a/Gemfile +++ b/Gemfile @@ -28,7 +28,7 @@ group :backend, :frontend, :core, :api do gem 'database_cleaner', '~> 1.3', require: false gem 'factory_bot_rails', '~> 4.8', require: false gem 'rspec-activemodel-mocks', '~> 1.1', require: false - gem 'rspec-rails', '~> 3.7', require: false + gem 'rspec-rails', '~> 4.0.0.beta2', require: false gem 'simplecov', require: false gem 'with_model', require: false gem 'rails-controller-testing', require: false From 9e73b806c939dd8e5dc2fed486103bdf3e4f890d Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Mon, 26 Aug 2019 17:48:10 +0200 Subject: [PATCH 04/14] Bump awesome_nested_set to 3.2.0 for Rails 6 compatibility awesome_nested_set 3.2.0 is the first release to provide Rails 6 compatibility. --- core/solidus_core.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index 697a84667b9..aecf95ee0f1 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -29,8 +29,8 @@ Gem::Specification.new do |s| s.add_dependency 'activemerchant', '~> 1.66' s.add_dependency 'acts_as_list', '~> 0.3' - s.add_dependency 'awesome_nested_set', '~> 3.0', '>= 3.0.1' s.add_dependency 'cancancan', '~> 2.2' + s.add_dependency 'awesome_nested_set', '~> 3.2' s.add_dependency 'carmen', '~> 1.1.0' s.add_dependency 'discard', '~> 1.0' s.add_dependency 'friendly_id', '~> 5.0' From ee7cf08930f25d119bd28ce0d5504a724fceee86 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Thu, 29 Aug 2019 10:54:32 +0200 Subject: [PATCH 05/14] Allow CanCanCan 3 for Rails 6 compatibility CanCanCan 3.0.0 is the first release with Rails 6 compatibility. --- core/solidus_core.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index aecf95ee0f1..f49492fe454 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -29,8 +29,8 @@ Gem::Specification.new do |s| s.add_dependency 'activemerchant', '~> 1.66' s.add_dependency 'acts_as_list', '~> 0.3' - s.add_dependency 'cancancan', '~> 2.2' s.add_dependency 'awesome_nested_set', '~> 3.2' + s.add_dependency 'cancancan', ['>= 2.2', '< 4.0'] s.add_dependency 'carmen', '~> 1.1.0' s.add_dependency 'discard', '~> 1.0' s.add_dependency 'friendly_id', '~> 5.0' From cb6f72b725b94595d4b0451832462fcb21840e00 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Mon, 26 Aug 2019 17:42:16 +0200 Subject: [PATCH 06/14] Bump state_machines-activerecord for Rails 6 support state_machines-activerecord used to have a level constraint on ActiveRecord that prevented us from using it with Rails 6. This was removed[1] in 0.6.0. [1]: https://github.com/state-machines/state_machines-activerecord/commit/e7857a670fac67475ae6ad9ffeeddd165ec36441 --- core/solidus_core.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index f49492fe454..37809ae662f 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -39,5 +39,5 @@ Gem::Specification.new do |s| s.add_dependency 'paperclip', ['>= 4.2', '< 6'] s.add_dependency 'paranoia', '~> 2.4' s.add_dependency 'ransack', '~> 2.0' - s.add_dependency 'state_machines-activerecord', '~> 0.4' + s.add_dependency 'state_machines-activerecord', '~> 0.6' end From 7a731c019bb9df4e984fda4f1e11dcd73ea7fda0 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Thu, 22 Aug 2019 16:46:19 +0200 Subject: [PATCH 07/14] Add missing policy scopes to DefaultCustomer --- core/lib/spree/permission_sets/default_customer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/spree/permission_sets/default_customer.rb b/core/lib/spree/permission_sets/default_customer.rb index ed2268b5be4..61cb2b66eef 100644 --- a/core/lib/spree/permission_sets/default_customer.rb +++ b/core/lib/spree/permission_sets/default_customer.rb @@ -8,7 +8,7 @@ def activate! can :display, OptionType can :display, OptionValue can :create, Order - can [:read, :update], Order do |order, token| + can [:read, :update], Order, Order.where(user: user) do |order, token| order.user == user || (order.guest_token.present? && token == order.guest_token) end cannot :update, Order do |order| From 06df181451cb699d5115f8901c92ac9a3a497c5d Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Fri, 21 Jun 2019 15:32:56 +0200 Subject: [PATCH 08/14] Make code compatible with Zeitwerk Zeitwerk expects file paths to match module names exactly, so we need to use #prepend for files that extend existing modules. --- core/app/models/spree/product/scopes.rb | 427 ++++++++++++------------ core/app/models/spree/variant/scopes.rb | 66 ++-- core/config/initializers/inflections.rb | 5 + 3 files changed, 259 insertions(+), 239 deletions(-) create mode 100644 core/config/initializers/inflections.rb diff --git a/core/app/models/spree/product/scopes.rb b/core/app/models/spree/product/scopes.rb index da17327388c..3c26eca6f1c 100644 --- a/core/app/models/spree/product/scopes.rb +++ b/core/app/models/spree/product/scopes.rb @@ -2,159 +2,162 @@ module Spree class Product < Spree::Base - cattr_accessor :search_scopes do - [] - end + module Scopes + def self.prepended(base) + base.class_eval do + cattr_accessor :search_scopes do + [] + end - def self.add_search_scope(name, &block) - singleton_class.send(:define_method, name.to_sym, &block) - search_scopes << name.to_sym - end + def self.add_search_scope(name, &block) + singleton_class.send(:define_method, name.to_sym, &block) + search_scopes << name.to_sym + end - def self.property_conditions(property) - properties = Property.table_name - case property - when String then { "#{properties}.name" => property } - when Property then { "#{properties}.id" => property.id } - else { "#{properties}.id" => property.to_i } - end - end + def self.property_conditions(property) + properties = Property.table_name + case property + when String then { "#{properties}.name" => property } + when Property then { "#{properties}.id" => property.id } + else { "#{properties}.id" => property.to_i } + end + end - scope :ascend_by_updated_at, -> { order(updated_at: :asc) } - scope :descend_by_updated_at, -> { order(updated_at: :desc) } - scope :ascend_by_name, -> { order(name: :asc) } - scope :descend_by_name, -> { order(name: :desc) } + scope :ascend_by_updated_at, -> { order(updated_at: :asc) } + scope :descend_by_updated_at, -> { order(updated_at: :desc) } + scope :ascend_by_name, -> { order(name: :asc) } + scope :descend_by_name, -> { order(name: :desc) } - add_search_scope :ascend_by_master_price do - joins(master: :default_price).order(Spree::Price.arel_table[:amount].asc) - end + add_search_scope :ascend_by_master_price do + joins(master: :default_price).order(Spree::Price.arel_table[:amount].asc) + end - add_search_scope :descend_by_master_price do - joins(master: :default_price).order(Spree::Price.arel_table[:amount].desc) - end + add_search_scope :descend_by_master_price do + joins(master: :default_price).order(Spree::Price.arel_table[:amount].desc) + end - add_search_scope :price_between do |low, high| - joins(master: :default_price).where(Price.table_name => { amount: low..high }) - end + add_search_scope :price_between do |low, high| + joins(master: :default_price).where(Price.table_name => { amount: low..high }) + end - add_search_scope :master_price_lte do |price| - joins(master: :default_price).where("#{price_table_name}.amount <= ?", price) - end + add_search_scope :master_price_lte do |price| + joins(master: :default_price).where("#{price_table_name}.amount <= ?", price) + end - add_search_scope :master_price_gte do |price| - joins(master: :default_price).where("#{price_table_name}.amount >= ?", price) - end + add_search_scope :master_price_gte do |price| + joins(master: :default_price).where("#{price_table_name}.amount >= ?", price) + end - # This scope selects products in taxon AND all its descendants - # If you need products only within one taxon use - # - # Spree::Product.joins(:taxons).where(Taxon.table_name => { id: taxon.id }) - # - # If you're using count on the result of this scope, you must use the - # `:distinct` option as well: - # - # Spree::Product.in_taxon(taxon).count(distinct: true) - # - # This is so that the count query is distinct'd: - # - # SELECT COUNT(DISTINCT "spree_products"."id") ... - # - # vs. - # - # SELECT COUNT(*) ... - add_search_scope :in_taxon do |taxon| - includes(:classifications) - .where('spree_products_taxons.taxon_id' => taxon.self_and_descendants.pluck(:id)) - .select(Spree::Classification.arel_table[:position]) - .order(Spree::Classification.arel_table[:position].asc) - end + # This scope selects products in taxon AND all its descendants + # If you need products only within one taxon use + # + # Spree::Product.joins(:taxons).where(Taxon.table_name => { id: taxon.id }) + # + # If you're using count on the result of this scope, you must use the + # `:distinct` option as well: + # + # Spree::Product.in_taxon(taxon).count(distinct: true) + # + # This is so that the count query is distinct'd: + # + # SELECT COUNT(DISTINCT "spree_products"."id") ... + # + # vs. + # + # SELECT COUNT(*) ... + add_search_scope :in_taxon do |taxon| + includes(:classifications) + .where('spree_products_taxons.taxon_id' => taxon.self_and_descendants.pluck(:id)) + .select(Spree::Classification.arel_table[:position]) + .order(Spree::Classification.arel_table[:position].asc) + end - # This scope selects products in all taxons AND all its descendants - # If you need products only within one taxon use - # - # Spree::Product.taxons_id_eq([x,y]) - add_search_scope :in_taxons do |*taxons| - taxons = get_taxons(taxons) - taxons.first ? prepare_taxon_conditions(taxons) : where(nil) - end + # This scope selects products in all taxons AND all its descendants + # If you need products only within one taxon use + # + # Spree::Product.taxons_id_eq([x,y]) + add_search_scope :in_taxons do |*taxons| + taxons = get_taxons(taxons) + taxons.first ? prepare_taxon_conditions(taxons) : where(nil) + end - # a scope that finds all products having property specified by name, object or id - add_search_scope :with_property do |property| - joins(:properties).where(property_conditions(property)) - end + # a scope that finds all products having property specified by name, object or id + add_search_scope :with_property do |property| + joins(:properties).where(property_conditions(property)) + end - # a simple test for product with a certain property-value pairing - # note that it can test for properties with NULL values, but not for absent values - add_search_scope :with_property_value do |property, value| - joins(:properties) - .where("#{Spree::ProductProperty.table_name}.value = ?", value) - .where(property_conditions(property)) - end + # a simple test for product with a certain property-value pairing + # note that it can test for properties with NULL values, but not for absent values + add_search_scope :with_property_value do |property, value| + joins(:properties) + .where("#{Spree::ProductProperty.table_name}.value = ?", value) + .where(property_conditions(property)) + end - add_search_scope :with_option do |option| - option_types = Spree::OptionType.table_name - conditions = case option - when String then { "#{option_types}.name" => option } - when OptionType then { "#{option_types}.id" => option.id } - else { "#{option_types}.id" => option.to_i } - end + add_search_scope :with_option do |option| + option_types = Spree::OptionType.table_name + conditions = case option + when String then { "#{option_types}.name" => option } + when OptionType then { "#{option_types}.id" => option.id } + else { "#{option_types}.id" => option.to_i } + end - joins(:option_types).where(conditions) - end + joins(:option_types).where(conditions) + end - add_search_scope :with_option_value do |option, value| - option_values = Spree::OptionValue.table_name - option_type_id = case option - when String then Spree::OptionType.find_by(name: option) || option.to_i - when Spree::OptionType then option.id - else option.to_i - end + add_search_scope :with_option_value do |option, value| + option_values = Spree::OptionValue.table_name + option_type_id = case option + when String then Spree::OptionType.find_by(name: option) || option.to_i + when Spree::OptionType then option.id + else option.to_i + end - conditions = "#{option_values}.name = ? AND #{option_values}.option_type_id = ?", value, option_type_id - group('spree_products.id').joins(variants_including_master: :option_values).where(conditions) - end + conditions = "#{option_values}.name = ? AND #{option_values}.option_type_id = ?", value, option_type_id + group('spree_products.id').joins(variants_including_master: :option_values).where(conditions) + end - # Finds all products which have either: - # 1) have an option value with the name matching the one given - # 2) have a product property with a value matching the one given - add_search_scope :with do |value| - includes(variants_including_master: :option_values). - includes(:product_properties). - where("#{Spree::OptionValue.table_name}.name = ? OR #{Spree::ProductProperty.table_name}.value = ?", value, value) - end + # Finds all products which have either: + # 1) have an option value with the name matching the one given + # 2) have a product property with a value matching the one given + add_search_scope :with do |value| + includes(variants_including_master: :option_values). + includes(:product_properties). + where("#{Spree::OptionValue.table_name}.name = ? OR #{Spree::ProductProperty.table_name}.value = ?", value, value) + end - # Finds all products that have a name containing the given words. - add_search_scope :in_name do |words| - like_any([:name], prepare_words(words)) - end + # Finds all products that have a name containing the given words. + add_search_scope :in_name do |words| + like_any([:name], prepare_words(words)) + end - # Finds all products that have a name or meta_keywords containing the given words. - add_search_scope :in_name_or_keywords do |words| - like_any([:name, :meta_keywords], prepare_words(words)) - end + # Finds all products that have a name or meta_keywords containing the given words. + add_search_scope :in_name_or_keywords do |words| + like_any([:name, :meta_keywords], prepare_words(words)) + end - # Finds all products that have a name, description, meta_description or meta_keywords containing the given keywords. - add_search_scope :in_name_or_description do |words| - like_any([:name, :description, :meta_description, :meta_keywords], prepare_words(words)) - end + # Finds all products that have a name, description, meta_description or meta_keywords containing the given keywords. + add_search_scope :in_name_or_description do |words| + like_any([:name, :description, :meta_description, :meta_keywords], prepare_words(words)) + end - # Finds all products that have the ids matching the given collection of ids. - # Alternatively, you could use find(collection_of_ids), but that would raise an exception if one product couldn't be found - add_search_scope :with_ids do |*ids| - where(id: ids) - end + # Finds all products that have the ids matching the given collection of ids. + # Alternatively, you could use find(collection_of_ids), but that would raise an exception if one product couldn't be found + add_search_scope :with_ids do |*ids| + where(id: ids) + end - # Sorts products from most popular (popularity is extracted from how many - # times use has put product in cart, not completed orders) - # - # there is alternative faster and more elegant solution, it has small drawback though, - # it doesn stack with other scopes :/ - # - # joins: "LEFT OUTER JOIN (SELECT line_items.variant_id as vid, COUNT(*) as cnt FROM line_items GROUP BY line_items.variant_id) AS popularity_count ON variants.id = vid", - # order: 'COALESCE(cnt, 0) DESC' - add_search_scope :descend_by_popularity do - joins(:master). - order(%{ + # Sorts products from most popular (popularity is extracted from how many + # times use has put product in cart, not completed orders) + # + # there is alternative faster and more elegant solution, it has small drawback though, + # it doesn stack with other scopes :/ + # + # joins: "LEFT OUTER JOIN (SELECT line_items.variant_id as vid, COUNT(*) as cnt FROM line_items GROUP BY line_items.variant_id) AS popularity_count ON variants.id = vid", + # order: 'COALESCE(cnt, 0) DESC' + add_search_scope :descend_by_popularity do + joins(:master). + order(%{ COALESCE(( SELECT COUNT(#{Spree::LineItem.quoted_table_name}.id) @@ -168,93 +171,97 @@ def self.property_conditions(property) popular_variants.product_id = #{Spree::Product.quoted_table_name}.id ), 0) DESC }) - end - - add_search_scope :not_deleted do - where("#{Spree::Product.quoted_table_name}.deleted_at IS NULL or #{Spree::Product.quoted_table_name}.deleted_at >= ?", Time.current) - end - - scope :with_master_price, -> do - joins(:master).where(Spree::Price.where(Spree::Variant.arel_table[:id].eq(Spree::Price.arel_table[:variant_id])).arel.exists) - end - - # Can't use add_search_scope for this as it needs a default argument - def self.available(available_on = nil) - with_master_price.where("#{Spree::Product.quoted_table_name}.available_on <= ?", available_on || Time.current) - end - search_scopes << :available - - add_search_scope :taxons_name_eq do |name| - group("spree_products.id").joins(:taxons).where(Spree::Taxon.arel_table[:name].eq(name)) - end - - def self.with_variant_sku_cont(sku) - sku_match = "%#{sku}%" - variant_table = Spree::Variant.arel_table - subquery = Spree::Variant.where(variant_table[:sku].matches(sku_match).and(variant_table[:product_id].eq(arel_table[:id]))) - where(subquery.arel.exists) - end + end - def self.distinct_by_product_ids(sort_order = nil) - Spree::Deprecation.warn "Product.distinct_by_product_ids is deprecated and should not be used" - - sort_column = sort_order.split(" ").first - - # Postgres will complain when using ordering by expressions not present in - # SELECT DISTINCT. e.g. - # - # PG::InvalidColumnReference: ERROR: for SELECT DISTINCT, ORDER BY - # expressions must appear in select list. e.g. - # - # SELECT DISTINCT "spree_products".* FROM "spree_products" LEFT OUTER JOIN - # "spree_variants" ON "spree_variants"."product_id" = "spree_products"."id" AND "spree_variants"."is_master" = 't' - # AND "spree_variants"."deleted_at" IS NULL LEFT OUTER JOIN "spree_prices" ON - # "spree_prices"."variant_id" = "spree_variants"."id" AND "spree_prices"."currency" = 'USD' - # AND "spree_prices"."deleted_at" IS NULL WHERE "spree_products"."deleted_at" IS NULL AND ('t'='t') - # ORDER BY "spree_prices"."amount" ASC LIMIT 10 OFFSET 0 - # - # Don't allow sort_column, a variable coming from params, - # to be anything but a column in the database - if ActiveRecord::Base.connection.adapter_name == 'PostgreSQL' && !column_names.include?(sort_column) - all - else - distinct - end - end + add_search_scope :not_deleted do + where("#{Spree::Product.quoted_table_name}.deleted_at IS NULL or #{Spree::Product.quoted_table_name}.deleted_at >= ?", Time.current) + end - class << self - private + scope :with_master_price, -> do + joins(:master).where(Spree::Price.where(Spree::Variant.arel_table[:id].eq(Spree::Price.arel_table[:variant_id])).arel.exists) + end + # Can't use add_search_scope for this as it needs a default argument + def self.available(available_on = nil) + with_master_price.where("#{Spree::Product.quoted_table_name}.available_on <= ?", available_on || Time.current) + end + search_scopes << :available - def price_table_name - Spree::Price.quoted_table_name - end + add_search_scope :taxons_name_eq do |name| + group("spree_products.id").joins(:taxons).where(Spree::Taxon.arel_table[:name].eq(name)) + end - # specifically avoid having an order for taxon search (conflicts with main order) - def prepare_taxon_conditions(taxons) - ids = taxons.map { |taxon| taxon.self_and_descendants.pluck(:id) }.flatten.uniq - joins(:taxons).where("#{Spree::Taxon.table_name}.id" => ids) - end + def self.with_variant_sku_cont(sku) + sku_match = "%#{sku}%" + variant_table = Spree::Variant.arel_table + subquery = Spree::Variant.where(variant_table[:sku].matches(sku_match).and(variant_table[:product_id].eq(arel_table[:id]))) + where(subquery.arel.exists) + end - # Produce an array of keywords for use in scopes. - # Always return array with at least an empty string to avoid SQL errors - def prepare_words(words) - return [''] if words.blank? - a = words.split(/[,\s]/).map(&:strip) - a.any? ? a : [''] - end + def self.distinct_by_product_ids(sort_order = nil) + Spree::Deprecation.warn "Product.distinct_by_product_ids is deprecated and should not be used" + + sort_column = sort_order.split(" ").first + + # Postgres will complain when using ordering by expressions not present in + # SELECT DISTINCT. e.g. + # + # PG::InvalidColumnReference: ERROR: for SELECT DISTINCT, ORDER BY + # expressions must appear in select list. e.g. + # + # SELECT DISTINCT "spree_products".* FROM "spree_products" LEFT OUTER JOIN + # "spree_variants" ON "spree_variants"."product_id" = "spree_products"."id" AND "spree_variants"."is_master" = 't' + # AND "spree_variants"."deleted_at" IS NULL LEFT OUTER JOIN "spree_prices" ON + # "spree_prices"."variant_id" = "spree_variants"."id" AND "spree_prices"."currency" = 'USD' + # AND "spree_prices"."deleted_at" IS NULL WHERE "spree_products"."deleted_at" IS NULL AND ('t'='t') + # ORDER BY "spree_prices"."amount" ASC LIMIT 10 OFFSET 0 + # + # Don't allow sort_column, a variable coming from params, + # to be anything but a column in the database + if ActiveRecord::Base.connection.adapter_name == 'PostgreSQL' && !column_names.include?(sort_column) + all + else + distinct + end + end - def get_taxons(*ids_or_records_or_names) - taxons = Spree::Taxon.table_name - ids_or_records_or_names.flatten.map { |t| - case t - when Integer then Spree::Taxon.find_by(id: t) - when ActiveRecord::Base then t - when String - Spree::Taxon.find_by(name: t) || - Spree::Taxon.where("#{taxons}.permalink LIKE ? OR #{taxons}.permalink = ?", "%/#{t}/", "#{t}/").first + class << self + private + + def price_table_name + Spree::Price.quoted_table_name + end + + # specifically avoid having an order for taxon search (conflicts with main order) + def prepare_taxon_conditions(taxons) + ids = taxons.map { |taxon| taxon.self_and_descendants.pluck(:id) }.flatten.uniq + joins(:taxons).where("#{Spree::Taxon.table_name}.id" => ids) + end + + # Produce an array of keywords for use in scopes. + # Always return array with at least an empty string to avoid SQL errors + def prepare_words(words) + return [''] if words.blank? + a = words.split(/[,\s]/).map(&:strip) + a.any? ? a : [''] + end + + def get_taxons(*ids_or_records_or_names) + taxons = Spree::Taxon.table_name + ids_or_records_or_names.flatten.map { |t| + case t + when Integer then Spree::Taxon.find_by(id: t) + when ActiveRecord::Base then t + when String + Spree::Taxon.find_by(name: t) || + Spree::Taxon.where("#{taxons}.permalink LIKE ? OR #{taxons}.permalink = ?", "%/#{t}/", "#{t}/").first + end + }.compact.flatten.uniq + end end - }.compact.flatten.uniq + end end + + ::Spree::Product.prepend self end end end diff --git a/core/app/models/spree/variant/scopes.rb b/core/app/models/spree/variant/scopes.rb index bccd016113d..bdc25341557 100644 --- a/core/app/models/spree/variant/scopes.rb +++ b/core/app/models/spree/variant/scopes.rb @@ -2,41 +2,49 @@ module Spree class Variant < Spree::Base - # FIXME: WARNING tested only under sqlite and postgresql - scope :descend_by_popularity, -> { - order(Arel.sql("COALESCE((SELECT COUNT(*) FROM #{Spree::LineItem.quoted_table_name} GROUP BY #{Spree::LineItem.quoted_table_name}.variant_id HAVING #{Spree::LineItem.quoted_table_name}.variant_id = #{Spree::Variant.quoted_table_name}.id), 0) DESC")) - } - - class << self - # Returns variants that match a given option value - # - # Example: - # - # product.variants_including_master.has_option(OptionType.find_by(name: 'shoe-size'), OptionValue.find_by(name: '8')) - def has_option(option_type, *option_values) - option_types = Spree::OptionType.table_name - - option_type_conditions = case option_type - when OptionType then { "#{option_types}.name" => option_type.name } - when String then { "#{option_types}.name" => option_type } - else { "#{option_types}.id" => option_type } - end + module Scopes + def self.prepended(base) + base.class_eval do + # FIXME: WARNING tested only under sqlite and postgresql + scope :descend_by_popularity, -> { + order(Arel.sql("COALESCE((SELECT COUNT(*) FROM #{Spree::LineItem.quoted_table_name} GROUP BY #{Spree::LineItem.quoted_table_name}.variant_id HAVING #{Spree::LineItem.quoted_table_name}.variant_id = #{Spree::Variant.quoted_table_name}.id), 0) DESC")) + } + + class << self + # Returns variants that match a given option value + # + # Example: + # + # product.variants_including_master.has_option(OptionType.find_by(name: 'shoe-size'), OptionValue.find_by(name: '8')) + def has_option(option_type, *option_values) + option_types = Spree::OptionType.table_name + + option_type_conditions = case option_type + when OptionType then { "#{option_types}.name" => option_type.name } + when String then { "#{option_types}.name" => option_type } + else { "#{option_types}.id" => option_type } + end - relation = joins(option_values: :option_type).where(option_type_conditions) + relation = joins(option_values: :option_type).where(option_type_conditions) - option_values.each do |option_value| - option_value_conditions = case option_value - when OptionValue then { "#{Spree::OptionValue.table_name}.name" => option_value.name } - when String then { "#{Spree::OptionValue.table_name}.name" => option_value } - else { "#{Spree::OptionValue.table_name}.id" => option_value } + option_values.each do |option_value| + option_value_conditions = case option_value + when OptionValue then { "#{Spree::OptionValue.table_name}.name" => option_value.name } + when String then { "#{Spree::OptionValue.table_name}.name" => option_value } + else { "#{Spree::OptionValue.table_name}.id" => option_value } + end + relation = relation.where(option_value_conditions) + end + + relation + end + + alias_method :has_options, :has_option end - relation = relation.where(option_value_conditions) end - - relation end - alias_method :has_options, :has_option + ::Spree::Variant.prepend self end end end diff --git a/core/config/initializers/inflections.rb b/core/config/initializers/inflections.rb new file mode 100644 index 00000000000..0ae0cd267ee --- /dev/null +++ b/core/config/initializers/inflections.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +ActiveSupport::Inflector.inflections(:en) do |inflect| + inflect.acronym 'RMA' +end From 3856d95d6701c2226bc7cfa6c4408c9e105d6e52 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Fri, 21 Jun 2019 14:42:40 +0200 Subject: [PATCH 09/14] Fix Search::Base spec for Rails 6 compatibility In Rails 6, ActiveRecord will always cast numbers to decimals when comparing them against a decimal column. --- core/spec/lib/search/base_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/spec/lib/search/base_spec.rb b/core/spec/lib/search/base_spec.rb index df3d470dae2..31f281213f3 100644 --- a/core/spec/lib/search/base_spec.rb +++ b/core/spec/lib/search/base_spec.rb @@ -60,7 +60,11 @@ search: { "price_range_any" => ["Under $10.00", "$10.00 - $15.00"] } } searcher = Spree::Core::Search::Base.new(params) expect(searcher.send(:get_base_scope).to_sql).to match /<= 10/ - expect(searcher.send(:get_base_scope).to_sql).to match /between 10 and 15/i + if Rails.gem_version >= Gem::Version.new('6.0.0') + expect(searcher.send(:get_base_scope).to_sql).to match /between 10\.0 and 15\.0/i + else + expect(searcher.send(:get_base_scope).to_sql).to match /between 10 and 15/i + end expect(searcher.retrieve_products.count).to eq(2) end From f95eb942aeb029c736c0657bc286e371b59e79a2 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Fri, 21 Jun 2019 14:42:43 +0200 Subject: [PATCH 10/14] Fix Event spec for Rails 6 compatibility In Rails 6, ActiveSupport notifiers use two separate instance variables for string subscribers and object subscribers, so we need to use different setup/teardown logic for the test. --- core/spec/lib/spree/event_spec.rb | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/spec/lib/spree/event_spec.rb b/core/spec/lib/spree/event_spec.rb index 26cfdd4f472..ed7503c090c 100644 --- a/core/spec/lib/spree/event_spec.rb +++ b/core/spec/lib/spree/event_spec.rb @@ -18,14 +18,26 @@ before do # ActiveSupport::Notifications does not provide an interface to clean all # subscribers at once, so some low level brittle code is required - @old_subscribers = notifier.instance_variable_get('@subscribers').dup + if Rails.gem_version >= Gem::Version.new('6.0.0') + @old_string_subscribers = notifier.instance_variable_get('@string_subscribers').dup + @old_other_subscribers = notifier.instance_variable_get('@other_subscribers').dup + notifier.instance_variable_get('@string_subscribers').clear + notifier.instance_variable_get('@other_subscribers').clear + else + @old_subscribers = notifier.instance_variable_get('@subscribers').dup + notifier.instance_variable_get('@subscribers').clear + end @old_listeners = notifier.instance_variable_get('@listeners_for').dup - notifier.instance_variable_get('@subscribers').clear notifier.instance_variable_get('@listeners_for').clear end after do - notifier.instance_variable_set '@subscribers', @old_subscribers + if Rails.gem_version >= Gem::Version.new('6.0.0') + notifier.instance_variable_set '@string_subscribers', @old_string_subscribers + notifier.instance_variable_set '@other_subscribers', @old_other_subscribers + else + notifier.instance_variable_set '@subscribers', @old_subscribers + end notifier.instance_variable_set '@listeners_for', @old_listeners end From 7b5f095822193d744acc46991fb1d2f0cb1a0434 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Thu, 22 Aug 2019 17:10:17 +0200 Subject: [PATCH 11/14] Fix usage of ActiveRecord migration API for Rails 6 --- .../testing_support/dummy_app/rake_tasks.rb | 8 +++++-- ..._remove_code_from_spree_promotions_spec.rb | 22 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/core/lib/spree/testing_support/dummy_app/rake_tasks.rb b/core/lib/spree/testing_support/dummy_app/rake_tasks.rb index 573c9ba0826..fa49f5c62b7 100644 --- a/core/lib/spree/testing_support/dummy_app/rake_tasks.rb +++ b/core/lib/spree/testing_support/dummy_app/rake_tasks.rb @@ -38,8 +38,12 @@ def initialize(gem_root:, lib_name:) # railties:install:migrations and then db:migrate. # Migrations should be run one directory at a time ActiveRecord::Migrator.migrations_paths.each do |path| - if defined?(ActiveRecord::MigrationContext) - # Rails >= 5.2 + if Rails.gem_version >= Gem::Version.new('6.0.0') + ActiveRecord::MigrationContext.new( + [path], + ActiveRecord::SchemaMigration + ).migrate + elsif Rails.gem_version >= Gem::Version.new('5.2.0') ActiveRecord::MigrationContext.new([path]).migrate else ActiveRecord::Migrator.migrate(path) diff --git a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb index 1ad32aee15f..c33b98b9c1e 100644 --- a/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb +++ b/core/spec/migrate/20190106184413_remove_code_from_spree_promotions_spec.rb @@ -6,8 +6,12 @@ RSpec.describe RemoveCodeFromSpreePromotions do let(:migrations_paths) { ActiveRecord::Migrator.migrations_paths } let(:migrations) do - if ActiveRecord::Base.connection.respond_to?(:migration_context) - # Rails >= 5.2 + if Rails.gem_version >= Gem::Version.new('6.0.0') + ActiveRecord::MigrationContext.new( + migrations_paths, + ActiveRecord::SchemaMigration + ).migrations + elsif Rails.gem_version >= Gem::Version.new('5.2.0') ActiveRecord::MigrationContext.new(migrations_paths).migrations else ActiveRecord::Migrator.migrations(migrations_paths) @@ -16,7 +20,13 @@ let(:previous_version) { 20180710170104 } let(:current_version) { 20190106184413 } - subject { ActiveRecord::Migrator.new(:up, migrations, current_version).migrate } + subject do + if Rails.gem_version >= Gem::Version.new('6.0.0') + ActiveRecord::Migrator.new(:up, migrations, ActiveRecord::SchemaMigration, current_version).migrate + else + ActiveRecord::Migrator.new(:up, migrations, current_version).migrate + end + end # This is needed for MySQL since it is not able to rollback to the previous # state when database schema changes within that transaction. @@ -28,7 +38,11 @@ # Silence migrations output in specs report. ActiveRecord::Migration.suppress_messages do # Migrate back to the previous version - ActiveRecord::Migrator.new(:down, migrations, previous_version).migrate + if Rails.gem_version >= Gem::Version.new('6.0.0') + ActiveRecord::Migrator.new(:down, migrations, ActiveRecord::SchemaMigration, previous_version).migrate + else + ActiveRecord::Migrator.new(:down, migrations, previous_version).migrate + end # If other tests using Spree::Promotion ran before this one, Rails has # stored information about table's columns and we need to reset those # since the migration changed the database structure. From c4f65f5ee5f8db7076158a4a610b207da4aa09b1 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Fri, 23 Aug 2019 13:12:06 +0200 Subject: [PATCH 12/14] Update Teaspoon for Rails 6 support The changes to the CI configuration are needed because teaspoon_env is now[1] searched starting from Rails.root when Rails is available, which breaks our test suite because Rails.root is spec/dummy, while teaspoon_env.rb is in spec. The monkey-patch was removed because Teaspoon now supports passing options to Selenium[2]. [1]: https://github.com/jejacks0n/teaspoon/commit/5b912da349a11d12ccdf1705a1aeb79e4f1e3a57 [2]: https://github.com/jejacks0n/teaspoon/pull/537 --- Gemfile | 4 ++-- backend/spec/teaspoon_env.rb | 31 ++++++++----------------------- bin/build | 2 +- bin/build-ci | 16 +++++++++++++--- 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/Gemfile b/Gemfile index 344c2df4986..b56064684fe 100644 --- a/Gemfile +++ b/Gemfile @@ -46,8 +46,8 @@ group :frontend do end group :backend do - gem 'teaspoon', require: false - gem 'teaspoon-mocha', require: false + gem 'teaspoon', github: 'jejacks0n/teaspoon', require: false + gem 'teaspoon-mocha', github: 'jejacks0n/teaspoon', require: false end group :utils do diff --git a/backend/spec/teaspoon_env.rb b/backend/spec/teaspoon_env.rb index bed9812ba8c..803f98dd76c 100644 --- a/backend/spec/teaspoon_env.rb +++ b/backend/spec/teaspoon_env.rb @@ -2,27 +2,8 @@ ENV['RAILS_ENV'] = 'test' -# Teaspoon doesn't allow you to pass client driver options to the Selenium WebDriver. This monkey patch -# is a temporary fix until this PR is merged: https://github.com/jejacks0n/teaspoon/pull/519. require 'teaspoon/driver/selenium' -Teaspoon::Driver::Selenium.class_eval do - def run_specs(runner, url) - driver = ::Selenium::WebDriver.for(driver_options[:client_driver], @options.except(:client_driver) || {}) - driver.navigate.to(url) - - ::Selenium::WebDriver::Wait.new(driver_options).until do - done = driver.execute_script("return window.Teaspoon && window.Teaspoon.finished") - driver.execute_script("return window.Teaspoon && window.Teaspoon.getMessages() || []").each do |line| - runner.process("#{line}\n") - end - done - end - ensure - driver.quit if driver - end -end - # Similar to setup described in # https://github.com/jejacks0n/teaspoon/wiki/Micro-Applications @@ -38,10 +19,14 @@ def run_specs(runner, url) config.fixture_paths = ["spec/javascripts/fixtures"] config.driver = :selenium - capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( - chromeOptions: { args: %w(headless disable-gpu window-size=1920,1440) } - ) - config.driver_options = { client_driver: :chrome, desired_capabilities: capabilities } + config.driver_options = { + client_driver: :chrome, + selenium_options: { + options: Selenium::WebDriver::Chrome::Options.new( + args: %w(headless disable-gpu window-size=1920,1440), + ), + }, + } config.suite do |suite| suite.use_framework :mocha, "2.3.3" diff --git a/bin/build b/bin/build index bf08e1ec5a7..e09a7776aa1 100755 --- a/bin/build +++ b/bin/build @@ -22,7 +22,7 @@ echo "* Testing Solidus Backend *" echo "***************************" cd ../backend bundle exec rspec spec -bundle exec teaspoon +bundle exec teaspoon --require=../backend/spec/teaspoon_env.rb # Solidus Core echo "************************" diff --git a/bin/build-ci b/bin/build-ci index b63ee8e579f..4668c19fd2f 100755 --- a/bin/build-ci +++ b/bin/build-ci @@ -1,6 +1,7 @@ #!/usr/bin/env ruby # frozen_string_literal: true +require 'fileutils' require 'pathname' class Project @@ -159,7 +160,9 @@ class Project end def run_teaspoon - run_test_cmd(%w[bundle exec teaspoon] + teaspoon_arguments) + cmd = %w[bundle exec teaspoon] + teaspoon_arguments + + run_test_cmd(cmd) end def run_test_cmd(args) @@ -170,11 +173,18 @@ class Project end def teaspoon_arguments + args = [] + + args << '--require=spec/teaspoon_env.rb' + if report_dir - %W[--format documentation,junit>#{report_dir}/rspec/#{name}_js.xml] + FileUtils.mkdir_p("#{report_dir}/rspec") + args << "--format=documentation,junit>#{report_dir}/rspec/#{name}_js.xml" else - %w[--format documentation] + args << '--format=documentation' end + + args end def rspec_arguments From 91daf841bd6318ccf53d6e975e396d57183a4ca7 Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Thu, 29 Aug 2019 16:27:13 +0200 Subject: [PATCH 13/14] Make Rails 6 the default in CircleCI --- .circleci/config.yml | 12 ++++++------ Gemfile | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1e43d632152..ec9e4980356 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -121,20 +121,20 @@ jobs: - setup - test - postgres_rails60: + postgres_rails52: executor: postgres parallelism: *parallelism environment: - RAILS_VERSION: '~> 6.0.0' + RAILS_VERSION: '~> 5.2.0' steps: - setup - test - mysql_rails60: + mysql_rails52: executor: mysql parallelism: *parallelism environment: - RAILS_VERSION: '~> 6.0.0' + RAILS_VERSION: '~> 5.2.0' steps: - setup - test @@ -150,8 +150,8 @@ workflows: - mysql - postgres_rails51 - mysql_rails51 - - postgres_rails60 - - mysql_rails60 + - postgres_rails52 + - mysql_rails52 - stoplight/push: project: solidus/solidus-api git_token: $STOPLIGHT_GIT_TOKEN diff --git a/Gemfile b/Gemfile index b56064684fe..e12347e5f93 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ source 'https://rubygems.org' group :backend, :frontend, :core, :api do gemspec require: false - rails_version = ENV['RAILS_VERSION'] || '~> 5.2.0' + rails_version = ENV['RAILS_VERSION'] || '~> 6.0.0' gem 'rails', rails_version, require: false platforms :ruby do From 614ecbe4c4ea85a0ae27762ccb6a2e9726c062ea Mon Sep 17 00:00:00 2001 From: Alessandro Desantis Date: Mon, 2 Sep 2019 12:11:45 +0200 Subject: [PATCH 14/14] Update readme to indicate we support Rails 6 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 36820bbe502..6f1275da3d7 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ Begin by making sure you have required for Paperclip. (You can install it using [Homebrew](https://brew.sh) if you're on a Mac.) -To add solidus, begin with a Rails 5 application and a database configured and +To add solidus, begin with a Rails 5/6 application and a database configured and created. Add the following to your Gemfile. ```ruby