Merge remote-tracking branch 'upstream/master' into pipeline-emails

* upstream/master: (372 commits)
  Enable Lint/StringConversionInInterpolation cop and autocorrect offenses
  resolve duplicated changelog entry
  credit myself :smile:
  change determine conditions
  override subject method in devise mailer
  follow the styleguide: Don't use parentheses around a literal
  wrap subject with method subject
  move spec back into shared example `an email sent from GitLab`
  stub config settings in spec
  remove empty line at block body end
  remove extra entry
  create new test in `spec/mailers/notify_spec.rb`
  move changelog to 8.13
  add configurable email subject suffix
  Fixes sidebar navigation.
  Convert "SSH Keys" Spinach features to RSpec
  Enable import/export back for non-admins
  Update gitlab-shell to 3.6.3
  Updated artwork of empty group state.
  Better empty state for Groups view.
  ...

1 files changed, 2 insertions, 15 deletions

diff --git a/lib/api/members.rb b/lib/api/members.rb
index 37f0a6512f4..a18ce769e29 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -59,13 +59,6 @@ module API
           authorize_admin_source!(source_type, source)
           required_attributes! [:user_id, :access_level]
 
-          access_requester = source.requesters.find_by(user_id: params[:user_id])
-          if access_requester
-            # We pass current_user = access_requester so that the requester doesn't
-            # receive a "access denied" email
-            ::Members::DestroyService.new(access_requester, access_requester.user).execute
-          end
-
           member = source.members.find_by(user_id: params[:user_id])
 
           # This is to ensure back-compatibility but 409 behavior should be used
@@ -73,18 +66,12 @@ module API
           conflict!('Member already exists') if source_type == 'group' && member
 
           unless member
-            source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
-            member = source.members.find_by(user_id: params[:user_id])
+            member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
           end
 
-          if member
+          if member.persisted? && member.valid?
             present member.user, with: Entities::Member, member: member
           else
-            # Since `source.add_user` doesn't return a member object, we have to
-            # build a new one and populate its errors in order to render them.
-            member = source.members.build(attributes_for_keys([:user_id, :access_level, :expires_at]))
-            member.valid? # populate the errors
-
             # This is to ensure back-compatibility but 400 behavior should be used
             # for all validation errors in 9.0!
             render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level)