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
+              #   message_posted(message) => tmail object
-              #   Mailer.deliver_message_posted(message, recipients) => sends an email to the recipients
+              #   Mailer.deliver_message_posted(message) => sends an email to the recipients
-              def message_posted(message, 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

@@ -17,14 +17,6
17		17
18	class MessageObserver < ActiveRecord::Observer	18	class MessageObserver < ActiveRecord::Observer
19	def after_create(message)	19	def after_create(message)
20	recipients = []	20	Mailer.deliver_message_posted(message) if Setting.notified_events.include?('message_posted')
21	# send notification to the topic watchers
22	recipients += message.root.watcher_recipients
23	# send notification to the board watchers
24	recipients += message.board.watcher_recipients
25	# send notification to project members who want to be notified
26	recipients += message.board.project.recipients
27	recipients = recipients.compact.uniq
28	Mailer.deliver_message_posted(message, recipients) if !recipients.empty? && Setting.notified_events.include?('message_posted')
29	end	21	end
30	end	22	end

app/models/news.rb +11 0

@@ -29,6 +29,17 class News < ActiveRecord::Base
29	acts_as_activity_provider :find_options => {:include => [:project, :author]},	29	acts_as_activity_provider :find_options => {:include => [:project, :author]},
30	:author_key => :author_id	30	:author_key => :author_id
31		31
		32	def visible?(user=User.current)
		33	!user.nil? && user.allowed_to?(:view_news, project)
		34	end
		35
		36	# Returns the mail adresses of users that should be notified
		37	def recipients
		38	notified = project.notified_users
		39	notified.reject! {\|user\| !visible?(user)}
		40	notified.collect(&:mail)
		41	end
		42
32	# returns latest news for projects visible by user	43	# returns latest news for projects visible by user
33	def self.latest(user = User.current, count = 5)	44	def self.latest(user = User.current, count = 5)
34	find(:all, :limit => count, :conditions => Project.allowed_to_condition(user, :view_news), :include => [ :author, :project ], :order => "#{News.table_name}.created_on DESC")	45	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

               validates_length_of :comments, :maximum => 255, :allow_nil => true
               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

@@ -147,7 +147,7 class MailerTest < ActiveSupport::TestCase
147	def test_message_posted_message_id	147	def test_message_posted_message_id
148	ActionMailer::Base.deliveries.clear	148	ActionMailer::Base.deliveries.clear
149	message = Message.find(1)	149	message = Message.find(1)
150	Mailer.deliver_message_posted(message, ~~message~~.~~author~~.~~mail~~)	150	Mailer.deliver_message_posted(message)
151	mail = ActionMailer::Base.deliveries.last	151	mail = ActionMailer::Base.deliveries.last
152	assert_not_nil mail	152	assert_not_nil mail
153	assert_equal Mailer.message_id_for(message), mail.message_id	153	assert_equal Mailer.message_id_for(message), mail.message_id
@@ -157,13 +157,47 class MailerTest < ActiveSupport::TestCase
157	def test_reply_posted_message_id	157	def test_reply_posted_message_id
158	ActionMailer::Base.deliveries.clear	158	ActionMailer::Base.deliveries.clear
159	message = Message.find(3)	159	message = Message.find(3)
160	Mailer.deliver_message_posted(message, ~~message~~.~~author~~.~~mail~~)	160	Mailer.deliver_message_posted(message)
161	mail = ActionMailer::Base.deliveries.last	161	mail = ActionMailer::Base.deliveries.last
162	assert_not_nil mail	162	assert_not_nil mail
163	assert_equal Mailer.message_id_for(message), mail.message_id	163	assert_equal Mailer.message_id_for(message), mail.message_id
164	assert_equal Mailer.message_id_for(message.parent), mail.references.first.to_s	164	assert_equal Mailer.message_id_for(message.parent), mail.references.first.to_s
165	end	165	end
166		166
		167	context("#issue_add") do
		168	setup do
		169	ActionMailer::Base.deliveries.clear
		170	Setting.bcc_recipients = '1'
		171	@issue = Issue.find(1)
		172	end
		173
		174	should "notify project members" do
		175	assert Mailer.deliver_issue_add(@issue)
		176	assert last_email.bcc.include?('dlopper@somenet.foo')
		177	end
		178
		179	should "not notify project members that are not allow to view the issue" do
		180	Role.find(2).remove_permission!(:view_issues)
		181	assert Mailer.deliver_issue_add(@issue)
		182	assert !last_email.bcc.include?('dlopper@somenet.foo')
		183	end
		184
		185	should "notify issue watchers" do
		186	user = User.find(9)
		187	Watcher.create!(:watchable => @issue, :user => user)
		188	assert Mailer.deliver_issue_add(@issue)
		189	assert last_email.bcc.include?(user.mail)
		190	end
		191
		192	should "not notify watchers not allowed to view the issue" do
		193	user = User.find(9)
		194	Watcher.create!(:watchable => @issue, :user => user)
		195	Role.non_member.remove_permission!(:view_issues)
		196	assert Mailer.deliver_issue_add(@issue)
		197	assert !last_email.bcc.include?(user.mail)
		198	end
		199	end
		200
167	# test mailer methods for each language	201	# test mailer methods for each language
168	def test_issue_add	202	def test_issue_add
169	issue = Issue.find(1)	203	issue = Issue.find(1)
@@ -211,7 +245,7 class MailerTest < ActiveSupport::TestCase
211	recipients = recipients.compact.uniq	245	recipients = recipients.compact.uniq
212	valid_languages.each do \|lang\|	246	valid_languages.each do \|lang\|
213	Setting.default_language = lang.to_s	247	Setting.default_language = lang.to_s
214	assert Mailer.deliver_message_posted(message, ~~recipients~~)	248	assert Mailer.deliver_message_posted(message)
215	end	249	end
216	end	250	end
217		251
@@ -256,4 +290,10 class MailerTest < ActiveSupport::TestCase
256	assert mail.bcc.include?('dlopper@somenet.foo')	290	assert mail.bcc.include?('dlopper@somenet.foo')
257	assert mail.body.include?('Bug #3: Error 281 when updating a recipe')	291	assert mail.body.include?('Bug #3: Error 281 when updating a recipe')
258	end	292	end
		293
		294	def last_email
		295	mail = ActionMailer::Base.deliveries.last
		296	assert_not_nil mail
		297	mail
		298	end
259	end	299	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