Reports Refactor 4: Final Touch

Split report_template
Clean code
Adds spec
This commit is contained in:
Sebastian Castro
2022-04-07 19:28:19 +02:00
committed by Jean-Baptiste Bellet
parent b259f59d1e
commit 5105ea345f
22 changed files with 420 additions and 183 deletions

View File

@@ -537,7 +537,7 @@ Metrics/ClassLength:
- 'lib/open_food_network/order_cycle_permissions.rb'
- 'lib/reporting/reports/payments/payments_report.rb'
- 'lib/reporting/reports/xero_invoices/base.rb'
- 'lib/reporting/report_grouper.rb'
- 'lib/reporting/report_rows_builder.rb'
# Offense count: 39
# Configuration parameters: IgnoredMethods, Max.

View File

@@ -5,14 +5,14 @@
- data.each do |group_or_row|
- if group_or_row[:is_group].present?
/ Header Row
- if group_or_row[:header].present? && params[:display_header_row].present?
- if group_or_row[:header].present? && report.display_header_row?
%tr
%td.header-row{ colspan: report.table_headers.count, class: group_or_row[:header_class] }
= group_or_row[:header].html_safe
/ Rows
= render partial: 'admin/reports/row_group', locals: { report: report, data: group_or_row[:data] }
/ Summary Row
- if group_or_row[:summary_row].present? && params[:display_summary_row].present?
- if group_or_row[:summary_row].present? && report.display_summary_row?
%tr.summary_row{ class: group_or_row[:summary_row_class] }
- group_or_row[:summary_row].to_h.each do |key, value|
%td= format_cell(value)

View File

@@ -0,0 +1,45 @@
# frozen_string_literal: true
module Reporting
class ReportHeadersBuilder
attr_reader :report
def initialize(report)
@report = report
end
def table_headers
report.columns.keys.filter{ |key| !key.in?(fields_to_hide) }.map do |key|
translate_header(key)
end
end
def available_headers
report.columns.keys.map { |key| [translate_header(key), key] }
end
def fields_to_hide
if report.display_header_row?
report.formatted_rules.map { |rule| rule[:fields_used_in_header] }.flatten.reject(&:blank?)
else
[]
end.concat(params_fields_to_hide)
end
private
def translate_header(key)
# Quite some headers use currency interpolation, so providing it by default
default_params = { currency: currency_symbol, currency_symbol: currency_symbol }
report.custom_headers[key] || I18n.t("report_header_#{key}", **default_params)
end
def currency_symbol
Spree::Money.currency_symbol
end
def params_fields_to_hide
report.params[:fields_to_hide]&.map(&:to_sym) || []
end
end
end

View File

@@ -1,30 +0,0 @@
# frozen_string_literal: true
# This is the old way of managing report, by loading Models from the DB and building
# The result from those models
module Reporting
class ReportObjectTemplate < ReportTemplate
# rubocop:disable Rails/Delegate
# Not delegating for now cause not all subclasses are ready to use reportGrouper
# so they can implement this method themseves
def table_rows
grouper.table_rows
end
# rubocop:enable Rails/Delegate
# The search result, an ActiveRecord Array
def query_result
raise NotImplementedError
end
# Convert the query_result into expected row result (which will be displayed)
# Example
# {
# name: proc { |model| model.display_name },
# best_friend: proc { |model| model.friends.first.first_name }
# }
def columns
raise NotImplementedError
end
end
end

View File

@@ -3,30 +3,29 @@
# This is the new report template that use QueryBuilder to directly get the data from the DB
module Reporting
class ReportQueryTemplate < ReportTemplate
def report_data
@report_data ||= report_query.raw_result
end
alias_method :query_result, :report_data
def report_query
raise NotImplementedError
end
# ReportQueryTemplate work differently than ReportObjectTemplate
def report_data
@report_data ||= report_query.raw_result
end
# ReportQueryTemplate work differently than standard reports
# Here the query_result is already the expected result, so we just create
# a fake columns method to copy the sql result into the row result
def columns
report_data.columns.map { |field| [field.to_sym, proc { |data| data[field] }] }.to_h
end
def table_rows
report_data.rows
end
def search
visible_line_items_relation.ransack(ransack_params)
end
def query_result
report_data.to_a
end
private
def ransacked_line_items_relation

View File

@@ -8,6 +8,18 @@ module Reporting
@report = report
end
def raw_render?
@report.params[:report_format].in?(['json', 'csv'])
end
def display_header_row?
@report.params[:display_header_row].present? && !raw_render?
end
def display_summary_row?
@report.params[:display_summary_row].present? && !raw_render?
end
def table_headers
@report.table_headers || []
end
@@ -16,19 +28,8 @@ module Reporting
@report.table_rows || []
end
def as_json
# columns methods give the headers code, but as not reports are implementing it
# we fallback with the translated headers with table_headers
headers = begin
@report.columns.keys
rescue NotImplementedError, NoMethodError
table_headers
end
table_rows.map do |row|
result = {}
headers.zip(row) { |a, b| result[a.to_sym] = b }
result
end.as_json
def as_json(_context_controller = nil)
@report.rows.map(&:to_h).as_json
end
def as_arrays
@@ -39,12 +40,8 @@ module Reporting
SpreadsheetArchitect.to_csv(headers: table_headers, data: table_rows)
end
def to_ods(_context_controller = nil)
SpreadsheetArchitect.to_ods(headers: table_headers, data: table_rows)
end
def to_xlsx(_context_controller = nil)
SpreadsheetArchitect.to_xlsx(headers: table_headers, data: table_rows)
SpreadsheetArchitect.to_xlsx(spreadsheets_options)
end
def to_pdf(context_controller)
@@ -56,5 +53,33 @@ module Reporting
)
)
end
private
def spreadsheets_options
{
headers: table_headers,
data: table_rows,
freeze_headers: true,
row_style: spreadsheets_style,
header_style: spreadsheets_style.merge({ bg_color: "f7f6f6", bold: true }),
conditional_row_styles: [
{
# Detect header_row: the row is nil except for first cell
if: proc { |row_data, _row_index|
row_data.compact.count == 1 && row_data[0].present?
},
styles: { font_size: 12, bold: true }
},
],
}
end
def spreadsheets_style
{
font_name: 'system-ui',
alignment: { horizontal: :left, vertical: :bottom }
}
end
end
end

View File

@@ -1,7 +1,7 @@
# frozen_string_literal: true
module Reporting
class ReportGrouper
class ReportRowsBuilder
attr_reader :report
def initialize(report)
@@ -46,8 +46,14 @@ module Reporting
def extract_rows(data, result)
data.each do |group_or_row|
if group_or_row[:is_group].present?
# Header Row
if group_or_row[:header].present? && report.display_header_row?
result << OpenStruct.new(header: group_or_row[:header])
end
# Normal Row
extract_rows(group_or_row[:data], result)
if group_or_row[:summary_row].present? && report.params[:display_summary_row].present?
# Summary Row
if group_or_row[:summary_row].present? && report.display_summary_row?
result << group_or_row[:summary_row]
end
else
@@ -99,7 +105,7 @@ module Reporting
rule[:sort_by].call(group_key)
else
# downcase for better comparaison
group_key.is_a?(String) ? group_key.downcase : group_key
group_key.is_a?(String) ? group_key.downcase : group_key.to_s
end
end.to_h
end

View File

@@ -6,8 +6,12 @@ module Reporting
attr_accessor :user, :params, :ransack_params
delegate :as_json, :as_arrays, :to_csv, :to_xlsx, :to_ods, :to_pdf, :to_json, to: :renderer
delegate :raw_render?, :display_header_row?, :display_summary_row?, to: :renderer
delegate :rows, :table_rows, :grouped_data, to: :rows_builder
delegate :available_headers, :table_headers, :fields_to_hide, to: :headers_builder
delegate :formatted_rules, :header_option?, :summary_row_option?, to: :ruler
delegate :grouped_data, :rows, to: :grouper
def initialize(user, params = {})
@user = user
@@ -28,24 +32,19 @@ module Reporting
Ransack::Search.new(Spree::Order)
end
def available_headers
columns.is_a?(Hash) ? columns.keys.map { |key| [translate_header(key), key] } : nil
rescue NotImplementedError
nil
# The search result, usually an ActiveRecord Array
def query_result
raise NotImplementedError
end
# Can be re implemented in subclasses if they not use yet the new syntax
# with columns method
def table_headers
columns.keys.filter{ |key| !key.in?(fields_to_hide) }.map do |key|
translate_header(key)
end
end
def translate_header(key)
# Quite some headers use currency interpolation, so providing it by default
default_params = { currency: currency_symbol, currency_symbol: currency_symbol }
custom_headers[key] || I18n.t("report_header_#{key}", **default_params)
# Convert the query_result into expected row result (which will be displayed)
# Example
# {
# name: proc { |model| model.display_name },
# best_friend: proc { |model| model.friends.first.first_name }
# }
def columns
raise NotImplementedError
end
# Headers are automatically translated with table_headers method
@@ -54,22 +53,6 @@ module Reporting
{}
end
def table_rows
raise NotImplementedError
end
def fields_to_hide
if params[:display_header_row]
formatted_rules.map { |rule| rule[:fields_used_in_header] }.flatten.reject(&:blank?)
else
[]
end.concat(params_fields_to_hide)
end
def params_fields_to_hide
params[:fields_to_hide]&.map(&:to_sym) || []
end
# Rules for grouping, ordering, and summary rows
# Rule Full Example. In the following item reference the query_result item and
# row the transformation of this item into the expected result
@@ -101,16 +84,16 @@ module Reporting
private
def raw_render?
params[:report_format].in?(['json', 'csv'])
end
def renderer
@renderer ||= ReportRenderer.new(self)
end
def grouper
@grouper ||= ReportGrouper.new(self)
def rows_builder
@rows_builder ||= ReportRowsBuilder.new(self)
end
def headers_builder
@headers_builder ||= ReportHeadersBuilder.new(self)
end
def ruler

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module BulkCoop
class Base < ReportObjectTemplate
class Base < ReportTemplate
def message
I18n.t("spree.admin.reports.customer_names_message.customer_names_tip")
end

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module Customers
class Base < ReportObjectTemplate
class Base < ReportTemplate
def query_result
filter Spree::Order.managed_by(@user)
.distributed_by_user(@user)

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module EnterpriseFeeSummary
class Base < ReportObjectTemplate
class Base < ReportTemplate
attr_accessor :permissions, :parameters
def initialize(user, params = {})
@@ -19,8 +19,8 @@ module Reporting
@parameters.authorize!(@permissions)
end
def translate_header(key)
I18n.t("header.#{key}", scope: i18n_scope)
def custom_headers
data_attributes.map { |attr| [attr, I18n.t("header.#{attr}", scope: i18n_scope)] }.to_h
end
def i18n_scope

View File

@@ -3,14 +3,14 @@
module Reporting
module Reports
module OrderCycleManagement
class Base < ReportObjectTemplate
class Base < ReportTemplate
DEFAULT_DATE_INTERVAL = { from: -1.month, to: 1.day }.freeze
def initialize(user, params = {})
super(user, params)
params[:q] ||= {}
params[:q][:completed_at_gt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:from]
params[:q][:completed_at_lt] ||= Time.zone.today + DEFAULT_DATE_INTERVAL[:to]
super(user, params)
end
def search
@@ -19,7 +19,7 @@ module Reporting
not_state(:canceled).
distributed_by_user(@user).
managed_by(@user).
ransack(params[:q])
ransack(ransack_params)
end
# This result is used in _order_cucle_management.html so caching it

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module OrdersAndDistributors
class Base < ReportObjectTemplate
class Base < ReportTemplate
def initialize(user, params = {})
super(user, params)
end
@@ -38,7 +38,7 @@ module Reporting
def search
permissions.visible_orders.select("DISTINCT spree_orders.*").
complete.not_state(:canceled).
ransack(@params[:q])
ransack(ransack_params)
end
def query_result
@@ -55,7 +55,7 @@ module Reporting
private
def permissions
@permissions ||= ::Permissions::Order.new(user, params[:q])
@permissions ||= ::Permissions::Order.new(user, ransack_params)
end
end
end

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module OrdersAndFulfillment
class Base < ReportObjectTemplate
class Base < ReportTemplate
def initialize(user, params = {})
super(user, params)
@@ -31,7 +31,7 @@ module Reporting
def order_permissions
return @order_permissions unless @order_permissions.nil?
@order_permissions = ::Permissions::Order.new(@user, params[:q])
@order_permissions = ::Permissions::Order.new(@user, ransack_params)
end
def report_line_items

View File

@@ -8,12 +8,8 @@ module Reporting
I18n.t("spree.admin.reports.customer_names_message.customer_names_tip")
end
def primary_model
Spree::LineItem
end
def report_query
Queries::QueryBuilder.new(primary_model).
Queries::QueryBuilder.new(Spree::LineItem).
scoped_to_orders(visible_orders_relation).
scoped_to_line_items(ransacked_line_items_relation).
with_managed_orders(managed_orders_relation).
@@ -29,6 +25,8 @@ module Reporting
ordered_by(ordering_fields)
end
private
def select_fields
lambda do
{
@@ -47,8 +45,6 @@ module Reporting
end
end
private
def row_header(row)
result = "#{row.last_name} #{row.first_name}"
result += " (#{row.customer_code})" if row.customer_code

View File

@@ -3,9 +3,9 @@
module Reporting
module Reports
module Payments
class Base < ReportObjectTemplate
class Base < ReportTemplate
def search
Spree::Order.complete.not_state(:canceled).managed_by(@user).ransack(params[:q])
Spree::Order.complete.not_state(:canceled).managed_by(@user).ransack(ransack_params)
end
def query_result

View File

@@ -5,7 +5,7 @@ require 'open_food_network/scope_variant_to_hub'
module Reporting
module Reports
module ProductsAndInventory
class Base < ReportObjectTemplate
class Base < ReportTemplate
def query_result
filter(child_variants)
end

View File

@@ -3,10 +3,10 @@
module Reporting
module Reports
module SalesTax
class Base < ReportObjectTemplate
class Base < ReportTemplate
def search
permissions = ::Permissions::Order.new(user)
permissions.editable_orders.complete.not_state(:canceled).ransack(params[:q])
permissions.editable_orders.complete.not_state(:canceled).ransack(ransack_params)
end
def query_result

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module UsersAndEnterprises
class Base < ReportObjectTemplate
class Base < ReportTemplate
def initialize(user, params = {})
super(user, params)
end

View File

@@ -3,7 +3,7 @@
module Reporting
module Reports
module XeroInvoices
class Base < ReportObjectTemplate
class Base < ReportTemplate
def initialize(user, params = {})
params.reverse_merge!(report_subtype: 'summary',
invoice_date: Time.zone.today,
@@ -37,7 +37,7 @@ module Reporting
def search
permissions = ::Permissions::Order.new(@user)
permissions.editable_orders.complete.not_state(:canceled).ransack(params[:q])
permissions.editable_orders.complete.not_state(:canceled).ransack(ransack_params)
end
# In the new way of managing reports, query_result should be an ActiveRecordRelation

View File

@@ -9,24 +9,20 @@ describe Reporting::ReportRenderer do
{ "id" => 2, "name" => "onions", "quantity" => 6 }
]
}
let(:report) { OpenStruct.new(table_headers: data.first.keys, table_rows: data.map(&:values)) }
let(:report) {
OpenStruct.new(
columns: {
id: proc { |row| row["id"] },
name: proc { |row| row["name"] },
quantity: proc { |row| row["quantity"] },
},
rows: data,
table_headers: data.first.keys,
table_rows: data.map(&:values)
)
}
let(:service) { described_class.new(report) }
describe "#table_headers" do
it "returns the report's table headers" do
expect(service.table_headers).to eq ["id", "name", "quantity"]
end
end
describe "#table_rows" do
it "returns the report's table rows" do
expect(service.table_rows).to eq [
[1, "carrots", 3],
[2, "onions", 6]
]
end
end
describe "#as_json" do
it "returns the report's data as hashes" do
expect(service.as_json).to eq data.as_json
@@ -42,40 +38,4 @@ describe Reporting::ReportRenderer do
]
end
end
describe "exporting to different formats" do
let(:spreadsheet_architect) { SpreadsheetArchitect }
before do
allow(spreadsheet_architect).to receive(:to_csv) {}
allow(spreadsheet_architect).to receive(:to_ods) {}
allow(spreadsheet_architect).to receive(:to_xlsx) {}
end
describe "#to_csv" do
it "exports as csv" do
service.to_csv
expect(spreadsheet_architect).to have_received(:to_csv).
with(headers: service.table_headers, data: service.table_rows)
end
end
describe "#to_ods" do
it "exports as ods" do
service.to_ods
expect(spreadsheet_architect).to have_received(:to_ods).
with(headers: service.table_headers, data: service.table_rows)
end
end
describe "#to_xslx" do
it "exports as xlsx" do
service.to_xlsx
expect(spreadsheet_architect).to have_received(:to_xlsx).
with(headers: service.table_headers, data: service.table_rows)
end
end
end
end

View File

@@ -0,0 +1,253 @@
# frozen_string_literal: true
require 'spec_helper'
# rubocop:disable Metrics/ModuleLength
module Reporting
describe ReportTemplate do
let(:user) { create(:user) }
let(:params) { {} }
subject { described_class.new(user, params) }
# rubocop:disable Metrics/AbcSize
def check_report
# Mock using instance variables
allow(subject).to receive(:columns).and_return(@columns)
allow(subject).to receive(:query_result).and_return(@query_result)
allow(subject).to receive(:rules).and_return(@rules) if @rules.present?
if @custom_headers.present?
allow(subject).to receive(:custom_headers).and_return(@custom_headers)
end
# Check result depending on existing instance variables
expect(subject.rows.map(&:to_h)).to eq(@expected_rows) if @expected_rows.present?
expect(subject.table_rows).to eq(@expected_table_rows) if @expected_table_rows.present?
expect(subject.table_headers).to eq(@expected_headers) if @expected_headers.present?
end
# rubocop:enable Metrics/AbcSize
describe ".columns" do
before do
@query_result = [
OpenStruct.new(hub: { name: "My Hub" }, product: { name: "Apple", price: 5 })
]
end
it "handle procs" do
@columns = {
hub: proc { |item| item.hub[:name] }
}
@expected_rows = [
{ hub: "My Hub" }
]
check_report
end
it "handles symbols" do
@columns = {
hub: :hub_name
}
allow(subject).to receive(:hub_name).and_return("Transformed Hub Name")
@expected_rows = [
{ hub: "Transformed Hub Name" }
]
check_report
end
end
describe ".table_headers" do
before do
@columns = {
hub: proc { |item| item.hub[:name] },
product: proc { |item| item.product[:name] },
price: proc { |item| item.product[:price] },
}
end
it "uses the columns keys" do
@expected_headers = ['Hub', 'Product', 'Price']
check_report
end
it "handles custom_headers" do
@custom_headers = {
product: 'Custom Product',
not_existing_key: "My Key"
}
@expected_headers = ['Hub', 'Custom Product', 'Price']
check_report
end
describe "fields_to_hide" do
let(:params) { { fields_to_hide: [:product], report_format: 'json' } }
it "works" do
@expected_headers = ['Hub', 'Price']
check_report
end
end
end
describe ".table_rows" do
before do
@columns = {
price: proc { |item| item.product[:price] },
hub: proc { |item| item.hub[:name] }
}
@query_result = [
OpenStruct.new(hub: { name: "My Hub" }, product: { name: "Apple", price: 5 }),
OpenStruct.new(hub: { name: "My Other Hub" }, product: { name: "Apple", price: 12 })
]
end
it "get correct data" do
@expected_table_rows = [
[5, "My Hub"],
[12, "My Other Hub"],
]
check_report
end
end
describe ".rules" do
describe "#group_by" do
before do
@columns = {
hub: proc { |item| item.hub },
customer: proc { |item| item.customer },
product: proc { |item| item.product },
quantity: proc { |item| item.quantity },
}
@query_result = [
OpenStruct.new(hub: "Hub 1", customer: "John", product: "Apple", quantity: 4),
OpenStruct.new(hub: "Hub 2", customer: "John", product: "Pear", quantity: 3),
OpenStruct.new(hub: "Hub 2", customer: "John", product: "Apple", quantity: 5),
OpenStruct.new(hub: "Hub 1", customer: "Abby", product: "Orange", quantity: 6),
]
end
it "works with symbol or proc" do
@rules = [
{ group_by: proc { |_i, row| row.hub }, fields_used_in_header: [:hub], header: true },
{ group_by: :customer, header: true }
]
allow(subject).to receive(:display_header_row?).and_return(true)
@expected_rows = [
{ header: "Hub 1" },
{ header: "Abby" },
{ product: "Orange", quantity: 6 },
{ header: "John" },
{ product: "Apple", quantity: 4 },
{ header: "Hub 2" },
{ header: "John" },
{ product: "Pear", quantity: 3 },
{ product: "Apple", quantity: 5 },
]
check_report
end
end
describe "#sort_by" do
before do
@columns = {
hub_name: proc { |item| item.hub[:name] }
}
hub1 = { name: "Hub 1", popularity: 5 }
hub2 = { name: "Hub 2", popularity: 2 }
@query_result = [
OpenStruct.new(hub: hub2),
OpenStruct.new(hub: hub1)
]
end
it "use default sort" do
@rules = [{
group_by: proc { |item, _row| item.hub }
}]
@expected_rows = [
{ hub_name: "Hub 1" },
{ hub_name: "Hub 2" },
]
check_report
end
it "use sort_by proc" do
@rules = [{
group_by: proc { |item, _row| item.hub },
sort_by: proc { |hub| hub[:popularity] }
}]
@expected_rows = [
{ hub_name: "Hub 2" },
{ hub_name: "Hub 1" }
]
check_report
end
end
describe "#summary_row" do
before do
@query_result = [
OpenStruct.new(hub: "Hub 1", customer: "John", product: "Apple", quantity: 4),
OpenStruct.new(hub: "Hub 2", customer: "John", product: "Pear", quantity: 3),
OpenStruct.new(hub: "Hub 2", customer: "John", product: "Apple", quantity: 5),
OpenStruct.new(hub: "Hub 1", customer: "Abby", product: "Orange", quantity: 6),
]
end
it "groups and sum" do
@columns = {
hub: proc { |item| item.hub },
quantity: proc { |item| item.quantity },
count: proc { |_item| "" },
}
@rules = [{
group_by: :hub,
summary_row: proc do |group_key, items, rows|
{ count: "#{group_key} count=#{items.count}", quantity: rows.sum(&:quantity) }
end,
summary_row_label: "TOTAL"
}]
@expetec_rows = [
{ hub: "Hub 1", quantity: 4, count: "" },
{ hub: "Hub 1", quantity: 6, count: "" },
{ hub: "TOTAL", quantity: 10, count: "Hub 1 count=2" },
{ hub: "Hub 2", quantity: 3, count: "" },
{ hub: "Hub 2", quantity: 5, count: "" },
{ hub: "TOTAL", quantity: 8, count: "Hub 2 count=2" }
]
check_report
end
end
describe "should not group when for JSON" do
before do
@query_result = [
OpenStruct.new(hub: "Hub 1", customer: "John", quantity: 4)
]
@columns = {
hub: proc { |item| item.hub },
customer: proc { |item| item.customer },
quantity: proc { |item| item.quantity },
}
@rules = [{
group_by: :hub,
header: true,
summary_row: proc do |_group_key, _items, rows|
{ quantity: rows.sum(&:quantity) }
end
}]
end
let(:params) { { fields_to_hide: [:customer], report_format: 'json' } }
it "works" do
@expetec_rows = [
{ hub: "Hub 1", quantity: 4 }
]
check_report
end
end
end
end
end
# rubocop:enable Metrics/ModuleLength