From b7a274642ff256278269324be3fbcbf636dcdb4c Mon Sep 17 00:00:00 2001 From: Remko Date: Mon, 12 Oct 2026 13:23:10 +0200 Subject: [PATCH 1/2] feat(contexts): add a menu to applications An application (context) gets a menu of items that each open a view, a table or a link. Items are loaded with the context in one chunked query, and an item whose view or table no longer exists is reported as broken, computed in SQL. Deleting an application deletes its menu. - a menu item's technical name is unique within its application, kept so by a unique index on (context_id, technical_name) - the item target type is typed as view, table or url in the API - the mapper groups the joined context rows in one place instead of three Co-authored-by: Ruben van der Linde Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko --- lib/Db/Context.php | 8 +- lib/Db/ContextMapper.php | 85 +++++++----------- lib/Db/MenuItem.php | 83 +++++++++++++++++ lib/Db/MenuItemMapper.php | 89 +++++++++++++++++++ .../Version2400Date20261012020000.php | 50 +++++++++++ lib/ResponseDefinitions.php | 14 +++ lib/Service/ContextService.php | 3 + openapi.json | 69 +++++++++++++- src/types/openapi/openapi.ts | 18 ++++ tests/unit/Db/MenuItemTest.php | 66 ++++++++++++++ tests/unit/Service/ContextServiceTest.php | 15 ++++ 11 files changed, 443 insertions(+), 57 deletions(-) create mode 100644 lib/Db/MenuItem.php create mode 100644 lib/Db/MenuItemMapper.php create mode 100644 lib/Migration/Version2400Date20261012020000.php create mode 100644 tests/unit/Db/MenuItemTest.php diff --git a/lib/Db/Context.php b/lib/Db/Context.php index 3fa14f56b5..333cbfda3e 100644 --- a/lib/Db/Context.php +++ b/lib/Db/Context.php @@ -28,6 +28,8 @@ * @method setNodes(array $value): void * @method getPages(): array * @method setPages(array $value): void + * @method getMenuItems(): array + * @method setMenuItems(array $value): void */ class Context extends EntitySuper implements JsonSerializable { protected ?string $name = null; @@ -40,8 +42,9 @@ class Context extends EntitySuper implements JsonSerializable { protected ?array $sharing = null; protected ?array $nodes = null; protected ?array $pages = null; + protected ?array $menuItems = null; - protected const VIRTUAL_PROPERTIES = ['sharing', 'nodes', 'pages']; + protected const VIRTUAL_PROPERTIES = ['sharing', 'nodes', 'pages', 'menuItems']; public function __construct() { $this->addType('id', 'integer'); @@ -56,7 +59,8 @@ public function jsonSerialize(): array { 'iconName' => $this->getIcon(), 'description' => $this->getDescription(), 'owner' => $this->getOwnerId(), - 'ownerType' => $this->getOwnerType() + 'ownerType' => $this->getOwnerType(), + 'menuItems' => array_values($this->menuItems ?? []), ]; // extended data diff --git a/lib/Db/ContextMapper.php b/lib/Db/ContextMapper.php index 0da199335e..d8e6d56ed8 100644 --- a/lib/Db/ContextMapper.php +++ b/lib/Db/ContextMapper.php @@ -27,6 +27,7 @@ public function __construct( IDBConnection $db, protected UserHelper $userHelper, protected GroupHelper $groupHelper, + protected MenuItemMapper $menuItemMapper, ) { parent::__construct($db, $this->table, Context::class); } @@ -76,7 +77,10 @@ protected function getFindContextBaseQuery(?string $userId): IQueryBuilder { return $qb; } - protected function formatResultRows(array $rows, ?string $userId) { + /** + * @param array> $menuItemsByContext + */ + protected function formatResultRows(array $rows, ?string $userId, array $menuItemsByContext = []) { $formatted = [ 'id' => $rows[0]['id'], 'name' => $rows[0]['name'], @@ -141,6 +145,11 @@ protected function formatResultRows(array $rows, ?string $userId) { return $carry; }, []); + $formatted['menu_items'] = array_map( + static fn (MenuItem $menuItem): array => $menuItem->jsonSerialize(), + $menuItemsByContext[(int)$rows[0]['id']] ?? [], + ); + return $this->mapRowToEntity($formatted); } @@ -154,26 +163,9 @@ public function findAll(?string $userId = null): array { $result = $qb->executeQuery(); $r = $result->fetchAllAssociative(); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; - } - $contextIds = array_keys($contextIds); - unset($row); - - $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ((int)$row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); - } - - return $resultEntities; + return $this->formatResultSet($r, $userId, true); } + public function findForNavBar(string $userId): array { $qb = $this->getFindContextBaseQuery($userId); $groupIDs = $this->userHelper->getGroupIdsForUser($userId); @@ -204,24 +196,27 @@ public function findForNavBar(string $userId): array { $result = $qb->executeQuery(); $r = $result->fetchAllAssociative(); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; + // the navigation bar shows the applications only, not their menus + return $this->formatResultSet($r, $userId, false); + } + + /** + * Groups the joined rows per context, loading the menus of all contexts in one query. + * + * @return Context[] + * @throws Exception + */ + private function formatResultSet(array $rows, ?string $userId, bool $withMenuItems): array { + $rowsByContext = []; + foreach ($rows as $row) { + $rowsByContext[(int)$row['id']][] = $row; } - $contextIds = array_keys($contextIds); - unset($row); + $menuItemsByContext = $withMenuItems ? $this->menuItemMapper->findByContextIds(array_keys($rowsByContext)) : []; $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ((int)$row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); + foreach ($rowsByContext as $contextRows) { + $resultEntities[] = $this->formatResultRows($contextRows, $userId, $menuItemsByContext); } - return $resultEntities; } @@ -240,7 +235,7 @@ public function findById(int $contextId, ?string $userId = null): Context { throw new NotFoundError('Context does not exist'); } - return $this->formatResultRows($r, $userId); + return $this->formatResultRows($r, $userId, $this->menuItemMapper->findByContextIds([$contextId])); } /** @@ -256,25 +251,7 @@ public function findAllContainingNode(int $nodeType, int $nodeId, string $userId $result = $qb->executeQuery(); $r = $result->fetchAllAssociative(); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; - } - $contextIds = array_keys($contextIds); - unset($row); - - $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ($row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); - } - - return $resultEntities; + return $this->formatResultSet($r, $userId, true); } /** diff --git a/lib/Db/MenuItem.php b/lib/Db/MenuItem.php new file mode 100644 index 0000000000..606cac32b0 --- /dev/null +++ b/lib/Db/MenuItem.php @@ -0,0 +1,83 @@ +addType('id', 'integer'); + $this->addType('contextId', 'integer'); + $this->addType('targetId', 'integer'); + $this->addType('order', 'integer'); + $this->addType('broken', 'boolean'); + } + + /** + * @return array{id: int, contextId: int, label: string, icon: string|null, targetType: 'view'|'table'|'url', targetId: int|null, url: string|null, technicalName: string|null, order: int, broken: bool} + */ + public function jsonSerialize(): array { + return [ + 'id' => $this->getId(), + 'contextId' => $this->getContextId(), + 'label' => $this->getLabel() ?? '', + 'icon' => $this->getIcon(), + 'targetType' => match ($this->targetType) { + self::TARGET_VIEW => self::TARGET_VIEW, + self::TARGET_TABLE => self::TARGET_TABLE, + default => self::TARGET_URL, + }, + 'targetId' => $this->getTargetId(), + 'url' => $this->getUrl(), + 'technicalName' => $this->getTechnicalName(), + 'order' => $this->getOrder() ?? 0, + 'broken' => $this->broken, + ]; + } +} diff --git a/lib/Db/MenuItemMapper.php b/lib/Db/MenuItemMapper.php new file mode 100644 index 0000000000..61c2d4ac2d --- /dev/null +++ b/lib/Db/MenuItemMapper.php @@ -0,0 +1,89 @@ + */ +class MenuItemMapper extends QBMapper { + private const DB_CHUNK_SIZE = 1_000; + + protected string $table = 'tables_contexts_menu_item'; + + public function __construct(IDBConnection $db) { + parent::__construct($db, $this->table, MenuItem::class); + } + + /** + * @return MenuItem[] ordered by their position in the menu + * @throws Exception + */ + public function findByContextId(int $contextId): array { + return $this->findByContextIds([$contextId])[$contextId] ?? []; + } + + /** + * The menu items of the given contexts, each list ordered by position. An item whose + * view or table no longer exists is marked broken. + * + * @param int[] $contextIds + * @return array> keyed by context id + * @throws Exception + */ + public function findByContextIds(array $contextIds): array { + if ($contextIds === []) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $brokenExpression = sprintf( + 'CASE WHEN (m.target_type = %s AND v.id IS NULL) OR (m.target_type = %s AND t.id IS NULL) THEN 1 ELSE 0 END', + $qb->createNamedParameter(MenuItem::TARGET_VIEW), + $qb->createNamedParameter(MenuItem::TARGET_TABLE), + ); + $qb->select('m.*') + ->selectAlias($qb->createFunction($brokenExpression), 'broken') + ->from($this->table, 'm') + ->leftJoin('m', 'tables_views', 'v', $qb->expr()->andX( + $qb->expr()->eq('v.id', 'm.target_id'), + $qb->expr()->eq('m.target_type', $qb->createNamedParameter(MenuItem::TARGET_VIEW)), + )) + ->leftJoin('m', 'tables_tables', 't', $qb->expr()->andX( + $qb->expr()->eq('t.id', 'm.target_id'), + $qb->expr()->eq('m.target_type', $qb->createNamedParameter(MenuItem::TARGET_TABLE)), + )) + ->where($qb->expr()->in('m.context_id', $qb->createParameter('contextIds'))) + ->orderBy('m.context_id', 'ASC') + ->addOrderBy('m.order', 'ASC') + ->addOrderBy('m.id', 'ASC'); + + $byContext = []; + foreach (array_chunk(array_values(array_unique($contextIds)), self::DB_CHUNK_SIZE) as $chunk) { + $qb->setParameter('contextIds', $chunk, IQueryBuilder::PARAM_INT_ARRAY); + foreach ($this->findEntities($qb) as $menuItem) { + $byContext[$menuItem->getContextId()][] = $menuItem; + } + } + return $byContext; + } + + /** + * @throws Exception + */ + public function deleteAllByContextId(int $contextId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->table) + ->where($qb->expr()->eq('context_id', $qb->createNamedParameter($contextId, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } +} diff --git a/lib/Migration/Version2400Date20261012020000.php b/lib/Migration/Version2400Date20261012020000.php new file mode 100644 index 0000000000..2e22707336 --- /dev/null +++ b/lib/Migration/Version2400Date20261012020000.php @@ -0,0 +1,50 @@ +hasTable('tables_contexts_menu_item')) { + $table = $schema->createTable('tables_contexts_menu_item'); + $table->addColumn('id', Types::INTEGER, ['autoincrement' => true, 'notnull' => true]); + $table->addColumn('context_id', Types::INTEGER, ['notnull' => true]); + $table->addColumn('label', Types::STRING, ['notnull' => true, 'length' => 200]); + $table->addColumn('icon', Types::STRING, ['notnull' => false, 'length' => 64]); + $table->addColumn('target_type', Types::STRING, ['notnull' => true, 'length' => 16]); + $table->addColumn('target_id', Types::INTEGER, ['notnull' => false]); + $table->addColumn('url', Types::STRING, ['notnull' => false, 'length' => 2000]); + $table->addColumn('technical_name', Types::STRING, ['notnull' => true, 'length' => 200]); + $table->addColumn('order', Types::INTEGER, ['notnull' => true, 'default' => 0]); + $table->setPrimaryKey(['id']); + } + + $table = $schema->getTable('tables_contexts_menu_item'); + if (!$table->hasIndex('tables_ctx_menu_name_uq')) { + $table->addUniqueIndex(['context_id', 'technical_name'], 'tables_ctx_menu_name_uq'); + } + + return $schema; + } +} diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index c935d72524..391bd93715 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -226,6 +226,19 @@ * errors_count: int, * } * + * @psalm-type TablesMenuItem = array{ + * id: int, + * contextId: int, + * label: string, + * icon: string|null, + * targetType: 'view'|'table'|'url', + * targetId: int|null, + * url: string|null, + * technicalName: string|null, + * order: int, + * broken: bool, + * } + * * @psalm-type TablesContext = array{ * id: int, * name: string, @@ -233,6 +246,7 @@ * description: string, * owner: string, * ownerType: int, + * menuItems: list, * } * * @psalm-type TablesContextNavigation = array{ diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index edfd7d9a6e..32d0ed0c28 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -14,6 +14,7 @@ use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; @@ -61,6 +62,7 @@ public function __construct( protected IURLGenerator $urlGenerator, private TableMapper $tableMapper, private ViewMapper $viewMapper, + private MenuItemMapper $menuItemMapper, ) { } @@ -316,6 +318,7 @@ public function delete(int $contextId, string $userId): Context { $this->atomic(function () use ($context): void { $this->shareService->deleteAllForContext($context); + $this->menuItemMapper->deleteAllByContextId($context->getId()); $this->contextNodeRelMapper->deleteAllByContextId($context->getId()); $pageIds = $this->pageMapper->getPageIdsForContext($context->getId()); foreach ($pageIds as $pageId) { diff --git a/openapi.json b/openapi.json index 80e327a7a7..a816a48ef5 100644 --- a/openapi.json +++ b/openapi.json @@ -279,7 +279,8 @@ "iconName", "description", "owner", - "ownerType" + "ownerType", + "menuItems" ], "properties": { "id": { @@ -301,6 +302,12 @@ "ownerType": { "type": "integer", "format": "int64" + }, + "menuItems": { + "type": "array", + "items": { + "$ref": "#/components/schemas/MenuItem" + } } } }, @@ -408,6 +415,66 @@ } } }, + "MenuItem": { + "type": "object", + "required": [ + "id", + "contextId", + "label", + "icon", + "targetType", + "targetId", + "url", + "technicalName", + "order", + "broken" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "contextId": { + "type": "integer", + "format": "int64" + }, + "label": { + "type": "string" + }, + "icon": { + "type": "string", + "nullable": true + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url" + ] + }, + "targetId": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "url": { + "type": "string", + "nullable": true + }, + "technicalName": { + "type": "string", + "nullable": true + }, + "order": { + "type": "integer", + "format": "int64" + }, + "broken": { + "type": "boolean" + } + } + }, "NotifyConfig": { "type": "object", "required": [ diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index e8d5e3a0c9..9558685b0a 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1152,6 +1152,7 @@ export type components = { readonly owner: string; /** Format: int64 */ readonly ownerType: number; + readonly menuItems: readonly components["schemas"]["MenuItem"][]; }; readonly ContextNavigation: { /** Format: int64 */ @@ -1186,6 +1187,23 @@ export type components = { readonly shareToken: string; readonly url: string; }; + readonly MenuItem: { + /** Format: int64 */ + readonly id: number; + /** Format: int64 */ + readonly contextId: number; + readonly label: string; + readonly icon: string | null; + /** @enum {string} */ + readonly targetType: "view" | "table" | "url"; + /** Format: int64 */ + readonly targetId: number | null; + readonly url: string | null; + readonly technicalName: string | null; + /** Format: int64 */ + readonly order: number; + readonly broken: boolean; + }; readonly NotifyConfig: { readonly "notify-assigned": boolean; readonly "notify-column": boolean; diff --git a/tests/unit/Db/MenuItemTest.php b/tests/unit/Db/MenuItemTest.php new file mode 100644 index 0000000000..d4db8ee4d2 --- /dev/null +++ b/tests/unit/Db/MenuItemTest.php @@ -0,0 +1,66 @@ +setContextId(3); + + $this->assertSame([ + 'id' => null, + 'contextId' => 3, + 'label' => '', + 'icon' => null, + 'targetType' => MenuItem::TARGET_URL, + 'targetId' => null, + 'url' => null, + 'technicalName' => null, + 'order' => 0, + 'broken' => false, + ], $item->jsonSerialize()); + } + + public function testJsonSerializeCarriesEveryField(): void { + $item = new MenuItem(); + $item->setId(9); + $item->setContextId(3); + $item->setLabel('Intake home'); + $item->setIcon('home'); + $item->setTargetType(MenuItem::TARGET_VIEW); + $item->setTargetId(6); + $item->setTechnicalName('home'); + $item->setOrder(20); + + $json = $item->jsonSerialize(); + + $this->assertSame(9, $json['id']); + $this->assertSame('Intake home', $json['label']); + $this->assertSame('view', $json['targetType']); + $this->assertSame(6, $json['targetId']); + $this->assertSame('home', $json['technicalName']); + $this->assertSame(20, $json['order']); + } + + public function testBrokenIsReportedButNeverStored(): void { + $item = new MenuItem(); + $item->setBroken(true); + + $this->assertTrue($item->jsonSerialize()['broken']); + $this->assertArrayNotHasKey('broken', $item->getUpdatedFields()); + } + + public function testTargetTypesAreTheThreeKnownOnes(): void { + $this->assertSame(['view', 'table', 'url'], MenuItem::TARGET_TYPES); + } +} diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 266827b7cd..9369e5f9e7 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -12,6 +12,7 @@ use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; @@ -33,6 +34,7 @@ final class ContextServiceTest extends TestCase { private MockObject $contextNodeRelMapper; private MockObject $pageMapper; private MockObject $pageContentMapper; + private MockObject $menuItemMapper; protected function setUp(): void { parent::setUp(); @@ -41,6 +43,7 @@ protected function setUp(): void { $this->contextNodeRelMapper = $this->createMock(ContextNodeRelationMapper::class); $this->pageMapper = $this->createMock(PageMapper::class); $this->pageContentMapper = $this->createMock(PageContentMapper::class); + $this->menuItemMapper = $this->createMock(MenuItemMapper::class); $logger = $this->createMock(LoggerInterface::class); $permissionsService = $this->createMock(PermissionsService::class); $userManager = $this->createMock(IUserManager::class); @@ -66,9 +69,21 @@ protected function setUp(): void { $urlGenerator, $this->createMock(TableMapper::class), $this->createMock(ViewMapper::class), + $this->menuItemMapper, ); } + public function testDeleteRemovesTheMenuOfTheApplication(): void { + $context = new Context(); + $context->id = 7; + $this->contextMapper->method('findById')->with(7, 'alice')->willReturn($context); + $this->pageMapper->method('getPageIdsForContext')->willReturn([]); + + $this->menuItemMapper->expects($this->once())->method('deleteAllByContextId')->with(7); + + $this->service->delete(7, 'alice'); + } + public function testUpdateReordersStartPageContentsFromSubmittedNodes(): void { $context = new Context(); $context->id = 7; From 49a30118b60aa17567fe1aec4e307d9c0e0b70d5 Mon Sep 17 00:00:00 2001 From: Remko Date: Tue, 13 Oct 2026 10:39:21 +0200 Subject: [PATCH 2/2] test(contexts): cover the menu SQL and require a technical name on menu items - MenuItemMapperTest runs against a real database: items pointing at a deleted view or table come back broken and links never do, items are ordered and grouped per application, and ContextMapper::findById() and findAll() carry the menu - MenuItemTest serializes each of the three target types - technicalName is a string in the entity and the API: the column is not null Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko --- lib/Db/MenuItem.php | 8 +- lib/ResponseDefinitions.php | 2 +- openapi.json | 3 +- src/types/openapi/openapi.ts | 2 +- tests/unit/Db/MenuItemMapperTest.php | 152 +++++++++++++++++++++++++++ tests/unit/Db/MenuItemTest.php | 20 +++- 6 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 tests/unit/Db/MenuItemMapperTest.php diff --git a/lib/Db/MenuItem.php b/lib/Db/MenuItem.php index 606cac32b0..aca2cafb62 100644 --- a/lib/Db/MenuItem.php +++ b/lib/Db/MenuItem.php @@ -24,8 +24,8 @@ * @method setTargetId(?int $targetId): void * @method getUrl(): ?string * @method setUrl(?string $url): void - * @method getTechnicalName(): ?string - * @method setTechnicalName(?string $technicalName): void + * @method getTechnicalName(): string + * @method setTechnicalName(string $technicalName): void * @method getOrder(): int * @method setOrder(int $order): void * @method isBroken(): bool @@ -60,7 +60,7 @@ public function __construct() { } /** - * @return array{id: int, contextId: int, label: string, icon: string|null, targetType: 'view'|'table'|'url', targetId: int|null, url: string|null, technicalName: string|null, order: int, broken: bool} + * @return array{id: int, contextId: int, label: string, icon: string|null, targetType: 'view'|'table'|'url', targetId: int|null, url: string|null, technicalName: string, order: int, broken: bool} */ public function jsonSerialize(): array { return [ @@ -75,7 +75,7 @@ public function jsonSerialize(): array { }, 'targetId' => $this->getTargetId(), 'url' => $this->getUrl(), - 'technicalName' => $this->getTechnicalName(), + 'technicalName' => $this->technicalName ?? '', 'order' => $this->getOrder() ?? 0, 'broken' => $this->broken, ]; diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 391bd93715..3f5c67a009 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -234,7 +234,7 @@ * targetType: 'view'|'table'|'url', * targetId: int|null, * url: string|null, - * technicalName: string|null, + * technicalName: string, * order: int, * broken: bool, * } diff --git a/openapi.json b/openapi.json index a816a48ef5..14caeecc44 100644 --- a/openapi.json +++ b/openapi.json @@ -463,8 +463,7 @@ "nullable": true }, "technicalName": { - "type": "string", - "nullable": true + "type": "string" }, "order": { "type": "integer", diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 9558685b0a..12fa95af55 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1199,7 +1199,7 @@ export type components = { /** Format: int64 */ readonly targetId: number | null; readonly url: string | null; - readonly technicalName: string | null; + readonly technicalName: string; /** Format: int64 */ readonly order: number; readonly broken: boolean; diff --git a/tests/unit/Db/MenuItemMapperTest.php b/tests/unit/Db/MenuItemMapperTest.php new file mode 100644 index 0000000000..1fd0f3c58e --- /dev/null +++ b/tests/unit/Db/MenuItemMapperTest.php @@ -0,0 +1,152 @@ +cleanupTablesData(); + $this->menuItemMapper = new MenuItemMapper($this->connectionAdapter); + $this->contextMapper = new ContextMapper( + $this->connectionAdapter, + $this->createMock(UserHelper::class), + $this->createMock(GroupHelper::class), + $this->menuItemMapper, + ); + } + + public function testItemsWhoseViewOrTableIsGoneAreBroken(): void { + $contextId = $this->createContext('Intake'); + $tableId = $this->createTestTable()['id']; + $viewId = $this->createView($tableId); + $this->createMenuItem($contextId, 'live_view', MenuItem::TARGET_VIEW, $viewId); + $this->createMenuItem($contextId, 'gone_view', MenuItem::TARGET_VIEW, $viewId + 100); + $this->createMenuItem($contextId, 'live_table', MenuItem::TARGET_TABLE, $tableId); + $this->createMenuItem($contextId, 'gone_table', MenuItem::TARGET_TABLE, $tableId + 100); + // A link points at neither, so its target id never makes it broken + $this->createMenuItem($contextId, 'link', MenuItem::TARGET_URL, $viewId + 100); + + $broken = []; + foreach ($this->menuItemMapper->findByContextId($contextId) as $menuItem) { + $broken[$menuItem->getTechnicalName()] = $menuItem->isBroken(); + } + + $this->assertSame([ + 'live_view' => false, + 'gone_view' => true, + 'live_table' => false, + 'gone_table' => true, + 'link' => false, + ], $broken); + } + + public function testItemsAreOrderedAndGroupedPerApplication(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'second', MenuItem::TARGET_URL, null, 20); + $this->createMenuItem($reportsId, 'overview', MenuItem::TARGET_URL, null, 0); + $this->createMenuItem($intakeId, 'first', MenuItem::TARGET_URL, null, 10); + + $menus = $this->menuItemMapper->findByContextIds([$intakeId, $reportsId]); + + $this->assertSame([ + $intakeId => ['first', 'second'], + $reportsId => ['overview'], + ], array_map(self::technicalNames(...), $menus)); + $this->assertSame([], $this->menuItemMapper->findByContextIds([])); + } + + public function testFindByIdIncludesTheMenu(): void { + $contextId = $this->createContext('Intake'); + $this->createMenuItem($contextId, 'home', MenuItem::TARGET_URL, null); + + $menuItems = $this->contextMapper->findById($contextId)->getMenuItems(); + + $this->assertSame(['home'], array_column($menuItems, 'technicalName')); + } + + public function testFindAllIncludesTheMenus(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'home', MenuItem::TARGET_URL, null); + $this->createMenuItem($reportsId, 'overview', MenuItem::TARGET_URL, null); + + $menus = []; + foreach ($this->contextMapper->findAll() as $context) { + $menus[$context->getId()] = array_column($context->getMenuItems(), 'technicalName'); + } + ksort($menus); + + $this->assertSame([$intakeId => ['home'], $reportsId => ['overview']], $menus); + } + + /** + * @param list $menuItems + * @return list + */ + private static function technicalNames(array $menuItems): array { + return array_map(static fn (MenuItem $menuItem): string => $menuItem->getTechnicalName(), $menuItems); + } + + private function createContext(string $name): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_contexts_context') + ->values([ + 'name' => $qb->createNamedParameter($name), + 'icon' => $qb->createNamedParameter('star'), + 'description' => $qb->createNamedParameter(''), + 'owner_id' => $qb->createNamedParameter('user1'), + 'owner_type' => $qb->createNamedParameter(0), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function createView(int $tableId): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_views') + ->values([ + 'table_id' => $qb->createNamedParameter($tableId), + 'title' => $qb->createNamedParameter('Open requests'), + 'technical_name' => $qb->createNamedParameter('open_requests'), + 'description' => $qb->createNamedParameter(''), + 'created_by' => $qb->createNamedParameter('user1'), + 'created_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + 'last_edit_by' => $qb->createNamedParameter('user1'), + 'last_edit_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function createMenuItem(int $contextId, string $technicalName, string $targetType, ?int $targetId, int $order = 0): void { + $menuItem = new MenuItem(); + $menuItem->setContextId($contextId); + $menuItem->setLabel($technicalName); + $menuItem->setTargetType($targetType); + $menuItem->setTargetId($targetId); + $menuItem->setTechnicalName($technicalName); + $menuItem->setOrder($order); + $this->menuItemMapper->insert($menuItem); + } +} diff --git a/tests/unit/Db/MenuItemTest.php b/tests/unit/Db/MenuItemTest.php index d4db8ee4d2..e2da546fc9 100644 --- a/tests/unit/Db/MenuItemTest.php +++ b/tests/unit/Db/MenuItemTest.php @@ -25,7 +25,7 @@ public function testJsonSerializeUsesSafeDefaults(): void { 'targetType' => MenuItem::TARGET_URL, 'targetId' => null, 'url' => null, - 'technicalName' => null, + 'technicalName' => '', 'order' => 0, 'broken' => false, ], $item->jsonSerialize()); @@ -52,6 +52,24 @@ public function testJsonSerializeCarriesEveryField(): void { $this->assertSame(20, $json['order']); } + public static function targetTypeProvider(): array { + return [ + 'view' => [MenuItem::TARGET_VIEW], + 'table' => [MenuItem::TARGET_TABLE], + 'url' => [MenuItem::TARGET_URL], + ]; + } + + /** + * @dataProvider targetTypeProvider + */ + public function testJsonSerializeKeepsEachTargetType(string $targetType): void { + $item = new MenuItem(); + $item->setTargetType($targetType); + + $this->assertSame($targetType, $item->jsonSerialize()['targetType']); + } + public function testBrokenIsReportedButNeverStored(): void { $item = new MenuItem(); $item->setBroken(true);