Skip to content

Commit b94ad8d

Browse files
AndyScherzingerAndyScherzinger
committed
fix(tables): address review comments
AI-assistant: Claude Code v2.1.117 (Claude Sonnet 4.6) Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
1 parent 5732e49 commit b94ad8d

12 files changed

Lines changed: 206 additions & 94 deletions

‎lib/Controller/ApiTablesController.php‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -187,18 +187,18 @@ public function createFromScheme(string $title, string $emoji, string $descripti
187187
$colMap[$column['id']] = $col->getId();
188188
}
189189
if (!empty($columnOrder) || !empty($sort)) {
190-
$remappedColumnOrder = !empty($columnOrder) ? array_map(static function (array $entry) use ($colMap): array {
190+
$remappedColumnOrder = !empty($columnOrder) ? ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
191191
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
192192
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
193193
}
194194
return $entry;
195-
}, $columnOrder) : null;
196-
$remappedSort = !empty($sort) ? array_map(static function (array $entry) use ($colMap): array {
195+
}, $columnOrder)) : null;
196+
$remappedSort = !empty($sort) ? SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
197197
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
198198
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
199199
}
200200
return $entry;
201-
}, $sort) : null;
201+
}, $sort)) : null;
202202
$table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort);
203203
}
204204
foreach ($views as $view) {
@@ -315,23 +315,19 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s
315315
$sort = json_decode($sort, true) ?? null;
316316
}
317317
try {
318-
if ($columnSettings !== null) {
319-
if (!is_array($columnSettings)) {
320-
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
321-
}
322-
ColumnSettings::createFromInputArray($columnSettings);
318+
if ($columnSettings !== null && !is_array($columnSettings)) {
319+
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
323320
}
324-
if ($sort !== null) {
325-
if (!is_array($sort)) {
326-
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
327-
}
328-
SortRuleSet::createFromInputArray($sort);
321+
if ($sort !== null && !is_array($sort)) {
322+
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
329323
}
324+
$columnSettingsObj = $columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null;
325+
$sortObj = $sort !== null ? SortRuleSet::createFromInputArray($sort) : null;
330326
} catch (\InvalidArgumentException $e) {
331327
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
332328
}
333329
try {
334-
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize());
330+
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettingsObj, $sortObj)->jsonSerialize());
335331
} catch (PermissionError $e) {
336332
return $this->handlePermissionError($e);
337333
} catch (\InvalidArgumentException $e) {

‎lib/Controller/TableController.php‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,17 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b
8080
$sort = json_decode($sort, true) ?? null;
8181
}
8282
return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) {
83-
if ($columnSettings !== null) {
84-
if (!is_array($columnSettings)) {
85-
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
86-
}
87-
ColumnSettings::createFromInputArray($columnSettings);
83+
if ($columnSettings !== null && !is_array($columnSettings)) {
84+
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
8885
}
89-
if ($sort !== null) {
90-
if (!is_array($sort)) {
91-
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
92-
}
93-
SortRuleSet::createFromInputArray($sort);
86+
if ($sort !== null && !is_array($sort)) {
87+
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
9488
}
95-
return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort);
89+
return $this->service->update(
90+
$id, $title, $emoji, null, $archived, $this->userId,
91+
$columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null,
92+
$sort !== null ? SortRuleSet::createFromInputArray($sort) : null,
93+
);
9694
});
9795
}
9896
}

‎lib/Db/Table.php‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use OCA\Tables\Model\Permissions;
1212
use OCA\Tables\Model\SortRuleSet;
1313
use OCA\Tables\ResponseDefinitions;
14-
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
14+
use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
1515

1616
/**
1717
* @psalm-suppress PropertyNotSetInConstructor
@@ -44,7 +44,7 @@
4444
* @method setRowsCount(int $rowsCount)
4545
* @method getColumnsCount(): int
4646
* @method setColumnsCount(int $columnsCount)
47-
* @method getViews(): ?array
47+
* @method getViews(): ?array<TablesView>
4848
* @method setViews(array $views)
4949
* @method getColumns(): array
5050
* @method setColumns(array $columns)
@@ -126,27 +126,26 @@ private function getSharePermissions(): ?Permissions {
126126
}
127127

128128
/**
129-
* @psalm-suppress MismatchingDocblockReturnType
130129
* @return TablesView[]
131130
*/
132131
private function getViewsArray(): array {
133132
return $this->getViews() ?: [];
134133
}
135134

136135
/**
137-
* @psalm-suppress MismatchingDocblockReturnType
138136
* @return int[]
139137
*/
140138
public function getColumnOrderArray(): array {
141139
$columnSettings = $this->getColumnOrderSettingsArray();
142-
usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
140+
usort($columnSettings, static function (ColumnOrderInformation $a, ColumnOrderInformation $b) {
143141
return $a->getOrder() - $b->getOrder();
144142
});
145-
return array_map(static fn (ViewColumnInformation $vci): int => $vci->getId(), $columnSettings);
143+
/** @var list<ColumnOrderInformation> $columnSettings */
144+
return array_map(static fn (ColumnOrderInformation $vci): int => $vci->getId(), $columnSettings);
146145
}
147146

148147
/**
149-
* @return array<ViewColumnInformation>
148+
* @return list<ColumnOrderInformation>
150149
*/
151150
public function getColumnOrderSettingsArray(): array {
152151
$columns = $this->getArray($this->getColumnOrder());
@@ -155,18 +154,17 @@ public function getColumnOrderSettingsArray(): array {
155154
}
156155

157156
if (is_array($columns[array_key_first($columns)] ?? null)) {
158-
return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns));
157+
return array_values(array_map(static fn (array $a): ColumnOrderInformation => ColumnOrderInformation::fromArray($a), $columns));
159158
}
160159

161160
$result = [];
162161
foreach ($columns as $index => $columnId) {
163-
$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
162+
$result[] = new ColumnOrderInformation((int)$columnId, order: (int)$index + 1);
164163
}
165164
return $result;
166165
}
167166

168167
/**
169-
* @psalm-suppress MismatchingDocblockReturnType
170168
* @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
171169
*/
172170
public function getSortArray(): array {
@@ -176,9 +174,8 @@ public function getSortArray(): array {
176174

177175
private function getArray(?string $json): array {
178176
if ($json !== '' && $json !== null && $json !== 'null') {
179-
return \json_decode($json, true);
180-
} else {
181-
return [];
177+
return \json_decode($json, true) ?? [];
182178
}
179+
return [];
183180
}
184181
}

‎lib/Model/ColumnSettings.php‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,19 @@
1111

1212
use Generator;
1313
use JsonSerializable;
14+
use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
1415
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
1516

1617
class ColumnSettings implements JsonSerializable {
1718
/**
18-
* @param ViewColumnInformation[] $columnSettings
19+
* @param ColumnOrderInformation[] $columnSettings
1920
*/
2021
public function __construct(
2122
protected array $columnSettings,
2223
) {
2324
foreach ($this->columnSettings as $columnSetting) {
24-
if (!$columnSetting instanceof ViewColumnInformation) {
25-
throw new \InvalidArgumentException('Provided column settings must be of type ViewColumnInformation');
25+
if (!$columnSetting instanceof ColumnOrderInformation) {
26+
throw new \InvalidArgumentException('Provided column settings must be of type ColumnOrderInformation');
2627
}
2728
}
2829
}
@@ -33,7 +34,24 @@ public function columnInformation(): Generator {
3334
}
3435
}
3536

37+
/**
38+
* Creates column settings with only columnId and order (for table use).
39+
*/
3640
public static function createFromInputArray(array $inputColumnSettings): self {
41+
$columnSettings = [];
42+
foreach ($inputColumnSettings as $inputColumnSetting) {
43+
if (!is_array($inputColumnSetting)) {
44+
throw new \InvalidArgumentException('Each column settings entry must be an array');
45+
}
46+
$columnSettings[] = ColumnOrderInformation::fromArray($inputColumnSetting);
47+
}
48+
return new self($columnSettings);
49+
}
50+
51+
/**
52+
* Creates column settings with view-specific fields (columnId, order, readonly, mandatory).
53+
*/
54+
public static function createViewSettingsFromInputArray(array $inputColumnSettings): self {
3755
$columnSettings = [];
3856
foreach ($inputColumnSettings as $inputColumnSetting) {
3957
if (!is_array($inputColumnSetting)) {
@@ -45,6 +63,6 @@ public static function createFromInputArray(array $inputColumnSettings): self {
4563
}
4664

4765
public function jsonSerialize(): array {
48-
return array_map(static fn (ViewColumnInformation $vci) => $vci->jsonSerialize(), $this->columnSettings);
66+
return array_map(static fn (ColumnOrderInformation $vci) => $vci->jsonSerialize(), $this->columnSettings);
4967
}
5068
}

‎lib/Model/SortRuleSet.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ public static function createFromInputArray(array $data): self {
5050
return new self($sortRules);
5151
}
5252

53+
/**
54+
* @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
55+
*/
5356
public function jsonSerialize(): array {
5457
return array_map(static fn (SortRule $s) => $s->jsonSerialize(), $this->sortRules);
5558
}

‎lib/Model/ViewUpdateInput.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public static function fromInputArray(array $data): self {
8484
title: ($data['title'] ?? null) ? new Title($data['title']) : null,
8585
description: $data['description'] ?? null,
8686
emoji: ($data['emoji'] ?? null) ? new Emoji($data['emoji']) : null,
87-
columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createFromInputArray($data['columnSettings']) : null,
87+
columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createViewSettingsFromInputArray($data['columnSettings']) : null,
8888
filterSet: ($data['filter'] ?? null) ? FilterSet::createFromInputArray($data['filter']) : null,
8989
sortRuleSet: ($data['sort'] ?? null) ? SortRuleSet::createFromInputArray($data['sort']) : null,
9090
);

‎lib/Service/TableService.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ public function delete(int $id, ?string $userId = null): Table {
476476
* @throws NotFoundError
477477
* @throws PermissionError
478478
*/
479-
public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?array $columnSettings = null, ?array $sort = null): Table {
479+
public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?ColumnSettings $columnSettings = null, ?SortRuleSet $sort = null): Table {
480480
$userId = $this->permissionsService->preCheckUserId($userId);
481481

482482
try {
@@ -509,10 +509,10 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip
509509
$table->setDescription($description);
510510
}
511511
if ($columnSettings !== null) {
512-
$table->setColumnOrder(\json_encode(ColumnSettings::createFromInputArray($columnSettings)->jsonSerialize()));
512+
$table->setColumnOrder(\json_encode($columnSettings->jsonSerialize()));
513513
}
514514
if ($sort !== null) {
515-
$table->setSort(\json_encode(SortRuleSet::createFromInputArray($sort)->jsonSerialize()));
515+
$table->setSort(\json_encode($sort->jsonSerialize()));
516516
}
517517
$table->setLastEditBy($userId);
518518
$table->setLastEditAt($time->format('Y-m-d H:i:s'));

‎lib/Service/ValueObject/ColumnOrderInformation.php‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Tables\Service\ValueObject;
10+
11+
use ArrayAccess;
12+
use JsonSerializable;
13+
14+
/**
15+
* Value object representing the column id and display order for a table.
16+
* This is the base type for table column ordering; ViewColumnInformation
17+
* extends it with view-specific fields (readonly, mandatory).
18+
*
19+
* @template-implements ArrayAccess<string, bool|int>
20+
*/
21+
class ColumnOrderInformation implements ArrayAccess, JsonSerializable {
22+
public const KEY_ID = 'columnId';
23+
public const KEY_ORDER = 'order';
24+
25+
/** @var array{columnId: int, order: int} */
26+
protected array $data;
27+
protected const KEYS = [
28+
self::KEY_ID,
29+
self::KEY_ORDER,
30+
];
31+
32+
public function __construct(int $columnId, int $order) {
33+
$this->offsetSet(self::KEY_ID, $columnId);
34+
$this->offsetSet(self::KEY_ORDER, $order);
35+
}
36+
37+
public function getId(): int {
38+
return (int)$this->offsetGet(self::KEY_ID);
39+
}
40+
41+
public function getOrder(): int {
42+
return (int)$this->offsetGet(self::KEY_ORDER);
43+
}
44+
45+
public static function fromArray(array $data): self {
46+
if (!isset($data[self::KEY_ID], $data[self::KEY_ORDER])) {
47+
throw new \InvalidArgumentException('Column settings entry is missing required fields: columnId and order are required');
48+
}
49+
return new self((int)$data[self::KEY_ID], (int)$data[self::KEY_ORDER]);
50+
}
51+
52+
public function offsetExists(mixed $offset): bool {
53+
return in_array((string)$offset, static::KEYS);
54+
}
55+
56+
public function offsetGet(mixed $offset): bool|int {
57+
return $this->data[$offset];
58+
}
59+
60+
public function offsetSet(mixed $offset, mixed $value): void {
61+
if (!$this->offsetExists($offset)) {
62+
return;
63+
}
64+
$this->data[$offset] = $this->ensureType((string)$offset, $value);
65+
}
66+
67+
public function offsetUnset(mixed $offset): void {
68+
if (!$this->offsetExists($offset)) {
69+
return;
70+
}
71+
unset($this->data[(string)$offset]);
72+
}
73+
74+
/**
75+
* @return array{columnId: int, order: int}
76+
*/
77+
public function jsonSerialize(): array {
78+
return $this->data;
79+
}
80+
81+
protected function ensureType(string $offset, mixed $value): int|bool {
82+
return match ($offset) {
83+
self::KEY_ID,
84+
self::KEY_ORDER => (int)$value,
85+
default => throw new \InvalidArgumentException("Invalid offset: $offset"),
86+
};
87+
}
88+
}

‎lib/Service/ValueObject/SortRule.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ public function __construct(
2222
}
2323
}
2424

25+
/**
26+
* @return array{columnId: int, mode: 'ASC'|'DESC'}
27+
*/
2528
public function jsonSerialize(): array {
2629
return [
2730
'columnId' => $this->columnId,

0 commit comments

Comments
 (0)