mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Send In-Reply-To and References on a reply (#227)
* Send In-Reply-To and References on a reply buildEmail() never wrote them, so every reply from the web composer or JMAP reached the recipient as a new conversation. References ends with the parent even when the draft's list lacks it (RFC 5322 3.6.4). Closes #206 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * References ends with the parent even when the draft listed it earlier normaliseList() keeps the first of a repeated id, so a draft whose References already named the parent somewhere other than last (a JMAP client's own order) sent a chain that did not end with it. The parent is now taken out of the list before it is appended. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: Paul Lützner <karatektus@gmail.com>
This commit is contained in:
3 files changed
+159
No files matched your search
@@ -8,6 +8,14 @@ The published image tags: `latest` follows the most recent release below,
|
||||
|
||||
## Unreleased
|
||||
|
||||
**A reply threads for the person who gets it.** A reply sent from plMail, in the web composer or
|
||||
over JMAP, reached the recipient without `In-Reply-To` or `References`, so Outlook, Gmail and Apple
|
||||
Mail showed it as a new conversation instead of part of the one it answered. The draft always held
|
||||
both; the mail built from it never carried them. They are written now, and `References` always
|
||||
ends with the message answered, as RFC 5322 asks, even when the draft's own list left it out. A mail
|
||||
that is not a reply is unchanged, and replies already sent stay as they went out. No schema change.
|
||||
[#206](https://github.com/karatektus/pl_mail/issues/206), by @phdelodder.
|
||||
|
||||
**A send that fails for a passing reason is tried again, and one that finally fails is marked on
|
||||
the draft.** Every failed send used to be dropped at once, whatever the reason, so a one-minute
|
||||
Gmail rate limit or a short outage lost the mail and only a toast said so. A rate limit, a provider
|
||||
|
||||
@@ -401,6 +401,7 @@ class MessageSendService
|
||||
|
||||
$this->applyPriority($message, $email);
|
||||
$this->applyReceiptRequest($message, $email, $fromAddress);
|
||||
$this->applyThreadingHeaders($message, $email);
|
||||
|
||||
$toAddresses = $message->toAddresses;
|
||||
if (null !== $toAddresses) {
|
||||
@@ -457,6 +458,40 @@ class MessageSendService
|
||||
return $email;
|
||||
}
|
||||
|
||||
/**
|
||||
* In-Reply-To and References, so a reply threads for whoever receives it.
|
||||
*
|
||||
* The draft always carried them (ReplyDraftBuilder fills them, and JMAP
|
||||
* Email/set stores what the client sent) and nothing wrote them onto the
|
||||
* mail, so every reply left as a message of its own and Outlook, Gmail and
|
||||
* Apple Mail showed a new conversation. The ids are written bracket-less
|
||||
* because that is how Symfony's ID header takes them.
|
||||
*
|
||||
* References ends with the parent: RFC 5322 §3.6.4 has a reply carry its
|
||||
* parent's References plus the parent's Message-ID, and a draft whose
|
||||
* list lacks the parent (a JMAP client that set only inReplyTo) would
|
||||
* otherwise send a chain that stops one short. The parent is taken out of
|
||||
* the list before it is appended, so a list that already names it
|
||||
* somewhere else — a client's own ordering — still ends with it rather
|
||||
* than keeping it where normaliseList()'s de-duplication found it first.
|
||||
*/
|
||||
private function applyThreadingHeaders(Message $message, Email $email): void
|
||||
{
|
||||
$inReplyTo = MessageIdHelper::normaliseList($message->inReplyTo);
|
||||
$references = MessageIdHelper::normaliseList(array_merge(
|
||||
array_diff(MessageIdHelper::normaliseList($message->references), $inReplyTo),
|
||||
$inReplyTo,
|
||||
));
|
||||
|
||||
if ([] !== $inReplyTo) {
|
||||
$email->getHeaders()->addIdHeader('In-Reply-To', $inReplyTo);
|
||||
}
|
||||
|
||||
if ([] !== $references) {
|
||||
$email->getHeaders()->addIdHeader('References', $references);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Both priority headers, or neither.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\Imap;
|
||||
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Service\Imap\MessageSendService;
|
||||
use ReflectionMethod;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
use Symfony\Component\Mime\Email;
|
||||
use Symfony\Component\Mime\Header\IdentificationHeader;
|
||||
|
||||
/**
|
||||
* #206. A reply left plMail without In-Reply-To or References, so the
|
||||
* recipient's client showed it as a new conversation.
|
||||
*
|
||||
* The draft always had the ids; buildEmail() never wrote them. The assertion
|
||||
* is made on the MIME the mail is built into, because the bug lived between
|
||||
* the stored row and the wire. buildEmail() is private and reached by
|
||||
* reflection, as in OutgoingSenderNameAndImagesTest, and nothing is persisted.
|
||||
*/
|
||||
final class OutgoingThreadingHeadersTest extends KernelTestCase
|
||||
{
|
||||
public function testAReplyCarriesItsParentAndTheChainAboveIt(): void
|
||||
{
|
||||
$message = $this->message();
|
||||
$message->inReplyTo = ['parent@example.test'];
|
||||
$message->references = ['root@example.test', 'parent@example.test'];
|
||||
|
||||
$email = $this->build($message);
|
||||
|
||||
self::assertSame(['parent@example.test'], $this->ids($email, 'In-Reply-To'));
|
||||
self::assertSame(['root@example.test', 'parent@example.test'], $this->ids($email, 'References'));
|
||||
}
|
||||
|
||||
public function testReferencesEndsWithTheParentEvenWhenTheDraftLeftItOut(): void
|
||||
{
|
||||
$message = $this->message();
|
||||
$message->inReplyTo = ['parent@example.test'];
|
||||
$message->references = ['root@example.test'];
|
||||
|
||||
$references = $this->ids($this->build($message), 'References');
|
||||
|
||||
self::assertSame(['root@example.test', 'parent@example.test'], $references);
|
||||
}
|
||||
|
||||
/**
|
||||
* A list that names the parent somewhere other than last — a JMAP client
|
||||
* is free to send its own order — still ends with it, which is the one
|
||||
* position a threading client reads it from.
|
||||
*/
|
||||
public function testReferencesEndsWithTheParentEvenWhenTheDraftListedItEarlier(): void
|
||||
{
|
||||
$message = $this->message();
|
||||
$message->inReplyTo = ['parent@example.test'];
|
||||
$message->references = ['parent@example.test', 'root@example.test'];
|
||||
|
||||
$references = $this->ids($this->build($message), 'References');
|
||||
|
||||
self::assertSame(['root@example.test', 'parent@example.test'], $references);
|
||||
}
|
||||
|
||||
public function testAReplyWithNoReferencesListStillGetsTheParentAsOne(): void
|
||||
{
|
||||
$message = $this->message();
|
||||
$message->inReplyTo = ['<parent@example.test>'];
|
||||
|
||||
$email = $this->build($message);
|
||||
|
||||
self::assertSame(['parent@example.test'], $this->ids($email, 'In-Reply-To'));
|
||||
self::assertSame(['parent@example.test'], $this->ids($email, 'References'));
|
||||
}
|
||||
|
||||
public function testAMailThatIsNotAReplyGetsNeitherHeader(): void
|
||||
{
|
||||
$headers = $this->build($this->message())->getHeaders();
|
||||
|
||||
self::assertFalse($headers->has('In-Reply-To'));
|
||||
self::assertFalse($headers->has('References'));
|
||||
}
|
||||
|
||||
/** @return list<string> */
|
||||
private function ids(Email $email, string $name): array
|
||||
{
|
||||
$header = $email->getHeaders()->get($name);
|
||||
|
||||
self::assertInstanceOf(IdentificationHeader::class, $header, $name . ' should be on the mail');
|
||||
|
||||
return $header->getIds();
|
||||
}
|
||||
|
||||
private function build(Message $message): Email
|
||||
{
|
||||
$service = self::getContainer()->get(MessageSendService::class);
|
||||
|
||||
return new ReflectionMethod($service, 'buildEmail')->invoke($service, $message, $message->account);
|
||||
}
|
||||
|
||||
private function message(): Message
|
||||
{
|
||||
$account = new Account();
|
||||
$account->name = 'John Smith';
|
||||
$account->email = 'me@example.test';
|
||||
|
||||
$message = new Message();
|
||||
$message->account = $account;
|
||||
$message->fromAddress = 'me@example.test';
|
||||
$message->subject = 'Re: Hello';
|
||||
$message->bodyHtml = '<p>Hello</p>';
|
||||
$message->toAddresses = [['name' => 'Rike', 'address' => 'rike@example.test']];
|
||||
|
||||
return $message;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user