From 5d263959cfd04501304a0423791c205e55ea337d Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Thu, 22 Oct 2026 15:10:23 +0200 Subject: [PATCH] feat(contexts): add the data model for grid pages Pages get a title, a named icon, a description and a technical name that is unique within their application. Without a name a page is called page_, the start page dashboard, and page_ is kept for those automatic names. Widgets get their own table: the page, the type, an optional title, icon and description, the position and size on the grid, and the settings as one JSON object. A widget source links a data widget to a table or view of the same application. One migration adds all of it and names the pages that already exist. The technical name format is checked by one validator, now also used for views and columns. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ruben van der Linde --- appinfo/info.xml | 2 +- lib/Controller/ContextController.php | 7 +- lib/Db/Page.php | 27 +++ lib/Db/PageMapper.php | 17 ++ lib/Db/Widget.php | 107 +++++++++++ lib/Db/WidgetMapper.php | 78 ++++++++ lib/Db/WidgetSource.php | 31 +++ lib/Db/WidgetSourceMapper.php | 99 ++++++++++ lib/Helper/IconHelper.php | 22 +++ .../Version2400Date20261022000000.php | 176 ++++++++++++++++++ lib/Service/ContextService.php | 42 ++++- lib/Service/ViewService.php | 7 +- lib/Validation/ColumnDtoValidator.php | 8 +- lib/Validation/TechnicalNameValidator.php | 30 +++ package-lock.json | 4 +- package.json | 2 +- tests/unit/Database/DbErrorStub.php | 31 +++ tests/unit/Db/PageTest.php | 127 +++++++++++++ tests/unit/Db/WidgetMapperTest.php | 123 ++++++++++++ tests/unit/Db/WidgetSourceMapperTest.php | 84 +++++++++ tests/unit/Db/WidgetTest.php | 67 +++++++ tests/unit/Helper/IconHelperTest.php | 26 +++ .../Version2400Date20261022000000Test.php | 174 +++++++++++++++++ tests/unit/Service/ContextServiceTest.php | 103 ++++++++++ .../Service/ViewServiceTechnicalNameTest.php | 47 +++++ .../Validation/TechnicalNameValidatorTest.php | 34 ++++ tests/unit/bootstrap.php | 1 + 27 files changed, 1457 insertions(+), 19 deletions(-) create mode 100644 lib/Db/Widget.php create mode 100644 lib/Db/WidgetMapper.php create mode 100644 lib/Db/WidgetSource.php create mode 100644 lib/Db/WidgetSourceMapper.php create mode 100644 lib/Helper/IconHelper.php create mode 100644 lib/Migration/Version2400Date20261022000000.php create mode 100644 lib/Validation/TechnicalNameValidator.php create mode 100644 tests/unit/Database/DbErrorStub.php create mode 100644 tests/unit/Db/PageTest.php create mode 100644 tests/unit/Db/WidgetMapperTest.php create mode 100644 tests/unit/Db/WidgetSourceMapperTest.php create mode 100644 tests/unit/Db/WidgetTest.php create mode 100644 tests/unit/Helper/IconHelperTest.php create mode 100644 tests/unit/Migration/Version2400Date20261022000000Test.php create mode 100644 tests/unit/Service/ViewServiceTechnicalNameTest.php create mode 100644 tests/unit/Validation/TechnicalNameValidatorTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 8545043c14..d0ced3fb1b 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -26,7 +26,7 @@ Share your tables and views with users and groups within your cloud. Have a good time and manage whatever you want. ]]> - 2.4.0-dev.0 + 2.4.0-dev.1 AGPL-3.0-or-later Nextcloud GmbH and Nextcloud contributors Tables diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index e87f8ca440..434e2137bb 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -15,6 +15,7 @@ use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Helper\IconHelper; use OCA\Tables\Middleware\Attribute\RequirePermission; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnService; @@ -386,11 +387,7 @@ public function importScheme(int $contextId, string $name, string $iconName, str } protected function isValidIcon(string $iconName): bool { - if ($iconName === '' || !preg_match('/^[a-zA-Z0-9-]+$/', $iconName)) { - return false; - } - $iconPath = dirname(__DIR__, 2) . '/img/material/' . $iconName . '.svg'; - return file_exists($iconPath); + return IconHelper::isValid($iconName); } /** diff --git a/lib/Db/Page.php b/lib/Db/Page.php index 5ceeb1bd00..44cff5db53 100644 --- a/lib/Db/Page.php +++ b/lib/Db/Page.php @@ -15,22 +15,49 @@ * @method setContextId(int $value): void * @method getPageType(): string * @method setPageType(string $value): void + * @method getTechnicalName(): string + * @method setTechnicalName(string $value): void + * @method getTitle(): ?string + * @method setTitle(?string $value): void + * @method getIcon(): ?string + * @method setIcon(?string $value): void + * @method getDescription(): ?string + * @method setDescription(?string $value): void */ class Page extends Entity implements \JsonSerializable { public const TYPE_STARTPAGE = 'startpage'; + public const TYPE_GRID = 'grid'; + public const START_PAGE_NAME = 'dashboard'; + public const DEFAULT_NAME_PREFIX = 'page_'; protected ?int $contextId = null; protected ?string $pageType = null; + protected ?string $technicalName = null; + protected ?string $title = null; + protected ?string $icon = null; + protected ?string $description = null; public function __construct() { $this->addType('id', 'integer'); } + /** + * Whether a technical name has the form of the automatic `page_`. Such names are + * reserved, so a given name never collides with the name a later page gets. + */ + public static function isDefaultName(string $technicalName): bool { + return preg_match('/^' . self::DEFAULT_NAME_PREFIX . '[0-9]+$/', $technicalName) === 1; + } + public function jsonSerialize(): array { return [ 'id' => $this->getId(), 'contextId' => $this->getContextId(), 'pageType' => $this->getPageType(), + 'technicalName' => $this->getTechnicalName(), + 'title' => $this->getTitle(), + 'icon' => $this->getIcon(), + 'description' => $this->getDescription(), ]; } } diff --git a/lib/Db/PageMapper.php b/lib/Db/PageMapper.php index b7a1ce428a..cd85c17f94 100644 --- a/lib/Db/PageMapper.php +++ b/lib/Db/PageMapper.php @@ -8,17 +8,34 @@ namespace OCA\Tables\Db; use OCP\AppFramework\Db\QBMapper; +use OCP\AppFramework\Db\TTransactional; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; /** @template-extends QBMapper */ class PageMapper extends QBMapper { + use TTransactional; + protected string $table = 'tables_contexts_page'; public function __construct(IDBConnection $db) { parent::__construct($db, $this->table, Page::class); } + /** + * Inserts a page under the default technical name `page_`. + * + * @throws Exception + */ + public function insertWithDefaultName(Page $page): Page { + return $this->atomic(function () use ($page): Page { + $page = $this->insert($page); + $page->setTechnicalName(Page::DEFAULT_NAME_PREFIX . $page->getId()); + return $this->update($page); + }, $this->db); + } + /** * @return int[] */ diff --git a/lib/Db/Widget.php b/lib/Db/Widget.php new file mode 100644 index 0000000000..45dba3831b --- /dev/null +++ b/lib/Db/Widget.php @@ -0,0 +1,107 @@ +addType('id', 'integer'); + $this->addType('pageId', 'integer'); + $this->addType('gridX', 'integer'); + $this->addType('gridY', 'integer'); + $this->addType('gridWidth', 'integer'); + $this->addType('gridHeight', 'integer'); + } + + /** + * @return array + */ + public function getSettingsArray(): array { + $settings = $this->getSettings(); + if ($settings === null || $settings === '') { + return []; + } + $decoded = json_decode($settings, true); + /** @var array */ + return is_array($decoded) ? $decoded : []; + } + + /** + * Stores the settings as one JSON object, `{}` when there are none. + * + * @param array $settings + * @throws \JsonException + */ + public function setSettingsArray(array $settings): void { + $this->setSettings(json_encode((object)$settings, JSON_THROW_ON_ERROR)); + } + + /** + * The settings as an object, so they serialise as a JSON object also when empty. + */ + public function getSettingsObject(): \stdClass { + return (object)$this->getSettingsArray(); + } + + public function jsonSerialize(): array { + return [ + 'id' => $this->getId(), + 'pageId' => $this->getPageId(), + 'type' => $this->getType(), + 'title' => $this->getTitle(), + 'icon' => $this->getIcon(), + 'description' => $this->getDescription(), + 'gridX' => $this->getGridX(), + 'gridY' => $this->getGridY(), + 'gridWidth' => $this->getGridWidth(), + 'gridHeight' => $this->getGridHeight(), + 'settings' => $this->getSettingsObject(), + ]; + } +} diff --git a/lib/Db/WidgetMapper.php b/lib/Db/WidgetMapper.php new file mode 100644 index 0000000000..9843b30c77 --- /dev/null +++ b/lib/Db/WidgetMapper.php @@ -0,0 +1,78 @@ + */ +class WidgetMapper extends QBMapper { + protected string $table = 'tables_contexts_widget'; + + public function __construct(IDBConnection $db) { + parent::__construct($db, $this->table, Widget::class); + } + + /** + * @param int[] $pageIds + * @return Widget[] ordered by page and grid position + * @throws Exception + */ + public function findByPageIds(array $pageIds): array { + if ($pageIds === []) { + return []; + } + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->table) + ->where($qb->expr()->in('page_id', $qb->createNamedParameter($pageIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('page_id', 'ASC') + ->addOrderBy('grid_y', 'ASC') + ->addOrderBy('grid_x', 'ASC') + ->addOrderBy('id', 'ASC'); + + return $this->findEntities($qb); + } + + /** + * @return int[] + * @throws Exception + */ + public function findIdsByPageId(int $pageId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('id') + ->from($this->table) + ->where($qb->expr()->eq('page_id', $qb->createNamedParameter($pageId, IQueryBuilder::PARAM_INT))); + + $result = $qb->executeQuery(); + $ids = []; + while ($row = $result->fetchAssociative()) { + $ids[] = (int)$row['id']; + } + $result->closeCursor(); + return $ids; + } + + /** + * @param int[] $widgetIds + * @throws Exception + */ + public function deleteByIds(array $widgetIds): void { + if ($widgetIds === []) { + return; + } + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->table) + ->where($qb->expr()->in('id', $qb->createNamedParameter($widgetIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + } +} diff --git a/lib/Db/WidgetSource.php b/lib/Db/WidgetSource.php new file mode 100644 index 0000000000..30091a6d2f --- /dev/null +++ b/lib/Db/WidgetSource.php @@ -0,0 +1,31 @@ +addType('widgetId', 'integer'); + $this->addType('nodeRelId', 'integer'); + } +} diff --git a/lib/Db/WidgetSourceMapper.php b/lib/Db/WidgetSourceMapper.php new file mode 100644 index 0000000000..9041e7d849 --- /dev/null +++ b/lib/Db/WidgetSourceMapper.php @@ -0,0 +1,99 @@ + + */ +class WidgetSourceMapper extends QBMapper { + protected string $table = 'tables_contexts_widget_source'; + + public function __construct(IDBConnection $db) { + parent::__construct($db, $this->table, WidgetSource::class); + } + + /** + * @param int[] $widgetIds + * @return array node relation ids by widget id + * @throws Exception + */ + public function findNodeRelIdsByWidgetIds(array $widgetIds): array { + if ($widgetIds === []) { + return []; + } + $qb = $this->db->getQueryBuilder(); + $qb->select('widget_id', 'node_rel_id') + ->from($this->table) + ->where($qb->expr()->in('widget_id', $qb->createNamedParameter($widgetIds, IQueryBuilder::PARAM_INT_ARRAY))); + + $result = $qb->executeQuery(); + $sources = []; + while ($row = $result->fetchAssociative()) { + $sources[(int)$row['widget_id']] = (int)$row['node_rel_id']; + } + $result->closeCursor(); + return $sources; + } + + /** + * Points a widget at a node relation, or removes its source when null. + * + * @throws Exception + */ + public function setSource(int $widgetId, ?int $nodeRelId): void { + $this->deleteByWidgetIds([$widgetId]); + if ($nodeRelId === null) { + return; + } + $qb = $this->db->getQueryBuilder(); + $qb->insert($this->table) + ->values([ + 'widget_id' => $qb->createNamedParameter($widgetId, IQueryBuilder::PARAM_INT), + 'node_rel_id' => $qb->createNamedParameter($nodeRelId, IQueryBuilder::PARAM_INT), + ]) + ->executeStatement(); + } + + /** + * @param int[] $widgetIds + * @throws Exception + */ + public function deleteByWidgetIds(array $widgetIds): void { + if ($widgetIds === []) { + return; + } + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->table) + ->where($qb->expr()->in('widget_id', $qb->createNamedParameter($widgetIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + } + + /** + * @param int[] $nodeRelIds + * @throws Exception + */ + public function deleteByNodeRelIds(array $nodeRelIds): void { + if ($nodeRelIds === []) { + return; + } + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->table) + ->where($qb->expr()->in('node_rel_id', $qb->createNamedParameter($nodeRelIds, IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + } +} diff --git a/lib/Helper/IconHelper.php b/lib/Helper/IconHelper.php new file mode 100644 index 0000000000..a007b06946 --- /dev/null +++ b/lib/Helper/IconHelper.php @@ -0,0 +1,22 @@ +`. + */ +class Version2400Date20261022000000 extends SimpleMigrationStep { + private const PREFIX = 'tables_contexts_'; + private const PAGE_TABLE = self::PREFIX . 'page'; + private const PAGE_NAME_INDEX = 'tables_ctx_page_name_uq'; + private const START_PAGE_NAME = 'dashboard'; + + public function __construct( + private readonly IDBConnection $db, + ) { + } + + #[Override] + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $this->extendPageTable($schema); + $this->haveWidgetTable($schema); + $this->haveWidgetSourceTable($schema); + + return $schema; + } + + private function extendPageTable(ISchemaWrapper $schema): void { + if (!$schema->hasTable(self::PAGE_TABLE)) { + return; + } + $table = $schema->getTable(self::PAGE_TABLE); + if (!$table->hasColumn('title')) { + $table->addColumn('title', Types::STRING, ['notnull' => false, 'length' => 200]); + } + if (!$table->hasColumn('icon')) { + $table->addColumn('icon', Types::STRING, ['notnull' => false, 'length' => 64]); + } + if (!$table->hasColumn('description')) { + $table->addColumn('description', Types::TEXT, ['notnull' => false]); + } + if (!$table->hasColumn('technical_name')) { + $table->addColumn('technical_name', Types::STRING, ['notnull' => false, 'length' => TechnicalNameValidator::MAX_LENGTH]); + } + // Also serves lookups by context_id. Safe while existing pages have no name yet: + // pgsql, mysql and sqlite, the databases Tables supports, treat NULLs in a unique + // index as distinct. + if (!$table->hasIndex(self::PAGE_NAME_INDEX)) { + $table->addUniqueIndex(['context_id', 'technical_name'], self::PAGE_NAME_INDEX); + } + } + + #[Override] + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $select = $this->db->getQueryBuilder(); + $select->select('id', 'context_id', 'page_type', 'technical_name') + ->from(self::PAGE_TABLE) + ->orderBy('context_id') + ->addOrderBy('id'); + $result = $select->executeQuery(); + $rows = $result->fetchAllAssociative(); + $result->closeCursor(); + + $names = self::backfillNames($rows); + if ($names === []) { + return; + } + + $update = $this->db->getQueryBuilder(); + $update->update(self::PAGE_TABLE) + ->set('technical_name', $update->createParameter('technicalName')) + ->where($update->expr()->eq('id', $update->createParameter('pageId'))); + + $this->db->beginTransaction(); + try { + foreach ($names as $pageId => $technicalName) { + $update->setParameter('technicalName', $technicalName); + $update->setParameter('pageId', $pageId, IQueryBuilder::PARAM_INT); + $update->executeStatement(); + } + $this->db->commit(); + } catch (\Throwable $e) { + $this->db->rollBack(); + throw $e; + } + } + + /** + * The technical name of every page row that has none, by page id. The start page of + * an application gets `dashboard`, unless an earlier page holds it; every other page + * gets `page_`. + * + * @param list> $rows page rows + * @return array + */ + public static function backfillNames(array $rows): array { + usort($rows, static fn (array $a, array $b): int => [(int)$a['context_id'], (int)$a['id']] <=> [(int)$b['context_id'], (int)$b['id']]); + + $taken = []; + foreach ($rows as $row) { + if (is_string($row['technical_name'] ?? null) && $row['technical_name'] !== '') { + $taken[(int)$row['context_id']][$row['technical_name']] = true; + } + } + + $names = []; + foreach ($rows as $row) { + if (is_string($row['technical_name'] ?? null) && $row['technical_name'] !== '') { + continue; + } + $contextId = (int)$row['context_id']; + $technicalName = $row['page_type'] === Page::TYPE_STARTPAGE && !isset($taken[$contextId][self::START_PAGE_NAME]) + ? self::START_PAGE_NAME + : 'page_' . (int)$row['id']; + $taken[$contextId][$technicalName] = true; + $names[(int)$row['id']] = $technicalName; + } + return $names; + } + + private function haveWidgetTable(ISchemaWrapper $schema): void { + if ($schema->hasTable(self::PREFIX . 'widget')) { + return; + } + $table = $schema->createTable(self::PREFIX . 'widget'); + $table->addColumn('id', Types::INTEGER, ['autoincrement' => true, 'notnull' => true]); + $table->addColumn('page_id', Types::INTEGER, ['notnull' => true]); + $table->addColumn('type', Types::STRING, ['notnull' => true, 'length' => 64]); + $table->addColumn('title', Types::STRING, ['notnull' => false, 'length' => 200]); + $table->addColumn('icon', Types::STRING, ['notnull' => false, 'length' => 64]); + $table->addColumn('description', Types::TEXT, ['notnull' => false]); + $table->addColumn('grid_x', Types::SMALLINT, ['notnull' => true, 'default' => 0]); + $table->addColumn('grid_y', Types::INTEGER, ['notnull' => true, 'default' => 0]); + $table->addColumn('grid_width', Types::SMALLINT, ['notnull' => true, 'default' => 1]); + $table->addColumn('grid_height', Types::SMALLINT, ['notnull' => true, 'default' => 1]); + $table->addColumn('settings', Types::TEXT, ['notnull' => false]); + $table->setPrimaryKey(['id']); + $table->addIndex(['page_id'], 'tables_ctx_widget_page_idx'); + } + + private function haveWidgetSourceTable(ISchemaWrapper $schema): void { + if ($schema->hasTable(self::PREFIX . 'widget_source')) { + return; + } + $table = $schema->createTable(self::PREFIX . 'widget_source'); + $table->addColumn('widget_id', Types::INTEGER, ['notnull' => true]); + $table->addColumn('node_rel_id', Types::INTEGER, ['notnull' => true]); + $table->setPrimaryKey(['widget_id']); + $table->addIndex(['node_rel_id'], 'tables_ctx_wsource_node_idx'); + } +} diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index 468d3de83e..d3c161163b 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -30,6 +30,7 @@ use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\ViewSettings; +use OCA\Tables\Validation\TechnicalNameValidator; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; @@ -525,6 +526,7 @@ protected function insertPage(Context $context): void { $page = new Page(); $page->setContextId($context->getId()); $page->setPageType(Page::TYPE_STARTPAGE); + $page->setTechnicalName(Page::START_PAGE_NAME); $this->pageMapper->insert($page); $addedPage = $page->jsonSerialize(); @@ -631,11 +633,17 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C // insert pages $pageMap = []; if (!empty($context['pages'])) { - foreach ($context['pages'] as $pageArr) { + $technicalNames = $this->importedPageNames($context['pages']); + foreach ($context['pages'] as $pageKey => $pageArr) { $page = new Page(); $page->setContextId($newContext->getId()); $page->setPageType($pageArr['page_type'] ?? $pageArr['pageType'] ?? ''); - $page = $this->pageMapper->insert($page); + if ($technicalNames[$pageKey] === null) { + $page = $this->pageMapper->insertWithDefaultName($page); + } else { + $page->setTechnicalName($technicalNames[$pageKey]); + $page = $this->pageMapper->insert($page); + } $pageMap[$pageArr['id'] ?? null] = $page; // insert page conten @@ -656,6 +664,36 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C return $newContext; } + /** + * 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 + * reserved form `page_`, a start page without one gets `dashboard` while it is + * free, and null leaves the default name. + * + * @param array $pages exported pages + * @return array + */ + private function importedPageNames(array $pages): array { + $names = []; + $taken = []; + foreach ($pages as $key => $pageArr) { + $name = is_array($pageArr) ? ($pageArr['technical_name'] ?? $pageArr['technicalName'] ?? null) : null; + $names[$key] = null; + if (is_string($name) && TechnicalNameValidator::isValid($name) && !Page::isDefaultName($name) && !isset($taken[$name])) { + $names[$key] = $name; + $taken[$name] = true; + } + } + foreach ($pages as $key => $pageArr) { + $isStartPage = is_array($pageArr) && ($pageArr['page_type'] ?? $pageArr['pageType'] ?? '') === Page::TYPE_STARTPAGE; + if ($names[$key] === null && $isStartPage && !isset($taken[Page::START_PAGE_NAME])) { + $names[$key] = Page::START_PAGE_NAME; + $taken[Page::START_PAGE_NAME] = true; + } + } + return $names; + } + /** * @param int $contextId * diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index c50ae3aadd..3742a7c7cc 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -36,6 +36,7 @@ use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ValueObject\ViewColumnInformation; +use OCA\Tables\Validation\TechnicalNameValidator; use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -811,10 +812,10 @@ private function buildDefaultTechnicalName(int $viewId): string { * @throws BadRequestError */ private function assertTechnicalNameValid(string $technicalName): void { - if (strlen($technicalName) > 200) { - throw new BadRequestError('Technical name must not exceed 200 characters.'); + if (TechnicalNameValidator::isTooLong($technicalName)) { + throw new BadRequestError('Technical name must not exceed ' . TechnicalNameValidator::MAX_LENGTH . ' characters.'); } - if (!preg_match('/^[a-z][a-z0-9_]*$/', $technicalName)) { + if (!TechnicalNameValidator::hasValidCharacters($technicalName)) { throw new BadRequestError('Technical name must start with a lowercase letter and contain only lowercase letters, numbers, and underscores.'); } } diff --git a/lib/Validation/ColumnDtoValidator.php b/lib/Validation/ColumnDtoValidator.php index b2e0c1da49..058763c665 100644 --- a/lib/Validation/ColumnDtoValidator.php +++ b/lib/Validation/ColumnDtoValidator.php @@ -12,8 +12,6 @@ use OCA\Tables\Service\ValueObject\Title; class ColumnDtoValidator { - private const TECHNICAL_NAME_MAX_LENGTH = 200; - /** * reserved keys to avoid confusion with meta fields and common row keys. */ @@ -69,15 +67,15 @@ private function validateTechnicalName(ColumnDto $columnDto): void { throw new BadRequestError('Technical name must not be empty.'); } - if (strlen($technicalName) > self::TECHNICAL_NAME_MAX_LENGTH) { - throw new BadRequestError('Technical name must be maximum ' . self::TECHNICAL_NAME_MAX_LENGTH . ' characters long.'); + if (TechnicalNameValidator::isTooLong($technicalName)) { + throw new BadRequestError('Technical name must be maximum ' . TechnicalNameValidator::MAX_LENGTH . ' characters long.'); } if (in_array($technicalName, self::RESERVED_TECHNICAL_NAMES, true)) { throw new BadRequestError('Technical name is reserved.'); } - if (preg_match('/^[a-z][a-z0-9_]*$/', $technicalName) !== 1) { + if (!TechnicalNameValidator::hasValidCharacters($technicalName)) { throw new BadRequestError('Technical name must start with a letter and contain only lowercase letters, digits, and underscores.'); } } diff --git a/lib/Validation/TechnicalNameValidator.php b/lib/Validation/TechnicalNameValidator.php new file mode 100644 index 0000000000..f93af6856b --- /dev/null +++ b/lib/Validation/TechnicalNameValidator.php @@ -0,0 +1,30 @@ + self::MAX_LENGTH; + } + + public static function hasValidCharacters(string $technicalName): bool { + return preg_match(self::PATTERN, $technicalName) === 1; + } +} diff --git a/package-lock.json b/package-lock.json index 9b5294e5ff..d058728a3f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "tables", - "version": "2.4.0-dev.0", + "version": "2.4.0-dev.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tables", - "version": "2.4.0-dev.0", + "version": "2.4.0-dev.1", "license": "AGPL-3.0-or-later", "dependencies": { "@material-symbols/svg-400": "^0.48.0", diff --git a/package.json b/package.json index 6a504ce3da..9864f0075b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "tables", "description": "Manage data within tables.", - "version": "2.4.0-dev.0", + "version": "2.4.0-dev.1", "author": "Nextcloud GmbH and Nextcloud contributors", "bugs": { "url": "https://github.com/nextcloud/tables/issues" diff --git a/tests/unit/Database/DbErrorStub.php b/tests/unit/Database/DbErrorStub.php new file mode 100644 index 0000000000..17ac605167 --- /dev/null +++ b/tests/unit/Database/DbErrorStub.php @@ -0,0 +1,31 @@ +reason; + } +} diff --git a/tests/unit/Db/PageTest.php b/tests/unit/Db/PageTest.php new file mode 100644 index 0000000000..e30ab5507d --- /dev/null +++ b/tests/unit/Db/PageTest.php @@ -0,0 +1,127 @@ +mapper = new PageMapper($this->connectionAdapter); + } + + protected function tearDown(): void { + foreach ([self::CONTEXT, self::OTHER_CONTEXT] as $contextId) { + foreach ($this->mapper->getPageIdsForContext($contextId) as $pageId) { + $this->mapper->deleteByPageId($pageId); + } + } + parent::tearDown(); + } + + public function testAGridPageKeepsItsTitleIconAndDescription(): void { + $page = new Page(); + $page->setContextId(self::CONTEXT); + $page->setPageType(Page::TYPE_GRID); + $page->setTechnicalName('overview'); + $page->setTitle('Overview'); + $page->setIcon('star'); + $page->setDescription('Open tasks by team'); + $page = $this->mapper->insert($page); + + $this->assertSame([ + 'id' => $page->getId(), + 'contextId' => self::CONTEXT, + 'pageType' => 'grid', + 'technicalName' => 'overview', + 'title' => 'Overview', + 'icon' => 'star', + 'description' => 'Open tasks by team', + ], $this->load($page->getId())->jsonSerialize()); + } + + public function testAStartPageNeedsNoTitleIconOrDescription(): void { + $page = new Page(); + $page->setContextId(self::CONTEXT); + $page->setPageType(Page::TYPE_STARTPAGE); + $page->setTechnicalName('dashboard'); + $page = $this->mapper->insert($page); + + $loaded = $this->load($page->getId()); + $this->assertSame(Page::TYPE_STARTPAGE, $loaded->getPageType()); + $this->assertNull($loaded->getTitle()); + $this->assertNull($loaded->getIcon()); + $this->assertNull($loaded->getDescription()); + } + + public function testAPageWithoutAGivenNameIsNamedByItsId(): void { + $page = new Page(); + $page->setContextId(self::CONTEXT); + $page->setPageType(Page::TYPE_GRID); + $page->setTitle('Overview'); + $page = $this->mapper->insertWithDefaultName($page); + + $this->assertSame('page_' . $page->getId(), $page->getTechnicalName()); + $this->assertSame('page_' . $page->getId(), $this->load($page->getId())->getTechnicalName()); + } + + public function testOnlyTheAutomaticFormIsADefaultName(): void { + $this->assertTrue(Page::isDefaultName('page_7')); + $this->assertTrue(Page::isDefaultName('page_123456')); + foreach (['page', 'page_', 'page_7a', 'page_x', 'my_page_7', 'item_7', 'dashboard'] as $name) { + $this->assertFalse(Page::isDefaultName($name), $name); + } + } + + public function testATechnicalNameIsUniqueWithinAnApplication(): void { + $this->insertNamed(self::CONTEXT, 'sales'); + $this->insertNamed(self::OTHER_CONTEXT, 'sales'); + + try { + $this->insertNamed(self::CONTEXT, 'sales'); + $this->fail('The unique index accepted a second page named sales'); + } catch (Exception $e) { + $this->assertSame(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION, $e->getReason()); + } + $this->assertCount(1, $this->mapper->getPageIdsForContext(self::CONTEXT)); + } + + private function insertNamed(int $contextId, string $technicalName): Page { + $page = new Page(); + $page->setContextId($contextId); + $page->setPageType(Page::TYPE_GRID); + $page->setTechnicalName($technicalName); + return $this->mapper->insert($page); + } + + private function load(int $pageId): Page { + $qb = $this->connectionAdapter->getQueryBuilder(); + $qb->select('*') + ->from('tables_contexts_page') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($pageId, IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + $row = $result->fetchAssociative(); + $result->closeCursor(); + $this->assertIsArray($row); + return Page::fromRow($row); + } +} diff --git a/tests/unit/Db/WidgetMapperTest.php b/tests/unit/Db/WidgetMapperTest.php new file mode 100644 index 0000000000..75e0219e17 --- /dev/null +++ b/tests/unit/Db/WidgetMapperTest.php @@ -0,0 +1,123 @@ +mapper = new WidgetMapper($this->connectionAdapter); + } + + protected function tearDown(): void { + $this->mapper->deleteByIds(array_merge( + $this->mapper->findIdsByPageId(self::PAGE), + $this->mapper->findIdsByPageId(self::OTHER_PAGE), + )); + parent::tearDown(); + } + + public function testAWidgetIsStoredWithAllItsFields(): void { + $widget = $this->widget(self::PAGE, 'header', 'Welcome', 2, 3, 8, 4, ['heading' => 'Projects', 'showTitle' => false]); + $widget->setIcon('star'); + $widget->setDescription('What the team works on'); + $inserted = $this->mapper->insert($widget); + + $found = $this->mapper->findByPageIds([self::PAGE]); + + $this->assertCount(1, $found); + $serialized = $found[0]->jsonSerialize(); + $serialized['settings'] = json_encode($serialized['settings']); + $this->assertSame([ + 'id' => $inserted->getId(), + 'pageId' => self::PAGE, + 'type' => 'header', + 'title' => 'Welcome', + 'icon' => 'star', + 'description' => 'What the team works on', + 'gridX' => 2, + 'gridY' => 3, + 'gridWidth' => 8, + 'gridHeight' => 4, + 'settings' => '{"heading":"Projects","showTitle":false}', + ], $serialized); + } + + public function testWidgetsAreFoundByPageInGridOrder(): void { + $bottom = $this->insert(self::PAGE, 'data', null, 0, 5); + $topRight = $this->insert(self::PAGE, 'text', null, 6, 0); + $topLeft = $this->insert(self::PAGE, 'text', null, 0, 0); + $other = $this->insert(self::OTHER_PAGE, 'text', null, 0, 0); + + $ids = static fn (array $widgets): array => array_map(static fn (Widget $widget): int => $widget->getId(), $widgets); + + $this->assertSame([$topLeft->getId(), $topRight->getId(), $bottom->getId()], $ids($this->mapper->findByPageIds([self::PAGE]))); + $this->assertSame( + [$topLeft->getId(), $topRight->getId(), $bottom->getId(), $other->getId()], + $ids($this->mapper->findByPageIds([self::OTHER_PAGE, self::PAGE])), + ); + $this->assertSame([], $this->mapper->findByPageIds([])); + } + + public function testIdsOfAPageAreListed(): void { + $first = $this->insert(self::PAGE, 'text', null, 0, 0); + $second = $this->insert(self::PAGE, 'text', null, 0, 1); + $this->insert(self::OTHER_PAGE, 'text', null, 0, 0); + + $ids = $this->mapper->findIdsByPageId(self::PAGE); + sort($ids); + + $this->assertSame([$first->getId(), $second->getId()], $ids); + } + + public function testOnlyTheGivenWidgetsAreDeleted(): void { + $deleted = $this->insert(self::PAGE, 'text', null, 0, 0); + $kept = $this->insert(self::PAGE, 'text', null, 0, 1); + + $this->mapper->deleteByIds([$deleted->getId()]); + $this->mapper->deleteByIds([]); + + $this->assertSame([$kept->getId()], $this->mapper->findIdsByPageId(self::PAGE)); + } + + /** + * @param array $settings + */ + private function insert(int $pageId, string $type, ?string $title, int $x, int $y, int $width = 6, int $height = 2, array $settings = []): Widget { + return $this->mapper->insert($this->widget($pageId, $type, $title, $x, $y, $width, $height, $settings)); + } + + /** + * @param array $settings + */ + private function widget(int $pageId, string $type, ?string $title, int $x, int $y, int $width = 6, int $height = 2, array $settings = []): Widget { + $widget = new Widget(); + $widget->setPageId($pageId); + $widget->setType($type); + $widget->setTitle($title); + $widget->setGridX($x); + $widget->setGridY($y); + $widget->setGridWidth($width); + $widget->setGridHeight($height); + $widget->setSettingsArray($settings); + return $widget; + } +} diff --git a/tests/unit/Db/WidgetSourceMapperTest.php b/tests/unit/Db/WidgetSourceMapperTest.php new file mode 100644 index 0000000000..7d0ba0ac62 --- /dev/null +++ b/tests/unit/Db/WidgetSourceMapperTest.php @@ -0,0 +1,84 @@ +mapper = new WidgetSourceMapper($this->connectionAdapter); + } + + protected function tearDown(): void { + $this->mapper->deleteByWidgetIds(self::WIDGETS); + parent::tearDown(); + } + + public function testSourcesAreReadByWidgetId(): void { + [$first, $second, $third] = self::WIDGETS; + $this->mapper->setSource($first, 11); + $this->mapper->setSource($second, 12); + + $this->assertSame([$first => 11, $second => 12], $this->sorted($this->mapper->findNodeRelIdsByWidgetIds(self::WIDGETS))); + $this->assertSame([], $this->mapper->findNodeRelIdsByWidgetIds([$third])); + $this->assertSame([], $this->mapper->findNodeRelIdsByWidgetIds([])); + } + + public function testSettingASourceReplacesTheOldOneAndNullRemovesIt(): void { + [$first, $second] = self::WIDGETS; + $this->mapper->setSource($first, 21); + $this->mapper->setSource($second, 22); + + $this->mapper->setSource($first, 23); + $this->mapper->setSource($second, null); + + $this->assertSame([$first => 23], $this->mapper->findNodeRelIdsByWidgetIds(self::WIDGETS)); + } + + public function testDeletingByWidgetKeepsOtherWidgetsSources(): void { + [$first, $second] = self::WIDGETS; + $this->mapper->setSource($first, 31); + $this->mapper->setSource($second, 32); + + $this->mapper->deleteByWidgetIds([$first]); + + $this->assertSame([$second => 32], $this->mapper->findNodeRelIdsByWidgetIds(self::WIDGETS)); + } + + public function testDeletingByNodeRelationRemovesEverySourceShowingIt(): void { + [$first, $second, $third] = self::WIDGETS; + $this->mapper->setSource($first, 41); + $this->mapper->setSource($second, 41); + $this->mapper->setSource($third, 42); + + $this->mapper->deleteByNodeRelIds([41]); + $this->mapper->deleteByNodeRelIds([]); + + $this->assertSame([$third => 42], $this->mapper->findNodeRelIdsByWidgetIds(self::WIDGETS)); + } + + /** + * @param array $sources + * @return array + */ + private function sorted(array $sources): array { + ksort($sources); + return $sources; + } +} diff --git a/tests/unit/Db/WidgetTest.php b/tests/unit/Db/WidgetTest.php new file mode 100644 index 0000000000..0fa92e0fb9 --- /dev/null +++ b/tests/unit/Db/WidgetTest.php @@ -0,0 +1,67 @@ +setSettingsArray(['text' => 'Projects', 'showTitle' => false, 'level' => 2, 'ratio' => 1.5, 'color' => null]); + + $this->assertSame('{"text":"Projects","showTitle":false,"level":2,"ratio":1.5,"color":null}', $widget->getSettings()); + $this->assertSame(['text' => 'Projects', 'showTitle' => false, 'level' => 2, 'ratio' => 1.5, 'color' => null], $widget->getSettingsArray()); + } + + public function testNoSettingsAreStoredAsAnEmptyObject(): void { + $widget = new Widget(); + $widget->setSettingsArray([]); + + $this->assertSame('{}', $widget->getSettings()); + $this->assertSame([], $widget->getSettingsArray()); + } + + public function testNoStoredValueReadsAsNoSettings(): void { + $this->assertSame([], (new Widget())->getSettingsArray()); + } + + /** + * A stored value that is not a JSON object reads as no settings rather than failing. + */ + public function testAStoredValueThatIsNotAnObjectReadsAsNoSettings(): void { + $widget = new Widget(); + foreach (['', '"text"', '42', 'null', '{broken'] as $stored) { + $widget->setSettings($stored); + $this->assertSame([], $widget->getSettingsArray(), $stored); + } + } + + public function testSettingsSerialiseAsAJsonObjectAlsoWhenEmpty(): void { + $widget = new Widget(); + $widget->setSettingsArray([]); + $this->assertSame('{}', json_encode($widget->jsonSerialize()['settings'])); + + $widget->setSettingsArray(['text' => 'Projects']); + $this->assertSame('{"text":"Projects"}', json_encode($widget->jsonSerialize()['settings'])); + } + + /** + * Databases may hand integers back as strings; the entities cast them. + */ + public function testGridValuesAndIdsComeBackAsIntegers(): void { + $widget = Widget::fromRow(['id' => '7', 'page_id' => '3', 'grid_x' => '1', 'grid_y' => '20', 'grid_width' => '6', 'grid_height' => '4']); + $this->assertSame([7, 3, 1, 20, 6, 4], [$widget->getId(), $widget->getPageId(), $widget->getGridX(), $widget->getGridY(), $widget->getGridWidth(), $widget->getGridHeight()]); + + $source = WidgetSource::fromRow(['widget_id' => '7', 'node_rel_id' => '12']); + $this->assertSame([7, 12], [$source->getWidgetId(), $source->getNodeRelId()]); + } +} diff --git a/tests/unit/Helper/IconHelperTest.php b/tests/unit/Helper/IconHelperTest.php new file mode 100644 index 0000000000..37ef05b5c6 --- /dev/null +++ b/tests/unit/Helper/IconHelperTest.php @@ -0,0 +1,26 @@ +assertTrue(IconHelper::isValid('star')); + $this->assertTrue(IconHelper::isValid('home')); + } + + public function testAnUnknownOrMalformedNameIsNotValid(): void { + foreach (['', 'no-such-icon', '../img/material/star', 'star.svg', 'star ', '📊'] as $name) { + $this->assertFalse(IconHelper::isValid($name), $name); + } + } +} diff --git a/tests/unit/Migration/Version2400Date20261022000000Test.php b/tests/unit/Migration/Version2400Date20261022000000Test.php new file mode 100644 index 0000000000..3c27b12cb3 --- /dev/null +++ b/tests/unit/Migration/Version2400Date20261022000000Test.php @@ -0,0 +1,174 @@ +migration = new Version2400Date20261022000000($this->connectionAdapter); + } + + protected function tearDown(): void { + $qb = $this->connectionAdapter->getQueryBuilder(); + $qb->delete('tables_contexts_page') + ->where($qb->expr()->in('context_id', $qb->createNamedParameter([self::CONTEXT, self::OTHER_CONTEXT], IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + parent::tearDown(); + } + + public function testTheUpgradeNamesExistingPagesAndASecondRunChangesNothing(): void { + $startPage = $this->insertPage(self::CONTEXT, 'startpage', null, null); + $secondStartPage = $this->insertPage(self::CONTEXT, 'startpage', null, null); + $sales = $this->insertPage(self::CONTEXT, 'grid', 'Sales', null); + $untitled = $this->insertPage(self::CONTEXT, 'grid', null, null); + $otherStartPage = $this->insertPage(self::OTHER_CONTEXT, 'startpage', null, null); + + $this->migration->postSchemaChange(new NullOutput(), fn () => $this->schema(), []); + + $expected = [ + $startPage => 'dashboard', + $secondStartPage => 'page_' . $secondStartPage, + $sales => 'page_' . $sales, + $untitled => 'page_' . $untitled, + $otherStartPage => 'dashboard', + ]; + $this->assertSame($expected, $this->names()); + + $this->migration->postSchemaChange(new NullOutput(), fn () => $this->schema(), []); + $this->assertSame($expected, $this->names()); + } + + public function testASecondSchemaRunChangesNothing(): void { + $schema = $this->schema(); + $before = $this->describePageTable($schema); + + $this->migration->changeSchema(new NullOutput(), fn () => $schema, []); + + $this->assertSame($before, $this->describePageTable($schema)); + $this->assertContains('technical_name', $before['columns']); + $this->assertContains('tables_ctx_page_name_uq', $before['indexes']); + // The unique index starts with context_id and serves lookups by it. + $this->assertNotContains('tables_ctx_page_ctx_idx', $before['indexes']); + } + + public function testTheUniqueIndexRejectsASecondPageWithTheSameName(): void { + $this->insertPage(self::CONTEXT, 'grid', 'Sales', 'sales'); + $this->insertPage(self::OTHER_CONTEXT, 'grid', 'Sales', 'sales'); + + try { + $this->insertPage(self::CONTEXT, 'grid', 'Sales', 'sales'); + $this->fail('The unique index accepted a second page named sales'); + } catch (Exception $e) { + $this->assertSame(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION, $e->getReason()); + } + } + + public function testTheDatabaseAllowsPagesWithoutANameThatTheCodeNeverWrites(): void { + $this->assertFalse($this->schema()->getTable('tables_contexts_page')->getColumn('technical_name')->getNotnull()); + + $first = $this->insertPage(self::CONTEXT, 'grid', null, null); + $second = $this->insertPage(self::CONTEXT, 'grid', null, null); + + $this->assertSame([$first => null, $second => null], $this->names()); + } + public function testBackfillNamesTheStartPageDashboardAndOtherPagesByTheirId(): void { + $names = Version2400Date20261022000000::backfillNames([ + ['id' => 1, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => null], + ['id' => 2, 'context_id' => 10, 'page_type' => 'grid', 'technical_name' => null], + ['id' => 3, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => ''], + ['id' => 6, 'context_id' => 11, 'page_type' => 'startpage', 'technical_name' => null], + ]); + + $this->assertSame([1 => 'dashboard', 2 => 'page_2', 3 => 'page_3', 6 => 'dashboard'], $names); + } + + public function testBackfillLeavesNamedPagesAloneAndGivesTheirNamesToNoOne(): void { + $this->assertSame([], Version2400Date20261022000000::backfillNames([ + ['id' => 1, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => 'dashboard'], + ['id' => 2, 'context_id' => 10, 'page_type' => 'grid', 'technical_name' => 'sales'], + ])); + $this->assertSame([], Version2400Date20261022000000::backfillNames([])); + $this->assertSame([1 => 'page_1'], Version2400Date20261022000000::backfillNames([ + ['id' => 1, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => null], + ['id' => 2, 'context_id' => 10, 'page_type' => 'grid', 'technical_name' => 'dashboard'], + ])); + } + + public function testTheLowerPageIdKeepsDashboardWhateverTheRowOrder(): void { + $names = Version2400Date20261022000000::backfillNames([ + ['id' => 9, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => null], + ['id' => 3, 'context_id' => 10, 'page_type' => 'startpage', 'technical_name' => null], + ]); + + $this->assertSame([3 => 'dashboard', 9 => 'page_9'], $names); + } + + private function insertPage(int $contextId, string $pageType, ?string $title, ?string $technicalName): int { + $qb = $this->connectionAdapter->getQueryBuilder(); + $qb->insert('tables_contexts_page') + ->values([ + 'context_id' => $qb->createNamedParameter($contextId, IQueryBuilder::PARAM_INT), + 'page_type' => $qb->createNamedParameter($pageType), + 'title' => $qb->createNamedParameter($title), + 'technical_name' => $qb->createNamedParameter($technicalName), + ]) + ->executeStatement(); + return $qb->getLastInsertId(); + } + + /** + * @return array technical name by page id + */ + private function names(): array { + $qb = $this->connectionAdapter->getQueryBuilder(); + $qb->select('id', 'technical_name') + ->from('tables_contexts_page') + ->where($qb->expr()->in('context_id', $qb->createNamedParameter([self::CONTEXT, self::OTHER_CONTEXT], IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('id'); + $result = $qb->executeQuery(); + $names = []; + while ($row = $result->fetchAssociative()) { + $names[(int)$row['id']] = $row['technical_name']; + } + $result->closeCursor(); + return $names; + } + + private function schema(): SchemaWrapper { + return new SchemaWrapper($this->connection); + } + + /** + * @return array{columns: list, indexes: list} + */ + private function describePageTable(SchemaWrapper $schema): array { + $table = $schema->getTable('tables_contexts_page'); + $columns = array_map(static fn ($column) => $column->getName(), $table->getColumns()); + $indexes = array_map(static fn ($index) => $index->getName(), $table->getIndexes()); + sort($columns); + sort($indexes); + return ['columns' => $columns, 'indexes' => $indexes]; + } +} diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 266827b7cd..4e290e699c 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -16,6 +16,7 @@ use OCA\Tables\Db\PageContent; 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 OCP\EventDispatcher\IEventDispatcher; @@ -156,6 +157,108 @@ public function testUpdateReordersStartPageContentsFromSubmittedNodes(): void { self::assertSame(10, $updatedContext->getPages()[301]['content'][502]['order']); } + public function testANewApplicationGetsAStartPageNamedDashboard(): void { + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { + $context->id = 7; + return $context; + }); + $inserted = []; + $this->pageMapper->expects($this->once())->method('insert') + ->willReturnCallback(static function (Page $page) use (&$inserted): Page { + $page->id = 70; + $inserted[] = $page; + return $page; + }); + + $context = $this->service->create('Projects', 'star', '', [], 'user-1', 0); + + self::assertSame(Page::TYPE_STARTPAGE, $inserted[0]->getPageType()); + self::assertSame('dashboard', $inserted[0]->getTechnicalName()); + self::assertSame('dashboard', $context->getPages()[70]['technicalName']); + } + + public function testImportedPagesKeepFreeNamesAndTheRestGetTheDefault(): void { + $table = new Table(); + $table->id = 40; + $table->setOwnership('user-1'); + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { + $context->id = 8; + return $context; + }); + $this->contextNodeRelMapper->method('insert')->willReturnArgument(0); + $names = []; + $pageId = 80; + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page) use (&$names, &$pageId): Page { + $page->id = $pageId++; + $names[$page->getId()] = $page->getTechnicalName(); + return $page; + }); + $this->pageMapper->method('insertWithDefaultName')->willReturnCallback(static function (Page $page) use (&$names, &$pageId): Page { + $page->id = $pageId++; + $page->setTechnicalName('page_' . $page->getId()); + $names[$page->getId()] = $page->getTechnicalName(); + return $page; + }); + + $this->service->importContext($table, [ + 'name' => 'Projects', + 'nodes' => [['id' => 1, 'node_id' => 4, 'node_type' => 0]], + 'pages' => [ + ['id' => 1, 'page_type' => Page::TYPE_STARTPAGE], + ['id' => 2, 'page_type' => Page::TYPE_GRID, 'title' => 'Sales', 'technical_name' => 'sales'], + ['id' => 3, 'page_type' => Page::TYPE_GRID, 'title' => 'More sales', 'technical_name' => 'sales'], + ['id' => 4, 'page_type' => Page::TYPE_GRID, 'title' => 'Other', 'technical_name' => 'Bad Name'], + ['id' => 5, 'page_type' => Page::TYPE_GRID, 'title' => 'Team overview'], + ['id' => 6, 'page_type' => Page::TYPE_STARTPAGE, 'technicalName' => 'home'], + ['id' => 7, 'page_type' => Page::TYPE_GRID, 'title' => 'Copied', 'technical_name' => 'page_2'], + ], + ], 4); + + self::assertSame([ + 80 => 'dashboard', + 81 => 'sales', + 82 => 'page_82', + 83 => 'page_83', + 84 => 'page_84', + 85 => 'home', + 86 => 'page_86', + ], $names); + } + + public function testAnImportedStartPageLeavesDashboardToThePageExportedWithIt(): void { + $table = new Table(); + $table->id = 40; + $table->setOwnership('user-1'); + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { + $context->id = 8; + return $context; + }); + $this->contextNodeRelMapper->method('insert')->willReturnArgument(0); + $names = []; + $pageId = 80; + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page) use (&$names, &$pageId): Page { + $page->id = $pageId++; + $names[$page->getId()] = $page->getTechnicalName(); + return $page; + }); + $this->pageMapper->method('insertWithDefaultName')->willReturnCallback(static function (Page $page) use (&$names, &$pageId): Page { + $page->id = $pageId++; + $names[$page->getId()] = 'page_' . $page->getId(); + return $page; + }); + + $this->service->importContext($table, [ + 'name' => 'Projects', + 'nodes' => [['id' => 1, 'node_id' => 4, 'node_type' => 0]], + 'pages' => [ + ['id' => 1, 'page_type' => Page::TYPE_STARTPAGE], + ['id' => 2, 'page_type' => Page::TYPE_GRID, 'title' => 'Board', 'technical_name' => 'dashboard'], + ], + ], 4); + + self::assertSame([80 => 'page_80', 81 => 'dashboard'], $names); + } + private function buildPageContent(int $id, int $pageId, int $nodeRelId, int $order): PageContent { $pageContent = new PageContent(); $pageContent->id = $id; diff --git a/tests/unit/Service/ViewServiceTechnicalNameTest.php b/tests/unit/Service/ViewServiceTechnicalNameTest.php new file mode 100644 index 0000000000..d45907eac9 --- /dev/null +++ b/tests/unit/Service/ViewServiceTechnicalNameTest.php @@ -0,0 +1,47 @@ + [str_repeat('a', 201), 'Technical name must not exceed 200 characters.'], + 'empty' => ['', 'Technical name must start with a lowercase letter and contain only lowercase letters, numbers, and underscores.'], + 'upper case' => ['Sales', 'Technical name must start with a lowercase letter and contain only lowercase letters, numbers, and underscores.'], + 'starts with a digit' => ['1sales', 'Technical name must start with a lowercase letter and contain only lowercase letters, numbers, and underscores.'], + 'dash' => ['sales-2', 'Technical name must start with a lowercase letter and contain only lowercase letters, numbers, and underscores.'], + ]; + } + + /** + * @dataProvider invalidNames + */ + public function testAnInvalidNameIsRefusedWithItsMessage(string $technicalName, string $message): void { + $this->expectException(BadRequestError::class); + $this->expectExceptionMessage($message); + $this->assertValid($technicalName); + } + + public function testValidNamesAreAccepted(): void { + $this->expectNotToPerformAssertions(); + $this->assertValid('sales'); + $this->assertValid('view_12'); + $this->assertValid('a' . str_repeat('b', 199)); + } + + private function assertValid(string $technicalName): void { + $method = new \ReflectionMethod(ViewService::class, 'assertTechnicalNameValid'); + $method->invoke((new \ReflectionClass(ViewService::class))->newInstanceWithoutConstructor(), $technicalName); + } +} diff --git a/tests/unit/Validation/TechnicalNameValidatorTest.php b/tests/unit/Validation/TechnicalNameValidatorTest.php new file mode 100644 index 0000000000..c9326d3848 --- /dev/null +++ b/tests/unit/Validation/TechnicalNameValidatorTest.php @@ -0,0 +1,34 @@ +assertTrue(TechnicalNameValidator::isValid('open_cases_2')); + $this->assertTrue(TechnicalNameValidator::isValid(str_repeat('a', 200))); + foreach (['', 'Sales', '2026', '_sales', 'open-cases', 'café', 'has space', "sales\n", "sales\r\n", str_repeat('a', 201)] as $name) { + $this->assertFalse(TechnicalNameValidator::isValid($name), $name); + } + } + + public function testIsTooLong(): void { + $this->assertFalse(TechnicalNameValidator::isTooLong(str_repeat('a', 200))); + $this->assertTrue(TechnicalNameValidator::isTooLong(str_repeat('a', 201))); + } + + public function testHasValidCharactersIgnoresTheLength(): void { + $this->assertTrue(TechnicalNameValidator::hasValidCharacters(str_repeat('a', 201))); + $this->assertFalse(TechnicalNameValidator::hasValidCharacters('1sales')); + $this->assertFalse(TechnicalNameValidator::hasValidCharacters("sales\n")); + } +} diff --git a/tests/unit/bootstrap.php b/tests/unit/bootstrap.php index ce471574ad..f070cdbd48 100644 --- a/tests/unit/bootstrap.php +++ b/tests/unit/bootstrap.php @@ -20,6 +20,7 @@ require_once __DIR__ . '/../../../../tests/autoload.php'; require_once __DIR__ . '/Database/DatabaseTestCase.php'; +require_once __DIR__ . '/Database/DbErrorStub.php'; require_once __DIR__ . '/Db/Row2MapperTestDependencies.php';