From baff4b5399610d615f9c4aa77d20bb353746d096 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 7 Jul 2023 14:19:01 +0200 Subject: [PATCH 01/16] Rename 'distributor' to 'hub' --- app/views/admin/reports/filters/_customers.html.haml | 2 +- config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml index caa88f4e5d..9b70704208 100644 --- a/app/views/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -1,5 +1,5 @@ .row - .alpha.two.columns= label_tag nil, t(:report_customers_distributor) + .alpha.two.columns= label_tag nil, t(:report_customers_hub) .omega.fourteen.columns = select_tag(:distributor_id, options_from_collection_for_select(@data.distributors, :id, :name, params[:distributor_id]), diff --git a/config/locales/en.yml b/config/locales/en.yml index de1194ca81..e82f92235a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2912,6 +2912,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using hub_sidebar_red: "red" order_cycles_closed_for_hub: "The hub you have selected is temporarily closed for orders. Please try again later." report_customers_distributor: "Distributor" + report_customers_hub: "Hub" report_customers_supplier: "Supplier" report_customers_cycle: "Order Cycle" report_customers_type: "Report Type" From 9e295146cbd6f30076038022d3295c0917283128 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 7 Jul 2023 14:20:58 +0200 Subject: [PATCH 02/16] Remove supplier filter --- .../admin/reports/filters/_customers.html.haml | 6 ------ lib/reporting/reports/customers/base.rb | 16 +--------------- spec/lib/reports/customers_report_spec.rb | 14 -------------- 3 files changed, 1 insertion(+), 35 deletions(-) diff --git a/app/views/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml index 9b70704208..a72720c9e9 100644 --- a/app/views/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -4,12 +4,6 @@ = select_tag(:distributor_id, options_from_collection_for_select(@data.distributors, :id, :name, params[:distributor_id]), {:include_blank => true, :class => "select2 fullwidth light"}) -.row - .alpha.two.columns= label_tag nil, t(:report_customers_supplier) - .omega.fourteen.columns - = select_tag(:supplier_id, - options_from_collection_for_select(@data.suppliers, :id, :name, params[:supplier_id]), - {:include_blank => true, :class => "select2 fullwidth light"}) .row .alpha.two.columns= label_tag nil, t(:report_customers_cycle) .omega.fourteen.columns diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb index cad550789d..759178b6df 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -12,21 +12,7 @@ module Reporting end def filter(orders) - filter_to_supplier filter_to_distributor filter_to_order_cycle orders - end - - def filter_to_supplier(orders) - if params[:supplier_id].to_i > 0 - orders.select do |order| - order.line_items.includes(:product) - .where("spree_products.supplier_id = ?", params[:supplier_id].to_i) - .references(:product) - .count - .positive? - end - else - orders - end + filter_to_distributor filter_to_order_cycle orders end def filter_to_distributor(orders) diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 005e486199..c5f987885e 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -284,20 +284,6 @@ module Reporting expect(subject.filter(orders)).to eq(orders) end - it "returns orders with a specific supplier" do - supplier = create(:supplier_enterprise) - supplier2 = create(:supplier_enterprise) - product1 = create(:simple_product, supplier:) - product2 = create(:simple_product, supplier: supplier2) - order1 = create(:order) - order2 = create(:order) - order1.line_items << create(:line_item, product: product1) - order2.line_items << create(:line_item, product: product2) - - allow(subject).to receive(:params).and_return(supplier_id: supplier.id) - expect(subject.filter(orders)).to eq([order1]) - end - it "filters to a specific distributor" do d1 = create(:distributor_enterprise) d2 = create(:distributor_enterprise) From 4f332504af69d4a158c5c9c079b8ef60771e4a64 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 7 Jul 2023 14:21:13 +0200 Subject: [PATCH 03/16] Add new line at the end of file --- app/views/admin/reports/filters/_customers.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml index a72720c9e9..f4aa2ca77b 100644 --- a/app/views/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -9,4 +9,4 @@ .omega.fourteen.columns = select_tag(:order_cycle_id, options_for_select(report_order_cycle_options(@data.order_cycles), params[:order_cycle_id]), - {:include_blank => true, :class => "select2 fullwidth light"}) \ No newline at end of file + {:include_blank => true, :class => "select2 fullwidth light"}) From c134de850cdde7ed55421af4ddaf75ae5b59b821 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 09:26:34 +0200 Subject: [PATCH 04/16] Add filter on `completed_at` of an order --- .../admin/reports/filters/_customers.html.haml | 2 ++ lib/reporting/reports/customers/base.rb | 14 +++++++++++++- spec/lib/reports/customers_report_spec.rb | 10 ++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/views/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml index f4aa2ca77b..8cbb78b39b 100644 --- a/app/views/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -1,3 +1,5 @@ += render 'admin/reports/date_range_form', f: f + .row .alpha.two.columns= label_tag nil, t(:report_customers_hub) .omega.fourteen.columns diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb index 759178b6df..1773be4620 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -12,7 +12,19 @@ module Reporting end def filter(orders) - filter_to_distributor filter_to_order_cycle orders + filter_to_completed_at filter_to_distributor filter_to_order_cycle orders + end + + def filter_to_completed_at(orders) + if params[:q] && + params[:q][:completed_at_gt].present? && + params[:q][:completed_at_lt].present? + orders.where("completed_at >= ? AND completed_at <= ?", + params[:q][:completed_at_gt], + params[:q][:completed_at_lt]) + else + orders + end end def filter_to_distributor(orders) diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index c5f987885e..df8c3e4aa9 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -284,6 +284,16 @@ module Reporting expect(subject.filter(orders)).to eq(orders) end + it "filters to a specific completed_at date" do + o1 = create(:order, completed_at: 1.day.ago) + o2 = create(:order, completed_at: 3.days.ago) + o3 = create(:order, completed_at: 5.days.ago) + + allow(subject).to receive(:params).and_return({ q: { completed_at_gt: 1.day.before(o2.completed_at), + completed_at_lt: 1.day.after(o2.completed_at) } }) + expect(subject.filter(orders)).to eq([o2]) + end + it "filters to a specific distributor" do d1 = create(:distributor_enterprise) d2 = create(:distributor_enterprise) From cc26da6560e4ba1d39f841b59a8c08ac4df7f11d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 11:26:25 +0200 Subject: [PATCH 05/16] We'll need to get all the orders per line to count and sum them; prepare it --- lib/reporting/reports/customers/addresses.rb | 18 +++++++++--------- spec/lib/reports/customers_report_spec.rb | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb index 28362586f7..569a509224 100644 --- a/lib/reporting/reports/customers/addresses.rb +++ b/lib/reporting/reports/customers/addresses.rb @@ -15,19 +15,19 @@ module Reporting hub_id: order.distributor_id, shipping_method_id: order.shipping_method&.id, } - end.values.map(&:first) + end.values end def columns { - first_name: proc { |order| order.billing_address.firstname }, - last_name: proc { |order| order.billing_address.lastname }, - billing_address: proc { |order| order.billing_address.address_and_city }, - email: proc { |order| order.email }, - phone: proc { |order| order.billing_address.phone }, - hub: proc { |order| order.distributor&.name }, - hub_address: proc { |order| order.distributor&.address&.address_and_city }, - shipping_method: proc { |order| order.shipping_method&.name }, + first_name: proc { |orders| orders.first.billing_address.firstname }, + last_name: proc { |orders| orders.first.billing_address.lastname }, + billing_address: proc { |orders| orders.first.billing_address.address_and_city }, + email: proc { |orders| orders.first.email }, + phone: proc { |orders| orders.first.billing_address.phone }, + hub: proc { |orders| orders.first.distributor&.name }, + hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, + shipping_method: proc { |orders| orders.first.shipping_method&.name }, } end diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index df8c3e4aa9..fdd7926328 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -74,7 +74,7 @@ module Reporting o = create(:order, distributor: d, bill_address: a) o.shipments << create(:shipment) - allow(subject).to receive(:query_result).and_return [o] + allow(subject).to receive(:query_result).and_return [[o]] expect(subject.table_rows).to eq([[ a.firstname, a.lastname, [a.address1, a.address2, a.city].join(" "), @@ -106,7 +106,7 @@ module Reporting end it "returns only one row per customer" do - expect(subject.query_result).to match_array [o1] + expect(subject.query_result).to match_array [[o1, o2]] expect(subject.table_rows.size).to eq(1) expect(subject.table_rows) .to eq([[ From 137820273226a425d293fa802fff237c73c8752d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 11:32:57 +0200 Subject: [PATCH 06/16] Disable rubocop warning. Not sure how to handle it actually --- lib/reporting/reports/customers/addresses.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb index 569a509224..964de8ce5c 100644 --- a/lib/reporting/reports/customers/addresses.rb +++ b/lib/reporting/reports/customers/addresses.rb @@ -18,6 +18,7 @@ module Reporting end.values end + # rubocop:disable Metrics/AbcSize def columns { first_name: proc { |orders| orders.first.billing_address.firstname }, @@ -30,6 +31,7 @@ module Reporting shipping_method: proc { |orders| orders.first.shipping_method&.name }, } end + # rubocop:enable Metrics/AbcSize def skip_duplicate_rows? true From d55098f94fcb28ebb0851d82267c9e48bb16ea5b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 11:48:59 +0200 Subject: [PATCH 07/16] Add the number of orders for the customer --- lib/reporting/reports/customers/addresses.rb | 1 + spec/lib/reports/customers_report_spec.rb | 18 +++++++++--------- spec/system/admin/reports_spec.rb | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb index 964de8ce5c..1790613e4c 100644 --- a/lib/reporting/reports/customers/addresses.rb +++ b/lib/reporting/reports/customers/addresses.rb @@ -29,6 +29,7 @@ module Reporting hub: proc { |orders| orders.first.distributor&.name }, hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, shipping_method: proc { |orders| orders.first.shipping_method&.name }, + total_orders: proc { |orders| orders.count }, } end # rubocop:enable Metrics/AbcSize diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index fdd7926328..e49b10c267 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -65,7 +65,7 @@ module Reporting it "returns headers for addresses" do expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method"]) + "Shipping Method", "Total Number of Orders"]) end it "builds a table from a list of variants" do @@ -81,7 +81,7 @@ module Reporting o.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o.shipping_method.name + o.shipping_method.name, 1 ]]) end @@ -105,7 +105,7 @@ module Reporting end end - it "returns only one row per customer" do + it "returns only one row per customer and count the number of orders" do expect(subject.query_result).to match_array [[o1, o2]] expect(subject.table_rows.size).to eq(1) expect(subject.table_rows) @@ -114,7 +114,7 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name + o1.shipping_method.name, 2 ]]) end @@ -136,13 +136,13 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name + o1.shipping_method.name, 1 ], [ a.firstname, a.lastname, [a.address1, a.address2, a.city].join(" "), o2.email, a.phone, d2.name, [d2.address.address1, d2.address.address2, d2.address.city].join(" "), - o2.shipping_method.name + o2.shipping_method.name, 1 ]]) end end @@ -161,7 +161,7 @@ module Reporting context "when the shipping method column is being included" do let(:fields_to_show) do [:first_name, :last_name, :billing_address, :email, :phone, :hub, :hub_address, - :shipping_method] + :shipping_method, :total_orders] end subject { Addresses.new(user, { fields_to_show: }) } @@ -178,7 +178,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name + o1.shipping_method.name, 1 ], [ a.firstname, @@ -188,7 +188,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - sm2.name + sm2.name, 1 ] ] ) diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index e5c3b6863c..174974d885 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -173,7 +173,7 @@ describe ' table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([ ["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method"].map(&:upcase) + "Shipping Method", "Total Number of Orders"].map(&:upcase) ].sort) end end From d2fbaa7cfd8384382f6a3ab890c03908f5b961b3 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 12:23:49 +0200 Subject: [PATCH 08/16] Add total amount spent at the shop for the customer --- lib/reporting/reports/customers/addresses.rb | 1 + spec/lib/reports/customers_report_spec.rb | 16 ++++++++-------- spec/system/admin/reports_spec.rb | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb index 1790613e4c..cb3936ba94 100644 --- a/lib/reporting/reports/customers/addresses.rb +++ b/lib/reporting/reports/customers/addresses.rb @@ -30,6 +30,7 @@ module Reporting hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, shipping_method: proc { |orders| orders.first.shipping_method&.name }, total_orders: proc { |orders| orders.count }, + total_incl_tax: proc { |orders| orders.sum(&:total) }, } end # rubocop:enable Metrics/AbcSize diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index e49b10c267..f1a0187f03 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -65,7 +65,7 @@ module Reporting it "returns headers for addresses" do expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method", "Total Number of Orders"]) + "Shipping Method", "Total Number of Orders", "Total incl. tax ($)"]) end it "builds a table from a list of variants" do @@ -81,7 +81,7 @@ module Reporting o.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o.shipping_method.name, 1 + o.shipping_method.name, 1, o.total ]]) end @@ -114,7 +114,7 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 2 + o1.shipping_method.name, 2, o1.total + o2.total ]]) end @@ -136,13 +136,13 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 1 + o1.shipping_method.name, 1, o1.total ], [ a.firstname, a.lastname, [a.address1, a.address2, a.city].join(" "), o2.email, a.phone, d2.name, [d2.address.address1, d2.address.address2, d2.address.city].join(" "), - o2.shipping_method.name, 1 + o2.shipping_method.name, 1, o2.total ]]) end end @@ -161,7 +161,7 @@ module Reporting context "when the shipping method column is being included" do let(:fields_to_show) do [:first_name, :last_name, :billing_address, :email, :phone, :hub, :hub_address, - :shipping_method, :total_orders] + :shipping_method, :total_orders, :total_incl_tax] end subject { Addresses.new(user, { fields_to_show: }) } @@ -178,7 +178,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 1 + o1.shipping_method.name, 1, o1.total ], [ a.firstname, @@ -188,7 +188,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - sm2.name, 1 + sm2.name, 1, o2.total ] ] ) diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 174974d885..5bc9f1d843 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -173,7 +173,7 @@ describe ' table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([ ["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method", "Total Number of Orders"].map(&:upcase) + "Shipping Method", "Total Number of Orders", "Total incl. tax ($)"].map(&:upcase) ].sort) end end From c6c9cdca6557f19b2d4230d8b249001dded3b5b5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 13:39:51 +0200 Subject: [PATCH 09/16] Add last completed order date --- config/locales/en.yml | 1 + lib/reporting/reports/customers/addresses.rb | 5 +++++ spec/lib/reports/customers_report_spec.rb | 21 +++++++++++--------- spec/system/admin/reports_spec.rb | 3 ++- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index e82f92235a..f676290ebe 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3107,6 +3107,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using report_header_total_taxable_admin: Total taxable admin adjustments (tax inclusive) report_line_cost_of_produce: Cost of produce report_line_line_items: line items + report_header_last_completed_order_date: Last completed order date report_xero_configuration: Xero Configuration initial_invoice_number: "Initial invoice number" invoice_date: "Invoice date" diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb index cb3936ba94..a5540e40d3 100644 --- a/lib/reporting/reports/customers/addresses.rb +++ b/lib/reporting/reports/customers/addresses.rb @@ -19,6 +19,7 @@ module Reporting end # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/CyclomaticComplexity def columns { first_name: proc { |orders| orders.first.billing_address.firstname }, @@ -31,9 +32,13 @@ module Reporting shipping_method: proc { |orders| orders.first.shipping_method&.name }, total_orders: proc { |orders| orders.count }, total_incl_tax: proc { |orders| orders.sum(&:total) }, + last_completed_order_date: proc { |orders| + orders.max_by(&:completed_at)&.completed_at&.to_date + }, } end # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/CyclomaticComplexity def skip_duplicate_rows? true diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index f1a0187f03..529664b97c 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -65,7 +65,8 @@ module Reporting it "returns headers for addresses" do expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method", "Total Number of Orders", "Total incl. tax ($)"]) + "Shipping Method", "Total Number of Orders", "Total incl. tax ($)", + "Last completed order date"]) end it "builds a table from a list of variants" do @@ -81,7 +82,7 @@ module Reporting o.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o.shipping_method.name, 1, o.total + o.shipping_method.name, 1, o.total, "none" ]]) end @@ -100,12 +101,14 @@ module Reporting shipping_method: sm) } before do + o1.update(completed_at: Time.zone.yesterday) + o2.update(completed_at: Time.zone.today) [o1, o2].each do |order| order.update!(email: "test@test.com") end end - it "returns only one row per customer and count the number of orders" do + it "returns only one row per customer with the right data" do expect(subject.query_result).to match_array [[o1, o2]] expect(subject.table_rows.size).to eq(1) expect(subject.table_rows) @@ -114,7 +117,7 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 2, o1.total + o2.total + o1.shipping_method.name, 2, o1.total + o2.total, o2.completed_at.strftime("%Y-%m-%d") ]]) end @@ -136,13 +139,13 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 1, o1.total + o1.shipping_method.name, 1, o1.total, o1.completed_at.strftime("%Y-%m-%d") ], [ a.firstname, a.lastname, [a.address1, a.address2, a.city].join(" "), o2.email, a.phone, d2.name, [d2.address.address1, d2.address.address2, d2.address.city].join(" "), - o2.shipping_method.name, 1, o2.total + o2.shipping_method.name, 1, o2.total, o2.completed_at.strftime("%Y-%m-%d") ]]) end end @@ -161,7 +164,7 @@ module Reporting context "when the shipping method column is being included" do let(:fields_to_show) do [:first_name, :last_name, :billing_address, :email, :phone, :hub, :hub_address, - :shipping_method, :total_orders, :total_incl_tax] + :shipping_method, :total_orders, :total_incl_tax, :last_completed_order_date] end subject { Addresses.new(user, { fields_to_show: }) } @@ -178,7 +181,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 1, o1.total + o1.shipping_method.name, 1, o1.total, o1.completed_at.strftime("%Y-%m-%d") ], [ a.firstname, @@ -188,7 +191,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - sm2.name, 1, o2.total + sm2.name, 1, o2.total, o2.completed_at.strftime("%Y-%m-%d") ] ] ) diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 5bc9f1d843..447e557e40 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -173,7 +173,8 @@ describe ' table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([ ["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method", "Total Number of Orders", "Total incl. tax ($)"].map(&:upcase) + "Shipping Method", "Total Number of Orders", "Total incl. tax ($)", + "Last completed order date"].map(&:upcase) ].sort) end end From 5edc8d8ce1425e46539056d4361638fc0aeb3a9e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 14:02:35 +0200 Subject: [PATCH 10/16] Delete Mailing List report --- config/locales/en.yml | 1 - .../reports/customers/mailing_list.rb | 29 ------------ lib/reporting/reports/list.rb | 1 - .../admin/reports_controller_spec.rb | 1 - spec/lib/reports/customers_report_spec.rb | 45 ------------------- spec/system/admin/reports_spec.rb | 25 +++-------- 6 files changed, 7 insertions(+), 95 deletions(-) delete mode 100644 lib/reporting/reports/customers/mailing_list.rb diff --git a/config/locales/en.yml b/config/locales/en.yml index f676290ebe..9c7a55f749 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1530,7 +1530,6 @@ en: all_products: All products inventory: Inventory (on hand) lettuce_share: LettuceShare - mailing_list: Mailing List addresses: Addresses payment_methods: Payment Methods Report delivery: Delivery Report diff --git a/lib/reporting/reports/customers/mailing_list.rb b/lib/reporting/reports/customers/mailing_list.rb deleted file mode 100644 index b334ce93e0..0000000000 --- a/lib/reporting/reports/customers/mailing_list.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Reporting - module Reports - module Customers - class MailingList < Base - def query_result - super.group_by do |order| - { - email: order.email, - first_name: order.billing_address.firstname, - last_name: order.billing_address.lastname, - suburb: order.billing_address.city, - } - end.values.map(&:first) - end - - def columns - { - email: proc { |order| order.email }, - first_name: proc { |order| order.billing_address.firstname }, - last_name: proc { |order| order.billing_address.lastname }, - suburb: proc { |order| order.billing_address.city }, - } - end - end - end - end -end diff --git a/lib/reporting/reports/list.rb b/lib/reporting/reports/list.rb index c15742fbef..4e129ac3b8 100644 --- a/lib/reporting/reports/list.rb +++ b/lib/reporting/reports/list.rb @@ -55,7 +55,6 @@ module Reporting def customers_report_types [ - [i18n_translate("mailing_list"), :mailing_list], [i18n_translate("addresses"), :addresses] ] end diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index b7eff12bcb..25ebfff0ce 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -262,7 +262,6 @@ describe Admin::ReportsController, type: :controller do it "should have report types for customers" do expect(subject.reports[:customers]).to eq([ - ["Mailing List", :mailing_list], ["Addresses", :addresses] ]) end diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 529664b97c..c54772aec0 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -14,51 +14,6 @@ module Reporting end subject { Base.new user, {} } - describe "mailing list report" do - subject { MailingList.new user, {} } - - it "returns headers for mailing_list" do - expect(subject.table_headers).to eq(["Email", "First Name", "Last Name", "Suburb"]) - end - - it "builds a table from a list of variants" do - order = double(:order, email: "test@test.com") - address = double(:billing_address, firstname: "Firsty", - lastname: "Lasty", city: "Suburbia") - allow(order).to receive(:billing_address).and_return address - allow(subject).to receive(:query_result).and_return [order] - - expect(subject.table_rows).to eq([[ - "test@test.com", "Firsty", "Lasty", "Suburbia" - ]]) - end - - context "when there are multiple orders for the same customer" do - let!(:address) { - create(:bill_address, firstname: "Firsty", - lastname: "Lasty", city: "Suburbia") - } - let!(:order1) { - create(:order_with_totals_and_distribution, :completed, bill_address: address) - } - let!(:order2) { - create(:order_with_totals_and_distribution, :completed, bill_address: address) - } - before do - [order1, order2].each do |order| - order.update!(email: "test@test.com") - end - end - it "returns only one row per customer" do - expect(subject.query_result).to match_array [order1] - expect(subject.table_rows.size).to eq(1) - expect(subject.table_rows).to eq([[ - "test@test.com", "Firsty", "Lasty", "Suburbia" - ]]) - end - end - end - describe "addresses report" do subject { Addresses.new user, {} } diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 447e557e40..9e595d1250 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -39,12 +39,12 @@ describe ' it "can run the customers report" do login_as_admin visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list + report_type: :customers, report_subtype: :addresses ) generate_report expect(page).to have_selector "#report-table" - expect(page).to have_content "EMAIL FIRST NAME" + expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" end it "renders UTF-8 characters" do @@ -64,18 +64,18 @@ describe ' # Run the report: login_as_admin visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list + report_type: :customers, report_subtype: :addresses ) generate_report expect(page).to have_content "Späti" - expect(page).to have_content "EMAIL FIRST NAME" + expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" expect(page).to have_content "Müller" end it "displays a friendly timeout message and offers download" do login_as_admin visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list + report_type: :customers, report_subtype: :addresses ) stub_const("ReportJob::NOTIFICATION_TIME", 0) @@ -113,7 +113,7 @@ describe ' it "allows the report to finish before the loading screen is rendered" do login_as_admin visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list + report_type: :customers, report_subtype: :addresses ) # The controller wants to execute the ReportJob in the background. @@ -130,7 +130,7 @@ describe ' click_button "Go" expect(page).to have_selector "#report-table table" - expect(page).to have_content "EMAIL FIRST NAME" + expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" # Now that we see the report, we need to make sure that it's not replaced # by the "loading" spinner when the controller action finishes. @@ -154,17 +154,6 @@ describe ' visit admin_reports_path end - it "customers report" do - click_link "Mailing List" - click_button "Go" - - rows = find("table.report__table").all("thead tr") - table = rows.map { |r| r.all("th").map { |c| c.text.strip } } - expect(table.sort).to eq([ - ["Email", "First Name", "Last Name", "Suburb"].map(&:upcase) - ].sort) - end - it "customers report" do click_link "Addresses" click_button "Go" From 82ccdcca706757671982c5acfc0ed5a28d406a3d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 15:03:56 +0200 Subject: [PATCH 11/16] Customers report has only one report: Customers No more `addresses` report + Fix pre-existing rubocop issues + Create method to simplify and remove CyclomaticComplexity error --- config/locales/en.yml | 1 - lib/reporting/reports/customers/addresses.rb | 49 ------------------- lib/reporting/reports/customers/base.rb | 43 +++++++++++++++- lib/reporting/reports/list.rb | 8 +-- .../admin/reports_controller_spec.rb | 6 --- spec/lib/reports/customers_report_spec.rb | 12 ++--- spec/system/admin/reports_spec.rb | 20 +++----- 7 files changed, 54 insertions(+), 85 deletions(-) delete mode 100644 lib/reporting/reports/customers/addresses.rb diff --git a/config/locales/en.yml b/config/locales/en.yml index 9c7a55f749..b279f8f91b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1530,7 +1530,6 @@ en: all_products: All products inventory: Inventory (on hand) lettuce_share: LettuceShare - addresses: Addresses payment_methods: Payment Methods Report delivery: Delivery Report sales_tax_totals_by_producer: Sales Tax Totals By Producer diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb deleted file mode 100644 index a5540e40d3..0000000000 --- a/lib/reporting/reports/customers/addresses.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -module Reporting - module Reports - module Customers - class Addresses < Base - def query_result - super.group_by do |order| - { - first_name: order.billing_address.firstname, - last_name: order.billing_address.lastname, - billing_address: order.billing_address.address_and_city, - email: order.email, - phone: order.billing_address.phone, - hub_id: order.distributor_id, - shipping_method_id: order.shipping_method&.id, - } - end.values - end - - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/CyclomaticComplexity - def columns - { - first_name: proc { |orders| orders.first.billing_address.firstname }, - last_name: proc { |orders| orders.first.billing_address.lastname }, - billing_address: proc { |orders| orders.first.billing_address.address_and_city }, - email: proc { |orders| orders.first.email }, - phone: proc { |orders| orders.first.billing_address.phone }, - hub: proc { |orders| orders.first.distributor&.name }, - hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, - shipping_method: proc { |orders| orders.first.shipping_method&.name }, - total_orders: proc { |orders| orders.count }, - total_incl_tax: proc { |orders| orders.sum(&:total) }, - last_completed_order_date: proc { |orders| - orders.max_by(&:completed_at)&.completed_at&.to_date - }, - } - end - # rubocop:enable Metrics/AbcSize - # rubocop:enable Metrics/CyclomaticComplexity - - def skip_duplicate_rows? - true - end - end - end - end -end diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb index 1773be4620..a2d3880882 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -5,16 +5,51 @@ module Reporting module Customers class Base < ReportTemplate def query_result - filter Spree::Order.managed_by(@user) + filter(Spree::Order.managed_by(@user) .distributed_by_user(@user) .complete.not_state(:canceled) - .order(:id) + .order(:id)) + .group_by do |order| + { + first_name: order.billing_address.firstname, + last_name: order.billing_address.lastname, + billing_address: order.billing_address.address_and_city, + email: order.email, + phone: order.billing_address.phone, + hub_id: order.distributor_id, + shipping_method_id: order.shipping_method&.id, + } + end.values end + # rubocop:disable Metrics/AbcSize + def columns + { + first_name: proc { |orders| orders.first.billing_address.firstname }, + last_name: proc { |orders| orders.first.billing_address.lastname }, + billing_address: proc { |orders| orders.first.billing_address.address_and_city }, + email: proc { |orders| orders.first.email }, + phone: proc { |orders| orders.first.billing_address.phone }, + hub: proc { |orders| orders.first.distributor&.name }, + hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, + shipping_method: proc { |orders| orders.first.shipping_method&.name }, + total_orders: proc { |orders| orders.count }, + total_incl_tax: proc { |orders| orders.sum(&:total) }, + last_completed_order_date: proc { |orders| last_completed_order_date(orders) }, + } + end + # rubocop:enable Metrics/AbcSize + def filter(orders) filter_to_completed_at filter_to_distributor filter_to_order_cycle orders end + def skip_duplicate_rows? + true + end + + private + def filter_to_completed_at(orders) if params[:q] && params[:q][:completed_at_gt].present? && @@ -42,6 +77,10 @@ module Reporting orders end end + + def last_completed_order_date(orders) + orders.max_by(&:completed_at)&.completed_at&.to_date + end end end end diff --git a/lib/reporting/reports/list.rb b/lib/reporting/reports/list.rb index 4e129ac3b8..8ce546f590 100644 --- a/lib/reporting/reports/list.rb +++ b/lib/reporting/reports/list.rb @@ -13,7 +13,7 @@ module Reporting bulk_coop: bulk_coop_report_types, payments: payments_report_types, orders_and_fulfillment: orders_and_fulfillment_report_types, - customers: customers_report_types, + customers: [], products_and_inventory: products_and_inventory_report_types, users_and_enterprises: [], enterprise_fee_summary:, @@ -53,12 +53,6 @@ module Reporting ] end - def customers_report_types - [ - [i18n_translate("addresses"), :addresses] - ] - end - def enterprise_fee_summary [ [i18n_translate('enterprise_fee_summary.name'), :fee_summary], diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 25ebfff0ce..4f99de87c6 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -260,12 +260,6 @@ describe Admin::ReportsController, type: :controller do context "My Customers" do before { controller_login_as_admin } - it "should have report types for customers" do - expect(subject.reports[:customers]).to eq([ - ["Addresses", :addresses] - ]) - end - context "with distributors and suppliers" do let(:distributors) { [coordinator1, distributor1, distributor2] } let(:suppliers) { [supplier1, supplier2] } diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index c54772aec0..4edc21c45a 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -15,8 +15,6 @@ module Reporting subject { Base.new user, {} } describe "addresses report" do - subject { Addresses.new user, {} } - it "returns headers for addresses" do expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", @@ -121,7 +119,7 @@ module Reporting [:first_name, :last_name, :billing_address, :email, :phone, :hub, :hub_address, :shipping_method, :total_orders, :total_incl_tax, :last_completed_order_date] end - subject { Addresses.new(user, { fields_to_show: }) } + subject { Base.new(user, { fields_to_show: }) } it "returns one row per customer per shipping method" do expect(subject.query_result.size).to eq(2) @@ -157,7 +155,7 @@ module Reporting let(:fields_to_show) do [:first_name, :last_name, :billing_address, :email, :phone, :hub, :hub_address] end - subject { Addresses.new(user, { fields_to_show: }) } + subject { Base.new(user, { fields_to_show: }) } it "returns a single row for the customer, otherwise it would return two identical rows" do @@ -184,13 +182,13 @@ module Reporting it "fetches completed orders" do o1 = create(:order) o2 = create(:order, completed_at: 1.day.ago) - expect(subject.query_result).to eq([o2]) + expect(subject.query_result).to eq([[o2]]) end it "does not show cancelled orders" do o1 = create(:order, state: "canceled", completed_at: 1.day.ago) o2 = create(:order, completed_at: 1.day.ago) - expect(subject.query_result).to eq([o2]) + expect(subject.query_result).to eq([[o2]]) end end end @@ -220,7 +218,7 @@ module Reporting o2 = create(:order, distributor: d2, completed_at: 1.day.ago) expect(subject).to receive(:filter).with([o1]).and_return([o1]) - expect(subject.query_result).to eq([o1]) + expect(subject.query_result).to eq([[o1]]) end it "does not show orders through a hub that the current user does not manage" do diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 9e595d1250..ee646f8267 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -38,9 +38,7 @@ describe ' it "can run the customers report" do login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :addresses - ) + visit admin_report_path(report_type: :customers) generate_report expect(page).to have_selector "#report-table" @@ -63,9 +61,7 @@ describe ' # Run the report: login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :addresses - ) + visit admin_report_path(report_type: :customers) generate_report expect(page).to have_content "Späti" expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" @@ -74,9 +70,7 @@ describe ' it "displays a friendly timeout message and offers download" do login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :addresses - ) + visit admin_report_path(report_type: :customers) stub_const("ReportJob::NOTIFICATION_TIME", 0) generate_report @@ -112,9 +106,7 @@ describe ' it "allows the report to finish before the loading screen is rendered" do login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :addresses - ) + visit admin_report_path(report_type: :customers) # The controller wants to execute the ReportJob in the background. # But we change the logic here, execute it immediately and then wait @@ -155,7 +147,9 @@ describe ' end it "customers report" do - click_link "Addresses" + within "table.index" do + click_link "Customers" + end click_button "Go" rows = find("table.report__table").all("thead tr") From adc5bf6e938d1284a410c46661dddb2fa311ca74 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 21 Jul 2023 09:24:56 +1000 Subject: [PATCH 12/16] Simplify date filter in customers report --- lib/reporting/reports/customers/base.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb index a2d3880882..09b1cb2170 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -51,15 +51,12 @@ module Reporting private def filter_to_completed_at(orders) - if params[:q] && - params[:q][:completed_at_gt].present? && - params[:q][:completed_at_lt].present? - orders.where("completed_at >= ? AND completed_at <= ?", - params[:q][:completed_at_gt], - params[:q][:completed_at_lt]) - else - orders - end + min = params.dig(:q, :completed_at_gt) + max = params.dig(:q, :completed_at_lt) + + return orders if min.blank? || max.blank? + + orders.where(completed_at: [min..max]) end def filter_to_distributor(orders) From 5eab033a80d611ae220fc57ac71e4b3e91b918e0 Mon Sep 17 00:00:00 2001 From: jibees Date: Fri, 21 Jul 2023 09:29:39 +0200 Subject: [PATCH 13/16] Improve indentation Co-Authored-By: Maikel --- spec/lib/reports/customers_report_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 4edc21c45a..c172e9d766 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -245,8 +245,12 @@ module Reporting o2 = create(:order, completed_at: 3.days.ago) o3 = create(:order, completed_at: 5.days.ago) - allow(subject).to receive(:params).and_return({ q: { completed_at_gt: 1.day.before(o2.completed_at), - completed_at_lt: 1.day.after(o2.completed_at) } }) + allow(subject).to receive(:params).and_return( + q: { + completed_at_gt: 1.day.before(o2.completed_at), + completed_at_lt: 1.day.after(o2.completed_at) + } + ) expect(subject.filter(orders)).to eq([o2]) end From d6c10170da130facb989909ef0ea4bdb510fd88d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 24 Jul 2023 11:39:08 +0200 Subject: [PATCH 14/16] Use plain text instead of computed date in specs https://github.com/openfoodfoundation/openfoodnetwork/wiki/Code-Conventions#prefer-plain-text-over-method-calls-in-expected-values --- spec/lib/reports/customers_report_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index c172e9d766..6dfd70ca51 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -54,8 +54,8 @@ module Reporting shipping_method: sm) } before do - o1.update(completed_at: Time.zone.yesterday) - o2.update(completed_at: Time.zone.today) + o1.update(completed_at: "2023-01-01") + o2.update(completed_at: "2023-01-02") [o1, o2].each do |order| order.update!(email: "test@test.com") end @@ -70,7 +70,7 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 2, o1.total + o2.total, o2.completed_at.strftime("%Y-%m-%d") + o1.shipping_method.name, 2, o1.total + o2.total, "2023-01-02" ]]) end @@ -92,13 +92,13 @@ module Reporting [a.address1, a.address2, a.city].join(" "), o1.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - o1.shipping_method.name, 1, o1.total, o1.completed_at.strftime("%Y-%m-%d") + o1.shipping_method.name, 1, o1.total, "2023-01-01" ], [ a.firstname, a.lastname, [a.address1, a.address2, a.city].join(" "), o2.email, a.phone, d2.name, [d2.address.address1, d2.address.address2, d2.address.city].join(" "), - o2.shipping_method.name, 1, o2.total, o2.completed_at.strftime("%Y-%m-%d") + o2.shipping_method.name, 1, o2.total, "2023-01-02" ]]) end end From 9ce89125a4a248b2808bef09c7e0474cbea6504f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 8 Aug 2023 15:38:08 +0200 Subject: [PATCH 15/16] Fix linter error: line is too long --- spec/lib/reports/customers_report_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 6dfd70ca51..30fa001e47 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -18,7 +18,8 @@ module Reporting it "returns headers for addresses" do expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method", "Total Number of Orders", "Total incl. tax ($)", + "Shipping Method", "Total Number of Orders", + "Total incl. tax ($)", "Last completed order date"]) end From 2803f1c6b23a12a2588a14a78dd316e626733124 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 7 Sep 2023 14:14:27 +0200 Subject: [PATCH 16/16] Group all orders by customer_id, email and distributor_id Therefore have one (and only) row per customer Co-Authored-By: Maikel --- lib/reporting/reports/customers/base.rb | 33 ++++++++++++----------- spec/lib/reports/customers_report_spec.rb | 7 +++-- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb index 09b1cb2170..a1c2382e55 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -11,13 +11,8 @@ module Reporting .order(:id)) .group_by do |order| { - first_name: order.billing_address.firstname, - last_name: order.billing_address.lastname, - billing_address: order.billing_address.address_and_city, - email: order.email, - phone: order.billing_address.phone, + customer_id: order.customer_id || order.email, hub_id: order.distributor_id, - shipping_method_id: order.shipping_method&.id, } end.values end @@ -25,14 +20,18 @@ module Reporting # rubocop:disable Metrics/AbcSize def columns { - first_name: proc { |orders| orders.first.billing_address.firstname }, - last_name: proc { |orders| orders.first.billing_address.lastname }, - billing_address: proc { |orders| orders.first.billing_address.address_and_city }, - email: proc { |orders| orders.first.email }, - phone: proc { |orders| orders.first.billing_address.phone }, - hub: proc { |orders| orders.first.distributor&.name }, - hub_address: proc { |orders| orders.first.distributor&.address&.address_and_city }, - shipping_method: proc { |orders| orders.first.shipping_method&.name }, + first_name: proc { |orders| last_completed_order(orders).billing_address.firstname }, + last_name: proc { |orders| last_completed_order(orders).billing_address.lastname }, + billing_address: proc { |orders| + last_completed_order(orders).billing_address.address_and_city + }, + email: proc { |orders| last_completed_order(orders).email }, + phone: proc { |orders| last_completed_order(orders).billing_address.phone }, + hub: proc { |orders| last_completed_order(orders).distributor&.name }, + hub_address: proc { |orders| + last_completed_order(orders).distributor&.address&.address_and_city + }, + shipping_method: proc { |orders| last_completed_order(orders).shipping_method&.name }, total_orders: proc { |orders| orders.count }, total_incl_tax: proc { |orders| orders.sum(&:total) }, last_completed_order_date: proc { |orders| last_completed_order_date(orders) }, @@ -75,8 +74,12 @@ module Reporting end end + def last_completed_order(orders) + orders.max_by(&:completed_at) + end + def last_completed_order_date(orders) - orders.max_by(&:completed_at)&.completed_at&.to_date + last_completed_order(orders).completed_at&.to_date end end end diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 30fa001e47..8dddf65a3b 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -44,15 +44,18 @@ module Reporting let!(:a) { create(:bill_address) } let!(:d){ create(:distributor_enterprise) } let!(:sm) { create(:shipping_method, distributors: [d]) } + let!(:customer) { create(:customer) } let!(:o1) { create(:order_with_totals_and_distribution, :completed, distributor: d, bill_address: a, - shipping_method: sm) + shipping_method: sm, + customer:) } let!(:o2) { create(:order_with_totals_and_distribution, :completed, distributor: d, bill_address: a, - shipping_method: sm) + shipping_method: sm, + customer:) } before do o1.update(completed_at: "2023-01-01")