From 5fea6679545e0f3d56dc7245eb3125c8da9f4dce Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Fri, 24 Jul 2015 16:44:22 +0200 Subject: [PATCH] Return recipients as users instead of mail addresses --- app/models/comment_observer.rb | 3 +-- app/models/journal_notification_mailer.rb | 7 ++----- app/models/message_observer.rb | 3 +-- app/models/news_observer.rb | 3 +-- app/models/project.rb | 4 ++-- app/models/wiki_content.rb | 3 +-- app/models/wiki_content_observer.rb | 10 +++++----- app/models/work_package.rb | 5 ++--- lib/plugins/acts_as_event/lib/acts_as_event.rb | 5 ++--- .../lib/redmine/acts/journalized/deprecated.rb | 3 +-- lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb | 6 ++---- spec/legacy/unit/user_spec.rb | 6 +++--- spec/legacy/unit/watcher_spec.rb | 4 ++-- .../work_package/work_package_action_mailer_spec.rb | 6 +++--- spec/models/work_package/work_package_copy_spec.rb | 4 ++-- 15 files changed, 30 insertions(+), 42 deletions(-) diff --git a/app/models/comment_observer.rb b/app/models/comment_observer.rb index 8319457331..49652db7b3 100644 --- a/app/models/comment_observer.rb +++ b/app/models/comment_observer.rb @@ -34,8 +34,7 @@ class CommentObserver < ActiveRecord::Observer if comment.commented.is_a?(News) news = comment.commented recipients = news.recipients + news.watcher_recipients - users = User.find_all_by_mails(recipients) - users.each do |user| + recipients.each do |user| UserMailer.news_comment_added(user, comment, User.current).deliver end end diff --git a/app/models/journal_notification_mailer.rb b/app/models/journal_notification_mailer.rb index 13462524a9..acef70d7ed 100644 --- a/app/models/journal_notification_mailer.rb +++ b/app/models/journal_notification_mailer.rb @@ -42,9 +42,7 @@ class JournalNotificationMailer def handle_work_package_create(work_package) if Setting.notified_events.include?('work_package_added') recipients = work_package.recipients + work_package.watcher_recipients - users = User.find_all_by_mails(recipients.uniq) - - users.each do |user| + recipients.uniq.each do |user| job = DeliverWorkPackageCreatedJob.new(user.id, work_package.id, User.current.id) Delayed::Job.enqueue job @@ -56,8 +54,7 @@ class JournalNotificationMailer if send_update_notification?(journal) issue = journal.journable recipients = issue.recipients + issue.watcher_recipients - users = User.find_all_by_mails(recipients.uniq) - users.each do |user| + recipients.uniq.each do |user| job = DeliverWorkPackageUpdatedJob.new(user.id, journal.id, User.current.id) Delayed::Job.enqueue job end diff --git a/app/models/message_observer.rb b/app/models/message_observer.rb index 40621f2d37..c3eb4311b6 100644 --- a/app/models/message_observer.rb +++ b/app/models/message_observer.rb @@ -33,8 +33,7 @@ class MessageObserver < ActiveRecord::Observer recipients = message.recipients recipients += message.root.watcher_recipients recipients += message.board.watcher_recipients - users = User.find_all_by_mails(recipients.uniq) - users.each do |user| + recipients.uniq.each do |user| UserMailer.message_posted(user, message, User.current).deliver end end diff --git a/app/models/news_observer.rb b/app/models/news_observer.rb index fac62827e4..ba678f110f 100644 --- a/app/models/news_observer.rb +++ b/app/models/news_observer.rb @@ -30,8 +30,7 @@ class NewsObserver < ActiveRecord::Observer def after_create(news) if Setting.notified_events.include?('news_added') - users = User.find_all_by_mails(news.recipients) - users.each do |user| + news.recipients.each do |user| UserMailer.news_added(user, news, User.current).deliver end end diff --git a/app/models/project.rb b/app/models/project.rb index 1d2b9e4c28..d72854a983 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -616,9 +616,9 @@ class Project < ActiveRecord::Base possible_responsible_members.map(&:principal).compact.sort end - # Returns the mail adresses of users that should be always notified on project events + # Returns users that should be always notified on project events def recipients - notified_users.map(&:mail) + notified_users end # Returns the users that should be notified on project events diff --git a/app/models/wiki_content.rb b/app/models/wiki_content.rb index b6428160df..f2111cfb82 100644 --- a/app/models/wiki_content.rb +++ b/app/models/wiki_content.rb @@ -68,8 +68,7 @@ class WikiContent < ActiveRecord::Base # Returns the mail adresses of users that should be notified def recipients notified = project.notified_users - notified.reject! { |user| !visible?(user) } - notified.map(&:mail) + notified.reject { |user| !visible?(user) } end # FIXME: Deprecate diff --git a/app/models/wiki_content_observer.rb b/app/models/wiki_content_observer.rb index 0091a7802a..4499c68674 100644 --- a/app/models/wiki_content_observer.rb +++ b/app/models/wiki_content_observer.rb @@ -31,8 +31,7 @@ class WikiContentObserver < ActiveRecord::Observer def after_create(wiki_content) if Setting.notified_events.include?('wiki_content_added') recipients = wiki_content.recipients + wiki_content.page.wiki.watcher_recipients - users = User.find_all_by_mails(recipients.uniq) - users.each do |user| + recipients.uniq.each do |user| UserMailer.wiki_content_added(user, wiki_content, User.current).deliver end end @@ -40,9 +39,10 @@ class WikiContentObserver < ActiveRecord::Observer def after_update(wiki_content) if wiki_content.text_changed? && Setting.notified_events.include?('wiki_content_updated') - recipients = wiki_content.recipients + wiki_content.page.wiki.watcher_recipients + wiki_content.page.watcher_recipients - users = User.find_all_by_mails(recipients.uniq) - users.each do |user| + recipients = wiki_content.recipients + recipients += wiki_content.page.wiki.watcher_recipients + recipients += wiki_content.page.watcher_recipients + recipients.uniq.each do |user| UserMailer.wiki_content_updated(user, wiki_content, User.current).deliver end end diff --git a/app/models/work_package.rb b/app/models/work_package.rb index d63357109e..5ce8aee221 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -490,7 +490,7 @@ class WorkPackage < ActiveRecord::Base end # >>> issues.rb >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - # Returns the mail addresses of users that should be notified + # Returns users that should be notified def recipients notified = project.notified_users # Author and assignee are always notified unless they have been @@ -505,8 +505,7 @@ class WorkPackage < ActiveRecord::Base end notified.uniq! # Remove users that can not view the issue - notified.reject! { |user| !visible?(user) } - notified.map(&:mail) + notified.reject { |user| !visible?(user) } end def done_ratio diff --git a/lib/plugins/acts_as_event/lib/acts_as_event.rb b/lib/plugins/acts_as_event/lib/acts_as_event.rb index cafe3c49ca..f3cfc0b621 100644 --- a/lib/plugins/acts_as_event/lib/acts_as_event.rb +++ b/lib/plugins/acts_as_event/lib/acts_as_event.rb @@ -92,12 +92,11 @@ module Redmine end end - # Returns the mail adresses of users that should be notified + # Returns users that should be notified def recipients notified = [] notified = project.notified_users if project - notified.reject! { |user| !visible?(user) } - notified.map(&:mail) + notified.reject { |user| !visible?(user) } end module ClassMethods diff --git a/lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/deprecated.rb b/lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/deprecated.rb index a0b8fb4843..eb11a61b09 100644 --- a/lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/deprecated.rb +++ b/lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/deprecated.rb @@ -55,8 +55,7 @@ module Redmine::Acts::Journalized def recipients notified = [] notified = project.notified_users if project - notified.reject! { |user| !visible?(user) } - notified.map(&:mail) + notified.reject { |user| !visible?(user) } end def current_journal 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 09fb14726a..eb4c596d27 100644 --- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb +++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb @@ -146,12 +146,10 @@ module Redmine watcher_user_ids.any? { |uid| uid == user.id })) end - # Returns an array of watchers' email addresses + # Returns an array of watchers def watcher_recipients notified = watcher_users.active.where(['mail_notification != ?', 'none']) - notified.select! { |user| possible_watcher?(user) } - - notified.map(&:mail).compact + notified.select { |user| possible_watcher?(user) } end module ClassMethods; end diff --git a/spec/legacy/unit/user_spec.rb b/spec/legacy/unit/user_spec.rb index 3a666254ac..379ff1a28b 100644 --- a/spec/legacy/unit/user_spec.rb +++ b/spec/legacy/unit/user_spec.rb @@ -364,7 +364,7 @@ describe User, type: :model do @jsmith.notified_project_ids = [] @jsmith.save @jsmith.reload - assert @jsmith.projects.first.recipients.include?(@jsmith.mail) + assert @jsmith.projects.first.recipients.include?(@jsmith) end it 'should mail notification selected' do @@ -372,7 +372,7 @@ describe User, type: :model do @jsmith.notified_project_ids = [1] @jsmith.save @jsmith.reload - assert Project.find(1).recipients.include?(@jsmith.mail) + assert Project.find(1).recipients.include?(@jsmith) end it 'should mail notification only my events' do @@ -380,7 +380,7 @@ describe User, type: :model do @jsmith.notified_project_ids = [] @jsmith.save @jsmith.reload - assert !@jsmith.projects.first.recipients.include?(@jsmith.mail) + assert !@jsmith.projects.first.recipients.include?(@jsmith) end it 'should comments sorting preference' do diff --git a/spec/legacy/unit/watcher_spec.rb b/spec/legacy/unit/watcher_spec.rb index ff8578ffbb..adebe82166 100644 --- a/spec/legacy/unit/watcher_spec.rb +++ b/spec/legacy/unit/watcher_spec.rb @@ -92,10 +92,10 @@ describe Watcher do assert @issue.watcher_recipients.empty? assert @issue.add_watcher(@user) - assert_contains @issue.watcher_recipients, @user.mail + assert_contains @issue.watcher_recipients, @user @user.update_attribute :mail_notification, 'none' - assert_does_not_contain @issue.watcher_recipients, @user.mail + assert_does_not_contain @issue.watcher_recipients, @user end it 'should unwatch' do diff --git a/spec/models/work_package/work_package_action_mailer_spec.rb b/spec/models/work_package/work_package_action_mailer_spec.rb index cd5f707962..4d0cad3c2e 100644 --- a/spec/models/work_package/work_package_action_mailer_spec.rb +++ b/spec/models/work_package/work_package_action_mailer_spec.rb @@ -46,8 +46,8 @@ describe WorkPackage, type: :model do before do ActionMailer::Base.deliveries.clear - allow(work_package).to receive(:recipients).and_return([user_1.mail]) - allow(work_package).to receive(:watcher_recipients).and_return([user_2.mail]) + allow(work_package).to receive(:recipients).and_return([user_1]) + allow(work_package).to receive(:watcher_recipients).and_return([user_2]) work_package.save end @@ -94,7 +94,7 @@ describe WorkPackage, type: :model do subject { work_package.recipients } - it { is_expected.to include(user_1.mail) } + it { is_expected.to include(user_1) } end end end diff --git a/spec/models/work_package/work_package_copy_spec.rb b/spec/models/work_package/work_package_copy_spec.rb index e8445c5ae2..073ee8131e 100644 --- a/spec/models/work_package/work_package_copy_spec.rb +++ b/spec/models/work_package/work_package_copy_spec.rb @@ -191,12 +191,12 @@ describe WorkPackage, type: :model do context 'pre-condition' do subject { work_package.recipients } - it { is_expected.to include(work_package.author.mail) } + it { is_expected.to include(work_package.author) } end subject { copy.recipients } - it { is_expected.not_to include(copy.author.mail) } + it { is_expected.not_to include(copy.author) } end describe 'with children' do