Commit 940ed58e authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'commit-comments' of https://gitlab.com/jeroenj/gitlab-ce into...

Merge branch 'commit-comments' of https://gitlab.com/jeroenj/gitlab-ce into jeroenj/gitlab-ce-commit-comments
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	CHANGELOG
parents c58edd7c 43075865
...@@ -10,6 +10,7 @@ v 7.5.0 ...@@ -10,6 +10,7 @@ v 7.5.0
- Tie up loose ends with annotated tags: API & UI (Sean Edge) - Tie up loose ends with annotated tags: API & UI (Sean Edge)
- Return valid json for deleting branch via API (sponsored by O'Reilly Media) - Return valid json for deleting branch via API (sponsored by O'Reilly Media)
- Expose username in project events API (sponsored by O'Reilly Media) - Expose username in project events API (sponsored by O'Reilly Media)
- Adds comments to commits in the API
v 7.4.3 v 7.4.3
- Fix raw snippets view - Fix raw snippets view
......
...@@ -317,7 +317,7 @@ class Note < ActiveRecord::Base ...@@ -317,7 +317,7 @@ class Note < ActiveRecord::Base
end end
def diff_file_index def diff_file_index
line_code.split('_')[0] line_code.split('_')[0] if line_code
end end
def diff_file_name def diff_file_name
...@@ -333,11 +333,11 @@ class Note < ActiveRecord::Base ...@@ -333,11 +333,11 @@ class Note < ActiveRecord::Base
end end
def diff_old_line def diff_old_line
line_code.split('_')[1].to_i line_code.split('_')[1].to_i if line_code
end end
def diff_new_line def diff_new_line
line_code.split('_')[2].to_i line_code.split('_')[2].to_i if line_code
end end
def generate_line_code(line) def generate_line_code(line)
...@@ -358,6 +358,20 @@ class Note < ActiveRecord::Base ...@@ -358,6 +358,20 @@ class Note < ActiveRecord::Base
@diff_line @diff_line
end end
def diff_line_type
return @diff_line_type if @diff_line_type
if diff
diff_lines.each do |line|
if generate_line_code(line) == self.line_code
@diff_line_type = line.type
end
end
end
@diff_line_type
end
def truncated_diff_lines def truncated_diff_lines
max_number_of_lines = 16 max_number_of_lines = 16
prev_match_line = nil prev_match_line = nil
......
...@@ -93,3 +93,66 @@ Parameters: ...@@ -93,3 +93,66 @@ Parameters:
} }
] ]
``` ```
## Get the comments of a commit
Get the comments of a commit in a project.
```
GET /projects/:id/repository/commits/:sha/comments
```
Parameters:
- `id` (required) - The ID of a project
- `sha` (required) - The name of a repository branch or tag or if not given the default branch
```json
[
{
"note": "this code is really nice",
"author": {
"id": 11,
"username": "admin",
"email": "admin@local.host",
"name": "Administrator",
"state": "active",
"created_at": "2014-03-06T08:17:35.000Z"
}
}
]
```
## Post comment to commit
Adds a comment to a commit. Optionally you can post comments on a specific line of a commit. Therefor both `path`, `line_new` and `line_old` are required.
```
POST /projects/:id/repository/commits/:sha/comments
```
Parameters:
- `id` (required) - The ID of a project
- `sha` (required) - The name of a repository branch or tag or if not given the default branch
- `note` (required) - Text of comment
- `path` (optional) - The file path
- `line` (optional) - The line number
- `line_type` (optional) - The line type (new or old)
```json
{
"author": {
"id": 1,
"username": "admin",
"email": "admin@local.host",
"name": "Administrator",
"blocked": false,
"created_at": "2012-04-29T08:46:00Z"
},
"note": "text1",
"path": "example.rb",
"line": 5,
"line_type": "new"
}
```
...@@ -50,6 +50,67 @@ module API ...@@ -50,6 +50,67 @@ module API
not_found! "Commit" unless commit not_found! "Commit" unless commit
commit.diffs commit.diffs
end end
# Get a commit's comments
#
# Parameters:
# id (required) - The ID of a project
# sha (required) - The commit hash
# Examples:
# GET /projects/:id/repository/commits/:sha/comments
get ':id/repository/commits/:sha/comments' do
sha = params[:sha]
commit = user_project.repository.commit(sha)
not_found! 'Commit' unless commit
notes = Note.where(commit_id: commit.id)
present paginate(notes), with: Entities::CommitNote
end
# Post comment to commit
#
# Parameters:
# id (required) - The ID of a project
# sha (required) - The commit hash
# note (required) - Text of comment
# path (optional) - The file path
# line (optional) - The line number
# line_type (optional) - The type of line (new or old)
# Examples:
# POST /projects/:id/repository/commits/:sha/comments
post ':id/repository/commits/:sha/comments' do
required_attributes! [:note]
sha = params[:sha]
commit = user_project.repository.commit(sha)
not_found! 'Commit' unless commit
opts = {
note: params[:note],
noteable_type: 'Commit',
commit_id: commit.id
}
if params[:path] && params[:line] && params[:line_type]
commit.diffs.each do |diff|
next unless diff.new_path == params[:path]
lines = Gitlab::Diff::Parser.new.parse(diff.diff.lines.to_a)
lines.each do |line|
next unless line.new_pos == params[:line].to_i && line.type == params[:line_type]
break opts[:line_code] = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
end
break if opts[:line_code]
end
end
note = ::Notes::CreateService.new(user_project, current_user, opts).execute
if note.save
present note, with: Entities::CommitNote
else
not_found!
end
end
end end
end end
end end
...@@ -179,6 +179,14 @@ module API ...@@ -179,6 +179,14 @@ module API
expose :author, using: Entities::UserBasic expose :author, using: Entities::UserBasic
end end
class CommitNote < Grape::Entity
expose :note
expose(:path) { |note| note.diff_file_name }
expose(:line) { |note| note.diff_new_line }
expose(:line_type) { |note| note.diff_line_type }
expose :author, using: Entities::UserBasic
end
class Event < Grape::Entity class Event < Grape::Entity
expose :title, :project_id, :action_name expose :title, :project_id, :action_name
expose :target_id, :target_type, :author_id expose :target_id, :target_type, :author_id
......
...@@ -8,6 +8,7 @@ describe API::API, api: true do ...@@ -8,6 +8,7 @@ describe API::API, api: true do
let!(:project) { create(:project, creator_id: user.id) } let!(:project) { create(:project, creator_id: user.id) }
let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) }
let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) }
let!(:note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') }
before { project.team << [user, :reporter] } before { project.team << [user, :reporter] }
...@@ -81,4 +82,68 @@ describe API::API, api: true do ...@@ -81,4 +82,68 @@ describe API::API, api: true do
end end
end end
end end
describe 'GET /projects:id/repository/commits/:sha/comments' do
context 'authorized user' do
it 'should return merge_request comments' do
get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user)
response.status.should == 200
json_response.should be_an Array
json_response.length.should == 1
json_response.first['note'].should == 'a comment on a commit'
json_response.first['author']['id'].should == user.id
end
it 'should return a 404 error if merge_request_id not found' do
get api("/projects/#{project.id}/repository/commits/1234ab/comments", user)
response.status.should == 404
end
end
context 'unauthorized user' do
it 'should not return the diff of the selected commit' do
get api("/projects/#{project.id}/repository/commits/1234ab/comments")
response.status.should == 401
end
end
end
describe 'POST /projects:id/repository/commits/:sha/comments' do
context 'authorized user' do
it 'should return comment' do
post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment'
response.status.should == 201
json_response['note'].should == 'My comment'
json_response['path'].should be_nil
json_response['line'].should be_nil
json_response['line_type'].should be_nil
end
it 'should return the inline comment' do
post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.diffs.first.new_path, line: 7, line_type: 'new'
response.status.should == 201
json_response['note'].should == 'My comment'
json_response['path'].should == project.repository.commit.diffs.first.new_path
json_response['line'].should == 7
json_response['line_type'].should == 'new'
end
it 'should return 400 if note is missing' do
post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user)
response.status.should == 400
end
it 'should return 404 if note is attached to non existent commit' do
post api("/projects/#{project.id}/repository/commits/1234ab/comments", user), note: 'My comment'
response.status.should == 404
end
end
context 'unauthorized user' do
it 'should not return the diff of the selected commit' do
post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments")
response.status.should == 401
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