feat(UserMigration): Overwork migration to include all settings (text blocks)#12784
Open
DerDreschner wants to merge 2 commits intofeat/user-migration-trusted-sendersfrom
Open
feat(UserMigration): Overwork migration to include all settings (text blocks)#12784DerDreschner wants to merge 2 commits intofeat/user-migration-trusted-sendersfrom
DerDreschner wants to merge 2 commits intofeat/user-migration-trusted-sendersfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for migrating Mail “text blocks” as part of the user migration flow, including deletion of existing text blocks before import and new unit coverage for export/import.
Changes:
- Add
TextBlocksMigrationServiceto export/import/delete text blocks during user migration. - Wire the new service into
MailAccountMigratorlifecycle (export/import/delete existing data). - Add unit tests covering text block export/import and invalid-import cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
lib/UserMigration/Service/TextBlocksMigrationService.php |
Implements export/import/delete of text blocks with basic validation and logging. |
lib/UserMigration/MailAccountMigrator.php |
Hooks text block migration into existing migration orchestration. |
tests/Unit/UserMigration/Service/TextBlocksMigrationServiceTest.php |
Adds unit tests for exporting/importing text blocks and invalid JSON handling. |
| */ | ||
| public function importTextBlocks(IUser $user, IImportSource $importSource, OutputInterface $output): void { | ||
| $output->writeln( | ||
| $this->l10n->t('Importing text blocks for user %s', $user->getUID()), |
| $textBlocksFileContent = $importSource->getFileContents(self::TEXT_BLOCKS_FILE); | ||
| } catch (UserMigrationException) { | ||
| $output->writeln( | ||
| $this->l10n->t('Text blocks for user %s not found. Continue...', $user->getUID()), |
|
|
||
| public function deleteAllTextBlocks(IUser $user, OutputInterface $output): void { | ||
| $output->writeln( | ||
| $this->l10n->t('Delete existing text blocks for user %s', $user->getUID()), |
Comment on lines
+105
to
+111
| $trustedSenderArrayIsValid = is_array($textBlock); | ||
|
|
||
| $titleIsValid = $trustedSenderArrayIsValid | ||
| && array_key_exists('title', $textBlock) | ||
| && is_string($textBlock['title']); | ||
|
|
||
| $contentIsValid = $trustedSenderArrayIsValid |
Comment on lines
+42
to
+43
| } catch (JsonException|UserMigrationException) { | ||
| throw new UserMigrationException("Failed to export text blocks for user {$user->getUID()}"); |
Comment on lines
+65
to
+66
| $trustedSendersList = [$textBlock1, $textBlock2]; | ||
| $this->importSource->expects(self::once())->method('getFileContents')->with(TextBlocksMigrationService::TEXT_BLOCKS_FILE)->willReturn(json_encode($trustedSendersList)); |
78ece05 to
d7b2aea
Compare
451cab5 to
d87ab82
Compare
d87ab82 to
dbd5edd
Compare
…c changes) Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
… blocks) Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
d7b2aea to
48c698f
Compare
ChristophWurst
approved these changes
Apr 24, 2026
Member
ChristophWurst
left a comment
There was a problem hiding this comment.
Nice how the separate class keeps it relatively simple and testable 👍
| } | ||
|
|
||
| public function testImportNoFileIsBeingIgnored(): void { | ||
| $this->importSource->expects(self::once())->method('getFileContents')->with(TextBlocksMigrationService::TEXT_BLOCKS_FILE)->willThrowException(new UserMigrationException()); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR belongs to a series of PRs that will enhance the user migration to include all settings.
Included changes in this PR:
I didn't include any unit tests for the removal code as that only calls another function.
Please ignore the failed CI run regarding the unconventional commit message. That happens as I tag another feature branch for better overview here.
Refs #12001