Merge branch 'reports' into bugfix

app/controllers/spree/admin/reports_controller_decorator.rb

@@ -6,24 +6,6 @@ require 'open_food_network/order_grouper'
 require 'open_food_network/customers_report'
 
 Spree::Admin::ReportsController.class_eval do
-  # Fetches user's distributors, suppliers and order_cycles
-  before_filter :load_data, only: [:customers, :products_and_inventory]
-
-  # Render a partial for orders and fulfillment description
-  respond_override :index => { :html => { :success => lambda {
-    @reports[:orders_and_fulfillment][:description] =
-      render_to_string(partial: 'orders_and_fulfillment_description', layout: false, locals: {report_types: REPORT_TYPES[:orders_and_fulfillment]}).html_safe
-    @reports[:products_and_inventory][:description] =
-      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
-  } } }
-
-  # OVERRIDING THIS so we use a method not a constant for available reports
-  def index
-    @reports = available_reports
-    respond_with(@reports)
-  end
 
   REPORT_TYPES = {
     orders_and_fulfillment: [
@@ -42,6 +24,26 @@ Spree::Admin::ReportsController.class_eval do
     ]
   }
 
+  # Fetches user's distributors, suppliers and order_cycles
+  before_filter :load_data, only: [:customers, :products_and_inventory]
+
+  # Render a partial for orders and fulfillment description
+  respond_override :index => { :html => { :success => lambda {
+    @reports[:orders_and_fulfillment][:description] =
+      render_to_string(partial: 'orders_and_fulfillment_description', layout: false, locals: {report_types: REPORT_TYPES[:orders_and_fulfillment]}).html_safe
+    @reports[:products_and_inventory][:description] =
+      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
+  } } }
+
+
+  # Overide spree reports list.
+  def index
+    @reports = authorized_reports
+    respond_with(@reports)
+  end
+
   # This action is short because we refactored it like bosses
   def customers
     @report_types = REPORT_TYPES[:customers]
@@ -601,19 +603,18 @@ Spree::Admin::ReportsController.class_eval do
     @order_cycles = OrderCycle.active_or_complete.accessible_by(spree_current_user).order('orders_close_at DESC')
   end
 
-  def available_reports
+  def authorized_reports
     reports = {
       :orders_and_distributors => {:name => "Orders And Distributors", :description => "Orders with distributor details"},
       :bulk_coop => {:name => "Bulk Co-Op", :description => "Reports for Bulk Co-Op orders"},
       :payments => {:name => "Payment Reports", :description => "Reports for Payments"},
       :orders_and_fulfillment => {:name => "Orders & Fulfillment Reports", :description => ''},
       :customers => {:name => "Customers", :description => 'Customer details'},
-      :products_and_inventory => {:name => "Products & Inventory", :description => ''}
+      :products_and_inventory => {:name => "Products & Inventory", :description => ''},
+      :sales_total => { :name => "Sales Total", :description => "Sales Total For All Orders" }
     }
-    if spree_current_user.has_spree_role? 'admin'
-      reports[:sales_total] = { :name => "Sales Total", :description => "Sales Total For All Orders" }
-    end
-    reports
+    # Return only reports the user is authorized to view.
+    reports.select { |action| can? action, :report }
   end
 
   def total_units(line_items)

app/models/spree/ability_decorator.rb

@@ -1,6 +1,8 @@
 class AbilityDecorator
   include CanCan::Ability
 
+  # All abilites are allocated from this initialiser, currently in 5 chunks.
+  # Spree also defines other abilities.
   def initialize(user)
     add_base_abilities user if is_new_user? user
     add_enterprise_management_abilities user if can_manage_enterprises? user
@@ -9,19 +11,22 @@ class AbilityDecorator
     add_relationship_management_abilities user if can_manage_relationships? user
   end
 
-
+  # New users have no enterprises.
   def is_new_user?(user)
     user.enterprises.blank?
   end
 
+  # Users can manage an enterprise if they have one.
   def can_manage_enterprises?(user)
     user.enterprises.present?
   end
 
+  # Users can manage products if they have an enterprise.
   def can_manage_products?(user)
     can_manage_enterprises? user
   end
 
+  # Users can manage orders if they have a sells own/any enterprise.
   def can_manage_orders?(user)
     ( user.enterprises.map(&:type) & %w(single full) ).any?
   end
@@ -30,6 +35,7 @@ class AbilityDecorator
     can_manage_enterprises? user
   end
 
+  # New users can create an enterprise, and gain other permissions from doing this.
   def add_base_abilities(user)
     can [:create], Enterprise
   end
@@ -47,6 +53,12 @@ class AbilityDecorator
     can [:read, :edit, :update, :bulk_update], Enterprise do |enterprise|
       user.enterprises.include? enterprise
     end
+
+    # All enterprises can have fees, though possibly suppliers don't need them?
+    can [:index, :create], EnterpriseFee
+    can [:admin, :read, :edit, :bulk_update, :destroy], EnterpriseFee do |enterprise_fee|
+      user.enterprises.include? enterprise_fee.enterprise
+    end
   end
 
   def add_product_management_abilities(user)
@@ -66,6 +78,9 @@ class AbilityDecorator
 
     can [:admin, :index, :read, :search], Spree::Taxon
     can [:admin, :index, :read, :create, :edit], Spree::Classification
+
+    # Reports page
+    can [:admin, :index, :customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory], :report
   end
 
   def add_order_management_abilities(user)
@@ -76,7 +91,7 @@ class AbilityDecorator
       # during the order creation process from the admin backend
       order.distributor.nil? || user.enterprises.include?(order.distributor)
     end
-    can [:admin, :bulk_management], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor?)
+    can [:admin, :bulk_management], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor)
     can [:admin, :create], Spree::LineItem
 
     can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Payment
@@ -90,11 +105,6 @@ class AbilityDecorator
     end
     can [:for_order_cycle], Enterprise
 
-    can [:index, :create], EnterpriseFee
-    can [:admin, :read, :edit, :bulk_update, :destroy], EnterpriseFee do |enterprise_fee|
-      user.enterprises.include? enterprise_fee.enterprise
-    end
-
     can [:admin, :index, :read, :create, :edit, :update], ExchangeVariant
     can [:admin, :index, :read, :create, :edit, :update], Exchange
     can [:admin, :index, :read, :create, :edit, :update], ExchangeFee
@@ -111,7 +121,7 @@ class AbilityDecorator
     end
 
     # Reports page
-    can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory], :report
+    can [:admin, :index, :customers, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory], :report
   end
 
 

spec/models/spree/ability_spec.rb

@@ -151,6 +151,14 @@ module Spree
           should_not have_ability(:destroy, for: er2)
         end
 
+        it "should be able to read some reports" do
+          should have_ability([:admin, :index, :customers, :orders_and_fulfillment, :products_and_inventory], for: :reports)
+        end
+
+        it "should not be able to read other reports" do
+          should_not have_ability([:sales_total, :group_buys, :bulk_coop, :payments], for: :reports)
+        end
+
       end
 
       context "when is a distributor enterprise user" do
@@ -237,6 +245,15 @@ module Spree
         it "should not be able to destroy enterprise relationships for other enterprises" do
           should_not have_ability(:destroy, for: er1)
         end
+
+        it "should be able to read some reports" do
+          should have_ability([:admin, :index, :customers, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory], for: :reports)
+        end
+
+        it "should not be able to read other reports" do
+          should_not have_ability([:sales_total], for: :reports)
+        end
+
       end
 
       context 'Order Cycle co-ordinator' do