From 247cc24d513ac212c2940bc825afad6fcf56d9ff Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Mon, 8 Nov 2021 08:42:43 +0100 Subject: [PATCH] Add notification reason --- app/models/issue.rb | 1 + app/models/mailer.rb | 88 ++++++++---- app/models/news.rb | 4 +- app/models/project.rb | 4 +- app/models/user.rb | 1 + app/models/user_notification_reason.rb | 44 ++++++ app/views/layouts/mailer.html.erb | 12 +- app/views/layouts/mailer.text.erb | 6 +- config/locales/en.yml | 7 + config/settings.yml | 4 +- .../lib/acts_as_watchable.rb | 1 + lib/redmine/acts/mentionable.rb | 1 + test/test_helper.rb | 36 +++++ test/unit/comment_test.rb | 8 ++ test/unit/mailer_test.rb | 126 +++++++++++++++--- test/unit/message_test.rb | 15 ++- test/unit/news_test.rb | 7 + 17 files changed, 312 insertions(+), 53 deletions(-) create mode 100644 app/models/user_notification_reason.rb diff --git a/app/models/issue.rb b/app/models/issue.rb index 17802d227..35cc91d28 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1120,6 +1120,7 @@ class Issue < ActiveRecord::Base notified = notified.map {|n| n.is_a?(Group) ? n.users : n}.flatten notified.uniq! notified = notified.select {|u| u.active? && u.notify_about?(self)} + notified.map {|user| user.notification_reason = UserNotificationReason::INVOLVED} notified += project.notified_users notified += project.users.preload(:preference).select(&:notify_about_high_priority_issues?) if priority.high? diff --git a/app/models/mailer.rb b/app/models/mailer.rb index 2fe9673c9..72183e30e 100644 --- a/app/models/mailer.rb +++ b/app/models/mailer.rb @@ -78,18 +78,20 @@ class Mailer < ActionMailer::Base end # Builds a mail for notifying user about a new issue - def issue_add(user, issue) + def issue_add(user, issue, reason = nil) redmine_headers 'Project' => issue.project.identifier, 'Issue-Tracker' => issue.tracker.name, 'Issue-Id' => issue.id, 'Issue-Author' => issue.author.login, 'Issue-Assignee' => assignee_for_header(issue) redmine_headers 'Issue-Priority' => issue.priority.name if issue.priority + redmine_headers 'Reason' => reason if reason message_id issue references issue @author = issue.author @issue = issue @user = user + @reason = reason @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue) subject = "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}]" subject += " (#{issue.status.name})" if Setting.show_status_changes_in_mail_subject? @@ -103,14 +105,16 @@ class Mailer < ActionMailer::Base # Example: # Mailer.deliver_issue_add(issue) def self.deliver_issue_add(issue) - users = issue.notified_users | issue.notified_watchers | issue.notified_mentions + users = issue.notified_users + issue.notified_watchers + issue.notified_mentions + users = UserNotificationReason.reorder_by_priority(users) + users.each do |user| - issue_add(user, issue).deliver_later + issue_add(user, issue, user.notification_reason).deliver_later end end # Builds a mail for notifying user about an issue update - def issue_edit(user, journal) + def issue_edit(user, journal, reason = nil) issue = journal.journalized redmine_headers 'Project' => issue.project.identifier, 'Issue-Tracker' => issue.tracker.name, @@ -118,6 +122,7 @@ class Mailer < ActionMailer::Base 'Issue-Author' => issue.author.login, 'Issue-Assignee' => assignee_for_header(issue) redmine_headers 'Issue-Priority' => issue.priority.name if issue.priority + redmine_headers 'Reason' => reason if reason message_id journal references issue @author = journal.user @@ -126,6 +131,7 @@ class Mailer < ActionMailer::Base s += issue.subject @issue = issue @user = user + @reason = reason @journal = journal @journal_details = journal.visible_details @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue, :anchor => "change-#{journal.id}") @@ -139,21 +145,25 @@ class Mailer < ActionMailer::Base # Example: # Mailer.deliver_issue_edit(journal) def self.deliver_issue_edit(journal) - users = journal.notified_users | journal.notified_watchers | journal.notified_mentions | journal.journalized.notified_mentions + users = journal.notified_users + journal.notified_mentions + journal.journalized.notified_mentions + journal.notified_watchers + users = UserNotificationReason.reorder_by_priority(users) + users.select! do |user| journal.notes? || journal.visible_details(user).any? end users.each do |user| - issue_edit(user, journal).deliver_later + issue_edit(user, journal, user.notification_reason).deliver_later end end # Builds a mail to user about a new document. - def document_added(user, document, author) + def document_added(user, document, author, reason = nil) redmine_headers 'Project' => document.project.identifier + redmine_headers 'Reason' => reason if reason @author = author @document = document @user = user + @reason = reason @document_url = url_for(:controller => 'documents', :action => 'show', :id => document) mail :to => user, :subject => "[#{document.project.name}] #{l(:label_document_new)}: #{document.title}" @@ -166,12 +176,12 @@ class Mailer < ActionMailer::Base def self.deliver_document_added(document, author) users = document.notified_users users.each do |user| - document_added(user, document, author).deliver_later + document_added(user, document, author, user.notification_reason).deliver_later end end # Builds a mail to user about new attachements. - def attachments_added(user, attachments) + def attachments_added(user, attachments, reason = nil) container = attachments.first.container added_to = '' added_to_url = '' @@ -188,8 +198,10 @@ class Mailer < ActionMailer::Base added_to = "#{l(:label_document)}: #{container.title}" end redmine_headers 'Project' => container.project.identifier + redmine_headers 'Reason' => reason if reason @attachments = attachments @user = user + @reason = reason @added_to = added_to @added_to_url = added_to_url mail :to => user, @@ -210,18 +222,20 @@ class Mailer < ActionMailer::Base end users.each do |user| - attachments_added(user, attachments).deliver_later + attachments_added(user, attachments, user.notification_reason).deliver_later end end # Builds a mail to user about a new news. - def news_added(user, news) + def news_added(user, news, reason = nil) redmine_headers 'Project' => news.project.identifier + redmine_headers 'Reason' => reason if reason @author = news.author message_id news references news @news = news @user = user + @reason = reason @news_url = url_for(:controller => 'news', :action => 'show', :id => news) mail :to => user, :subject => "[#{news.project.name}] #{l(:label_news)}: #{news.title}" @@ -232,22 +246,26 @@ class Mailer < ActionMailer::Base # Example: # Mailer.deliver_news_added(news) def self.deliver_news_added(news) - users = news.notified_users | news.notified_watchers_for_added_news + users = news.notified_users + news.notified_watchers_for_added_news + users = UserNotificationReason.reorder_by_priority(users) + users.each do |user| - news_added(user, news).deliver_later + news_added(user, news, user.notification_reason).deliver_later end end # Builds a mail to user about a new news comment. - def news_comment_added(user, comment) + def news_comment_added(user, comment, reason = nil) news = comment.commented redmine_headers 'Project' => news.project.identifier + redmine_headers 'Reason' => reason if reason @author = comment.author message_id comment references news @news = news @comment = comment @user = user + @reason = reason @news_url = url_for(:controller => 'news', :action => 'show', :id => news) mail :to => user, :subject => "Re: [#{news.project.name}] #{l(:label_news)}: #{news.title}" @@ -259,21 +277,25 @@ class Mailer < ActionMailer::Base # Mailer.deliver_news_comment_added(comment) def self.deliver_news_comment_added(comment) news = comment.commented - users = news.notified_users | news.notified_watchers + users = news.notified_users + news.notified_watchers + users = UserNotificationReason.reorder_by_priority(users) + users.each do |user| - news_comment_added(user, comment).deliver_later + news_comment_added(user, comment, user.notification_reason).deliver_later end end # Builds a mail to user about a new message. - def message_posted(user, message) + def message_posted(user, message, reason = nil) redmine_headers 'Project' => message.project.identifier, 'Topic-Id' => (message.parent_id || message.id) + redmine_headers 'Reason' => reason if reason @author = message.author message_id message references message.root @message = message @user = user + @reason = reason @message_url = url_for(message.event_url) mail :to => user, :subject => "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] #{message.subject}" @@ -285,22 +307,25 @@ class Mailer < ActionMailer::Base # Mailer.deliver_message_posted(message) def self.deliver_message_posted(message) users = message.notified_users - users |= message.root.notified_watchers - users |= message.board.notified_watchers + users += message.root.notified_watchers + users += message.board.notified_watchers + users = UserNotificationReason.reorder_by_priority(users) users.each do |user| - message_posted(user, message).deliver_later + message_posted(user, message, user.notification_reason).deliver_later end end # Builds a mail to user about a new wiki content. - def wiki_content_added(user, wiki_content) + def wiki_content_added(user, wiki_content, reason = nil) redmine_headers 'Project' => wiki_content.project.identifier, 'Wiki-Page-Id' => wiki_content.page.id + redmine_headers 'Reason' => reason if reason @author = wiki_content.author message_id wiki_content @wiki_content = wiki_content @user = user + @reason = reason @wiki_content_url = url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, :id => wiki_content.page.title) @@ -316,20 +341,24 @@ class Mailer < ActionMailer::Base # Example: # Mailer.deliver_wiki_content_added(wiki_content) def self.deliver_wiki_content_added(wiki_content) - users = wiki_content.notified_users | wiki_content.page.wiki.notified_watchers | wiki_content.notified_mentions + users = wiki_content.notified_users + wiki_content.page.wiki.notified_watchers + wiki_content.notified_mentions + users = UserNotificationReason.reorder_by_priority(users) + users.each do |user| - wiki_content_added(user, wiki_content).deliver_later + wiki_content_added(user, wiki_content, user.notification_reason).deliver_later end end # Builds a mail to user about an update of the specified wiki content. - def wiki_content_updated(user, wiki_content) + def wiki_content_updated(user, wiki_content, reason = nil) redmine_headers 'Project' => wiki_content.project.identifier, 'Wiki-Page-Id' => wiki_content.page.id + redmine_headers 'Reason' => reason if reason @author = wiki_content.author message_id wiki_content @wiki_content = wiki_content @user = user + @reason = reason @wiki_content_url = url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, @@ -351,12 +380,13 @@ class Mailer < ActionMailer::Base # Mailer.deliver_wiki_content_updated(wiki_content) def self.deliver_wiki_content_updated(wiki_content) users = wiki_content.notified_users - users |= wiki_content.page.notified_watchers - users |= wiki_content.page.wiki.notified_watchers - users |= wiki_content.notified_mentions + users += wiki_content.page.notified_watchers + users += wiki_content.page.wiki.notified_watchers + users += wiki_content.notified_mentions + users = UserNotificationReason.reorder_by_priority(users) users.each do |user| - wiki_content_updated(user, wiki_content).deliver_later + wiki_content_updated(user, wiki_content, user.notification_reason).deliver_later end end @@ -485,6 +515,7 @@ class Mailer < ActionMailer::Base field: (options[:field] && l(options[:field])), value: options[:value]) @title = options[:title] && l(options[:title]) + @reason = UserNotificationReason::ADMIN @remote_ip = options[:remote_ip] || @sender.remote_ip @url = options[:url] && (options[:url].is_a?(Hash) ? url_for(options[:url]) : options[:url]) redmine_headers 'Url' => @url @@ -521,6 +552,7 @@ class Mailer < ActionMailer::Base # Build a mail to user about application settings changes made by sender. def settings_updated(user, sender, changes, options={}) @sender = sender + @reason = UserNotificationReason::ADMIN redmine_headers 'Sender' => sender.login @changes = changes @remote_ip = options[:remote_ip] || @sender.remote_ip diff --git a/app/models/news.rb b/app/models/news.rb index 8860d6c97..2dea67fd7 100644 --- a/app/models/news.rb +++ b/app/models/news.rb @@ -56,7 +56,9 @@ class News < ActiveRecord::Base end def notified_users - project.users.select {|user| user.notify_about?(self) && user.allowed_to?(:view_news, project)} + users = project.users.select {|user| user.notify_about?(self) && user.allowed_to?(:view_news, project)} + users.map {|user| user.notification_reason = UserNotificationReason::SUBSCRIBED} + users end def recipients diff --git a/app/models/project.rb b/app/models/project.rb index 58346d373..539bd0346 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -626,7 +626,9 @@ class Project < ActiveRecord::Base # Returns the users that should be notified on project events def notified_users - users.where('members.mail_notification = ? OR users.mail_notification = ?', true, 'all') + notified_users = users.where('members.mail_notification = ? OR users.mail_notification = ?', true, 'all') + notified_users.map {|user| user.notification_reason = UserNotificationReason::SUBSCRIBED} + notified_users end # Returns a scope of all custom fields enabled for project issues diff --git a/app/models/user.rb b/app/models/user.rb index cd923e46b..1dcbcb2f4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -102,6 +102,7 @@ class User < Principal attr_accessor :password, :password_confirmation, :generate_password attr_accessor :last_before_login_on attr_accessor :remote_ip + attr_accessor :notification_reason LOGIN_LENGTH_LIMIT = 60 MAIL_LENGTH_LIMIT = 254 diff --git a/app/models/user_notification_reason.rb b/app/models/user_notification_reason.rb new file mode 100644 index 000000000..262ede0cd --- /dev/null +++ b/app/models/user_notification_reason.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006-2023 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class UserNotificationReason + INVOLVED = 'involved' + MENTIONED = 'mentioned' + WATCHER = 'watcher' + SUBSCRIBED = 'subscribed' + ADMIN = 'admin' + + REASON_PRIORITY = [ + INVOLVED, + MENTIONED, + WATCHER, + SUBSCRIBED, + ADMIN + ].freeze + + def self.reorder_by_priority(users) + users = users.sort_by { |r| priority(r.notification_reason) } + users.uniq! + users + end + + def self.priority(reason) + REASON_PRIORITY.index(reason) || REASON_PRIORITY.length + 1 + end +end diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index fff36c0cb..a6c889e38 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -75,8 +75,14 @@ table, td, th { <% end -%> <%= yield %>
-<% if Setting.emails_footer.present? -%> -<%= Redmine::WikiFormatting.to_html(Setting.text_formatting, Setting.emails_footer).html_safe %> -<% end -%> + + <% if @reason %> + <%= l("text_notification_reason.#{@reason}") %>
+ <% end %> + <%= l(:text_notification_settings, link: link_to(nil, url_for(controller: 'my', action: 'account'))).html_safe %> + <% if Setting.emails_footer.present? -%> + <%= Redmine::WikiFormatting.to_html(Setting.text_formatting, Setting.emails_footer).html_safe %> + <% end -%> + diff --git a/app/views/layouts/mailer.text.erb b/app/views/layouts/mailer.text.erb index a4c69862b..434f4f734 100644 --- a/app/views/layouts/mailer.text.erb +++ b/app/views/layouts/mailer.text.erb @@ -2,7 +2,11 @@ <%= Setting.emails_header %> <% end -%> <%= yield %> -<% if Setting.emails_footer.present? -%> -- +<% if @reason %> +<%= l("text_notification_reason.#{@reason}") %> +<% end %> +<%= l(:text_notification_settings, link: url_for(controller: 'my', action: 'account')) %> +<% if Setting.emails_footer.present? -%> <%= Setting.emails_footer %> <% end -%> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2bbd776a3..1889808dd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1414,3 +1414,10 @@ en: text_user_destroy_confirmation: "Are you sure you want to delete this user and remove all references to them? This cannot be undone. Often, locking a user instead of deleting them is the better solution. To confirm, please enter their login (%{login}) below." text_project_destroy_enter_identifier: "To confirm, please enter the project's identifier (%{identifier}) below." field_name_or_email_or_login: Name, email or login + text_notification_reason: + admin: You have received this security notification because you are an administrator. + subscribed: You have received this notification because you have subscribed to it. + involved: You have received this notification because you are involved in. + watcher: You have received this notification because you are watching it. + mentioned: You have received this notification because you have been mentioned in. + text_notification_settings: "To change your notification preferences, please click here: %{link}." diff --git a/config/settings.yml b/config/settings.yml index 062b25d19..773923612 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -298,9 +298,7 @@ repository_log_display_limit: ui_theme: default: '' emails_footer: - default: |- - You have received this notification because you have either subscribed to it, or are involved in it. - To change your notification preferences, please click here: http://hostname/my/account + default: '' gravatar_enabled: default: 0 gravatar_default: diff --git a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb index b6d4ba455..034ab910a 100644 --- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb +++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb @@ -118,6 +118,7 @@ module Redmine if respond_to?(:visible?) notified.reject! {|user| !visible?(user)} end + notified.map {|user| user.notification_reason = UserNotificationReason::WATCHER} notified end diff --git a/lib/redmine/acts/mentionable.rb b/lib/redmine/acts/mentionable.rb index f8332637e..9d7a0e57e 100644 --- a/lib/redmine/acts/mentionable.rb +++ b/lib/redmine/acts/mentionable.rb @@ -48,6 +48,7 @@ module Redmine if respond_to?(:visible?) notified.select! {|user| visible?(user)} end + notified.map {|user| user.notification_reason = UserNotificationReason::MENTIONED} notified end diff --git a/test/test_helper.rb b/test/test_helper.rb index 23ba199fa..0a3ef5006 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -285,6 +285,42 @@ class ActiveSupport::TestCase (mail.multipart? ? mail.parts.first : mail).body.encoded end + # Verifies mail notification deliveries match the expected deliveries and the reasons + def assert_mail_deliveries_reasons(expected, deliveries) + # First assert that number of deliveries match the number of expected deliveries + assert_equal expected.size, deliveries.size + + deliveries.each do |mail| + assert_not_nil mail + + # Assert that the email is sent to only one recipient + assert_equal 1, mail.to.size + + recipient = mail.to.first + + # Assert that the actual recipient is part of the expected recipients + assert expected[recipient] + + # Get expected notification reason + reason = expected[recipient] + # Remove recipient from the expected hash + expected.delete(recipient) + + mail_html_part = mail.parts.detect {|part| part.content_type.include?('text/html')} + mail_text_part = mail.parts.detect {|part| part.content_type.include?('text/html')} + + # Assert mail body contains the expected notification reason for both parts (html / text) + assert_include l("text_notification_reason.#{reason}"), mail_html_part.body.encoded + assert_include l("text_notification_reason.#{reason}"), mail_text_part.body.encoded + + # Assert reason header + assert_equal reason, mail.header['X-Redmine-Reason'].to_s + end + + # Assert that we iterate through all the expected recipients + assert_equal 0, expected.size + end + # Returns the lft value for a new root issue def new_issue_lft 1 diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index 68ae898f1..fac37f055 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -20,6 +20,7 @@ require_relative '../test_helper' class CommentTest < ActiveSupport::TestCase + include Redmine::I18n fixtures :users, :email_addresses, :news, :comments, :projects, :enabled_modules, :user_preferences, :roles, :members, :member_roles @@ -44,6 +45,13 @@ class CommentTest < ActiveSupport::TestCase Comment.create!(:commented => @news, :author => @jsmith, :comments => "my comment") end end + + recipients = { + "jsmith@somenet.foo" => "watcher", + "dlopper@somenet.foo" => "subscribed", + } + + assert_mail_deliveries_reasons(recipients, ActionMailer::Base.deliveries) end def test_validate diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index 889035a27..5f10ddbf4 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -486,8 +486,6 @@ class MailerTest < ActiveSupport::TestCase # @jsmith and @dlopper are members of the project # admin is mentioned # @dlopper won't receive duplicated notifications - assert_equal 3, ActionMailer::Base.deliveries.size - assert_include User.find(1).mail, recipients end def test_issue_add_should_include_enabled_fields @@ -543,6 +541,17 @@ class MailerTest < ActiveSupport::TestCase end end + def test_issue_add_should_contain_notification_reason + issue = Issue.find(1) + assert Mailer.deliver_issue_add(issue) + + recipients = { + "jsmith@somenet.foo" => "involved", + "dlopper@somenet.foo" => "subscribed", + } + assert_mail_deliveries_reasons(recipients, ActionMailer::Base.deliveries) + end + def test_issue_edit_subject_should_include_status_changes_if_setting_is_enabled with_settings :show_status_changes_in_mail_subject => 1 do issue = Issue.find(2) @@ -645,11 +654,12 @@ class MailerTest < ActiveSupport::TestCase ActionMailer::Base.deliveries.clear Mailer.deliver_issue_edit(journal) - # @jsmith and @dlopper are members of the project - # admin is mentioned in the updated description - # @dlopper won't receive duplicated notifications - assert_equal 3, ActionMailer::Base.deliveries.size - assert_include User.find(1).mail, recipients + recipients = { + "jsmith@somenet.foo" => "involved", + "dlopper@somenet.foo" => "involved", + "admin@somenet.foo" => "mentioned", + } + assert_mail_deliveries_reasons(recipients, ActionMailer::Base.deliveries) end def test_issue_edit_should_notify_mentioned_users_in_notes @@ -665,6 +675,27 @@ class MailerTest < ActiveSupport::TestCase # @dlopper won't receive duplicated notifications assert_equal 3, ActionMailer::Base.deliveries.size assert_include User.find(1).mail, recipients + + recipients = { + "jsmith@somenet.foo" => "involved", + "dlopper@somenet.foo" => "involved", + "admin@somenet.foo" => "mentioned", + } + assert_mail_deliveries_reasons(recipients, ActionMailer::Base.deliveries) + end + + def test_issue_edit_should_contain_notification_reason + journal = Journal.generate!(journalized: Issue.find(2), user: User.find(1), notes: 'Hello') + + ActionMailer::Base.deliveries.clear + Mailer.deliver_issue_edit(journal) + + # assert notification reason + assert_include "You have received this notification because you are watching it.", text_part.body.encoded + assert_include "You have received this notification because you are watching it.", html_part.body.encoded + + # Assert notification reason + assert_equal 'watcher', last_email.header['X-Redmine-Reason'].to_s end def test_issue_should_send_email_notification_with_suppress_empty_fields @@ -714,7 +745,12 @@ class MailerTest < ActiveSupport::TestCase assert last_email.to.any? assert_select_email do assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files" + assert_select "span[class=?] span", "footer", text: "You have received this notification because you have subscribed to it." end + assert_include "You have received this notification because you have subscribed to it.", text_part.body.encoded + + # Assert notification reason header + assert_equal 'subscribed', last_email.header['X-Redmine-Reason'].to_s end def test_project_file_added @@ -724,7 +760,47 @@ class MailerTest < ActiveSupport::TestCase assert last_email.to.any? assert_select_email do assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files" + assert_select "span[class=?] span", "footer", text: "You have received this notification because you have subscribed to it." end + assert_include "You have received this notification because you have subscribed to it.", text_part.body.encoded + + # Assert notification reason header + assert_equal 'subscribed', last_email.header['X-Redmine-Reason'].to_s + end + + def test_document_file_added + attachements = [Attachment.find_by_container_type('Document')] + + assert Mailer.deliver_attachments_added(attachements) + assert_not_nil last_email.to + assert last_email.to.any? + assert_select_email do + assert_select "a[href=?]", "http://localhost:3000/documents/1" + assert_select "span[class=?] span", "footer", text: "You have received this notification because you have subscribed to it." + end + assert_include "You have received this notification because you have subscribed to it.", text_part.body.encoded + + # Assert notification reason header + assert_equal 'subscribed', last_email.header['X-Redmine-Reason'].to_s + end + + def test_document_added + project = Project.find(1) + doc = Document.new(:project => project, :title => 'New document', :category => Enumeration.find_by_name('User documentation')) + doc.save + + assert Mailer.deliver_document_added(doc, User.find_by_id(3)) + assert_not_nil last_email.to + assert last_email.to.any? + assert_select_email do + assert_select "a[href=?]", "http://localhost:3000/documents/#{doc.id}" + assert_select "span[class=?] span", "footer", text: "You have received this notification because you have subscribed to it." + end + assert_include "You have received this notification because you have subscribed to it.", text_part.body.encoded + + # Assert headers + assert_equal project.identifier, last_email.header['X-Redmine-Project'].to_s + assert_equal 'subscribed', last_email.header['X-Redmine-Reason'].to_s end def test_news_added_should_notify_project_news_watchers @@ -759,7 +835,13 @@ class MailerTest < ActiveSupport::TestCase assert_select 'a[href=?]', 'http://localhost:3000/projects/ecookbook/wiki/CookBook_documentation', :text => 'CookBook documentation' + assert_select "span[class=?] span", 'footer', text: "You have received this notification because you have subscribed to it." end + + assert_include "You have received this notification because you have subscribed to it.", text_part.body.encoded + + # Assert headers + assert_equal 'subscribed', last_email.header['X-Redmine-Reason'].to_s end end @@ -785,6 +867,14 @@ class MailerTest < ActiveSupport::TestCase 'http://localhost:3000/projects/ecookbook/wiki/CookBook_documentation', :text => 'CookBook documentation' end + + recipients = { + "admin@somenet.foo" => "watcher", + "jsmith@somenet.foo" => "subscribed", + "dlopper@somenet.foo" => "subscribed", + } + + assert_mail_deliveries_reasons(recipients, ActionMailer::Base.deliveries) end def test_wiki_content_updated_should_notify_mentioned_users_in_updated_content @@ -981,7 +1071,7 @@ class MailerTest < ActiveSupport::TestCase def test_security_notification set_language_if_valid User.find(1).language - with_settings :emails_footer => "footer without link" do + with_settings :emails_footer => "footer without extra link" do sender = User.find(2) sender.remote_ip = '192.168.1.1' assert( @@ -996,8 +1086,11 @@ class MailerTest < ActiveSupport::TestCase assert_mail_body_match '192.168.1.1', mail assert_mail_body_match I18n.t(:notice_account_password_updated), mail assert_select_email do - assert_select "h1", false - assert_select "a", false + assert_select 'span[class=?]', 'footer' do + assert_select 'span', text: 'You have received this security notification because you are an administrator.' + assert_select 'a', true + assert_select 'p', text: "footer without extra link" + end end end end @@ -1031,7 +1124,7 @@ class MailerTest < ActiveSupport::TestCase ) ) assert_select_email do - assert_select "a", false + assert_select "a", true assert_select "h1", :text => I18n.t(:label_my_account) end end @@ -1117,7 +1210,7 @@ class MailerTest < ActiveSupport::TestCase with_settings :plain_text_mail => 0 do assert Mailer.test_email(User.find(1)).deliver_now assert_select_email do - assert_select ".footer" do + assert_select "span[class=?]", "footer" do assert_select "strong", :text => "Footer content" end end @@ -1126,6 +1219,7 @@ class MailerTest < ActiveSupport::TestCase assert Mailer.test_email(User.find(1)).deliver_now mail = last_email assert_include "\n-- \n", mail.body.decoded + assert_include "To change your notification preferences, please click here: http://localhost:3000/my/account.", mail.body.decoded assert_include "*Footer content*", mail.body.decoded end end @@ -1136,13 +1230,15 @@ class MailerTest < ActiveSupport::TestCase with_settings :plain_text_mail => 0 do assert Mailer.test_email(User.find(1)).deliver_now assert_select_email do - assert_select ".footer", false + assert_select 'span[class=?]', 'footer' do + assert_select 'p', false + end end end with_settings :plain_text_mail => 1 do assert Mailer.test_email(User.find(1)).deliver_now - mail = last_email - assert_not_include "\n-- \n", mail.body.decoded + assert_equal "This is a test email sent by Redmine.\nRedmine URL: http://localhost:3000/\n\n-- \nTo change your notification preferences, please click here: http://localhost:3000/my/account.\n", +last_email.body.decoded end end end diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb index 82446c0a3..190f4c02b 100644 --- a/test/unit/message_test.rb +++ b/test/unit/message_test.rb @@ -20,6 +20,7 @@ require_relative '../test_helper' class MessageTest < ActiveSupport::TestCase + include Redmine::I18n fixtures :projects, :roles, :members, :member_roles, :boards, :messages, :users, :watchers, :enabled_modules @@ -36,7 +37,11 @@ class MessageTest < ActiveSupport::TestCase message = Message.new(:board => @board, :subject => 'Test message', :content => 'Test message content', :author => @user) - assert message.save + + with_settings :notified_events => %w(message_posted) do + assert message.save + end + @board.reload # topics count incremented assert_equal topics_count + 1, @board[:topics_count] @@ -45,6 +50,14 @@ class MessageTest < ActiveSupport::TestCase assert_equal message, @board.last_message # author should be watching the message assert message.watched_by?(@user) + + expected = { + "admin@somenet.foo" => "watcher", + "jsmith@somenet.foo" => "subscribed", + "dlopper@somenet.foo" => "subscribed" + } + + assert_mail_deliveries_reasons(expected, ActionMailer::Base.deliveries) end def test_reply diff --git a/test/unit/news_test.rb b/test/unit/news_test.rb index a9d39c60b..95ff86b15 100644 --- a/test/unit/news_test.rb +++ b/test/unit/news_test.rb @@ -20,6 +20,7 @@ require_relative '../test_helper' class NewsTest < ActiveSupport::TestCase + include Redmine::I18n fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles, :enabled_modules, :news def valid_news @@ -38,6 +39,12 @@ class NewsTest < ActiveSupport::TestCase assert news.save end assert_equal 2, ActionMailer::Base.deliveries.size + + expected = { + "jsmith@somenet.foo" => "subscribed", + "dlopper@somenet.foo" => "subscribed" + } + assert_mail_deliveries_reasons(expected, ActionMailer::Base.deliveries) end def test_should_include_news_for_projects_with_news_enabled -- 2.39.3 (Apple Git-145)