diff options
-rw-r--r-- | app/assets/stylesheets/pages/notes.scss | 17 | ||||
-rw-r--r-- | app/controllers/concerns/renders_notes.rb | 11 | ||||
-rw-r--r-- | app/helpers/issuables_helper.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 9 | ||||
-rw-r--r-- | app/models/merge_request.rb | 5 | ||||
-rw-r--r-- | app/models/note.rb | 5 | ||||
-rw-r--r-- | app/views/projects/notes/_actions.html.haml | 8 | ||||
-rw-r--r-- | spec/models/concerns/issuable_spec.rb | 12 |
8 files changed, 41 insertions, 30 deletions
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 45f2aed1531..077e661a77e 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -620,15 +620,26 @@ 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; + + &.note-role-access { + top: -2px; + padding: 0 7px; + border: 1px solid $border-color; + border-radius: $label-border-radius; + } + + &.note-role-special { + top: -3px; + text-shadow: 0 0 15px white; + } } + /** * Line note button on the side of diffs */ diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index b6cf366c05c..4791bc561a4 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -2,7 +2,7 @@ module RendersNotes 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) + preload_first_time_contribution_for_authors(noteable, notes) Banzai::NoteRenderer.render(notes, @project, current_user) notes @@ -21,12 +21,9 @@ module RendersNotes 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? + def preload_first_time_contribution_for_authors(noteable, notes) + return unless noteable.is_a?(Issuable) && noteable.first_contribution? - same_author = lambda {|n| n.author_id == issuable.author_id} - notes.each do |note| - note.specialize!(Note::SpecialRole::FIRST_TIME_CONTRIBUTOR, &same_author) - end + notes.each {|n| n.specialize_for_first_contribution!(noteable)} end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 11fae16f04d..fbc1ba55d42 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -138,7 +138,7 @@ module IssuablesHelper end output << " ".html_safe - output << issuable_first_contribution_icon if issuable.first_contribution? + 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") @@ -176,7 +176,7 @@ module IssuablesHelper end def issuable_first_contribution_icon - content_tag(:span, class: 'fa-stack has-tooltip', title: _('1st contribution!')) do + 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 diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0cc60dddc69..765ccf540c3 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -335,9 +335,10 @@ module Issuable 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? + ## + # Override in issuable specialization + # + def first_contribution?(*) + false end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b82f49d7073..a5b037d94c2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -960,6 +960,11 @@ 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 diff --git a/app/models/note.rb b/app/models/note.rb index 28cd54bd81c..b1fe60aa387 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -236,6 +236,11 @@ class Note < ActiveRecord::Base 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) + end def editable? !system? diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml index cf634faccfc..22c81f76930 100644 --- a/app/views/projects/notes/_actions.html.haml +++ b/app/views/projects/notes/_actions.html.haml @@ -1,8 +1,8 @@ - if note.has_special_role?(Note::SpecialRole::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 = note_max_access_for_user(note) - %span.note-role= Gitlab::Access.human_access(access) + %span.note-role.note-role-special.has-tooltip{ title: _("This user hasn't yet contributed code 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) diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 9948340d976..dc959c58ba1 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -539,20 +539,12 @@ describe Issuable do 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) } - let(:first_time_contributor_issue_other_project) { create(:issue, author: first_time_contributor, project: other_project) } - it "is true when you don't have any merged MR" do + it "is false even without merged MR" do expect(merged_mr).to be - expect(first_time_contributor_issue).to be_first_contribution + expect(first_time_contributor_issue).not_to be_first_contribution expect(contributor_issue).not_to be_first_contribution end - - it "handles multiple projects separately" do - expect(merged_mr).to be - expect(merged_mr_other_project).to be - expect(first_time_contributor_issue).to be_first_contribution - expect(first_time_contributor_issue_other_project).not_to be_first_contribution - end end end end |