Merge branch 'show-all-pipelines-from-all-diffs' into 'master'

Show all pipelines from all merge_request_diffs

This way we could also show pipelines from commits which
were discarded due to a force push.

Closes #21889

See merge request !6414

4 files changed, 79 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 34edeafd4d9..d80f7fd002e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -45,6 +45,7 @@ v 8.12.0 (unreleased)
   - Added horizontal padding on build page sidebar on code coverage block. !6196 (Vitaly Baev)
   - Change merge_error column from string to text type
   - Reduce contributions calendar data payload (ClemMakesApps)
+  - Show all pipelines for merge requests even from discarded commits !6414
   - Replace contributions calendar timezone payload with dates (ClemMakesApps)
   - Add `web_url` field to issue, merge request, and snippet API objects (Ben Boeckel)
   - Enable pipeline events by default !6278
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 4c51c979599..2dcf7f89bfc 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -764,10 +764,23 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def all_pipelines
-    @all_pipelines ||=
-      if diff_head_sha && source_project
-        source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch)
-      end
+    return unless source_project
+
+    @all_pipelines ||= begin
+      sha = if persisted?
+              all_commits_sha
+            else
+              diff_head_sha
+            end
+
+      source_project.pipelines.order(id: :desc).
+        where(sha: sha, ref: source_branch)
+    end
+  end
+
+  # Note that this could also return SHA from now dangling commits
+  def all_commits_sha
+    merge_request_diffs.flat_map(&:commits_sha).uniq
   end
 
   def merge_commit
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 18c583add88..7362886e9f5 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -117,6 +117,14 @@ class MergeRequestDiff < ActiveRecord::Base
     project.commit(head_commit_sha)
   end
 
+  def commits_sha
+    if @commits
+      commits.map(&:sha)
+    else
+      st_commits.map { |commit| commit[:id] }
+    end
+  end
+
   def diff_refs
     return unless start_commit_sha || base_commit_sha
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 7cb97113c04..433aba7747b 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -495,15 +495,62 @@ describe MergeRequest, models: true do
   end
 
   describe '#all_pipelines' do
-    let!(:pipelines) do
-      subject.merge_request_diff.commits.map do |commit|
-        create(:ci_empty_pipeline, project: subject.source_project, sha: commit.id, ref: subject.source_branch)
+    shared_examples 'returning pipelines with proper ordering' do
+      let!(:all_pipelines) do
+        subject.all_commits_sha.map do |sha|
+          create(:ci_empty_pipeline,
+                 project: subject.source_project,
+                 sha: sha,
+                 ref: subject.source_branch)
+        end
+      end
+
+      it 'returns all pipelines' do
+        expect(subject.all_pipelines).not_to be_empty
+        expect(subject.all_pipelines).to eq(all_pipelines.reverse)
+      end
+    end
+
+    context 'with single merge_request_diffs' do
+      it_behaves_like 'returning pipelines with proper ordering'
+    end
+
+    context 'with multiple irrelevant merge_request_diffs' do
+      before do
+        subject.update(target_branch: 'markdown')
+      end
+
+      it_behaves_like 'returning pipelines with proper ordering'
+    end
+
+    context 'with unsaved merge request' do
+      subject { build(:merge_request) }
+
+      let!(:pipeline) do
+        create(:ci_empty_pipeline,
+               project: subject.project,
+               sha: subject.diff_head_sha,
+               ref: subject.source_branch)
       end
+
+      it 'returns pipelines from diff_head_sha' do
+        expect(subject.all_pipelines).to contain_exactly(pipeline)
+      end
+    end
+  end
+
+  describe '#all_commits_sha' do
+    let(:all_commits_sha) do
+      subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq
+    end
+
+    before do
+      subject.update(target_branch: 'markdown')
     end
 
-    it 'returns a pipelines from source projects with proper ordering' do
-      expect(subject.all_pipelines).not_to be_empty
-      expect(subject.all_pipelines).to eq(pipelines.reverse)
+    it 'returns all SHA from all merge_request_diffs' do
+      expect(subject.merge_request_diffs.size).to eq(2)
+      expect(subject.all_commits_sha).to eq(all_commits_sha)
     end
   end