Skip to content

feat(pages): fill the start page of an application with widgets #3211

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.

Sign up for GitHub

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

Open
rubenvdlinde wants to merge 2 commits into from
Open

feat(pages): fill the start page of an application with widgets #3211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3dc77a
feat(pages): fill the start page of an application with widgets
rubenvdlinde Nov 2, 2026
05ea37e
chore(api): regenerate the OpenAPI spec and TypeScript types
rubenvdlinde Nov 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion appinfo/info.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

]]></description>
<version>2.5.0-dev.0</version>
<version>2.5.0-dev.1</version>

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3135, on the same base, makes the same bump from 2.5.0-dev.0 to 2.5.0-dev.1 and ships its own step (Version2500Date20261103000000). The two hunks are identical, so they merge without a conflict and whichever PR lands second leaves the version as it is. Nextcloud only upgrades an app when the info.xml version differs from installed_version, so on every instance that already took the first one (dev instances tracking main) the second one's step stays pending until the next bump: if #3135 goes first, existing start pages stay unfilled there; in the other order, the menu endpoints run against a missing tables_contexts_menu_item. Same mechanism as #3131 (comment) with a different pair: whichever lands second needs to bump past what its base has by then, and the PRs stacked on it (#3212 is at dev.2) one higher each.

<licence>AGPL-3.0-or-later</licence>
<author>Nextcloud GmbH and Nextcloud contributors</author>
<namespace>Tables</namespace>
Expand Down
5 changes: 3 additions & 2 deletions lib/Controller/ContextPageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ public function create(int $contextId, string $title, ?string $technicalName = n
* [api v2] Change a page or its widgets
*
* Fields left out keep their value; an empty icon or description removes it. A given
* `widgets` list replaces the widgets of the grid page: listed widgets with an `id`
* are updated, others are added, and widgets not listed are deleted.
* `widgets` list replaces the widgets of the page, a grid page or the start page:

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the start page holds widgets, the descriptions around this one still say grid page: widgetTypes() is published as "[api v2] Get the widget types a grid page can hold" (openapi.json, openapi.ts), and the class docblocks of GridWidgetTypes ("The widget types a grid page can hold"), WidgetService ("Stores the widgets of grid pages") and Widget ("A widget on a grid page") say the same. The new migration's docblock calls the start page "a grid page", although its page_type stays startpage and PageService tells the two apart. "replaces the widgets of the page" here and "a page" in the others, then composer run openapi, keeps the API docs from telling clients that the start page takes no widgets.

* listed widgets with an `id` are updated, others are added, and widgets not listed
* are deleted.
*
* A widget has a `type`, `gridX`, `gridY`, `gridWidth` and `gridHeight` on a grid of
* 12 columns, `settings` following the schema of its type, and optionally an `id`,
Expand Down
119 changes: 119 additions & 0 deletions lib/Migration/Version2500Date20261030000000.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Tables\Migration;

use Closure;
use OCA\Tables\Db\Page;
use OCA\Tables\Service\GridWidgetTypes;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use Override;

/**
* The start page of an application is a grid page. Every start page without widgets is
* filled the way a new one is: a header with the name and description of its application,
* then one table or view widget per table or view, in the order the start page listed them.
* A start page that has widgets is left alone, so a second run changes nothing.
*/
class Version2500Date20261030000000 extends SimpleMigrationStep {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things about where this step sits in the stack.

It needs the widget tables that Version2500Date20261022000000 (#3131) creates, and steps run in name order. #3131 (comment) asks to give that step a current date before it lands; once it sorts after 20261030, this step has to move with it, or every upgrade from 2.4 runs it first and aborts on the missing tables_contexts_widget (fresh installs only apply the schema and don't notice). #3212's Version2500Date20261030010000 returns early when the widget table doesn't exist yet, so in the same situation it would leave out its columns without an error, on fresh installs as well.

And the fill happens at the upgrade to this PR, while only #3216 shows it; the description invites shipping this one earlier. Everything users add or reorder through Edit application in between only reaches page_content (see the comment on insertPage()), and applications created in between are filled once at creation, so when #3216 arrives, Home shows a snapshot from the earlier upgrade, which this step won't refresh (it skips pages that have widgets). Keeping the fill, create() and the import included, in the same release as #3216 avoids that.

private const PREFIX = 'tables_contexts_';

public function __construct(
private readonly IDBConnection $db,
) {
}

#[Override]
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
foreach ($this->emptyStartPages() as $startPage) {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGENTS.md: "Never build a IQueryBuilder query inside a loop. Construct the query once before the loop using $qb->createParameter('name')…". nodeRelIdsInPageOrder() builds a SELECT for every start page and insertWidget() one or two INSERTs for every widget, while #3131's step in this stack prepares its UPDATE once. The per-page SELECT is also a full scan: tables_contexts_page_content has only its primary key, and on the dev instance EXPLAIN of this query shows type: ALL with a filesort. No start page has widgets before this release, so every application goes through the loop on upgrade: the step reads all of page_content once per application, which grows with the square of the number of applications, and commits once per application, in maintenance mode and without progress output.

One SELECT for the content of all start pages (page_content joined to its relation and page, page_type = 'startpage', ordered by page_id, order and id, grouped in PHP), the two INSERTs prepared once with createParameter(), one transaction per chunk of pages and a progress bar on $output keep it linear.

$widgets = GridWidgetTypes::startPageWidgets(

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to #3131 (comment), one step further: besides Page::TYPE_STARTPAGE, this step runs the live GridWidgetTypes::startPageWidgets() and writes its result with frozen INSERTs. The docblock makes that deliberate ("filled the way a new one is"), but "a new one" is whatever the release running the step does. When a later release changes the generated layout or the header settings (the colour rework from #3132 (comment), dropping the header, other heights), instances upgrading from 2.4 straight to it get that shape from this 2.5.0 step, and a later step that converts 2.5.0 header settings meets rows it doesn't expect; instances that went through 2.5 and those that skipped it end up with different start pages for the same data. This step's test then has to be changed to the new layout, which makes the drift official.

Version000700Date20230916000000 copies a definition for this reason, and #3131's step freezes 'dashboard' as its own constant. Copying the layout in (width 12, heights 2 and 5, the five header settings, a header only for a non-blank name, at most 100 widgets, 'startpage') keeps the step frozen and leaves startPageWidgets() free to change and to validate (see the comment there).

(string)$startPage['name'],
(string)($startPage['description'] ?? ''),
$this->nodeRelIdsInPageOrder((int)$startPage['page_id']),
);
$this->db->beginTransaction();
try {
foreach ($widgets as $widget) {
$this->insertWidget((int)$startPage['page_id'], $widget);
}
$this->db->commit();
} catch (\Throwable $e) {
$this->db->rollBack();
throw $e;
}
}
}

/**
* @return list<array<string, mixed>> start pages without widgets, with the name and description of their application
*/
private function emptyStartPages(): array {
$qb = $this->db->getQueryBuilder();
$qb->select('p.id as page_id', 'c.name', 'c.description')
->from(self::PREFIX . 'page', 'p')
->innerJoin('p', self::PREFIX . 'context', 'c', $qb->expr()->eq('c.id', 'p.context_id'))
->leftJoin('p', self::PREFIX . 'widget', 'w', $qb->expr()->eq('w.page_id', 'p.id'))
->where($qb->expr()->eq('p.page_type', $qb->createNamedParameter(Page::TYPE_STARTPAGE)))
->andWhere($qb->expr()->isNull('w.id'))
->orderBy('p.id');
$result = $qb->executeQuery();
$rows = $result->fetchAllAssociative();
$result->closeCursor();
return $rows;
}

/**
* @return list<int> the tables and views a start page lists, as node relations of its application, in page order
*/
private function nodeRelIdsInPageOrder(int $pageId): array {
$qb = $this->db->getQueryBuilder();
$qb->select('pc.node_rel_id')
->from(self::PREFIX . 'page_content', 'pc')
->innerJoin('pc', self::PREFIX . 'rel_context_node', 'r', $qb->expr()->eq('r.id', 'pc.node_rel_id'))
->where($qb->expr()->eq('pc.page_id', $qb->createNamedParameter($pageId, IQueryBuilder::PARAM_INT)))
->orderBy('pc.order')
->addOrderBy('pc.id');
$result = $qb->executeQuery();
$nodeRelIds = array_map(intval(...), $result->fetchFirstColumn());
$result->closeCursor();
return array_values(array_unique($nodeRelIds));
}

/**
* @param array{type: string, position: array{gridX: int, gridY: int, gridWidth: int, gridHeight: int}, settings: array<string, mixed>, nodeRelId: ?int} $widget
*/
private function insertWidget(int $pageId, array $widget): void {
$qb = $this->db->getQueryBuilder();
$qb->insert(self::PREFIX . 'widget')
->values([
'page_id' => $qb->createNamedParameter($pageId, IQueryBuilder::PARAM_INT),
'type' => $qb->createNamedParameter($widget['type']),
'grid_x' => $qb->createNamedParameter($widget['position']['gridX'], IQueryBuilder::PARAM_INT),
'grid_y' => $qb->createNamedParameter($widget['position']['gridY'], IQueryBuilder::PARAM_INT),
'grid_width' => $qb->createNamedParameter($widget['position']['gridWidth'], IQueryBuilder::PARAM_INT),
'grid_height' => $qb->createNamedParameter($widget['position']['gridHeight'], IQueryBuilder::PARAM_INT),
'settings' => $qb->createNamedParameter($widget['settings'], IQueryBuilder::PARAM_JSON),
])
->executeStatement();
if ($widget['nodeRelId'] === null) {
return;
}
$widgetId = $qb->getLastInsertId();
$source = $this->db->getQueryBuilder();
$source->insert(self::PREFIX . 'widget_source')
->values([
'widget_id' => $source->createNamedParameter($widgetId, IQueryBuilder::PARAM_INT),
'node_rel_id' => $source->createNamedParameter($widget['nodeRelId'], IQueryBuilder::PARAM_INT),
])
->executeStatement();
}
}
15 changes: 15 additions & 0 deletions lib/Service/ContextService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ protected function insertPage(Context $context): void {
$addedPage = $page->jsonSerialize();

$i = 1;
$nodeRelIds = [];
$contextNodes = $context->getNodes();
if ($contextNodes) {
foreach ($contextNodes as $node) {
Expand All @@ -575,9 +576,14 @@ protected function insertPage(Context $context): void {
$addedPage['content'][$pageContent->getId()] = $pageContent->jsonSerialize();
// the content is already embedded in the page
unset($addedPage['content'][$pageContent->getId()]['pageId']);
$nodeRelIds[] = (int)$node['id'];
}
}

foreach (GridWidgetTypes::startPageWidgets($context->getName(), $context->getDescription(), $nodeRelIds) as $widget) {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only create(), the migration and the import fill the start page; everything that changes an application's tables and views afterwards still goes to page_content alone. update() (the Edit application dialog, and Import scheme, whose importScheme() ends in update()) adds a node through addNodeToContextAndStartpage() without a widget, and the order set in the dialog, like PUT /api/2/contexts/{contextId}/pages/{pageId} (updateContentOrder()), only rewrites page_content.order. From #3216 on the start page is rendered from its widgets, and page_content is left as the order of the dialog's own list (EditContext.vue). So a table added after creation never appears on Home, reordering in the dialog has no visible effect, and an application created empty and then filled through Import scheme shows its header and nothing else. With the "widget stays and asks for a new table" agreed in #3099, a table removed in the dialog, or deleted by its owner, also leaves a full-width "No table or view to show" box for every member, and adding it back places nothing. #3216's e2e tests already work around this, with addTableWidgetToStartPage() after saving the dialog in four tests.

Reproduced on the dev instance: an application created with two tables got a header and two data widgets. PUT /api/2/contexts/{id} adding a third table added its page_content row but no widget; a second PUT with the nodes in the order third, first reordered page_content, while the widgets stayed header, first table, and a data widget without source, with the third table nowhere.

The description accepts that tables added later aren't placed, but with Edit application and Import scheme unchanged that means the start page stops matching the application right after it is created, and the same URL now has a PATCH for its widgets and a PUT for an order nothing shows. Putting the rule back where every addition passes, addNodeRelToPage() appending a data widget below the lowest one of the start page (with insertPage() and importContext() using the same append), keeps both dialogs meaningful. The tables overview widget proposed in #3099 (comment), one widget listing the application's tables and views, would follow every change by construction.

$this->widgetService->insert($page->getId(), $widget['type'], null, $widget['position'], $widget['settings'], $widget['nodeRelId']);

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every new application now pays the per-widget pattern from #3132 (comment): WidgetService::insert() checks each source with a SELECT although the relation was inserted into this application a moment earlier in the same transaction, and setSource() deletes the source of every freshly inserted id, the header's included. On the dev instance the profiler shows 14 widget statements for POST /api/2/contexts with three tables (4 widget INSERTs, 4 source DELETEs, 3 source SELECTs, 3 source INSERTs); 7 of them do something.

The loop is copied into importContext() (line 746). Inserting the list startPageWidgets() returns with one WidgetService call, syncPage() with 'id' => null rows (one source check per page) or an insert without the check and the DELETE, used from both places, saves those statements and keeps the two fills from drifting apart.

}

$context->setPages([$addedPage['id'] => $addedPage]);
}

Expand Down Expand Up @@ -718,6 +724,7 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C
$pageMap[$pageArr['id'] ?? null] = $page;

// insert page conten
$contentRelIds = [];
if (!empty($pageArr['content'])) {
foreach ($pageArr['content'] as $contentArr) {
$oldNodeRelId = self::importedKey($contentArr['node_rel_id'] ?? $contentArr['nodeRelId'] ?? null);
Expand All @@ -730,6 +737,14 @@ public function importContext(Table $table, array $context, int $tableOldId): ?C
$pageContent->setNodeRelId($newNodeRel->getId());
$pageContent->setOrder($contentArr['order'] ?? 100);
$this->pageContentMapper->insert($pageContent);
$contentRelIds[] = $newNodeRel->getId();
}
}

// A start page exported before pages held widgets starts filled, like a new one
if ($page->getPageType() === Page::TYPE_STARTPAGE && !array_key_exists('widgets', $pageArr)) {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A missing widgets key doesn't reliably mean "exported before pages held widgets": from #3132 on, the export goes through getPagesWithWidgets(), which writes widgets: [] for every page, and the API refuses widgets on the start page until this PR. An archive exported by a build that has #3132 or #3133 but not this PR (a release or pre-release tagged between the merges, or a dev instance of main in that window) therefore imports with a start page that stays empty, which with #3216 is a Home with neither header nor tables. The import can't tell that apart from a start page someone emptied on purpose, which the description wants kept.

Merging #3132 to this PR without a tag in between avoids it. Otherwise, bumping the migrator version (TablesMigrator uses the default from TMigratorBasicVersionHandling, and import() already reads getMigratorVersion()) and filling the start pages of archives with an older version gives the import a signal it can rely on.

foreach (GridWidgetTypes::startPageWidgets($newContext->getName(), $newContext->getDescription(), array_values(array_unique($contentRelIds))) as $widget) {
$this->widgetService->insert($page->getId(), $widget['type'], null, $widget['position'], $widget['settings'], $widget['nodeRelId']);
}
}

Expand Down
42 changes: 38 additions & 4 deletions lib/Service/GridWidgetTypes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class GridWidgetTypes {
public const TYPE_DATA = 'data';

public const GRID_COLUMNS = 12;
public const HEADER_HEIGHT = 2;
public const DATA_HEIGHT = 5;
public const GRID_MAX_HEIGHT = 100;
public const GRID_MAX_Y = 10000;
public const TITLE_MAX_LENGTH = 200;
Expand All @@ -51,8 +53,8 @@ public function all(): array {
self::TYPE_HEADER => [
'type' => self::TYPE_HEADER,
'title' => $this->l10n->t('Header'),
'defaultWidth' => 12,
'defaultHeight' => 2,
'defaultWidth' => self::GRID_COLUMNS,
'defaultHeight' => self::HEADER_HEIGHT,
'hasSource' => false,
'settings' => [
'heading' => ['type' => 'string', 'title' => $this->l10n->t('Heading'), 'default' => '', 'required' => true],
Expand All @@ -79,14 +81,46 @@ public function all(): array {
self::TYPE_DATA => [
'type' => self::TYPE_DATA,
'title' => $this->l10n->t('Table or view'),
'defaultWidth' => 12,
'defaultHeight' => 5,
'defaultWidth' => self::GRID_COLUMNS,
'defaultHeight' => self::DATA_HEIGHT,
'hasSource' => true,
'settings' => [],
],
];
}

/**
* The widgets a start page starts with: a header with the name and description of its
* application, then one full width table or view widget per node, in the given order.
* Without a name there is no header.
*
* @param list<int> $nodeRelIds node relations of the application, in page order
* @return list<array{type: string, position: array{gridX: int, gridY: int, gridWidth: int, gridHeight: int}, settings: array<string, TablesWidgetValue>, nodeRelId: ?int}>
*/
public static function startPageWidgets(string $name, string $description, array $nodeRelIds): array {
$widgets = [];
$gridY = 0;
if (trim($name) !== '') {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessibility: the generated header repeats what the page around the start page already shows. In #3216, Context.vue renders the application's name as <h1> and its description above the page bar on every page, and GridPage.vue deliberately gives the start page no heading of its own ("The start page sits under the title of its application"). The header widget then renders the same name as <h2> and the description once more, as the Home screenshot in #3099 (comment) shows ("Intake portal" and "Requests from residents" twice). In heading navigation the name comes twice in a row, and screen reader users hear the description, when there is one, twice; through the migration this applies to every existing application. Because the header has no title, GridWidget.vue also labels its <section> with the type name, so every Home starts with a region landmark called "Header", which says nothing about its content.

#3216 keeps both on purpose, because the header is an ordinary widget people can change or remove, and offers to drop the name above the page bar instead. But the header is a copy that nothing keeps current: ContextService::update() changes the name and description without touching it, so after a rename (in Edit application, or by Import scheme, which passes the scheme's name to update()) the page shows the new name above the page bar and the old one in the header, for every member until the owner edits the widget. Reproduced on the dev instance: after PUT /api/2/contexts/{id} with a new name and description, the header still had the old heading and subheading. Dropping the name above the page bar would leave that stale copy as the start page's only title.

Filling the start page with its tables and views only keeps it the way it looks today; the data widgets already work that way, showing their table's live title instead of a copy. A header stays one "Add widget" away for owners who want one. If a generated header stays, it needs to follow the application, for example a header setting that shows the application's name instead of storing it.

$widgets[] = [
'type' => self::TYPE_HEADER,
'position' => ['gridX' => 0, 'gridY' => $gridY, 'gridWidth' => self::GRID_COLUMNS, 'gridHeight' => self::HEADER_HEIGHT],
'settings' => ['heading' => $name, 'subheading' => $description, 'textAlign' => 'start', 'backgroundColor' => '', 'textColor' => ''],

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing checks these settings against the header schema: WidgetService::insert() and the migration store them as they are, while every later save of the page runs each widget through sanitizeSettings(). An application's description has no length limit (a TEXT column, unchecked by ContextController and by the dialogs' plain <input>), so a description that encodes to more than SETTINGS_MAX_BYTES gives a start page that can't be saved as it is: nothing on it can be moved, added or removed until someone shortens a subheading they never typed, and an account export and import drops the header ("Skipping an imported widget"). With the \uXXXX escaping from #3132 (comment) that takes about 10000 characters in a non-Latin script.

Reproduced on the dev instance: POST /api/2/contexts with a description of 10001 CJK characters (and another with 60000 ASCII characters) returned 200, and PATCH .../pages/{startPageId} sending the widgets back exactly as GET returned them answered 400 "The settings of a header widget take more than 60000 bytes."

The literal also repeats the schema's defaults for textAlign and the two colours, which already changed once in this stack (#3132 (comment)) and may again (#3132 (comment)); a setting that is renamed or added there without this line produces headers the API then refuses. If the header stays (see above), $this->sanitizeSettings(self::TYPE_HEADER, ['heading' => $name, 'subheading' => $description]), with the subheading cut to a length that always fits (the 2000 characters pages and widgets allow for descriptions, for example), keeps one source of defaults and rules. ContextService already has $this->gridWidgetTypes injected, so this can be an instance method; the migration shouldn't call it either way (see the comment there).

'nodeRelId' => null,
];
$gridY += self::HEADER_HEIGHT;
}
foreach (array_slice($nodeRelIds, 0, self::MAX_WIDGETS_PER_PAGE - count($widgets)) as $nodeRelId) {

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing says when this cuts: an application with more than 99 tables and views (nothing limits that) gets data widgets for the first 99 only, at creation, on upgrade and on import, while importContext() logs the same cut for exported widgets a few lines further down. With #3216 the rest simply aren't on Home, and the page is then at MAX_WIDGETS_PER_PAGE, so adding any widget answers 400 until the owner removes widgets they never placed. A log line here, and a message on the migration's unused $output, would at least tell someone.

$widgets[] = [
'type' => self::TYPE_DATA,
'position' => ['gridX' => 0, 'gridY' => $gridY, 'gridWidth' => self::GRID_COLUMNS, 'gridHeight' => self::DATA_HEIGHT],
'settings' => [],
'nodeRelId' => $nodeRelId,
];
$gridY += self::DATA_HEIGHT;
}
return $widgets;
}

/**
* @return TablesWidgetType
* @throws BadRequestError
Expand Down
5 changes: 1 addition & 4 deletions lib/Service/PageService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public function create(int $contextId, string $title, ?string $technicalName, ?s
*/
public function update(int $contextId, int $pageId, ?string $title, ?string $technicalName, ?string $icon, ?string $description, ?array $widgets, string $userId): array {
$context = $this->getManageableContext($contextId, $userId);
$pageArray = $this->getPageArray($context, $pageId);
$this->getPageArray($context, $pageId);

@AndyScherzinger AndyScherzinger Nov 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With its result discarded this reads like a leftover, but it is the only check that $pageId belongs to $contextId: pageMapper->findById() below loads any page, and RequirePermission and getManageableContext() only look at the application. The "update it" case of testAnApplicationOrPageOutsideTheUsersReachIsNotFound() would catch its removal, but nothing here says why it's needed. Keeping the result and checking the type there, in_array($pageArray['page_type'], [Page::TYPE_GRID, Page::TYPE_STARTPAGE], true), makes it read as the guard it is and keeps widgets off pages of other types, which older imports could create (#3133 (comment)).


$page = $this->pageMapper->findById($pageId);
if ($title !== null) {
Expand All @@ -138,9 +138,6 @@ public function update(int $contextId, int $pageId, ?string $title, ?string $tec

$rows = null;
if ($widgets !== null) {
if ($pageArray['page_type'] !== Page::TYPE_GRID) {
throw new BadRequestError('Only grid pages hold widgets.');
}
$rows = $this->readWidgets($widgets, $context, $pageId);
}

Expand Down
2 changes: 1 addition & 1 deletion openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14979,7 +14979,7 @@
"patch": {
"operationId": "context_page-update",
"summary": "[api v2] Change a page or its widgets",
"description": "Fields left out keep their value; an empty icon or description removes it. A given `widgets` list replaces the widgets of the grid page: listed widgets with an `id` are updated, others are added, and widgets not listed are deleted.\nA widget has a `type`, `gridX`, `gridY`, `gridWidth` and `gridHeight` on a grid of 12 columns, `settings` following the schema of its type, and optionally an `id`, `title`, `icon` and `description`. A data widget's `source` is `{nodeRelId}` of a table or view of this application, or null.",
"description": "Fields left out keep their value; an empty icon or description removes it. A given `widgets` list replaces the widgets of the page, a grid page or the start page: listed widgets with an `id` are updated, others are added, and widgets not listed are deleted.\nA widget has a `type`, `gridX`, `gridY`, `gridWidth` and `gridHeight` on a grid of 12 columns, `settings` following the schema of its type, and optionally an `id`, `title`, `icon` and `description`. A data widget's `source` is `{nodeRelId}` of a table or view of this application, or null.",
"tags": [
"context_page"
],
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "tables",
"description": "Manage data within tables.",
"version": "2.5.0-dev.0",
"version": "2.5.0-dev.1",
"author": "Nextcloud GmbH and Nextcloud contributors",
"bugs": {
"url": "https://github.com/nextcloud/tables/issues"
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ export type paths = {
readonly head?: never;
/**
* [api v2] Change a page or its widgets
* @description Fields left out keep their value; an empty icon or description removes it. A given `widgets` list replaces the widgets of the grid page: listed widgets with an `id` are updated, others are added, and widgets not listed are deleted.
* @description Fields left out keep their value; an empty icon or description removes it. A given `widgets` list replaces the widgets of the page, a grid page or the start page: listed widgets with an `id` are updated, others are added, and widgets not listed are deleted.
* A widget has a `type`, `gridX`, `gridY`, `gridWidth` and `gridHeight` on a grid of 12 columns, `settings` following the schema of its type, and optionally an `id`, `title`, `icon` and `description`. A data widget's `source` is `{nodeRelId}` of a table or view of this application, or null.
*/
readonly patch: operations["context_page-update"];
Expand Down
Loading
Loading