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

@@ -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

lib/reporting/reports/customers/addresses.rb

@@ -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

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

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],

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] }

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

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")