diff options
author | micael.bergeron <micael.bergeron@solutionstlm.com> | 2017-07-27 16:38:52 -0400 |
---|---|---|
committer | micael.bergeron <micael.bergeron@solutionstlm.com> | 2017-09-06 09:00:57 -0400 |
commit | 5e600db21ce5d6544f559ea6d25aab2858dd465e (patch) | |
tree | 9f1d2232c47efb7c2731bc820652b59cf2ebd1f4 | |
parent | b97f9629cabadca1125351a8aa514791524dea3f (diff) | |
download | gitlab-ce-5e600db21ce5d6544f559ea6d25aab2858dd465e.tar.gz |
fix #35161, add a first-time contributor badge
a new badge will be added when an user that doesn't yet
have any merged merge request is discussing on either issues or
merge requests that he created.
this is indented for people to use extra care when discussing with
a new contributor.
-rw-r--r-- | app/helpers/notes_helper.rb | 5 | ||||
-rw-r--r-- | app/views/projects/notes/_actions.html.haml | 3 | ||||
-rw-r--r-- | spec/helpers/notes_helper_spec.rb | 65 |
3 files changed, 73 insertions, 0 deletions
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 083ace65b09..1e7d346ab8f 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -76,6 +76,11 @@ module NotesHelper 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? + end + def discussion_path(discussion) if discussion.for_merge_request? return unless discussion.diff_discussion? diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml index fb07141d2ac..38548cbe93a 100644 --- a/app/views/projects/notes/_actions.html.haml +++ b/app/views/projects/notes/_actions.html.haml @@ -1,6 +1,9 @@ - access = note_max_access_for_user(note) - if access %span.note-role= access +- if note_first_contribution_for_user?(note) + %span.note-role.has-tooltip{ title: _("This user hasn't yet contributed code to this project. Handle with care.")} + = _('First-time contributor') - if note.resolvable? - can_resolve = can?(current_user, :resolve_note, note) diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 68540dd4e59..613d6d6a423 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -39,6 +39,71 @@ describe NotesHelper do end end + describe '#note_first_contribution_for_user?' do + let(:project) { create(:empty_project) } + let(:other_project) { create(:empty_project) } + + let(:contributor) { create(:user) } + let(:first_time_contributor) { create(:user) } + + # there should be a way to disable the lazy loading on these + 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 "notes for a merge request" do + let(:guest_merge_request) { create(:merge_request, author: guest, target_project: project, source_project: project) } + let(:contributor_note) { create(:note, noteable: merged_mr, author: contributor, project: merged_mr.project) } + let(:first_time_contributor_note) { create(:note, noteable: open_mr, author: first_time_contributor, project: open_mr.project) } + let(:first_time_contributor_note_other_project) { create(:note, noteable: merged_mr_other_project, author: first_time_contributor, project: merged_mr_other_project.project) } + let(:guest_note) { create(:note, noteable: guest_merge_request, author: first_time_contributor, project: project) } + + it "is true when you don't have any merged MR" do + expect(helper.note_first_contribution_for_user? contributor_note).to be false + expect(helper.note_first_contribution_for_user? first_time_contributor_note).to be true + end + + it "handle multiple projects separately" do + expect(helper.note_first_contribution_for_user? first_time_contributor_note).to be true + expect(helper.note_first_contribution_for_user? first_time_contributor_note_other_project).to be false + end + + it "should only apply to the noteable's author" do + expect(helper.note_first_contribution_for_user? guest_note).to be false + end + end + + context "notes for an issue" do + let(:guest_issue) { create(:issue, author: guest, project: project) } + 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) } + + let(:contributor_note) { create(:note, noteable: contributor_issue, author: contributor, project: project) } + let(:first_time_contributor_note) { create(:note, noteable: first_time_contributor_issue, author: first_time_contributor, project: project) } + let(:first_time_contributor_note_other_project) { create(:note, noteable: first_time_contributor_issue_other_project, author: first_time_contributor, project: other_project) } + let(:guest_note) { create(:note, noteable: guest_issue, author: first_time_contributor, project: project) } + + it "is true when you don't have any merged MR" do + expect(merged_mr).to be + expect(helper.note_first_contribution_for_user? contributor_note).to be false + expect(helper.note_first_contribution_for_user? first_time_contributor_note).to be true + end + + it "handle multiple projects separately" do + expect(merged_mr).to be + expect(merged_mr_other_project).to be + expect(helper.note_first_contribution_for_user? first_time_contributor_note).to be true + expect(helper.note_first_contribution_for_user? first_time_contributor_note_other_project).to be false + end + + it "should only apply to the noteable's author" do + expect(merged_mr).to be + expect(helper.note_first_contribution_for_user? guest_note).to be false + end + end + end + describe '#discussion_path' do let(:project) { create(:project, :repository) } |