diff --git a/app/models/activity_feed_observer.rb b/app/models/activity_feed_observer.rb index 4f8672e73..352b52f19 100644 --- a/app/models/activity_feed_observer.rb +++ b/app/models/activity_feed_observer.rb @@ -41,6 +41,7 @@ class ActivityFeedObserver < ActiveRecord::Observer :project_id => record.project.id, :issue_title => record.title, :project_name => record.project.name, :project_owner => record.project.owner.uname} ) end + Comment.create_link_on_issues_from_item(record) when 'Comment' return if record.automatic @@ -145,7 +146,8 @@ class ActivityFeedObserver < ActiveRecord::Observer :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} ) end - + # dont remove outdated issues link + Comment.create_link_on_issues_from_item(record) when 'BuildList' if record.mass_build.blank? && ( # Do not show mass build activity in activity feeds record.status_changed? && BUILD_LIST_STATUSES.include?(record.status) || diff --git a/app/models/comment.rb b/app/models/comment.rb index 636d9e170..3650af49f 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -4,7 +4,7 @@ class Comment < ActiveRecord::Base # User/Project#Num # User#Num # #Num - ISSUES_REGEX = /(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?[#!][0-9]+/ + ISSUES_REGEX = /(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?#[0-9]+/ belongs_to :commentable, :polymorphic => true, :touch => true belongs_to :user @@ -137,49 +137,47 @@ class Comment < ActiveRecord::Base 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 - commits - end current_ability = Ability.new(linker) + case + when item.is_a?(GitHook) + elements = commits + when item.is_a?(Issue) + elements = [[item, item.title], [item, item.body]] + opts = {:created_from_issue_id => item.id} + when item.commentable_type == 'Issue' + elements = [[item, item.body]] + opts = {:created_from_issue_id => item.commentable_id} + when item.commentable_type == 'Grit::Commit' + elements = [[item, item.body]] + opts = {:created_from_commit_hash => item.commentable_id} + else + raise "Unsupported item type #{item.class.name}!" + end + elements.each do |element| element[1].scan(ISSUES_REGEX).each do |hash| - delimiter = if hash.include? '!' - '!' - elsif hash.include? '#' - '#' - else - raise 'Unknown delimiter for the hash tag!' - end - issue = Issue.find_by_hash_tag hash, current_ability, item.project, delimiter + issue = Issue.find_by_hash_tag hash, current_ability, item.project next unless 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.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 + next if opts[:created_from_issue_id] == issue.id + # dont create duplicate link to issue + next if Comment.find_existing_automatic_comment issue, opts + if item.is_a?(GitHook) + opts = {:created_from_commit_hash => element[0].hex} + # dont create link to outdated commit + next if !item.project.repo.commit(element[0]) 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 = {: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 + if opts[:created_from_commit_hash] + comment.created_from_commit_hash = opts[:created_from_commit_hash] + elsif opts[:created_from_issue_id] + comment.data.merge!(:comment_id => item.id) if item.is_a? Comment + comment.created_from_issue_id = opts[:created_from_issue_id] else - comment.data.merge! :comment_id => item.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 + raise 'Unsupported opts for automatic comment!' end comment.save end @@ -204,4 +202,10 @@ class Comment < ActiveRecord::Base end end end + + def self.find_existing_automatic_comment issue, opts + find_dup = opts.merge(:automatic => true, :commentable_type => issue.class.name, + :commentable_id => issue.id) + Comment.exists? find_dup + end end diff --git a/app/models/group.rb b/app/models/group.rb index e10edf9ad..6eb0ce1c4 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -51,6 +51,10 @@ class Group < Avatar false end + def fullname + return description.present? ? "#{uname} (#{description})" : uname + end + protected def add_owner_to_members diff --git a/app/models/issue.rb b/app/models/issue.rb index 4e1ec3e1b..c9236a489 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -67,8 +67,8 @@ class Issue < ActiveRecord::Base recipients end - def self.find_by_hash_tag hash_tag, current_ability, project, delimiter = '#' - hash_tag =~ /([a-zA-Z0-9\-_]*\/)?([a-zA-Z0-9\-_]*)?#{delimiter}([0-9]+)/ + def self.find_by_hash_tag hash_tag, current_ability, project + hash_tag =~ /([a-zA-Z0-9\-_]*\/)?([a-zA-Z0-9\-_]*)?#([0-9]+)/ owner_uname = Regexp.last_match[1].presence || Regexp.last_match[2].presence || project.owner.uname project_name = Regexp.last_match[1] ? Regexp.last_match[2] : project.name serial_id = Regexp.last_match[3] diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index db69cf852..b665017a1 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -3,7 +3,8 @@ class CommentPresenter < ApplicationPresenter include PullRequestHelper attr_accessor :comment, :options - attr_reader :header, :image, :date, :caption, :content, :buttons, :is_reference_to_issue + attr_reader :header, :image, :date, :caption, :content, :buttons, :is_reference_to_issue, + :reference_project def initialize(comment, opts = {}) @is_reference_to_issue = !!(comment.automatic && comment.created_from_issue_id) # is it reference issue from another issue @@ -14,7 +15,7 @@ class CommentPresenter < ApplicationPresenter else issue = Issue.where(:id => comment.created_from_issue_id).first @referenced_issue = issue.pull_request || issue - if issue && Comment.exists?(comment.data[:comment_id]) + if issue && (comment.data[:comment_id].nil? || Comment.exists?(comment.data[:comment_id])) title = if issue == opts[:commentable] "#{issue.serial_id}" elsif issue.project.owner == opts[:commentable].project.owner @@ -110,4 +111,8 @@ class CommentPresenter < ApplicationPresenter pull_status_label @referenced_issue end.html_safe end + + def reference_project + @referenced_issue.project + end end diff --git a/app/presenters/git_presenters/commit_as_message_presenter.rb b/app/presenters/git_presenters/commit_as_message_presenter.rb index 26bc17c7b..988f324cd 100644 --- a/app/presenters/git_presenters/commit_as_message_presenter.rb +++ b/app/presenters/git_presenters/commit_as_message_presenter.rb @@ -3,16 +3,17 @@ class GitPresenters::CommitAsMessagePresenter < ApplicationPresenter include CommitHelper attr_accessor :commit - attr_reader :header, :image, :date, :caption, :content, :expandable, :is_reference_to_issue, :committer + attr_reader :header, :image, :date, :caption, :content, :expandable, + :is_reference_to_issue, :committer, :reference_project def initialize(commit, opts = {}) comment = opts[:comment] @is_reference_to_issue = !!comment # is it reference issue from commit @project = if comment - Project.where(:id => opts[:comment].data[:from_project_id]).first - else - opts[:project] - end + Project.where(:id => comment.data[:from_project_id]).first + else + opts[:project] + end commit = commit || @project.repo.commit(comment.created_from_commit_hash.to_s(16)) if @project if @project && commit @@ -76,6 +77,10 @@ class GitPresenters::CommitAsMessagePresenter < ApplicationPresenter false end + def reference_project + @project + end + protected def committer_link diff --git a/app/views/shared/_feed_message.html.haml b/app/views/shared/_feed_message.html.haml index 2caadaf11..f3c26aa68 100644 --- a/app/views/shared/_feed_message.html.haml +++ b/app/views/shared/_feed_message.html.haml @@ -1,27 +1,28 @@ -.activity{:id => presenter.comment_id? ? presenter.comment_anchor : ''} - .top - - if presenter.buttons? - %span.buttons= raw presenter.buttons.join(' | ').html_safe - .image - %img{:alt => "avatar", :src => presenter.image} - .text - -#.imaged move up a line. - %span.name= presenter.header - .both - %span.date= presenter.date - .both - - if presenter.caption? - %span.subject= presenter.caption - - if presenter.expandable? and presenter.content? - %span.data-expander.collapsed{:id => "expand#{item_no}"}   +-if !presenter.is_reference_to_issue || can?(:show, presenter.reference_project) + .activity{:id => presenter.comment_id? ? presenter.comment_anchor : ''} + .top + - if presenter.buttons? + %span.buttons= raw presenter.buttons.join(' | ').html_safe + .image + %img{:alt => "avatar", :src => presenter.image} + .text + -#.imaged move up a line. + %span.name= presenter.header .both - .both - - if presenter.content? - %div - =presenter.issue_referenced_state if presenter.issue_referenced_state? - .fulltext{:class => "#{presenter.expandable? ? "hidden" : ''} #{presenter.caption? ? "" : "alone"}", - :id => presenter.expandable? ? "content-expand#{item_no}" : ''} - .md_and_cm{:class => presenter.is_reference_to_issue ? '' : 'cm-s-default'} - =presenter.is_reference_to_issue ? presenter.content : markdown(presenter.content) + %span.date= presenter.date .both + - if presenter.caption? + %span.subject= presenter.caption + - if presenter.expandable? and presenter.content? + %span.data-expander.collapsed{:id => "expand#{item_no}"}   + .both + .both + - if presenter.content? + %div + =presenter.issue_referenced_state if presenter.issue_referenced_state? + .fulltext{:class => "#{presenter.expandable? ? "hidden" : ''} #{presenter.caption? ? "" : "alone"}", + :id => presenter.expandable? ? "content-expand#{item_no}" : ''} + .md_and_cm{:class => presenter.is_reference_to_issue ? '' : 'cm-s-default'} + =presenter.is_reference_to_issue ? presenter.content : markdown(presenter.content) + .both diff --git a/config/locales/models/comment.en.yml b/config/locales/models/comment.en.yml index 5def48633..ed1f6f504 100644 --- a/config/locales/models/comment.en.yml +++ b/config/locales/models/comment.en.yml @@ -29,10 +29,10 @@ en: reference_format: Reference Format reference_format_example: | for members: @abf - for issues: - #123 abf#123 abf/rosa-build#123 - for pull requests: - !123 abf!123 abf/rosa-build!123 + for issues and pull requests: + #123 + abf#123 + abf/rosa-build#123 for commits: 123456 issues: Issues diff --git a/config/locales/models/comment.ru.yml b/config/locales/models/comment.ru.yml index e4c5b0c4c..0b8c199a4 100644 --- a/config/locales/models/comment.ru.yml +++ b/config/locales/models/comment.ru.yml @@ -29,10 +29,10 @@ ru: reference_format: Формат ссылок reference_format_example: | для участников: @abf - для задач: - #123 abf#123 abf/rosa-build#123 - для пул реквестов: - !123 abf!123 abf/rosa-build!123 + для задач и пул реквестов: + #123 + abf#123 + abf/rosa-build#123 для коммитов: 123456 issues: Задачи pull_requests: Пул реквесты diff --git a/lib/modules/models/markdown.rb b/lib/modules/models/markdown.rb index 55fe6c288..9b7a5fe14 100644 --- a/lib/modules/models/markdown.rb +++ b/lib/modules/models/markdown.rb @@ -9,14 +9,10 @@ module Modules # # Supported reference formats are: # * @foo for team members - # * for issues: + # * for issues & pull requests: # * #123 # * abf#123 # * abf/rosa-build#123 - # * for pull requests: - # * !123 - # * abf!123 - # * abf/rosa-build!123 # * 123456 for commits # # It also parses Emoji codes to insert images. See @@ -104,15 +100,14 @@ module Modules REFERENCE_PATTERN = %r{ (?[\W\/])? # Prefix ( # Reference - @(?[a-zA-Z][a-zA-Z0-9_\-\.]*) # User uname + @(?[a-zA-Z][a-zA-Z0-9_\-\.]*) # User/Group uname |(?(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?\#[0-9]+) # Issue ID - |(?(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?\![0-9]+) # PR ID |(?[\h]{6,40}) # Commit ID ) (?\W)? # Suffix }x.freeze - TYPES = [:user, :issue, :pull_request, :commit].freeze + TYPES = [:user, :issue, :commit].freeze def parse_references(text) # parse reference links @@ -166,27 +161,25 @@ module Modules end def reference_user(identifier) - if member = @project.all_members.select {|u| u.uname == identifier} #.joins(:user).where(users: {uname: identifier}).first - link_to("@#{identifier}", user_path(identifier), html_options.merge(class: "gfm gfm-team_member #{html_options[:class]}")) if member + member = User.where(uname: identifier).first || Group.where(uname: identifier).first + if member + link_to("@#{identifier}", "/#{identifier}", html_options.merge(title: member.fullname, class: "gfm gfm-member #{html_options[:class]}")) end end def reference_issue(identifier) if issue = Issue.find_by_hash_tag(identifier, current_ability, @project) - url = project_issue_path(issue.project.owner, issue.project.name, issue.serial_id) - title = "#{Issue.model_name.human}: #{issue.title}" + if issue.pull_request + title = "#{PullRequest.model_name.human}: #{pull_request.title}" + project_pull_request_path(pull_request.to_project, pull_request) + else + title = "#{Issue.model_name.human}: #{issue.title}" + url = project_issue_path(issue.project.owner, issue.project.name, issue.serial_id) + end link_to(identifier, url, html_options.merge(title: title, class: "gfm gfm-issue #{html_options[:class]}")) end end - def reference_pull_request(identifier) - issue = Issue.find_by_hash_tag(identifier, current_ability, @project, '!') - if pull_request = issue.pull_request - title = "#{PullRequest.model_name.human}: #{pull_request.title}" - link_to(identifier, project_pull_request_path(pull_request.to_project, pull_request), html_options.merge(title: title, class: "gfm gfm-pull_request #{html_options[:class]}")) - end - end - def reference_commit(identifier) if commit = @project.repo.commit(identifier) link_to shortest_hash_id(commit.id), commit_path(@project, commit.id) diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index a3e3e6724..ec836dd0f 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -22,7 +22,7 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML code_class = "class=\"#{language.downcase}\"" if language.present? <<-HTML -
#{code}
+
#{code}
HTML end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index ef67f810f..2226a8c13 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -160,6 +160,7 @@ describe Comment do :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}]" @@ -167,6 +168,31 @@ describe Comment do Comment.where(:automatic => true, :created_from_issue_id => @issue.id).count.should == 2 end + + it 'should create automatic comment by issue title' do + issue = FactoryGirl.create(:issue, :project => @project, :user => @user, + :title => "link to ##{@issue.serial_id}") + Comment.where(:automatic => true, + :created_from_issue_id => issue.id).count.should == 1 + end + + it 'should create automatic comment from issue body' do + issue = FactoryGirl.create(:issue, :project => @project, :user => @user, + :body => "link to ##{@issue.serial_id}") + Comment.where(:automatic => true, + :created_from_issue_id => issue.id).count.should == 1 + end + + it 'should create only one automatic comment from issue title and body' do + issue = FactoryGirl.create(:issue, :project => @project, :user => @user, + :title => "link to ##{@issue.serial_id} in title", + :body => "link to ##{@issue.serial_id} in body") + Comment.where(:automatic => true, + :created_from_issue_id => issue.id).count.should == 1 + end + + + end end end