diff --git a/app/views/admin/reports/_table.html.haml b/app/views/admin/reports/_table.html.haml
index 42f7ec67cf..091d70e632 100644
--- a/app/views/admin/reports/_table.html.haml
+++ b/app/views/admin/reports/_table.html.haml
@@ -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)
diff --git a/lib/reporting/order_grouper.rb b/lib/reporting/order_grouper.rb
deleted file mode 100644
index dc78dfff8c..0000000000
--- a/lib/reporting/order_grouper.rb
+++ /dev/null
@@ -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
diff --git a/lib/reporting/report_loader.rb b/lib/reporting/report_loader.rb
index 29dee2f794..f38245c209 100644
--- a/lib/reporting/report_loader.rb
+++ b/lib/reporting/report_loader.rb
@@ -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
diff --git a/spec/lib/reports/bulk_coop_report_spec.rb b/spec/lib/reports/bulk_coop_report_spec.rb
index cca38b28d4..b648e6cbe6 100644
--- a/spec/lib/reports/bulk_coop_report_spec.rb
+++ b/spec/lib/reports/bulk_coop_report_spec.rb
@@ -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) }
diff --git a/spec/lib/reports/order_grouper_spec.rb b/spec/lib/reports/order_grouper_spec.rb
deleted file mode 100644
index 630b5de196..0000000000
--- a/spec/lib/reports/order_grouper_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/lib/reports/report_loader_spec.rb b/spec/lib/reports/report_loader_spec.rb
index 1bb8ff00ad..930c5af1ac 100644
--- a/spec/lib/reports/report_loader_spec.rb
+++ b/spec/lib/reports/report_loader_spec.rb
@@ -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"] }