diff options
author | micael.bergeron <micael.bergeron@solutionstlm.com> | 2017-07-29 18:04:42 +0300 |
---|---|---|
committer | micael.bergeron <micael.bergeron@solutionstlm.com> | 2017-09-06 16:00:57 +0300 |
commit | 966b1128d884a318dad4277e23368334fe67e836 (patch) | |
tree | c0007ce9697dfe0fe93b52e619e21e8341c35120 /app | |
parent | 8fe1aa5dbbb41cdefffb7177d9eda44ac8652cc7 (diff) |
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
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/concerns/renders_notes.rb | 9 | ||||
-rw-r--r-- | app/controllers/projects/commit_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests/diffs_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 6 | ||||
-rw-r--r-- | app/controllers/projects/snippets_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/snippets_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/issuables_helper.rb | 9 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 7 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 5 | ||||
-rw-r--r-- | app/models/note.rb | 25 | ||||
-rw-r--r-- | app/models/project_team.rb | 2 | ||||
-rw-r--r-- | app/views/projects/notes/_actions.html.haml | 7 |
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 << " ".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) |