From ab67a4f80c8bc4ebae1325d5e9181201f44c063f Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 14:45:04 +0100 Subject: [PATCH 01/24] Bring base controller from spree --- app/controllers/spree/base_controller.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 app/controllers/spree/base_controller.rb diff --git a/app/controllers/spree/base_controller.rb b/app/controllers/spree/base_controller.rb new file mode 100644 index 0000000000..6ce3e456ac --- /dev/null +++ b/app/controllers/spree/base_controller.rb @@ -0,0 +1,16 @@ +require 'cancan' +require_dependency 'spree/core/controller_helpers/strong_parameters' + +class Spree::BaseController < ApplicationController + include Spree::Core::ControllerHelpers::Auth + include Spree::Core::ControllerHelpers::RespondWith + include Spree::Core::ControllerHelpers::SSL + include Spree::Core::ControllerHelpers::Common + include Spree::Core::ControllerHelpers::Search + include Spree::Core::ControllerHelpers::StrongParameters + include Spree::Core::ControllerHelpers::Search + + respond_to :html +end + +require 'spree/i18n/initializer' From fdd21d7d7d9480161662c991accdbff6b08b0bb8 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 14:46:04 +0100 Subject: [PATCH 02/24] Fix easy rubocop issues --- app/controllers/spree/base_controller.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/app/controllers/spree/base_controller.rb b/app/controllers/spree/base_controller.rb index 6ce3e456ac..b660bf0672 100644 --- a/app/controllers/spree/base_controller.rb +++ b/app/controllers/spree/base_controller.rb @@ -1,16 +1,20 @@ +# frozen_string_literal: true + require 'cancan' require_dependency 'spree/core/controller_helpers/strong_parameters' -class Spree::BaseController < ApplicationController - include Spree::Core::ControllerHelpers::Auth - include Spree::Core::ControllerHelpers::RespondWith - include Spree::Core::ControllerHelpers::SSL - include Spree::Core::ControllerHelpers::Common - include Spree::Core::ControllerHelpers::Search - include Spree::Core::ControllerHelpers::StrongParameters - include Spree::Core::ControllerHelpers::Search +module Spree + class BaseController < ApplicationController + include Spree::Core::ControllerHelpers::Auth + include Spree::Core::ControllerHelpers::RespondWith + include Spree::Core::ControllerHelpers::SSL + include Spree::Core::ControllerHelpers::Common + include Spree::Core::ControllerHelpers::Search + include Spree::Core::ControllerHelpers::StrongParameters + include Spree::Core::ControllerHelpers::Search - respond_to :html + respond_to :html + end end require 'spree/i18n/initializer' From 388d575cc8a742c439220585c2c3a0e37b1f9681 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 14:49:07 +0100 Subject: [PATCH 03/24] Remove strong parameters and search helpers, they are not used in OFN --- app/controllers/spree/base_controller.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/controllers/spree/base_controller.rb b/app/controllers/spree/base_controller.rb index b660bf0672..4d20570e20 100644 --- a/app/controllers/spree/base_controller.rb +++ b/app/controllers/spree/base_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'cancan' -require_dependency 'spree/core/controller_helpers/strong_parameters' module Spree class BaseController < ApplicationController @@ -9,9 +8,6 @@ module Spree include Spree::Core::ControllerHelpers::RespondWith include Spree::Core::ControllerHelpers::SSL include Spree::Core::ControllerHelpers::Common - include Spree::Core::ControllerHelpers::Search - include Spree::Core::ControllerHelpers::StrongParameters - include Spree::Core::ControllerHelpers::Search respond_to :html end From 84d7538b1b5964fc26f852daad3231cd1c5663b8 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:09:08 +0100 Subject: [PATCH 04/24] Bring i18n code from spree --- lib/spree/i18n/base.rb | 17 +++++++++++++++++ lib/spree/i18n/initializer.rb | 1 + 2 files changed, 18 insertions(+) create mode 100644 lib/spree/i18n/base.rb create mode 100644 lib/spree/i18n/initializer.rb diff --git a/lib/spree/i18n/base.rb b/lib/spree/i18n/base.rb new file mode 100644 index 0000000000..765c8ad169 --- /dev/null +++ b/lib/spree/i18n/base.rb @@ -0,0 +1,17 @@ +module Spree + module ViewContext + def self.context=(context) + @context = context + end + + def self.context + @context + end + + def view_context + super.tap do |context| + Spree::ViewContext.context = context + end + end + end +end diff --git a/lib/spree/i18n/initializer.rb b/lib/spree/i18n/initializer.rb new file mode 100644 index 0000000000..79f5917cb2 --- /dev/null +++ b/lib/spree/i18n/initializer.rb @@ -0,0 +1 @@ +Spree::BaseController.send(:include, Spree::ViewContext) From c75341838e10752cf5db51e6ab5d40b4819e1172 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:13:16 +0100 Subject: [PATCH 05/24] Bring core.rb from spree_core --- lib/spree/core.rb | 56 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 lib/spree/core.rb diff --git a/lib/spree/core.rb b/lib/spree/core.rb new file mode 100644 index 0000000000..2ae848cf18 --- /dev/null +++ b/lib/spree/core.rb @@ -0,0 +1,56 @@ +require 'rails/all' +require 'active_merchant' +require 'acts_as_list' +require 'awesome_nested_set' +require 'cancan' +require 'kaminari' +require 'mail' +require 'paperclip' +require 'paranoia' +require 'ransack' +require 'state_machine' + +module Spree + + mattr_accessor :user_class + + def self.user_class + if @@user_class.is_a?(Class) + raise "Spree.user_class MUST be a String object, not a Class object." + elsif @@user_class.is_a?(String) + @@user_class.constantize + end + end + + # Used to configure Spree. + # + # Example: + # + # Spree.config do |config| + # config.site_name = "An awesome Spree site" + # end + # + # This method is defined within the core gem on purpose. + # Some people may only wish to use the Core part of Spree. + def self.config(&block) + yield(Spree::Config) + end +end + +require 'spree/core/version' +require 'spree/core/engine' + +require 'spree/i18n' +require 'spree/money' +require 'spree/promo/coupon_applicator' + +require 'spree/core/delegate_belongs_to' +require 'spree/core/ext/active_record' +require 'spree/core/permalinks' +require 'spree/core/token_resource' +require 'spree/core/calculated_adjustments' +require 'spree/core/product_duplicator' + +ActiveRecord::Base.class_eval do + include CollectiveIdea::Acts::NestedSet +end From 89e5221dc5d54fd35e60922499eb91b6781ced8a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:17:10 +0100 Subject: [PATCH 06/24] Fix easy rubocop issues --- lib/spree/core.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/spree/core.rb b/lib/spree/core.rb index 2ae848cf18..a99db5ac9d 100644 --- a/lib/spree/core.rb +++ b/lib/spree/core.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails/all' require 'active_merchant' require 'acts_as_list' @@ -11,15 +13,16 @@ require 'ransack' require 'state_machine' module Spree - mattr_accessor :user_class def self.user_class if @@user_class.is_a?(Class) raise "Spree.user_class MUST be a String object, not a Class object." - elsif @@user_class.is_a?(String) - @@user_class.constantize end + + return unless @@user_class.is_a?(String) + + @@user_class.constantize end # Used to configure Spree. From 38c5a9e105be09276f1c6162f779418eb87d6d6b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:20:25 +0100 Subject: [PATCH 07/24] Remove coupon applicator, it's not used in ofn --- lib/spree/core.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/spree/core.rb b/lib/spree/core.rb index a99db5ac9d..a72bd8b5f8 100644 --- a/lib/spree/core.rb +++ b/lib/spree/core.rb @@ -45,7 +45,6 @@ require 'spree/core/engine' require 'spree/i18n' require 'spree/money' -require 'spree/promo/coupon_applicator' require 'spree/core/delegate_belongs_to' require 'spree/core/ext/active_record' From efeda61e4026591e6db6de2595209c307f28e3c3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:21:16 +0100 Subject: [PATCH 08/24] Bring i18n.rb from spree --- lib/spree/i18n.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 lib/spree/i18n.rb diff --git a/lib/spree/i18n.rb b/lib/spree/i18n.rb new file mode 100644 index 0000000000..12cd95a2fb --- /dev/null +++ b/lib/spree/i18n.rb @@ -0,0 +1,36 @@ +require 'i18n' +require 'active_support/core_ext/array/extract_options' +require 'spree/i18n/base' + +module Spree + extend ActionView::Helpers::TranslationHelper + + class << self + # Add spree namespace and delegate to Rails TranslationHelper for some nice + # extra functionality. e.g return reasonable strings for missing translations + def translate(*args) + @virtual_path = virtual_path + + options = args.extract_options! + options[:scope] = [*options[:scope]].unshift(:spree) + args << options + super(*args) + end + + alias_method :t, :translate + + def context + Spree::ViewContext.context + end + + def virtual_path + if context + path = context.instance_variable_get("@virtual_path") + + if path + path.gsub(/spree/, '') + end + end + end + end +end From 12a5a266fd66e21c0d3ba366668e478f7886d60b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:22:35 +0100 Subject: [PATCH 09/24] Fix easy rubocop issues --- lib/spree/i18n.rb | 14 ++-- spec/lib/spree/i18n_spec.rb | 123 ++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 spec/lib/spree/i18n_spec.rb diff --git a/lib/spree/i18n.rb b/lib/spree/i18n.rb index 12cd95a2fb..7780bc3ad7 100644 --- a/lib/spree/i18n.rb +++ b/lib/spree/i18n.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'i18n' require 'active_support/core_ext/array/extract_options' require 'spree/i18n/base' @@ -24,13 +26,13 @@ module Spree end def virtual_path - if context - path = context.instance_variable_get("@virtual_path") + return unless context - if path - path.gsub(/spree/, '') - end - end + path = context.instance_variable_get("@virtual_path") + + return unless path + + path.gsub(/spree/, '') end end end diff --git a/spec/lib/spree/i18n_spec.rb b/spec/lib/spree/i18n_spec.rb new file mode 100644 index 0000000000..b4373cb4a5 --- /dev/null +++ b/spec/lib/spree/i18n_spec.rb @@ -0,0 +1,123 @@ +require 'rspec/expectations' +require 'spree/i18n' +require 'spree/testing_support/i18n' + +describe "i18n" do + before do + I18n.backend.store_translations(:en, + { + :spree => { + :foo => "bar", + :bar => { + :foo => "bar within bar scope", + :invalid => nil, + :legacy_translation => "back in the day..." + }, + :invalid => nil, + :legacy_translation => "back in the day..." + } + }) + end + + it "translates within the spree scope" do + Spree.normal_t(:foo).should eql("bar") + Spree.translate(:foo).should eql("bar") + end + + it "translates within the spree scope using a path" do + Spree.stub(:virtual_path).and_return('bar') + + Spree.normal_t('.legacy_translation').should eql("back in the day...") + Spree.translate('.legacy_translation').should eql("back in the day...") + end + + it "raise error without any context when using a path" do + expect { + Spree.normal_t('.legacy_translation') + }.to raise_error + + expect { + Spree.translate('.legacy_translation') + }.to raise_error + end + + it "prepends a string scope" do + Spree.normal_t(:foo, :scope => "bar").should eql("bar within bar scope") + end + + it "prepends to an array scope" do + Spree.normal_t(:foo, :scope => ["bar"]).should eql("bar within bar scope") + end + + it "returns two translations" do + Spree.normal_t([:foo, 'bar.foo']).should eql(["bar", "bar within bar scope"]) + end + + it "returns reasonable string for missing translations" do + Spree.t(:missing_entry).should include(" [:else, :where]) + Spree.check_missing_translations + assert_missing_translation("else") + assert_missing_translation("else.where") + assert_missing_translation("else.where.missing") + end + + it "does not log present translations" do + Spree.t(:foo) + Spree.check_missing_translations + Spree.missing_translation_messages.should be_empty + end + + it "does not break when asked for multiple translations" do + Spree.t [:foo, 'bar.foo'] + Spree.check_missing_translations + Spree.missing_translation_messages.should be_empty + end + end + + context "unused translations" do + def assert_unused_translation(key) + key = key_with_locale(key) + message = Spree.unused_translation_messages.detect { |m| m == key } + message.should_not(be_nil, "expected '#{key}' to be unused, but it was used.") + end + + def assert_used_translation(key) + key = key_with_locale(key) + message = Spree.unused_translation_messages.detect { |m| m == key } + message.should(be_nil, "expected '#{key}' to be used, but it wasn't.") + end + + it "logs translations that aren't used" do + Spree.check_unused_translations + assert_unused_translation("bar.legacy_translation") + assert_unused_translation("legacy_translation") + end + + it "does not log used translations" do + Spree.t(:foo) + Spree.check_unused_translations + assert_used_translation("foo") + end + end + end +end From 9a09f420c1f00c6a1f2a1ac27850635ab2691578 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:26:59 +0100 Subject: [PATCH 10/24] Modernize spec --- spec/lib/spree/i18n_spec.rb | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/spec/lib/spree/i18n_spec.rb b/spec/lib/spree/i18n_spec.rb index b4373cb4a5..b79a5e705f 100644 --- a/spec/lib/spree/i18n_spec.rb +++ b/spec/lib/spree/i18n_spec.rb @@ -1,22 +1,26 @@ +# frozen_string_literal: true + require 'rspec/expectations' require 'spree/i18n' require 'spree/testing_support/i18n' describe "i18n" do before do - I18n.backend.store_translations(:en, - { - :spree => { - :foo => "bar", - :bar => { - :foo => "bar within bar scope", - :invalid => nil, - :legacy_translation => "back in the day..." - }, - :invalid => nil, - :legacy_translation => "back in the day..." + I18n.backend.store_translations( + :en, + { + spree: { + foo: "bar", + bar: { + foo: "bar within bar scope", + invalid: nil, + legacy_translation: "back in the day..." + }, + invalid: nil, + legacy_translation: "back in the day..." + } } - }) + ) end it "translates within the spree scope" do @@ -42,11 +46,11 @@ describe "i18n" do end it "prepends a string scope" do - Spree.normal_t(:foo, :scope => "bar").should eql("bar within bar scope") + Spree.normal_t(:foo, scope: "bar").should eql("bar within bar scope") end it "prepends to an array scope" do - Spree.normal_t(:foo, :scope => ["bar"]).should eql("bar within bar scope") + Spree.normal_t(:foo, scope: ["bar"]).should eql("bar within bar scope") end it "returns two translations" do @@ -74,7 +78,7 @@ describe "i18n" do end it "logs missing translations" do - Spree.t(:missing, :scope => [:else, :where]) + Spree.t(:missing, scope: [:else, :where]) Spree.check_missing_translations assert_missing_translation("else") assert_missing_translation("else.where") From 724a88344e74091cba5d620b6072f4485ce0a189 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:27:37 +0100 Subject: [PATCH 11/24] Run transpec --- spec/lib/spree/i18n_spec.rb | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/spec/lib/spree/i18n_spec.rb b/spec/lib/spree/i18n_spec.rb index b79a5e705f..386dd9d2ec 100644 --- a/spec/lib/spree/i18n_spec.rb +++ b/spec/lib/spree/i18n_spec.rb @@ -24,15 +24,15 @@ describe "i18n" do end it "translates within the spree scope" do - Spree.normal_t(:foo).should eql("bar") - Spree.translate(:foo).should eql("bar") + expect(Spree.normal_t(:foo)).to eql("bar") + expect(Spree.translate(:foo)).to eql("bar") end it "translates within the spree scope using a path" do - Spree.stub(:virtual_path).and_return('bar') + allow(Spree).to receive(:virtual_path).and_return('bar') - Spree.normal_t('.legacy_translation').should eql("back in the day...") - Spree.translate('.legacy_translation').should eql("back in the day...") + expect(Spree.normal_t('.legacy_translation')).to eql("back in the day...") + expect(Spree.translate('.legacy_translation')).to eql("back in the day...") end it "raise error without any context when using a path" do @@ -46,19 +46,19 @@ describe "i18n" do end it "prepends a string scope" do - Spree.normal_t(:foo, scope: "bar").should eql("bar within bar scope") + expect(Spree.normal_t(:foo, scope: "bar")).to eql("bar within bar scope") end it "prepends to an array scope" do - Spree.normal_t(:foo, scope: ["bar"]).should eql("bar within bar scope") + expect(Spree.normal_t(:foo, scope: ["bar"])).to eql("bar within bar scope") end it "returns two translations" do - Spree.normal_t([:foo, 'bar.foo']).should eql(["bar", "bar within bar scope"]) + expect(Spree.normal_t([:foo, 'bar.foo'])).to eql(["bar", "bar within bar scope"]) end it "returns reasonable string for missing translations" do - Spree.t(:missing_entry).should include(" Date: Mon, 6 Jul 2020 15:28:41 +0100 Subject: [PATCH 12/24] Bring money.rb from spree --- lib/spree/money.rb | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 lib/spree/money.rb diff --git a/lib/spree/money.rb b/lib/spree/money.rb new file mode 100644 index 0000000000..a98d36deac --- /dev/null +++ b/lib/spree/money.rb @@ -0,0 +1,40 @@ +require 'money' + +module Spree + class Money + attr_reader :money + + delegate :cents, :to => :money + + def initialize(amount, options={}) + @money = ::Money.parse([amount, (options[:currency] || Spree::Config[:currency])].join) + @options = {} + @options[:with_currency] = Spree::Config[:display_currency] + @options[:symbol_position] = Spree::Config[:currency_symbol_position].to_sym + @options[:no_cents] = Spree::Config[:hide_cents] + @options[:decimal_mark] = Spree::Config[:currency_decimal_mark] + @options[:thousands_separator] = Spree::Config[:currency_thousands_separator] + @options.merge!(options) + # Must be a symbol because the Money gem doesn't do the conversion + @options[:symbol_position] = @options[:symbol_position].to_sym + end + + def to_s + @money.format(@options) + end + + def to_html(options = { :html => true }) + output = @money.format(@options.merge(options)) + if options[:html] + # 1) prevent blank, breaking spaces + # 2) prevent escaping of HTML character entities + output = output.gsub(" ", " ").html_safe + end + output + end + + def ==(obj) + @money == obj.money + end + end +end From a78d615936b9fa9ce131cc1d422f7924cf01f5f0 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:30:06 +0100 Subject: [PATCH 13/24] Bring money_spec from spree_core --- spec/lib/spree/money_spec.rb | 136 +++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 spec/lib/spree/money_spec.rb diff --git a/spec/lib/spree/money_spec.rb b/spec/lib/spree/money_spec.rb new file mode 100644 index 0000000000..c90b648ab0 --- /dev/null +++ b/spec/lib/spree/money_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: false + +# coding: utf-8 +require 'spec_helper' + +describe Spree::Money do + before do + configure_spree_preferences do |config| + config.currency = "USD" + config.currency_symbol_position = :before + config.display_currency = false + end + end + + it "formats correctly" do + money = Spree::Money.new(10) + money.to_s.should == "$10.00" + end + + it "can get cents" do + money = Spree::Money.new(10) + money.cents.should == 1000 + end + + context "with currency" do + it "passed in option" do + money = Spree::Money.new(10, :with_currency => true, :html => false) + money.to_s.should == "$10.00 USD" + end + + it "config option" do + Spree::Config[:display_currency] = true + money = Spree::Money.new(10, :html => false) + money.to_s.should == "$10.00 USD" + end + end + + context "hide cents" do + it "hides cents suffix" do + Spree::Config[:hide_cents] = true + money = Spree::Money.new(10) + money.to_s.should == "$10" + end + + it "shows cents suffix" do + Spree::Config[:hide_cents] = false + money = Spree::Money.new(10) + money.to_s.should == "$10.00" + end + end + + context "currency parameter" do + context "when currency is specified in Canadian Dollars" do + it "uses the currency param over the global configuration" do + money = Spree::Money.new(10, :currency => 'CAD', :with_currency => true, :html => false) + money.to_s.should == "$10.00 CAD" + end + end + + context "when currency is specified in Japanese Yen" do + it "uses the currency param over the global configuration" do + money = Spree::Money.new(100, :currency => 'JPY', :html => false) + money.to_s.should == "¥100" + end + end + end + + context "symbol positioning" do + it "passed in option" do + money = Spree::Money.new(10, :symbol_position => :after, :html => false) + money.to_s.should == "10.00 $" + end + + it "passed in option string" do + money = Spree::Money.new(10, :symbol_position => "after", :html => false) + money.to_s.should == "10.00 $" + end + + it "config option" do + Spree::Config[:currency_symbol_position] = :after + money = Spree::Money.new(10, :html => false) + money.to_s.should == "10.00 $" + end + end + + context "JPY" do + before do + configure_spree_preferences do |config| + config.currency = "JPY" + config.currency_symbol_position = :before + config.display_currency = false + end + end + + it "formats correctly" do + money = Spree::Money.new(1000, :html => false) + money.to_s.should == "¥1,000" + end + end + + context "EUR" do + before do + configure_spree_preferences do |config| + config.currency = "EUR" + config.currency_symbol_position = :after + config.display_currency = false + end + end + + # Regression test for #2634 + it "formats as plain by default" do + money = Spree::Money.new(10) + money.to_s.should == "10.00 €" + end + + # Regression test for #2632 + it "acknowledges decimal mark option" do + Spree::Config[:currency_decimal_mark] = "," + money = Spree::Money.new(10) + money.to_s.should == "10,00 €" + end + + # Regression test for #2632 + it "acknowledges thousands separator option" do + Spree::Config[:currency_thousands_separator] = "." + money = Spree::Money.new(1000) + money.to_s.should == "1.000.00 €" + end + + it "formats as HTML if asked (nicely) to" do + money = Spree::Money.new(10) + # The HTML'ified version of "10.00 €" + money.to_html.should == "10.00 €" + end + end +end From 50e6ce92b3581c73720dbf9b9514f4adc5092662 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:34:03 +0100 Subject: [PATCH 14/24] Fix easy rubocop issues --- lib/spree/money.rb | 12 +++++++----- spec/lib/spree/money_spec.rb | 19 +++++++++---------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/spree/money.rb b/lib/spree/money.rb index a98d36deac..413c2f0e8c 100644 --- a/lib/spree/money.rb +++ b/lib/spree/money.rb @@ -1,12 +1,14 @@ +# frozen_string_literal: false + require 'money' module Spree class Money attr_reader :money - delegate :cents, :to => :money + delegate :cents, to: :money - def initialize(amount, options={}) + def initialize(amount, options = {}) @money = ::Money.parse([amount, (options[:currency] || Spree::Config[:currency])].join) @options = {} @options[:with_currency] = Spree::Config[:display_currency] @@ -23,7 +25,7 @@ module Spree @money.format(@options) end - def to_html(options = { :html => true }) + def to_html(options = { html: true }) output = @money.format(@options.merge(options)) if options[:html] # 1) prevent blank, breaking spaces @@ -33,8 +35,8 @@ module Spree output end - def ==(obj) - @money == obj.money + def ==(other) + @money == other.money end end end diff --git a/spec/lib/spree/money_spec.rb b/spec/lib/spree/money_spec.rb index c90b648ab0..f4fb0948d3 100644 --- a/spec/lib/spree/money_spec.rb +++ b/spec/lib/spree/money_spec.rb @@ -1,6 +1,5 @@ # frozen_string_literal: false -# coding: utf-8 require 'spec_helper' describe Spree::Money do @@ -24,13 +23,13 @@ describe Spree::Money do context "with currency" do it "passed in option" do - money = Spree::Money.new(10, :with_currency => true, :html => false) + money = Spree::Money.new(10, with_currency: true, html: false) money.to_s.should == "$10.00 USD" end it "config option" do Spree::Config[:display_currency] = true - money = Spree::Money.new(10, :html => false) + money = Spree::Money.new(10, html: false) money.to_s.should == "$10.00 USD" end end @@ -52,14 +51,14 @@ describe Spree::Money do context "currency parameter" do context "when currency is specified in Canadian Dollars" do it "uses the currency param over the global configuration" do - money = Spree::Money.new(10, :currency => 'CAD', :with_currency => true, :html => false) + money = Spree::Money.new(10, currency: 'CAD', with_currency: true, html: false) money.to_s.should == "$10.00 CAD" end end context "when currency is specified in Japanese Yen" do it "uses the currency param over the global configuration" do - money = Spree::Money.new(100, :currency => 'JPY', :html => false) + money = Spree::Money.new(100, currency: 'JPY', html: false) money.to_s.should == "¥100" end end @@ -67,18 +66,18 @@ describe Spree::Money do context "symbol positioning" do it "passed in option" do - money = Spree::Money.new(10, :symbol_position => :after, :html => false) + money = Spree::Money.new(10, symbol_position: :after, html: false) money.to_s.should == "10.00 $" end it "passed in option string" do - money = Spree::Money.new(10, :symbol_position => "after", :html => false) + money = Spree::Money.new(10, symbol_position: "after", html: false) money.to_s.should == "10.00 $" end it "config option" do Spree::Config[:currency_symbol_position] = :after - money = Spree::Money.new(10, :html => false) + money = Spree::Money.new(10, html: false) money.to_s.should == "10.00 $" end end @@ -93,7 +92,7 @@ describe Spree::Money do end it "formats correctly" do - money = Spree::Money.new(1000, :html => false) + money = Spree::Money.new(1000, html: false) money.to_s.should == "¥1,000" end end @@ -129,7 +128,7 @@ describe Spree::Money do it "formats as HTML if asked (nicely) to" do money = Spree::Money.new(10) - # The HTML'ified version of "10.00 €" + # The HTMLified version of the euro sign money.to_html.should == "10.00 €" end end From 7b30008e8b0346a62a30e6a2ea94589c3eff8dd5 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:34:51 +0100 Subject: [PATCH 15/24] Run transpec --- spec/lib/spree/money_spec.rb | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/spec/lib/spree/money_spec.rb b/spec/lib/spree/money_spec.rb index f4fb0948d3..139806137c 100644 --- a/spec/lib/spree/money_spec.rb +++ b/spec/lib/spree/money_spec.rb @@ -13,24 +13,24 @@ describe Spree::Money do it "formats correctly" do money = Spree::Money.new(10) - money.to_s.should == "$10.00" + expect(money.to_s).to eq("$10.00") end it "can get cents" do money = Spree::Money.new(10) - money.cents.should == 1000 + expect(money.cents).to eq(1000) end context "with currency" do it "passed in option" do money = Spree::Money.new(10, with_currency: true, html: false) - money.to_s.should == "$10.00 USD" + expect(money.to_s).to eq("$10.00 USD") end it "config option" do Spree::Config[:display_currency] = true money = Spree::Money.new(10, html: false) - money.to_s.should == "$10.00 USD" + expect(money.to_s).to eq("$10.00 USD") end end @@ -38,13 +38,13 @@ describe Spree::Money do it "hides cents suffix" do Spree::Config[:hide_cents] = true money = Spree::Money.new(10) - money.to_s.should == "$10" + expect(money.to_s).to eq("$10") end it "shows cents suffix" do Spree::Config[:hide_cents] = false money = Spree::Money.new(10) - money.to_s.should == "$10.00" + expect(money.to_s).to eq("$10.00") end end @@ -52,14 +52,14 @@ describe Spree::Money do context "when currency is specified in Canadian Dollars" do it "uses the currency param over the global configuration" do money = Spree::Money.new(10, currency: 'CAD', with_currency: true, html: false) - money.to_s.should == "$10.00 CAD" + expect(money.to_s).to eq("$10.00 CAD") end end context "when currency is specified in Japanese Yen" do it "uses the currency param over the global configuration" do money = Spree::Money.new(100, currency: 'JPY', html: false) - money.to_s.should == "¥100" + expect(money.to_s).to eq("¥100") end end end @@ -67,18 +67,18 @@ describe Spree::Money do context "symbol positioning" do it "passed in option" do money = Spree::Money.new(10, symbol_position: :after, html: false) - money.to_s.should == "10.00 $" + expect(money.to_s).to eq("10.00 $") end it "passed in option string" do money = Spree::Money.new(10, symbol_position: "after", html: false) - money.to_s.should == "10.00 $" + expect(money.to_s).to eq("10.00 $") end it "config option" do Spree::Config[:currency_symbol_position] = :after money = Spree::Money.new(10, html: false) - money.to_s.should == "10.00 $" + expect(money.to_s).to eq("10.00 $") end end @@ -93,7 +93,7 @@ describe Spree::Money do it "formats correctly" do money = Spree::Money.new(1000, html: false) - money.to_s.should == "¥1,000" + expect(money.to_s).to eq("¥1,000") end end @@ -106,30 +106,30 @@ describe Spree::Money do end end - # Regression test for #2634 + # Regression test for Spree #2634 it "formats as plain by default" do money = Spree::Money.new(10) - money.to_s.should == "10.00 €" + expect(money.to_s).to eq("10.00 €") end - # Regression test for #2632 + # Regression test for Spree #2632 it "acknowledges decimal mark option" do Spree::Config[:currency_decimal_mark] = "," money = Spree::Money.new(10) - money.to_s.should == "10,00 €" + expect(money.to_s).to eq("10,00 €") end - # Regression test for #2632 + # Regression test for Spree #2632 it "acknowledges thousands separator option" do Spree::Config[:currency_thousands_separator] = "." money = Spree::Money.new(1000) - money.to_s.should == "1.000.00 €" + expect(money.to_s).to eq("1.000.00 €") end it "formats as HTML if asked (nicely) to" do money = Spree::Money.new(10) # The HTMLified version of the euro sign - money.to_html.should == "10.00 €" + expect(money.to_html).to eq("10.00 €") end end end From 95698fac37efb03ed6083550cb0d1e21d60740ab Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:36:13 +0100 Subject: [PATCH 16/24] Bring responder from spree_core --- lib/spree/responder.rb | 45 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 lib/spree/responder.rb diff --git a/lib/spree/responder.rb b/lib/spree/responder.rb new file mode 100644 index 0000000000..b87d0f0daa --- /dev/null +++ b/lib/spree/responder.rb @@ -0,0 +1,45 @@ +module Spree + class Responder < ::ActionController::Responder #:nodoc: + + attr_accessor :on_success, :on_failure + + def initialize(controller, resources, options={}) + super + + class_name = controller.class.name.to_sym + action_name = options.delete(:action_name) + + if result = Spree::BaseController.spree_responders[class_name].try(:[], action_name).try(:[], self.format.to_sym) + self.on_success = handler(controller, result, :success) + self.on_failure = handler(controller, result, :failure) + end + end + + def to_html + super and return if !(on_success || on_failure) + has_errors? ? controller.instance_exec(&on_failure) : controller.instance_exec(&on_success) + end + + def to_format + super and return if !(on_success || on_failure) + has_errors? ? controller.instance_exec(&on_failure) : controller.instance_exec(&on_success) + end + + private + + def handler(controller, result, status) + return result if result.respond_to? :call + + case result + when Hash + if result[status].is_a? Symbol + controller.method(result[status]) + else + result[status] + end + when Symbol + controller.method(result) + end + end + end +end From 2c65cea911b33ce8b16bb5eee2d8a3d501a5989a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 6 Jul 2020 15:40:10 +0100 Subject: [PATCH 17/24] Fix easy rubocop issues --- lib/spree/responder.rb | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/spree/responder.rb b/lib/spree/responder.rb index b87d0f0daa..3c0e60aa5b 100644 --- a/lib/spree/responder.rb +++ b/lib/spree/responder.rb @@ -1,27 +1,39 @@ +# frozen_string_literal: true + module Spree class Responder < ::ActionController::Responder #:nodoc: - attr_accessor :on_success, :on_failure - def initialize(controller, resources, options={}) + def initialize(controller, resources, options = {}) super class_name = controller.class.name.to_sym action_name = options.delete(:action_name) - if result = Spree::BaseController.spree_responders[class_name].try(:[], action_name).try(:[], self.format.to_sym) - self.on_success = handler(controller, result, :success) - self.on_failure = handler(controller, result, :failure) - end + result = Spree::BaseController.spree_responders[class_name]. + try(:[], action_name). + try(:[], self.format.to_sym) + return unless result + + self.on_success = handler(controller, result, :success) + self.on_failure = handler(controller, result, :failure) end def to_html - super and return if !(on_success || on_failure) + if !(on_success || on_failure) + super + return + end + has_errors? ? controller.instance_exec(&on_failure) : controller.instance_exec(&on_success) end def to_format - super and return if !(on_success || on_failure) + if !(on_success || on_failure) + super + return + end + has_errors? ? controller.instance_exec(&on_failure) : controller.instance_exec(&on_success) end From 58da11fde760769c1d75fcd27c65eaca0c11b80e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 10 Jul 2020 16:16:52 +0100 Subject: [PATCH 18/24] Bring Environment Calculators and Environment Extension from spree_core --- lib/spree/core/environment/calculators.rb | 12 +++++++++++ lib/spree/core/environment_extension.rb | 25 +++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 lib/spree/core/environment/calculators.rb create mode 100644 lib/spree/core/environment_extension.rb diff --git a/lib/spree/core/environment/calculators.rb b/lib/spree/core/environment/calculators.rb new file mode 100644 index 0000000000..ae86072c19 --- /dev/null +++ b/lib/spree/core/environment/calculators.rb @@ -0,0 +1,12 @@ +module Spree + module Core + class Environment + class Calculators + include EnvironmentExtension + + attr_accessor :shipping_methods, :tax_rates + end + end + end +end + diff --git a/lib/spree/core/environment_extension.rb b/lib/spree/core/environment_extension.rb new file mode 100644 index 0000000000..ecbd31a9a3 --- /dev/null +++ b/lib/spree/core/environment_extension.rb @@ -0,0 +1,25 @@ +module Spree + module Core + module EnvironmentExtension + extend ActiveSupport::Concern + + def add_class(name) + self.instance_variable_set "@#{name}", Set.new + + create_method( "#{name}=".to_sym ) { |val| + instance_variable_set( "@" + name, val) + } + + create_method(name.to_sym) do + instance_variable_get( "@" + name ) + end + end + + private + + def create_method(name, &block) + self.class.send(:define_method, name, &block) + end + end + end +end From 56b83b6bb54b93c4aca20037e3e8f82b5a523aa2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 10 Jul 2020 16:18:44 +0100 Subject: [PATCH 19/24] Fix easy rubocop issues --- lib/spree/core/environment/calculators.rb | 3 ++- lib/spree/core/environment_extension.rb | 10 ++++++---- lib/spree/i18n/base.rb | 2 ++ lib/spree/i18n/initializer.rb | 4 +++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/spree/core/environment/calculators.rb b/lib/spree/core/environment/calculators.rb index ae86072c19..a5f60555f7 100644 --- a/lib/spree/core/environment/calculators.rb +++ b/lib/spree/core/environment/calculators.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree module Core class Environment @@ -9,4 +11,3 @@ module Spree end end end - diff --git a/lib/spree/core/environment_extension.rb b/lib/spree/core/environment_extension.rb index ecbd31a9a3..7b05908752 100644 --- a/lib/spree/core/environment_extension.rb +++ b/lib/spree/core/environment_extension.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + module Spree module Core module EnvironmentExtension extend ActiveSupport::Concern def add_class(name) - self.instance_variable_set "@#{name}", Set.new + instance_variable_set "@#{name}", Set.new create_method( "#{name}=".to_sym ) { |val| instance_variable_set( "@" + name, val) @@ -17,9 +19,9 @@ module Spree private - def create_method(name, &block) - self.class.send(:define_method, name, &block) - end + def create_method(name, &block) + self.class.__send__(:define_method, name, &block) + end end end end diff --git a/lib/spree/i18n/base.rb b/lib/spree/i18n/base.rb index 765c8ad169..88c2d3106d 100644 --- a/lib/spree/i18n/base.rb +++ b/lib/spree/i18n/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree module ViewContext def self.context=(context) diff --git a/lib/spree/i18n/initializer.rb b/lib/spree/i18n/initializer.rb index 79f5917cb2..85757681fe 100644 --- a/lib/spree/i18n/initializer.rb +++ b/lib/spree/i18n/initializer.rb @@ -1 +1,3 @@ -Spree::BaseController.send(:include, Spree::ViewContext) +# frozen_string_literal: true + +Spree::BaseController.__send__(:include, Spree::ViewContext) From 2e3702550dd8905a03a67ce394ab0efcf82fe54e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 11 Jul 2020 16:04:10 +0100 Subject: [PATCH 20/24] Bring a number of files from spree_core needed in OFN --- lib/spree/core/calculated_adjustments.rb | 75 ++++++++++++++++ lib/spree/core/delegate_belongs_to.rb | 89 +++++++++++++++++++ lib/spree/core/gateway_error.rb | 5 ++ lib/spree/core/mail_interceptor.rb | 22 +++++ lib/spree/core/mail_settings.rb | 60 +++++++++++++ lib/spree/core/permalinks.rb | 71 +++++++++++++++ lib/spree/core/s3_support.rb | 25 ++++++ lib/spree/core/token_resource.rb | 27 ++++++ lib/spree/product_duplicator.rb | 61 +++++++++++++ .../spree/core/calculated_adjustments_spec.rb | 69 ++++++++++++++ spec/lib/spree/core/mail_interceptor_spec.rb | 78 ++++++++++++++++ spec/lib/spree/core/mail_settings_spec.rb | 89 +++++++++++++++++++ spec/lib/spree/core/token_resource_spec.rb | 32 +++++++ spec/lib/spree/product_duplicator_spec.rb | 87 ++++++++++++++++++ 14 files changed, 790 insertions(+) create mode 100644 lib/spree/core/calculated_adjustments.rb create mode 100644 lib/spree/core/delegate_belongs_to.rb create mode 100644 lib/spree/core/gateway_error.rb create mode 100644 lib/spree/core/mail_interceptor.rb create mode 100644 lib/spree/core/mail_settings.rb create mode 100644 lib/spree/core/permalinks.rb create mode 100644 lib/spree/core/s3_support.rb create mode 100644 lib/spree/core/token_resource.rb create mode 100644 lib/spree/product_duplicator.rb create mode 100644 spec/lib/spree/core/calculated_adjustments_spec.rb create mode 100644 spec/lib/spree/core/mail_interceptor_spec.rb create mode 100644 spec/lib/spree/core/mail_settings_spec.rb create mode 100644 spec/lib/spree/core/token_resource_spec.rb create mode 100644 spec/lib/spree/product_duplicator_spec.rb diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb new file mode 100644 index 0000000000..221a9926f0 --- /dev/null +++ b/lib/spree/core/calculated_adjustments.rb @@ -0,0 +1,75 @@ +module Spree + module Core + module CalculatedAdjustments + def self.included(klass) + klass.class_eval do + has_one :calculator, :class_name => "Spree::Calculator", :as => :calculable, :dependent => :destroy + accepts_nested_attributes_for :calculator + validates :calculator, :presence => true + + def self.calculators + spree_calculators.send model_name_without_spree_namespace + end + + def calculator_type + calculator.class.to_s if calculator + end + + def calculator_type=(calculator_type) + klass = calculator_type.constantize if calculator_type + self.calculator = klass.new if klass && !self.calculator.is_a?(klass) + end + + # Creates a new adjustment for the target object (which is any class that has_many :adjustments) and + # sets amount based on the calculator as applied to the calculable argument (Order, LineItems[], Shipment, etc.) + # By default the adjustment will not be considered mandatory + def create_adjustment(label, target, calculable, mandatory=false, state="closed") + # Adjustment calculations done on Spree::Shipment objects MUST + # be done on their to_package'd variants instead + # It's only the package that contains the correct information. + # See https://github.com/spree/spree_active_shipping/pull/96 et. al + old_calculable = calculable + calculable = calculable.to_package if calculable.is_a?(Spree::Shipment) + amount = compute_amount(calculable) + return if amount == 0 && !mandatory + target.adjustments.create( + :amount => amount, + :source => old_calculable, + :originator => self, + :label => label, + :mandatory => mandatory, + :state => state + ) + end + + # Updates the amount of the adjustment using our Calculator and calling the +compute+ method with the +calculable+ + # referenced passed to the method. + def update_adjustment(adjustment, calculable) + # Adjustment calculations done on Spree::Shipment objects MUST + # be done on their to_package'd variants instead + # It's only the package that contains the correct information. + # See https://github.com/spree/spree_active_shipping/pull/96 et. al + calculable = calculable.to_package if calculable.is_a?(Spree::Shipment) + adjustment.update_column(:amount, compute_amount(calculable)) + end + + # Calculate the amount to be used when creating an adjustment + # NOTE: May be overriden by classes where this module is included into. + # Such as Spree::Promotion::Action::CreateAdjustment. + def compute_amount(calculable) + self.calculator.compute(calculable) + end + + private + def self.model_name_without_spree_namespace + self.to_s.tableize.gsub('/', '_').sub('spree_', '') + end + + def self.spree_calculators + Rails.application.config.spree.calculators + end + end + end + end + end +end diff --git a/lib/spree/core/delegate_belongs_to.rb b/lib/spree/core/delegate_belongs_to.rb new file mode 100644 index 0000000000..2adb978557 --- /dev/null +++ b/lib/spree/core/delegate_belongs_to.rb @@ -0,0 +1,89 @@ +## +# Creates methods on object which delegate to an association proxy. +# see delegate_belongs_to for two uses +# +# Todo - integrate with ActiveRecord::Dirty to make sure changes to delegate object are noticed +# Should do +# class User < ActiveRecord::Base; delegate_belongs_to :contact, :firstname; end +# class Contact < ActiveRecord::Base; end +# u = User.first +# u.changed? # => false +# u.firstname = 'Bobby' +# u.changed? # => true +# +# Right now the second call to changed? would return false +# +# Todo - add has_one support. fairly straightforward addition +## +module DelegateBelongsTo + extend ActiveSupport::Concern + + module ClassMethods + + @@default_rejected_delegate_columns = ['created_at','created_on','updated_at','updated_on','lock_version','type','id','position','parent_id','lft','rgt'] + mattr_accessor :default_rejected_delegate_columns + + ## + # Creates methods for accessing and setting attributes on an association. Uses same + # default list of attributes as delegates_to_association. + # @todo Integrate this with ActiveRecord::Dirty, so if you set a property through one of these setters and then call save on this object, it will save the associated object automatically. + # delegate_belongs_to :contact + # delegate_belongs_to :contact, [:defaults] ## same as above, and useless + # delegate_belongs_to :contact, [:defaults, :address, :fullname], :class_name => 'VCard' + ## + def delegate_belongs_to(association, *attrs) + opts = attrs.extract_options! + initialize_association :belongs_to, association, opts + attrs = get_association_column_names(association) if attrs.empty? + attrs.concat get_association_column_names(association) if attrs.delete :defaults + attrs.each do |attr| + class_def attr do |*args| + if args.empty? + send(:delegator_for, association).send(attr) + else + send(:delegator_for, association).send(attr, *args) + end + end + class_def "#{attr}=" do |val| + send(:delegator_for, association).send("#{attr}=", val) + end + end + end + + protected + + def get_association_column_names(association, without_default_rejected_delegate_columns=true) + begin + association_klass = reflect_on_association(association).klass + methods = association_klass.column_names + methods.reject!{|x|default_rejected_delegate_columns.include?(x.to_s)} if without_default_rejected_delegate_columns + return methods + rescue + return [] + end + end + + ## + # initialize_association :belongs_to, :contact + def initialize_association(type, association, opts={}) + raise 'Illegal or unimplemented association type.' unless [:belongs_to].include?(type.to_s.to_sym) + send type, association, opts if reflect_on_association(association).nil? + end + + private + + def class_def(name, method=nil, &blk) + class_eval { method.nil? ? define_method(name, &blk) : define_method(name, method) } + end + + end + + def delegator_for(association) + send("#{association}=", self.class.reflect_on_association(association).klass.new) if send(association).nil? + send(association) + end + protected :delegator_for + +end + +ActiveRecord::Base.send :include, DelegateBelongsTo diff --git a/lib/spree/core/gateway_error.rb b/lib/spree/core/gateway_error.rb new file mode 100644 index 0000000000..5a6ddda30b --- /dev/null +++ b/lib/spree/core/gateway_error.rb @@ -0,0 +1,5 @@ +module Spree + module Core + class GatewayError < RuntimeError; end + end +end diff --git a/lib/spree/core/mail_interceptor.rb b/lib/spree/core/mail_interceptor.rb new file mode 100644 index 0000000000..e0a1400ab8 --- /dev/null +++ b/lib/spree/core/mail_interceptor.rb @@ -0,0 +1,22 @@ +# Allows us to intercept any outbound mail message and make last minute changes +# (such as specifying a "from" address or sending to a test email account) +# +# See http://railscasts.com/episodes/206-action-mailer-in-rails-3 for more details. +module Spree + module Core + class MailInterceptor + def self.delivering_email(message) + return unless MailSettings.override? + + if Config[:intercept_email].present? + message.subject = "#{message.to} #{message.subject}" + message.to = Config[:intercept_email] + end + + if Config[:mail_bcc].present? + message.bcc ||= Config[:mail_bcc] + end + end + end + end +end diff --git a/lib/spree/core/mail_settings.rb b/lib/spree/core/mail_settings.rb new file mode 100644 index 0000000000..757144af8e --- /dev/null +++ b/lib/spree/core/mail_settings.rb @@ -0,0 +1,60 @@ +module Spree + module Core + class MailSettings + MAIL_AUTH = ['None', 'plain', 'login', 'cram_md5'] + SECURE_CONNECTION_TYPES = ['None','SSL','TLS'] + + # Override the Rails application mail settings based on preferences + # This makes it possible to configure the mail settings through an admin + # interface instead of requiring changes to the Rails envrionment file + def self.init + self.new.override! if override? + end + + def self.override? + Config.override_actionmailer_config + end + + def override! + if Config.enable_mail_delivery + ActionMailer::Base.default_url_options[:host] ||= Config.site_url + ActionMailer::Base.smtp_settings = mail_server_settings + ActionMailer::Base.perform_deliveries = true + else + ActionMailer::Base.perform_deliveries = false + end + end + + private + def mail_server_settings + settings = if need_authentication? + basic_settings.merge(user_credentials) + else + basic_settings + end + + settings.merge :enable_starttls_auto => secure_connection? + end + + def user_credentials + { :user_name => Config.smtp_username, + :password => Config.smtp_password } + end + + def basic_settings + { :address => Config.mail_host, + :domain => Config.mail_domain, + :port => Config.mail_port, + :authentication => Config.mail_auth_type } + end + + def need_authentication? + Config.mail_auth_type != 'None' + end + + def secure_connection? + Config.secure_connection_type == 'TLS' + end + end + end +end diff --git a/lib/spree/core/permalinks.rb b/lib/spree/core/permalinks.rb new file mode 100644 index 0000000000..6e0fcc8735 --- /dev/null +++ b/lib/spree/core/permalinks.rb @@ -0,0 +1,71 @@ +require 'stringex' + +module Spree + module Core + module Permalinks + extend ActiveSupport::Concern + + included do + class_attribute :permalink_options + end + + module ClassMethods + def make_permalink(options={}) + options[:field] ||= :permalink + self.permalink_options = options + + if self.connected? + if self.table_exists? && self.column_names.include?(permalink_options[:field].to_s) + before_validation(:on => :create) { save_permalink } + end + end + end + + def find_by_param(value, *args) + self.send("find_by_#{permalink_field}", value, *args) + end + + def find_by_param!(value, *args) + self.send("find_by_#{permalink_field}!", value, *args) + end + + def permalink_field + permalink_options[:field] + end + + def permalink_prefix + permalink_options[:prefix] || "" + end + + def permalink_order + order = permalink_options[:order] + "#{order} ASC," if order + end + end + + def generate_permalink + "#{self.class.permalink_prefix}#{Array.new(9){rand(9)}.join}" + end + + def save_permalink(permalink_value=self.to_param) + self.with_lock do + permalink_value ||= generate_permalink + + field = self.class.permalink_field + # Do other links exist with this permalink? + other = self.class.where("#{self.class.table_name}.#{field} LIKE ?", "#{permalink_value}%") + if other.any? + # Find the existing permalink with the highest number, and increment that number. + # (If none of the existing permalinks have a number, this will evaluate to 1.) + number = other.map { |o| o.send(field)[/-(\d+)$/, 1].to_i }.max + 1 + permalink_value += "-#{number.to_s}" + end + write_attribute(field, permalink_value) + end + end + end + end +end + +ActiveRecord::Base.send :include, Spree::Core::Permalinks +ActiveRecord::Relation.send :include, Spree::Core::Permalinks diff --git a/lib/spree/core/s3_support.rb b/lib/spree/core/s3_support.rb new file mode 100644 index 0000000000..8ae8354391 --- /dev/null +++ b/lib/spree/core/s3_support.rb @@ -0,0 +1,25 @@ +module Spree + module Core + # This module exists to reduce duplication in S3 settings between + # the Image and Taxon models in Spree + module S3Support + extend ActiveSupport::Concern + + included do + def self.supports_s3(field) + # Load user defined paperclip settings + config = Spree::Config + if config[:use_s3] + s3_creds = { :access_key_id => config[:s3_access_key], :secret_access_key => config[:s3_secret], :bucket => config[:s3_bucket] } + self.attachment_definitions[field][:storage] = :s3 + self.attachment_definitions[field][:s3_credentials] = s3_creds + self.attachment_definitions[field][:s3_headers] = ActiveSupport::JSON.decode(config[:s3_headers]) + self.attachment_definitions[field][:bucket] = config[:s3_bucket] + self.attachment_definitions[field][:s3_protocol] = config[:s3_protocol].downcase unless config[:s3_protocol].blank? + self.attachment_definitions[field][:s3_host_alias] = config[:s3_host_alias] unless config[:s3_host_alias].blank? + end + end + end + end + end +end diff --git a/lib/spree/core/token_resource.rb b/lib/spree/core/token_resource.rb new file mode 100644 index 0000000000..48697f29b2 --- /dev/null +++ b/lib/spree/core/token_resource.rb @@ -0,0 +1,27 @@ +module Spree + module Core + module TokenResource + module ClassMethods + def token_resource + has_one :tokenized_permission, :as => :permissable + delegate :token, :to => :tokenized_permission, :allow_nil => true + after_create :create_token + end + end + + def create_token + permission = build_tokenized_permission + permission.token = token = ::SecureRandom::hex(8) + permission.save! + token + end + + def self.included(receiver) + receiver.extend ClassMethods + end + end + end +end + +ActiveRecord::Base.class_eval { include Spree::Core::TokenResource } + diff --git a/lib/spree/product_duplicator.rb b/lib/spree/product_duplicator.rb new file mode 100644 index 0000000000..f563223f70 --- /dev/null +++ b/lib/spree/product_duplicator.rb @@ -0,0 +1,61 @@ +module Spree + class ProductDuplicator + attr_accessor :product + + def initialize(product) + @product = product + end + + def duplicate + new_product = duplicate_product + + # don't dup the actual variants, just the characterising types + new_product.option_types = product.option_types if product.has_variants? + + # allow site to do some customization + new_product.send(:duplicate_extra, product) if new_product.respond_to?(:duplicate_extra) + new_product.save! + new_product + end + + protected + + def duplicate_product + product.dup.tap do |new_product| + new_product.name = "COPY OF #{product.name}" + new_product.taxons = product.taxons + new_product.created_at = nil + new_product.deleted_at = nil + new_product.updated_at = nil + new_product.product_properties = reset_properties + new_product.master = duplicate_master + end + end + + def duplicate_master + master = product.master + master.dup.tap do |new_master| + new_master.sku = "COPY OF #{master.sku}" + new_master.deleted_at = nil + new_master.images = master.images.map { |image| duplicate_image image } + new_master.price = master.price + new_master.currency = master.currency + end + end + + def duplicate_image(image) + new_image = image.dup + new_image.assign_attributes(:attachment => image.attachment.clone) + new_image + end + + def reset_properties + product.product_properties.map do |prop| + prop.dup.tap do |new_prop| + new_prop.created_at = nil + new_prop.updated_at = nil + end + end + end + end +end diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb new file mode 100644 index 0000000000..eecfee445f --- /dev/null +++ b/spec/lib/spree/core/calculated_adjustments_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction with an actual class that +# extends ActiveRecord::Base and has a corresponding table in the database. So we'll just test it using Order and +# ShippingMethod instead since those classes are including the module. +describe Spree::Core::CalculatedAdjustments do + + let(:calculator) { mock_model(Spree::Calculator, :compute => 10, :[]= => nil) } + + it "should add has_one :calculator relationship" do + assert Spree::ShippingMethod.reflect_on_all_associations(:has_one).map(&:name).include?(:calculator) + end + + let(:tax_rate) { Spree::TaxRate.new(:calculator => calculator) } + + context "#create_adjustment and its resulting adjustment" do + let(:order) { Spree::Order.create } + let(:target) { order } + + it "should be associated with the target" do + target.adjustments.should_receive(:create) + tax_rate.create_adjustment("foo", target, order) + end + + it "should have the correct originator and an amount derived from the calculator and supplied calculable" do + adjustment = tax_rate.create_adjustment("foo", target, order) + adjustment.should_not be_nil + adjustment.amount.should == 10 + adjustment.source.should == order + adjustment.originator.should == tax_rate + end + + it "should be mandatory if true is supplied for that parameter" do + adjustment = tax_rate.create_adjustment("foo", target, order, true) + adjustment.should be_mandatory + end + + context "when the calculator returns 0" do + before { calculator.stub :compute => 0 } + + context "when adjustment is mandatory" do + before { tax_rate.create_adjustment("foo", target, order, true) } + + it "should create an adjustment" do + Spree::Adjustment.count.should == 1 + end + end + + context "when adjustment is not mandatory" do + before { tax_rate.create_adjustment("foo", target, order, false) } + + it "should not create an adjustment" do + Spree::Adjustment.count.should == 0 + end + end + end + + end + + context "#update_adjustment" do + it "should update the adjustment using its calculator (and the specified source)" do + adjustment = double(:adjustment).as_null_object + calculable = double :calculable + adjustment.should_receive(:update_column).with(:amount, 10) + tax_rate.update_adjustment(adjustment, calculable) + end + end + +end diff --git a/spec/lib/spree/core/mail_interceptor_spec.rb b/spec/lib/spree/core/mail_interceptor_spec.rb new file mode 100644 index 0000000000..9375a0471c --- /dev/null +++ b/spec/lib/spree/core/mail_interceptor_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +# We'll use the OrderMailer as a quick and easy way to test. IF it works here +# it works for all email (in theory.) +describe Spree::OrderMailer do + let(:order) { Spree::Order.new(:email => "customer@example.com") } + let(:message) { Spree::OrderMailer.confirm_email(order) } + + before(:all) do + ActionMailer::Base.perform_deliveries = true + ActionMailer::Base.deliveries.clear + end + + context "#deliver" do + before do + ActionMailer::Base.delivery_method = :test + end + + after { ActionMailer::Base.deliveries.clear } + + it "should use the from address specified in the preference" do + Spree::Config[:mails_from] = "no-reply@foobar.com" + message.deliver + @email = ActionMailer::Base.deliveries.first + @email.from.should == ["no-reply@foobar.com"] + end + + it "should use the provided from address" do + Spree::Config[:mails_from] = "preference@foobar.com" + message.from = "override@foobar.com" + message.to = "test@test.com" + message.deliver + email = ActionMailer::Base.deliveries.first + email.from.should == ["override@foobar.com"] + email.to.should == ["test@test.com"] + end + + it "should add the bcc email when provided" do + Spree::Config[:mail_bcc] = "bcc-foo@foobar.com" + message.deliver + @email = ActionMailer::Base.deliveries.first + @email.bcc.should == ["bcc-foo@foobar.com"] + end + + context "when intercept_email is provided" do + it "should strip the bcc recipients" do + message.bcc.should be_blank + end + + it "should strip the cc recipients" do + message.cc.should be_blank + end + + it "should replace the receipient with the specified address" do + Spree::Config[:intercept_email] = "intercept@foobar.com" + message.deliver + @email = ActionMailer::Base.deliveries.first + @email.to.should == ["intercept@foobar.com"] + end + + it "should modify the subject to include the original email" do + Spree::Config[:intercept_email] = "intercept@foobar.com" + message.deliver + @email = ActionMailer::Base.deliveries.first + @email.subject.match(/customer@example\.com/).should be_true + end + end + + context "when intercept_mode is not provided" do + it "should not modify the recipient" do + Spree::Config[:intercept_email] = "" + message.deliver + @email = ActionMailer::Base.deliveries.first + @email.to.should == ["customer@example.com"] + end + end + end +end diff --git a/spec/lib/spree/core/mail_settings_spec.rb b/spec/lib/spree/core/mail_settings_spec.rb new file mode 100644 index 0000000000..8528a6226d --- /dev/null +++ b/spec/lib/spree/core/mail_settings_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +module Spree + module Core + describe MailSettings do + let!(:subject) { MailSettings.new } + + context "override option is true" do + before { Config.override_actionmailer_config = true } + + context "init" do + it "calls override!" do + MailSettings.should_receive(:new).and_return(subject) + subject.should_receive(:override!) + MailSettings.init + end + end + + context "enable delivery" do + before { Config.enable_mail_delivery = true } + + context "overrides appplication defaults" do + + context "authentication method is none" do + before do + Config.mail_host = "smtp.example.com" + Config.mail_domain = "example.com" + Config.mail_port = 123 + Config.mail_auth_type = MailSettings::SECURE_CONNECTION_TYPES[0] + Config.smtp_username = "schof" + Config.smtp_password = "hellospree!" + Config.secure_connection_type = "TLS" + subject.override! + end + + it { ActionMailer::Base.smtp_settings[:address].should == "smtp.example.com" } + it { ActionMailer::Base.smtp_settings[:domain].should == "example.com" } + it { ActionMailer::Base.smtp_settings[:port].should == 123 } + it { ActionMailer::Base.smtp_settings[:authentication].should == "None" } + it { ActionMailer::Base.smtp_settings[:enable_starttls_auto].should be_true } + + it "doesnt touch user name config" do + ActionMailer::Base.smtp_settings[:user_name].should == nil + end + + it "doesnt touch password config" do + ActionMailer::Base.smtp_settings[:password].should == nil + end + end + end + + context "when mail_auth_type is other than none" do + before do + Config.mail_auth_type = "login" + Config.smtp_username = "schof" + Config.smtp_password = "hellospree!" + subject.override! + end + + context "overrides user credentials" do + it { ActionMailer::Base.smtp_settings[:user_name].should == "schof" } + it { ActionMailer::Base.smtp_settings[:password].should == "hellospree!" } + end + end + end + + context "do not enable delivery" do + before do + Config.enable_mail_delivery = false + subject.override! + end + + it { ActionMailer::Base.perform_deliveries.should be_false } + end + end + + context "override option is false" do + before { Config.override_actionmailer_config = false } + + context "init" do + it "doesnt calls override!" do + subject.should_not_receive(:override!) + MailSettings.init + end + end + end + end + end +end diff --git a/spec/lib/spree/core/token_resource_spec.rb b/spec/lib/spree/core/token_resource_spec.rb new file mode 100644 index 0000000000..40ac8f838a --- /dev/null +++ b/spec/lib/spree/core/token_resource_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction with an actual class that +# extends ActiveRecord::Base and has a corresponding table in the database. So we'll just test it using Order instead +# since those classes are including the module. +describe Spree::Core::TokenResource do + let(:order) { Spree::Order.new } + let(:permission) { mock_model(Spree::TokenizedPermission) } + + it 'should add has_one :tokenized_permission relationship' do + assert Spree::Order.reflect_on_all_associations(:has_one).map(&:name).include?(:tokenized_permission) + end + + context '#token' do + it 'should return the token of the associated permission' do + order.stub :tokenized_permission => permission + permission.stub :token => 'foo' + order.token.should == 'foo' + end + + it 'should return nil if there is no associated permission' do + order.token.should be_nil + end + end + + context '#create_token' do + it 'should create a randomized 16 character token' do + token = order.create_token + token.size.should == 16 + end + end +end diff --git a/spec/lib/spree/product_duplicator_spec.rb b/spec/lib/spree/product_duplicator_spec.rb new file mode 100644 index 0000000000..e9fef3ae07 --- /dev/null +++ b/spec/lib/spree/product_duplicator_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +module Spree + describe Spree::ProductDuplicator do + let(:product) do + double 'Product', + :name => "foo", + :taxons => [], + :product_properties => [property], + :master => variant, + :has_variants? => false + end + + let(:new_product) do + double 'New Product', + :save! => true + end + + let(:property) do + double 'Property' + end + + let(:new_property) do + double 'New Property' + end + + let(:variant) do + double 'Variant', + :sku => "12345", + :price => 19.99, + :currency => "AUD", + :images => [image] + end + + let(:new_variant) do + double 'New Variant', + :sku => "12345" + end + + let(:image) do + double 'Image', + :attachment => double('Attachment') + end + + let(:new_image) do + double 'New Image' + end + + + before do + product.should_receive(:dup).and_return(new_product) + variant.should_receive(:dup).and_return(new_variant) + image.should_receive(:dup).and_return(new_image) + property.should_receive(:dup).and_return(new_property) + end + + it "can duplicate a product" do + duplicator = Spree::ProductDuplicator.new(product) + new_product.should_receive(:name=).with("COPY OF foo") + new_product.should_receive(:taxons=).with([]) + new_product.should_receive(:product_properties=).with([new_property]) + new_product.should_receive(:created_at=).with(nil) + new_product.should_receive(:updated_at=).with(nil) + new_product.should_receive(:deleted_at=).with(nil) + new_product.should_receive(:master=).with(new_variant) + + new_variant.should_receive(:sku=).with("COPY OF 12345") + new_variant.should_receive(:deleted_at=).with(nil) + new_variant.should_receive(:images=).with([new_image]) + new_variant.should_receive(:price=).with(variant.price) + new_variant.should_receive(:currency=).with(variant.currency) + + image.attachment.should_receive(:clone).and_return(image.attachment) + + new_image.should_receive(:assign_attributes). + with(:attachment => image.attachment). + and_return(new_image) + + new_property.should_receive(:created_at=).with(nil) + new_property.should_receive(:updated_at=).with(nil) + + duplicator.duplicate + end + + end +end + From 03bb1f053a25140a3b186b22099729a7e22553a5 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 11 Jul 2020 16:43:42 +0100 Subject: [PATCH 21/24] Fix easy rubocop issues --- lib/spree/core/calculated_adjustments.rb | 42 +++++++------ lib/spree/core/delegate_belongs_to.rb | 63 ++++++++++--------- lib/spree/core/gateway_error.rb | 2 + lib/spree/core/mail_interceptor.rb | 8 ++- lib/spree/core/mail_settings.rb | 57 +++++++++-------- lib/spree/core/permalinks.rb | 46 +++++++------- lib/spree/core/s3_support.rb | 26 +++++--- lib/spree/core/token_resource.rb | 9 +-- lib/spree/product_duplicator.rb | 6 +- .../spree/core/calculated_adjustments_spec.rb | 28 ++++----- spec/lib/spree/core/mail_interceptor_spec.rb | 18 +++--- spec/lib/spree/core/mail_settings_spec.rb | 7 ++- spec/lib/spree/core/token_resource_spec.rb | 15 +++-- spec/lib/spree/product_duplicator_spec.rb | 31 +++++---- 14 files changed, 199 insertions(+), 159 deletions(-) diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb index 221a9926f0..e735ec4993 100644 --- a/lib/spree/core/calculated_adjustments.rb +++ b/lib/spree/core/calculated_adjustments.rb @@ -1,14 +1,16 @@ +# frozen_string_literal: true + module Spree module Core module CalculatedAdjustments def self.included(klass) klass.class_eval do - has_one :calculator, :class_name => "Spree::Calculator", :as => :calculable, :dependent => :destroy + has_one :calculator, class_name: "Spree::Calculator", as: :calculable, dependent: :destroy accepts_nested_attributes_for :calculator - validates :calculator, :presence => true + validates :calculator, presence: true def self.calculators - spree_calculators.send model_name_without_spree_namespace + spree_calculators.__send__(model_name_without_spree_namespace) end def calculator_type @@ -17,13 +19,14 @@ module Spree def calculator_type=(calculator_type) klass = calculator_type.constantize if calculator_type - self.calculator = klass.new if klass && !self.calculator.is_a?(klass) + self.calculator = klass.new if klass && !calculator.is_a?(klass) end - # Creates a new adjustment for the target object (which is any class that has_many :adjustments) and - # sets amount based on the calculator as applied to the calculable argument (Order, LineItems[], Shipment, etc.) + # Creates a new adjustment for the target object + # (which is any class that has_many :adjustments) and sets amount based on the + # calculator as applied to the given calculable (Order, LineItems[], Shipment, etc.) # By default the adjustment will not be considered mandatory - def create_adjustment(label, target, calculable, mandatory=false, state="closed") + def create_adjustment(label, target, calculable, mandatory = false, state = "closed") # Adjustment calculations done on Spree::Shipment objects MUST # be done on their to_package'd variants instead # It's only the package that contains the correct information. @@ -31,19 +34,21 @@ module Spree old_calculable = calculable calculable = calculable.to_package if calculable.is_a?(Spree::Shipment) amount = compute_amount(calculable) - return if amount == 0 && !mandatory + return if amount.zero? && !mandatory + target.adjustments.create( - :amount => amount, - :source => old_calculable, - :originator => self, - :label => label, - :mandatory => mandatory, - :state => state + amount: amount, + source: old_calculable, + originator: self, + label: label, + mandatory: mandatory, + state: state ) end - # Updates the amount of the adjustment using our Calculator and calling the +compute+ method with the +calculable+ - # referenced passed to the method. + # Updates the amount of the adjustment using our Calculator and + # calling the +compute+ method with the +calculable+ + # referenced passed to the method. def update_adjustment(adjustment, calculable) # Adjustment calculations done on Spree::Shipment objects MUST # be done on their to_package'd variants instead @@ -57,12 +62,13 @@ module Spree # NOTE: May be overriden by classes where this module is included into. # Such as Spree::Promotion::Action::CreateAdjustment. def compute_amount(calculable) - self.calculator.compute(calculable) + calculator.compute(calculable) end private + def self.model_name_without_spree_namespace - self.to_s.tableize.gsub('/', '_').sub('spree_', '') + to_s.tableize.gsub('/', '_').sub('spree_', '') end def self.spree_calculators diff --git a/lib/spree/core/delegate_belongs_to.rb b/lib/spree/core/delegate_belongs_to.rb index 2adb978557..cf471ebb74 100644 --- a/lib/spree/core/delegate_belongs_to.rb +++ b/lib/spree/core/delegate_belongs_to.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + ## # Creates methods on object which delegate to an association proxy. # see delegate_belongs_to for two uses @@ -19,14 +21,14 @@ module DelegateBelongsTo extend ActiveSupport::Concern module ClassMethods - - @@default_rejected_delegate_columns = ['created_at','created_on','updated_at','updated_on','lock_version','type','id','position','parent_id','lft','rgt'] + @@default_rejected_delegate_columns = ['created_at', 'created_on', 'updated_at', + 'updated_on', 'lock_version', 'type', 'id', + 'position', 'parent_id', 'lft', 'rgt'] mattr_accessor :default_rejected_delegate_columns ## # Creates methods for accessing and setting attributes on an association. Uses same # default list of attributes as delegates_to_association. - # @todo Integrate this with ActiveRecord::Dirty, so if you set a property through one of these setters and then call save on this object, it will save the associated object automatically. # delegate_belongs_to :contact # delegate_belongs_to :contact, [:defaults] ## same as above, and useless # delegate_belongs_to :contact, [:defaults, :address, :fullname], :class_name => 'VCard' @@ -39,51 +41,54 @@ module DelegateBelongsTo attrs.each do |attr| class_def attr do |*args| if args.empty? - send(:delegator_for, association).send(attr) + __send__(:delegator_for, association).__send__(attr) else - send(:delegator_for, association).send(attr, *args) + __send__(:delegator_for, association).__send__(attr, *args) end end class_def "#{attr}=" do |val| - send(:delegator_for, association).send("#{attr}=", val) + __send__(:delegator_for, association).__send__("#{attr}=", val) end end end protected - def get_association_column_names(association, without_default_rejected_delegate_columns=true) - begin - association_klass = reflect_on_association(association).klass - methods = association_klass.column_names - methods.reject!{|x|default_rejected_delegate_columns.include?(x.to_s)} if without_default_rejected_delegate_columns - return methods - rescue - return [] - end + def get_association_column_names(association, without_default_rejected_delegate_columns = true) + association_klass = reflect_on_association(association).klass + methods = association_klass.column_names + if without_default_rejected_delegate_columns + methods.reject!{ |x| default_rejected_delegate_columns.include?(x.to_s) } + end + methods + rescue + [] + end + + ## + # initialize_association :belongs_to, :contact + def initialize_association(type, association, opts = {}) + unless [:belongs_to].include?(type.to_s.to_sym) + raise 'Illegal or unimplemented association type.' end - ## - # initialize_association :belongs_to, :contact - def initialize_association(type, association, opts={}) - raise 'Illegal or unimplemented association type.' unless [:belongs_to].include?(type.to_s.to_sym) - send type, association, opts if reflect_on_association(association).nil? - end + __send__(type, association, opts) if reflect_on_association(association).nil? + end private - def class_def(name, method=nil, &blk) - class_eval { method.nil? ? define_method(name, &blk) : define_method(name, method) } - end - + def class_def(name, method = nil, &blk) + class_eval { method.nil? ? define_method(name, &blk) : define_method(name, method) } + end end def delegator_for(association) - send("#{association}=", self.class.reflect_on_association(association).klass.new) if send(association).nil? - send(association) + if __send__(association).nil? + __send__("#{association}=", self.class.reflect_on_association(association).klass.new) + end + __send__(association) end protected :delegator_for - end -ActiveRecord::Base.send :include, DelegateBelongsTo +ActiveRecord::Base.__send__(:include, DelegateBelongsTo) diff --git a/lib/spree/core/gateway_error.rb b/lib/spree/core/gateway_error.rb index 5a6ddda30b..f90310bc54 100644 --- a/lib/spree/core/gateway_error.rb +++ b/lib/spree/core/gateway_error.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree module Core class GatewayError < RuntimeError; end diff --git a/lib/spree/core/mail_interceptor.rb b/lib/spree/core/mail_interceptor.rb index e0a1400ab8..87ae2e33ab 100644 --- a/lib/spree/core/mail_interceptor.rb +++ b/lib/spree/core/mail_interceptor.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Allows us to intercept any outbound mail message and make last minute changes # (such as specifying a "from" address or sending to a test email account) # @@ -13,9 +15,9 @@ module Spree message.to = Config[:intercept_email] end - if Config[:mail_bcc].present? - message.bcc ||= Config[:mail_bcc] - end + return if Config[:mail_bcc].blank? + + message.bcc ||= Config[:mail_bcc] end end end diff --git a/lib/spree/core/mail_settings.rb b/lib/spree/core/mail_settings.rb index 757144af8e..80664d1c64 100644 --- a/lib/spree/core/mail_settings.rb +++ b/lib/spree/core/mail_settings.rb @@ -1,14 +1,16 @@ +# frozen_string_literal: true + module Spree module Core class MailSettings - MAIL_AUTH = ['None', 'plain', 'login', 'cram_md5'] - SECURE_CONNECTION_TYPES = ['None','SSL','TLS'] + MAIL_AUTH = ['None', 'plain', 'login', 'cram_md5'].freeze + SECURE_CONNECTION_TYPES = ['None', 'SSL', 'TLS'].freeze # Override the Rails application mail settings based on preferences # This makes it possible to configure the mail settings through an admin # interface instead of requiring changes to the Rails envrionment file def self.init - self.new.override! if override? + new.override! if override? end def self.override? @@ -26,35 +28,36 @@ module Spree end private - def mail_server_settings - settings = if need_authentication? - basic_settings.merge(user_credentials) - else - basic_settings - end - settings.merge :enable_starttls_auto => secure_connection? - end + def mail_server_settings + settings = if need_authentication? + basic_settings.merge(user_credentials) + else + basic_settings + end - def user_credentials - { :user_name => Config.smtp_username, - :password => Config.smtp_password } - end + settings.merge(enable_starttls_auto: secure_connection?) + end - def basic_settings - { :address => Config.mail_host, - :domain => Config.mail_domain, - :port => Config.mail_port, - :authentication => Config.mail_auth_type } - end + def user_credentials + { user_name: Config.smtp_username, + password: Config.smtp_password } + end - def need_authentication? - Config.mail_auth_type != 'None' - end + def basic_settings + { address: Config.mail_host, + domain: Config.mail_domain, + port: Config.mail_port, + authentication: Config.mail_auth_type } + end - def secure_connection? - Config.secure_connection_type == 'TLS' - end + def need_authentication? + Config.mail_auth_type != 'None' + end + + def secure_connection? + Config.secure_connection_type == 'TLS' + end end end end diff --git a/lib/spree/core/permalinks.rb b/lib/spree/core/permalinks.rb index 6e0fcc8735..6dc1fb9c6a 100644 --- a/lib/spree/core/permalinks.rb +++ b/lib/spree/core/permalinks.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'stringex' module Spree @@ -10,23 +12,23 @@ module Spree end module ClassMethods - def make_permalink(options={}) + def make_permalink(options = {}) options[:field] ||= :permalink self.permalink_options = options - if self.connected? - if self.table_exists? && self.column_names.include?(permalink_options[:field].to_s) - before_validation(:on => :create) { save_permalink } - end - end + return unless connected? && + table_exists? && + column_names.include?(permalink_options[:field].to_s) + + before_validation(on: :create) { save_permalink } end def find_by_param(value, *args) - self.send("find_by_#{permalink_field}", value, *args) + __send__("find_by_#{permalink_field}", value, *args) end def find_by_param!(value, *args) - self.send("find_by_#{permalink_field}!", value, *args) + __send__("find_by_#{permalink_field}!", value, *args) end def permalink_field @@ -44,22 +46,24 @@ module Spree end def generate_permalink - "#{self.class.permalink_prefix}#{Array.new(9){rand(9)}.join}" + "#{self.class.permalink_prefix}#{Array.new(9) { rand(9) }.join}" end - def save_permalink(permalink_value=self.to_param) - self.with_lock do + def save_permalink(permalink_value = to_param) + with_lock do permalink_value ||= generate_permalink field = self.class.permalink_field - # Do other links exist with this permalink? - other = self.class.where("#{self.class.table_name}.#{field} LIKE ?", "#{permalink_value}%") - if other.any? - # Find the existing permalink with the highest number, and increment that number. - # (If none of the existing permalinks have a number, this will evaluate to 1.) - number = other.map { |o| o.send(field)[/-(\d+)$/, 1].to_i }.max + 1 - permalink_value += "-#{number.to_s}" - end + + # Do other links exist with this permalink? + other = self.class. + where("#{self.class.table_name}.#{field} LIKE ?", "#{permalink_value}%") + if other.any? + # Find the existing permalink with the highest number, and increment that number. + # (If none of the existing permalinks have a number, this will evaluate to 1.) + number = other.map { |o| o.__send__(field)[/-(\d+)$/, 1].to_i }.max + 1 + permalink_value += "-#{number}" + end write_attribute(field, permalink_value) end end @@ -67,5 +71,5 @@ module Spree end end -ActiveRecord::Base.send :include, Spree::Core::Permalinks -ActiveRecord::Relation.send :include, Spree::Core::Permalinks +ActiveRecord::Base.__send__ :include, Spree::Core::Permalinks +ActiveRecord::Relation.__send__ :include, Spree::Core::Permalinks diff --git a/lib/spree/core/s3_support.rb b/lib/spree/core/s3_support.rb index 8ae8354391..92229c15f5 100644 --- a/lib/spree/core/s3_support.rb +++ b/lib/spree/core/s3_support.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree module Core # This module exists to reduce duplication in S3 settings between @@ -9,15 +11,23 @@ module Spree def self.supports_s3(field) # Load user defined paperclip settings config = Spree::Config - if config[:use_s3] - s3_creds = { :access_key_id => config[:s3_access_key], :secret_access_key => config[:s3_secret], :bucket => config[:s3_bucket] } - self.attachment_definitions[field][:storage] = :s3 - self.attachment_definitions[field][:s3_credentials] = s3_creds - self.attachment_definitions[field][:s3_headers] = ActiveSupport::JSON.decode(config[:s3_headers]) - self.attachment_definitions[field][:bucket] = config[:s3_bucket] - self.attachment_definitions[field][:s3_protocol] = config[:s3_protocol].downcase unless config[:s3_protocol].blank? - self.attachment_definitions[field][:s3_host_alias] = config[:s3_host_alias] unless config[:s3_host_alias].blank? + return unless config[:use_s3] + + s3_creds = { access_key_id: config[:s3_access_key], + secret_access_key: config[:s3_secret], + bucket: config[:s3_bucket] } + attachment_definitions[field][:storage] = :s3 + attachment_definitions[field][:s3_credentials] = s3_creds + attachment_definitions[field][:s3_headers] = ActiveSupport::JSON. + decode(config[:s3_headers]) + attachment_definitions[field][:bucket] = config[:s3_bucket] + if config[:s3_protocol].present? + attachment_definitions[field][:s3_protocol] = config[:s3_protocol].downcase end + + return if config[:s3_host_alias].blank? + + attachment_definitions[field][:s3_host_alias] = config[:s3_host_alias] end end end diff --git a/lib/spree/core/token_resource.rb b/lib/spree/core/token_resource.rb index 48697f29b2..8d4173d11b 100644 --- a/lib/spree/core/token_resource.rb +++ b/lib/spree/core/token_resource.rb @@ -1,17 +1,19 @@ +# frozen_string_literal: true + module Spree module Core module TokenResource module ClassMethods def token_resource - has_one :tokenized_permission, :as => :permissable - delegate :token, :to => :tokenized_permission, :allow_nil => true + has_one :tokenized_permission, as: :permissable + delegate :token, to: :tokenized_permission, allow_nil: true after_create :create_token end end def create_token permission = build_tokenized_permission - permission.token = token = ::SecureRandom::hex(8) + permission.token = token = ::SecureRandom.hex(8) permission.save! token end @@ -24,4 +26,3 @@ module Spree end ActiveRecord::Base.class_eval { include Spree::Core::TokenResource } - diff --git a/lib/spree/product_duplicator.rb b/lib/spree/product_duplicator.rb index f563223f70..8866f85fa5 100644 --- a/lib/spree/product_duplicator.rb +++ b/lib/spree/product_duplicator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class ProductDuplicator attr_accessor :product @@ -13,7 +15,7 @@ module Spree new_product.option_types = product.option_types if product.has_variants? # allow site to do some customization - new_product.send(:duplicate_extra, product) if new_product.respond_to?(:duplicate_extra) + new_product.__send__(:duplicate_extra, product) if new_product.respond_to?(:duplicate_extra) new_product.save! new_product end @@ -45,7 +47,7 @@ module Spree def duplicate_image(image) new_image = image.dup - new_image.assign_attributes(:attachment => image.attachment.clone) + new_image.assign_attributes(attachment: image.attachment.clone) new_image end diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb index eecfee445f..63cd46e4e7 100644 --- a/spec/lib/spree/core/calculated_adjustments_spec.rb +++ b/spec/lib/spree/core/calculated_adjustments_spec.rb @@ -1,17 +1,19 @@ +# frozen_string_literal: true + require 'spec_helper' -# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction with an actual class that -# extends ActiveRecord::Base and has a corresponding table in the database. So we'll just test it using Order and -# ShippingMethod instead since those classes are including the module. +# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction +# with an actual class that extends ActiveRecord::Base and has a corresponding table in the DB. +# So we'll just test it using Order and ShippingMethod. These classes are including the module. describe Spree::Core::CalculatedAdjustments do - let(:calculator) { mock_model(Spree::Calculator, :compute => 10, :[]= => nil) } it "should add has_one :calculator relationship" do - assert Spree::ShippingMethod.reflect_on_all_associations(:has_one).map(&:name).include?(:calculator) + assert Spree::ShippingMethod. + reflect_on_all_associations(:has_one).map(&:name).include?(:calculator) end - let(:tax_rate) { Spree::TaxRate.new(:calculator => calculator) } + let(:tax_rate) { Spree::TaxRate.new(calculator: calculator) } context "#create_adjustment and its resulting adjustment" do let(:order) { Spree::Order.create } @@ -25,9 +27,9 @@ describe Spree::Core::CalculatedAdjustments do it "should have the correct originator and an amount derived from the calculator and supplied calculable" do adjustment = tax_rate.create_adjustment("foo", target, order) adjustment.should_not be_nil - adjustment.amount.should == 10 - adjustment.source.should == order - adjustment.originator.should == tax_rate + expect(adjustment.amount).to eq 10 + expect(adjustment.source).to eq order + expect(adjustment.originator).to eq tax_rate end it "should be mandatory if true is supplied for that parameter" do @@ -36,13 +38,13 @@ describe Spree::Core::CalculatedAdjustments do end context "when the calculator returns 0" do - before { calculator.stub :compute => 0 } + before { calculator.stub(compute: 0) } context "when adjustment is mandatory" do before { tax_rate.create_adjustment("foo", target, order, true) } it "should create an adjustment" do - Spree::Adjustment.count.should == 1 + expect(Spree::Adjustment.count).to eq 1 end end @@ -50,11 +52,10 @@ describe Spree::Core::CalculatedAdjustments do before { tax_rate.create_adjustment("foo", target, order, false) } it "should not create an adjustment" do - Spree::Adjustment.count.should == 0 + expect(Spree::Adjustment.count).to eq 0 end end end - end context "#update_adjustment" do @@ -65,5 +66,4 @@ describe Spree::Core::CalculatedAdjustments do tax_rate.update_adjustment(adjustment, calculable) end end - end diff --git a/spec/lib/spree/core/mail_interceptor_spec.rb b/spec/lib/spree/core/mail_interceptor_spec.rb index 9375a0471c..e60275c1b9 100644 --- a/spec/lib/spree/core/mail_interceptor_spec.rb +++ b/spec/lib/spree/core/mail_interceptor_spec.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + require 'spec_helper' # We'll use the OrderMailer as a quick and easy way to test. IF it works here # it works for all email (in theory.) describe Spree::OrderMailer do - let(:order) { Spree::Order.new(:email => "customer@example.com") } + let(:order) { Spree::Order.new(email: "customer@example.com") } let(:message) { Spree::OrderMailer.confirm_email(order) } before(:all) do @@ -22,7 +24,7 @@ describe Spree::OrderMailer do Spree::Config[:mails_from] = "no-reply@foobar.com" message.deliver @email = ActionMailer::Base.deliveries.first - @email.from.should == ["no-reply@foobar.com"] + expect(@email.from).to eq ["no-reply@foobar.com"] end it "should use the provided from address" do @@ -31,15 +33,15 @@ describe Spree::OrderMailer do message.to = "test@test.com" message.deliver email = ActionMailer::Base.deliveries.first - email.from.should == ["override@foobar.com"] - email.to.should == ["test@test.com"] + expect(email.from).to eq ["override@foobar.com"] + expect(email.to).to eq ["test@test.com"] end it "should add the bcc email when provided" do Spree::Config[:mail_bcc] = "bcc-foo@foobar.com" message.deliver @email = ActionMailer::Base.deliveries.first - @email.bcc.should == ["bcc-foo@foobar.com"] + expect(@email.bcc).to eq ["bcc-foo@foobar.com"] end context "when intercept_email is provided" do @@ -55,14 +57,14 @@ describe Spree::OrderMailer do Spree::Config[:intercept_email] = "intercept@foobar.com" message.deliver @email = ActionMailer::Base.deliveries.first - @email.to.should == ["intercept@foobar.com"] + expect(@email.to).to eq ["intercept@foobar.com"] end it "should modify the subject to include the original email" do Spree::Config[:intercept_email] = "intercept@foobar.com" message.deliver @email = ActionMailer::Base.deliveries.first - @email.subject.match(/customer@example\.com/).should be_true + expect(@email.subject.match(/customer@example\.com/)).to be_true end end @@ -71,7 +73,7 @@ describe Spree::OrderMailer do Spree::Config[:intercept_email] = "" message.deliver @email = ActionMailer::Base.deliveries.first - @email.to.should == ["customer@example.com"] + expect(@email.to).to eq ["customer@example.com"] end end end diff --git a/spec/lib/spree/core/mail_settings_spec.rb b/spec/lib/spree/core/mail_settings_spec.rb index 8528a6226d..58ea85153f 100644 --- a/spec/lib/spree/core/mail_settings_spec.rb +++ b/spec/lib/spree/core/mail_settings_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' module Spree @@ -20,7 +22,6 @@ module Spree before { Config.enable_mail_delivery = true } context "overrides appplication defaults" do - context "authentication method is none" do before do Config.mail_host = "smtp.example.com" @@ -40,11 +41,11 @@ module Spree it { ActionMailer::Base.smtp_settings[:enable_starttls_auto].should be_true } it "doesnt touch user name config" do - ActionMailer::Base.smtp_settings[:user_name].should == nil + expect(ActionMailer::Base.smtp_settings[:user_name]).to be_nil end it "doesnt touch password config" do - ActionMailer::Base.smtp_settings[:password].should == nil + expect(ActionMailer::Base.smtp_settings[:password]).to be_nil end end end diff --git a/spec/lib/spree/core/token_resource_spec.rb b/spec/lib/spree/core/token_resource_spec.rb index 40ac8f838a..a434b94d7b 100644 --- a/spec/lib/spree/core/token_resource_spec.rb +++ b/spec/lib/spree/core/token_resource_spec.rb @@ -1,20 +1,23 @@ +# frozen_string_literal: true + require 'spec_helper' -# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction with an actual class that -# extends ActiveRecord::Base and has a corresponding table in the database. So we'll just test it using Order instead -# since those classes are including the module. +# Its pretty difficult to test this module in isolation b/c it needs to work in conjunction +# with an actual class that extends ActiveRecord::Base and has a corresponding table in the DB. +# So we'll just test it using Order instead since it included the module. describe Spree::Core::TokenResource do let(:order) { Spree::Order.new } let(:permission) { mock_model(Spree::TokenizedPermission) } it 'should add has_one :tokenized_permission relationship' do - assert Spree::Order.reflect_on_all_associations(:has_one).map(&:name).include?(:tokenized_permission) + assert Spree::Order. + reflect_on_all_associations(:has_one).map(&:name).include?(:tokenized_permission) end context '#token' do it 'should return the token of the associated permission' do - order.stub :tokenized_permission => permission - permission.stub :token => 'foo' + order.stub tokenized_permission: permission + permission.stub token: 'foo' order.token.should == 'foo' end diff --git a/spec/lib/spree/product_duplicator_spec.rb b/spec/lib/spree/product_duplicator_spec.rb index e9fef3ae07..0a9a702eed 100644 --- a/spec/lib/spree/product_duplicator_spec.rb +++ b/spec/lib/spree/product_duplicator_spec.rb @@ -1,19 +1,21 @@ +# frozen_string_literal: true + require 'spec_helper' module Spree describe Spree::ProductDuplicator do let(:product) do double 'Product', - :name => "foo", - :taxons => [], - :product_properties => [property], - :master => variant, - :has_variants? => false + name: "foo", + taxons: [], + product_properties: [property], + master: variant, + has_variants?: false end let(:new_product) do double 'New Product', - :save! => true + save!: true end let(:property) do @@ -26,27 +28,26 @@ module Spree let(:variant) do double 'Variant', - :sku => "12345", - :price => 19.99, - :currency => "AUD", - :images => [image] + sku: "12345", + price: 19.99, + currency: "AUD", + images: [image] end let(:new_variant) do double 'New Variant', - :sku => "12345" + sku: "12345" end let(:image) do double 'Image', - :attachment => double('Attachment') + attachment: double('Attachment') end let(:new_image) do double 'New Image' end - before do product.should_receive(:dup).and_return(new_product) variant.should_receive(:dup).and_return(new_variant) @@ -73,7 +74,7 @@ module Spree image.attachment.should_receive(:clone).and_return(image.attachment) new_image.should_receive(:assign_attributes). - with(:attachment => image.attachment). + with(attachment: image.attachment). and_return(new_image) new_property.should_receive(:created_at=).with(nil) @@ -81,7 +82,5 @@ module Spree duplicator.duplicate end - end end - From 95ffff50872268e120940d3ee446b902ba530812 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 11 Jul 2020 16:59:35 +0100 Subject: [PATCH 22/24] Fix specs brought from spree --- .../spree/core/calculated_adjustments_spec.rb | 14 +++++++++----- spec/lib/spree/core/mail_interceptor_spec.rb | 6 +++--- spec/lib/spree/core/mail_settings_spec.rb | 16 ++++++++-------- spec/lib/spree/core/token_resource_spec.rb | 8 ++++---- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb index 63cd46e4e7..b701fff3e0 100644 --- a/spec/lib/spree/core/calculated_adjustments_spec.rb +++ b/spec/lib/spree/core/calculated_adjustments_spec.rb @@ -6,15 +6,19 @@ require 'spec_helper' # with an actual class that extends ActiveRecord::Base and has a corresponding table in the DB. # So we'll just test it using Order and ShippingMethod. These classes are including the module. describe Spree::Core::CalculatedAdjustments do - let(:calculator) { mock_model(Spree::Calculator, :compute => 10, :[]= => nil) } + let(:calculator) { build(:calculator) } + let(:tax_rate) { Spree::TaxRate.new(calculator: calculator) } + + before do + allow(calculator).to receive(:compute) { 10 } + allow(calculator).to receive(:[]) { nil } + end it "should add has_one :calculator relationship" do assert Spree::ShippingMethod. reflect_on_all_associations(:has_one).map(&:name).include?(:calculator) end - let(:tax_rate) { Spree::TaxRate.new(calculator: calculator) } - context "#create_adjustment and its resulting adjustment" do let(:order) { Spree::Order.create } let(:target) { order } @@ -26,7 +30,7 @@ describe Spree::Core::CalculatedAdjustments do it "should have the correct originator and an amount derived from the calculator and supplied calculable" do adjustment = tax_rate.create_adjustment("foo", target, order) - adjustment.should_not be_nil + expect(adjustment).not_to be_nil expect(adjustment.amount).to eq 10 expect(adjustment.source).to eq order expect(adjustment.originator).to eq tax_rate @@ -34,7 +38,7 @@ describe Spree::Core::CalculatedAdjustments do it "should be mandatory if true is supplied for that parameter" do adjustment = tax_rate.create_adjustment("foo", target, order, true) - adjustment.should be_mandatory + expect(adjustment).to be_mandatory end context "when the calculator returns 0" do diff --git a/spec/lib/spree/core/mail_interceptor_spec.rb b/spec/lib/spree/core/mail_interceptor_spec.rb index e60275c1b9..d47538cb3c 100644 --- a/spec/lib/spree/core/mail_interceptor_spec.rb +++ b/spec/lib/spree/core/mail_interceptor_spec.rb @@ -46,11 +46,11 @@ describe Spree::OrderMailer do context "when intercept_email is provided" do it "should strip the bcc recipients" do - message.bcc.should be_blank + expect(message.bcc).to be_blank end it "should strip the cc recipients" do - message.cc.should be_blank + expect(message.cc).to be_blank end it "should replace the receipient with the specified address" do @@ -64,7 +64,7 @@ describe Spree::OrderMailer do Spree::Config[:intercept_email] = "intercept@foobar.com" message.deliver @email = ActionMailer::Base.deliveries.first - expect(@email.subject.match(/customer@example\.com/)).to be_true + expect(@email.subject.match(/customer@example\.com/)).to be_truthy end end diff --git a/spec/lib/spree/core/mail_settings_spec.rb b/spec/lib/spree/core/mail_settings_spec.rb index 58ea85153f..753f8465ee 100644 --- a/spec/lib/spree/core/mail_settings_spec.rb +++ b/spec/lib/spree/core/mail_settings_spec.rb @@ -34,11 +34,11 @@ module Spree subject.override! end - it { ActionMailer::Base.smtp_settings[:address].should == "smtp.example.com" } - it { ActionMailer::Base.smtp_settings[:domain].should == "example.com" } - it { ActionMailer::Base.smtp_settings[:port].should == 123 } - it { ActionMailer::Base.smtp_settings[:authentication].should == "None" } - it { ActionMailer::Base.smtp_settings[:enable_starttls_auto].should be_true } + it { expect(ActionMailer::Base.smtp_settings[:address]).to eq "smtp.example.com" } + it { expect(ActionMailer::Base.smtp_settings[:domain]).to eq "example.com" } + it { expect(ActionMailer::Base.smtp_settings[:port]).to eq 123 } + it { expect(ActionMailer::Base.smtp_settings[:authentication]).to eq "None" } + it { expect(ActionMailer::Base.smtp_settings[:enable_starttls_auto]).to be_truthy } it "doesnt touch user name config" do expect(ActionMailer::Base.smtp_settings[:user_name]).to be_nil @@ -59,8 +59,8 @@ module Spree end context "overrides user credentials" do - it { ActionMailer::Base.smtp_settings[:user_name].should == "schof" } - it { ActionMailer::Base.smtp_settings[:password].should == "hellospree!" } + it { expect(ActionMailer::Base.smtp_settings[:user_name]).to eq "schof" } + it { expect(ActionMailer::Base.smtp_settings[:password]).to eq "hellospree!" } end end end @@ -71,7 +71,7 @@ module Spree subject.override! end - it { ActionMailer::Base.perform_deliveries.should be_false } + it { expect(ActionMailer::Base.perform_deliveries).to be_falsy } end end diff --git a/spec/lib/spree/core/token_resource_spec.rb b/spec/lib/spree/core/token_resource_spec.rb index a434b94d7b..b5be7195c0 100644 --- a/spec/lib/spree/core/token_resource_spec.rb +++ b/spec/lib/spree/core/token_resource_spec.rb @@ -7,7 +7,7 @@ require 'spec_helper' # So we'll just test it using Order instead since it included the module. describe Spree::Core::TokenResource do let(:order) { Spree::Order.new } - let(:permission) { mock_model(Spree::TokenizedPermission) } + let(:permission) { double(Spree::TokenizedPermission) } it 'should add has_one :tokenized_permission relationship' do assert Spree::Order. @@ -18,18 +18,18 @@ describe Spree::Core::TokenResource do it 'should return the token of the associated permission' do order.stub tokenized_permission: permission permission.stub token: 'foo' - order.token.should == 'foo' + expect(order.token).to eq 'foo' end it 'should return nil if there is no associated permission' do - order.token.should be_nil + expect(order.token).to be_nil end end context '#create_token' do it 'should create a randomized 16 character token' do token = order.create_token - token.size.should == 16 + expect(token.size).to eq 16 end end end From ebf9be41bbe7ff2bd8a7d780aa0fd459cf89c7bf Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 11 Jul 2020 17:02:03 +0100 Subject: [PATCH 23/24] Transpec specs --- .../spree/core/calculated_adjustments_spec.rb | 6 +-- spec/lib/spree/core/mail_settings_spec.rb | 6 +-- spec/lib/spree/core/token_resource_spec.rb | 4 +- spec/lib/spree/product_duplicator_spec.rb | 40 +++++++++---------- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb index b701fff3e0..b9210c4f29 100644 --- a/spec/lib/spree/core/calculated_adjustments_spec.rb +++ b/spec/lib/spree/core/calculated_adjustments_spec.rb @@ -24,7 +24,7 @@ describe Spree::Core::CalculatedAdjustments do let(:target) { order } it "should be associated with the target" do - target.adjustments.should_receive(:create) + expect(target.adjustments).to receive(:create) tax_rate.create_adjustment("foo", target, order) end @@ -42,7 +42,7 @@ describe Spree::Core::CalculatedAdjustments do end context "when the calculator returns 0" do - before { calculator.stub(compute: 0) } + before { allow(calculator).to receive_messages(compute: 0) } context "when adjustment is mandatory" do before { tax_rate.create_adjustment("foo", target, order, true) } @@ -66,7 +66,7 @@ describe Spree::Core::CalculatedAdjustments do it "should update the adjustment using its calculator (and the specified source)" do adjustment = double(:adjustment).as_null_object calculable = double :calculable - adjustment.should_receive(:update_column).with(:amount, 10) + expect(adjustment).to receive(:update_column).with(:amount, 10) tax_rate.update_adjustment(adjustment, calculable) end end diff --git a/spec/lib/spree/core/mail_settings_spec.rb b/spec/lib/spree/core/mail_settings_spec.rb index 753f8465ee..a8db4d5eed 100644 --- a/spec/lib/spree/core/mail_settings_spec.rb +++ b/spec/lib/spree/core/mail_settings_spec.rb @@ -12,8 +12,8 @@ module Spree context "init" do it "calls override!" do - MailSettings.should_receive(:new).and_return(subject) - subject.should_receive(:override!) + expect(MailSettings).to receive(:new).and_return(subject) + expect(subject).to receive(:override!) MailSettings.init end end @@ -80,7 +80,7 @@ module Spree context "init" do it "doesnt calls override!" do - subject.should_not_receive(:override!) + expect(subject).not_to receive(:override!) MailSettings.init end end diff --git a/spec/lib/spree/core/token_resource_spec.rb b/spec/lib/spree/core/token_resource_spec.rb index b5be7195c0..087c5b9350 100644 --- a/spec/lib/spree/core/token_resource_spec.rb +++ b/spec/lib/spree/core/token_resource_spec.rb @@ -16,8 +16,8 @@ describe Spree::Core::TokenResource do context '#token' do it 'should return the token of the associated permission' do - order.stub tokenized_permission: permission - permission.stub token: 'foo' + allow(order).to receive_messages tokenized_permission: permission + allow(permission).to receive_messages token: 'foo' expect(order.token).to eq 'foo' end diff --git a/spec/lib/spree/product_duplicator_spec.rb b/spec/lib/spree/product_duplicator_spec.rb index 0a9a702eed..7978465a5e 100644 --- a/spec/lib/spree/product_duplicator_spec.rb +++ b/spec/lib/spree/product_duplicator_spec.rb @@ -49,36 +49,36 @@ module Spree end before do - product.should_receive(:dup).and_return(new_product) - variant.should_receive(:dup).and_return(new_variant) - image.should_receive(:dup).and_return(new_image) - property.should_receive(:dup).and_return(new_property) + expect(product).to receive(:dup).and_return(new_product) + expect(variant).to receive(:dup).and_return(new_variant) + expect(image).to receive(:dup).and_return(new_image) + expect(property).to receive(:dup).and_return(new_property) end it "can duplicate a product" do duplicator = Spree::ProductDuplicator.new(product) - new_product.should_receive(:name=).with("COPY OF foo") - new_product.should_receive(:taxons=).with([]) - new_product.should_receive(:product_properties=).with([new_property]) - new_product.should_receive(:created_at=).with(nil) - new_product.should_receive(:updated_at=).with(nil) - new_product.should_receive(:deleted_at=).with(nil) - new_product.should_receive(:master=).with(new_variant) + expect(new_product).to receive(:name=).with("COPY OF foo") + expect(new_product).to receive(:taxons=).with([]) + expect(new_product).to receive(:product_properties=).with([new_property]) + expect(new_product).to receive(:created_at=).with(nil) + expect(new_product).to receive(:updated_at=).with(nil) + expect(new_product).to receive(:deleted_at=).with(nil) + expect(new_product).to receive(:master=).with(new_variant) - new_variant.should_receive(:sku=).with("COPY OF 12345") - new_variant.should_receive(:deleted_at=).with(nil) - new_variant.should_receive(:images=).with([new_image]) - new_variant.should_receive(:price=).with(variant.price) - new_variant.should_receive(:currency=).with(variant.currency) + expect(new_variant).to receive(:sku=).with("COPY OF 12345") + expect(new_variant).to receive(:deleted_at=).with(nil) + expect(new_variant).to receive(:images=).with([new_image]) + expect(new_variant).to receive(:price=).with(variant.price) + expect(new_variant).to receive(:currency=).with(variant.currency) - image.attachment.should_receive(:clone).and_return(image.attachment) + expect(image.attachment).to receive(:clone).and_return(image.attachment) - new_image.should_receive(:assign_attributes). + expect(new_image).to receive(:assign_attributes). with(attachment: image.attachment). and_return(new_image) - new_property.should_receive(:created_at=).with(nil) - new_property.should_receive(:updated_at=).with(nil) + expect(new_property).to receive(:created_at=).with(nil) + expect(new_property).to receive(:updated_at=).with(nil) duplicator.duplicate end From be3a10b2b1ef34aefe3a7ccfefd4b5e3b7f5f3cf Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 28 Jul 2020 19:01:14 +0200 Subject: [PATCH 24/24] Fix some easy rubocop issues --- lib/spree/core.rb | 2 +- lib/spree/core/calculated_adjustments.rb | 4 ++-- lib/spree/core/delegate_belongs_to.rb | 2 +- lib/spree/core/permalinks.rb | 4 ++-- lib/spree/i18n/initializer.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/spree/core.rb b/lib/spree/core.rb index a72bd8b5f8..5a7d98dbd2 100644 --- a/lib/spree/core.rb +++ b/lib/spree/core.rb @@ -35,7 +35,7 @@ module Spree # # This method is defined within the core gem on purpose. # Some people may only wish to use the Core part of Spree. - def self.config(&block) + def self.config yield(Spree::Config) end end diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb index e735ec4993..c502c00099 100644 --- a/lib/spree/core/calculated_adjustments.rb +++ b/lib/spree/core/calculated_adjustments.rb @@ -65,15 +65,15 @@ module Spree calculator.compute(calculable) end - private - def self.model_name_without_spree_namespace to_s.tableize.gsub('/', '_').sub('spree_', '') end + private_class_method :model_name_without_spree_namespace def self.spree_calculators Rails.application.config.spree.calculators end + private_class_method :spree_calculators end end end diff --git a/lib/spree/core/delegate_belongs_to.rb b/lib/spree/core/delegate_belongs_to.rb index cf471ebb74..bfbf0b1fc0 100644 --- a/lib/spree/core/delegate_belongs_to.rb +++ b/lib/spree/core/delegate_belongs_to.rb @@ -91,4 +91,4 @@ module DelegateBelongsTo protected :delegator_for end -ActiveRecord::Base.__send__(:include, DelegateBelongsTo) +ActiveRecord::Base.include(DelegateBelongsTo) diff --git a/lib/spree/core/permalinks.rb b/lib/spree/core/permalinks.rb index 6dc1fb9c6a..b9d8a83274 100644 --- a/lib/spree/core/permalinks.rb +++ b/lib/spree/core/permalinks.rb @@ -71,5 +71,5 @@ module Spree end end -ActiveRecord::Base.__send__ :include, Spree::Core::Permalinks -ActiveRecord::Relation.__send__ :include, Spree::Core::Permalinks +ActiveRecord::Base.include(Spree::Core::Permalinks) +ActiveRecord::Relation.include(Spree::Core::Permalinks) diff --git a/lib/spree/i18n/initializer.rb b/lib/spree/i18n/initializer.rb index 85757681fe..64b649f708 100644 --- a/lib/spree/i18n/initializer.rb +++ b/lib/spree/i18n/initializer.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -Spree::BaseController.__send__(:include, Spree::ViewContext) +Spree::BaseController.include(Spree::ViewContext)