raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-15 23:57:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use browser validation for calculator values

Browse Source 
They're all numbers, so we can reliably validate in the browser, removing the need for additional validation logic.
They will still be validated server-side, and in the unlikely even that there is an error, the generic 'Calculator is invalid' message will appear, with the relevant fields highlighted red. I think that's fine.
This commit is contained in:
David Cook
2023-04-05 15:16:12 +10:00
parent f0110d20a2
commit 0f3e2ae572
6 changed files with 18 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
app/helpers/spree/admin/base_helper.rb
View File

@@ -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

15
app/models/concerns/nested_calculator_validation.rb
View File

@@ -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

2
app/models/spree/payment_method.rb
View File

@@ -17,7 +17,7 @@ module Spree
validates :name, presence: true
validate :distributor_validation
include NestedCalculatorValidation
validates_associated :calculator
after_initialize :init

86
spec/models/concerns/nested_calculator_validation_spec.rb
View File

@@ -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

3
spec/models/spree/payment_method_spec.rb
View File

@@ -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) }

11
spec/system/admin/payment_method_spec.rb
View File

@@ -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