Report Refactor 1: Payments

app/controllers/spree/admin/reports_controller.rb

@@ -63,17 +63,8 @@ module Spree
       end
 
       def payments
-        # -- Prepare Form Options
         @distributors = my_distributors
-        @report_type = params[:report_type]
-
-        # -- Build Report with Order Grouper
-        @report = OpenFoodNetwork::PaymentsReport.new spree_current_user, raw_params,
-                                                      render_content?
-        @table = order_grouper_table
-        csv_file_name = "payments_#{timestamp}.csv"
-
-        render_report(@report.header, @table, params[:csv], csv_file_name)
+        render_report2
       end
 
       def orders_and_fulfillment

app/views/spree/admin/reports/filters/_payments.html.haml

@@ -0,0 +1,6 @@
+= render 'spree/admin/reports/date_range_form', f: f
+
+.row
+  .alpha.two.columns= label_tag nil, t(:report_distributor)
+  .omega.fourteen.columns
+    = f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => t(:report_all)}, {:class => "select2 fullwidth light"})

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

@@ -1,19 +0,0 @@
-= form_for @report.search, :url => spree.payments_admin_reports_path do |f|
-  = render 'date_range_form', f: f
-
-  .row
-    .four.columns.alpha
-      = label_tag nil, t(:report_distributor)
-      = f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => t(:report_all)}, {:class => "select2 fullwidth"})
-  = label_tag nil, t(:report_customers_type)
-  %br
-  = select_tag(:report_type, options_for_select([[t(:report_payment_by),:payments_by_payment_type],[t(:report_itemised_payment),:itemised_payment_totals],[t(:report_payment_totals),:payment_totals]], @report_type))
-  %br
-  %br
-  = check_box_tag :csv
-  = label_tag :csv, t(:report_customers_csv)
-  %br
-  %br
-  = button t(:search)
-
-= render "table", id: "listing_orders", msg_option: t(:search)

lib/open_food_network/payments_report.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'open_food_network/order_grouper'
+
 module OpenFoodNetwork
   class PaymentsReport
     attr_reader :params
@@ -10,8 +12,8 @@ module OpenFoodNetwork
       @render_table = render_table
     end
 
-    def header
-      case params[:report_type]
+    def table_headers
+      case params[:report_subtype]
       when "payments_by_payment_type"
         I18n.t(:report_header_payment_type)
         [I18n.t(:report_header_payment_state), I18n.t(:report_header_distributor), I18n.t(:report_header_payment_type),
@@ -48,7 +50,7 @@ module OpenFoodNetwork
         order.payments.select(&:completed?)
       end.flatten
 
-      case params[:report_type]
+      case params[:report_subtype]
       when "payments_by_payment_type"
         payments
       when "itemised_payment_totals"
@@ -60,8 +62,13 @@ module OpenFoodNetwork
       end
     end
 
+    def table_rows
+      order_grouper = OpenFoodNetwork::OrderGrouper.new rules, columns, self
+      order_grouper.table(table_items)
+    end
+
     def rules
-      case params[:report_type]
+      case params[:report_subtype]
       when "payments_by_payment_type"
         [{ group_by: proc { |payment| payment.order.payment_state },
            sort_by: proc { |payment_state| payment_state } },
@@ -90,7 +97,7 @@ module OpenFoodNetwork
     end
 
     def columns
-      case params[:report_type]
+      case params[:report_subtype]
       when "payments_by_payment_type"
         [proc { |payments| payments.first.order.payment_state },
          proc { |payments| payments.first.order.distributor.name },

spec/system/admin/reports/payments_report_spec.rb

@@ -37,10 +37,10 @@ describe "Payments Reports" do
     it "shows orders with payment state, their balance and totals" do
       visit spree.payments_admin_reports_path
 
-      select I18n.t(:report_itemised_payment), from: "report_type"
+      select I18n.t(:report_itemised_payment), from: "report_subtype"
       find("[type='submit']").click
 
-      expect(page.find("#listing_orders thead tr").text).to have_content([
+      expect(page.find("table.report__table thead tr").text).to have_content([
         I18n.t(:report_header_payment_state),
         I18n.t(:report_header_distributor),
         I18n.t(:report_header_product_total_price, currency: currency_symbol),
@@ -49,7 +49,7 @@ describe "Payments Reports" do
         I18n.t(:report_header_total_price, currency: currency_symbol)
       ].join(" ").upcase)
 
-      expect(page.find("#listing_orders tbody tr").text).to have_content([
+      expect(page.find("table.report__table tbody tr").text).to have_content([
         order.payment_state,
         order.distributor.name,
         order.item_total.to_f + other_order.item_total.to_f,
@@ -74,10 +74,10 @@ describe "Payments Reports" do
     it 'shows orders with payment state, their balance and and payment totals' do
       visit spree.payments_admin_reports_path
 
-      select I18n.t(:report_payment_totals), from: "report_type"
+      select I18n.t(:report_payment_totals), from: "report_subtype"
       find("[type='submit']").click
 
-      expect(page.find("#listing_orders thead tr").text).to have_content([
+      expect(page.find("table.report__table thead tr").text).to have_content([
         I18n.t(:report_header_payment_state),
         I18n.t(:report_header_distributor),
         I18n.t(:report_header_product_total_price, currency: currency_symbol),
@@ -88,7 +88,7 @@ describe "Payments Reports" do
         I18n.t(:report_header_outstanding_balance_price, currency: currency_symbol),
       ].join(" ").upcase)
 
-      expect(page.find("#listing_orders tbody tr").text).to have_content([
+      expect(page.find("table.report__table tbody tr").text).to have_content([
         order.payment_state,
         order.distributor.name,
         order.item_total.to_f + other_order.item_total.to_f,

spec/system/admin/reports_spec.rb

@@ -69,7 +69,7 @@ describe '
 
     it "payment method report" do
       click_link "Payment Methods Report"
-      click_button "Search"
+      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([
@@ -245,7 +245,7 @@ describe '
         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
+        expect(all('table.report__table tbody tr').count).to eq(4) # Two rows per order
       end
     end