diff --git a/app/controllers/team_users_controller.rb b/app/controllers/team_users_controller.rb index 5b96752dc..2293f0c70 100644 --- a/app/controllers/team_users_controller.rb +++ b/app/controllers/team_users_controller.rb @@ -1,6 +1,7 @@ # TeamUsersController manages the creation/removal/update of members of a team. class TeamUsersController < ApplicationController before_action :set_team_user + before_action :promoted_owner, only: [:create, :update] before_action :only_owner, only: [:update, :destroy] after_action :verify_authorized @@ -8,6 +9,9 @@ class TeamUsersController < ApplicationController # POST /team_users def create + # Promote this user if it is a Portus admin. + @team_user.role = TeamUser.roles[:owner] if @promoted_role + if @team_user.errors.empty? && @team_user.save @team_user.create_activity!(:add_member, current_user) respond_with @team_user @@ -18,6 +22,13 @@ def create # PATCH/PUT /team_users/1 def update + # Send an error if an admin was about to get demoted. + if @promoted_role + @team_user.errors.add(:user, "cannot be demoted because it's a Portus admin") + respond_with @team_user.errors, status: :unprocessable_entity + return + end + team_user_params = params.require(:team_user).permit(:role) old_role = @team_user.role @@ -68,6 +79,18 @@ def set_team_user authorize @team_user end + # Sets the @promoted_role instance variable if a Portus admin is going to be + # set a role other than owner. + def promoted_owner + return if @team_user.user.nil? + + tu = params.require(:team_user).permit(:role) + role = TeamUser.roles[tu["role"]] + + return if role == TeamUser.roles[:owner] || !@team_user.user.admin? + @promoted_role = true + end + # Responds with an error if the client is trying to remove the only owner of # the team through either the update or the destroy methods. def only_owner diff --git a/app/views/team_users/create.js.erb b/app/views/team_users/create.js.erb index a9b4c1d89..aa6110ed1 100644 --- a/app/views/team_users/create.js.erb +++ b/app/views/team_users/create.js.erb @@ -2,7 +2,11 @@ $('#float-alert p').html("<%= escape_javascript(@team_user.errors.full_messages.join('
')) %>"); <% else %> $("<%= escape_javascript(render @team_user) %>").appendTo("#team_users"); - $('#float-alert p').html("New user added to the team"); + <% if @promoted_role %> + $('#float-alert p').html("New user added to the team (promoted to owner because it is a Portus admin)."); + <% else %> + $('#float-alert p').html("New user added to the team"); + <% end %> $('#add_team_user_form').fadeOut(); $('#add_team_user_btn i').addClass("fa-chevron-down") $('#add_team_user_btn i').removeClass("fa-chevron-up") diff --git a/spec/controllers/team_users_controller_spec.rb b/spec/controllers/team_users_controller_spec.rb index b7e33144e..a89583c2b 100644 --- a/spec/controllers/team_users_controller_spec.rb +++ b/spec/controllers/team_users_controller_spec.rb @@ -55,6 +55,16 @@ expect(assigns(:team_user).errors).to be_empty end + it "does not allow an admin to be demoted" do + user = create(:admin) + team.owners << user + + put :update, id: team.team_users.find_by(user: user).id, + team_user: { role: "contributor" }, format: "js" + expect(response.status).to eq 422 + expect(team.owners.exists?(user.id)).to be true + end + it "forces a page reload when the current user changes his role" do user = create(:user) team.owners << user @@ -76,6 +86,18 @@ expect(team.owners.exists?(new_user.id)).to be true end + it "sets admins as owners always" do + new_user = create(:admin) + post :create, + team_user: { + team: team.name, + user: new_user.username, + role: TeamUser.roles["viewer"] + }, + format: "js" + expect(team.owners.exists?(new_user.id)).to be true + end + it "returns an error if the user is not found" do post :create, team_user: { team: team.name, user: "ghost", role: TeamUser.roles["owner"] }, diff --git a/spec/features/teams_spec.rb b/spec/features/teams_spec.rb index cdadb7e99..48727aff7 100644 --- a/spec/features/teams_spec.rb +++ b/spec/features/teams_spec.rb @@ -104,6 +104,7 @@ describe "teams#show" do let!(:another) { create(:user) } + let!(:another_admin) { create(:admin) } before :each do visit team_path(team) @@ -143,6 +144,20 @@ expect(page).to have_content("Contributor") end + scenario "An admin can only be added as a team owner", js: true do + find("#add_team_user_btn").click + wait_for_effect_on("#add_team_user_form") + find("#team_user_role").select "Contributor" + find("#team_user_user").set another_admin.username + find("#add_team_user_form .btn").click + + wait_for_ajax + wait_for_effect_on("#float-alert") + + expect(page).to have_content("New user added to the team (promoted to") + expect(page).to have_content("Owner") + end + scenario "New team members have to exist on the system", js: true do find("#add_team_user_btn").click wait_for_effect_on("#add_team_user_form")