Merge branch 'group-owners-association' into 'master'

Turn Group#owners into a has_many association

## What does this MR do?

This turns the regular method `Group#owners` into a `has_many` association.

## Are there points in the code the reviewer needs to double check?

As far as I can tell there's no way to do this without using an intermediate association, but perhaps I'm missing something. The reason an intermediate association is needed is because the supplied Proc is applied to the _final_ association (the one returning users), this means that when using a single `has_many` you can't filter out any intermediate rows (e.g. group members).

## Why was this MR needed?

This code being a regular method would prevent eager loading of the owners of a Group, turning it into a `has_many` association resolves this problem. This was discovered in !4410.

## What are the relevant issue numbers?

None.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- [ ] Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4676

2 files changed, 18 insertions, 4 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index b8dffe9f5b9..e66e04371b2 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -9,6 +9,12 @@ class Group < Namespace
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
   alias_method :members, :group_members
   has_many :users, -> { where(members: { requested_at: nil }) }, through: :group_members
+
+  has_many :owners,
+    -> { where(members: { access_level: Gitlab::Access::OWNER }) },
+    through: :group_members,
+    source: :user
+
   has_many :project_group_links, dependent: :destroy
   has_many :shared_projects, through: :project_group_links, source: :project
   has_many :notification_settings, dependent: :destroy, as: :source
@@ -88,10 +94,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)
     user_ids.each do |user_id|
       Member.add_user(self.group_members, user_id, access_level, current_user)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index ccdcb29f773..2c19aa3f67f 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -158,6 +158,18 @@ describe Group, models: true do
     it { expect(group.has_master?(@members[:requester])).to be_falsey }
   end
 
+  describe '#owners' do
+    let(:owner) { create(:user) }
+    let(:developer) { create(:user) }
+
+    it 'returns the owners of a Group' do
+      group.add_owner(owner)
+      group.add_developer(developer)
+
+      expect(group.owners).to eq([owner])
+    end
+  end
+
   def setup_group_members(group)
     members = {
       owner: create(:user),