refactor(mail): make a category's owner and name non-null, and satisfy PHPStan

InboxCategory::$usr and $name are NOT NULL columns, so they are declared that
way (Message::$account is the precedent) instead of nullable with a
check nobody can reach. FilterDescriber keys its category names by the user
identifier rather than reading an id off a UserInterface, which adds nothing
to the baseline. Test fixtures stop storing threads they never read back.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
maniandClaude Sonnet 5.5 committed 2026-10-12 14:32:16 +02:00
1 parent 23a3ada6c0
commit 1298d1edce
7 files changed
+17 -26

No files matched your search

+2 -2
View File
@@ -50,10 +50,10 @@ class InboxCategory
#[ORM\ManyToOne]
#[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')]
public ?User $usr = null;
public User $usr;
#[ORM\Column(length: self::NAME_MAX_LENGTH)]
public ?string $name = null;
public string $name;
/** Left to right among the user's own tabs; ties break on id. */
#[ORM\Column(options: ['default' => 0])]
+2 -8
View File
@@ -63,13 +63,7 @@ final readonly class InboxCategoryManager
*/
public function rename(InboxCategory $category, string $name): void
{
$owner = $category->usr;
if (null === $owner) {
throw new \LogicException('A category with no owner cannot be renamed.');
}
$category->name = $this->validName($owner, $name, $category);
$category->name = $this->validName($category->usr, $name, $category);
$this->em->flush();
}
@@ -113,7 +107,7 @@ final readonly class InboxCategoryManager
continue;
}
if (mb_strtolower((string) $existing->name) === mb_strtolower($name)) {
if (mb_strtolower($existing->name) === mb_strtolower($name)) {
throw new InvalidInboxCategoryException('settings.categories.error.taken');
}
}
+5 -3
View File
@@ -41,7 +41,7 @@ final class FilterDescriber implements ResetInterface
/** @var array<int, array<int,string>> userId => (integrationId => name) */
private array $integrationNames = [];
/** @var array<int, array<int,string>> userId => (categoryId => name) */
/** @var array<string, array<int,string>> user identifier => (categoryId => name) */
private array $categoryNames = [];
/** Whose labels the sentence currently being built may name. */
@@ -311,13 +311,15 @@ final class FilterDescriber implements ResetInterface
return $missing;
}
$userId = (int) $this->subject->id;
// Keyed by the user identifier rather than by an id: all that is known
// here is a UserInterface, which has no id to read.
$userId = $this->subject->getUserIdentifier();
if (false === array_key_exists($userId, $this->categoryNames)) {
$map = [];
foreach ($this->categoryRepository->findForUserOrdered($this->subject) as $category) {
$map[(int) $category->id] = (string) $category->name;
$map[(int) $category->id] = $category->name;
}
$this->categoryNames[$userId] = $map;
@@ -59,11 +59,6 @@ final class InboxTabQueriesTest extends KernelTestCase
private Label $inbox;
private InboxCategory $shipping;
private MessageThread $carrierNew;
private MessageThread $promoNew;
private MessageThread $primaryRead;
private MessageThread $updateRead;
protected function setUp(): void
{
self::bootKernel();
@@ -85,10 +80,10 @@ final class InboxTabQueriesTest extends KernelTestCase
$this->em->persist($this->shipping);
$this->em->flush();
$this->carrierNew = $this->inboxThread('carrier, new', MessageCategory::Promotions, $this->shipping, unread: 1);
$this->promoNew = $this->inboxThread('promo, new', MessageCategory::Promotions, null, unread: 1);
$this->primaryRead = $this->inboxThread('primary, read', MessageCategory::Primary, null, unread: 0);
$this->updateRead = $this->inboxThread('update, read', MessageCategory::Updates, $this->shipping, unread: 0);
$this->inboxThread('carrier, new', MessageCategory::Promotions, $this->shipping, unread: 1);
$this->inboxThread('promo, new', MessageCategory::Promotions, null, unread: 1);
$this->inboxThread('primary, read', MessageCategory::Primary, null, unread: 0);
$this->inboxThread('update, read', MessageCategory::Updates, $this->shipping, unread: 0);
}
protected function tearDown(): void
+2 -2
View File
@@ -147,10 +147,10 @@ final class MessageThreaderTest extends TestCase
$this->threaderWithThreadLookup(null)->assignThread($message, new Account());
self::assertSame($shipping, $message->thread?->userCategory);
self::assertSame($shipping, $message->thread->userCategory);
self::assertSame(
MessageCategory::Promotions,
$message->thread?->category,
$message->thread->category,
'the built-in category must still be the one the rules decided',
);
}
+1 -1
View File
@@ -70,7 +70,7 @@ final class InboxTabsTest extends TestCase
$tabs = new InboxTabs($repository);
self::assertSame('c12', $tabs->resolve($this->user('reader'), 'c12')?->value);
self::assertSame('Shipping', $tabs->resolve($this->user('reader'), 'c12')?->name);
self::assertSame('Shipping', $tabs->resolve($this->user('reader'), 'c12')->name);
self::assertNull($tabs->resolve($this->user('stranger'), 'c12'), 'resolved a category the asking reader does not own');
}
+1 -1
View File
@@ -67,7 +67,7 @@ final class RuleCategoryFilerTest extends TestCase
self::assertSame($this->shipping, $message->thread?->userCategory);
self::assertSame(
MessageCategory::Promotions,
$message->thread?->category,
$message->thread->category,
'filing must leave the built-in category alone: JMAP clients are served it',
);
}