From ada00045713bc1ae560e58c11fcd513282cf183f 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. The app version moves to 2.5.0-dev.0 so that instances on 2.4.0 run the migration. 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 | 110 +++++++++++ lib/Db/WidgetMapper.php | 78 ++++++++ lib/Db/WidgetSource.php | 31 +++ lib/Db/WidgetSourceMapper.php | 99 ++++++++++ lib/Helper/IconHelper.php | 22 +++ .../Version2500Date20261022000000.php | 176 ++++++++++++++++++ lib/Service/ContextService.php | 69 ++++++- 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 | 111 +++++++++++ tests/unit/Db/WidgetMapperTest.php | 123 ++++++++++++ tests/unit/Db/WidgetSourceMapperTest.php | 67 +++++++ tests/unit/Db/WidgetTest.php | 34 ++++ tests/unit/Helper/IconHelperTest.php | 26 +++ .../Version2500Date20261022000000Test.php | 153 +++++++++++++++ tests/unit/Service/ContextServiceTest.php | 85 +++++++++ .../Service/ViewServiceTechnicalNameTest.php | 37 ++++ .../Validation/TechnicalNameValidatorTest.php | 23 +++ tests/unit/bootstrap.php | 1 + 27 files changed, 1361 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/Version2500Date20261022000000.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/Version2500Date20261022000000Test.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 dd34d3a45f..4edcef88db 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 + 2.5.0-dev.0 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..db625c051d 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..8a0caa3fbd --- /dev/null +++ b/lib/Db/Widget.php @@ -0,0 +1,110 @@ +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, + * ordered by name: a JSON column does not keep the stored order on every database. + */ + public function getSettingsObject(): \stdClass { + $settings = $this->getSettingsArray(); + ksort($settings); + return (object)$settings; + } + + 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 Version2500Date20261022000000 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::JSON, ['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 828897ad6f..1294fda7db 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -32,6 +32,7 @@ use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\ViewSettings; use OCA\Tables\Service\Support\AuditLogServiceInterface; +use OCA\Tables\Validation\TechnicalNameValidator; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; @@ -547,6 +548,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(); @@ -603,6 +605,32 @@ protected function insertNodesFromArray(Context $context, array $nodes): void { $context->setNodes($addedNodes); } + /** + * The exported pages that can be imported: a start page or a grid page, and at most + * one start page. Others are skipped with a warning. + * + * @param array $pages exported pages + * @return array> + */ + private function importablePages(array $pages): array { + $importable = []; + $hasStartPage = false; + foreach ($pages as $key => $pageArr) { + $type = is_array($pageArr) ? ($pageArr['page_type'] ?? $pageArr['pageType'] ?? null) : null; + if ($type !== Page::TYPE_STARTPAGE && $type !== Page::TYPE_GRID) { + $this->logger->warning('Skipping an imported page: its type is not ' . Page::TYPE_STARTPAGE . ' or ' . Page::TYPE_GRID . '.'); + continue; + } + if ($type === Page::TYPE_STARTPAGE && $hasStartPage) { + $this->logger->warning('Skipping an imported page: an application has one start page.'); + continue; + } + $hasStartPage = $hasStartPage || $type === Page::TYPE_STARTPAGE; + $importable[$key] = $pageArr; + } + return $importable; + } + /** * @param Table $table * @param array $context @@ -653,11 +681,18 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C // insert pages $pageMap = []; if (!empty($context['pages'])) { - foreach ($context['pages'] as $pageArr) { + $pages = $this->importablePages($context['pages']); + $technicalNames = $this->importedPageNames($pages); + foreach ($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 @@ -678,6 +713,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 7ee532d32f..d2b18037a2 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -38,6 +38,7 @@ use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\Support\AuditLogServiceInterface; 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; @@ -809,10 +810,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 a142c3ca0f..3a5f9d5939 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "tables", - "version": "2.4.0", + "version": "2.5.0-dev.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tables", - "version": "2.4.0", + "version": "2.5.0-dev.0", "license": "AGPL-3.0-or-later", "dependencies": { "@material-symbols/svg-400": "^0.48.1", diff --git a/package.json b/package.json index 858a25a311..d3d93cceba 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "tables", "description": "Manage data within tables.", - "version": "2.4.0", + "version": "2.5.0-dev.0", "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..6f81f8d89a --- /dev/null +++ b/tests/unit/Db/PageTest.php @@ -0,0 +1,111 @@ +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 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 { + foreach (['page_7' => true, 'page_123456' => true, 'page' => false, 'page_' => false, 'page_7a' => false, 'my_page_7' => false, 'dashboard' => false] as $name => $isDefault) { + $this->assertSame($isDefault, 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..9b5161f968 --- /dev/null +++ b/tests/unit/Db/WidgetSourceMapperTest.php @@ -0,0 +1,67 @@ +mapper = new WidgetSourceMapper($this->connectionAdapter); + } + + protected function tearDown(): void { + $this->mapper->deleteByWidgetIds(self::WIDGETS); + parent::tearDown(); + } + + public function testSettingASourceReplacesTheOldOneAndNullRemovesIt(): void { + [$first, $second, $third] = 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)); + $this->assertSame([], $this->mapper->findNodeRelIdsByWidgetIds([$third])); + $this->assertSame([], $this->mapper->findNodeRelIdsByWidgetIds([])); + } + + 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)); + } +} diff --git a/tests/unit/Db/WidgetTest.php b/tests/unit/Db/WidgetTest.php new file mode 100644 index 0000000000..449702439c --- /dev/null +++ b/tests/unit/Db/WidgetTest.php @@ -0,0 +1,34 @@ + ['{"text":"Projects","showTitle":false,"level":2}', '{"level":2,"showTitle":false,"text":"Projects"}'], + 'no settings' => [null, '{}'], + 'not an object' => ['"text"', '{}'], + 'broken' => ['{broken', '{}'], + ]; + } + + /** + * @dataProvider storedSettings + */ + public function testSettingsSerialiseAsAJsonObject(?string $stored, string $serialised): void { + $widget = new Widget(); + $widget->setSettings($stored); + + $this->assertSame($serialised, json_encode($widget->jsonSerialize()['settings'])); + } +} 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/Version2500Date20261022000000Test.php b/tests/unit/Migration/Version2500Date20261022000000Test.php new file mode 100644 index 0000000000..762e128fb7 --- /dev/null +++ b/tests/unit/Migration/Version2500Date20261022000000Test.php @@ -0,0 +1,153 @@ +migration = new Version2500Date20261022000000($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']); + } + + public function testTheDatabaseAllowsPagesWithoutANameThatTheCodeNeverWrites(): void { + $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 static function backfills(): array { + return [ + 'a start page whose name another page holds' => [ + [[1, 'startpage', null], [2, 'grid', 'dashboard']], + [1 => 'page_1'], + ], + 'the lower id keeps dashboard whatever the row order' => [ + [[9, 'startpage', null], [3, 'startpage', null]], + [3 => 'dashboard', 9 => 'page_9'], + ], + 'an empty name counts as none' => [ + [[1, 'grid', ''], [2, 'grid', 'sales']], + [1 => 'page_1'], + ], + ]; + } + + /** + * @dataProvider backfills + * @param list $pages id, type and technical name in one application + */ + public function testBackfillNames(array $pages, array $expected): void { + $rows = array_map(static fn (array $page): array => ['id' => $page[0], 'context_id' => 10, 'page_type' => $page[1], 'technical_name' => $page[2]], $pages); + + $this->assertSame($expected, Version2500Date20261022000000::backfillNames($rows)); + } + + 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 67b94c8cca..8cfec52e6e 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -19,6 +19,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 OCA\Tables\Service\Support\AuditLogServiceInterface; @@ -283,6 +284,90 @@ public function testDeleteLogsCriticalAction(): void { $this->service->delete(9, 'alice'); } + 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 static function importedPages(): array { + $start = Page::TYPE_STARTPAGE; + $grid = Page::TYPE_GRID; + return [ + 'free names are kept, the rest get the default' => [ + [ + ['id' => 1, 'page_type' => $start], + ['id' => 2, 'page_type' => $grid, 'technical_name' => 'sales'], + ['id' => 3, 'page_type' => $grid, 'technical_name' => 'sales'], + ['id' => 4, 'page_type' => $grid, 'technical_name' => 'Bad Name'], + ['id' => 5, 'page_type' => $grid], + ['id' => 6, 'page_type' => $grid, 'technical_name' => 'page_2'], + ], + [80 => [$start, 'dashboard'], 81 => [$grid, 'sales'], 82 => [$grid, 'page_82'], 83 => [$grid, 'page_83'], 84 => [$grid, 'page_84'], 85 => [$grid, 'page_85']], + ], + 'the start page leaves dashboard to the page exported with it' => [ + [['id' => 1, 'page_type' => $start], ['id' => 2, 'page_type' => $grid, 'technical_name' => 'dashboard']], + [80 => [$start, 'page_80'], 81 => [$grid, 'dashboard']], + ], + 'only one start page and grid pages are imported' => [ + [ + ['id' => 1, 'pageType' => $start, 'technicalName' => 'home'], + ['id' => 2, 'page_type' => $start], + ['id' => 3, 'page_type' => 'chart'], + ['id' => 4], + 'not a page', + ['id' => 5, 'page_type' => $grid], + ], + [80 => [$start, 'home'], 81 => [$grid, 'page_81']], + ], + ]; + } + + /** + * @dataProvider importedPages + * @param array $expected type and technical name by page id + */ + public function testImportedPages(array $pages, array $expected): 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); + $inserted = []; + $pageId = 80; + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page) use (&$inserted, &$pageId): Page { + $page->id = $pageId++; + $inserted[$page->getId()] = [$page->getPageType(), $page->getTechnicalName()]; + return $page; + }); + $this->pageMapper->method('insertWithDefaultName')->willReturnCallback(static function (Page $page) use (&$inserted, &$pageId): Page { + $page->id = $pageId++; + $inserted[$page->getId()] = [$page->getPageType(), 'page_' . $page->getId()]; + return $page; + }); + + $this->service->importContext($table, ['name' => 'Projects', 'nodes' => [['id' => 1, 'node_id' => 4, 'node_type' => 0]], 'pages' => $pages], 4); + + self::assertSame($expected, $inserted); + } + 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..1fb10d9f9e --- /dev/null +++ b/tests/unit/Service/ViewServiceTechnicalNameTest.php @@ -0,0 +1,37 @@ + [str_repeat('a', 201), 'Technical name must not exceed 200 characters.'], + 'upper case' => ['Sales', '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); + } + + 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..499cb28447 --- /dev/null +++ b/tests/unit/Validation/TechnicalNameValidatorTest.php @@ -0,0 +1,23 @@ +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); + } + } +} 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';