feat(mail): drag a conversation onto, and out of, the reader's own tab

Dropping on one of the reader's own tabs files the conversation there and pins
it, leaving the built-in category alone (JMAP clients are served that column).
Dropping on a built-in tab from one clears the reader's tab, so a conversation
is never in two. "Already there" is a statement about both columns. The route
resolves the posted key against the dropping reader's own categories, so
somebody else's category is refused like a made-up one and nothing moves.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/Controller/Mail/BulkStatusController.php

@@ -8,13 +8,14 @@ use App\Controller\ChecksCsrf;
 use App\Controller\RendersTurboStreams;
 use App\Domain\Enum\Job\JobKind;
 use App\Domain\Enum\Mail\LabelRole;
-use App\Domain\Enum\Mail\MessageCategory;
 use App\Entity\Job\BackgroundJob;
 use App\Entity\User\User;
 use App\Infrastructure\Messaging\Message\RunBulkStatusMessage;
 use App\Repository\Label\LabelRepository;
+use App\Repository\Mail\InboxCategoryRepository;
 use App\Repository\Mail\MessageThreadRepository;
 use App\Security\Voter\OwnershipVoter;
+use App\Service\Mail\InboxTabs;
 use App\Service\Mail\MailPlacement;
 use App\Service\Mail\MessagePurger;
 use App\Service\Mail\MoveToService;
@@ -91,6 +92,8 @@ final class BulkStatusController extends AbstractController
         private readonly StatusUndoService       $undo,
         private readonly MessagePurger           $purger,
         private readonly MailPlacement           $placement,
+        private readonly InboxTabs               $inboxTabs,
+        private readonly InboxCategoryRepository $inboxCategories,
     ) {
     }
 
@@ -211,7 +214,10 @@ final class BulkStatusController extends AbstractController
         $destination = null;
 
         if ('category' === $action) {
-            $category = MessageCategory::tryFrom((string) ($body['category'] ?? ''));
+            // Resolved against THIS reader's own tabs as well as the five, and
+            // a key that is somebody else's category is refused exactly like a
+            // made-up one: see InboxTabs::resolve().
+            $category = $this->inboxTabs->resolve($user, (string) ($body['category'] ?? ''));
 
             // tryFrom and a refusal, where MailController falls back to Primary
             // on the same input. The difference is what the two are: `?tab=` is
@@ -397,8 +403,18 @@ final class BulkStatusController extends AbstractController
         // strip counts — so this one is applied to the threads directly and
         // returns before the grouping it has no use for.
         if ('category' === $action) {
+            // The entity for one of the reader's own tabs, the enum for a
+            // built-in one: setCategory() writes whichever column it names.
+            $target = true === $category->isCustom()
+                ? $this->inboxCategories->find((int) $category->categoryId)
+                : $category->builtIn;
+
+            if (null === $target) {
+                throw $this->createAccessDeniedException('Unknown inbox category.');
+            }
+
             foreach ($threads as $thread) {
-                $this->status->setCategory($thread, $category);
+                $this->status->setCategory($thread, $target);
             }
 
             return $this->renderTurboStream('thread/status/_category.stream.html.twig', [

src/Service/Mail/ThreadStatusUpdater.php

@@ -10,6 +10,7 @@ use App\Infrastructure\Messaging\Message\ExtractEventsMessage;
 use App\Domain\DTO\Mail\RemoteFlagState;
 use App\Domain\Enum\Mail\LabelRole;
 use App\Domain\Enum\Mail\MessageCategory;
+use App\Entity\Mail\InboxCategory;
 use App\Domain\Enum\Mail\MessageFlag;
 use App\Entity\Label\Label;
 use App\Entity\Mail\Account;
@@ -370,13 +371,33 @@ final readonly class ThreadStatusUpdater
      *
      * @return bool whether anything changed
      */
-    public function setCategory(MessageThread $thread, MessageCategory $category): bool
+    public function setCategory(MessageThread $thread, MessageCategory|InboxCategory $category): bool
     {
-        if ($category === $thread->category && null !== $thread->categoryPinnedAt) {
-            return false;
+        // TWO COLUMNS, AND A DROP WRITES THE RIGHT ONE. The reader's own tab is
+        // MessageThread::$userCategory and a built-in one is $category, and a
+        // conversation is in exactly one tab, so moving between the two kinds has
+        // to clear the other: a drop onto Primary from "Shipping" that left
+        // $userCategory set would leave it in Shipping, and a drop onto Shipping
+        // leaves the built-in column alone because that one is what JMAP serves.
+        //
+        // "Already there" is therefore a statement about BOTH columns. A
+        // conversation whose built-in category matches the drop but which is
+        // still in one of the reader's tabs is not already there.
+        if ($category instanceof InboxCategory) {
+            if ($category === $thread->userCategory && null !== $thread->categoryPinnedAt) {
+                return false;
+            }
+
+            $thread->userCategory = $category;
+        } else {
+            if ($category === $thread->category && null === $thread->userCategory && null !== $thread->categoryPinnedAt) {
+                return false;
+            }
+
+            $thread->category     = $category;
+            $thread->userCategory = null;
         }
 
-        $thread->category         = $category;
         $thread->categoryPinnedAt = new DateTimeImmutable();
 
         // The thread's own row moved between two tabs; no message changed, so

templates/thread/status/_category.stream.html.twig

@@ -13,7 +13,7 @@ Saying where it went is the whole of the feedback.
 Variables:
   count     how many conversations were re-filed
   threads   the conversations, so their rows can be removed
-  category  the destination tab, for the toast
+  category  the destination tab (an InboxTab), for the toast
 #}
 {% include 'thread/status/_bulk.stream.html.twig' with {
     count:   count,
@@ -21,4 +21,4 @@ Variables:
     leaves:  true,
 } only %}
 
-{% include '_partials/_toast.stream.html.twig' with { message: 'toast.categorised'|trans({ '%category%': ('tabs.' ~ category.value)|trans }) } only %}
+{% include '_partials/_toast.stream.html.twig' with { message: 'toast.categorised'|trans({ '%category%': category.isCustom ? category.name : ('tabs.' ~ category.value)|trans }) } only %}

tests/Controller/Mail/BulkCategoryDropTest.php

@@ -0,0 +1,151 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Mail;
+
+use App\Domain\Enum\Mail\LabelRole;
+use App\Domain\Enum\Mail\MessageCategory;
+use App\Entity\Mail\InboxCategory;
+use App\Entity\Mail\MessageThread;
+use App\Entity\User\User;
+use App\Tests\Support\Mail\SeedsMarkerFixtures;
+use Doctrine\DBAL\Connection;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\KernelBrowser;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+/**
+ * The drop on a category tab, from the browser's end, for a tab the reader made.
+ *
+ * A drop is a WRITE, so an unrecognised key is refused rather than quietly filed
+ * under Primary (BulkMoveGuardTest holds that for the built-ins). What is new is
+ * whose key counts: only the dropping reader's own categories. Somebody else's
+ * category id in the body is the same refusal as a made-up one, and it must
+ * leave the conversation exactly where it was.
+ */
+final class BulkCategoryDropTest extends WebTestCase
+{
+    use SeedsMarkerFixtures;
+
+    private KernelBrowser $client;
+    private Connection $connection;
+
+    protected function setUp(): void
+    {
+        $this->client = static::createClient();
+        $this->client->disableReboot();
+
+        $container        = static::getContainer();
+        $this->em         = $container->get(EntityManagerInterface::class);
+        $this->connection = $container->get(Connection::class);
+
+        $this->connection->beginTransaction();
+
+        $this->user    = $this->seedUser();
+        $this->account = $this->seedAccount();
+        $this->inbox   = $this->seedLabel('Inbox', LabelRole::Inbox);
+
+        $this->client->loginUser($this->user);
+    }
+
+    protected function tearDown(): void
+    {
+        if (true === isset($this->connection) && true === $this->connection->isTransactionActive()) {
+            $this->connection->rollBack();
+        }
+
+        parent::tearDown();
+    }
+
+    public function testADropOnTheReadersOwnTabFilesTheConversationThere(): void
+    {
+        $shipping = $this->category($this->user, 'Shipping');
+        $thread   = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+
+        $this->drop('c' . $shipping->id, [(int) $thread->id]);
+
+        self::assertResponseIsSuccessful();
+
+        $filed = $this->reload($thread);
+        self::assertSame($shipping->id, $filed->userCategory?->id);
+        self::assertNotNull($filed->categoryPinnedAt);
+        self::assertSame(MessageCategory::Promotions, $filed->category);
+    }
+
+    public function testADropOnABuiltInTabStillWorksAndTakesTheConversationOutOfTheReadersTab(): void
+    {
+        $shipping             = $this->category($this->user, 'Shipping');
+        $thread               = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+        $thread->userCategory = $shipping;
+        $this->em->flush();
+
+        $this->drop('primary', [(int) $thread->id]);
+
+        self::assertResponseIsSuccessful();
+
+        $moved = $this->reload($thread);
+        self::assertNull($moved->userCategory);
+        self::assertSame(MessageCategory::Primary, $moved->category);
+    }
+
+    public function testSomebodyElsesCategoryIsRefusedAndNothingMoves(): void
+    {
+        $theirs = $this->category($this->seedUser(), 'Theirs');
+        $thread = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+
+        $this->drop('c' . $theirs->id, [(int) $thread->id]);
+
+        self::assertResponseStatusCodeSame(403);
+
+        $untouched = $this->reload($thread);
+        self::assertNull($untouched->userCategory, 'a conversation was filed under somebody else\'s category');
+        self::assertNull($untouched->categoryPinnedAt);
+    }
+
+    // ── fixture ──────────────────────────────────────────────────────────────
+
+    /**
+     * The row as the database has it now. The request cleared the entity
+     * manager, so the object the test holds is detached and refreshing it
+     * would throw.
+     */
+    private function reload(MessageThread $thread): MessageThread
+    {
+        $id = $thread->id;
+
+        $this->em->clear();
+
+        $fresh = $this->em->find(MessageThread::class, $id);
+        self::assertNotNull($fresh);
+
+        return $fresh;
+    }
+
+    /** @param list<int> $ids */
+    private function drop(string $key, array $ids): void
+    {
+        $crawler = $this->client->request('GET', '/mail/inbox');
+        $token   = (string) $crawler->filter('meta[name="csrf-token"]')->attr('content');
+
+        $this->client->request(
+            'POST',
+            '/status/bulk/category',
+            [],
+            [],
+            ['CONTENT_TYPE' => 'application/json', 'HTTP_X_CSRF_TOKEN' => $token],
+            (string) json_encode(['ids' => $ids, 'category' => $key]),
+        );
+    }
+
+    private function category(User $owner, string $name): InboxCategory
+    {
+        $category       = new InboxCategory();
+        $category->usr  = $owner;
+        $category->name = $name;
+        $this->em->persist($category);
+        $this->em->flush();
+
+        return $category;
+    }
+}

tests/Service/Mail/ThreadMoveToOwnTabTest.php

@@ -0,0 +1,129 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Mail;
+
+use App\Domain\Enum\Mail\LabelRole;
+use App\Domain\Enum\Mail\MessageCategory;
+use App\Entity\Mail\InboxCategory;
+use App\Service\Mail\ThreadStatusUpdater;
+use App\Tests\Support\Mail\SeedsMarkerFixtures;
+use Doctrine\DBAL\Connection;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * Dropping a conversation on a tab the reader made, and on a built-in one from
+ * there.
+ *
+ * Two ways to get it wrong, and both leave the strip lying: a drop onto the
+ * reader's own tab that rewrites the BUILT-IN category (JMAP clients are served
+ * that column, so their phone would show the conversation in a tab the browser
+ * has never heard of), and a drop back onto a built-in tab that leaves the
+ * conversation in the reader's tab as well (it would be in two tabs, or in the
+ * one the person just moved it out of).
+ *
+ * Both drops are pinned, for the reason ThreadMoveTest gives for the built-in
+ * ones: most-recent-wins would otherwise undo the move at the next message.
+ */
+final class ThreadMoveToOwnTabTest extends KernelTestCase
+{
+    use SeedsMarkerFixtures;
+
+    private Connection $connection;
+    private ThreadStatusUpdater $status;
+    private InboxCategory $shipping;
+
+    protected function setUp(): void
+    {
+        self::bootKernel();
+
+        $container        = self::getContainer();
+        $this->em         = $container->get(EntityManagerInterface::class);
+        $this->connection = $container->get(Connection::class);
+        $this->status     = $container->get(ThreadStatusUpdater::class);
+
+        $this->connection->beginTransaction();
+
+        $this->user    = $this->seedUser();
+        $this->account = $this->seedAccount();
+        $this->inbox   = $this->seedLabel('Inbox', LabelRole::Inbox);
+
+        $this->shipping       = new InboxCategory();
+        $this->shipping->usr  = $this->user;
+        $this->shipping->name = 'Shipping';
+        $this->em->persist($this->shipping);
+        $this->em->flush();
+    }
+
+    protected function tearDown(): void
+    {
+        if (true === $this->connection->isTransactionActive()) {
+            $this->connection->rollBack();
+        }
+
+        parent::tearDown();
+    }
+
+    public function testADropOnTheReadersOwnTabFilesAndPinsItAndLeavesTheBuiltInCategoryAlone(): void
+    {
+        $thread = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+
+        self::assertTrue($this->status->setCategory($thread, $this->shipping));
+
+        self::assertSame($this->shipping, $thread->userCategory);
+        self::assertNotNull($thread->categoryPinnedAt, 'the drop is not pinned, so the next message undoes it');
+        self::assertSame(
+            MessageCategory::Promotions,
+            $thread->category,
+            'a drop on the reader\'s own tab rewrote the built-in category JMAP serves',
+        );
+    }
+
+    public function testADropBackOntoABuiltInTabTakesItOutOfTheReadersTab(): void
+    {
+        $thread = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+        $this->status->setCategory($thread, $this->shipping);
+
+        self::assertTrue($this->status->setCategory($thread, MessageCategory::Primary));
+
+        self::assertNull($thread->userCategory, 'the conversation is in a built-in tab AND the reader\'s own');
+        self::assertSame(MessageCategory::Primary, $thread->category);
+        self::assertNotNull($thread->categoryPinnedAt);
+    }
+
+    /**
+     * Dropping where it already is changes nothing, and says so — for the reader's
+     * own tab as for the five. The companion is the pair above: the same call on
+     * a conversation that is NOT there yet does change it.
+     */
+    public function testADropWhereTheConversationAlreadyIsIsNotAChange(): void
+    {
+        $thread = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+
+        self::assertTrue($this->status->setCategory($thread, $this->shipping));
+        self::assertFalse($this->status->setCategory($thread, $this->shipping), 'a repeated drop reported a change');
+
+        self::assertTrue($this->status->setCategory($thread, MessageCategory::Primary));
+        self::assertFalse($this->status->setCategory($thread, MessageCategory::Primary));
+    }
+
+    /**
+     * A built-in drop onto a conversation that is pinned to a built-in tab but
+     * ALSO has a tab of the reader's: the built-in column already matches, and
+     * "nothing to do" would leave it in the reader's tab. The case the old
+     * equality check would have got wrong.
+     */
+    public function testABuiltInDropStillMovesAConversationWhoseBuiltInCategoryAlreadyMatches(): void
+    {
+        $thread                   = $this->thread('a notice', MessageCategory::Promotions, unread: 1);
+        $thread->userCategory     = $this->shipping;
+        $thread->categoryPinnedAt = new \DateTimeImmutable();
+        $this->em->flush();
+
+        self::assertTrue($this->status->setCategory($thread, MessageCategory::Promotions));
+
+        self::assertNull($thread->userCategory);
+    }
+}