jro_apps/redmine Commit - r3055:bb477a3a0fe7

Make sure users don't get notified for thing they can not view (#3589)....

Jean-Philippe Lang -

r3055:bb477a3a0fe7

parent child

Context file:

r3055:bb477a3a0fe7

Collapse all files

app/models/document.rb +11 0

                validates_presence_of :project, :title, :category
                validates_length_of :title, :maximum => 60
+               def visible?(user=User.current)
+                 !user.nil? && user.allowed_to?(:view_documents, project)
+               end
                def after_initialize
                  if new_record?
                    self.category ||= DocumentCategory.default
                  end
                  @updated_on
                end
+               # Returns the mail adresses of users that should be notified
+               def recipients
+                 notified = project.notified_users
+                 notified.reject! {|user| !visible?(user)}
+                 notified.collect(&:mail)
+               end
              end

app/models/mailer.rb +13 -10

                #   Mailer.deliver_document_added(document) => sends an email to the document's project recipients
                def document_added(document)
                  redmine_headers 'Project' => document.project.identifier
-                 recipients document.project.recipients
+                 recipients document.recipients
                  subject "[#{document.project.name}] #{l(:label_document_new)}: #{document.title}"
                  body :document => document,
                       :document_url => url_for(:controller => 'documents', :action => 'show', :id => document)
                  when 'Project'
                    added_to_url = url_for(:controller => 'projects', :action => 'list_files', :id => container)
                    added_to = "#{l(:label_project)}: #{container}"
+                   recipients container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)}
                  when 'Version'
                    added_to_url = url_for(:controller => 'projects', :action => 'list_files', :id => container.project_id)
                    added_to = "#{l(:label_version)}: #{container.name}"
+                   recipients container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)}
                  when 'Document'
                    added_to_url = url_for(:controller => 'documents', :action => 'show', :id => container.id)
                    added_to = "#{l(:label_document)}: #{container.title}"
+                   recipients container.recipients
                  end
                  redmine_headers 'Project' => container.project.identifier
-                 recipients container.project.recipients
                  subject "[#{container.project.name}] #{l(:label_attachment_new)}"
                  body :attachments => attachments,
                       :added_to => added_to,
                def news_added(news)
                  redmine_headers 'Project' => news.project.identifier
                  message_id news
-                 recipients news.project.recipients
+                 recipients news.recipients
                  subject "[#{news.project.name}] #{l(:label_news)}: #{news.title}"
                  body :news => news,
                       :news_url => url_for(:controller => 'news', :action => 'show', :id => news)
                  render_multipart('news_added', body)
                end
-               # Builds a tmail object used to email the specified recipients of the specified message that was posted.
+               # Builds a tmail object used to email the recipients of the specified message that was posted.
                #
                # Example:
-               #   message_posted(message, recipients) => tmail object
-               #   Mailer.deliver_message_posted(message, recipients) => sends an email to the recipients
-               def message_posted(message, recipients)
+               #   message_posted(message) => tmail object
+               #   Mailer.deliver_message_posted(message) => sends an email to the recipients
+               def message_posted(message)
                  redmine_headers 'Project' => message.project.identifier,
                                  'Topic-Id' => (message.parent_id || message.id)
                  message_id message
                  references message.parent unless message.parent.nil?
-                 recipients(recipients)
+                 recipients(message.recipients)
+                 cc((message.root.watcher_recipients + message.board.watcher_recipients).uniq - @recipients)
                  subject "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] #{message.subject}"
                  body :message => message,
                       :message_url => url_for(:controller => 'messages', :action => 'show', :board_id => message.board_id, :id => message.root)
                  redmine_headers 'Project' => wiki_content.project.identifier,
                                  'Wiki-Page-Id' => wiki_content.page.id
                  message_id wiki_content
-                 recipients wiki_content.project.recipients
+                 recipients wiki_content.recipients
                  cc(wiki_content.page.wiki.watcher_recipients - recipients)
                  subject "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_added, :page => wiki_content.page.pretty_title)}"
                  body :wiki_content => wiki_content,
                  redmine_headers 'Project' => wiki_content.project.identifier,
                                  'Wiki-Page-Id' => wiki_content.page.id
                  message_id wiki_content
-                 recipients wiki_content.project.recipients
+                 recipients wiki_content.recipients
                  cc(wiki_content.page.wiki.watcher_recipients + wiki_content.page.watcher_recipients - recipients)
                  subject "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_updated, :page => wiki_content.page.pretty_title)}"
                  body :wiki_content => wiki_content,

app/models/message.rb +7 0

                  usr && usr.logged? && (usr.allowed_to?(:delete_messages, project) || (self.author == usr && usr.allowed_to?(:delete_own_messages, project)))
                end
+               # Returns the mail adresses of users that should be notified
+               def recipients
+                 notified = project.notified_users
+                 notified.reject! {|user| !visible?(user)}
+                 notified.collect(&:mail)
+               end
                private
                def add_author_as_watcher

app/models/message_observer.rb +1 -9

              class MessageObserver < ActiveRecord::Observer
                def after_create(message)
-                 recipients = []
-                 # send notification to the topic watchers
-                 recipients += message.root.watcher_recipients
-                 # send notification to the board watchers
-                 recipients += message.board.watcher_recipients
-                 # send notification to project members who want to be notified
-                 recipients += message.board.project.recipients
-                 recipients = recipients.compact.uniq
-                 Mailer.deliver_message_posted(message, recipients) if !recipients.empty? && Setting.notified_events.include?('message_posted')
+                 Mailer.deliver_message_posted(message) if Setting.notified_events.include?('message_posted')
                end
              end

app/models/news.rb +11 0

                acts_as_activity_provider :find_options => {:include => [:project, :author]},
                                          :author_key => :author_id
+               def visible?(user=User.current)
+                 !user.nil? && user.allowed_to?(:view_news, project)
+               end
+               # Returns the mail adresses of users that should be notified
+               def recipients
+                 notified = project.notified_users
+                 notified.reject! {|user| !visible?(user)}
+                 notified.collect(&:mail)
+               end
                # returns latest news for projects visible by user
                def self.latest(user = User.current, count = 5)
                  find(:all, :limit => count, :conditions => Project.allowed_to_condition(user, :view_news), :include => [ :author, :project ], :order => "#{News.table_name}.created_on DESC")

app/models/wiki_content.rb +11 0

                acts_as_versioned
+               def visible?(user=User.current)
+                 page.visible?(user)
+               end
                def project
                  page.project
                end
+               # Returns the mail adresses of users that should be notified
+               def recipients
+                 notified = project.notified_users
+                 notified.reject! {|user| !visible?(user)}
+                 notified.collect(&:mail)
+               end
                class Version
                  belongs_to :page, :class_name => '::WikiPage', :foreign_key => 'page_id'
                  belongs_to :author, :class_name => '::User', :foreign_key => 'author_id'

test/unit/mailer_test.rb +43 -3

                def test_message_posted_message_id
                  ActionMailer::Base.deliveries.clear
                  message = Message.find(1)
-                 Mailer.deliver_message_posted(message, message.author.mail)
+                 Mailer.deliver_message_posted(message)
                  mail = ActionMailer::Base.deliveries.last
                  assert_not_nil mail
                  assert_equal Mailer.message_id_for(message), mail.message_id
                def test_reply_posted_message_id
                  ActionMailer::Base.deliveries.clear
                  message = Message.find(3)
-                 Mailer.deliver_message_posted(message, message.author.mail)
+                 Mailer.deliver_message_posted(message)
                  mail = ActionMailer::Base.deliveries.last
                  assert_not_nil mail
                  assert_equal Mailer.message_id_for(message), mail.message_id
                  assert_equal Mailer.message_id_for(message.parent), mail.references.first.to_s
                end
+               context("#issue_add") do
+                 setup do
+                   ActionMailer::Base.deliveries.clear
+                   Setting.bcc_recipients = '1'
+                   @issue = Issue.find(1)
+                 end
+                 should "notify project members" do
+                   assert Mailer.deliver_issue_add(@issue)
+                   assert last_email.bcc.include?('dlopper@somenet.foo')
+                 end
+                 should "not notify project members that are not allow to view the issue" do
+                   Role.find(2).remove_permission!(:view_issues)
+                   assert Mailer.deliver_issue_add(@issue)
+                   assert !last_email.bcc.include?('dlopper@somenet.foo')
+                 end
+                 should "notify issue watchers" do
+                   user = User.find(9)
+                   Watcher.create!(:watchable => @issue, :user => user)
+                   assert Mailer.deliver_issue_add(@issue)
+                   assert last_email.bcc.include?(user.mail)
+                 end
+                 should "not notify watchers not allowed to view the issue" do
+                   user = User.find(9)
+                   Watcher.create!(:watchable => @issue, :user => user)
+                   Role.non_member.remove_permission!(:view_issues)
+                   assert Mailer.deliver_issue_add(@issue)
+                   assert !last_email.bcc.include?(user.mail)
+                 end
+               end
                # test mailer methods for each language
                def test_issue_add
                  issue = Issue.find(1)
                  recipients = recipients.compact.uniq
                  valid_languages.each do |lang|
                    Setting.default_language = lang.to_s
-                   assert Mailer.deliver_message_posted(message, recipients)
+                   assert Mailer.deliver_message_posted(message)
                  end
                end
                  assert mail.bcc.include?('dlopper@somenet.foo')
                  assert mail.body.include?('Bug #3: Error 281 when updating a recipe')
                end
+               def last_email
+                 mail = ActionMailer::Base.deliveries.last
+                 assert_not_nil mail
+                 mail
+               end
              end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages