use first mail in tickets instead of last to choose reply address. mitigates problem with shipping confirmation mails

core/mail/tests/v2/test_mails.py

@@ -656,7 +656,7 @@ dGVzdGltYWdl
             recipient='test2@localhost',
             issue_thread=issue_thread,
         )
-        mail1_reply = Email.objects.create(
+        mail2 = Email.objects.create(
             subject='Message received',
             body='Thank you for your message.',
             sender='test2@localhost',
@@ -664,9 +664,16 @@ dGVzdGltYWdl
             in_reply_to=mail1.reference,
             issue_thread=issue_thread,
         )
+        Email.objects.create(
+            subject='Re: Message received',
+            body='any updates?',
+            sender='test3@test',
+            recipient='test2@localhost',
+            in_reply_to=mail2.reference,
+            issue_thread=issue_thread,
+        )
         from aiosmtpd.smtp import Envelope
         from asgiref.sync import async_to_sync
-        from email.message import EmailMessage
         import aiosmtplib
         import logging
         logging.disable(logging.CRITICAL)
@@ -682,33 +689,33 @@ dGVzdGltYWdl
         result = async_to_sync(handler.handle_DATA)(server, session, envelope)
         logging.disable(logging.NOTSET)
         self.assertEqual('250 Message accepted for delivery', result)
-        self.assertEqual(3, len(Email.objects.all()))
+        self.assertEqual(4, len(Email.objects.all()))
-        self.assertEqual(3, len(Email.objects.filter(issue_thread=issue_thread)))
+        self.assertEqual(4, len(Email.objects.filter(issue_thread=issue_thread)))
         self.assertEqual(1, len(IssueThread.objects.all()))
         aiosmtplib.send.assert_not_called()
-        self.assertEqual(Email.objects.all()[2].subject, 'foo')
+        self.assertEqual(Email.objects.all()[3].subject, 'foo')
-        self.assertEqual(Email.objects.all()[2].sender, '<test1@test>')
+        self.assertEqual(Email.objects.all()[3].sender, '<test1@test>')
-        self.assertEqual(Email.objects.all()[2].recipient, 'ticket+{}@test'.format(issue_thread.uuid))
+        self.assertEqual(Email.objects.all()[3].recipient, 'ticket+{}@test'.format(issue_thread.uuid))
-        self.assertEqual(Email.objects.all()[2].body, 'bar')
+        self.assertEqual(Email.objects.all()[3].body, 'bar')
-        self.assertEqual(Email.objects.all()[2].issue_thread, issue_thread)
+        self.assertEqual(Email.objects.all()[3].issue_thread, issue_thread)
-        self.assertEqual(Email.objects.all()[2].reference, '<3@test>')
+        self.assertEqual(Email.objects.all()[3].reference, '<3@test>')
         self.assertEqual('test subject', IssueThread.objects.all()[0].name)
         response = self.client.post(f'/api/2/tickets/{issue_thread.id}/reply/', {
             'message': 'test'
         })
-        aiosmtplib.send.assert_called_once();
+        aiosmtplib.send.assert_called_once()
         self.assertEqual(response.status_code, 201)
-        self.assertEqual(4, len(Email.objects.all()))
+        self.assertEqual(5, len(Email.objects.all()))
-        self.assertEqual(4, len(Email.objects.filter(issue_thread=issue_thread)))
+        self.assertEqual(5, len(Email.objects.filter(issue_thread=issue_thread)))
         self.assertEqual(1, len(IssueThread.objects.all()))
-        self.assertEqual(Email.objects.all()[3].subject, 'Re: test subject [#{0}]'.format(issue_thread.short_uuid()))
+        self.assertEqual(Email.objects.all()[4].subject, 'Re: test subject [#{0}]'.format(issue_thread.short_uuid()))
-        self.assertEqual(Email.objects.all()[3].sender, 'test2@localhost')
+        self.assertEqual(Email.objects.all()[4].sender, 'test2@localhost')
-        self.assertEqual(Email.objects.all()[3].recipient, 'test1@test')
+        self.assertEqual(Email.objects.all()[4].recipient, 'test1@test')
-        self.assertEqual(Email.objects.all()[3].body, 'test')
+        self.assertEqual(Email.objects.all()[4].body, 'test')
-        self.assertEqual(Email.objects.all()[3].issue_thread, issue_thread)
+        self.assertEqual(Email.objects.all()[4].issue_thread, issue_thread)
-        self.assertTrue(Email.objects.all()[3].reference.startswith("<"))
+        self.assertTrue(Email.objects.all()[4].reference.startswith("<"))
-        self.assertTrue(Email.objects.all()[3].reference.endswith("@localhost>"))
+        self.assertTrue(Email.objects.all()[4].reference.endswith("@localhost>"))
-        self.assertEqual(Email.objects.all()[3].in_reply_to, mail1.reference)
+        self.assertEqual(Email.objects.all()[4].in_reply_to, mail1.reference)
         self.assertEqual('test subject', IssueThread.objects.all()[0].name)
         self.assertEqual('pending_new', IssueThread.objects.all()[0].state)
         self.assertEqual(None, IssueThread.objects.all()[0].assigned_to)

core/tickets/api_v2.py

@@ -113,15 +113,17 @@ def reply(request, pk):
     issue = IssueThread.objects.get(pk=pk)
     # email = issue.reply(request.data['body']) # TODO evaluate if this is a useful abstraction
     references = collect_references(issue)
-    most_recent = Email.objects.filter(issue_thread=issue, recipient__endswith='@' + MAIL_DOMAIN).order_by(
+    first_mail = Email.objects.filter(issue_thread=issue, recipient__endswith='@' + MAIL_DOMAIN).order_by(
-        '-timestamp').first()
+        'timestamp').first()
+    if not first_mail:
+        return Response({'status': 'error', 'message': 'no previous mail found, reply would not make sense.'}, status=status.HTTP_400_BAD_REQUEST)
     mail = Email.objects.create(
         issue_thread=issue,
-        sender=most_recent.recipient,
+        sender=first_mail.recipient,
-        recipient=most_recent.sender,
+        recipient=first_mail.sender,
-        subject=f'Re: {most_recent.subject}',
+        subject=f'Re: {issue.name} [#{issue.short_uuid()}]',
         body=request.data['message'],
-        in_reply_to=most_recent.reference,
+        in_reply_to=first_mail.reference,
     )
     log = logging.getLogger('mail.log')
     async_to_sync(send_smtp)(make_reply(mail, references), log)