From 322644d4da4adb9abc6b2eb1b36f3a99d64ad842 Mon Sep 17 00:00:00 2001 From: Rello Date: Mon, 11 May 2026 14:16:54 +0200 Subject: [PATCH 01/43] feat: add persistent per-view layout modes Signed-off-by: Rello Signed-off-by: Rello AI-assistant: Codex (GPT-5) Co-Authored-By: Andy Scherzinger Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../component/ViewSettingsSavedLayout.cy.js | 53 +++ lib/Constants/ViewUpdatableParameters.php | 2 + lib/Controller/Api1Controller.php | 7 +- lib/Controller/ApiTablesController.php | 7 + lib/Controller/ViewController.php | 6 +- lib/Db/View.php | 17 + .../Version1000Date20260318000000.php | 45 +++ lib/Model/ViewSettings.php | 61 ++++ lib/Model/ViewUpdateInput.php | 62 +++- lib/ResponseDefinitions.php | 2 + lib/Service/StructureService.php | 52 +-- lib/Service/TableTemplateService.php | 2 +- lib/Service/ViewService.php | 49 ++- openapi.json | 64 ++++ playwright/e2e/view-layout.spec.ts | 111 ++++++ src/modules/main/sections/MainWrapper.vue | 13 +- src/modules/modals/ViewSettings.vue | 145 +++++++- .../partials/NavigationViewItem.vue | 2 + src/pages/View.vue | 10 +- src/shared/components/ncTable/NcTable.vue | 10 + .../ncTable/sections/CustomTable.vue | 345 +++++++++++++++++- src/types/openapi/openapi.ts | 22 ++ tests/unit/Db/ViewTest.php | 70 ++++ tests/unit/Model/ViewUpdateInputTest.php | 66 ++++ tests/unit/Service/StructureServiceTest.php | 26 ++ 25 files changed, 1200 insertions(+), 49 deletions(-) create mode 100644 cypress/component/ViewSettingsSavedLayout.cy.js create mode 100644 lib/Migration/Version1000Date20260318000000.php create mode 100644 lib/Model/ViewSettings.php create mode 100644 playwright/e2e/view-layout.spec.ts create mode 100644 tests/unit/Db/ViewTest.php diff --git a/cypress/component/ViewSettingsSavedLayout.cy.js b/cypress/component/ViewSettingsSavedLayout.cy.js new file mode 100644 index 0000000000..624af3879e --- /dev/null +++ b/cypress/component/ViewSettingsSavedLayout.cy.js @@ -0,0 +1,53 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import ViewSettings from '../../src/modules/modals/ViewSettings.vue' + +const VIEW = { + id: 1, + tableId: 1, + title: 'A view', + layout: 'table', + columnSettings: [{ columnId: 1, order: 0 }], + viewSettings: { cardBackgroundSource: null, cardTitleSource: null }, + sort: [], + filter: [], +} + +describe('View settings layout', () => { + it('offers the layout a viewer switched to when the modified view is saved', () => { + // Every other local adjustment is merged into "Save modified View", so the layout + // the viewer is actually looking at has to be the one it offers to save. + mountDialog({ layout: 'gallery' }).then(({ wrapper }) => { + cy.wrap(null) + .then(() => wrapper.vm.reset()) + .then(() => expect(wrapper.vm.layout).to.equal('gallery')) + }) + }) + + it('offers the saved layout while the viewer has not switched', () => { + mountDialog({ hiddenColumns: [] }).then(({ wrapper }) => { + cy.wrap(null) + .then(() => wrapper.vm.reset()) + .then(() => expect(wrapper.vm.layout).to.equal('table')) + }) + }) +}) + +/** + * Mounts the view settings dialog for an existing view with a local view setting. + * + * @param {object} viewSetting the local adjustments the viewer made + * @return {Cypress.Chainable} the mount result + */ +function mountDialog(viewSetting) { + return cy.mount(ViewSettings, { + props: { + showModal: false, + view: VIEW, + createView: false, + viewSetting, + }, + }) +} diff --git a/lib/Constants/ViewUpdatableParameters.php b/lib/Constants/ViewUpdatableParameters.php index b6a5d8139b..fe636201c4 100644 --- a/lib/Constants/ViewUpdatableParameters.php +++ b/lib/Constants/ViewUpdatableParameters.php @@ -18,4 +18,6 @@ enum ViewUpdatableParameters: string { case COLUMN_SETTINGS = 'columns'; case TECHNICAL_NAME = 'technicalName'; case SIDEBAR_ORDER = 'sidebarOrder'; + case LAYOUT = 'layout'; + case VIEW_SETTINGS = 'viewSettings'; } diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index e55287c238..1e5984a4d9 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -323,6 +323,7 @@ public function indexViews(int $tableId): DataResponse { * @param string $title Title for the view * @param string|null $emoji Emoji for the view * @param string|null $technicalName Technical name for the view + * @param 'table'|'tiles'|'gallery'|null $layout Layout for the view * * @return DataResponse|DataResponse * @@ -336,9 +337,9 @@ public function indexViews(int $tableId): DataResponse { #[CORS] #[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - public function createView(int $tableId, string $title, ?string $emoji, ?string $technicalName = null): DataResponse { + public function createView(int $tableId, string $title, ?string $emoji, ?string $technicalName = null, ?string $layout = null): DataResponse { try { - return new DataResponse($this->viewService->create($title, $emoji, $this->tableService->find($tableId), null, $technicalName)->jsonSerialize()); + return new DataResponse($this->viewService->create($title, $emoji, $this->tableService->find($tableId), null, $technicalName, layout: $layout)->jsonSerialize()); } catch (PermissionError $e) { $this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]); $message = ['message' => $e->getMessage()]; @@ -398,6 +399,8 @@ public function getView(int $viewId): DataResponse { * columns?: list, * columnSettings?: list, * sort?: list, + * layout?: 'table'|'tiles'|'gallery'|null, + * viewSettings?: array{cardBackgroundSource?: int|null, cardTitleSource?: int|null}, * filter?: list}>> * } $data fields of the view with their new values * @return DataResponse|DataResponse diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 105f71f0a5..78520be288 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -209,6 +209,11 @@ public function createFromScheme(string $title, string $emoji, string $descripti $table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort); } foreach ($views as $view) { + // A layout this version does not know is dropped rather than rejected, so the rest + // of the scheme still imports. + $schemeLayout = in_array($view['layout'] ?? null, ['table', 'tiles', 'gallery'], true) + ? $view['layout'] + : null; $newView = $this->viewService->create( $view['title'], $view['emoji'], @@ -216,6 +221,7 @@ public function createFromScheme(string $title, string $emoji, string $descripti $this->userId, technicalName: $view['technicalName'] ?? null, uuid: $view['uuid'] ?? null, + layout: $schemeLayout, ); $inputColumnsArray = []; @@ -250,6 +256,7 @@ public function createFromScheme(string $title, string $emoji, string $descripti 'description' => $view['description'] ?? '', 'sort' => $newSort, 'filter' => $newFilter, + 'layout' => $schemeLayout, ]) )); } diff --git a/lib/Controller/ViewController.php b/lib/Controller/ViewController.php index 847996861a..b4f7f55ed9 100644 --- a/lib/Controller/ViewController.php +++ b/lib/Controller/ViewController.php @@ -56,8 +56,10 @@ public function show(int $id): DataResponse { #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] - public function create(int $tableId, string $title, ?string $emoji): DataResponse { - return $this->handleError(fn () => $this->service->create($title, $emoji, $this->getTable($tableId, true))); + public function create(int $tableId, string $title, ?string $emoji, ?string $layout = null): DataResponse { + return $this->handleError(function () use ($tableId, $title, $emoji, $layout) { + return $this->service->create($title, $emoji, $this->getTable($tableId, true), null, layout: $layout); + }); } #[NoAdminRequired] diff --git a/lib/Db/View.php b/lib/Db/View.php index fb86c02c9f..215897725b 100644 --- a/lib/Db/View.php +++ b/lib/Db/View.php @@ -13,6 +13,7 @@ use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\Permissions; use OCA\Tables\Model\SortRuleSet; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ValueObject\ViewColumnInformation; use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; @@ -50,6 +51,10 @@ * @method setEmoji(string $emoji) * @method getDescription(): string * @method setDescription(string $description) + * @method getLayout(): ?string + * @method setLayout(?string $layout) + * @method getViewSettings(): ?string + * @method setViewSettings(?string $viewSettings) * @method getIsShared(): bool * @method setIsShared(bool $isShared) * @method getOnSharePermissions(): ?Permissions @@ -88,6 +93,8 @@ class View extends EntitySuper implements JsonSerializable { protected ?string $columns = null; // json protected ?string $sort = null; // json protected ?string $filter = null; // json + protected ?string $layout = null; + protected ?string $viewSettings = null; // json protected ?int $externalId = null; protected ?string $shareToken = null; @@ -212,6 +219,14 @@ public function setFilterArray(array $array):void { $this->setFilter(\json_encode($array)); } + public function getLayoutNormalized(): string { + return in_array($this->layout, ['tiles', 'gallery'], true) ? $this->layout : 'table'; + } + + public function getViewSettingsObject(): ViewSettings { + return ViewSettings::createFromInputArray($this->getArray($this->getViewSettings())); + } + private function getSharePermissions(): ?Permissions { return $this->getOnSharePermissions(); } @@ -244,6 +259,8 @@ public function jsonSerialize(): array { 'ownerDisplayName' => $this->ownerDisplayName, 'isFederated' => $this->isFederated(), 'sidebarOrder' => $this->sidebarOrder, + 'layout' => $this->getLayoutNormalized(), + 'viewSettings' => $this->getViewSettingsObject()->jsonSerialize(), ]; $serialisedJson['filter'] = $this->getFilterArray(); diff --git a/lib/Migration/Version1000Date20260318000000.php b/lib/Migration/Version1000Date20260318000000.php new file mode 100644 index 0000000000..a96408e5ee --- /dev/null +++ b/lib/Migration/Version1000Date20260318000000.php @@ -0,0 +1,45 @@ +hasTable($tableName)) { + return null; + } + + $table = $schema->getTable($tableName); + if (!$table->hasColumn('layout')) { + $table->addColumn('layout', Types::STRING, [ + 'notnull' => false, + 'length' => 16, + ]); + } + if (!$table->hasColumn('view_settings')) { + $table->addColumn('view_settings', Types::JSON, [ + 'notnull' => false, + ]); + } + + return $schema; + } +} diff --git a/lib/Model/ViewSettings.php b/lib/Model/ViewSettings.php new file mode 100644 index 0000000000..a018a81df6 --- /dev/null +++ b/lib/Model/ViewSettings.php @@ -0,0 +1,61 @@ +cardBackgroundSource; + } + + public function getCardTitleSource(): ?int { + return $this->cardTitleSource; + } + + /** + * @return array{cardBackgroundSource: int|null, cardTitleSource: int|null} + */ + public function jsonSerialize(): array { + return [ + 'cardBackgroundSource' => $this->cardBackgroundSource, + 'cardTitleSource' => $this->cardTitleSource, + ]; + } + + private static function nullableIntFromArray(array $data, string $key): ?int { + if (!array_key_exists($key, $data) || $data[$key] === null) { + return null; + } + + if (!is_int($data[$key])) { + throw new InvalidArgumentException('Invalid ' . $key . ' value.'); + } + + return $data[$key]; + } +} diff --git a/lib/Model/ViewUpdateInput.php b/lib/Model/ViewUpdateInput.php index 8cfd5c0a49..09552c109a 100644 --- a/lib/Model/ViewUpdateInput.php +++ b/lib/Model/ViewUpdateInput.php @@ -31,6 +31,8 @@ public function __construct( protected readonly ?FilterSet $filterSet = null, protected readonly ?SortRuleSet $sortRuleSet = null, protected readonly ?int $sidebarOrder = null, + protected readonly ?string $layout = null, + protected readonly ?ViewSettings $viewSettings = null, ) { } @@ -59,6 +61,12 @@ public function updateDetail(): Generator { if ($this->sortRuleSet) { yield ViewUpdatableParameters::SORT => $this->sortRuleSet; } + if ($this->layout !== null) { + yield ViewUpdatableParameters::LAYOUT => $this->layout; + } + if ($this->viewSettings !== null) { + yield ViewUpdatableParameters::VIEW_SETTINGS => $this->viewSettings; + } } /** @@ -70,13 +78,15 @@ public function updateDetail(): Generator { * columns?: list, * columnSettings?: list, * sort?: list, + * layout?: 'table'|'tiles'|'gallery'|null, + * viewSettings?: array{cardBackgroundSource?: int|null, cardTitleSource?: int|null}|string, * filter?: list}>>, * sidebarOrder?: int * } $data * @param array $columnsMap */ public static function fromInputArray(array $data, array $columnsMap = []): self { - $data = self::transformJsonToArrayInPayload($data, ['columnSettings', 'filter', 'sort']); + $data = self::transformJsonToArrayInPayload($data, ['columnSettings', 'filter', 'sort', 'viewSettings']); if (isset($data['columns']) && !isset($data['columnSettings'])) { $logger = Server::get(LoggerInterface::class); @@ -91,6 +101,9 @@ public static function fromInputArray(array $data, array $columnsMap = []): self $data['columnSettings'] = $value; } + $layout = self::normalizeLayout($data['layout'] ?? null); + $viewSettings = self::createViewSettingsFromInputData($data); + return new self( title: ($data['title'] ?? null) ? new Title($data['title']) : null, technicalName: $data['technicalName'] ?? null, @@ -100,9 +113,56 @@ public static function fromInputArray(array $data, array $columnsMap = []): self filterSet: isset($data['filter']) ? FilterSet::createFromInputArray($data['filter'], $columnsMap) : null, sortRuleSet: isset($data['sort']) ? SortRuleSet::createFromInputArray($data['sort'], $columnsMap) : null, sidebarOrder: (array_key_exists('sidebarOrder', $data) && $data['sidebarOrder'] !== null) ? (int)$data['sidebarOrder'] : null, + layout: $layout, + viewSettings: $viewSettings, ); } + private static function createViewSettingsFromInputData(array $data): ?ViewSettings { + if (array_key_exists('viewSettings', $data)) { + if ($data['viewSettings'] === null) { + return new ViewSettings(); + } + if (!is_array($data['viewSettings'])) { + throw new \InvalidArgumentException('Invalid viewSettings value.'); + } + return ViewSettings::createFromInputArray($data['viewSettings']); + } + + $legacyKeys = ['cardBackgroundSource', 'cardTitleSource']; + $hasLegacySettings = false; + foreach ($legacyKeys as $legacyKey) { + if (array_key_exists($legacyKey, $data)) { + $hasLegacySettings = true; + break; + } + } + if (!$hasLegacySettings) { + return null; + } + + return ViewSettings::createFromInputArray([ + 'cardBackgroundSource' => $data['cardBackgroundSource'] ?? null, + 'cardTitleSource' => $data['cardTitleSource'] ?? null, + ]); + } + + private static function normalizeLayout(mixed $layout): ?string { + if ($layout === null || $layout === '') { + return null; + } + + if (!is_string($layout)) { + throw new \InvalidArgumentException('Invalid layout value.'); + } + + if (!in_array($layout, ['table', 'tiles', 'gallery'], true)) { + throw new \InvalidArgumentException('Invalid layout value.'); + } + + return $layout; + } + protected static function transformJsonToArrayInPayload(array $input, array $keys): array { $output = $input; foreach ($keys as $targetKey) { diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index c935d72524..194aa0a24a 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -32,6 +32,8 @@ * columnSettings:list, * sort: list, * filter: list}>>, + * layout: 'table'|'tiles'|'gallery', + * viewSettings: array{cardBackgroundSource: int|null, cardTitleSource: int|null}, * isShared: bool, * favorite: bool, * onSharePermissions: ?array{ diff --git a/lib/Service/StructureService.php b/lib/Service/StructureService.php index a63c4c4ffd..3a3d1e9962 100644 --- a/lib/Service/StructureService.php +++ b/lib/Service/StructureService.php @@ -287,32 +287,34 @@ protected function isViewModified($viewA, $viewB): bool { $viewB = $viewB->jsonSerialize(); } - $jsonViewA = json_encode( - [ - 'title' => $viewA['title'], - 'technicalName' => $viewA['technicalName'], - 'description' => $viewA['description'], - 'emoji' => $viewA['emoji'], - 'columns' => $viewA['columns'], - 'columnSettings' => $viewA['columnSettings'], - 'sort' => $viewA['sort'], - 'filter' => $viewA['filter'], - ] - ); - $jsonViewB = json_encode( - [ - 'title' => $viewB['title'], - 'technicalName' => $viewB['technicalName'], - 'description' => $viewB['description'], - 'emoji' => $viewB['emoji'], - 'columns' => $viewB['columns'], - 'columnSettings' => $viewB['columnSettings'], - 'sort' => $viewB['sort'], - 'filter' => $viewB['filter'], - ] - ); + return $this->comparableView($viewA) !== $this->comparableView($viewB); + } - return $jsonViewA !== $jsonViewB; + /** + * The comparable form of a view. + * + * A scheme written before the layout existed, or by hand, carries neither key. Both + * sides are normalised so such a view does not read as modified on every import, + * which would cost a write, an activity entry and a federation notification each. + */ + private function comparableView(array $view): array { + $viewSettings = is_array($view['viewSettings'] ?? null) ? $view['viewSettings'] : []; + + return [ + 'title' => $view['title'], + 'technicalName' => $view['technicalName'], + 'description' => $view['description'], + 'emoji' => $view['emoji'], + 'columns' => $view['columns'], + 'columnSettings' => $view['columnSettings'], + 'sort' => $view['sort'], + 'filter' => $view['filter'], + 'layout' => in_array($view['layout'] ?? null, ['tiles', 'gallery'], true) ? $view['layout'] : 'table', + 'viewSettings' => [ + 'cardBackgroundSource' => $viewSettings['cardBackgroundSource'] ?? null, + 'cardTitleSource' => $viewSettings['cardTitleSource'] ?? null, + ], + ]; } protected function resolveColumnOrderChanges(array $currentSchema, array $updateSchema): void { diff --git a/lib/Service/TableTemplateService.php b/lib/Service/TableTemplateService.php index 3da17e6ecd..771df1aac3 100644 --- a/lib/Service/TableTemplateService.php +++ b/lib/Service/TableTemplateService.php @@ -822,7 +822,7 @@ private function createRow(Table $table, array $values): void { private function createView(Table $table, array $data): void { try { $inputData = ViewUpdateInput::fromInputArray($data); - $view = $this->viewService->create($data['title'], $data['emoji'], $table); + $view = $this->viewService->create($data['title'], $data['emoji'], $table, layout: $data['layout'] ?? null); $this->viewService->update($view->getId(), $inputData); } catch (PermissionError $e) { $this->logger->warning('Cannot create view, permission denied: ' . $e->getMessage()); diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index c20ff4bedb..6dcdf7b398 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -31,6 +31,7 @@ use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\Permissions; use OCA\Tables\Model\SortRuleSet; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ValueObject\ViewColumnInformation; @@ -185,6 +186,7 @@ public function create( ?string $userId = null, ?string $technicalName = null, ?string $uuid = null, + ?string $layout = null, ): View { /** @var string $userId */ $userId = $this->permissionsService->preCheckUserId($userId, false); // $userId is set @@ -202,6 +204,7 @@ public function create( $item->setEmoji($emoji); } $item->setDescription(''); + $item->setLayout(in_array($layout, ['tiles', 'gallery'], true) ? $layout : null); $item->setTableId($table->getId()); $item->setCreatedBy($userId); $item->setLastEditBy($userId); @@ -268,14 +271,16 @@ public function update(int $id, ViewUpdateInput $data, ?string $userId = null, b $this->assertTechnicalNameValid($value); } - if ($value instanceof JsonSerializable) { - $insertableValue = json_encode($value); - } + $insertableValue = $value instanceof JsonSerializable + ? json_encode($value) + : $value; $setterMethod = 'set' . ucfirst((string)$parameter->value); - $view->$setterMethod($insertableValue ?? $value); + $view->$setterMethod($insertableValue); } + $this->assertCardSourceColumnsAreValid($view); + $time = new DateTime(); $view->setLastEditBy($userId); $view->setLastEditAt($time->format('Y-m-d H:i:s')); @@ -322,6 +327,29 @@ protected function assertInputColumnsAreValid(View $view, string $userId, Column } } + /** + * Ensures that card view settings reference columns that are part of the view. + * @throws InvalidArgumentException + */ + protected function assertCardSourceColumnsAreValid(View $view): void { + $viewColumnIds = $view->getColumnIds(); + if (empty($viewColumnIds)) { + return; + } + + $viewSettings = $view->getViewSettingsObject(); + + $backgroundSource = $viewSettings->getCardBackgroundSource(); + if ($backgroundSource !== null && !in_array($backgroundSource, $viewColumnIds, true)) { + throw new InvalidArgumentException('Invalid cardBackgroundSource column ID: ' . $backgroundSource); + } + + $titleSource = $viewSettings->getCardTitleSource(); + if ($titleSource !== null && !in_array($titleSource, $viewColumnIds, true)) { + throw new InvalidArgumentException('Invalid cardTitleSource column ID: ' . $titleSource); + } + } + /** * @param int $id * @param string|null $userId @@ -682,6 +710,8 @@ public function importView(int $tableId, array $view, string $userId): View { $item->setColumns(json_encode($view['columnSettings'])); $item->setSort(json_encode($view['sort'])); $item->setFilter(json_encode($view['filter'])); + $item->setLayout(in_array($view['layout'] ?? null, ['tiles', 'gallery'], true) ? $view['layout'] : null); + $item->setViewSettings(json_encode($this->createImportedViewSettings($view))); try { $importedView = $this->mapper->insert($item); if ($item->getTechnicalName() === null || $item->getTechnicalName() === '') { @@ -725,4 +755,15 @@ private function handleViewPersistDbException(\OCP\DB\Exception $e, string $cont $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError($context . ': ' . $e->getMessage()); } + + private function createImportedViewSettings(array $view): ViewSettings { + if (isset($view['viewSettings']) && is_array($view['viewSettings'])) { + return ViewSettings::createFromInputArray($view['viewSettings']); + } + + return ViewSettings::createFromInputArray([ + 'cardBackgroundSource' => $view['cardBackgroundSource'] ?? null, + 'cardTitleSource' => $view['cardTitleSource'] ?? null, + ]); + } } diff --git a/openapi.json b/openapi.json index 80e327a7a7..c140ed6423 100644 --- a/openapi.json +++ b/openapi.json @@ -1008,6 +1008,8 @@ "columnSettings", "sort", "filter", + "layout", + "viewSettings", "isShared", "favorite", "onSharePermissions", @@ -1209,6 +1211,33 @@ } } }, + "layout": { + "type": "string", + "enum": [ + "table", + "tiles", + "gallery" + ] + }, + "viewSettings": { + "type": "object", + "required": [ + "cardBackgroundSource", + "cardTitleSource" + ], + "properties": { + "cardBackgroundSource": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "cardTitleSource": { + "type": "integer", + "format": "int64", + "nullable": true + } + } + }, "isShared": { "type": "boolean" }, @@ -2080,6 +2109,17 @@ "nullable": true, "default": null, "description": "Technical name for the view" + }, + "layout": { + "type": "string", + "nullable": true, + "default": null, + "enum": [ + "table", + "tiles", + "gallery" + ], + "description": "Layout for the view" } } } @@ -2399,6 +2439,30 @@ } } }, + "layout": { + "type": "string", + "nullable": true, + "enum": [ + "table", + "tiles", + "gallery" + ] + }, + "viewSettings": { + "type": "object", + "properties": { + "cardBackgroundSource": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "cardTitleSource": { + "type": "integer", + "format": "int64", + "nullable": true + } + } + }, "filter": { "type": "array", "items": { diff --git a/playwright/e2e/view-layout.spec.ts b/playwright/e2e/view-layout.spec.ts new file mode 100644 index 0000000000..3a6c62c2d5 --- /dev/null +++ b/playwright/e2e/view-layout.spec.ts @@ -0,0 +1,111 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { test, expect } from '../support/fixtures' +import type { Page } from '@playwright/test' +import { + clickOnTableThreeDotMenu, + createTable, + createTextLineColumn, + fillInValueTextLine, + loadTable, + loadView, +} from '../support/commands' + +const tableTitle = 'Layout mode test table' +const defaultViewTitle = 'Layout default table view' +const tilesViewTitle = 'Layout tiles view' +const galleryViewTitle = 'Layout gallery view' + +async function setupLayoutTable(page: Page) { + await createTable(page, tableTitle) + await createTextLineColumn(page, 'preview', '', '', true) + await createTextLineColumn(page, 'title', '', '', false) + await createTextLineColumn(page, 'category', '', '', false) + + const addRow = async (preview: string, title: string, category: string) => { + await page.locator('[data-cy="createRowBtn"]').click() + await fillInValueTextLine(page, 'preview', preview) + await fillInValueTextLine(page, 'title', title) + await fillInValueTextLine(page, 'category', category) + await page.locator('[data-cy="createRowSaveButton"]').click() + } + + await addRow('not-a-preview-url', 'First layout row', 'Catalogue') +} + +async function createLayoutView(page: Page, title: string, layout?: 'tiles' | 'gallery') { + await loadTable(page, tableTitle) + await clickOnTableThreeDotMenu(page, 'Create view') + + const titleInput = page.locator('[data-cy="viewSettingsDialogTitleInput"]') + await titleInput.waitFor({ state: 'visible', timeout: 10000 }) + await titleInput.fill(title) + + if (layout) { + await page.locator(`[data-cy="viewLayout${layout === 'tiles' ? 'Tiles' : 'Gallery'}"]`).click() + } + + const createViewReqPromise = page.waitForResponse( + response => response.url().includes('/apps/tables/view') && response.request().method() === 'POST', + ) + await page.locator('[data-cy="modifyViewBtn"]').click() + await createViewReqPromise + + await expect(page.locator('[data-cy="navigationViewItem"]').filter({ hasText: title }).first()).toBeVisible() +} + +async function expectTableLayout(page: Page) { + await expect(page.locator('[data-cy="customTableRow"]').filter({ hasText: 'First layout row' }).first()).toBeVisible() + await expect(page.locator('[data-cy="tilesLayoutCard"]')).toHaveCount(0) + await expect(page.locator('[data-cy="galleryLayoutCard"]')).toHaveCount(0) +} + +async function expectTilesLayout(page: Page) { + const cards = page.locator('[data-cy="tilesLayoutCard"]') + await expect(cards).toHaveCount(1) + await expect(cards.filter({ hasText: 'First layout row' }).first()).toBeVisible() + await expect(page.locator('[data-cy="galleryLayoutBody"]')).toHaveCount(0) + await expect(page.locator('[data-cy="customTableRow"]')).toHaveCount(0) +} + +async function expectGalleryLayout(page: Page) { + const cards = page.locator('[data-cy="galleryLayoutCard"]') + await expect(cards).toHaveCount(1) + + const firstCard = cards.filter({ hasText: 'First layout row' }).first() + await expect(firstCard).toBeVisible() + await expect(firstCard.locator('[data-cy="galleryMetadataItem"]').filter({ hasText: 'category' })).toContainText('Catalogue') + await expect(page.locator('[data-cy="customTableRow"]')).toHaveCount(0) +} + +test.describe('View layout modes', () => { + test('renders and persists table, tiles, and gallery layouts for views', async ({ userPage: { page } }) => { + test.setTimeout(120000) + await page.goto('/index.php/apps/tables') + await setupLayoutTable(page) + await loadTable(page, tableTitle) + + await createLayoutView(page, defaultViewTitle) + await loadView(page, defaultViewTitle) + await expectTableLayout(page) + + await createLayoutView(page, tilesViewTitle, 'tiles') + await loadView(page, tilesViewTitle) + await expectTilesLayout(page) + + await page.reload({ waitUntil: 'domcontentloaded' }) + await expect(page.locator('.icon-loading').first()).toBeHidden({ timeout: 10000 }) + await expectTilesLayout(page) + + await createLayoutView(page, galleryViewTitle, 'gallery') + await loadView(page, galleryViewTitle) + await expectGalleryLayout(page) + + await loadTable(page, tableTitle) + await loadView(page, galleryViewTitle) + await expectGalleryLayout(page) + }) +}) diff --git a/src/modules/main/sections/MainWrapper.vue b/src/modules/main/sections/MainWrapper.vue index 225d1cd8be..dd03b3ac5f 100644 --- a/src/modules/main/sections/MainWrapper.vue +++ b/src/modules/main/sections/MainWrapper.vue @@ -167,7 +167,18 @@ export default { // Since we show one page at a time, no need keep other tables in the store this.clearState() - this.viewSetting = {} + this.viewSetting = { + layout: this.isView ? (this.element?.layout ?? 'table') : 'table', + } + if (this.isView) { + this.viewSetting = { + ...this.viewSetting, + viewSettings: { + cardBackgroundSource: this.element?.viewSettings?.cardBackgroundSource ?? null, + cardTitleSource: this.element?.viewSettings?.cardTitleSource ?? null, + }, + } + } if (this.element?.sort?.length) { this.viewSetting.presetSorting = [...this.element.sort] } diff --git a/src/modules/modals/ViewSettings.vue b/src/modules/modals/ViewSettings.vue index 0066ab5e0e..aa25e0718a 100644 --- a/src/modules/modals/ViewSettings.vue +++ b/src/modules/modals/ViewSettings.vue @@ -53,6 +53,48 @@ :generated-filters="viewSetting ? generatedView.filter : null" :columns="allColumns" /> + + + + + + + + + +
+
+
+ {{ t('tables', 'Background source') }} +
+ +
+
+
+ {{ t('tables', 'Title source') }} +
+ +
+
+ @@ -222,6 +413,146 @@ export default { margin-top: 0!important; } +.selected-page{ + padding-inline-start: 5px; + + display:inline-flex; + align-items: center; +} + +.page-number{ + padding-inline: 5px; +} + +.large-width{ + width: 100vw !important; + inset-inline-start: 0 !important; +} + +.pagination-items{ + background-color: var(--color-main-background); + border-radius: var(--border-radius-large); + pointer-events: all; + + display: flex; + align-items: center; +} + +.pagination-footer{ + box-shadow: var(--box-shadow); + filter: drop-shadow(0 1px 6px var(--color-box-shadow)); + padding-bottom: 20px; + width: calc(100vw - 316px); + pointer-events: none; + + display: flex; + justify-content: center; + align-items: center; + + :deep(.v-select) { + min-width: 95px !important; + } +} + +.container { + min-width: 0; +} + +.container--cards { + width: var(--app-content-width, 100%); + max-width: var(--app-content-width, 100%); +} + +.card-layout { + width: 100%; + display: grid; + grid-auto-flow: row; + gap: 16px; + padding-inline: calc(var(--default-grid-baseline) * 2); + padding-top: 8px; + grid-template-columns: repeat(auto-fit, minmax(min(100%, 220px), 1fr)); +} + +.layout-card { + width: 100%; + max-width: 100%; + padding: 0; + border: 1px solid var(--color-border-dark); + border-radius: var(--border-radius-large); + overflow: hidden; + background: var(--color-main-background); + text-align: start; + cursor: pointer; + color: var(--color-main-text); +} + +.layout-card__image-wrapper { + position: relative; + aspect-ratio: 3 / 2; + background: var(--color-background-dark); +} + +.layout-card__image { + width: 100%; + height: 100%; + object-fit: cover; + display: block; +} + +.layout-card__no-image { + display: flex; + align-items: center; + justify-content: center; + height: 100%; + color: var(--color-text-maxcontrast); +} + +.layout-card__title-banner { + position: absolute; + inset-inline: 0; + bottom: 0; + padding: 12px; + background: rgba(0,0,0,0.4); + color: #fff; + text-align: start; + font-weight: 600; + display: flex; + justify-content: flex-start; +} + +.layout-card__body { + padding: 12px; + max-height: 168px; + overflow: hidden; +} + +.layout-card__metadata { + list-style: none; + padding: 0; + margin: 0; + display: flex; + flex-direction: column; + gap: 14px; +} + +.layout-card__metadata-label { + font-size: 12px; + font-weight: 600; + color: var(--color-text-maxcontrast); + margin-bottom: 2px; +} + +.layout-card__metadata-value { + font-weight: 400; + color: var(--color-main-text); + white-space: normal; + overflow-wrap: anywhere; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; +} + :deep(table) { position: relative; border-collapse: collapse; @@ -233,7 +564,6 @@ export default { * { border: none; } - // white-space: nowrap; td, th { padding-inline-end: 8px; @@ -274,7 +604,6 @@ export default { } tbody { - td { text-align: start; vertical-align: middle; @@ -298,7 +627,6 @@ export default { background-color: inherit; } - // viewer integration .editor-wrapper { min-width: 100px; overflow-y: auto; @@ -320,7 +648,6 @@ export default { } } - // inline editing .inline-editing-container { position: relative; width: 100%; @@ -389,15 +716,12 @@ export default { } tr>td.sticky:last-child { - // visibility: hidden; opacity: 0; } tr:hover>td:last-child { - // visibility: visible; opacity: 1; } - } .table-row-leave-active { @@ -413,5 +737,4 @@ export default { margin-bottom: 0 !important; transform: translateX(-1rem); } - diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index e8d5e3a0c9..2c3d2b29ab 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1401,6 +1401,14 @@ export type components = { readonly uuid?: string; }[]; }[])[]; + /** @enum {string} */ + readonly layout: "table" | "tiles" | "gallery"; + readonly viewSettings: { + /** Format: int64 */ + readonly cardBackgroundSource: number | null; + /** Format: int64 */ + readonly cardTitleSource: number | null; + }; readonly isShared: boolean; readonly favorite: boolean; readonly onSharePermissions: { @@ -1917,6 +1925,12 @@ export interface operations { * @default null */ readonly technicalName?: string | null; + /** + * @description Layout for the view + * @default null + * @enum {string|null} + */ + readonly layout?: "table" | "tiles" | "gallery" | null; }; }; }; @@ -2085,6 +2099,14 @@ export interface operations { /** @enum {string} */ readonly mode: "ASC" | "DESC"; }[]; + /** @enum {string|null} */ + readonly layout?: "table" | "tiles" | "gallery" | null; + readonly viewSettings?: { + /** Format: int64 */ + readonly cardBackgroundSource?: number | null; + /** Format: int64 */ + readonly cardTitleSource?: number | null; + }; readonly filter?: readonly (readonly { /** Format: int64 */ readonly columnId: number; diff --git a/tests/unit/Db/ViewTest.php b/tests/unit/Db/ViewTest.php new file mode 100644 index 0000000000..76e6c3095b --- /dev/null +++ b/tests/unit/Db/ViewTest.php @@ -0,0 +1,70 @@ +assertSame('table', (new View())->getLayoutNormalized()); + } + + public function testAStoredLayoutIsKept(): void { + $view = new View(); + $view->setLayout('gallery'); + + $this->assertSame('gallery', $view->getLayoutNormalized()); + } + + /** + * A row may hold a layout only a newer version knows, so it renders as a table rather + * than as nothing at all. + */ + public function testAnUnknownStoredLayoutRendersAsATable(): void { + $view = new View(); + $view->setLayout('carousel'); + + $this->assertSame('table', $view->getLayoutNormalized()); + } + + public function testStoredCardSourcesAreRead(): void { + $view = new View(); + $view->setViewSettings(json_encode(['cardBackgroundSource' => 3, 'cardTitleSource' => 4])); + + $viewSettings = $view->getViewSettingsObject(); + $this->assertSame(3, $viewSettings->getCardBackgroundSource()); + $this->assertSame(4, $viewSettings->getCardTitleSource()); + } + + /** + * @dataProvider provideUnusableViewSettings + */ + public function testUnusableViewSettingsReadAsNoSources(?string $stored): void { + // One malformed row must not fail every endpoint that serialises the view. + $view = new View(); + $view->setViewSettings($stored); + + $viewSettings = $view->getViewSettingsObject(); + $this->assertNull($viewSettings->getCardBackgroundSource()); + $this->assertNull($viewSettings->getCardTitleSource()); + } + + /** + * @return array + */ + public static function provideUnusableViewSettings(): array { + return [ + 'never written' => [null], + 'empty' => [''], + 'json null' => ['null'], + ]; + } +} diff --git a/tests/unit/Model/ViewUpdateInputTest.php b/tests/unit/Model/ViewUpdateInputTest.php index 8df9e05705..4ae037e7c3 100644 --- a/tests/unit/Model/ViewUpdateInputTest.php +++ b/tests/unit/Model/ViewUpdateInputTest.php @@ -9,7 +9,9 @@ namespace OCA\Tables\Tests\Unit\Model; +use InvalidArgumentException; use OCA\Tables\Constants\ViewUpdatableParameters; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use PHPUnit\Framework\TestCase; @@ -37,4 +39,68 @@ public function testUpdateDetailIncludesEmptyDescription(): void { $this->assertArrayHasKey(ViewUpdatableParameters::DESCRIPTION->value, $updates); $this->assertSame('', $updates[ViewUpdatableParameters::DESCRIPTION->value]); } + + public function testUpdateDetailCarriesAKnownLayout(): void { + $input = ViewUpdateInput::fromInputArray(['layout' => 'gallery']); + + $this->assertSame('gallery', $this->updates($input)[ViewUpdatableParameters::LAYOUT->value]); + } + + public function testUpdateDetailOmitsAnAbsentLayout(): void { + $this->assertArrayNotHasKey( + ViewUpdatableParameters::LAYOUT->value, + $this->updates(ViewUpdateInput::fromInputArray(['layout' => ''])), + ); + } + + public function testAnUnknownLayoutIsRejected(): void { + $this->expectException(InvalidArgumentException::class); + + ViewUpdateInput::fromInputArray(['layout' => 'carousel']); + } + + public function testALayoutThatIsNotAStringIsRejected(): void { + $this->expectException(InvalidArgumentException::class); + + ViewUpdateInput::fromInputArray(['layout' => 1]); + } + + public function testViewSettingsArriveAsJsonFromTheClient(): void { + $input = ViewUpdateInput::fromInputArray([ + 'viewSettings' => json_encode(['cardBackgroundSource' => 3, 'cardTitleSource' => null]), + ]); + + $viewSettings = $this->updates($input)[ViewUpdatableParameters::VIEW_SETTINGS->value]; + $this->assertInstanceOf(ViewSettings::class, $viewSettings); + $this->assertSame(3, $viewSettings->getCardBackgroundSource()); + $this->assertNull($viewSettings->getCardTitleSource()); + } + + public function testNullViewSettingsClearTheStoredSources(): void { + // A cleared setting still has to reach the view, so it is an empty value object + // rather than the absent case that updateDetail() skips. + $input = ViewUpdateInput::fromInputArray(['viewSettings' => null]); + + $viewSettings = $this->updates($input)[ViewUpdatableParameters::VIEW_SETTINGS->value]; + $this->assertNull($viewSettings->getCardBackgroundSource()); + $this->assertNull($viewSettings->getCardTitleSource()); + } + + public function testViewSettingsThatAreNotAStructureAreRejected(): void { + $this->expectException(InvalidArgumentException::class); + + ViewUpdateInput::fromInputArray(['viewSettings' => 5]); + } + + /** + * @return array + */ + private function updates(ViewUpdateInput $input): array { + $updates = []; + foreach ($input->updateDetail() as $parameter => $value) { + $updates[$parameter->value] = $value; + } + + return $updates; + } } diff --git a/tests/unit/Service/StructureServiceTest.php b/tests/unit/Service/StructureServiceTest.php index d5104ace92..ac092ca884 100644 --- a/tests/unit/Service/StructureServiceTest.php +++ b/tests/unit/Service/StructureServiceTest.php @@ -32,4 +32,30 @@ public function testNoChanges() { $this->assertSame([], $this->service->removedColumns()); $this->assertSame([], $this->service->modifiedColumns()); } + + /** + * The current state comes from View::jsonSerialize(), which always carries layout and + * viewSettings; a scheme written before they existed carries neither. + */ + public function testViewsWithoutLayoutKeysAreNotModified(): void { + $current = $this->originalSchema; + foreach ($current['views'] as &$view) { + $view['layout'] = 'table'; + $view['viewSettings'] = ['cardBackgroundSource' => null, 'cardTitleSource' => null]; + } + unset($view); + + $this->service->resolveChanges($current, $this->originalSchema); + + $this->assertSame([], $this->service->modifiedViews()); + } + + public function testAChangedLayoutMarksTheViewModified(): void { + $update = $this->originalSchema; + $update['views'][0]['layout'] = 'gallery'; + + $this->service->resolveChanges($this->originalSchema, $update); + + $this->assertCount(1, $this->service->modifiedViews()); + } } From 86bf04cd78cc9e0f0b3fcc539c6a1ab03351e425 Mon Sep 17 00:00:00 2001 From: Rello Date: Wed, 17 Jun 2026 10:56:03 +0200 Subject: [PATCH 02/43] fix(views): resolve test feedback on the layout settings Folds the two rounds of review fixes: the card source handling in ApiTablesController and the settings dialog, then the scheme and migration paths in ViewService, TablesMigrator and the context page, with the migrator test that covers them. Signed-off-by: Rello Co-Authored-By: Andy Scherzinger Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 20 +++-- lib/Model/ViewSettings.php | 26 ++++++ lib/Service/ViewService.php | 12 +++ lib/UserMigration/TablesMigrator.php | 12 +++ src/modules/modals/ViewSettings.vue | 31 +++---- src/pages/Context.vue | 27 +++++- .../ncTable/sections/CustomTable.vue | 4 +- tests/unit/TablesMigratorTest.php | 86 +++++++++++++++++++ 8 files changed, 189 insertions(+), 29 deletions(-) diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 78520be288..eeaaff8d0a 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -17,6 +17,7 @@ use OCA\Tables\Middleware\Attribute\RequirePermission; use OCA\Tables\Model\ColumnSettings; use OCA\Tables\Model\SortRuleSet; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnService; @@ -251,14 +252,17 @@ public function createFromScheme(string $title, string $emoji, string $descripti return $filter; }, $filters), $view['filter']); - $this->viewService->update($newView->getId(), ViewUpdateInput::fromInputArray( - array_merge($inputColumnsArray, [ - 'description' => $view['description'] ?? '', - 'sort' => $newSort, - 'filter' => $newFilter, - 'layout' => $schemeLayout, - ]) - )); + $inputData = array_merge($inputColumnsArray, [ + 'description' => $view['description'] ?? '', + 'sort' => $newSort, + 'filter' => $newFilter, + 'layout' => $schemeLayout, + ]); + if (isset($view['viewSettings']) && is_array($view['viewSettings'])) { + $inputData['viewSettings'] = ViewSettings::remapSources($view['viewSettings'], $colMap); + } + + $this->viewService->update($newView->getId(), ViewUpdateInput::fromInputArray($inputData)); } $this->db->commit(); return new DataResponse($table->jsonSerialize()); diff --git a/lib/Model/ViewSettings.php b/lib/Model/ViewSettings.php index a018a81df6..d555eacb0f 100644 --- a/lib/Model/ViewSettings.php +++ b/lib/Model/ViewSettings.php @@ -13,6 +13,8 @@ use JsonSerializable; class ViewSettings implements JsonSerializable { + public const SOURCE_KEYS = ['cardBackgroundSource', 'cardTitleSource']; + public function __construct( protected readonly ?int $cardBackgroundSource = null, protected readonly ?int $cardTitleSource = null, @@ -58,4 +60,28 @@ private static function nullableIntFromArray(array $data, string $key): ?int { return $data[$key]; } + + /** + * Card sources rewritten through a map of old to new column id. + * + * A source that does not resolve is dropped rather than carried over: the same id + * addresses a different column once the data has moved, so keeping it would point + * the card at unrelated content. + * + * @param array $columnIdMap + */ + public static function remapSources(array $viewSettings, array $columnIdMap): array { + foreach (self::SOURCE_KEYS as $sourceKey) { + if (!array_key_exists($sourceKey, $viewSettings) || $viewSettings[$sourceKey] === null) { + continue; + } + + $sourceId = $viewSettings[$sourceKey]; + $viewSettings[$sourceKey] = is_int($sourceId) && $sourceId > 0 + ? ($columnIdMap[$sourceId] ?? null) + : null; + } + + return $viewSettings; + } } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 6dcdf7b398..b1d10c0354 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -609,12 +609,24 @@ public function deleteColumnDataFromViews(int $columnId, Table $table): void { columnSettings: new ColumnSettings($applicableViewColumnInformationRecords), filterSet: FilterSet::createFromInputArray($applicableFilterArray), sortRuleSet: SortRuleSet::createFromInputArray($filteredSortingRules), + viewSettings: $this->removeColumnFromViewSettings($view->getViewSettingsObject(), $columnId), ); $this->update($view->getId(), $viewUpdateInput); } } + private function removeColumnFromViewSettings(ViewSettings $viewSettings, int $columnId): ViewSettings { + return new ViewSettings( + cardBackgroundSource: $viewSettings->getCardBackgroundSource() === $columnId + ? null + : $viewSettings->getCardBackgroundSource(), + cardTitleSource: $viewSettings->getCardTitleSource() === $columnId + ? null + : $viewSettings->getCardTitleSource(), + ); + } + protected function removeColumnFromFilters(array $originalFilterSetArray, int $columnId): array { $applicableFilterSetArray = []; foreach ($originalFilterSetArray as $filterGroupArray) { diff --git a/lib/UserMigration/TablesMigrator.php b/lib/UserMigration/TablesMigrator.php index 7a039ad38a..22ff712e0e 100644 --- a/lib/UserMigration/TablesMigrator.php +++ b/lib/UserMigration/TablesMigrator.php @@ -27,6 +27,7 @@ use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; use OCA\Tables\Service\FavoritesService; @@ -356,11 +357,22 @@ private function importViews(IImportSource $importSource, array $tableIdMap, arr } unset($setting); } + $view = $this->remapViewCardSources($view, $columnIdMap); $this->viewService->importView($newTableId, $view, $userId); } } } + private function remapViewCardSources(array $view, array $columnIdMap): array { + if (isset($view['viewSettings']) && is_array($view['viewSettings'])) { + $view['viewSettings'] = ViewSettings::remapSources($view['viewSettings'], $columnIdMap); + + return $view; + } + + return ViewSettings::remapSources($view, $columnIdMap); + } + /** * @param IImportSource $importSource * @param Table $newTable diff --git a/src/modules/modals/ViewSettings.vue b/src/modules/modals/ViewSettings.vue index aa25e0718a..81f9765c53 100644 --- a/src/modules/modals/ViewSettings.vue +++ b/src/modules/modals/ViewSettings.vue @@ -264,13 +264,7 @@ export default { }, backgroundSourceValue: { get() { - const stored = this.mutableView?.viewSettings?.cardBackgroundSource - // Use stored value only if the column still exists in the accessible options - if (stored !== null && stored !== undefined) { - const exists = this.cardSourceOptions.some(opt => opt.id === stored) - if (exists) return stored - } - return this.cardSourceOptions[0]?.id ?? null + return this.resolveCardSourceValue(this.mutableView?.viewSettings?.cardBackgroundSource, 0) }, set(value) { this.ensureMutableViewSettings() @@ -279,14 +273,7 @@ export default { }, titleSourceValue: { get() { - const stored = this.mutableView?.viewSettings?.cardTitleSource - // Use stored value only if the column still exists in the accessible options - if (stored !== null && stored !== undefined) { - const exists = this.cardSourceOptions.some(opt => opt.id === stored) - if (exists) return stored - } - // Fallback: second option if available, otherwise first - return this.cardSourceOptions[1]?.id ?? this.cardSourceOptions[0]?.id ?? null + return this.resolveCardSourceValue(this.mutableView?.viewSettings?.cardTitleSource, 1) }, set(value) { this.ensureMutableViewSettings() @@ -469,6 +456,7 @@ export default { readonly: col.viewColumnInformation?.readonly, mandatory: col.viewColumnInformation?.mandatory ?? false, })) + this.persistCardSourceFallbacks() const data = { data: { title: this.title, @@ -525,6 +513,19 @@ export default { }) } }, + resolveCardSourceValue(stored, fallbackIndex) { + if (stored !== null && stored !== undefined) { + const exists = this.cardSourceOptions.some(option => option.id === stored) + if (exists) return stored + } + + return this.cardSourceOptions[fallbackIndex]?.id ?? this.cardSourceOptions[0]?.id ?? null + }, + persistCardSourceFallbacks() { + this.ensureMutableViewSettings() + this.$set(this.mutableView.viewSettings, 'cardBackgroundSource', this.resolveCardSourceValue(this.mutableView.viewSettings.cardBackgroundSource, 0)) + this.$set(this.mutableView.viewSettings, 'cardTitleSource', this.resolveCardSourceValue(this.mutableView.viewSettings.cardTitleSource, 1)) + }, loadEmoji() { const emojis = ['😀', '😃', '😄', '😁', '😆', '😅', '🤣', '😂', '🙂', '🙃', '🫠', '😉', '😊', '😇'] return emojis[~~(Math.random() * emojis.length)] diff --git a/src/pages/Context.vue b/src/pages/Context.vue index 3e85a8167e..c1236e73ff 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -23,14 +23,14 @@
@@ -80,7 +80,7 @@ export default { return { loading: true, icon: null, - viewSetting: {}, + resourceViewSettings: {}, context: null, contextResources: [], errorMessage: null, @@ -175,6 +175,7 @@ export default { this.isReloading = true this.loading = true this.contextResources = [] + this.resourceViewSettings = {} try { await this.loadContext({ id: this.activeContextId }) @@ -223,6 +224,7 @@ export default { }) table.key = (table.id).toString() table.isView = false + this.$set(this.resourceViewSettings, table.key, this.createViewSetting(false, table)) this.contextResources.push(table) } @@ -242,6 +244,7 @@ export default { }) view.key = 'view-' + (view.id).toString() view.isView = true + this.$set(this.resourceViewSettings, view.key, this.createViewSetting(true, view)) this.contextResources.push(view) } } @@ -271,6 +274,24 @@ export default { createColumn(isView, element) { emit('tables:column:create', { isView, element }) }, + createViewSetting(isView, element) { + const viewSetting = { + layout: isView ? (element?.layout ?? 'table') : 'table', + } + + if (isView) { + viewSetting.viewSettings = { + cardBackgroundSource: element?.viewSettings?.cardBackgroundSource ?? null, + cardTitleSource: element?.viewSettings?.cardTitleSource ?? null, + } + } + + if (element?.sort?.length) { + viewSetting.presetSorting = [...element.sort] + } + + return viewSetting + }, async downloadCSV(element, isView) { const access = await this.validateExportAccess({ id: element.id, diff --git a/src/shared/components/ncTable/sections/CustomTable.vue b/src/shared/components/ncTable/sections/CustomTable.vue index a2c50773c6..533990ceec 100644 --- a/src/shared/components/ncTable/sections/CustomTable.vue +++ b/src/shared/components/ncTable/sections/CustomTable.vue @@ -64,9 +64,7 @@ loading="lazy" decoding="async" class="layout-card__image"> -
- {{ t('tables', 'No image') }} -
+
{{ getCardTitle(row) }}
diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 8dd0eb54f3..31b22308a1 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -286,4 +286,90 @@ public function rollBack(): void { $newTable->getSort() ); } + + public function testImportRemapsViewCardSources(): void { + $user = $this->createMock(IUser::class); + $importSource = $this->createMock(IImportSource::class); + $output = new NullOutput(); + + $user->method('getUID')->willReturn('user1'); + $importSource->method('getMigratorVersion')->willReturn(1); + + $tableData = [ + 'id' => 1, + 'title' => 'Test', + ]; + $columns = [ + ['id' => 10, 'tableId' => 1], + ['id' => 11, 'tableId' => 1], + ]; + $views = [ + [ + 'tableId' => 1, + 'title' => 'Nested settings', + 'columnSettings' => [['columnId' => 10, 'order' => 0]], + 'viewSettings' => [ + 'cardBackgroundSource' => 10, + 'cardTitleSource' => 11, + ], + ], + [ + 'tableId' => 1, + 'title' => 'Legacy settings', + 'columnSettings' => [['columnId' => 11, 'order' => 0]], + 'cardBackgroundSource' => 11, + 'cardTitleSource' => 10, + ], + ]; + + $importSource->method('getFileContents')->willReturnCallback(static function (string $file) use ($tableData, $columns, $views): string { + return match ($file) { + 'tables.json' => json_encode([$tableData]), + 'columns.json' => json_encode($columns), + 'views.json' => json_encode($views), + default => json_encode([]), + }; + }); + + $newTable = new Table(); + $newTable->setId(100); + $this->tableService->method('importTable')->willReturn($newTable); + $this->columnService->method('importColumn')->willReturnOnConsecutiveCalls(20, 21); + $this->rowService->method('importRow')->willReturn(1); + + $this->tableMapper->method('getDBConnection')->willReturn(new class { + public function beginTransaction(): void { + } + public function commit(): void { + } + public function rollBack(): void { + } + }); + + $importedViews = []; + $this->viewService->expects($this->exactly(2)) + ->method('importView') + ->with( + $this->identicalTo(100), + $this->callback(static function (array $view) use (&$importedViews): bool { + $importedViews[] = $view; + return true; + }), + $this->identicalTo('user1'), + ); + + $this->migrator->import($user, $importSource, $output); + + $this->assertSame( + [ + 'cardBackgroundSource' => 20, + 'cardTitleSource' => 21, + ], + $importedViews[0]['viewSettings'] + ); + $this->assertSame(20, $importedViews[0]['columnSettings'][0]['columnId']); + $this->assertSame(21, $importedViews[1]['cardBackgroundSource']); + $this->assertSame(20, $importedViews[1]['cardTitleSource']); + $this->assertSame(21, $importedViews[1]['columnSettings'][0]['columnId']); + } } From 382abb9cfc6cf0bbd790eb9d875c5df8d47fb69d Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Sat, 29 Aug 2026 23:46:42 +0100 Subject: [PATCH 03/43] style(views): apply lint and rector fixes Signed-off-by: Enjeck C. --- lib/Controller/ViewController.php | 4 +--- src/modules/modals/ViewSettings.vue | 16 ++++++++-------- src/pages/Context.vue | 12 ++++++------ tests/unit/TablesMigratorTest.php | 6 ++---- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/Controller/ViewController.php b/lib/Controller/ViewController.php index b4f7f55ed9..000f8377fe 100644 --- a/lib/Controller/ViewController.php +++ b/lib/Controller/ViewController.php @@ -57,9 +57,7 @@ public function show(int $id): DataResponse { #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] public function create(int $tableId, string $title, ?string $emoji, ?string $layout = null): DataResponse { - return $this->handleError(function () use ($tableId, $title, $emoji, $layout) { - return $this->service->create($title, $emoji, $this->getTable($tableId, true), null, layout: $layout); - }); + return $this->handleError(fn() => $this->service->create($title, $emoji, $this->getTable($tableId, true), null, layout: $layout)); } #[NoAdminRequired] diff --git a/src/modules/modals/ViewSettings.vue b/src/modules/modals/ViewSettings.vue index 81f9765c53..26bbddc5d3 100644 --- a/src/modules/modals/ViewSettings.vue +++ b/src/modules/modals/ViewSettings.vue @@ -268,7 +268,7 @@ export default { }, set(value) { this.ensureMutableViewSettings() - this.$set(this.mutableView.viewSettings, 'cardBackgroundSource', value ?? null) + this.mutableView.viewSettings.cardBackgroundSource = value ?? null }, }, titleSourceValue: { @@ -277,7 +277,7 @@ export default { }, set(value) { this.ensureMutableViewSettings() - this.$set(this.mutableView.viewSettings, 'cardTitleSource', value ?? null) + this.mutableView.viewSettings.cardTitleSource = value ?? null }, }, saveText() { @@ -497,8 +497,8 @@ export default { const secondColumnId = this.mutableView.columnSettings?.[1]?.columnId ?? firstColumnId const backgroundSource = this.mutableView.viewSettings.cardBackgroundSource ?? this.viewSetting?.viewSettings?.cardBackgroundSource ?? firstColumnId const titleSource = this.mutableView.viewSettings.cardTitleSource ?? this.viewSetting?.viewSettings?.cardTitleSource ?? secondColumnId - this.$set(this.mutableView.viewSettings, 'cardBackgroundSource', backgroundSource) - this.$set(this.mutableView.viewSettings, 'cardTitleSource', titleSource) + this.mutableView.viewSettings.cardBackgroundSource = backgroundSource + this.mutableView.viewSettings.cardTitleSource = titleSource this.errorTitle = false this.selectedColumns = this.mutableView.columnSettings ? this.mutableView.columnSettings.map(item => item.columnId) : null this.allColumns = [] @@ -507,10 +507,10 @@ export default { }, ensureMutableViewSettings() { if (!this.mutableView.viewSettings) { - this.$set(this.mutableView, 'viewSettings', { + this.mutableView.viewSettings = { cardBackgroundSource: null, cardTitleSource: null, - }) + } } }, resolveCardSourceValue(stored, fallbackIndex) { @@ -523,8 +523,8 @@ export default { }, persistCardSourceFallbacks() { this.ensureMutableViewSettings() - this.$set(this.mutableView.viewSettings, 'cardBackgroundSource', this.resolveCardSourceValue(this.mutableView.viewSettings.cardBackgroundSource, 0)) - this.$set(this.mutableView.viewSettings, 'cardTitleSource', this.resolveCardSourceValue(this.mutableView.viewSettings.cardTitleSource, 1)) + this.mutableView.viewSettings.cardBackgroundSource = this.resolveCardSourceValue(this.mutableView.viewSettings.cardBackgroundSource, 0) + this.mutableView.viewSettings.cardTitleSource = this.resolveCardSourceValue(this.mutableView.viewSettings.cardTitleSource, 1) }, loadEmoji() { const emojis = ['😀', '😃', '😄', '😁', '😆', '😅', '🤣', '😂', '🙂', '🙃', '🫠', '😉', '😊', '😇'] diff --git a/src/pages/Context.vue b/src/pages/Context.vue index c1236e73ff..1d567c5443 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -22,15 +22,15 @@
-
-
@@ -224,7 +224,7 @@ export default { }) table.key = (table.id).toString() table.isView = false - this.$set(this.resourceViewSettings, table.key, this.createViewSetting(false, table)) + this.resourceViewSettings[table.key] = this.createViewSetting(false, table) this.contextResources.push(table) } @@ -244,7 +244,7 @@ export default { }) view.key = 'view-' + (view.id).toString() view.isView = true - this.$set(this.resourceViewSettings, view.key, this.createViewSetting(true, view)) + this.resourceViewSettings[view.key] = this.createViewSetting(true, view) this.contextResources.push(view) } } diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 31b22308a1..4303f3e425 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -322,14 +322,12 @@ public function testImportRemapsViewCardSources(): void { ], ]; - $importSource->method('getFileContents')->willReturnCallback(static function (string $file) use ($tableData, $columns, $views): string { - return match ($file) { + $importSource->method('getFileContents')->willReturnCallback(static fn(string $file): string => match ($file) { 'tables.json' => json_encode([$tableData]), 'columns.json' => json_encode($columns), 'views.json' => json_encode($views), default => json_encode([]), - }; - }); + }); $newTable = new Table(); $newTable->setId(100); From f1408077b41456a8cf24c1f298bde8f67e0cb1b0 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 1 Sep 2026 15:09:53 +0200 Subject: [PATCH 04/43] style(views): apply the php-cs-fixer arrow function spacing Use fn () instead of fn() in ViewController and TablesMigratorTest, as php-cs-fixer requires. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- lib/Controller/ViewController.php | 2 +- tests/unit/TablesMigratorTest.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Controller/ViewController.php b/lib/Controller/ViewController.php index 000f8377fe..d9b647cac1 100644 --- a/lib/Controller/ViewController.php +++ b/lib/Controller/ViewController.php @@ -57,7 +57,7 @@ public function show(int $id): DataResponse { #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] public function create(int $tableId, string $title, ?string $emoji, ?string $layout = null): DataResponse { - return $this->handleError(fn() => $this->service->create($title, $emoji, $this->getTable($tableId, true), null, layout: $layout)); + return $this->handleError(fn () => $this->service->create($title, $emoji, $this->getTable($tableId, true), null, layout: $layout)); } #[NoAdminRequired] diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 4303f3e425..d4b3763b5c 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -322,12 +322,12 @@ public function testImportRemapsViewCardSources(): void { ], ]; - $importSource->method('getFileContents')->willReturnCallback(static fn(string $file): string => match ($file) { - 'tables.json' => json_encode([$tableData]), - 'columns.json' => json_encode($columns), - 'views.json' => json_encode($views), - default => json_encode([]), - }); + $importSource->method('getFileContents')->willReturnCallback(static fn (string $file): string => match ($file) { + 'tables.json' => json_encode([$tableData]), + 'columns.json' => json_encode($columns), + 'views.json' => json_encode($views), + default => json_encode([]), + }); $newTable = new Table(); $newTable->setId(100); From e5c54c2845c154d0954ef8e93467f84ef83c2b36 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 1 Sep 2026 17:33:59 +0200 Subject: [PATCH 05/43] fix(views): reject invalid layout values when creating a view View creation silently coerced any unsupported layout to the table layout and still reported success, while updates reject the same values in ViewUpdateInput::normalizeLayout(). Reuse that normalizer on create so a typo returns a bad request instead of persisting a different layout than the caller asked for. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- lib/Model/ViewUpdateInput.php | 2 +- lib/Service/ViewService.php | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/Model/ViewUpdateInput.php b/lib/Model/ViewUpdateInput.php index 09552c109a..11ac9bbc60 100644 --- a/lib/Model/ViewUpdateInput.php +++ b/lib/Model/ViewUpdateInput.php @@ -147,7 +147,7 @@ private static function createViewSettingsFromInputData(array $data): ?ViewSetti ]); } - private static function normalizeLayout(mixed $layout): ?string { + public static function normalizeLayout(mixed $layout): ?string { if ($layout === null || $layout === '') { return null; } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index b1d10c0354..caf5fada92 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -204,7 +204,11 @@ public function create( $item->setEmoji($emoji); } $item->setDescription(''); - $item->setLayout(in_array($layout, ['tiles', 'gallery'], true) ? $layout : null); + try { + $item->setLayout(ViewUpdateInput::normalizeLayout($layout)); + } catch (InvalidArgumentException $e) { + throw new BadRequestError($e->getMessage()); + } $item->setTableId($table->getId()); $item->setCreatedBy($userId); $item->setLastEditBy($userId); From ff56baf05cae76d45f5c3971961a6d6acb556ffb Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 1 Sep 2026 17:34:05 +0200 Subject: [PATCH 06/43] fix(views): remap card sources when importing a scheme Importing a table or context scheme remapped columnSettings, sort and filter through the uuid map but persisted the exported card source column ids unchanged, so tiles/gallery views could reference a nonexistent column or, after an id collision, show data from an unrelated column of the target table. Resolve both card sources through the columnId/columnUuid pairs the exported column settings already carry, and drop a source that cannot be resolved rather than keeping a stale id. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- lib/Model/ViewSettings.php | 30 ++++++++++++++++++++++++++---- lib/Model/ViewUpdateInput.php | 6 +++--- lib/Service/ContextService.php | 2 ++ lib/Service/StructureService.php | 17 +++++++++++++++++ lib/Service/TableService.php | 2 ++ 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/lib/Model/ViewSettings.php b/lib/Model/ViewSettings.php index d555eacb0f..ac472e018f 100644 --- a/lib/Model/ViewSettings.php +++ b/lib/Model/ViewSettings.php @@ -11,6 +11,7 @@ use InvalidArgumentException; use JsonSerializable; +use OCA\Tables\Db\Column; class ViewSettings implements JsonSerializable { public const SOURCE_KEYS = ['cardBackgroundSource', 'cardTitleSource']; @@ -22,15 +23,36 @@ public function __construct( } /** - * @param array{cardBackgroundSource?: int|null, cardTitleSource?: int|null} $data + * @param array{cardBackgroundSource?: int|null, cardTitleSource?: int|null, cardBackgroundSourceUuid?: string, cardTitleSourceUuid?: string} $data + * @param array $columnsMap */ - public static function createFromInputArray(array $data): self { + public static function createFromInputArray(array $data, array $columnsMap = []): self { return new self( - cardBackgroundSource: self::nullableIntFromArray($data, 'cardBackgroundSource'), - cardTitleSource: self::nullableIntFromArray($data, 'cardTitleSource'), + cardBackgroundSource: self::resolveSource($data, 'cardBackgroundSource', $columnsMap), + cardTitleSource: self::resolveSource($data, 'cardTitleSource', $columnsMap), ); } + /** + * A scheme carries the column uuid, because the ids of the table it was exported from mean + * nothing here. A source that no longer resolves is dropped rather than kept, as a stale id + * would otherwise point at an unrelated column. + * + * @param array $columnsMap + */ + private static function resolveSource(array $data, string $key, array $columnsMap): ?int { + $uuidKey = $key . 'Uuid'; + if (array_key_exists($uuidKey, $data)) { + $uuid = $data[$uuidKey]; + if (!is_string($uuid) || !isset($columnsMap[$uuid])) { + return null; + } + return $columnsMap[$uuid]->getId(); + } + + return self::nullableIntFromArray($data, $key); + } + public function getCardBackgroundSource(): ?int { return $this->cardBackgroundSource; } diff --git a/lib/Model/ViewUpdateInput.php b/lib/Model/ViewUpdateInput.php index 11ac9bbc60..35dd02b133 100644 --- a/lib/Model/ViewUpdateInput.php +++ b/lib/Model/ViewUpdateInput.php @@ -102,7 +102,7 @@ public static function fromInputArray(array $data, array $columnsMap = []): self } $layout = self::normalizeLayout($data['layout'] ?? null); - $viewSettings = self::createViewSettingsFromInputData($data); + $viewSettings = self::createViewSettingsFromInputData($data, $columnsMap); return new self( title: ($data['title'] ?? null) ? new Title($data['title']) : null, @@ -118,7 +118,7 @@ public static function fromInputArray(array $data, array $columnsMap = []): self ); } - private static function createViewSettingsFromInputData(array $data): ?ViewSettings { + private static function createViewSettingsFromInputData(array $data, array $columnsMap = []): ?ViewSettings { if (array_key_exists('viewSettings', $data)) { if ($data['viewSettings'] === null) { return new ViewSettings(); @@ -126,7 +126,7 @@ private static function createViewSettingsFromInputData(array $data): ?ViewSetti if (!is_array($data['viewSettings'])) { throw new \InvalidArgumentException('Invalid viewSettings value.'); } - return ViewSettings::createFromInputArray($data['viewSettings']); + return ViewSettings::createFromInputArray($data['viewSettings'], $columnsMap); } $legacyKeys = ['cardBackgroundSource', 'cardTitleSource']; diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index edfd7d9a6e..468d3de83e 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -29,6 +29,7 @@ use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\SortRuleSet; +use OCA\Tables\Model\ViewSettings; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; @@ -822,6 +823,7 @@ public function importScheme(int $contextId, string $name, string $iconName, str $columnsMap[$column->getUuid()] = $column; } foreach ($tableScheme['views'] as $viewData) { + $viewData['viewSettings'] = ViewSettings::createFromInputArray($viewData['viewSettings'] ?? [], $columnsMap)->jsonSerialize(); $viewData['columnSettings'] = ColumnSettings::createViewSettingsFromInputArray($viewData['columnSettings'], $columnsMap)->jsonSerialize(); $viewData['sort'] = SortRuleSet::createFromInputArray($viewData['sort'], $columnsMap)->jsonSerialize(); $viewData['filter'] = FilterSet::createFromInputArray($viewData['filter'], $columnsMap)->jsonSerialize(); diff --git a/lib/Service/StructureService.php b/lib/Service/StructureService.php index 3a3d1e9962..e6b33aae2e 100644 --- a/lib/Service/StructureService.php +++ b/lib/Service/StructureService.php @@ -11,6 +11,7 @@ use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Service\ValueObject\ViewColumnInformation; class StructureService { @@ -213,6 +214,22 @@ protected function prepareViewsData(array $views, array $columnsMap): array { unset($views[$i]['columnSettings'][$j]['columnId']); } } + if (isset($view['viewSettings']) && is_array($view['viewSettings'])) { + foreach (ViewSettings::SOURCE_KEYS as $sourceKey) { + $sourceId = $view['viewSettings'][$sourceKey] ?? null; + if ($sourceId === null) { + continue; + } + + // The scheme travels to another instance, where a column id of this one addresses + // something else entirely. A source that has no uuid to carry it is therefore + // dropped rather than exported as a bare number. + unset($views[$i]['viewSettings'][$sourceKey]); + if (isset($columnsMap[$sourceId])) { + $views[$i]['viewSettings'][$sourceKey . 'Uuid'] = $columnsMap[$sourceId]['uuid']; + } + } + } foreach ($view['sort'] as $j => $col) { if (isset($columnsMap[$col['columnId']])) { $views[$i]['sort'][$j]['columnUuid'] = $columnsMap[$col['columnId']]['uuid']; diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index 5d5b704cbd..adf59034df 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -28,6 +28,7 @@ use OCA\Tables\Model\Permissions; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\TableScheme; +use OCA\Tables\Model\ViewSettings; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ValueObject\Title; @@ -794,6 +795,7 @@ public function updateTableStructure(int $tableId, array $columns, array $views, // Add views foreach ($views['addViews'] as $view) { + $view['viewSettings'] = ViewSettings::createFromInputArray($view['viewSettings'] ?? [], $columnsMap)->jsonSerialize(); $view['columnSettings'] = ColumnSettings::createViewSettingsFromInputArray($view['columnSettings'], $columnsMap)->jsonSerialize(); $view['sort'] = SortRuleSet::createFromInputArray($view['sort'], $columnsMap)->jsonSerialize(); $view['filter'] = FilterSet::createFromInputArray($view['filter'], $columnsMap)->jsonSerialize(); From 2179dc1eb287d9e024ac93625a6edc00209b2dcf Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 1 Sep 2026 17:34:06 +0200 Subject: [PATCH 07/43] fix(views): render the saved card layout on public shares PublicElement bound a localViewSetting that was never declared and passed no view setting to TableView, so a view saved as tiles or gallery always fell back to the table layout when opened through a public link. Initialise the setting from the shared view and pass it down to the renderer. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- src/modules/main/sections/PublicElement.vue | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/modules/main/sections/PublicElement.vue b/src/modules/main/sections/PublicElement.vue index 026f14b1d8..56fc910e61 100644 --- a/src/modules/main/sections/PublicElement.vue +++ b/src/modules/main/sections/PublicElement.vue @@ -8,7 +8,7 @@
- { this.showCreateRow = true @@ -149,6 +156,16 @@ export default { methods: { t, + + buildViewSetting() { + return { + layout: this.element?.layout ?? 'table', + viewSettings: { + cardBackgroundSource: this.element?.viewSettings?.cardBackgroundSource ?? null, + cardTitleSource: this.element?.viewSettings?.cardTitleSource ?? null, + }, + } + }, }, } From a4fe0c3262eb11820b2e7183a7d2207dc1e4d157 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 1 Sep 2026 17:34:06 +0200 Subject: [PATCH 08/43] fix(views): keep the actions menu available in card layouts The named actions slot was only rendered inside the table header, which is skipped for tiles and gallery, so opening a card layout hid the whole action menu including the view settings entry used to switch the layout back. Render the slot above the card grid when a card layout is active. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- src/shared/components/ncTable/sections/CustomTable.vue | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/shared/components/ncTable/sections/CustomTable.vue b/src/shared/components/ncTable/sections/CustomTable.vue index 533990ceec..1eae4dc582 100644 --- a/src/shared/components/ncTable/sections/CustomTable.vue +++ b/src/shared/components/ncTable/sections/CustomTable.vue @@ -4,6 +4,9 @@ --> diff --git a/src/modules/main/sections/View.vue b/src/modules/main/sections/View.vue index d0e7864a67..7d3f5c4ca5 100644 --- a/src/modules/main/sections/View.vue +++ b/src/modules/main/sections/View.vue @@ -102,6 +102,8 @@ import ListBoxOutline from 'vue-material-design-icons/ListBoxOutline.vue' import ElementTitle from './ElementTitle.vue' import TableDescription from './TableDescription.vue' +import { hasLocalViewAdjustments } from '../../../shared/utils/viewSetting.js' + export default { components: { TableDescription, @@ -158,7 +160,7 @@ export default { }, computed: { isViewSettingSet() { - return !(!this.localViewSetting || ((!this.localViewSetting.hiddenColumns || this.localViewSetting.hiddenColumns.length === 0) && (!this.localViewSetting.sorting) && (!this.localViewSetting.filter || this.localViewSetting.filter.length === 0))) + return hasLocalViewAdjustments(this.localViewSetting) }, }, watch: { diff --git a/src/pages/Context.vue b/src/pages/Context.vue index 1d567c5443..7fe0cf2da2 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -224,7 +224,7 @@ export default { }) table.key = (table.id).toString() table.isView = false - this.resourceViewSettings[table.key] = this.createViewSetting(false, table) + this.resourceViewSettings[table.key] = this.createViewSetting(table) this.contextResources.push(table) } @@ -244,7 +244,7 @@ export default { }) view.key = 'view-' + (view.id).toString() view.isView = true - this.resourceViewSettings[view.key] = this.createViewSetting(true, view) + this.resourceViewSettings[view.key] = this.createViewSetting(view) this.contextResources.push(view) } } @@ -274,17 +274,8 @@ export default { createColumn(isView, element) { emit('tables:column:create', { isView, element }) }, - createViewSetting(isView, element) { - const viewSetting = { - layout: isView ? (element?.layout ?? 'table') : 'table', - } - - if (isView) { - viewSetting.viewSettings = { - cardBackgroundSource: element?.viewSettings?.cardBackgroundSource ?? null, - cardTitleSource: element?.viewSettings?.cardTitleSource ?? null, - } - } + createViewSetting(element) { + const viewSetting = {} if (element?.sort?.length) { viewSetting.presetSorting = [...element.sort] diff --git a/src/shared/components/ncTable/NcTable.vue b/src/shared/components/ncTable/NcTable.vue index f0c7ef84b1..dd03af898f 100644 --- a/src/shared/components/ncTable/NcTable.vue +++ b/src/shared/components/ncTable/NcTable.vue @@ -44,6 +44,8 @@ deselect-all-rows -> unselect all rows, e.g. after deleting selected rows
unselect all rows, e.g. after deleting selected rows
column.id === preferredColumnId) ?? this.columns[1] ?? this.columns[0] ?? null @@ -491,7 +502,7 @@ export default { getBackgroundColumn() { // No fallback: a card without a configured background shows no image, which is what // hasCardBackground reports to the styling. - const preferredColumnId = this.localViewSetting?.viewSettings?.cardBackgroundSource + const preferredColumnId = this.viewSettings?.cardBackgroundSource return this.columns.find(column => column.id === preferredColumnId) ?? null }, // Stops at what a card shows rather than reading every column of every row first: diff --git a/src/shared/utils/viewSetting.js b/src/shared/utils/viewSetting.js new file mode 100644 index 0000000000..8ccacd8784 --- /dev/null +++ b/src/shared/utils/viewSetting.js @@ -0,0 +1,25 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * Whether the viewer has adjusted the view for themselves. + * + * These are the unsaved tweaks that "Reset local adjustments" clears, the layout a + * viewer switched to included. Several components show a cue for this state, so they + * all have to agree on what counts. + * + * @param {object|null} viewSetting the local view setting of a table or view + * @return {boolean} true when anything is set + */ +export function hasLocalViewAdjustments(viewSetting) { + if (!viewSetting) { + return false + } + + return viewSetting.hiddenColumns?.length > 0 + || !!viewSetting.sorting + || viewSetting.filter?.length > 0 + || !!viewSetting.layout +} diff --git a/src/views/ContentReferenceWidget.vue b/src/views/ContentReferenceWidget.vue index a5205f1182..7216e81d26 100644 --- a/src/views/ContentReferenceWidget.vue +++ b/src/views/ContentReferenceWidget.vue @@ -21,6 +21,8 @@
Date: Wed, 9 Sep 2026 11:00:57 +0200 Subject: [PATCH 36/43] feat(views): let viewers switch layout without changing the view Only a manager can open the view settings, so a recipient of a shared view had no way to read it as anything but the layout it was saved with. The toolbar gains a menu button at its trailing edge listing the three layouts, each with its own icon. Choosing one writes the local view setting, so it applies at once, is never persisted, and is cleared again by "Reset local adjustments" or by leaving the view. The trigger carries the icon of the current layout and is labelled "Switch layout", since an icon-only button would otherwise have no accessible name. It appears only where the view was set up for cards, meaning a card title source is configured or the saved layout is already tiles or gallery: cards of a view without a title source would carry nothing but row numbers. That is a presentation choice, not a constraint, so nothing on the write path enforces it. An embedded widget declines the switch outright, being a snapshot of a view inside someone else's document, and the smart picker preview inherits that; public links and contexts keep it. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- cypress/component/LayoutToggle.cy.js | 118 ++++++++++++++++++ src/modules/main/sections/MainWrapper.vue | 11 +- src/pages/View.vue | 6 +- src/shared/components/ncTable/NcTable.vue | 5 + .../ncTable/sections/CustomTable.vue | 9 +- .../components/ncTable/sections/Options.vue | 92 ++++++++++++++ src/shared/utils/viewSetting.js | 19 +++ src/views/ContentReferenceWidget.vue | 3 + 8 files changed, 253 insertions(+), 10 deletions(-) create mode 100644 cypress/component/LayoutToggle.cy.js diff --git a/cypress/component/LayoutToggle.cy.js b/cypress/component/LayoutToggle.cy.js new file mode 100644 index 0000000000..d68da79149 --- /dev/null +++ b/cypress/component/LayoutToggle.cy.js @@ -0,0 +1,118 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import NcTable from '../../src/shared/components/ncTable/NcTable.vue' + +const TOGGLE = '[data-cy="layoutToggle"]' +const TILES_OPTION = '[data-cy="layoutToggleTiles"]' +const GALLERY_OPTION = '[data-cy="layoutToggleGallery"]' +const CARD = '[data-cy="tilesLayoutCard"]' +const ROW = 'tr[data-cy="customTableRow"]' + +describe('Layout toggle', () => { + let richObject = {} + + before(() => { + cy.fixture('widgets/richObject.json') + .then(richObjectFixture => { + richObject = richObjectFixture + }) + }) + + it('stays hidden on a view that is not set up for cards', () => { + mountTable(richObject, { layout: 'table', viewSettings: null }) + + cy.get(TOGGLE).should('not.exist') + }) + + it('shows once a card title source is configured, with a descriptive label', () => { + mountTable(richObject, { layout: 'table', viewSettings: { cardBackgroundSource: null, cardTitleSource: richObject.columns[0].id } }) + + cy.get(TOGGLE).should('exist') + cy.get(`${TOGGLE} button`).first().should('have.attr', 'aria-label', 'Switch layout') + // The hover text sits on the wrapper, which is what NcActions passes it to. + cy.get(TOGGLE).should('have.attr', 'title', 'Switch layout') + }) + + it('shows on a view already saved with a card layout', () => { + mountTable(richObject, { layout: 'gallery', viewSettings: null }) + + cy.get(TOGGLE).should('exist') + }) + + it('leaves no adjustment when the saved layout is picked again', () => { + mountTable(richObject, { layout: 'gallery', viewSettings: null }) + .then(({ wrapper }) => { + cy.get(TOGGLE).find('button').first().click() + cy.get(GALLERY_OPTION).click() + .then(() => { + const emitted = wrapper.emitted('update:viewSetting') + // Nothing to reset: the choice matches what the view already is. + expect(emitted[emitted.length - 1][0]).to.not.have.property('layout') + }) + }) + }) + + it('stays hidden where the embedder declines it', () => { + mountTable(richObject, { layout: 'gallery', viewSettings: null }, { canSwitchLayout: false }) + + cy.get('[data-cy="galleryLayoutCard"]').should('exist') + cy.get(TOGGLE).should('not.exist') + }) + + it('switches the rendering without touching the saved layout', () => { + mountTable(richObject, { layout: 'table', viewSettings: { cardBackgroundSource: null, cardTitleSource: richObject.columns[0].id } }) + .then(({ wrapper }) => { + cy.get(ROW).should('exist') + cy.get(TOGGLE).find('button').first().click() + cy.get(TILES_OPTION).click() + // The menu closes on selection rather than staying over the cards. + cy.get(TILES_OPTION).should('not.exist') + + cy.get(CARD).should('exist') + cy.get(ROW).should('not.exist') + .then(() => { + // The view keeps the layout it was given; only the local setting moved. + expect(wrapper.props('layout')).to.equal('table') + }) + }) + }) + + it('falls back to the saved layout when the local choice is cleared', () => { + mountTable(richObject, { layout: 'gallery', viewSettings: null }) + .then(({ wrapper }) => { + cy.get('[data-cy="galleryLayoutCard"]').should('exist') + cy.get(TOGGLE).should('exist') + + cy.wrap(null).then(() => wrapper.setProps({ viewSetting: { layout: 'table' } })) + cy.get(ROW).should('exist') + + cy.wrap(null).then(() => wrapper.setProps({ viewSetting: {} })) + cy.get('[data-cy="galleryLayoutCard"]').should('exist') + }) + }) +}) + +/** + * Mounts NcTable as a view with the given saved layout. + * + * @param {object} richObject the widget fixture holding the column and row templates + * @param {object} view the saved layout and card sources of the view + * @param {object} overrides further props to mount with + * @return {Cypress.Chainable} the mount result + */ +function mountTable(richObject, view, overrides = {}) { + return cy.mount(NcTable, { + props: { + rows: richObject.rows, + columns: richObject.columns, + elementId: richObject.id, + isView: true, + layout: view.layout, + viewSettings: view.viewSettings, + viewSetting: {}, + ...overrides, + }, + }) +} diff --git a/src/modules/main/sections/MainWrapper.vue b/src/modules/main/sections/MainWrapper.vue index 225d1cd8be..ba2ccb8f7e 100644 --- a/src/modules/main/sections/MainWrapper.vue +++ b/src/modules/main/sections/MainWrapper.vue @@ -8,10 +8,10 @@
- +
@@ -38,13 +38,14 @@ export default { data() { return { errorMessage: null, + localLayout: null, } }, computed: { ...mapState(useTablesStore, ['activeViewId', 'activeView']), isCardLayout() { - return CARD_LAYOUTS.includes(this.activeView?.layout) + return CARD_LAYOUTS.includes(this.localLayout ?? this.activeView?.layout) }, }, @@ -53,6 +54,7 @@ export default { immediate: true, handler() { this.errorMessage = null + this.localLayout = null this.checkView() }, }, diff --git a/src/shared/components/ncTable/NcTable.vue b/src/shared/components/ncTable/NcTable.vue index dd03af898f..db05bdd3b0 100644 --- a/src/shared/components/ncTable/NcTable.vue +++ b/src/shared/components/ncTable/NcTable.vue @@ -46,6 +46,7 @@ deselect-all-rows -> unselect all rows, e.g. after deleting selected rows
+ + + + + {{ t('tables', 'Table') }} + + + + {{ t('tables', 'Tiles') }} + + + + {{ t('tables', 'Gallery') }} + + +
{{ n('tables', '%n selected row', '%n selected rows', selectedRows.length, {}) }} @@ -73,6 +104,11 @@ import TrayArrowDown from 'vue-material-design-icons/TrayArrowDown.vue' import viewportHelper from '../../../mixins/viewportHelper.js' import SearchForm from '../partials/SearchForm.vue' import PaginationBlock from './PaginationBlock.vue' +import ViewListOutline from 'vue-material-design-icons/ViewListOutline.vue' +import ViewGridOutline from 'vue-material-design-icons/ViewGridOutline.vue' +import ViewGalleryOutline from 'vue-material-design-icons/ViewGalleryOutline.vue' +import { CARD_LAYOUTS, LAYOUT_GALLERY, LAYOUT_TABLE, LAYOUT_TILES } from '../../../constants.ts' +import { resolveLayout } from '../../../utils/viewSetting.js' import { translate as t, translatePlural as n } from '@nextcloud/l10n' export default { @@ -88,6 +124,9 @@ export default { Delete, TrayArrowDown, PaginationBlock, + ViewListOutline, + ViewGridOutline, + ViewGalleryOutline, }, mixins: [viewportHelper], @@ -121,6 +160,18 @@ export default { type: Array, default: null, }, + layout: { + type: String, + default: null, + }, + canSwitchLayout: { + type: Boolean, + default: true, + }, + viewSettings: { + type: Object, + default: null, + }, viewSetting: { type: Object, default: null, @@ -132,6 +183,7 @@ export default { }, emits: [ + 'update:viewSetting', 'create-row', 'delete-selected-rows', 'download-filtered-csv', @@ -144,6 +196,27 @@ export default { }, computed: { + LAYOUT_TABLE: () => LAYOUT_TABLE, + LAYOUT_TILES: () => LAYOUT_TILES, + LAYOUT_GALLERY: () => LAYOUT_GALLERY, + // What the view itself is saved with, ignoring any local switch. + savedLayout() { + return resolveLayout(null, this.layout) + }, + activeLayout() { + return resolveLayout(this.viewSetting, this.layout) + }, + // Switching layout is a reading preference anyone may use, but it is only offered + // where the view was set up for cards: without a title source the cards would carry + // nothing but row numbers. + showLayoutToggle() { + if (!this.canSwitchLayout || !this.isView || !(this.columns?.length > 0)) { + return false + } + + return (this.viewSettings?.cardTitleSource ?? null) !== null + || CARD_LAYOUTS.includes(this.layout) + }, getSelectedRows() { const rows = [] this.selectedRows.forEach(id => { @@ -172,6 +245,21 @@ export default { }, methods: { + // The choice stays in the local view setting, so it is never persisted and is + // dropped again by "Reset local adjustments". + setLayout(layout) { + const viewSetting = { ...(this.viewSetting ?? {}) } + + // Picking the layout the view is already saved with is not an adjustment, so it + // leaves nothing behind for "Reset local adjustments" to offer. + if (layout === this.savedLayout) { + delete viewSetting.layout + } else { + viewSetting.layout = layout + } + + this.$emit('update:viewSetting', viewSetting) + }, t, n, updateOptionsDivWidth() { @@ -278,4 +366,8 @@ export default { } } +.layout-toggle { + margin-inline-start: auto; + align-self: center; +} diff --git a/src/shared/utils/viewSetting.js b/src/shared/utils/viewSetting.js index 8ccacd8784..ad38e69a2c 100644 --- a/src/shared/utils/viewSetting.js +++ b/src/shared/utils/viewSetting.js @@ -3,6 +3,25 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +import { CARD_LAYOUTS, LAYOUT_TABLE } from '../constants.ts' + +/** + * The layout to render with. + * + * A viewer may switch layout for themselves; that choice lives in the local view + * setting and is never saved, so it falls back to the layout of the view itself. + * A value this version does not know renders as a table rather than as nothing. + * + * @param {object|null} viewSetting the local view setting of a table or view + * @param {string|null} savedLayout the layout the view is stored with + * @return {string} one of the known layouts + */ +export function resolveLayout(viewSetting, savedLayout) { + const layout = viewSetting?.layout ?? savedLayout + + return CARD_LAYOUTS.includes(layout) ? layout : LAYOUT_TABLE +} + /** * Whether the viewer has adjusted the view for themselves. * diff --git a/src/views/ContentReferenceWidget.vue b/src/views/ContentReferenceWidget.vue index 7216e81d26..c37494b2ce 100644 --- a/src/views/ContentReferenceWidget.vue +++ b/src/views/ContentReferenceWidget.vue @@ -12,6 +12,7 @@
+ Date: Wed, 9 Sep 2026 11:51:18 +0200 Subject: [PATCH 37/43] fix(tables): stop nesting interactive content inside the card button A card was a
    @@ -92,7 +104,7 @@
- +
@@ -228,6 +240,8 @@ export default { ? this.columns.filter(column => column.id !== titleColumn?.id && column.id !== backgroundColumn?.id) : [] + const scope = `${this.isView ? 'view' : 'table'}-${this.elementId}` + return this.currentPageRows.map(row => { // Indexed once per row: the helpers below would otherwise scan row.data again for // every column, which is a full pass per cell on a wide table. @@ -235,18 +249,20 @@ export default { return { id: row.id, + // Several tables can share a page, so the id the control points at must be unique. + titleId: `tables-card-title-${scope}-${row.id}`, previewUrl: this.getPreviewUrl(cells, backgroundColumn), title: this.getDisplayValue(titleColumn, cells) || `${t('tables', 'Row')} ${row.id}`, metadata: this.getGalleryMetadata(metadataColumns, cells), } }) }, - isGalleryLayout() { - return this.currentLayout === LAYOUT_GALLERY - }, hasRichTitle() { return this.isRichColumn(this.getTitleColumn()) }, + isGalleryLayout() { + return this.currentLayout === LAYOUT_GALLERY + }, currentPageRows() { return this.rows.slice((this.pageNumber - 1) * this.rowsPerPage, ((this.pageNumber - 1) * this.rowsPerPage) + this.rowsPerPage) }, @@ -416,6 +432,15 @@ export default { disableRowAnimation() { this.rowAnimation = false }, + // Rich text may put a link in a card. A click on one follows the link and must not + // also open the row, so only clicks outside a link activate the card. + activateCard(rowId, event) { + if (event.target?.closest?.('a')) { + return + } + + this.$emit('edit-row', rowId) + }, getPreviewUrl(cells, backgroundColumn) { if (!this.canRenderPreviews) { return null @@ -586,6 +611,11 @@ export default { grid-template-columns: repeat(auto-fit, minmax(min(100%, 220px), 1fr)); } +.layout-card__open:focus-visible { + outline: 2px solid var(--color-main-text); + outline-offset: -2px; +} + .layout-card { width: 100%; max-width: 100%; @@ -627,6 +657,8 @@ export default { .layout-card__title-banner { position: absolute; + /* Above the overlay so a link in a rich title stays clickable. */ + z-index: 1; inset-inline: 0; bottom: 0; padding: 12px; @@ -652,6 +684,16 @@ export default { bottom: auto; } +.layout-card__open { + position: absolute; + inset: 0; + appearance: none; + background: none; + border: none; + padding: 0; + cursor: pointer; +} + .layout-card__title-text { display: -webkit-box; -webkit-line-clamp: var(--card-title-lines, 2); From 9b187ef3c649651e825813b3a9d766e3055c5a26 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 9 Sep 2026 12:13:18 +0200 Subject: [PATCH 38/43] docs(agents): carve out presentational references from soft-invalidation Card sources drop to null when they no longer resolve, which contradicted the soft-invalidation rule as written. The rule now says why a reference that only selects how a row is displayed is treated differently from one that decides which rows or values are returned: an id that fails to remap on import may address a live column of an unrelated table, and a view without card sources falls back to the text-only card layout, so there is no broken state for a user to repair. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- AGENTS.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index f1086ac034..bf465e2119 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -254,6 +254,15 @@ On import/export, selection option IDs change. `ColumnService::importColumn()` m When a stored rule, filter, or condition references a column or option that no longer exists (due to deletion, type change, or import remapping), mark it `broken: true` instead of deleting it. Surface the broken state in the UI. Provide an auto-clear path that removes the flag when the rule is next saved in a valid state. +**Exception — presentational references.** A reference that only selects how a row is *displayed*, rather than which rows or values are returned, is dropped to `null` instead. Card sources (`viewSettings.cardBackgroundSource`, `viewSettings.cardTitleSource`) are the current case, handled in `ViewSettings::resolveSource()`, `ApiTablesController::remapViewSettings()` and `TablesMigrator::remapViewCardSource()`. + +Two reasons this differs from a filter or sort rule: + +- Column ids are only unique per instance, so an id that fails to remap on import is not merely stale — after import it may address a live column of an unrelated table. Keeping it would point the card at someone else's data, which is worse than losing the setting. +- Losing it degrades gracefully. A view with no card sources renders the text-only card layout, which is a supported state the UI already handles, so there is no broken condition for the user to repair. + +A reference that changes which rows or values are returned still follows the rule above. + ### XSS / CSS injection - Never use `v-html`, `innerHTML`, `eval`, or `new Function` with user-supplied values. From 258131122f9fc15e136c59c02eb738de92a0d381 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 9 Sep 2026 13:04:42 +0200 Subject: [PATCH 39/43] feat(federation): ship the view layout to a federated receiver notifyNodeUpdate() and the share payload carried only title and emoji, so a federated gallery view rendered as a table on the receiving instance with nothing to say the layout had been lost. The layout travels with both, and a value the receiving version does not know is ignored rather than stored. The card sources stay behind on purpose: they are column ids of the sending instance and would address unrelated columns after the hop, so a federated card layout renders text-only. Adding them later means carrying column uuids, which the payload can gain without breaking a receiver, since unknown keys are ignored. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- lib/Federation/FederationProvider.php | 18 ++++++++++ lib/Service/FederationService.php | 5 +++ .../Federation/FederationProviderTest.php | 36 ++++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/lib/Federation/FederationProvider.php b/lib/Federation/FederationProvider.php index d9b0155471..94fd4d29c3 100644 --- a/lib/Federation/FederationProvider.php +++ b/lib/Federation/FederationProvider.php @@ -10,6 +10,7 @@ namespace OCA\Tables\Federation; use OCA\Tables\Constants\ShareReceiverType; +use OCA\Tables\Constants\ViewLayout; use OCA\Tables\Db\Share; use OCA\Tables\Db\ShareMapper; use OCA\Tables\Db\Table; @@ -149,6 +150,7 @@ private function buildNodeForFederationShare(ICloudFederationShare $share, array if ($node instanceof View) { $node->setDescription(''); + $node->setLayout(self::layoutFromMetaData($metaData)); } if (property_exists($node, 'uuid')) { @@ -226,6 +228,22 @@ private function handleNodeUpdate(string $providerId, array $notification): void if (isset($notification['emoji'])) { $node->setEmoji($notification['emoji']); } + if ($node instanceof View && isset($notification['layout'])) { + $layout = self::layoutFromMetaData($notification); + if ($layout !== null) { + $node->setLayout($layout); + } + } $this->getMapperForNodeType($nodeType)->update($node); } + + /** + * The layout a remote instance named, or null when this version does not know it. + * + * The value comes from another server, so it is only accepted as one of the layouts + * this version defines. + */ + private static function layoutFromMetaData(array $data): ?string { + return ViewLayout::tryFromMixed($data['layout'] ?? null)?->value; + } } diff --git a/lib/Service/FederationService.php b/lib/Service/FederationService.php index f09d3d0a84..af5a804760 100644 --- a/lib/Service/FederationService.php +++ b/lib/Service/FederationService.php @@ -147,6 +147,10 @@ public function sendShare(Share $share): void { 'emoji' => $node->getEmoji(), 'nodeType' => $share->getNodeType(), 'uuid' => property_exists($node, 'uuid') ? $node->getUuid() : null, + // The card sources stay behind: they are column ids of this instance and would + // address unrelated columns on the receiving one, so the receiver renders the + // text-only card layout. + 'layout' => $node instanceof View ? $node->getLayoutNormalized() : null, ]), (string)$share->getNodeId(), $ownerCloudId->getId(), @@ -212,6 +216,7 @@ public function notifyNodeUpdate(Table|View $node, string $nodeType): void { 'title' => $node->getTitle(), 'emoji' => $node->getEmoji(), 'nodeType' => $nodeType, + 'layout' => $node instanceof View ? $node->getLayoutNormalized() : null, ] ); } diff --git a/tests/unit/Federation/FederationProviderTest.php b/tests/unit/Federation/FederationProviderTest.php index 07336a3d1e..92d23f8253 100644 --- a/tests/unit/Federation/FederationProviderTest.php +++ b/tests/unit/Federation/FederationProviderTest.php @@ -104,7 +104,7 @@ public function testShareReceivedCreatesView(): void { $share->method('getOwner')->willReturn('admin@nextcloud.local'); $share->method('getShareSecret')->willReturn('secret456'); $share->method('getSharedBy')->willReturn('admin@nextcloud.local'); - $share->method('getDescription')->willReturn(json_encode(['emoji' => '🦆', 'nodeType' => 'view'])); + $share->method('getDescription')->willReturn(json_encode(['emoji' => '🦆', 'nodeType' => 'view', 'layout' => 'gallery'])); $this->viewMapper->expects($this->once()) ->method('insert') @@ -113,6 +113,7 @@ public function testShareReceivedCreatesView(): void { $this->assertEquals('Federation View', $view->getTitle()); $this->assertEquals(5, $view->getExternalId()); $this->assertEquals('🦆', $view->getEmoji()); + $this->assertEquals('gallery', $view->getLayout()); return $view; }); @@ -179,6 +180,39 @@ public function testNotificationReceivedNodeUpdate(): void { $this->assertEquals('🦆', $table->getEmoji()); } + public function testNotificationReceivedNodeUpdateAppliesLayout(): void { + $view = new View(); + $view->setId(1); + $view->setExternalId(2); + $view->setShareToken('abcdefghijklmnop'); + $view->setLayout('table'); + + $this->viewMapper->method('findByExternalIdAndToken')->willReturn($view); + $this->viewMapper->expects($this->exactly(2))->method('update')->with($view); + + $this->provider->notificationReceived( + FederationProvider::NOTIFICATION_UPDATE_NODE, + '2', + [ + 'sharedSecret' => 'abcdefghijklmnop', + 'nodeType' => 'view', + 'layout' => 'tiles', + ] + ); + $this->assertEquals('tiles', $view->getLayout()); + + $this->provider->notificationReceived( + FederationProvider::NOTIFICATION_UPDATE_NODE, + '2', + [ + 'sharedSecret' => 'abcdefghijklmnop', + 'nodeType' => 'view', + 'layout' => 'carousel', + ] + ); + $this->assertEquals('tiles', $view->getLayout(), 'an unknown layout is ignored'); + } + public function testNotificationReceiveNodeDelete(): void { $table = new Table(); $table->setId(1); From 0d837c07f3f00f6f8a91089b1cc025f0f9a8b9a8 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sat, 12 Sep 2026 13:22:51 +0200 Subject: [PATCH 40/43] fix(contexts): render an application opened by its own URL Opening an application by its URL, rather than by clicking it in the navigation, left the page blank: setContext() looked the context up with findIndex() and assigned straight to that index, so on the first load -- when the list has not been fetched yet and the lookup returns -1 -- contexts[-1] named a property instead of appending. The getters iterate the array, so activeContext stayed undefined and every branch of the template was false. Reading the icon off it threw first, leaving "An error occurred while loading the application" instead. Append when the context is not in the list yet, as the store convention for findIndex lookups requires, and drop the second copy of the icon assignment. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- src/pages/Context.vue | 2 -- src/store/store.js | 7 +++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pages/Context.vue b/src/pages/Context.vue index 7fe0cf2da2..420961d6e7 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -191,8 +191,6 @@ export default { this.icon = await this.getContextIcon(this.activeContext.iconName) - this.icon = await this.getContextIcon(this.activeContext.iconName) - if (this.context && this.context.pages) { const pages = Object.values(this.context.pages) const startPage = pages.find(p => p.page_type === 'startpage') diff --git a/src/store/store.js b/src/store/store.js index 444eaa5c55..c6d9e213df 100644 --- a/src/store/store.js +++ b/src/store/store.js @@ -125,6 +125,13 @@ export const useTablesStore = defineStore('store', { }, setContext(context) { const index = this.contexts.findIndex(c => c.id === context.id) + if (index === -1) { + // Loading one application by its URL fetches it before the list is there, and + // contexts[-1] would name a property rather than append: the getters iterate, + // so they would never see it and the page would render empty. + this.contexts.push(context) + return + } this.contexts[index] = context }, setActiveRowId(rowId) { From 55d49d21e03dfb7db5fb6380cf005583d2d8aa2f Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 17 Sep 2026 09:43:53 +0200 Subject: [PATCH 41/43] refactor(tables): derive a file preview link in one place The card layout and the link cell each grew their own way of reading a file id out of a link and building a preview request, and the two disagreed. The link cell matched a lowercase fileid only, which is not the spelling the server writes, so a preview URL never resolved through it; it also read an id out of a link to another host and then requested that id from this one. The card layout, in turn, anchored the id to the end of the path and did not know the openfile parameter that a Files deep link carries. Move the matcher and the URL builder to a shared helper that takes what both need: the link is parsed rather than pattern matched, a foreign origin is refused, fileid and openfile are read whatever their case, and the id may be followed by more path. Each caller keeps what is its own -- the link cell only previews a files reference, while a card background may be any column and so probes the fields a stored link may hold -- and both now prefer an id the picker recorded outright. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- cypress/component/CardPreviewUrl.cy.js | 35 ++---- cypress/component/FilePreview.cy.js | 70 +++++++++++ cypress/component/LinkCellPreview.cy.js | 119 ++++++++++++++++++ .../ncTable/partials/TableCellLink.vue | 20 ++- .../ncTable/sections/CustomTable.vue | 42 ++----- src/shared/utils/filePreview.js | 79 ++++++++++++ 6 files changed, 292 insertions(+), 73 deletions(-) create mode 100644 cypress/component/FilePreview.cy.js create mode 100644 cypress/component/LinkCellPreview.cy.js create mode 100644 src/shared/utils/filePreview.js diff --git a/cypress/component/CardPreviewUrl.cy.js b/cypress/component/CardPreviewUrl.cy.js index 9074fc7a87..5c4a0b9ab6 100644 --- a/cypress/component/CardPreviewUrl.cy.js +++ b/cypress/component/CardPreviewUrl.cy.js @@ -14,35 +14,11 @@ describe('Card preview URL', () => { new TextLineColumn({ id: 2, title: 'Image', type: ColumnTypes.TextLine }), ] - it('takes the file id from a same origin link, whatever its spelling', () => { - mountCards('x').then(({ wrapper }) => { - const origin = window.location.origin - const id = value => wrapper.vm.getLocalFileId(value) - - expect(id(`${origin}/index.php/f/123`)).to.equal('123') - expect(id(`${origin}/index.php/f/123/`)).to.equal('123') - expect(id('/index.php/f/456')).to.equal('456', 'a relative link resolves against this origin') - expect(id(`${origin}/index.php/core/preview?fileId=7&x=9999`)).to.equal('7') - expect(id(`${origin}/apps/files/?FILEID=8`)).to.equal('8', 'the parameter name is not case sensitive') - }) - }) - - it('refuses anything that does not name a file on this server', () => { - mountCards('x').then(({ wrapper }) => { - const id = value => wrapper.vm.getLocalFileId(value) - - expect(id('https://evil.example/index.php/f/42')).to.equal(null, 'another host is never followed') - expect(id('https://evil.example/index.php/core/preview?fileId=42')).to.equal(null) - expect(id('not a link')).to.equal(null) - expect(id(`${window.location.origin}/index.php/f/abc`)).to.equal(null, 'the id has to be numeric') - }) - }) - it('rebuilds the request instead of forwarding the stored one', () => { mountCards(`${window.location.origin}/index.php/core/preview?fileId=7&x=9999&evil=1`) cy.get(`${CARD} img`).should('have.attr', 'src').then(src => { - expect(src).to.match(/\/core\/preview\?fileId=7&x=\d+&y=\d+&a=true$/) + expect(src).to.match(/\/core\/preview\?fileId=7&x=\d+&y=\d+&a=1$/) expect(src).not.to.contain('evil') expect(src).not.to.contain('9999') }) @@ -54,6 +30,15 @@ describe('Card preview URL', () => { cy.get(`${CARD} img`).should('have.attr', 'src').and('match', /fileId=55&/) }) + it('prefers the file id a picked file stores over the link it also carries', () => { + mountCards(JSON.stringify({ + resourceUrl: `${window.location.origin}/index.php/f/55`, + attributes: { fileId: 66 }, + })) + + cy.get(`${CARD} img`).should('have.attr', 'src').and('match', /fileId=66&/) + }) + it('renders no image for a value it will not follow', () => { mountCards('https://evil.example/index.php/f/42') diff --git a/cypress/component/FilePreview.cy.js b/cypress/component/FilePreview.cy.js new file mode 100644 index 0000000000..ead4612b70 --- /dev/null +++ b/cypress/component/FilePreview.cy.js @@ -0,0 +1,70 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { buildPreviewUrl, findLocalFileId, isFileId } from '../../src/shared/utils/filePreview.js' + +describe('Local file preview links', () => { + const origin = () => window.location.origin + + it('takes the file id from a same origin link, whatever its spelling', () => { + expect(findLocalFileId(`${origin()}/index.php/f/123`)).to.equal('123') + expect(findLocalFileId(`${origin()}/index.php/f/123/`)).to.equal('123') + expect(findLocalFileId('/index.php/f/456')).to.equal('456', 'a relative link resolves against this origin') + expect(findLocalFileId(`${origin()}/index.php/f/123?dir=/x`)).to.equal('123') + expect(findLocalFileId(`${origin()}/index.php/f/123#anchor`)).to.equal('123') + }) + + // The card layout anchored the id to the end of the path, the link cell did not. + it('reads the id from a path that continues after it', () => { + expect(findLocalFileId(`${origin()}/index.php/f/123/preview`)).to.equal('123') + }) + + // The link cell only matched a lowercase fileid, which is not the spelling the server + // itself writes, so a preview URL never resolved through it. + it('matches the file id parameter whatever its case', () => { + expect(findLocalFileId(`${origin()}/core/preview?fileId=7&x=64`)).to.equal('7') + expect(findLocalFileId(`${origin()}/core/preview?fileid=7`)).to.equal('7') + expect(findLocalFileId(`${origin()}/apps/files/?FILEID=8`)).to.equal('8') + expect(findLocalFileId(`${origin()}/s/token/download?fileId=5`)).to.equal('5') + }) + + // Files deep links carry the id in openfile; only the link cell understood those. + it('reads the id a Files deep link carries', () => { + expect(findLocalFileId(`${origin()}/apps/files/files/9?openfile=9`)).to.equal('9') + expect(findLocalFileId(`${origin()}/apps/files/?dir=/&openfile=42`)).to.equal('42') + }) + + it('refuses anything that does not name a file on this server', () => { + expect(findLocalFileId('https://evil.example/index.php/f/42')).to.equal(null, 'another host is never followed') + expect(findLocalFileId('https://evil.example/core/preview?fileId=42')).to.equal(null) + expect(findLocalFileId('https://evil.example/apps/files/?openfile=42')).to.equal(null) + expect(findLocalFileId('not a link')).to.equal(null) + expect(findLocalFileId(`${origin()}/index.php/f/abc`)).to.equal(null, 'the id has to be numeric') + expect(findLocalFileId('')).to.equal(null) + expect(findLocalFileId(null)).to.equal(null) + expect(findLocalFileId(undefined)).to.equal(null) + }) + + it('unescapes the slashes a stored link may carry', () => { + expect(findLocalFileId(`${origin()}\\/index.php\\/f\\/77`)).to.equal('77') + }) + + it('accepts an id the picker stored outright, and only a usable one', () => { + expect(isFileId(12)).to.equal(true) + expect(isFileId('12')).to.equal(true) + expect(isFileId(0)).to.equal(false) + expect(isFileId(-1)).to.equal(false) + expect(isFileId(1.5)).to.equal(false) + expect(isFileId('abc')).to.equal(false) + expect(isFileId(null)).to.equal(false) + expect(isFileId(undefined)).to.equal(false) + }) + + it('rebuilds the request rather than forwarding the stored one', () => { + const url = buildPreviewUrl('7', 512) + + expect(url).to.match(/\/core\/preview\?fileId=7&x=512&y=512&a=1$/) + expect(buildPreviewUrl(7, 64)).to.match(/\/core\/preview\?fileId=7&x=64&y=64&a=1$/) + }) +}) diff --git a/cypress/component/LinkCellPreview.cy.js b/cypress/component/LinkCellPreview.cy.js new file mode 100644 index 0000000000..081df1609a --- /dev/null +++ b/cypress/component/LinkCellPreview.cy.js @@ -0,0 +1,119 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import TableCellLink from '../../src/shared/components/ncTable/partials/TableCellLink.vue' + +const PREVIEW = 'img.cell-link__preview' + +describe('Link cell preview', () => { + it('resolves the file id of a canonical file link, and renders it', () => { + mountCell(filesValue(`${window.location.origin}/index.php/f/123`)).then(({ wrapper }) => { + expect(wrapper.vm.fileId).to.equal('123') + expect(wrapper.vm.imagePreviewSrc).to.match(/fileId=123&x=64&y=64&a=1$/) + }) + + cy.get(PREVIEW).should('exist') + }) + + // The matcher used to anchor the id to the end of the path. + it('resolves the file id when the path continues after it', () => { + expectFileId(`${window.location.origin}/index.php/f/123/preview`, '123') + }) + + // The matcher used to look for a lowercase fileid only, which is not the spelling the + // server writes, so a preview URL never resolved through it. + it('resolves the file id parameter the server itself writes', () => { + expectFileId(`${window.location.origin}/index.php/core/preview?fileId=7`, '7') + }) + + it('resolves the file id a Files deep link carries', () => { + expectFileId(`${window.location.origin}/index.php/apps/files/files/9?openfile=9`, '9') + }) + + it('prefers the id the picker stored over the link it also carries', () => { + mountCell(filesValue(`${window.location.origin}/index.php/f/123`, { attributes: { fileId: 456 } })) + .then(({ wrapper }) => expect(wrapper.vm.fileId).to.equal('456')) + }) + + // An instance reachable under several trusted domains stores the link with whichever + // host the row was filled in on, so a viewer on another one sees a different origin. + // The id the picker recorded still resolves, which is what keeps that case working. + it('still resolves a picked file whose link names another trusted domain', () => { + mountCell(filesValue('https://other-domain.example/index.php/f/123', { attributes: { fileId: 3979 } })) + .then(({ wrapper }) => { + expect(wrapper.vm.fileId).to.equal('3979') + expect(wrapper.vm.imagePreviewSrc).to.match(/fileId=3979&/) + }) + }) + + // A link to another server used to yield a preview of the local file with that id. + // Asserted on the computed rather than the markup: a preview that merely fails to load + // also removes its image, so an empty cell would prove nothing on its own. + it('resolves no file id for a link to another server', () => { + expectFileId('https://evil.example/index.php/f/42', null) + }) + + it('resolves no file id for a link that is not a file reference', () => { + mountCell(JSON.stringify({ + providerId: 'url', + title: 'Just a link', + resourceUrl: `${window.location.origin}/index.php/f/123`, + })).then(({ wrapper }) => { + expect(wrapper.vm.fileId).to.equal(null) + expect(wrapper.vm.imagePreviewSrc).to.equal(null) + }) + }) + + it('builds no preview while the column does not ask for one', () => { + mountCell(filesValue(`${window.location.origin}/index.php/f/123`), { showPreview: false }) + .then(({ wrapper }) => { + expect(wrapper.vm.fileId).to.equal('123', 'the id still resolves') + expect(wrapper.vm.imagePreviewSrc).to.equal(null, 'but nothing is requested') + }) + }) +}) + +/** + * Mounts a link cell and asserts the file id it resolves. + * + * @param {string} url the link the files reference carries + * @param {string|null} expected the id that must come out of it + */ +function expectFileId(url, expected) { + mountCell(filesValue(url)).then(({ wrapper }) => expect(wrapper.vm.fileId).to.equal(expected)) +} + +/** + * A files reference carrying the given link. + * + * @param {string} url the link + * @param {object} extra further fields of the stored value + * @return {string} the raw cell value + */ +function filesValue(url, extra = {}) { + return JSON.stringify({ + providerId: 'files', + title: 'A picked file', + resourceUrl: url, + value: url, + ...extra, + }) +} + +/** + * Mounts a link cell holding the given value. + * + * @param {string} value the raw cell value + * @param {object} customSettings the column's link settings + * @return {Cypress.Chainable} the mount result + */ +function mountCell(value, customSettings = { showPreview: true, imagePreviewSize: 64 }) { + return cy.mount(TableCellLink, { + props: { + column: { id: 1, title: 'Link', customSettings }, + rowId: 1, + value, + }, + }) +} diff --git a/src/shared/components/ncTable/partials/TableCellLink.vue b/src/shared/components/ncTable/partials/TableCellLink.vue index a183aeca4d..b85619b984 100644 --- a/src/shared/components/ncTable/partials/TableCellLink.vue +++ b/src/shared/components/ncTable/partials/TableCellLink.vue @@ -71,7 +71,7 @@