mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Give a connected Gmail or Microsoft account its own colour (#60)
AccountCreator hands out Account::$colorIndex from the lowest free palette slot, but OAuthAccountLinker builds its Account without going through it, so every OAuth account kept the column default of 0 and wore the same dot in the sidebar and the same corner mark on every message row. A new OAuth account now takes the lowest free slot, as a password account does. A reconnect finds the existing row and leaves its colour alone. Accounts connected before this keep the colour they have. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6c719b3327
commit
ed76af23a5
4 files changed
+137
-1
No files matched your search
@@ -8,6 +8,14 @@ The published image tags: `latest` follows the most recent release below,
|
||||
|
||||
## Unreleased
|
||||
|
||||
**A Gmail or Microsoft account gets its own colour when you connect it.** Accounts added with a
|
||||
password were already given the first colour no other account was wearing; accounts connected
|
||||
through Google or Microsoft skipped that step and all kept the first colour, so their dots in the
|
||||
sidebar and the corner mark on every message row looked the same. They now take the first free
|
||||
colour like any other. Accounts you connected before this keep the colour they have; the swatch on
|
||||
the account's row in Settings → Mail accounts changes it. No migration.
|
||||
[#60](https://github.com/karatektus/pl_mail/pull/60), by @Flouss.
|
||||
|
||||
**The mail list is readable beside an open message.** A row went to a single line once the list was
|
||||
about 575px wide, which left the subject around 120px and its preview wrapped next to it ("Facture
|
||||
pour Co…") whenever the reading pane was open, or on a large phone. A row now goes to one line only
|
||||
|
||||
@@ -211,7 +211,7 @@ final readonly class AccountCreator
|
||||
*
|
||||
* @param iterable<Account> $existing
|
||||
*/
|
||||
private function freeColorIndex(iterable $existing): int
|
||||
public function freeColorIndex(iterable $existing): int
|
||||
{
|
||||
$taken = [];
|
||||
|
||||
|
||||
@@ -202,6 +202,13 @@ final readonly class OAuthAccountLinker
|
||||
$account->email = $email;
|
||||
$account->name = $duplicate ? sprintf('%s (%s)', $email, ucfirst($provider->value)) : $email;
|
||||
$account->isActive = true;
|
||||
|
||||
// AccountCreator hands the colour out at creation and this path
|
||||
// does not go through it, so without this every OAuth account kept
|
||||
// the column default of 0 and wore the same dot as the others.
|
||||
$account->colorIndex = $this->accountCreator->freeColorIndex(
|
||||
$this->accounts->findForUserOrdered($user),
|
||||
);
|
||||
}
|
||||
|
||||
$account->username = $email;
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Service\OAuth;
|
||||
|
||||
use App\Domain\Enum\Account\MailProvider;
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\User\User;
|
||||
use App\Service\OAuth\OAuthAccountLinker;
|
||||
use App\Tests\Command\MailFixtures;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use League\OAuth2\Client\Token\AccessTokenInterface;
|
||||
use ReflectionMethod;
|
||||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
|
||||
|
||||
/**
|
||||
* A connected Gmail or Microsoft account gets its own colour, like any other.
|
||||
*
|
||||
* The report: accounts did not get different accent colours when created.
|
||||
* AccountCreator hands out `colorIndex` from the lowest free palette slot, but
|
||||
* OAuthAccountLinker builds its Account without going through it, so every
|
||||
* OAuth account kept the column default of 0 — the same dot in the sidebar and
|
||||
* the same corner mark on every message row, which is the one job that mark has.
|
||||
*
|
||||
* Expected values are the palette slots, derived by hand: the user below holds
|
||||
* a password account in slot 0, so the first OAuth account takes 1 and the
|
||||
* second takes 2. "Every OAuth account stays at 0" and "the slot is never
|
||||
* assigned at all" both fail that, and each is a different way for it to break.
|
||||
*/
|
||||
final class OAuthAccountLinkerColourTest extends KernelTestCase
|
||||
{
|
||||
private OAuthAccountLinker $linker;
|
||||
private EntityManagerInterface $em;
|
||||
private Connection $connection;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
self::bootKernel();
|
||||
|
||||
$this->linker = self::getContainer()->get(OAuthAccountLinker::class);
|
||||
$this->em = self::getContainer()->get(EntityManagerInterface::class);
|
||||
$this->connection = self::getContainer()->get(Connection::class);
|
||||
|
||||
$this->connection->beginTransaction();
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
if (true === $this->connection->isTransactionActive()) {
|
||||
$this->connection->rollBack();
|
||||
}
|
||||
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testEachConnectedAccountTakesTheLowestFreeColour(): void
|
||||
{
|
||||
$user = MailFixtures::user($this->em, 'oauth-colour');
|
||||
$existing = MailFixtures::account($this->em, $user);
|
||||
|
||||
$existing->colorIndex = 0;
|
||||
$this->em->flush();
|
||||
|
||||
$google = $this->upsert($user, MailProvider::Google, $this->address('google'), $this->token());
|
||||
$microsoft = $this->upsert($user, MailProvider::Microsoft, $this->address('microsoft'), $this->token());
|
||||
|
||||
self::assertSame(0, $existing->colorIndex, 'the account that was there keeps its colour');
|
||||
self::assertSame(1, $google->colorIndex, 'slot 0 is taken, so the first OAuth account gets slot 1');
|
||||
self::assertSame(2, $microsoft->colorIndex, 'slots 0 and 1 are taken, so the next gets slot 2');
|
||||
}
|
||||
|
||||
public function testReconnectingAnAccountDoesNotRepaintIt(): void
|
||||
{
|
||||
$user = MailFixtures::user($this->em, 'oauth-colour');
|
||||
$address = $this->address('google');
|
||||
|
||||
$first = $this->upsert($user, MailProvider::Google, $address, $this->token());
|
||||
$other = $this->upsert($user, MailProvider::Microsoft, $this->address('microsoft'), $this->token());
|
||||
|
||||
self::assertSame(0, $first->colorIndex);
|
||||
self::assertSame(1, $other->colorIndex);
|
||||
|
||||
// The same grant coming back is found again, not created again.
|
||||
$again = $this->upsert($user, MailProvider::Google, $address, $this->token());
|
||||
|
||||
self::assertSame($first->id, $again->id);
|
||||
self::assertSame(0, $again->colorIndex, 'a colour is the account\'s own and never moves');
|
||||
}
|
||||
|
||||
/**
|
||||
* link() is upsert() plus push registration and alias seeding, and both of
|
||||
* those call the provider for real. Nothing about the colour depends on them.
|
||||
*/
|
||||
private function upsert(
|
||||
User $user,
|
||||
MailProvider $provider,
|
||||
string $email,
|
||||
AccessTokenInterface $token,
|
||||
): Account {
|
||||
return new ReflectionMethod(OAuthAccountLinker::class, 'upsert')
|
||||
->invoke($this->linker, $user, $provider, $email, $token);
|
||||
}
|
||||
|
||||
private function address(string $prefix): string
|
||||
{
|
||||
return sprintf('%s-%s@example.test', $prefix, uniqid('', true));
|
||||
}
|
||||
|
||||
private function token(): AccessTokenInterface
|
||||
{
|
||||
$token = $this->createStub(AccessTokenInterface::class);
|
||||
$token->method('getToken')->willReturn('access-token');
|
||||
$token->method('getRefreshToken')->willReturn(null);
|
||||
$token->method('getExpires')->willReturn(null);
|
||||
$token->method('getValues')->willReturn([]);
|
||||
|
||||
return $token;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user