From 6f6ae309c6ea0b7bbc8785d989a7b03f0b7242b0 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Sun, 2 Nov 2014 11:26:39 +0000
Subject: [PATCH 01/12] 266 UK: Adding first UK report - Payment Methods Report
 - to find balances per ordercycle for multiple payment method options.
 Working, but not complete to spec yet

---
 .../admin/reports_controller_decorator.rb     | 23 ++++++-
 app/helpers/spree/reports_helper.rb           | 19 ++++++
 app/models/spree/ability_decorator.rb         |  2 +-
 ...der_cycle_management_description.html.haml |  4 ++
 .../reports/order_cycle_management.html.haml  | 38 +++++++++++
 config/routes.rb                              |  1 +
 .../order_cycle_management_report.rb          | 65 +++++++++++++++++++
 spec/features/admin/reports_spec.rb           | 16 +++++
 8 files changed, 165 insertions(+), 3 deletions(-)
 create mode 100644 app/views/spree/admin/reports/_order_cycle_management_description.html.haml
 create mode 100644 app/views/spree/admin/reports/order_cycle_management.html.haml
 create mode 100644 lib/open_food_network/order_cycle_management_report.rb

diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb
index 22dd41549f..7ffb32d097 100644
--- a/app/controllers/spree/admin/reports_controller_decorator.rb
+++ b/app/controllers/spree/admin/reports_controller_decorator.rb
@@ -4,6 +4,7 @@ require 'open_food_network/products_and_inventory_report'
 require 'open_food_network/group_buy_report'
 require 'open_food_network/order_grouper'
 require 'open_food_network/customers_report'
+require 'open_food_network/order_cycle_management_report'
 
 Spree::Admin::ReportsController.class_eval do
 
@@ -21,11 +22,14 @@ Spree::Admin::ReportsController.class_eval do
     customers: [
       ["Mailing List", :mailing_list],
       ["Addresses", :addresses]
+    ],
+    order_cycle_management: [
+      ["Payment Methods Report", :payment_methods_report]
     ]
   }
 
   # Fetches user's distributors, suppliers and order_cycles
-  before_filter :load_data, only: [:customers, :products_and_inventory]
+  before_filter :load_data, only: [:customers, :products_and_inventory, :order_cycle_management]
 
   # Render a partial for orders and fulfillment description
   respond_override :index => { :html => { :success => lambda {
@@ -35,6 +39,8 @@ Spree::Admin::ReportsController.class_eval do
       render_to_string(partial: 'products_and_inventory_description', layout: false, locals: {report_types: REPORT_TYPES[:products_and_inventory]}).html_safe
     @reports[:customers][:description] =
       render_to_string(partial: 'customers_description', layout: false, locals: {report_types: REPORT_TYPES[:customers]}).html_safe
+    @reports[:order_cycle_management][:description] =
+      render_to_string(partial: 'order_cycle_management_description', layout: false, locals: {report_types: REPORT_TYPES[:order_cycle_management]}).html_safe
   } } }
 
 
@@ -53,6 +59,18 @@ Spree::Admin::ReportsController.class_eval do
     render_report(@report.header, @report.table, params[:csv], "customers.csv")
   end
 
+  def order_cycle_management
+    @report_types = REPORT_TYPES[:order_cycle_management]
+    @report_type = params[:report_type]
+    @report = OpenFoodNetwork::OrderCycleManagementReport.new spree_current_user, params
+
+    @search = Spree::Order.complete.not_state(:canceled).managed_by(spree_current_user).search(params[:q])
+
+    @orders = @search.result
+ 
+    render_report(@report.header, @report.table, params[:csv], "customers.csv")
+  end
+
   def orders_and_distributors
     params[:q] = {} unless params[:q]
 
@@ -613,7 +631,8 @@ Spree::Admin::ReportsController.class_eval do
       :orders_and_fulfillment => {:name => "Orders & Fulfillment Reports", :description => ''},
       :customers => {:name => "Customers", :description => 'Customer details'},
       :products_and_inventory => {:name => "Products & Inventory", :description => ''},
-      :sales_total => { :name => "Sales Total", :description => "Sales Total For All Orders" }
+      :order_cycle_management => {:name => "UK Order Cycle Management", :description => ''}
+      
     }
     # Return only reports the user is authorized to view.
     reports.select { |action| can? action, :report }
diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb
index 573b790051..0378e7db46 100644
--- a/app/helpers/spree/reports_helper.rb
+++ b/app/helpers/spree/reports_helper.rb
@@ -7,5 +7,24 @@ module Spree
         [ "#{oc.name} &nbsp; (#{orders_open_at} - #{orders_close_at})".html_safe, oc.id ]
       end
     end
+
+    #lin-d-hop
+    #Find the payment methods options for reporting. 
+    #I don't like that this is done in two loops, but redundant list entries
+    #  were created otherwise... 
+    def report_payment_method_options(orders)
+      payment_method_list = {}
+      orders.map do |o|
+        payment_method_name = o.payments.first.payment_method.andand.name
+        payment_method_id = o.payments.first.payment_method.andand.id
+        payment_method_list[payment_method_name] = payment_method_id
+      end
+      payment_method_list.each do |key, value|
+        [ "#{value}".html_safe,  key]
+      end
+    end
+
+    
+
   end
 end
diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb
index 23994a4580..e0f4663831 100644
--- a/app/models/spree/ability_decorator.rb
+++ b/app/models/spree/ability_decorator.rb
@@ -122,7 +122,7 @@ class AbilityDecorator
     end
 
     # Reports page
-    can [:admin, :index, :customers, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory], :report
+    can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], :report
   end
 
 
diff --git a/app/views/spree/admin/reports/_order_cycle_management_description.html.haml b/app/views/spree/admin/reports/_order_cycle_management_description.html.haml
new file mode 100644
index 0000000000..d4d20ae66e
--- /dev/null
+++ b/app/views/spree/admin/reports/_order_cycle_management_description.html.haml
@@ -0,0 +1,4 @@
+%ul{style: "margin-left: 12pt"}
+  - report_types.each do |report_type|
+    %li 
+      = link_to report_type[0], "#{order_cycle_management_admin_reports_url}?report_type=#{report_type[1]}"
diff --git a/app/views/spree/admin/reports/order_cycle_management.html.haml b/app/views/spree/admin/reports/order_cycle_management.html.haml
new file mode 100644
index 0000000000..a88e55ddde
--- /dev/null
+++ b/app/views/spree/admin/reports/order_cycle_management.html.haml
@@ -0,0 +1,38 @@
+= form_tag spree.order_cycle_management_admin_reports_url do |f|
+  %br
+  = label_tag nil, "Order Cycle: "
+  = select_tag(:order_cycle_id,
+    options_for_select(report_order_cycle_options(@order_cycles), params[:order_cycle_id]),
+              include_blank: true)
+  %br
+  %br
+  = label_tag nil, "Payment Methods (hold Ctrl to select multiple payment methods)"
+  %br
+
+  = select_tag(:payment_method_id, 
+      options_for_select(report_payment_method_options(@orders), params[:payment_method_id]), 
+      multiple: true, include_blank: true) 
+  %br
+  %br
+  = check_box_tag :csv
+  = label_tag :csv, "Download as csv"
+  %br
+  %br
+  = button t(:search)
+
+%br
+%br
+%table#listing_order_payment_methods.index
+  %thead
+    %tr{'data-hook' => "orders_header"}
+      - @report.header.each do |heading|
+        %th=heading
+  %tbody
+    - @report.table.each do |row|
+      %tr
+        - row.each do |column|
+          %td= column
+    - if @report.table.empty?
+      %tr
+        %td{:colspan => "2"}= t(:none)
+
diff --git a/config/routes.rb b/config/routes.rb
index 62f55065fc..608f9a0f3b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -112,6 +112,7 @@ end
 
 Spree::Core::Engine.routes.prepend do
   match '/admin/reports/orders_and_distributors' => 'admin/reports#orders_and_distributors', :as => "orders_and_distributors_admin_reports",  :via  => [:get, :post]
+ match '/admin/reports/order_cycle_management' => 'admin/reports#order_cycle_management', :as => "order_cycle_management_admin_reports",  :via  => [:get, :post]
   match '/admin/reports/group_buys' => 'admin/reports#group_buys', :as => "group_buys_admin_reports",  :via  => [:get, :post]
   match '/admin/reports/bulk_coop' => 'admin/reports#bulk_coop', :as => "bulk_coop_admin_reports",  :via  => [:get, :post]
   match '/admin/reports/payments' => 'admin/reports#payments', :as => "payments_admin_reports",  :via  => [:get, :post]
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
new file mode 100644
index 0000000000..70c5de8f29
--- /dev/null
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -0,0 +1,65 @@
+module OpenFoodNetwork
+  class OrderCycleManagementReport
+    attr_reader :params
+    def initialize(user, params = {})
+      @params = params
+      @user = user
+    end
+
+    def header
+      ["First Name", "Last Name", "Email", "Phone", "Hub", "Payment Method", "Amount ", "Amount Paid"]
+    
+    end
+
+    def table
+      orders.map do |order|
+        ba = order.billing_address
+        da = order.distributor.andand.address
+        [ba.firstname,
+          ba.lastname,
+          order.email,
+           ba.phone,
+           order.distributor.andand.name,
+           order.payments.first.andand.payment_method.andand.name,
+	   order.payments.first.amount
+        ]       
+      end
+    end
+
+    def orders
+      filter Spree::Order
+    end
+
+    def filter(orders)
+      filter_for_user filter_active filter_to_order_cycle filter_to_payment_method orders
+    end
+  
+    def filter_for_user (orders)
+       orders.managed_by(@user).distributed_by_user(@user)
+    end	
+
+    def filter_active (orders)
+       orders.complete.where("spree_orders.state != ?", :cancelled)
+    end
+
+    def filter_to_payment_method (orders)
+      if params.has_key? (:payment_method_id)
+        orders.joins(:payments)
+          .where(:spree_payments => {:payment_method_id => params[:payment_method_id]} )
+      else
+        orders
+      end
+    end
+
+    def filter_to_order_cycle(orders)
+      if params[:order_cycle_id].to_i > 0
+        orders.where(order_cycle_id: params[:order_cycle_id])
+      else
+        orders
+      end
+    end
+
+  
+  end
+end
+
diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index a1f1bcfec3..82cdbf59ba 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -57,6 +57,22 @@ feature %q{
     end
   end
 
+  describe "Order cycle management report" do
+    before do
+      login_to_admin_section
+      click_link "Reports"
+    end
+
+    scenario "order payment method report" do
+      click_link "Order Cycle Management"
+      rows = find("table#listing_order_payment_method").all("thead tr")
+      table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
+      table.sort.should == [
+        ["First Name", "Last Name", "Email", "Phone", "Hub", "Payment Method", "Amount", "Amount Paid"]
+      ].sort
+    end
+  end
+
   scenario "orders and distributors report" do
     login_to_admin_section
     click_link 'Reports'

From 932d571d2ce2e763a932df5f72e5f93ad21e9eae Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Wed, 12 Nov 2014 11:47:26 +0000
Subject: [PATCH 02/12] 266: Updating to incorporate Rohans suggestions.
 Searching on payment method name rather than id

---
 app/helpers/spree/reports_helper.rb                  | 12 +++++-------
 app/models/spree/order_decorator.rb                  |  6 ++++++
 .../admin/reports/order_cycle_management.html.haml   |  4 ++--
 .../order_cycle_management_report.rb                 |  5 ++---
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb
index 0378e7db46..5854ede743 100644
--- a/app/helpers/spree/reports_helper.rb
+++ b/app/helpers/spree/reports_helper.rb
@@ -13,14 +13,12 @@ module Spree
     #I don't like that this is done in two loops, but redundant list entries
     #  were created otherwise... 
     def report_payment_method_options(orders)
-      payment_method_list = {}
-      orders.map do |o|
-        payment_method_name = o.payments.first.payment_method.andand.name
-        payment_method_id = o.payments.first.payment_method.andand.id
-        payment_method_list[payment_method_name] = payment_method_id
+      payment_method_list = []
+      orders.map do |o| 
+        payment_method_list << o.payments.first.payment_method.andand.name
       end
-      payment_method_list.each do |key, value|
-        [ "#{value}".html_safe,  key]
+      payment_method_list.uniq.each do |pm|
+        [ "#{pm}".html_safe,  pm]
       end
     end
 
diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index fcc6054f3b..228410a116 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -66,6 +66,12 @@ Spree::Order.class_eval do
     where("state != ?", state)
   }
 
+  scope :with_payment_method_name, lambda { |payment_method_name|
+    joins(:payments => :payment_method).
+      where('spree_payment_methods.name = ?', payment_method_name).
+      select('DISTINCT spree_orders.*')
+  }
+
 
   # -- Methods
   def products_available_from_new_distribution
diff --git a/app/views/spree/admin/reports/order_cycle_management.html.haml b/app/views/spree/admin/reports/order_cycle_management.html.haml
index a88e55ddde..a40281e255 100644
--- a/app/views/spree/admin/reports/order_cycle_management.html.haml
+++ b/app/views/spree/admin/reports/order_cycle_management.html.haml
@@ -9,8 +9,8 @@
   = label_tag nil, "Payment Methods (hold Ctrl to select multiple payment methods)"
   %br
 
-  = select_tag(:payment_method_id, 
-      options_for_select(report_payment_method_options(@orders), params[:payment_method_id]), 
+  = select_tag(:payment_method_name, 
+      options_for_select(report_payment_method_options(@orders), params[:payment_method_name]), 
       multiple: true, include_blank: true) 
   %br
   %br
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 70c5de8f29..3208ab2d33 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -43,9 +43,8 @@ module OpenFoodNetwork
     end
 
     def filter_to_payment_method (orders)
-      if params.has_key? (:payment_method_id)
-        orders.joins(:payments)
-          .where(:spree_payments => {:payment_method_id => params[:payment_method_id]} )
+      if params.has_key? (:payment_method_name)
+        orders.with_payment_method_name(params[:payment_method_name])
       else
         orders
       end

From 15f29f4c8ecd661453b5091f0eab3f116fc1ed24 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Wed, 12 Nov 2014 18:52:25 +0000
Subject: [PATCH 03/12] 266: Adding ability to search by distribution

---
 app/helpers/spree/reports_helper.rb                 | 10 +++++++++-
 .../admin/reports/order_cycle_management.html.haml  |  7 ++++++-
 .../order_cycle_management_report.rb                | 13 +++++++++++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb
index 5854ede743..967cd7fd36 100644
--- a/app/helpers/spree/reports_helper.rb
+++ b/app/helpers/spree/reports_helper.rb
@@ -22,7 +22,15 @@ module Spree
       end
     end
 
-    
+    def report_distribution_options(orders)
+      distribution_list = []
+      orders.map do |o| 
+        distribution_list << o.shipping_method.andand.name 
+      end
+      distribution_list.uniq.each do |pm|
+        [ "#{pm}".html_safe,  pm]
+      end
+    end    
 
   end
 end
diff --git a/app/views/spree/admin/reports/order_cycle_management.html.haml b/app/views/spree/admin/reports/order_cycle_management.html.haml
index a40281e255..3428cf3977 100644
--- a/app/views/spree/admin/reports/order_cycle_management.html.haml
+++ b/app/views/spree/admin/reports/order_cycle_management.html.haml
@@ -11,7 +11,12 @@
 
   = select_tag(:payment_method_name, 
       options_for_select(report_payment_method_options(@orders), params[:payment_method_name]), 
-      multiple: true, include_blank: true) 
+               multiple: true, include_blank: true) 
+  %br
+  %br
+  = select_tag(:distribution_name, 
+      options_for_select(report_distribution_options(@orders), params[:distribution_name]), 
+	       include_blank: true) 
   %br
   %br
   = check_box_tag :csv
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 3208ab2d33..fb4da5468d 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -7,7 +7,7 @@ module OpenFoodNetwork
     end
 
     def header
-      ["First Name", "Last Name", "Email", "Phone", "Hub", "Payment Method", "Amount ", "Amount Paid"]
+      ["First Name", "Last Name", "Email", "Phone", "Hub", "Shipping Method", "Payment Method", "Amount ", "Amount Paid"]
     
     end
 
@@ -20,6 +20,7 @@ module OpenFoodNetwork
           order.email,
            ba.phone,
            order.distributor.andand.name,
+           order.shipping_method.andand.name,
            order.payments.first.andand.payment_method.andand.name,
 	   order.payments.first.amount
         ]       
@@ -31,7 +32,7 @@ module OpenFoodNetwork
     end
 
     def filter(orders)
-      filter_for_user filter_active filter_to_order_cycle filter_to_payment_method orders
+      filter_for_user filter_active filter_to_order_cycle filter_to_payment_method filter_to_distribution orders
     end
   
     def filter_for_user (orders)
@@ -50,6 +51,14 @@ module OpenFoodNetwork
       end
     end
 
+    def filter_to_distribution (orders)
+      if params.has_key? (:distribution_name)
+        orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:distribution_name])
+      else
+        orders
+      end
+    end
+
     def filter_to_order_cycle(orders)
       if params[:order_cycle_id].to_i > 0
         orders.where(order_cycle_id: params[:order_cycle_id])

From 03b59eae75dda5cbb945bd35cecfb56db870d021 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Fri, 14 Nov 2014 15:56:07 +0000
Subject: [PATCH 04/12] 266: Updating with rohans suggestions to tidy up and
 'rubify' the code. Thanks for the tips Rohan!

---
 .../admin/reports_controller_decorator.rb     |  2 +-
 app/helpers/spree/reports_helper.rb           | 20 ++-----------------
 .../order_cycle_management_report.rb          |  4 ++--
 3 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb
index 7ffb32d097..b1deb5c3ed 100644
--- a/app/controllers/spree/admin/reports_controller_decorator.rb
+++ b/app/controllers/spree/admin/reports_controller_decorator.rb
@@ -631,7 +631,7 @@ Spree::Admin::ReportsController.class_eval do
       :orders_and_fulfillment => {:name => "Orders & Fulfillment Reports", :description => ''},
       :customers => {:name => "Customers", :description => 'Customer details'},
       :products_and_inventory => {:name => "Products & Inventory", :description => ''},
-      :order_cycle_management => {:name => "UK Order Cycle Management", :description => ''}
+      :order_cycle_management => {:name => "Order Cycle Management", :description => ''}
       
     }
     # Return only reports the user is authorized to view.
diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb
index 967cd7fd36..5860625991 100644
--- a/app/helpers/spree/reports_helper.rb
+++ b/app/helpers/spree/reports_helper.rb
@@ -8,28 +8,12 @@ module Spree
       end
     end
 
-    #lin-d-hop
-    #Find the payment methods options for reporting. 
-    #I don't like that this is done in two loops, but redundant list entries
-    #  were created otherwise... 
     def report_payment_method_options(orders)
-      payment_method_list = []
-      orders.map do |o| 
-        payment_method_list << o.payments.first.payment_method.andand.name
-      end
-      payment_method_list.uniq.each do |pm|
-        [ "#{pm}".html_safe,  pm]
-      end
+      orders.map { |o| o.payments.first.payment_method.andand.name }.uniq
     end
 
     def report_distribution_options(orders)
-      distribution_list = []
-      orders.map do |o| 
-        distribution_list << o.shipping_method.andand.name 
-      end
-      distribution_list.uniq.each do |pm|
-        [ "#{pm}".html_safe,  pm]
-      end
+      orders.map { |o| o.shipping_method.andand.name  }.uniq      
     end    
 
   end
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index fb4da5468d..2087f901de 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -44,7 +44,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_payment_method (orders)
-      if params.has_key? (:payment_method_name)
+      if params[:payment_method_name].present?
         orders.with_payment_method_name(params[:payment_method_name])
       else
         orders
@@ -52,7 +52,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_distribution (orders)
-      if params.has_key? (:distribution_name)
+      if params[:distribution_name].present?
         orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:distribution_name])
       else
         orders

From 35c27bf516f91c13025f0818f98e5db8836567a3 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Thu, 27 Nov 2014 20:36:41 +0000
Subject: [PATCH 05/12] First specs for additional scope to order model. Not
 liking the repeated code so would appreciate feedback to get rid of it.
 Thanks!

---
 spec/models/spree/order_spec.rb | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 63c315a18a..9326368e87 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -77,7 +77,7 @@ describe Spree::Order do
       subject.update_distribution_charge!
     end
 
-    it "ensures the correct adjustment(s) are created for order cycles" do
+    it "ensures the correct adjustment(s) are created for order cycles" do 
       EnterpriseFee.stub(:clear_all_adjustments_on_order)
       line_item = double(:line_item)
       subject.stub(:line_items) { [line_item] }
@@ -320,15 +320,39 @@ describe Spree::Order do
     end
   end
 
-   describe "scopes" do
+  describe "scopes" do
     describe "not_state" do
       it "finds only orders not in specified state" do
         o = FactoryGirl.create(:completed_order_with_totals)
         o.cancel!
-
         Spree::Order.not_state(:canceled).should_not include o
       end
     end
+
+    describe "with payment method name" do
+      
+      it "returns the order with payment method name" do
+        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
+        my_order = FactoryGirl.create(:order) 
+        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
+	Spree::Order.with_payment_method_name("PM Test").should include my_order
+      end
+     
+      it "doesn't return rows with a different payment method name" do
+        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
+        my_order = FactoryGirl.create(:order) 
+        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
+        Spree::Order.with_payment_method_name("PM Test2").should_not include my_order
+      end
+  
+      it "doesn't return duplicate rows" do
+        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
+        my_order = FactoryGirl.create(:order) 
+        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method) 
+        payment2 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
+	Spree::Order.with_payment_method_name("PM Test").length.should == 1
+      end
+    end
   end
 
   describe "shipping address prepopulation" do
@@ -365,5 +389,4 @@ describe Spree::Order do
       order.create_shipment!
     end
   end
-
 end

From c2c51a55316b0974cb4b9ac71eb3433795707045 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Tue, 2 Dec 2014 12:24:34 +0000
Subject: [PATCH 06/12] Fixing typo in feature spec

---
 spec/features/admin/reports_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index 82cdbf59ba..d53c072a4b 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -65,7 +65,7 @@ feature %q{
 
     scenario "order payment method report" do
       click_link "Order Cycle Management"
-      rows = find("table#listing_order_payment_method").all("thead tr")
+      rows = find("table#listing_order_payment_methods").all("thead tr")
       table = rows.map { |r| r.all("th").map { |c| c.text.strip } }
       table.sort.should == [
         ["First Name", "Last Name", "Email", "Phone", "Hub", "Payment Method", "Amount", "Amount Paid"]

From a5ae1c490cadd7f62bb1a155fd1fe69b380b814f Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Tue, 2 Dec 2014 17:48:04 +0000
Subject: [PATCH 07/12] Fixing typo in report found through testing

---
 lib/open_food_network/order_cycle_management_report.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 2087f901de..0f722e94d3 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -40,7 +40,7 @@ module OpenFoodNetwork
     end	
 
     def filter_active (orders)
-       orders.complete.where("spree_orders.state != ?", :cancelled)
+       orders.complete.where("spree_orders.state != ?", :canceled)
     end
 
     def filter_to_payment_method (orders)

From 633a8a49e2d88e2d6b8f38e4f876ebd0aee4261d Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Wed, 3 Dec 2014 00:35:53 +0000
Subject: [PATCH 08/12] updating spec based on the wise advice of Rohan

---
 spec/models/spree/order_spec.rb | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 9326368e87..f06402cc02 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -330,27 +330,25 @@ describe Spree::Order do
     end
 
     describe "with payment method name" do
-      
+      let!(:o1) { create(:order) }
+      let!(:o2) { create(:order) }
+      let!(:pm1) { create(:payment_method, name: 'foo') }
+      let!(:pm2) { create(:payment_method, name: 'bar') }
+      let!(:p1) { create(:payment, order: o1, payment_method: pm1) }
+      let!(:p2) { create(:payment, order: o2, payment_method: pm2) }
+
       it "returns the order with payment method name" do
-        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
-        my_order = FactoryGirl.create(:order) 
-        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
-	Spree::Order.with_payment_method_name("PM Test").should include my_order
+	Spree::Order.with_payment_method_name('foo').should == [o1]
       end
      
       it "doesn't return rows with a different payment method name" do
-        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
-        my_order = FactoryGirl.create(:order) 
-        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
-        Spree::Order.with_payment_method_name("PM Test2").should_not include my_order
+        Spree::Order.with_payment_method_name('foobar').should_not include o1
+        Spree::Order.with_payment_method_name('foobar').should_not include o2
       end
   
       it "doesn't return duplicate rows" do
-        my_payment_method = FactoryGirl.create(:payment_method, :name => "PM Test") 
-        my_order = FactoryGirl.create(:order) 
-        payment1 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method) 
-        payment2 = FactoryGirl.create(:payment, :order => my_order, :payment_method => my_payment_method)
-	Spree::Order.with_payment_method_name("PM Test").length.should == 1
+        p2 = FactoryGirl.create(:payment, :order => o1, :payment_method => pm1)
+	Spree::Order.with_payment_method_name('foo').length.should == 1
       end
     end
   end

From f878e1803724d1d98ab38a0f5e32998179c9a4b6 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Mon, 8 Dec 2014 16:25:18 +0000
Subject: [PATCH 09/12] Update that works better with the specs

---
 .../order_cycle_management_report.rb             | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 0f722e94d3..1db4804c33 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -28,23 +28,15 @@ module OpenFoodNetwork
     end
 
     def orders
-      filter Spree::Order
+      filter Spree::Order.managed_by(@user).distributed_by_user(@user).complete.where("spree_orders.state != ?", :canceled)
     end
 
     def filter(orders)
-      filter_for_user filter_active filter_to_order_cycle filter_to_payment_method filter_to_distribution orders
-    end
-  
-    def filter_for_user (orders)
-       orders.managed_by(@user).distributed_by_user(@user)
-    end	
-
-    def filter_active (orders)
-       orders.complete.where("spree_orders.state != ?", :canceled)
+      filter_to_order_cycle filter_to_payment_method filter_to_distribution orders
     end
 
     def filter_to_payment_method (orders)
-      if params[:payment_method_name].present?
+      if params[:payment_method_name].to_i > 0
         orders.with_payment_method_name(params[:payment_method_name])
       else
         orders
@@ -52,7 +44,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_distribution (orders)
-      if params[:distribution_name].present?
+      if params[:distribution_name].to_i > 0
         orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:distribution_name])
       else
         orders

From 079781576b07ab596b3ca724156e608cf16e19f8 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Thu, 11 Dec 2014 12:47:56 +0000
Subject: [PATCH 10/12] Adding new specs and a couple updates the lib/report

---
 .../order_cycle_management_report.rb          |   6 +-
 .../order_cycle_management_report_spec.rb     | 127 ++++++++++++++++++
 2 files changed, 130 insertions(+), 3 deletions(-)
 create mode 100644 spec/lib/open_food_network/order_cycle_management_report_spec.rb

diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 1db4804c33..0ac838c027 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -36,7 +36,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_payment_method (orders)
-      if params[:payment_method_name].to_i > 0
+      if params[:payment_method_name].present?
         orders.with_payment_method_name(params[:payment_method_name])
       else
         orders
@@ -44,7 +44,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_distribution (orders)
-      if params[:distribution_name].to_i > 0
+      if params[:distribution_name].present?
         orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:distribution_name])
       else
         orders
@@ -52,7 +52,7 @@ module OpenFoodNetwork
     end
 
     def filter_to_order_cycle(orders)
-      if params[:order_cycle_id].to_i > 0
+      if params[:order_cycle_id].present?
         orders.where(order_cycle_id: params[:order_cycle_id])
       else
         orders
diff --git a/spec/lib/open_food_network/order_cycle_management_report_spec.rb b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
new file mode 100644
index 0000000000..74069f6849
--- /dev/null
+++ b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
@@ -0,0 +1,127 @@
+require 'spec_helper'
+
+module OpenFoodNetwork
+  describe OrderCycleManagementReport do
+    context "as a site admin" do
+      let(:user) do 
+        user = create(:user)
+        user.spree_roles << Spree::Role.find_or_create_by_name!("admin")
+        user
+      end
+      subject { OrderCycleManagementReport.new user }
+
+      describe "fetching orders" do
+        it "fetches completed orders" do
+          o1 = create(:order)
+          o2 = create(:order, completed_at: 1.day.ago)
+          subject.orders.should == [o2]
+        end
+
+        it "does not show cancelled orders" do
+          o1 = create(:order, state: "canceled", completed_at: 1.day.ago)
+          o2 = create(:order, completed_at: 1.day.ago)
+          subject.orders.should == [o2]
+        end
+      end
+    end
+
+    context "as an enterprise user" do
+      let(:user) do 
+        user = create(:user)
+        user.spree_roles = []
+        user.save!
+        user
+      end
+
+      subject { OrderCycleManagementReport.new user }
+
+      describe "fetching orders" do
+        let(:supplier) { create(:supplier_enterprise) }
+        let(:product) { create(:simple_product, supplier: supplier) }
+        let(:order) { create(:order, completed_at: 1.day.ago) }
+
+        it "only shows orders managed by the current user" do
+          d1 = create(:distributor_enterprise)
+          d1.enterprise_roles.build(user: user).save
+          d2 = create(:distributor_enterprise)
+          d2.enterprise_roles.build(user: create(:user)).save
+
+          o1 = create(:order, distributor: d1, completed_at: 1.day.ago)
+          o2 = create(:order, distributor: d2, completed_at: 1.day.ago)
+
+          subject.should_receive(:filter).with([o1]).and_return([o1])
+          subject.orders.should == [o1]
+        end
+
+
+        it "does not show orders through a hub that the current user does not manage" do
+          # Given a supplier enterprise with an order for one of its products
+          supplier.enterprise_roles.build(user: user).save
+          order.line_items << create(:line_item, product: product)
+
+          # When I fetch orders, I should see no orders
+          subject.should_receive(:filter).with([]).and_return([])
+          subject.orders.should == []
+        end
+      end
+
+      describe "filtering orders" do
+        let(:orders) { Spree::Order.scoped }
+        let(:supplier) { create(:supplier_enterprise) }
+
+        it "returns all orders sans-params" do
+          subject.filter(orders).should == orders
+        end
+
+        it "filters to a specific order cycle" do
+          oc1 = create(:simple_order_cycle)
+          oc2 = create(:simple_order_cycle)
+          order1 = create(:order, order_cycle: oc1)
+          order2 = create(:order, order_cycle: oc2)
+
+          subject.stub(:params).and_return(order_cycle_id: oc1.id)
+          subject.filter(orders).should == [order1]
+        end
+
+	it "filters to a payment method" do
+          pm1 = create(:payment_method, name: "PM1")
+          pm2 = create(:payment_method, name: "PM2")
+	  order1 = create(:order)
+	  order2 = create(:order)
+	  payment1 = create(:payment, :order => order1, :payment_method => pm1)
+	  payment2 = create(:payment, :order => order2, :payment_method => pm2)
+
+          subject.stub(:params).and_return(payment_method_name: pm1.name)
+          subject.filter(orders).should == [order1]
+        end
+
+	it "filters to a shipping method" do
+          sm1 = create(:shipping_method, name: "ship1")
+          sm2 = create(:shipping_method, name: "ship2")
+          order1 = create(:order, shipping_method: sm1)
+          order2 = create(:order, shipping_method: sm2)
+
+          subject.stub(:params).and_return(distribution_name: sm1.name)
+          subject.filter(orders).should == [order1]
+        end
+
+        it "should do all the filters at once" do
+          pm1 = create(:payment_method, name: "PM1")
+          sm1 = create(:shipping_method, name: "ship1")
+          oc1 = create(:simple_order_cycle)
+          order1 = create(:order, order_cycle: oc1,shipping_method: sm1)
+	  payment1 = create(:payment, :order => order1, :payment_method => pm1)
+
+          subject.stub(:params).and_return(
+            order_cycle_id: oc1.id,
+            distribution_name: sm1.name,
+            payment_method_name: pm1.name)
+          subject.filter(orders)
+
+         end
+
+	
+      end
+    end
+  end
+end

From eeae72352b7c3f6568a7e51cdaec76923e13b2de Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Fri, 12 Dec 2014 10:11:33 +0000
Subject: [PATCH 11/12] Renamed methods and vars to better fit naming
 conventions

---
 app/helpers/spree/reports_helper.rb                       | 2 +-
 .../spree/admin/reports/order_cycle_management.html.haml  | 5 +++--
 lib/open_food_network/order_cycle_management_report.rb    | 8 ++++----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/app/helpers/spree/reports_helper.rb b/app/helpers/spree/reports_helper.rb
index 5860625991..22004577eb 100644
--- a/app/helpers/spree/reports_helper.rb
+++ b/app/helpers/spree/reports_helper.rb
@@ -12,7 +12,7 @@ module Spree
       orders.map { |o| o.payments.first.payment_method.andand.name }.uniq
     end
 
-    def report_distribution_options(orders)
+    def report_shipping_options(orders)
       orders.map { |o| o.shipping_method.andand.name  }.uniq      
     end    
 
diff --git a/app/views/spree/admin/reports/order_cycle_management.html.haml b/app/views/spree/admin/reports/order_cycle_management.html.haml
index 3428cf3977..4581d32bc6 100644
--- a/app/views/spree/admin/reports/order_cycle_management.html.haml
+++ b/app/views/spree/admin/reports/order_cycle_management.html.haml
@@ -14,8 +14,9 @@
                multiple: true, include_blank: true) 
   %br
   %br
-  = select_tag(:distribution_name, 
-      options_for_select(report_distribution_options(@orders), params[:distribution_name]), 
+  = label_tag nil, "Shipping Method: "
+  = select_tag(:shipping_method_name, 
+      options_for_select(report_shipping_options(@orders), params[:shipping_method_name]), 
 	       include_blank: true) 
   %br
   %br
diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 0ac838c027..bd7974b56e 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -32,7 +32,7 @@ module OpenFoodNetwork
     end
 
     def filter(orders)
-      filter_to_order_cycle filter_to_payment_method filter_to_distribution orders
+      filter_to_order_cycle filter_to_payment_method filter_to_shipping_method orders
     end
 
     def filter_to_payment_method (orders)
@@ -43,9 +43,9 @@ module OpenFoodNetwork
       end
     end
 
-    def filter_to_distribution (orders)
-      if params[:distribution_name].present?
-        orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:distribution_name])
+    def filter_to_shipping_method (orders)
+      if params[:shipping_method_name].present?
+        orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:shipping_method_name])
       else
         orders
       end

From 7e49bd634e443dfadbe954017342fddd33da6861 Mon Sep 17 00:00:00 2001
From: Lynne Davis <lynne@deanforestfoodhub.org.uk>
Date: Fri, 12 Dec 2014 18:23:43 +0000
Subject: [PATCH 12/12] Updated the specs with Rohans suggestions

---
 .../order_cycle_management_report.rb          |  2 +-
 .../order_cycle_management_report_spec.rb     | 46 ++++++++-----------
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index bd7974b56e..f1529482ed 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -43,7 +43,7 @@ module OpenFoodNetwork
       end
     end
 
-    def filter_to_shipping_method (orders)
+    def filter_to_shipping_method (orders)  
       if params[:shipping_method_name].present?
         orders.joins(:shipping_method).where("spree_shipping_methods.name = ?", params[:shipping_method_name])
       else
diff --git a/spec/lib/open_food_network/order_cycle_management_report_spec.rb b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
index 74069f6849..c010d6f253 100644
--- a/spec/lib/open_food_network/order_cycle_management_report_spec.rb
+++ b/spec/lib/open_food_network/order_cycle_management_report_spec.rb
@@ -1,5 +1,7 @@
 require 'spec_helper'
 
+include AuthenticationWorkflow
+
 module OpenFoodNetwork
   describe OrderCycleManagementReport do
     context "as a site admin" do
@@ -26,12 +28,7 @@ module OpenFoodNetwork
     end
 
     context "as an enterprise user" do
-      let(:user) do 
-        user = create(:user)
-        user.spree_roles = []
-        user.save!
-        user
-      end
+       let!(:user) { create_enterprise_user }
 
       subject { OrderCycleManagementReport.new user }
 
@@ -66,17 +63,22 @@ module OpenFoodNetwork
       end
 
       describe "filtering orders" do
-        let(:orders) { Spree::Order.scoped }
-        let(:supplier) { create(:supplier_enterprise) }
+        let!(:orders) { Spree::Order.scoped }
+        let!(:supplier) { create(:supplier_enterprise) }
 
-        it "returns all orders sans-params" do
+        let!(:oc1) { create(:simple_order_cycle) }
+        let!(:pm1) { create(:payment_method, name: "PM1") }
+        let!(:sm1) { create(:shipping_method, name: "ship1") }
+        let!(:order1) { create(:order, shipping_method: sm1, order_cycle: oc1) }
+        let!(:payment1) { create(:payment, order: order1, payment_method: pm1) }
+   
+	it "returns all orders sans-params" do
           subject.filter(orders).should == orders
         end
 
         it "filters to a specific order cycle" do
-          oc1 = create(:simple_order_cycle)
+          
           oc2 = create(:simple_order_cycle)
-          order1 = create(:order, order_cycle: oc1)
           order2 = create(:order, order_cycle: oc2)
 
           subject.stub(:params).and_return(order_cycle_id: oc1.id)
@@ -84,39 +86,31 @@ module OpenFoodNetwork
         end
 
 	it "filters to a payment method" do
-          pm1 = create(:payment_method, name: "PM1")
-          pm2 = create(:payment_method, name: "PM2")
-	  order1 = create(:order)
+          
+          pm2 = create(:payment_method, name: "PM2")  
 	  order2 = create(:order)
-	  payment1 = create(:payment, :order => order1, :payment_method => pm1)
-	  payment2 = create(:payment, :order => order2, :payment_method => pm2)
+	  payment2 = create(:payment, order: order2, payment_method: pm2)
 
           subject.stub(:params).and_return(payment_method_name: pm1.name)
           subject.filter(orders).should == [order1]
         end
 
 	it "filters to a shipping method" do
-          sm1 = create(:shipping_method, name: "ship1")
+         
           sm2 = create(:shipping_method, name: "ship2")
-          order1 = create(:order, shipping_method: sm1)
           order2 = create(:order, shipping_method: sm2)
 
-          subject.stub(:params).and_return(distribution_name: sm1.name)
+          subject.stub(:params).and_return(shipping_method_name: sm1.name)
           subject.filter(orders).should == [order1]
         end
 
         it "should do all the filters at once" do
-          pm1 = create(:payment_method, name: "PM1")
-          sm1 = create(:shipping_method, name: "ship1")
-          oc1 = create(:simple_order_cycle)
-          order1 = create(:order, order_cycle: oc1,shipping_method: sm1)
-	  payment1 = create(:payment, :order => order1, :payment_method => pm1)
 
           subject.stub(:params).and_return(
             order_cycle_id: oc1.id,
-            distribution_name: sm1.name,
+            shipping_method_name: sm1.name,
             payment_method_name: pm1.name)
-          subject.filter(orders)
+          subject.filter(orders).should == [order1]
 
          end