remove self mail prevention

Notification creation prevents self notification which in turn prevents self mailing

app/mailers/application_mailer.rb

@@ -73,12 +73,6 @@ class ApplicationMailer < ActionMailer::Base
       "#{hash}@#{host}"
     end
 
-    def remove_self_notifications(message, author)
-      if author.pref && message.to.present?
-        message.to = message.to.reject { |address| address == author.mail }
-      end
-    end
-
     def mail_timestamp(object)
       object.send(object.respond_to?(:created_at) ? :created_at : :updated_at)
     end
@@ -110,18 +104,6 @@ class ApplicationMailer < ActionMailer::Base
     super(headers, &block)
   end
 
-  # like #mail, but contains special author based filters
-  # currently only:
-  #  - remove_self_notifications
-  # might be refactored at a later time to be as generic as Interceptors
-  def mail_for_author(author, headers = {}, &block)
-    message = mail headers, &block
-
-    self.class.remove_self_notifications(message, author)
-
-    message
-  end
-
   def message_id(object, user)
     headers['Message-ID'] = "<#{self.class.generate_message_id(object, user)}>"
   end

app/mailers/user_mailer.rb

@@ -78,7 +78,7 @@ class UserMailer < ApplicationMailer
     end
   end
 
-  def news_added(user, news, author)
+  def news_added(user, news)
     @news = news
 
     open_project_headers 'Type'    => 'News'
@@ -89,7 +89,7 @@ class UserMailer < ApplicationMailer
     with_locale_for(user) do
       subject = "#{News.model_name.human}: #{@news.title}"
       subject = "[#{@news.project.name}] #{subject}" if @news.project
-      mail_for_author author, to: user.mail, subject: subject
+      mail to: user.mail, subject: subject
     end
   end
 
@@ -111,7 +111,7 @@ class UserMailer < ApplicationMailer
     end
   end
 
-  def news_comment_added(user, comment, author)
+  def news_comment_added(user, comment)
     @comment = comment
     @news    = @comment.commented
 
@@ -123,11 +123,11 @@ class UserMailer < ApplicationMailer
     with_locale_for(user) do
       subject = "#{News.model_name.human}: #{@news.title}"
       subject = "Re: [#{@news.project.name}] #{subject}" if @news.project
-      mail_for_author author, to: user.mail, subject: subject
+      mail to: user.mail, subject: subject
     end
   end
 
-  def wiki_content_added(user, wiki_content, author)
+  def wiki_content_added(user, wiki_content)
     @wiki_content = wiki_content
 
     open_project_headers 'Project' => @wiki_content.project.identifier,
@@ -138,11 +138,11 @@ class UserMailer < ApplicationMailer
 
     with_locale_for(user) do
       subject = "[#{@wiki_content.project.name}] #{t(:mail_subject_wiki_content_added, id: @wiki_content.page.title)}"
-      mail_for_author author, to: user.mail, subject: subject
+      mail to: user.mail, subject: subject
     end
   end
 
-  def wiki_content_updated(user, wiki_content, author)
+  def wiki_content_updated(user, wiki_content)
     @wiki_content  = wiki_content
     @wiki_diff_url = url_for(controller: '/wiki',
                              action: :diff,
@@ -158,16 +158,16 @@ class UserMailer < ApplicationMailer
 
     with_locale_for(user) do
       subject = "[#{@wiki_content.project.name}] #{t(:mail_subject_wiki_content_updated, id: @wiki_content.page.title)}"
-      mail_for_author author, to: user.mail, subject: subject
+      mail to: user.mail, subject: subject
     end
   end
 
-  def message_posted(user, message, author)
+  def message_posted(user, message)
     @message     = message
     @message_url = topic_url(@message.root, r: @message.id, anchor: "message-#{@message.id}")
 
     open_project_headers 'Project' => @message.project.identifier,
-                         'Wiki-Page-Id' => @message.parent_id || @message.id,
+                         'Message-Id' => @message.parent_id || @message.id,
                          'Type' => 'Forum'
 
     message_id @message, user
@@ -175,7 +175,7 @@ class UserMailer < ApplicationMailer
 
     with_locale_for(user) do
       subject = "[#{@message.forum.project.name} - #{@message.forum.name} - msg#{@message.root.id}] #{@message.subject}"
-      mail_for_author author, to: user.mail, subject: subject
+      mail to: user.mail, subject: subject
     end
   end
 

app/mailers/work_package_mailer.rb

@@ -37,15 +37,13 @@ class WorkPackageMailer < ApplicationMailer
       set_work_package_headers(work_package)
 
       message_id journal, recipient
-      references work_package, recipient
 
       with_locale_for(recipient) do
-        mail_for_author author,
-                        to: recipient.mail,
-                        subject: I18n.t(:'mail.mention.subject',
-                                        user_name: author.name,
-                                        id: work_package.id,
-                                        subject: work_package.subject)
+        mail to: recipient.mail,
+             subject: I18n.t(:'mail.mention.subject',
+                             user_name: author.name,
+                             id: work_package.id,
+                             subject: work_package.subject)
       end
     end
   end
@@ -58,7 +56,6 @@ class WorkPackageMailer < ApplicationMailer
 
       set_work_package_headers(work_package)
       message_id work_package, user
-      references work_package, user
 
       with_locale_for(user) do
         mail to: user.mail, subject: subject_for_work_package(work_package)

app/services/notifications/mail_service/comment_strategy.rb

@@ -32,8 +32,7 @@ module Notifications::MailService::CommentStrategy
       UserMailer
         .news_comment_added(
           notification.recipient,
-          notification.resource,
-          notification.resource.author || DeletedUser.first
+          notification.resource
         )
         .deliver_later
     end

app/services/notifications/mail_service/message_strategy.rb

@@ -32,8 +32,7 @@ module Notifications::MailService::MessageStrategy
       UserMailer
         .message_posted(
           notification.recipient,
-          notification.resource,
-          notification.actor || DeletedUser.first
+          notification.resource
         )
         .deliver_later
     end

app/services/notifications/mail_service/news_strategy.rb

@@ -34,8 +34,7 @@ module Notifications::MailService::NewsStrategy
       UserMailer
         .news_added(
           notification.recipient,
-          notification.journal.journable,
-          notification.journal.user || DeletedUser.first
+          notification.journal.journable
         )
         .deliver_later
     end

app/services/notifications/mail_service/wiki_content_strategy.rb

@@ -34,8 +34,7 @@ module Notifications::MailService::WikiContentStrategy
       UserMailer
         .send(method,
               notification.recipient,
-              notification.journal.journable,
-              notification.journal.user || DeletedUser.first)
+              notification.journal.journable)
         .deliver_later
     end
 

spec/mailers/shared_examples.rb

@@ -27,18 +27,3 @@ shared_examples_for 'mail is not sent' do
     expect(deliveries).to be_empty
   end
 end
-
-shared_examples_for 'does not send mails to author' do
-  let(:user) { FactoryBot.build_stubbed(:user) }
-
-  context 'when mail is for another user' do
-    it_behaves_like 'mail is sent'
-  end
-
-  context 'when mail is for author' do
-    let(:recipient) { user }
-
-    it_behaves_like 'mail is not sent'
-  end
-end
-

spec/mailers/user_mailer_spec.rb

@@ -117,19 +117,17 @@ describe UserMailer, type: :mailer do
     let(:wiki_content) { FactoryBot.create(:wiki_content) }
 
     before do
-      described_class.wiki_content_added(recipient, wiki_content, user).deliver_now
+      described_class.wiki_content_added(recipient, wiki_content).deliver_now
     end
 
     it_behaves_like 'mail is sent'
-
-    it_behaves_like 'does not send mails to author'
   end
 
   describe '#wiki_content_updated' do
     let(:wiki_content) { FactoryBot.create(:wiki_content) }
 
     before do
-      described_class.wiki_content_updated(recipient, wiki_content, user).deliver_now
+      described_class.wiki_content_updated(recipient, wiki_content).deliver_now
     end
 
     it_behaves_like 'mail is sent'
@@ -137,8 +135,6 @@ describe UserMailer, type: :mailer do
     it 'links to the latest version diff page' do
       expect(deliveries.first.body.encoded).to include 'diff/1'
     end
-
-    it_behaves_like 'does not send mails to author'
   end
 
   describe '#message_posted' do
@@ -151,7 +147,7 @@ describe UserMailer, type: :mailer do
     end
 
     before do
-      described_class.message_posted(recipient, message, user).deliver_now
+      described_class.message_posted(recipient, message).deliver_now
     end
 
     it_behaves_like 'mail is sent' do
@@ -171,8 +167,6 @@ describe UserMailer, type: :mailer do
                         href: topic_url(message, host: Setting.host_name, r: message.id, anchor: "message-#{message.id}"))
       end
     end
-
-    it_behaves_like 'does not send mails to author'
   end
 
   describe '#account_information' do
@@ -194,7 +188,7 @@ describe UserMailer, type: :mailer do
     let(:news) { FactoryBot.build_stubbed(:news) }
 
     before do
-      described_class.news_added(recipient, news, user).deliver_now
+      described_class.news_added(recipient, news).deliver_now
     end
 
     it_behaves_like 'mail is sent' do
@@ -203,8 +197,6 @@ describe UserMailer, type: :mailer do
           .to eql(described_class.generate_message_id(news, recipient))
       end
     end
-
-    it_behaves_like 'does not send mails to author'
   end
 
   describe '#news_comment_added' do
@@ -212,12 +204,10 @@ describe UserMailer, type: :mailer do
     let(:comment) { FactoryBot.build_stubbed(:comment, commented: news) }
 
     before do
-      described_class.news_comment_added(recipient, comment, user).deliver_now
+      described_class.news_comment_added(recipient, comment).deliver_now
     end
 
     it_behaves_like 'mail is sent'
-
-    it_behaves_like 'does not send mails to author'
   end
 
   describe '#password_lost' do
@@ -274,12 +264,11 @@ describe UserMailer, type: :mailer do
                 .and_return project
       end
     end
-    let(:author) { FactoryBot.build_stubbed(:user) }
 
     describe 'same user' do
       subject do
         message_ids = [message, message2].map do |m|
-          described_class.message_posted(user, m, author).message_id
+          described_class.message_posted(user, m).message_id
         end
 
         message_ids.uniq.count
@@ -293,7 +282,7 @@ describe UserMailer, type: :mailer do
 
       subject do
         message_ids = [user, user2].map do |user|
-          described_class.message_posted(user, message, author).message_id
+          described_class.message_posted(user, message).message_id
         end
 
         message_ids.uniq.count

spec/mailers/work_package_mailer_spec.rb

@@ -95,13 +95,6 @@ describe WorkPackageMailer, type: :mailer do
         .to eql "<openproject.work_package-#{recipient.id}-#{work_package.id}.#{created_at}@example.net>"
     end
 
-    it 'has a references header' do
-      created_at = work_package.created_at.strftime('%Y%m%d%H%M%S')
-
-      expect(mail['References'].value)
-        .to eql "<openproject.work_package-#{recipient.id}-#{work_package.id}.#{created_at}@example.net>"
-    end
-
     it 'has a work package assignee header' do
       expect(mail['X-OpenProject-WorkPackage-Assignee'].value)
         .to eql work_package.assigned_to.login

spec/services/notifications/mail_service_spec.rb

@@ -174,8 +174,7 @@ describe Notifications::MailService, type: :model do
         expect(UserMailer)
           .to have_received(:wiki_content_added)
                 .with(recipient,
-                      journal.journable,
-                      journal.user)
+                      journal.journable)
 
         expect(mail)
           .to have_received(:deliver_later)
@@ -191,8 +190,7 @@ describe Notifications::MailService, type: :model do
         expect(UserMailer)
           .to have_received(:wiki_content_updated)
                 .with(recipient,
-                      journal.journable,
-                      journal.user)
+                      journal.journable)
 
         expect(mail)
           .to have_received(:deliver_later)
@@ -255,8 +253,7 @@ describe Notifications::MailService, type: :model do
         expect(UserMailer)
           .to have_received(:news_added)
                 .with(recipient,
-                      journal.journable,
-                      journal.user)
+                      journal.journable)
 
         expect(mail)
           .to have_received(:deliver_later)
@@ -314,8 +311,7 @@ describe Notifications::MailService, type: :model do
       expect(UserMailer)
         .to have_received(:message_posted)
               .with(recipient,
-                    journal.journable,
-                    actor)
+                    journal.journable)
 
       expect(mail)
         .to have_received(:deliver_later)