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.

8 files changed, 130 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 909af5fc053..3fa0e67037e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
   - Cache rendered markdown in the database, rather than Redis
   - 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
   - API: Ability to retrieve version information (Robert Schilling)
   - Fix permission for setting an issue's due date
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 8a61669822c..17cbfd0e66f 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -83,14 +83,15 @@
   };
 
   // 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;
     field = $(field_selector);
     closest_submit = field.closest('form').find(button_selector);
     if (rstrip(field.val()) === "") {
       closest_submit.disable();
     }
-    return field.on('input', function() {
+    return field.on(event_name, function() {
       if (rstrip($(this).val()) === "") {
         return closest_submit.disable();
       } else {
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 18cd800c619..3a373e4a946 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -21,6 +21,10 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def create
+    if params[:user_ids].empty?
+      return redirect_to group_group_members_path(@group), alert: 'No users specified.'
+    end
+
     @group.add_users(
       params[:user_ids].split(','),
       params[:access_level],
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index 2a07d154853..2bac48c5490 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -25,6 +25,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   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(
       params[:user_ids].split(','),
       params[:access_level],
@@ -32,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
       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
 
   def update
diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml
index ebf9aca7700..b295ad0e182 100644
--- a/app/views/groups/group_members/index.html.haml
+++ b/app/views/groups/group_members/index.html.haml
@@ -29,3 +29,6 @@
     %ul.content-list
       = render partial: 'shared/members/member', collection: @members, as: :member
     = paginate @members, theme: 'gitlab'
+
+:javascript
+  window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change');
diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml
index bdeb704b6da..3904647b608 100644
--- a/app/views/projects/project_members/index.html.haml
+++ b/app/views/projects/project_members/index.html.haml
@@ -26,3 +26,6 @@
 
   = render 'team', members: @project_members
   = paginate @project_members, theme: "gitlab"
+
+:javascript
+  window.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change');
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index ad15b3f8f40..82eebe6f2d4 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -13,6 +13,60 @@ describe Groups::GroupMembersController do
     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
     let(:member) { create(:group_member, :developer, group: group) }
 
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 5e487241d07..44d907eafd7 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -13,6 +13,63 @@ describe Projects::ProjectMembersController do
     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
     let(:member) { create(:project_member, :developer, project: project) }