diff --git a/app/views/admin/reports/filters/_customers.html.haml b/app/views/admin/reports/filters/_customers.html.haml index caa88f4e5d..8cbb78b39b 100644 --- a/app/views/admin/reports/filters/_customers.html.haml +++ b/app/views/admin/reports/filters/_customers.html.haml @@ -1,18 +1,14 @@ += render 'admin/reports/date_range_form', f: f + .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]), {: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 = 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"}) diff --git a/config/locales/en.yml b/config/locales/en.yml index de1194ca81..b279f8f91b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1530,8 +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 sales_tax_totals_by_producer: Sales Tax Totals By Producer @@ -2912,6 +2910,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" @@ -3106,6 +3105,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 deleted file mode 100644 index 28362586f7..0000000000 --- a/lib/reporting/reports/customers/addresses.rb +++ /dev/null @@ -1,40 +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.map(&:first) - 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 }, - } - end - - 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 cad550789d..a1c2382e55 100644 --- a/lib/reporting/reports/customers/base.rb +++ b/lib/reporting/reports/customers/base.rb @@ -5,28 +5,57 @@ 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| + { + customer_id: order.customer_id || order.email, + hub_id: order.distributor_id, + } + end.values end + # rubocop:disable Metrics/AbcSize + def columns + { + 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) }, + } + end + # rubocop:enable Metrics/AbcSize + def filter(orders) - filter_to_supplier filter_to_distributor filter_to_order_cycle orders + filter_to_completed_at 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 + def skip_duplicate_rows? + true + end + + private + + def filter_to_completed_at(orders) + 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) @@ -44,6 +73,14 @@ module Reporting orders end end + + def last_completed_order(orders) + orders.max_by(&:completed_at) + end + + def last_completed_order_date(orders) + last_completed_order(orders).completed_at&.to_date + end end end end 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..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,13 +53,6 @@ module Reporting ] end - def customers_report_types - [ - [i18n_translate("mailing_list"), :mailing_list], - [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 b7eff12bcb..4f99de87c6 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -260,13 +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([ - ["Mailing List", :mailing_list], - ["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 005e486199..8dddf65a3b 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -14,58 +14,13 @@ 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, {} } - 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", + "Total incl. tax ($)", + "Last completed order date"]) end it "builds a table from a list of variants" do @@ -74,14 +29,14 @@ 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(" "), 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, o.total, "none" ]]) end @@ -89,24 +44,29 @@ 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") + o2.update(completed_at: "2023-01-02") [o1, o2].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 [o1] + 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) .to eq([[ @@ -114,7 +74,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, o1.total + o2.total, "2023-01-02" ]]) end @@ -136,13 +96,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, 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 + o2.shipping_method.name, 1, o2.total, "2023-01-02" ]]) end end @@ -161,9 +121,9 @@ 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, :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) @@ -178,7 +138,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, o1.total, o1.completed_at.strftime("%Y-%m-%d") ], [ a.firstname, @@ -188,7 +148,7 @@ module Reporting a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), - sm2.name + sm2.name, 1, o2.total, o2.completed_at.strftime("%Y-%m-%d") ] ] ) @@ -199,7 +159,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 @@ -226,13 +186,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 @@ -262,7 +222,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 @@ -284,18 +244,18 @@ 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) + 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(supplier_id: supplier.id) - expect(subject.filter(orders)).to eq([order1]) + 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 diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index e5c3b6863c..ee646f8267 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -38,13 +38,11 @@ describe ' it "can run the customers report" do login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list - ) + visit admin_report_path(report_type: :customers) 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 @@ -63,20 +61,16 @@ describe ' # Run the report: login_as_admin - visit admin_report_path( - report_type: :customers, report_subtype: :mailing_list - ) + visit admin_report_path(report_type: :customers) 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 - ) + 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: :mailing_list - ) + 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 @@ -130,7 +122,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. @@ -155,25 +147,17 @@ describe ' 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" + within "table.index" do + click_link "Customers" + end 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([ ["First Name", "Last Name", "Billing Address", "Email", "Phone", "Hub", "Hub Address", - "Shipping Method"].map(&:upcase) + "Shipping Method", "Total Number of Orders", "Total incl. tax ($)", + "Last completed order date"].map(&:upcase) ].sort) end end