From 3f9cb0abd7361750a4a683105650beee176cffaf Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Wed, 4 Nov 2026 07:32:32 +0100 Subject: [PATCH 1/2] feat(contexts): carry the menu through export and import The menu now travels with its application through a scheme export and import, and when a user's data is moved to another instance. An export writes each item with pages by technical name, tables and views by uuid, and the group of an item by the group's position in the exported menu. The title it shows is written along, for the import preview. An import replaces the menu: it finds pages by technical name and tables and views among the nodes of the application, so a menu never adds a table to it. Every item is checked like the API checks it, and an item that fails or whose target did not come along is skipped and logged with the reason, as is every item past the 200th. An item whose group did not come before it, or was skipped, moves to the top level. Moving a user's data keeps the links, the items for the pages and the items for the moved table. View items are skipped, as that import has no mapping from old view ids to new ones yet. The scheme preview shows the menu the import would save, next to the current one, and the items it would skip. The import screen has a Menu switch, like Resources: when it is off, the application keeps its menu. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ruben van der Linde --- .../ImportContextSchemePreview.cy.js | 81 ++++++++ lib/Controller/ContextController.php | 9 +- lib/Model/ContextScheme.php | 6 + lib/Service/ContextService.php | 76 +++++++- lib/Service/MenuService.php | 174 ++++++++++++++++++ lib/UserMigration/TablesMigrator.php | 10 +- .../modals/ImportContextSchemePreview.vue | 45 +++++ .../Service/ContextPagesRoundTripTest.php | 2 + tests/unit/Service/ContextServiceTest.php | 1 + tests/unit/Service/MenuServiceTest.php | 96 +++++++++- tests/unit/TablesMigratorTest.php | 12 +- 11 files changed, 503 insertions(+), 9 deletions(-) create mode 100644 cypress/component/ImportContextSchemePreview.cy.js diff --git a/cypress/component/ImportContextSchemePreview.cy.js b/cypress/component/ImportContextSchemePreview.cy.js new file mode 100644 index 0000000000..d834d00442 --- /dev/null +++ b/cypress/component/ImportContextSchemePreview.cy.js @@ -0,0 +1,81 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import ImportContextSchemePreview from '../../src/modules/modals/ImportContextSchemePreview.vue' + +const MENU_SWITCH = '[data-cy="importSchemeMenu"]' +const SKIPPED = '[data-cy="importSchemeMenuSkipped"]' + +const schemeMenu = [ + { parent: null, title: 'Reports', displayTitle: 'Reports', icon: null, description: null, target: null }, + { parent: 0, title: null, displayTitle: 'Docs', icon: null, description: null, target: { url: 'https://example.org' } }, +] + +describe('Import application scheme preview', () => { + it('sends the menu of the scheme with the import', () => { + mountPreview({ from: [{ displayTitle: 'Old item' }], to: schemeMenu, skipped: [] }) + .then(({ wrapper }) => { + cy.contains('Old item').should('exist') + cy.contains('Docs').should('exist') + cy.wrap(null).then(() => { + expect(lastPayload(wrapper).menu).to.deep.equal(schemeMenu) + }) + }) + }) + + it('leaves the menu out when the switch is turned off', () => { + mountPreview({ from: [], to: schemeMenu, skipped: [] }) + .then(({ wrapper }) => { + cy.get(MENU_SWITCH).click({ force: true }) + cy.wrap(null).then(() => { + expect(lastPayload(wrapper)).to.not.have.property('menu') + }) + }) + }) + + it('lists the items the import would skip', () => { + mountPreview({ from: [], to: [], skipped: [{ index: 0, title: 'Missing table', reason: 'Its table is not part of the application.' }] }) + + cy.get(SKIPPED).should('contain', 'Missing table') + }) + + it('has no menu section for a preview without a menu', () => { + mountPreview(undefined) + .then(({ wrapper }) => { + cy.get(MENU_SWITCH).should('not.exist') + cy.wrap(null).then(() => { + expect(lastPayload(wrapper)).to.not.have.property('menu') + }) + }) + }) +}) + +/** + * @param {object|undefined} menu the menu part of the preview response + * @return {Cypress.Chainable} the mount result + */ +function mountPreview(menu) { + return cy.mount(ImportContextSchemePreview, { + props: { + previewData: { + name: { from: 'Intake', to: 'Intake' }, + icon: { from: 'star', to: 'star' }, + description: { from: '', to: '' }, + nodes: { from: [], to: [] }, + addTables: {}, + modifyTables: {}, + ...(menu === undefined ? {} : { menu }), + }, + }, + }) +} + +/** + * @param {object} wrapper the mounted component + * @return {object} the last import body the preview emitted + */ +function lastPayload(wrapper) { + const emitted = wrapper.emitted('update') + return emitted[emitted.length - 1][0] +} diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index 434e2137bb..e7955ce01e 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -317,6 +317,10 @@ public function exportScheme(int $contextId): DataResponse { /** * [api v2] Preview the changes that would be applied to a context scheme * + * When the scheme carries a `menu`, the changes hold `menu` with the current menu under + * `from`, the items an import would save under `to` and the items it would skip, with + * the reason, under `skipped`. + * * @param int $contextId ID of the context * @psalm-param TablesContext $updateScheme New scheme of the context * @@ -355,6 +359,7 @@ 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 + * @param list>|null $menu Menu of the scheme, replacing the menu of the context: pages by technical name, tables and views by uuid, the group of an item by its position in the list; items that cannot be used here are skipped. Left out, the menu stays as it is * * @return DataResponse, array{}>|DataResponse * @@ -365,10 +370,10 @@ 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 $menu = null): DataResponse { try { $this->db->beginTransaction(); - $context = $this->contextService->importScheme($contextId, $name, $iconName, $description, $nodes, $tables, $this->userId); + $context = $this->contextService->importScheme($contextId, $name, $iconName, $description, $nodes, $tables, $this->userId, $menu); $this->db->commit(); return new DataResponse($context->jsonSerialize()); } catch (\InvalidArgumentException $e) { diff --git a/lib/Model/ContextScheme.php b/lib/Model/ContextScheme.php index 3c511632c4..10e9519e89 100644 --- a/lib/Model/ContextScheme.php +++ b/lib/Model/ContextScheme.php @@ -18,6 +18,7 @@ public function __construct( protected ?array $nodes = [], protected ?array $pages = [], protected ?array $tables = [], + protected ?array $menu = [], ) { } @@ -45,6 +46,10 @@ public function getTables(): ?array { return $this->tables; } + public function getMenu(): ?array { + return $this->menu; + } + public function jsonSerialize(): mixed { return [ 'name' => $this->name, @@ -53,6 +58,7 @@ public function jsonSerialize(): mixed { 'nodes' => $this->nodes, 'pages' => $this->pages, 'tables' => $this->tables, + 'menu' => $this->menu, ]; } } diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index f2e6c4f155..12ffa5e96a 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -69,6 +69,7 @@ public function __construct( private WidgetService $widgetService, private GridWidgetTypes $gridWidgetTypes, private MenuItemMapper $menuItemMapper, + private MenuService $menuService, ) { } @@ -655,15 +656,20 @@ 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. + * * @param Table $table * @param array $context * @param int $tableOldId + * @param string|null $tableOldUuid * * @return Context|null * * @throws \Exception */ - public function importContext(Table $table, array $context, int $tableOldId): ?Context { + 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'])) { @@ -758,9 +764,48 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C } } + if (is_array($context['menu'] ?? null)) { + $tableRelation = reset($sourceRelMap); + $this->logSkippedMenuItems($this->menuService->import( + $this->contextMapper->findById($newContext->getId()), + $context['menu'], + static fn (string $kind, string $uuid): ?int => $kind === 'table' && $uuid === $tableOldUuid && $tableRelation !== false ? $tableRelation->getId() : null, + )); + } + return $newContext; } + /** + * @param list $skipped + */ + private function logSkippedMenuItems(array $skipped): void { + foreach ($skipped as $item) { + $this->logger->warning('Skipping imported menu item ' . $item['index'] . ': ' . $item['reason']); + } + } + + /** + * The node relation of an application for an exported table or view, found by uuid. + */ + private function nodeRelIdByUuid(Context $context, string $kind, string $uuid): ?int { + try { + $node = $kind === 'table' ? $this->tableMapper->findByUuid($uuid) : $this->viewMapper->findByUuid($uuid); + } catch (DoesNotExistException) { + return null; + } + if ($node === null) { + return null; + } + $nodeType = $kind === 'table' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW; + foreach ($context->getNodes() ?? [] as $relId => $relation) { + if ($relation['node_type'] === $nodeType && $relation['node_id'] === $node->getId()) { + return (int)$relId; + } + } + return null; + } + /** * The technical names of the pages of an imported application, by page key. A valid * exported name is kept unless an earlier page of the import uses it or it has the @@ -926,6 +971,7 @@ public function getScheme(int $contextId, string $userId): ContextScheme { array_values($nodes), $this->getPagesWithWidgets($context), array_values($tables), + $this->menuService->export($context), ); } @@ -995,6 +1041,24 @@ public function compareSchemeChanges(int $contextId, array $updateScheme, string } } + if (is_array($updateScheme['menu'] ?? null)) { + $schemeUuids = []; + foreach ($changes['nodes']['to'] ?? [] as $node) { + $kind = ($node['node_type'] ?? null) === Application::NODE_TYPE_VIEW ? 'view' : 'table'; + $schemeUuids[$kind][(string)($node['node_uuid'] ?? '')] = true; + } + ['items' => $items, 'skipped' => $skipped] = $this->menuService->readExported( + $context, + $updateScheme['menu'], + static fn (string $kind, string $uuid): ?int => isset($schemeUuids[$kind][$uuid]) ? 0 : null, + ); + $changes['menu'] = [ + 'from' => $currentScheme->getMenu(), + 'to' => array_column($items, 'entry'), + 'skipped' => $skipped, + ]; + } + return $changes; } @@ -1019,7 +1083,7 @@ 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 { + public function importScheme(int $contextId, string $name, string $iconName, string $description, array $nodes, array $tables, string $userId, ?array $menu = null): 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'])) { @@ -1080,6 +1144,14 @@ public function importScheme(int $contextId, string $name, string $iconName, str } $context = $this->update($contextId, $userId, $name, $iconName, $description, $resolvedNodes); + if ($menu !== null) { + $withNodes = $this->contextMapper->findById($contextId, $userId); + $this->logSkippedMenuItems($this->menuService->import( + $withNodes, + $menu, + fn (string $kind, string $uuid): ?int => $this->nodeRelIdByUuid($withNodes, $kind, $uuid), + )); + } return $context; } diff --git a/lib/Service/MenuService.php b/lib/Service/MenuService.php index 49a8fed24f..d3561dd9a2 100644 --- a/lib/Service/MenuService.php +++ b/lib/Service/MenuService.php @@ -34,6 +34,7 @@ * Reads need access to the application, writes need manage permissions on it. * * @psalm-import-type TablesMenuItem from ResponseDefinitions + * @psalm-type TablesMenuExportItem = array{parent: ?int, title: ?string, displayTitle: string, icon: ?string, description: ?string, target: array{page: string}|array{table: string}|array{view: string}|array{url: string}|null} */ class MenuService { use TTransactional; @@ -227,6 +228,179 @@ public function saveOrder(int $contextId, array $order, string $userId): array { return $this->findAll($contextId, $userId); } + /** + * The menu as an export carries it: a page by its technical name, a table or view by + * uuid, and the group of an item by the group's position in the exported menu. Each + * item keeps the title it shows, for a preview; an import reads only its own title. + * + * @return list + * @throws Exception + */ + public function export(Context $context): array { + $menu = $this->serialize($context, $this->menuItemMapper->findByContextId($context->getId())); + $pages = $context->getPages() ?? []; + $nodes = $context->getNodes() ?? []; + $nodeIds = [Application::NODE_TYPE_TABLE => [], Application::NODE_TYPE_VIEW => []]; + foreach ($menu as $item) { + $node = isset($item['target']['nodeRelId']) ? $nodes[$item['target']['nodeRelId']] : null; + if ($node !== null) { + $nodeIds[$node['node_type']][] = $node['node_id']; + } + } + $nodesById = [ + Application::NODE_TYPE_TABLE => $this->tableMapper->findMany($nodeIds[Application::NODE_TYPE_TABLE]), + Application::NODE_TYPE_VIEW => $this->viewMapper->findMany($nodeIds[Application::NODE_TYPE_VIEW]), + ]; + $positions = []; + $exported = []; + foreach ($menu as $item) { + $target = $item['target']; + if (isset($target['pageId'])) { + $target = ['page' => (string)$pages[$target['pageId']]['technical_name']]; + } elseif (isset($target['nodeRelId'])) { + $node = $nodes[$target['nodeRelId']]; + $entity = $nodesById[$node['node_type']][$node['node_id']] ?? null; + if ($entity === null) { + continue; + } + $target = [$node['node_type'] === Application::NODE_TYPE_TABLE ? 'table' : 'view' => (string)$entity->getUuid()]; + } + $positions[$item['id']] = count($exported); + $exported[] = [ + 'parent' => $item['parentId'] === null ? null : ($positions[$item['parentId']] ?? null), + 'title' => $item['title'], + 'displayTitle' => $item['displayTitle'], + 'icon' => $item['icon'], + 'description' => $item['description'], + 'target' => $target, + ]; + } + return $exported; + } + + /** + * Replaces the menu of an application with an exported one, as {@see self::readExported()} + * reads it, and returns the items it skipped. Callers check permissions. + * + * @param list $exported + * @param \Closure(string, string): ?int $nodeRelIdOf + * @return list + * @throws Exception + */ + public function import(Context $context, array $exported, \Closure $nodeRelIdOf): array { + ['items' => $items, 'skipped' => $skipped] = $this->readExported($context, $exported, $nodeRelIdOf); + + $this->atomic(function () use ($context, $items): void { + $this->menuItemMapper->deleteByContextId($context->getId()); + $idsByIndex = []; + foreach ($items as $position => ['item' => $item, 'index' => $index, 'parent' => $parent]) { + $item->setOrder($position); + $item->setParentId($parent === null ? null : $idsByIndex[$parent]); + $idsByIndex[$index] = $this->menuItemMapper->insert($item)->getId(); + } + }, $this->dbc); + + return $skipped; + } + + /** + * Reads an exported menu for an application. Every item is checked like the API + * checks it, and skipped, with the reason, when it fails, when its target did not + * come along, or past the {@see MenuItem::MAX_ITEMS_PER_APPLICATION}th item. A page is + * found by its technical name, a table or view by `$nodeRelIdOf`, which gives the node + * relation of the application for the kind (`table` or `view`) and uuid, or null. + * An item sits in the group at the position it names when that group came before it + * and was kept; otherwise it moves to the top. + * + * @param list $exported + * @param \Closure(string, string): ?int $nodeRelIdOf + * @return array{items: list, index: int, parent: ?int}>, skipped: list} + */ + public function readExported(Context $context, array $exported, \Closure $nodeRelIdOf): array { + $pageIds = []; + foreach ($context->getPages() ?? [] as $pageId => $page) { + $pageIds[(string)$page['technical_name']] = (int)$pageId; + } + + $items = []; + $skipped = []; + $groups = []; + foreach (array_values($exported) as $index => $entry) { + try { + if (!is_array($entry)) { + throw new BadRequestError('It is not an object.'); + } + if (count($items) >= MenuItem::MAX_ITEMS_PER_APPLICATION) { + throw new BadRequestError('A menu holds at most ' . MenuItem::MAX_ITEMS_PER_APPLICATION . ' items.'); + } + $item = new MenuItem(); + $item->setContextId($context->getId()); + $item->setTitle($this->validateTitle(self::importedText($entry, 'title'))); + $item->setIcon($this->validateIcon(self::importedText($entry, 'icon'))); + $item->setDescription($this->validateDescription(self::importedText($entry, 'description'))); + if (($entry['target'] ?? null) !== null) { + $item->setTarget($this->importedTarget($entry['target'], $pageIds, $nodeRelIdOf)); + } + $this->assertValid($item); + } catch (BadRequestError $e) { + $skipped[] = ['index' => $index, 'title' => is_array($entry) ? self::importedTitle($entry) : null, 'reason' => $e->getMessage()]; + continue; + } + $parent = $entry['parent'] ?? null; + $parent = !$item->isGroup() && is_int($parent) && isset($groups[$parent]) ? $parent : null; + if ($item->isGroup()) { + $groups[$index] = true; + } + $items[] = ['item' => $item, 'entry' => $entry, 'index' => $index, 'parent' => $parent]; + } + return ['items' => $items, 'skipped' => $skipped]; + } + + /** + * @param \Closure(string, string): ?int $nodeRelIdOf + * @param array $pageIds the pages of the application by technical name + * @return array{pageId: int}|array{nodeRelId: int}|array{url: string} + * @throws BadRequestError + */ + private function importedTarget(mixed $target, array $pageIds, \Closure $nodeRelIdOf): array { + if (is_array($target) && count($target) === 1) { + $kind = (string)array_key_first($target); + $value = $target[$kind]; + if ($kind === 'url' && is_string($value)) { + return ['url' => $this->validateUrl($value)]; + } + if ($kind === 'page' && is_string($value)) { + return isset($pageIds[$value]) ? ['pageId' => $pageIds[$value]] : throw new BadRequestError('The application has no page ' . $value . '.'); + } + if (($kind === 'table' || $kind === 'view') && is_string($value)) { + return ['nodeRelId' => $nodeRelIdOf($kind, $value) ?? throw new BadRequestError('Its ' . $kind . ' ' . $value . ' is not part of the application.')]; + } + } + throw new BadRequestError('Its target is not {page}, {table}, {view} or {url}.'); + } + + /** + * A text field of an exported item, or null when it is left out. + * + * @param array $entry + * @throws BadRequestError + */ + private static function importedText(array $entry, string $field): ?string { + $value = $entry[$field] ?? null; + if ($value !== null && !is_string($value)) { + throw new BadRequestError('Its ' . $field . ' is not a text.'); + } + return $value; + } + + /** + * @param array $entry + */ + private static function importedTitle(array $entry): ?string { + $title = $entry['displayTitle'] ?? $entry['title'] ?? null; + return is_string($title) ? $title : null; + } + /** * @throws NotFoundError * @throws PermissionError diff --git a/lib/UserMigration/TablesMigrator.php b/lib/UserMigration/TablesMigrator.php index 29edfa77b2..89a1780ce0 100644 --- a/lib/UserMigration/TablesMigrator.php +++ b/lib/UserMigration/TablesMigrator.php @@ -10,6 +10,7 @@ namespace OCA\Tables\UserMigration; use OCA\Tables\Db\ColumnMapper; +use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelationMapper; use OCA\Tables\Db\RowCellDatetime; @@ -31,6 +32,7 @@ use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; use OCA\Tables\Service\FavoritesService; +use OCA\Tables\Service\MenuService; use OCA\Tables\Service\RowService; use OCA\Tables\Service\ShareService; use OCA\Tables\Service\TableService; @@ -84,6 +86,7 @@ public function __construct( protected RowService $rowService, private ContextService $contextService, private ShareService $shareService, + private MenuService $menuService, ) { } @@ -125,7 +128,10 @@ public function export( foreach ($contexts as $context) { $context->setPages($this->contextService->getPagesWithWidgets($context)); } - $exportDestination->addFileContents(self::FILE_CONTEXTS, json_encode($contexts)); + $exportDestination->addFileContents(self::FILE_CONTEXTS, json_encode(array_map( + fn (Context $context): array => $context->jsonSerialize() + ['menu' => $this->menuService->export($context)], + $contexts, + ))); $contextIds = array_map(fn ($c) => $c->getId(), $contexts); $shares = $this->shareMapper->findAllSharesForTablesAndContexts($tableIds, $contextIds); @@ -440,7 +446,7 @@ private function importRows(IImportSource $importSource, Table $newTable, array */ private function importContexts(array $contexts, Table $newTable, array $table, array &$contextIdMap): void { foreach ($contexts as $context) { - $newContext = $this->contextService->importContext($newTable, $context, $table['id']); + $newContext = $this->contextService->importContext($newTable, $context, $table['id'], $table['uuid'] ?? null); if ($newContext !== null) { $contextIdMap[$context['id']] = $newContext->getId(); } diff --git a/src/modules/modals/ImportContextSchemePreview.vue b/src/modules/modals/ImportContextSchemePreview.vue index a7ac0f1911..1706971114 100644 --- a/src/modules/modals/ImportContextSchemePreview.vue +++ b/src/modules/modals/ImportContextSchemePreview.vue @@ -179,6 +179,41 @@ + + +
+
+ + {{ t('tables', 'Menu') }} + +
+
    +
  • + {{ t('tables', 'From') }}: +
    • +
  • + {{ item.displayTitle }} +
    • +
+
    +
  • + {{ t('tables', 'To') }}: +
    • +
  • + {{ item.displayTitle ?? item.title }} +
    • +
+
+
    +
  • + {{ t('tables', 'Skipped, not available here') }}: +
    • +
  • + {{ item.title }} +
    • +
+
+
@@ -236,6 +271,7 @@ export default { importIcon: true, importDescription: true, importNodes: true, + importMenu: true, addTablesConfig: [], removeColumnsConfig: [], modifyTablesConfig: [], @@ -273,6 +309,11 @@ export default { this.emitPayload() } }, + importMenu: { + handler() { + this.emitPayload() + }, + }, addTablesConfig: { handler() { this.emitPayload() @@ -333,6 +374,10 @@ export default { }, nodes: this.importNodes ? this.previewData.nodes.to : this.previewData.nodes.from, } + // without a menu the import leaves the menu of the application as it is + if (this.importMenu && this.previewData.menu) { + payload.menu = this.previewData.menu.to + } this.$emit('update', payload) }, diff --git a/tests/unit/Service/ContextPagesRoundTripTest.php b/tests/unit/Service/ContextPagesRoundTripTest.php index 0765e28533..6548611618 100644 --- a/tests/unit/Service/ContextPagesRoundTripTest.php +++ b/tests/unit/Service/ContextPagesRoundTripTest.php @@ -27,6 +27,7 @@ use OCA\Tables\Helper\UserHelper; use OCA\Tables\Service\ContextService; use OCA\Tables\Service\GridWidgetTypes; +use OCA\Tables\Service\MenuService; use OCA\Tables\Service\PermissionsService; use OCA\Tables\Service\ShareService; use OCA\Tables\Service\Support\AuditLogServiceInterface; @@ -86,6 +87,7 @@ protected function setUp(): void { $this->widgetService, new GridWidgetTypes($l10n), $this->createMock(MenuItemMapper::class), + $this->createMock(MenuService::class), ); } diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index a18c3020da..ba5ae6d882 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -82,6 +82,7 @@ protected function setUp(): void { $this->widgetService, new GridWidgetTypes($l10n), $this->createMock(MenuItemMapper::class), + $this->createMock(MenuService::class), ); } diff --git a/tests/unit/Service/MenuServiceTest.php b/tests/unit/Service/MenuServiceTest.php index 32616ab89c..b3ab1cea99 100644 --- a/tests/unit/Service/MenuServiceTest.php +++ b/tests/unit/Service/MenuServiceTest.php @@ -17,6 +17,7 @@ use OCA\Tables\Db\Page; 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\Db\WidgetMapper; @@ -35,6 +36,7 @@ use OCA\Tables\Service\Support\AuditLogServiceInterface; use OCA\Tables\Service\WidgetService; use OCA\Tables\Tests\Unit\Database\DatabaseTestCase; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\IL10N; use OCP\INavigationManager; use OCP\IURLGenerator; @@ -48,6 +50,7 @@ class MenuServiceTest extends DatabaseTestCase { private const OWNER = 'menu-service-owner'; private const TABLE = 4151; + private const TABLE_UUID = 'a5d8e2c4-4151-4cde-9f00-000000004151'; private MenuService $menuService; private ContextService $contextService; @@ -74,6 +77,12 @@ protected function setUp(): void { $this->permissionsService->method('canManageContext')->willReturnCallback(fn (): bool => $this->canManage); $tableMapper = $this->createMock(TableMapper::class); $tableMapper->method('findIdToTitleMap')->willReturnCallback(static fn (array $ids): array => in_array(self::TABLE, $ids, true) ? [self::TABLE => 'Projects'] : []); + $table = new Table(); + $table->id = self::TABLE; + $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); $this->contextService = new ContextService( $contextMapper, @@ -94,9 +103,9 @@ protected function setUp(): void { $widgetService, new GridWidgetTypes($l10n), $this->menuItemMapper, + $this->menuService, ); $this->pageService = new PageService($contextMapper, $pageMapper, $pageContentMapper, $widgetService, $this->permissionsService, new GridWidgetTypes($l10n), $this->connectionAdapter, $this->menuItemMapper); - $this->menuService = new MenuService($contextMapper, $this->menuItemMapper, $this->permissionsService, $tableMapper, $this->createMock(ViewMapper::class), $this->connectionAdapter); } protected function tearDown(): void { @@ -306,6 +315,91 @@ public function testOnlyAManagerChangesTheMenu(\Closure $change): void { $change($this->menuService, $contextId, $itemId); } + public function testAMenuGoesThroughASchemeExportAndImport(): void { + [$contextId, , $pageId, $nodeRelId] = $this->createApplication(); + $group = $this->menuService->create($contextId, 'Links', 'star', 'Elsewhere', null, self::OWNER); + $site = $this->menuService->create($contextId, null, null, null, ['url' => 'https://nextcloud.com'], self::OWNER); + $this->menuService->create($contextId, null, null, null, ['nodeRelId' => $nodeRelId], self::OWNER); + $this->menuService->create($contextId, null, null, null, ['pageId' => $pageId], self::OWNER); + $menu = $this->menuService->findAll($contextId, self::OWNER); + $this->menuService->saveOrder($contextId, array_map(static fn (array $item): array => ['id' => $item['id'], 'parentId' => $item['id'] === $site['id'] ? $group['id'] : null], $menu), self::OWNER); + $exported = $this->menuService->export($this->contextService->findById($contextId, self::OWNER)); + + $imported = $this->contextService->create('Imported', 'tables', '', [['id' => self::TABLE, 'type' => Application::NODE_TYPE_TABLE]], self::OWNER, Application::OWNER_TYPE_USER); + $this->contextIds[] = $imported->getId(); + $this->contextService->importScheme($imported->getId(), 'Imported', 'tables', '', [['node_type' => Application::NODE_TYPE_TABLE, 'node_uuid' => self::TABLE_UUID, 'permissions' => 1]], ['addTables' => [], 'modifyTables' => []], self::OWNER, $exported); + + $shape = static fn (array $items): array => array_map(static fn (array $item): array => [$item['displayTitle'], $item['parent'], $item['target'], $item['icon']], $items); + $expected = $shape($exported); + unset($expected[4]); + $expected[0][0] = 'Imported'; + $this->assertSame(array_values($expected), $shape($this->menuService->export($this->contextService->findById($imported->getId(), self::OWNER)))); + $this->assertSame(['dashboard', 1], [$exported[0]['target']['page'], $exported[2]['parent']]); + } + + public function testAMovedUsersMenuKeepsItsTableItsPagesAndItsLinks(): void { + [$contextId, , $pageId, $nodeRelId] = $this->createApplication(); + $this->menuService->create($contextId, 'Projects', null, null, ['nodeRelId' => $nodeRelId], self::OWNER); + $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); + $this->contextIds[] = $moved->getId(); + + $menu = $this->menuService->findAll($moved->getId(), self::OWNER); + $movedContext = $this->contextService->findById($moved->getId(), self::OWNER); + $this->assertSame(['Sales', 'Projects', 'https://nextcloud.com'], array_column($menu, 'displayTitle')); + $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 static function unusableItems(): array { + $link = ['title' => 'Site', 'target' => ['url' => 'https://nextcloud.com']]; + return [ + 'not an object' => [['Reports'], 'It is not an object.'], + 'a page that did not come along' => [[['target' => ['page' => 'missing']]], 'The application has no page missing.'], + 'a table that did not come along' => [[['target' => ['table' => 'f0000000-0000-4000-8000-000000000000']]], 'is not part of the application'], + 'an unknown kind of target' => [[['title' => 'Row', 'target' => ['row' => 'x']]], 'Its target is not'], + 'a link that is not http' => [[['target' => ['url' => 'ftp://nextcloud.com']]], 'A link is an http or https address'], + 'a group without a title' => [[['target' => null]], 'A group needs a title.'], + 'a title that is not a text' => [[['title' => ['Site'], 'target' => ['url' => 'https://nextcloud.com']]], 'Its title is not a text.'], + 'two targets' => [[['target' => ['url' => 'https://nextcloud.com', 'page' => 'dashboard']]], 'Its target is not'], + 'one item past the limit' => [array_fill(0, MenuItem::MAX_ITEMS_PER_APPLICATION + 1, $link), 'A menu holds at most 200 items.'], + ]; + } + + /** + * @dataProvider unusableItems + */ + public function testAnImportSkipsTheItemsItCannotUseWithTheReason(array $exported, string $reason): void { + [$contextId] = $this->createApplication(); + + $skipped = $this->menuService->import($this->contextService->findById($contextId, self::OWNER), $exported, static fn (): ?int => null); + + $this->assertCount(1, $skipped); + $this->assertStringContainsString($reason, $skipped[0]['reason']); + $this->assertCount(count($exported) - 1, $this->menuService->findAll($contextId, self::OWNER)); + } + + public function testAnImportedItemMovesToTheTopWhenItsGroupDidNotComeFirst(): void { + [$contextId] = $this->createApplication(); + $exported = [ + ['title' => 'Site', 'parent' => 1, 'target' => ['url' => 'https://nextcloud.com']], + ['title' => 'Links', 'target' => null], + ['title' => 'Docs', 'parent' => 1, 'target' => ['url' => 'https://docs.nextcloud.com']], + ['title' => 'Help', 'parent' => 0, 'target' => ['url' => 'https://help.nextcloud.com']], + ]; + + $this->menuService->import($this->contextService->findById($contextId, self::OWNER), $exported, static fn (): ?int => null); + + $menu = $this->menuService->findAll($contextId, self::OWNER); + $this->assertSame([null, null, $menu[1]['id'], null], array_column($menu, 'parentId')); + } + /** * An application "Sales" with its start page, a grid page "Overview" and a table. * diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 7de0be2617..5786cb3ce0 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -26,6 +26,7 @@ use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; use OCA\Tables\Service\FavoritesService; +use OCA\Tables\Service\MenuService; use OCA\Tables\Service\RowService; use OCA\Tables\Service\ShareService; use OCA\Tables\Service\TableService; @@ -60,6 +61,7 @@ class TablesMigratorTest extends TestCase { private $rowService; private $contextService; private $shareService; + private $menuService; protected function setUp(): void { $this->l10n = $this->createMock(IL10N::class); @@ -83,6 +85,7 @@ protected function setUp(): void { $this->contextService = $this->createMock(ContextService::class); $this->shareService = $this->createMock(ShareService::class); + $this->menuService = $this->createMock(MenuService::class); $this->migrator = new TablesMigrator( $this->l10n, $this->tableMapper, @@ -103,7 +106,8 @@ protected function setUp(): void { $this->columnService, $this->rowService, $this->contextService, - $this->shareService + $this->shareService, + $this->menuService, ); } @@ -150,7 +154,7 @@ public function testExportSuccess(): void { $this->assertTrue(true); } - public function testExportWritesTheWidgetsOfEveryContextPage(): void { + public function testExportWritesTheWidgetsAndTheMenuOfEveryContext(): void { $user = $this->createMock(IUser::class); $exportDestination = $this->createMock(IExportDestination::class); $user->method('getUID')->willReturn('user1'); @@ -169,6 +173,9 @@ public function testExportWritesTheWidgetsOfEveryContextPage(): void { $this->contextService->method('getPagesWithWidgets')->with($context) ->willReturn([5 => ['id' => 5, 'page_type' => 'grid', 'content' => [], 'widgets' => [$widget]]]); + $menu = [['parent' => null, 'title' => 'Site', 'displayTitle' => 'Site', 'icon' => null, 'description' => null, 'target' => ['url' => 'https://nextcloud.com']]]; + $this->menuService->method('export')->with($context)->willReturn($menu); + $files = []; $exportDestination->method('addFileContents')->willReturnCallback(static function (string $path, string $content) use (&$files): void { $files[$path] = $content; @@ -177,6 +184,7 @@ public function testExportWritesTheWidgetsOfEveryContextPage(): void { $contexts = json_decode($files['contexts.json'], true); $this->assertSame([$widget], $contexts[0]['pages'][5]['widgets']); + $this->assertSame($menu, $contexts[0]['menu']); } public function testExportFailure(): void { From 6368c264a57cc1a47b085b148f0330dbd1d2c2f5 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Wed, 4 Nov 2026 07:32:32 +0100 Subject: [PATCH 2/2] chore(api): regenerate the OpenAPI spec and TypeScript types Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ruben van der Linde --- openapi.json | 13 +++++++++++++ src/types/openapi/openapi.ts | 12 +++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/openapi.json b/openapi.json index d7278645e7..3c043c7fea 100644 --- a/openapi.json +++ b/openapi.json @@ -13703,6 +13703,7 @@ "post": { "operationId": "context-preview-scheme-changes", "summary": "[api v2] Preview the changes that would be applied to a context scheme", + "description": "When the scheme carries a `menu`, the changes hold `menu` with the current menu under `from`, the items an import would save under `to` and the items it would skip, with the reason, under `skipped`.", "tags": [ "context" ], @@ -14059,6 +14060,18 @@ } } } + }, + "menu": { + "type": "array", + "nullable": true, + "default": null, + "description": "Menu of the scheme, replacing the menu of the context: pages by technical name, tables and views by uuid, the group of an item by its position in the list; items that cannot be used here are skipped. Left out, the menu stays as it is", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } } } } diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 8132ea5b8f..dc24155625 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -891,7 +891,10 @@ export type paths = { }; readonly get?: never; readonly put?: never; - /** [api v2] Preview the changes that would be applied to a context scheme */ + /** + * [api v2] Preview the changes that would be applied to a context scheme + * @description When the scheme carries a `menu`, the changes hold `menu` with the current menu under `from`, the items an import would save under `to` and the items it would skip, with the reason, under `skipped`. + */ readonly post: operations["context-preview-scheme-changes"]; readonly delete?: never; readonly options?: never; @@ -7921,6 +7924,13 @@ export interface operations { readonly addTables: readonly components["schemas"]["Table"][]; readonly modifyTables: readonly components["schemas"]["Table"][]; }; + /** + * @description Menu of the scheme, replacing the menu of the context: pages by technical name, tables and views by uuid, the group of an item by its position in the list; items that cannot be used here are skipped. Left out, the menu stays as it is + * @default null + */ + readonly menu?: readonly { + readonly [key: string]: Record; + }[] | null; }; }; };