From 66f59ab6856f4c4efbbc47193faf975e3cecdb3d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 2 Apr 2013 14:35:25 +0600 Subject: [PATCH] [#19] fix creating & showing comments from commit message --- app/models/comment.rb | 41 +++++++++++-------- .../commit_as_message_presenter.rb | 4 +- app/views/projects/comments/_list.html.haml | 2 +- spec/models/comment_for_commit_spec.rb | 19 +++++---- spec/models/comment_spec.rb | 22 ++++++---- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 820df3ccb..f892ec682 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -135,12 +135,12 @@ class Comment < ActiveRecord::Base return true end - def self.create_link_on_issues_from_item item, opts = {} + def self.create_link_on_issues_from_item item, commits = nil linker = item.user elements = if item.is_a? Comment [[item, item.body]] elsif item.is_a? GitHook - opts[:commits] + commits end current_ability = Ability.new(linker) @@ -155,26 +155,31 @@ class Comment < ActiveRecord::Base end issue = Issue.find_by_hash_tag hash, current_ability, item.project, delimiter next unless issue - next if issue == item.try(:commentable) # dont create link to the same issue - # dont create duplicate link to issue + # dont create link to the same issue + next if item.respond_to?(:commentable) && issue == item.try(:commentable) find_dup = {:automatic => true, :commentable_type => issue.class.name, :commentable_id => issue.id} - if (item.commentable_type == 'Issue' && - Comment.exists?(find_dup.merge :created_from_issue_id => item.commentable_id)) || - (item.commentable_type == 'Grit::Commit' && - Comment.exists?(find_dup.merge :created_from_commit_hash => item.commentable_id)) - next + if item.is_a? GitHook + find_dup.merge! :created_from_commit_hash => element[0].hex + elsif item.commentable_type == 'Issue' + find_dup.merge! :created_from_issue_id => item.commentable_id + elsif item.commentable_type == 'Grit::Commit' + find_dup.merge! :created_from_commit_hash => item.commentable_id end + next if Comment.exists? find_dup # dont create duplicate link to issue + comment = linker.comments.new :body => 'automatic comment' comment.commentable, comment.project, comment.automatic = issue, issue.project, true - comment.data = {:comment_id => item.id, :from_project_id => item.project.id} - if item.is_a?(Comment) && item.commentable_type == 'Issue' - comment.created_from_issue_id = item.commentable_id - elsif item.is_a?(Comment) && item.commentable_type == 'Grit::Commit' - comment.created_from_commit_hash = item.commentable_id - elsif item.is_a? GitHook - repo_commit = git_hook.project.repo.commit element[0] - next unless repo_commit - comment.data.merge! :commit_hash => commit[0] + comment.data = {:from_project_id => item.project.id} + if item.is_a? GitHook + next unless item.project.repo.commit element[0] + comment.created_from_commit_hash = element[0].hex + else + comment.data = {:from_project_id => item.project.id} + if item.commentable_type == 'Issue' + comment.created_from_issue_id = item.commentable_id + elsif item.commentable_type == 'Grit::Commit' + comment.created_from_commit_hash = item.commentable_id + end end comment.save end diff --git a/app/presenters/git_presenters/commit_as_message_presenter.rb b/app/presenters/git_presenters/commit_as_message_presenter.rb index e2a553d3a..bb7fd0b56 100644 --- a/app/presenters/git_presenters/commit_as_message_presenter.rb +++ b/app/presenters/git_presenters/commit_as_message_presenter.rb @@ -14,7 +14,7 @@ class GitPresenters::CommitAsMessagePresenter < ApplicationPresenter opts[:project] end if @project - commit = commit || @project.repo.commit(comment.created_from_issue_id) + commit = commit || @project.repo.commit(comment.created_from_commit_hash.to_s(16)) @committer = User.where(:email => commit.committer.email).first || commit.committer @commit_hash = commit.id @@ -22,7 +22,7 @@ class GitPresenters::CommitAsMessagePresenter < ApplicationPresenter @commit_message = commit.message else @committer = t('layout.commits.unknown_committer') - @commit_hash = comment.data[:commit_hash] + @commit_hash = comment.created_from_commit_hash @committed_date = @authored_date = comment.created_at @commit_message = t('layout.commits.deleted') end diff --git a/app/views/projects/comments/_list.html.haml b/app/views/projects/comments/_list.html.haml index 43f7a5df0..f7edaff1d 100644 --- a/app/views/projects/comments/_list.html.haml +++ b/app/views/projects/comments/_list.html.haml @@ -2,7 +2,7 @@ .hr %h3#block-list= t("layout.comments.comments_header") - list.each do |comment| - -if !comment.created_from_commit_hash + -unless comment.created_from_commit_hash = render 'projects/comments/comment', :comment => comment, :data => {:project => project, :commentable => commentable} -else - GitPresenters::CommitAsMessagePresenter.present(nil, :comment => comment) do |presenter| diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index c0e261e03..9972a699a 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -283,41 +283,46 @@ describe Comment do it 'should create automatic comment' do create_comment_in_commit(@commit, @project, "test link to ##{@issue.serial_id}; [##{@second_issue.serial_id}]") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_commit_hash => @commit.id.hex).count.should == 1 end it 'should create automatic comment in the another project issue' do body = "[#{@another_project.name_with_owner}##{@issue_in_another_project.serial_id}]" create_comment_in_commit(@commit, @project, body) Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @issue_in_another_project.id).count.should == 1 + :commentable_id => @issue_in_another_project.id, + :created_from_commit_hash => @commit.id.hex).count.should == 1 end it 'should create automatic comment in the same name project issue' do body = "[#{@same_name_project.owner.uname}##{@issue_in_same_name_project.serial_id}]" create_comment_in_commit(@commit, @project, body) Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @issue_in_same_name_project.id).count.should == 1 + :commentable_id => @issue_in_same_name_project.id, + :created_from_commit_hash => @commit.id.hex).count.should == 1 end it 'should not create duplicate automatic comment' do create_comment_in_commit(@commit, @project, "test link to [##{@second_issue.serial_id}]") create_comment_in_commit(@commit, @project, "test duplicate link to [##{@second_issue.serial_id}]") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_commit_hash => @commit.id.hex).count.should == 1 end it 'should not create duplicate automatic comment from one' do create_comment_in_commit(@commit, @project, "test link to [##{@second_issue.serial_id}]; ##{@second_issue.serial_id}") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_commit_hash => @commit.id.hex).count.should == 1 end it 'should create two automatic comment' do body = "test ##{@second_issue.serial_id}" + " && [#{@another_project.name_with_owner}##{@issue_in_another_project.serial_id}]" - p "body: #{body}" create_comment_in_commit(@commit, @project, body) - Comment.where(:automatic => true).count.should == 2 + Comment.where(:automatic => true, + :created_from_commit_hash => @commit.id.hex).count.should == 2 end end end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 9034bae8d..ef67f810f 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -120,46 +120,52 @@ describe Comment do it 'should create automatic comment' do create_comment_in_issue(@issue, "test link to ##{@issue.serial_id}; [##{@second_issue.serial_id}]") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should not create automatic comment to the same issue' do create_comment_in_issue(@issue, "test link to ##{@issue.serial_id}; [##{@second_issue.serial_id}]") - Comment.where(:automatic => true).count.should == 1 + Comment.where(:automatic => true, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should create automatic comment in the another project issue' do body = "[#{@another_project.name_with_owner}##{@issue_in_another_project.serial_id}]" create_comment_in_issue(@issue, body) Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @issue_in_another_project.id).count.should == 1 + :commentable_id => @issue_in_another_project.id, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should create automatic comment in the same name project issue' do body = "[#{@same_name_project.owner.uname}##{@issue_in_same_name_project.serial_id}]" create_comment_in_issue(@issue, body) Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @issue_in_same_name_project.id).count.should == 1 + :commentable_id => @issue_in_same_name_project.id, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should not create duplicate automatic comment' do create_comment_in_issue(@issue, "test link to [##{@second_issue.serial_id}]") create_comment_in_issue(@issue, "test duplicate link to [##{@second_issue.serial_id}]") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should not create duplicate automatic comment from one' do create_comment_in_issue(@issue, "test link to [##{@second_issue.serial_id}]; ##{@second_issue.serial_id}") Comment.where(:automatic => true, :commentable_type => 'Issue', - :commentable_id => @second_issue.id).count.should == 1 + :commentable_id => @second_issue.id, + :created_from_issue_id => @issue.id).count.should == 1 end it 'should create two automatic comment' do body = "test ##{@second_issue.serial_id}" + " && [#{@another_project.name_with_owner}##{@issue_in_another_project.serial_id}]" - p "body: #{body}" create_comment_in_issue(@issue, body) - Comment.where(:automatic => true).count.should == 2 + Comment.where(:automatic => true, + :created_from_issue_id => @issue.id).count.should == 2 end end end