Merge branch '35161_first_time_contributor_badge' into 'master'

app/assets/stylesheets/pages/notes.scss

@@ -516,7 +516,7 @@ ul.notes {
 }
 
 .note-actions-item {
-  margin-left: 15px;
+  margin-left: 12px;
   display: flex;
   align-items: center;
 
@@ -620,15 +620,25 @@ ul.notes {
 
 .note-role {
   position: relative;
-  padding: 0 7px;
+  display: inline-block;
   color: $notes-role-color;
   font-size: 12px;
   line-height: 20px;
-  border: 1px solid $border-color;
-  border-radius: $label-border-radius;
+  margin: 0 3px;
+
+  &.note-role-access {
+    padding: 0 7px;
+    border: 1px solid $border-color;
+    border-radius: $label-border-radius;
+  }
+
+  &.note-role-special {
+    text-shadow: 0 0 15px $gl-text-color-inverted;
+  }
 }
 
 
+
 /**
  * Line note button on the side of diffs
  */

app/controllers/concerns/renders_notes.rb

 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)
     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(noteable, notes)
+    return unless noteable.is_a?(Issuable) && noteable.first_contribution?
+
+    notes.each {|n| n.specialize_for_first_contribution!(noteable)}
+  end
 end

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

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

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

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

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
 

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

app/helpers/issuables_helper.rb

@@ -134,6 +134,8 @@ module IssuablesHelper
     end
 
     output << "&ensp;".html_safe
+    output << content_tag(:span, (issuable_first_contribution_icon if issuable.first_contribution?), class: 'has-tooltip', title: _('1st 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")
 
@@ -169,6 +171,13 @@ module IssuablesHelper
     html.html_safe
   end
 
+  def issuable_first_contribution_icon
+    content_tag(:span, class: 'fa-stack') 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

app/helpers/notes_helper.rb

@@ -73,7 +73,7 @@ module NotesHelper
   end
 
   def note_max_access_for_user(note)
-    note.project.team.human_max_access(note.author_id)
+    note.project.team.max_member_access(note.author_id)
   end
 
   def discussion_path(discussion)

app/models/concerns/issuable.rb

@@ -334,4 +334,11 @@ module Issuable
     metrics = self.metrics || create_metrics
     metrics.record!
   end
+
+  ##
+  # Override in issuable specialization
+  #
+  def first_contribution?
+    false
+  end
 end

app/models/merge_request.rb

@@ -960,6 +960,12 @@ class MergeRequest < ActiveRecord::Base
     Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache
   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
+
   private
 
   def write_ref

app/models/note.rb

@@ -15,6 +15,16 @@ 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
 
   cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
@@ -32,9 +42,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
@@ -141,6 +154,10 @@ class Note < ActiveRecord::Base
         .group(:noteable_id)
         .where(noteable_type: type, noteable_id: ids)
     end
+
+    def has_special_role?(role, note)
+      note.special_role == role
+    end
   end
 
   def cross_reference?
@@ -206,6 +223,22 @@ 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)
+    self.class.has_special_role?(role, self)
+  end
+
+  def specialize_for_first_contribution!(noteable)
+    return unless noteable.author_id == self.author_id
+
+    self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
+  end
+
   def editable?
     !system?
   end

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.

app/views/projects/notes/_actions.html.haml

-- access = note_max_access_for_user(note)
-- if access
-  %span.note-role= access
+- if note.has_special_role?(Note::SpecialRole::FIRST_TIME_CONTRIBUTOR)
+  %span.note-role.note-role-special.has-tooltip{ title: _("This is the author's first Merge Request to this project. Handle with care.") }
+    = issuable_first_contribution_icon
+- if access = note_max_access_for_user(note)
+  %span.note-role.note-role-access= Gitlab::Access.human_access(access)
 
 - if note.resolvable?
   - can_resolve = can?(current_user, :resolve_note, note)

changelogs/unreleased/35161_first_time_contributor_badge.yml

+---
+title: "First-time contributor badge"
+merge_request: 13143
+author: Micaël Bergeron <micaelbergeron@gmail.com>

lib/gitlab/access.rb

@@ -67,10 +67,14 @@ module Gitlab
       def protection_values
         protection_options.values
       end
+
+      def human_access(access)
+        options_with_owner.key(access)
+      end
     end
 
     def human_access
-      Gitlab::Access.options_with_owner.key(access_field)
+      Gitlab::Access.human_access(access_field)
     end
 
     def owner?

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -56,6 +56,28 @@ describe Projects::MergeRequestsController do
 
         expect(response).to be_success
       end
+
+      context "loads notes" do
+        let(:first_contributor) { create(:user) }
+        let(:contributor) { create(:user) }
+        let(:merge_request) { create(:merge_request, author: first_contributor, target_project: project, source_project: project) }
+        let(:contributor_merge_request) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
+        # the order here is important
+        # as the controller reloads these from DB, references doesn't correspond after
+        let!(:first_contributor_note) { create(:note, author: first_contributor, noteable: merge_request, project: project) }
+        let!(:contributor_note) { create(:note, author: contributor, noteable: merge_request, project: project) }
+        let!(:owner_note) { create(:note, author: user, noteable: merge_request, project: project) }
+
+        it "with special_role FIRST_TIME_CONTRIBUTOR" do
+          go(format: :html)
+
+          notes = assigns(:notes)
+          expect(notes).to match(a_collection_containing_exactly(an_object_having_attributes(special_role: Note::SpecialRole::FIRST_TIME_CONTRIBUTOR),
+                                                                 an_object_having_attributes(special_role: nil),
+                                                                 an_object_having_attributes(special_role: nil)
+                                                                ))
+        end
+      end
     end
 
     describe 'as json' do

spec/features/merge_requests/diff_notes_avatars_spec.rb

@@ -139,7 +139,7 @@ feature 'Diff note avatars', js: true do
         end
 
         page.within find("[id='#{position.line_code(project.repository)}']") do
-          find('.diff-notes-collapse').click
+          find('.diff-notes-collapse').trigger('click')
 
           expect(page).to have_selector('img.js-diff-comment-avatar', count: 2)
         end
@@ -152,7 +152,7 @@ feature 'Diff note avatars', js: true do
           page.within '.js-discussion-note-form' do
             find('.js-note-text').native.send_keys('Test')
 
-            find('.js-comment-button').trigger 'click'
+            find('.js-comment-button').trigger('click')
 
             wait_for_requests
           end

spec/helpers/notes_helper_spec.rb

@@ -23,10 +23,10 @@ describe NotesHelper do
   end
 
   describe "#notes_max_access_for_users" do
-    it 'returns human access levels' do
-      expect(helper.note_max_access_for_user(owner_note)).to eq('Owner')
-      expect(helper.note_max_access_for_user(master_note)).to eq('Master')
-      expect(helper.note_max_access_for_user(reporter_note)).to eq('Reporter')
+    it 'returns access levels' do
+      expect(helper.note_max_access_for_user(owner_note)).to eq(Gitlab::Access::OWNER)
+      expect(helper.note_max_access_for_user(master_note)).to eq(Gitlab::Access::MASTER)
+      expect(helper.note_max_access_for_user(reporter_note)).to eq(Gitlab::Access::REPORTER)
     end
 
     it 'handles access in different projects' do
@@ -34,8 +34,8 @@ describe NotesHelper do
       second_project.team << [master, :reporter]
       other_note = create(:note, author: master, project: second_project)
 
-      expect(helper.note_max_access_for_user(master_note)).to eq('Master')
-      expect(helper.note_max_access_for_user(other_note)).to eq('Reporter')
+      expect(helper.note_max_access_for_user(master_note)).to eq(Gitlab::Access::MASTER)
+      expect(helper.note_max_access_for_user(other_note)).to eq(Gitlab::Access::REPORTER)
     end
   end
 

spec/models/concerns/issuable_spec.rb

@@ -480,4 +480,71 @@ describe Issuable do
       end
     end
   end
+
+  describe '#first_contribution?' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project, namespace: group) }
+    let(:other_project) { create(:project) }
+    let(:owner) { create(:owner) }
+    let(:master) { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:guest) { create(:user) }
+
+    let(:contributor) { create(:user) }
+    let(:first_time_contributor) { create(:user) }
+
+    before do
+      group.add_owner(owner)
+      project.add_master(master)
+      project.add_reporter(reporter)
+      project.add_guest(guest)
+      project.add_guest(contributor)
+      project.add_guest(first_time_contributor)
+    end
+
+    let(:merged_mr) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
+    let(:open_mr)  { create(:merge_request, author: first_time_contributor, target_project: project, source_project: project) }
+    let(:merged_mr_other_project) { create(:merge_request, :merged, author: first_time_contributor, target_project: other_project, source_project: other_project) }
+
+    context "for merge requests" do
+      it "is false for MASTER" do
+        mr = create(:merge_request, author: master, target_project: project, source_project: project)
+
+        expect(mr).not_to be_first_contribution
+      end
+
+      it "is false for OWNER" do
+        mr = create(:merge_request, author: owner, target_project: project, source_project: project)
+
+        expect(mr).not_to be_first_contribution
+      end
+
+      it "is false for REPORTER" do
+        mr = create(:merge_request, author: reporter, target_project: project, source_project: project)
+
+        expect(mr).not_to be_first_contribution
+      end
+
+      it "is true when you don't have any merged MR" do
+        expect(open_mr).to be_first_contribution
+        expect(merged_mr).not_to be_first_contribution
+      end
+
+      it "handles multiple projects separately" do
+        expect(open_mr).to be_first_contribution
+        expect(merged_mr_other_project).not_to be_first_contribution
+      end
+    end
+
+    context "for issues" do
+      let(:contributor_issue) { create(:issue, author: contributor, project: project) }
+      let(:first_time_contributor_issue) { create(:issue, author: first_time_contributor, project: project) }
+
+      it "is false even without merged MR" do
+        expect(merged_mr).to be
+        expect(first_time_contributor_issue).not_to be_first_contribution
+        expect(contributor_issue).not_to be_first_contribution
+      end
+    end
+  end
 end