Commit 2b06e46f authored by Sincheol (David) Kim's avatar Sincheol (David) Kim

Merge branch '354116-handle-merge_request_diff_head_sha-in-the-createnotes-api' into 'master'

Resolve "Handle merge_request_diff_head_sha in the CreateNotes api"

See merge request gitlab-org/gitlab!82065
parents 9c2682cf 876ed6d8
...@@ -399,10 +399,11 @@ Parameters: ...@@ -399,10 +399,11 @@ Parameters:
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
|---------------------|----------------|----------|------------------------------------------------------------------------------------------------------------------------------| |---------------------|----------------|----------|------------------------------------------------------------------------------------------------------------------------------|
| `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) | | `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) |
| `merge_request_iid` | integer | yes | The IID of a project merge request | | `merge_request_iid` | integer | yes | The IID of a project merge request |
| `body` | string | yes | The content of a note. Limited to 1,000,000 characters. | | `body` | string | yes | The content of a note. Limited to 1,000,000 characters. |
| `created_at` | string | no | Date time string, ISO 8601 formatted. Example: `2016-03-11T03:45:40Z` (requires administrator or project/group owner rights) | | `created_at` | string | no | Date time string, ISO 8601 formatted. Example: `2016-03-11T03:45:40Z` (requires administrator or project/group owner rights) |
| `merge_request_diff_sha`| string | no | The SHA of the head commit which is used to ensure that the merge request hasn't been updated since the API request was sent. This is required for the /merge quick action |
### Modify existing merge request note ### Modify existing merge request note
......
...@@ -75,6 +75,7 @@ module API ...@@ -75,6 +75,7 @@ module API
requires :body, type: String, desc: 'The content of a note' requires :body, type: String, desc: 'The content of a note'
optional :confidential, type: Boolean, desc: 'Confidentiality note flag, default is false' optional :confidential, type: Boolean, desc: 'Confidentiality note flag, default is false'
optional :created_at, type: String, desc: 'The creation date of the note' optional :created_at, type: String, desc: 'The creation date of the note'
optional :merge_request_diff_head_sha, type: String, desc: 'The SHA of the head commit'
end end
post ":id/#{noteables_str}/:noteable_id/notes", feature_category: feature_category do post ":id/#{noteables_str}/:noteable_id/notes", feature_category: feature_category do
allowlist = allowlist =
...@@ -87,7 +88,8 @@ module API ...@@ -87,7 +88,8 @@ module API
noteable_type: noteables_str.classify, noteable_type: noteables_str.classify,
noteable_id: noteable.id, noteable_id: noteable.id,
confidential: params[:confidential], confidential: params[:confidential],
created_at: params[:created_at] created_at: params[:created_at],
merge_request_diff_head_sha: params[:merge_request_diff_head_sha]
} }
note = create_note(noteable, opts) note = create_note(noteable, opts)
......
...@@ -228,44 +228,83 @@ RSpec.describe API::Notes do ...@@ -228,44 +228,83 @@ RSpec.describe API::Notes do
end end
let(:request_body) { 'Hi!' } let(:request_body) { 'Hi!' }
let(:params) { { body: request_body } }
let(:request_path) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes" } let(:request_path) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes" }
subject { post api(request_path, user), params: { body: request_body } } subject { post api(request_path, user), params: params }
context 'a command only note' do context 'a command only note' do
let(:request_body) { "/spend 1h" } context '/spend' do
let(:request_body) { "/spend 1h" }
before do before do
project.add_developer(user) project.add_developer(user)
end end
it 'returns 202 Accepted status' do it 'returns 202 Accepted status' do
subject subject
expect(response).to have_gitlab_http_status(:accepted) expect(response).to have_gitlab_http_status(:accepted)
end end
it 'does not actually create a new note' do it 'does not actually create a new note' do
expect { subject }.not_to change { Note.where(system: false).count } expect { subject }.not_to change { Note.where(system: false).count }
end end
it 'does however create a system note about the change', :sidekiq_inline do it 'does however create a system note about the change', :sidekiq_inline do
expect { subject }.to change { Note.system.count }.by(1) expect { subject }.to change { Note.system.count }.by(1)
end end
it 'applies the commands' do
expect { subject }.to change { merge_request.reset.total_time_spent }
end
it 'reports the changes' do
subject
it 'applies the commands' do expect(json_response).to include(
expect { subject }.to change { merge_request.reset.total_time_spent } 'commands_changes' => include(
'spend_time' => include('duration' => 3600)
),
'summary' => include('Added 1h spent time.')
)
end
end end
it 'reports the changes' do context '/merge' do
subject let(:request_body) { "/merge" }
let(:project) { create(:project, :public, :repository) }
let(:merge_request) { create(:merge_request_with_multiple_diffs, source_project: project, target_project: project, author: user) }
let(:params) { { body: request_body, merge_request_diff_head_sha: merge_request.diff_head_sha } }
before do
project.add_developer(user)
end
it 'returns 202 Accepted status' do
subject
expect(response).to have_gitlab_http_status(:accepted)
end
it 'does not actually create a new note' do
expect { subject }.not_to change { Note.where(system: false).count }
end
it 'applies the commands' do
expect { subject }.to change { merge_request.reload.merge_jid.present? }.from(false).to(true)
end
expect(json_response).to include( it 'reports the changes' do
'commands_changes' => include( subject
'spend_time' => include('duration' => 3600)
), expect(json_response).to include(
'summary' => include('Added 1h spent time.') 'commands_changes' => include(
) 'merge' => merge_request.diff_head_sha
),
'summary' => ['Merged this merge request.']
)
end
end end
end end
......
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