feat(contexts): nest menu items one level deep

lib/Controller/ContextController.php

@@ -102,7 +102,7 @@ public function show(int $contextId): DataResponse {
 	 * @param string $iconName Material design icon name of the context
 	 * @param string $description Descriptive text of the context
 	 * @psalm-param list<array{id: int, type: int, permissions?: int}> $nodes optional nodes to be connected to this context
-	 * @psalm-param ?list<array{label: string, icon?: string, description?: string, targetType?: 'view'|'table'|'url'|'page', targetId?: int, url?: string, technicalName?: string}> $menuItems optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page
+	 * @psalm-param ?list<array{label: string, icon?: string, description?: string, targetType?: 'view'|'table'|'url'|'page', targetId?: int, url?: string, technicalName?: string, parentIndex?: int}> $menuItems optional menu of the context, in order; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page
 	 *
 	 * @return DataResponse<Http::STATUS_OK, TablesContext, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN, array{message: string}, array{}>
 	 *
@@ -144,7 +144,7 @@ public function create(string $name, string $iconName, string $description = '',
 	 * @param ?string $iconName provide this parameter to set a new icon
 	 * @param ?string $description provide this parameter to set a new description
 	 * @param ?array{id: int, type: int, permissions: int, order: int} $nodes provide this parameter to set a new list of nodes.
-	 * @psalm-param ?list<array{label: string, icon?: string, description?: string, targetType?: 'view'|'table'|'url'|'page', targetId?: int, url?: string, technicalName?: string}> $menuItems provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target
+	 * @psalm-param ?list<array{label: string, icon?: string, description?: string, targetType?: 'view'|'table'|'url'|'page', targetId?: int, url?: string, technicalName?: string, parentIndex?: int}> $menuItems provide this parameter to replace the menu; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target
 	 *
 	 * @return DataResponse<Http::STATUS_OK, TablesContext, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND|Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST, array{message: string}, array{}>
 	 *

lib/Model/MenuItemInput.php

@@ -15,8 +15,8 @@
 use OCA\Tables\Validation\TechnicalNameValidator;
 
 /**
- * A submitted menu item, checked on its own: its label, icon, description, target and
- * technical name. Whether a target may be used is up to the application it is saved in.
+ * A submitted menu item, checked on its own: its label, icon, description, target,
+ * technical name and the position of its parent. Whether a target may be used is up to the application it is saved in.
  */
 class MenuItemInput {
 	public const MAX_LABEL_LENGTH = 200;
@@ -35,6 +35,7 @@ public function __construct(
 		private readonly ?int $targetId,
 		private readonly ?string $url,
 		private readonly ?string $technicalName,
+		private readonly ?int $parentIndex = null,
 		private readonly bool $allowNewStartPage = false,
 	) {
 		if (trim($this->label) === '' || mb_strlen($this->label) > self::MAX_LABEL_LENGTH) {
@@ -86,6 +87,10 @@ public static function createFromInputArray(mixed $data, bool $allowNewStartPage
 				throw new InvalidArgumentException('The menu item property "' . $optional . '" must be a string.');
 			}
 		}
+		$parentIndex = $data['parentIndex'] ?? null;
+		if ($parentIndex !== null && (!is_int($parentIndex) || $parentIndex < 0)) {
+			throw new InvalidArgumentException('The parent of a menu item is given as the position of that parent in the menu.');
+		}
 
 		return new self(
 			label: trim($data['label']),
@@ -95,11 +100,15 @@ public static function createFromInputArray(mixed $data, bool $allowNewStartPage
 			targetId: $isUrl ? null : $targetId,
 			url: $isUrl ? trim((string)($data['url'] ?? '')) : null,
 			technicalName: self::emptyToNull($data['technicalName'] ?? null),
+			parentIndex: $parentIndex,
 			allowNewStartPage: $allowNewStartPage,
 		);
 	}
 
 	/**
+	 * A menu with one level of nesting: an item may sit under a top-level item of the
+	 * same menu, named by its position.
+	 *
 	 * @param bool $allowNewStartPage see createFromInputArray()
 	 * @return list<self>
 	 * @throws InvalidArgumentException
@@ -108,7 +117,14 @@ public static function createListFromInputArray(mixed $data, bool $allowNewStart
 		if (!is_array($data) || !array_is_list($data)) {
 			throw new InvalidArgumentException('The menu items must be a list.');
 		}
-		return array_map(static fn (mixed $item): self => self::createFromInputArray($item, $allowNewStartPage), $data);
+		$items = array_map(static fn (mixed $item): self => self::createFromInputArray($item, $allowNewStartPage), $data);
+		foreach ($items as $item) {
+			$parentIndex = $item->getParentIndex();
+			if ($parentIndex !== null && (!isset($items[$parentIndex]) || $items[$parentIndex]->getParentIndex() !== null)) {
+				throw new InvalidArgumentException('A menu item can only sit under a top-level item of the same menu.');
+			}
+		}
+		return $items;
 	}
 
 	/**
@@ -124,7 +140,7 @@ public function pointsAtNewStartPage(): bool {
 	 * @throws InvalidArgumentException
 	 */
 	public function withPageId(int $pageId): self {
-		return new self($this->label, $this->icon, $this->description, MenuItem::TARGET_PAGE, $pageId, null, $this->technicalName);
+		return new self($this->label, $this->icon, $this->description, MenuItem::TARGET_PAGE, $pageId, null, $this->technicalName, $this->parentIndex);
 	}
 
 	/**
@@ -175,4 +191,11 @@ public function getUrl(): ?string {
 	public function getTechnicalName(): ?string {
 		return $this->technicalName;
 	}
+
+	/**
+	 * The position of the parent item in the same menu, or null for a top-level item.
+	 */
+	public function getParentIndex(): ?int {
+		return $this->parentIndex;
+	}
 }

lib/Service/ContextService.php

@@ -329,38 +329,53 @@ protected function replaceMenu(Context $context, array $menuItems, ?string $user
 	}
 
 	/**
-	 * Inserts the items of a menu in their order. An item with a kept technical name is
-	 * saved under it and refused when another save took it; every other item is named
-	 * `item_<id>`.
+	 * Inserts the items of a menu, top-level items first so a child can point at the id
+	 * of its parent; the order keeps the sequence of the menu. An item with a kept
+	 * technical name is saved under it and refused when another save took it; every
+	 * other item is named `item_<id>`.
 	 *
 	 * @param list<MenuItemInput> $menuItems
 	 * @param array<int, string> $keptNames technical names by position
 	 * @throws Exception|InvalidArgumentException
 	 */
 	private function insertMenuItems(int $contextId, array $menuItems, array $keptNames): void {
-		foreach ($menuItems as $position => $item) {
-			$menuItem = new MenuItem();
-			$menuItem->setContextId($contextId);
-			$menuItem->setLabel($item->getLabel());
-			$menuItem->setIcon($item->getIcon());
-			$menuItem->setDescription($item->getDescription());
-			$menuItem->setTargetType($item->getTargetType());
-			$menuItem->setTargetId($item->getTargetId());
-			$menuItem->setUrl($item->getUrl());
-			$menuItem->setOrder(($position + 1) * 10);
-
-			if (!isset($keptNames[$position])) {
-				$this->menuItemMapper->insertWithDefaultName($menuItem);
-				continue;
-			}
-			$menuItem->setTechnicalName($keptNames[$position]);
-			try {
-				$this->menuItemMapper->insert($menuItem);
-			} catch (Exception $e) {
-				if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
-					throw $e;
+		$topLevelIds = [];
+		foreach ([false, true] as $childPass) {
+			foreach ($menuItems as $position => $item) {
+				$parentIndex = $item->getParentIndex();
+				if (($parentIndex !== null) !== $childPass) {
+					continue;
+				}
+				if ($parentIndex !== null && !isset($topLevelIds[$parentIndex])) {
+					throw new InvalidArgumentException('A menu item can only sit under a top-level item of the same menu.');
+				}
+				$menuItem = new MenuItem();
+				$menuItem->setContextId($contextId);
+				$menuItem->setLabel($item->getLabel());
+				$menuItem->setIcon($item->getIcon());
+				$menuItem->setDescription($item->getDescription());
+				$menuItem->setTargetType($item->getTargetType());
+				$menuItem->setTargetId($item->getTargetId());
+				$menuItem->setUrl($item->getUrl());
+				$menuItem->setOrder(($position + 1) * 10);
+				$menuItem->setParentId($parentIndex === null ? null : $topLevelIds[$parentIndex]);
+
+				if (isset($keptNames[$position])) {
+					$menuItem->setTechnicalName($keptNames[$position]);
+					try {
+						$this->menuItemMapper->insert($menuItem);
+					} catch (Exception $e) {
+						if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+							throw $e;
+						}
+						throw new InvalidArgumentException('The technical name ' . $keptNames[$position] . ' is already used by a menu item of this application.');
+					}
+				} else {
+					$this->menuItemMapper->insertWithDefaultName($menuItem);
+				}
+				if ($parentIndex === null) {
+					$topLevelIds[$position] = $menuItem->getId();
 				}
-				throw new InvalidArgumentException('The technical name ' . $keptNames[$position] . ' is already used by a menu item of this application.');
 			}
 		}
 	}

openapi.json

@@ -12313,7 +12313,7 @@
                                         "type": "array",
                                         "nullable": true,
                                         "default": null,
-                                        "description": "optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page",
+                                        "description": "optional menu of the context, in order; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page",
                                         "items": {
                                             "type": "object",
                                             "required": [
@@ -12347,6 +12347,10 @@
                                                 },
                                                 "technicalName": {
                                                     "type": "string"
+                                                },
+                                                "parentIndex": {
+                                                    "type": "integer",
+                                                    "format": "int64"
                                                 }
                                             }
                                         }
@@ -12787,7 +12791,7 @@
                                         "type": "array",
                                         "nullable": true,
                                         "default": null,
-                                        "description": "provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target",
+                                        "description": "provide this parameter to replace the menu; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target",
                                         "items": {
                                             "type": "object",
                                             "required": [
@@ -12821,6 +12825,10 @@
                                                 },
                                                 "technicalName": {
                                                     "type": "string"
+                                                },
+                                                "parentIndex": {
+                                                    "type": "integer",
+                                                    "format": "int64"
                                                 }
                                             }
                                         }

src/types/openapi/openapi.ts

@@ -7030,7 +7030,7 @@ export interface operations {
                         readonly permissions?: number;
                     }[];
                     /**
-                     * @description optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page
+                     * @description optional menu of the context, in order; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page
                      * @default null
                      */
                     readonly menuItems?: readonly {
@@ -7043,6 +7043,8 @@ export interface operations {
                         readonly targetId?: number;
                         readonly url?: string;
                         readonly technicalName?: string;
+                        /** Format: int64 */
+                        readonly parentIndex?: number;
                     }[] | null;
                 };
             };
@@ -7235,7 +7237,7 @@ export interface operations {
                         readonly order: number;
                     } | null;
                     /**
-                     * @description provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target
+                     * @description provide this parameter to replace the menu; parentIndex is the position of the parent item, for one level of nesting; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target
                      * @default null
                      */
                     readonly menuItems?: readonly {
@@ -7248,6 +7250,8 @@ export interface operations {
                         readonly targetId?: number;
                         readonly url?: string;
                         readonly technicalName?: string;
+                        /** Format: int64 */
+                        readonly parentIndex?: number;
                     }[] | null;
                 };
             };

tests/unit/Model/MenuItemInputTest.php

@@ -97,6 +97,52 @@ public function testOnlyAPageItemMayLeaveOutItsIdWhenTheStartPageIsAllowed(): vo
 		MenuItemInput::createFromInputArray(['label' => 'X', 'targetType' => 'view'], allowNewStartPage: true);
 	}
 
+	public function testKeepsTheParentPositionForOneLevelOfNesting(): void {
+		$items = MenuItemInput::createListFromInputArray([
+			['label' => 'Cases', 'targetType' => 'table', 'targetId' => 4],
+			['label' => 'Open', 'targetType' => 'view', 'targetId' => 6, 'parentIndex' => 0],
+			['label' => 'Docs', 'url' => 'https://example.org', 'parentIndex' => null],
+		]);
+
+		$this->assertSame([null, 0, null], array_map(static fn (MenuItemInput $item) => $item->getParentIndex(), $items));
+	}
+
+	public static function invalidNesting(): array {
+		return [
+			'under a child' => [[null, 0, 1], 'top-level'],
+			'under itself' => [[null, 1], 'top-level'],
+			'under a position that does not exist' => [[null, 5], 'top-level'],
+			'given as a string' => [[null, '0'], 'position of that parent'],
+			'given as a negative number' => [[null, -1], 'position of that parent'],
+		];
+	}
+
+	/**
+	 * @dataProvider invalidNesting
+	 */
+	public function testRejectsInvalidNesting(array $parentIndexes, string $message): void {
+		$items = array_map(
+			static fn (mixed $parentIndex, int $position): array => ['label' => 'Item ' . $position, 'url' => 'https://example.org/' . $position, 'parentIndex' => $parentIndex],
+			$parentIndexes,
+			array_keys($parentIndexes),
+		);
+
+		$this->expectException(InvalidArgumentException::class);
+		$this->expectExceptionMessage($message);
+		MenuItemInput::createListFromInputArray($items);
+	}
+
+	public function testResolvingTheStartPageKeepsTheParent(): void {
+		[, $child] = MenuItemInput::createListFromInputArray([
+			['label' => 'Cases', 'targetType' => 'table', 'targetId' => 4],
+			['label' => 'Overview', 'targetType' => 'page', 'parentIndex' => 0],
+		], allowNewStartPage: true);
+
+		$resolved = $child->withPageId(3);
+		$this->assertSame(0, $resolved->getParentIndex());
+		$this->assertSame(3, $resolved->getTargetId());
+	}
+
 	public function testRejectsAMenuThatIsNotAList(): void {
 		$this->expectException(InvalidArgumentException::class);
 		$this->expectExceptionMessage('must be a list');