From 88e3e69b40e04a451260d352cc996ee2174b8c6d Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Wed, 4 Nov 2026 09:02:57 +0100 Subject: [PATCH] fix(contexts): map each node of an imported application to its own table or view Moving a user's data imported an application once for every table it held, and pointed every node of each copy at that one table, views included. The migrator now imports the tables and views first and then each application once, mapping every node to the table or view it was imported as. Page contents and widget sources follow the node they showed, and menu items that open a table or a view keep theirs; a node that was not imported is left out. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ruben van der Linde --- lib/Service/ContextService.php | 104 +++++++++--------- lib/UserMigration/TablesMigrator.php | 29 ++--- .../Service/ContextPagesRoundTripTest.php | 62 +++++++++-- tests/unit/Service/ContextServiceTest.php | 80 ++++++++------ tests/unit/Service/MenuServiceTest.php | 43 +++++++- tests/unit/TablesMigratorTest.php | 46 ++++++++ 6 files changed, 253 insertions(+), 111 deletions(-) diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index 12ffa5e96a..5397807de0 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -21,7 +21,6 @@ use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; use OCA\Tables\Db\PageMapper; -use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Errors\BadRequestError; @@ -656,32 +655,26 @@ private function importablePages(array $pages): array { } /** - * Imports an application of a user's data that holds the given table. Its menu keeps - * the links, the items for its pages and the items for the table, found by the uuid - * it was exported with. + * Imports an exported application with each node pointing at the table or view it was + * imported as. A node that was not imported is left out, together with the page + * contents and widget sources showing it. An application none of whose nodes was + * imported is not imported. * - * @param Table $table - * @param array $context - * @param int $tableOldId - * @param string|null $tableOldUuid - * - * @return Context|null + * @param array $context exported application + * @param array $tableIds new table ids by exported id + * @param array $viewIds new view ids by exported id * * @throws \Exception */ - public function importContext(Table $table, array $context, int $tableOldId, ?string $tableOldUuid = null): ?Context { - // checking if any node in $context['nodes'] matches the old table id - $hasMatchingNode = false; - if (!empty($context['nodes'])) { - foreach ($context['nodes'] as $nodeArr) { - if (($nodeArr['node_id'] ?? null) == $tableOldId) { - $hasMatchingNode = true; - break; - } + public function importContext(array $context, string $ownerId, array $tableIds, array $viewIds): ?Context { + $nodes = []; + foreach (is_array($context['nodes'] ?? null) ? $context['nodes'] : [] as $nodeArr) { + $node = is_array($nodeArr) ? self::importedNode($nodeArr, $tableIds, $viewIds) : null; + if ($node !== null) { + $nodes[] = [$nodeArr, ...$node]; } } - - if (!$hasMatchingNode) { + if ($nodes === []) { return null; } @@ -689,32 +682,21 @@ public function importContext(Table $table, array $context, int $tableOldId, ?st $newContext->setName($context['name'] ?? ''); $newContext->setIcon($context['iconName'] ?? ''); $newContext->setDescription($context['description'] ?? ''); - $newContext->setOwnerId($table->getOwnership() ?? ''); + $newContext->setOwnerId($ownerId); $newContext->setOwnerType($context['ownerType'] ?? 0); $this->contextMapper->insert($newContext); - // insert nodes $nodeRelMap = []; - // the relations of the table being imported, the only node a widget source can keep - $sourceRelMap = []; - if (!empty($context['nodes'])) { - foreach ($context['nodes'] as $nodeArr) { - $nodeType = $nodeArr['node_type'] ?? $nodeArr['nodeType'] ?? 0; - $contextNodeRel = new ContextNodeRelation(); - $contextNodeRel->setContextId($newContext->getId()); - $contextNodeRel->setNodeId($table->getId()); - $contextNodeRel->setNodeType($nodeType); - $contextNodeRel->setPermissions($nodeArr['permissions'] ?? 0); - $contextNodeRel = $this->contextNodeRelMapper->insert($contextNodeRel); - $oldRelId = self::importedKey($nodeArr['id'] ?? null); - if ($oldRelId === null) { - continue; - } + foreach ($nodes as [$nodeArr, $nodeType, $nodeId]) { + $contextNodeRel = new ContextNodeRelation(); + $contextNodeRel->setContextId($newContext->getId()); + $contextNodeRel->setNodeId($nodeId); + $contextNodeRel->setNodeType($nodeType); + $contextNodeRel->setPermissions($nodeArr['permissions'] ?? 0); + $contextNodeRel = $this->contextNodeRelMapper->insert($contextNodeRel); + $oldRelId = self::importedKey($nodeArr['id'] ?? null); + if ($oldRelId !== null) { $nodeRelMap[$oldRelId] = $contextNodeRel; - $oldNodeId = $nodeArr['node_id'] ?? $nodeArr['nodeId'] ?? null; - if (is_numeric($nodeType) && (int)$nodeType === Application::NODE_TYPE_TABLE && is_numeric($oldNodeId) && (int)$oldNodeId === $tableOldId) { - $sourceRelMap[$oldRelId] = $contextNodeRel; - } } } @@ -759,23 +741,45 @@ public function importContext(Table $table, array $context, int $tableOldId, ?st $this->logger->warning('Skipping ' . (count($widgets) - GridWidgetTypes::MAX_WIDGETS_PER_PAGE) . ' imported widgets: a page holds at most ' . GridWidgetTypes::MAX_WIDGETS_PER_PAGE . '.'); } foreach (array_slice($widgets, 0, GridWidgetTypes::MAX_WIDGETS_PER_PAGE) as $index => $widgetArr) { - $this->importWidget($page->getId(), $index, $widgetArr, $sourceRelMap); + $this->importWidget($page->getId(), $index, $widgetArr, $nodeRelMap); } } } if (is_array($context['menu'] ?? null)) { - $tableRelation = reset($sourceRelMap); + $imported = $this->contextMapper->findById($newContext->getId()); $this->logSkippedMenuItems($this->menuService->import( - $this->contextMapper->findById($newContext->getId()), + $imported, $context['menu'], - static fn (string $kind, string $uuid): ?int => $kind === 'table' && $uuid === $tableOldUuid && $tableRelation !== false ? $tableRelation->getId() : null, + fn (string $kind, string $uuid): ?int => $this->nodeRelIdByUuid($imported, $kind, $uuid), )); } return $newContext; } + /** + * The node type and new id of an exported node, or null when it was not imported. + * + * @param array $nodeArr + * @param array $tableIds new table ids by exported id + * @param array $viewIds new view ids by exported id + * @return array{0: int, 1: int}|null + */ + private static function importedNode(array $nodeArr, array $tableIds, array $viewIds): ?array { + $nodeType = $nodeArr['node_type'] ?? $nodeArr['nodeType'] ?? null; + $oldNodeId = $nodeArr['node_id'] ?? $nodeArr['nodeId'] ?? null; + if (!is_int($oldNodeId)) { + return null; + } + $nodeId = match ($nodeType) { + Application::NODE_TYPE_TABLE => $tableIds[$oldNodeId] ?? null, + Application::NODE_TYPE_VIEW => $viewIds[$oldNodeId] ?? null, + default => null, + }; + return $nodeId === null ? null : [$nodeType, $nodeId]; + } + /** * @param list $skipped */ @@ -852,11 +856,11 @@ public function getPagesWithWidgets(Context $context): array { /** * Inserts an exported widget on a page. Its source is remapped to the imported node - * relation of the table being imported, and dropped when it showed another node. A - * widget that would not be saved through the API, for its type, settings, title, - * description, place on the grid or the shape of its source, is skipped. + * relation of its node, and dropped when that node was not imported. A widget that + * would not be saved through the API, for its type, settings, title, description, + * place on the grid or the shape of its source, is skipped. * - * @param array $nodeRelMap imported node relations of the table by their exported id + * @param array $nodeRelMap imported node relations by their exported id * @throws Exception */ private function importWidget(int $pageId, int $index, mixed $widgetArr, array $nodeRelMap): void { diff --git a/lib/UserMigration/TablesMigrator.php b/lib/UserMigration/TablesMigrator.php index 89a1780ce0..853dcabde7 100644 --- a/lib/UserMigration/TablesMigrator.php +++ b/lib/UserMigration/TablesMigrator.php @@ -195,7 +195,6 @@ public function import( $contexts = json_decode($importSource->getFileContents(self::FILE_CONTEXTS), true, self::JSON_DEPTH, self::JSON_OPTIONS); $tableIdMap = []; - $contextIdMap = []; $columnIdMap = []; $rowIdMap = []; $userId = $user->getUID(); @@ -236,12 +235,11 @@ public function import( } $rowIdMap = $this->importRows($importSource, $newTable, $table, $rowIdMap); - - $this->importContexts($contexts, $newTable, $table, $contextIdMap); $tableIdMap[$table['id']] = $newTable->getId(); } - $this->importViews($importSource, $tableIdMap, $columnIdMap, $userId); + $viewIdMap = $this->importViews($importSource, $tableIdMap, $columnIdMap, $userId); + $contextIdMap = $this->importContexts($contexts, $userId, $tableIdMap, $viewIdMap); $this->importShares($importSource, $tableIdMap, $contextIdMap, $userId); $this->importRowCells( @@ -351,10 +349,11 @@ private function importRowCells(array $data, array $rowIdMap, array $columnIdMap * @param array $columnIdMap * @param string $userId * - * @return void + * @return array new view ids by exported id */ - private function importViews(IImportSource $importSource, array $tableIdMap, array $columnIdMap, string $userId): void { + private function importViews(IImportSource $importSource, array $tableIdMap, array $columnIdMap, string $userId): array { $views = json_decode($importSource->getFileContents(self::FILE_VIEWS), true, self::JSON_DEPTH, self::JSON_OPTIONS); + $viewIdMap = []; foreach ($views as $view) { if (isset($tableIdMap[$view['tableId']])) { $newTableId = $tableIdMap[$view['tableId']]; @@ -367,9 +366,10 @@ private function importViews(IImportSource $importSource, array $tableIdMap, arr unset($setting); } $view = $this->remapViewCardSources($view, $columnIdMap); - $this->viewService->importView($newTableId, $view, $userId); + $viewIdMap[$view['id']] = $this->viewService->importView($newTableId, $view, $userId)->getId(); } } + return $viewIdMap; } private function remapViewCardSources(array $view, array $columnIdMap): array { @@ -437,20 +437,23 @@ private function importRows(IImportSource $importSource, Table $newTable, array } /** + * Imports each application once, with its nodes pointing at the imported tables and views. + * * @param array $contexts - * @param Table $newTable - * @param array $table - * @param array $contextIdMap + * @param array $tableIdMap new table ids by exported id + * @param array $viewIdMap new view ids by exported id * - * @return void + * @return array new application ids by exported id */ - private function importContexts(array $contexts, Table $newTable, array $table, array &$contextIdMap): void { + private function importContexts(array $contexts, string $userId, array $tableIdMap, array $viewIdMap): array { + $contextIdMap = []; foreach ($contexts as $context) { - $newContext = $this->contextService->importContext($newTable, $context, $table['id'], $table['uuid'] ?? null); + $newContext = $this->contextService->importContext($context, $userId, $tableIdMap, $viewIdMap); if ($newContext !== null) { $contextIdMap[$context['id']] = $newContext->getId(); } } + return $contextIdMap; } /** diff --git a/tests/unit/Service/ContextPagesRoundTripTest.php b/tests/unit/Service/ContextPagesRoundTripTest.php index 6548611618..90d0035618 100644 --- a/tests/unit/Service/ContextPagesRoundTripTest.php +++ b/tests/unit/Service/ContextPagesRoundTripTest.php @@ -37,6 +37,7 @@ use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -51,6 +52,8 @@ class ContextPagesRoundTripTest extends DatabaseTestCase { private ContextNodeRelationMapper $nodeRelMapper; private PageMapper $pageMapper; private WidgetService $widgetService; + private PermissionsService&MockObject $permissionsService; + private TableMapper&MockObject $tableMapper; /** @var int[] */ private array $contextIds = []; @@ -68,13 +71,15 @@ protected function setUp(): void { new WidgetSourceMapper($this->connectionAdapter), $this->connectionAdapter, ); + $this->permissionsService = $this->createMock(PermissionsService::class); + $this->tableMapper = $this->createMock(TableMapper::class); $this->service = new ContextService( $this->contextMapper, $this->nodeRelMapper, $this->pageMapper, new PageContentMapper($this->connectionAdapter), $this->createMock(LoggerInterface::class), - $this->createMock(PermissionsService::class), + $this->permissionsService, $this->createMock(IUserManager::class), $this->createMock(AuditLogServiceInterface::class), $this->connectionAdapter, @@ -82,7 +87,7 @@ protected function setUp(): void { false, $this->createMock(INavigationManager::class), $this->createMock(IURLGenerator::class), - $this->createMock(TableMapper::class), + $this->tableMapper, $this->createMock(ViewMapper::class), $this->widgetService, new GridWidgetTypes($l10n), @@ -103,9 +108,6 @@ protected function tearDown(): void { public function testExportedWidgetsAreImportedWithTheirNode(): void { [$context, $relation, $gridPageId] = $this->createContextWithGridPage(); - $table = new Table(); - $table->id = 4242; - $table->setOwnership(self::OWNER); $exported = json_decode(json_encode(array_merge( $this->contextMapper->findById($context->getId(), self::OWNER)->jsonSerialize(), @@ -113,7 +115,7 @@ public function testExportedWidgetsAreImportedWithTheirNode(): void { )), true); $exported['nodes'][$relation->getId()]['node_id'] = 4242; - $imported = $this->service->importContext($table, $exported, 4242); + $imported = $this->service->importContext($exported, self::OWNER, [4242 => 4242], []); $this->assertNotNull($imported); $this->contextIds[] = $imported->getId(); @@ -139,9 +141,6 @@ public function testExportedWidgetsAreImportedWithTheirNode(): void { public function testAnUnknownIconIsDroppedOnImport(): void { [$context, $relation, $gridPageId] = $this->createContextWithGridPage(); - $table = new Table(); - $table->id = 4243; - $table->setOwnership(self::OWNER); $exported = json_decode(json_encode(array_merge( $this->contextMapper->findById($context->getId(), self::OWNER)->jsonSerialize(), ['pages' => $this->service->getPagesWithWidgets($this->contextMapper->findById($context->getId(), self::OWNER))], @@ -150,7 +149,7 @@ public function testAnUnknownIconIsDroppedOnImport(): void { $exported['pages'][$gridPageId]['icon'] = 'no-such-icon'; $exported['pages'][$gridPageId]['widgets'][0]['icon'] = '../star'; - $imported = $this->service->importContext($table, $exported, 4243); + $imported = $this->service->importContext($exported, self::OWNER, [4243 => 4243], []); $this->assertNotNull($imported); $this->contextIds[] = $imported->getId(); @@ -177,6 +176,49 @@ public function testRemovingTheNodeKeepsTheWidgetAndDeletingTheContextRemovesIt( $this->assertSame([], $this->pageMapper->getPageIdsForContext($context->getId())); } + public function testAnApplicationOfTwoTablesAndAViewIsImportedWithEachNodeOnItsOwnTarget(): void { + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $this->tableMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => array_combine($ids, array_map(static fn (): Table => new Table(), $ids))); + $node = static fn (int $id, int $type): array => ['id' => $id, 'type' => $type, 'permissions' => Application::PERMISSION_READ]; + $context = $this->service->create('Two tables', 'tables', '', [ + $node(4151, Application::NODE_TYPE_TABLE), + $node(4152, Application::NODE_TYPE_TABLE), + $node(4153, Application::NODE_TYPE_VIEW), + ], self::OWNER, Application::OWNER_TYPE_USER); + $this->contextIds[] = $context->getId(); + $stored = $this->contextMapper->findById($context->getId(), self::OWNER); + $relationIds = array_column($stored->getNodes(), 'id', 'node_id'); + $page = new Page(); + $page->setContextId($context->getId()); + $page->setPageType(Page::TYPE_GRID); + $page->setTechnicalName('overview'); + $page = $this->pageMapper->insert($page); + foreach ([4151, 4152, 4153] as $y => $nodeId) { + $this->widgetService->insert($page->getId(), 'data', 'Node ' . $nodeId, ['gridX' => 0, 'gridY' => $y * 5, 'gridWidth' => 12, 'gridHeight' => 5], [], $relationIds[$nodeId]); + } + $stored = $this->contextMapper->findById($context->getId(), self::OWNER); + $exported = json_decode(json_encode(array_merge($stored->jsonSerialize(), ['pages' => $this->service->getPagesWithWidgets($stored)])), true); + + $imported = $this->service->importContext($exported, self::OWNER, [4151 => 5151, 4152 => 5152], [4153 => 5153]); + $this->assertNotNull($imported); + $this->contextIds[] = $imported->getId(); + + $importedContext = $this->contextMapper->findById($imported->getId(), self::OWNER); + $nodes = array_values($importedContext->getNodes()); + $this->assertSame( + [[5151, Application::NODE_TYPE_TABLE], [5152, Application::NODE_TYPE_TABLE], [5153, Application::NODE_TYPE_VIEW]], + array_map(static fn (array $node): array => [$node['node_id'], $node['node_type']], $nodes), + ); + $importedRelationIds = array_column($nodes, 'id', 'node_id'); + $startPage = array_values(array_filter($importedContext->getPages(), static fn (array $page): bool => $page['page_type'] === Page::TYPE_STARTPAGE))[0]; + $this->assertEqualsCanonicalizing(array_values($importedRelationIds), array_column($startPage['content'], 'node_rel_id')); + $gridPage = array_values(array_filter($this->service->getPagesWithWidgets($importedContext), static fn (array $page): bool => $page['page_type'] === Page::TYPE_GRID))[0]; + $this->assertSame( + ['Node 4151' => $importedRelationIds[5151], 'Node 4152' => $importedRelationIds[5152], 'Node 4153' => $importedRelationIds[5153]], + array_combine(array_column($gridPage['widgets'], 'title'), array_map(static fn (array $widget): ?int => $widget['source']['nodeRelId'] ?? null, $gridPage['widgets'])), + ); + } + /** * @return array{0: Context, 1: ContextNodeRelation, 2: int} */ diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index ba5ae6d882..33b81b594d 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -20,7 +20,6 @@ use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; use OCA\Tables\Db\PageMapper; -use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Service\Support\AuditLogServiceInterface; @@ -328,9 +327,6 @@ public function testAFailedNodeRemovalRollsBackTheDeletedSources(): void { } public function testImportContextRemapsSourcesAndSkipsInvalidWidgets(): void { - $table = new Table(); - $table->id = 77; - $table->setOwnership('user-1'); $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { $context->id = 8; return $context; @@ -358,7 +354,7 @@ public function testImportContextRemapsSourcesAndSkipsInvalidWidgets(): void { return new \OCA\Tables\Db\Widget(); }); - $this->service->importContext($table, [ + $this->service->importContext([ 'name' => 'Projects', 'nodes' => [['id' => 101, 'node_id' => 5, 'node_type' => 0, 'permissions' => 1]], 'pages' => [[ @@ -379,7 +375,7 @@ public function testImportContextRemapsSourcesAndSkipsInvalidWidgets(): void { ['type' => 'text', 'title' => ' Notes ', 'gridX' => 6, 'gridY' => 11, 'gridWidth' => 6, 'gridHeight' => 2, 'settings' => [], 'source' => null], ], ]], - ], 5); + ], 'user-1', [5 => 77], []); self::assertSame(['overview', 'Overview', 'star', 'At a glance'], [$insertedPages[0]->getTechnicalName(), $insertedPages[0]->getTitle(), $insertedPages[0]->getIcon(), $insertedPages[0]->getDescription()]); self::assertCount(4, $inserted); @@ -448,9 +444,6 @@ public static function importedPages(): array { * @param array $expected type and technical name by page id */ public function testImportedPages(array $pages, array $expected): void { - $table = new Table(); - $table->id = 40; - $table->setOwnership('user-1'); $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { $context->id = 8; return $context; @@ -469,7 +462,7 @@ public function testImportedPages(array $pages, array $expected): void { return $page; }); - $this->service->importContext($table, ['name' => 'Projects', 'nodes' => [['id' => 1, 'node_id' => 4, 'node_type' => 0]], 'pages' => $pages], 4); + $this->service->importContext(['name' => 'Projects', 'nodes' => [['id' => 1, 'node_id' => 4, 'node_type' => 0]], 'pages' => $pages], 'user-1', [4 => 40], []); self::assertSame($expected, $inserted); } @@ -524,40 +517,66 @@ private static function dataWidget(int $gridY, mixed $source): array { return ['type' => 'data', 'title' => null, 'gridX' => 0, 'gridY' => $gridY, 'gridWidth' => 12, 'gridHeight' => 5, 'settings' => [], 'source' => $source]; } - public function testAnImportedWidgetSourceKeepsOnlyTheTableBeingImported(): void { + public function testAnApplicationIsImportedOnceWithEachNodeOnItsOwnTableOrView(): void { $exported = [ 'name' => 'Projects', 'nodes' => [ - ['id' => 101, 'node_id' => 5, 'node_type' => 0, 'permissions' => 1], - ['id' => 102, 'node_id' => 6, 'node_type' => 0, 'permissions' => 1], + ['id' => 101, 'node_id' => 5, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 1], + ['id' => 102, 'node_id' => 6, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 3], + ['id' => 103, 'node_id' => 5, 'node_type' => Application::NODE_TYPE_VIEW, 'permissions' => 1], + ['id' => 104, 'node_id' => 9, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 1], ], 'pages' => [[ 'id' => 301, 'page_type' => Page::TYPE_GRID, 'technical_name' => 'overview', - 'content' => [], - 'widgets' => [self::dataWidget(0, ['nodeRelId' => 101]), self::dataWidget(5, ['nodeRelId' => 102])], + 'content' => [['node_rel_id' => 101, 'order' => 10], ['node_rel_id' => 103, 'order' => 20], ['node_rel_id' => 104, 'order' => 30]], + 'widgets' => [ + self::dataWidget(0, ['nodeRelId' => 101]), + self::dataWidget(5, ['nodeRelId' => 102]), + self::dataWidget(10, ['nodeRelId' => 103]), + self::dataWidget(15, ['nodeRelId' => 104]), + ], ]], ]; $record = $this->recordImport(); - $table = new Table(); - $table->id = 78; - $table->setOwnership('user-1'); + $contents = []; + $this->pageContentMapper->method('insert')->willReturnCallback(static function (PageContent $content) use (&$contents): PageContent { + $contents[] = $content->getNodeRelId(); + return $content; + }); + + $imported = $this->service->importContext($exported, 'user-1', [5 => 77, 6 => 78], [5 => 90]); + + self::assertNotNull($imported); + self::assertSame('user-1', $imported->getOwnerId()); + self::assertSame( + [[Application::NODE_TYPE_TABLE, 77, 1], [Application::NODE_TYPE_TABLE, 78, 3], [Application::NODE_TYPE_VIEW, 90, 1]], + array_map(static fn (ContextNodeRelation $relation): array => [$relation->getNodeType(), $relation->getNodeId(), $relation->getPermissions()], $record['relations']), + ); + self::assertSame([501, 503], $contents); + self::assertSame([501, 502, 503, null], array_column($record['widgets'], 5)); + } + + public function testAnApplicationWithoutImportedNodesIsNotImported(): void { + $this->contextMapper->expects($this->never())->method('insert'); - // the migrator imports the application once per table it holds; this is table 6 - $this->service->importContext($table, $exported, 6); + $imported = $this->service->importContext([ + 'name' => 'Projects', + 'nodes' => [ + ['id' => 101, 'node_id' => 5, 'node_type' => Application::NODE_TYPE_TABLE], + ['id' => 102, 'node_id' => '6', 'node_type' => Application::NODE_TYPE_TABLE], + ['id' => 103, 'node_id' => 6, 'node_type' => 7], + ], + ], 'user-1', [6 => 78], [5 => 90]); - $relationOfTable6 = $record['relations'][1]; - self::assertSame([null, $relationOfTable6->getId()], array_column($record['widgets'], 5)); + self::assertNull($imported); } public function testABrokenExportFileSkipsTheWidgetInsteadOfFailing(): void { $record = $this->recordImport(); - $table = new Table(); - $table->id = 77; - $table->setOwnership('user-1'); - $this->service->importContext($table, [ + $this->service->importContext([ 'name' => 'Projects', 'nodes' => [['id' => [101], 'node_id' => 5, 'node_type' => 0, 'permissions' => 1], ['id' => 102, 'node_id' => 5, 'node_type' => 0, 'permissions' => 1]], 'pages' => [[ @@ -575,7 +594,7 @@ public function testABrokenExportFileSkipsTheWidgetInsteadOfFailing(): void { self::dataWidget(20, ['nodeRelId' => 102]), ], ]], - ], 5); + ], 'user-1', [5 => 77], []); self::assertSame([null, null], [$record['pages'][0]->getTitle(), $record['pages'][0]->getDescription()]); self::assertCount(1, $record['widgets']); @@ -584,19 +603,16 @@ public function testABrokenExportFileSkipsTheWidgetInsteadOfFailing(): void { public function testAnImportedPageKeepsAtMostTheMaximumNumberOfWidgets(): void { $record = $this->recordImport(); - $table = new Table(); - $table->id = 77; - $table->setOwnership('user-1'); $widgets = []; for ($i = 0; $i < GridWidgetTypes::MAX_WIDGETS_PER_PAGE + 5; $i++) { $widgets[] = ['type' => 'text', 'title' => null, 'gridX' => 0, 'gridY' => $i * 2, 'gridWidth' => 6, 'gridHeight' => 2, 'settings' => [], 'source' => null]; } - $this->service->importContext($table, [ + $this->service->importContext([ 'name' => 'Projects', 'nodes' => [['id' => 101, 'node_id' => 5, 'node_type' => 0, 'permissions' => 1]], 'pages' => [['id' => 301, 'page_type' => Page::TYPE_GRID, 'technical_name' => 'one', 'content' => [], 'widgets' => $widgets]], - ], 5); + ], 'user-1', [5 => 77], []); self::assertCount(GridWidgetTypes::MAX_WIDGETS_PER_PAGE, $record['widgets']); } diff --git a/tests/unit/Service/MenuServiceTest.php b/tests/unit/Service/MenuServiceTest.php index b3ab1cea99..6b141167aa 100644 --- a/tests/unit/Service/MenuServiceTest.php +++ b/tests/unit/Service/MenuServiceTest.php @@ -19,6 +19,7 @@ use OCA\Tables\Db\PageMapper; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; +use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Db\WidgetMapper; use OCA\Tables\Db\WidgetSourceMapper; @@ -51,6 +52,8 @@ class MenuServiceTest extends DatabaseTestCase { private const OWNER = 'menu-service-owner'; private const TABLE = 4151; private const TABLE_UUID = 'a5d8e2c4-4151-4cde-9f00-000000004151'; + private const VIEW = 4152; + private const VIEW_UUID = 'a5d8e2c4-4152-4cde-9f00-000000004152'; private MenuService $menuService; private ContextService $contextService; @@ -82,7 +85,14 @@ protected function setUp(): void { $table->setUuid(self::TABLE_UUID); $tableMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => in_array(self::TABLE, $ids, true) ? [self::TABLE => $table] : []); $tableMapper->method('findByUuid')->willReturnCallback(static fn (string $uuid): Table => $uuid === self::TABLE_UUID ? $table : throw new DoesNotExistException('No table')); - $this->menuService = new MenuService($contextMapper, $this->menuItemMapper, $this->permissionsService, $tableMapper, $this->createMock(ViewMapper::class), $this->connectionAdapter); + $viewMapper = $this->createMock(ViewMapper::class); + $viewMapper->method('findIdToTitleMap')->willReturnCallback(static fn (array $ids): array => in_array(self::VIEW, $ids, true) ? [self::VIEW => 'Team'] : []); + $view = new View(); + $view->id = self::VIEW; + $view->setUuid(self::VIEW_UUID); + $viewMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => in_array(self::VIEW, $ids, true) ? [self::VIEW => $view] : []); + $viewMapper->method('findByUuid')->willReturnCallback(static fn (string $uuid): View => $uuid === self::VIEW_UUID ? $view : throw new DoesNotExistException('No view')); + $this->menuService = new MenuService($contextMapper, $this->menuItemMapper, $this->permissionsService, $tableMapper, $viewMapper, $this->connectionAdapter); $this->contextService = new ContextService( $contextMapper, @@ -99,7 +109,7 @@ protected function setUp(): void { $this->createMock(INavigationManager::class), $this->createMock(IURLGenerator::class), $tableMapper, - $this->createMock(ViewMapper::class), + $viewMapper, $widgetService, new GridWidgetTypes($l10n), $this->menuItemMapper, @@ -343,12 +353,9 @@ public function testAMovedUsersMenuKeepsItsTableItsPagesAndItsLinks(): void { $this->menuService->create($contextId, null, null, null, ['url' => 'https://nextcloud.com'], self::OWNER); $context = $this->contextService->findById($contextId, self::OWNER); $exported = json_decode(json_encode($context->jsonSerialize() + ['pages' => $this->contextService->getPagesWithWidgets($context), 'menu' => $this->menuService->export($context)]), true); - $movedTable = new Table(); - $movedTable->id = self::TABLE; - $movedTable->setOwnership(self::OWNER); $exported['menu'][] = ['title' => 'Elsewhere', 'target' => ['table' => 'f0000000-0000-4000-8000-000000000000']]; - $moved = $this->contextService->importContext($movedTable, $exported, self::TABLE, self::TABLE_UUID); + $moved = $this->contextService->importContext($exported, self::OWNER, [self::TABLE => self::TABLE], []); $this->contextIds[] = $moved->getId(); $menu = $this->menuService->findAll($moved->getId(), self::OWNER); @@ -357,6 +364,30 @@ public function testAMovedUsersMenuKeepsItsTableItsPagesAndItsLinks(): void { $this->assertSame([['pageId' => (int)array_key_first(array_filter($movedContext->getPages(), static fn (array $page): bool => $page['page_type'] === Page::TYPE_STARTPAGE))], ['nodeRelId' => (int)array_key_first($movedContext->getNodes())]], [$menu[0]['target'], $menu[1]['target']]); } + public function testAMovedUsersMenuFollowsTheTablesAndViewsTheyWereImportedAs(): void { + $context = $this->contextService->create('Sales', 'tables', '', [ + ['id' => self::TABLE, 'type' => Application::NODE_TYPE_TABLE], + ['id' => self::VIEW, 'type' => Application::NODE_TYPE_VIEW], + ], self::OWNER, Application::OWNER_TYPE_USER); + $this->contextIds[] = $context->getId(); + $relationIds = array_flip(array_column($this->contextService->findById($context->getId(), self::OWNER)->getNodes(), 'node_id', 'id')); + $this->menuService->create($context->getId(), null, null, null, ['nodeRelId' => $relationIds[self::TABLE]], self::OWNER); + $this->menuService->create($context->getId(), null, null, null, ['nodeRelId' => $relationIds[self::VIEW]], self::OWNER); + $context = $this->contextService->findById($context->getId(), self::OWNER); + $exported = json_decode(json_encode($context->jsonSerialize() + ['pages' => $this->contextService->getPagesWithWidgets($context), 'menu' => $this->menuService->export($context)]), true); + $exported['nodes'][$relationIds[self::TABLE]]['node_id'] = 11; + $exported['nodes'][$relationIds[self::VIEW]]['node_id'] = 12; + $exported['menu'][] = ['title' => 'Elsewhere', 'target' => ['view' => 'f0000000-0000-4000-8000-000000000000']]; + + $moved = $this->contextService->importContext($exported, self::OWNER, [11 => self::TABLE], [12 => self::VIEW]); + $this->contextIds[] = $moved->getId(); + + $movedRelationIds = array_flip(array_column($this->contextService->findById($moved->getId(), self::OWNER)->getNodes(), 'node_id', 'id')); + $menu = array_slice($this->menuService->findAll($moved->getId(), self::OWNER), -2); + $this->assertSame(['Projects', 'Team'], array_column($menu, 'displayTitle')); + $this->assertSame([['nodeRelId' => $movedRelationIds[self::TABLE]], ['nodeRelId' => $movedRelationIds[self::VIEW]]], array_column($menu, 'target')); + } + public static function unusableItems(): array { $link = ['title' => 'Site', 'target' => ['url' => 'https://nextcloud.com']]; return [ diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 5786cb3ce0..8ade5e50da 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -22,6 +22,7 @@ use OCA\Tables\Db\ShareMapper; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; +use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; @@ -342,6 +343,7 @@ public function testImportRemapsViewCardSources(): void { ]; $views = [ [ + 'id' => 30, 'tableId' => 1, 'title' => 'Nested settings', 'columnSettings' => [['columnId' => 10, 'order' => 0]], @@ -351,6 +353,7 @@ public function testImportRemapsViewCardSources(): void { ], ], [ + 'id' => 31, 'tableId' => 1, 'title' => 'Without settings', 'columnSettings' => [['columnId' => 11, 'order' => 0]], @@ -404,4 +407,47 @@ public function rollBack(): void { $this->assertArrayNotHasKey('viewSettings', $importedViews[1]); $this->assertSame(21, $importedViews[1]['columnSettings'][0]['columnId']); } + + public function testImportImportsEachApplicationOnceWithItsTablesAndViews(): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user1'); + $importSource = $this->createMock(IImportSource::class); + $importSource->method('getMigratorVersion')->willReturn(1); + $context = ['id' => 7, 'name' => 'Projects', 'nodes' => [ + ['id' => 1, 'node_id' => 1, 'node_type' => 0], + ['id' => 2, 'node_id' => 2, 'node_type' => 0], + ['id' => 3, 'node_id' => 30, 'node_type' => 1], + ]]; + $importSource->method('getFileContents')->willReturnCallback(static fn (string $file): string => match ($file) { + 'tables.json' => json_encode([['id' => 1, 'title' => 'Tasks'], ['id' => 2, 'title' => 'People']]), + 'views.json' => json_encode([['id' => 30, 'tableId' => 2, 'title' => 'Team']]), + 'contexts.json' => json_encode([$context]), + default => json_encode([]), + }); + $this->tableMapper->method('getDBConnection')->willReturn(new class { + public function beginTransaction(): void { + } + public function commit(): void { + } + public function rollBack(): void { + } + }); + $this->tableService->method('importTable')->willReturnCallback(static function (array $table): Table { + $newTable = new Table(); + $newTable->setId($table['id'] * 100); + return $newTable; + }); + $this->viewService->method('importView')->willReturnCallback(static function (int $tableId, array $view): View { + $newView = new View(); + $newView->setId($view['id'] * 10); + return $newView; + }); + + $this->contextService->expects($this->once()) + ->method('importContext') + ->with($context, 'user1', [1 => 100, 2 => 200], [30 => 300]) + ->willReturn($this->createMock(Context::class)); + + $this->migrator->import($user, $importSource, new NullOutput()); + } }