mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 10:04:18 +00:00
Carry category sorting in the config backup
User::$categorySorting (source, overrideProvider, tabs) was never written into a config backup, so a restore put everyone back on rules, the provider's categories and a tabbed inbox. It now travels through toArray()/applyArray() like Appearance and AiPreferences do. A backup without the key restores the defaults; an unknown source falls back to the rules without losing the two switches beside it. ConfigBackupCompletenessTest now fails for any #[ORM\Embedded] on User that the export does not carry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0056fc0981
commit
da5a0bc9ee
9 files changed
+260
-4
No files matched your search
@@ -17,6 +17,12 @@ while a dialog or menu is open. On for everyone; Settings → General → Keyboa
|
||||
them off per person. No migration.
|
||||
[#36](https://github.com/karatektus/pl_mail/issues/36).
|
||||
|
||||
**A config backup now carries how each person sorts their mail.** Settings → Categories — rules or
|
||||
the assistant, whether that overrules the provider's own categories, and whether the inbox has tabs
|
||||
at all — was never written into the file, so a restore put everyone back on rules, the provider's
|
||||
categories and a tabbed inbox. All three now travel with the user. Backups made before this still
|
||||
restore, onto those same defaults. No migration.
|
||||
|
||||
## v0.3.2 — 2026-10-09
|
||||
|
||||
### Security
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<!-- translated-from: install/config-backup.md sha1:aea7c7b52f687249d5b959194d61ddb6e0fb84d2 -->
|
||||
<!-- translated-from: install/config-backup.md sha1:9b19dac8e3efc0f076350b7bfa6b3915ba0cc396 -->
|
||||
# Konfigurationssicherung
|
||||
|
||||
Die *Konfiguration* einer Installation ist nicht dasselbe wie ihre *Daten*, und beide gehen auf
|
||||
@@ -163,7 +163,9 @@ Pro Person:
|
||||
das Konto Anzeigename, Passwort-HASH (nie ein Passwort — den Klartext
|
||||
gibt es nirgends), Administratorrolle, Sprache, Zeitzone,
|
||||
Erscheinungsbild, Oberflächen-Einstellungen,
|
||||
Assistenten-Einstellungen, Onboarding-Stand
|
||||
Assistenten-Einstellungen, die Kategorien (Regeln oder
|
||||
Assistent, ob das den Anbieter überstimmt, Tabs an oder
|
||||
aus), Onboarding-Stand
|
||||
zweiter Faktor das TOTP-Geheimnis, sein Bestätigungsdatum und die noch
|
||||
ungenutzten Wiederherstellungscodes (bereits SHA-256-Digests)
|
||||
App-Passwörter Name, Hinweis und Hash je Zugangsdatum, mit lastUsedAt und
|
||||
|
||||
@@ -152,7 +152,8 @@ alone on import — restoring your Firebase key cannot switch somebody's assista
|
||||
the account display name, password HASH (never a password — the
|
||||
plaintext exists nowhere), admin role, locale, timezone,
|
||||
appearance, interface preferences, assistant preferences,
|
||||
onboarding state
|
||||
category sorting (rules or assistant, whether it overrules
|
||||
the provider, tabs on or off), onboarding state
|
||||
two-factor the TOTP secret, its confirmation date, and the unused
|
||||
recovery codes (already SHA-256 digests)
|
||||
app passwords name, hint and hash per credential, with lastUsedAt and
|
||||
|
||||
@@ -106,4 +106,83 @@ class CategorySorting
|
||||
{
|
||||
return $this->sourceEnum()->usesAssistant();
|
||||
}
|
||||
|
||||
/**
|
||||
* The export payload, which is every field.
|
||||
*
|
||||
* The same contract AiPreferences::toArray() keeps, for the same reason: a
|
||||
* config backup carries this through toArray()/applyArray() rather than
|
||||
* through a line per column in ConfigBackupUsers, so a fourth decision
|
||||
* added above travels by being listed here and nowhere else.
|
||||
*
|
||||
* It was missing until three columns had shipped, and nothing noticed. A
|
||||
* restore put everybody back on the rules, the provider's own categories
|
||||
* and a tabbed inbox — three defaults that each look like a working
|
||||
* installation, which is exactly why nobody reported it as a lost setting.
|
||||
* ConfigBackupUsersTest asserts the round trip now, because an embeddable
|
||||
* that is simply never mentioned is not something a test of the mentioned
|
||||
* ones can catch.
|
||||
*
|
||||
* The source leaves as its stored string and not through sourceEnum(): a
|
||||
* value this build does not recognise is still what the person chose on
|
||||
* the build that wrote it, and reading it charitably is the importing
|
||||
* side's business.
|
||||
*
|
||||
* @return array<string, mixed>
|
||||
*/
|
||||
public function toArray(): array
|
||||
{
|
||||
return [
|
||||
'version' => 1,
|
||||
'source' => $this->source,
|
||||
'overrideProvider' => $this->overrideProvider,
|
||||
'tabs' => $this->tabs,
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $data
|
||||
*/
|
||||
public function applyArray(array $data): static
|
||||
{
|
||||
// Each key on its own, so a document that carries only some of them —
|
||||
// written before the tabs switch existed, say — restores what it has
|
||||
// and leaves the rest on the default, which is what the application
|
||||
// did for that person at the time.
|
||||
if (true === isset($data['source'])) {
|
||||
// from_() and not from(): a document written by a build that
|
||||
// offered a third source must restore the rest of the person's
|
||||
// setup rather than throwing, and the rules are a working answer.
|
||||
// Stored as the recognised value, never the raw one — the column
|
||||
// is sixteen characters wide and a hand-edited file is not.
|
||||
$this->source = CategorySource::from_($data['source'])->value;
|
||||
}
|
||||
|
||||
if (true === isset($data['overrideProvider'])) {
|
||||
$this->overrideProvider = self::boolean($data['overrideProvider']);
|
||||
}
|
||||
|
||||
if (true === isset($data['tabs'])) {
|
||||
$this->tabs = self::boolean($data['tabs']);
|
||||
}
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* A switch out of a document, the same way AiPreferences reads one.
|
||||
*
|
||||
* A backup file carries real booleans, but it is also a file somebody may
|
||||
* have edited, and "0" is truthy to PHP's cast. Both spellings are named
|
||||
* rather than trusted to (bool) — for `tabs` in particular, where a wrong
|
||||
* guess changes the shape of somebody's inbox.
|
||||
*/
|
||||
private static function boolean(mixed $value): bool
|
||||
{
|
||||
if (true === is_bool($value)) {
|
||||
return $value;
|
||||
}
|
||||
|
||||
return false === in_array($value, ['0', 0, 'false', '', null], true);
|
||||
}
|
||||
}
|
||||
@@ -227,6 +227,12 @@ final readonly class ConfigBackupUserRestorer
|
||||
// document written before these existed: the defaults are what the
|
||||
// application already did.
|
||||
$user->aiPreferences->applyArray($this->map($document, 'aiPreferences'));
|
||||
// The same again, and here the empty map is every backup written
|
||||
// before this key was carried — which is all of them up to v0.3.2.
|
||||
// Those restore onto rules, the provider's categories and a tabbed
|
||||
// inbox: what such a restore has always produced, so an old file does
|
||||
// not start behaving differently.
|
||||
$user->categorySorting->applyArray($this->map($document, 'categorySorting'));
|
||||
|
||||
foreach ($this->map($document, 'settings') as $key => $value) {
|
||||
$user->setSetting($key, $value);
|
||||
|
||||
@@ -316,6 +316,11 @@ final readonly class ConfigBackupUsers
|
||||
// USER_SETTINGS' curated allowlist below does not give — it omits
|
||||
// several bag keys today with nothing catching it.
|
||||
'aiPreferences' => $user->aiPreferences->toArray(),
|
||||
// The third embeddable, and the one that was forgotten: what sorts
|
||||
// this person's mail into tabs, whether that overrules the
|
||||
// provider, and whether there are tabs at all. Carried the way the
|
||||
// two above are, so the next decision added to it travels too.
|
||||
'categorySorting' => $user->categorySorting->toArray(),
|
||||
'settings' => $this->settingsOf($user),
|
||||
'createdAt' => $user->createdAt->format(DateTimeInterface::ATOM),
|
||||
// Carried for the reason export() gives about deletedAt: being
|
||||
|
||||
@@ -6,10 +6,21 @@ namespace App\Tests\Service\Backup;
|
||||
|
||||
use App\Entity\Mail\Account;
|
||||
use App\Entity\User\User;
|
||||
use App\Repository\Calendar\BookingPageRepository;
|
||||
use App\Repository\Calendar\CalendarRepository;
|
||||
use App\Repository\Calendar\CalendarShareLinkRepository;
|
||||
use App\Repository\Integration\IntegrationRepository;
|
||||
use App\Repository\Label\LabelRepository;
|
||||
use App\Repository\Mail\AccountRepository;
|
||||
use App\Repository\Rule\MailRuleRepository;
|
||||
use App\Repository\User\ApiTokenRepository;
|
||||
use App\Repository\User\UserRepository;
|
||||
use App\Service\Backup\ConfigBackupDatabase;
|
||||
use App\Service\Backup\ConfigBackupUsers;
|
||||
use Doctrine\ORM\Mapping\Embedded;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use ReflectionClass;
|
||||
use ReflectionProperty;
|
||||
|
||||
/**
|
||||
* The backup has to fail when it stops being complete, and it could not.
|
||||
@@ -28,7 +39,7 @@ use ReflectionClass;
|
||||
* whole AI configuration silently dropped — silently because a section that
|
||||
* does not exist produces no row on the review page to warn about.
|
||||
*
|
||||
* These two tests are the direction that was missing. Neither compares the
|
||||
* The tests here are the direction that was missing. None compares the
|
||||
* backup against itself: each compares it against the application, and fails
|
||||
* closed. Adding a setting or an admin-configured table is then a red suite
|
||||
* until somebody has decided whether it travels — a cheap decision, and the
|
||||
@@ -70,6 +81,61 @@ final class ConfigBackupCompletenessTest extends TestCase
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every embeddable on the user travels, under its own property name.
|
||||
*
|
||||
* The third direction, added after `categorySorting` was found missing.
|
||||
* The two tests above watch the settings bag; an embeddable is the OTHER
|
||||
* place a person's preferences live, and nothing watched it. Appearance and
|
||||
* AiPreferences were carried because somebody remembered; CategorySorting
|
||||
* shipped three columns over two releases without being, and a restore put
|
||||
* everybody's inbox back the way it started.
|
||||
*
|
||||
* Discovered by reflection and not named, unlike the admin tables below,
|
||||
* because here there is no judgement to make: an `#[ORM\Embedded]` on User
|
||||
* is by construction something that person configured. Should one ever
|
||||
* appear that must stay behind, this is the test to give an excluded map,
|
||||
* with reasons, the way the two above have one.
|
||||
*
|
||||
* The exporter is built over stubs: exportUser() reads the embeddables off
|
||||
* the entity itself, and every repository it also consults answers "none"
|
||||
* for a user that was never persisted — which is the right answer.
|
||||
*/
|
||||
public function testEveryUserEmbeddableIsCarried(): void
|
||||
{
|
||||
$exported = new ConfigBackupUsers(
|
||||
self::createStub(UserRepository::class),
|
||||
self::createStub(AccountRepository::class),
|
||||
self::createStub(ApiTokenRepository::class),
|
||||
self::createStub(IntegrationRepository::class),
|
||||
self::createStub(LabelRepository::class),
|
||||
self::createStub(MailRuleRepository::class),
|
||||
self::createStub(CalendarRepository::class),
|
||||
self::createStub(CalendarShareLinkRepository::class),
|
||||
self::createStub(BookingPageRepository::class),
|
||||
)->exportUser(new User());
|
||||
|
||||
$embedded = array_filter(
|
||||
new ReflectionClass(User::class)->getProperties(),
|
||||
static fn (ReflectionProperty $property): bool => [] !== $property->getAttributes(Embedded::class),
|
||||
);
|
||||
|
||||
self::assertNotEmpty($embedded, 'User declares no embeddables — has the mapping changed?');
|
||||
|
||||
foreach ($embedded as $property) {
|
||||
self::assertIsArray(
|
||||
$exported[$property->getName()] ?? null,
|
||||
sprintf(
|
||||
'User::$%s is an embeddable a person configures, and a config backup does not carry '
|
||||
. 'it. Give it toArray()/applyArray() and a line each in ConfigBackupUsers::exportUser() '
|
||||
. 'and ConfigBackupUserRestorer::createUser(), or a restore silently puts everybody '
|
||||
. 'back on its defaults — which is how the category sorting was lost.',
|
||||
$property->getName(),
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Every admin-configured singleton is in the database section.
|
||||
*
|
||||
|
||||
@@ -7,6 +7,7 @@ namespace App\Tests\Service\Backup;
|
||||
use App\Domain\Enum\Account\MailProvider;
|
||||
use App\Domain\Enum\Ai\PromptSlot;
|
||||
use App\Domain\Enum\Integration\Provider;
|
||||
use App\Domain\Enum\Mail\CategorySource;
|
||||
use App\Entity\Ai\AiSettings;
|
||||
use App\Entity\Integration\IntegrationProviderConfig;
|
||||
use App\Entity\Integration\MailProviderConfig;
|
||||
@@ -157,6 +158,9 @@ final class ConfigBackupRoundTripTest extends KernelTestCase
|
||||
$this->check('user.appearance.fontScale', 1.125, $user->appearance->fontScale);
|
||||
$this->check('user.appearance.previewLines', 2, $user->appearance->previewLines);
|
||||
$this->check('user.aiPreferences.aboutMe', 'Ich baue Fahrräder.', $user->aiPreferences->aboutMe);
|
||||
$this->check('user.categorySorting.source', CategorySource::Assistant->value, $user->categorySorting->source);
|
||||
$this->check('user.categorySorting.overrideProvider', true, $user->categorySorting->overrideProvider);
|
||||
$this->check('user.categorySorting.tabs', false, $user->categorySorting->tabs);
|
||||
$this->check('user.signature (file)', 'anna-signature.png', $user->signature);
|
||||
|
||||
// ── The user settings bag ─────────────────────────────────────────
|
||||
@@ -371,6 +375,10 @@ final class ConfigBackupRoundTripTest extends KernelTestCase
|
||||
|
||||
$user->aiPreferences->aboutMe = 'Ich baue Fahrräder.';
|
||||
|
||||
$user->categorySorting->source = CategorySource::Assistant->value;
|
||||
$user->categorySorting->overrideProvider = true;
|
||||
$user->categorySorting->tabs = false;
|
||||
|
||||
$user->setSetting(User::SETTING_CLOCK, '24h');
|
||||
$user->setSetting(User::SETTING_SEARCH_SORT, 'oldest');
|
||||
$user->setSetting(User::SETTING_CALENDAR_PANE_WIDTH, 420);
|
||||
|
||||
@@ -17,7 +17,9 @@ use App\Domain\Enum\Mail\LabelRole;
|
||||
use App\Domain\DTO\Backup\ConfigBackupPlan;
|
||||
use App\Domain\DTO\Backup\ConfigBackupPlanItem;
|
||||
use App\Entity\Calendar\Calendar;
|
||||
use App\Domain\Enum\Mail\CategorySource;
|
||||
use App\Entity\Embeddable\AiPreferences;
|
||||
use App\Entity\Embeddable\CategorySorting;
|
||||
use App\Entity\Calendar\CalendarShareLink;
|
||||
use App\Entity\Integration\Integration;
|
||||
use App\Entity\Label\Label;
|
||||
@@ -146,6 +148,10 @@ final class ConfigBackupUsersTest extends KernelTestCase
|
||||
array_keys((new AiPreferences())->toArray()),
|
||||
array_keys($document['users'][self::EMAIL]['aiPreferences']),
|
||||
);
|
||||
self::assertSame(
|
||||
array_keys((new CategorySorting())->toArray()),
|
||||
array_keys($document['users'][self::EMAIL]['categorySorting']),
|
||||
);
|
||||
|
||||
// Everything past this line is the other install: a different key, and
|
||||
// nobody at home.
|
||||
@@ -195,6 +201,15 @@ final class ConfigBackupUsersTest extends KernelTestCase
|
||||
self::assertSame('Halte dich kurz.', $user->aiPreferences->systemPrompt);
|
||||
self::assertSame(ReplyContext::Thread, $user->aiPreferences->replyContext);
|
||||
|
||||
// ── How this person's mail is sorted ──────────────────────────────
|
||||
// Named literally for the reason summaryOff is above. All three were
|
||||
// absent from the document until v0.3.2, and a restore that drops them
|
||||
// hands back an inbox in tabs the person had switched off, sorted by
|
||||
// rules they had replaced — with nothing on the screen saying so.
|
||||
self::assertSame(CategorySource::Assistant->value, $user->categorySorting->source);
|
||||
self::assertTrue($user->categorySorting->overrideProvider);
|
||||
self::assertFalse($user->categorySorting->tabs);
|
||||
|
||||
// ── The second factor ─────────────────────────────────────────────
|
||||
// Read back through the entity, so this is the value the new install's
|
||||
// key decrypts to — and it is the string the authenticator app was
|
||||
@@ -541,6 +556,68 @@ final class ConfigBackupUsersTest extends KernelTestCase
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* A backup written before the sorting decisions were carried restores the
|
||||
* person onto the defaults, and everything else about them as before.
|
||||
*
|
||||
* Not a hypothetical: it is every file an operator is holding from v0.3.2
|
||||
* or earlier. The key is removed rather than emptied, because absent is
|
||||
* what those files actually look like.
|
||||
*/
|
||||
public function testABackupWithoutSortingDecisionsRestoresTheDefaults(): void
|
||||
{
|
||||
$this->seedTheOperator();
|
||||
|
||||
$document = $this->exporter->document();
|
||||
|
||||
unset($document['users'][self::EMAIL]['categorySorting']);
|
||||
|
||||
$this->becomeADifferentInstallation();
|
||||
$this->importer->apply($document);
|
||||
|
||||
$this->entityManager->flush();
|
||||
$this->entityManager->clear();
|
||||
|
||||
$user = static::getContainer()->get(UserRepository::class)->findOneBy(['email' => self::EMAIL]);
|
||||
|
||||
self::assertInstanceOf(User::class, $user, 'a document without the key no longer restores the user');
|
||||
self::assertSame(CategorySource::Rules->value, $user->categorySorting->source);
|
||||
self::assertFalse($user->categorySorting->overrideProvider);
|
||||
self::assertTrue($user->categorySorting->tabs);
|
||||
// The neighbouring embeddable, to show the missing key cost nothing else.
|
||||
self::assertTrue($user->aiPreferences->searchOff);
|
||||
}
|
||||
|
||||
/**
|
||||
* A source this build has never heard of falls back to the rules, and the
|
||||
* two switches beside it are still restored.
|
||||
*
|
||||
* The file a NEWER plMail wrote, or one somebody edited. Throwing here
|
||||
* would lose the whole user over one word; storing the word as it came
|
||||
* would put a value in a sixteen-character column that nothing can read.
|
||||
*/
|
||||
public function testAnUnknownSortingSourceFallsBackWithoutLosingTheRest(): void
|
||||
{
|
||||
$this->seedTheOperator();
|
||||
|
||||
$document = $this->exporter->document();
|
||||
|
||||
$document['users'][self::EMAIL]['categorySorting']['source'] = 'a-source-from-a-future-release';
|
||||
|
||||
$this->becomeADifferentInstallation();
|
||||
$this->importer->apply($document);
|
||||
|
||||
$this->entityManager->flush();
|
||||
$this->entityManager->clear();
|
||||
|
||||
$user = static::getContainer()->get(UserRepository::class)->findOneBy(['email' => self::EMAIL]);
|
||||
|
||||
self::assertInstanceOf(User::class, $user, 'an unknown source cost the whole user');
|
||||
self::assertSame(CategorySource::Rules->value, $user->categorySorting->source);
|
||||
self::assertTrue($user->categorySorting->overrideProvider);
|
||||
self::assertFalse($user->categorySorting->tabs);
|
||||
}
|
||||
|
||||
/**
|
||||
* A document from before users were part of the format restores exactly as
|
||||
* it did then.
|
||||
@@ -656,6 +733,12 @@ final class ConfigBackupUsersTest extends KernelTestCase
|
||||
$user->aiPreferences->systemPrompt = 'Halte dich kurz.';
|
||||
$user->aiPreferences->replyContext = ReplyContext::Thread;
|
||||
|
||||
// And every sorting decision, likewise: none of the three left where a
|
||||
// new User starts, `tabs` included — its default is the only true one.
|
||||
$user->categorySorting->source = CategorySource::Assistant->value;
|
||||
$user->categorySorting->overrideProvider = true;
|
||||
$user->categorySorting->tabs = false;
|
||||
|
||||
$user->restoreTwoFactor(self::TOTP_SECRET, new \DateTimeImmutable('2026-02-03T04:05:06+00:00'));
|
||||
$user->backupCodes = [User::hashBackupCode('recovery-code-one'), User::hashBackupCode('recovery-code-two')];
|
||||
$user->setSetting(User::SETTING_ONBOARDING_COMPLETED_AT, '2026-02-01T00:00:00+00:00');
|
||||
|
||||
Reference in new issue
Block a user