raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-03-04 02:31:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Report Refactor 3 Clean

Browse Source 
No longer need to handle old report class
Remove no longer used OrderGrouper service


sq
This commit is contained in:
Sebastian Castro
2022-04-12 21:07:57 +02:00
committed by Jean-Baptiste Bellet
parent cd30012334
commit c7c5965eb5
6 changed files with 15 additions and 288 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/views/admin/reports/_table.html.haml
View File

@@ -11,12 +11,5 @@
- if report.grouped_data.present?
= render partial: 'admin/reports/row_group', locals: { report: report, data: report.grouped_data }
- else
- report.table_rows.each do |row|
- if row
%tr
- row.each do |cell|
%td
= cell
- if report.table_rows.empty?
%tr
%td{colspan: report.table_headers.count}= t(:none)
%tr
%td{colspan: report.table_headers.count}= t(:none)

88
lib/reporting/order_grouper.rb
View File

@@ -1,88 +0,0 @@
# frozen_string_literal: true
module Reporting
class OrderGrouper
def initialize(rules, column_constructors, report = nil)
@rules = rules
@column_constructors = column_constructors
@report = report
end
def build_tree(items, remaining_rules)
rules = remaining_rules.clone
if rules.any?
rule = rules.delete_at(0) # Remove current rule for subsequent groupings
group_and_sort(rule, rules, items)
else
items
end
end
def group_and_sort(rule, remaining_rules, items)
branch = {}
groups = items.group_by { |item| rule[:group_by].call(item) }
sorted_groups = groups.sort_by { |key, _value| rule[:sort_by].call(key) }
sorted_groups.each do |property, items_by_property|
branch[property] = build_tree(items_by_property, remaining_rules)
next if rule[:summary_columns].nil? || is_leaf_node(branch[property])
branch[property][:summary_row] = {
items: items_by_property,
columns: rule[:summary_columns]
}
end
branch
end
def build_table(groups)
rows = []
if is_leaf_node(groups)
rows << build_row(groups)
else
groups.each do |key, group|
if key == :summary_row
rows << build_summary_row(group[:columns], group[:items])
else
build_table(group).each { |g| rows << g }
end
end
end
rows
end
def table(items)
tree = build_tree(items, @rules)
build_table(tree)
end
private
def build_cell(column_constructor, items)
if column_constructor.is_a?(Symbol)
@report.__send__(column_constructor, items)
else
column_constructor.call(items)
end
end
def build_row(groups)
@column_constructors.map do |column_constructor|
build_cell(column_constructor, groups)
end
end
def build_summary_row(summary_row_column_constructors, items)
summary_row_column_constructors.map do |summary_row_column_constructor|
build_cell(summary_row_column_constructor, items)
end
end
def is_leaf_node(node)
node.is_a? Array
end
end
end

9
lib/reporting/report_loader.rb
View File

@@ -4,17 +4,14 @@ module Reporting
class ReportLoader
def initialize(report_type, report_subtype = nil)
@report_type = report_type
@report_subtype = report_subtype || "base"
@report_subtype = report_subtype
end
# We currently use two types of report : old report inheriting from ReportObjectReport
# And new ones inheriting gtom ReportQueryReport
# They use different namespace and we try to load them both
def report_class
"#{report_module}::#{report_type.camelize}Report".constantize
"#{report_module}::#{report_subtype.camelize}".constantize
rescue NameError
begin
"#{report_module}::#{report_subtype.camelize}".constantize
"#{report_module}::Base".constantize
rescue NameError
raise Reporting::Errors::ReportNotFound
end

4
spec/lib/reports/bulk_coop_report_spec.rb
View File

@@ -177,8 +177,8 @@ module Reporting
end
# Yes, I know testing a private method is bad practice but report's design, tighly coupling
# Reporting::OrderGrouper and Base, makes it
# very hard to make things testeable without ending up in a wormwhole. This is a trade-off.
# makes it very hard to make things testeable without ending up in a wormwhole.
# This is a trade-off.
describe '#customer_payments_amount_owed' do
let(:params) { {} }
let(:user) { build(:user) }

175
spec/lib/reports/order_grouper_spec.rb
View File

@@ -1,175 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
module Reporting
describe OrderGrouper do
before(:each) do
@items = [1, 2, 3, 4]
end
context "constructing the table" do
it "should build a tree then build a table" do
rules = [{ group_by: proc { |sentence|
sentence.paragraph.chapter
}, sort_by: proc { |chapter|
chapter.name
}, summary_columns: [proc { |is|
is.first.paragraph.chapter.name
}, proc { |_is|
"TOTAL"
}, proc { |_is|
""
}, proc { |is|
is.sum(&:property1)
}] },
{ group_by: proc { |sentence| sentence.paragraph }, sort_by: proc { |paragraph|
paragraph.name
} }]
columns = [proc { |is| is.first.paragraph.chapter.name }, proc { |is|
is.first.paragraph.name
}, proc { |is|
is.first.name
}, proc { |is|
is.sum(&:property1)
}]
subject = OrderGrouper.new rules, columns
tree = double(:tree)
expect(subject).to receive(:build_tree).with(@items, rules).and_return(tree)
expect(subject).to receive(:build_table).with(tree)
subject.table(@items)
end
end
context "grouping items without rules" do
it "returns the original array when no rules are provided" do
rules = []
column1 = double(:col1)
column2 = double(:col2)
columns = [column1, column2]
subject = OrderGrouper.new rules, columns
expect(rules).to receive(:clone).and_return(rules)
expect(subject.build_tree(@items, rules)).to eq(@items)
end
end
context "grouping items with rules" do
before(:each) do
@rule1 = double(:rule1)
rule2 = double(:rule2)
@rules = [@rule1, rule2]
@remaining_rules = [rule2]
column1 = double(:col1)
column2 = double(:col2)
@columns = [column1, column2]
end
it "builds branches by removing a rule from 'rules' and running group_and_sort" do
subject = OrderGrouper.new @rules, @columns
expect(@rules).to receive(:clone).and_return(@rules)
expect(@rules).to receive(:delete_at).with(0)
grouped_tree = double(:grouped_tree)
expect(subject).to receive(:group_and_sort).and_return(grouped_tree)
expect(subject.build_tree(@items, @rules)).to eq(grouped_tree)
end
it "separates the first rule from rules before sending to group_and_sort" do
subject = OrderGrouper.new @rules, @columns
grouped_tree = double(:grouped_tree)
expect(subject).to receive(:group_and_sort).with(@rule1, @rules[1..],
@items).and_return(grouped_tree)
expect(subject.build_tree(@items, @rules)).to eq(grouped_tree)
end
it "should group, then sort, send each group to build_tree, and return a branch" do
summary_columns_object = double(:summary_columns)
allow(@rule1).to receive(:[]).with(:summary_columns) { summary_columns_object }
subject = OrderGrouper.new @rules, @columns
number_of_categories = 3
groups = double(:groups)
expect(@items).to receive(:group_by).and_return(groups)
sorted_groups = {}
1.upto(number_of_categories) { |i|
sorted_groups[i] = double(:group, name: "Group #{i}" )
}
expect(groups).to receive(:sort_by).and_return(sorted_groups)
group = { group1: 1, group2: 2, group3: 3 }
expect(subject).to receive(:build_tree).exactly(number_of_categories).times.and_return(group)
group_tree = {}
1.upto(number_of_categories) { |i| group_tree[i] = group }
1.upto(number_of_categories) { |i| group_tree[i][:summary_row] = summary_columns_object }
expect(subject.group_and_sort(@rule1, @remaining_rules, @items)).to eq(group_tree)
end
end
context "building the table Array" do
before(:each) do
rule1 = double(:rule1)
rule2 = double(:rule2)
@rules = [rule1, rule2]
@column1 = double(:col1, call: "Column1")
@column2 = double(:col2, call: "Column2")
@columns = [@column1, @column2]
sumcol1 = double(:sumcol1, call: "SumColumn1")
sumcol2 = double(:sumcol2, call: "SumColumn2")
@sumcols = [sumcol1, sumcol2]
item1 = double(:item1)
item2 = double(:item2)
item3 = double(:item3)
@items1 = [item1, item2]
@items2 = [item2, item3]
@items3 = [item3, item1]
end
it "should return columns when given an Array" do
subject = OrderGrouper.new @rules, @columns
expect(@column1).to receive(:call)
expect(@column2).to receive(:call)
expect(subject.build_table(@items1)).to eq([["Column1", "Column2"]])
end
it "should return a row for each key-value pair when given a Hash" do
groups = { items1: @items1, items2: @items2, items3: @items3 }
subject = OrderGrouper.new @rules, @columns
# subject.should_receive(:build_table).exactly(2).times
expected_return = []
groups.length.times { expected_return << ["Column1", "Column2"] }
expect(subject.build_table(groups)).to eq(expected_return)
end
it "should return an extra row when a :summary_row key appears in a given Hash" do
groups = { items1: @items1, items2: @items2, items3: @items3,
summary_row: { items: { items2: @items2, items3: @items3 }, columns: @sumcols } }
subject = OrderGrouper.new @rules, @columns
expected_return = []
groups.each do |key, _group|
expected_return << if key == :summary_row
["SumColumn1", "SumColumn2"]
else
["Column1", "Column2"]
end
end
expect(subject.build_table(groups)).to eq(expected_return)
end
end
end
end

16
spec/lib/reports/report_loader_spec.rb
View File

@@ -30,14 +30,6 @@ describe Reporting::ReportLoader do
end
end
describe "given report type and subtype for old reports" do
let(:arguments) { ["orange", "subtype"] }
it "returns a report class when given type and subtype" do
expect(service.report_class).to eq Reporting::Reports::Orange::OrangeReport
end
end
describe "given report type only" do
context "when the report has no subtypes" do
let(:arguments) { ["bananas"] }
@@ -47,6 +39,14 @@ describe Reporting::ReportLoader do
end
end
context "when the subtype is not implemented, fallback to base" do
let(:arguments) { ["bananas", "not_existing"] }
it "returns base class" do
expect(service.report_class).to eq Reporting::Reports::Bananas::Base
end
end
context "given a report type that does not exist" do
let(:arguments) { ["apples"] }