Commit b7f2365f authored by Johannes Altmanninger's avatar Johannes Altmanninger

Merge Request Discussions API: accept commit_id to create discussions on commit

When creating a new thread on a merge request by using the [MR Discussions
API](https://docs.gitlab.com/ee/api/discussions.html#create-new-merge-request-thread),
the thread is always on the merge request's full diff.

This patch allows passing the "commit_id" parameter to associate the thread
with the desired commit.  Additionally, the "commit_id" field of a note
is exposed if the note is a diff note on a merge request.

Use case: for merge requests with many commits, I like to add my threads
on individual commits, so reviewers can immediately find the commit that
introduced the discussed lines.  This can be done in the web interface by
commenting on the diff of a commit within a merge request.  This commit
allows to do the same thing using the API.
parent 58bc9bea
---
title: Allow passing `commit_id` when creating MR discussions via the API and expose `commit_id` for MR diff notes
merge_request: 47130
author: Johannes Altmanninger @krobelus
type: added
...@@ -773,6 +773,7 @@ Diff comments also contain position: ...@@ -773,6 +773,7 @@ Diff comments also contain position:
"noteable_id": 3, "noteable_id": 3,
"noteable_type": "Merge request", "noteable_type": "Merge request",
"noteable_iid": null, "noteable_iid": null,
"commit_id": "4803c71e6b1833ca72b8b26ef2ecd5adc8a38031",
"position": { "position": {
"base_sha": "b5d6e7b1613fca24d250fa8e5bc7bcc3dd6002ef", "base_sha": "b5d6e7b1613fca24d250fa8e5bc7bcc3dd6002ef",
"start_sha": "7c9c2ead8a320fb7ba0b4e234bd9529a2614e306", "start_sha": "7c9c2ead8a320fb7ba0b4e234bd9529a2614e306",
...@@ -828,6 +829,8 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/a ...@@ -828,6 +829,8 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/a
### Create new merge request thread ### Create new merge request thread
> The `commit id` entry was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47130) in GitLab 13.7.
Creates a new thread to a single project merge request. This is similar to creating Creates a new thread to a single project merge request. This is similar to creating
a note but other comments (replies) can be added to it later. a note but other comments (replies) can be added to it later.
...@@ -842,6 +845,7 @@ Parameters: ...@@ -842,6 +845,7 @@ Parameters:
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
| `merge_request_iid` | integer | yes | The IID of a merge request | | `merge_request_iid` | integer | yes | The IID of a merge request |
| `body` | string | yes | The content of the thread | | `body` | string | yes | The content of the thread |
| `commit_id` | string | no | SHA referencing commit to start this thread on |
| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) | | `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
| `position` | hash | no | Position when creating a diff note | | `position` | hash | no | Position when creating a diff note |
| `position[base_sha]` | string | yes | Base commit SHA in the source branch | | `position[base_sha]` | string | yes | Base commit SHA in the source branch |
......
...@@ -104,6 +104,7 @@ module API ...@@ -104,6 +104,7 @@ module API
position: params[:position], position: params[:position],
id_key => noteable.id id_key => noteable.id
} }
opts[:commit_id] = params[:commit_id] if noteable.is_a?(MergeRequest) && type == 'DiffNote'
note = create_note(noteable, opts) note = create_note(noteable, opts)
......
...@@ -14,6 +14,7 @@ module API ...@@ -14,6 +14,7 @@ module API
expose :created_at, :updated_at expose :created_at, :updated_at
expose :system?, as: :system expose :system?, as: :system
expose :noteable_id, :noteable_type expose :noteable_id, :noteable_type
expose :commit_id, if: ->(note, options) { note.noteable_type == "MergeRequest" && note.is_a?(DiffNote) }
expose :position, if: ->(note, options) { note.is_a?(DiffNote) } do |note| expose :position, if: ->(note, options) { note.is_a?(DiffNote) } do |note|
note.position.to_h note.position.to_h
......
...@@ -61,6 +61,37 @@ RSpec.describe API::Discussions do ...@@ -61,6 +61,37 @@ RSpec.describe API::Discussions do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
end end
context "when a commit parameter is given" do
it "creates the discussion on that commit within the merge request" do
# SHAs of "feature" and its parent in spec/support/gitlab-git-test.git
mr_commit = '0b4bc9a49b562e85de7cc9e834518ea6828729b9'
parent_commit = 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f'
file = "files/ruby/feature.rb"
line_range = {
"start_line_code" => Gitlab::Git.diff_line_code(file, 1, 1),
"end_line_code" => Gitlab::Git.diff_line_code(file, 1, 1),
"start_line_type" => "text",
"end_line_type" => "text"
}
position = build(
:text_diff_position,
:added,
file: file,
new_line: 1,
line_range: line_range,
base_sha: parent_commit,
head_sha: mr_commit,
start_sha: parent_commit
)
post api("/projects/#{project.id}/merge_requests/#{noteable['iid']}/discussions", user),
params: { body: 'MR discussion on commit', position: position.to_h, commit_id: mr_commit }
expect(response).to have_gitlab_http_status(:created)
expect(json_response['notes'].first['commit_id']).to eq(mr_commit)
end
end
end end
context 'when noteable is a Commit' do context 'when noteable is a Commit' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment