From: Jean-Philippe Lang Date: Sat, 6 Oct 2018 13:08:52 +0000 (+0000) Subject: Send individual emails for each mail recipient (#26791). X-Git-Tag: 4.0.0~100 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=015ca366341ba2fa9f1d7903907ddab19a87865f;p=redmine.git Send individual emails for each mail recipient (#26791). We are creating multipe mails per class notification event, one per recipient, wrapped in a Mailer::MultiMessage object to send them all at once. We keep the existing interface of all class methods intended to be used by external code the same as they were before, with one exception: We provide additional recipient addresses in options[:recipients] for Mailer.security_notification. Since the first-class recipients have to be users to render individual mails for them, additional recipient addresses have to be provided with some other channel. By providing additional recipients in options[:recipients], we can solve the use-case for address change notifications for users, which probably is the only real use-case for having to use a plain email address instead of a User as a notification recipient. Patch by Holger Just and Marius BALTEANU. git-svn-id: http://svn.redmine.org/redmine/trunk@17583 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/models/email_address.rb b/app/models/email_address.rb index d144e2f88..ffd5d6b4f 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -52,7 +52,7 @@ class EmailAddress < ActiveRecord::Base # in that case, the user is just being created and # should not receive this email. if user.mails != [address] - deliver_security_notification(user, + deliver_security_notification( message: :mail_body_security_notification_add, field: :field_mail, value: address @@ -63,25 +63,26 @@ class EmailAddress < ActiveRecord::Base # send a security notification to user that an email has been changed (notified/not notified) def deliver_security_notification_update if saved_change_to_address? - recipients = [user, address_before_last_save] options = { + recipients: [address_before_last_save], message: :mail_body_security_notification_change_to, field: :field_mail, value: address } elsif saved_change_to_notify? - recipients = [user, address] options = { + recipients: [address], message: notify_before_last_save ? :mail_body_security_notification_notify_disabled : :mail_body_security_notification_notify_enabled, value: address } end - deliver_security_notification(recipients, options) + deliver_security_notification(options) end # send a security notification to user that an email address was deleted def deliver_security_notification_destroy - deliver_security_notification([user, address], + deliver_security_notification( + recipients: [address], message: :mail_body_security_notification_remove, field: :field_mail, value: address @@ -89,8 +90,8 @@ class EmailAddress < ActiveRecord::Base end # generic method to send security notifications for email addresses - def deliver_security_notification(recipients, options={}) - Mailer.security_notification(recipients, + def deliver_security_notification(options={}) + Mailer.security_notification(user, options.merge( title: :label_my_account, url: {controller: 'my', action: 'account'} diff --git a/app/models/mailer.rb b/app/models/mailer.rb index 466ebe467..4427d333d 100644 --- a/app/models/mailer.rb +++ b/app/models/mailer.rb @@ -26,6 +26,113 @@ class Mailer < ActionMailer::Base include Redmine::I18n include Roadie::Rails::Automatic + # This class wraps multiple generated `Mail::Message` objects and allows to + # deliver them all at once. It is usually used to handle multiple mails for + # different receivers created by a single mail event. The wrapped mails can + # then be delivered in one go. + # + # The public interface of the class resembles a single mail message. You can + # directly use any of the deliver_* methods to send the contained messages + # now or later. + class MultiMessage + attr_reader :mails + + # @param mails [Array] an Array of mails or Procs which create + # mail objects and allow to call a method on it. + def initialize(action, *args) + @action = action + @args = args + + @mails = [] + end + + def for(users) + Array.wrap(users).each do |user| + @mails << ActionMailer::MessageDelivery.new(Mailer, @action, user, *@args) + end + self + end + + def deliver_later(options = {}) + enqueue_delivery :deliver_now, options + end + + def deliver_later!(options = {}) + enqueue_delivery :deliver_now!, options + end + + def processed? + @mails.any?(&:processed?) + end + + # @return [Object] the delivery method of the first mail. + # Usually, this is the very same value for all mails and matches the + # default value of the Mailer class + def delivery_method + (@mails.first || ActionMailer::Base::NullMail.new).delivery_method + end + + # @return [ActionMailer::Base] the delivery handler of the first mail. This + # is always the `Mailer` class. + def delivery_handler + (@mails.first || ActionMailer::Base::NullMail.new).delivery_handler + end + + private + + def method_missing(method, *args, &block) + if method =~ /\Adeliver([_!?]|\z)/ + @mails.each do |mail| + mail.public_send(method, *args, &block) + end + else + super + end + end + + def respond_to_missing(method, *args) + method =~ /\Adeliver([_!?]|\z)/ || method == 'processed?' || super + end + + # This method is slightly adapted from ActionMailer::MessageDelivery + def enqueue_delivery(delivery_method, options = {}) + if processed? + ::Kernel.raise "You've accessed the message before asking to " \ + "deliver it later, so you may have made local changes that would " \ + "be silently lost if we enqueued a job to deliver it. Why? Only " \ + "the mailer method *arguments* are passed with the delivery job! " \ + "Do not access the message in any way if you mean to deliver it " \ + "later. Workarounds: 1. don't touch the message before calling " \ + "#deliver_later, 2. only touch the message *within your mailer " \ + "method*, or 3. use a custom Active Job instead of #deliver_later." + else + args = 'Mailer', @action.to_s, delivery_method.to_s, *@args + ::ActionMailer::DeliveryJob.set(options).perform_later(*args) + end + end + end + + def process(action, *args) + user = args.shift + raise ArgumentError, "First argument has to be a user, was #{user.inspect}" unless user.is_a?(User) + + initial_user = User.current + initial_language = ::I18n.locale + begin + User.current = user + + lang = find_language(user.language) if user.logged? + lang ||= Setting.default_language + set_language_if_valid(lang) + + super(action, *args) + ensure + User.current = initial_user + ::I18n.locale = initial_language + end + end + + def self.default_url_options options = {:protocol => Setting.protocol} if Setting.host_name.to_s =~ /\A(https?\:\/\/)?(.+?)(\:(\d+))?(\/.+)?\z/i @@ -39,8 +146,11 @@ class Mailer < ActionMailer::Base options end - # Builds a mail for notifying to_users and cc_users about a new issue - def issue_add(issue, to_users, cc_users) + # Builds a mail for notifying the current user about a new issue + # + # Example: + # issue_add(issue) => Mail::Message object + def issue_add(issue) redmine_headers 'Project' => issue.project.identifier, 'Issue-Id' => issue.id, 'Issue-Author' => issue.author.login @@ -49,24 +159,34 @@ class Mailer < ActionMailer::Base references issue @author = issue.author @issue = issue - @users = to_users + cc_users + @users = [User.current] @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue) - mail :to => to_users, - :cc => cc_users, + mail :to => User.current, :subject => "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] (#{issue.status.name}) #{issue.subject}" end # Notifies users about a new issue + # + # Example: + # Mailer.issue_add(journal).deliver => sends emails to the project's recipients + def self.issue_add(issue) + users = issue.notified_users | issue.notified_watchers + MultiMessage.new(:issue_add, issue).for(users) + end + + # Notifies users about a new issue + # + # Example: + # Mailer.deliver_issue_add(issue) => sends emails to the project's recipients def self.deliver_issue_add(issue) - to = issue.notified_users - cc = issue.notified_watchers - to - issue.each_notification(to + cc) do |users| - issue_add(issue, to & users, cc & users).deliver - end + issue_add(issue).deliver end - # Builds a mail for notifying to_users and cc_users about an issue update - def issue_edit(journal, to_users, cc_users) + # Builds a mail for notifying the current user about an issue update + # + # Example: + # issue_edit(journal) => Mail::Message object + def issue_edit(journal) issue = journal.journalized redmine_headers 'Project' => issue.project.identifier, 'Issue-Id' => issue.id, @@ -79,57 +199,88 @@ class Mailer < ActionMailer::Base s << "(#{issue.status.name}) " if journal.new_value_for('status_id') s << issue.subject @issue = issue - @users = to_users + cc_users + @users = [User.current] @journal = journal - @journal_details = journal.visible_details(@users.first) + @journal_details = journal.visible_details @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue, :anchor => "change-#{journal.id}") - mail :to => to_users, - :cc => cc_users, + + mail :to => User.current, :subject => s end + # Build a MultiMessage to notify users about an issue update + # + # Example: + # Mailer.issue_edit(journal).deliver => sends emails to the project's recipients + def self.issue_edit(journal) + users = journal.notified_users + users |= journal.notified_watchers + users.select! do |user| + journal.notes? || journal.visible_details(user).any? + end + MultiMessage.new(:issue_edit, journal).for(users) + end + # Notifies users about an issue update + # + # Example: + # Mailer.deliver_issue_edit(journal) => sends emails to the project's recipients def self.deliver_issue_edit(journal) - issue = journal.journalized.reload - to = journal.notified_users - cc = journal.notified_watchers - to - journal.each_notification(to + cc) do |users| - issue.each_notification(users) do |users2| - issue_edit(journal, to & users2, cc & users2).deliver - end - end + issue_edit(journal).deliver end - def reminder(user, issues, days) - set_language_if_valid user.language + # Builds a Mail::Message object used to send en email reminder to the current + # user about their due issues. + # + # Example: + # reminder(issues, days) => Mail::Message object + def reminder(issues, days) @issues = issues @days = days @issues_url = url_for(:controller => 'issues', :action => 'index', - :set_filter => 1, :assigned_to_id => user.id, + :set_filter => 1, :assigned_to_id => User.current.id, :sort => 'due_date:asc') - mail :to => user, + mail :to => User.current, :subject => l(:mail_subject_reminder, :count => issues.size, :days => days) end - # Builds a Mail::Message object used to email users belonging to the added document's project. + # Builds a Mail::Message object used to email the given user about their due + # issues + # + # Example: + # Mailer.reminder(user, issues, days, author).deliver => sends an email to the user + def self.reminder(user, issues, days) + MultiMessage.new(:reminder, issues, days).for(user) + end + + # Builds a Mail::Message object used to email the current user that a document + # was added. # # Example: - # document_added(document) => Mail::Message object - # Mailer.document_added(document).deliver => sends an email to the document's project recipients - def document_added(document) + # document_added(document, author) => Mail::Message object + def document_added(document, author) redmine_headers 'Project' => document.project.identifier - @author = User.current + @author = author @document = document @document_url = url_for(:controller => 'documents', :action => 'show', :id => document) - mail :to => document.notified_users, + mail :to => User.current, :subject => "[#{document.project.name}] #{l(:label_document_new)}: #{document.title}" end - # Builds a Mail::Message object used to email recipients of a project when an attachements are added. + # Build a MultiMessage to notify users about an added document. + # + # Example: + # Mailer.document_added(document).deliver => sends emails to the document's project recipients + def self.document_added(document) + users = document.notified_users + MultiMessage.new(:document_added, document, User.current).for(users) + end + + # Builds a Mail::Message object used to email the current user when + # attachements are added. # # Example: # attachments_added(attachments) => Mail::Message object - # Mailer.attachments_added(attachments).deliver => sends an email to the project's recipients def attachments_added(attachments) container = attachments.first.container added_to = '' @@ -139,29 +290,42 @@ class Mailer < ActionMailer::Base when 'Project' added_to_url = url_for(:controller => 'files', :action => 'index', :project_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 => 'files', :action => 'index', :project_id => container.project) 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.notified_users end redmine_headers 'Project' => container.project.identifier @attachments = attachments @added_to = added_to @added_to_url = added_to_url - mail :to => recipients, + mail :to => User.current, :subject => "[#{container.project.name}] #{l(:label_attachment_new)}" end - # Builds a Mail::Message object used to email recipients of a news' project when a news item is added. + # Build a MultiMessage to notify users about an added attachment + # + # Example: + # Mailer.attachments_added(attachments).deliver => sends emails to the project's recipients + def self.attachments_added(attachments) + container = attachments.first.container + case container.class.name + when 'Project', 'Version' + users = container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)} + when 'Document' + users = container.notified_users + end + + MultiMessage.new(:attachments_added, attachments).for(users) + end + + # Builds a Mail::Message object used to email the current user when a news + # item is added. # # Example: # news_added(news) => Mail::Message object - # Mailer.news_added(news).deliver => sends an email to the news' project recipients def news_added(news) redmine_headers 'Project' => news.project.identifier @author = news.author @@ -169,16 +333,24 @@ class Mailer < ActionMailer::Base references news @news = news @news_url = url_for(:controller => 'news', :action => 'show', :id => news) - mail :to => news.notified_users, - :cc => news.notified_watchers_for_added_news, + mail :to => User.current, :subject => "[#{news.project.name}] #{l(:label_news)}: #{news.title}" end - # Builds a Mail::Message object used to email recipients of a news' project when a news comment is added. + # Build a MultiMessage to notify users about a new news item + # + # Example: + # Mailer.news_added(news).deliver => sends emails to the news' project recipients + def self.news_added(news) + users = news.notified_users | news.notified_watchers_for_added_news + MultiMessage.new(:news_added, news).for(users) + end + + # Builds a Mail::Message object used to email the current user when a news + # comment is added. # # Example: # news_comment_added(comment) => Mail::Message object - # Mailer.news_comment_added(comment) => sends an email to the news' project recipients def news_comment_added(comment) news = comment.commented redmine_headers 'Project' => news.project.identifier @@ -188,64 +360,87 @@ class Mailer < ActionMailer::Base @news = news @comment = comment @news_url = url_for(:controller => 'news', :action => 'show', :id => news) - mail :to => news.notified_users, - :cc => news.notified_watchers, + mail :to => User.current, :subject => "Re: [#{news.project.name}] #{l(:label_news)}: #{news.title}" end - # Builds a Mail::Message object used to email the recipients of the specified message that was posted. + # Build a MultiMessage to notify users about a new news comment + # + # Example: + # Mailer.news_comment_added(comment).deliver => sends emails to the news' project recipients + def self.news_comment_added(comment) + news = comment.commented + users = news.notified_users | news.notified_watchers + + MultiMessage.new(:news_comment_added, comment).for(users) + end + + # Builds a Mail::Message object used to email the current user that the + # specified message was posted. # # Example: # message_posted(message) => Mail::Message object - # Mailer.message_posted(message).deliver => sends an email to the recipients def message_posted(message) redmine_headers 'Project' => message.project.identifier, 'Topic-Id' => (message.parent_id || message.id) @author = message.author message_id message references message.root - recipients = message.notified_users - cc = ((message.root.notified_watchers + message.board.notified_watchers).uniq - recipients) @message = message @message_url = url_for(message.event_url) - mail :to => recipients, - :cc => cc, + mail :to => User.current, :subject => "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] #{message.subject}" end - # Builds a Mail::Message object used to email the recipients of a project of the specified wiki content was added. + # Build a MultiMessage to notify users about a new forum message + # + # Example: + # Mailer.message_posted(message).deliver => sends emails to the recipients + def self.message_posted(message) + users = message.notified_users + users |= message.root.notified_watchers + users |= message.board.notified_watchers + + MultiMessage.new(:message_posted, message).for(users) + end + + # Builds a Mail::Message object used to email the current user that the + # specified wiki content was added. # # Example: # wiki_content_added(wiki_content) => Mail::Message object - # Mailer.wiki_content_added(wiki_content).deliver => sends an email to the project's recipients def wiki_content_added(wiki_content) redmine_headers 'Project' => wiki_content.project.identifier, 'Wiki-Page-Id' => wiki_content.page.id @author = wiki_content.author message_id wiki_content - recipients = wiki_content.notified_users - cc = wiki_content.page.wiki.notified_watchers - recipients @wiki_content = wiki_content @wiki_content_url = url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, :id => wiki_content.page.title) - mail :to => recipients, - :cc => cc, + mail :to => User.current, :subject => "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_added, :id => wiki_content.page.pretty_title)}" end - # Builds a Mail::Message object used to email the recipients of a project of the specified wiki content was updated. + # Build a MultiMessage to notify users about added wiki content + # + # Example: + # Mailer.wiki_content_added(wiki_content).deliver => send emails to the project's recipients + def self.wiki_content_added(wiki_content) + users = wiki_content.notified_users | wiki_content.page.wiki.notified_watchers + MultiMessage.new(:wiki_content_added, wiki_content).for(users) + end + + # Builds a Mail::Message object used to email the current user about an update + # of the specified wiki content. # # Example: # wiki_content_updated(wiki_content) => Mail::Message object - # Mailer.wiki_content_updated(wiki_content).deliver => sends an email to the project's recipients def wiki_content_updated(wiki_content) redmine_headers 'Project' => wiki_content.project.identifier, 'Wiki-Page-Id' => wiki_content.page.id @author = wiki_content.author message_id wiki_content - recipients = wiki_content.notified_users - cc = wiki_content.page.wiki.notified_watchers + wiki_content.page.notified_watchers - recipients @wiki_content = wiki_content @wiki_content_url = url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, @@ -253,56 +448,92 @@ class Mailer < ActionMailer::Base @wiki_diff_url = url_for(:controller => 'wiki', :action => 'diff', :project_id => wiki_content.project, :id => wiki_content.page.title, :version => wiki_content.version) - mail :to => recipients, - :cc => cc, + mail :to => User.current, :subject => "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_updated, :id => wiki_content.page.pretty_title)}" end - # Builds a Mail::Message object used to email the specified user their account information. + # Build a MultiMessage to notify users about the update of the specified wiki content # # Example: - # account_information(user, password) => Mail::Message object - # Mailer.account_information(user, password).deliver => sends account information to the user - def account_information(user, password) - set_language_if_valid user.language - @user = user + # Mailer.wiki_content_updated(wiki_content).deliver => sends an email to the project's recipients + def self.wiki_content_updated(wiki_content) + users = wiki_content.notified_users + users |= wiki_content.page.notified_watchers + users |= wiki_content.page.wiki.notified_watchers + + MultiMessage.new(:wiki_content_updated, wiki_content).for(users) + end + + # Builds a Mail::Message object used to email the current user their account information. + # + # Example: + # account_information(password) => Mail::Message object + def account_information(password) + @user = User.current @password = password @login_url = url_for(:controller => 'account', :action => 'login') - mail :to => user.mail, + mail :to => User.current.mail, :subject => l(:mail_subject_register, Setting.app_title) end - # Builds a Mail::Message object used to email all active administrators of an account activation request. + # Build a MultiMessage to mail a user their account information + # + # Example: + # Mailer.account_information(user, password).deliver => sends account information to the user + def self.account_information(user, password) + MultiMessage.new(:account_information, password).for(user) + end + + # Builds a Mail::Message object used to email the current user about an account activation request. # # Example: # account_activation_request(user) => Mail::Message object - # Mailer.account_activation_request(user).deliver => sends an email to all active administrators def account_activation_request(user) - # Send the email to all active administrators - recipients = User.active.where(:admin => true) @user = user @url = url_for(:controller => 'users', :action => 'index', :status => User::STATUS_REGISTERED, :sort_key => 'created_on', :sort_order => 'desc') - mail :to => recipients, + mail :to => User.current, :subject => l(:mail_subject_account_activation_request, Setting.app_title) end - # Builds a Mail::Message object used to email the specified user that their account was activated by an administrator. + # Build a MultiMessage to email all active administrators of an account activation request. # # Example: - # account_activated(user) => Mail::Message object - # Mailer.account_activated(user).deliver => sends an email to the registered user - def account_activated(user) - set_language_if_valid user.language - @user = user + # Mailer.account_activation_request(user).deliver => sends an email to all active administrators + def self.account_activation_request(user) + # Send the email to all active administrators + users = User.active.where(:admin => true) + MultiMessage.new(:account_activation_request, user).for(users) + end + + # Builds a Mail::Message object used to email the account of the current user + # was activated by an administrator. + # + # Example: + # account_activated => Mail::Message object + def account_activated + @user = User.current @login_url = url_for(:controller => 'account', :action => 'login') - mail :to => user.mail, + mail :to => User.current.mail, :subject => l(:mail_subject_register, Setting.app_title) end + # Build a MultiMessage to email the specified user that their account was + # activated by an administrator. + # + # Example: + # Mailer.account_activated(user).deliver => sends an email to the registered user + def self.account_activated(user) + MultiMessage.new(:account_activated).for(user) + end + + # Builds a Mail::Message object used to email the lost password token to the + # token's user (or a different recipient). + # + # Example: + # lost_password(token) => Mail::Message object def lost_password(token, recipient=nil) - set_language_if_valid(token.user.language) recipient ||= token.user.mail @token = token @url = url_for(:controller => 'account', :action => 'lost_password', :token => token.value) @@ -310,6 +541,15 @@ class Mailer < ActionMailer::Base :subject => l(:mail_subject_lost_password, Setting.app_title) end + # Build a MultiMessage to email the token's user (or a different recipient) + # the lost password token for the token's user. + # + # Example: + # Mailer.lost_password(token).deliver => sends an email to the user + def self.lost_password(token, recipient=nil) + MultiMessage.new(:lost_password, token, recipient).for(token.user) + end + # Notifies user that his password was updated def self.password_updated(user, options={}) # Don't send a notification to the dummy email address when changing the password @@ -326,39 +566,92 @@ class Mailer < ActionMailer::Base ).deliver end + # Builds a Mail::Message object used to email the user activation link to the + # token's user. + # + # Example: + # register(token) => Mail::Message object def register(token) - set_language_if_valid(token.user.language) @token = token @url = url_for(:controller => 'account', :action => 'activate', :token => token.value) mail :to => token.user.mail, :subject => l(:mail_subject_register, Setting.app_title) end - def security_notification(recipients, options={}) - @user = Array(recipients).detect{|r| r.is_a? User } - set_language_if_valid(@user.try :language) + # Build a MultiMessage to email the user activation link to the token's user. + # + # Example: + # Mailer.register(token).deliver => sends an email to the token's user + def self.register(token) + MultiMessage.new(:register, token).for(token.user) + end + + # Build a Mail::Message object to email the current user and the additional + # recipients given in options[:recipients] about a security related event. + # + # Example: + # security_notification(users, + # message: :mail_body_security_notification_add, + # field: :field_mail, + # value: address + # ) => Mail::Message object + def security_notification(sender, options={}) + @sender = sender + redmine_headers 'Sender' => sender.login @message = l(options[:message], field: (options[:field] && l(options[:field])), value: options[:value] ) @title = options[:title] && l(options[:title]) - @originator = options[:originator] || User.current + @originator = options[:originator] || sender @remote_ip = options[:remote_ip] || @originator.remote_ip @url = options[:url] && (options[:url].is_a?(Hash) ? url_for(options[:url]) : options[:url]) redmine_headers 'Sender' => @originator.login redmine_headers 'Url' => @url - mail :to => recipients, + mail :to => [User.current, *options[:recipients]].uniq, :subject => "[#{Setting.app_title}] #{l(:mail_subject_security_notification)}" end - def settings_updated(recipients, changes) - redmine_headers 'Sender' => User.current.login + # Build a MultiMessage to email the given users about a security related event. + # + # You can specify additional recipients in options[:recipients]. These will be + # added to all generated mails for all given users. Usually, you'll want to + # give only a single user when setting the additional recipients. + # + # Example: + # Mailer.security_notification(users, + # message: :mail_body_security_notification_add, + # field: :field_mail, + # value: address + # ).deliver => sends a security notification to the given user(s) + def self.security_notification(users, options={}) + sender = User.current + MultiMessage.new(:security_notification, sender, options).for(users) + end + + # Build a Mail::Message object to email the current user about an updated + # setting. + # + # Example: + # settings_updated(sender, [:host_name]) => Mail::Message object + def settings_updated(sender, changes) + @sender = sender + redmine_headers 'Sender' => sender.login @changes = changes @url = url_for(controller: 'settings', action: 'index') - mail :to => recipients, + mail :to => User.current, :subject => "[#{Setting.app_title}] #{l(:mail_subject_security_notification)}" end + # Build a MultiMessage to email the given users about an update of a setting. + # + # Example: + # Mailer.settings_updated(users, [:host_name]).deliver => sends emails to the given user(s) about the update + def self.settings_updated(users, changes) + sender = User.current + MultiMessage.new(:settings_updated, sender, changes).for(users) + end + # Notifies admins about settings changes def self.security_settings_updated(changes) return unless changes.present? @@ -367,13 +660,24 @@ class Mailer < ActionMailer::Base settings_updated(users, changes).deliver end - def test_email(user) - set_language_if_valid(user.language) + # Build a Mail::Message object with a test email for the current user + # + # Example: + # test_email => Mail::Message object + def test_email @url = url_for(:controller => 'welcome') - mail :to => user.mail, + mail :to => User.current.mail, :subject => 'Redmine test' end + # Build a MultiMessage to send a test email the given user + # + # Example: + # Mailer.test_email(user).deliver => send an email to the given user + def self.test_email(user) + MultiMessage.new(:test_email).for(user) + end + # Sends reminders to issue assignees # Available options: # * :days => how many days in the future to remind about (defaults to 7) @@ -483,7 +787,7 @@ class Mailer < ActionMailer::Base headers[:references] = @references_objects.collect {|o| "<#{self.class.references_for(o)}>"}.join(' ') end - m = if block_given? + if block_given? super headers, &block else super headers do |format| @@ -491,15 +795,6 @@ class Mailer < ActionMailer::Base format.html unless Setting.plain_text_mail? end end - set_language_if_valid @initial_language - - m - end - - def initialize(*args) - @initial_language = current_language - set_language_if_valid Setting.default_language - super end def self.deliver_mail(mail) @@ -521,6 +816,8 @@ class Mailer < ActionMailer::Base if m = method.to_s.match(%r{^deliver_(.+)$}) ActiveSupport::Deprecation.warn "Mailer.deliver_#{m[1]}(*args) is deprecated. Use Mailer.#{m[1]}(*args).deliver instead." send(m[1], *args).deliver + elsif action_methods.include?(method.to_s) + MultiMessage.new(method, *args).for(User.current) else super end diff --git a/app/views/mailer/security_notification.html.erb b/app/views/mailer/security_notification.html.erb index 309e9437f..84394d1ab 100644 --- a/app/views/mailer/security_notification.html.erb +++ b/app/views/mailer/security_notification.html.erb @@ -9,5 +9,4 @@

<%= l(:field_user) %>: <%= @originator.login %>
<%= l(:field_remote_ip) %>: <%= @remote_ip %>
-<%= l(:label_date) %>: <%= format_time Time.now, true, @user %>

- +<%= l(:label_date) %>: <%= format_time Time.now, true %>

diff --git a/app/views/mailer/security_notification.text.erb b/app/views/mailer/security_notification.text.erb index 5be036b7a..7ff3f3d57 100644 --- a/app/views/mailer/security_notification.text.erb +++ b/app/views/mailer/security_notification.text.erb @@ -4,5 +4,4 @@ <%= l(:field_user) %>: <%= @originator.login %> <%= l(:field_remote_ip) %>: <%= @remote_ip %> -<%= l(:label_date) %>: <%= format_time Time.now, true, @user %> - +<%= l(:label_date) %>: <%= format_time Time.now, true %> diff --git a/app/views/mailer/settings_updated.html.erb b/app/views/mailer/settings_updated.html.erb index 8596089a2..c50eb9ea1 100644 --- a/app/views/mailer/settings_updated.html.erb +++ b/app/views/mailer/settings_updated.html.erb @@ -8,7 +8,7 @@ <%= link_to @url, @url %> -

<%= l(:field_user) %>: <%= User.current.login %>
-<%= l(:field_remote_ip) %>: <%= User.current.remote_ip %>
+

<%= l(:field_user) %>: <%= @sender.login %>
+<%= l(:field_remote_ip) %>: <%= @sender.remote_ip %>
<%= l(:label_date) %>: <%= format_time Time.now, true %>

diff --git a/app/views/mailer/settings_updated.text.erb b/app/views/mailer/settings_updated.text.erb index 51a2a8f6a..9b0be909d 100644 --- a/app/views/mailer/settings_updated.text.erb +++ b/app/views/mailer/settings_updated.text.erb @@ -6,7 +6,7 @@ <%= @url %> -<%= l(:field_user) %>: <%= User.current.login %> -<%= l(:field_remote_ip) %>: <%= User.current.remote_ip %> +<%= l(:field_user) %>: <%= @sender.login %> +<%= l(:field_remote_ip) %>: <%= @sender.remote_ip %> <%= l(:label_date) %>: <%= format_time Time.now, true %> diff --git a/test/functional/documents_controller_test.rb b/test/functional/documents_controller_test.rb index 0a7aed72b..c8c9fd57c 100644 --- a/test/functional/documents_controller_test.rb +++ b/test/functional/documents_controller_test.rb @@ -132,7 +132,7 @@ LOREM assert_equal Enumeration.find(2), document.category assert_equal 1, document.attachments.size assert_equal 'testfile.txt', document.attachments.first.filename - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_with_failure diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index a5fb68b59..17af8981c 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -152,7 +152,7 @@ class IssuesControllerTest < Redmine::ControllerTest :f => ['tracker_id'], :op => { 'tracker_id' => '=' - }, + }, :v => { 'tracker_id' => ['1'] } @@ -253,10 +253,10 @@ class IssuesControllerTest < Redmine::ControllerTest :f => [filter_name], :op => { filter_name => '=' - }, + }, :v => { filter_name => ['Foo'] - }, + }, :c => ['project'] } assert_response :success @@ -1459,7 +1459,7 @@ class IssuesControllerTest < Redmine::ControllerTest :f => ['start_date'], :op => { :start_date => '=' - }, + }, :format => 'csv' } assert_equal 'text/csv', @response.content_type @@ -2678,7 +2678,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => 3, :description => 'Prefilled', :custom_field_values => { - '2' => 'Custom field value'} + '2' => 'Custom field value'} } } @@ -2802,7 +2802,7 @@ class IssuesControllerTest < Redmine::ControllerTest assert !t.disabled_core_fields.include?('parent_issue_id') get :new, :params => { - :project_id => 1, issue: { parent_issue_id: 1 + :project_id => 1, issue: { parent_issue_id: 1 } } assert_response :success @@ -2812,7 +2812,7 @@ class IssuesControllerTest < Redmine::ControllerTest t.save! assert t.disabled_core_fields.include?('parent_issue_id') get :new, :params => { - :project_id => 1, issue: { parent_issue_id: 1 + :project_id => 1, issue: { parent_issue_id: 1 } } assert_response :success @@ -2870,7 +2870,7 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :tracker_id => 2, :status_id => 1 - }, + }, :was_default_status => 1 } assert_response :success @@ -2889,7 +2889,7 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :project_id => 1, :fixed_version_id => '' - }, + }, :form_update_triggered_by => 'issue_project_id' } assert_response :success @@ -2917,7 +2917,7 @@ class IssuesControllerTest < Redmine::ControllerTest :start_date => '2010-11-07', :estimated_hours => '', :custom_field_values => { - '2' => 'Value for field 2'} + '2' => 'Value for field 2'} } } end @@ -2976,7 +2976,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 5, :estimated_hours => '', :custom_field_values => { - '2' => 'Value for field 2'} + '2' => 'Value for field 2'} } } end @@ -3002,7 +3002,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 5, :estimated_hours => '', :custom_field_values => { - '2' => 'Value for field 2'} + '2' => 'Value for field 2'} } } end @@ -3023,7 +3023,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => 3, :subject => 'This is first issue', :priority_id => 5 - }, + }, :continue => '' } end @@ -3065,7 +3065,7 @@ class IssuesControllerTest < Redmine::ControllerTest :description => 'This is the description', :priority_id => 5, :custom_field_values => { - '1' => ['', 'MySQL', 'Oracle']} + '1' => ['', 'MySQL', 'Oracle']} } } end @@ -3088,7 +3088,7 @@ class IssuesControllerTest < Redmine::ControllerTest :description => 'This is the description', :priority_id => 5, :custom_field_values => { - '1' => ['']} + '1' => ['']} } } end @@ -3111,7 +3111,7 @@ class IssuesControllerTest < Redmine::ControllerTest :description => 'This is the description', :priority_id => 5, :custom_field_values => { - field.id.to_s => ['', '2', '3']} + field.id.to_s => ['', '2', '3']} } } end @@ -3159,8 +3159,8 @@ class IssuesControllerTest < Redmine::ControllerTest :due_date => '', :custom_field_values => { cf1.id.to_s => '', cf2.id.to_s => '' - } - + } + } } assert_response :success @@ -3189,8 +3189,8 @@ class IssuesControllerTest < Redmine::ControllerTest :due_date => '', :custom_field_values => { cf1.id.to_s => '', cf2.id.to_s => [''] - } - + } + } } assert_response :success @@ -3219,8 +3219,8 @@ class IssuesControllerTest < Redmine::ControllerTest :due_date => '2012-07-16', :custom_field_values => { cf1.id.to_s => 'value1', cf2.id.to_s => 'value2' - } - + } + } } assert_response 302 @@ -3246,7 +3246,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => 1, :status_id => 1, :subject => 'Test' - + } } assert_response 302 @@ -3424,7 +3424,7 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 3, :tracker_id => 2, :subject => 'Foo' - }, + }, :continue => '1' } assert_redirected_to '/issues/new?issue%5Bproject_id%5D=3&issue%5Btracker_id%5D=2' @@ -3477,13 +3477,13 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 5, :estimated_hours => '', :custom_field_values => { - '2' => 'Value for field 2'} + '2' => 'Value for field 2'} } } end assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end end @@ -3536,7 +3536,7 @@ class IssuesControllerTest < Redmine::ControllerTest post :create, :params => { :project_id => 1, :issue => { - :tracker => "A param can not be a Tracker" + :tracker => "A param can not be a Tracker" } } end @@ -3553,11 +3553,11 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 1, :issue => { :tracker_id => '1', - :subject => 'With attachment' - }, + :subject => 'With attachment' + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} + 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} } } end @@ -3588,11 +3588,11 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 1, :issue => { :tracker_id => '1', - :subject => 'With attachment' - }, + :subject => 'With attachment' + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} + 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} } } end @@ -3614,11 +3614,11 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 1, :issue => { :tracker_id => '1', - :subject => '' - }, + :subject => '' + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} + 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} } } assert_response :success @@ -3645,11 +3645,11 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 1, :issue => { :tracker_id => '1', - :subject => '' - }, + :subject => '' + }, :attachments => { 'p0' => { - 'token' => attachment.token} + 'token' => attachment.token} } } assert_response :success @@ -3671,11 +3671,11 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => 1, :issue => { :tracker_id => '1', - :subject => 'Saved attachments' - }, + :subject => 'Saved attachments' + }, :attachments => { 'p0' => { - 'token' => attachment.token} + 'token' => attachment.token} } } assert_response 302 @@ -3997,7 +3997,7 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => '1', :tracker_id => '1', :status_id => '1' - }, + }, :was_default_status => '1' } end @@ -4041,7 +4041,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => '3', :status_id => '1', :subject => 'Copy with attachments' - }, + }, :copy_attachments => '1' } end @@ -4089,7 +4089,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => '3', :status_id => '1', :subject => 'Copy with attachments' - }, + }, :copy_attachments => '1', :attachments => { '1' => { @@ -4188,7 +4188,7 @@ class IssuesControllerTest < Redmine::ControllerTest :tracker_id => '3', :status_id => '1', :subject => 'Copy with subtasks' - }, + }, :copy_subtasks => '1' } end @@ -4212,8 +4212,8 @@ class IssuesControllerTest < Redmine::ControllerTest :status_id => '1', :subject => 'Copy with subtasks', :custom_field_values => { - '2' => 'Foo'} - }, + '2' => 'Foo'} + }, :copy_subtasks => '1' } end @@ -4394,12 +4394,12 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :status_id => 5, - :priority_id => 7 - }, + :priority_id => 7 + }, :time_entry => { :hours => '2.5', :comments => 'test_get_edit_with_params', - :activity_id => 10 + :activity_id => 10 } } assert_response :success @@ -4638,7 +4638,7 @@ class IssuesControllerTest < Redmine::ControllerTest :project_id => '1', :tracker_id => '2', :priority_id => '6' - + } } end @@ -4701,9 +4701,9 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :subject => 'Custom field change', :custom_field_values => { - '1' => ['', 'Oracle', 'PostgreSQL'] - } - + '1' => ['', 'Oracle', 'PostgreSQL'] + } + } } end @@ -4724,12 +4724,12 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :status_id => 2, :assigned_to_id => 3, - :notes => 'Assigned to dlopper' - }, + :notes => 'Assigned to dlopper' + }, :time_entry => { :hours => '', :comments => '', - :activity_id => TimeEntryActivity.first + :activity_id => TimeEntryActivity.first } } end @@ -4755,7 +4755,7 @@ class IssuesControllerTest < Redmine::ControllerTest put :update, :params => { :id => 1, :issue => { - :notes => notes + :notes => notes } } end @@ -4823,12 +4823,12 @@ class IssuesControllerTest < Redmine::ControllerTest put :update, :params => { :id => 1, :issue => { - :notes => '2.5 hours added' - }, + :notes => '2.5 hours added' + }, :time_entry => { :hours => '2.5', :comments => 'test_put_update_with_note_and_spent_time', - :activity_id => TimeEntryActivity.first.id + :activity_id => TimeEntryActivity.first.id } } end @@ -4883,10 +4883,10 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :notes => '' - }, + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} + 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} } } end @@ -4922,11 +4922,11 @@ class IssuesControllerTest < Redmine::ControllerTest put :update, :params => { :id => 1, :issue => { - :subject => '' - }, + :subject => '' + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} + 'file' => uploaded_test_file('testfile.txt', 'text/plain'), 'description' => 'test file'} } } assert_response :success @@ -4952,11 +4952,11 @@ class IssuesControllerTest < Redmine::ControllerTest put :update, :params => { :id => 1, :issue => { - :subject => '' - }, + :subject => '' + }, :attachments => { 'p0' => { - 'token' => attachment.token} + 'token' => attachment.token} } } assert_response :success @@ -4979,10 +4979,10 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :notes => 'Attachment added' - }, + }, :attachments => { 'p0' => { - 'token' => attachment.token} + 'token' => attachment.token} } } assert_redirected_to '/issues/1' @@ -5007,10 +5007,10 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :notes => '' - }, + }, :attachments => { '1' => { - 'file' => uploaded_test_file('testfile.txt', 'text/plain')} + 'file' => uploaded_test_file('testfile.txt', 'text/plain')} } } assert_redirected_to :action => 'show', :id => '1' @@ -5030,7 +5030,7 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :notes => 'Removing attachments', :deleted_attachment_ids => ['1', '5'] - + } } end @@ -5057,7 +5057,7 @@ class IssuesControllerTest < Redmine::ControllerTest :subject => '', :notes => 'Removing attachments', :deleted_attachment_ids => ['1', '5'] - + } } end @@ -5100,10 +5100,10 @@ class IssuesControllerTest < Redmine::ControllerTest :subject => new_subject, :priority_id => '6', :category_id => '1' # no change - + } } - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end end @@ -5116,7 +5116,7 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :notes => notes - }, + }, :time_entry => { "comments"=>"", "activity_id"=>"", "hours"=>"2z" } @@ -5138,7 +5138,7 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :notes => notes - }, + }, :time_entry => { "comments"=>"this is my comment", "activity_id"=>"", "hours"=>"" } @@ -5160,7 +5160,7 @@ class IssuesControllerTest < Redmine::ControllerTest :id => issue.id, :issue => { :fixed_version_id => 4 - + } } @@ -5178,8 +5178,8 @@ class IssuesControllerTest < Redmine::ControllerTest :id => issue.id, :issue => { :fixed_version_id => 4 - - }, + + }, :back_url => '/issues' } @@ -5195,8 +5195,8 @@ class IssuesControllerTest < Redmine::ControllerTest :id => issue.id, :issue => { :fixed_version_id => 4 - - }, + + }, :back_url => 'http://google.com' } @@ -5212,7 +5212,7 @@ class IssuesControllerTest < Redmine::ControllerTest :issue => { :status_id => 6, :notes => 'Notes' - }, + }, :prev_issue_id => 8, :next_issue_id => 12, :issue_position => 2, @@ -5501,7 +5501,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 7, :assigned_to_id => '', :custom_field_values => { - '2' => ''} + '2' => ''} } } @@ -5531,7 +5531,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => '', :assigned_to_id => group.id, :custom_field_values => { - '2' => ''} + '2' => ''} } } @@ -5550,7 +5550,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 7, :assigned_to_id => '', :custom_field_values => { - '2' => ''} + '2' => ''} } } @@ -5578,14 +5578,14 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => 7, :assigned_to_id => '', :custom_field_values => { - '2' => ''} + '2' => ''} } } assert_response 403 assert_not_equal "Bulk should fail", Journal.last.notes end - def test_bullk_update_should_send_a_notification + def test_bulk_update_should_send_a_notification @request.session[:user_id] = 2 ActionMailer::Base.deliveries.clear with_settings :notified_events => %w(issue_updated) do @@ -5599,7 +5599,7 @@ class IssuesControllerTest < Redmine::ControllerTest } } assert_response 302 - assert_equal 2, ActionMailer::Base.deliveries.size + assert_equal 5, ActionMailer::Base.deliveries.size end end @@ -5626,7 +5626,7 @@ class IssuesControllerTest < Redmine::ControllerTest :id => 1, :issue => { :project_id => '2' - }, + }, :follow => '1' } assert_redirected_to '/issues/1' @@ -5638,7 +5638,7 @@ class IssuesControllerTest < Redmine::ControllerTest :id => [1, 2], :issue => { :project_id => '2' - }, + }, :follow => '1' } assert_redirected_to '/projects/onlinestore/issues' @@ -5745,7 +5745,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => '', :assigned_to_id => '', :custom_field_values => { - '2' => '777'} + '2' => '777'} } } @@ -5768,7 +5768,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => '', :assigned_to_id => '', :custom_field_values => { - '1' => '__none__'} + '1' => '__none__'} } } assert_response 302 @@ -5788,7 +5788,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => '', :assigned_to_id => '', :custom_field_values => { - '1' => ['MySQL', 'Oracle']} + '1' => ['MySQL', 'Oracle']} } } @@ -5812,7 +5812,7 @@ class IssuesControllerTest < Redmine::ControllerTest :priority_id => '', :assigned_to_id => '', :custom_field_values => { - '1' => ['__none__']} + '1' => ['__none__']} } } assert_response 302 @@ -5969,7 +5969,7 @@ class IssuesControllerTest < Redmine::ControllerTest :ids => issue_ids, :issue => { :project_id => '2' - }, + }, :copy => '1' } end @@ -5991,7 +5991,7 @@ class IssuesControllerTest < Redmine::ControllerTest :ids => [1, 2, 3], :issue => { :project_id => '2' - }, + }, :copy => '1' } assert_response 302 @@ -6006,7 +6006,7 @@ class IssuesControllerTest < Redmine::ControllerTest :ids => [1, 2, 3], :issue => { :project_id => '' - }, + }, :copy => '1' } assert_response 403 @@ -6020,7 +6020,7 @@ class IssuesControllerTest < Redmine::ControllerTest :ids => [1, 2, 3], :issue => { :project_id => '1' - }, + }, :copy => '1' } assert_response 403 @@ -6047,7 +6047,7 @@ class IssuesControllerTest < Redmine::ControllerTest :status_id => '', :start_date => '', :due_date => '' - + } } end @@ -6087,7 +6087,7 @@ class IssuesControllerTest < Redmine::ControllerTest :status_id => '1', :start_date => '2009-12-01', :due_date => '2009-12-31' - + } } end @@ -6117,7 +6117,7 @@ class IssuesControllerTest < Redmine::ControllerTest :status_id => '3', :start_date => '2009-12-01', :due_date => '2009-12-31' - + } } end @@ -6140,7 +6140,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_attachments => '0', :issue => { :project_id => '' - + } } end @@ -6160,7 +6160,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_attachments => '1', :issue => { :project_id => '' - + } } end @@ -6178,7 +6178,7 @@ class IssuesControllerTest < Redmine::ControllerTest :link_copy => '1', :issue => { :project_id => '1' - + } } end @@ -6196,7 +6196,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_subtasks => '0', :issue => { :project_id => '' - + } } end @@ -6214,7 +6214,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_subtasks => '1', :issue => { :project_id => '' - + } } end @@ -6233,7 +6233,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_watchers => '1', :issue => { :project_id => '' - + } } end @@ -6253,7 +6253,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy_subtasks => '1', :issue => { :project_id => '' - + } } end @@ -6268,7 +6268,7 @@ class IssuesControllerTest < Redmine::ControllerTest :copy => '1', :issue => { :project_id => 2 - }, + }, :follow => '1' } issue = Issue.order('id DESC').first diff --git a/test/functional/issues_custom_fields_visibility_test.rb b/test/functional/issues_custom_fields_visibility_test.rb index 1ce32bf79..485b55be8 100644 --- a/test/functional/issues_custom_fields_visibility_test.rb +++ b/test/functional/issues_custom_fields_visibility_test.rb @@ -285,7 +285,8 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest assert_response 302 end end - assert_equal users_to_test.values.uniq.size, ActionMailer::Base.deliveries.size + + assert_equal users_to_test.keys.size, ActionMailer::Base.deliveries.size # tests that each user receives 1 email with the custom fields he is allowed to see only users_to_test.each do |user, fields| mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} @@ -322,7 +323,7 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest } assert_response 302 end - assert_equal users_to_test.values.uniq.size, ActionMailer::Base.deliveries.size + assert_equal users_to_test.keys.size, ActionMailer::Base.deliveries.size # tests that each user receives 1 email with the custom fields he is allowed to see only users_to_test.each do |user, fields| mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb index 0f3d937db..b129cdd33 100644 --- a/test/functional/messages_controller_test.rb +++ b/test/functional/messages_controller_test.rb @@ -136,14 +136,17 @@ class MessagesControllerTest < Redmine::ControllerTest assert_equal 2, message.author_id assert_equal 1, message.board_id - mail = ActionMailer::Base.deliveries.last - assert_not_nil mail - assert_equal "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] Test created message", mail.subject - assert_mail_body_match 'Message body', mail + mails = ActionMailer::Base.deliveries + assert_not_empty mails + mails.each do |mail| + assert_equal "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] Test created message", mail.subject + assert_mail_body_match 'Message body', mail + end + # author - assert mail.bcc.include?('jsmith@somenet.foo') + assert_equal ['jsmith@somenet.foo'], mails[0].bcc # project member - assert mail.bcc.include?('dlopper@somenet.foo') + assert_equal ['dlopper@somenet.foo'], mails[1].bcc end def test_get_edit diff --git a/test/functional/news_controller_test.rb b/test/functional/news_controller_test.rb index f62d21bf1..90a5a1112 100644 --- a/test/functional/news_controller_test.rb +++ b/test/functional/news_controller_test.rb @@ -127,7 +127,7 @@ class NewsControllerTest < Redmine::ControllerTest assert_equal 'This is the description', news.description assert_equal User.find(2), news.author assert_equal Project.find(1), news.project - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_post_create_with_attachment diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb index 8586d8fc7..a1c3cf8dc 100644 --- a/test/unit/changeset_test.rb +++ b/test/unit/changeset_test.rb @@ -46,7 +46,7 @@ class ChangesetTest < ActiveSupport::TestCase fixed = Issue.find(1) assert fixed.closed? assert_equal 90, fixed.done_ratio - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_ref_keywords diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index 4597fd9e7..fe9776fc3 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -36,7 +36,7 @@ class CommentTest < ActiveSupport::TestCase Watcher.create!(:watchable => @news, :user => @jsmith) with_settings :notified_events => %w(news_comment_added) do - assert_difference 'ActionMailer::Base.deliveries.size' do + assert_difference 'ActionMailer::Base.deliveries.size', 2 do Comment.create!(:commented => @news, :author => @jsmith, :comments => "my comment") end end diff --git a/test/unit/document_test.rb b/test/unit/document_test.rb index 9adcd4724..e4243dd60 100644 --- a/test/unit/document_test.rb +++ b/test/unit/document_test.rb @@ -41,7 +41,7 @@ class DocumentTest < ActiveSupport::TestCase doc = Document.new(:project => Project.find(1), :title => 'New document', :category => Enumeration.find_by_name('User documentation')) assert doc.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_with_default_category diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index a6ace2fa0..432dd8745 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2529,7 +2529,7 @@ class IssueTest < ActiveSupport::TestCase :subject => 'test_create', :estimated_hours => '1:30') with_settings :notified_events => %w(issue_added) do assert issue.save - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end end @@ -2541,7 +2541,7 @@ class IssueTest < ActiveSupport::TestCase :subject => 'test_create', :estimated_hours => '1:30') with_settings :notified_events => %w(issue_added issue_updated) do assert issue.save - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end end @@ -2568,7 +2568,8 @@ class IssueTest < ActiveSupport::TestCase issue.init_journal User.find(1) issue.assigned_to = nil issue.save! - assert_include user.mail, ActionMailer::Base.deliveries.last.bcc + + assert_include [user.mail], ActionMailer::Base.deliveries.map(&:bcc) end end @@ -2581,7 +2582,7 @@ class IssueTest < ActiveSupport::TestCase issue.subject = 'Subjet update' with_settings :notified_events => %w(issue_updated) do assert issue.save - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size ActionMailer::Base.deliveries.clear stale.init_journal(User.find(1)) diff --git a/test/unit/journal_observer_test.rb b/test/unit/journal_observer_test.rb index e9eac32e5..17695538a 100644 --- a/test/unit/journal_observer_test.rb +++ b/test/unit/journal_observer_test.rb @@ -36,7 +36,7 @@ class JournalObserverTest < ActiveSupport::TestCase with_settings :notified_events => %w(issue_updated) do assert journal.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_should_not_send_email_notification_with_notify_set_to_false @@ -71,7 +71,7 @@ class JournalObserverTest < ActiveSupport::TestCase with_settings :notified_events => %w(issue_note_added) do assert journal.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_should_not_send_email_notification_without_issue_note_added @@ -95,7 +95,7 @@ class JournalObserverTest < ActiveSupport::TestCase with_settings :notified_events => %w(issue_status_updated) do assert issue.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_should_not_send_email_notification_without_issue_status_updated @@ -132,7 +132,7 @@ class JournalObserverTest < ActiveSupport::TestCase with_settings :notified_events => %w(issue_assigned_to_updated) do assert issue.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_should_not_send_email_notification_without_issue_assignee_updated @@ -157,7 +157,7 @@ class JournalObserverTest < ActiveSupport::TestCase with_settings :notified_events => %w(issue_priority_updated) do assert issue.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_create_should_not_send_email_notification_without_issue_priority_updated diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb index cca405a2e..9bbedd1d5 100644 --- a/test/unit/journal_test.rb +++ b/test/unit/journal_test.rb @@ -50,7 +50,7 @@ class JournalTest < ActiveSupport::TestCase journal = issue.init_journal(user, issue) assert journal.save - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_should_not_save_journal_with_blank_notes_and_no_details diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index 401198fed..8aa2cdbe8 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -440,10 +440,11 @@ class MailHandlerTest < ActiveSupport::TestCase ) end - # only 1 email for the new issue notification - assert_equal 1, ActionMailer::Base.deliveries.size - email = ActionMailer::Base.deliveries.first - assert_include 'Ticket by unknown user', email.subject + # only 2 emails for the new issue notification + assert_equal 2, ActionMailer::Base.deliveries.size + ActionMailer::Base.deliveries.each do |email| + assert_include 'Ticket by unknown user', email.subject + end end def test_created_user_should_have_mail_notification_to_none_with_no_notification_option @@ -877,7 +878,7 @@ class MailHandlerTest < ActiveSupport::TestCase def test_update_issue_should_send_email_notification journal = submit_email('ticket_reply.eml') assert journal.is_a?(Journal) - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 3, ActionMailer::Base.deliveries.size end def test_update_issue_should_not_set_defaults diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index 049db8198..0908e9e63 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -412,11 +412,15 @@ class MailerTest < ActiveSupport::TestCase journal.save! Role.find(2).add_permission! :view_private_notes - Mailer.deliver_issue_edit(journal) - assert_equal %w(dlopper@somenet.foo jsmith@somenet.foo), ActionMailer::Base.deliveries.last.bcc.sort + assert_difference 'ActionMailer::Base.deliveries.size', 2 do + Mailer.deliver_issue_edit(journal) + end + assert_equal %w(dlopper@somenet.foo jsmith@somenet.foo), ActionMailer::Base.deliveries.last(2).flat_map(&:bcc).sort Role.find(2).remove_permission! :view_private_notes - Mailer.deliver_issue_edit(journal) + assert_difference 'ActionMailer::Base.deliveries.size', 1 do + Mailer.deliver_issue_edit(journal) + end assert_equal %w(jsmith@somenet.foo), ActionMailer::Base.deliveries.last.bcc.sort end @@ -534,7 +538,7 @@ class MailerTest < ActiveSupport::TestCase def test_wiki_content_added content = WikiContent.find(1) with_each_language_as_default do - assert_difference 'ActionMailer::Base.deliveries.size' do + assert_difference 'ActionMailer::Base.deliveries.size', 2 do assert Mailer.wiki_content_added(content).deliver assert_select_email do assert_select 'a[href=?]', @@ -548,7 +552,7 @@ class MailerTest < ActiveSupport::TestCase def test_wiki_content_updated content = WikiContent.find(1) with_each_language_as_default do - assert_difference 'ActionMailer::Base.deliveries.size' do + assert_difference 'ActionMailer::Base.deliveries.size', 2 do assert Mailer.wiki_content_updated(content).deliver assert_select_email do assert_select 'a[href=?]', diff --git a/test/unit/news_test.rb b/test/unit/news_test.rb index 7899ec2c2..03a868a91 100644 --- a/test/unit/news_test.rb +++ b/test/unit/news_test.rb @@ -34,7 +34,7 @@ class NewsTest < ActiveSupport::TestCase with_settings :notified_events => %w(news_added) do assert news.save end - assert_equal 1, ActionMailer::Base.deliveries.size + assert_equal 2, ActionMailer::Base.deliveries.size end def test_should_include_news_for_projects_with_news_enabled diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb index 59f011f9f..9552d0948 100644 --- a/test/unit/repository_test.rb +++ b/test/unit/repository_test.rb @@ -262,14 +262,15 @@ class RepositoryTest < ActiveSupport::TestCase assert_equal User.find_by_login('dlopper'), journal.user assert_equal 'Applied in changeset r2.', journal.notes - # 2 email notifications - assert_equal 2, ActionMailer::Base.deliveries.size - mail = ActionMailer::Base.deliveries.first - assert_not_nil mail - assert mail.subject.starts_with?( - "[#{fixed_issue.project.name} - #{fixed_issue.tracker.name} ##{fixed_issue.id}]") - assert_mail_body_match( - "Status changed from #{old_status} to #{fixed_issue.status}", mail) + # 5 email notifications, 2 for #1, 3 for #2 + assert_equal 5, ActionMailer::Base.deliveries.size + ActionMailer::Base.deliveries.first(2).each do |mail| + assert_not_nil mail + assert mail.subject.starts_with?( + "[#{fixed_issue.project.name} - #{fixed_issue.tracker.name} ##{fixed_issue.id}]") + assert_mail_body_match( + "Status changed from #{old_status} to #{fixed_issue.status}", mail) + end # ignoring commits referencing an issue of another project assert_equal [], Issue.find(4).changesets diff --git a/test/unit/wiki_content_test.rb b/test/unit/wiki_content_test.rb index d3cb74427..1b6dcf095 100644 --- a/test/unit/wiki_content_test.rb +++ b/test/unit/wiki_content_test.rb @@ -53,8 +53,10 @@ class WikiContentTest < ActiveSupport::TestCase assert page.save end - assert_equal 1, ActionMailer::Base.deliveries.size - assert_include 'wiki page has been added', mail_body(ActionMailer::Base.deliveries.last) + assert_equal 2, ActionMailer::Base.deliveries.size + ActionMailer::Base.deliveries.each do |mail| + assert_include 'wiki page has been added', mail_body(mail) + end end def test_update_should_be_versioned @@ -100,8 +102,10 @@ class WikiContentTest < ActiveSupport::TestCase assert content.save end - assert_equal 1, ActionMailer::Base.deliveries.size - assert_include 'wiki page has been updated', mail_body(ActionMailer::Base.deliveries.last) + assert_equal 2, ActionMailer::Base.deliveries.size + ActionMailer::Base.deliveries.each do |mail| + assert_include 'wiki page has been updated', mail_body(mail) + end end def test_fetch_history