From d456edce5004898d064ed634d1f787a5c8f7ed4f Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 21 Apr 2026 15:55:48 +0200 Subject: [PATCH 01/11] fix(tables): Add input validation on ApiTables and Table controllers AI-assistant: Claude Code v2.1.116 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 47 ++++++++++++++++++++++++-- lib/Controller/TableController.php | 20 +++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index fbc1fb7f92..a387a3358c 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -132,12 +132,23 @@ public function showScheme(int $id): DataResponse { * @param list $views views * @param list $columnOrder Default column order settings * @param list $sort Default sort rules - * @return DataResponse|DataResponse + * @return DataResponse|DataResponse * * 200: Tables returned + * 400: Invalid request data */ #[NoAdminRequired] public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views, array $columnOrder = [], array $sort = []): DataResponse { + foreach ($columnOrder as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { + return new DataResponse(['message' => 'Invalid columnOrder format: each entry requires columnId (int) and order (int)'], Http::STATUS_BAD_REQUEST); + } + } + foreach ($sort as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { + return new DataResponse(['message' => 'Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'], Http::STATUS_BAD_REQUEST); + } + } try { $this->db->beginTransaction(); $table = $this->service->create($title, 'custom', $emoji, $description); @@ -240,6 +251,14 @@ public function createFromScheme(string $title, string $emoji, string $descripti } $this->db->commit(); return new DataResponse($table->jsonSerialize()); + } catch (\InvalidArgumentException $e) { + try { + $this->db->rollBack(); + } catch (\OCP\DB\Exception $re) { + return $this->handleError($re); + } + $this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]); + return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } catch (InternalError|Exception $e) { try { $this->db->rollBack(); @@ -281,9 +300,10 @@ public function create(string $title, ?string $emoji, ?string $description, stri * @param string $description the tables description * @param list|string|null $columnSettings Default column order settings (array or JSON string) * @param list|string|null $sort Default sort rules (array or JSON string) - * @return DataResponse|DataResponse + * @return DataResponse|DataResponse * * 200: Tables returned + * 400: Invalid request data * 403: No permissions * 404: Not found */ @@ -296,10 +316,33 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s if (is_string($sort)) { $sort = json_decode($sort, true) ?? null; } + if ($columnSettings !== null) { + if (!is_array($columnSettings)) { + return new DataResponse(['message' => 'Invalid columnSettings: must be a JSON array'], Http::STATUS_BAD_REQUEST); + } + foreach ($columnSettings as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { + return new DataResponse(['message' => 'Invalid columnSettings format: each entry requires columnId (int) and order (int)'], Http::STATUS_BAD_REQUEST); + } + } + } + if ($sort !== null) { + if (!is_array($sort)) { + return new DataResponse(['message' => 'Invalid sort: must be a JSON array'], Http::STATUS_BAD_REQUEST); + } + foreach ($sort as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { + return new DataResponse(['message' => 'Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'], Http::STATUS_BAD_REQUEST); + } + } + } try { return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize()); } catch (PermissionError $e) { return $this->handlePermissionError($e); + } catch (\InvalidArgumentException $e) { + $this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]); + return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } catch (InternalError $e) { return $this->handleError($e); } catch (NotFoundError $e) { diff --git a/lib/Controller/TableController.php b/lib/Controller/TableController.php index e2b2849bf9..60297128a4 100644 --- a/lib/Controller/TableController.php +++ b/lib/Controller/TableController.php @@ -78,6 +78,26 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b $sort = json_decode($sort, true) ?? null; } return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) { + if ($columnSettings !== null) { + if (!is_array($columnSettings)) { + throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); + } + foreach ($columnSettings as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { + throw new \InvalidArgumentException('Invalid columnSettings format: each entry requires columnId (int) and order (int)'); + } + } + } + if ($sort !== null) { + if (!is_array($sort)) { + throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); + } + foreach ($sort as $entry) { + if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { + throw new \InvalidArgumentException('Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'); + } + } + } return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort); }); } From 1b5d657da8c25ae60ffdf07b35c5c44cee8e120c Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 21 Apr 2026 15:56:40 +0200 Subject: [PATCH 02/11] fix(tables): Add input validation on column and sort-rule-set model and ViewColumnInfo object AI-assistant: Claude Code v2.1.116 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Model/ColumnSettings.php | 3 +++ lib/Model/SortRuleSet.php | 3 +++ lib/Service/ValueObject/ViewColumnInformation.php | 3 +++ 3 files changed, 9 insertions(+) diff --git a/lib/Model/ColumnSettings.php b/lib/Model/ColumnSettings.php index 12c9b09feb..23a42d4431 100644 --- a/lib/Model/ColumnSettings.php +++ b/lib/Model/ColumnSettings.php @@ -36,6 +36,9 @@ public function columnInformation(): Generator { public static function createFromInputArray(array $inputColumnSettings): self { $columnSettings = []; foreach ($inputColumnSettings as $inputColumnSetting) { + if (!is_array($inputColumnSetting)) { + throw new \InvalidArgumentException('Each column settings entry must be an array'); + } $columnSettings[] = ViewColumnInformation::fromArray($inputColumnSetting); } return new self($columnSettings); diff --git a/lib/Model/SortRuleSet.php b/lib/Model/SortRuleSet.php index 0555867e97..db3b40a5c8 100644 --- a/lib/Model/SortRuleSet.php +++ b/lib/Model/SortRuleSet.php @@ -35,6 +35,9 @@ public function __construct( public static function createFromInputArray(array $data): self { $sortRules = []; foreach ($data as $inputSortRule) { + if (!is_array($inputSortRule)) { + throw new InvalidArgumentException('Each sort rule entry must be an array'); + } if (!isset($inputSortRule['columnId'], $inputSortRule['mode'])) { throw new InvalidArgumentException('Required sort parameters are missing'); } diff --git a/lib/Service/ValueObject/ViewColumnInformation.php b/lib/Service/ValueObject/ViewColumnInformation.php index 4049e8fdcb..4bb1941e41 100644 --- a/lib/Service/ValueObject/ViewColumnInformation.php +++ b/lib/Service/ValueObject/ViewColumnInformation.php @@ -58,6 +58,9 @@ public function isMandatory(): bool { } public static function fromArray(array $data): static { + if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) { + throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required'); + } $vci = new static( (int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER], From 0bcb09b6eff5f5377877658db0204ecd65c70519 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 21 Apr 2026 16:38:32 +0200 Subject: [PATCH 03/11] docs(tables): Update openAPI spec Signed-off-by: Andy Scherzinger --- openapi.json | 76 ++++++++++++++++++++++++++++++++++++ src/types/openapi/openapi.ts | 32 +++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/openapi.json b/openapi.json index 9180fc32bf..28a8ab31e1 100644 --- a/openapi.json +++ b/openapi.json @@ -7088,6 +7088,44 @@ } } }, + "400": { + "description": "Invalid request data", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, "403": { "description": "No permissions", "content": { @@ -7811,6 +7849,44 @@ } } }, + "400": { + "description": "Invalid request data", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, "500": { "description": "", "content": { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index aeadcd3fd2..6610ff6c51 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -4550,6 +4550,22 @@ export interface operations { }; }; }; + /** @description Invalid request data */ + readonly 400: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; /** @description Current user is not logged in */ readonly 401: { headers: { @@ -4859,6 +4875,22 @@ export interface operations { }; }; }; + /** @description Invalid request data */ + readonly 400: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; /** @description Current user is not logged in */ readonly 401: { headers: { From 78b1a5ae23d4cb720e1ba83d81010077cfaa7812 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 22 Apr 2026 00:30:25 +0200 Subject: [PATCH 04/11] fix(tables): Use value objects for input validation in controllers AI-assistant: Claude Code v2.1.116 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 42 +++++++++++--------------- lib/Controller/TableController.php | 14 +++------ lib/Db/Table.php | 2 +- lib/Model/ColumnSettings.php | 6 ++-- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index a387a3358c..d56202c17e 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -14,6 +14,8 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\ColumnSettings; +use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\ViewUpdateInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnService; @@ -139,15 +141,11 @@ public function showScheme(int $id): DataResponse { */ #[NoAdminRequired] public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views, array $columnOrder = [], array $sort = []): DataResponse { - foreach ($columnOrder as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { - return new DataResponse(['message' => 'Invalid columnOrder format: each entry requires columnId (int) and order (int)'], Http::STATUS_BAD_REQUEST); - } - } - foreach ($sort as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { - return new DataResponse(['message' => 'Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'], Http::STATUS_BAD_REQUEST); - } + try { + ColumnSettings::createFromInputArray($columnOrder); + SortRuleSet::createFromInputArray($sort); + } catch (\InvalidArgumentException $e) { + return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } try { $this->db->beginTransaction(); @@ -316,25 +314,21 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s if (is_string($sort)) { $sort = json_decode($sort, true) ?? null; } - if ($columnSettings !== null) { - if (!is_array($columnSettings)) { - return new DataResponse(['message' => 'Invalid columnSettings: must be a JSON array'], Http::STATUS_BAD_REQUEST); - } - foreach ($columnSettings as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { - return new DataResponse(['message' => 'Invalid columnSettings format: each entry requires columnId (int) and order (int)'], Http::STATUS_BAD_REQUEST); + try { + if ($columnSettings !== null) { + if (!is_array($columnSettings)) { + throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); } + ColumnSettings::createFromInputArray($columnSettings); } - } - if ($sort !== null) { - if (!is_array($sort)) { - return new DataResponse(['message' => 'Invalid sort: must be a JSON array'], Http::STATUS_BAD_REQUEST); - } - foreach ($sort as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { - return new DataResponse(['message' => 'Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'], Http::STATUS_BAD_REQUEST); + if ($sort !== null) { + if (!is_array($sort)) { + throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); } + SortRuleSet::createFromInputArray($sort); } + } catch (\InvalidArgumentException $e) { + return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } try { return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize()); diff --git a/lib/Controller/TableController.php b/lib/Controller/TableController.php index 60297128a4..6cc23800b4 100644 --- a/lib/Controller/TableController.php +++ b/lib/Controller/TableController.php @@ -9,6 +9,8 @@ use OCA\Tables\AppInfo\Application; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\ColumnSettings; +use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Service\TableService; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -82,21 +84,13 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b if (!is_array($columnSettings)) { throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); } - foreach ($columnSettings as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['order'])) { - throw new \InvalidArgumentException('Invalid columnSettings format: each entry requires columnId (int) and order (int)'); - } - } + ColumnSettings::createFromInputArray($columnSettings); } if ($sort !== null) { if (!is_array($sort)) { throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); } - foreach ($sort as $entry) { - if (!is_array($entry) || !isset($entry['columnId'], $entry['mode']) || !in_array($entry['mode'], ['ASC', 'DESC'], true)) { - throw new \InvalidArgumentException('Invalid sort format: each entry requires columnId (int) and mode (ASC or DESC)'); - } - } + SortRuleSet::createFromInputArray($sort); } return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort); }); diff --git a/lib/Db/Table.php b/lib/Db/Table.php index 98479d45d7..08d2b5f52b 100644 --- a/lib/Db/Table.php +++ b/lib/Db/Table.php @@ -154,7 +154,7 @@ public function getColumnOrderSettingsArray(): array { return []; } - if (is_array(reset($columns))) { + if (is_array($columns[array_key_first($columns)] ?? null)) { return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns)); } diff --git a/lib/Model/ColumnSettings.php b/lib/Model/ColumnSettings.php index 23a42d4431..d0c9cd88cb 100644 --- a/lib/Model/ColumnSettings.php +++ b/lib/Model/ColumnSettings.php @@ -13,7 +13,7 @@ use JsonSerializable; use OCA\Tables\Service\ValueObject\ViewColumnInformation; -class ColumnSettings implements JSONSerializable { +class ColumnSettings implements JsonSerializable { /** * @param ViewColumnInformation[] $columnSettings */ @@ -44,7 +44,7 @@ public static function createFromInputArray(array $inputColumnSettings): self { return new self($columnSettings); } - public function jsonSerialize(): mixed { - return $this->columnSettings; + public function jsonSerialize(): array { + return array_map(static fn (ViewColumnInformation $vci) => $vci->jsonSerialize(), $this->columnSettings); } } From a24f8775f9fb409ea7d9e4d80cf2b855ce99efc0 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 22 Apr 2026 00:32:20 +0200 Subject: [PATCH 05/11] test(tables): add further tests for ColumnSetting and SortRuleSet AI-assistant: Claude Code v2.1.116 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- tests/unit/Model/ColumnSettingsTest.php | 56 ++++++++++++++++++++++ tests/unit/Model/SortRuleSetTest.php | 62 +++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 tests/unit/Model/ColumnSettingsTest.php create mode 100644 tests/unit/Model/SortRuleSetTest.php diff --git a/tests/unit/Model/ColumnSettingsTest.php b/tests/unit/Model/ColumnSettingsTest.php new file mode 100644 index 0000000000..87f9783a86 --- /dev/null +++ b/tests/unit/Model/ColumnSettingsTest.php @@ -0,0 +1,56 @@ + 1, 'order' => 0, 'readonly' => false], + ['columnId' => 2, 'order' => 1, 'readonly' => true], + ]; + $settings = ColumnSettings::createFromInputArray($data); + $serialized = $settings->jsonSerialize(); + + $this->assertCount(2, $serialized); + $this->assertSame(1, $serialized[0]['columnId']); + $this->assertSame(0, $serialized[0]['order']); + $this->assertSame(2, $serialized[1]['columnId']); + $this->assertSame(1, $serialized[1]['order']); + } + + public function testCreateFromInputArrayWithEmptyArray(): void { + $settings = ColumnSettings::createFromInputArray([]); + $this->assertSame([], $settings->jsonSerialize()); + } + + public function testCreateFromInputArrayThrowsWhenEntryIsNotAnArray(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createFromInputArray(['not-an-array']); + } + + public function testCreateFromInputArrayThrowsWhenEntryIsAnInteger(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createFromInputArray([42]); + } + + public function testCreateFromInputArrayThrowsWhenColumnIdIsMissing(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createFromInputArray([['order' => 0]]); + } + + public function testCreateFromInputArrayThrowsWhenOrderIsMissing(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createFromInputArray([['columnId' => 1]]); + } +} diff --git a/tests/unit/Model/SortRuleSetTest.php b/tests/unit/Model/SortRuleSetTest.php new file mode 100644 index 0000000000..87e03cb301 --- /dev/null +++ b/tests/unit/Model/SortRuleSetTest.php @@ -0,0 +1,62 @@ + 1, 'mode' => 'ASC'], + ['columnId' => 2, 'mode' => 'DESC'], + ]; + $ruleSet = SortRuleSet::createFromInputArray($data); + $serialized = $ruleSet->jsonSerialize(); + + $this->assertCount(2, $serialized); + $this->assertSame(1, $serialized[0]['columnId']); + $this->assertSame('ASC', $serialized[0]['mode']); + $this->assertSame(2, $serialized[1]['columnId']); + $this->assertSame('DESC', $serialized[1]['mode']); + } + + public function testCreateFromInputArrayWithEmptyArray(): void { + $ruleSet = SortRuleSet::createFromInputArray([]); + $this->assertSame([], $ruleSet->jsonSerialize()); + } + + public function testCreateFromInputArrayThrowsWhenEntryIsNotAnArray(): void { + $this->expectException(InvalidArgumentException::class); + SortRuleSet::createFromInputArray(['not-an-array']); + } + + public function testCreateFromInputArrayThrowsWhenEntryIsAnInteger(): void { + $this->expectException(InvalidArgumentException::class); + SortRuleSet::createFromInputArray([42]); + } + + public function testCreateFromInputArrayThrowsWhenColumnIdIsMissing(): void { + $this->expectException(InvalidArgumentException::class); + SortRuleSet::createFromInputArray([['mode' => 'ASC']]); + } + + public function testCreateFromInputArrayThrowsWhenModeIsMissing(): void { + $this->expectException(InvalidArgumentException::class); + SortRuleSet::createFromInputArray([['columnId' => 1]]); + } + + public function testCreateFromInputArrayThrowsWhenModeIsInvalid(): void { + $this->expectException(InvalidArgumentException::class); + SortRuleSet::createFromInputArray([['columnId' => 1, 'mode' => 'INVALID']]); + } +} From b738eb0185cb3db818eda2439b25e05005795296 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 22 Apr 2026 00:57:13 +0200 Subject: [PATCH 06/11] fix(tables): address review comments AI-assistant: Claude Code v2.1.117 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 26 +++--- lib/Controller/TableController.php | 20 ++--- lib/Db/Table.php | 34 +++---- lib/Model/ColumnSettings.php | 26 +++++- lib/Model/SortRuleSet.php | 3 + lib/Model/ViewUpdateInput.php | 2 +- lib/ResponseDefinitions.php | 3 +- lib/Service/TableService.php | 6 +- .../ValueObject/ColumnOrderInformation.php | 90 +++++++++++++++++++ lib/Service/ValueObject/SortRule.php | 3 + .../ValueObject/ViewColumnInformation.php | 60 +++---------- openapi.json | 10 +-- src/types/openapi/openapi.ts | 2 +- tests/unit/Model/ColumnSettingsTest.php | 48 ++++++++++ tests/unit/Service/TableServiceTest.php | 2 +- 15 files changed, 228 insertions(+), 107 deletions(-) create mode 100644 lib/Service/ValueObject/ColumnOrderInformation.php diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index d56202c17e..2a3f0321b2 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -187,18 +187,18 @@ public function createFromScheme(string $title, string $emoji, string $descripti $colMap[$column['id']] = $col->getId(); } if (!empty($columnOrder) || !empty($sort)) { - $remappedColumnOrder = !empty($columnOrder) ? array_map(static function (array $entry) use ($colMap): array { + $remappedColumnOrder = !empty($columnOrder) ? ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { if (isset($entry['columnId']) && $entry['columnId'] > 0) { $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; } return $entry; - }, $columnOrder) : null; - $remappedSort = !empty($sort) ? array_map(static function (array $entry) use ($colMap): array { + }, $columnOrder)) : null; + $remappedSort = !empty($sort) ? SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { if (isset($entry['columnId']) && $entry['columnId'] > 0) { $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; } return $entry; - }, $sort) : null; + }, $sort)) : null; $table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort); } foreach ($views as $view) { @@ -315,23 +315,19 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s $sort = json_decode($sort, true) ?? null; } try { - if ($columnSettings !== null) { - if (!is_array($columnSettings)) { - throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); - } - ColumnSettings::createFromInputArray($columnSettings); + if ($columnSettings !== null && !is_array($columnSettings)) { + throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); } - if ($sort !== null) { - if (!is_array($sort)) { - throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); - } - SortRuleSet::createFromInputArray($sort); + if ($sort !== null && !is_array($sort)) { + throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); } + $columnSettingsObj = $columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null; + $sortObj = $sort !== null ? SortRuleSet::createFromInputArray($sort) : null; } catch (\InvalidArgumentException $e) { return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } try { - return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize()); + return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettingsObj, $sortObj)->jsonSerialize()); } catch (PermissionError $e) { return $this->handlePermissionError($e); } catch (\InvalidArgumentException $e) { diff --git a/lib/Controller/TableController.php b/lib/Controller/TableController.php index 6cc23800b4..07045e71c6 100644 --- a/lib/Controller/TableController.php +++ b/lib/Controller/TableController.php @@ -80,19 +80,17 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b $sort = json_decode($sort, true) ?? null; } return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) { - if ($columnSettings !== null) { - if (!is_array($columnSettings)) { - throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); - } - ColumnSettings::createFromInputArray($columnSettings); + if ($columnSettings !== null && !is_array($columnSettings)) { + throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array'); } - if ($sort !== null) { - if (!is_array($sort)) { - throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); - } - SortRuleSet::createFromInputArray($sort); + if ($sort !== null && !is_array($sort)) { + throw new \InvalidArgumentException('Invalid sort: must be a JSON array'); } - return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort); + return $this->service->update( + $id, $title, $emoji, null, $archived, $this->userId, + $columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null, + $sort !== null ? SortRuleSet::createFromInputArray($sort) : null, + ); }); } } diff --git a/lib/Db/Table.php b/lib/Db/Table.php index 08d2b5f52b..418ce443d2 100644 --- a/lib/Db/Table.php +++ b/lib/Db/Table.php @@ -11,7 +11,7 @@ use OCA\Tables\Model\Permissions; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\ResponseDefinitions; -use OCA\Tables\Service\ValueObject\ViewColumnInformation; +use OCA\Tables\Service\ValueObject\ColumnOrderInformation; /** * @psalm-suppress PropertyNotSetInConstructor @@ -44,7 +44,7 @@ * @method setRowsCount(int $rowsCount) * @method getColumnsCount(): int * @method setColumnsCount(int $columnsCount) - * @method getViews(): ?array + * @method getViews(): ?array * @method setViews(array $views) * @method getColumns(): array * @method setColumns(array $columns) @@ -109,14 +109,17 @@ public function jsonSerialize(): array { 'lastEditAt' => $this->lastEditAt ?: '', 'archived' => $this->archived, 'isShared' => (bool)$this->isShared, - 'favorite' => $this->favorite, + 'favorite' => (bool)$this->favorite, 'onSharePermissions' => $this->getSharePermissions()?->jsonSerialize(), 'hasShares' => (bool)$this->hasShares, 'rowsCount' => $this->rowsCount ?: 0, 'columnsCount' => $this->columnsCount ?: 0, - 'views' => $this->getViewsArray(), - 'description' => $this->description ?:'', - 'columnOrder' => $this->getColumnOrderSettingsArray(), + 'description' => $this->description ?: '', + 'views' => array_values($this->getViewsArray()), + 'columnOrder' => array_map( + static fn (ColumnOrderInformation $c) => ['columnId' => $c->getId(), 'order' => $c->getOrder()], + $this->getColumnOrderSettingsArray() + ), 'sort' => $this->getSortArray(), ]; } @@ -126,7 +129,6 @@ private function getSharePermissions(): ?Permissions { } /** - * @psalm-suppress MismatchingDocblockReturnType * @return TablesView[] */ private function getViewsArray(): array { @@ -134,19 +136,19 @@ private function getViewsArray(): array { } /** - * @psalm-suppress MismatchingDocblockReturnType * @return int[] */ public function getColumnOrderArray(): array { $columnSettings = $this->getColumnOrderSettingsArray(); - usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) { + usort($columnSettings, static function (ColumnOrderInformation $a, ColumnOrderInformation $b) { return $a->getOrder() - $b->getOrder(); }); - return array_map(static fn (ViewColumnInformation $vci): int => $vci->getId(), $columnSettings); + /** @var list $columnSettings */ + return array_map(static fn (ColumnOrderInformation $vci): int => $vci->getId(), $columnSettings); } /** - * @return array + * @return list */ public function getColumnOrderSettingsArray(): array { $columns = $this->getArray($this->getColumnOrder()); @@ -155,18 +157,17 @@ public function getColumnOrderSettingsArray(): array { } if (is_array($columns[array_key_first($columns)] ?? null)) { - return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns)); + return array_values(array_map(static fn (array $a): ColumnOrderInformation => ColumnOrderInformation::fromArray($a), $columns)); } $result = []; foreach ($columns as $index => $columnId) { - $result[] = new ViewColumnInformation($columnId, order: (int)$index + 1); + $result[] = new ColumnOrderInformation((int)$columnId, order: (int)$index + 1); } return $result; } /** - * @psalm-suppress MismatchingDocblockReturnType * @return list */ public function getSortArray(): array { @@ -176,9 +177,8 @@ public function getSortArray(): array { private function getArray(?string $json): array { if ($json !== '' && $json !== null && $json !== 'null') { - return \json_decode($json, true); - } else { - return []; + return \json_decode($json, true) ?? []; } + return []; } } diff --git a/lib/Model/ColumnSettings.php b/lib/Model/ColumnSettings.php index d0c9cd88cb..c380c6f4c8 100644 --- a/lib/Model/ColumnSettings.php +++ b/lib/Model/ColumnSettings.php @@ -11,18 +11,19 @@ use Generator; use JsonSerializable; +use OCA\Tables\Service\ValueObject\ColumnOrderInformation; use OCA\Tables\Service\ValueObject\ViewColumnInformation; class ColumnSettings implements JsonSerializable { /** - * @param ViewColumnInformation[] $columnSettings + * @param ColumnOrderInformation[] $columnSettings */ public function __construct( protected array $columnSettings, ) { foreach ($this->columnSettings as $columnSetting) { - if (!$columnSetting instanceof ViewColumnInformation) { - throw new \InvalidArgumentException('Provided column settings must be of type ViewColumnInformation'); + if (!$columnSetting instanceof ColumnOrderInformation) { + throw new \InvalidArgumentException('Provided column settings must be of type ColumnOrderInformation'); } } } @@ -33,7 +34,24 @@ public function columnInformation(): Generator { } } + /** + * Creates column settings with only columnId and order (for table use). + */ public static function createFromInputArray(array $inputColumnSettings): self { + $columnSettings = []; + foreach ($inputColumnSettings as $inputColumnSetting) { + if (!is_array($inputColumnSetting)) { + throw new \InvalidArgumentException('Each column settings entry must be an array'); + } + $columnSettings[] = ColumnOrderInformation::fromArray($inputColumnSetting); + } + return new self($columnSettings); + } + + /** + * Creates column settings with view-specific fields (columnId, order, readonly, mandatory). + */ + public static function createViewSettingsFromInputArray(array $inputColumnSettings): self { $columnSettings = []; foreach ($inputColumnSettings as $inputColumnSetting) { if (!is_array($inputColumnSetting)) { @@ -45,6 +63,6 @@ public static function createFromInputArray(array $inputColumnSettings): self { } public function jsonSerialize(): array { - return array_map(static fn (ViewColumnInformation $vci) => $vci->jsonSerialize(), $this->columnSettings); + return array_map(static fn (ColumnOrderInformation $vci) => $vci->jsonSerialize(), $this->columnSettings); } } diff --git a/lib/Model/SortRuleSet.php b/lib/Model/SortRuleSet.php index db3b40a5c8..55e95c55cc 100644 --- a/lib/Model/SortRuleSet.php +++ b/lib/Model/SortRuleSet.php @@ -50,6 +50,9 @@ public static function createFromInputArray(array $data): self { return new self($sortRules); } + /** + * @return list + */ public function jsonSerialize(): array { return array_map(static fn (SortRule $s) => $s->jsonSerialize(), $this->sortRules); } diff --git a/lib/Model/ViewUpdateInput.php b/lib/Model/ViewUpdateInput.php index cd9d8f9a12..3e01ef9789 100644 --- a/lib/Model/ViewUpdateInput.php +++ b/lib/Model/ViewUpdateInput.php @@ -84,7 +84,7 @@ public static function fromInputArray(array $data): self { title: ($data['title'] ?? null) ? new Title($data['title']) : null, description: $data['description'] ?? null, emoji: ($data['emoji'] ?? null) ? new Emoji($data['emoji']) : null, - columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createFromInputArray($data['columnSettings']) : null, + columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createViewSettingsFromInputArray($data['columnSettings']) : null, filterSet: ($data['filter'] ?? null) ? FilterSet::createFromInputArray($data['filter']) : null, sortRuleSet: ($data['sort'] ?? null) ? SortRuleSet::createFromInputArray($data['sort']) : null, ); diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 32edc938c3..1dd4d5f6a8 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -64,9 +64,10 @@ * }, * hasShares: bool, * rowsCount: int, + * description: string, * views: list, * columnsCount: int, - * columnOrder: list, + * columnOrder: list, * sort: list, * } * diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index de9133ea00..726ee1c73e 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -476,7 +476,7 @@ public function delete(int $id, ?string $userId = null): Table { * @throws NotFoundError * @throws PermissionError */ - public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?array $columnSettings = null, ?array $sort = null): Table { + public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?ColumnSettings $columnSettings = null, ?SortRuleSet $sort = null): Table { $userId = $this->permissionsService->preCheckUserId($userId); try { @@ -509,10 +509,10 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip $table->setDescription($description); } if ($columnSettings !== null) { - $table->setColumnOrder(\json_encode(ColumnSettings::createFromInputArray($columnSettings)->jsonSerialize())); + $table->setColumnOrder(\json_encode($columnSettings->jsonSerialize())); } if ($sort !== null) { - $table->setSort(\json_encode(SortRuleSet::createFromInputArray($sort)->jsonSerialize())); + $table->setSort(\json_encode($sort->jsonSerialize())); } $table->setLastEditBy($userId); $table->setLastEditAt($time->format('Y-m-d H:i:s')); diff --git a/lib/Service/ValueObject/ColumnOrderInformation.php b/lib/Service/ValueObject/ColumnOrderInformation.php new file mode 100644 index 0000000000..9223e483c9 --- /dev/null +++ b/lib/Service/ValueObject/ColumnOrderInformation.php @@ -0,0 +1,90 @@ + + */ +class ColumnOrderInformation implements ArrayAccess, JsonSerializable { + public const KEY_ID = 'columnId'; + public const KEY_ORDER = 'order'; + + protected array $data = []; + protected const KEYS = [ + self::KEY_ID, + self::KEY_ORDER, + ]; + + public function __construct(int $columnId, int $order) { + $this->offsetSet(self::KEY_ID, $columnId); + $this->offsetSet(self::KEY_ORDER, $order); + } + + public function getId(): int { + return (int)$this->offsetGet(self::KEY_ID); + } + + public function getOrder(): int { + return (int)$this->offsetGet(self::KEY_ORDER); + } + + public static function fromArray(array $data): self { + if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) { + throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required'); + } + return new self((int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER]); + } + + public function offsetExists(mixed $offset): bool { + return in_array((string)$offset, static::KEYS); + } + + public function offsetGet(mixed $offset): bool|int { + return $this->data[$offset]; + } + + public function offsetSet(mixed $offset, mixed $value): void { + if (!$this->offsetExists($offset)) { + return; + } + $this->data[$offset] = $this->ensureType((string)$offset, $value); + } + + public function offsetUnset(mixed $offset): void { + if (!$this->offsetExists($offset)) { + return; + } + unset($this->data[(string)$offset]); + } + + /** + * @return array{columnId: int, order: int, ...} + */ + public function jsonSerialize(): array { + return [ + self::KEY_ID => $this->getId(), + self::KEY_ORDER => $this->getOrder(), + ]; + } + + protected function ensureType(string $offset, mixed $value): int|bool { + return match ($offset) { + self::KEY_ID, + self::KEY_ORDER => (int)$value, + default => throw new \InvalidArgumentException("Invalid offset: $offset"), + }; + } +} diff --git a/lib/Service/ValueObject/SortRule.php b/lib/Service/ValueObject/SortRule.php index 2836ee31f5..c229817a36 100644 --- a/lib/Service/ValueObject/SortRule.php +++ b/lib/Service/ValueObject/SortRule.php @@ -22,6 +22,9 @@ public function __construct( } } + /** + * @return array{columnId: int, mode: 'ASC'|'DESC'} + */ public function jsonSerialize(): array { return [ 'columnId' => $this->columnId, diff --git a/lib/Service/ValueObject/ViewColumnInformation.php b/lib/Service/ValueObject/ViewColumnInformation.php index 4bb1941e41..8ff1b3c295 100644 --- a/lib/Service/ValueObject/ViewColumnInformation.php +++ b/lib/Service/ValueObject/ViewColumnInformation.php @@ -8,20 +8,13 @@ namespace OCA\Tables\Service\ValueObject; -use ArrayAccess; -use JsonSerializable; - /** - * @template-implements ArrayAccess + * Extends ColumnOrderInformation with view-specific fields (readonly, mandatory). */ -class ViewColumnInformation implements ArrayAccess, JsonSerializable { - public const KEY_ID = 'columnId'; - public const KEY_ORDER = 'order'; +class ViewColumnInformation extends ColumnOrderInformation { public const KEY_READONLY = 'readonly'; public const KEY_MANDATORY = 'mandatory'; - /** @var array{columnId: int, order: int, readonly?: bool, mandatory?: bool} */ - protected array $data; protected const KEYS = [ self::KEY_ID, self::KEY_ORDER, @@ -35,20 +28,11 @@ public function __construct( bool $readonly = false, bool $mandatory = false, ) { - $this->offsetSet(self::KEY_ID, $columnId); - $this->offsetSet(self::KEY_ORDER, $order); + parent::__construct($columnId, $order); $this->offsetSet(self::KEY_READONLY, $readonly); $this->offsetSet(self::KEY_MANDATORY, $mandatory); } - public function getId(): int { - return (int)$this->offsetGet(self::KEY_ID); - } - - public function getOrder(): int { - return (int)$this->offsetGet(self::KEY_ORDER); - } - public function isReadonly(): bool { return (bool)$this->offsetGet(self::KEY_READONLY); } @@ -61,51 +45,31 @@ public static function fromArray(array $data): static { if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) { throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required'); } - $vci = new static( + return new static( (int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER], (bool)($data[self::KEY_READONLY] ?? false), (bool)($data[self::KEY_MANDATORY] ?? false), ); - - return $vci; - } - - public function offsetExists(mixed $offset): bool { - return in_array((string)$offset, self::KEYS); - } - - public function offsetGet(mixed $offset): bool|int { - return $this->data[$offset]; - } - - public function offsetSet(mixed $offset, mixed $value): void { - if (!$this->offsetExists($offset)) { - return; - } - - $this->data[$offset] = $this->ensureType($offset, $value); - } - - public function offsetUnset(mixed $offset): void { - if (!$this->offsetExists($offset)) { - return; - } - unset($this->data[(string)$offset]); } /** - * @return array{columnId: int, order: int, readonly?: bool, mandatory?: bool} + * @return array{columnId: int, order: int, readonly: bool, mandatory: bool} */ public function jsonSerialize(): array { - return $this->data; + return [ + self::KEY_ID => $this->getId(), + self::KEY_ORDER => $this->getOrder(), + self::KEY_READONLY => $this->isReadonly(), + self::KEY_MANDATORY => $this->isMandatory(), + ]; } protected function ensureType(string $offset, mixed $value): int|bool { return match ($offset) { self::KEY_ID, self::KEY_ORDER => (int)$value, - self::KEY_READONLY => (bool)$value, + self::KEY_READONLY, self::KEY_MANDATORY => (bool)$value, default => throw new \InvalidArgumentException("Invalid offset: $offset"), }; diff --git a/openapi.json b/openapi.json index 28a8ab31e1..6ab031fc0d 100644 --- a/openapi.json +++ b/openapi.json @@ -756,6 +756,7 @@ "onSharePermissions", "hasShares", "rowsCount", + "description", "views", "columnsCount", "columnOrder", @@ -835,6 +836,9 @@ "type": "integer", "format": "int64" }, + "description": { + "type": "string" + }, "views": { "type": "array", "items": { @@ -851,8 +855,7 @@ "type": "object", "required": [ "columnId", - "order", - "readonly" + "order" ], "properties": { "columnId": { @@ -862,9 +865,6 @@ "order": { "type": "integer", "format": "int64" - }, - "readonly": { - "type": "boolean" } } } diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 6610ff6c51..2478af53c3 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1132,6 +1132,7 @@ export type components = { readonly hasShares: boolean; /** Format: int64 */ readonly rowsCount: number; + readonly description: string; readonly views: readonly components["schemas"]["View"][]; /** Format: int64 */ readonly columnsCount: number; @@ -1140,7 +1141,6 @@ export type components = { readonly columnId: number; /** Format: int64 */ readonly order: number; - readonly readonly: boolean; }[]; readonly sort: readonly { /** Format: int64 */ diff --git a/tests/unit/Model/ColumnSettingsTest.php b/tests/unit/Model/ColumnSettingsTest.php index 87f9783a86..509158d242 100644 --- a/tests/unit/Model/ColumnSettingsTest.php +++ b/tests/unit/Model/ColumnSettingsTest.php @@ -10,6 +10,8 @@ namespace OCA\Tables\Tests\Unit\Model; use OCA\Tables\Model\ColumnSettings; +use OCA\Tables\Service\ValueObject\ColumnOrderInformation; +use OCA\Tables\Service\ValueObject\ViewColumnInformation; use PHPUnit\Framework\TestCase; class ColumnSettingsTest extends TestCase { @@ -29,6 +31,15 @@ public function testCreateFromInputArrayWithValidData(): void { $this->assertSame(1, $serialized[1]['order']); } + public function testCreateFromInputArrayProducesColumnOrderInformation(): void { + $settings = ColumnSettings::createFromInputArray([ + ['columnId' => 1, 'order' => 0], + ]); + $entries = iterator_to_array($settings->columnInformation()); + $this->assertInstanceOf(ColumnOrderInformation::class, $entries[0]); + $this->assertNotInstanceOf(ViewColumnInformation::class, $entries[0]); + } + public function testCreateFromInputArrayWithEmptyArray(): void { $settings = ColumnSettings::createFromInputArray([]); $this->assertSame([], $settings->jsonSerialize()); @@ -53,4 +64,41 @@ public function testCreateFromInputArrayThrowsWhenOrderIsMissing(): void { $this->expectException(\InvalidArgumentException::class); ColumnSettings::createFromInputArray([['columnId' => 1]]); } + + public function testCreateViewSettingsFromInputArrayProducesViewColumnInformation(): void { + $data = [ + ['columnId' => 1, 'order' => 0, 'readonly' => true, 'mandatory' => false], + ['columnId' => 2, 'order' => 1, 'readonly' => false, 'mandatory' => true], + ]; + $settings = ColumnSettings::createViewSettingsFromInputArray($data); + $entries = iterator_to_array($settings->columnInformation()); + + $this->assertInstanceOf(ViewColumnInformation::class, $entries[0]); + $this->assertTrue($entries[0]->isReadonly()); + $this->assertFalse($entries[0]->isMandatory()); + $this->assertInstanceOf(ViewColumnInformation::class, $entries[1]); + $this->assertFalse($entries[1]->isReadonly()); + $this->assertTrue($entries[1]->isMandatory()); + } + + public function testCreateViewSettingsFromInputArraySerialisesAllFields(): void { + $data = [['columnId' => 3, 'order' => 2, 'readonly' => true, 'mandatory' => true]]; + $settings = ColumnSettings::createViewSettingsFromInputArray($data); + $serialized = $settings->jsonSerialize(); + + $this->assertSame(3, $serialized[0]['columnId']); + $this->assertSame(2, $serialized[0]['order']); + $this->assertTrue($serialized[0]['readonly']); + $this->assertTrue($serialized[0]['mandatory']); + } + + public function testCreateViewSettingsFromInputArrayThrowsWhenEntryIsNotAnArray(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createViewSettingsFromInputArray(['not-an-array']); + } + + public function testCreateViewSettingsFromInputArrayThrowsWhenColumnIdIsMissing(): void { + $this->expectException(\InvalidArgumentException::class); + ColumnSettings::createViewSettingsFromInputArray([['order' => 0]]); + } } diff --git a/tests/unit/Service/TableServiceTest.php b/tests/unit/Service/TableServiceTest.php index ea48f9d68f..919661b377 100644 --- a/tests/unit/Service/TableServiceTest.php +++ b/tests/unit/Service/TableServiceTest.php @@ -73,7 +73,7 @@ public function testNullSortInputOnUninitializedTableLeavesFieldNull(): void { } public function testGetSchemeColumnOrderRoundTrip(): void { - $columnOrder = [['columnId' => 3, 'order' => 1, 'readonly' => false, 'mandatory' => false]]; + $columnOrder = [['columnId' => 3, 'order' => 1]]; $table = new Table(); $table->setColumnOrder(\json_encode($columnOrder)); From 03a290f0288548179afc30274d11cf7ed638ad34 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 24 Apr 2026 16:14:42 +0200 Subject: [PATCH 07/11] fix(tables): Don't use coalescing operator over multiple lines with a callable in between AI-assistant: Copilot v1.0.6 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 30 +++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 2a3f0321b2..1d996cc5f8 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -187,18 +187,24 @@ public function createFromScheme(string $title, string $emoji, string $descripti $colMap[$column['id']] = $col->getId(); } if (!empty($columnOrder) || !empty($sort)) { - $remappedColumnOrder = !empty($columnOrder) ? ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { - if (isset($entry['columnId']) && $entry['columnId'] > 0) { - $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; - } - return $entry; - }, $columnOrder)) : null; - $remappedSort = !empty($sort) ? SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { - if (isset($entry['columnId']) && $entry['columnId'] > 0) { - $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; - } - return $entry; - }, $sort)) : null; + $remappedColumnOrder = null; + if (!empty($columnOrder)) { + $remappedColumnOrder = ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { + if (isset($entry['columnId']) && $entry['columnId'] > 0) { + $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; + } + return $entry; + }, $columnOrder)); + } + $remappedSort = null; + if (!empty($sort)) { + $remappedSort = SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { + if (isset($entry['columnId']) && $entry['columnId'] > 0) { + $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; + } + return $entry; + }, $sort)); + } $table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort); } foreach ($views as $view) { From 71620ecc7a39cf763d2855161dee24dd9aa3de16 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 24 Apr 2026 16:17:07 +0200 Subject: [PATCH 08/11] docs(sdpx): Bump year to 2026 Signed-off-by: Andy Scherzinger --- lib/Service/ValueObject/ColumnOrderInformation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/ValueObject/ColumnOrderInformation.php b/lib/Service/ValueObject/ColumnOrderInformation.php index 9223e483c9..8196cf9087 100644 --- a/lib/Service/ValueObject/ColumnOrderInformation.php +++ b/lib/Service/ValueObject/ColumnOrderInformation.php @@ -2,7 +2,7 @@ declare(strict_types=1); /** - * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ From 3efa881b93b9ddd137b67c032a0497e141f475d0 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 24 Apr 2026 16:28:03 +0200 Subject: [PATCH 09/11] fix(tables): use parent logic/implementation, revert formatting change AI-assistant: Copilot v1.0.6 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- .../ValueObject/ColumnOrderInformation.php | 6 +++++- .../ValueObject/ViewColumnInformation.php | 16 +++++----------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/Service/ValueObject/ColumnOrderInformation.php b/lib/Service/ValueObject/ColumnOrderInformation.php index 8196cf9087..e7d0a12945 100644 --- a/lib/Service/ValueObject/ColumnOrderInformation.php +++ b/lib/Service/ValueObject/ColumnOrderInformation.php @@ -42,10 +42,14 @@ public function getOrder(): int { } public static function fromArray(array $data): self { + static::assertRequiredFields($data); + return new self((int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER]); + } + + protected static function assertRequiredFields(array $data): void { if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) { throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required'); } - return new self((int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER]); } public function offsetExists(mixed $offset): bool { diff --git a/lib/Service/ValueObject/ViewColumnInformation.php b/lib/Service/ValueObject/ViewColumnInformation.php index 8ff1b3c295..6b2204b13f 100644 --- a/lib/Service/ValueObject/ViewColumnInformation.php +++ b/lib/Service/ValueObject/ViewColumnInformation.php @@ -42,9 +42,7 @@ public function isMandatory(): bool { } public static function fromArray(array $data): static { - if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) { - throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required'); - } + static::assertRequiredFields($data); return new static( (int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER], @@ -57,21 +55,17 @@ public static function fromArray(array $data): static { * @return array{columnId: int, order: int, readonly: bool, mandatory: bool} */ public function jsonSerialize(): array { - return [ - self::KEY_ID => $this->getId(), - self::KEY_ORDER => $this->getOrder(), + return array_merge(parent::jsonSerialize(), [ self::KEY_READONLY => $this->isReadonly(), self::KEY_MANDATORY => $this->isMandatory(), - ]; + ]); } protected function ensureType(string $offset, mixed $value): int|bool { return match ($offset) { - self::KEY_ID, - self::KEY_ORDER => (int)$value, - self::KEY_READONLY, + self::KEY_READONLY => (bool)$value, self::KEY_MANDATORY => (bool)$value, - default => throw new \InvalidArgumentException("Invalid offset: $offset"), + default => parent::ensureType($offset, $value), }; } } From ec960c8a5d3fd69b149fc287ecb84738dc1f3bc9 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 24 Apr 2026 16:41:55 +0200 Subject: [PATCH 10/11] fix(psalm): Using an explicit array literal with the parent's typed accessors (getId(), getOrder()) This gives Psalm the concrete shape it needs while still avoiding any real logic duplication. array_merge() always produces a widened array type in Psalm's eyes, so it can never satisfy a fixed-shape return type. AI-assistant: Copilot v1.0.6 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger --- lib/Service/ValueObject/ViewColumnInformation.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Service/ValueObject/ViewColumnInformation.php b/lib/Service/ValueObject/ViewColumnInformation.php index 6b2204b13f..3d263c91a5 100644 --- a/lib/Service/ValueObject/ViewColumnInformation.php +++ b/lib/Service/ValueObject/ViewColumnInformation.php @@ -55,10 +55,12 @@ public static function fromArray(array $data): static { * @return array{columnId: int, order: int, readonly: bool, mandatory: bool} */ public function jsonSerialize(): array { - return array_merge(parent::jsonSerialize(), [ + return [ + self::KEY_ID => $this->getId(), + self::KEY_ORDER => $this->getOrder(), self::KEY_READONLY => $this->isReadonly(), self::KEY_MANDATORY => $this->isMandatory(), - ]); + ]; } protected function ensureType(string $offset, mixed $value): int|bool { From f036c0db4367d9cc6d63537aa59136ec7940983f Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 24 Apr 2026 16:43:12 +0200 Subject: [PATCH 11/11] fix: remove isset() - already verified, hence the first isset() is not necessary anymore Signed-off-by: Andy Scherzinger --- lib/Controller/ApiTablesController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 1d996cc5f8..7510929b37 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -190,7 +190,7 @@ public function createFromScheme(string $title, string $emoji, string $descripti $remappedColumnOrder = null; if (!empty($columnOrder)) { $remappedColumnOrder = ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { - if (isset($entry['columnId']) && $entry['columnId'] > 0) { + if ($entry['columnId'] > 0) { $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; } return $entry; @@ -199,7 +199,7 @@ public function createFromScheme(string $title, string $emoji, string $descripti $remappedSort = null; if (!empty($sort)) { $remappedSort = SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array { - if (isset($entry['columnId']) && $entry['columnId'] > 0) { + if ($entry['columnId'] > 0) { $entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId']; } return $entry;