WIP: refactor the first-contributor to Issuable

this will remove the need make N queries (per-note) at the
cost of having to mark notes with an attribute

this opens up the possibility for other special roles for notes

13 files changed, 59 insertions, 21 deletions

diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb
index 41c3114ad1e..cdcfefd90c9 100644
--- a/app/controllers/concerns/renders_notes.rb
+++ b/app/controllers/concerns/renders_notes.rb
@@ -1,7 +1,8 @@
 module RendersNotes
-  def prepare_notes_for_rendering(notes)
+  def prepare_notes_for_rendering(notes, noteable=nil)
     preload_noteable_for_regular_notes(notes)
     preload_max_access_for_authors(notes, @project)
+    preload_first_time_contribution_for_authors(noteable, notes) if noteable.is_a?(Issuable)
     Banzai::NoteRenderer.render(notes, @project, current_user)
 
     notes
@@ -19,4 +20,10 @@ module RendersNotes
   def preload_noteable_for_regular_notes(notes)
     ActiveRecord::Associations::Preloader.new.preload(notes.reject(&:for_commit?), :noteable)
   end
+
+  def preload_first_time_contribution_for_authors(issuable, notes)
+    return unless issuable.first_contribution?
+    same_author = lambda {|n| n.author_id == issuable.author_id}
+    notes.select(&same_author).each {|note| note.special_role = :first_time_contributor}
+  end
 end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 6de125e7e80..1a775def506 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -127,7 +127,7 @@ class Projects::CommitController < Projects::ApplicationController
     @discussions = commit.discussions
 
     @notes = (@grouped_diff_discussions.values.flatten + @discussions).flat_map(&:notes)
-    @notes = prepare_notes_for_rendering(@notes)
+    @notes = prepare_notes_for_rendering(@notes, @commit)
   end
 
   def assign_change_commit_vars
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index dc9e6f71152..ab9f132b502 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -85,7 +85,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @note     = @project.notes.new(noteable: @issue)
 
     @discussions = @issue.discussions
-    @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
+    @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
 
     respond_to do |format|
       format.html
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index 330b7df4541..109418c73f7 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -61,6 +61,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
 
     @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@compare.diff_refs)
-    @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes))
+    @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes), @merge_request)
   end
 end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 5095d7fd445..6c4a783e11a 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -60,12 +60,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
         # Build a note object for comment form
         @note = @project.notes.new(noteable: @merge_request)
 
-        @discussions = @merge_request.discussions
-        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
-
         @noteable = @merge_request
         @commits_count = @merge_request.commits_count
 
+        @discussions = @merge_request.discussions
+        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
+
         labels
 
         set_pipeline_variables
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index d07143d294f..7c19aa7bb23 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -64,7 +64,7 @@ class Projects::SnippetsController < Projects::ApplicationController
         @noteable = @snippet
 
         @discussions = @snippet.discussions
-        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
+        @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
         render 'show'
       end
 
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 8c3abd0a085..c1cdc7c9831 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -66,7 +66,7 @@ class SnippetsController < ApplicationController
     @noteable = @snippet
 
     @discussions = @snippet.discussions
-    @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
+    @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
 
     respond_to do |format|
       format.html do
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 717abf2082d..11fae16f04d 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -138,6 +138,8 @@ module IssuablesHelper
     end
 
     output << "&ensp;".html_safe
+    output << issuable_first_contribution_icon if issuable.first_contribution?
+
     output << content_tag(:span, (issuable.task_status if issuable.tasks?), id: "task_status", class: "hidden-xs hidden-sm")
     output << content_tag(:span, (issuable.task_status_short if issuable.tasks?), id: "task_status_short", class: "hidden-md hidden-lg")
 
@@ -173,6 +175,13 @@ module IssuablesHelper
     html.html_safe
   end
 
+  def issuable_first_contribution_icon
+    content_tag(:span, class: 'fa-stack has-tooltip', title: _('1st contribution!')) do
+      concat(icon('certificate', class: "fa-stack-2x"))
+      concat(content_tag(:strong, '1', class: 'fa-inverse fa-stack-1x'))
+    end 
+  end
+
   def assigned_issuables_count(issuable_type)
     case issuable_type
     when :issues
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 1e7d346ab8f..ce028195e51 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -73,12 +73,7 @@ module NotesHelper
   end
 
   def note_max_access_for_user(note)
-    note.project.team.human_max_access(note.author_id)
-  end
-
-  def note_first_contribution_for_user?(note)
-    note.noteable.author_id == note.author_id &&
-    note.project.merge_requests.merged.where(author_id: note.author_id).empty?
+    note.project.team.max_member_access(note.author_id)
   end
 
   def discussion_path(discussion)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 681c3241dbb..7b1c4e7a2d5 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -334,4 +334,9 @@ module Issuable
     metrics = self.metrics || create_metrics
     metrics.record!
   end
+
+  def first_contribution?
+    return false if project.team.max_member_access(author_id) > Gitlab::Access::GUEST
+    project.merge_requests.merged.where(author_id: author_id).empty?
+  end
 end
diff --git a/app/models/note.rb b/app/models/note.rb
index 1073c115630..d04bccc6e81 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -15,7 +15,18 @@ class Note < ActiveRecord::Base
   include IgnorableColumn
   include Editable
 
+  module SpecialRole
+    FIRST_TIME_CONTRIBUTOR = :first_time_contributor
+
+    class << self
+      def values
+        constants.map {|const| self.const_get(const)}
+      end
+    end
+  end
+
   ignore_column :original_discussion_id
+  ignore_column :special_role
 
   cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
 
@@ -32,9 +43,12 @@ class Note < ActiveRecord::Base
   # Banzai::ObjectRenderer
   attr_accessor :user_visible_reference_count
 
-  # Attribute used to store the attributes that have ben changed by quick actions.
+  # Attribute used to store the attributes that have been changed by quick actions.
   attr_accessor :commands_changes
 
+  # A special role that may be displayed on issuable's discussions
+  attr_accessor :special_role
+  
   default_value_for :system, false
 
   attr_mentionable :note, pipeline: :note
@@ -206,6 +220,15 @@ class Note < ActiveRecord::Base
     super(noteable_type.to_s.classify.constantize.base_class.to_s)
   end
 
+  def special_role=(role)
+    raise "Role is undefined, #{role} not found in #{SpecialRole.values}" unless SpecialRole.values.include? role
+    @special_role = role
+  end
+
+  def has_special_role?(role)
+    return @special_role == role
+  end
+
   def editable?
     !system?
   end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 674eacd28e8..09049824ff7 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -150,7 +150,7 @@ class ProjectTeam
   end
 
   def human_max_access(user_id)
-    Gitlab::Access.options_with_owner.key(max_member_access(user_id))
+    Gitlab::Access.human_access(max_member_access(user_id))
   end
 
   # Determine the maximum access level for a group of users in bulk.
diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index 38548cbe93a..926ceff3ee2 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -1,9 +1,8 @@
-- access = note_max_access_for_user(note)
-- if access
-  %span.note-role= access
-- if note_first_contribution_for_user?(note)
+- if note.has_special_role? :first_time_contributor
   %span.note-role.has-tooltip{ title: _("This user hasn't yet contributed code to this project. Handle with care.")}
     = _('First-time contributor')
+- elsif access = access = note_max_access_for_user(note)
+  %span.note-role= Gitlab::Access.human_access(access)
 
 - if note.resolvable?
   - can_resolve = can?(current_user, :resolve_note, note)