Merge branch 'fix_stale_owner2' into 'master'

Prevent stale data in LDAP group sync last owner check

Representing https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/529

Fixes gitlab-org/gitlab-ce#17764

Turns out that switching to an AR association has resolved the issue. However, a merge problem from CE to EE caused the owners method to not be removed so it took precedence over the association. This removes that method and adds a test to ensure we don't have stale owner data in the future. 

See merge request !531

app/models/group.rb

@@ -104,10 +104,6 @@ class Group < Namespace
     end
   end
 
-  def owners
-    @owners ||= group_members.owners.includes(:user).map(&:user)
-  end
-
   def add_users(user_ids, access_level, current_user = nil, skip_notification: false)
     user_ids.each do |user_id|
       Member.add_user(self.group_members, user_id, access_level, current_user, skip_notification: skip_notification)

spec/lib/gitlab/ldap/group_sync_spec.rb

@@ -160,18 +160,17 @@ describe Gitlab::LDAP::GroupSync, lib: true do
             group_access: Gitlab::Access::DEVELOPER,
             provider: 'ldapmain'
           )
-          group1.add_users([user1.id],
+
+          group1.add_users([user1.id, user2.id],
             Gitlab::Access::OWNER, skip_notification: true)
         end
 
-        it 'refuses to downgrade the last owner' do
-          expect { group_sync.sync_groups }
-            .not_to change {
-              group1.members.where(
-                user_id: user1.id,
-                access_level: Gitlab::Access::OWNER
-              ).any?
-            }
+        # Check two users in a loop to uncover any stale group owner data
+        it 'downgrades one user but not the other' do
+          group_sync.sync_groups
+
+          expect(group1.members.pluck(:access_level).sort)
+            .to eq([Gitlab::Access::DEVELOPER, Gitlab::Access::OWNER])
         end
 
         context 'when user is a member of two groups from different providers' do