Report Refactor 3: Customers

lib/reporting/reports/customers/addresses.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Reporting
+  module Reports
+    module Customers
+      class Addresses < Base
+        def columns
+          {
+            first_name: proc { |order| order.billing_address.firstname },
+            last_name: proc { |order| order.billing_address.lastname },
+            billing_address: proc { |order| address_from(order.billing_address) },
+            email: proc { |order| order.email },
+            phone: proc { |order| order.billing_address.phone },
+            hub: proc { |order| order.distributor&.name },
+            hub_address: proc { |order| address_from(order.distributor&.address) },
+            shipping_method: proc { |order| order.shipping_method&.name },
+          }
+        end
+
+        private
+
+        def address_from(address)
+          [address&.address1, address&.address2, address&.city].join(" ")
+        end
+      end
+    end
+  end
+end

lib/reporting/reports/customers/base.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Reporting
+  module Reports
+    module Customers
+      class Base < ReportObjectTemplate
+        def query_result
+          filter Spree::Order.managed_by(@user)
+            .distributed_by_user(@user)
+            .complete.not_state(:canceled)
+        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
+        end
+
+        def filter_to_distributor(orders)
+          if params[:distributor_id].to_i > 0
+            orders.where(distributor_id: params[:distributor_id])
+          else
+            orders
+          end
+        end
+
+        def filter_to_order_cycle(orders)
+          if params[:order_cycle_id].to_i > 0
+            orders.where(order_cycle_id: params[:order_cycle_id])
+          else
+            orders
+          end
+        end
+      end
+    end
+  end
+end

lib/reporting/reports/customers/customers_report.rb

@@ -1,95 +0,0 @@
-# frozen_string_literal: true
-
-module Reporting
-  module Reports
-    module Customers
-      class CustomersReport < ReportObjectTemplate
-        def table_headers
-          if is_mailing_list?
-            [I18n.t(:report_header_email),
-             I18n.t(:report_header_first_name),
-             I18n.t(:report_header_last_name),
-             I18n.t(:report_header_suburb)]
-          else
-            [I18n.t(:report_header_first_name),
-             I18n.t(:report_header_last_name),
-             I18n.t(:report_header_billing_address),
-             I18n.t(:report_header_email),
-             I18n.t(:report_header_phone),
-             I18n.t(:report_header_hub),
-             I18n.t(:report_header_hub_address),
-             I18n.t(:report_header_shipping_method)]
-          end
-        end
-
-        def table_rows
-          orders.map do |order|
-            if is_mailing_list?
-              [order.email,
-               order.billing_address.firstname,
-               order.billing_address.lastname,
-               order.billing_address.city]
-            else
-              ba = order.billing_address
-              da = order.distributor&.address
-              [ba.firstname,
-               ba.lastname,
-               [ba.address1, ba.address2, ba.city].join(" "),
-               order.email,
-               ba.phone,
-               order.distributor&.name,
-               [da&.address1, da&.address2, da&.city].join(" "),
-               order.shipping_method&.name]
-            end
-          end
-        end
-
-        def orders
-          filter Spree::Order.managed_by(@user)
-                             .distributed_by_user(@user)
-                             .complete.not_state(:canceled)
-        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
-        end
-
-        def filter_to_distributor(orders)
-          if params[:distributor_id].to_i > 0
-            orders.where(distributor_id: params[:distributor_id])
-          else
-            orders
-          end
-        end
-
-        def filter_to_order_cycle(orders)
-          if params[:order_cycle_id].to_i > 0
-            orders.where(order_cycle_id: params[:order_cycle_id])
-          else
-            orders
-          end
-        end
-
-        private
-
-        def is_mailing_list?
-          params[:report_subtype] == "mailing_list"
-        end
-      end
-    end
-  end
-end

lib/reporting/reports/customers/mailing_list.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Reporting
+  module Reports
+    module Customers
+      class MailingList < Base
+        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

spec/controllers/admin/reports_controller_spec.rb

@@ -295,8 +295,8 @@ describe Admin::ReportsController, type: :controller do
       expect(assigns(:report_subtypes)).to eq(subject.reports[:customers])
     end
 
-    it "creates a CustomersReport" do
-      allow(Reporting::Reports::Customers::CustomersReport).to receive(:new)
+    it "creates a report object" do
+      allow(Reporting::Reports::Customers::Base).to receive(:new)
         .and_return(report = double(:report))
       allow(report).to receive(:table_headers).and_return []
       allow(report).to receive(:table_rows).and_return []

spec/lib/reports/customers_report_spec.rb

@@ -5,19 +5,17 @@ require 'spec_helper'
 module Reporting
   module Reports
     module Customers
-      describe CustomersReport do
+      describe Base do
         context "as a site admin" do
           let(:user) do
             user = create(:user)
             user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin')
             user
           end
-          subject { CustomersReport.new user, {} }
+          subject { Base.new user, {} }
 
           describe "mailing list report" do
-            before do
-              allow(subject).to receive(:params).and_return(report_subtype: "mailing_list")
-            end
+            subject { MailingList.new user, {} }
 
             it "returns headers for mailing_list" do
               expect(subject.table_headers).to eq(["Email", "First Name", "Last Name", "Suburb"])
@@ -28,7 +26,7 @@ module Reporting
               address = double(:billing_address, firstname: "Firsty",
                                                  lastname: "Lasty", city: "Suburbia")
               allow(order).to receive(:billing_address).and_return address
-              allow(subject).to receive(:orders).and_return [order]
+              allow(subject).to receive(:query_result).and_return [order]
 
               expect(subject.table_rows).to eq([[
                                                  "test@test.com", "Firsty", "Lasty", "Suburbia"
@@ -37,9 +35,7 @@ module Reporting
           end
 
           describe "addresses report" do
-            before do
-              allow(subject).to receive(:params).and_return(report_subtype: "addresses")
-            end
+            subject { Addresses.new user, {} }
 
             it "returns headers for addresses" do
               expect(subject.table_headers).to eq(["First Name", "Last Name", "Billing Address", "Email",
@@ -52,7 +48,7 @@ module Reporting
               o = create(:order, distributor: d, bill_address: a)
               o.shipments << create(:shipment)
 
-              allow(subject).to receive(:orders).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(" "),
@@ -67,13 +63,13 @@ module Reporting
             it "fetches completed orders" do
               o1 = create(:order)
               o2 = create(:order, completed_at: 1.day.ago)
-              expect(subject.orders).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.orders).to eq([o2])
+              expect(subject.query_result).to eq([o2])
             end
           end
         end
@@ -86,7 +82,7 @@ module Reporting
             user
           end
 
-          subject { CustomersReport.new user, {} }
+          subject { Base.new user, {} }
 
           describe "fetching orders" do
             let(:supplier) { create(:supplier_enterprise) }
@@ -103,7 +99,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.orders).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
@@ -113,7 +109,7 @@ module Reporting
 
               # When I fetch orders, I should see no orders
               expect(subject).to receive(:filter).with([]).and_return([])
-              expect(subject.orders).to eq([])
+              expect(subject.query_result).to eq([])
             end
           end