From 291557a98ead0a31127bcb7d364af59c271d2b34 Mon Sep 17 00:00:00 2001 From: Remko Date: Wed, 14 Oct 2026 11:04:22 +0200 Subject: [PATCH 1/2] feat(contexts): technical names for applications - an application can get a technical name: lowercase letters, numbers and underscores, unique per owner, set on create and update; an empty name removes it - /apps/tables/app/{name} opens it; the owner's own application wins over one shared with the same name, and others' applications are linked by id - the app navigation links the owner's applications by their name - the name travels with an exported scheme Signed-off-by: Remko Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Controller/ContextController.php | 13 ++- lib/Controller/PageController.php | 17 ++- lib/Db/Context.php | 4 + lib/Db/ContextMapper.php | 48 ++++++++ .../Version2400Date20261014000000.php | 42 +++++++ lib/Model/ContextScheme.php | 9 +- lib/ResponseDefinitions.php | 1 + lib/Service/ContextService.php | 42 ++++++- openapi.json | 23 ++++ src/types/openapi/openapi.ts | 16 +++ .../Db/ContextMapperTechnicalNameTest.php | 103 ++++++++++++++++++ tests/unit/Model/ContextSchemeTest.php | 15 +++ tests/unit/Service/ContextServiceTest.php | 78 ++++++++++++- 13 files changed, 392 insertions(+), 19 deletions(-) create mode 100644 lib/Migration/Version2400Date20261014000000.php create mode 100644 tests/unit/Db/ContextMapperTechnicalNameTest.php diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index 9d363a87b3..40c55de1db 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -103,6 +103,7 @@ public function show(int $contextId): DataResponse { * @param string $description Descriptive text of the context * @psalm-param list $nodes optional nodes to be connected to this context * @psalm-param ?list $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 + * @param ?string $technicalName names the context in its address, lowercase letters, numbers and underscores; unique per owner * * @return DataResponse|DataResponse * @@ -111,7 +112,7 @@ public function show(int $contextId): DataResponse { * 403: lacking permissions on a resource */ #[NoAdminRequired] - public function create(string $name, string $iconName, string $description = '', array $nodes = [], ?array $menuItems = null): DataResponse { + public function create(string $name, string $iconName, string $description = '', array $nodes = [], ?array $menuItems = null, ?string $technicalName = null): DataResponse { try { if (!$this->isValidIcon($iconName)) { return new DataResponse(['message' => 'Invalid icon name'], Http::STATUS_BAD_REQUEST); @@ -124,6 +125,7 @@ public function create(string $name, string $iconName, string $description = '', $this->userId, 0, $menuItems !== null ? MenuItemInput::createListFromInputArray($menuItems) : null, + $technicalName, )->jsonSerialize()); } catch (Exception $e) { return $this->handleError($e); @@ -143,6 +145,7 @@ public function create(string $name, string $iconName, string $description = '', * @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 $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 + * @param ?string $technicalName provide this parameter to set a new technical name; an empty one removes it * * @return DataResponse|DataResponse * @@ -154,7 +157,7 @@ public function create(string $name, string $iconName, string $description = '', * @CanManageContext */ #[NoAdminRequired] - public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null): DataResponse { + public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null, ?string $technicalName = null): DataResponse { try { if ($iconName !== null && !$this->isValidIcon($iconName)) { return new DataResponse(['message' => 'Invalid icon name'], Http::STATUS_BAD_REQUEST); @@ -168,6 +171,7 @@ public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, $nodes, $menuItems !== null ? MenuItemInput::createListFromInputArray($menuItems) : null, + $technicalName, )->jsonSerialize()); } catch (Exception|MultipleObjectsReturnedException $e) { return $this->handleError($e); @@ -364,6 +368,7 @@ public function previewSchemeChanges(int $contextId, array $updateScheme): DataR * @psalm-param array{addTables: list, modifyTables: list} $tables Tables to be added or modified * @psalm-param list $menuItems Menu of the context, targets by uuid, parents by technical name * @psalm-param list> $gridViews Views without a table that the menu points at + * @param ?string $technicalName technical name of the context * * @return DataResponse, array{}>|DataResponse * @@ -374,9 +379,9 @@ 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, array $menuItems = [], array $gridViews = []): DataResponse { + public function importScheme(int $contextId, string $name, string $iconName, string $description, array $nodes, array $tables, array $menuItems = [], array $gridViews = [], ?string $technicalName = null): DataResponse { try { - $scheme = ContextScheme::createFromInputArray(['name' => $name, 'icon' => $iconName, 'description' => $description, 'nodes' => $nodes, 'tables' => $tables, 'menuItems' => $menuItems, 'gridViews' => $gridViews]); + $scheme = ContextScheme::createFromInputArray(['name' => $name, 'icon' => $iconName, 'description' => $description, 'nodes' => $nodes, 'tables' => $tables, 'menuItems' => $menuItems, 'gridViews' => $gridViews, 'technicalName' => $technicalName]); } catch (InvalidArgumentException $e) { return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index c4c6cb41e9..441e766ab4 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -8,6 +8,7 @@ namespace OCA\Tables\Controller; use OCA\Tables\AppInfo\Application; +use OCA\Tables\Db\ContextMapper; use OCA\Tables\Service\NodeService; use OCA\Tables\Service\ShareService; use OCA\Text\Event\LoadEditor; @@ -32,6 +33,8 @@ public function __construct( protected IInitialState $initialState, protected ShareService $shareService, protected NodeService $nodeService, + protected ContextMapper $contextMapper, + protected ?string $userId, ) { parent::__construct(Application::APP_ID, $request); } @@ -58,18 +61,20 @@ public function index(): TemplateResponse { } /** - * Render default template - * - * @psalm-param int<0, max> $appId + * Render the template for one application, addressed by id or by technical name */ #[NoAdminRequired] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] - public function context(int $contextId): TemplateResponse { - $navId = Application::APP_ID . '_application_' . $contextId; + public function context(string $contextId): TemplateResponse { + $id = ctype_digit($contextId) ? (int)$contextId : ($this->userId === null ? null : $this->contextMapper->findIdByTechnicalName($contextId, $this->userId)); + if ($id === null) { + return $this->index(); + } + $navId = Application::APP_ID . '_application_' . $id; $this->navigationManager->setActiveEntry($navId); - $this->initialState->provideInitialState('contextId', $contextId); + $this->initialState->provideInitialState('contextId', $id); return $this->index(); } diff --git a/lib/Db/Context.php b/lib/Db/Context.php index 333cbfda3e..9c433e89b9 100644 --- a/lib/Db/Context.php +++ b/lib/Db/Context.php @@ -21,6 +21,8 @@ * @method setOwnerId(string $value): void * @method getOwnerType(): int * @method setOwnerType(int $value): void + * @method getTechnicalName(): ?string + * @method setTechnicalName(?string $value): void * * @method getSharing(): array * @method setSharing(array $value): void @@ -37,6 +39,7 @@ class Context extends EntitySuper implements JsonSerializable { protected ?string $description = null; protected ?string $ownerId = null; protected ?int $ownerType = null; + protected ?string $technicalName = null; // virtual properties protected ?array $sharing = null; @@ -60,6 +63,7 @@ public function jsonSerialize(): array { 'description' => $this->getDescription(), 'owner' => $this->getOwnerId(), 'ownerType' => $this->getOwnerType(), + 'technicalName' => $this->getTechnicalName(), 'menuItems' => array_values($this->menuItems ?? []), ]; diff --git a/lib/Db/ContextMapper.php b/lib/Db/ContextMapper.php index d8e6d56ed8..9e374ee261 100644 --- a/lib/Db/ContextMapper.php +++ b/lib/Db/ContextMapper.php @@ -88,6 +88,7 @@ protected function formatResultRows(array $rows, ?string $userId, array $menuIte 'description' => $rows[0]['description'], 'owner_id' => $rows[0]['owner_id'], 'owner_type' => $rows[0]['owner_type'], + 'technical_name' => $rows[0]['technical_name'] ?? null, ]; $formatted['sharing'] = array_reduce($rows, function (array $carry, array $item) use ($userId) { @@ -238,6 +239,53 @@ public function findById(int $contextId, ?string $userId = null): Context { return $this->formatResultRows($r, $userId, $this->menuItemMapper->findByContextIds([$contextId])); } + /** + * The id of the context with this technical name that the user owns, else the oldest one + * shared with them. Technical names are unique per owner only. + * + * @throws Exception + */ + public function findIdByTechnicalName(string $technicalName, string $userId): ?int { + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct(['c.id', 'c.owner_id']) + ->from($this->table, 'c'); + $this->applyOwnedOrSharedQuery($qb, $userId); + $qb->andWhere($qb->expr()->eq('c.technical_name', $qb->createNamedParameter($technicalName, IQueryBuilder::PARAM_STR))) + ->orderBy('c.id', 'ASC'); + $result = $qb->executeQuery(); + $sharedId = null; + while (($row = $result->fetchAssociative()) !== false) { + if ($row['owner_id'] === $userId) { + $result->closeCursor(); + return (int)$row['id']; + } + $sharedId ??= (int)$row['id']; + } + $result->closeCursor(); + return $sharedId; + } + + /** + * Whether the owner already has another context with this technical name. + * + * @throws Exception + */ + public function isTechnicalNameTaken(string $technicalName, string $ownerId, ?int $exceptContextId = null): bool { + $qb = $this->db->getQueryBuilder(); + $qb->select('id') + ->from($this->table) + ->where($qb->expr()->eq('technical_name', $qb->createNamedParameter($technicalName, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('owner_id', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR))) + ->setMaxResults(1); + if ($exceptContextId !== null) { + $qb->andWhere($qb->expr()->neq('id', $qb->createNamedParameter($exceptContextId, IQueryBuilder::PARAM_INT))); + } + $result = $qb->executeQuery(); + $taken = $result->fetchOne() !== false; + $result->closeCursor(); + return $taken; + } + /** * @return Context[] * @throws Exception diff --git a/lib/Migration/Version2400Date20261014000000.php b/lib/Migration/Version2400Date20261014000000.php new file mode 100644 index 0000000000..d8b20f906c --- /dev/null +++ b/lib/Migration/Version2400Date20261014000000.php @@ -0,0 +1,42 @@ +hasTable('tables_contexts_context')) { + $table = $schema->getTable('tables_contexts_context'); + if (!$table->hasColumn('technical_name')) { + $table->addColumn('technical_name', Types::STRING, [ + 'notnull' => false, + 'default' => null, + 'length' => 200, + ]); + } + } + + return $schema; + } +} diff --git a/lib/Model/ContextScheme.php b/lib/Model/ContextScheme.php index 51e29bd141..36108610d3 100644 --- a/lib/Model/ContextScheme.php +++ b/lib/Model/ContextScheme.php @@ -21,6 +21,7 @@ public function __construct( protected ?array $tables = [], protected array $menuItems = [], protected array $gridViews = [], + protected ?string $technicalName = null, ) { } @@ -43,7 +44,7 @@ public static function createFromInputArray(array $data): self { if (!is_string($data['name'] ?? null) || trim($data['name']) === '') { throw new InvalidArgumentException('A scheme needs a name.'); } - foreach (['icon', 'description'] as $optional) { + foreach (['icon', 'description', 'technicalName'] as $optional) { if (isset($data[$optional]) && !is_string($data[$optional])) { throw new InvalidArgumentException('The scheme property "' . $optional . '" must be a string.'); } @@ -57,9 +58,14 @@ public static function createFromInputArray(array $data): self { tables: $tables, menuItems: array_values($data['menuItems'] ?? []), gridViews: array_values($data['gridViews'] ?? []), + technicalName: isset($data['technicalName']) && $data['technicalName'] !== '' ? $data['technicalName'] : null, ); } + public function getTechnicalName(): ?string { + return $this->technicalName; + } + public function getName(): ?string { return $this->name ?? ''; } @@ -108,6 +114,7 @@ public function jsonSerialize(): mixed { 'tables' => $this->tables, 'menuItems' => $this->menuItems, 'gridViews' => $this->gridViews, + 'technicalName' => $this->technicalName, ]; } } diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index c3221957dd..83c50c598c 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -287,6 +287,7 @@ * description: string, * owner: string, * ownerType: int, + * technicalName: string|null, * menuItems: list, * } * diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index 3d0c7bc7e7..14ae7c1f0a 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -103,7 +103,9 @@ public function addToNavigation(string $userId): void { $iconUrl = $this->urlGenerator->imagePath('core', 'places/default-app-icon.svg'); } - $contextUrl = $this->urlGenerator->linkToRoute('tables.page.context', ['contextId' => $context->getId()]); + // Technical names are unique per owner only, so applications of others are linked by id + $isOwner = $context->getOwnerType() === Application::OWNER_TYPE_USER && $context->getOwnerId() === $userId; + $contextUrl = $this->urlGenerator->linkToRoute('tables.page.context', ['contextId' => ($isOwner ? $context->getTechnicalName() : null) ?: $context->getId()]); $this->navigationManager->add([ 'id' => Application::APP_ID . '_application_' . $context->getId(), @@ -137,15 +139,17 @@ public function findById(int $id, ?string $userId): Context { /** * @psalm-param list $nodes * @param list|null $menuItems + * @param string|null $technicalName names the application in its address; unique per owner * @throws Exception|PermissionError|InvalidArgumentException */ - public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType, ?array $menuItems = null): Context { + public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType, ?array $menuItems = null, ?string $technicalName = null): Context { $context = new Context(); $context->setName(trim($name)); $context->setIcon(trim($iconName)); $context->setDescription(trim($description)); $context->setOwnerId($ownerId); $context->setOwnerType($ownerType); + $context->setTechnicalName($this->normalizeTechnicalName($technicalName, $ownerId, null)); $this->atomic(function () use ($context, $nodes, $menuItems, $ownerId): void { $this->contextMapper->insert($context); @@ -381,6 +385,27 @@ private function addMenuTargetsAsNodes(Context $context, array $menuItems, array $context->setPages($pages); } + /** + * The technical name of an application as it is stored: trimmed, and null when blank. + * + * @throws InvalidArgumentException when the name is malformed or the owner already uses it + * @throws Exception + */ + protected function normalizeTechnicalName(?string $technicalName, string $ownerId, ?int $contextId): ?string { + if ($technicalName === null) { + return null; + } + $technicalName = trim($technicalName); + if ($technicalName === '') { + return null; + } + MenuItemInput::assertTechnicalNameValid($technicalName); + if ($this->contextMapper->isTechnicalNameTaken($technicalName, $ownerId, $contextId)) { + throw new InvalidArgumentException('An application with the technical name "' . $technicalName . '" already exists.'); + } + return $technicalName; + } + protected function technicalNameFromLabel(string $label): string { $name = strtolower(trim(preg_replace('/[^a-z0-9]+/i', '_', $label) ?? '', '_')); $name = preg_replace('/^[^a-z]+/', '', $name) ?? ''; @@ -390,15 +415,16 @@ protected function technicalNameFromLabel(string $label): string { /** * @psalm-param list $nodes * @param list|null $menuItems + * @param string|null $technicalName a new technical name; blank clears it, null leaves it * @throws Exception * @throws DoesNotExistException * @throws PermissionError|MultipleObjectsReturnedException|InvalidArgumentException */ - public function update(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null): Context { + public function update(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null, ?string $technicalName = null): Context { // A transaction that reads before it writes can lose a write lock to a concurrent request // (SQLite answers "database is locked" at once); running it again then succeeds return $this->atomicRetry( - fn (): Context => $this->applyUpdate($contextId, $userId, $name, $iconName, $description, $nodes, $menuItems), + fn (): Context => $this->applyUpdate($contextId, $userId, $name, $iconName, $description, $nodes, $menuItems, $technicalName), $this->dbc, ); } @@ -410,12 +436,15 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i * @throws DoesNotExistException * @throws PermissionError|MultipleObjectsReturnedException|InvalidArgumentException */ - private function applyUpdate(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems): Context { + private function applyUpdate(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems, ?string $technicalName): Context { $context = $this->contextMapper->findById($contextId, $userId); if ($name !== null) { $context->setName(trim($name)); } + if ($technicalName !== null) { + $context->setTechnicalName($this->normalizeTechnicalName($technicalName, $context->getOwnerId(), $contextId)); + } if ($iconName !== null) { $context->setIcon(trim($iconName)); } @@ -967,6 +996,7 @@ public function getScheme(int $contextId, string $userId): ContextScheme { array_values($tables), $menuItems, $gridViews, + $context->getTechnicalName(), ); } @@ -1352,6 +1382,6 @@ public function importScheme(int $contextId, ContextScheme $scheme, string $user } } - return $this->update($contextId, $userId, $scheme->getName(), $scheme->getIcon() ?? '', $scheme->getDescription() ?? '', $resolvedNodes, $this->resolveMenuItems($scheme->getMenuItems())); + return $this->update($contextId, $userId, $scheme->getName(), $scheme->getIcon() ?? '', $scheme->getDescription() ?? '', $resolvedNodes, $this->resolveMenuItems($scheme->getMenuItems()), $scheme->getTechnicalName()); } } diff --git a/openapi.json b/openapi.json index 9021871df1..bde3d3cbe6 100644 --- a/openapi.json +++ b/openapi.json @@ -280,6 +280,7 @@ "description", "owner", "ownerType", + "technicalName", "menuItems" ], "properties": { @@ -303,6 +304,10 @@ "type": "integer", "format": "int64" }, + "technicalName": { + "type": "string", + "nullable": true + }, "menuItems": { "type": "array", "items": { @@ -12351,6 +12356,12 @@ } } } + }, + "technicalName": { + "type": "string", + "nullable": true, + "default": null, + "description": "names the context in its address, lowercase letters, numbers and underscores; unique per owner" } } } @@ -12826,6 +12837,12 @@ } } } + }, + "technicalName": { + "type": "string", + "nullable": true, + "default": null, + "description": "provide this parameter to set a new technical name; an empty one removes it" } } } @@ -13906,6 +13923,12 @@ "type": "object" } } + }, + "technicalName": { + "type": "string", + "nullable": true, + "default": null, + "description": "technical name of the context" } } } diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 01a2d5d330..a304db1080 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1192,6 +1192,7 @@ export type components = { readonly owner: string; /** Format: int64 */ readonly ownerType: number; + readonly technicalName: string | null; readonly menuItems: readonly components["schemas"]["MenuItem"][]; }; readonly ContextNavigation: { @@ -7038,6 +7039,11 @@ export interface operations { /** Format: int64 */ readonly parentIndex?: number; }[] | null; + /** + * @description names the context in its address, lowercase letters, numbers and underscores; unique per owner + * @default null + */ + readonly technicalName?: string | null; }; }; }; @@ -7244,6 +7250,11 @@ export interface operations { /** Format: int64 */ readonly parentIndex?: number; }[] | null; + /** + * @description provide this parameter to set a new technical name; an empty one removes it + * @default null + */ + readonly technicalName?: string | null; }; }; }; @@ -7700,6 +7711,11 @@ export interface operations { readonly gridViews?: readonly { readonly [key: string]: Record; }[]; + /** + * @description technical name of the context + * @default null + */ + readonly technicalName?: string | null; }; }; }; diff --git a/tests/unit/Db/ContextMapperTechnicalNameTest.php b/tests/unit/Db/ContextMapperTechnicalNameTest.php new file mode 100644 index 0000000000..a37421fb0d --- /dev/null +++ b/tests/unit/Db/ContextMapperTechnicalNameTest.php @@ -0,0 +1,103 @@ +cleanupTablesData(); + $userHelper = $this->createMock(UserHelper::class); + $userHelper->method('getGroupIdsForUser')->willReturn([]); + $this->contextMapper = new ContextMapper( + $this->connectionAdapter, + $userHelper, + $this->createMock(GroupHelper::class), + new MenuItemMapper($this->connectionAdapter), + ); + } + + public function testTheOwnApplicationWinsOverOneSharedWithTheSameName(): void { + $shared = $this->createContext('bob', 'intake'); + $this->shareWith($shared, 'bob', 'alice'); + $own = $this->createContext('alice', 'intake'); + + $this->assertSame($own, $this->contextMapper->findIdByTechnicalName('intake', 'alice')); + $this->assertSame($shared, $this->contextMapper->findIdByTechnicalName('intake', 'bob')); + } + + public function testAnApplicationSharedWithTheUserIsFoundByItsName(): void { + $shared = $this->createContext('bob', 'intake'); + $this->shareWith($shared, 'bob', 'alice'); + + $this->assertSame($shared, $this->contextMapper->findIdByTechnicalName('intake', 'alice')); + } + + public function testAnApplicationOfSomeoneElseIsNotFound(): void { + $this->createContext('bob', 'intake'); + + $this->assertNull($this->contextMapper->findIdByTechnicalName('intake', 'alice')); + $this->assertNull($this->contextMapper->findIdByTechnicalName('reports', 'bob')); + } + + public function testANameIsTakenPerOwnerAndNotByTheApplicationItself(): void { + $intake = $this->createContext('alice', 'intake'); + + $this->assertTrue($this->contextMapper->isTechnicalNameTaken('intake', 'alice')); + $this->assertFalse($this->contextMapper->isTechnicalNameTaken('intake', 'alice', $intake)); + $this->assertFalse($this->contextMapper->isTechnicalNameTaken('intake', 'bob')); + } + + private function createContext(string $ownerId, ?string $technicalName): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_contexts_context') + ->values([ + 'name' => $qb->createNamedParameter('Intake'), + 'icon' => $qb->createNamedParameter('star'), + 'description' => $qb->createNamedParameter(''), + 'owner_id' => $qb->createNamedParameter($ownerId), + 'owner_type' => $qb->createNamedParameter(0), + 'technical_name' => $qb->createNamedParameter($technicalName), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function shareWith(int $contextId, string $sender, string $receiver): void { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_shares') + ->values([ + 'sender' => $qb->createNamedParameter($sender), + 'receiver' => $qb->createNamedParameter($receiver), + 'receiver_type' => $qb->createNamedParameter('user'), + 'node_id' => $qb->createNamedParameter($contextId), + 'node_type' => $qb->createNamedParameter('context'), + 'permission_read' => $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), + 'permission_create' => $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), + 'permission_update' => $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), + 'permission_delete' => $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), + 'permission_manage' => $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), + 'created_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + 'last_edit_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + ]) + ->executeStatement(); + } +} diff --git a/tests/unit/Model/ContextSchemeTest.php b/tests/unit/Model/ContextSchemeTest.php index 5da5236f1e..8ffb0812f3 100644 --- a/tests/unit/Model/ContextSchemeTest.php +++ b/tests/unit/Model/ContextSchemeTest.php @@ -33,6 +33,21 @@ public function testCreateFromInputCarriesEveryPart(): void { $this->assertSame(self::TABLES, $scheme->getTables()); } + public function testCreateFromInputCarriesTheTechnicalNameAndTreatsABlankOneAsNone(): void { + $named = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'technicalName' => 'intake']); + $blank = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'technicalName' => '']); + + $this->assertSame('intake', $named->getTechnicalName()); + $this->assertSame('intake', $named->jsonSerialize()['technicalName']); + $this->assertNull($blank->getTechnicalName()); + } + + public function testCreateFromInputRejectsATechnicalNameThatIsNotAString(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('"technicalName" must be a string'); + ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES, 'technicalName' => ['intake']]); + } + public function testCreateFromInputWithoutAMenuHasAnEmptyOne(): void { $scheme = ContextScheme::createFromInputArray(['name' => 'Intake', 'tables' => self::TABLES]); diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 35a2501451..0626e39ab0 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -48,6 +48,7 @@ final class ContextServiceTest extends TestCase { private MockObject $tableMapper; private MockObject $viewMapper; private MockObject $permissionsService; + private MockObject $urlGenerator; protected function setUp(): void { parent::setUp(); @@ -66,7 +67,7 @@ protected function setUp(): void { $dbConnection = $this->createMock(IDBConnection::class); $shareService = $this->createMock(ShareService::class); $navigationManager = $this->createMock(INavigationManager::class); - $urlGenerator = $this->createMock(IURLGenerator::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->service = new ContextService( $this->contextMapper, @@ -81,7 +82,7 @@ protected function setUp(): void { $shareService, false, $navigationManager, - $urlGenerator, + $this->urlGenerator, $this->tableMapper, $this->viewMapper, $this->menuItemMapper, @@ -397,6 +398,79 @@ public function testDeleteRemovesTheApplicationBeforeWhatBelongsToIt(): void { $this->assertSame(['application', 'menu'], $calls); } + public function testUpdateRejectsAMalformedTechnicalName(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + + $this->expectException(InvalidArgumentException::class); + $this->service->update(7, 'user-1', null, null, null, null, null, 'Not A Name'); + } + + public function testUpdateRejectsATechnicalNameTheOwnerAlreadyUses(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('isTechnicalNameTaken')->with('intake', 'user-1', 7)->willReturn(true); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('already exists'); + $this->service->update(7, 'user-1', null, null, null, null, null, 'intake'); + } + + public function testUpdateClearsTheTechnicalNameWhenItIsBlank(): void { + $context = $this->contextWithMenu(7, []); + $context->setTechnicalName('old'); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + + $updated = $this->service->update(7, 'user-1', null, null, null, null, null, ' '); + + $this->assertNull($updated->getTechnicalName()); + } + + public function testUpdateLeavesTheTechnicalNameAloneWhenNoneIsGiven(): void { + $context = $this->contextWithMenu(7, []); + $context->setTechnicalName('intake'); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->contextMapper->expects($this->never())->method('isTechnicalNameTaken'); + + $updated = $this->service->update(7, 'user-1', 'Renamed', null, null, null); + + $this->assertSame('intake', $updated->getTechnicalName()); + } + + public function testCreateStoresTheTechnicalName(): void { + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { + $context->setId(8); + return $context; + }); + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page): Page { + $page->setId(3); + return $page; + }); + $this->menuItemMapper->method('findByContextId')->willReturn([]); + + $created = $this->service->create('Intake portal', 'heart', '', [], 'user-1', 0, [], 'intake'); + + $this->assertSame('intake', $created->getTechnicalName()); + } + + public function testNavigationLinksOwnApplicationsByTechnicalNameAndOthersById(): void { + $own = $this->contextWithMenu(7, []); + $own->setTechnicalName('intake'); + $shared = $this->contextWithMenu(8, []); + $shared->setOwnerId('someone-else'); + $shared->setTechnicalName('intake'); + $this->contextMapper->method('findForNavBar')->willReturn([$own, $shared]); + $linked = []; + $this->urlGenerator->method('linkToRoute')->willReturnCallback(static function (string $route, array $parameters) use (&$linked): string { + $linked[] = $parameters['contextId']; + return '/app/' . $parameters['contextId']; + }); + + $this->service->addToNavigation('user-1'); + + $this->assertSame(['intake', 8], $linked); + } + /** * The export and the import resolution are private because their only entry points * need the server container; they are exercised directly here. From 6383311fe73bc3afa86b5fa6406ea321da72f58f Mon Sep 17 00:00:00 2001 From: Remko Date: Wed, 14 Oct 2026 11:05:09 +0200 Subject: [PATCH 2/2] feat(applications): set the technical name in the dialogs - the create and edit dialogs take an optional technical name - the overview shows and opens the application at its readable address for its owner, and by id for everyone else Signed-off-by: Remko Assisted-by: ClaudeCode:claude-opus-5-5 --- playwright/e2e/application.spec.ts | 17 +++++++++++++++++ src/modules/modals/CreateContext.vue | 10 ++++++++++ src/modules/modals/EditContext.vue | 11 +++++++++++ src/pages/Context.vue | 4 +++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/playwright/e2e/application.spec.ts b/playwright/e2e/application.spec.ts index 946d54a616..8a694da85d 100644 --- a/playwright/e2e/application.spec.ts +++ b/playwright/e2e/application.spec.ts @@ -79,6 +79,23 @@ test.describe('Applications', () => { await expect(row).toContainText('All requests') }) + test('An application with a technical name is opened at its readable address', async ({ userPage: { page } }) => { + // Technical names are unique per owner; a fresh one keeps reruns on the same instance apart + const technicalName = 'intake_' + Date.now() + await page.goto('/index.php/apps/tables') + await createContext(page, 'named application') + await loadContext(page, 'named application') + + await page.getByRole('button', { name: 'Application settings' }).click() + await page.getByRole('menuitem', { name: 'Edit name, icon and members' }).click() + await page.locator('[data-cy="editContextTechnicalName"]').fill(technicalName) + await page.locator('[data-cy="editContextSubmitBtn"]').click() + await expect(page.locator('[data-cy="context-address"]')).toContainText('/apps/tables/app/' + technicalName) + + await page.goto('/index.php/apps/tables/app/' + technicalName) + await expect(page.locator('[data-cy="application-nav-header"]')).toContainText('named application') + }) + test('A page with widgets added on the overview gets a menu entry and opens', async ({ userPage: { page } }) => { await page.goto('/index.php/apps/tables') await createContext(page, 'overview application') diff --git a/src/modules/modals/CreateContext.vue b/src/modules/modals/CreateContext.vue index d5ab502808..8e2d8b27c2 100644 --- a/src/modules/modals/CreateContext.vue +++ b/src/modules/modals/CreateContext.vue @@ -36,6 +36,13 @@ +
+
+ {{ t('tables', 'Technical name') }} +
+ +
{{ t('tables', 'Resources') }} @@ -118,6 +125,7 @@ export default { customTitleChosen: false, errorTitle: false, description: '', + technicalName: '', menuItems: [], resources: [], receivers: [], @@ -197,6 +205,7 @@ export default { iconName: this.icon.name, description: this.description, nodes: dataResources, + technicalName: this.technicalName.trim() || null, menuItems: this.menuItems.map(item => toMenuItemPayload(item, this.menuItems)), } // adding share to oneself to have navigation display control @@ -221,6 +230,7 @@ export default { }, reset() { this.title = '' + this.technicalName = '' this.menuItems = [] this.errorTitle = false this.setIcon(this.randomIcon()) diff --git a/src/modules/modals/EditContext.vue b/src/modules/modals/EditContext.vue index b096360b74..24de46da87 100644 --- a/src/modules/modals/EditContext.vue +++ b/src/modules/modals/EditContext.vue @@ -30,6 +30,13 @@
+
+
+ {{ t('tables', 'Technical name') }} +
+ +
{{ t('tables', 'Resources') }} @@ -114,6 +121,7 @@ export default { svg: null, }, description: '', + technicalName: '', errorTitle: false, resources: [], receivers: [], @@ -144,6 +152,7 @@ export default { this.title = context.name this.setIcon(this.localContext.iconName) this.description = context.description + this.technicalName = context.technicalName ?? '' this.resources = context ? this.getContextResources(context) : [] this.receivers = context ? this.getContextReceivers(context) : [] this.showInNavigationDefault = this.getNavDisplay(context) @@ -177,6 +186,7 @@ export default { iconName: this.icon.name, description: this.description, nodes: dataResources, + technicalName: this.technicalName.trim(), } const context = this.getContext(this.contextId) // adding share to oneself to have navigation display control @@ -202,6 +212,7 @@ export default { this.errorTitle = false this.icon.name = 'equalizer' this.description = '' + this.technicalName = context?.technicalName ?? '' this.resources = context ? this.getContextResources(context) : [] this.receivers = context ? this.getContextReceivers(context) : [] this.prepareDeleteContext = false diff --git a/src/pages/Context.vue b/src/pages/Context.vue index dd6e639e6b..a8b416e016 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -504,7 +504,9 @@ export default { return this.standaloneContextId !== null }, applicationUrl() { - return window.location.origin + generateUrl('/apps/tables/app/' + this.activeContext?.id) + // Technical names are unique per owner only, so an application of someone else is addressed by id + const technicalName = this.activeContext && this.ownsContext(this.activeContext) ? this.activeContext.technicalName : null + return window.location.origin + generateUrl('/apps/tables/app/' + (technicalName || this.activeContext?.id)) }, selectedMenuItem() { const technicalName = this.$route.params.itemName