Repository navigation
feat(contexts): data model for grid pages #3131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that the rule lives in |
||
| 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); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern for technical names as noted about menu entries in #3100 (comment) |
||
| 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_<id>`. 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(), | ||
| ]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<Page> */ | ||||||||
| 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_<id>`. | ||||||||
| * | ||||||||
| * @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()); | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| return $this->update($page); | ||||||||
| }, $this->db); | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * @return int[] | ||||||||
| */ | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Tables\Db; | ||
|
|
||
| use OCP\AppFramework\Db\Entity; | ||
|
|
||
| /** | ||
| * A widget on a grid page: its type, optional title, icon and description, its place | ||
| * on the grid and its settings, stored as one JSON object. | ||
| * | ||
| * @psalm-type TablesWidgetSettingValue = string|int|float|bool|null | ||
| * | ||
| * @method getPageId(): int | ||
| * @method setPageId(int $value): void | ||
| * @method getType(): string | ||
| * @method setType(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 | ||
| * @method getGridX(): int | ||
| * @method setGridX(int $value): void | ||
| * @method getGridY(): int | ||
| * @method setGridY(int $value): void | ||
| * @method getGridWidth(): int | ||
| * @method setGridWidth(int $value): void | ||
| * @method getGridHeight(): int | ||
| * @method setGridHeight(int $value): void | ||
| * @method getSettings(): ?string | ||
| * @method setSettings(?string $value): void | ||
| */ | ||
| class Widget extends Entity implements \JsonSerializable { | ||
| protected ?int $pageId = null; | ||
| protected ?string $type = null; | ||
| protected ?string $title = null; | ||
| protected ?string $icon = null; | ||
| protected ?string $description = null; | ||
| protected ?int $gridX = null; | ||
| protected ?int $gridY = null; | ||
| protected ?int $gridWidth = null; | ||
| protected ?int $gridHeight = null; | ||
| protected ?string $settings = null; | ||
|
|
||
| public function __construct() { | ||
| $this->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<string, TablesWidgetSettingValue> | ||
| */ | ||
| public function getSettingsArray(): array { | ||
| $settings = $this->getSettings(); | ||
| if ($settings === null || $settings === '') { | ||
| return []; | ||
| } | ||
| $decoded = json_decode($settings, true); | ||
| /** @var array<string, TablesWidgetSettingValue> */ | ||
| return is_array($decoded) ? $decoded : []; | ||
| } | ||
|
|
||
| /** | ||
| * Stores the settings as one JSON object, `{}` when there are none. | ||
| * | ||
| * @param array<string, TablesWidgetSettingValue> $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 { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing in the stack calls this outside the tests. The API and export shape is built by hand in |
||
| 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(), | ||
| ]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Tables\Db; | ||
|
|
||
| use OCP\AppFramework\Db\QBMapper; | ||
| use OCP\DB\Exception; | ||
| use OCP\DB\QueryBuilder\IQueryBuilder; | ||
| use OCP\IDBConnection; | ||
|
|
||
| /** @template-extends QBMapper<Widget> */ | ||
| 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))) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AGENTS.md: "IN clauses must be chunked to at most 1 000 items for Oracle compatibility. Use a named constant ( In the stack, deleting or exporting an application passes every widget id of that application. With 100 widgets per page and no page limit, more than 1000 ids are reachable, and the server then logs "More than 1000 expressions in a list are not allowed on Oracle." at error level on every database. When chunking Related, in #3132: |
||
| ->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(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Tables\Db; | ||
|
|
||
| use OCP\AppFramework\Db\Entity; | ||
|
|
||
| /** | ||
| * The node of its application a data widget shows. The widget id is the key; | ||
| * the inherited id is not used. | ||
| * | ||
| * @method getWidgetId(): int | ||
| * @method setWidgetId(int $value): void | ||
| * @method getNodeRelId(): int | ||
| * @method setNodeRelId(int $value): void | ||
| */ | ||
| class WidgetSource extends Entity { | ||
| protected ?int $widgetId = null; | ||
| protected ?int $nodeRelId = null; | ||
|
|
||
| public function __construct() { | ||
| $this->addType('widgetId', 'integer'); | ||
| $this->addType('nodeRelId', 'integer'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2476 bumps to the same
2.5.0-dev.0and ships its own migration (Version2500Date20261031000000). The two branches merge without a conflict on this line, so whichever lands second leaves the version unchanged, and Nextcloud only upgrades an app when the info.xml version differs frominstalled_version(AppManager::isUpgradeRequired()).On an instance that already runs the first one (dev or nightly setups tracking main),
Version2500Date20261022000000then stays pending until the next bump:tables_contexts_pagehas notechnical_namecolumn, andContextService::insertPage()now always writes it, so creating any application fails with an unknown-column error. A dev instance that ran #2476 is in exactly that state (installed_version2.5.0-dev.0, page table withouttechnical_name). In the other merge order, #2476's archive tables are the ones missing.Whichever of the two lands second needs to bump past what main has at that point, and #3135 then one higher.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing on the ordering, checked on a dev instance running #2476: it has already applied
Version2500Date20261031000000andVersion2500Date20261101000000, while this PR's migration isVersion2500Date20261022000000, so it sorts before both. Whichever lands second, please rename it to the then-current target version and a current date, as AGENTS.md asks for long-lived PRs, so it sorts after everything already applied.