From c75687b51924a481dd05cf00f9fc28a3d8fd7735 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 31 Jan 2019 10:51:29 +0100
Subject: Update members service to cope with errors

---
 app/services/members/create_service.rb                     | 14 ++++++++++++--
 ...-ui-bug-adding-group-members-with-lower-permissions.yml |  5 +++++
 spec/services/members/create_service_spec.rb               |  9 +++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml

diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb
index 714b8586737..cf710fef52b 100644
--- a/app/services/members/create_service.rb
+++ b/app/services/members/create_service.rb
@@ -19,9 +19,19 @@ module Members
         current_user: current_user
       )
 
-      members.each { |member| after_execute(member: member) }
+      errors = []
 
-      success
+      members.each do |member|
+        if member.errors.any?
+          errors << "#{member.user.username}: #{member.errors.full_messages.to_sentence}"
+        else
+          after_execute(member: member)
+        end
+      end
+
+      return success unless errors.any?
+
+      error(errors.to_sentence)
     end
 
     private
diff --git a/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml b/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml
new file mode 100644
index 00000000000..f22524ef4b2
--- /dev/null
+++ b/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml
@@ -0,0 +1,5 @@
+---
+title: Resolve UI bug adding group members with lower permissions
+merge_request: 24820
+author:
+type: fixed
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index 5c01463d757..3bc05182932 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -36,4 +36,13 @@ describe Members::CreateService do
     expect(result[:message]).to be_present
     expect(project.users).not_to include project_user
   end
+
+  it 'does not add an invalid member' do
+    params = { user_ids: project_user.id.to_s, access_level: -1 }
+    result = described_class.new(user, params).execute(project)
+
+    expect(result[:status]).to eq(:error)
+    expect(result[:message]).to include(project_user.username)
+    expect(project.users).not_to include project_user
+  end
 end
-- 
cgit v1.2.1