From 45413acafc5456d5bda1635cfff0b0991529d7bb Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 15 Oct 2026 19:56:06 +0200 Subject: [PATCH 1/2] feat(contexts): export and import menus and pages An exported application scheme now carries its menu and its pages with their widgets, so an application moves between instances with its pages and menu intact. - Export: pages come as the page API returns them; data widgets name their node by its relation id in the scheme's nodes. Menu items name tables and views by uuid, pages by their id in the scheme and their parent by technical name. An item whose target is gone is left out. - Import: pages come after the nodes, the menu after the pages. The start page is filled only when the scheme arranged it on the grid; a grid page replaces the page with the same technical name or is added. A data widget whose table or view does not exist here, or that the importer cannot add, is left out. A menu item that is invalid or whose target cannot be used is skipped; a child whose parent was skipped moves to the top. - Older schemes: every grid view of the scheme becomes a grid page with its widgets in place, its data widgets pointing at their table or view as a node; a menu item that opened it opens that page, and its node is no longer imported as a view. - Validation: the import request is validated into a ContextScheme in the controller; a malformed scheme is a 400, and every failure rolls the import back. - Dialog: a "Menu and pages" switch in the preview brings both along; pages only come with the resources they show. - API: menuItems, pages and gridViews on the import endpoint (OpenAPI and TS types regenerated). Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Controller/ContextController.php | 21 +- lib/Model/ContextScheme.php | 89 ++++ lib/Service/ContextSchemeService.php | 457 ++++++++++++++++++ lib/Service/ContextService.php | 66 ++- openapi.json | 69 +++ playwright/e2e/application.spec.ts | 134 ++++- src/modules/modals/ImportContextScheme.vue | 35 +- .../modals/ImportContextSchemePreview.vue | 67 +++ src/types/openapi/openapi.ts | 30 ++ .../unit/Controller/ContextControllerTest.php | 67 ++- tests/unit/Model/ContextSchemeTest.php | 113 +++++ .../unit/Service/ContextSchemeServiceTest.php | 259 ++++++++++ tests/unit/Service/ContextServiceTest.php | 105 +++- 13 files changed, 1488 insertions(+), 24 deletions(-) create mode 100644 lib/Service/ContextSchemeService.php create mode 100644 tests/unit/Model/ContextSchemeTest.php create mode 100644 tests/unit/Service/ContextSchemeServiceTest.php diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index 72fbad908b..02bfacaec3 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -16,6 +16,7 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\MenuItemInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnService; @@ -361,6 +362,9 @@ public function previewSchemeChanges(int $contextId, array $updateScheme): DataR * @param string $description Description of the context * @param list $nodes Ordered meta data of the related nodes * @psalm-param array{addTables: list, modifyTables: list} $tables Tables to be added or modified + * @psalm-param ?list $menuItems Menu of the context: tables and views by uuid, pages by their id in the scheme, parents by technical name; leave it out to keep the current menu + * @psalm-param ?list> $pages Pages of the context with their widgets (a scheme of an older version keys them by id); data widgets name their node by its id in the nodes; leave it out to keep the current pages + * @psalm-param list> $gridViews Views without a table from a scheme of an older version; each becomes a page * * @return DataResponse, array{}>|DataResponse * @@ -371,16 +375,24 @@ public function previewSchemeChanges(int $contextId, array $updateScheme): DataR */ #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_MANAGE, typeParam: 'context', idParam: 'contextId')] - public function importScheme(int $contextId, string $name, string $iconName, string $description, array $nodes, array $tables): DataResponse { + public function importScheme(int $contextId, string $name, string $iconName, string $description, array $nodes, array $tables, ?array $menuItems = null, ?array $pages = null, array $gridViews = []): DataResponse { + try { + $scheme = ContextScheme::createFromInputArray(['name' => $name, 'icon' => $iconName, 'description' => $description, 'nodes' => $nodes, 'tables' => $tables, 'menuItems' => $menuItems, 'pages' => $pages, 'gridViews' => $gridViews]); + } catch (InvalidArgumentException $e) { + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); + } try { $this->db->beginTransaction(); - $context = $this->contextService->importScheme($contextId, $name, $iconName, $description, $nodes, $tables, $this->userId); + $context = $this->contextService->importScheme($contextId, $scheme, $this->userId); $this->db->commit(); return new DataResponse($context->jsonSerialize()); } catch (\InvalidArgumentException $e) { $this->db->rollBack(); return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); - } catch (Exception|MultipleObjectsReturnedException $e) { + } catch (BadRequestError $e) { + $this->db->rollBack(); + return $this->handleBadRequestError($e); + } catch (Exception|MultipleObjectsReturnedException|InternalError $e) { $this->db->rollBack(); return $this->handleError($e); } catch (PermissionError $e) { @@ -389,6 +401,9 @@ public function importScheme(int $contextId, string $name, string $iconName, str } catch (DoesNotExistException $e) { $this->db->rollBack(); return $this->handleNotFoundError(new NotFoundError($e->getMessage(), $e->getCode(), $e)); + } catch (NotFoundError $e) { + $this->db->rollBack(); + return $this->handleNotFoundError($e); } } diff --git a/lib/Model/ContextScheme.php b/lib/Model/ContextScheme.php index 3c511632c4..d37107acbe 100644 --- a/lib/Model/ContextScheme.php +++ b/lib/Model/ContextScheme.php @@ -7,6 +7,7 @@ namespace OCA\Tables\Model; +use InvalidArgumentException; use JsonSerializable; class ContextScheme implements JsonSerializable { @@ -18,9 +19,76 @@ public function __construct( protected ?array $nodes = [], protected ?array $pages = [], protected ?array $tables = [], + protected ?array $menuItems = [], + protected array $gridViews = [], ) { } + /** + * Builds the scheme from an import request and refuses what the import could not handle. + * + * @param array $data + * @throws InvalidArgumentException + */ + public static function createFromInputArray(array $data): self { + $tables = $data['tables'] ?? null; + if (!is_array($tables) || !isset($tables['addTables'], $tables['modifyTables']) || !is_array($tables['addTables']) || !is_array($tables['modifyTables'])) { + throw new InvalidArgumentException('Invalid tables structure provided: expected addTables and modifyTables lists.'); + } + foreach (['nodes', 'pages', 'menuItems', 'gridViews'] as $list) { + if (isset($data[$list]) && !is_array($data[$list])) { + throw new InvalidArgumentException('The scheme property "' . $list . '" must be a list.'); + } + } + if (!is_string($data['name'] ?? null) || trim($data['name']) === '') { + throw new InvalidArgumentException('A scheme needs a name.'); + } + foreach (['icon', 'description'] as $optional) { + if (isset($data[$optional]) && !is_string($data[$optional])) { + throw new InvalidArgumentException('The scheme property "' . $optional . '" must be a string.'); + } + } + foreach ($data['pages'] ?? [] as $page) { + if (!is_array($page) || (isset($page['content']) && !is_array($page['content']))) { + throw new InvalidArgumentException('Every page of a scheme must be an object, with its content as a list.'); + } + } + self::assertGridViewsValid($data['gridViews'] ?? []); + return new self( + name: $data['name'], + icon: $data['icon'] ?? null, + description: $data['description'] ?? null, + nodes: array_values($data['nodes'] ?? []), + // A scheme without pages or without a menu leaves those of the application alone + pages: isset($data['pages']) ? $data['pages'] : null, + tables: $tables, + menuItems: isset($data['menuItems']) ? array_values($data['menuItems']) : null, + gridViews: array_values($data['gridViews'] ?? []), + ); + } + + /** + * Grid views come from schemes of an older version, which kept pages in views without a table. + * Every one needs a uuid and a title, and its grid is an object. + * + * @throws InvalidArgumentException + */ + private static function assertGridViewsValid(array $gridViews): void { + foreach ($gridViews as $gridView) { + if (!is_array($gridView)) { + throw new InvalidArgumentException('Every entry of "gridViews" must be an object.'); + } + foreach (['uuid', 'title'] as $required) { + if (!is_string($gridView[$required] ?? null) || trim($gridView[$required]) === '') { + throw new InvalidArgumentException('Every entry of "gridViews" needs a "' . $required . '".'); + } + } + if (isset($gridView['grid']) && !is_array($gridView['grid'])) { + throw new InvalidArgumentException('The grid view property "grid" must be an object.'); + } + } + } + public function getName(): ?string { return $this->name ?? ''; } @@ -37,6 +105,10 @@ public function getNodes(): ?array { return $this->nodes; } + /** + * Pages with their widgets; data widgets name their node by its id in the nodes of the scheme. + * Null when the scheme has no pages, which leaves the pages of the application as they are. + */ public function getPages(): ?array { return $this->pages; } @@ -45,6 +117,22 @@ public function getTables(): ?array { return $this->tables; } + /** + * Menu items point at tables and views by uuid and at pages by their id in the scheme, so a + * scheme can be imported on another instance. Null when the scheme has no menu, which leaves + * the application's menu as it is. + */ + public function getMenuItems(): ?array { + return $this->menuItems; + } + + /** + * Views without a table from a scheme of an older version; the import turns each into a page. + */ + public function getGridViews(): array { + return $this->gridViews; + } + public function jsonSerialize(): mixed { return [ 'name' => $this->name, @@ -53,6 +141,7 @@ public function jsonSerialize(): mixed { 'nodes' => $this->nodes, 'pages' => $this->pages, 'tables' => $this->tables, + 'menuItems' => $this->menuItems, ]; } } diff --git a/lib/Service/ContextSchemeService.php b/lib/Service/ContextSchemeService.php new file mode 100644 index 0000000000..c6fb8ce850 --- /dev/null +++ b/lib/Service/ContextSchemeService.php @@ -0,0 +1,457 @@ +> + * @throws NotFoundError|Exception + */ + public function exportPages(int $contextId, string $userId): array { + return $this->pageService->findAll($contextId, $userId); + } + + /** + * Menu items with their targets as uuids, or as the id of a page of the scheme. An item whose + * target is gone is left out. + * + * @return list> + */ + public function exportMenuItems(Context $context): array { + $menuItems = []; + $technicalNamesById = array_column($context->getMenuItems() ?? [], 'technicalName', 'id'); + foreach ($context->getMenuItems() ?? [] as $item) { + if ($item['broken'] ?? false) { + continue; + } + $exported = [ + 'label' => $item['label'], + 'icon' => $item['icon'], + 'targetType' => $item['targetType'], + 'targetUuid' => null, + 'targetPageId' => null, + 'url' => $item['url'], + 'technicalName' => $item['technicalName'], + 'parentTechnicalName' => $item['parentId'] !== null ? ($technicalNamesById[$item['parentId']] ?? null) : null, + ]; + try { + match ($item['targetType']) { + MenuItem::TARGET_TABLE => $exported['targetUuid'] = $this->tableMapper->find((int)$item['targetId'])->getUuid(), + MenuItem::TARGET_VIEW => $exported['targetUuid'] = $this->viewMapper->find((int)$item['targetId'])->getUuid(), + MenuItem::TARGET_PAGE => $exported['targetPageId'] = $item['targetId'], + default => null, + }; + } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + $this->logger->warning('Menu item target could not be exported, the item is skipped', ['exception' => $e]); + continue; + } + $menuItems[] = $exported; + } + return $menuItems; + } + + /** + * Brings the pages of a scheme into an application whose nodes the import already set. The + * start page of the scheme fills the start page here, but only when it was arranged on the + * grid; otherwise the start page keeps the content the nodes gave it. A grid page replaces the + * page with the same technical name, or is added. Every grid view of an older scheme becomes a + * grid page. A data widget whose table or view does not exist here, or that the importer cannot + * add to the application, is left out. + * + * @return array{pages: array, gridViews: array} the local page ids by page id in the scheme, and by grid view uuid + * @throws BadRequestError|NotFoundError|PermissionError|Exception + */ + public function importPages(Context $context, ContextScheme $scheme, string $userId): array { + $contextId = $context->getId(); + $nodeKeys = []; + foreach ($context->getNodes() ?? [] as $node) { + $nodeKeys[$this->nodeKey((int)$node['node_type'], (int)$node['node_id'])] = true; + } + $schemeNodes = $this->schemeNodes($scheme->getNodes() ?? []); + + $localPages = $this->pageService->findAll($contextId, $userId); + $startPageId = null; + $pagesByTechnicalName = []; + foreach ($localPages as $page) { + if ($page['page_type'] === Page::TYPE_STARTPAGE) { + $startPageId = $page['id']; + } elseif ($page['technical_name'] !== null) { + $pagesByTechnicalName[$page['technical_name']] = $page['id']; + } + } + + $pageIds = []; + foreach ($scheme->getPages() ?? [] as $key => $page) { + if (!is_array($page)) { + continue; + } + $sourceId = $page['id'] ?? $key; + $content = PageContentHelper::readContent($this->storedContent($page['content'] ?? [])); + if (($page['page_type'] ?? $page['pageType'] ?? null) === Page::TYPE_STARTPAGE) { + if ($startPageId === null) { + continue; + } + $pageIds[$sourceId] = $startPageId; + if ($this->isArrangedOnTheGrid($page['content'] ?? [])) { + $widgets = $this->widgetsFromContent($content, $schemeNodes, $nodeKeys, $userId); + $this->pageService->saveWidgets($contextId, $startPageId, $widgets, $userId); + } + continue; + } + + $technicalName = $page['technical_name'] ?? $page['technicalName'] ?? null; + $technicalName = is_string($technicalName) && Page::isValidTechnicalName($technicalName) ? $technicalName : null; + $title = $this->pageTitle($page['title'] ?? null, $technicalName); + if ($title === null) { + $this->logger->info('Page skipped on import, it has no title'); + continue; + } + $pageId = $technicalName !== null ? ($pagesByTechnicalName[$technicalName] ?? null) : null; + if ($pageId === null) { + $pageId = $this->pageService->create($contextId, $title, $technicalName, $userId)['id']; + if ($technicalName !== null) { + $pagesByTechnicalName[$technicalName] = $pageId; + } + } else { + $this->pageService->update($contextId, $pageId, $title, null, $userId); + } + $pageIds[$sourceId] = $pageId; + $widgets = $this->widgetsFromContent($content, $schemeNodes, $nodeKeys, $userId); + $this->pageService->saveWidgets($contextId, $pageId, $widgets, $userId); + } + + $gridViewPageIds = []; + foreach ($scheme->getGridViews() as $gridView) { + $title = $this->pageTitle($gridView['title'], null); + $pageId = $this->pageService->create($contextId, (string)$title, null, $userId)['id']; + $widgets = $this->widgetsFromGridView($gridView['grid'] ?? null, $nodeKeys, $userId); + $this->pageService->saveWidgets($contextId, $pageId, $widgets, $userId); + $gridViewPageIds[$gridView['uuid']] = $pageId; + } + + return ['pages' => $pageIds, 'gridViews' => $gridViewPageIds]; + } + + /** + * Menu items of a scheme with their targets turned into local ids. Items that are invalid, or + * whose target does not exist here or cannot be managed by the importer, are skipped. An item + * of an older scheme that opened a grid view opens the page that grid view became. A child + * names its parent by technical name; when the parent was skipped, the child moves to the top. + * A technical name that an earlier item already uses is dropped, so the item gets a derived one. + * + * @param list $menuItems + * @param array $pageIds local page ids by page id in the scheme + * @param array $gridViewPageIds local page ids by grid view uuid + * @return list + */ + public function resolveMenuItems(array $menuItems, string $userId, array $pageIds, array $gridViewPageIds): array { + $prepared = []; + $usedTechnicalNames = []; + foreach ($menuItems as $item) { + if (!is_array($item)) { + continue; + } + $targetType = $item['targetType'] ?? MenuItem::TARGET_URL; + $targetUuid = is_string($item['targetUuid'] ?? null) ? $item['targetUuid'] : null; + if ($targetType === MenuItem::TARGET_VIEW && $targetUuid !== null && isset($gridViewPageIds[$targetUuid])) { + $item['targetType'] = MenuItem::TARGET_PAGE; + $item['targetId'] = $gridViewPageIds[$targetUuid]; + } elseif ($targetType === MenuItem::TARGET_PAGE) { + $targetPageId = $item['targetPageId'] ?? null; + $item['targetId'] = (is_int($targetPageId) || is_string($targetPageId)) ? ($pageIds[$targetPageId] ?? null) : null; + } elseif ($targetType !== MenuItem::TARGET_URL) { + $item['targetId'] = is_string($targetType) ? $this->resolveTargetId($targetType, $targetUuid, $userId) : null; + } + $parentTechnicalName = is_string($item['parentTechnicalName'] ?? null) ? $item['parentTechnicalName'] : null; + unset($item['targetUuid'], $item['targetPageId'], $item['parentTechnicalName'], $item['parentIndex']); + try { + MenuItemInput::createFromInputArray($item); + } catch (InvalidArgumentException $e) { + $this->logger->info('Menu item skipped on import: {reason}', ['reason' => $e->getMessage()]); + continue; + } + $technicalName = $item['technicalName'] ?? null; + if (is_string($technicalName) && isset($usedTechnicalNames[$technicalName])) { + unset($item['technicalName']); + } elseif (is_string($technicalName)) { + $usedTechnicalNames[$technicalName] = true; + } + $prepared[] = ['item' => $item, 'parent' => $parentTechnicalName]; + } + + $parentPositions = []; + foreach ($prepared as $position => $entry) { + $technicalName = $entry['item']['technicalName'] ?? null; + if ($entry['parent'] === null && is_string($technicalName) && !isset($parentPositions[$technicalName])) { + $parentPositions[$technicalName] = $position; + } + } + $items = []; + foreach ($prepared as $entry) { + $entry['item']['parentIndex'] = $entry['parent'] === null ? null : ($parentPositions[$entry['parent']] ?? null); + $items[] = $entry['item']; + } + return MenuItemInput::createListFromInputArray($items); + } + + /** + * The local table or view of every node of the scheme, by its relation id in the scheme. + * + * @param array $nodes + * @return array + */ + private function schemeNodes(array $nodes): array { + $resolved = []; + foreach ($nodes as $node) { + if (!is_array($node) || !isset($node['id'], $node['node_type']) || !is_string($node['node_uuid'] ?? null)) { + continue; + } + $nodeType = (int)$node['node_type']; + $nodeId = $this->localId($nodeType === Application::NODE_TYPE_TABLE ? MenuItem::TARGET_TABLE : MenuItem::TARGET_VIEW, $node['node_uuid']); + if ($nodeId !== null) { + $resolved[(int)$node['id']] = ['nodeType' => $nodeType, 'nodeId' => $nodeId]; + } + } + return $resolved; + } + + /** + * Page content from a scheme in the form it is stored in: an exported page has its + * configuration and content apart, the database keeps them together. + * + * @param mixed $content + * @return array> + */ + private function storedContent(mixed $content): array { + if (!is_array($content)) { + return []; + } + $stored = []; + foreach ($content as $key => $entry) { + if (!is_array($entry)) { + continue; + } + if (is_array($entry['configuration'] ?? null) || is_array($entry['content'] ?? null)) { + $entry['configuration'] = [ + 'configuration' => is_array($entry['configuration'] ?? null) ? $entry['configuration'] : [], + 'content' => is_array($entry['content'] ?? null) ? $entry['content'] : [], + ]; + unset($entry['content']); + } + $stored[$key] = $entry; + } + return $stored; + } + + /** + * Whether page content was arranged on the grid: it has a widget that is not a table or view, + * or a position. Content of an older scheme only lists its tables and views in order. + */ + private function isArrangedOnTheGrid(mixed $content): bool { + if (!is_array($content)) { + return false; + } + foreach ($content as $entry) { + if (!is_array($entry)) { + continue; + } + $type = $entry['type'] ?? null; + if ((is_string($type) && $type !== '' && $type !== GridWidgetTypes::TYPE_DATA) + || ($entry['grid_y'] ?? $entry['gridY'] ?? null) !== null) { + return true; + } + } + return false; + } + + /** + * The widgets to save for page content read from a scheme. + * + * @param array> $content read by PageContentHelper + * @param array $schemeNodes + * @param array $nodeKeys the nodes of the application + * @return list> + */ + private function widgetsFromContent(array $content, array $schemeNodes, array &$nodeKeys, string $userId): array { + $widgets = []; + foreach ($content as $entry) { + $widget = [ + 'type' => $entry['type'], + 'configuration' => $entry['configuration'], + 'content' => $entry['content'], + 'gridX' => $entry['grid_x'], + 'gridY' => $entry['grid_y'], + 'gridWidth' => $entry['grid_width'], + 'gridHeight' => $entry['grid_height'], + ]; + if ($entry['type'] === GridWidgetTypes::TYPE_DATA) { + $node = $entry['node_rel_id'] !== null ? ($schemeNodes[$entry['node_rel_id']] ?? null) : null; + if ($node === null || !$this->canPlace($node, $nodeKeys, $userId)) { + $this->logger->info('Data widget skipped on import, its table or view does not exist here'); + continue; + } + $widget['node'] = $node; + } + $widgets[] = $widget; + } + return $widgets; + } + + /** + * The widgets of a grid view of an older scheme, placed as its layout placed them. A data + * widget named its table or view in its content; on a page that is the widget's node. + * + * @param array $nodeKeys the nodes of the application + * @return list> + */ + private function widgetsFromGridView(mixed $grid, array &$nodeKeys, string $userId): array { + $layout = []; + foreach (is_array($grid) && is_array($grid['layout'] ?? null) ? $grid['layout'] : [] as $item) { + if (is_array($item) && isset($item['widgetId'])) { + $layout[(string)$item['widgetId']] = $item; + } + } + $widgets = []; + foreach (is_array($grid) && is_array($grid['widgets'] ?? null) ? $grid['widgets'] : [] as $gridWidget) { + $position = is_array($gridWidget) ? ($layout[(string)($gridWidget['id'] ?? '')] ?? null) : null; + if ($position === null || !is_string($gridWidget['type'] ?? null)) { + continue; + } + $content = is_array($gridWidget['content'] ?? null) ? $gridWidget['content'] : []; + $widget = [ + 'type' => $gridWidget['type'], + 'configuration' => is_array($gridWidget['configuration'] ?? null) ? $gridWidget['configuration'] : [], + 'content' => $content, + 'gridX' => $position['gridX'] ?? null, + 'gridY' => $position['gridY'] ?? null, + 'gridWidth' => $position['gridWidth'] ?? null, + 'gridHeight' => $position['gridHeight'] ?? null, + ]; + if ($gridWidget['type'] === GridWidgetTypes::TYPE_DATA) { + $target = is_array($content['target'] ?? null) ? $content['target'] : []; + unset($widget['content']['target']); + $targetType = $target['type'] ?? null; + $nodeId = is_string($targetType) && is_string($target['uuid'] ?? null) ? $this->localId($targetType, $target['uuid']) : null; + $node = $nodeId === null ? null : [ + 'nodeType' => $targetType === MenuItem::TARGET_TABLE ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, + 'nodeId' => $nodeId, + ]; + if ($node === null || !$this->canPlace($node, $nodeKeys, $userId)) { + $this->logger->info('Grid widget skipped on import, its table or view does not exist here', ['widget' => $gridWidget['id'] ?? null]); + continue; + } + $widget['node'] = $node; + } + $widgets[] = $widget; + } + return $widgets; + } + + /** + * Whether a data widget can show a node: it is part of the application, or the importer can + * manage it, so saving the page adds it. + * + * @param array{nodeType: int, nodeId: int} $node + * @param array $nodeKeys the nodes of the application, extended by the nodes that saving adds + */ + private function canPlace(array $node, array &$nodeKeys, string $userId): bool { + $key = $this->nodeKey($node['nodeType'], $node['nodeId']); + if (isset($nodeKeys[$key])) { + return true; + } + if (!$this->permissionsService->canManageNodeById($node['nodeType'], $node['nodeId'], $userId)) { + return false; + } + $nodeKeys[$key] = true; + return true; + } + + /** + * The local id of a table or view a menu item names by uuid, or null when it does not exist + * here or the importer cannot manage it. + */ + private function resolveTargetId(string $targetType, ?string $uuid, string $userId): ?int { + $id = $uuid === null ? null : $this->localId($targetType, $uuid); + if ($id === null) { + return null; + } + $nodeType = $targetType === MenuItem::TARGET_TABLE ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW; + return $this->permissionsService->canManageNodeById($nodeType, $id, $userId) ? $id : null; + } + + /** + * The id of the table or view with the given uuid here, or null. + */ + private function localId(string $targetType, string $uuid): ?int { + if ($uuid === '') { + return null; + } + try { + return match ($targetType) { + MenuItem::TARGET_TABLE => $this->tableMapper->findByUuid($uuid)->getId(), + MenuItem::TARGET_VIEW => $this->viewMapper->findByUuid($uuid)?->getId(), + default => null, + }; + } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception) { + return null; + } + } + + /** + * A page title from a scheme, falling back to the technical name; null when there is neither. + */ + private function pageTitle(mixed $title, ?string $technicalName): ?string { + $title = is_string($title) ? trim($title) : ''; + if ($title === '') { + return $technicalName; + } + return mb_substr($title, 0, self::PAGE_TITLE_MAX_LENGTH); + } + + private function nodeKey(int $nodeType, int $nodeId): string { + return sprintf('t%di%d', $nodeType, $nodeId); + } +} diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index a7a4bfb11d..f540d86912 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -1067,8 +1067,8 @@ protected function importPageContent(Page $page, array $contents, array $nodeRel */ public function getScheme(int $contextId, string $userId): ContextScheme { $context = $this->contextMapper->findById($contextId); - $tableService = \OCP\Server::get(TableService::class); - $viewService = \OCP\Server::get(ViewService::class); + $tableService = $this->tableService(); + $viewService = $this->viewService(); $nodes = $context->getNodes(); $tables = []; @@ -1099,13 +1099,15 @@ public function getScheme(int $contextId, string $userId): ContextScheme { } } + $schemeService = $this->contextSchemeService(); return new ContextScheme( $context->getName(), $context->getIcon(), $context->getDescription(), array_values($nodes), - $context->getPages(), + $schemeService->exportPages($contextId, $userId), array_values($tables), + $schemeService->exportMenuItems($context), ); } @@ -1199,16 +1201,15 @@ public function compareSchemeChanges(int $contextId, array $updateScheme, string * @throws NotFoundExceptionInterface * @throws PermissionError */ - public function importScheme(int $contextId, string $name, string $iconName, string $description, array $nodes, array $tables, string $userId): Context { - // Validate the structure of the columns and views arrays - if (!isset($tables['addTables']) || !is_array($tables['addTables']) - || !isset($tables['modifyTables']) || !is_array($tables['modifyTables'])) { - throw new BadRequestError('Invalid tables structure provided.'); - } + public function importScheme(int $contextId, ContextScheme $scheme, string $userId): Context { + $tables = $scheme->getTables() ?? ['addTables' => [], 'modifyTables' => []]; + $nodes = $scheme->getNodes() ?? []; + // Grid views of an older scheme become pages, not nodes + $gridViewUuids = array_column($scheme->getGridViews(), 'uuid'); - $tableService = \OCP\Server::get(TableService::class); - $viewService = \OCP\Server::get(ViewService::class); - $columnService = \OCP\Server::get(ColumnService::class); + $tableService = $this->tableService(); + $viewService = $this->viewService(); + $columnService = $this->columnService(); foreach ($tables['addTables'] as $tableScheme) { $tableService->validateTableScheme($tableScheme); @@ -1250,7 +1251,13 @@ public function importScheme(int $contextId, string $name, string $iconName, str 'permissions' => $node['permissions'], ]; } elseif ($node['node_type'] === Application::NODE_TYPE_VIEW) { + if (in_array($node['node_uuid'], $gridViewUuids, true)) { + continue; + } $view = $this->viewMapper->findByUuid($node['node_uuid']); + if ($view === null) { + throw new DoesNotExistException('View ' . $node['node_uuid'] . ' does not exist'); + } $resolvedNodes[] = [ 'id' => $view->getId(), 'type' => $node['node_type'], @@ -1259,8 +1266,39 @@ public function importScheme(int $contextId, string $name, string $iconName, str } } - $context = $this->update($contextId, $userId, $name, $iconName, $description, $resolvedNodes); + $context = $this->update($contextId, $userId, $scheme->getName(), $scheme->getIcon() ?? '', $scheme->getDescription() ?? '', $resolvedNodes); - return $context; + // Pages come after the nodes their data widgets show, the menu after the pages it opens + $schemeService = $this->contextSchemeService(); + $pageIds = ['pages' => [], 'gridViews' => []]; + if ($scheme->getPages() !== null || $scheme->getGridViews() !== []) { + $pageIds = $schemeService->importPages($this->contextMapper->findById($contextId, $userId), $scheme, $userId); + } + if ($scheme->getMenuItems() !== null) { + $menuItems = $schemeService->resolveMenuItems($scheme->getMenuItems(), $userId, $pageIds['pages'], $pageIds['gridViews']); + $this->update($contextId, $userId, null, null, null, null, $menuItems); + } + + return $this->contextMapper->findById($contextId, $userId); + } + + /** + * The services the scheme export and import use. They come from the server container because they + * depend on this service themselves; a test can replace them here. + */ + protected function tableService(): TableService { + return \OCP\Server::get(TableService::class); + } + + protected function viewService(): ViewService { + return \OCP\Server::get(ViewService::class); + } + + protected function columnService(): ColumnService { + return \OCP\Server::get(ColumnService::class); + } + + protected function contextSchemeService(): ContextSchemeService { + return \OCP\Server::get(ContextSchemeService::class); } } diff --git a/openapi.json b/openapi.json index 520221afe2..778b243212 100644 --- a/openapi.json +++ b/openapi.json @@ -13832,6 +13832,75 @@ } } } + }, + "menuItems": { + "type": "array", + "nullable": true, + "default": null, + "description": "Menu of the context: tables and views by uuid, pages by their id in the scheme, parents by technical name; leave it out to keep the current menu", + "items": { + "type": "object", + "required": [ + "label", + "targetType" + ], + "properties": { + "label": { + "type": "string" + }, + "icon": { + "type": "string" + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url", + "page" + ] + }, + "targetUuid": { + "type": "string" + }, + "targetPageId": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + }, + "technicalName": { + "type": "string" + }, + "parentTechnicalName": { + "type": "string" + } + } + } + }, + "pages": { + "type": "array", + "nullable": true, + "default": null, + "description": "Pages of the context with their widgets (a scheme of an older version keys them by id); data widgets name their node by its id in the nodes; leave it out to keep the current pages", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } + }, + "gridViews": { + "type": "array", + "default": [], + "description": "Views without a table from a scheme of an older version; each becomes a page", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } } } } diff --git a/playwright/e2e/application.spec.ts b/playwright/e2e/application.spec.ts index ea5753c9f0..c952c4edc4 100644 --- a/playwright/e2e/application.spec.ts +++ b/playwright/e2e/application.spec.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { APIRequestContext } from '@playwright/test' +import type { APIRequestContext, Page } from '@playwright/test' import { test, expect } from '../support/fixtures' import { ocsRequest, type TestUser } from '../support/api' import { createContext, loadContext } from '../support/commands' @@ -204,4 +204,136 @@ test.describe('Applications', () => { await expect(page).toHaveURL(/\/menu\/reports$/) await expect(page.locator('[data-cy="gridPage"]')).toBeVisible() }) + + test('A scheme imported through the dialog brings its menu and its pages', async ({ userPage: { page, user } }) => { + const { contextId: sourceId } = await createApplicationWithMenu(page.request, user, 'exported application') + const scheme = await exportScheme(page.request, user, sourceId) + const targetId = await createEmptyApplication(page.request, user, 'importing application') + + const response = await importThroughDialog(page, targetId, JSON.stringify(scheme), true) + + expect(response.status()).toBe(200) + const target = await getApplication(page.request, user, targetId) + expect(target.menuItems.map((item: { label: string }) => item.label)).toEqual(['Dashboard', 'Front page', 'Cases', 'Handbook']) + const frontPageItem = target.menuItems.find((item: { label: string }) => item.label === 'Front page') + const frontPage = Object.values(target.pages as Record).find(entry => entry.title === 'Front page') + expect(frontPageItem.targetId).toBe(frontPage!.id) + expect(frontPageItem.broken).toBe(false) + + await page.goto(`/index.php/apps/tables/app/${targetId}`) + await page.locator('[data-cy="application-nav-item"]').filter({ hasText: 'Front page' }).getByRole('link').click() + await expect(page.getByText('Welcome on the front page')).toBeVisible() + }) + + test('A scheme imported with its menu and pages switched off keeps the current menu', async ({ userPage: { page, user } }) => { + const { contextId: sourceId } = await createApplicationWithMenu(page.request, user, 'exported application') + const scheme = await exportScheme(page.request, user, sourceId) + const targetId = await createEmptyApplication(page.request, user, 'importing application') + + const response = await importThroughDialog(page, targetId, JSON.stringify(scheme), false) + + expect(response.status()).toBe(200) + expect(response.request().postDataJSON()).not.toHaveProperty('menuItems') + expect(response.request().postDataJSON()).not.toHaveProperty('pages') + const target = await getApplication(page.request, user, targetId) + expect(target.menuItems.map((item: { label: string }) => item.label)).toEqual(['Dashboard']) + expect(Object.keys(target.pages)).toHaveLength(1) + }) + + test('A scheme of an older version turns its grid views into pages', async ({ userPage: { page, user } }) => { + const tableResponse = await ocsRequest(page.request, user, { method: 'POST', url: '/index.php/apps/tables/api/1/tables', data: { title: 'old scheme cases' } }) + const table = await tableResponse.json() + const targetId = await createEmptyApplication(page.request, user, 'old scheme application') + + const response = await ocsRequest(page.request, user, { + method: 'POST', + url: `${API}/contexts/${targetId}/scheme/import?format=json`, + data: { + name: 'old scheme application', + iconName: 'star', + description: '', + nodes: [], + tables: { addTables: [], modifyTables: [] }, + menuItems: [{ label: 'Intake home', targetType: 'view', targetUuid: 'aaaaaaaa-0000-4000-8000-000000000001', technicalName: 'intake_home' }], + gridViews: [{ + uuid: 'aaaaaaaa-0000-4000-8000-000000000001', + title: 'Intake home', + grid: { + widgets: [ + { id: 'w1', type: 'header', configuration: { title: '', showTitle: false }, content: { title: 'Welcome from an older scheme' } }, + { id: 'w2', type: 'data', configuration: { title: '', showTitle: false }, content: { target: { type: 'table', id: 1, uuid: table.uuid } } }, + ], + layout: [ + { widgetId: 'w1', gridX: 0, gridY: 0, gridWidth: 12, gridHeight: 2 }, + { widgetId: 'w2', gridX: 0, gridY: 2, gridWidth: 12, gridHeight: 5 }, + ], + }, + }], + }, + }) + + expect(response.status()).toBe(200) + const target = await getApplication(page.request, user, targetId) + const intake = Object.values(target.pages as Record }>) + .find(entry => entry.title === 'Intake home') + expect(intake!.page_type).toBe('grid') + expect(Object.values(intake!.content).map(entry => entry.type).sort()).toEqual(['data', 'header']) + expect(target.menuItems).toHaveLength(1) + expect(target.menuItems[0]).toMatchObject({ label: 'Intake home', targetType: 'page', targetId: intake!.id, broken: false }) + const nodeIds = Object.values(target.nodes as Record).map(node => node.node_id) + expect(nodeIds).toContain(table.id) + + await page.goto(`/index.php/apps/tables/app/${targetId}`) + await expect(page).toHaveURL(/\/menu\/intake_home$/) + await expect(page.getByText('Welcome from an older scheme')).toBeVisible() + await expect(page.locator('h1', { hasText: 'old scheme cases' })).toBeVisible() + }) }) + +async function getApplication(request: APIRequestContext, user: TestUser, contextId: number) { + const response = await ocsRequest(request, user, { method: 'GET', url: `${API}/contexts/${contextId}?format=json` }) + return (await response.json()).ocs.data +} + +async function exportScheme(request: APIRequestContext, user: TestUser, contextId: number) { + const response = await ocsRequest(request, user, { method: 'GET', url: `${API}/contexts/${contextId}/scheme/export?format=json` }) + return (await response.json()).ocs.data +} + +async function createEmptyApplication(request: APIRequestContext, user: TestUser, name: string): Promise { + const response = await ocsRequest(request, user, { method: 'POST', url: `${API}/contexts?format=json`, data: { name, iconName: 'star' } }) + return (await response.json()).ocs.data.id +} + +/** + * Imports a scheme file into an application through the "Import a scheme" action of its overview. + * + * @param page the page of the test user + * @param contextId the application to import into + * @param fileContent the scheme file + * @param withMenuAndPages whether the "Menu and pages" switch of the preview stays on + */ +async function importThroughDialog(page: Page, contextId: number, fileContent: string, withMenuAndPages: boolean) { + await page.goto(`/index.php/apps/tables/#/application/${contextId}`) + await page.locator('[data-cy="context-actions"]').getByRole('button').first().click() + await page.locator('[data-cy="context-import-scheme"]').click() + + const dialog = page.getByRole('dialog', { name: /application scheme/ }) + await expect(dialog).toBeVisible() + await dialog.locator('input[type=file]').setInputFiles({ + name: 'exported-application.json', + mimeType: 'application/json', + buffer: Buffer.from(fileContent), + }) + await dialog.getByRole('button', { name: 'Preview' }).click() + const menuSection = dialog.locator('[data-cy="importSchemeMenu"]') + await expect(menuSection).toBeVisible() + if (!withMenuAndPages) { + await menuSection.getByText('Menu and pages', { exact: true }).click() + } + const importResponse = page.waitForResponse(r => r.url().includes('/scheme/import') && r.request().method() === 'POST') + await dialog.getByRole('button', { name: 'Import' }).click() + const response = await importResponse + await expect(dialog).toContainText('Import completed successfully.') + return response +} diff --git a/src/modules/modals/ImportContextScheme.vue b/src/modules/modals/ImportContextScheme.vue index 57b695b996..ef9db3b61b 100644 --- a/src/modules/modals/ImportContextScheme.vue +++ b/src/modules/modals/ImportContextScheme.vue @@ -41,7 +41,12 @@
- +
@@ -145,6 +150,8 @@ export default { importFinished: false, preview: null, schemeData: null, + // The scheme as read from the file; the preview rebuilds schemeData from the parts chosen + uploadedScheme: null, waitForReload: false, mimeTypes: [ 'application/json', @@ -156,6 +163,18 @@ export default { computed: { ...mapState(useTablesStore, ['getContext', 'tables', 'views', 'activeContextId']), + /** + * The pages of the uploaded scheme as a list; a file from an older version keys them by id. + * + * @return {Array|null} + */ + uploadedPages() { + const pages = this.uploadedScheme?.pages + if (!pages || typeof pages !== 'object') { + return null + } + return Array.isArray(pages) ? pages : Object.values(pages) + }, importFileName() { const fileName = this.selectedUploadFile ? this.selectedUploadFile.name : '' @@ -241,8 +260,17 @@ export default { this.loading = false } }, + /** + * Takes the scheme as chosen in the preview. When it brings the pages, the grid views of a file + * from an older version come along, as each becomes a page. + * + * @param {object} newScheme the scheme the preview built + */ onUpdateScheme(newScheme) { - this.schemeData = newScheme + const gridViews = this.uploadedScheme?.gridViews + this.schemeData = newScheme.pages && Array.isArray(gridViews) && gridViews.length > 0 + ? { ...newScheme, gridViews } + : newScheme }, async actionImport() { if (!this.schemeData) { @@ -300,7 +328,8 @@ export default { reader.readAsText(this.selectedUploadFile, 'UTF-8') reader.onload = (evt) => { try { - this.schemeData = JSON.parse(evt.target.result) + this.uploadedScheme = JSON.parse(evt.target.result) + this.schemeData = this.uploadedScheme } catch (parseError) { this.importFailed = true this.errorMessage = t('tables', 'The selected file is not valid JSON.') diff --git a/src/modules/modals/ImportContextSchemePreview.vue b/src/modules/modals/ImportContextSchemePreview.vue index a7ac0f1911..fc7328183a 100644 --- a/src/modules/modals/ImportContextSchemePreview.vue +++ b/src/modules/modals/ImportContextSchemePreview.vue @@ -179,6 +179,44 @@ + + +
+
+ + {{ t('tables', 'Menu and pages') }} + +
+
    +
  • + {{ t('tables', 'From') }}: +
    • +
  • + {{ item.label }} +
    • +
+
    +
  • + {{ t('tables', 'To') }}: +
    • +
  • + {{ item.label }} +
    • +
+
    +
  • + {{ t('tables', 'Pages') }}: +
    • +
  • + {{ page.title || (page.page_type === 'startpage' ? t('tables', 'Dashboard') : '') }} +
    • +
+

+ {{ t('tables', 'Pages come along with the resources they show.') }} +

+
+
+
@@ -226,6 +264,21 @@ export default { } }, }, + // The menu of the uploaded scheme; null when the file has none, which keeps the current menu + menuItems: { + type: Array, + default: null, + }, + // The pages of the uploaded scheme; null when the file has none, which keeps the current pages + pages: { + type: Array, + default: null, + }, + // The menu the application has now + currentMenuItems: { + type: Array, + default: () => [], + }, }, emits: ['update'], @@ -236,6 +289,7 @@ export default { importIcon: true, importDescription: true, importNodes: true, + importMenu: true, addTablesConfig: [], removeColumnsConfig: [], modifyTablesConfig: [], @@ -268,6 +322,11 @@ export default { this.emitPayload() } }, + importMenu: { + handler() { + this.emitPayload() + }, + }, importNodes: { handler() { this.emitPayload() @@ -333,6 +392,14 @@ export default { }, nodes: this.importNodes ? this.previewData.nodes.to : this.previewData.nodes.from, } + // Left out, the import keeps the current menu and pages. Data widgets name their node by its + // id in the nodes of the file, so pages only come along with those nodes. + if (this.importMenu && this.menuItems) { + payload.menuItems = this.menuItems + } + if (this.importMenu && this.pages && this.importNodes) { + payload.pages = this.pages + } this.$emit('update', payload) }, diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 2c4f3ca00a..e68ab081c7 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -7703,6 +7703,36 @@ export interface operations { readonly addTables: readonly components["schemas"]["Table"][]; readonly modifyTables: readonly components["schemas"]["Table"][]; }; + /** + * @description Menu of the context: tables and views by uuid, pages by their id in the scheme, parents by technical name; leave it out to keep the current menu + * @default null + */ + readonly menuItems?: readonly { + readonly label: string; + readonly icon?: string; + /** @enum {string} */ + readonly targetType: "view" | "table" | "url" | "page"; + readonly targetUuid?: string; + /** Format: int64 */ + readonly targetPageId?: number; + readonly url?: string; + readonly technicalName?: string; + readonly parentTechnicalName?: string; + }[] | null; + /** + * @description Pages of the context with their widgets (a scheme of an older version keys them by id); data widgets name their node by its id in the nodes; leave it out to keep the current pages + * @default null + */ + readonly pages?: readonly { + readonly [key: string]: Record; + }[] | null; + /** + * @description Views without a table from a scheme of an older version; each becomes a page + * @default [] + */ + readonly gridViews?: readonly { + readonly [key: string]: Record; + }[]; }; }; }; diff --git a/tests/unit/Controller/ContextControllerTest.php b/tests/unit/Controller/ContextControllerTest.php index 2bd2f5cd65..0e98753a5b 100644 --- a/tests/unit/Controller/ContextControllerTest.php +++ b/tests/unit/Controller/ContextControllerTest.php @@ -11,6 +11,10 @@ use OCA\Tables\Controller\ContextController; use OCA\Tables\Db\Context; +use OCA\Tables\Errors\BadRequestError; +use OCA\Tables\Errors\InternalError; +use OCA\Tables\Errors\NotFoundError; +use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\MenuItemInput; use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; @@ -27,10 +31,14 @@ final class ContextControllerTest extends TestCase { private ContextService&MockObject $contextService; private ContextController $controller; + private IDBConnection&MockObject $db; + + private const TABLES = ['addTables' => [], 'modifyTables' => []]; protected function setUp(): void { parent::setUp(); $this->contextService = $this->createMock(ContextService::class); + $this->db = $this->createMock(IDBConnection::class); $this->controller = new ContextController( $this->createMock(IRequest::class), $this->createMock(LoggerInterface::class), @@ -38,7 +46,7 @@ protected function setUp(): void { 'user-1', $this->contextService, $this->createMock(TableService::class), - $this->createMock(IDBConnection::class), + $this->db, $this->createMock(ColumnService::class), $this->createMock(ViewService::class), ); @@ -61,6 +69,63 @@ public function testCreatePassesAPageItemWithoutAnIdOnAsTheNewStartPage(): void $this->assertSame(Http::STATUS_OK, $response->getStatus()); } + public function testImportOfAMalformedSchemeIsABadRequestThatNeverReachesTheService(): void { + $this->contextService->expects($this->never())->method('importScheme'); + + $response = $this->controller->importScheme(7, 'Intake', 'star', '', [], ['addTables' => []]); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + } + + public function testImportThatTheServiceRefusesAsABadRequestRollsBackWithA400(): void { + $this->contextService->method('importScheme')->willThrowException(new BadRequestError('Unknown widget type')); + $this->db->expects($this->once())->method('rollBack'); + $this->db->expects($this->never())->method('commit'); + + $response = $this->controller->importScheme(7, 'Intake', 'star', '', [], self::TABLES); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + } + + public function testImportThatFailsInternallyRollsBackWithA500(): void { + $this->contextService->method('importScheme')->willThrowException(new InternalError('insert failed')); + $this->db->expects($this->once())->method('rollBack'); + + $response = $this->controller->importScheme(7, 'Intake', 'star', '', [], self::TABLES); + + $this->assertSame(Http::STATUS_INTERNAL_SERVER_ERROR, $response->getStatus()); + } + + public function testImportOfAPageThatIsGoneRollsBackWithA404(): void { + $this->contextService->method('importScheme')->willThrowException(new NotFoundError('page not found')); + $this->db->expects($this->once())->method('rollBack'); + + $response = $this->controller->importScheme(7, 'Intake', 'star', '', [], self::TABLES); + + $this->assertSame(Http::STATUS_NOT_FOUND, $response->getStatus()); + } + + public function testImportPassesTheMenuPagesAndGridViewsOnAndLeavesAbsentOnesNull(): void { + $this->contextService->expects($this->exactly(2))->method('importScheme') + ->willReturnCallback(static function (int $contextId, ContextScheme $scheme): Context { + static $call = 0; + $call++; + if ($call === 1) { + TestCase::assertNull($scheme->getMenuItems()); + TestCase::assertNull($scheme->getPages()); + TestCase::assertSame([], $scheme->getGridViews()); + } else { + TestCase::assertSame([], $scheme->getMenuItems()); + TestCase::assertSame([['id' => 3, 'page_type' => 'grid', 'title' => 'Reports']], $scheme->getPages()); + TestCase::assertSame('grid-1', $scheme->getGridViews()[0]['uuid']); + } + return new Context(); + }); + + $this->controller->importScheme(7, 'Intake', 'star', '', [], self::TABLES); + $this->controller->importScheme(7, 'Intake', 'star', '', [], self::TABLES, [], [['id' => 3, 'page_type' => 'grid', 'title' => 'Reports']], [['uuid' => 'grid-1', 'title' => 'Old page']]); + } + public function testUpdateRefusesAPageItemWithoutAnId(): void { $this->contextService->expects($this->never())->method('update'); diff --git a/tests/unit/Model/ContextSchemeTest.php b/tests/unit/Model/ContextSchemeTest.php new file mode 100644 index 0000000000..a47ad203a1 --- /dev/null +++ b/tests/unit/Model/ContextSchemeTest.php @@ -0,0 +1,113 @@ + [], 'modifyTables' => []]; + + public function testCreateFromInputCarriesEveryPart(): void { + $scheme = ContextScheme::createFromInputArray([ + 'name' => 'Intake', 'icon' => 'inbox', 'description' => 'desc', + 'nodes' => [['node_type' => 0, 'node_uuid' => 'u1', 'permissions' => 1]], + 'tables' => self::TABLES, + 'menuItems' => [['label' => 'Home', 'targetType' => 'page', 'targetPageId' => 3]], + 'pages' => [['id' => 3, 'page_type' => 'grid', 'title' => 'Home', 'content' => []]], + 'gridViews' => [['uuid' => 'v1', 'title' => 'Old home']], + ]); + + $this->assertSame('Intake', $scheme->getName()); + $this->assertSame('inbox', $scheme->getIcon()); + $this->assertCount(1, $scheme->getNodes()); + $this->assertCount(1, $scheme->getMenuItems()); + $this->assertCount(1, $scheme->getPages()); + $this->assertCount(1, $scheme->getGridViews()); + $this->assertSame(self::TABLES, $scheme->getTables()); + } + + public function testCreateFromInputKeepsAnAbsentMenuOrPagesApartFromEmptyOnes(): void { + $without = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES]); + $empty = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'menuItems' => [], 'pages' => []]); + + $this->assertNull($without->getMenuItems()); + $this->assertNull($without->getPages()); + $this->assertSame([], $empty->getMenuItems()); + $this->assertSame([], $empty->getPages()); + $this->assertSame([], $without->getGridViews()); + } + + public function testCreateFromInputRejectsAPageThatIsNotAnObject(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Every page'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'pages' => ['home']]); + } + + public function testCreateFromInputRejectsPageContentThatIsNotAList(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Every page'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'pages' => [['page_type' => 'grid', 'content' => 'widgets']]]); + } + + /** + * @dataProvider provideInvalidGridViews + */ + public function testCreateFromInputRejectsAnInvalidGridView(array $gridView, string $message): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'gridViews' => [$gridView]]); + } + + /** + * @return array, string}> + */ + public static function provideInvalidGridViews(): array { + return [ + 'no uuid' => [['title' => 'Home'], '"uuid"'], + 'a blank title' => [['uuid' => 'v1', 'title' => ' '], '"title"'], + 'a grid that is not an object' => [['uuid' => 'v1', 'title' => 'Home', 'grid' => 'widgets'], '"grid" must be an object'], + ]; + } + + public function testCreateFromInputAcceptsAGridViewAsAnOlderSchemeExportedIt(): void { + $scheme = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'gridViews' => [[ + 'uuid' => 'v1', 'title' => 'Home', 'emoji' => '🏠', 'description' => null, 'type' => 'grid', + 'createdAt' => '2026-10-14 12:00:00', 'lastEditAt' => '2026-10-14 12:00:00', 'grid' => ['widgets' => [], 'layout' => []], + ]]]); + + $this->assertCount(1, $scheme->getGridViews()); + } + + public function testCreateFromInputRejectsTablesWithoutBothLists(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('addTables'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => ['addTables' => []]]); + } + + public function testCreateFromInputRejectsAMissingName(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('name'); + ContextScheme::createFromInputArray(['name' => ' ', 'tables' => self::TABLES]); + } + + public function testCreateFromInputRejectsANonListMenu(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('menuItems'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'menuItems' => 'home']); + } + + public function testCreateFromInputRejectsAnIconThatIsNotAString(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('"icon" must be a string'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'icon' => 7]); + } +} diff --git a/tests/unit/Service/ContextSchemeServiceTest.php b/tests/unit/Service/ContextSchemeServiceTest.php new file mode 100644 index 0000000000..6223a62e4e --- /dev/null +++ b/tests/unit/Service/ContextSchemeServiceTest.php @@ -0,0 +1,259 @@ +pageService = $this->createMock(PageService::class); + $this->tableMapper = $this->createMock(TableMapper::class); + $this->viewMapper = $this->createMock(ViewMapper::class); + $this->permissionsService = $this->createMock(PermissionsService::class); + $this->service = new ContextSchemeService( + $this->pageService, + $this->tableMapper, + $this->viewMapper, + $this->permissionsService, + $this->createMock(LoggerInterface::class), + ); + + // Tables and views exist here under the uuids "table-" and "view-" + $this->tableMapper->method('findByUuid')->willReturnCallback(static function (string $uuid): Table { + if (!str_starts_with($uuid, 'table-')) { + throw new DoesNotExistException('no table ' . $uuid); + } + $table = new Table(); + $table->setId((int)substr($uuid, 6)); + return $table; + }); + $this->viewMapper->method('findByUuid')->willReturnCallback(static function (string $uuid): ?View { + if (!str_starts_with($uuid, 'view-')) { + return null; + } + $view = new View(); + $view->setId((int)substr($uuid, 5)); + return $view; + }); + $this->tableMapper->method('find')->willReturnCallback(static function (int $id): Table { + if ($id === 404) { + throw new DoesNotExistException('gone'); + } + $table = new Table(); + $table->setId($id); + $table->setUuid('table-' . $id); + return $table; + }); + $this->viewMapper->method('find')->willReturnCallback(static function (int $id): View { + $view = new View(); + $view->setId($id); + $view->setUuid('view-' . $id); + return $view; + }); + } + + private function context(array $nodes = [], array $menuItems = []): Context { + $context = new Context(); + $context->id = 7; + $context->setNodes($nodes); + $context->setPages([]); + $context->setMenuItems($menuItems); + return $context; + } + + private function menuItem(int $id, string $technicalName, string $targetType, ?int $targetId, ?int $parentId = null, bool $broken = false): array { + return [ + 'id' => $id, 'contextId' => 7, 'label' => ucfirst($technicalName), 'icon' => null, 'targetType' => $targetType, 'targetId' => $targetId, + 'url' => $targetType === 'url' ? 'https://example.org' : null, 'technicalName' => $technicalName, 'order' => $id * 10, + 'parentId' => $parentId, 'broken' => $broken, + ]; + } + + /** + * @param list> $pages the pages the application has here + * @param array>> $saved collects the widgets saved per page + * @param list $created collects the pages created + */ + private function expectPages(array $pages, array &$saved, array &$created = []): void { + $this->pageService->method('findAll')->willReturn($pages); + $this->pageService->method('saveWidgets')->willReturnCallback(static function (int $contextId, int $pageId, array $widgets) use (&$saved): array { + $saved[$pageId] = $widgets; + return []; + }); + $this->pageService->method('create')->willReturnCallback(static function (int $contextId, string $title, ?string $technicalName) use (&$created): array { + $created[] = [$title, $technicalName]; + return ['id' => 100 + count($created)]; + }); + } + + private function startPage(int $id = 1): array { + return ['id' => $id, 'page_type' => 'startpage', 'title' => null, 'technical_name' => null, 'content' => []]; + } + + public function testExportNamesTablesAndViewsByUuidAndPagesByIdAndSkipsBrokenItems(): void { + $context = $this->context(menuItems: [ + $this->menuItem(1, 'dashboard', 'page', 3), + $this->menuItem(2, 'cases', 'table', 5, 1), + $this->menuItem(3, 'open', 'view', 6), + $this->menuItem(4, 'gone', 'page', 9, null, true), + $this->menuItem(5, 'lost', 'table', 404), + $this->menuItem(6, 'docs', 'url', null), + ]); + + $exported = $this->service->exportMenuItems($context); + + $this->assertSame(['dashboard', 'cases', 'open', 'docs'], array_column($exported, 'technicalName')); + $this->assertSame([3, null, null, null], array_column($exported, 'targetPageId')); + $this->assertSame([null, 'table-5', 'view-6', null], array_column($exported, 'targetUuid')); + $this->assertSame([null, 'dashboard', null, null], array_column($exported, 'parentTechnicalName')); + } + + public function testImportMapsPageTargetsAndTurnsAGridViewTargetIntoItsPage(): void { + $this->permissionsService->method('canManageNodeById')->willReturnCallback(static fn (int $type, int $id): bool => $id !== 8); + + $items = $this->service->resolveMenuItems([ + ['label' => 'Home', 'targetType' => 'page', 'targetPageId' => 3, 'technicalName' => 'home'], + ['label' => 'Old page', 'targetType' => 'view', 'targetUuid' => 'grid-1'], + ['label' => 'Cases', 'targetType' => 'table', 'targetUuid' => 'table-5', 'parentTechnicalName' => 'home'], + ['label' => 'Not mine', 'targetType' => 'table', 'targetUuid' => 'table-8'], + ['label' => 'Not here', 'targetType' => 'view', 'targetUuid' => 'missing'], + ['label' => 'Gone page', 'targetType' => 'page', 'targetPageId' => 4], + ['label' => 'Docs', 'targetType' => 'url', 'url' => 'https://example.org', 'parentTechnicalName' => 'skipped'], + ], self::USER, [3 => 30], ['grid-1' => 31]); + + $this->assertSame(['Home', 'Old page', 'Cases', 'Docs'], array_map(static fn ($item) => $item->getLabel(), $items)); + $this->assertSame(['page', 'page', 'table', 'url'], array_map(static fn ($item) => $item->getTargetType(), $items)); + $this->assertSame([30, 31, 5, null], array_map(static fn ($item) => $item->getTargetId(), $items)); + $this->assertSame([null, null, 0, null], array_map(static fn ($item) => $item->getParentIndex(), $items)); + } + + public function testImportGivesALaterItemWithATakenTechnicalNameADerivedOne(): void { + $items = $this->service->resolveMenuItems([ + ['label' => 'One', 'targetType' => 'url', 'url' => 'https://example.org/1', 'technicalName' => 'docs'], + ['label' => 'Two', 'targetType' => 'url', 'url' => 'https://example.org/2', 'technicalName' => 'docs'], + ], self::USER, [], []); + + $this->assertSame(['docs', null], array_map(static fn ($item) => $item->getTechnicalName(), $items)); + } + + public function testImportFillsTheStartPageOnlyWhenItWasArrangedOnTheGrid(): void { + $saved = []; + $this->expectPages([$this->startPage()], $saved); + $context = $this->context([10 => ['id' => 10, 'node_id' => 5, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 1]]); + $nodes = [['id' => 70, 'node_type' => Application::NODE_TYPE_TABLE, 'node_uuid' => 'table-5', 'permissions' => 1]]; + + $listed = $this->service->importPages($context, new ContextScheme('App', '', '', $nodes, [ + '2' => ['id' => 2, 'page_type' => 'startpage', 'content' => ['11' => ['id' => 11, 'order' => 10, 'node_rel_id' => 70]]], + ]), self::USER); + $this->assertSame([], $saved, 'a start page that only lists its tables keeps what the nodes gave it'); + $this->assertSame([2 => 1], $listed['pages']); + + $this->service->importPages($context, new ContextScheme('App', '', '', $nodes, [[ + 'id' => 2, 'page_type' => 'startpage', 'content' => [ + ['type' => 'header', 'configuration' => ['title' => '', 'showTitle' => false], 'content' => ['title' => 'Welcome'], 'grid_x' => 0, 'grid_y' => 0, 'grid_width' => 12, 'grid_height' => 2, 'node_rel_id' => null], + ['type' => 'data', 'configuration' => [], 'content' => [], 'grid_x' => 0, 'grid_y' => 2, 'grid_width' => 6, 'grid_height' => 5, 'node_rel_id' => 70], + ], + ]]), self::USER); + $this->assertSame(['header', 'data'], array_column($saved[1], 'type')); + $this->assertSame('Welcome', $saved[1][0]['content']['title']); + $this->assertSame([0, 2], array_column($saved[1], 'gridY')); + $this->assertSame(['nodeType' => Application::NODE_TYPE_TABLE, 'nodeId' => 5], $saved[1][1]['node']); + } + + public function testImportReplacesAPageWithTheSameTechnicalNameAndAddsTheOthers(): void { + $saved = []; + $created = []; + $this->expectPages([$this->startPage(), ['id' => 4, 'page_type' => 'grid', 'title' => 'Old', 'technical_name' => 'reports', 'content' => []]], $saved, $created); + $this->pageService->expects($this->once())->method('update')->with(7, 4, 'Reports', null, self::USER); + + $result = $this->service->importPages($this->context(), new ContextScheme('App', '', '', [], [ + ['id' => 20, 'page_type' => 'grid', 'title' => 'Reports', 'technical_name' => 'reports', 'content' => []], + ['id' => 21, 'page_type' => 'grid', 'title' => 'Intake', 'technical_name' => 'intake', 'content' => [ + ['type' => 'text', 'configuration' => [], 'content' => ['text' => 'Hello'], 'grid_x' => 0, 'grid_y' => 0, 'grid_width' => 6, 'grid_height' => 2], + ]], + ['id' => 22, 'page_type' => 'grid', 'title' => '', 'technical_name' => 'Not valid', 'content' => []], + ]), self::USER); + + $this->assertSame([20 => 4, 21 => 101], $result['pages']); + $this->assertSame([['Intake', 'intake']], $created); + $this->assertSame([], $saved[4]); + $this->assertSame('Hello', $saved[101][0]['content']['text']); + } + + public function testImportLeavesOutDataWidgetsWhoseNodeIsNotHereOrCannotBeAdded(): void { + $saved = []; + $this->expectPages([$this->startPage()], $saved); + $this->permissionsService->method('canManageNodeById')->willReturnCallback(static fn (int $type, int $id): bool => $id === 6); + $nodes = [ + ['id' => 70, 'node_type' => Application::NODE_TYPE_TABLE, 'node_uuid' => 'table-5', 'permissions' => 1], + ['id' => 71, 'node_type' => Application::NODE_TYPE_VIEW, 'node_uuid' => 'view-6', 'permissions' => 1], + ['id' => 72, 'node_type' => Application::NODE_TYPE_TABLE, 'node_uuid' => 'elsewhere', 'permissions' => 1], + ]; + $data = static fn (int $relId, int $y): array => ['type' => 'data', 'configuration' => [], 'content' => [], 'grid_x' => 0, 'grid_y' => $y, 'grid_width' => 12, 'grid_height' => 5, 'node_rel_id' => $relId]; + + $this->service->importPages($this->context(), new ContextScheme('App', '', '', $nodes, [ + ['id' => 20, 'page_type' => 'grid', 'title' => 'Data', 'content' => [$data(70, 0), $data(71, 5), $data(72, 10), $data(99, 15)]], + ]), self::USER); + + $this->assertSame([['nodeType' => Application::NODE_TYPE_VIEW, 'nodeId' => 6]], array_column($saved[101], 'node')); + } + + public function testImportTurnsAGridViewOfAnOlderSchemeIntoAPage(): void { + $saved = []; + $created = []; + $this->expectPages([$this->startPage()], $saved, $created); + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $context = $this->context([10 => ['id' => 10, 'node_id' => 5, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 1]]); + + $result = $this->service->importPages($context, new ContextScheme('App', '', '', [], null, [], null, [[ + 'uuid' => 'grid-1', + 'title' => 'Intake home', + 'grid' => [ + 'widgets' => [ + ['id' => 'w1', 'type' => 'header', 'configuration' => ['showTitle' => false], 'content' => ['title' => 'Welcome']], + ['id' => 'w2', 'type' => 'data', 'configuration' => [], 'content' => ['target' => ['type' => 'table', 'id' => 99, 'uuid' => 'table-5']]], + ['id' => 'w3', 'type' => 'data', 'configuration' => [], 'content' => ['target' => ['type' => 'view', 'id' => 98, 'uuid' => 'not-here']]], + ['id' => 'w4', 'type' => 'text', 'configuration' => [], 'content' => ['text' => 'Not placed']], + ], + 'layout' => [ + ['widgetId' => 'w1', 'gridX' => 0, 'gridY' => 0, 'gridWidth' => 12, 'gridHeight' => 2], + ['widgetId' => 'w2', 'gridX' => 0, 'gridY' => 2, 'gridWidth' => 8, 'gridHeight' => 5], + ['widgetId' => 'w3', 'gridX' => 8, 'gridY' => 2, 'gridWidth' => 4, 'gridHeight' => 5], + ], + ], + ]]), self::USER); + + $this->assertSame(['grid-1' => 101], $result['gridViews']); + $this->assertSame([['Intake home', null]], $created); + $this->assertSame(['header', 'data'], array_column($saved[101], 'type')); + $this->assertSame(['nodeType' => Application::NODE_TYPE_TABLE, 'nodeId' => 5], $saved[101][1]['node']); + $this->assertSame([], $saved[101][1]['content']); + $this->assertSame([8, 5], [$saved[101][1]['gridWidth'], $saved[101][1]['gridHeight']]); + } +} diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 22ab7c2344..4803727e22 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -27,6 +27,7 @@ use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\MenuItemInput; use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; @@ -48,6 +49,7 @@ final class ContextServiceTest extends TestCase { private MockObject $tableMapper; private MockObject $viewMapper; private MockObject $permissionsService; + private array $constructorArgs; protected function setUp(): void { parent::setUp(); @@ -70,7 +72,7 @@ protected function setUp(): void { $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnCallback(static fn (string $text): string => 'translated ' . $text); - $this->service = new ContextService( + $this->constructorArgs = [ $this->contextMapper, $this->contextNodeRelMapper, $this->pageMapper, @@ -88,7 +90,106 @@ protected function setUp(): void { $this->viewMapper, $this->menuItemMapper, $l10n, - ); + ]; + $this->service = new ContextService(...$this->constructorArgs); + } + + /** + * A service whose updates, scheme service and table services are mocked, to follow an import. + * + * @param list $calls collects the steps of the import in order + */ + private function importingService(ContextSchemeService&MockObject $schemeService, array &$calls): ContextService&MockObject { + $service = $this->getMockBuilder(ContextService::class) + ->setConstructorArgs($this->constructorArgs) + ->onlyMethods(['update', 'contextSchemeService', 'tableService', 'viewService', 'columnService']) + ->getMock(); + $service->method('contextSchemeService')->willReturn($schemeService); + $service->method('tableService')->willReturn($this->createMock(TableService::class)); + $service->method('viewService')->willReturn($this->createMock(ViewService::class)); + $service->method('columnService')->willReturn($this->createMock(ColumnService::class)); + $service->method('update')->willReturnCallback(function (int $contextId, string $userId, ?string $name, ?string $icon, ?string $description, ?array $nodes, ?array $menuItems = null) use (&$calls): Context { + $calls[] = $nodes !== null ? 'nodes:' . json_encode(array_column($nodes, 'id')) : 'menu:' . count($menuItems ?? []); + return $this->contextWithMenu($contextId, []); + }); + $this->contextMapper->method('findById')->willReturnCallback(fn (int $id): Context => $this->contextWithMenu($id, [])); + return $service; + } + + private function scheme(array $nodes, ?array $pages, ?array $menuItems, array $gridViews = []): ContextScheme { + return new ContextScheme('Intake', 'star', '', $nodes, $pages, ['addTables' => [], 'modifyTables' => []], $menuItems, $gridViews); + } + + public function testImportSetsTheNodesThenThePagesThenTheMenu(): void { + $calls = []; + $schemeService = $this->createMock(ContextSchemeService::class); + $schemeService->expects($this->once())->method('importPages')->willReturnCallback(static function () use (&$calls): array { + $calls[] = 'pages'; + return ['pages' => [3 => 30], 'gridViews' => ['grid-1' => 31]]; + }); + $schemeService->expects($this->once())->method('resolveMenuItems') + ->with([['label' => 'Home']], 'user-1', [3 => 30], ['grid-1' => 31]) + ->willReturn([MenuItemInput::createFromInputArray(['label' => 'Home', 'targetType' => 'page', 'targetId' => 30])]); + $table = new Table(); + $table->setId(5); + $this->tableMapper->method('findByUuid')->with('table-uuid')->willReturn($table); + $service = $this->importingService($schemeService, $calls); + + $service->importScheme(7, $this->scheme( + [ + ['id' => 70, 'node_type' => Application::NODE_TYPE_TABLE, 'node_uuid' => 'table-uuid', 'permissions' => 1], + ['id' => 71, 'node_type' => Application::NODE_TYPE_VIEW, 'node_uuid' => 'grid-1', 'permissions' => 1], + ], + [['id' => 3, 'page_type' => 'startpage', 'content' => []]], + [['label' => 'Home']], + [['uuid' => 'grid-1', 'title' => 'Old page']], + ), 'user-1'); + + // The grid view node is no node any more: it became a page + $this->assertSame(['nodes:[5]', 'pages', 'menu:1'], $calls); + } + + public function testImportOfASchemeWithoutPagesOrMenuLeavesBothAlone(): void { + $calls = []; + $schemeService = $this->createMock(ContextSchemeService::class); + $schemeService->expects($this->never())->method('importPages'); + $schemeService->expects($this->never())->method('resolveMenuItems'); + $service = $this->importingService($schemeService, $calls); + + $service->importScheme(7, $this->scheme([], null, null), 'user-1'); + + $this->assertSame(['nodes:[]'], $calls); + } + + public function testImportOfAnEmptyMenuClearsTheMenu(): void { + $calls = []; + $schemeService = $this->createMock(ContextSchemeService::class); + $schemeService->method('resolveMenuItems')->with([])->willReturn([]); + $service = $this->importingService($schemeService, $calls); + + $service->importScheme(7, $this->scheme([], null, []), 'user-1'); + + $this->assertSame(['nodes:[]', 'menu:0'], $calls); + } + + public function testExportCarriesThePagesAndTheMenu(): void { + $context = $this->contextWithMenu(7, []); + $this->contextMapper->method('findById')->willReturn($context); + $schemeService = $this->createMock(ContextSchemeService::class); + $schemeService->method('exportPages')->with(7, 'user-1')->willReturn([['id' => 3, 'page_type' => 'startpage', 'content' => []]]); + $schemeService->method('exportMenuItems')->with($context)->willReturn([['label' => 'Home', 'targetType' => 'page', 'targetPageId' => 3]]); + $service = $this->getMockBuilder(ContextService::class) + ->setConstructorArgs($this->constructorArgs) + ->onlyMethods(['contextSchemeService', 'tableService', 'viewService']) + ->getMock(); + $service->method('contextSchemeService')->willReturn($schemeService); + $service->method('tableService')->willReturn($this->createMock(TableService::class)); + $service->method('viewService')->willReturn($this->createMock(ViewService::class)); + + $scheme = $service->getScheme(7, 'user-1')->jsonSerialize(); + + $this->assertSame([['id' => 3, 'page_type' => 'startpage', 'content' => []]], $scheme['pages']); + $this->assertSame([['label' => 'Home', 'targetType' => 'page', 'targetPageId' => 3]], $scheme['menuItems']); } private function contextWithMenu(int $id, array $menuItems): Context { From 15471f4e97a55c00f52bf4615b2a448afcc1beea Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 15 Oct 2026 23:21:31 +0200 Subject: [PATCH 2/2] feat(applications): match imported pages only by technical name Every page and grid view of an imported scheme gets a technical name: its own, else one made from its title, unique within the scheme and apart from the start page. A page with that name is replaced, so importing a scheme again adds no page twice. Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/ContextSchemeService.php | 85 +++++++++++++------ playwright/e2e/application.spec.ts | 21 +++++ .../unit/Service/ContextSchemeServiceTest.php | 46 +++++++++- 3 files changed, 123 insertions(+), 29 deletions(-) diff --git a/lib/Service/ContextSchemeService.php b/lib/Service/ContextSchemeService.php index c6fb8ce850..dfab488542 100644 --- a/lib/Service/ContextSchemeService.php +++ b/lib/Service/ContextSchemeService.php @@ -20,6 +20,7 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; use OCA\Tables\Helper\PageContentHelper; +use OCA\Tables\Helper\TechnicalNameHelper; use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\MenuItemInput; use OCP\AppFramework\Db\DoesNotExistException; @@ -96,10 +97,11 @@ public function exportMenuItems(Context $context): array { /** * Brings the pages of a scheme into an application whose nodes the import already set. The * start page of the scheme fills the start page here, but only when it was arranged on the - * grid; otherwise the start page keeps the content the nodes gave it. A grid page replaces the - * page with the same technical name, or is added. Every grid view of an older scheme becomes a - * grid page. A data widget whose table or view does not exist here, or that the importer cannot - * add to the application, is left out. + * grid; otherwise the start page keeps the content the nodes gave it. Every grid page, and every + * grid view of an older scheme, gets a technical name: its own, else one made from its title, + * unique within the scheme and apart from the start page here. It replaces the page with that + * technical name, or is added. A data widget whose table or view does not exist here, or that + * the importer cannot add to the application, is left out. * * @return array{pages: array, gridViews: array} the local page ids by page id in the scheme, and by grid view uuid * @throws BadRequestError|NotFoundError|PermissionError|Exception @@ -114,14 +116,17 @@ public function importPages(Context $context, ContextScheme $scheme, string $use $localPages = $this->pageService->findAll($contextId, $userId); $startPageId = null; + $reserved = []; $pagesByTechnicalName = []; foreach ($localPages as $page) { if ($page['page_type'] === Page::TYPE_STARTPAGE) { $startPageId = $page['id']; - } elseif ($page['technical_name'] !== null) { + $reserved[$page['technical_name']] = true; + } else { $pagesByTechnicalName[$page['technical_name']] = $page['id']; } } + $technicalNames = $this->schemeTechnicalNames($scheme, $reserved); $pageIds = []; foreach ($scheme->getPages() ?? [] as $key => $page) { @@ -142,31 +147,15 @@ public function importPages(Context $context, ContextScheme $scheme, string $use continue; } - $technicalName = $page['technical_name'] ?? $page['technicalName'] ?? null; - $technicalName = is_string($technicalName) && Page::isValidTechnicalName($technicalName) ? $technicalName : null; - $title = $this->pageTitle($page['title'] ?? null, $technicalName); - if ($title === null) { - $this->logger->info('Page skipped on import, it has no title'); - continue; - } - $pageId = $technicalName !== null ? ($pagesByTechnicalName[$technicalName] ?? null) : null; - if ($pageId === null) { - $pageId = $this->pageService->create($contextId, $title, $technicalName, $userId)['id']; - if ($technicalName !== null) { - $pagesByTechnicalName[$technicalName] = $pageId; - } - } else { - $this->pageService->update($contextId, $pageId, $title, null, $userId); - } + $pageId = $this->importPage($contextId, $page['title'] ?? null, $technicalNames['page:' . $key], $pagesByTechnicalName, $userId); $pageIds[$sourceId] = $pageId; $widgets = $this->widgetsFromContent($content, $schemeNodes, $nodeKeys, $userId); $this->pageService->saveWidgets($contextId, $pageId, $widgets, $userId); } $gridViewPageIds = []; - foreach ($scheme->getGridViews() as $gridView) { - $title = $this->pageTitle($gridView['title'], null); - $pageId = $this->pageService->create($contextId, (string)$title, null, $userId)['id']; + foreach ($scheme->getGridViews() as $key => $gridView) { + $pageId = $this->importPage($contextId, $gridView['title'], $technicalNames['gridView:' . $key], $pagesByTechnicalName, $userId); $widgets = $this->widgetsFromGridView($gridView['grid'] ?? null, $nodeKeys, $userId); $this->pageService->saveWidgets($contextId, $pageId, $widgets, $userId); $gridViewPageIds[$gridView['uuid']] = $pageId; @@ -443,7 +432,7 @@ private function localId(string $targetType, string $uuid): ?int { /** * A page title from a scheme, falling back to the technical name; null when there is neither. */ - private function pageTitle(mixed $title, ?string $technicalName): ?string { + private function pageTitle(mixed $title, string $technicalName): string { $title = is_string($title) ? trim($title) : ''; if ($title === '') { return $technicalName; @@ -451,6 +440,52 @@ private function pageTitle(mixed $title, ?string $technicalName): ?string { return mb_substr($title, 0, self::PAGE_TITLE_MAX_LENGTH); } + /** + * The local page with the technical name, renamed to the title of the scheme, or a new page. + * + * @param array $pagesByTechnicalName local page ids by technical name; a new page is added + * @throws BadRequestError|NotFoundError|PermissionError|Exception + */ + private function importPage(int $contextId, mixed $title, string $technicalName, array &$pagesByTechnicalName, string $userId): int { + $title = $this->pageTitle($title, $technicalName); + if (isset($pagesByTechnicalName[$technicalName])) { + $this->pageService->update($contextId, $pagesByTechnicalName[$technicalName], $title, null, $userId); + return $pagesByTechnicalName[$technicalName]; + } + $pageId = $this->pageService->create($contextId, $title, $technicalName, $userId)['id']; + $pagesByTechnicalName[$technicalName] = $pageId; + return $pageId; + } + + /** + * The technical name of every grid page and grid view of a scheme, keyed `page:` and + * `gridView:`. + * + * @param array $reserved names the pages of the scheme cannot get + * @return array + */ + private function schemeTechnicalNames(ContextScheme $scheme, array $reserved): array { + $pages = []; + foreach ($scheme->getPages() ?? [] as $key => $page) { + if (!is_array($page) || ($page['page_type'] ?? $page['pageType'] ?? null) === Page::TYPE_STARTPAGE) { + continue; + } + $technicalName = $page['technical_name'] ?? $page['technicalName'] ?? null; + $pages['page:' . $key] = [ + 'name' => is_string($technicalName) ? $technicalName : null, + 'fallback' => TechnicalNameHelper::pageFallback(false, is_string($page['title'] ?? null) ? $page['title'] : null), + ]; + } + foreach ($scheme->getGridViews() as $key => $gridView) { + $technicalName = $gridView['technicalName'] ?? $gridView['technical_name'] ?? null; + $pages['gridView:' . $key] = [ + 'name' => is_string($technicalName) ? $technicalName : null, + 'fallback' => TechnicalNameHelper::pageFallback(false, $gridView['title']), + ]; + } + return TechnicalNameHelper::assign($pages, $reserved); + } + private function nodeKey(int $nodeType, int $nodeId): string { return sprintf('t%di%d', $nodeType, $nodeId); } diff --git a/playwright/e2e/application.spec.ts b/playwright/e2e/application.spec.ts index c952c4edc4..ccc4bed234 100644 --- a/playwright/e2e/application.spec.ts +++ b/playwright/e2e/application.spec.ts @@ -225,6 +225,26 @@ test.describe('Applications', () => { await expect(page.getByText('Welcome on the front page')).toBeVisible() }) + test('A scheme imported twice matches its pages by technical name and adds none again', async ({ userPage: { page, user } }) => { + const { contextId: sourceId } = await createApplicationWithMenu(page.request, user, 'exported application') + const scheme = await exportScheme(page.request, user, sourceId) + const targetId = await createEmptyApplication(page.request, user, 'importing application') + + for (let run = 0; run < 2; run++) { + const response = await ocsRequest(page.request, user, { + method: 'POST', + url: `${API}/contexts/${targetId}/scheme/import?format=json`, + data: { name: 'importing application', iconName: 'star', description: '', nodes: [], tables: { addTables: [], modifyTables: [] }, menuItems: scheme.menuItems, pages: scheme.pages }, + }) + expect(response.status()).toBe(200) + } + + const target = await getApplication(page.request, user, targetId) + const names = Object.values(target.pages as Record).map(entry => entry.technical_name).sort() + const sourceNames = Object.values((await getApplication(page.request, user, sourceId)).pages as Record).map(entry => entry.technical_name).sort() + expect(names).toEqual(sourceNames) + }) + test('A scheme imported with its menu and pages switched off keeps the current menu', async ({ userPage: { page, user } }) => { const { contextId: sourceId } = await createApplicationWithMenu(page.request, user, 'exported application') const scheme = await exportScheme(page.request, user, sourceId) @@ -277,6 +297,7 @@ test.describe('Applications', () => { const intake = Object.values(target.pages as Record }>) .find(entry => entry.title === 'Intake home') expect(intake!.page_type).toBe('grid') + expect((intake as unknown as { technical_name: string }).technical_name).toBe('intake_home') expect(Object.values(intake!.content).map(entry => entry.type).sort()).toEqual(['data', 'header']) expect(target.menuItems).toHaveLength(1) expect(target.menuItems[0]).toMatchObject({ label: 'Intake home', targetType: 'page', targetId: intake!.id, broken: false }) diff --git a/tests/unit/Service/ContextSchemeServiceTest.php b/tests/unit/Service/ContextSchemeServiceTest.php index 6223a62e4e..2e9a76f092 100644 --- a/tests/unit/Service/ContextSchemeServiceTest.php +++ b/tests/unit/Service/ContextSchemeServiceTest.php @@ -113,7 +113,7 @@ private function expectPages(array $pages, array &$saved, array &$created = []): } private function startPage(int $id = 1): array { - return ['id' => $id, 'page_type' => 'startpage', 'title' => null, 'technical_name' => null, 'content' => []]; + return ['id' => $id, 'page_type' => 'startpage', 'title' => null, 'technical_name' => 'dashboard', 'content' => []]; } public function testExportNamesTablesAndViewsByUuidAndPagesByIdAndSkipsBrokenItems(): void { @@ -200,12 +200,50 @@ public function testImportReplacesAPageWithTheSameTechnicalNameAndAddsTheOthers( ['id' => 22, 'page_type' => 'grid', 'title' => '', 'technical_name' => 'Not valid', 'content' => []], ]), self::USER); - $this->assertSame([20 => 4, 21 => 101], $result['pages']); - $this->assertSame([['Intake', 'intake']], $created); + $this->assertSame([20 => 4, 21 => 101, 22 => 102], $result['pages']); + $this->assertSame([['Intake', 'intake'], ['not_valid', 'not_valid']], $created); $this->assertSame([], $saved[4]); $this->assertSame('Hello', $saved[101][0]['content']['text']); } + public function testImportNamesPagesWithoutTechnicalNameAndMatchesThemByThatName(): void { + $saved = []; + $created = []; + $this->expectPages([$this->startPage(), ['id' => 4, 'page_type' => 'grid', 'title' => 'Old', 'technical_name' => 'open_cases', 'content' => []]], $saved, $created); + $this->pageService->expects($this->once())->method('update')->with(7, 4, 'Open cases', null, self::USER); + + $result = $this->service->importPages($this->context(), new ContextScheme('App', '', '', [], [ + ['id' => 20, 'page_type' => 'grid', 'title' => 'Open cases', 'content' => []], + ['id' => 21, 'page_type' => 'grid', 'title' => 'Open cases', 'content' => []], + ['id' => 22, 'page_type' => 'grid', 'title' => 'Board', 'technical_name' => 'dashboard', 'content' => []], + ['id' => 23, 'page_type' => 'grid', 'title' => 'Reports', 'technical_name' => 'reports', 'content' => []], + ['id' => 24, 'page_type' => 'grid', 'title' => 'Reports again', 'technical_name' => 'reports', 'content' => []], + ]), self::USER); + + $this->assertSame([20 => 4, 21 => 101, 22 => 102, 23 => 103, 24 => 104], $result['pages']); + $this->assertSame([ + ['Open cases', 'open_cases_2'], + ['Board', 'dashboard_2'], + ['Reports', 'reports'], + ['Reports again', 'reports_2'], + ], $created); + } + + public function testImportMatchesAGridViewOfAnOlderSchemeByItsTechnicalName(): void { + $saved = []; + $created = []; + $this->expectPages([$this->startPage(), ['id' => 4, 'page_type' => 'grid', 'title' => 'Intake', 'technical_name' => 'intake', 'content' => []]], $saved, $created); + $this->pageService->expects($this->once())->method('update')->with(7, 4, 'Intake home', null, self::USER); + + $result = $this->service->importPages($this->context(), new ContextScheme('App', '', '', [], null, [], null, [ + ['uuid' => 'grid-1', 'title' => 'Intake home', 'technicalName' => 'intake'], + ['uuid' => 'grid-2', 'title' => 'Archive'], + ]), self::USER); + + $this->assertSame(['grid-1' => 4, 'grid-2' => 101], $result['gridViews']); + $this->assertSame([['Archive', 'archive']], $created); + } + public function testImportLeavesOutDataWidgetsWhoseNodeIsNotHereOrCannotBeAdded(): void { $saved = []; $this->expectPages([$this->startPage()], $saved); @@ -250,7 +288,7 @@ public function testImportTurnsAGridViewOfAnOlderSchemeIntoAPage(): void { ]]), self::USER); $this->assertSame(['grid-1' => 101], $result['gridViews']); - $this->assertSame([['Intake home', null]], $created); + $this->assertSame([['Intake home', 'intake_home']], $created); $this->assertSame(['header', 'data'], array_column($saved[101], 'type')); $this->assertSame(['nodeType' => Application::NODE_TYPE_TABLE, 'nodeId' => 5], $saved[101][1]['node']); $this->assertSame([], $saved[101][1]['content']);