From 0d715ce61551b4acc9e54f165d200b5034d4409c Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Fri, 10 Oct 2014 19:36:37 +1100 Subject: [PATCH 1/6] split report permissions --- app/models/spree/ability_decorator.rb | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 5fdb4c2342..a62eae3935 100644 --- a/app/models/spree/ability_decorator.rb +++ b/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, :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_fulfillment, :products_and_inventory], :report end From e44fed2ff0cb6d7ea6a8a733caf1a672c94ac9c7 Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Sun, 12 Oct 2014 14:00:32 +1100 Subject: [PATCH 2/6] add authorization to reports listings on index page --- .../spree/admin/reports_controller_decorator.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 0abfd783b5..c7f5aa363a 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -608,12 +608,10 @@ Spree::Admin::ReportsController.class_eval do :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 + reports.select { |action, details| can? action, :report } end def total_units(line_items) From 46df14c0d96290ba928d8d35decec94c22d7936e Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Sun, 12 Oct 2014 14:13:52 +1100 Subject: [PATCH 3/6] refator reports controller a little --- .../admin/reports_controller_decorator.rb | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index c7f5aa363a..3a3ca9de48 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/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,7 +603,7 @@ 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"}, @@ -611,7 +613,8 @@ Spree::Admin::ReportsController.class_eval do :products_and_inventory => {:name => "Products & Inventory", :description => ''}, :sales_total => { :name => "Sales Total", :description => "Sales Total For All Orders" } } - reports.select { |action, details| can? action, :report } + # Return only reports the user is authorized to view. + reports.select { |action| can? action, :report } end def total_units(line_items) From 1577c01a77300830196eb9115c6907dda2e0b965 Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Sun, 12 Oct 2014 22:21:59 +1100 Subject: [PATCH 4/6] add reports abilities specs --- spec/models/spree/ability_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 2d2d1cfb01..62e1973e7f 100644 --- a/spec/models/spree/ability_spec.rb +++ b/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 From 9343c3608b8999dd3be8d39883be60564023a96c Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Sun, 12 Oct 2014 22:22:35 +1100 Subject: [PATCH 5/6] allow supplier enterprise manager to see bulk coop reports --- app/models/spree/ability_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index a62eae3935..abf4aeac66 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -80,7 +80,7 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit], Spree::Classification # Reports page - can [:admin, :index, :customers, :orders_and_fulfillment, :products_and_inventory], :report + can [:admin, :index, :customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory], :report end def add_order_management_abilities(user) From fd7191f476549d187fdef676540b1f7b149f92d4 Mon Sep 17 00:00:00 2001 From: Rafael Schouten Date: Thu, 16 Oct 2014 05:26:38 +1100 Subject: [PATCH 6/6] add missing orders_and_distributors perm --- app/models/spree/ability_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index abf4aeac66..2d4831ec0b 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -121,7 +121,7 @@ class AbilityDecorator end # Reports page - can [:admin, :index, :customers, :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