From 71576fd7db18bb3dd10a5352fab3ec3f71ed1fe4 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sat, 14 Mar 2020 11:04:16 +0100 Subject: [PATCH 1/4] Refactor PerItem calculator to ease readability --- app/models/spree/calculator/per_item_decorator.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/spree/calculator/per_item_decorator.rb b/app/models/spree/calculator/per_item_decorator.rb index 1c5369b48a..e385ad8bf8 100644 --- a/app/models/spree/calculator/per_item_decorator.rb +++ b/app/models/spree/calculator/per_item_decorator.rb @@ -13,14 +13,15 @@ module Spree def compute(object = nil) return 0 if object.nil? - preferred_amount * line_items_for(object).reduce(0) do |sum, value| - value_to_add = if matching_products.blank? || matching_products.include?(value.product) - value.quantity + number_of_line_items = line_items_for(object).reduce(0) do |sum, line_item| + value_to_add = if matching_products.blank? || matching_products.include?(line_item.product) + line_item.quantity else 0 end sum + value_to_add end + preferred_amount * number_of_line_items end end end From 5a83b12c6643b65b7c15322da859bdd7e4724546 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sat, 14 Mar 2020 11:04:49 +0100 Subject: [PATCH 2/4] Make EnterpriseFee report SQL readable --- .../reports/enterprise_fee_summary/scope.rb | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb index d6bcb8554a..96fba8b7a4 100644 --- a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb +++ b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb @@ -342,12 +342,21 @@ module OrderManagement def group_data chain_to_scope do - group("enterprise_fees.id", "enterprises.id", "customers.id", "hubs.id", - "spree_payment_methods.id", "spree_shipping_methods.id", - "adjustment_metadata.enterprise_role", "spree_tax_categories.id", - "product_tax_categories.id", "spree_adjustments.source_type", - "adjustment_source_distributors.id", "incoming_exchange_enterprises.id", - "outgoing_exchange_enterprises.id") + group( + "enterprise_fees.id", + "enterprises.id", + "customers.id", + "hubs.id", + "spree_payment_methods.id", + "spree_shipping_methods.id", + "adjustment_metadata.enterprise_role", + "spree_tax_categories.id", + "product_tax_categories.id", + "spree_adjustments.source_type", + "adjustment_source_distributors.id", + "incoming_exchange_enterprises.id", + "outgoing_exchange_enterprises.id" + ) end end @@ -355,12 +364,16 @@ module OrderManagement chain_to_scope do select( <<-JOIN_STRING.strip_heredoc - SUM(spree_adjustments.amount) AS total_amount, spree_payment_methods.name AS - payment_method_name, spree_shipping_methods.name AS shipping_method_name, - hubs.name AS hub_name, enterprises.name AS enterprise_name, - enterprise_fees.fee_type AS fee_type, customers.name AS customer_name, - customers.email AS customer_email, enterprise_fees.fee_type AS fee_type, - enterprise_fees.name AS fee_name, spree_tax_categories.name AS tax_category_name, + SUM(spree_adjustments.amount) AS total_amount, + spree_payment_methods.name AS payment_method_name, + spree_shipping_methods.name AS shipping_method_name, + hubs.name AS hub_name, + enterprises.name AS enterprise_name, + enterprise_fees.fee_type AS fee_type, + customers.name AS customer_name, + customers.email AS customer_email, + enterprise_fees.name AS fee_name, + spree_tax_categories.name AS tax_category_name, enterprise_fees.inherits_tax_category AS enterprise_fee_inherits_tax_category, product_tax_categories.name AS product_tax_category_name, adjustment_metadata.enterprise_role AS placement_enterprise_role, From 05eadac935ca490e23345f9f58c0cf89343305dd Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sat, 14 Mar 2020 11:05:21 +0100 Subject: [PATCH 3/4] Fix some filtering test cases for fee report The calculators of the adjustments related to the enterprise fees created in the test setup have a preferred_amount of 0. So, when computed, the adjustments' amounts end up being `0 = 0 * 1 line_item`. Then, the ReportService filters these out in `#exclude_groups_with_zero_total` from the result set. This is why the assertions can't find them in `totals`. --- .../enterprise_fee_summary/report_service_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb index cd75340b35..ca7bfab0f5 100644 --- a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb +++ b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb @@ -517,9 +517,9 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do let!(:producer_b) { create(:supplier_enterprise, name: "Producer B") } let!(:producer_c) { create(:supplier_enterprise, name: "Producer C") } - let!(:fee_a) { create(:enterprise_fee, name: "Fee A", enterprise: producer_a) } - let!(:fee_b) { create(:enterprise_fee, name: "Fee B", enterprise: producer_b) } - let!(:fee_c) { create(:enterprise_fee, name: "Fee C", enterprise: producer_c) } + let!(:fee_a) { create(:enterprise_fee, name: "Fee A", enterprise: producer_a, amount: 1) } + let!(:fee_b) { create(:enterprise_fee, name: "Fee B", enterprise: producer_b, amount: 1) } + let!(:fee_c) { create(:enterprise_fee, name: "Fee C", enterprise: producer_c, amount: 1) } let!(:product_a) { create(:product, supplier: producer_a) } let!(:product_b) { create(:product, supplier: producer_b) } @@ -588,9 +588,9 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do end describe "for specified enterprise fees" do - let!(:fee_a) { create(:enterprise_fee, name: "Fee A", enterprise: distributor) } - let!(:fee_b) { create(:enterprise_fee, name: "Fee B", enterprise: distributor) } - let!(:fee_c) { create(:enterprise_fee, name: "Fee C", enterprise: distributor) } + let!(:fee_a) { create(:enterprise_fee, name: "Fee A", enterprise: distributor, amount: 1) } + let!(:fee_b) { create(:enterprise_fee, name: "Fee B", enterprise: distributor, amount: 1) } + let!(:fee_c) { create(:enterprise_fee, name: "Fee C", enterprise: distributor, amount: 1) } let!(:variant) { prepare_variant(outgoing_exchange_fees: variant_outgoing_exchange_fees) } let!(:variant_outgoing_exchange_fees) { [fee_a, fee_b, fee_c] } From 6486e5f9082a3d01617e3eb5bafa0abdd5d7304e Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 16 Mar 2020 13:17:11 +0100 Subject: [PATCH 4/4] Provide a non-zero amount for ship/pay calculator This way the adjustment's total_amount is not 0 and thus, not filtered out but the report service. --- .../report_service_spec.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb index ca7bfab0f5..a8575168b3 100644 --- a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb +++ b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb @@ -610,10 +610,14 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do describe "for specified shipping methods" do let!(:shipping_method_a) do - create(:shipping_method, name: "Shipping A", distributors: [distributor]) + method = create(:shipping_method, name: "Shipping A", distributors: [distributor]) + method.calculator.update_attribute(:preferred_amount, 1) + method end let!(:shipping_method_b) do - create(:shipping_method, name: "Shipping B", distributors: [distributor]) + method = create(:shipping_method, name: "Shipping B", distributors: [distributor]) + method.calculator.update_attribute(:preferred_amount, 1) + method end let!(:shipping_method_c) do create(:shipping_method, name: "Shipping C", distributors: [distributor]) @@ -638,10 +642,14 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do describe "for specified payment methods" do let!(:payment_method_a) do - create(:payment_method, name: "Payment A", distributors: [distributor]) + method = create(:payment_method, name: "Payment A", distributors: [distributor]) + method.calculator.update_attribute(:preferred_amount, 1) + method end let!(:payment_method_b) do - create(:payment_method, name: "Payment B", distributors: [distributor]) + method = create(:payment_method, name: "Payment B", distributors: [distributor]) + method.calculator.update_attribute(:preferred_amount, 1) + method end let!(:payment_method_c) do create(:payment_method, name: "Payment C", distributors: [distributor])