mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
feat(mail): add inbox categories a reader makes themselves
A new inbox_category table, and two nullable columns that file mail under one: message.rule_category_id (a filter's verdict, kept apart from message.category because five writers recompute that and would erase it) and message_thread.user_category_id (the conversation's tab). Both references are ON DELETE SET NULL, so removing a category un-files its mail in the database. The built-in category columns, their enum mapping and everything JMAP serves from them are untouched. Nothing is backfilled and nobody's inbox changes until a category and a filter filing into it exist. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
maniandClaude Sonnet 5.5 committed
1 parent
5debbdbc01
commit
d3e0782cf7
6 files changed
+440
No files matched your search
@@ -0,0 +1,79 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace DoctrineMigrations;
|
||||
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\Migrations\AbstractMigration;
|
||||
|
||||
/**
|
||||
* Inbox categories the reader makes themselves: the table, and the two columns
|
||||
* that file mail and conversations under one.
|
||||
*
|
||||
* See App\Entity\Mail\InboxCategory for what a category is, Message::$ruleCategory
|
||||
* for why the filter's verdict has a column of its own, and
|
||||
* MessageThread::$userCategory for how a conversation is shown in one.
|
||||
*
|
||||
* NOTHING IS BACKFILLED, and nothing should be: every existing message and
|
||||
* thread has no user category, which is true of all of them. Nobody's inbox
|
||||
* changes shape until they make a category and a filter that files into it.
|
||||
*
|
||||
* Both new columns are nullable with no default, so adding them rewrites no row
|
||||
* on the two largest tables in the schema. Their indexes are partial for the
|
||||
* reason idx_message_category_held is: the two questions asked of them — which
|
||||
* mail is in this category, and what to un-file when one is removed — both start
|
||||
* from "has one", and on an installation with no categories that is no row at
|
||||
* all. An unconditional index would be one more entry per message and per
|
||||
* thread, on every installation, for a feature most have never switched on.
|
||||
*
|
||||
* The foreign keys are ON DELETE SET NULL, which is the point: removing a
|
||||
* category un-files its mail in the database, leaving no pointer at a row that
|
||||
* is gone. The owner's foreign key is CASCADE, so a category goes with its user.
|
||||
*/
|
||||
final class Version20261012140000 extends AbstractMigration
|
||||
{
|
||||
public function getDescription(): string
|
||||
{
|
||||
return 'Add inbox_category, and message.rule_category_id / message_thread.user_category_id to file mail under one';
|
||||
}
|
||||
|
||||
public function up(Schema $schema): void
|
||||
{
|
||||
$this->addSql(<<<'SQL'
|
||||
CREATE TABLE inbox_category (
|
||||
id SERIAL NOT NULL,
|
||||
usr_id INT NOT NULL,
|
||||
name VARCHAR(40) NOT NULL,
|
||||
color VARCHAR(30) DEFAULT NULL,
|
||||
position INT DEFAULT 0 NOT NULL,
|
||||
created_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL,
|
||||
updated_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL,
|
||||
PRIMARY KEY(id)
|
||||
)
|
||||
SQL);
|
||||
$this->addSql('CREATE INDEX idx_inbox_category_usr_position ON inbox_category (usr_id, position)');
|
||||
$this->addSql('ALTER TABLE inbox_category ADD CONSTRAINT FK_inbox_category_usr FOREIGN KEY (usr_id) REFERENCES "user" (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE');
|
||||
|
||||
$this->addSql('ALTER TABLE message ADD rule_category_id INT DEFAULT NULL');
|
||||
$this->addSql('ALTER TABLE message ADD CONSTRAINT FK_message_rule_category FOREIGN KEY (rule_category_id) REFERENCES inbox_category (id) ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE');
|
||||
$this->addSql('CREATE INDEX idx_message_rule_category ON message (rule_category_id) WHERE rule_category_id IS NOT NULL');
|
||||
|
||||
$this->addSql('ALTER TABLE message_thread ADD user_category_id INT DEFAULT NULL');
|
||||
$this->addSql('ALTER TABLE message_thread ADD CONSTRAINT FK_message_thread_user_category FOREIGN KEY (user_category_id) REFERENCES inbox_category (id) ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE');
|
||||
$this->addSql('CREATE INDEX idx_message_thread_user_category ON message_thread (user_category_id) WHERE user_category_id IS NOT NULL');
|
||||
}
|
||||
|
||||
public function down(Schema $schema): void
|
||||
{
|
||||
$this->addSql('DROP INDEX idx_message_thread_user_category');
|
||||
$this->addSql('ALTER TABLE message_thread DROP CONSTRAINT FK_message_thread_user_category');
|
||||
$this->addSql('ALTER TABLE message_thread DROP user_category_id');
|
||||
|
||||
$this->addSql('DROP INDEX idx_message_rule_category');
|
||||
$this->addSql('ALTER TABLE message DROP CONSTRAINT FK_message_rule_category');
|
||||
$this->addSql('ALTER TABLE message DROP rule_category_id');
|
||||
|
||||
$this->addSql('DROP TABLE inbox_category');
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,67 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Entity\Mail;
|
||||
|
||||
use App\Domain\Trait\TimestampableTrait;
|
||||
use App\Entity\User\User;
|
||||
use App\Repository\Mail\InboxCategoryRepository;
|
||||
use Doctrine\ORM\Mapping as ORM;
|
||||
|
||||
/**
|
||||
* An inbox tab somebody made themselves.
|
||||
*
|
||||
* The five Gmail categories are an enum and stay one: they are what a provider
|
||||
* says, what the assistant is asked to choose between, and the closed
|
||||
* vocabulary JMAP clients are documented to draw their tabs from. This is the
|
||||
* other kind — a name the reader picked, filled by a filter's "File in
|
||||
* category" action, and shown beside the built-in tabs while it holds mail.
|
||||
*
|
||||
* A CATEGORY BELONGS TO THE USER, NOT TO AN ACCOUNT, for the reason Label does:
|
||||
* a person's "Shipping" is one tab across every mailbox they own.
|
||||
*
|
||||
* It is referenced by id from two places — Message::$ruleCategory and
|
||||
* MessageThread::$userCategory — and both references are ON DELETE SET NULL, so
|
||||
* the database un-files the mail when a category goes and nothing is left
|
||||
* pointing at a row that no longer exists. Pins are the one thing that cannot
|
||||
* be left to a foreign key; see InboxCategoryRemover.
|
||||
*
|
||||
* Name uniqueness per user is the service layer's job (case-insensitively), not
|
||||
* a constraint: an expression index cannot be written in Doctrine attributes
|
||||
* and would drift from the schema diff, the same trade Label documents.
|
||||
*/
|
||||
#[ORM\Entity(repositoryClass: InboxCategoryRepository::class)]
|
||||
#[ORM\Table(name: 'inbox_category')]
|
||||
// The only lookup is "this user's categories, in order" — on every inbox render
|
||||
// and every filter edit — so owner leads and position follows.
|
||||
#[ORM\Index(name: 'idx_inbox_category_usr_position', columns: ['usr_id', 'position'])]
|
||||
#[ORM\HasLifecycleCallbacks]
|
||||
class InboxCategory
|
||||
{
|
||||
use TimestampableTrait;
|
||||
|
||||
public const int NAME_MAX_LENGTH = 40;
|
||||
|
||||
#[ORM\Id]
|
||||
#[ORM\GeneratedValue]
|
||||
#[ORM\Column]
|
||||
public private(set) ?int $id = null;
|
||||
|
||||
#[ORM\ManyToOne]
|
||||
#[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')]
|
||||
public ?User $usr = null;
|
||||
|
||||
#[ORM\Column(length: self::NAME_MAX_LENGTH)]
|
||||
public ?string $name = null;
|
||||
|
||||
/**
|
||||
* A palette token, the way Label::$color is. Null takes the neutral tone.
|
||||
*/
|
||||
#[ORM\Column(length: 30, nullable: true)]
|
||||
public ?string $color = null;
|
||||
|
||||
/** Left to right among the user's own tabs; ties break on id. */
|
||||
#[ORM\Column(options: ['default' => 0])]
|
||||
public int $position = 0;
|
||||
}
|
||||
@@ -447,6 +447,31 @@ class Message extends MessageModel
|
||||
#[ORM\Column(name: 'ai_category', enumType: MessageCategory::class, nullable: true)]
|
||||
public ?MessageCategory $aiCategory = null;
|
||||
|
||||
/**
|
||||
* The category a FILTER filed this message under, when it is one the
|
||||
* reader made themselves.
|
||||
*
|
||||
* A THIRD OPINION, AND THE REASON IT IS ITS OWN COLUMN
|
||||
* ────────────────────────────────────────────────────
|
||||
* Not written into $category, for the reason $aiCategory is not, and for a
|
||||
* sharper one besides. $category is a pure function of persisted data that
|
||||
* five writers recompute — arrival, the backfill, the re-sort a settings
|
||||
* change triggers, the assistant's release, Gmail enrichment — and every one
|
||||
* of them overwrites it. A filter's verdict written there would be erased by
|
||||
* the next re-sort. Here nothing recomputes it: only a filter's action
|
||||
* writes it, and only removing the category clears it.
|
||||
*
|
||||
* $category keeps the built-in the rules decided, which is also what a JMAP
|
||||
* client is served: its tab vocabulary is closed, so a message filed in a
|
||||
* tab of the reader's own still reads as the built-in it would have been in.
|
||||
*
|
||||
* ON DELETE SET NULL: removing the category un-files its mail in the
|
||||
* database, with no sweep for the application to forget.
|
||||
*/
|
||||
#[ORM\ManyToOne]
|
||||
#[ORM\JoinColumn(name: 'rule_category_id', nullable: true, onDelete: 'SET NULL')]
|
||||
public ?InboxCategory $ruleCategory = null;
|
||||
|
||||
/**
|
||||
* The Gmail account that can act on this message, when it is not this
|
||||
* message's own account.
|
||||
|
||||
@@ -46,6 +46,26 @@ class MessageThread
|
||||
#[ORM\Column(nullable: true, enumType: MessageCategory::class)]
|
||||
public ?MessageCategory $category = null;
|
||||
|
||||
/**
|
||||
* The conversation's tab when it is one the reader made themselves.
|
||||
*
|
||||
* Beside $category rather than instead of it: that column stays one of the
|
||||
* five built-ins, so its enum mapping, every recompute that writes it and
|
||||
* the JMAP vocabulary built on it are untouched. The tab a thread is SHOWN
|
||||
* in is this when set, else $category.
|
||||
*
|
||||
* Resolved like $category — most-recent-wins over the thread's messages'
|
||||
* Message::$ruleCategory — and pinned by the same $categoryPinnedAt, so a
|
||||
* conversation dragged onto one of the reader's own tabs stays there.
|
||||
*
|
||||
* ON DELETE SET NULL: the thread falls back to its built-in tab when the
|
||||
* category is removed. A pin cannot be cleared by a foreign key; see
|
||||
* InboxCategoryRemover.
|
||||
*/
|
||||
#[ORM\ManyToOne]
|
||||
#[ORM\JoinColumn(name: 'user_category_id', nullable: true, onDelete: 'SET NULL')]
|
||||
public ?InboxCategory $userCategory = null;
|
||||
|
||||
/**
|
||||
* When somebody put this conversation in its category by hand.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Repository\Mail;
|
||||
|
||||
use App\Entity\Mail\InboxCategory;
|
||||
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
|
||||
use Doctrine\Persistence\ManagerRegistry;
|
||||
use Symfony\Component\Security\Core\User\UserInterface;
|
||||
|
||||
/**
|
||||
* @extends ServiceEntityRepository<InboxCategory>
|
||||
*/
|
||||
class InboxCategoryRepository extends ServiceEntityRepository
|
||||
{
|
||||
public function __construct(ManagerRegistry $registry)
|
||||
{
|
||||
parent::__construct($registry, InboxCategory::class);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every category the user made, in the order their tabs are drawn.
|
||||
*
|
||||
* @return list<InboxCategory>
|
||||
*/
|
||||
public function findForUserOrdered(UserInterface $user): array
|
||||
{
|
||||
/** @var list<InboxCategory> */
|
||||
return $this->findBy(['usr' => $user], ['position' => 'ASC', 'id' => 'ASC']);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,217 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Entity\Mail;
|
||||
|
||||
use App\Domain\Enum\Mail\ThreadingMethod;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\Mail\InboxCategory;
|
||||
use App\Entity\Mail\Message;
|
||||
use App\Entity\Mail\MessageThread;
|
||||
use App\Entity\User\User;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
|
||||
/**
|
||||
* What the database guarantees about a category somebody made themselves.
|
||||
*
|
||||
* Two promises, and both are enforced by the schema rather than by anything the
|
||||
* application remembers to do — which is the only kind that holds when a row is
|
||||
* removed by a console command, a cascade from the user, or a restore:
|
||||
*
|
||||
* - A category belongs to its owner and goes with them.
|
||||
* - Taking a category away takes the FILING away and nothing else. The mail
|
||||
* and the conversations that were in it stay exactly where they are, with no
|
||||
* category pointer left dangling at a row that no longer exists. A dangling
|
||||
* pointer here would be a thread no tab query can place.
|
||||
*/
|
||||
final class InboxCategoryTest extends KernelTestCase
|
||||
{
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$container = self::getContainer();
|
||||
$this->em = $container->get(EntityManagerInterface::class);
|
||||
$this->connection = $container->get(Connection::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testACategoryIsStoredForItsOwnerAndKeepsItsOrder(): void
|
||||
{
|
||||
$account = $this->seedAccount();
|
||||
|
||||
$first = $this->category($account->usr, 'Shipping', 0);
|
||||
$second = $this->category($account->usr, 'Receipts', 1);
|
||||
|
||||
$this->em->clear();
|
||||
|
||||
$rows = $this->connection->fetchAllAssociative(
|
||||
'SELECT name, position FROM inbox_category WHERE usr_id = :usr ORDER BY position',
|
||||
['usr' => $account->usr->id],
|
||||
);
|
||||
|
||||
self::assertSame([['name' => 'Shipping', 'position' => 0], ['name' => 'Receipts', 'position' => 1]], $rows);
|
||||
self::assertNotNull($first->id);
|
||||
self::assertNotNull($second->id);
|
||||
}
|
||||
|
||||
public function testRemovingTheOwnerRemovesTheirCategories(): void
|
||||
{
|
||||
// A user with no account: an account's own foreign key to its user is
|
||||
// not what this is about, and would refuse the delete before it began.
|
||||
$user = $this->seedUser();
|
||||
$this->category($user, 'Shipping', 0);
|
||||
$userId = (int) $user->id;
|
||||
|
||||
$this->connection->executeStatement('DELETE FROM "user" WHERE id = :id', ['id' => $userId]);
|
||||
|
||||
self::assertSame(
|
||||
0,
|
||||
(int) $this->connection->fetchOne('SELECT COUNT(*) FROM inbox_category WHERE usr_id = :id', ['id' => $userId]),
|
||||
'a deleted user left categories behind',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The invariant itself, with its presence companion: the sibling filed under
|
||||
* a DIFFERENT category must keep its filing, or "everything was nulled" and
|
||||
* "only the right rows were nulled" would look the same.
|
||||
*/
|
||||
public function testRemovingACategoryUnfilesItsMailAndThreadsAndNothingElse(): void
|
||||
{
|
||||
$account = $this->seedAccount();
|
||||
$doomed = $this->category($account->usr, 'Shipping', 0);
|
||||
$survivor = $this->category($account->usr, 'Receipts', 1);
|
||||
|
||||
$inDoomed = $this->threadWithMessage($account, 'filed under the doomed category');
|
||||
$inSurvivor = $this->threadWithMessage($account, 'filed under the surviving category');
|
||||
|
||||
$inDoomed['message']->ruleCategory = $doomed;
|
||||
$inDoomed['thread']->userCategory = $doomed;
|
||||
$inSurvivor['message']->ruleCategory = $survivor;
|
||||
$inSurvivor['thread']->userCategory = $survivor;
|
||||
$this->em->flush();
|
||||
|
||||
$doomedId = (int) $doomed->id;
|
||||
$this->connection->executeStatement('DELETE FROM inbox_category WHERE id = :id', ['id' => $doomedId]);
|
||||
|
||||
$messageRows = $this->connection->fetchAllKeyValue(
|
||||
'SELECT id, rule_category_id FROM message WHERE id IN (:a, :b)',
|
||||
['a' => $inDoomed['message']->id, 'b' => $inSurvivor['message']->id],
|
||||
);
|
||||
$threadRows = $this->connection->fetchAllKeyValue(
|
||||
'SELECT id, user_category_id FROM message_thread WHERE id IN (:a, :b)',
|
||||
['a' => $inDoomed['thread']->id, 'b' => $inSurvivor['thread']->id],
|
||||
);
|
||||
|
||||
self::assertNull($messageRows[$inDoomed['message']->id], 'the message still points at a category that is gone');
|
||||
self::assertNull($threadRows[$inDoomed['thread']->id], 'the thread still points at a category that is gone');
|
||||
self::assertSame($survivor->id, (int) $messageRows[$inSurvivor['message']->id], 'a sibling message lost its filing');
|
||||
self::assertSame($survivor->id, (int) $threadRows[$inSurvivor['thread']->id], 'a sibling thread lost its filing');
|
||||
|
||||
self::assertSame(
|
||||
2,
|
||||
(int) $this->connection->fetchOne(
|
||||
'SELECT COUNT(*) FROM message WHERE id IN (:a, :b)',
|
||||
['a' => $inDoomed['message']->id, 'b' => $inSurvivor['message']->id],
|
||||
),
|
||||
'removing a category removed mail',
|
||||
);
|
||||
}
|
||||
|
||||
// ── fixture ──────────────────────────────────────────────────────────────
|
||||
|
||||
private function category(User $user, string $name, int $position): InboxCategory
|
||||
{
|
||||
$category = new InboxCategory();
|
||||
$category->usr = $user;
|
||||
$category->name = $name;
|
||||
$category->position = $position;
|
||||
$this->em->persist($category);
|
||||
$this->em->flush();
|
||||
|
||||
return $category;
|
||||
}
|
||||
|
||||
/** @return array{thread: MessageThread, message: Message} */
|
||||
private function threadWithMessage(Account $account, string $subject): array
|
||||
{
|
||||
$thread = new MessageThread();
|
||||
$thread->account = $account;
|
||||
$thread->subject = $subject;
|
||||
$thread->normalizedSubject = strtolower($subject);
|
||||
$thread->lastMessageAt = new \DateTimeImmutable('-1 hour');
|
||||
$thread->threadingMethod = ThreadingMethod::References;
|
||||
$thread->unreadCount = 0;
|
||||
$thread->messageCount = 1;
|
||||
$this->em->persist($thread);
|
||||
|
||||
$message = new Message();
|
||||
$message->account = $account;
|
||||
$message->subject = $subject;
|
||||
$message->fromAddress = 'sender@example.test';
|
||||
$message->receivedAt = new \DateTimeImmutable('-1 hour');
|
||||
$message->hasAttachments = false;
|
||||
$message->messageId = sprintf('<cat-%s@example.test>', uniqid('', true));
|
||||
$thread->addMessage($message);
|
||||
$this->em->persist($message);
|
||||
$this->em->flush();
|
||||
|
||||
return ['thread' => $thread, 'message' => $message];
|
||||
}
|
||||
|
||||
private function seedUser(): User
|
||||
{
|
||||
$user = new User();
|
||||
$user->email = 'cat-' . uniqid('', true) . '@example.test';
|
||||
$user->nameFirst = 'Category';
|
||||
$user->nameLast = 'Owner';
|
||||
$user->roles = ['ROLE_USER'];
|
||||
$user->password = 'x';
|
||||
$this->em->persist($user);
|
||||
$this->em->flush();
|
||||
|
||||
return $user;
|
||||
}
|
||||
|
||||
private function seedAccount(): Account
|
||||
{
|
||||
$user = $this->seedUser();
|
||||
|
||||
$email = 'cat-fixture-' . uniqid('', true) . '@example.test';
|
||||
|
||||
$account = new Account();
|
||||
$account->usr = $user;
|
||||
$account->email = $email;
|
||||
$account->username = $email;
|
||||
$account->imapHost = 'localhost';
|
||||
$account->imapPort = 993;
|
||||
$account->imapEncryption = 'ssl';
|
||||
$account->smtpHost = 'localhost';
|
||||
$account->smtpPort = 587;
|
||||
$account->smtpEncryption = 'starttls';
|
||||
$account->password = 'x';
|
||||
$account->authType = 'password';
|
||||
$account->isActive = true;
|
||||
$this->em->persist($account);
|
||||
$this->em->flush();
|
||||
|
||||
return $account;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user