diff --git a/backend/app/controllers/spree/admin/users_controller.rb b/backend/app/controllers/spree/admin/users_controller.rb index 0fac0445196..ea43f7b7d8d 100644 --- a/backend/app/controllers/spree/admin/users_controller.rb +++ b/backend/app/controllers/spree/admin/users_controller.rb @@ -124,7 +124,7 @@ def model_class def collection return @collection if @collection - @search = Spree.user_class.ransack(params[:q]) + @search = super.ransack(params[:q]) @collection = @search.result.includes(:spree_roles) @collection = @collection.includes(:spree_orders) @collection = @collection.page(params[:page]).per(Spree::Config[:admin_products_per_page]) @@ -141,6 +141,10 @@ def user_params attributes += [{ spree_role_ids: [] }] end + if can? :manage, Spree::StockLocation + attributes += [{ stock_location_ids: [] }] + end + unless can? :update_password, @user attributes -= [:password, :password_confirmation] end @@ -161,24 +165,27 @@ def sign_in_if_change_own_password end def load_roles - @roles = Spree::Role.all + @roles = Spree::Role.accessible_by(current_ability) if @user @user_roles = @user.spree_roles end end def load_stock_locations - @stock_locations = Spree::StockLocation.all + @stock_locations = Spree::StockLocation.accessible_by(current_ability) end def set_roles - if user_params[:spree_role_ids] && can?(:manage, Spree::Role) - @user.spree_roles = Spree::Role.where(id: user_params[:spree_role_ids]) + if user_params[:spree_role_ids] + @user.spree_roles = Spree::Role.accessible_by(current_ability).where(id: user_params[:spree_role_ids]) end end def set_stock_locations - @user.stock_locations = Spree::StockLocation.where(id: (params[:user][:stock_location_ids] || [])) + if user_params[:stock_location_ids] + @user.stock_locations = + Spree::StockLocation.accessible_by(current_ability).where(id: user_params[:stock_location_ids]) + end end end end diff --git a/backend/spec/controllers/spree/admin/users_controller_spec.rb b/backend/spec/controllers/spree/admin/users_controller_spec.rb index 6606dc96aba..181dacfd02a 100644 --- a/backend/spec/controllers/spree/admin/users_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/users_controller_spec.rb @@ -18,6 +18,49 @@ } end + context "#index" do + stub_authorization! do |_user| + can :manage, Spree.user_class + end + + context "when the user can manage all users" do + it "assigns a list of all users as @collection" do + get :index, params: { id: user.id } + expect(assigns(:collection)).to eq [user] + end + + it "assigns a ransack search for Spree.user_class" do + get :index, params: { id: user.id } + expect(assigns[:search]).to be_a Ransack::Search + expect(assigns[:search].klass).to eq Spree.user_class + end + end + + context "when user cannot manage some users" do + stub_authorization! do |_user| + can :manage, Spree.user_class + cannot :manage, Spree.user_class, email: 'not_accessible_user@example.com' + end + + it "assigns a list of accessible users as @collection" do + create(:user, email: 'not_accessible_user@example.com') + + get :index, params: { id: user.id } + expect(assigns(:collection)).to eq [user] + end + end + + context "when Spree.user_class does not respond to #accessible_by" do + it "assigns a list of all users as @collection" do + allow(Spree.user_class).to receive(:respond_to?).and_call_original + allow(Spree.user_class).to receive(:respond_to?).with(:accessible_by).and_return(false) + + get :index, params: { id: user.id } + expect(assigns(:collection)).to eq [user] + end + end + end + context "#show" do stub_authorization! do |_user| can [:admin, :manage], Spree.user_class @@ -69,6 +112,68 @@ end end + context '#new' do + context "when the user can manage all roles" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :index, Spree::Role + end + + it "assigns a list of all roles as @roles" do + role = create(:role) + + get :new, params: { id: user.id } + expect(assigns(:roles)).to eq [role] + end + end + + context "when user cannot list some roles" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :index, Spree::Role + cannot :index, Spree::Role, name: 'not_accessible_role' + end + + it "assigns a list of accessible roles as @roles" do + accessible_role = create(:role, name: 'accessible_role') + create(:role, name: 'not_accessible_role') + + get :new, params: { id: user.id } + expect(assigns(:roles)).to eq [accessible_role] + end + end + + context "when the user can manage all stock_locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :index, Spree::StockLocation + end + + it "assigns a list of all stock_locations as @stock_locations" do + stock_location = create(:stock_location) + + get :new, params: { id: user.id } + expect(assigns(:stock_locations)).to eq [stock_location] + end + end + + context "when user cannot list some stock_locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :index, Spree::StockLocation + cannot :index, Spree::StockLocation, name: 'not_accessible_stock_location' + end + + it "assigns a list of accessible stock_locations as @stock_locations" do + accessible_stock_location = create(:stock_location, name: 'accessible_stock_location') + create(:stock_location, name: 'not_accessible_stock_location') + + get :new, params: { id: user.id } + expect(assigns(:stock_locations)).to eq [accessible_stock_location] + end + end + end + describe "#create" do let(:dummy_role) { Spree::Role.create(name: "dummyrole") } @@ -99,6 +204,11 @@ def user end context "when the user cannot manage roles" do + stub_authorization! do |_user| + can :manage, Spree.user_class + cannot :manage, Spree::Role + end + it "cannot assign users roles" do post :create, params: { user: { name: "Bob Bloggs", spree_role_ids: [dummy_role.id] } } expect(user.spree_roles).to eq([]) @@ -110,6 +220,21 @@ def user end end + context "when the user can manage only some roles" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::Role + cannot :manage, Spree::Role, name: "not_accessible_role" + end + + it "can assign accessible roles to user" do + role1 = Spree::Role.create(name: "accessible_role") + role2 = Spree::Role.create(name: "not_accessible_role") + post :create, params: { user: { spree_role_ids: [role1.id, role2.id] } } + expect(user.spree_roles).to eq([role1]) + end + end + it "can create a shipping_address" do post :create, params: { user: { ship_address_attributes: valid_address_attributes } } expect(user.reload.ship_address.city).to eq('New York') @@ -120,11 +245,45 @@ def user expect(user.reload.bill_address.city).to eq('New York') end - it "can set stock locations" do - location = Spree::StockLocation.create(name: "my_location") - location_2 = Spree::StockLocation.create(name: "my_location_2") - post :create, params: { user: { stock_location_ids: [location.id, location_2.id] } } - expect(user.stock_locations).to match_array([location, location_2]) + context "when the user can manage stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::StockLocation + end + + it "can create user with stock locations" do + location = Spree::StockLocation.create(name: "my_location") + post :create, params: { user: { stock_location_ids: [location.id] } } + expect(user.stock_locations).to eq([location]) + end + end + + context "when the user cannot manage stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + cannot :manage, Spree::StockLocation + end + + it "cannot assign users stock locations" do + location = Spree::StockLocation.create(name: "my_location") + post :create, params: { user: { stock_location_ids: [location.id] } } + expect(user.stock_locations).to eq([]) + end + end + + context "when the user can manage only some stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::StockLocation + cannot :manage, Spree::StockLocation, name: "not_accessible_location" + end + + it "can assign accessible stock locations to user" do + location1 = Spree::StockLocation.create(name: "accessible_location") + location2 = Spree::StockLocation.create(name: "not_accessible_location") + post :create, params: { user: { stock_location_ids: [location1.id, location2.id] } } + expect(user.stock_locations).to eq([location1]) + end end end @@ -179,6 +338,21 @@ def user end end + context "when the user can manage only some stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::Role + cannot :manage, Spree::Role, name: "not_accessible_role" + end + + it "can update accessible roles to user" do + role1 = Spree::Role.create(name: "accessible_role") + role2 = Spree::Role.create(name: "not_accessible_role") + put :update, params: { id: user.id, user: { spree_role_ids: [role1.id, role2.id] } } + expect(user.reload.spree_roles).to eq([role1]) + end + end + context "allowed to update email" do stub_authorization! do |_user| can [:admin, :update, :update_email], Spree.user_class @@ -236,11 +410,49 @@ def user expect(user.reload.bill_address.city).to eq('New York') end - it "can set stock locations" do - location = Spree::StockLocation.create(name: "my_location") - location_2 = Spree::StockLocation.create(name: "my_location_2") - post :update, params: { id: user.id, user: { stock_location_ids: [location.id, location_2.id] } } - expect(user.stock_locations).to match_array([location, location_2]) + context "when the user can manage stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::StockLocation + end + + it "can update user stock locations" do + location1 = Spree::StockLocation.create(name: "my_location") + location2 = Spree::StockLocation.create(name: "my_location2") + user.stock_locations = [location1] + put :update, params: { id: user.id, user: { stock_location_ids: [location2.id] } } + expect(user.reload.stock_locations).to eq([location2]) + end + end + + context "when the user cannot manage stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + cannot :manage, Spree::StockLocation + end + + it "cannot update users stock locations" do + location1 = Spree::StockLocation.create(name: "my_location") + location2 = Spree::StockLocation.create(name: "my_location2") + user.stock_locations = [location1] + put :update, params: { id: user.id, user: { stock_location_ids: [location2.id] } } + expect(user.reload.stock_locations).to eq([location1]) + end + end + + context "when the user can manage only some stock locations" do + stub_authorization! do |_user| + can :manage, Spree.user_class + can :manage, Spree::StockLocation + cannot :manage, Spree::StockLocation, name: "not_accessible_location" + end + + it "can update accessible stock locations to user" do + location1 = Spree::StockLocation.create(name: "accessible_location") + location2 = Spree::StockLocation.create(name: "not_accessible_location") + put :update, params: { id: user.id, user: { stock_location_ids: [location1.id, location2.id] } } + expect(user.reload.stock_locations).to eq([location1]) + end end end