REST API: add field merge_user to MR response

This change aims to make REST response more similar to GraphQL. Additionally, we deprecated merged_by field. Changelog: added

REST API: add field merge_user to MR response
This change aims to make REST response more similar to GraphQL. Additionally, we deprecated merged_by field. Changelog: added
5246e6c9 · Piotr Stankowski · 8fa4d3c1 · 5246e6c9 · 5246e6c9 · 5246e6c9
Commit 5246e6c9 authored Jan 06, 2022 by Piotr Stankowski
5 changed files
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -9,6 +9,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 > - `reference` was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20354) in GitLab 12.10 in favour of `references`.
 > - `reviewer_username` and `reviewer_id` were [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49341) in GitLab 13.8.
 > - `draft` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63473) as a replacement for `work_in_progress` in GitLab 14.0.
+> - `merge_user` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) as an eventual replacement for `merged_by` in GitLab 14.7.

 Every API call to merge requests must be authenticated.

@@ -42,6 +43,11 @@ This approach is generally slower and more resource-intensive, but isn't subject
 placed on database-backed diffs. [Limits inherent to Gitaly](../development/diffs.md#diff-limits)
 still apply.

+- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) in GitLab 14.7,
+field `merge_user` can be either user who merged this merge request, 
+user who set it to merge when pipeline succeeds or `null`.
+Field `merged_by` (user who merged this merge request or `null`) has been deprecated.
+
 ## List merge requests

 Get all merge requests the authenticated user has access to. By
@@ -111,7 +117,15 @@ Parameters:
    "title": "test1",
    "description": "fixed login page css paddings",
    "state": "merged",
-    "merged_by": {
+    "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+      "id": 87854,
+      "name": "Douwe Maan",
+      "username": "DouweM",
+      "state": "active",
+      "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+      "web_url": "https://gitlab.com/DouweM"
+    },
+    "merge_user": {
      "id": 87854,
      "name": "Douwe Maan",
      "username": "DouweM",
@@ -295,7 +309,15 @@ Parameters:
    "title": "test1",
    "description": "fixed login page css paddings",
    "state": "merged",
-    "merged_by": {
+    "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+      "id": 87854,
+      "name": "Douwe Maan",
+      "username": "DouweM",
+      "state": "active",
+      "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+      "web_url": "https://gitlab.com/DouweM"
+    },
+    "merge_user": {
      "id": 87854,
      "name": "Douwe Maan",
      "username": "DouweM",
@@ -481,7 +503,15 @@ Parameters:
    "title": "test1",
    "description": "fixed login page css paddings",
    "state": "merged",
-    "merged_by": {
+    "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+      "id": 87854,
+      "name": "Douwe Maan",
+      "username": "DouweM",
+      "state": "active",
+      "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+      "web_url": "https://gitlab.com/DouweM"
+    },
+    "merge_user": {
      "id": 87854,
      "name": "Douwe Maan",
      "username": "DouweM",
@@ -717,7 +747,15 @@ Parameters:
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -1159,7 +1197,15 @@ POST /projects/:id/merge_requests
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -1330,7 +1376,15 @@ Must include at least one non-required attribute from above.
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -1516,7 +1570,15 @@ Parameters:
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -1705,7 +1767,15 @@ Parameters:
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -2006,7 +2076,15 @@ Example response:
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",
@@ -2166,7 +2244,15 @@ Example response:
  "squash": false,
  "subscribed": false,
  "changes_count": "1",
-  "merged_by": {
+  "merged_by": { // Deprecated and will be removed in API v5, use `merge_user` instead
+    "id": 87854,
+    "name": "Douwe Maan",
+    "username": "DouweM",
+    "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/-/system/user/avatar/87854/avatar.png",
+    "web_url": "https://gitlab.com/DouweM"
+  },
+  "merge_user": {
    "id": 87854,
    "name": "Douwe Maan",
    "username": "DouweM",

--- a/lib/api/entities/merge_request_basic.rb
+++ b/lib/api/entities/merge_request_basic.rb
@@ -3,9 +3,13 @@
 module API
  module Entities
    class MergeRequestBasic < IssuableEntity
+      # Deprecated in favour of merge_user
      expose :merged_by, using: Entities::UserBasic do |merge_request, _options|
        merge_request.metrics&.merged_by
      end
+      expose :merge_user, using: Entities::UserBasic do |merge_request|
+        merge_request.metrics&.merged_by || merge_request.merge_user
+      end
      expose :merged_at do |merge_request, _options|
        merge_request.metrics&.merged_at
      end

--- a/spec/fixtures/api/schemas/public_api/v4/merge_request.json
+++ b/spec/fixtures/api/schemas/public_api/v4/merge_request.json
@@ -20,6 +20,18 @@
        },
        "additionalProperties": false
      },
+      "merge_user": {
+        "type": ["object", "null"],
+        "properties": {
+          "name": { "type": "string" },
+          "username": { "type": "string" },
+          "id": { "type": "integer" },
+          "state": { "type": "string" },
+          "avatar_url": { "type": "uri" },
+          "web_url": { "type": "uri" }
+        },
+        "additionalProperties": false
+      },
      "merged_at": { "type": ["string", "null"] },
      "closed_by": {
        "type": ["object", "null"],

--- a/spec/lib/api/entities/merge_request_basic_spec.rb
+++ b/spec/lib/api/entities/merge_request_basic_spec.rb
@@ -21,7 +21,8 @@ RSpec.describe ::API::Entities::MergeRequestBasic do
  it 'includes basic fields' do
    is_expected.to include(
      draft: merge_request.draft?,
-      work_in_progress: merge_request.draft?
+      work_in_progress: merge_request.draft?,
+      merge_user: nil
    )
  end


--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1269,6 +1269,7 @@ RSpec.describe API::MergeRequests do
        get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)

        expect(json_response).to include('merged_by',
+          'merge_user',
          'merged_at',
          'closed_by',
          'closed_at',
@@ -1279,9 +1280,10 @@ RSpec.describe API::MergeRequests do
      end

      it 'returns correct values' do
-        get api("/projects/#{project.id}/merge_requests/#{merge_request.reload.iid}", user)
+        get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)

        expect(json_response['merged_by']['id']).to eq(merge_request.metrics.merged_by_id)
+        expect(json_response['merge_user']['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)
@@ -1292,6 +1294,32 @@ RSpec.describe API::MergeRequests do
      end
    end

+    context 'merge_user' do
+      context 'when MR is set to MWPS' do
+        let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project, target_project: project) }
+
+        it 'returns user who set MWPS' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['merge_user']['id']).to eq(user.id)
+        end
+
+        context 'when MR is already merged' do
+          before do
+            merge_request.metrics.update!(merged_by: user2)
+          end
+
+          it 'returns user who actually merged' do
+            get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['merge_user']['id']).to eq(user2.id)
+          end
+        end
+      end
+    end
+
    context 'head_pipeline' do
      let(:project) { create(:project, :repository) }
      let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', title: "Test") }