Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 cypress/e2e/tables-table.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Manage a table', () => {
// Updating the title is flaky, so skipping it for now
// cy.get('[data-cy="editTableModal"] [data-cy="editTableTitleInput"]').should('be.visible').should('be.enabled')
// cy.get('[data-cy="editTableModal"] [data-cy="editTableTitleInput"]').clear().type('ToDo list')
cy.get('.modal__content #description-editor .tiptap.ProseMirror').type('Updated ToDo List description')
cy.get('[data-cy="editTableModal"] #description-editor .tiptap.ProseMirror').type('Updated ToDo List description')
cy.get('[data-cy="editTableSaveBtn"]').should('be.enabled').click()

cy.wait(10).get('.toastify.toast-success').should('be.visible')
Expand Down
31 changes: 28 additions & 3 deletions lib/Controller/ApiTablesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,14 @@ public function showScheme(int $id): DataResponse {
* @param string $description description
* @param list<TablesColumn> $columns columns
* @param list<TablesView> $views views
* @param list<array{columnId: int, order: int, readonly: bool}> $columnOrder Default column order settings
* @param list<array{columnId: int, mode: 'ASC'|'DESC'}> $sort Default sort rules
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: Tables returned
*/
#[NoAdminRequired]
public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views): DataResponse {
public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views, array $columnOrder = [], array $sort = []): DataResponse {
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.

did not check further down the code, but $columnOrder and $sort are not tested or sanitized for expected input format.

try {
$this->db->beginTransaction();
$table = $this->service->create($title, 'custom', $emoji, $description);
Expand Down Expand Up @@ -175,6 +177,21 @@ public function createFromScheme(string $title, string $emoji, string $descripti
);
$colMap[$column['id']] = $col->getId();
}
if (!empty($columnOrder) || !empty($sort)) {
$remappedColumnOrder = !empty($columnOrder) ? array_map(static function (array $entry) use ($colMap): array {
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
}
return $entry;
}, $columnOrder) : null;
$remappedSort = !empty($sort) ? array_map(static function (array $entry) use ($colMap): array {
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
}
return $entry;
}, $sort) : null;
$table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort);
}
foreach ($views as $view) {
$newView = $this->viewService->create(
$view['title'],
Expand Down Expand Up @@ -262,6 +279,8 @@ public function create(string $title, ?string $emoji, ?string $description, stri
* @param string|null $emoji New table emoji
* @param bool $archived whether the table is archived
* @param string $description the tables description
* @param list<array{columnId: int, order: int, readonly: bool}>|string|null $columnSettings Default column order settings (array or JSON string)
* @param list<array{columnId: int, mode: 'ASC'|'DESC'}>|string|null $sort Default sort rules (array or JSON string)
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
*
* 200: Tables returned
Expand All @@ -270,9 +289,15 @@ public function create(string $title, ?string $emoji, ?string $description, stri
*/
#[NoAdminRequired]
#[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'id')]
public function update(int $id, ?string $title = null, ?string $emoji = null, ?string $description = null, ?bool $archived = null): DataResponse {
public function update(int $id, ?string $title = null, ?string $emoji = null, ?string $description = null, ?bool $archived = null, null|array|string $columnSettings = null, null|array|string $sort = null): DataResponse {
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.

same here for $sort

if (is_string($columnSettings)) {
$columnSettings = json_decode($columnSettings, true) ?? null;
}
if (is_string($sort)) {
$sort = json_decode($sort, true) ?? null;
}
try {
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId)->jsonSerialize());
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize());
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (InternalError $e) {
Expand Down
12 changes: 9 additions & 3 deletions lib/Controller/TableController.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,15 @@ public function destroy(int $id): DataResponse {

#[NoAdminRequired]
#[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'id')]
public function update(int $id, ?string $title = null, ?string $emoji = null, ?bool $archived = null): DataResponse {
return $this->handleError(function () use ($id, $title, $emoji, $archived) {
return $this->service->update($id, $title, $emoji, null, $archived, $this->userId);
public function update(int $id, ?string $title = null, ?string $emoji = null, ?bool $archived = null, null|array|string $columnSettings = null, null|array|string $sort = null): DataResponse {
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.

and here

if (is_string($columnSettings)) {
$columnSettings = json_decode($columnSettings, true) ?? null;
}
if (is_string($sort)) {
$sort = json_decode($sort, true) ?? null;
}
return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) {
return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort);
});
}
}
60 changes: 60 additions & 0 deletions lib/Db/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

use JsonSerializable;
use OCA\Tables\Model\Permissions;
use OCA\Tables\Model\SortRuleSet;
use OCA\Tables\ResponseDefinitions;
use OCA\Tables\Service\ValueObject\ViewColumnInformation;

/**
* @psalm-suppress PropertyNotSetInConstructor
Expand Down Expand Up @@ -46,6 +48,10 @@
* @method setViews(array $views)
* @method getColumns(): array
* @method setColumns(array $columns)
* @method getColumnOrder(): ?string
* @method setColumnOrder(?string $columnOrder)
* @method getSort(): ?string
* @method setSort(?string $sort)
* @method getCreatedBy(): string
* @method setCreatedBy(string $createdBy)
* @method getCreatedAt(): string
Expand All @@ -66,6 +72,9 @@ class Table extends EntitySuper implements JsonSerializable {
protected bool $archived = false;
protected ?string $description = null;

protected ?string $columnOrder = null; // json
protected ?string $sort = null; // json

// virtual properties
protected ?bool $isShared = null;
protected ?Permissions $onSharePermissions = null;
Expand Down Expand Up @@ -107,6 +116,8 @@ public function jsonSerialize(): array {
'columnsCount' => $this->columnsCount ?: 0,
'views' => $this->getViewsArray(),
'description' => $this->description ?:'',
'columnOrder' => $this->getColumnOrderSettingsArray(),
'sort' => $this->getSortArray(),
];
}

Expand All @@ -121,4 +132,53 @@ private function getSharePermissions(): ?Permissions {
private function getViewsArray(): array {
return $this->getViews() ?: [];
}

/**
* @psalm-suppress MismatchingDocblockReturnType
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.

red flag

* @return int[]
*/
public function getColumnOrderArray(): array {
$columnSettings = $this->getColumnOrderSettingsArray();
usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
return $a->getOrder() - $b->getOrder();
});
return array_map(static fn (ViewColumnInformation $vci): int => $vci->getId(), $columnSettings);
}

/**
* @return array<ViewColumnInformation>
*/
public function getColumnOrderSettingsArray(): array {
$columns = $this->getArray($this->getColumnOrder());
if (empty($columns)) {
return [];
}

if (is_array(reset($columns))) {
return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns));
}

$result = [];
foreach ($columns as $index => $columnId) {
$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
}
return $result;
}

/**
* @psalm-suppress MismatchingDocblockReturnType
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.

red flag

Copy link
Copy Markdown
Member

@blizzz blizzz Apr 21, 2026

Choose a reason for hiding this comment

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

I am also not entirely confident about the code generated here, but it is a bit vague gut feeling yet.

* @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
*/
public function getSortArray(): array {
$rawSortRules = $this->getArray($this->getSort());
return SortRuleSet::createFromInputArray($rawSortRules)->jsonSerialize();
}

private function getArray(?string $json): array {
if ($json !== '' && $json !== null && $json !== 'null') {
return \json_decode($json, true);
} else {
return [];
}
}
}
50 changes: 50 additions & 0 deletions lib/Migration/Version2010Date20260414000000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?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 OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use Override;

class Version2010Date20260414000000 extends SimpleMigrationStep {

#[Override]
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->hasTable('tables_tables')) {
return null;
}

$tablesTables = $schema->getTable('tables_tables');

if (!$tablesTables->hasColumn('column_order')) {
$tablesTables->addColumn('column_order', Types::TEXT, [
'notnull' => false,
'default' => null,
'comment' => 'JSON array of ViewColumnInformation — default column order for the table',
]);
}

if (!$tablesTables->hasColumn('sort')) {
$tablesTables->addColumn('sort', Types::TEXT, [
'notnull' => false,
'default' => null,
'comment' => 'JSON array of sort rules — default row sort for the table',
]);
}

return $schema;
}
}
10 changes: 8 additions & 2 deletions lib/Model/TableScheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ class TableScheme implements JsonSerializable {
protected ?array $views = null;
protected ?string $description = null;
protected ?string $tablesVersion = null;
protected array $columnOrder = [];
protected array $sort = [];

public function __construct(string $title, string $emoji, array $columns, array $view, string $description, string $tablesVersion) {
public function __construct(string $title, string $emoji, array $columns, array $view, string $description, string $tablesVersion, array $columnOrder = [], array $sort = []) {
$this->tablesVersion = $tablesVersion;
$this->title = $title;
$this->emoji = $emoji;
$this->columns = $columns;
$this->description = $description;
$this->views = $view;
$this->columnOrder = $columnOrder;
$this->sort = $sort;
}

public function getTitle():string {
Expand All @@ -43,8 +47,10 @@ public function jsonSerialize(): array {
'emoji' => $this->emoji,
'columns' => $this->columns,
'views' => $this->views,
'description' => $this->description ?:'',
'description' => $this->description ?: '',
'tablesVersion' => $this->tablesVersion,
'columnOrder' => $this->columnOrder,
'sort' => $this->sort,
];
}

Expand Down
10 changes: 5 additions & 5 deletions lib/Model/ViewUpdateInput.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ public static function fromInputArray(array $data): self {
}

return new self(
title: $data['title'] ? new Title($data['title']) : null,
title: ($data['title'] ?? null) ? new Title($data['title']) : null,
description: $data['description'] ?? null,
emoji: $data['emoji'] ? new Emoji($data['emoji']) : null,
columnSettings: $data['columnSettings'] ? ColumnSettings::createFromInputArray($data['columnSettings']) : null,
filterSet: $data['filter'] ? FilterSet::createFromInputArray($data['filter']) : null,
sortRuleSet: $data['sort'] ? SortRuleSet::createFromInputArray($data['sort']) : null,
emoji: ($data['emoji'] ?? null) ? new Emoji($data['emoji']) : null,
columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createFromInputArray($data['columnSettings']) : null,
filterSet: ($data['filter'] ?? null) ? FilterSet::createFromInputArray($data['filter']) : null,
sortRuleSet: ($data['sort'] ?? null) ? SortRuleSet::createFromInputArray($data['sort']) : null,
);
}

Expand Down
2 changes: 2 additions & 0 deletions lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
* rowsCount: int,
* views: list<TablesView>,
* columnsCount: int,
* columnOrder: list<array{columnId: int, order: int, readonly: bool}>,
* sort: list<array{columnId: int, mode: 'ASC'|'DESC'}>,
* }
*
* @psalm-type TablesIndex = array{
Expand Down
37 changes: 35 additions & 2 deletions lib/Service/ColumnService.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class ColumnService extends SuperService {

private ColumnDtoValidator $columnDtoValidator;

/** @var array<int, int[]> Per-request cache of sorted column-id order, keyed by tableId. */
private array $columnOrderCache = [];

public function __construct(
PermissionsService $permissionsService,
LoggerInterface $logger,
Expand Down Expand Up @@ -74,10 +77,40 @@ public function __construct(
* @throws InternalError
* @throws PermissionError
*/
public function findAllByTable(int $tableId, ?string $userId = null): array {
public function findAllByTable(int $tableId, ?string $userId = null, ?Table $table = null): array {
if ($this->permissionsService->canReadColumnsByTableId($tableId, $userId)) {
try {
return $this->enhanceColumns($this->mapper->findAllByTable($tableId));
$columns = $this->enhanceColumns($this->mapper->findAllByTable($tableId));

// Apply table-level default column order when set.
// Use a per-request cache to avoid a redundant DB fetch when the
// caller did not pass the Table entity and the TableMapper cache
// is cold (e.g. CLI/migration paths with $userId = '').
if (!array_key_exists($tableId, $this->columnOrderCache)) {
$entity = $table ?? $this->tableMapper->find($tableId);
$this->columnOrderCache[$tableId] = $entity->getColumnOrderArray();
}
$columnOrder = $this->columnOrderCache[$tableId];
if (!empty($columnOrder)) {
$indexed = [];
foreach ($columns as $column) {
$indexed[$column->getId()] = $column;
}
$ordered = [];
foreach ($columnOrder as $id) {
if (isset($indexed[$id])) {
$ordered[] = $indexed[$id];
unset($indexed[$id]);
}
}
// append any columns not listed in the order (e.g. newly added)
foreach ($indexed as $column) {
$ordered[] = $column;
}
return $ordered;
}

return $columns;
} catch (\OCP\DB\Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
Expand Down
5 changes: 4 additions & 1 deletion lib/Service/RowService.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ public function findAllByTable(int $tableId, string $userId, ?int $limit = null,
$tableColumns = $this->columnMapper->findAllByTable($tableId);
$showColumnIds = array_map(fn (Column $column) => $column->getId(), $tableColumns);

return $this->row2Mapper->findAll($showColumnIds, $tableId, $limit, $offset, null, null, $userId);
$table = $this->tableMapper->find($tableId);
$sort = $table->getSortArray() ?: null;

return $this->row2Mapper->findAll($showColumnIds, $tableId, $limit, $offset, null, $sort, $userId);
} else {
throw new PermissionError('no read access to table id = ' . $tableId);
}
Expand Down
Loading
Loading