Gracefully handle orphaned member invites

For some reason, member invite tokens can be cleared without saving an
associated user. These entries cause 500 errors to show up in the member
view, and they can't be fixed easily without manually deleting entries
in the database.

This commit does two things:

1. Remove these orphaned entries from the database.

2. Gracefully handle them in the code: log an exception with details and
   provide a message in the remove button that allows project/group admins
   to delete these entries.

On GitLab.com, the migration removes 19 rows with the latest orphaned
entry created on 2020-04-16. That suggests the underlying bug is still
there but does not happen very often.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/23198

app/helpers/members_helper.rb

@@ -15,7 +15,18 @@ module MembersHelper
       elsif member.invite?
         "revoke the invitation for #{member.invite_email} to join"
       else
+        if member.user
           "remove #{member.user.name} from"
+        else
+          e = RuntimeError.new("Data integrity error: no associated user for member ID #{member.id}")
+          Gitlab::ErrorTracking.track_exception(e,
+            member_id: member.id,
+            invite_email: member.invite_email,
+            invite_accepted_at: member.invite_accepted_at,
+            source_id: member.source_id,
+            source_type: member.source_type)
+          "remove this orphaned member from"
+        end
       end
 
     "#{text} #{action} the #{member.source.human_name} #{source_text(member)}?"

changelogs/unreleased/sh-fix-orphaned-invited-members.yml

+---
+title: Gracefully handle orphaned member invites
+merge_request: 30355
+author:
+type: fixed

db/migrate/20200424050250_remove_orphaned_invited_members.rb

+# frozen_string_literal: true
+
+class RemoveOrphanedInvitedMembers < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    # As of 2020-04-23, there are 19 entries on GitLab.com that match this criteria.
+    execute "DELETE FROM members WHERE user_id IS NULL AND invite_token IS NULL AND invite_accepted_at IS NOT NULL"
+  end
+
+  def down
+  end
+end

db/structure.sql

@@ -13429,5 +13429,6 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200423080334
 20200423080607
 20200423081409
+20200424050250
 \.
 

spec/helpers/members_helper_spec.rb

@@ -22,6 +22,17 @@ describe MembersHelper do
     it { expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group?" }
     it { expect(remove_member_message(group_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{group.name} group?" }
     it { expect(remove_member_message(group_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{group.name} group?" }
+
+    context 'an accepted user invitation with no user associated' do
+      before do
+        group_member_invite.update(invite_email: "#{SecureRandom.hex}@example.com", invite_token: nil, user_id: nil)
+      end
+
+      it 'logs an exception and shows orphaned status' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(anything, hash_including(:member_id, :invite_email, :invite_accepted_at))
+        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
   end
 
   describe '#remove_member_title' do

spec/migrations/remove_orphaned_invited_members_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20200424050250_remove_orphaned_invited_members.rb')
+
+describe RemoveOrphanedInvitedMembers do
+  let(:members_table) { table(:members) }
+  let(:users_table) { table(:users) }
+  let(:namespaces_table) { table(:namespaces) }
+  let(:projects_table) { table(:projects) }
+
+  let!(:user1) { users_table.create!(name: 'user1', email: 'user1@example.com', projects_limit: 1) }
+  let!(:user2) { users_table.create!(name: 'user2', email: 'user2@example.com', projects_limit: 1) }
+  let!(:group) { namespaces_table.create!(type: 'Group', name: 'group', path: 'group') }
+  let!(:project) { projects_table.create!(name: 'project', path: 'project', namespace_id: group.id) }
+
+  let!(:member1) { create_member(user_id: user1.id, source_type: 'Project', source_id: project.id, access_level: 10) }
+  let!(:member2) { create_member(user_id: user2.id, source_type: 'Group', source_id: group.id, access_level: 20) }
+
+  let!(:invited_member1) do
+    create_member(user_id: nil, source_type: 'Project', source_id: project.id,
+                  invite_token: SecureRandom.hex, invite_accepted_at: Time.now,
+                  access_level: 20)
+  end
+  let!(:invited_member2) do
+    create_member(user_id: nil, source_type: 'Group', source_id: group.id,
+                  invite_token: SecureRandom.hex, invite_accepted_at: Time.now,
+                  access_level: 20)
+  end
+
+  let!(:orphaned_member1) do
+    create_member(user_id: nil, source_type: 'Project', source_id: project.id,
+                  invite_accepted_at: Time.now, access_level: 30)
+  end
+  let!(:orphaned_member2) do
+    create_member(user_id: nil, source_type: 'Group', source_id: group.id,
+                  invite_accepted_at: Time.now, access_level: 20)
+  end
+
+  it 'removes orphaned invited members but keeps current members' do
+    expect { migrate! }.to change { members_table.count }.from(6).to(4)
+
+    expect(members_table.all.pluck(:id)).to contain_exactly(member1.id, member2.id, invited_member1.id, invited_member2.id)
+  end
+
+  def create_member(options)
+    members_table.create!(
+      {
+        notification_level: 0,
+        ldap: false,
+        override: false
+      }.merge(options)
+    )
+  end
+end