add controller spec

also fix some code styling issues

app/models/concerns/issuable.rb

@@ -338,7 +338,7 @@ module Issuable
   ##
   # Override in issuable specialization
   #
-  def first_contribution?(*)
+  def first_contribution?
     false
   end
 end

app/models/merge_request.rb

@@ -960,8 +960,9 @@ class MergeRequest < ActiveRecord::Base
     Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache
   end
 
-  def first_contribution?(*)
+  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
 

app/models/note.rb

@@ -47,7 +47,7 @@ class Note < ActiveRecord::Base
 
   # 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
@@ -233,15 +233,12 @@ class Note < ActiveRecord::Base
     self.class.has_special_role?(role, self)
   end
 
-  def specialize!(role)
-    self.special_role = role if !block_given? || yield(self)
-  end
-
   def specialize_for_first_contribution!(noteable)
     return unless noteable.author_id == self.author_id
-    specialize!(Note::SpecialRole::FIRST_TIME_CONTRIBUTOR)
+
+    self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
   end
- 
+
   def editable?
     !system?
   end

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

 - if note.has_special_role?(Note::SpecialRole::FIRST_TIME_CONTRIBUTOR)
-  %span.note-role.note-role-special.has-tooltip{ title: _("This user hasn't yet contributed code to this project. Handle with care.") }
+  %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)

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/models/concerns/issuable_spec.rb

@@ -501,7 +501,7 @@ describe Issuable do
       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) }
@@ -515,13 +515,13 @@ describe Issuable do
 
       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
 

spec/models/note_spec.rb

@@ -696,25 +696,4 @@ describe Note do
       note.destroy!
     end
   end
-
-  describe '#specialize!' do
-    let(:note) { build(:note) }
-    let(:role) { Note::SpecialRole::FIRST_TIME_CONTRIBUTOR }
-
-    it 'should set special role without a block' do
-      expect { note.specialize!(role) }.to change { note.special_role }.from(nil).to(role)
-    end
-
-    it 'should set special role with a truthy block' do
-      tautology = -> (*) { true }
-      
-      expect { note.specialize!(role, &tautology) }.to change { note.special_role }.from(nil).to(role)
-    end
-
-    it 'should not set special role with a falsey block' do
-      contradiction = -> (*) { false }
-
-      expect { note.specialize!(role, &contradiction) }.not_to change { note.special_role }
-    end
-  end
 end