Merge pull request #95 from abf/rosa-build:19-improvements

[#19] improvements for the reference

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) ||

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

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

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]

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

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

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
 

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

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: Пул реквесты

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{
         (?<prefix>[\W\/])?                                                     # Prefix
         (                                                                      # Reference
-           @(?<user>[a-zA-Z][a-zA-Z0-9_\-\.]*)                                 # User uname
+           @(?<user>[a-zA-Z][a-zA-Z0-9_\-\.]*)                                 # User/Group uname
           |(?<issue>(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?\#[0-9]+)        # Issue ID
-          |(?<pull_request>(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?\![0-9]+) # PR ID
           |(?<commit>[\h]{6,40})                                               # Commit ID
         )
         (?<suffix>\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)

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
 
-       <pre><code #{code_class}">#{code}</code></pre>
+       <pre><code #{code_class}>#{code}</code></pre>
 
     HTML
   end

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