From bb89412ffb22de2b188580e9ac6f4bd6e2f4df84 Mon Sep 17 00:00:00 2001
From: haseeb <haseebeqx@gmail.com>
Date: Fri, 2 Mar 2018 16:48:55 +0000
Subject: [PATCH] expose metrics in merge request api

---
 .../41905_merge_request_and_issue_metrics.yml |  5 +++
 doc/api/merge_requests.md                     | 44 ++++++++++++++-----
 lib/api/entities.rb                           | 44 +++++++++++++++++--
 lib/api/merge_requests.rb                     |  2 +-
 spec/requests/api/merge_requests_spec.rb      | 40 +++++++++++++++++
 5 files changed, 118 insertions(+), 17 deletions(-)
 create mode 100644 changelogs/unreleased/41905_merge_request_and_issue_metrics.yml

diff --git a/changelogs/unreleased/41905_merge_request_and_issue_metrics.yml b/changelogs/unreleased/41905_merge_request_and_issue_metrics.yml
new file mode 100644
index 0000000000..c9e23360e3
--- /dev/null
+++ b/changelogs/unreleased/41905_merge_request_and_issue_metrics.yml
@@ -0,0 +1,5 @@
+---
+title: expose more metrics in merge requests api
+merge_request: 16589
+author: haseebeqx
+type: added
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 6ce021cb4b..cb9b061876 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -261,20 +261,20 @@ Parameters:
   "upvotes": 0,
   "downvotes": 0,
   "author": {
-    "id": 1,
-    "username": "admin",
-    "email": "admin@example.com",
-    "name": "Administrator",
-    "state": "active",
-    "created_at": "2012-04-29T08:46:00Z"
+    "state" : "active",
+    "web_url" : "https://gitlab.example.com/root",
+    "avatar_url" : null,
+    "username" : "root",
+    "id" : 1,
+    "name" : "Administrator"
   },
   "assignee": {
-    "id": 1,
-    "username": "admin",
-    "email": "admin@example.com",
-    "name": "Administrator",
-    "state": "active",
-    "created_at": "2012-04-29T08:46:00Z"
+    "state" : "active",
+    "web_url" : "https://gitlab.example.com/root",
+    "avatar_url" : null,
+    "username" : "root",
+    "id" : 1,
+    "name" : "Administrator"
   },
   "source_project_id": 2,
   "target_project_id": 3,
@@ -308,6 +308,26 @@ Parameters:
     "total_time_spent": 0,
     "human_time_estimate": null,
     "human_total_time_spent": null
+  },
+  "closed_at": "2018-01-19T14:36:11.086Z",
+  "latest_build_started_at": null,
+  "latest_build_finished_at": null,
+  "first_deployed_to_production_at": null,
+  "pipeline": {
+    "id": 8,
+    "ref": "master",
+    "sha": "2dc6aa325a317eda67812f05600bdf0fcdc70ab0",
+    "status": "created"
+  },
+  "merged_by": null,
+  "merged_at": null,
+  "closed_by": {
+    "state" : "active",
+    "web_url" : "https://gitlab.example.com/root",
+    "avatar_url" : null,
+    "username" : "root",
+    "id" : 1,
+    "name" : "Administrator"
   }
 }
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index c88fcf9472..0c8ec7dd5f 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -481,6 +481,10 @@ module API
       expose :id
     end
 
+    class PipelineBasic < Grape::Entity
+      expose :id, :sha, :ref, :status
+    end
+
     class MergeRequestSimple < ProjectEntity
       expose :title
       expose :web_url do |merge_request, options|
@@ -546,6 +550,42 @@ module API
       expose :changes_count do |merge_request, _options|
         merge_request.merge_request_diff.real_size
       end
+
+      expose :merged_by, using: Entities::UserBasic do |merge_request, _options|
+        merge_request.metrics&.merged_by
+      end
+
+      expose :merged_at do |merge_request, _options|
+        merge_request.metrics&.merged_at
+      end
+
+      expose :closed_by, using: Entities::UserBasic do |merge_request, _options|
+        merge_request.metrics&.latest_closed_by
+      end
+
+      expose :closed_at do |merge_request, _options|
+        merge_request.metrics&.latest_closed_at
+      end
+
+      expose :latest_build_started_at, if: -> (_, options) { build_available?(options) } do |merge_request, _options|
+        merge_request.metrics&.latest_build_started_at
+      end
+
+      expose :latest_build_finished_at, if: -> (_, options) { build_available?(options) } do |merge_request, _options|
+        merge_request.metrics&.latest_build_finished_at
+      end
+
+      expose :first_deployed_to_production_at, if: -> (_, options) { build_available?(options) } do |merge_request, _options|
+        merge_request.metrics&.first_deployed_to_production_at
+      end
+
+      expose :pipeline, using: Entities::PipelineBasic, if: -> (_, options) { build_available?(options) } do |merge_request, _options|
+        merge_request.metrics&.pipeline
+      end
+
+      def build_available?(options)
+        options[:project]&.feature_available?(:builds, options[:current_user])
+      end
     end
 
     class MergeRequestChanges < MergeRequest
@@ -909,10 +949,6 @@ module API
       expose :filename, :size
     end
 
-    class PipelineBasic < Grape::Entity
-      expose :id, :sha, :ref, :status
-    end
-
     class JobBasic < Grape::Entity
       expose :id, :status, :stage, :name, :ref, :tag, :coverage
       expose :created_at, :started_at, :finished_at
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 4ffd4895c7..16d0f005f2 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -222,7 +222,7 @@ module API
       get ':id/merge_requests/:merge_request_iid/changes' do
         merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
-        present merge_request, with: Entities::MergeRequestChanges, current_user: current_user
+        present merge_request, with: Entities::MergeRequestChanges, current_user: current_user, project: user_project
       end
 
       desc 'Get the merge request pipelines' do
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 658cedd6b5..e8eb01f6c3 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -9,6 +9,7 @@ describe API::MergeRequests do
   let(:non_member)  { create(:user) }
   let!(:project)    { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
   let(:milestone)   { create(:milestone, title: '1.0.0', project: project) }
+  let(:pipeline)    { create(:ci_empty_pipeline) }
   let(:milestone1)   { create(:milestone, title: '0.9', project: project) }
   let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
   let!(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
@@ -500,6 +501,45 @@ describe API::MergeRequests do
       expect(json_response['changes_count']).to eq(merge_request.merge_request_diff.real_size)
     end
 
+    context 'merge_request_metrics' do
+      before do
+        merge_request.metrics.update!(merged_by: user,
+                                      latest_closed_by: user,
+                                      latest_closed_at: 1.hour.ago,
+                                      merged_at: 2.hours.ago,
+                                      pipeline: pipeline,
+                                      latest_build_started_at: 3.hours.ago,
+                                      latest_build_finished_at: 1.hour.ago,
+                                      first_deployed_to_production_at: 3.minutes.ago)
+      end
+
+      it 'has fields from merge request metrics' do
+        get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+        expect(json_response).to include('merged_by',
+          'merged_at',
+          'closed_by',
+          'closed_at',
+          'latest_build_started_at',
+          'latest_build_finished_at',
+          'first_deployed_to_production_at',
+          'pipeline')
+      end
+
+      it 'returns correct values' do
+        get api("/projects/#{project.id}/merge_requests/#{merge_request.reload.iid}", user)
+
+        expect(json_response['merged_by']['id']).to eq(merge_request.metrics.merged_by_id)
+        expect(Time.parse json_response['merged_at']).to be_like_time(merge_request.metrics.merged_at)
+        expect(json_response['closed_by']['id']).to eq(merge_request.metrics.latest_closed_by_id)
+        expect(Time.parse json_response['closed_at']).to be_like_time(merge_request.metrics.latest_closed_at)
+        expect(json_response['pipeline']['id']).to eq(merge_request.metrics.pipeline_id)
+        expect(Time.parse json_response['latest_build_started_at']).to be_like_time(merge_request.metrics.latest_build_started_at)
+        expect(Time.parse json_response['latest_build_finished_at']).to be_like_time(merge_request.metrics.latest_build_finished_at)
+        expect(Time.parse json_response['first_deployed_to_production_at']).to be_like_time(merge_request.metrics.first_deployed_to_production_at)
+      end
+    end
+
     it "returns a 404 error if merge_request_iid not found" do
       get api("/projects/#{project.id}/merge_requests/999", user)
       expect(response).to have_gitlab_http_status(404)
-- 
2.30.9