Merge branch '3845-committers-cannot-approve-mr-ce' into 'master'

Add committers and authors methods on MergeRequest

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

4 files changed, 54 insertions, 3 deletions

diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb
index ac93393ac3a..f031a74c5bd 100644
--- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb
@@ -71,7 +71,7 @@ describe Projects::MergeRequests::CreationsController do
         expect(response).to be_success
 
         total = assigns(:total_commit_count)
-        expect(assigns(:commits)).to be_an Array
+        expect(assigns(:commits)).to be_an CommitCollection
         expect(total).to be > 0
         expect(assigns(:hidden_commit_count)).to eq(0)
         expect(response).to have_gitlab_http_status(200)
diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb
index 066fe7d154e..005005b236b 100644
--- a/spec/models/commit_collection_spec.rb
+++ b/spec/models/commit_collection_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe CommitCollection do
   let(:project) { create(:project, :repository) }
-  let(:commit) { project.commit }
+  let(:commit) { project.commit("c1c67abbaf91f624347bb3ae96eabe3a1b742478") }
 
   describe '#each' do
     it 'yields every commit' do
@@ -12,6 +12,29 @@ describe CommitCollection do
     end
   end
 
+  describe '.committers' do
+    it 'returns a relation of users when users are found' do
+      user = create(:user, email: commit.committer_email.upcase)
+      collection = described_class.new(project, [commit])
+
+      expect(collection.committers).to contain_exactly(user)
+    end
+
+    it 'returns empty array when committers cannot be found' do
+      collection = described_class.new(project, [commit])
+
+      expect(collection.committers).to be_empty
+    end
+
+    it 'excludes authors of merge commits' do
+      commit = project.commit("60ecb67744cb56576c30214ff52294f8ce2def98")
+      create(:user, email: commit.committer_email.upcase)
+      collection = described_class.new(project, [commit])
+
+      expect(collection.committers).to be_empty
+    end
+  end
+
   describe '#with_pipeline_status' do
     it 'sets the pipeline status for every commit so no additional queries are necessary' do
       create(
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 96d49e86dab..e18b29df321 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -991,6 +991,34 @@ describe MergeRequest do
     end
   end
 
+  describe '#committers' do
+    it 'returns all the committers of every commit in the merge request' do
+      users = subject.commits.map(&:committer_email).uniq.map do |email|
+        create(:user, email: email)
+      end
+
+      expect(subject.committers).to match_array(users)
+    end
+
+    it 'returns an empty array if no committer is associated with a user' do
+      expect(subject.committers).to be_empty
+    end
+  end
+
+  describe '#authors' do
+    it 'returns a list with all the committers in the merge request and author' do
+      users = subject.commits.map(&:committer_email).uniq.map do |email|
+        create(:user, email: email)
+      end
+
+      expect(subject.authors).to match_array([subject.author, *users])
+    end
+
+    it 'returns only the author if no committer is associated with a user' do
+      expect(subject.authors).to contain_exactly(subject.author)
+    end
+  end
+
   describe '#hook_attrs' do
     it 'delegates to Gitlab::HookData::MergeRequestBuilder#build' do
       builder = double
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 1d9c75dedce..1169ed5f9f2 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -509,7 +509,7 @@ describe MergeRequests::RefreshService do
           committed_date: Time.now
         )
 
-        allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit])
+        allow_any_instance_of(MergeRequest).to receive(:commits).and_return(CommitCollection.new(@project, [commit], 'feature'))
       end
 
       context 'when the merge request is sourced from the same project' do