Show merge request on pipeline email

Description

Everytime I received failed pipelines, I want to open the merge request first before heading to the failed specs. So far, to do that there is no option. I have to open the pipeline page, and find a way from there up to the merge request page--which is not straightforward.

Proposal

Add the merge request ID in email send after pipeline build failed/succeeed.

Links / references

@adamco Thanks for the proposal! I can see how that addition would be useful

@dimitrieh What do you think about this addition to the pipeline emails?

added ~19173 emails feature proposal ~13921 pipeline labels

I thought we're already doing that?

Note that it would only show the merge request if the pipeline's commit is the head commit of the merge request. So "outdated" pipelines won't show the merge request link. This is done due to performance consideration.

We could see if this would perform well enough: (so that it would show the merge request link if it has even been linked to a merge request)

diff --git a/app/mailers/emails/pipelines.rb b/app/mailers/emails/pipelines.rb
index 9460a6cd2b..3d2af1c7cc 100644
--- a/app/mailers/emails/pipelines.rb
+++ b/app/mailers/emails/pipelines.rb
@@ -13,7 +13,7 @@ module Emails
     def pipeline_mail(pipeline, recipients, status)
       @project = pipeline.project
       @pipeline = pipeline
-      @merge_request = pipeline.merge_requests.first
+      @merge_request = pipeline.first_merge_request
       add_headers
 
       # We use bcc here because we don't want to generate this emails for a
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index fab8497ec7..8d1ef6267e 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -367,6 +367,15 @@ module Ci
         .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id }
     end
 
+    def first_merge_request
+      @first_merge_request ||= project.merge_requests
+        .where(source_branch: ref)
+        .order(id: :desc)
+        .to_a.find do |merge_request|
+          merge_request.all_pipelines.where(id: id).exists?
+        end
+    end
+
     def detailed_status(current_user)
       Gitlab::Ci::Status::Pipeline::Factory
         .new(self, current_user)

Could we detect the merge request ID through the build?

If yes as is possible if I see through the UI, we can easily fetch one of those build, and find out the merge request ID somewhere from there. So probably it doesn't need to send a lot of queries.

@adamco Due to the schema we have right now, it could potentially need a lot of queries to get the data. It's more expensive to get merge requests from a particular pipeline than get pipelines from a particular merge request.

However I could see that if you never create two merge requests from the same branch ever, it might not be too bad, or I should say it's as bad as showing all pipelines from the merge request.

@godfat thanks for your explanation, but really I still want to clarify here haha.

Is that still slow if we do: get the ci-pipeline in question -> get one of the ci-build -> get merge request -> get reference ?

Thank you

@adamco No problems :)

You could see my patch above: https://gitlab.com/gitlab-org/gitlab-ce/issues/27687#note_22843932

It's getting the pipeline, getting all the merge requests from the same branch with the pipeline, then finding all the pipelines from all the merge requests, then finding which merge request has the same pipeline.

What you're describing won't work, because:

0. There's no need to get any ci-build. There's no more extra information out there.
1. There's no association from ci-pipeline/ci-build to merge requests. We'll need to make a few queries to get the merge requests, and then we'll need to find the pipelines from the merge requests, and then compare which merge request has the pipeline we want.

This is done due to performance consideration. by @godfat

@ayufan would we solve this if we were to solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23902 ?

@dimitrieh Yes, I think so.

@dimitrieh Wait, I just took a closer look. I think that's more than we need for this. I think what we need here is a direct reference from pipeline to merge request or vice versa. That issue is more about running pipelines for forks, I think.

thanks for the kind explanation :)

feel free to close it :)

That being said, I think #23708 (closed) would be enough for this one. But it's closed because #23902 (moved) is also covering that.

@adamco Let's keep this open because I think it would be useful :) Also maybe the performance won't be that bad after all. I don't know. Personally I would prefer to give it a try. (I was implementing it that way originally, but simplifying it in the end like now)

could I do it, and when the major improvement/link between MR<->Pipeline is made, we could upgrade?

because I'd really love to have it, hahaha, I.

but yeah, it's up to the Gitlab team.

@adamco i agree with you that it would be nice to have that as well

I have been trying to get these references from MR to pipeline throughout the application for some time and it keeps being on my mind but is being halted by https://gitlab.com/gitlab-org/gitlab-ce/issues/23902. So until we have that, we can get this improved in more than just this place

mentioned in issue #27179 (closed)

mentioned in issue #31040 (closed)

Closing this issue in favour of a newer issue that has been scheduled:

• https://gitlab.com/gitlab-org/gitlab-ce/issues/28982

closed