diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index e467e8356c..077cddbca8 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -7,7 +7,6 @@ module Spree before_action :load_data before_action :validate_payment_method_provider, only: [:create] before_action :load_hubs, only: [:new, :edit, :update] - before_action :validate_calculator_preferred_value, only: [:update] respond_to :html @@ -17,6 +16,10 @@ module Spree @payment_method = payment_method_class.constantize.new(base_params) @object = @payment_method + if !base_params["calculator_type"] && gateway_params["calculator_type"] + @payment_method.calculator_type = gateway_params["calculator_type"] + end + load_hubs if @payment_method.save @@ -44,6 +47,7 @@ module Spree redirect_to spree.edit_admin_payment_method_path(@payment_method) else respond_with(@payment_method) + clear_preference_cache end end @@ -182,33 +186,23 @@ module Spree end end + # Ensure the calculator to be updated is the correct type + if params_for_update["calculator_type"] && params_for_update["calculator_attributes"] + add_type_to_calculator_attributes(params_for_update) + end + params_for_update end end - def validate_calculator_preferred_value - return if calculator_preferred_values.all? do |value| - preferred_value_from_params = gateway_params.dig(:calculator_attributes, value) - preferred_value_from_params.nil? || Float(preferred_value_from_params, - exception: false) + def clear_preference_cache + @payment_method.calculator.preferences.each_key do |key| + Rails.cache.delete(@payment_method.calculator.preference_cache_key(key)) end - - flash[:error] = I18n.t(:calculator_preferred_value_error) - redirect_to spree.edit_admin_payment_method_path(@payment_method) end - def calculator_preferred_values - [ - :preferred_amount, - :preferred_flat_percent, - :preferred_flat_percent, - :preferred_first_item, - :preferred_additional_item, - :preferred_max_items, - :preferred_normal_amount, - :preferred_discount_amount, - :preferred_minimal_amount - ] + def add_type_to_calculator_attributes(hash) + hash["calculator_attributes"]["type"] = hash["calculator_type"] end end end diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index 3983cf3fda..d527b04d1d 100644 --- a/app/helpers/spree/admin/base_helper.rb +++ b/app/helpers/spree/admin/base_helper.rb @@ -29,8 +29,8 @@ module Spree def preference_field_tag(name, value, options) case options[:type] - when :integer - text_field_tag(name, value, preference_field_options(options)) + when :integer, :decimal + number_field_tag(name, value, preference_field_options(options)) when :boolean hidden_field_tag(name, 0) + check_box_tag(name, 1, value, preference_field_options(options)) @@ -49,8 +49,8 @@ module Spree def preference_field_for(form, field, options, object) case options[:type] - when :integer - form.text_field(field, preference_field_options(options)) + when :integer, :decimal + form.number_field(field, preference_field_options(options)) when :boolean form.check_box(field, preference_field_options(options)) when :string @@ -80,26 +80,24 @@ module Spree end def preference_field_options(options) - field_options = case options[:type] - when :integer - { size: 10, - class: 'input_integer' } - when :boolean - {} - when :string - { size: 10, - class: 'input_string fullwidth' } - when :password - { size: 10, - class: 'password_string fullwidth' } - when :text - { rows: 15, - cols: 85, - class: 'fullwidth' } - else - { size: 10, - class: 'input_string fullwidth' } - end + field_options = + case options[:type] + when :integer + { size: 10, class: 'input_integer', step: 1 } + when :decimal + # Allow any number of decimal places + { size: 10, class: 'input_integer', step: :any } + when :boolean + {} + when :string + { size: 10, class: 'input_string fullwidth' } + when :password + { size: 10, class: 'password_string fullwidth' } + when :text + { rows: 15, cols: 85, class: 'fullwidth' } + else + { size: 10, class: 'input_string fullwidth' } + end field_options.merge!( readonly: options[:readonly], diff --git a/app/models/calculator/flat_percent_item_total.rb b/app/models/calculator/flat_percent_item_total.rb index d4c2067444..f0d7a330e5 100644 --- a/app/models/calculator/flat_percent_item_total.rb +++ b/app/models/calculator/flat_percent_item_total.rb @@ -1,14 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlatPercentItemTotal < Spree::Calculator - extend Spree::LocalizedNumber - preference :flat_percent, :decimal, default: 0 - localize_number :preferred_flat_percent + validates :preferred_flat_percent, + numericality: true def self.description Spree.t(:flat_percent) diff --git a/app/models/calculator/flat_percent_per_item.rb b/app/models/calculator/flat_percent_per_item.rb index f9e197c594..e078340d7b 100644 --- a/app/models/calculator/flat_percent_per_item.rb +++ b/app/models/calculator/flat_percent_per_item.rb @@ -1,18 +1,15 @@ # frozen_string_literal: true -require 'spree/localized_number' - class Calculator::FlatPercentPerItem < Spree::Calculator # Spree's FlatPercentItemTotal calculator sums all amounts, and then calculates a percentage # on them. # In the cart, we display line item individual amounts rounded, so to have consistent # calculations we do the same internally. Here, we round adjustments at the individual # item level first, then multiply by the item quantity. - extend Spree::LocalizedNumber - preference :flat_percent, :decimal, default: 0 - localize_number :preferred_flat_percent + validates :preferred_flat_percent, + numericality: true def self.description I18n.t(:flat_percent_per_item) diff --git a/app/models/calculator/flat_rate.rb b/app/models/calculator/flat_rate.rb index a8ab771b98..a130b88c17 100644 --- a/app/models/calculator/flat_rate.rb +++ b/app/models/calculator/flat_rate.rb @@ -1,14 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlatRate < Spree::Calculator - extend Spree::LocalizedNumber - preference :amount, :decimal, default: 0 - localize_number :preferred_amount + validates :preferred_amount, + numericality: true def self.description I18n.t(:flat_rate_per_order) diff --git a/app/models/calculator/flexi_rate.rb b/app/models/calculator/flexi_rate.rb index bb6bb1ff9b..1959aec67e 100644 --- a/app/models/calculator/flexi_rate.rb +++ b/app/models/calculator/flexi_rate.rb @@ -1,17 +1,14 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class FlexiRate < Spree::Calculator - extend Spree::LocalizedNumber - preference :first_item, :decimal, default: 0.0 preference :additional_item, :decimal, default: 0.0 preference :max_items, :integer, default: 0 - localize_number :preferred_first_item, - :preferred_additional_item + validates :preferred_first_item, + :preferred_additional_item, + numericality: true def self.description I18n.t(:flexible_rate) diff --git a/app/models/calculator/per_item.rb b/app/models/calculator/per_item.rb index f8cce8e7a7..0df18ae3d4 100644 --- a/app/models/calculator/per_item.rb +++ b/app/models/calculator/per_item.rb @@ -1,14 +1,11 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class PerItem < Spree::Calculator - extend Spree::LocalizedNumber - preference :amount, :decimal, default: 0 - localize_number :preferred_amount + validates :preferred_amount, + numericality: true def self.description I18n.t(:flat_rate_per_item) diff --git a/app/models/calculator/price_sack.rb b/app/models/calculator/price_sack.rb index 910b663b1d..217928581f 100644 --- a/app/models/calculator/price_sack.rb +++ b/app/models/calculator/price_sack.rb @@ -1,18 +1,15 @@ # frozen_string_literal: false -require 'spree/localized_number' - module Calculator class PriceSack < Spree::Calculator - extend Spree::LocalizedNumber - preference :minimal_amount, :decimal, default: 0 preference :normal_amount, :decimal, default: 0 preference :discount_amount, :decimal, default: 0 - localize_number :preferred_minimal_amount, - :preferred_normal_amount, - :preferred_discount_amount + validates :preferred_minimal_amount, + :preferred_normal_amount, + :preferred_discount_amount, + numericality: true def self.description I18n.t(:price_sack) diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index f90a526f7c..bbb9d4053d 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -17,6 +17,7 @@ module Spree validates :name, presence: true validate :distributor_validation + validates_associated :calculator after_initialize :init @@ -40,9 +41,8 @@ module Spree where(id: non_unique_matches.map(&:id)) } - scope :for_distributor, lambda { |distributor| - joins(:distributors). - where('enterprises.id = ?', distributor) + scope :for_distributor, ->(distributor) { + joins(:distributors).where('enterprises.id = ?', distributor) } scope :for_subscriptions, -> { where(type: Subscription::ALLOWED_PAYMENT_METHOD_TYPES) } diff --git a/app/models/spree/preference.rb b/app/models/spree/preference.rb index dfef2a146d..ea2c63688c 100644 --- a/app/models/spree/preference.rb +++ b/app/models/spree/preference.rb @@ -22,7 +22,8 @@ module Spree when :password self[:value].to_s when :decimal - BigDecimal(self[:value].to_s).round(2, BigDecimal::ROUND_HALF_UP) + BigDecimal(self[:value].to_s, exception: false)&.round(2, BigDecimal::ROUND_HALF_UP) || + self[:value] when :integer self[:value].to_i when :boolean diff --git a/app/models/spree/preferences/preferable.rb b/app/models/spree/preferences/preferable.rb index 2c0f61b2e2..af775ab37d 100644 --- a/app/models/spree/preferences/preferable.rb +++ b/app/models/spree/preferences/preferable.rb @@ -117,7 +117,7 @@ module Spree value.to_s when :decimal value = 0 if value.blank? - BigDecimal(value.to_s).round(2, BigDecimal::ROUND_HALF_UP) + BigDecimal(value.to_s, exception: false)&.round(2, BigDecimal::ROUND_HALF_UP) || value when :integer value.to_i when :boolean diff --git a/app/views/spree/admin/payment_methods/_providers.html.haml b/app/views/spree/admin/payment_methods/_providers.html.haml index aa1fce7fe8..b25a7b09c5 100644 --- a/app/views/spree/admin/payment_methods/_providers.html.haml +++ b/app/views/spree/admin/payment_methods/_providers.html.haml @@ -3,6 +3,6 @@ .alpha.four.columns = label :payment_method, :type, t('.provider') .omega.twelve.columns - = collection_select(:payment_method, :type, @providers, :to_s, :clean_name, (!@object.persisted? ? { :selected => "Spree::PaymentMethod::Check"} : {}), { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) + = collection_select(:payment_method, :type, @providers, :to_s, :clean_name, {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) %div{"ng-include" => "include_html" } diff --git a/config/locales/en.yml b/config/locales/en.yml index 42e86b3243..c8a9ab96fc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -79,6 +79,17 @@ en: orders_close_at: Close date variant_override: count_on_hand: "On Hand" + spree/payment_method/calculator: + preferred_flat_percent: "Calculator Flat Percent:" + preferred_amount: "Calculator Amount:" + preferred_first_item: "Calculator First Item:" + preferred_additional_item: "Calculator Additional Item Cost:" + preferred_max_items: "Calculator Max Items:" + preferred_minimal_amount: "Calculator Minimal Amount:" + preferred_normal_amount: "Calculator Normal Amount:" + preferred_discount_amount: "Calculator Discount Amount:" + preferred_unit_from_list: "Calculator Unit From List:" + preferred_per_unit: "Calculator Per Unit:" errors: models: spree/user: diff --git a/lib/spree/localized_number.rb b/lib/spree/localized_number.rb index 77270d49ee..a1ba18accd 100644 --- a/lib/spree/localized_number.rb +++ b/lib/spree/localized_number.rb @@ -21,7 +21,7 @@ module Spree elsif Spree::Config.enable_localized_number? @invalid_localized_number ||= [] @invalid_localized_number << attribute - number = nil + number = nil unless is_a?(Spree::Calculator) end if has_attribute?(attribute) # In this case it's a regular AR attribute with standard setters @@ -44,7 +44,8 @@ module Spree def self.valid_localizable_number?(number) return true unless number.is_a?(String) || number.respond_to?(:to_d) - return false if number.to_s =~ /[.,]\d{2}[.,]/ + # Invalid if only two digits between dividers, or if any non-number characters + return false if number.to_s =~ /[.,]\d{2}[.,]/ || number.to_s =~ /[^0-9,.]+/ true end diff --git a/lib/tasks/sample_data/payment_method_factory.rb b/lib/tasks/sample_data/payment_method_factory.rb index 4febf09fa4..ba545b2d11 100644 --- a/lib/tasks/sample_data/payment_method_factory.rb +++ b/lib/tasks/sample_data/payment_method_factory.rb @@ -53,8 +53,8 @@ module SampleData environment: Rails.env, distributor_ids: [enterprise.id] ) - calculator.calculable = payment_method - calculator.save! + payment_method.calculator = calculator + payment_method.save! end end end diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index cc4dc35e65..19206bd0b5 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -142,6 +142,9 @@ RSpec.configure do |config| config.infer_spec_type_from_file_location! + # You can use `rspec -n` to run only failed specs. + config.example_status_persistence_file_path = "tmp/rspec-status.txt" + # Helpers config.include FactoryBot::Syntax::Methods config.include JsonSpec::Helpers diff --git a/spec/lib/spree/localized_number_spec.rb b/spec/lib/spree/localized_number_spec.rb index b13f8ddf6e..bafe0c822d 100644 --- a/spec/lib/spree/localized_number_spec.rb +++ b/spec/lib/spree/localized_number_spec.rb @@ -59,5 +59,11 @@ describe Spree::LocalizedNumber do expect(described_class.valid_localizable_number?(1599.99)).to eql true end end + + context "with letters" do + it "returns false" do + expect(described_class.valid_localizable_number?('invalid')).to eql false + end + end end end diff --git a/spec/models/calculator/flat_percent_item_total_spec.rb b/spec/models/calculator/flat_percent_item_total_spec.rb index 7d34211259..a0c57c6266 100644 --- a/spec/models/calculator/flat_percent_item_total_spec.rb +++ b/spec/models/calculator/flat_percent_item_total_spec.rb @@ -8,14 +8,12 @@ describe Calculator::FlatPercentItemTotal do before { allow(calculator).to receive_messages preferred_flat_percent: 10 } + it { is_expected.to validate_numericality_of(:preferred_flat_percent) } + it "computes amount correctly for a single line item" do expect(calculator.compute(line_item)).to eq(1.0) end - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_flat_percent] - end - it "computes amount correctly for a given OrderManagement::Stock::Package" do order = double(:order, line_items: [line_item] ) package = double(:package, order: order) diff --git a/spec/models/calculator/flat_percent_per_item_spec.rb b/spec/models/calculator/flat_percent_per_item_spec.rb index 6f16a9fe6a..ac873a0ba7 100644 --- a/spec/models/calculator/flat_percent_per_item_spec.rb +++ b/spec/models/calculator/flat_percent_per_item_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' describe Calculator::FlatPercentPerItem do let(:calculator) { Calculator::FlatPercentPerItem.new preferred_flat_percent: 20 } + it { is_expected.to validate_numericality_of(:preferred_flat_percent) } + it "calculates for a simple line item" do line_item = Spree::LineItem.new price: 50, quantity: 2 expect(calculator.compute(line_item)).to eq 20 @@ -14,8 +16,4 @@ describe Calculator::FlatPercentPerItem do line_item = Spree::LineItem.new price: 0.86, quantity: 8 expect(calculator.compute(line_item)).to eq 1.36 end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_flat_percent] - end end diff --git a/spec/models/calculator/flat_rate_spec.rb b/spec/models/calculator/flat_rate_spec.rb index cb0cce62d2..92eaa864d0 100644 --- a/spec/models/calculator/flat_rate_spec.rb +++ b/spec/models/calculator/flat_rate_spec.rb @@ -7,7 +7,5 @@ describe Calculator::FlatRate do before { allow(calculator).to receive_messages preferred_amount: 10 } - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_amount] - end + it { is_expected.to validate_numericality_of(:preferred_amount) } end diff --git a/spec/models/calculator/flexi_rate_spec.rb b/spec/models/calculator/flexi_rate_spec.rb index 81bbc95ee6..e5a777a412 100644 --- a/spec/models/calculator/flexi_rate_spec.rb +++ b/spec/models/calculator/flexi_rate_spec.rb @@ -12,6 +12,9 @@ describe Calculator::FlexiRate do ) end + it { is_expected.to validate_numericality_of(:preferred_first_item) } + it { is_expected.to validate_numericality_of(:preferred_additional_item) } + context 'when nb of items ordered is above preferred max' do let(:quantity) { 4.0 } @@ -32,9 +35,4 @@ describe Calculator::FlexiRate do Calculator::FlexiRate.new(preferred_first_item: 1, preferred_additional_item: 1, preferred_max_items: 1) end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", - [:preferred_first_item, :preferred_additional_item] - end end diff --git a/spec/models/calculator/per_item_spec.rb b/spec/models/calculator/per_item_spec.rb index cde6cae8f5..914ca33dfa 100644 --- a/spec/models/calculator/per_item_spec.rb +++ b/spec/models/calculator/per_item_spec.rb @@ -7,12 +7,10 @@ describe Calculator::PerItem do let(:shipping_calculable) { double(:calculable) } let(:line_item) { build_stubbed(:line_item, quantity: 5) } + it { is_expected.to validate_numericality_of(:preferred_amount) } + it "correctly calculates on a single line item object" do allow(calculator).to receive_messages(calculable: shipping_calculable) expect(calculator.compute(line_item).to_f).to eq(50) # 5 x 10 end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_amount] - end end diff --git a/spec/models/calculator/price_sack_spec.rb b/spec/models/calculator/price_sack_spec.rb index 0034b20963..771b8428ec 100644 --- a/spec/models/calculator/price_sack_spec.rb +++ b/spec/models/calculator/price_sack_spec.rb @@ -12,6 +12,10 @@ describe Calculator::PriceSack do end let(:line_item) { build_stubbed(:line_item, price: price, quantity: 2) } + it { is_expected.to validate_numericality_of(:preferred_minimal_amount) } + it { is_expected.to validate_numericality_of(:preferred_normal_amount) } + it { is_expected.to validate_numericality_of(:preferred_discount_amount) } + context 'when the order amount is below preferred minimal' do let(:price) { 2 } @@ -75,10 +79,4 @@ describe Calculator::PriceSack do end end end - - context "extends LocalizedNumber" do - it_behaves_like "a model using the LocalizedNumber module", - [:preferred_minimal_amount, :preferred_normal_amount, - :preferred_discount_amount] - end end diff --git a/spec/models/spree/preference_spec.rb b/spec/models/spree/preference_spec.rb index 780434f768..0eb32f9837 100644 --- a/spec/models/spree/preference_spec.rb +++ b/spec/models/spree/preference_spec.rb @@ -11,7 +11,7 @@ describe Spree::Preference do expect(@preference).to be_valid end - describe "type coversion for values" do + describe "type conversion for values" do def round_trip_preference(key, value, value_type) p = Spree::Preference.new p.value = value @@ -58,6 +58,15 @@ describe Spree::Preference do expect(pref.value_type).to eq value_type.to_s end + it "incovertible :decimal" do + value_type = :decimal + value = "invalid" + key = "decimal_key" + pref = round_trip_preference(key, value, value_type) + expect(pref.value).to eq value + expect(pref.value_type).to eq value_type.to_s + end + it ":string" do value_type = :string value = "This is a string" diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index 8b7edba9f0..e95e069de0 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -165,6 +165,13 @@ describe Spree::Preferences::Preferable do @a.set_preference(:if_decimal, '') expect(@a.preferences[:if_decimal]).to eq 0.0 end + + context "when the value cannot be converted to BigDecimal" do + it "returns the original value" do + @a.set_preference(:if_decimal, "invalid") + expect(@a.preferences[:if_decimal]).to eq "invalid" + end + end end context "converts boolean preferences to boolean values" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index da264f58e3..5422778d4a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,7 +12,4 @@ RSpec.configure do |config| # Fix encoding issue in Rails 5.0; allows passing empty arrays or hashes as params. config.before(:each, type: :controller) { @request.env["CONTENT_TYPE"] = 'application/json' } - - # You can use `rspec -n` to run only failed specs. - config.example_status_persistence_file_path = "tmp/rspec-status.txt" end diff --git a/spec/support/localized_number_helper.rb b/spec/support/localized_number_helper.rb index 62ed435de0..75e8803915 100644 --- a/spec/support/localized_number_helper.rb +++ b/spec/support/localized_number_helper.rb @@ -21,3 +21,24 @@ shared_examples "a model using the LocalizedNumber module" do |attributes| end end end + +shared_examples "a Spree Calculator model using the LocalizedNumber module" do |attributes| + before do + allow(Spree::Config).to receive(:enable_localized_number?).and_return true + end + + attributes.each do |attribute| + setter = "#{attribute}=" + + it do + should_not validate_numericality_of(attribute). + with_message(I18n.t('activerecord.errors.messages.calculator_preferred_value_error')) + end + + it "does not modify the attribute value when it is invalid" do + subject.send(setter, 'invalid') + subject.valid? + expect(subject.send(attribute)).to eq 'invalid' + end + end +end diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index c6937a0327..e6de2bca7e 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -112,6 +112,45 @@ describe ' expect(page).to have_field "payment_method_distributor_ids_#{@distributors[2].id}", checked: false end + + it "retains and displays data that was just submitted" do + login_as_admin + visit spree.edit_admin_general_settings_path + click_link 'Payment Methods' + click_link 'New Payment Method' + + fill_in "payment_method_name", with: 'Cheque payment method' + fill_in "payment_method_description", with: "Payment description" + select2_select "PayPal Express", from: "payment_method_type" + select2_select "Flat Percent", from: 'calc_type' + + click_button 'Create' + + expect(page).to have_field "payment_method_name", with: "Cheque payment method" + expect(page).to have_field "payment_method_description", with: "Payment description" + expect(page).to have_select("payment_method_type", selected: "PayPal Express") + expect(page).to have_select("calc_type", selected: "Flat Percent") + + select2_select "Price Sack", from: 'calc_type' + + click_button 'Create' + + expect(page).to have_select("calc_type", selected: "Price Sack") + expect(page).to have_field "payment_method_name", with: "Cheque payment method" + expect(page).to have_field "payment_method_description", with: "Payment description" + expect(page).to have_select("payment_method_type", selected: "PayPal Express") + + check "payment_method_distributor_ids_#{@distributors[0].id}" + + click_button 'Create' + + expect(page).to have_field "payment_method_distributor_ids_#{@distributors[0].id}", + checked: true + expect(page).to have_select("calc_type", selected: "Price Sack") + expect(page).to have_field "payment_method_name", with: "Cheque payment method" + expect(page).to have_field "payment_method_description", with: "Payment description" + expect(page).to have_select("payment_method_type", selected: "PayPal Express") + end end it "updating a payment method" do @@ -345,4 +384,134 @@ describe ' end end end + + context "when updating a payment method with invalid data" do + let(:payment_method) { create(:payment_method, :flat_rate, amount: 10) } + + # Persist preferences to test that when preferred values are not found in the cache, + # they are fetched from the database and displayed correctly + before do + Spree::Preferences::Store.instance.persistence = true + login_as_admin + visit spree.edit_admin_payment_method_path payment_method + fill_in "payment_method_name", with: "" + fill_in "payment_method_description", with: "Edited description" + uncheck "payment_method_distributor_ids_#{@distributors[0].id}" + # Although text is not permitted by input[type=number], let's see what happens + fill_in "Amount", with: 'invalid' + click_button 'Update' + end + + after do + Spree::Preferences::Store.instance.persistence = false + end + + it "displays error details" do + expect(page).to have_content("2 errors") + + expect(page).to have_content("Name can't be blank") + expect(page).to have_content("At least one hub must be selected") + + # Highlighting invalid fields + within '.field_with_errors:has(#payment_method_name)' do + expect(page).to have_field "payment_method_name" + end + + within '#hubs' do + expect(page).to have_selector(".red") + end + end + + it "keeps information that was just edited, even after multiple unsuccessful updates" do + expect(page).to have_field "payment_method_name", with: '' + expect(page).to have_field "payment_method_description", with: "Edited description" + expect(page).to have_unchecked_field "payment_method_distributor_ids_#{@distributors[0].id}" + + fill_in "payment_method_name", with: "Test name" + fill_in "Amount", with: 'invalid string' + click_button 'Update' + + expect(page).to have_field "payment_method_name", with: 'Test name' + expect(page).to have_field "payment_method_description", with: "Edited description" + expect(page).to have_unchecked_field "payment_method_distributor_ids_#{@distributors[0].id}" + end + + it 'displays data fetched from the database after navigating away from the page' do + click_link 'Back To Payment Methods List' + click_link href: /#{spree.edit_admin_payment_method_path(payment_method)}/ + + expect(page).to have_field 'Amount', with: '10.0' + expect(page).not_to have_field 'payment_method_name', with: '' + expect(page).not_to have_field 'payment_method_description', with: 'Edited description' + expect(page).to have_checked_field "payment_method_distributor_ids_#{@distributors[0].id}" + end + end + + context 'when updating payment method with invalid data and changing calculator type' do + let(:payment_method) { create(:payment_method, :flat_rate, amount: 10) } + + # Persist preferences to test that when preferred values are not found in the cache, + # they are fetched from the database and displayed correctly + before do + Spree::Preferences::Store.instance.persistence = true + login_as_admin + visit spree.edit_admin_payment_method_path payment_method + fill_in 'payment_method_name', with: '' + fill_in 'payment_method_description', with: 'Edited description' + uncheck "payment_method_distributor_ids_#{@distributors[0].id}" + select2_select 'Flat Percent', from: 'calc_type' + click_button 'Update' + end + + after do + Spree::Preferences::Store.instance.persistence = false + end + + it 'displays the number of errors' do + expect(page).to have_content('2 errors') + end + + it 'displays error messages' do + expect(page).to have_content("Name can't be blank") + expect(page).to have_content('At least one hub must be selected') + end + + it 'displays the new calculator fields' do + expect(page).to have_field 'Flat Percent' + end + + it 'highlights invalid fields' do + within '.field_with_errors:has(#payment_method_name)' do + expect(page).to have_field 'payment_method_name' + end + + within '#hubs' do + expect(page).to have_selector('.red') + end + end + + it 'keeps information that was just edited, even after multiple unsuccessful updates' do + expect(page).to have_field 'payment_method_name', with: '' + expect(page).to have_field 'payment_method_description', with: 'Edited description' + expect(page).to have_unchecked_field "payment_method_distributor_ids_#{@distributors[0].id}" + + fill_in 'payment_method_name', with: 'Test name' + fill_in 'Flat Percent', with: 'invalid string' + click_button 'Update' + + expect(page).to have_field 'payment_method_name', with: 'Test name' + expect(page).to have_field 'payment_method_description', with: 'Edited description' + expect(page).to have_unchecked_field "payment_method_distributor_ids_#{@distributors[0].id}" + end + + it 'displays data fetched from the database after navigating away from the page' do + click_link 'Back To Payment Methods List' + click_link href: /#{spree.edit_admin_payment_method_path(payment_method)}/ + + expect(page).to have_field 'Amount', with: '10.0' + expect(page).not_to have_field 'payment_method_name', with: '' + expect(page).not_to have_field 'payment_method_description', with: 'Edited description' + expect(page).to have_checked_field "payment_method_distributor_ids_#{@distributors[0].id}" + end + end end