raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-27 01:43:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11185 from jibees/11155-improve-customer-addresses-report

Browse Source 
Admin, reports: improve customer addresses report
This commit is contained in:
Rachel Arnould
2023-09-22 18:06:47 +02:00
committed by GitHub
parent b913e5c851 2803f1c6b2
commit aa489d4cd6
9 changed files with 107 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
app/views/admin/reports/filters/_customers.html.haml
View File

@@ -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"})
{:include_blank => true, :class => "select2 fullwidth light"})

4
config/locales/en.yml
View File

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

40
lib/reporting/reports/customers/addresses.rb
View File

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

67
lib/reporting/reports/customers/base.rb
View File

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

29
lib/reporting/reports/customers/mailing_list.rb
View File

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

9
lib/reporting/reports/list.rb
View File

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

7
spec/controllers/admin/reports_controller_spec.rb
View File

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

112
spec/lib/reports/customers_report_spec.rb
View File

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

40
spec/system/admin/reports_spec.rb
View File

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