From e8eadcbf398f10408ffac823486c90d23c50a69a Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sun, 6 Sep 2020 09:03:06 -0700 Subject: [PATCH 01/24] Add a per-pound calculator and a spec for it --- .../admin/shipping_methods_controller.rb | 3 +- app/models/calculator/weight_lb.rb | 75 +++++++++++++++++++ config/application.rb | 6 +- config/locales/en.yml | 1 + .../admin/shipping_methods_controller_spec.rb | 9 +++ spec/factories/calculator_factory.rb | 5 ++ 6 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 app/models/calculator/weight_lb.rb diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index 5fa427772e..84b9619589 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -88,7 +88,8 @@ module Spree :require_ship_address, :tag_list, :calculator_type, distributor_ids: [], calculator_attributes: [ - :id, :preferred_currency, :preferred_amount, :preferred_per_kg, :preferred_flat_percent, + :id, :preferred_currency, :preferred_amount, :preferred_per_kg, + :preferred_per_lb, :preferred_flat_percent, :preferred_first_item, :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount ] diff --git a/app/models/calculator/weight_lb.rb b/app/models/calculator/weight_lb.rb new file mode 100644 index 0000000000..2af8af9778 --- /dev/null +++ b/app/models/calculator/weight_lb.rb @@ -0,0 +1,75 @@ +require 'spree/localized_number' + +module Calculator + class WeightLb < Spree::Calculator + extend Spree::LocalizedNumber + preference :per_lb, :decimal, default: 0.0 + localize_number :preferred_per_lb + + def self.description + I18n.t('spree.weight_lb') + end + + def compute(object) + line_items = line_items_for object + (total_weight(line_items) * preferred_per_lb).round(2) + end + + private + + def total_weight(line_items) + line_items.sum do |line_item| + line_item_weight(line_item) + end + end + + def line_item_weight(line_item) + if final_weight_volume_present?(line_item) + weight_per_final_weight_volume(line_item) + else + weight_per_variant(line_item) * line_item.quantity + end + end + + def weight_per_variant(line_item) + if variant_unit(line_item) == 'weight' + # The calculator price is per_lb so we need to convert unit_value to lb + convert_g_to_lb(line_item.variant.andand.unit_value) + else + line_item.variant.andand.weight || 0 + end + end + + def weight_per_final_weight_volume(line_item) + if variant_unit(line_item) == 'weight' + # The calculator price is per_lb so we need to convert final_weight_volume to lb + convert_g_to_lb(line_item.final_weight_volume) + else + weight_per_variant(line_item) * quantity_implied_in_final_weight_volume(line_item) + end + end + + # Example: 2 (line_item.quantity) wine glasses of 125mL (line_item.variant.unit_value) + # Customer ends up getting 350mL (line_item.final_weight_volume) of wine + # that represent 2.8 (quantity_implied_in_final_weight_volume) glasses of wine + def quantity_implied_in_final_weight_volume(line_item) + return line_item.quantity if line_item.variant.unit_value.to_f.zero? + + (1.0 * line_item.final_weight_volume / line_item.variant.unit_value).round(3) + end + + def final_weight_volume_present?(line_item) + line_item.respond_to?(:final_weight_volume) && line_item.final_weight_volume.present? + end + + def variant_unit(line_item) + line_item.variant.product.andand.variant_unit + end + + def convert_g_to_lb(value) + return 0 unless value + + value / 453.6 + end + end +end diff --git a/config/application.rb b/config/application.rb index 3d061b97be..83d92c60ba 100644 --- a/config/application.rb +++ b/config/application.rb @@ -54,7 +54,8 @@ module Openfoodnetwork Calculator::FlexiRate, Calculator::PerItem, Calculator::PriceSack, - Calculator::Weight + Calculator::Weight, + Calculator::WeightLb ] app.config.spree.calculators.add_class('enterprise_fees') @@ -64,7 +65,8 @@ module Openfoodnetwork Calculator::FlexiRate, Calculator::PerItem, Calculator::PriceSack, - Calculator::Weight + Calculator::Weight, + Calculator::WeightLb ] app.config.spree.calculators.add_class('payment_methods') diff --git a/config/locales/en.yml b/config/locales/en.yml index 6cbd131c09..6ff97734eb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3127,6 +3127,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using inventory: Inventory zipcode: Postcode weight: Weight (per kg) + weight_lb: Weight (per lb) error_user_destroy_with_orders: "Users with completed orders may not be deleted" cannot_create_payment_without_payment_methods: "You cannot create a payment for an order without any payment methods defined." please_define_payment_methods: "Please define some payment methods first." diff --git a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb index f209e87a81..247e5c66e2 100644 --- a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb @@ -38,6 +38,15 @@ describe Spree::Admin::ShippingMethodsController, type: :controller do expect(shipping_method.reload.calculator.preferred_per_kg).to eq 10 end + it "updates preferred_per_lb of a Weight (Lb) calculator" do + shipping_method.calculator = create(:weight_lb_calculator, calculable: shipping_method) + params[:shipping_method][:calculator_attributes][:preferred_per_lb] = 10 + + spree_post :update, params + + expect(shipping_method.reload.calculator.preferred_per_lb).to eq 10 + end + it "updates preferred_flat_percent of a FlatPercentPerItem calculator" do shipping_method.calculator = Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20, diff --git a/spec/factories/calculator_factory.rb b/spec/factories/calculator_factory.rb index 1268a8ccb7..c9ee25a34e 100644 --- a/spec/factories/calculator_factory.rb +++ b/spec/factories/calculator_factory.rb @@ -16,4 +16,9 @@ FactoryBot.define do after(:build) { |c| c.set_preference(:per_kg, 0.5) } after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } end + + factory :weight_lb_calculator, class: Calculator::WeightLb do + after(:build) { |c| c.set_preference(:per_lb, 0.5) } + after(:create) { |c| c.set_preference(:per_lb, 0.5); c.save! } + end end From 5793f0103da2af686915a4cecd0f6e2a95acafc0 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 7 Sep 2020 07:50:59 -0700 Subject: [PATCH 02/24] Revert "Add a per-pound calculator and a spec for it" This reverts commit e8eadcbf398f10408ffac823486c90d23c50a69a. --- .../admin/shipping_methods_controller.rb | 3 +- app/models/calculator/weight_lb.rb | 75 ------------------- config/application.rb | 6 +- config/locales/en.yml | 1 - .../admin/shipping_methods_controller_spec.rb | 9 --- spec/factories/calculator_factory.rb | 5 -- 6 files changed, 3 insertions(+), 96 deletions(-) delete mode 100644 app/models/calculator/weight_lb.rb diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index 84b9619589..5fa427772e 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -88,8 +88,7 @@ module Spree :require_ship_address, :tag_list, :calculator_type, distributor_ids: [], calculator_attributes: [ - :id, :preferred_currency, :preferred_amount, :preferred_per_kg, - :preferred_per_lb, :preferred_flat_percent, + :id, :preferred_currency, :preferred_amount, :preferred_per_kg, :preferred_flat_percent, :preferred_first_item, :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount ] diff --git a/app/models/calculator/weight_lb.rb b/app/models/calculator/weight_lb.rb deleted file mode 100644 index 2af8af9778..0000000000 --- a/app/models/calculator/weight_lb.rb +++ /dev/null @@ -1,75 +0,0 @@ -require 'spree/localized_number' - -module Calculator - class WeightLb < Spree::Calculator - extend Spree::LocalizedNumber - preference :per_lb, :decimal, default: 0.0 - localize_number :preferred_per_lb - - def self.description - I18n.t('spree.weight_lb') - end - - def compute(object) - line_items = line_items_for object - (total_weight(line_items) * preferred_per_lb).round(2) - end - - private - - def total_weight(line_items) - line_items.sum do |line_item| - line_item_weight(line_item) - end - end - - def line_item_weight(line_item) - if final_weight_volume_present?(line_item) - weight_per_final_weight_volume(line_item) - else - weight_per_variant(line_item) * line_item.quantity - end - end - - def weight_per_variant(line_item) - if variant_unit(line_item) == 'weight' - # The calculator price is per_lb so we need to convert unit_value to lb - convert_g_to_lb(line_item.variant.andand.unit_value) - else - line_item.variant.andand.weight || 0 - end - end - - def weight_per_final_weight_volume(line_item) - if variant_unit(line_item) == 'weight' - # The calculator price is per_lb so we need to convert final_weight_volume to lb - convert_g_to_lb(line_item.final_weight_volume) - else - weight_per_variant(line_item) * quantity_implied_in_final_weight_volume(line_item) - end - end - - # Example: 2 (line_item.quantity) wine glasses of 125mL (line_item.variant.unit_value) - # Customer ends up getting 350mL (line_item.final_weight_volume) of wine - # that represent 2.8 (quantity_implied_in_final_weight_volume) glasses of wine - def quantity_implied_in_final_weight_volume(line_item) - return line_item.quantity if line_item.variant.unit_value.to_f.zero? - - (1.0 * line_item.final_weight_volume / line_item.variant.unit_value).round(3) - end - - def final_weight_volume_present?(line_item) - line_item.respond_to?(:final_weight_volume) && line_item.final_weight_volume.present? - end - - def variant_unit(line_item) - line_item.variant.product.andand.variant_unit - end - - def convert_g_to_lb(value) - return 0 unless value - - value / 453.6 - end - end -end diff --git a/config/application.rb b/config/application.rb index 83d92c60ba..3d061b97be 100644 --- a/config/application.rb +++ b/config/application.rb @@ -54,8 +54,7 @@ module Openfoodnetwork Calculator::FlexiRate, Calculator::PerItem, Calculator::PriceSack, - Calculator::Weight, - Calculator::WeightLb + Calculator::Weight ] app.config.spree.calculators.add_class('enterprise_fees') @@ -65,8 +64,7 @@ module Openfoodnetwork Calculator::FlexiRate, Calculator::PerItem, Calculator::PriceSack, - Calculator::Weight, - Calculator::WeightLb + Calculator::Weight ] app.config.spree.calculators.add_class('payment_methods') diff --git a/config/locales/en.yml b/config/locales/en.yml index 6ff97734eb..6cbd131c09 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3127,7 +3127,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using inventory: Inventory zipcode: Postcode weight: Weight (per kg) - weight_lb: Weight (per lb) error_user_destroy_with_orders: "Users with completed orders may not be deleted" cannot_create_payment_without_payment_methods: "You cannot create a payment for an order without any payment methods defined." please_define_payment_methods: "Please define some payment methods first." diff --git a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb index 247e5c66e2..f209e87a81 100644 --- a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb @@ -38,15 +38,6 @@ describe Spree::Admin::ShippingMethodsController, type: :controller do expect(shipping_method.reload.calculator.preferred_per_kg).to eq 10 end - it "updates preferred_per_lb of a Weight (Lb) calculator" do - shipping_method.calculator = create(:weight_lb_calculator, calculable: shipping_method) - params[:shipping_method][:calculator_attributes][:preferred_per_lb] = 10 - - spree_post :update, params - - expect(shipping_method.reload.calculator.preferred_per_lb).to eq 10 - end - it "updates preferred_flat_percent of a FlatPercentPerItem calculator" do shipping_method.calculator = Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20, diff --git a/spec/factories/calculator_factory.rb b/spec/factories/calculator_factory.rb index c9ee25a34e..1268a8ccb7 100644 --- a/spec/factories/calculator_factory.rb +++ b/spec/factories/calculator_factory.rb @@ -16,9 +16,4 @@ FactoryBot.define do after(:build) { |c| c.set_preference(:per_kg, 0.5) } after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } end - - factory :weight_lb_calculator, class: Calculator::WeightLb do - after(:build) { |c| c.set_preference(:per_lb, 0.5) } - after(:create) { |c| c.set_preference(:per_lb, 0.5); c.save! } - end end From adb29a9c8f59be16a37e9e26058a55fc2545e2d5 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 7 Sep 2020 08:41:48 -0700 Subject: [PATCH 03/24] add preferred_unit to weight shipping calculator --- .../admin/shipping_methods_controller.rb | 2 +- app/models/calculator/weight.rb | 27 ++++++++++++------- config/locales/en.yml | 2 +- .../admin/shipping_methods_controller_spec.rb | 15 ++++++++--- spec/factories/calculator_factory.rb | 11 ++++++-- spec/features/consumer/shopping/cart_spec.rb | 2 +- spec/models/calculator/weight_spec.rb | 20 +++++++++----- 7 files changed, 55 insertions(+), 24 deletions(-) diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index 5fa427772e..b68dca98e3 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -88,7 +88,7 @@ module Spree :require_ship_address, :tag_list, :calculator_type, distributor_ids: [], calculator_attributes: [ - :id, :preferred_currency, :preferred_amount, :preferred_per_kg, :preferred_flat_percent, + :id, :preferred_currency, :preferred_amount, :preferred_unit, :preferred_per_unit, :preferred_flat_percent, :preferred_first_item, :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount ] diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index bb1f70c030..abdf626d05 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -3,8 +3,9 @@ require 'spree/localized_number' module Calculator class Weight < Spree::Calculator extend Spree::LocalizedNumber - preference :per_kg, :decimal, default: 0.0 - localize_number :preferred_per_kg + preference :per_unit, :decimal, default: 0.0 + preference :unit, :string, default: "kg" + localize_number :preferred_per_unit def self.description I18n.t('spree.weight') @@ -12,7 +13,7 @@ module Calculator def compute(object) line_items = line_items_for object - (total_weight(line_items) * preferred_per_kg).round(2) + (total_weight(line_items) * preferred_per_unit).round(2) end private @@ -33,8 +34,8 @@ module Calculator def weight_per_variant(line_item) if variant_unit(line_item) == 'weight' - # The calculator price is per_kg so we need to convert unit_value to kg - convert_g_to_kg(line_item.variant.andand.unit_value) + # Convert unit_value to the preferred unit + convert_weight(line_item.variant.andand.unit_value) else line_item.variant.andand.weight || 0 end @@ -42,8 +43,8 @@ module Calculator def weight_per_final_weight_volume(line_item) if variant_unit(line_item) == 'weight' - # The calculator price is per_kg so we need to convert final_weight_volume to kg - convert_g_to_kg(line_item.final_weight_volume) + # Convert final_weight_volume to the preferred unit + convert_weight(line_item.final_weight_volume) else weight_per_variant(line_item) * quantity_implied_in_final_weight_volume(line_item) end @@ -66,10 +67,16 @@ module Calculator line_item.variant.product.andand.variant_unit end - def convert_g_to_kg(value) + def convert_weight(value) return 0 unless value - - value / 1000 + if preferences[:unit] == "kg" + value / 1000 + elsif preferences[:unit] == "lb" + value / 453.6 + else + #TODO: handle this case without crashing? + raise "Unknown unit preference" + end end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 6cbd131c09..de5e0ffe4a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3126,7 +3126,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using inventory_error_flash_for_insufficient_quantity: "An item in your cart has become unavailable." inventory: Inventory zipcode: Postcode - weight: Weight (per kg) + weight: Weight (per kg or lb) error_user_destroy_with_orders: "Users with completed orders may not be deleted" cannot_create_payment_without_payment_methods: "You cannot create a payment for an order without any payment methods defined." please_define_payment_methods: "Please define some payment methods first." diff --git a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb index f209e87a81..9d05a8c440 100644 --- a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb @@ -29,13 +29,22 @@ describe Spree::Admin::ShippingMethodsController, type: :controller do expect(shipping_method.reload.calculator.preferred_currency).to eq "EUR" end - it "updates preferred_per_kg of a Weight calculator" do + it "updates preferred_per_unit of a Weight calculator" do shipping_method.calculator = create(:weight_calculator, calculable: shipping_method) - params[:shipping_method][:calculator_attributes][:preferred_per_kg] = 10 + params[:shipping_method][:calculator_attributes][:preferred_per_unit] = 10 spree_post :update, params - expect(shipping_method.reload.calculator.preferred_per_kg).to eq 10 + expect(shipping_method.reload.calculator.preferred_per_unit).to eq 10 + end + + it "updates preferred_unit of a Weight calculator" do + shipping_method.calculator = create(:weight_calculator, calculable: shipping_method) + params[:shipping_method][:calculator_attributes][:preferred_unit] = "kg" + + spree_post :update, params + + expect(shipping_method.reload.calculator.preferred_unit).to eq "kg" end it "updates preferred_flat_percent of a FlatPercentPerItem calculator" do diff --git a/spec/factories/calculator_factory.rb b/spec/factories/calculator_factory.rb index 1268a8ccb7..196fefa935 100644 --- a/spec/factories/calculator_factory.rb +++ b/spec/factories/calculator_factory.rb @@ -13,7 +13,14 @@ FactoryBot.define do end factory :weight_calculator, class: Calculator::Weight do - after(:build) { |c| c.set_preference(:per_kg, 0.5) } - after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } + after(:build) { |c| + c.set_preference(:per_unit, 0.5) + c.set_preference(:unit, "kg") + } + after(:create) { |c| + c.set_preference(:per_unit, 0.5) + c.set_preference(:unit, "kg") + c.save! + } end end diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index 5339aa99f7..9597e8a35e 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -118,7 +118,7 @@ feature "full-page cart", js: true do describe "admin weight calculated fees" do context "order with 2 line items" do let(:admin_fee) { - create(:enterprise_fee, calculator: Calculator::Weight.new(preferred_per_kg: 1), + create(:enterprise_fee, calculator: Calculator::Weight.new(preferred_per_unit: 1, preferred_unit: "kg"), enterprise: order_cycle.coordinator, fee_type: 'admin') } diff --git a/spec/models/calculator/weight_spec.rb b/spec/models/calculator/weight_spec.rb index 1ca1759b40..6c5a8e51e2 100644 --- a/spec/models/calculator/weight_spec.rb +++ b/spec/models/calculator/weight_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Calculator::Weight do - it_behaves_like "a model using the LocalizedNumber module", [:preferred_per_kg] + it_behaves_like "a model using the LocalizedNumber module", [:preferred_per_unit] it "computes shipping cost for an order by total weight" do variant1 = build(:variant, unit_value: 10_000) @@ -14,7 +14,8 @@ describe Calculator::Weight do order = double(:order, line_items: [line_item1, line_item2, line_item3]) - subject.set_preference(:per_kg, 5) + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit, "kg") expect(subject.compute(order)).to eq(350) # (10 * 1 + 20 * 3) * 5 end @@ -22,7 +23,10 @@ describe Calculator::Weight do let(:variant) { create(:variant, unit_value: 10_000) } let(:line_item) { create(:line_item, variant: variant, quantity: 2) } - before { subject.set_preference(:per_kg, 5) } + before { + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit, "kg") + } it "computes shipping cost for a line item" do expect(subject.compute(line_item)).to eq(100) # 10 * 2 * 5 @@ -55,7 +59,8 @@ describe Calculator::Weight do order = double(:order, line_items: [line_item1, line_item2]) object_with_order = double(:object_with_order, order: order) - subject.set_preference(:per_kg, 5) + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit, "kg") expect(subject.compute(object_with_order)).to eq(250) # (10 * 1 + 20 * 2) * 5 end @@ -63,7 +68,7 @@ describe Calculator::Weight do let!(:product) { create(:product, product_attributes) } let!(:variant) { create(:variant, variant_attributes.merge(product: product)) } - let(:calculator) { described_class.new(preferred_per_kg: 6) } + let(:calculator) { described_class.new(preferred_per_unit: 6, preferred_unit: "kg") } let(:line_item) do create(:line_item, variant: variant, quantity: 2).tap do |object| object.send(:calculate_final_weight_volume) @@ -158,7 +163,10 @@ describe Calculator::Weight do } let(:line_item) { create(:line_item, variant: variant, quantity: 1) } - before { subject.set_preference(:per_kg, 5) } + before { + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit, "kg") + } context "when unit_value is zero variant.weight is present" do let(:variant) { create(:variant, product: product, unit_value: 0, weight: 10.0) } From 5a5cbbd3181a71494de522c287a165950e9f98aa Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 7 Sep 2020 13:26:09 -0700 Subject: [PATCH 04/24] add drop down list for unit preference --- .../spree/admin/shipping_methods_controller.rb | 2 +- app/helpers/spree/admin/base_helper.rb | 18 ++++++++++++++---- app/models/calculator/weight.rb | 6 +++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index b68dca98e3..ebf3e8e432 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -88,7 +88,7 @@ module Spree :require_ship_address, :tag_list, :calculator_type, distributor_ids: [], calculator_attributes: [ - :id, :preferred_currency, :preferred_amount, :preferred_unit, :preferred_per_unit, :preferred_flat_percent, + :id, :preferred_currency, :preferred_amount, :preferred_unit_from_list, :preferred_per_unit, :preferred_flat_percent, :preferred_first_item, :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount ] diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index 6ffdc3dc2b..833ece1bf7 100644 --- a/app/helpers/spree/admin/base_helper.rb +++ b/app/helpers/spree/admin/base_helper.rb @@ -45,14 +45,14 @@ module Spree end end - def preference_field_for(form, field, options) + def preference_field_for(form, field, options, object) case options[:type] when :integer form.text_field(field, preference_field_options(options)) when :boolean form.check_box(field, preference_field_options(options)) when :string - form.text_field(field, preference_field_options(options)) + preference_field_for_text_field(form, field, options, object) when :password form.password_field(field, preference_field_options(options)) when :text @@ -62,6 +62,16 @@ module Spree end end + def preference_field_for_text_field(form, field, options, object) + if field.end_with?('_from_list') && object.respond_to?("#{field}_values") + list_values = object.__send__("#{field}_values") + selected_value = object.__send__(field) + form.select(field, options_for_select(list_values, selected_value), preference_field_options(options)) + else + form.text_field(field, preference_field_options(options)) + end + end + def preference_field_options(options) field_options = case options[:type] when :integer @@ -95,8 +105,8 @@ module Spree return unless object.respond_to?(:preferences) object.preferences.keys.map{ |key| - form.label("preferred_#{key}", Spree.t(key) + ": ") + - preference_field_for(form, "preferred_#{key}", type: object.preference_type(key)) + form.label("preferred_#{key}", Spree.t(key.to_s.gsub("_from_list", "")) + ": ") + + preference_field_for(form, "preferred_#{key}", { type: object.preference_type(key) }, object) }.join("
").html_safe end diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index abdf626d05..aa479e7d09 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -4,7 +4,7 @@ module Calculator class Weight < Spree::Calculator extend Spree::LocalizedNumber preference :per_unit, :decimal, default: 0.0 - preference :unit, :string, default: "kg" + preference :unit_from_list, :string, default: "kg" localize_number :preferred_per_unit def self.description @@ -16,6 +16,10 @@ module Calculator (total_weight(line_items) * preferred_per_unit).round(2) end + def preferred_unit_from_list_values + ["kg", "lb"] + end + private def total_weight(line_items) From c618ba3b2c58fd0dc5c9423f31ab4b1cbfcc1d4d Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 7 Sep 2020 13:37:59 -0700 Subject: [PATCH 05/24] style fixes --- app/controllers/spree/admin/shipping_methods_controller.rb | 7 ++++--- app/helpers/spree/admin/base_helper.rb | 6 ++++-- app/models/calculator/weight.rb | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index ebf3e8e432..f3b00eab4b 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -88,9 +88,10 @@ module Spree :require_ship_address, :tag_list, :calculator_type, distributor_ids: [], calculator_attributes: [ - :id, :preferred_currency, :preferred_amount, :preferred_unit_from_list, :preferred_per_unit, :preferred_flat_percent, - :preferred_first_item, :preferred_additional_item, :preferred_max_items, - :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount + :id, :preferred_currency, :preferred_amount, :preferred_unit_from_list, + :preferred_per_unit, :preferred_flat_percent, :preferred_first_item, + :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount, + :preferred_normal_amount, :preferred_discount_amount ] ) end diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index 833ece1bf7..f7741a3a1f 100644 --- a/app/helpers/spree/admin/base_helper.rb +++ b/app/helpers/spree/admin/base_helper.rb @@ -66,7 +66,8 @@ module Spree if field.end_with?('_from_list') && object.respond_to?("#{field}_values") list_values = object.__send__("#{field}_values") selected_value = object.__send__(field) - form.select(field, options_for_select(list_values, selected_value), preference_field_options(options)) + form.select(field, options_for_select(list_values, selected_value), + preference_field_options(options)) else form.text_field(field, preference_field_options(options)) end @@ -106,7 +107,8 @@ module Spree object.preferences.keys.map{ |key| form.label("preferred_#{key}", Spree.t(key.to_s.gsub("_from_list", "")) + ": ") + - preference_field_for(form, "preferred_#{key}", { type: object.preference_type(key) }, object) + preference_field_for(form, "preferred_#{key}", + { type: object.preference_type(key) }, object) }.join("
").html_safe end diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index aa479e7d09..55f8178c9c 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -73,12 +73,12 @@ module Calculator def convert_weight(value) return 0 unless value + if preferences[:unit] == "kg" value / 1000 elsif preferences[:unit] == "lb" value / 453.6 else - #TODO: handle this case without crashing? raise "Unknown unit preference" end end From d32ed6b48f1cffd68e008e7b12fcca0fd0c1d00b Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sat, 12 Sep 2020 10:43:14 -0700 Subject: [PATCH 06/24] improve styling on calculators UI --- app/helpers/spree/admin/base_helper.rb | 8 ++++---- app/models/calculator/weight.rb | 4 +++- .../enterprise_fees/_calculator_settings.html.haml | 5 ++++- .../payment_methods/_provider_settings.html.haml | 7 ++++++- .../spree/admin/shared/_calculator_fields.html.haml | 13 ++++++++++--- .../spree/admin/shipping_methods/_form.html.haml | 3 +-- 6 files changed, 28 insertions(+), 12 deletions(-) diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index f7741a3a1f..6fb6dc4ba6 100644 --- a/app/helpers/spree/admin/base_helper.rb +++ b/app/helpers/spree/admin/base_helper.rb @@ -105,11 +105,11 @@ module Spree def preference_fields(object, form) return unless object.respond_to?(:preferences) - object.preferences.keys.map{ |key| - form.label("preferred_#{key}", Spree.t(key.to_s.gsub("_from_list", "")) + ": ") + + object.preferences.keys.map { |key| + [form.label("preferred_#{key}", Spree.t(key.to_s.gsub("_from_list", "")) + ": "), preference_field_for(form, "preferred_#{key}", - { type: object.preference_type(key) }, object) - }.join("
").html_safe + { type: object.preference_type(key) }, object)] + } end def link_to_add_fields(name, target, options = {}) diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index 55f8178c9c..c2f93c2c70 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -3,8 +3,10 @@ require 'spree/localized_number' module Calculator class Weight < Spree::Calculator extend Spree::LocalizedNumber - preference :per_unit, :decimal, default: 0.0 + preference :unit_from_list, :string, default: "kg" + preference :per_unit, :decimal, default: 0.0 + localize_number :preferred_per_unit def self.description diff --git a/app/views/admin/enterprise_fees/_calculator_settings.html.haml b/app/views/admin/enterprise_fees/_calculator_settings.html.haml index 45e2a7c394..415992a008 100644 --- a/app/views/admin/enterprise_fees/_calculator_settings.html.haml +++ b/app/views/admin/enterprise_fees/_calculator_settings.html.haml @@ -7,6 +7,9 @@ - if !enterprise_fee.new_record? .calculator-settings = f.fields_for :calculator do |calculator_form| - = preference_fields(enterprise_fee.calculator, calculator_form) + - fields = preference_fields(enterprise_fee.calculator, calculator_form) + - fields.each do |f| + = f[0].html_safe + = f[1].html_safe = calculator_form_string diff --git a/app/views/spree/admin/payment_methods/_provider_settings.html.haml b/app/views/spree/admin/payment_methods/_provider_settings.html.haml index cbf24630b2..bf582636a7 100644 --- a/app/views/spree/admin/payment_methods/_provider_settings.html.haml +++ b/app/views/spree/admin/payment_methods/_provider_settings.html.haml @@ -10,4 +10,9 @@ = t(:provider_settings) .preference-settings = fields_for :payment_method, @payment_method do |payment_method_form| - = preference_fields(@payment_method, payment_method_form) + - fields = preference_fields(@payment_method, payment_method_form) + - fields.each do |f| + .field.alpha.three.columns + = f[0].html_safe + .field.omega.eight.columns + = f[1].html_safe diff --git a/app/views/spree/admin/shared/_calculator_fields.html.haml b/app/views/spree/admin/shared/_calculator_fields.html.haml index 062503c8d6..cafdf01e3c 100644 --- a/app/views/spree/admin/shared/_calculator_fields.html.haml +++ b/app/views/spree/admin/shared/_calculator_fields.html.haml @@ -2,12 +2,19 @@ %legend{align: "center"}= t(:calculator) #preference-settings .field - = f.label(:calculator_type, t(:calculator), for: 'calc_type') - = f.select(:calculator_type, @calculators.map { |c| [c.description, c.name] }, {}, {id: 'calc_type', class: 'select2 fullwidth'}) + .alpha.three.columns + = f.label(:calculator_type, t(:calculator), for: 'calc_type') + .omega.eight.columns + = f.select(:calculator_type, @calculators.map { |c| [c.description, c.name] }, {}, {id: 'calc_type', class: 'select2 fullwidth'}) - if !@object.new_record? .field .calculator-settings = f.fields_for :calculator do |calculator_form| - = preference_fields(@object.calculator, calculator_form) + - fields = preference_fields(@object.calculator, calculator_form) + - fields.each do |f| + .field.alpha.three.columns + = f[0].html_safe + .field.omega.eight.columns + = f[1].html_safe - if @object.calculator.respond_to?(:preferences) %span.calculator-settings-warning.info.warning= t(:calculator_settings_warning) diff --git a/app/views/spree/admin/shipping_methods/_form.html.haml b/app/views/spree/admin/shipping_methods/_form.html.haml index 13d7667745..b03085c7a0 100644 --- a/app/views/spree/admin/shipping_methods/_form.html.haml +++ b/app/views/spree/admin/shipping_methods/_form.html.haml @@ -44,8 +44,7 @@ %tags-with-translation#something{ object: "shippingMethod", 'find-tags' => 'findTags(query)' } .row - .alpha.eleven.columns - = render partial: 'spree/admin/shared/calculator_fields', locals: { f: f } + = render partial: 'spree/admin/shared/calculator_fields', locals: { f: f } .row .alpha.eleven.columns From 4123eb7c108cb232b6cdbcb071ca60bb757ec8bf Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sat, 12 Sep 2020 11:53:45 -0700 Subject: [PATCH 07/24] update specs; add specs to validate weight calculator preference is kg or lb --- app/models/calculator/weight.rb | 13 ++++++++++--- spec/models/calculator/weight_spec.rb | 24 +++++++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index c2f93c2c70..bac869c1f6 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -3,7 +3,6 @@ require 'spree/localized_number' module Calculator class Weight < Spree::Calculator extend Spree::LocalizedNumber - preference :unit_from_list, :string, default: "kg" preference :per_unit, :decimal, default: 0.0 @@ -13,6 +12,14 @@ module Calculator I18n.t('spree.weight') end + def set_preference(name, value) + if name == :unit_from_list && !["kg", "lb"].include?(value) + self.calculable.errors.add(:preferred_unit_from_list, "must be kg or lb") + else + send self.class.preference_setter_method(name), value + end + end + def compute(object) line_items = line_items_for object (total_weight(line_items) * preferred_per_unit).round(2) @@ -76,9 +83,9 @@ module Calculator def convert_weight(value) return 0 unless value - if preferences[:unit] == "kg" + if preferences[:unit_from_list] == "kg" value / 1000 - elsif preferences[:unit] == "lb" + elsif preferences[:unit_from_list] == "lb" value / 453.6 else raise "Unknown unit preference" diff --git a/spec/models/calculator/weight_spec.rb b/spec/models/calculator/weight_spec.rb index 6c5a8e51e2..702f93fab6 100644 --- a/spec/models/calculator/weight_spec.rb +++ b/spec/models/calculator/weight_spec.rb @@ -15,7 +15,7 @@ describe Calculator::Weight do order = double(:order, line_items: [line_item1, line_item2, line_item3]) subject.set_preference(:per_unit, 5) - subject.set_preference(:unit, "kg") + subject.set_preference(:unit_from_list, "kg") expect(subject.compute(order)).to eq(350) # (10 * 1 + 20 * 3) * 5 end @@ -25,7 +25,7 @@ describe Calculator::Weight do before { subject.set_preference(:per_unit, 5) - subject.set_preference(:unit, "kg") + subject.set_preference(:unit_from_list, "kg") } it "computes shipping cost for a line item" do @@ -60,7 +60,7 @@ describe Calculator::Weight do object_with_order = double(:object_with_order, order: order) subject.set_preference(:per_unit, 5) - subject.set_preference(:unit, "kg") + subject.set_preference(:unit_from_list, "kg") expect(subject.compute(object_with_order)).to eq(250) # (10 * 1 + 20 * 2) * 5 end @@ -68,7 +68,7 @@ describe Calculator::Weight do let!(:product) { create(:product, product_attributes) } let!(:variant) { create(:variant, variant_attributes.merge(product: product)) } - let(:calculator) { described_class.new(preferred_per_unit: 6, preferred_unit: "kg") } + let(:calculator) { described_class.new(preferred_per_unit: 6, preferred_unit_from_list: "kg") } let(:line_item) do create(:line_item, variant: variant, quantity: 2).tap do |object| object.send(:calculate_final_weight_volume) @@ -165,7 +165,7 @@ describe Calculator::Weight do before { subject.set_preference(:per_unit, 5) - subject.set_preference(:unit, "kg") + subject.set_preference(:unit_from_list, "kg") } context "when unit_value is zero variant.weight is present" do @@ -208,4 +208,18 @@ describe Calculator::Weight do end end end + + it "allows a preferred_unit of 'kg' and 'lb'" do + subject.calculable = build(:shipping_method) + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit_from_list, "kg") + expect(subject.calculable.errors.count).to eq(0) + end + + it "does not allow a preferred_unit of anything but 'kg' or 'lb'" do + subject.calculable = build(:shipping_method) + subject.set_preference(:per_unit, 5) + subject.set_preference(:unit_from_list, "kb") + expect(subject.calculable.errors.count).to eq(1) + end end From 37cfe65688b936dc54cc75fca16cd66536ddb7bb Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sat, 12 Sep 2020 12:32:06 -0700 Subject: [PATCH 08/24] migrate existing weight calculators --- db/migrate/20200912190210_update_weight_calculators.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 db/migrate/20200912190210_update_weight_calculators.rb diff --git a/db/migrate/20200912190210_update_weight_calculators.rb b/db/migrate/20200912190210_update_weight_calculators.rb new file mode 100644 index 0000000000..4ee5a0dfc0 --- /dev/null +++ b/db/migrate/20200912190210_update_weight_calculators.rb @@ -0,0 +1,10 @@ +class UpdateWeightCalculators < ActiveRecord::Migration + def change + Calculator::Weight.each { |calculator| + calculator.preferred_unit_from_list = 'kg' + calculator.preferred_per_unit = calculator.preferred_per_kg + calculator.preferences.delete(:preferred_per_kg) + calculator.save + } + end +end From 246934d8ba94e47d80e7dce5c1d288c393409a3a Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sat, 12 Sep 2020 12:35:34 -0700 Subject: [PATCH 09/24] update preference name in factor and specs --- .../spree/admin/shipping_methods_controller_spec.rb | 4 ++-- spec/factories/calculator_factory.rb | 6 +++--- spec/features/consumer/shopping/cart_spec.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb index 9d05a8c440..2c1fa2c0b6 100644 --- a/spec/controllers/spree/admin/shipping_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/shipping_methods_controller_spec.rb @@ -40,11 +40,11 @@ describe Spree::Admin::ShippingMethodsController, type: :controller do it "updates preferred_unit of a Weight calculator" do shipping_method.calculator = create(:weight_calculator, calculable: shipping_method) - params[:shipping_method][:calculator_attributes][:preferred_unit] = "kg" + params[:shipping_method][:calculator_attributes][:preferred_unit_from_list] = "kg" spree_post :update, params - expect(shipping_method.reload.calculator.preferred_unit).to eq "kg" + expect(shipping_method.reload.calculator.preferred_unit_from_list).to eq "kg" end it "updates preferred_flat_percent of a FlatPercentPerItem calculator" do diff --git a/spec/factories/calculator_factory.rb b/spec/factories/calculator_factory.rb index 196fefa935..1203d5b52c 100644 --- a/spec/factories/calculator_factory.rb +++ b/spec/factories/calculator_factory.rb @@ -15,11 +15,11 @@ FactoryBot.define do factory :weight_calculator, class: Calculator::Weight do after(:build) { |c| c.set_preference(:per_unit, 0.5) - c.set_preference(:unit, "kg") + c.set_preference(:unit_from_list, "kg") } - after(:create) { |c| + after(:create) { |c| c.set_preference(:per_unit, 0.5) - c.set_preference(:unit, "kg") + c.set_preference(:unit_from_list, "kg") c.save! } end diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index 9597e8a35e..d702ce7067 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -118,7 +118,7 @@ feature "full-page cart", js: true do describe "admin weight calculated fees" do context "order with 2 line items" do let(:admin_fee) { - create(:enterprise_fee, calculator: Calculator::Weight.new(preferred_per_unit: 1, preferred_unit: "kg"), + create(:enterprise_fee, calculator: Calculator::Weight.new(preferred_per_unit: 1, preferred_unit_from_list: "kg"), enterprise: order_cycle.coordinator, fee_type: 'admin') } From 6466829bdc2eccb326d0bc6d3d9d7970d4dc0521 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 17 Sep 2020 07:17:25 -0700 Subject: [PATCH 10/24] fix typo in weight calculator migration --- db/migrate/20200912190210_update_weight_calculators.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20200912190210_update_weight_calculators.rb b/db/migrate/20200912190210_update_weight_calculators.rb index 4ee5a0dfc0..bf027fd257 100644 --- a/db/migrate/20200912190210_update_weight_calculators.rb +++ b/db/migrate/20200912190210_update_weight_calculators.rb @@ -1,6 +1,6 @@ class UpdateWeightCalculators < ActiveRecord::Migration def change - Calculator::Weight.each { |calculator| + Calculator::Weight.all.each { |calculator| calculator.preferred_unit_from_list = 'kg' calculator.preferred_per_unit = calculator.preferred_per_kg calculator.preferences.delete(:preferred_per_kg) From 977e4e46f3449fd903f8efd18be06f053f7e536e Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 17 Sep 2020 07:36:28 -0700 Subject: [PATCH 11/24] remove explicit raise from convert_weight --- app/models/calculator/weight.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/calculator/weight.rb b/app/models/calculator/weight.rb index bac869c1f6..0933223eaf 100644 --- a/app/models/calculator/weight.rb +++ b/app/models/calculator/weight.rb @@ -81,14 +81,12 @@ module Calculator end def convert_weight(value) - return 0 unless value + return 0 unless value && ["kg", "lb"].include?(preferences[:unit_from_list]) if preferences[:unit_from_list] == "kg" value / 1000 elsif preferences[:unit_from_list] == "lb" value / 453.6 - else - raise "Unknown unit preference" end end end From 73149dc69544d7a55457e6b3919f97ee461b64df Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 17 Sep 2020 07:36:38 -0700 Subject: [PATCH 12/24] schema update --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index f56c95f405..38c53def20 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20200721135726) do +ActiveRecord::Schema.define(version: 20200912190210) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From 988abf7a8ce4698a7372fea2ff877dc1e7a8a759 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 17 Sep 2020 08:27:45 -0700 Subject: [PATCH 13/24] update preference field interface to be more clear --- app/helpers/spree/admin/base_helper.rb | 14 +++++++++++--- .../enterprise_fees/_calculator_settings.html.haml | 4 ++-- .../payment_methods/_provider_settings.html.haml | 4 ++-- .../admin/shared/_calculator_fields.html.haml | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb index 6fb6dc4ba6..7615f2b6c9 100644 --- a/app/helpers/spree/admin/base_helper.rb +++ b/app/helpers/spree/admin/base_helper.rb @@ -62,6 +62,10 @@ module Spree end end + # Here we show a text field for all string fields except when the field name ends in + # "_from_list", in that case we render a dropdown. + # In this specific case, to render the dropdown, the object provided must have a method named + # like "#{field}_values" that returns an array with the string options to be listed. def preference_field_for_text_field(form, field, options, object) if field.end_with?('_from_list') && object.respond_to?("#{field}_values") list_values = object.__send__("#{field}_values") @@ -102,13 +106,17 @@ module Spree ) end + # maps each preference to a hash containing the label and field html. + # E.g. { :label => "