From a793fc9f99ef2d3cbe8f7ecf7e6c36bfb27b72ef Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 26 Dec 2022 15:47:43 +0100 Subject: [PATCH 1/5] Create a "None" calculator for shipping and payment methods Only for payment and shipping methods --- app/models/calculator/none.rb | 13 +++++++++++++ config/application.rb | 6 ++++-- spec/system/admin/payment_method_spec.rb | 8 ++++++++ spec/system/admin/shipping_methods_spec.rb | 9 +++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 app/models/calculator/none.rb diff --git a/app/models/calculator/none.rb b/app/models/calculator/none.rb new file mode 100644 index 0000000000..92e3cbdfac --- /dev/null +++ b/app/models/calculator/none.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: false + +module Calculator + class None < Spree::Calculator + def self.description + I18n.t(:none) + end + + def compute(_object = nil) + 0 + end + end +end diff --git a/config/application.rb b/config/application.rb index 24880eafe0..106b3ed1a4 100644 --- a/config/application.rb +++ b/config/application.rb @@ -116,7 +116,8 @@ module Openfoodnetwork Calculator::FlexiRate, Calculator::PerItem, Calculator::PriceSack, - Calculator::Weight + Calculator::Weight, + Calculator::None ] app.config.spree.calculators.add_class('enterprise_fees') @@ -135,7 +136,8 @@ module Openfoodnetwork Calculator::FlatRate, Calculator::FlexiRate, Calculator::PerItem, - Calculator::PriceSack + Calculator::PriceSack, + Calculator::None ] app.config.spree.calculators.add_class('tax_rates') diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index 5af8257dd7..6210f6b72e 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -251,6 +251,14 @@ describe ' let!(:payment_method) { create(:payment_method, calculator: calculator) } before { login_as_admin_and_visit spree.edit_admin_payment_method_path payment_method } + it "handle the 'None' calculator" do + select2_select "None", from: 'calc_type' + click_button 'Update' + expect(page).to have_content("Payment Method has been successfully updated!") + expect(payment_method.reload.calculator_type).to eq "Calculator::None" + expect(page).to have_select "calc_type", selected: "None" + end + context "using Flat Percent calculator" do before { select2_select "Flat Percent", from: 'calc_type' } diff --git a/spec/system/admin/shipping_methods_spec.rb b/spec/system/admin/shipping_methods_spec.rb index 5ba477da16..ca3958fc4f 100644 --- a/spec/system/admin/shipping_methods_spec.rb +++ b/spec/system/admin/shipping_methods_spec.rb @@ -83,6 +83,15 @@ describe 'shipping methods' do expect(@shipping_method.reload.calculator_type).to eq("Calculator::PerItem") end + + it "handle when updating calculator type to 'None'" do + visit spree.edit_admin_shipping_method_path(@shipping_method) + + select2_select 'None', from: 'calc_type' + click_button 'Update' + + expect(@shipping_method.reload.calculator_type).to eq "Calculator::None" + end end context "as an enterprise user", js: true do From 7015cb30c3c765a6728fb35f8d4ce8d164b865d0 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 10 Jan 2023 11:38:36 +0100 Subject: [PATCH 2/5] Calculator 'None' is the default one for both shipping and payment method + update specs as well Update shipping_method.rb --- app/models/spree/payment_method.rb | 2 +- app/models/spree/shipping_method.rb | 6 ++++++ spec/system/admin/payment_method_spec.rb | 11 ++++++++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index d09c3c7c4c..f90a526f7c 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -113,7 +113,7 @@ module Spree end def init - self.calculator ||= ::Calculator::FlatRate.new(preferred_amount: 0) + self.calculator ||= ::Calculator::None.new end def has_distributor?(distributor) diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 589bc8ce8c..9c568fcd21 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -32,6 +32,8 @@ module Spree validate :at_least_one_shipping_category validates :display_on, inclusion: { in: DISPLAY_ON_OPTIONS.values }, allow_nil: true + after_initialize :init + after_save :touch_distributors scope :managed_by, lambda { |user| @@ -110,6 +112,10 @@ module Spree where(display_on: [nil, ""]) end + def init + self.calculator ||= ::Calculator::None.new + end + private def no_active_or_upcoming_order_cycle_distributors_with_only_one_shipping_method? diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index 6210f6b72e..55b2a412ec 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -247,10 +247,13 @@ describe ' end describe "Setting transaction fees", js: true do - let(:calculator) { build(:calculator) } - let!(:payment_method) { create(:payment_method, calculator: calculator) } + let!(:payment_method) { create(:payment_method) } before { login_as_admin_and_visit spree.edit_admin_payment_method_path payment_method } + it "set by default 'None' as calculator" do + expect(page).to have_select "calc_type", selected: "None" + end + it "handle the 'None' calculator" do select2_select "None", from: 'calc_type' click_button 'Update' @@ -273,9 +276,11 @@ describe ' end context "using Flat Rate (per order) calculator" do - # flat rate per order is the default calculator; no need select it and update page + before { select2_select "Flat Rate (per order)", from: 'calc_type' } it "inserts values which persist" do + expect(page).to have_content("you must save first before") + click_button 'Update' fill_in "Amount", with: 2.2 click_button 'Update' expect(page).to have_content("Payment Method has been successfully updated!") From a9097df1121c3bb7e7fe1fc49a8fef55a91f33f5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 10 Jan 2023 12:32:07 +0100 Subject: [PATCH 3/5] Already defined via `CalculatedAdjustments` --- app/models/spree/shipping_method.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 9c568fcd21..3105c11127 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -69,10 +69,6 @@ module Spree tracking_url.gsub(/:tracking/, tracking) unless tracking.blank? || tracking_url.blank? end - def self.calculators - spree_calculators.__send__ model_name_without_spree_namespace - end - # Some shipping methods are only meant to be set via backend def frontend? display_on != "back_end" From 719025f98e9566b37ffd677f20da94e9bf5fcd0f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 12 Jan 2023 17:12:03 +0100 Subject: [PATCH 4/5] Only set calculator if it's a new record (having an id) Unless we have a ActiveRecord::NotNullViolation ``` ActiveRecord::NotNullViolation: PG::NotNullViolation: ERROR: null value in column "calculable_id" of relation "spree_calculators" violates not-null constraint DETAIL: Failing row contains (9370, Calculator::None, null, Spree::ShippingMethod, 2023-01-12 15:09:44.381142, 2023-01-12 15:09:44.381142). # ------------------ # --- Caused by: --- # PG::NotNullViolation: # ERROR: null value in column "calculable_id" of relation "spree_calculators" violates not-null constraint # DETAIL: Failing row contains (9370, Calculator::None, null, Spree::ShippingMethod, 2023-01-12 15:09:44.381142, 2023-01-12 15:09:44.381142). ``` --- app/models/spree/shipping_method.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 3105c11127..b675e8fa1e 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -109,7 +109,7 @@ module Spree end def init - self.calculator ||= ::Calculator::None.new + self.calculator ||= ::Calculator::None.new if new_record? end private From 5a61722f8bb7c331c0fe5ca7fb97a75f951b07f4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 12 Jan 2023 17:30:33 +0100 Subject: [PATCH 5/5] + force `FlatRate` as calculator for payment methods in spec --- .../enterprise_fee_summary_report_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb index 007667eb80..6ea086c7dd 100644 --- a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb +++ b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb @@ -646,12 +646,12 @@ describe Reporting::Reports::EnterpriseFeeSummary::Base do describe "for specified payment methods" do let!(:payment_method_a) do - method = create(:payment_method, name: "Payment A", distributors: [distributor]) + method = create(:payment_method, :flat_rate, name: "Payment A", distributors: [distributor]) method.calculator.update_attribute(:preferred_amount, 1) method end let!(:payment_method_b) do - method = create(:payment_method, name: "Payment B", distributors: [distributor]) + method = create(:payment_method, :flat_rate, name: "Payment B", distributors: [distributor]) method.calculator.update_attribute(:preferred_amount, 1) method end