From b8334c8249b8e2cf0ff1a1eadbadc3b780b7ec17 Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Sun, 17 Feb 2019 16:45:47 +0000 Subject: [PATCH] Remove setting 'Blind carbon copy recipients (bcc)' --- app/models/mailer.rb | 7 -- app/views/settings/_notifications.html.erb | 2 - config/settings.yml | 2 - ...20190217164229_remove_bcc_recipients_setting.rb | 5 ++ test/functional/account_controller_test.rb | 16 ++--- test/functional/admin_controller_test.rb | 2 +- test/functional/email_addresses_controller_test.rb | 8 +-- test/functional/issues_controller_test.rb | 2 +- .../issues_custom_fields_visibility_test.rb | 79 +++++++++++----------- test/functional/messages_controller_test.rb | 4 +- test/functional/my_controller_test.rb | 4 +- test/functional/settings_controller_test.rb | 5 +- test/functional/users_controller_test.rb | 23 +++---- test/unit/issue_test.rb | 2 +- test/unit/mailer_test.rb | 24 +++---- 15 files changed, 84 insertions(+), 101 deletions(-) create mode 100644 db/migrate/20190217164229_remove_bcc_recipients_setting.rb diff --git a/app/models/mailer.rb b/app/models/mailer.rb index 2364774..fc3787a 100644 --- a/app/models/mailer.rb +++ b/app/models/mailer.rb @@ -651,13 +651,6 @@ class Mailer < ActionMailer::Base redmine_headers 'Sender' => @author.login end - # Blind carbon copy recipients - if Setting.bcc_recipients? - headers[:bcc] = [headers[:to], headers[:cc]].flatten.uniq.reject(&:blank?) - headers[:to] = nil - headers[:cc] = nil - end - if @message_id_object headers[:message_id] = "<#{self.class.message_id_for(@message_id_object)}>" end diff --git a/app/views/settings/_notifications.html.erb b/app/views/settings/_notifications.html.erb index 0f50514..67df9b7 100644 --- a/app/views/settings/_notifications.html.erb +++ b/app/views/settings/_notifications.html.erb @@ -4,8 +4,6 @@

<%= setting_text_field :mail_from, :size => 60 %>

-

<%= setting_check_box :bcc_recipients %>

-

<%= setting_check_box :plain_text_mail %>

diff --git a/config/settings.yml b/config/settings.yml index b6c0261..9c9a2f2 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -78,8 +78,6 @@ search_results_per_page: default: 10 mail_from: default: redmine@example.net -bcc_recipients: - default: 1 plain_text_mail: default: 0 text_formatting: diff --git a/db/migrate/20190217164229_remove_bcc_recipients_setting.rb b/db/migrate/20190217164229_remove_bcc_recipients_setting.rb new file mode 100644 index 0000000..1906ad8 --- /dev/null +++ b/db/migrate/20190217164229_remove_bcc_recipients_setting.rb @@ -0,0 +1,5 @@ +class RemoveBccRecipientsSetting < ActiveRecord::Migration[5.2] + def change + Setting.where(:name => 'bcc_recipients').delete_all + end +end diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index fbeb457..878888c 100644 --- a/test/functional/account_controller_test.rb +++ b/test/functional/account_controller_test.rb @@ -310,7 +310,7 @@ class AccountControllerTest < Redmine::ControllerTest :firstname => 'John', :lastname => 'Doe', :mail => 'register@example.com' - + } } assert_redirected_to '/my/account' @@ -324,7 +324,7 @@ class AccountControllerTest < Redmine::ControllerTest assert user.active? end end - + def test_post_register_with_registration_off_should_redirect with_settings :self_registration => '0' do assert_no_difference 'User.count' do @@ -336,7 +336,7 @@ class AccountControllerTest < Redmine::ControllerTest :firstname => 'John', :lastname => 'Doe', :mail => 'register@example.com' - + } } assert_redirected_to '/' @@ -355,11 +355,11 @@ class AccountControllerTest < Redmine::ControllerTest :firstname => 'John', :lastname => 'Doe', :mail => 'register@example.com' - - }, + + }, :pref => { :hide_mail => '1' - + } } end @@ -406,7 +406,7 @@ class AccountControllerTest < Redmine::ControllerTest end end mail = ActionMailer::Base.deliveries.last - assert_equal ['jsmith@somenet.foo'], mail.bcc + assert_equal ['jsmith@somenet.foo'], mail.to end def test_lost_password_using_additional_email_address_should_send_email_to_the_address @@ -422,7 +422,7 @@ class AccountControllerTest < Redmine::ControllerTest end end mail = ActionMailer::Base.deliveries.last - assert_equal ['anotherAddress@foo.bar'], mail.bcc + assert_equal ['anotherAddress@foo.bar'], mail.to end def test_lost_password_for_unknown_user_should_fail diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb index 3050d75..92af117 100644 --- a/test/functional/admin_controller_test.rb +++ b/test/functional/admin_controller_test.rb @@ -93,7 +93,7 @@ class AdminControllerTest < Redmine::ControllerTest mail = ActionMailer::Base.deliveries.last assert_not_nil mail user = User.find(1) - assert_equal [user.mail], mail.bcc + assert_equal [user.mail], mail.to end def test_test_email_failure_should_display_the_error diff --git a/test/functional/email_addresses_controller_test.rb b/test/functional/email_addresses_controller_test.rb index 5a31faf..8080872 100644 --- a/test/functional/email_addresses_controller_test.rb +++ b/test/functional/email_addresses_controller_test.rb @@ -133,8 +133,8 @@ class EmailAddressesControllerTest < Redmine::ControllerTest assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account' end # The old email address should be notified about a new address for security purposes - assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail) - assert [mail.bcc, mail.cc].flatten.include?('something@example.fr') + assert mail.to.include?(User.find(2).mail) + assert mail.to.include?('something@example.fr') end def test_update @@ -182,7 +182,7 @@ class EmailAddressesControllerTest < Redmine::ControllerTest assert_mail_body_match I18n.t(:mail_body_security_notification_notify_disabled, value: 'another@somenet.foo'), mail # The changed address should be notified for security purposes - assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo') + assert mail.to.include?('another@somenet.foo') end @@ -241,6 +241,6 @@ class EmailAddressesControllerTest < Redmine::ControllerTest assert_mail_body_match I18n.t(:mail_body_security_notification_remove, field: I18n.t(:field_mail), value: 'another@somenet.foo'), mail # The removed address should be notified for security purposes - assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo') + assert mail.to.include?('another@somenet.foo') end end diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 047c987..e101c4d 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -3419,7 +3419,7 @@ class IssuesControllerTest < Redmine::ControllerTest # Watchers notified mail = ActionMailer::Base.deliveries.last assert_not_nil mail - assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail) + assert mail.to.include?(User.find(3).mail) end def test_post_create_subissue diff --git a/test/functional/issues_custom_fields_visibility_test.rb b/test/functional/issues_custom_fields_visibility_test.rb index b1a6667..d34363d 100644 --- a/test/functional/issues_custom_fields_visibility_test.rb +++ b/test/functional/issues_custom_fields_visibility_test.rb @@ -172,8 +172,8 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest :id => @issue.id, :issue => { :custom_field_values => { - @field1.id.to_s => "User#{user.id}Value0", - @field2.id.to_s => "User#{user.id}Value1", + @field1.id.to_s => "User#{user.id}Value0", + @field2.id.to_s => "User#{user.id}Value1", @field3.id.to_s => "User#{user.id}Value2", } } @@ -268,30 +268,29 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest ActionMailer::Base.deliveries.clear @request.session[:user_id] = 1 - with_settings :bcc_recipients => '1' do assert_difference 'Issue.count' do - post :create, :params => { - :project_id => 1, - :issue => { - :tracker_id => 1, - :status_id => 1, - :subject => 'New issue', - :priority_id => 5, - :custom_field_values => { - @field1.id.to_s => 'Value0', @field2.id.to_s => 'Value1', @field3.id.to_s => 'Value2' - }, - :watcher_user_ids => users_to_test.keys.map(&:id) - - } + post :create, :params => { + :project_id => 1, + :issue => { + :tracker_id => 1, + :status_id => 1, + :subject => 'New issue', + :priority_id => 5, + :custom_field_values => { + @field1.id.to_s => 'Value0', @field2.id.to_s => 'Value1', @field3.id.to_s => 'Value2' + }, + :watcher_user_ids => users_to_test.keys.map(&:id) + } - assert_response 302 - end + } + + assert_response 302 end 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} + mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail} assert_equal 1, mails.size mail = mails.first @fields.each_with_index do |field, i| @@ -313,22 +312,21 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest end ActionMailer::Base.deliveries.clear @request.session[:user_id] = 1 - with_settings :bcc_recipients => '1' do - put :update, :params => { - :id => @issue.id, - :issue => { - :custom_field_values => { - @field1.id.to_s => 'NewValue0', @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2' - } - + put :update, :params => { + :id => @issue.id, + :issue => { + :custom_field_values => { + @field1.id.to_s => 'NewValue0', @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2' } + } - assert_response 302 - end + } + + assert_response 302 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} + mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail} assert_equal 1, mails.size mail = mails.first @fields.each_with_index do |field, i| @@ -350,20 +348,19 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest end ActionMailer::Base.deliveries.clear @request.session[:user_id] = 1 - with_settings :bcc_recipients => '1' do - put :update, :params => { - :id => @issue.id, - :issue => { - :custom_field_values => { - @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2' - } - + put :update, :params => { + :id => @issue.id, + :issue => { + :custom_field_values => { + @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2' } + } - assert_response 302 - end + } + assert_response 302 + users_to_test.each do |user, fields| - mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} + mails = ActionMailer::Base.deliveries.select {|m| m.to.include? user.mail} if (fields & [@field2, @field3]).any? assert_equal 1, mails.size, "User #{user.id} was not notified" else diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb index 715258f..5453f09 100644 --- a/test/functional/messages_controller_test.rb +++ b/test/functional/messages_controller_test.rb @@ -145,9 +145,9 @@ class MessagesControllerTest < Redmine::ControllerTest end # author - assert_equal ['jsmith@somenet.foo'], mails[0].bcc + assert_equal ['jsmith@somenet.foo'], mails[0].to # project member - assert_equal ['dlopper@somenet.foo'], mails[1].bcc + assert_equal ['dlopper@somenet.foo'], mails[1].to end def test_get_edit diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb index d8db74f..e962746 100644 --- a/test/functional/my_controller_test.rb +++ b/test/functional/my_controller_test.rb @@ -391,8 +391,8 @@ class MyControllerTest < Redmine::ControllerTest assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account' end # The old email address should be notified about the change for security purposes - assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail) - assert [mail.bcc, mail.cc].flatten.include?('foobar@example.com') + assert mail.to.include?(User.find(2).mail) + assert mail.to.include?('foobar@example.com') end def test_my_account_should_show_destroy_link diff --git a/test/functional/settings_controller_test.rb b/test/functional/settings_controller_test.rb index 2215aa3..5ad90ec 100644 --- a/test/functional/settings_controller_test.rb +++ b/test/functional/settings_controller_test.rb @@ -76,14 +76,12 @@ class SettingsControllerTest < Redmine::ControllerTest post :edit, :params => { :settings => { :mail_from => 'functional@test.foo', - :bcc_recipients => '0', :notified_events => %w(issue_added issue_updated news_added), :emails_footer => 'Test footer' } } assert_redirected_to '/settings' assert_equal 'functional@test.foo', Setting.mail_from - assert !Setting.bcc_recipients? assert_equal %w(issue_added issue_updated news_added), Setting.notified_events assert_equal 'Test footer', Setting.emails_footer end @@ -168,9 +166,8 @@ class SettingsControllerTest < Redmine::ControllerTest assert_select 'a[href^=?]', 'http://localhost:3000/settings' end # All admins should receive this - recipients = [mail.bcc, mail.cc].flatten User.active.where(admin: true).each do |admin| - assert_include admin.mail, recipients + assert_include admin.mail, mail.to end end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 1160ec7..7756874 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -211,8 +211,6 @@ class UsersControllerTest < Redmine::ControllerTest end def test_create - Setting.bcc_recipients = '1' - assert_difference 'User.count' do assert_difference 'ActionMailer::Base.deliveries.size' do post :create, :params => { @@ -242,7 +240,7 @@ class UsersControllerTest < Redmine::ControllerTest mail = ActionMailer::Base.deliveries.last assert_not_nil mail - assert_equal [user.mail], mail.bcc + assert_equal [user.mail], mail.to assert_mail_body_match 'secret', mail end @@ -369,7 +367,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.to].flatten.include?(admin.mail) } end end @@ -451,7 +449,6 @@ class UsersControllerTest < Redmine::ControllerTest u.status = User::STATUS_REGISTERED u.save! ActionMailer::Base.deliveries.clear - Setting.bcc_recipients = '1' put :update, :params => { :id => u.id, @@ -460,13 +457,12 @@ class UsersControllerTest < Redmine::ControllerTest assert u.reload.active? mail = ActionMailer::Base.deliveries.last assert_not_nil mail - assert_equal ['foo.bar@somenet.foo'], mail.bcc + assert_equal ['foo.bar@somenet.foo'], mail.to assert_mail_body_match ll('fr', :notice_account_activated), mail end def test_update_with_password_change_should_send_a_notification ActionMailer::Base.deliveries.clear - Setting.bcc_recipients = '1' put :update, :params => { :id => 2, @@ -478,13 +474,12 @@ class UsersControllerTest < Redmine::ControllerTest mail = ActionMailer::Base.deliveries.last assert_not_nil mail - assert_equal [u.mail], mail.bcc + assert_equal [u.mail], mail.to assert_mail_body_match 'newpass123', mail end def test_update_with_generate_password_should_email_the_password ActionMailer::Base.deliveries.clear - Setting.bcc_recipients = '1' put :update, :params => { :id => 2, @@ -582,7 +577,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] } end end @@ -602,7 +597,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] } end end @@ -622,7 +617,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] } end # if user is already locked, destroying should not send a second mail @@ -648,7 +643,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] } end end @@ -711,7 +706,7 @@ class UsersControllerTest < Redmine::ControllerTest # All admins should receive this User.where(admin: true, status: Principal::STATUS_ACTIVE).each do |admin| - assert_not_nil ActionMailer::Base.deliveries.detect{|mail| [mail.bcc, mail.cc].flatten.include?(admin.mail) } + assert_not_nil ActionMailer::Base.deliveries.detect{|mail| mail.to == [admin.mail] } end end end diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 3bbe751..34253c5 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2570,7 +2570,7 @@ class IssueTest < ActiveSupport::TestCase issue.assigned_to = nil issue.save! - assert_include [user.mail], ActionMailer::Base.deliveries.map(&:bcc) + assert_include [user.mail], ActionMailer::Base.deliveries.map(&:to) end end diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index d3ff874..4ca7de3 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -281,7 +281,7 @@ class MailerTest < ActiveSupport::TestCase user.pref.save User.current = user Mailer.deliver_news_added(news.reload) - assert_equal 1, last_email.bcc.size + assert_equal 1, last_email.to.size # nobody to notify user.pref.no_self_notified = true @@ -365,8 +365,8 @@ class MailerTest < ActiveSupport::TestCase issue = Issue.find(1) assert Mailer.deliver_issue_add(issue) - assert mail = ActionMailer::Base.deliveries.find {|m| m.bcc.include?('dlopper@somenet.foo')} - assert mail.bcc.include?('otheremail@somenet.foo') + assert mail = ActionMailer::Base.deliveries.find {|m| m.to.include?('dlopper@somenet.foo')} + assert mail.to.include?('otheremail@somenet.foo') end test "#issue_add should not notify project members that are not allow to view the issue" do @@ -488,8 +488,8 @@ class MailerTest < ActiveSupport::TestCase def test_version_file_added attachements = [ Attachment.find_by_container_type('Version') ] assert Mailer.deliver_attachments_added(attachements) - assert_not_nil last_email.bcc - assert last_email.bcc.any? + assert_not_nil last_email.to + assert last_email.to.any? assert_select_email do assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files" end @@ -498,8 +498,8 @@ class MailerTest < ActiveSupport::TestCase def test_project_file_added attachements = [ Attachment.find_by_container_type('Project') ] assert Mailer.deliver_attachments_added(attachements) - assert_not_nil last_email.bcc - assert last_email.bcc.any? + assert_not_nil last_email.to + assert last_email.to.any? assert_select_email do assert_select "a[href=?]", "http://localhost:3000/projects/ecookbook/files" end @@ -558,7 +558,7 @@ class MailerTest < ActiveSupport::TestCase Mailer.reminders(:days => 42) assert_equal 1, ActionMailer::Base.deliveries.size mail = last_email - assert mail.bcc.include?('dlopper@somenet.foo') + assert mail.to.include?('dlopper@somenet.foo') assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail assert_equal '1 issue(s) due in the next 42 days', mail.subject end @@ -570,7 +570,7 @@ class MailerTest < ActiveSupport::TestCase Mailer.reminders(:days => 42) assert_equal 1, ActionMailer::Base.deliveries.size mail = last_email - assert mail.bcc.include?('dlopper@somenet.foo') + assert mail.to.include?('dlopper@somenet.foo') assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail assert_equal "1 demande(s) arrivent à échéance (42)", mail.subject end @@ -587,7 +587,7 @@ class MailerTest < ActiveSupport::TestCase Mailer.reminders(:days => 42) assert_equal 1, ActionMailer::Base.deliveries.size mail = last_email - assert mail.bcc.include?('dlopper@somenet.foo') + assert mail.to.include?('dlopper@somenet.foo') assert_mail_body_no_match 'Closed issue', mail end end @@ -598,7 +598,7 @@ class MailerTest < ActiveSupport::TestCase Mailer.reminders(:days => 42, :users => ['3']) assert_equal 1, ActionMailer::Base.deliveries.size # No mail for dlopper mail = last_email - assert mail.bcc.include?('dlopper@somenet.foo') + assert mail.to.include?('dlopper@somenet.foo') assert_mail_body_match 'Bug #3: Error 281 when updating a recipe', mail end @@ -895,7 +895,7 @@ class MailerTest < ActiveSupport::TestCase # Returns an array of email addresses to which emails were sent def recipients - ActionMailer::Base.deliveries.map(&:bcc).flatten.sort + ActionMailer::Base.deliveries.map(&:to).flatten.sort end def last_email -- 2.1.4