rely on the reference mail header for wp update mails

app/mailers/application_mailer.rb

@@ -53,30 +53,6 @@ class ApplicationMailer < ActionMailer::Base
       ActionMailer::Base.perform_deliveries = old_state
     end
 
-    def generate_message_id(object, user)
-      # id + timestamp should reduce the odds of a collision
-      # as long as we don't send multiple emails for the same object
-      journable = object.is_a?(Journal) ? object.journable : object
-
-      timestamp = mail_timestamp(object)
-      hash = 'openproject'\
-           '.'\
-           "#{journable.class.name.demodulize.underscore}"\
-           '-'\
-           "#{user.id}"\
-           '-'\
-           "#{journable.id}"\
-           '.'\
-           "#{timestamp.strftime('%Y%m%d%H%M%S')}"
-      host = Setting.mail_from.to_s.gsub(%r{\A.*@}, '')
-      host = "#{::Socket.gethostname}.openproject" if host.empty?
-      "#{hash}@#{host}"
-    end
-
-    def mail_timestamp(object)
-      object.send(object.respond_to?(:created_at) ? :created_at : :updated_at)
-    end
-
     def host
       if OpenProject::Configuration.rails_relative_url_root.blank?
         Setting.host_name
@@ -105,11 +81,19 @@ class ApplicationMailer < ActionMailer::Base
   end
 
   def message_id(object, user)
-    headers['Message-ID'] = "<#{self.class.generate_message_id(object, user)}>"
+    headers['Message-ID'] = "<#{message_id_value(object, user)}>"
   end
 
-  def references(object, user)
-    headers['References'] = "<#{self.class.generate_message_id(object, user)}>"
+  def references(*objects)
+    refs = objects.map do |object|
+      if object.is_a?(Journal)
+        "<#{references_value(object.journable)}> <#{references_value(object)}>"
+      else
+        "<#{references_value(object)}>"
+      end
+    end
+
+    headers['References'] = refs.join(' ')
   end
 
   # Prepends given fields with 'X-OpenProject-' to save some duplication
@@ -129,6 +113,57 @@ class ApplicationMailer < ActionMailer::Base
       mail to: user.mail, subject: subject
     end
   end
+
+  # Generates a unique value for the Message-ID header.
+  # Contains:
+  # * an 'op' prefix
+  # * an object id part that consists of the object's class name and the id unless that part is provided as a string
+  # * the current time
+  # * the recipient's id
+  #
+  # Note that this values, as opposed to the one from #references_value is unique.
+  def message_id_value(object, recipient)
+    object_reference = case object
+                       when String
+                         object
+                       else
+                         "#{object.class.name.demodulize.underscore}-#{object.id}"
+                       end
+    hash = 'op'\
+           '.'\
+           "#{object_reference}"\
+           '.'\
+           "#{Time.current.strftime('%Y%m%d%H%M%S')}"\
+           '.'\
+           "#{recipient.id}"
+
+    "#{hash}@#{header_host_value}"
+  end
+
+  # Generates a value for the References header.
+  # Contains:
+  # * an 'op' prefix
+  # * an object id part that consists of the object's class name and the id
+  #
+  # Note that this values, as opposed to the one from #message_id_value is not unique.
+  # It in fact is aimed not not so that similar messages (i.e. those belonging to the same
+  # work package and journal) end up being grouped together.
+  #
+  # The value is used within the MailHandler to find the appropriate objects for update
+  # when a mail has been received.
+  def references_value(object)
+    hash = 'op'\
+           '.'\
+           "#{object.class.name.demodulize.underscore}-#{object.id}"
+
+    "#{hash}@#{header_host_value}"
+  end
+
+  def header_host_value
+    host = Setting.mail_from.to_s.gsub(%r{\A.*@}, '')
+    host = "#{::Socket.gethostname}.openproject" if host.empty?
+    host
+  end
 end
 
 ##

app/mailers/digest_mailer.rb

@@ -56,7 +56,7 @@ class DigestMailer < ApplicationMailer
     recipient = User.find(recipient_id)
 
     open_project_headers User: recipient.name
-    message_id nil, recipient
+    message_id 'digest', recipient
 
     @user = recipient
     @notification_ids = notification_ids

app/mailers/user_mailer.rb

@@ -83,6 +83,7 @@ class UserMailer < ApplicationMailer
     open_project_headers 'Project' => @news.project.identifier if @news.project
 
     message_id @news, user
+    references @news
 
     subject = "#{News.model_name.human}: #{@news.title}"
     subject = "[#{@news.project.name}] #{subject}" if @news.project
@@ -112,7 +113,7 @@ class UserMailer < ApplicationMailer
     open_project_headers 'Project' => @news.project.identifier if @news.project
 
     message_id @comment, user
-    references @news, user
+    references @news, @comment
 
     subject = "#{News.model_name.human}: #{@news.title}"
     subject = "Re: [#{@news.project.name}] #{subject}" if @news.project
@@ -150,7 +151,7 @@ class UserMailer < ApplicationMailer
 
     open_project_message_headers(@message)
     message_id @message, user
-    references @message.parent, user if @message.parent
+    references *[@message.parent, @message].compact
 
     send_mail(user,
               "[#{@message.forum.project.name} - #{@message.forum.name} - msg#{@message.root.id}] #{@message.subject}")

app/mailers/work_package_mailer.rb

@@ -42,6 +42,7 @@ class WorkPackageMailer < ApplicationMailer
       set_work_package_headers(@work_package)
 
       message_id journal, recipient
+      references journal
 
       with_locale_for(recipient) do
         mail to: recipient.mail,
@@ -61,6 +62,7 @@ class WorkPackageMailer < ApplicationMailer
 
       set_work_package_headers(work_package)
       message_id work_package, user
+      references work_package
 
       with_locale_for(user) do
         mail to: user.mail, subject: subject_for_work_package(work_package)

app/models/mail_handler.rb

@@ -136,12 +136,12 @@ class MailHandler < ActionMailer::Base
 
   private
 
-  MESSAGE_ID_RE = %r{^<?openproject\.([a-z0-9_]+)-(\d+)-(\d+)\.\d+@}
-  ISSUE_REPLY_SUBJECT_RE = %r{.+? - .+ #(\d+):}
-  MESSAGE_REPLY_SUBJECT_RE = %r{\[[^\]]*msg(\d+)\]}
+  REFERENCES_RE = %r{^<?op\.([a-z_]+)-(\d+)@}.freeze
+  ISSUE_REPLY_SUBJECT_RE = %r{.+? - .+ #(\d+):}.freeze
+  MESSAGE_REPLY_SUBJECT_RE = %r{\[[^\]]*msg(\d+)\]}.freeze
 
   def dispatch
-    if (m, object_id = dispatch_target_from_message_id)
+    if (m, object_id = dispatch_target_from_header)
       m.call(object_id)
     elsif (m = email.subject.match(ISSUE_REPLY_SUBJECT_RE))
       receive_work_package_reply(m[1].to_i)
@@ -171,12 +171,13 @@ class MailHandler < ActionMailer::Base
   end
 
   ##
-  # Find a matching method to dispatch to given the mail's message ID
-  def dispatch_target_from_message_id
-    headers = [email.references, email.in_reply_to].flatten.compact
-    if headers.detect { |h| h.to_s =~ MESSAGE_ID_RE }
+  # Find a matching method to dispatch to given the mail's references header.
+  # We set this header in outgoing emails to include an encoded reference to the object
+  def dispatch_target_from_header
+    headers = [email.references].flatten.compact
+    if headers.reverse.detect { |h| h.to_s =~ REFERENCES_RE }
       klass = $1
-      object_id = $3.to_i
+      object_id = $2.to_i
       method_name = :"receive_#{klass}_reply"
       if self.class.private_instance_methods.include?(method_name)
         return method(method_name), object_id
@@ -199,7 +200,12 @@ class MailHandler < ActionMailer::Base
     end
   end
 
-  alias :receive_issue :receive_work_package
+  def receive_journal_reply(journal_id)
+    journal = Journal.find_by(id: journal_id)
+    return unless journal
+
+    send(:"receive_#{journal.journable_type.underscore}_reply", journal.journable_id)
+  end
 
   # Adds a note to an existing work package
   def receive_work_package_reply(work_package_id)
@@ -220,16 +226,6 @@ class MailHandler < ActionMailer::Base
     end
   end
 
-  alias :receive_issue_reply :receive_work_package_reply
-
-  # Reply will be added to the issue
-  def receive_issue_journal_reply(journal_id)
-    journal = Journal.find_by(id: journal_id)
-    if journal and journal.journable.is_a? WorkPackage
-      receive_work_package_reply(journal.journable_id)
-    end
-  end
-
   # Receives a reply to a forum message
   def receive_message_reply(message_id)
     message = Message.find_by(id: message_id)
@@ -351,7 +347,7 @@ class MailHandler < ActionMailer::Base
   end
 
   # Returns a Hash of issue attributes extracted from keywords in the email body
-  def issue_attributes_from_keywords(issue)
+  def wp_attributes_from_keywords(issue)
     assigned_to = (k = get_keyword(:assigned_to, override: true)) && find_assignee_from_keyword(k, issue)
     project = issue.project
 
@@ -542,7 +538,7 @@ class MailHandler < ActionMailer::Base
   end
 
   def collect_wp_attributes_from_email_on_create(work_package)
-    attributes = issue_attributes_from_keywords(work_package)
+    attributes = wp_attributes_from_keywords(work_package)
     attributes
       .merge('custom_field_values' => custom_field_values_from_keywords(work_package),
              'subject' => email.subject.to_s.chomp[0, 255] || '(no subject)',
@@ -567,7 +563,7 @@ class MailHandler < ActionMailer::Base
   end
 
   def collect_wp_attributes_from_email_on_update(work_package)
-    attributes = issue_attributes_from_keywords(work_package)
+    attributes = wp_attributes_from_keywords(work_package)
     attributes
       .merge('custom_field_values' => custom_field_values_from_keywords(work_package),
              'journal_notes' => cleaned_up_text_body)

spec/fixtures/mail_handler/message_reply.eml

@@ -1,9 +1,9 @@
 Date: Tue, 16 Mar 2021 16:24:54 +0100
 To: notifications@openproject.com
-From: user@example.org
-Message-ID: <openproject.message-70917-12559.20200922103727@openproject.com>
-Subject: [OpenProject - General discussion - msg12559] Test
- fields in user stories
+From: j.doe@openproject.org
+Message-ID: <af603fe2-8888-43ae-91c5-e6efcd93cf98@mtasv.net>
+References: <op.message-70917@openproject.com>
+Subject: [OpenProject - General discussion - msg70917] Response to the original message
 Mime-Version: 1.0
 Content-Type: multipart/alternative;
  boundary="--==_mimepart_6050cdc2e4db_2257939a87567a";

spec/fixtures/mail_handler/wp_reply_with_quoted_reply_above.eml

@@ -3,7 +3,7 @@ Received: from osiris ([127.0.0.1])
 	by OSIRIS
 	with hMailServer ; Sun, 22 Jun 2008 12:28:07 +0200
 Message-ID: <000501c8d452$a95cd7e0$0a00a8c0@osiris>
-In-Reply-To: <openproject.issue-1-2.20060719210421@osiris>
+References: <op.work_package-2@osiris> <op.journal-891223@osiris>
 From: "John Smith" <JSmith@somenet.foo>
 To: <openproject@somenet.foo>
 Subject: Re: update to issue 2

spec/fixtures/mail_handler/wp_update_with_multiple_quoted_reply_above.eml

@@ -1,48 +0,0 @@
-Return-Path: <JSmith@somenet.foo>
-Received: from osiris ([127.0.0.1])
-	by OSIRIS
-	with hMailServer ; Sun, 22 Jun 2008 12:28:07 +0200
-Message-ID: <000501c8d452$a95cd7e0$0a00a8c0@osiris>
-In-Reply-To: <openproject.issue-1-2.20060719210421@osiris>
-From: "John Smith" <JSmith@somenet.foo>
-To: <openproject@somenet.foo>
-Subject: Re: update to issue 2
-Date: Sun, 22 Jun 2008 12:28:07 +0200
-MIME-Version: 1.0
-Content-Type: text/plain;
-	format=flowed;
-	charset="iso-8859-1";
-	reply-type=original
-Content-Transfer-Encoding: 7bit
-X-Priority: 3
-X-MSMail-Priority: Normal
-X-Mailer: Microsoft Outlook Express 6.00.2900.2869
-X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2869
-
-An update to the issue by the sender.
-
-Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas imperdiet
-turpis et odio. Integer eget pede vel dolor euismod varius. Phasellus
-blandit eleifend augue. Nulla facilisi. Duis id diam. Class aptent taciti
-sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. In
-in urna sed tellus aliquet lobortis. Morbi scelerisque tortor in dolor. Cras
-sagittis odio eu lacus. Aliquam sem tortor, consequat sit amet, vestibulum
-id, iaculis at, lectus. Fusce tortor libero, congue ut, euismod nec, luctus
-eget, eros. Pellentesque tortor enim, feugiat in, dignissim eget, tristique
-sed, mauris --- Pellentesque habitant morbi tristique senectus et netus et
-malesuada fames ac turpis egestas. Quisque sit amet libero. In hac habitasse
-platea dictumst.
-
->> > --- Reply above. Do not remove this line. ---
->> >
->> > Issue #6779 has been updated by Eric Davis.
->> >
->> > Subject changed from Projects with JSON to Project JSON API
->> > Status changed from New to Assigned
->> > Assignee set to Eric Davis
->> > Priority changed from Low to Normal
->> > Estimated time deleted (1.00)
->> >
->> > Looks like the JSON api for projects was missed. I'm going to be
->> > reviewing the existing APIs and trying to clean them up over the next
->> > few weeks.

spec/mailers/digest_mailer_spec.rb

@@ -94,8 +94,8 @@ describe DigestMailer, type: :mailer do
         .to receive(:current)
               .and_return(Time.current)
 
-      expect(mail['Message-ID']&.value)
-        .to eql "<openproject.digest-#{recipient.id}-#{Time.current.strftime('%Y%m%d%H%M%S')}@example.net>"
+      expect(mail.message_id)
+        .to eql "op.digest.#{Time.current.strftime('%Y%m%d%H%M%S')}.#{recipient.id}@example.net"
     end
 
     it 'sets the expected openproject headers' do

spec/mailers/member_mailer_spec.rb

@@ -47,6 +47,12 @@ describe MemberMailer, type: :mailer do
   let(:roles) { [FactoryBot.build_stubbed(:role), FactoryBot.build_stubbed(:role)] }
   let(:message) { nil }
 
+  around do |example|
+    Timecop.freeze(Time.current) do
+      example.run
+    end
+  end
+
   shared_examples_for 'has a subject' do |key|
     it "has a subject" do
       if project
@@ -82,7 +88,7 @@ describe MemberMailer, type: :mailer do
   shared_examples_for 'sets the expected message_id header' do
     it 'sets the expected message_id header' do
       expect(subject['Message-ID'].value)
-        .to eql "<openproject.member-#{current_user.id}-#{member.id}.#{member.created_at.strftime('%Y%m%d%H%M%S')}@example.net>"
+        .to eql "<op.member-#{member.id}.#{Time.current.strftime('%Y%m%d%H%M%S')}.#{current_user.id}@example.net>"
     end
   end
 

spec/mailers/user_mailer_spec.rb

@@ -49,6 +49,13 @@ describe UserMailer, type: :mailer do
   end
 
   let(:recipient) { FactoryBot.build_stubbed(:user) }
+  let(:current_time) { Time.current }
+
+  around do |example|
+    Timecop.freeze(current_time) do
+      example.run
+    end
+  end
 
   before do
     allow(work_package).to receive(:reload).and_return(work_package)
@@ -138,33 +145,62 @@ describe UserMailer, type: :mailer do
   end
 
   describe '#message_posted' do
-    let(:message) do
-      FactoryBot.build_stubbed(:message).tap do |msg|
-        allow(msg)
-          .to receive(:project)
-                .and_return(msg.forum.project)
-      end
-    end
-
     before do
       described_class.message_posted(recipient, message).deliver_now
     end
 
-    it_behaves_like 'mail is sent' do
-      it 'carries a message_id' do
-        expect(deliveries.first.message_id)
-          .to eql(described_class.generate_message_id(message, recipient))
+    context 'for a message without a parent' do
+      let(:message) do
+        FactoryBot.build_stubbed(:message).tap do |msg|
+          allow(msg)
+            .to receive(:project)
+                  .and_return(msg.forum.project)
+        end
       end
 
-      it 'has no references' do
-        expect(deliveries.first.references)
-          .to be_nil
+      it_behaves_like 'mail is sent' do
+        it 'carries a message_id' do
+          expect(deliveries.first.message_id)
+            .to eql "op.message-#{message.id}.#{current_time.strftime('%Y%m%d%H%M%S')}.#{recipient.id}@doe.com"
+        end
+
+        it 'references the message' do
+          expect(deliveries.first.references)
+            .to eql "op.message-#{message.id}@doe.com"
+        end
+
+        it 'includes a link to the message' do
+          expect(html_body)
+            .to have_link(message.subject,
+                          href: topic_url(message, host: Setting.host_name, r: message.id, anchor: "message-#{message.id}"))
+        end
       end
+    end
 
-      it 'includes a link to the message' do
-        expect(html_body)
-          .to have_link(message.subject,
-                        href: topic_url(message, host: Setting.host_name, r: message.id, anchor: "message-#{message.id}"))
+    context 'for a message with a parent' do
+      let(:parent) do
+        FactoryBot.build_stubbed(:message)
+      end
+
+      let(:message) do
+        FactoryBot.build_stubbed(:message, parent: parent).tap do |msg|
+          allow(msg)
+            .to receive(:project)
+                  .and_return(msg.forum.project)
+        end
+      end
+
+      it_behaves_like 'mail is sent' do
+        it 'carries a message_id' do
+          expect(deliveries.first.message_id)
+            .to eql "op.message-#{message.id}.#{current_time.strftime('%Y%m%d%H%M%S')}.#{recipient.id}@doe.com"
+        end
+
+        it 'references the message' do
+          expect(deliveries.first.references)
+            .to eql %W[op.message-#{parent.id}@doe.com
+                       op.message-#{message.id}@doe.com]
+        end
       end
     end
   end
@@ -194,7 +230,12 @@ describe UserMailer, type: :mailer do
     it_behaves_like 'mail is sent' do
       it 'carries a message_id' do
         expect(mail.message_id)
-          .to eql(described_class.generate_message_id(news, recipient))
+          .to eql "op.news-#{news.id}.#{current_time.strftime('%Y%m%d%H%M%S')}.#{recipient.id}@doe.com"
+      end
+
+      it 'references the news' do
+        expect(deliveries.first.references)
+          .to eql "op.news-#{news.id}@doe.com"
       end
     end
   end
@@ -207,7 +248,13 @@ describe UserMailer, type: :mailer do
       described_class.news_comment_added(recipient, comment).deliver_now
     end
 
-    it_behaves_like 'mail is sent'
+    it_behaves_like 'mail is sent' do
+      it 'references the news and the comment' do
+        expect(deliveries.first.references)
+          .to eql %W[op.news-#{news.id}@doe.com
+                     op.comment-#{comment.id}@doe.com]
+      end
+    end
   end
 
   describe '#password_lost' do
@@ -248,50 +295,6 @@ describe UserMailer, type: :mailer do
     end
   end
 
-  describe '#message_id' do
-    let(:project) { FactoryBot.build_stubbed(:project) }
-    let(:message) do
-      FactoryBot.build_stubbed(:message).tap do |m|
-        allow(m)
-          .to receive(:project)
-                .and_return project
-      end
-    end
-    let(:message2) do
-      FactoryBot.build_stubbed(:message).tap do |m|
-        allow(m)
-          .to receive(:project)
-                .and_return project
-      end
-    end
-
-    describe 'same user' do
-      subject do
-        message_ids = [message, message2].map do |m|
-          described_class.message_posted(user, m).message_id
-        end
-
-        message_ids.uniq.count
-      end
-
-      it { expect(subject).to eq(2) }
-    end
-
-    describe 'same timestamp' do
-      let(:user2) { FactoryBot.build_stubbed(:user) }
-
-      subject do
-        message_ids = [user, user2].map do |user|
-          described_class.message_posted(user, message).message_id
-        end
-
-        message_ids.uniq.count
-      end
-
-      it { expect(subject).to eq(2) }
-    end
-  end
-
   describe 'localization' do
     context 'with the user having a language configured',
             with_settings: { available_languages: %w[en de],

spec/mailers/work_package_mailer_spec.rb

@@ -89,10 +89,18 @@ describe WorkPackageMailer, type: :mailer do
     end
 
     it 'has a message id header' do
-      created_at = work_package.created_at.strftime('%Y%m%d%H%M%S')
+      Timecop.freeze(Time.current) do
+        expect(mail.message_id)
+          .to eql "op.journal-#{journal.id}.#{Time.current.strftime('%Y%m%d%H%M%S')}.#{recipient.id}@example.net"
+      end
+    end
+
+    it 'has a references header' do
+      journal_part = "op.journal-#{journal.id}@example.net"
+      work_package_part = "op.work_package-#{work_package.id}@example.net"
 
-      expect(mail['Message-ID'].value)
-        .to eql "<openproject.work_package-#{recipient.id}-#{work_package.id}.#{created_at}@example.net>"
+      expect(mail.references)
+        .to eql [work_package_part, journal_part]
     end
 
     it 'has a work package assignee header' do
@@ -106,16 +114,32 @@ describe WorkPackageMailer, type: :mailer do
 
     let(:watcher_changer) { author }
 
-    before do
-      described_class.watcher_changed(work_package, recipient, author, 'added').deliver_now
-      described_class.watcher_changed(work_package, recipient, author, 'removed').deliver_now
+    context 'for an added watcher' do
+      subject(:mail) { described_class.watcher_changed(work_package, recipient, author, 'added') }
+
+      it 'contains the WP subject in the mail subject' do
+        expect(mail.subject)
+          .to include(work_package.subject)
+      end
+
+      it 'has a references header' do
+        expect(mail.references)
+          .to eql "op.work_package-#{work_package.id}@example.net"
+      end
     end
 
-    include_examples 'multiple mails are sent', 2
+    context 'for a removed watcher' do
+      subject(:mail) { described_class.watcher_changed(work_package, recipient, author, 'removed') }
+
+      it 'contains the WP subject in the mail subject' do
+        expect(mail.subject)
+          .to include(work_package.subject)
+      end
 
-    it 'contains the WP subject in the mail subject' do
-      expect(deliveries.first.subject)
-        .to include(work_package.subject)
+      it 'has a references header' do
+        expect(mail.references)
+          .to eql "op.work_package-#{work_package.id}@example.net"
+      end
     end
   end
 end

spec/models/mail_handler_spec.rb

@@ -80,7 +80,7 @@ describe MailHandler, type: :model do
     end
   end
 
-  shared_context 'wp_update_with_quoted_reply_above' do
+  shared_context 'with a reply to a wp mention with quotes above' do
     let(:permissions) { %i[edit_work_packages view_work_packages] }
     let!(:user) do
       FactoryBot.create(:user,
@@ -90,54 +90,138 @@ describe MailHandler, type: :model do
     end
 
     let!(:work_package) do
-      FactoryBot.create(:work_package, id: 2, project: project)
+      FactoryBot.create(:work_package,
+                        id: 2,
+                        project: project).tap do |wp|
+        wp.journals.last.update_column(:id, 891223)
+      end
     end
 
     subject do
-      submit_email('wp_update_with_quoted_reply_above.eml')
+      submit_email('wp_reply_with_quoted_reply_above.eml')
     end
   end
 
-  shared_context 'wp_update_with_multiple_quoted_reply_above' do
-    let(:permissions) { %i[edit_work_packages view_work_packages] }
+  shared_context 'wp create with cc' do
+    let(:permissions) { %i[add_work_packages view_work_packages add_work_package_watchers] }
     let!(:user) do
       FactoryBot.create(:user,
                         mail: 'JSmith@somenet.foo',
+                        firstname: 'John',
+                        lastname: 'Smith',
+                        member_in_project: project,
+                        member_with_permissions: permissions)
+    end
+    let!(:cc_user) do
+      FactoryBot.create(:user,
+                        mail: 'dlopper@somenet.foo',
+                        firstname: 'D',
+                        lastname: 'Lopper',
+                        member_in_project: project,
+                        member_with_permissions: permissions)
+    end
+    let(:submit_options) { { issue: { project: project.identifier } } }
+
+    subject do
+      submit_email('ticket_with_cc.eml', **submit_options)
+    end
+  end
+
+  shared_context 'with a reply to a wp mention' do
+    let(:permissions) { %i[add_work_package_notes view_work_packages] }
+    let!(:user) do
+      FactoryBot.create(:user,
+                        mail: 'j.doe@openproject.org',
                         member_in_project: project,
                         member_with_permissions: permissions)
     end
 
     let!(:work_package) do
-      FactoryBot.create(:work_package, id: 2, project: project)
+      FactoryBot.create(:work_package,
+                        subject: 'Some subject of the bug',
+                        id: 39733,
+                        project: project).tap do |wp|
+        wp.journals.last.update_column(:id, 99999999)
+      end
     end
 
     subject do
-      submit_email('wp_update_with_multiple_quoted_reply_above.eml')
+      submit_email('wp_mention_reply.eml')
     end
   end
 
-  shared_context 'wp create with cc' do
-    let(:permissions) { %i[add_work_packages view_work_packages add_work_package_watchers] }
+  shared_context 'with a reply to a wp mention with attributes' do
+    let(:permissions) { %i[add_work_package_notes view_work_packages edit_work_packages] }
+    let(:role) do
+      FactoryBot.create(:role, permissions: permissions)
+    end
     let!(:user) do
       FactoryBot.create(:user,
-                        mail: 'JSmith@somenet.foo',
-                        firstname: 'John',
-                        lastname: 'Smith',
+                        mail: 'j.doe@openproject.org',
                         member_in_project: project,
-                        member_with_permissions: permissions)
+                        member_through_role: role)
     end
-    let!(:cc_user) do
+
+    let!(:work_package) do
+      FactoryBot.create(:work_package,
+                        subject: 'Some subject of the bug',
+                        id: 39733,
+                        project: project,
+                        status: original_status).tap do |wp|
+        wp.journals.last.update_column(:id, 99999999)
+      end
+    end
+    let!(:original_status) do
+      FactoryBot.create(:default_status)
+    end
+    let!(:resolved_status) do
+      FactoryBot.create(:status,
+                        name: 'Resolved').tap do |status|
+        FactoryBot.create(:workflow,
+                          old_status: original_status,
+                          new_status: status,
+                          role: role,
+                          type: work_package.type)
+      end
+    end
+    let!(:other_user) do
       FactoryBot.create(:user,
-                        mail: 'dlopper@somenet.foo',
-                        firstname: 'D',
-                        lastname: 'Lopper',
+                        mail: 'jsmith@somenet.foo',
+                        member_in_project: project,
+                        member_through_role: role)
+    end
+    let!(:float_cf) do
+      FactoryBot.create(:float_wp_custom_field,
+                        name: 'float field').tap do |cf|
+        project.work_package_custom_fields << cf
+        work_package.type.custom_fields << cf
+      end
+    end
+
+    subject do
+      submit_email('wp_mention_reply_with_attributes.eml')
+    end
+  end
+
+  shared_context 'with a reply to a message' do
+    let(:permissions) { %i[view_messages add_messages] }
+    let!(:user) do
+      FactoryBot.create(:user,
+                        mail: 'j.doe@openproject.org',
                         member_in_project: project,
                         member_with_permissions: permissions)
     end
-    let(:submit_options) { { issue: { project: project.identifier } } }
+
+    let!(:message) do
+      FactoryBot.create(:message,
+                        id: 70917,
+                        forum: FactoryBot.create(:forum, project: project)).tap do |wp|
+        wp.journals.last.update_column(:id, 99999999)
+      end
+    end
 
     subject do
-      submit_email('ticket_with_cc.eml', **submit_options)
+      submit_email('message_reply.eml')
     end
   end
 
@@ -162,7 +246,7 @@ describe MailHandler, type: :model do
       end
     end
 
-    context 'create work package' do
+    context 'when sending a mail not as a reply' do
       context 'in a given project' do
         let!(:status) { FactoryBot.create(:status, name: 'Resolved') }
         let!(:version) { FactoryBot.create(:version, name: 'alpha', project: project) }
@@ -402,7 +486,7 @@ describe MailHandler, type: :model do
       end
     end
 
-    describe 'update work package' do
+    context 'when sending a reply to work package mail' do
       let!(:mail_user) { FactoryBot.create :admin, mail: 'user@example.org' }
       let!(:work_package) { FactoryBot.create :work_package, project: project }
 
@@ -445,7 +529,7 @@ describe MailHandler, type: :model do
       end
 
       context 'with reply text' do
-        include_context 'wp_update_with_quoted_reply_above'
+        include_context 'with a reply to a wp mention with quotes above'
 
         it_behaves_like 'journal created'
 
@@ -466,6 +550,66 @@ describe MailHandler, type: :model do
         end
       end
 
+      context 'when replying to mention mail with only text' do
+        include_context 'with a reply to a wp mention'
+
+        it_behaves_like 'journal created'
+
+        it 'adds the content to the last journal' do
+          subject
+
+          expect(work_package.journals.reload.last.notes)
+            .to include 'The text of the reply.'
+        end
+
+        it 'does not alter any attributes' do
+          subject
+
+          expect(work_package.journals.reload.last.details)
+            .to be_empty
+        end
+
+        it 'performs the changes in the name of the sender' do
+          subject
+
+          expect(work_package.journals.reload.last.user)
+            .to eql user
+        end
+      end
+
+      context 'when replying to mention mail with text and attributes' do
+        include_context 'with a reply to a wp mention with attributes'
+
+        it_behaves_like 'journal created'
+
+        it 'adds the content to the last journal' do
+          subject
+
+          expect(work_package.journals.reload.last.notes)
+            .to include 'The text of the reply.'
+        end
+
+        it 'alters the attributes' do
+          subject
+
+          expect(work_package.journals.reload.last.details)
+            .to eql(
+              "due_date" => [nil, Date.parse("Fri, 31 Dec 2010")],
+              "status_id" => [original_status.id, resolved_status.id],
+              "assigned_to_id" => [nil, other_user.id],
+              "start_date" => [nil, Date.parse("Fri, 01 Jan 2010")],
+              "custom_fields_#{float_cf.id}" => [nil, "52.6"]
+            )
+        end
+
+        it 'performs the changes in the name of the sender' do
+          subject
+
+          expect(work_package.journals.reload.last.user)
+            .to eql user
+        end
+      end
+
       context 'with a custom field' do
         let(:work_package) { FactoryBot.create :work_package, project: project }
         let(:type) { FactoryBot.create :type }
@@ -515,6 +659,30 @@ describe MailHandler, type: :model do
       end
     end
 
+    context 'when sending a reply to a message mail' do
+      include_context 'with a reply to a message'
+
+      it 'creates a new message in the name of the sender', aggregate_failures: true do
+        expect(subject)
+          .to be_a Message
+
+        expect(subject.subject)
+          .to eql('Response to the original message')
+
+        expect(subject.content)
+          .to include('Test message')
+
+        expect(subject.author)
+          .to eql user
+
+        expect(subject.forum)
+          .to eql message.forum
+
+        expect(subject.parent)
+          .to eql message
+      end
+    end
+
     context 'truncate emails based on the Setting' do
       context 'with no setting', with_settings: { mail_handler_body_delimiters: '' } do
         include_context 'wp_on_given_project'
@@ -552,25 +720,7 @@ describe MailHandler, type: :model do
 
       context 'with a single quoted reply (e.g. reply to a OpenProject email notification)',
               with_settings: { mail_handler_body_delimiters: '--- Reply above. Do not remove this line. ---' } do
-        include_context 'wp_update_with_quoted_reply_above'
-
-        it_behaves_like 'journal created'
-
-        it 'truncates the email at the delimiter with the quoted reply symbols (>)' do
-          expect(subject.notes)
-            .to include('An update to the issue by the sender.')
-
-          expect(subject.notes)
-            .not_to match(Regexp.escape('--- Reply above. Do not remove this line. ---'))
-
-          expect(subject.notes)
-            .not_to include('Looks like the JSON api for projects was missed.')
-        end
-      end
-
-      context 'with multiple quoted replies (e.g. reply to a reply of a Redmine email notification)',
-              with_settings: { mail_handler_body_delimiters: '--- Reply above. Do not remove this line. ---' } do
-        include_context 'wp_update_with_quoted_reply_above'
+        include_context 'with a reply to a wp mention with quotes above'
 
         it_behaves_like 'journal created'
 
@@ -649,36 +799,6 @@ describe MailHandler, type: :model do
     end
   end
 
-  describe '#dispatch_target_from_message_id' do
-    let!(:mail_user) { FactoryBot.create :admin, mail: 'user@example.org' }
-    let(:instance) do
-      mh = MailHandler.new
-      mh.options = {}
-      mh
-    end
-    subject { instance.receive mail }
-
-    context 'receiving reply from work package' do
-      let(:mail) { Mail.new(read_email('work_package_reply.eml')) }
-
-      it 'calls the work package reply' do
-        expect(instance).to receive(:receive_work_package_reply).with(34540)
-
-        subject
-      end
-    end
-
-    context 'receiving reply from message' do
-      let(:mail) { Mail.new(read_email('message_reply.eml')) }
-
-      it 'calls the work package reply' do
-        expect(instance).to receive(:receive_message_reply).with(12559)
-
-        subject
-      end
-    end
-  end
-
   private
 
   FIXTURES_PATH = File.dirname(__FILE__) + '/../fixtures/mail_handler'

spec/workers/copy_project_job_spec.rb

@@ -230,10 +230,11 @@ describe CopyProjectJob, type: :model do
 
         it "notifies the user of the success" do
           mail = ActionMailer::Base.deliveries
-                   .find { |m| m.message_id.start_with? "openproject.project-#{user.id}-#{subject.id}" }
+                   .find { |m| m.message_id.start_with? "op.project-#{subject.id}" }
 
           expect(mail).to be_present
           expect(mail.subject).to eq "Created project #{subject.name}"
+          expect(mail.to).to eq [user.mail]
         end
       end
 
@@ -263,10 +264,11 @@ describe CopyProjectJob, type: :model do
 
         it "notifies the user of the success" do
           mail = ActionMailer::Base.deliveries
-            .find { |m| m.message_id.start_with? "openproject.project-#{user.id}-#{subject.id}" }
+                                   .find { |m| m.message_id.start_with? "op.project-#{subject.id}" }
 
           expect(mail).to be_present
           expect(mail.subject).to eq "Created project #{subject.name}"
+          expect(mail.to).to eq [user.mail]
         end
       end
     end

spec_legacy/fixtures/mail_handler/message_reply.eml

@@ -1,15 +0,0 @@
-Message-ID: <4974C93E.3070005@somenet.foo>
-Date: Mon, 19 Jan 2009 19:41:02 +0100
-From: "John Smith" <jsmith@somenet.foo>
-User-Agent: Thunderbird 2.0.0.19 (Windows/20081209)
-MIME-Version: 1.0
-To: openproject@somenet.foo
-Subject: Reply via email
-References: <openproject.message-1-2.20070512171800@somenet.foo>
-In-Reply-To: <openproject.message-1-2.20070512171800@somenet.foo>
-Content-Type: text/plain; charset=UTF-8; format=flowed
-Content-Transfer-Encoding: 7bit
-
-This is a reply to a forum message.
-
-

spec_legacy/fixtures/mail_handler/message_reply_by_subject.eml

@@ -1,13 +0,0 @@
-Message-ID: <4974C93E.3070005@somenet.foo>
-Date: Mon, 19 Jan 2009 19:41:02 +0100
-From: "John Smith" <jsmith@somenet.foo>
-User-Agent: Thunderbird 2.0.0.19 (Windows/20081209)
-MIME-Version: 1.0
-To: redmine@somenet.foo
-Subject: Re: [eCookbook - Help board - msg2] Reply to the first post
-Content-Type: text/plain; charset=UTF-8; format=flowed
-Content-Transfer-Encoding: 7bit
-
-This is a reply to a forum message.
-
-

spec_legacy/fixtures/mail_handler/ticket_reply.eml

@@ -1,74 +0,0 @@
-Return-Path: <jsmith@somenet.foo>
-Received: from osiris ([127.0.0.1])
-	by OSIRIS
-	with hMailServer ; Sat, 21 Jun 2008 18:41:39 +0200
-Message-ID: <006a01c8d3bd$ad9baec0$0a00a8c0@osiris>
-In-Reply-To: <openproject.issue-1-2.20060719210421@osiris>
-From: "John Smith" <jsmith@somenet.foo>
-To: <openproject@somenet.foo>
-References: <485d0ad366c88_d7014663a025f@osiris.tmail>
-Subject: Re: Add ingredients categories
-Date: Sat, 21 Jun 2008 18:41:39 +0200
-MIME-Version: 1.0
-Content-Type: multipart/alternative;
-	boundary="----=_NextPart_000_0067_01C8D3CE.711F9CC0"
-X-Priority: 3
-X-MSMail-Priority: Normal
-X-Mailer: Microsoft Outlook Express 6.00.2900.2869
-X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2869
-
-This is a multi-part message in MIME format.
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/plain;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-This is reply
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/html;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-=EF=BB=BF<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
-<HTML><HEAD>
-<META http-equiv=3DContent-Type content=3D"text/html; charset=3Dutf-8">
-<STYLE>BODY {
-	FONT-SIZE: 0.8em; COLOR: #484848; FONT-FAMILY: Verdana, sans-serif
-}
-BODY H1 {
-	FONT-SIZE: 1.2em; MARGIN: 0px; FONT-FAMILY: "Trebuchet MS", Verdana, =
-sans-serif
-}
-A {
-	COLOR: #2a5685
-}
-A:link {
-	COLOR: #2a5685
-}
-A:visited {
-	COLOR: #2a5685
-}
-A:hover {
-	COLOR: #c61a1a
-}
-A:active {
-	COLOR: #c61a1a
-}
-HR {
-	BORDER-RIGHT: 0px; BORDER-TOP: 0px; BACKGROUND: #ccc; BORDER-LEFT: 0px; =
-WIDTH: 100%; BORDER-BOTTOM: 0px; HEIGHT: 1px
-}
-.footer {
-	FONT-SIZE: 0.8em; FONT-STYLE: italic
-}
-</STYLE>
-
-<META content=3D"MSHTML 6.00.2900.2883" name=3DGENERATOR></HEAD>
-<BODY bgColor=3D#ffffff>
-<DIV><SPAN class=3Dfooter><FONT face=3DArial color=3D#000000 =
-size=3D2>This is=20
-reply</FONT></DIV></SPAN></BODY></HTML>
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0--
-

spec_legacy/fixtures/mail_handler/ticket_reply_by_message_id.eml

@@ -1,74 +0,0 @@
-Return-Path: <jsmith@somenet.foo>
-Received: from osiris ([127.0.0.1])
-	by OSIRIS
-	with hMailServer ; Sat, 21 Jun 2008 18:41:39 +0200
-Message-ID: <006a01c8d3bd$ad9baec0$0a00a8c0@osiris>
-In-Reply-To: <openproject.issue_journal-1-3.20110617112550@example.net>
-From: "John Smith" <jsmith@somenet.foo>
-To: <openproject@somenet.foo>
-References: <485d0ad366c88_d7014663a025f@osiris.tmail>
-Subject: Re: Add ingredients categories
-Date: Sat, 21 Jun 2008 18:41:39 +0200
-MIME-Version: 1.0
-Content-Type: multipart/alternative;
-	boundary="----=_NextPart_000_0067_01C8D3CE.711F9CC0"
-X-Priority: 3
-X-MSMail-Priority: Normal
-X-Mailer: Microsoft Outlook Express 6.00.2900.2869
-X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2869
-
-This is a multi-part message in MIME format.
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/plain;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-This is reply
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/html;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-=EF=BB=BF<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
-<HTML><HEAD>
-<META http-equiv=3DContent-Type content=3D"text/html; charset=3Dutf-8">
-<STYLE>BODY {
-	FONT-SIZE: 0.8em; COLOR: #484848; FONT-FAMILY: Verdana, sans-serif
-}
-BODY H1 {
-	FONT-SIZE: 1.2em; MARGIN: 0px; FONT-FAMILY: "Trebuchet MS", Verdana, =
-sans-serif
-}
-A {
-	COLOR: #2a5685
-}
-A:link {
-	COLOR: #2a5685
-}
-A:visited {
-	COLOR: #2a5685
-}
-A:hover {
-	COLOR: #c61a1a
-}
-A:active {
-	COLOR: #c61a1a
-}
-HR {
-	BORDER-RIGHT: 0px; BORDER-TOP: 0px; BACKGROUND: #ccc; BORDER-LEFT: 0px; =
-WIDTH: 100%; BORDER-BOTTOM: 0px; HEIGHT: 1px
-}
-.footer {
-	FONT-SIZE: 0.8em; FONT-STYLE: italic
-}
-</STYLE>
-
-<META content=3D"MSHTML 6.00.2900.2883" name=3DGENERATOR></HEAD>
-<BODY bgColor=3D#ffffff>
-<DIV><SPAN class=3Dfooter><FONT face=3DArial color=3D#000000 =
-size=3D2>This is=20
-reply</FONT></DIV></SPAN></BODY></HTML>
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0--
-

spec_legacy/fixtures/mail_handler/ticket_reply_with_status.eml

@@ -1,80 +0,0 @@
-Return-Path: <jsmith@somenet.foo>
-Received: from osiris ([127.0.0.1])
-	by OSIRIS
-	with hMailServer ; Sat, 21 Jun 2008 18:41:39 +0200
-Message-ID: <006a01c8d3bd$ad9baec0$0a00a8c0@osiris>
-From: "John Smith" <jsmith@somenet.foo>
-To: <openproject@somenet.foo>
-References: <485d0ad366c88_d7014663a025f@osiris.tmail>
-Subject: Re: Cookbook - New Feature #2: Add ingredients categories
-Date: Sat, 21 Jun 2008 18:41:39 +0200
-MIME-Version: 1.0
-Content-Type: multipart/alternative;
-	boundary="----=_NextPart_000_0067_01C8D3CE.711F9CC0"
-X-Priority: 3
-X-MSMail-Priority: Normal
-X-Mailer: Microsoft Outlook Express 6.00.2900.2869
-X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2869
-
-This is a multi-part message in MIME format.
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/plain;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-This is reply
-
-Status: Resolved
-due date: 2010-12-31
-Start Date:2010-01-01
-Assigned to: jsmith@somenet.foo
-float field: 52.6
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0
-Content-Type: text/html;
-	charset="utf-8"
-Content-Transfer-Encoding: quoted-printable
-
-=EF=BB=BF<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
-<HTML><HEAD>
-<META http-equiv=3DContent-Type content=3D"text/html; charset=3Dutf-8">
-<STYLE>BODY {
-	FONT-SIZE: 0.8em; COLOR: #484848; FONT-FAMILY: Verdana, sans-serif
-}
-BODY H1 {
-	FONT-SIZE: 1.2em; MARGIN: 0px; FONT-FAMILY: "Trebuchet MS", Verdana, =
-sans-serif
-}
-A {
-	COLOR: #2a5685
-}
-A:link {
-	COLOR: #2a5685
-}
-A:visited {
-	COLOR: #2a5685
-}
-A:hover {
-	COLOR: #c61a1a
-}
-A:active {
-	COLOR: #c61a1a
-}
-HR {
-	BORDER-RIGHT: 0px; BORDER-TOP: 0px; BACKGROUND: #ccc; BORDER-LEFT: 0px; =
-WIDTH: 100%; BORDER-BOTTOM: 0px; HEIGHT: 1px
-}
-.footer {
-	FONT-SIZE: 0.8em; FONT-STYLE: italic
-}
-</STYLE>
-
-<META content=3D"MSHTML 6.00.2900.2883" name=3DGENERATOR></HEAD>
-<BODY bgColor=3D#ffffff>
-<DIV><SPAN class=3Dfooter><FONT face=3DArial color=3D#000000 =
-size=3D2>This is=20
-reply Status: Resolved</FONT></DIV></SPAN></BODY></HTML>
-
-------=_NextPart_000_0067_01C8D3CE.711F9CC0--
-

spec_legacy/unit/mail_handler_spec.rb

@@ -267,72 +267,6 @@ describe MailHandler, type: :model do
     end
   end
 
-  it 'should add work package note' do
-    journal = submit_email('ticket_reply.eml')
-    assert journal.is_a?(Journal)
-    assert_equal User.find_by_login('jsmith'), journal.user
-    assert_equal WorkPackage.find(2), journal.journable
-    assert_match /This is reply/, journal.notes
-    assert_equal 'Feature request', journal.journable.type.name
-  end
-
-  specify 'reply to issue update (Journal) by message_id' do
-    Journal.delete_all
-    FactoryBot.create :work_package_journal, id: 3, version: 1, journable_id: 2
-    journal = submit_email('ticket_reply_by_message_id.eml')
-    assert journal.data.is_a?(Journal::WorkPackageJournal), "Email was a #{journal.data.class}"
-    assert_equal User.find_by_login('jsmith'), journal.user
-    assert_equal WorkPackage.find(2), journal.journable
-    assert_match /This is reply/, journal.notes
-    assert_equal 'Feature request', journal.journable.type.name
-  end
-
-  it 'should add work package note with attribute changes' do
-    # This email contains: 'Status: Resolved'
-    journal = submit_email('ticket_reply_with_status.eml')
-    assert journal.data.is_a?(Journal::WorkPackageJournal)
-    issue = WorkPackage.find(journal.journable.id)
-    assert_equal User.find_by_login('jsmith'), journal.user
-    assert_equal WorkPackage.find(2), journal.journable
-    assert_match /This is reply/, journal.notes
-    assert_equal 'Feature request', journal.journable.type.name
-    assert_equal Status.find_by(name: 'Resolved'), issue.status
-    assert_equal '2010-01-01', issue.start_date.to_s
-    assert_equal '2010-12-31', issue.due_date.to_s
-    assert_equal User.find_by_login('jsmith'), issue.assigned_to
-    assert_equal '52.6', issue.custom_value_for(CustomField.find_by(name: 'Float field')).value
-    # keywords should be removed from the email body
-    assert !journal.notes.match(/^Status:/i)
-    assert !journal.notes.match(/^Start Date:/i)
-  end
-
-  it 'should add work package note should not set defaults' do
-    journal = submit_email('ticket_reply.eml', issue: { type: 'Support request', priority: 'High' })
-    assert journal.is_a?(Journal)
-    assert_match /This is reply/, journal.notes
-    assert_equal 'Feature request', journal.journable.type.name
-    assert_equal 'Normal', journal.journable.priority.name
-  end
-
-  it 'should reply to a message' do
-    m = submit_email('message_reply.eml')
-    assert m.is_a?(Message)
-    assert !m.new_record?
-    m.reload
-    assert_equal 'Reply via email', m.subject
-    # The email replies to message #2 which is part of the thread of message #1
-    assert_equal Message.find(1), m.parent
-  end
-
-  it 'should reply to a message by subject' do
-    m = submit_email('message_reply_by_subject.eml')
-    assert m.is_a?(Message)
-    assert !m.new_record?
-    m.reload
-    assert_equal 'Reply to the first post', m.subject
-    assert_equal Message.find(1), m.parent
-  end
-
   it 'should strip tags of html only emails' do
     issue = submit_email('ticket_html_only.eml', issue: { project: 'ecookbook' })
     assert issue.is_a?(WorkPackage)