Merge branch 'fj-40053-error-500-members-list' into 'master'

Fix user membership destroy relation

Closes #40053

See merge request gitlab-org/gitlab-ce!15970

3 files changed, 98 insertions, 1 deletions

diff --git a/spec/migrations/clean_up_for_members_spec.rb b/spec/migrations/clean_up_for_members_spec.rb
new file mode 100644
index 00000000000..0258860d169
--- /dev/null
+++ b/spec/migrations/clean_up_for_members_spec.rb
@@ -0,0 +1,78 @@
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20171216111734_clean_up_for_members.rb')
+
+describe CleanUpForMembers, :migration do
+  let(:migration) { described_class.new }
+  let!(:group_member) { create_group_member }
+  let!(:unbinded_group_member) { create_group_member }
+  let!(:invited_group_member) { create_group_member(true) }
+  let!(:not_valid_group_member) { create_group_member }
+  let!(:project_member) { create_project_member }
+  let!(:invited_project_member) { create_project_member(true) }
+  let!(:unbinded_project_member) { create_project_member }
+  let!(:not_valid_project_member) { create_project_member }
+
+  it 'removes members without proper user_id' do
+    unbinded_group_member.update_column(:user_id, nil)
+    not_valid_group_member.update_column(:user_id, 9999)
+    unbinded_project_member.update_column(:user_id, nil)
+    not_valid_project_member.update_column(:user_id, 9999)
+
+    migrate!
+
+    expect(Member.all).not_to include(unbinded_group_member, not_valid_group_member, unbinded_project_member, not_valid_project_member)
+    expect(Member.all).to include(group_member, invited_group_member, project_member, invited_project_member)
+  end
+
+  def create_group_member(invited = false)
+    fill_member(GroupMember.new(group: create_group), invited)
+  end
+
+  def create_project_member(invited = false)
+    fill_member(ProjectMember.new(project: create_project), invited)
+  end
+
+  def fill_member(member_object, invited)
+    member_object.tap do |m|
+      m.access_level = 40
+      m.notification_level = 3
+
+      if invited
+        m.user_id = nil
+        m.invite_token = 'xxx'
+        m.invite_email = 'email@email.com'
+      else
+        m.user_id = create_user.id
+      end
+
+      m.save
+    end
+
+    member_object
+  end
+
+  def create_group
+    name = FFaker::Lorem.characters(10)
+
+    Group.create(name: name, path: name.downcase.gsub(/\s/, '_'))
+  end
+
+  def create_project
+    name = FFaker::Lorem.characters(10)
+    creator = create_user
+
+    Project.create(name: name,
+                   path: name.downcase.gsub(/\s/, '_'),
+                   namespace: creator.namespace,
+                   creator: creator)
+  end
+
+  def create_user
+    User.create(email: FFaker::Internet.email,
+                password: '12345678',
+                name: FFaker::Name.name,
+                username: FFaker::Internet.user_name,
+                confirmed_at: Time.now,
+                confirmation_token: nil)
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 2557ce71f2b..047a46886c7 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -22,7 +22,9 @@ describe User do
   describe 'associations' do
     it { is_expected.to have_one(:namespace) }
     it { is_expected.to have_many(:snippets).dependent(:destroy) }
-    it { is_expected.to have_many(:project_members).dependent(:destroy) }
+    it { is_expected.to have_many(:members) }
+    it { is_expected.to have_many(:project_members) }
+    it { is_expected.to have_many(:group_members) }
     it { is_expected.to have_many(:groups) }
     it { is_expected.to have_many(:keys).dependent(:destroy) }
     it { is_expected.to have_many(:deploy_keys).dependent(:destroy) }
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb
index 58a5bede3de..aeba9cd60bc 100644
--- a/spec/services/users/destroy_service_spec.rb
+++ b/spec/services/users/destroy_service_spec.rb
@@ -188,5 +188,22 @@ describe Users::DestroyService do
         end
       end
     end
+
+    describe "calls the before/after callbacks" do
+      it 'of project_members' do
+        expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:destroy).once
+
+        service.execute(user)
+      end
+
+      it 'of group_members' do
+        group_member = create(:group_member)
+        group_member.group.group_members.create(user: user, access_level: 40)
+
+        expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:destroy).once
+
+        service.execute(user)
+      end
+    end
   end
 end