From 959ebbcade63423048320daaa814634c41842a36 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 15 Apr 2015 15:56:21 +0200 Subject: [PATCH] Clean up code around commit mentions. --- app/controllers/projects/commit_controller.rb | 6 +++--- app/models/commit.rb | 19 +++++++++++++++++++ app/models/concerns/mentionable.rb | 4 ++-- app/models/note.rb | 8 -------- app/services/notification_service.rb | 11 +++-------- app/services/projects/participants_service.rb | 4 ++-- app/views/projects/commits/_commit.html.haml | 2 +- spec/services/notification_service_spec.rb | 2 +- 8 files changed, 31 insertions(+), 25 deletions(-) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 87e39f1363a..894cf93b9ae 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -10,11 +10,11 @@ class Projects::CommitController < Projects::ApplicationController def show return git_not_found! unless @commit - @line_notes = @project.notes.for_commit_id(commit.id).inline + @line_notes = commit.notes(@project).inline @diffs = @commit.diffs @note = @project.build_commit_note(commit) - @notes_count = @project.notes.for_commit_id(commit.id).count - @notes = @project.notes.for_commit_id(@commit.id).not_inline.fresh + @notes_count = commit.notes(@project).count + @notes = commit.notes(@project).not_inline.fresh @noteable = @commit @comments_allowed = @reply_allowed = true @comments_target = { diff --git a/app/models/commit.rb b/app/models/commit.rb index 006fa62c8f9..f54696c378c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -134,6 +134,25 @@ class Commit User.find_for_commit(committer_email, committer_name) end + def participants(project, current_user = nil) + users = [] + users << author + users << committer + mentions = [] + mentions << self.mentioned_users(current_user, project) + + notes(project).each do |note| + users << note.author + mentions << note.mentioned_users(current_user, project) + end + + users.concat(mentions.reduce([], :|)).uniq + end + + def notes(project) + project.notes.for_commit_id(self.id) + end + def method_missing(m, *args, &block) @raw.send(m, *args, &block) end diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b7882a2bb16..acd9a1edc48 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -42,10 +42,10 @@ module Mentionable Note.cross_reference_exists?(target, local_reference) end - def mentioned_users(current_user = nil) + def mentioned_users(current_user = nil, p = project) return [] if mentionable_text.blank? - ext = Gitlab::ReferenceExtractor.new(self.project, current_user) + ext = Gitlab::ReferenceExtractor.new(p, current_user) ext.analyze(mentionable_text) ext.users.uniq end diff --git a/app/models/note.rb b/app/models/note.rb index fdab4517df3..8ebeb371352 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -327,14 +327,6 @@ class Note < ActiveRecord::Base current_application_settings.max_attachment_size.megabytes.to_i end - def commit_author - @commit_author ||= - project.team.users.find_by(email: noteable.author_email) || - project.team.users.find_by(name: noteable.author_name) - rescue - nil - end - def cross_reference? note.start_with?(self.class.cross_reference_note_prefix) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index cfed7964c37..1337dca8354 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -127,17 +127,12 @@ class NotificationService recipients = [] - if note.commit_id.present? - recipients << note.commit_author - end - # Add all users participating in the thread (author, assignee, comment authors) participants = - if target.respond_to?(:participants) + if target.is_a?(Commit) + target.participants(note.project) + elsif target.respond_to?(:participants) target.participants - elsif target.is_a?(Commit) - author_ids = Note.for_commit_id(target.id).pluck(:author_id).uniq - User.where(id: author_ids) else note.mentioned_users end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index ae6260bcdab..dcbf348153d 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -21,8 +21,8 @@ module Projects merge_request = project.merge_requests.find_by_iid(id) merge_request ? merge_request.participants(current_user) : [] when "Commit" - author_ids = Note.for_commit_id(id).pluck(:author_id).uniq - User.where(id: author_ids) + commit = project.repository.commit(id) + commit ? commit.participants(project, current_user) : [] else [] end diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index c6026f96804..8e1aaa4d051 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -12,7 +12,7 @@ - if @note_counts - note_count = @note_counts.fetch(commit.id, 0) - else - - notes = project.notes.for_commit_id(commit.id) + - notes = commit.notes(project) - note_count = notes.user.count - if note_count > 0 diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index bfca2c88264..0faff48c059 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -149,7 +149,7 @@ describe NotificationService do before do build_team(note.project) - note.stub(:commit_author => @u_committer) + allow_any_instance_of(Commit).to receive(:author).and_return() end describe :new_note do -- GitLab