From 93751f9ccb97ec2d83004fb9b4b1016a77284576 Mon Sep 17 00:00:00 2001 From: Sebastian Castro Date: Sun, 3 Apr 2022 16:32:27 +0200 Subject: [PATCH] Report Refactor 3: Customers --- lib/reporting/reports/customers/addresses.rb | 28 ++++++ lib/reporting/reports/customers/base.rb | 49 ++++++++++ .../reports/customers/customers_report.rb | 95 ------------------- .../reports/customers/mailing_list.rb | 18 ++++ .../admin/reports_controller_spec.rb | 4 +- spec/lib/reports/customers_report_spec.rb | 26 +++-- 6 files changed, 108 insertions(+), 112 deletions(-) create mode 100644 lib/reporting/reports/customers/addresses.rb create mode 100644 lib/reporting/reports/customers/base.rb delete mode 100644 lib/reporting/reports/customers/customers_report.rb create mode 100644 lib/reporting/reports/customers/mailing_list.rb diff --git a/lib/reporting/reports/customers/addresses.rb b/lib/reporting/reports/customers/addresses.rb new file mode 100644 index 0000000000..e51edc9aee --- /dev/null +++ b/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 diff --git a/lib/reporting/reports/customers/base.rb b/lib/reporting/reports/customers/base.rb new file mode 100644 index 0000000000..f483d44de8 --- /dev/null +++ b/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 diff --git a/lib/reporting/reports/customers/customers_report.rb b/lib/reporting/reports/customers/customers_report.rb deleted file mode 100644 index 323bf97551..0000000000 --- a/lib/reporting/reports/customers/customers_report.rb +++ /dev/null @@ -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 diff --git a/lib/reporting/reports/customers/mailing_list.rb b/lib/reporting/reports/customers/mailing_list.rb new file mode 100644 index 0000000000..492a0abb4d --- /dev/null +++ b/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 diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 586efbe66e..584d0a135e 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/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 [] diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index dc73ac7617..f9a1c1a77e 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/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