diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index 3983cf3fda..07334812f8 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 @@ -82,8 +82,17 @@ module Spree def preference_field_options(options) field_options = case options[:type] when :integer - { size: 10, - class: 'input_integer' } + { + size: 10, + class: 'input_integer', + step: 1, + } + when :decimal + { + size: 10, + class: 'input_integer', + step: :any, # Allow any number of decimal places + } when :boolean {} when :string diff --git a/app/models/concerns/nested_calculator_validation.rb b/app/models/concerns/nested_calculator_validation.rb deleted file mode 100644 index 418996b82e..0000000000 --- a/app/models/concerns/nested_calculator_validation.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module NestedCalculatorValidation - extend ActiveSupport::Concern - - included do - validates_associated :calculator - after_validation :remove_calculator_error - end - - def remove_calculator_error - # Remove generic calculator message, in favour of messages provided by validates_associated - errors.delete(:calculator) if errors.key?(:calculator) - end -end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 90d227b840..bbb9d4053d 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -17,7 +17,7 @@ module Spree validates :name, presence: true validate :distributor_validation - include NestedCalculatorValidation + validates_associated :calculator after_initialize :init diff --git a/spec/models/concerns/nested_calculator_validation_spec.rb b/spec/models/concerns/nested_calculator_validation_spec.rb deleted file mode 100644 index 0488abf0d5..0000000000 --- a/spec/models/concerns/nested_calculator_validation_spec.rb +++ /dev/null @@ -1,86 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -shared_examples "a parent model that has a Calculator" do |parent_name| - context "when the associated Calculator is valid" do - let(:valid_parent) do - build(parent_name, calculator: Calculator::FlatRate.new(preferred_amount: 10)) - end - - it "is valid" do - expect(valid_parent).to be_valid - end - end - - context "when the associated Calculator is invalid" do - let(:invalid_parent) do - build(parent_name, calculator: Calculator::FlexiRate.new(preferred_first_item: "invalid")) - end - - before do - invalid_parent.valid? - end - - it "is invalid" do - expect(invalid_parent).not_to be_valid - end - - it "adds custom error messages to base" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages).to include(/^Calculator First Item: Invalid input./) - end - - it "has the correct number of errors messages" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages.count).to eq 1 - end - - it "does not include the generic Calculator error message" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages).not_to include(/^Calculator is invalid$/) - end - - it "does not include error message that begins with 'Calculator preferred'" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages).not_to include(/^Calculator preferred/) - end - - context "with multiple errors" do - let(:invalid_parent) do - build(parent_name, - calculator: Calculator::FlexiRate.new( - preferred_first_item: "invalid", - preferred_additional_item: "invalid", - )) - end - - it "adds custom error messages to base" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages[0]).to match(/^Calculator First Item: Invalid input./) - expect(error_messages[1]).to match(/^Calculator Additional Item Cost: Invalid input./) - end - - it "has the correct number of errors messages" do - error_messages = invalid_parent.errors.full_messages - expect(error_messages.count).to eq 2 - end - end - end - - context "when number localization is enabled and the associated Calculator is invalid" do - let(:localized_parent) do - build(parent_name, calculator: Calculator::FlatRate.new(preferred_amount: "invalid")) - end - - before do - allow(Spree::Config).to receive(:enable_localized_number?).and_return true - localized_parent.valid? - end - - it "adds custom error messages to base" do - error_messages = localized_parent.errors.full_messages - expect(error_messages).to include(/^Calculator Amount: has an invalid format./) - end - end -end diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index 9dd4ee1f4d..32dba2f627 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -1,14 +1,11 @@ # frozen_string_literal: true require 'spec_helper' -require './spec/models/concerns/nested_calculator_validation_spec' class Spree::Gateway::Test < Spree::Gateway end describe Spree::PaymentMethod do - it_behaves_like "a parent model that has a Calculator", :payment_method - describe "#available" do let(:enterprise) { create(:enterprise) } diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index 8dc1bac5e0..b8044e3012 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -395,6 +395,7 @@ describe ' 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 @@ -404,17 +405,12 @@ describe ' end it "displays error details" do - expect(page).to have_content("3 errors") + 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") - expect(page).to have_content("Calculator Amount: Invalid input. Please use only numbers.") # Highlighting invalid fields - within '.calculator-settings .field .field_with_errors' do - expect(page).to have_field "Amount" - end - within '.field_with_errors:has(#payment_method_name)' do expect(page).to have_field "payment_method_name" end @@ -425,7 +421,6 @@ describe ' end it "keeps information that was just edited, even after multiple unsuccessful updates" do - expect(page).to have_field "Amount", with: "invalid" 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}" @@ -435,7 +430,6 @@ describe ' click_button 'Update' expect(page).to have_field "payment_method_name", with: 'Test name' - expect(page).to have_field "Amount", with: "invalid string" 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 @@ -503,7 +497,6 @@ describe ' click_button 'Update' expect(page).to have_field 'payment_method_name', with: 'Test name' - expect(page).to have_field 'Flat Percent', with: 'invalid string' 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