Merge branch '290006-error-500-on-members-page-after-invitation-sent-via-api' into 'master'

Resolve Members page 500 error after Invitation sent via API

See merge request gitlab-org/gitlab!48937

app/helpers/members_helper.rb

@@ -6,14 +6,14 @@ module MembersHelper
     text = 'Are you sure you want to'
 
     action =
-      if member.request?
+      if member.invite?
+        "revoke the invitation for #{member.invite_email} to join"
+      elsif member.request?
         if member.user == user
           'withdraw your access request for'
         else
           "deny #{member.user.name}'s request to join"
         end
-      elsif member.invite?
-        "revoke the invitation for #{member.invite_email} to join"
       else
         if member.user
           "remove #{member.user.name} from"

app/services/members/invite_service.rb

@@ -20,8 +20,8 @@ module Members
 
       emails.each do |email|
         next if existing_member?(source, email)
-
         next if existing_invite?(source, email)
+        next if existing_request?(source, email)
 
         if existing_user?(email)
           add_existing_user_as_member(current_user, source, params, email)
@@ -44,8 +44,7 @@ module Members
         access_level: params[:access_level],
         invite_email: email,
         created_by_id: current_user.id,
-        expires_at: params[:expires_at],
-        requested_at: Time.current.utc)
+        expires_at: params[:expires_at])
 
       unless new_member.valid? && new_member.persisted?
         errors[params[:email]] = new_member.errors.full_messages.to_sentence
@@ -92,6 +91,17 @@ module Members
       false
     end
 
+    def existing_request?(source, email)
+      existing_request = source.requesters.with_user_by_email(email).exists?
+
+      if existing_request
+        errors[email] = "Member cannot be invited because they already requested to join #{source.name}"
+        return true
+      end
+
+      false
+    end
+
     def existing_user(email)
       User.find_by_email(email)
     end

changelogs/unreleased/290006-error-500-on-members-page-after-invitation-sent-via-api.yml

+---
+title: Resolve Members page 500 error after Invitation sent via API
+merge_request: 48937
+author:
+type: fixed

doc/api/invitations.md

@@ -97,7 +97,7 @@ Example response:
    {
      "id": 1,
      "invite_email": "member@example.org",
-     "invited_at": "2020-10-22T14:13:35Z",
+     "created_at": "2020-10-22T14:13:35Z",
      "access_level": 30,
      "expires_at": "2020-11-22T14:13:35Z",
      "user_name": "Raymond Smith",

lib/api/entities/invitation.rb

@@ -4,7 +4,7 @@ module API
   module Entities
     class Invitation < Grape::Entity
       expose :access_level
-      expose :requested_at
+      expose :created_at
       expose :expires_at
       expose :invite_email
       expose :invite_token

spec/helpers/members_helper_spec.rb

@@ -33,6 +33,16 @@ RSpec.describe MembersHelper do
         expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to remove this orphaned member from the #{group.name} group and any subresources?"
       end
     end
+
+    context 'a pending member invitation with no user associated' do
+      before do
+        project_member_invite.update_columns(invite_email: "#{SecureRandom.hex}@example.com", invite_token: 'some-token', user_id: nil)
+      end
+
+      it 'does not error when there is an invitation for the requestor' do
+        expect(remove_member_message(project_member_invite)).to eq "Are you sure you want to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.full_name} project?"
+      end
+    end
   end
 
   describe '#remove_member_title' do

spec/requests/api/invitations_spec.rb

@@ -58,7 +58,7 @@ RSpec.describe API::Invitations do
           it 'does not transform the requester into a proper member' do
             expect do
               post api("/#{source_type.pluralize}/#{source.id}/invitations", maintainer),
-                   params: { email: email, access_level: Member::MAINTAINER }
+                   params: { email: access_requester.email, access_level: Member::MAINTAINER }
 
               expect(response).to have_gitlab_http_status(:created)
             end.not_to change { source.members.count }
@@ -71,7 +71,7 @@ RSpec.describe API::Invitations do
                  params: { email: email, access_level: Member::DEVELOPER }
 
             expect(response).to have_gitlab_http_status(:created)
-          end.to change { source.requesters.count }.by(1)
+          end.to change { source.members.invite.count }.by(1)
         end
 
         it 'invites a list of new email addresses' do
@@ -82,7 +82,7 @@ RSpec.describe API::Invitations do
                  params: { email: email_list, access_level: Member::DEVELOPER }
 
             expect(response).to have_gitlab_http_status(:created)
-          end.to change { source.requesters.count }.by(2)
+          end.to change { source.members.invite.count }.by(2)
         end
       end
 
@@ -140,7 +140,7 @@ RSpec.describe API::Invitations do
           it 'invites a member' do
             expect do
               subject
-            end.to change { source.requesters.count }.by(1)
+            end.to change { source.members.invite.count }.by(1)
 
             expect(response).to have_gitlab_http_status(:created)
           end

spec/services/members/invite_service_spec.rb

@@ -63,4 +63,15 @@ RSpec.describe Members::InviteService do
     expect(result[:status]).to eq(:error)
     expect(result[:message][invited_member.invite_email]).to eq("Member already invited to #{project.name}")
   end
+
+  it 'does not add a member with an access_request' do
+    requested_member = create(:project_member, :access_request, project: project)
+
+    params = { email: requested_member.user.email,
+               access_level: Gitlab::Access::GUEST }
+    result = described_class.new(user, params).execute(project)
+
+    expect(result[:status]).to eq(:error)
+    expect(result[:message][requested_member.user.email]).to eq("Member cannot be invited because they already requested to join #{project.name}")
+  end
 end