Commit 20ed39e781fc49bd4ec6e04e862ebd4ece53a15b
Exists in
master
and in
4 other branches
Merge pull request #3898 from jacargentina/notifymentioned
Notify email to mentioned users; show participants ala github on issuables
Showing
8 changed files
with
81 additions
and
8 deletions
Show diff stats
app/assets/stylesheets/sections/issues.scss
app/helpers/projects_helper.rb
| @@ -17,7 +17,7 @@ module ProjectsHelper | @@ -17,7 +17,7 @@ module ProjectsHelper | ||
| 17 | end | 17 | end |
| 18 | 18 | ||
| 19 | def link_to_member(project, author, opts = {}) | 19 | def link_to_member(project, author, opts = {}) |
| 20 | - default_opts = { avatar: true } | 20 | + default_opts = { avatar: true, name: true, size: 16 } |
| 21 | opts = default_opts.merge(opts) | 21 | opts = default_opts.merge(opts) |
| 22 | 22 | ||
| 23 | return "(deleted)" unless author | 23 | return "(deleted)" unless author |
| @@ -25,10 +25,10 @@ module ProjectsHelper | @@ -25,10 +25,10 @@ module ProjectsHelper | ||
| 25 | author_html = "" | 25 | author_html = "" |
| 26 | 26 | ||
| 27 | # Build avatar image tag | 27 | # Build avatar image tag |
| 28 | - author_html << image_tag(gravatar_icon(author.try(:email)), width: 16, class: "avatar avatar-inline s16") if opts[:avatar] | 28 | + author_html << image_tag(gravatar_icon(author.try(:email), opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}") if opts[:avatar] |
| 29 | 29 | ||
| 30 | # Build name span tag | 30 | # Build name span tag |
| 31 | - author_html << content_tag(:span, sanitize(author.name), class: 'author') | 31 | + author_html << content_tag(:span, sanitize(author.name), class: 'author') if opts[:name] |
| 32 | 32 | ||
| 33 | author_html = author_html.html_safe | 33 | author_html = author_html.html_safe |
| 34 | 34 |
app/models/concerns/issuable.rb
| @@ -6,6 +6,7 @@ | @@ -6,6 +6,7 @@ | ||
| 6 | # | 6 | # |
| 7 | module Issuable | 7 | module Issuable |
| 8 | extend ActiveSupport::Concern | 8 | extend ActiveSupport::Concern |
| 9 | + include Mentionable | ||
| 9 | 10 | ||
| 10 | included do | 11 | included do |
| 11 | belongs_to :project | 12 | belongs_to :project |
| @@ -97,4 +98,18 @@ module Issuable | @@ -97,4 +98,18 @@ module Issuable | ||
| 97 | def votes_count | 98 | def votes_count |
| 98 | upvotes + downvotes | 99 | upvotes + downvotes |
| 99 | end | 100 | end |
| 101 | + | ||
| 102 | + # Return all users participating on the discussion | ||
| 103 | + def participants | ||
| 104 | + users = [] | ||
| 105 | + users << author | ||
| 106 | + users << assignee if is_assigned? | ||
| 107 | + mentions = [] | ||
| 108 | + mentions << self.mentioned_users | ||
| 109 | + notes.each do |note| | ||
| 110 | + users << note.author | ||
| 111 | + mentions << note.mentioned_users | ||
| 112 | + end | ||
| 113 | + users.concat(mentions.reduce([], :|)).uniq | ||
| 114 | + end | ||
| 100 | end | 115 | end |
| @@ -0,0 +1,37 @@ | @@ -0,0 +1,37 @@ | ||
| 1 | +# == Mentionable concern | ||
| 2 | +# | ||
| 3 | +# Contains common functionality shared between Issues and Notes | ||
| 4 | +# | ||
| 5 | +# Used by Issue, Note | ||
| 6 | +# | ||
| 7 | +module Mentionable | ||
| 8 | + extend ActiveSupport::Concern | ||
| 9 | + | ||
| 10 | + def mentioned_users | ||
| 11 | + users = [] | ||
| 12 | + return users if mentionable_text.blank? | ||
| 13 | + has_project = self.respond_to? :project | ||
| 14 | + matches = mentionable_text.scan(/@[a-zA-Z][a-zA-Z0-9_\-\.]*/) | ||
| 15 | + matches.each do |match| | ||
| 16 | + identifier = match.delete "@" | ||
| 17 | + if has_project | ||
| 18 | + id = project.users_projects.joins(:user).where(users: { username: identifier }).pluck(:user_id).first | ||
| 19 | + else | ||
| 20 | + id = User.where(username: identifier).pluck(:id).first | ||
| 21 | + end | ||
| 22 | + users << User.find(id) unless id.blank? | ||
| 23 | + end | ||
| 24 | + users.uniq | ||
| 25 | + end | ||
| 26 | + | ||
| 27 | + def mentionable_text | ||
| 28 | + if self.class == Issue | ||
| 29 | + description | ||
| 30 | + elsif self.class == Note | ||
| 31 | + note | ||
| 32 | + else | ||
| 33 | + nil | ||
| 34 | + end | ||
| 35 | + end | ||
| 36 | + | ||
| 37 | +end |
app/models/note.rb
| @@ -19,6 +19,8 @@ require 'carrierwave/orm/activerecord' | @@ -19,6 +19,8 @@ require 'carrierwave/orm/activerecord' | ||
| 19 | require 'file_size_validator' | 19 | require 'file_size_validator' |
| 20 | 20 | ||
| 21 | class Note < ActiveRecord::Base | 21 | class Note < ActiveRecord::Base |
| 22 | + include Mentionable | ||
| 23 | + | ||
| 22 | attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, | 24 | attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, |
| 23 | :attachment, :line_code, :commit_id | 25 | :attachment, :line_code, :commit_id |
| 24 | 26 |
app/services/notification_service.rb
| @@ -110,9 +110,11 @@ class NotificationService | @@ -110,9 +110,11 @@ class NotificationService | ||
| 110 | else | 110 | else |
| 111 | opts.merge!(noteable_id: note.noteable_id) | 111 | opts.merge!(noteable_id: note.noteable_id) |
| 112 | target = note.noteable | 112 | target = note.noteable |
| 113 | - recipients = [] | ||
| 114 | - recipients << target.assignee if target.respond_to?(:assignee) | ||
| 115 | - recipients << target.author if target.respond_to?(:author) | 113 | + if target.respond_to?(:participants) |
| 114 | + recipients = target.participants | ||
| 115 | + else | ||
| 116 | + recipients = [] | ||
| 117 | + end | ||
| 116 | end | 118 | end |
| 117 | 119 | ||
| 118 | # Get users who left comment in thread | 120 | # Get users who left comment in thread |
| @@ -181,7 +183,12 @@ class NotificationService | @@ -181,7 +183,12 @@ class NotificationService | ||
| 181 | end | 183 | end |
| 182 | 184 | ||
| 183 | def new_resource_email(target, method) | 185 | def new_resource_email(target, method) |
| 184 | - recipients = reject_muted_users([target.assignee], target.project) | 186 | + if target.respond_to?(:participants) |
| 187 | + recipients = target.participants | ||
| 188 | + else | ||
| 189 | + recipients = [] | ||
| 190 | + end | ||
| 191 | + recipients = reject_muted_users(recipients, target.project) | ||
| 185 | recipients = recipients.concat(project_watchers(target.project)).uniq | 192 | recipients = recipients.concat(project_watchers(target.project)).uniq |
| 186 | recipients.delete(target.author) | 193 | recipients.delete(target.author) |
| 187 | 194 |
app/views/issues/show.html.haml
| @@ -65,4 +65,9 @@ | @@ -65,4 +65,9 @@ | ||
| 65 | - else | 65 | - else |
| 66 | = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" | 66 | = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" |
| 67 | 67 | ||
| 68 | +.participants | ||
| 69 | + %cite.cgray #{@issue.participants.count} participants | ||
| 70 | + - @issue.participants.each do |participant| | ||
| 71 | + = link_to_member(@project, participant, name: false, size: 24) | ||
| 72 | + | ||
| 68 | .voting_notes#notes= render "notes/notes_with_form" | 73 | .voting_notes#notes= render "notes/notes_with_form" |
spec/services/notification_service_spec.rb
| @@ -19,7 +19,7 @@ describe NotificationService do | @@ -19,7 +19,7 @@ describe NotificationService do | ||
| 19 | describe 'Notes' do | 19 | describe 'Notes' do |
| 20 | context 'issue note' do | 20 | context 'issue note' do |
| 21 | let(:issue) { create(:issue, assignee: create(:user)) } | 21 | let(:issue) { create(:issue, assignee: create(:user)) } |
| 22 | - let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id) } | 22 | + let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced') } |
| 23 | 23 | ||
| 24 | before do | 24 | before do |
| 25 | build_team(note.project) | 25 | build_team(note.project) |
| @@ -30,6 +30,7 @@ describe NotificationService do | @@ -30,6 +30,7 @@ describe NotificationService do | ||
| 30 | should_email(@u_watcher.id) | 30 | should_email(@u_watcher.id) |
| 31 | should_email(note.noteable.author_id) | 31 | should_email(note.noteable.author_id) |
| 32 | should_email(note.noteable.assignee_id) | 32 | should_email(note.noteable.assignee_id) |
| 33 | + should_email(@u_mentioned.id) | ||
| 33 | should_not_email(note.author_id) | 34 | should_not_email(note.author_id) |
| 34 | should_not_email(@u_participating.id) | 35 | should_not_email(@u_participating.id) |
| 35 | should_not_email(@u_disabled.id) | 36 | should_not_email(@u_disabled.id) |
| @@ -235,9 +236,11 @@ describe NotificationService do | @@ -235,9 +236,11 @@ describe NotificationService do | ||
| 235 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) | 236 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) |
| 236 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) | 237 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) |
| 237 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) | 238 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) |
| 239 | + @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) | ||
| 238 | 240 | ||
| 239 | project.team << [@u_watcher, :master] | 241 | project.team << [@u_watcher, :master] |
| 240 | project.team << [@u_participating, :master] | 242 | project.team << [@u_participating, :master] |
| 241 | project.team << [@u_disabled, :master] | 243 | project.team << [@u_disabled, :master] |
| 244 | + project.team << [@u_mentioned, :master] | ||
| 242 | end | 245 | end |
| 243 | end | 246 | end |