From d5413b787666edf3d3653512c9dc8811eb609080 Mon Sep 17 00:00:00 2001 From: Jan Schulz-Hofen Date: Thu, 12 Dec 2019 17:10:47 +0100 Subject: [PATCH 4/4] Enable users to receive email notifications about high issues (only) --- app/models/issue.rb | 1 + app/models/user.rb | 4 ++ app/models/user_preference.rb | 7 ++++ app/views/users/_mail_notifications.html.erb | 9 ++++- config/locales/en.yml | 1 + test/functional/my_controller_test.rb | 37 ++++++++++++++++++ test/unit/issue_test.rb | 41 ++++++++++++++++++++ 7 files changed, 99 insertions(+), 1 deletion(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index efb55fafbb..63bc8cabe0 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1042,6 +1042,7 @@ class Issue < ActiveRecord::Base notified = notified.select {|u| u.active? && u.notify_about?(self)} notified += project.notified_users + notified += project.users.preload(:preference).select(&:notify_about_high_priority_issues?) if priority.high? notified.uniq! # Remove users that can not view the issue notified.reject! {|user| !visible?(user)} diff --git a/app/models/user.rb b/app/models/user.rb index e291bd98b8..a837697a45 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -811,6 +811,10 @@ class User < Principal end end + def notify_about_high_priority_issues? + self.pref.notify_about_high_priority_issues + end + def self.current=(user) RequestStore.store[:current_user] = user end diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 597ac9c901..98dead949a 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -33,6 +33,7 @@ class UserPreference < ActiveRecord::Base 'comments_sorting', 'warn_on_leaving_unsaved', 'no_self_notified', + 'notify_about_high_priority_issues', 'textarea_font', 'recently_used_projects', 'history_default_tab') @@ -51,6 +52,9 @@ class UserPreference < ActiveRecord::Base unless attributes && attributes.key?(:no_self_notified) self.no_self_notified = true end + unless attributes && attributes.key?(:notify_about_high_priority_issues) + self.notify_about_high_priority_issues = true + end end self.others ||= {} end @@ -87,6 +91,9 @@ class UserPreference < ActiveRecord::Base def no_self_notified; (self[:no_self_notified] == true || self[:no_self_notified] == '1'); end def no_self_notified=(value); self[:no_self_notified]=value; end + def notify_about_high_priority_issues; (self[:notify_about_high_priority_issues] == true || self[:notify_about_high_priority_issues] == '1'); end + def notify_about_high_priority_issues=(value); self[:notify_about_high_priority_issues]=value; end + def activity_scope; Array(self[:activity_scope]) ; end def activity_scope=(value); self[:activity_scope]=value ; end diff --git a/app/views/users/_mail_notifications.html.erb b/app/views/users/_mail_notifications.html.erb index 85842de56a..9cde170028 100644 --- a/app/views/users/_mail_notifications.html.erb +++ b/app/views/users/_mail_notifications.html.erb @@ -10,7 +10,7 @@ <%= content_tag 'fieldset', :id => 'notified-projects', :style => (@user.mail_notification == 'selected' ? '' : 'display:none;') do %> <%= toggle_checkboxes_link("#notified-projects input[type=checkbox]") %><%=l(:label_project_plural)%> <%= render_project_nested_lists(@user.projects) do |project| - content_tag('label', + content_tag('label', check_box_tag( 'user[notified_project_ids][]', project.id, @@ -24,6 +24,13 @@ <% end %> <%= fields_for :pref, @user.pref do |pref_fields| %> + +<% if IssuePriority.default_or_middle and high_priority = IssuePriority.where(['position > ?', IssuePriority.default_or_middle.position]).first %> +

+ <%= pref_fields.check_box :notify_about_high_priority_issues %> + +

+<% end %>

<%= pref_fields.check_box :no_self_notified %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 031c7ba875..d712ca88e6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -908,6 +908,7 @@ en: label_user_mail_option_only_assigned: "Only for things I watch or I am assigned to" label_user_mail_option_only_owner: "Only for things I watch or I am the owner of" label_user_mail_no_self_notified: "I don't want to be notified of changes that I make myself" + label_user_mail_notify_about_high_priority_issues_html: "Also notify me about issues with a priority of %{prio} or higher" label_registration_activation_by_email: account activation by email label_registration_manual_activation: manual account activation label_registration_automatic_activation: automatic account activation diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb index d5796b5518..52dbcb85ae 100644 --- a/test/functional/my_controller_test.rb +++ b/test/functional/my_controller_test.rb @@ -428,6 +428,43 @@ class MyControllerTest < Redmine::ControllerTest assert [mail.bcc, mail.cc].flatten.include?('foobar@example.com') end + def test_my_account_notify_about_high_priority_issues_preference + + # normally, preference should be shown + get :account + assert_select 'label[for="pref_notify_about_high_priority_issues"]' + + # preference should be persisted + put :account, :params => { + :pref => { + notify_about_high_priority_issues: '1' + } + } + assert User.find(2).notify_about_high_priority_issues? + + # preference should be hidden if there aren't any priorities + Issue.destroy_all + IssuePriority.destroy_all + get :account + assert_select 'label[for="pref_notify_about_high_priority_issues"]', false + + # preference should be hidden if there isn't a "high" priority + a = IssuePriority.create! name: 'A' + get :account + assert_select 'label[for="pref_notify_about_high_priority_issues"]', false + + # preference should be shown if there are at least two priorities (one low, one high) + b = IssuePriority.create! name: 'B' + get :account + assert_select 'label[for="pref_notify_about_high_priority_issues"]' + + # preference should be hidden if the highest priority is the default one, + # because that means that there is no "high" priority + b.update! is_default: true + get :account + assert_select 'label[for="pref_notify_about_high_priority_issues"]', false + end + def test_my_account_should_show_destroy_link get :account assert_select 'a[href="/my/account/destroy"]' diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 3203e2a3fa..346208d005 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2928,6 +2928,47 @@ class IssueTest < ActiveSupport::TestCase assert !issue.recipients.include?(issue.assigned_to.mail) end + test "Issue#recipients should include users who want to be notified about high issues but only when issue has high priority" do + user = User.generate! + user.pref.update! notify_about_high_priority_issues: true + Member.create!(:project_id => 1, :principal => user, :role_ids => [1]) + + # creation with high prio + issue = Issue.generate!(priority: IssuePriority.find(6)) + assert issue.recipients.include?(user.mail) + + # creation with default prio + issue = Issue.generate! + assert !issue.recipients.include?(user.mail) + + # update prio to high + issue.update! priority: IssuePriority.find(6) + assert issue.recipients.include?(user.mail) + + # update prio to low + issue.update! priority: IssuePriority.find(4) + assert !issue.recipients.include?(user.mail) + end + + test "Authors who don't want to be self-notified should not receive emails even when issue has high priority" do + user = User.generate! + user.pref.update! notify_about_high_priority_issues: true + user.pref.update! no_self_notified: true + + project = Project.find(1) + project.memberships.destroy_all + Member.create!(:project_id => 1, :principal => user, :role_ids => [1]) + + ActionMailer::Base.deliveries.clear + Issue.create(author: user, + priority: IssuePriority.find(6), + subject: 'test create', + project: project, + tracker: Tracker.first, + status: IssueStatus.first) + assert ActionMailer::Base.deliveries.empty? + end + def test_last_journal_id_with_journals_should_return_the_journal_id assert_equal 2, Issue.find(1).last_journal_id end -- 2.17.0