From 0cc8349c718ec696189b1a87aa49b0561c96931a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 22 Jul 2022 16:48:26 +0200 Subject: [PATCH 1/7] Use fee name for the adjustment label instead of type + update specs as well --- config/locales/en.yml | 2 +- lib/open_food_network/enterprise_fee_applicator.rb | 2 +- .../lib/open_food_network/enterprise_fee_applicator_spec.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 4ab0928132..155c2f2fa2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2907,7 +2907,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using action_required: "Action required" tag_rules: "Tag Rules" enterprise_fee_whole_order: Whole order - enterprise_fee_by: "%{type} fee by %{role} %{enterprise_name}" + enterprise_fee_by: "%{name} fee by %{role} %{enterprise_name}" validation_msg_relationship_already_established: "^That relationship is already established." validation_msg_at_least_one_hub: "^At least one hub must be selected" validation_msg_tax_category_cant_be_blank: "^Tax Category can't be blank" diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index e856ca3f8d..ede8554d23 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -30,7 +30,7 @@ module OpenFoodNetwork end def base_adjustment_label - I18n.t(:enterprise_fee_by, type: enterprise_fee.fee_type, role: role, + I18n.t(:enterprise_fee_by, name: enterprise_fee.name, role: role, enterprise_name: enterprise_fee.enterprise.name) end diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb index af7f4ddd7a..23d7952067 100644 --- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb @@ -64,7 +64,7 @@ module OpenFoodNetwork describe "making labels" do let(:variant) { double(:variant, product: double(:product, name: 'Bananas')) } let(:enterprise_fee) { - double(:enterprise_fee, fee_type: 'packing', + double(:enterprise_fee, name: 'packing name', enterprise: double(:enterprise, name: 'Ballantyne')) } let(:applicator) { EnterpriseFeeApplicator.new enterprise_fee, variant, 'distributor' } @@ -72,7 +72,7 @@ module OpenFoodNetwork describe "#line_item_adjustment_label" do it "makes an adjustment label for a line item" do expect(applicator.send(:line_item_adjustment_label)). - to eq("Bananas - packing fee by distributor Ballantyne") + to eq("Bananas - packing name fee by distributor Ballantyne") end end @@ -81,7 +81,7 @@ module OpenFoodNetwork it "makes an adjustment label for an order" do expect(applicator.send(:order_adjustment_label)). - to eq("Whole order - packing fee by distributor Ballantyne") + to eq("Whole order - packing name fee by distributor Ballantyne") end end end From 72df2867d4e06c57d99e7aab3d83aae721443468 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 22 Jul 2022 17:08:07 +0200 Subject: [PATCH 2/7] Remove admin_and_handling merging + update specs as well --- app/helpers/checkout_helper.rb | 12 ------------ app/views/spree/orders/_form.html.haml | 2 +- spec/helpers/checkout_helper_spec.rb | 2 +- spec/system/admin/invoice_print_spec.rb | 18 ++++++++++++------ 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index c4754e667e..aa5c069045 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -32,18 +32,6 @@ module CheckoutHelper } end - enterprise_fee_adjustments = adjustments.select { |a| - a.originator_type == 'EnterpriseFee' && a.adjustable_type != 'Spree::LineItem' - } - adjustments.reject! { |a| - a.originator_type == 'EnterpriseFee' && a.adjustable_type != 'Spree::LineItem' - } - unless exclude.include? :admin_and_handling - adjustments << Spree::Adjustment.new( - label: I18n.t(:orders_form_admin), amount: enterprise_fee_adjustments.sum(&:amount) - ) - end - adjustments end diff --git a/app/views/spree/orders/_form.html.haml b/app/views/spree/orders/_form.html.haml index 14b483e980..7199594fec 100644 --- a/app/views/spree/orders/_form.html.haml +++ b/app/views/spree/orders/_form.html.haml @@ -38,7 +38,7 @@ %span.order-total.distribution-total= display_line_item_fees_total_for(@order) %td - - checkout_adjustments_for(@order, exclude: [:line_item, :admin_and_handling]).reject{ |a| a.amount == 0 }.reverse_each do |adjustment| + - checkout_adjustments_for(@order, exclude: [:line_item]).reject{ |a| a.amount == 0 }.reverse_each do |adjustment| %tr.order-adjustment %td.text-right{:colspan => "3"} = adjustment.label diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index ef64fec69a..a06973240b 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -148,7 +148,7 @@ describe CheckoutHelper, type: :helper do expect(adjustments).to include shipping_adjustment admin_fee_summary = adjustments.last - expect(admin_fee_summary.label).to eq I18n.t(:orders_form_admin) + expect(admin_fee_summary.label).to eq "Shipping" expect(admin_fee_summary.amount).to eq 123 end diff --git a/spec/system/admin/invoice_print_spec.rb b/spec/system/admin/invoice_print_spec.rb index e7b2bddad1..01ac5d40e4 100644 --- a/spec/system/admin/invoice_print_spec.rb +++ b/spec/system/admin/invoice_print_spec.rb @@ -215,7 +215,8 @@ describe ' it "displays GST for enterprise fees" do pending "ii) for legend see picture on PR #9495" # enterprise fee of $20.00 - expect(page).to have_content "Admin & Handling 1 $20.00 $120.00" + expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} 1 $20.00 (included) $120.00" end it "displays the taxes correctly" do @@ -225,7 +226,8 @@ describe ' expect(page).to have_content "#{Spree::Product.second.name} 3 $250.08 $1,500.45" expect(page).to have_content "(1g)" # display as # Enterprise fee - expect(page).to have_content "Admin & Handling 1 $120.00" + expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} 1 $15.65 (included) $120.00" # Shipping expect(page).to have_content "Shipping 1 $9.14 (included) $100.55" # Order Totals @@ -257,7 +259,8 @@ describe ' expect(page).to have_content "(1g)" # display as expect(page).to have_content "3 $500.15 $1,500.45 20.0%" # Enterprise fee - expect(page).to have_content "Admin & Handling $120.00" + expect(page).to have_content "#{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} $120.00" # Shipping expect(page).to have_content "Shipping $100.55 10.0%" # Tax totals @@ -357,14 +360,16 @@ describe ' it "displays GST for enterprise fees" do pending "v) for legend see picture on PR #9495" # enterprise fee of $24.00 - expect(page).to have_content "Admin & Handling 1 $20.00 $120.00" + expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} 1 $20.00 $120.00" end it "displays the taxes correctly" do # header expect(page).to have_content "Item Qty GST Price" # Enterprise fee - expect(page).to have_content "Admin & Handling 1 $120.00" + expect(page).to have_content "Whole order - #{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} 1 $18.00 $120.00" # Shipping expect(page).to have_content "Shipping 1 $10.06 $100.55" # Order Totals @@ -395,7 +400,8 @@ describe ' expect(page).to have_content "(1g)" # display as expect(page).to have_content "3 $500.15 $1,500.45 20.0%" # Enterprise fee - expect(page).to have_content "Admin & Handling $120.00" + expect(page).to have_content "#{enterprise_fee.name} fee by coordinator " \ + "#{user1.enterprises.first.name} $120.00" # Shipping expect(page).to have_content "Shipping $100.55 10.0%" # Tax totals From 77e644e3d438d1676350ce2b705ce8da1fd1a5f8 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 26 Jul 2022 16:57:45 +0200 Subject: [PATCH 3/7] Get adjustment which is not the shipping one --- spec/helpers/checkout_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index a06973240b..2121601c1c 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -147,7 +147,7 @@ describe CheckoutHelper, type: :helper do shipping_adjustment = order.shipment_adjustments.first expect(adjustments).to include shipping_adjustment - admin_fee_summary = adjustments.last + admin_fee_summary = adjustments.reject { |a| a.id == shipping_adjustment.id }.first expect(admin_fee_summary.label).to eq "Shipping" expect(admin_fee_summary.amount).to eq 123 end From 52843de1da5b36f9e63c2ce6d9779ad37c4e6919 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 11 Aug 2022 16:30:34 +1000 Subject: [PATCH 4/7] Test against unique fee name The adjustment factory calls everything "Shipping" by default. This was confusing here because we were explicitely looking at a non-shipping fee. --- spec/helpers/checkout_helper_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 2121601c1c..55d17ef7e9 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -131,7 +131,7 @@ describe CheckoutHelper, type: :helper do let(:enterprise_fee) { create(:enterprise_fee, amount: 123) } let!(:fee_adjustment) { create(:adjustment, originator: enterprise_fee, adjustable: order, - order: order) + order: order, label: "Enterprise Fee") } before do @@ -148,7 +148,7 @@ describe CheckoutHelper, type: :helper do expect(adjustments).to include shipping_adjustment admin_fee_summary = adjustments.reject { |a| a.id == shipping_adjustment.id }.first - expect(admin_fee_summary.label).to eq "Shipping" + expect(admin_fee_summary.label).to eq "Enterprise Fee" expect(admin_fee_summary.amount).to eq 123 end From b297299838bc5acbf083a5500b4a6ca73604eafb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 11 Aug 2022 16:35:43 +1000 Subject: [PATCH 5/7] Simplify test of adjustments --- spec/helpers/checkout_helper_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 55d17ef7e9..cdb9918141 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -145,11 +145,8 @@ describe CheckoutHelper, type: :helper do adjustments = helper.checkout_adjustments_for(order) shipping_adjustment = order.shipment_adjustments.first - expect(adjustments).to include shipping_adjustment - admin_fee_summary = adjustments.reject { |a| a.id == shipping_adjustment.id }.first - expect(admin_fee_summary.label).to eq "Enterprise Fee" - expect(admin_fee_summary.amount).to eq 123 + expect(adjustments).to match_array [shipping_adjustment, fee_adjustment] end context "tax rate adjustments" do From 7b57f2358cae03510ce14651a6b70eb872dbd512 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Sep 2022 10:01:32 +0200 Subject: [PATCH 6/7] Create a new key instead of modifying the old one it could lead to issues when key is not updated and we should avoid that kind of issue. --- config/locales/en.yml | 2 +- lib/open_food_network/enterprise_fee_applicator.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 155c2f2fa2..612a81e32a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2907,7 +2907,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using action_required: "Action required" tag_rules: "Tag Rules" enterprise_fee_whole_order: Whole order - enterprise_fee_by: "%{name} fee by %{role} %{enterprise_name}" + enterprise_fee_by_name: "%{name} fee by %{role} %{enterprise_name}" validation_msg_relationship_already_established: "^That relationship is already established." validation_msg_at_least_one_hub: "^At least one hub must be selected" validation_msg_tax_category_cant_be_blank: "^Tax Category can't be blank" diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index ede8554d23..3f446e5870 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -30,8 +30,8 @@ module OpenFoodNetwork end def base_adjustment_label - I18n.t(:enterprise_fee_by, name: enterprise_fee.name, role: role, - enterprise_name: enterprise_fee.enterprise.name) + I18n.t(:enterprise_fee_by_name, name: enterprise_fee.name, role: role, + enterprise_name: enterprise_fee.enterprise.name) end def tax_category(target) From 8b89c901ef43cb14050eb20bb1bc1aa1ad3a0aac Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Oct 2022 16:37:42 +0200 Subject: [PATCH 7/7] Do not display "Admin and Handling" ie. fees total --- app/helpers/checkout_helper.rb | 4 ---- app/views/spree/orders/_form.html.haml | 7 ------- config/locales/en.yml | 1 - spec/system/consumer/shopping/cart_spec.rb | 6 +++--- 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index aa5c069045..4c8a2ea53e 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -35,10 +35,6 @@ module CheckoutHelper adjustments end - def display_line_item_fees_total_for(order) - Spree::Money.new order.adjustments.enterprise_fee.sum(:amount), currency: order.currency - end - def checkout_line_item_fees(order) order.line_item_adjustments.enterprise_fee end diff --git a/app/views/spree/orders/_form.html.haml b/app/views/spree/orders/_form.html.haml index 7199594fec..877d700dc4 100644 --- a/app/views/spree/orders/_form.html.haml +++ b/app/views/spree/orders/_form.html.haml @@ -30,13 +30,6 @@ %td.text-right %span.order-total.item-total= display_checkout_subtotal(@order) %td - -if display_line_item_fees_total_for(@order) != Spree::Money.new(0 , currency: @order.currency) - %tr - %td.text-right{colspan:"3"} - = t :orders_form_admin - %td.text-right - %span.order-total.distribution-total= display_line_item_fees_total_for(@order) - %td - checkout_adjustments_for(@order, exclude: [:line_item]).reject{ |a| a.amount == 0 }.reverse_each do |adjustment| %tr.order-adjustment diff --git a/config/locales/en.yml b/config/locales/en.yml index 612a81e32a..7042c893da 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2278,7 +2278,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using orders_form_empty_cart: "Empty cart" orders_form_update_cart: "Update" orders_form_subtotal: "Produce subtotal" - orders_form_admin: "Admin & Handling" orders_form_total: "Total" orders_oc_expired_headline: "Orders have closed for this order cycle" orders_oc_expired_text: "Sorry, orders for this order cycle closed %{time} ago! Please contact your hub directly to see if they can accept late orders." diff --git a/spec/system/consumer/shopping/cart_spec.rb b/spec/system/consumer/shopping/cart_spec.rb index e8caa48392..068a31019e 100644 --- a/spec/system/consumer/shopping/cart_spec.rb +++ b/spec/system/consumer/shopping/cart_spec.rb @@ -106,14 +106,14 @@ describe "full-page cart", js: true do visit main_app.cart_path end - it "shows admin and handlings row" do + it "shows enterprise fees row row" do expect(page).to have_selector('#cart-detail') - expect(page).to have_content('Admin & Handling') + expect(page).to have_content("Whole order - #{handling_fee.name} fee by distributor #{order_cycle.coordinator.name}") expect(page).to have_selector '.cart-item-price', text: with_currency(0.86) expect(page).to have_selector '.order-total.item-total', text: with_currency(2.58) - expect(page).to have_selector '.order-total.distribution-total', + expect(page).to have_selector '.order-adjustment .total', text: with_currency(1.00) expect(page).to have_selector '.order-total.grand-total', text: with_currency(3.58) # price * 3 + 1 end