From fa58068b2894b900d4b2519825411e0710557fc6 Mon Sep 17 00:00:00 2001
From: "Z.J. van de Weg" <zegerjan@gitlab.com>
Date: Wed, 5 Oct 2016 13:52:15 +0200
Subject: [PATCH] Refactor ci_status on MergeRequestController

---
 .../stylesheets/pages/merge_requests.scss     |  4 +-
 .../projects/merge_requests_controller.rb     | 43 ++++++++-----------
 app/models/environment.rb                     |  5 ++-
 app/models/merge_request.rb                   | 22 ++++------
 app/models/repository.rb                      |  4 +-
 5 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 96d5547154d..6a0fae8a3f9 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -122,8 +122,8 @@
   }
 
   .js-deployment-link {
-     display: inline-block;
-   }
+    display: inline-block;
+  }
 
   .mr-widget-body {
     h4 {
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 28225fbb762..00043c5a4c0 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -393,33 +393,28 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       end
     end
 
-    environments = @merge_request.environments
-    deployments = @merge_request.deployments
+    environments = @merge_request.environments.map do |environment|
+      next unless can?(current_user, :read_environment, environment)
 
-    if environments.present?
-      environments = environments.select { |e| can?(current_user, :read_environment, e) }.map do |environment|
-        project = environment.project
-        deployment = deployments.find { |d| d.environment == environment }
+      deployment = environment.first_deployment_for(@merge_request.diff_head_commit)
+      environment = {
+        name: environment.name,
+        id: environment.id,
+        url: namespace_project_environment_path(@project.namespace, @project, environment),
+        external_url: environment.external_url,
+        deployed_at: deployment ? deployment.created_at : nil
+      }
 
-        environment = {
-           name: environment.name,
-           id: environment.id,
-           url: namespace_project_environment_path(project.namespace, project, environment),
-           external_url: environment.external_url,
-           deployed_at: deployment ? deployment.created_at : nil
-         }
-
-        if environment[:external_url]
-          environment[:external_url_formatted] = environment[:external_url].gsub(/\A.*?:\/\//, '')
-        end
-
-        if environment[:deployed_at]
-          environment[:deployed_at_formatted] = environment[:deployed_at].to_time.in_time_zone.to_s(:medium)
-        end
-
-        environment
+      if environment[:external_url]
+        environment[:external_url_formatted] = environment[:external_url].gsub(/\A.*?:\/\//, '')
       end
-    end
+
+      if environment[:deployed_at]
+        environment[:deployed_at_formatted] = environment[:deployed_at].to_time.in_time_zone.to_s(:medium)
+      end
+
+      environment
+    end.compact
 
     response = {
       title: merge_request.title,
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 1c7d06906f3..c6cae81ce6a 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -48,12 +48,13 @@ class Environment < ActiveRecord::Base
     self.name == "production"
   end
 
-  def deployment_id_for(commit)
+  def first_deployment_for(commit)
     ref = project.repository.ref_name_for_sha(ref_path, commit.sha)
 
     return nil unless ref
 
-    ref.split('/').last.to_i
+    deployment_id = ref.split('/').last.to_i
+    deployments.find(deployment_id)
   end
 
   def ref_path
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 0e427378bac..5ccfe11a2a2 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -685,24 +685,18 @@ class MergeRequest < ActiveRecord::Base
     !pipeline || pipeline.success?
   end
 
-  def deployments
-    deployment_ids =
-      environments.map do |environment|
-        environment.deployment_id_for(diff_head_commit)
-      end.compact
-
-    Deployment.find(deployment_ids)
-  end
-
   def environments
     return [] unless diff_head_commit
 
-    environments = source_project.environments_for(
-      source_branch, diff_head_commit)
-    environments += target_project.environments_for(
-      target_branch, diff_head_commit, with_tags: true)
+    @environments ||=
+      begin
+        environments = source_project.environments_for(
+          source_branch, diff_head_commit)
+        environments += target_project.environments_for(
+          target_branch, diff_head_commit, with_tags: true)
 
-    environments.uniq
+        environments.uniq
+      end
   end
 
   def state_human_name
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 37833cf004f..72e473871fa 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -719,8 +719,8 @@ class Repository
     end
   end
 
-  def ref_name_for_sha(environment_ref_path, sha)
-    args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{environment_ref_path} --contains #{sha})
+  def ref_name_for_sha(ref_path, sha)
+    args = %W(#{Gitlab.config.git.bin_path} for-each-ref --count=1 #{ref_path} --contains #{sha})
 
     # Not found -> ["", 0]
     # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]