From 82ccdcca706757671982c5acfc0ed5a28d406a3d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 10 Jul 2023 15:03:56 +0200 Subject: [PATCH] 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")