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

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	CHANGELOG

6 files changed, 215 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2d09a608f12..977e0df8901 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,7 @@ v 7.5.0
   - Tie up loose ends with annotated tags: API & UI (Sean Edge)
   - Return valid json for deleting branch via 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
   - Fix raw snippets view
diff --git a/app/models/note.rb b/app/models/note.rb
index df45d023a00..5bf645bbd1d 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -317,7 +317,7 @@ class Note < ActiveRecord::Base
   end
 
   def diff_file_index
-    line_code.split('_')[0]
+    line_code.split('_')[0] if line_code
   end
 
   def diff_file_name
@@ -333,11 +333,11 @@ class Note < ActiveRecord::Base
   end
 
   def diff_old_line
-    line_code.split('_')[1].to_i
+    line_code.split('_')[1].to_i if line_code
   end
 
   def diff_new_line
-    line_code.split('_')[2].to_i
+    line_code.split('_')[2].to_i if line_code
   end
 
   def generate_line_code(line)
@@ -358,6 +358,20 @@ class Note < ActiveRecord::Base
     @diff_line
   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
     max_number_of_lines = 16
     prev_match_line = nil
diff --git a/doc/api/commits.md b/doc/api/commits.md
index 9475ecbaa67..eb8d6a43592 100644
--- a/doc/api/commits.md
+++ b/doc/api/commits.md
@@ -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"
+}
+```
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 4a67313430a..6c5391b98c8 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -50,6 +50,67 @@ module API
         not_found! "Commit" unless commit
         commit.diffs
       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
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 40696489ba2..42e4442365d 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -179,6 +179,14 @@ module API
       expose :author, using: Entities::UserBasic
     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
       expose :title, :project_id, :action_name
       expose :target_id, :target_type, :author_id
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 38e0a284c36..a3f58f50913 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -8,6 +8,7 @@ describe API::API, api: true  do
   let!(:project) { create(:project, creator_id: user.id) }
   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!(: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] }
 
@@ -81,4 +82,68 @@ describe API::API, api: true  do
       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