Reports Refactor 1

Use code closer to the new packing report controller
Handle nil @report_subtypes

.rubocop_todo.yml

@@ -560,7 +560,7 @@ Metrics/CyclomaticComplexity:
     - 'lib/open_food_network/customers_report.rb'
     - 'lib/open_food_network/enterprise_issue_validator.rb'
     - 'lib/open_food_network/group_buy_report.rb'
-    - 'lib/open_food_network/orders_and_fulfillments_report/customer_totals_report.rb'
+    - 'lib/open_food_network/orders_and_fulfillment_report/customer_totals_report.rb'
     - 'lib/open_food_network/payments_report.rb'
     - 'lib/open_food_network/xero_invoices_report.rb'
     - 'lib/spree/core/controller_helpers/order.rb'
@@ -716,7 +716,7 @@ Naming/VariableNumber:
     - 'app/controllers/spree/orders_controller.rb'
     - 'app/models/content_configuration.rb'
     - 'app/models/preference_sections/main_links_section.rb'
-    - 'lib/open_food_network/orders_and_fulfillments_report/customer_totals_report.rb'
+    - 'lib/open_food_network/orders_and_fulfillment_report/customer_totals_report.rb'
     - 'lib/spree/core/controller_helpers/common.rb'
     - 'spec/controllers/spree/admin/search_controller_spec.rb'
     - 'spec/factories/stock_location_factory.rb'
@@ -1092,7 +1092,7 @@ Style/MissingRespondToMissing:
 # Offense count: 1
 Style/MixinUsage:
   Exclude:
-    - 'lib/open_food_network/orders_and_fulfillments_report.rb'
+    - 'lib/open_food_network/orders_and_fulfillment_report.rb'
 
 # Offense count: 2
 # Cop supports --auto-correct.
@@ -1137,7 +1137,7 @@ Style/OptionalBooleanParameter:
     - 'lib/open_food_network/customers_report.rb'
     - 'lib/open_food_network/order_and_distributor_report.rb'
     - 'lib/open_food_network/order_cycle_management_report.rb'
-    - 'lib/open_food_network/orders_and_fulfillments_report.rb'
+    - 'lib/open_food_network/orders_and_fulfillment_report.rb'
     - 'lib/open_food_network/payments_report.rb'
     - 'lib/open_food_network/products_and_inventory_report_base.rb'
     - 'lib/open_food_network/users_and_enterprises_report.rb'
@@ -1236,7 +1236,7 @@ Style/StringConcatenation:
     - 'app/services/embedded_page_service.rb'
     - 'app/services/products_renderer.rb'
     - 'engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb'
-    - 'lib/open_food_network/orders_and_fulfillments_report/customer_totals_report.rb'
+    - 'lib/open_food_network/orders_and_fulfillment_report/customer_totals_report.rb'
     - 'lib/spree/api/controller_setup.rb'
     - 'lib/spree/core/environment_extension.rb'
     - 'spec/lib/open_food_network/order_grouper_spec.rb'

app/controllers/spree/admin/reports_controller.rb

@@ -14,12 +14,13 @@ require 'open_food_network/order_cycle_management_report'
 require 'open_food_network/sales_tax_report'
 require 'open_food_network/xero_invoices_report'
 require 'open_food_network/payments_report'
-require 'open_food_network/orders_and_fulfillments_report'
+require 'open_food_network/orders_and_fulfillment_report'
 
 module Spree
   module Admin
     class ReportsController < Spree::Admin::BaseController
       include Spree::ReportsHelper
+      include ReportsActions
       helper ::ReportsHelper
 
       ORDER_MANAGEMENT_ENGINE_REPORTS = [
@@ -32,7 +33,6 @@ module Spree
       before_action :cache_search_state
       # Fetches user's distributors, suppliers and order_cycles
       before_action :load_basic_data, only: [:customers, :products_and_inventory, :order_cycle_management]
-      before_action :load_associated_data, only: [:orders_and_fulfillment]
 
       respond_to :html
 
@@ -101,21 +101,21 @@ module Spree
       end
 
       def orders_and_fulfillment
-        raw_params[:q] ||= orders_and_fulfillment_default_filters
+        now = Time.zone.now
+        raw_params[:q] ||= {
+          completed_at_gt: (now - 1.month).beginning_of_day,
+          completed_at_lt: (now + 1.day).beginning_of_day
+        }
 
-        @report_types = report_types[:orders_and_fulfillment]
-        @report_type = params[:report_type]
+        form_options = Reporting::FrontendData.new(spree_current_user)
 
-        @include_blank = I18n.t(:all)
+        @distributors = form_options.distributors
+        @suppliers = form_options.suppliers
+        @order_cycles = form_options.order_cycles
 
-        # -- Build Report with Order Grouper
-        @report = OpenFoodNetwork::OrdersAndFulfillmentsReport.new spree_current_user,
-                                                                   raw_params,
-                                                                   render_content?
-        @table = order_grouper_table
-        csv_file_name = "#{params[:report_type]}_#{timestamp}.csv"
+        @report_message = I18n.t("spree.admin.reports.customer_names_message.customer_names_tip")
 
-        render_report(@report.header, @table, params[:csv], csv_file_name)
+        render_report2
       end
 
       def products_and_inventory
@@ -190,19 +190,27 @@ module Spree
         @searching
       end
 
+      def render_report2
+        @report_subtypes = report_types[action_name.to_sym]
+        @report_subtype = params[:report_subtype]
+        klass = "OpenFoodNetwork::#{action_name.camelize}Report".constantize
+        @report = klass.new spree_current_user, raw_params, render_content?
+
+        if report_format.present?
+          data = Reporting::ReportRenderer.new(@report).public_send("to_#{report_format}")
+          send_data data, filename: report_filename
+        else
+          @header = @report.table_headers
+          @table = @report.table_rows
+
+          render "show"
+        end
+      end
+
       def render_report(header, table, create_csv, csv_file_name)
         send_data csv_report(header, table), filename: csv_file_name if create_csv
         @header = header
         @table = table
-        # Rendering HTML is the default.
-      end
-
-      def load_associated_data
-        form_options = Reporting::FrontendData.new(spree_current_user)
-
-        @distributors = form_options.distributors
-        @suppliers = form_options.suppliers
-        @order_cycles = form_options.order_cycles
       end
 
       def csv_report(header, table)
@@ -299,12 +307,6 @@ module Spree
       def timestamp
         Time.zone.now.strftime("%Y%m%d")
       end
-
-      def orders_and_fulfillment_default_filters
-        now = Time.zone.now
-        { completed_at_gt: (now - 1.month).beginning_of_day,
-          completed_at_lt: (now + 1.day).beginning_of_day }
-      end
     end
   end
 end

app/views/admin/reports/_rendering_options.html.haml

@@ -1,4 +1,4 @@
-- if @report_subtypes.any?
+- if @report_subtypes.present? && @report_subtypes.count > 1
   .row
     .alpha.two.columns= label_tag nil, t(:report_type)
     .omega.fourteen.columns

app/views/spree/admin/reports/_table.html.haml

@@ -1,20 +1,19 @@
 - column_partials ||= {}
-- if render_content?
-  %table.report__table{id: id}
-    %thead
+%table.report__table
+  %thead
+    %tr
+      - @header.each do |heading|
+        %th= heading
+  %tbody
+    - @table.each do |row|
       %tr
-        - @header.each do |heading|
-          %th= heading
-    %tbody
-      - @table.each do |row|
-        %tr
-          - row.each_with_index do |cell_value, column_index|
-            %td
-              - partial = column_partials[column_index]
-              - if partial
-                = render partial, value: cell_value
-              - else
-                = cell_value
-      - if @table.empty?
-        %tr
-          %td{colspan: @header.count}= t(:none)
+        - row.each_with_index do |cell_value, column_index|
+          %td
+            - partial = column_partials[column_index]
+            - if partial
+              = render partial, value: cell_value
+            - else
+              = cell_value
+    - if @table.empty?
+      %tr
+        %td{colspan: @header.count}= t(:none)

app/views/spree/admin/reports/show.html.haml

@@ -0,0 +1,17 @@
+= form_for @report.search, :url => url_for(only_path: false) do |f|
+  %fieldset.no-border-bottom.print-hidden
+    %legend{ align: 'center'}= t(:report_filters)
+    = render partial: "spree/admin/reports/filters/#{action_name}", locals: { f: f }
+
+  %fieldset.print-hidden
+    %legend{ align: 'center'}= t(:report_render_options)
+    = render partial: "admin/reports/rendering_options"
+
+    .actions.filter-actions
+      = button t(:go), "report__submit-btn"
+
+- if @report_message.present?
+  %p.report__message.print-hidden= @report_message
+
+- if render_content?
+  = render "table"

lib/reporting/report_renderer.rb

@@ -9,11 +9,11 @@ module Reporting
     end
 
     def table_headers
-      @report.report_data.columns
+      @report.respond_to?(:report_data) ? @report.report_data.columns : @report.table_headers
     end
 
     def table_rows
-      @report.report_data.rows
+      @report.respond_to?(:report_data) ? @report.report_data.rows : @report.table_rows
     end
 
     def as_json

spec/system/admin/reports/packing_report_spec.rb

@@ -48,20 +48,20 @@ describe "Packing Reports", js: true do
         fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
         click_button 'Search'
 
-        rows = find("table#listing_orders").all("thead tr")
+        rows = find("table.report__table").all("thead tr")
         table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
         expect(table).to eq([
                               ["Hub", "Code", "First Name", "Last Name", "Supplier",
                                "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
                             ])
-        expect(page).to have_selector 'table#listing_orders tbody tr', count: 5 # Totals row per order
+        expect(page).to have_selector 'table.report__table tbody tr', count: 5 # Totals row per order
       end
 
       it "sorts alphabetically" do
         click_link "Pack By Customer"
         click_button 'Search'
 
-        rows = find("table#listing_orders").all("tr")
+        rows = find("table.report__table").all("tr")
         table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[3] }
         expect(table).to eq([
                               "LAST NAME",
@@ -81,13 +81,13 @@ describe "Packing Reports", js: true do
         fill_in 'q_completed_at_lt', with: '2013-04-25 16:00:00'
         click_button 'Search'
 
-        rows = find("table#listing_orders").all("thead tr")
+        rows = find("table.report__table").all("thead tr")
         table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
         expect(table).to eq([
                               ["Hub", "Supplier", "Code", "First Name", "Last Name",
                                "Product", "Variant", "Quantity", "TempControlled?"].map(&:upcase)
                             ])
-        expect(all('table#listing_orders tbody tr').count).to eq(4) # Totals row per supplier
+        expect(all('table.report__table tbody tr').count).to eq(4) # Totals row per supplier
       end
     end
   end

spec/system/admin/reports_spec.rb

@@ -38,10 +38,10 @@ describe '
 
     it "customers report" do
       click_link "Mailing List"
-      expect(page).to have_select('report_type', selected: 'Mailing List')
+      expect(page).to have_select('report_subtype', selected: 'Mailing List')
       click_button "Go"
 
-      rows = find("table#listing_customers").all("thead tr")
+      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)
@@ -50,10 +50,10 @@ describe '
 
     it "customers report" do
       click_link "Addresses"
-      expect(page).to have_select('report_type', selected: 'Addresses')
+      expect(page).to have_select('report_subtype', selected: 'Addresses')
 
       click_button "Go"
-      rows = find("table#listing_customers").all("thead tr")
+      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",
@@ -70,7 +70,7 @@ describe '
     it "payment method report" do
       click_link "Payment Methods Report"
       click_button "Search"
-      rows = find("table#listing_ocm_orders").all("thead tr")
+      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", "Hub", "Hub Code", "Email", "Phone", "Shipping Method",
@@ -81,7 +81,7 @@ describe '
     it "delivery report" do
       click_link "Delivery Report"
       click_button "Search"
-      rows = find("table#listing_ocm_orders").all("thead tr")
+      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", "Hub", "Hub Code", "Delivery Address", "Delivery Postcode",
@@ -242,7 +242,7 @@ describe '
         pick_datetime "#q_completed_at_gt", datetime_start
         pick_datetime "#q_completed_at_lt", datetime_end
 
-        select 'Order Cycle Customer Totals', from: 'report_type'
+        select 'Order Cycle Customer Totals', from: 'report_subtype'
         click_button 'Search'
         # Then I should see the rows for the first order but not the second
         expect(all('table#listing_orders tbody tr').count).to eq(4) # Two rows per order
@@ -349,7 +349,7 @@ describe '
     it "shows users and enterprises report" do
       click_button "Search"
 
-      rows = find("table#users_and_enterprises").all("tr")
+      rows = find("table.report__table").all("tr")
       table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[0..2] }
 
       expect(table.sort).to eq([
@@ -370,7 +370,7 @@ describe '
 
       click_button "Search"
 
-      rows = find("table#users_and_enterprises").all("tr")
+      rows = find("table.report__table").all("tr")
       table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[0..2] }
 
       expect(table.sort).to eq([
@@ -555,7 +555,7 @@ describe '
     private
 
     def xero_invoice_table
-      find("table#listing_invoices")
+      find("table.report__table")
     end
 
     def xero_invoice_header