From 9e519f1c0458c0fbc7387b43e4f4ef7a26d9f3e6 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 15 Dec 2017 15:15:13 -0800 Subject: [PATCH 1/5] Avoid `if: string` removed from Rails 5.2 Rails 5.2 has removed the ability to pass a string as the :if argument. Fortunately, we didn't really need it here. --- core/app/models/spree/stock_location.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app/models/spree/stock_location.rb b/core/app/models/spree/stock_location.rb index 8a338a6ac18..c29a52e7e95 100644 --- a/core/app/models/spree/stock_location.rb +++ b/core/app/models/spree/stock_location.rb @@ -24,7 +24,7 @@ class InvalidMovementError < StandardError; end scope :active, -> { where(active: true) } scope :order_default, -> { order(default: :desc, name: :asc) } - after_create :create_stock_items, if: "self.propagate_all_variants?" + after_create :create_stock_items, if: :propagate_all_variants? after_save :ensure_one_default def state_text From 885cbd40b78ab06ce4d0627c15b7ad36e4be2dcb Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 20 Dec 2017 09:29:02 -0800 Subject: [PATCH 2/5] Avoid dangerous argument warning under Rails 5.2 Rails 5.2 doesn't allow passing raw strings to order (to avoid accidentally allowing user-provided strings) --- core/app/models/spree/order.rb | 2 +- core/app/models/spree/price.rb | 2 +- core/app/models/spree/taxonomy.rb | 2 +- core/app/models/spree/variant/scopes.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 96486c522d7..4372118f2bf 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -157,7 +157,7 @@ def self.find_by_param!(value) scope :by_store, ->(store) { where(store_id: store.id) } # shows completed orders first, by their completed_at date, then uncompleted orders by their created_at - scope :reverse_chronological, -> { order('spree_orders.completed_at IS NULL', completed_at: :desc, created_at: :desc) } + scope :reverse_chronological, -> { order(Arel.sql('spree_orders.completed_at IS NULL'), completed_at: :desc, created_at: :desc) } def self.by_customer(customer) joins(:user).where("#{Spree.user_class.table_name}.email" => customer) diff --git a/core/app/models/spree/price.rb b/core/app/models/spree/price.rb index c91104910d4..5d61dc8d1a0 100644 --- a/core/app/models/spree/price.rb +++ b/core/app/models/spree/price.rb @@ -18,7 +18,7 @@ class Price < Spree::Base validates :currency, inclusion: { in: ::Money::Currency.all.map(&:iso_code), message: :invalid_code } validates :country, presence: true, unless: -> { for_any_country? } - scope :currently_valid, -> { order("country_iso IS NULL, updated_at DESC, id DESC") } + scope :currently_valid, -> { order(Arel.sql("country_iso IS NULL, updated_at DESC, id DESC")) } scope :for_master, -> { joins(:variant).where(spree_variants: { is_master: true }) } scope :for_variant, -> { joins(:variant).where(spree_variants: { is_master: false }) } scope :for_any_country, -> { where(country: nil) } diff --git a/core/app/models/spree/taxonomy.rb b/core/app/models/spree/taxonomy.rb index 52a5a21f228..73dcde3dfbb 100644 --- a/core/app/models/spree/taxonomy.rb +++ b/core/app/models/spree/taxonomy.rb @@ -9,7 +9,7 @@ class Taxonomy < Spree::Base after_save :set_name - default_scope -> { order(:position) } + default_scope -> { order(position: :asc) } private diff --git a/core/app/models/spree/variant/scopes.rb b/core/app/models/spree/variant/scopes.rb index 599a0c985d5..7163f6a94e9 100644 --- a/core/app/models/spree/variant/scopes.rb +++ b/core/app/models/spree/variant/scopes.rb @@ -2,7 +2,7 @@ module Spree class Variant < Spree::Base # FIXME: WARNING tested only under sqlite and postgresql scope :descend_by_popularity, -> { - order("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") + 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 From 6cdf34dbcf0501cb27561671a1b7c9b21efbbc21 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 20 Dec 2017 09:28:24 -0800 Subject: [PATCH 3/5] Remove secret_token from dummy_app secret_token is deprecated in Rails 5.2 --- core/lib/spree/testing_support/dummy_app.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index eec31c99c1e..6eb362a11e0 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -53,7 +53,6 @@ class Application < ::Rails::Application config.action_mailer.delivery_method = :test config.action_controller.allow_forgery_protection = false config.active_support.deprecation = :stderr - config.secret_token = 'SECRET_TOKEN' config.secret_key_base = 'SECRET_TOKEN' # Avoid issues if an old spec/dummy still exists From 2c70914f26fb999895ea9eb3ae1959a8dc1ff11e Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 20 Dec 2017 09:24:39 -0800 Subject: [PATCH 4/5] Avoid arel delegation warnings under Rails 5.2 Rails 5.2 has deprecated calling arel methods through delegation on an AR scope. --- core/app/models/spree/product/scopes.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/product/scopes.rb b/core/app/models/spree/product/scopes.rb index c8e42005368..35beecc0e3a 100644 --- a/core/app/models/spree/product/scopes.rb +++ b/core/app/models/spree/product/scopes.rb @@ -172,7 +172,7 @@ def self.property_conditions(property) end scope :with_master_price, -> do - joins(:master).where(Spree::Price.where(Spree::Variant.arel_table[:id].eq(Spree::Price.arel_table[:variant_id])).exists) + 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 @@ -189,7 +189,7 @@ 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.exists) + where(subquery.arel.exists) end def self.distinct_by_product_ids(sort_order = nil) From 9522e791c0ea5603afa709c4aebef61275824a66 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 2 Jan 2018 16:38:36 -0800 Subject: [PATCH 5/5] Use be_successful as be_success is soon deprecated --- .../spree/api/resource_controller_spec.rb | 18 +++++++++--------- .../spree/api/config_controller_spec.rb | 4 ++-- .../spree/api/orders_controller_spec.rb | 12 ++++++------ .../spree/api/shipments_controller_spec.rb | 2 +- .../spree/api/stock_items_controller_spec.rb | 2 +- .../api/stock_locations_controller_spec.rb | 14 +++++++------- .../spree/api/taxons_controller_spec.rb | 4 ++-- .../admin/payment_methods_controller_spec.rb | 2 +- .../spree/admin/prices_controller_spec.rb | 4 ++-- .../admin/promotion_codes_controller_spec.rb | 2 +- .../spree/admin/reports_controller_spec.rb | 2 +- .../spree/admin/resource_controller_spec.rb | 4 ++-- .../spree/admin/search_controller_spec.rb | 2 +- .../admin/stock_locations_controller_spec.rb | 4 ++-- .../spree/admin/users_controller_spec.rb | 2 +- 15 files changed, 39 insertions(+), 39 deletions(-) diff --git a/api/spec/controllers/spree/api/resource_controller_spec.rb b/api/spec/controllers/spree/api/resource_controller_spec.rb index 664cae1a9d2..143c13100a7 100644 --- a/api/spec/controllers/spree/api/resource_controller_spec.rb +++ b/api/spec/controllers/spree/api/resource_controller_spec.rb @@ -51,14 +51,14 @@ def permitted_widget_attributes it "returns no widgets" do get :index, params: { token: user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets']).to be_blank end context "it has authorization to read widgets" do it "returns widgets" do get :index, params: { token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets']).to include(hash_including( 'name' => 'a widget', 'position' => 1 @@ -70,26 +70,26 @@ def permitted_widget_attributes it "returns both widgets from comma separated string" do get :index, params: { ids: [widget.id, widget2.id].join(','), token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets'].size).to eq 2 end it "returns both widgets from multiple arguments" do get :index, params: { ids: [widget.id, widget2.id], token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets'].size).to eq 2 end it "returns one requested widgets" do get :index, params: { ids: widget2.id.to_s, token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets'].size).to eq 1 expect(json_response['widgets'][0]['id']).to eq widget2.id end it "returns no widgets if empty" do get :index, params: { ids: '', token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['widgets']).to be_empty end end @@ -107,7 +107,7 @@ def permitted_widget_attributes context "it has authorization read widgets" do it "returns widget details" do get :show, params: { id: widget.to_param, token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['name']).to eq 'a widget' end end @@ -122,7 +122,7 @@ def permitted_widget_attributes context "it is allowed to view a new widget" do it "can learn how to create a new widget" do get :new, params: { token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response["attributes"]).to eq(['name']) end end @@ -159,7 +159,7 @@ def permitted_widget_attributes context "it is authorized to update widgets" do it "can update a widget" do put :update, params: { id: widget.to_param, widget: { name: "another widget" }, token: admin_user.spree_api_key }, as: :json - expect(response).to be_success + expect(response).to be_successful expect(json_response['name']).to eq 'another widget' expect(widget.reload.name).to eq 'another widget' end diff --git a/api/spec/requests/spree/api/config_controller_spec.rb b/api/spec/requests/spree/api/config_controller_spec.rb index 3b02b5ee6fa..3b09cab2604 100644 --- a/api/spec/requests/spree/api/config_controller_spec.rb +++ b/api/spec/requests/spree/api/config_controller_spec.rb @@ -10,13 +10,13 @@ module Spree it "returns Spree::Money settings" do get '/api/config/money' - expect(response).to be_success + expect(response).to be_successful expect(json_response["symbol"]).to eq("$") end it "returns some configuration settings" do get '/api/config' - expect(response).to be_success + expect(response).to be_successful expect(json_response["default_country_iso"]).to eq("US") expect(json_response["default_country_id"]).to eq(default_country.id) end diff --git a/api/spec/requests/spree/api/orders_controller_spec.rb b/api/spec/requests/spree/api/orders_controller_spec.rb index 2ea5fd622db..928a4e45954 100644 --- a/api/spec/requests/spree/api/orders_controller_spec.rb +++ b/api/spec/requests/spree/api/orders_controller_spec.rb @@ -45,20 +45,20 @@ module Spree it "does not include unpermitted params, or allow overriding the user" do subject - expect(response).to be_success + expect(response).to be_successful order = Spree::Order.last expect(order.user).to eq current_api_user expect(order.email).to eq target_user.email end - it { is_expected.to be_success } + it { is_expected.to be_successful } context 'creating payment' do let(:attributes) { super().merge(payments_attributes: [{ payment_method_id: payment_method.id }]) } context "with allowed payment method" do let!(:payment_method) { create(:check_payment_method, name: "allowed" ) } - it { is_expected.to be_success } + it { is_expected.to be_successful } it "creates a payment" do expect { subject @@ -91,7 +91,7 @@ module Spree expect(order.created_at).to eq date_override end - it { is_expected.to be_success } + it { is_expected.to be_successful } end context 'when the line items have custom attributes' do @@ -126,7 +126,7 @@ module Spree }.to change { order.reload.email }.to("foo@foobar.com") end - it { is_expected.to be_success } + it { is_expected.to be_successful } it "does not associate users" do expect { @@ -145,7 +145,7 @@ module Spree context "with allowed payment method" do let!(:payment_method) { create(:check_payment_method, name: "allowed" ) } - it { is_expected.to be_success } + it { is_expected.to be_successful } it "creates a payment" do expect { subject diff --git a/api/spec/requests/spree/api/shipments_controller_spec.rb b/api/spec/requests/spree/api/shipments_controller_spec.rb index 09e4ae1ed48..6cdea3ed32f 100644 --- a/api/spec/requests/spree/api/shipments_controller_spec.rb +++ b/api/spec/requests/spree/api/shipments_controller_spec.rb @@ -368,7 +368,7 @@ it "returns the correct message" do subject - expect(response).to be_success + expect(response).to be_successful expect(parsed_response["success"]).to be true expect(parsed_response["message"]).to eq("Variants successfully transferred") end diff --git a/api/spec/requests/spree/api/stock_items_controller_spec.rb b/api/spec/requests/spree/api/stock_items_controller_spec.rb index 6cb7a8f0e66..f9bd56eb453 100644 --- a/api/spec/requests/spree/api/stock_items_controller_spec.rb +++ b/api/spec/requests/spree/api/stock_items_controller_spec.rb @@ -18,7 +18,7 @@ module Spree describe "#index" do it "can list stock items for an active stock location" do get spree.api_stock_location_stock_items_path(stock_location) - expect(response).to be_success + expect(response).to be_successful expect(json_response['stock_items'].first).to have_attributes(attributes) expect(json_response['stock_items'].first['variant']['sku']).to match /\ASKU-\d+\z/ end diff --git a/api/spec/requests/spree/api/stock_locations_controller_spec.rb b/api/spec/requests/spree/api/stock_locations_controller_spec.rb index 80d69c26290..8a02cade50c 100644 --- a/api/spec/requests/spree/api/stock_locations_controller_spec.rb +++ b/api/spec/requests/spree/api/stock_locations_controller_spec.rb @@ -14,7 +14,7 @@ module Spree describe "#index" do it "can see active stock locations" do get spree.api_stock_locations_path - expect(response).to be_success + expect(response).to be_successful stock_locations = json_response['stock_locations'].map { |sl| sl['name'] } expect(stock_locations).to include stock_location.name end @@ -22,7 +22,7 @@ module Spree it "cannot see inactive stock locations" do stock_location.update_attributes!(active: false) get spree.api_stock_locations_path - expect(response).to be_success + expect(response).to be_successful stock_locations = json_response['stock_locations'].map { |sl| sl['name'] } expect(stock_locations).not_to include stock_location.name end @@ -31,7 +31,7 @@ module Spree describe "#show" do it "can see active stock locations" do get spree.api_stock_location_path(stock_location) - expect(response).to be_success + expect(response).to be_successful expect(json_response['name']).to eq stock_location.name end @@ -77,7 +77,7 @@ module Spree describe "#index" do it "can see active stock locations" do get spree.api_stock_locations_path - expect(response).to be_success + expect(response).to be_successful stock_locations = json_response['stock_locations'].map { |sl| sl['name'] } expect(stock_locations).to include stock_location.name end @@ -85,7 +85,7 @@ module Spree it "can see inactive stock locations" do stock_location.update_attributes!(active: false) get spree.api_stock_locations_path - expect(response).to be_success + expect(response).to be_successful stock_locations = json_response['stock_locations'].map { |sl| sl['name'] } expect(stock_locations).to include stock_location.name end @@ -116,14 +116,14 @@ module Spree describe "#show" do it "can see active stock locations" do get spree.api_stock_location_path(stock_location) - expect(response).to be_success + expect(response).to be_successful expect(json_response['name']).to eq stock_location.name end it "can see inactive stock locations" do stock_location.update_attributes!(active: false) get spree.api_stock_location_path(stock_location) - expect(response).to be_success + expect(response).to be_successful expect(json_response['name']).to eq stock_location.name end end diff --git a/api/spec/requests/spree/api/taxons_controller_spec.rb b/api/spec/requests/spree/api/taxons_controller_spec.rb index 9bca27bdb46..dee23358843 100644 --- a/api/spec/requests/spree/api/taxons_controller_spec.rb +++ b/api/spec/requests/spree/api/taxons_controller_spec.rb @@ -131,11 +131,11 @@ module Spree it "handles exclude_data correctly" do get spree.api_taxon_products_path, params: { id: taxon.id, simple: true } - expect(response).to be_success + expect(response).to be_successful simple_response = json_response get spree.api_taxon_products_path, params: { id: taxon.id } - expect(response).to be_success + expect(response).to be_successful full_response = json_response expect(simple_response["products"][0]["description"]).to be_nil diff --git a/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb index d09d931bad9..6fe960b2ab3 100644 --- a/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -60,7 +60,7 @@ class GatewayWithPassword < PaymentMethod second_method.move_to_top end - it { is_expected.to be_success } + it { is_expected.to be_successful } it { is_expected.to render_template "index" } it "respects the order of payment methods by position" do diff --git a/backend/spec/controllers/spree/admin/prices_controller_spec.rb b/backend/spec/controllers/spree/admin/prices_controller_spec.rb index 2f1f5f396af..e8fd91a9bff 100644 --- a/backend/spec/controllers/spree/admin/prices_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/prices_controller_spec.rb @@ -11,7 +11,7 @@ subject { get :index, params: { product_id: product.slug } } - it { is_expected.to be_success } + it { is_expected.to be_successful } it 'assigns usable instance variables' do subject @@ -28,7 +28,7 @@ subject { get :index, params: { product_id: product.slug, variant_id: variant.id } } - it { is_expected.to be_success } + it { is_expected.to be_successful } it 'assigns usable instance variables' do subject diff --git a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb index 3d44545d1fa..32271184611 100644 --- a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb @@ -11,7 +11,7 @@ it "can create a promotion rule of a valid type" do get :index, params: { promotion_id: promotion.id, format: 'csv' } - expect(response).to be_success + expect(response).to be_successful parsed = CSV.parse(response.body, headers: true) expect(parsed.entries.map(&:to_h)).to eq([{ "Code" => code1.value }, { "Code" => code2.value }, { "Code" => code3.value }]) end diff --git a/backend/spec/controllers/spree/admin/reports_controller_spec.rb b/backend/spec/controllers/spree/admin/reports_controller_spec.rb index 6adeb8be8b4..c2ad0702425 100644 --- a/backend/spec/controllers/spree/admin/reports_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/reports_controller_spec.rb @@ -48,7 +48,7 @@ shared_examples 'sales total report' do it 'should respond with success' do - expect(response).to be_success + expect(response).to be_successful end it 'should set search to be a ransack search' do diff --git a/backend/spec/controllers/spree/admin/resource_controller_spec.rb b/backend/spec/controllers/spree/admin/resource_controller_spec.rb index 6f752f99e0a..59eda9d5322 100644 --- a/backend/spec/controllers/spree/admin/resource_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/resource_controller_spec.rb @@ -50,7 +50,7 @@ def model_class it 'succeeds' do subject - expect(response).to be_success + expect(response).to be_successful end end @@ -63,7 +63,7 @@ def model_class it 'succeeds' do subject - expect(response).to be_success + expect(response).to be_successful end end diff --git a/backend/spec/controllers/spree/admin/search_controller_spec.rb b/backend/spec/controllers/spree/admin/search_controller_spec.rb index 6788681dddf..5e405534886 100644 --- a/backend/spec/controllers/spree/admin/search_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/search_controller_spec.rb @@ -71,7 +71,7 @@ shared_examples_for 'product search' do it 'should respond with http success' do subject - expect(response).to be_success + expect(response).to be_successful end it 'should set the Surrogate-Control header' do diff --git a/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb b/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb index 594bb0fe731..9fec24b2aab 100644 --- a/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb @@ -23,7 +23,7 @@ module Admin it "can create a new stock location" do get :new - expect(response).to be_success + expect(response).to be_successful end end @@ -34,7 +34,7 @@ module Admin it "can create a new stock location" do get :new - expect(response).to be_success + expect(response).to be_successful end end end diff --git a/backend/spec/controllers/spree/admin/users_controller_spec.rb b/backend/spec/controllers/spree/admin/users_controller_spec.rb index 3fec043d665..672cff3a4f3 100644 --- a/backend/spec/controllers/spree/admin/users_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/users_controller_spec.rb @@ -37,7 +37,7 @@ it 'can visit index' do post :index - expect(response).to be_success + expect(response).to be_successful end it "allows admins to update a user's API key" do