Commit 036fac06 authored by Linus G Thiel's avatar Linus G Thiel Committed by Dmitriy Zaporozhets

Gracefully handle adding of no users to projects and groups

- Disable {project, group} members submit button if no users

  If no users are selected, the submit button should be disabled.

- Alert user when no users were added to {project, group}.

  When no users were selected for adding, an alert message is
  flashed that no users were added.

- Also, this commit adds a feedback when users were actually added to a
  project, in symmetry with how group members are handled.

Closes #22967, #23270.
parent 9dbd5b3c
...@@ -69,6 +69,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -69,6 +69,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Update Gitlab Shell to fix some problems with moving projects between storages - Update Gitlab Shell to fix some problems with moving projects between storages
- Cache rendered markdown in the database, rather than Redis - Cache rendered markdown in the database, rather than Redis
- Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references
- Better handle when no users were selected for adding to group or project. (Linus Thiel)
- Simplify Mentionable concern instance methods - Simplify Mentionable concern instance methods
- API: Ability to retrieve version information (Robert Schilling) - API: Ability to retrieve version information (Robert Schilling)
- Fix permission for setting an issue's due date - Fix permission for setting an issue's due date
......
...@@ -83,14 +83,15 @@ ...@@ -83,14 +83,15 @@
}; };
// Disable button if text field is empty // Disable button if text field is empty
window.disableButtonIfEmptyField = function(field_selector, button_selector) { window.disableButtonIfEmptyField = function(field_selector, button_selector, event_name) {
event_name = event_name || 'input';
var closest_submit, field; var closest_submit, field;
field = $(field_selector); field = $(field_selector);
closest_submit = field.closest('form').find(button_selector); closest_submit = field.closest('form').find(button_selector);
if (rstrip(field.val()) === "") { if (rstrip(field.val()) === "") {
closest_submit.disable(); closest_submit.disable();
} }
return field.on('input', function() { return field.on(event_name, function() {
if (rstrip($(this).val()) === "") { if (rstrip($(this).val()) === "") {
return closest_submit.disable(); return closest_submit.disable();
} else { } else {
......
...@@ -21,6 +21,10 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -21,6 +21,10 @@ class Groups::GroupMembersController < Groups::ApplicationController
end end
def create def create
if params[:user_ids].empty?
return redirect_to group_group_members_path(@group), alert: 'No users specified.'
end
@group.add_users( @group.add_users(
params[:user_ids].split(','), params[:user_ids].split(','),
params[:access_level], params[:access_level],
......
...@@ -25,6 +25,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -25,6 +25,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController
end end
def create def create
if params[:user_ids].empty?
return redirect_to namespace_project_project_members_path(@project.namespace, @project), alert: 'No users specified.'
end
@project.team.add_users( @project.team.add_users(
params[:user_ids].split(','), params[:user_ids].split(','),
params[:access_level], params[:access_level],
...@@ -32,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -32,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
current_user: current_user current_user: current_user
) )
redirect_to namespace_project_project_members_path(@project.namespace, @project) redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.'
end end
def update def update
......
...@@ -29,3 +29,6 @@ ...@@ -29,3 +29,6 @@
%ul.content-list %ul.content-list
= render partial: 'shared/members/member', collection: @members, as: :member = render partial: 'shared/members/member', collection: @members, as: :member
= paginate @members, theme: 'gitlab' = paginate @members, theme: 'gitlab'
:javascript
window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change');
...@@ -26,3 +26,6 @@ ...@@ -26,3 +26,6 @@
= render 'team', members: @project_members = render 'team', members: @project_members
= paginate @project_members, theme: "gitlab" = paginate @project_members, theme: "gitlab"
:javascript
window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change');
...@@ -13,6 +13,60 @@ describe Groups::GroupMembersController do ...@@ -13,6 +13,60 @@ describe Groups::GroupMembersController do
end end
end end
describe '#create' do
let(:group) { create(:group, :public) }
context 'when users are added' do
let(:user) { create(:user) }
let(:group_user) { create(:user) }
let(:member) do
group.add_developer(group_user)
group.members.find_by(user_id: group_user)
end
context 'when user does not have enough rights' do
before do
group.members.delete(member)
group.add_developer(user)
sign_in(user)
end
it 'returns 403' do
post :create, group_id: group,
user_ids: member
expect(response).to have_http_status(403)
expect(group.users).not_to include group_user
end
end
context 'when user has enough rights' do
before do
group.add_owner(user)
sign_in(user)
end
it 'adds user to members' do
post :create, group_id: group,
user_ids: member
expect(response).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).to include group_user
end
it 'adds no user to members' do
post :create, group_id: group,
user_ids: ''
expect(response).to set_flash.to 'No users specified.'
expect(response).to redirect_to(group_group_members_path(group))
expect(group.users).not_to include group_user
end
end
end
end
describe 'DELETE destroy' do describe 'DELETE destroy' do
let(:member) { create(:group_member, :developer, group: group) } let(:member) { create(:group_member, :developer, group: group) }
......
...@@ -13,6 +13,63 @@ describe Projects::ProjectMembersController do ...@@ -13,6 +13,63 @@ describe Projects::ProjectMembersController do
end end
end end
describe '#create' do
let(:project) { create(:project, :public) }
context 'when users are added' do
let(:user) { create(:user) }
let(:team_user) { create(:user) }
let(:member) do
project.team << [team_user, :developer]
project.members.find_by(user_id: team_user.id)
end
context 'when user does not have enough rights' do
before do
project.members.delete(member)
project.team << [user, :developer]
sign_in(user)
end
it 'returns 404' do
post :create, namespace_id: project.namespace,
project_id: project,
user_ids: member
expect(response).to have_http_status(404)
expect(project.users).not_to include team_user
end
end
context 'when user has enough rights' do
before do
project.team << [user, :master]
sign_in(user)
end
it 'adds user to members' do
post :create, namespace_id: project.namespace,
project_id: project,
user_ids: member
expect(response).to set_flash.to 'Users were successfully added.'
expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
expect(project.users).to include team_user
end
it 'adds no user to members' do
post :create, namespace_id: project.namespace,
project_id: project,
user_ids: ''
expect(response).to set_flash.to 'No users specified.'
expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
expect(project.users).not_to include team_user
end
end
end
end
describe 'DELETE destroy' do describe 'DELETE destroy' do
let(:member) { create(:project_member, :developer, project: project) } let(:member) { create(:project_member, :developer, project: project) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment