diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9987b816a..2ae73db94 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -69,7 +69,8 @@ class UsersController < ApplicationController def create @user = User.new(user_params.merge(org_params).merge(password_params)) - if @user.save + + if valid_attributes? && @user.save redirect_to created_user_redirect_path else unless @user.errors[:organisation].empty? @@ -103,6 +104,14 @@ class UsersController < ApplicationController private + def valid_attributes? + @user.valid? + if user_params[:role] && !current_user.assignable_roles.key?(user_params[:role].to_sym) + @user.errors.add :role, I18n.t("validations.role.invalid") + end + @user.errors.empty? + end + def format_error_messages errors = @user.errors.to_hash @user.errors.clear diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index b87382f00..2d1bb5dda 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -423,7 +423,7 @@ RSpec.describe UsersController, type: :request do context "when a search parameter is passed" do let!(:other_user_2) { FactoryBot.create(:user, organisation: user.organisation, name: "joe", email: "other@example.com") } let!(:other_user_3) { FactoryBot.create(:user, name: "User 5", organisation: user.organisation, email: "joe@example.com") } - let!(:other_org_user) { FactoryBot.create(:user, name: "User 4", email: "joe@other_example.com") } + let!(:other_org_user) { FactoryBot.create(:user, name: "User 4", email: "joe@otherexample.com") } before do get "/organisations/#{user.organisation.id}/users?search=#{search_param}" @@ -792,19 +792,6 @@ RSpec.describe UsersController, type: :request do expect { patch "/users/#{other_user.id}", headers:, params: } .to change { other_user.reload.active }.from(true).to(false) end - - context "when the user name is missing" do - let(:other_user) { FactoryBot.create(:user, name: nil, organisation: user.organisation) } - - before do - patch "/users/#{other_user.id}", headers:, params: - end - - it "uses the user's email" do - follow_redirect! - expect(page).to have_content(other_user.email) - end - end end context "and tries to activate deactivated user" do