Merge branch 'rs-event-titles' into 'master'

Add an Event's target's title to its reference link

Given an activity feed entry like:

> Douwe Maan commented on [issue #123] at [gitlab-org/gitlab-ce]

...the `issue #123` link will now have a `title` attribute.

Plus some minor refactorings, see individual commits for details.

See merge request !4090

app/helpers/events_helper.rb

@@ -3,7 +3,7 @@ module EventsHelper
     author = event.author
 
     if author
-      link_to author.name, user_path(author.username), title: h(author.name)
+      link_to author.name, user_path(author.username), title: author.name
     else
       event.author_name
     end
@@ -57,11 +57,7 @@ module EventsHelper
       words << event.ref_name
       words << "at"
     elsif event.commented?
-      if event.note_commit?
-        words << event.note_short_commit_id
-      else
-        words << "##{truncate event.note_target_iid}"
-      end
+      words << event.note_target_reference
       words << "at"
     elsif event.milestone?
       words << "##{event.target_iid}" if event.target_iid
@@ -84,21 +80,12 @@ module EventsHelper
     elsif event.merge_request?
       namespace_project_merge_request_url(event.project.namespace,
                                           event.project, event.merge_request)
-    elsif event.note? && event.note_commit?
+    elsif event.note? && event.commit_note?
       namespace_project_commit_url(event.project.namespace, event.project,
                                    event.note_target)
     elsif event.note?
       if event.note_target
-        if event.note_commit?
-          namespace_project_commit_path(event.project.namespace, event.project,
-                                        event.note_commit_id,
-                                        anchor: dom_id(event.target))
-        elsif event.note_project_snippet?
-          namespace_project_snippet_path(event.project.namespace,
-                                         event.project, event.note_target)
-        else
-          event_note_target_path(event)
-        end
+        event_note_target_path(event)
       end
     elsif event.push?
       push_event_feed_url(event)
@@ -134,42 +121,30 @@ module EventsHelper
   end
 
   def event_note_target_path(event)
-    if event.note? && event.note_commit?
-      namespace_project_commit_path(event.project.namespace, event.project,
-                                    event.note_target)
+    if event.note? && event.commit_note?
+      namespace_project_commit_path(event.project.namespace,
+                                    event.project,
+                                    event.note_target,
+                                    anchor: dom_id(event.target))
+    elsif event.project_snippet_note?
+      namespace_project_snippet_path(event.project.namespace,
+                                     event.project,
+                                     event.note_target,
+                                     anchor: dom_id(event.target))
     else
       polymorphic_path([event.project.namespace.becomes(Namespace),
                         event.project, event.note_target],
-                       anchor: dom_id(event.target))
+                        anchor: dom_id(event.target))
     end
   end
 
   def event_note_title_html(event)
     if event.note_target
-      if event.note_commit?
-        link_to(
-          namespace_project_commit_path(event.project.namespace, event.project,
-                                        event.note_commit_id,
-                                        anchor: dom_id(event.target), title: h(event.target_title)),
-          class: "commit_short_id"
-        ) do
-          "#{event.note_target_type} #{event.note_short_commit_id}"
-        end
-      elsif event.note_project_snippet?
-        link_to(namespace_project_snippet_path(event.project.namespace,
-                                               event.project,
-                                               event.note_target), title: h(event.project.name)) do
-          "#{event.note_target_type} #{truncate event.note_target.to_reference}"
-        end
-      else
-        link_to event_note_target_path(event) do
-          "#{event.note_target_type} #{truncate event.note_target.to_reference}"
-        end
+      link_to(event_note_target_path(event), title: event.target_title, class: 'has-tooltip') do
+        "#{event.note_target_type} #{event.note_target_reference}"
       end
     else
-      content_tag :strong do
-        "(deleted)"
-      end
+      content_tag(:strong, '(deleted)')
     end
   end
 

app/models/event.rb

@@ -80,7 +80,7 @@ class Event < ActiveRecord::Base
   end
 
   def target_title
-    target.title if target && target.respond_to?(:title)
+    target.try(:title)
   end
 
   def created?
@@ -266,28 +266,20 @@ class Event < ActiveRecord::Base
     branch? && project.default_branch != branch_name
   end
 
-  def note_commit_id
-    target.commit_id
-  end
-
   def target_iid
     target.respond_to?(:iid) ? target.iid : target_id
   end
 
-  def note_short_commit_id
-    Commit.truncate_sha(note_commit_id)
-  end
-
-  def note_commit?
-    target.noteable_type == "Commit"
+  def commit_note?
+    target.for_commit?
   end
 
   def issue_note?
-    note? && target && target.noteable_type == "Issue"
+    note? && target && target.for_issue?
   end
 
-  def note_project_snippet?
-    target.noteable_type == "Snippet"
+  def project_snippet_note?
+    target.for_snippet?
   end
 
   def note_target
@@ -295,19 +287,22 @@ class Event < ActiveRecord::Base
   end
 
   def note_target_id
-    if note_commit?
+    if commit_note?
       target.commit_id
     else
       target.noteable_id.to_s
     end
   end
 
-  def note_target_iid
-    if note_target.respond_to?(:iid)
-      note_target.iid
+  def note_target_reference
+    return unless note_target
+
+    # Commit#to_reference returns the full SHA, but we want the short one here
+    if commit_note?
+      note_target.short_id
     else
-      note_target_id
-    end.to_s
+      note_target.to_reference
+    end
   end
 
   def note_target_type

app/models/note.rb

@@ -19,6 +19,7 @@ class Note < ActiveRecord::Base
   delegate :gfm_reference, :local_reference, to: :noteable
   delegate :name, to: :project, prefix: true
   delegate :name, :email, to: :author, prefix: true
+  delegate :title, to: :noteable, allow_nil: true
 
   before_validation :set_award!
 

app/models/project.rb

@@ -50,6 +50,8 @@ class Project < ActiveRecord::Base
   attr_accessor :new_default_branch
   attr_accessor :old_path_with_namespace
 
+  alias_attribute :title, :name
+
   # Relations
   belongs_to :creator, foreign_key: 'creator_id', class_name: 'User'
   belongs_to :group, -> { where(type: Group) }, foreign_key: 'namespace_id'

app/views/events/event/_common.html.haml

@@ -4,7 +4,7 @@
     = event_action_name(event)
 
   - if event.target
-    %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target]
+    %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target], title: event.target_title
 
   = event_preposition(event)