Restrict ability to change managers of enterprises using update action

app/controllers/admin/enterprises_controller.rb

@@ -9,6 +9,7 @@ module Admin
     before_filter :override_owner, only: :create
     before_filter :check_can_change_owner, only: :update
     before_filter :check_can_change_bulk_owner, only: :bulk_update
+    before_filter :check_can_change_managers, only: :update
 
     helper 'spree/products'
     include OrderCyclesHelper
@@ -130,6 +131,12 @@ module Admin
       end
     end
 
+    def check_can_change_managers
+      unless ( spree_current_user == @enterprise.owner ) || spree_current_user.admin?
+        params[:enterprise].delete :user_ids
+      end
+    end
+
     # Overriding method on Spree's resource controller
     def location_after_save
       if params[:enterprise].key? :producer_properties_attributes

spec/controllers/admin/enterprises_controller_spec.rb

@@ -3,36 +3,39 @@ require 'spec_helper'
 module Admin
   describe EnterprisesController do
     include AuthenticationWorkflow
-    let(:distributor_owner) do
-      user = create(:user)
-      user.spree_roles = []
-      user
-    end
-    let(:distributor) { create(:distributor_enterprise, owner: distributor_owner ) }
-    let(:user) do
+    let(:user) { create_enterprise_user }
+    let(:distributor_manager) do
       user = create(:user)
       user.spree_roles = []
       distributor.enterprise_roles.build(user: user).save
       user
     end
+    let(:distributor_owner) do
+      user = create(:user)
+      user.spree_roles = []
+      user
+    end
     let(:admin_user) do
       user = create(:user)
       user.spree_roles << Spree::Role.find_or_create_by_name!('admin')
       user
     end
 
+    let(:distributor) { create(:distributor_enterprise, owner: distributor_owner ) }
+
+
     describe "creating an enterprise" do
       let(:country) { Spree::Country.find_by_name 'Australia' }
       let(:state) { Spree::State.find_by_name 'Victoria' }
       let(:enterprise_params) { {enterprise: {name: 'zzz', permalink: 'zzz', email: "bob@example.com", address_attributes: {address1: 'a', city: 'a', zipcode: 'a', country_id: country.id, state_id: state.id}}} }
 
       it "grants management permission if the current user is an enterprise user" do
-        controller.stub spree_current_user: user
-        enterprise_params[:enterprise][:owner_id] = user
+        controller.stub spree_current_user: distributor_manager
+        enterprise_params[:enterprise][:owner_id] = distributor_manager
 
         spree_put :create, enterprise_params
         enterprise = Enterprise.find_by_name 'zzz'
-        user.enterprise_roles.where(enterprise_id: enterprise).first.should be
+        distributor_manager.enterprise_roles.where(enterprise_id: enterprise).first.should be
       end
 
       it "does not grant management permission to admins" do
@@ -45,41 +48,12 @@ module Admin
       end
 
       it "it overrides the owner_id submitted by the user unless current_user is super admin" do
-        controller.stub spree_current_user: user
-        enterprise_params[:enterprise][:owner_id] = admin_user
+        controller.stub spree_current_user: distributor_manager
+        enterprise_params[:enterprise][:owner_id] = user
 
         spree_put :create, enterprise_params
         enterprise = Enterprise.find_by_name 'zzz'
-        user.enterprise_roles.where(enterprise_id: enterprise).first.should be
-      end
-    end
-
-    describe "updating an enterprise" do
-      it "allows current owner to change ownership" do
-        controller.stub spree_current_user: distributor_owner
-        update_params = { id: distributor, enterprise: { owner_id: user } }
-        spree_post :update, update_params
-
-        distributor.reload
-        expect(distributor.owner).to eq user
-      end
-
-      it "allows super admin to change ownership" do
-        controller.stub spree_current_user: admin_user
-        update_params = { id: distributor, enterprise: { owner_id: user } }
-        spree_post :update, update_params
-
-        distributor.reload
-        expect(distributor.owner).to eq user
-      end
-
-      it "does not allow managers to change ownership" do
-        controller.stub spree_current_user: user
-        update_params = { id: distributor, enterprise: { owner_id: user } }
-        spree_post :update, update_params
-
-        distributor.reload
-        expect(distributor.owner).to eq distributor_owner
+        distributor_manager.enterprise_roles.where(enterprise_id: enterprise).first.should be
       end
     end
 
@@ -88,14 +62,52 @@ module Admin
 
       context "as manager" do
         it "does not allow 'sells' to be changed" do
-          profile_enterprise.enterprise_roles.build(user: user).save
-          controller.stub spree_current_user: user
+          profile_enterprise.enterprise_roles.build(user: distributor_manager).save
+          controller.stub spree_current_user: distributor_manager
           enterprise_params = { id: profile_enterprise, enterprise: { sells: 'any' } }
 
           spree_put :update, enterprise_params
           profile_enterprise.reload
           expect(profile_enterprise.sells).to eq 'none'
         end
+
+        it "does not allow owner to be changed" do
+          controller.stub spree_current_user: distributor_manager
+          update_params = { id: distributor, enterprise: { owner_id: distributor_manager } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.owner).to eq distributor_owner
+        end
+
+        it "does not allow managers to be changed" do
+          controller.stub spree_current_user: distributor_manager
+          update_params = { id: distributor, enterprise: { user_ids: [distributor_owner.id,distributor_manager.id,user.id] } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.users).to_not include user
+        end
+      end
+
+      context "as owner" do
+        it "allows owner to be changed" do
+          controller.stub spree_current_user: distributor_owner
+          update_params = { id: distributor, enterprise: { owner_id: distributor_manager } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.owner).to eq distributor_manager
+        end
+
+        it "allows managers to be changed" do
+          controller.stub spree_current_user: distributor_owner
+          update_params = { id: distributor, enterprise: { user_ids: [distributor_owner.id,distributor_manager.id,user.id] } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.users).to include user
+        end
       end
 
       context "as super admin" do
@@ -107,6 +119,25 @@ module Admin
           profile_enterprise.reload
           expect(profile_enterprise.sells).to eq 'any'
         end
+
+
+        it "allows owner to be changed" do
+          controller.stub spree_current_user: admin_user
+          update_params = { id: distributor, enterprise: { owner_id: distributor_manager } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.owner).to eq distributor_manager
+        end
+
+        it "allows managers to be changed" do
+          controller.stub spree_current_user: admin_user
+          update_params = { id: distributor, enterprise: { user_ids: [distributor_owner.id,distributor_manager.id,user.id] } }
+          spree_post :update, update_params
+
+          distributor.reload
+          expect(distributor.users).to include user
+        end
       end
     end
 
@@ -114,7 +145,7 @@ module Admin
       let(:enterprise) { create(:enterprise, sells: 'none') }
 
       before do
-        controller.stub spree_current_user: user
+        controller.stub spree_current_user: distributor_manager
       end
 
       context "as a normal user" do
@@ -126,7 +157,7 @@ module Admin
 
       context "as a manager" do
         before do
-          enterprise.enterprise_roles.build(user: user).save
+          enterprise.enterprise_roles.build(user: distributor_manager).save
         end
 
         context "allows setting 'sells' to 'none'" do