Skip to content

Commit 7ad43a4

Browse files
blizzzblizzz
committed
refactor: turn columnSettings data from loose array to value object
Signed-off-by: Arthur Schiwon <[email protected]>
1 parent 985f792 commit 7ad43a4

File tree

6 files changed

+118
-54
lines changed

6 files changed

+118
-54
lines changed

‎lib/BackgroundJob/ConvertViewColumnsFormat.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
namespace OCA\Tables\BackgroundJob;
99

10+
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
1011
use OCP\AppFramework\Utility\ITimeFactory;
1112
use OCP\BackgroundJob\IJobList;
1213
use OCP\BackgroundJob\TimedJob;
@@ -77,10 +78,7 @@ private function processView(array $view, \OCP\DB\QueryBuilder\IQueryBuilder $up
7778
// Create new columns structure
7879
$newColumns = [];
7980
foreach ($columns as $order => $columnId) {
80-
$newColumns[] = [
81-
'columnId' => (int)$columnId,
82-
'order' => $order,
83-
];
81+
$newColumns[] = new ViewColumnInformation((int)$columnId, order: $order);
8482
}
8583

8684
// Execute update query

‎lib/Db/View.php

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use JsonSerializable;
1111
use OCA\Tables\Model\Permissions;
1212
use OCA\Tables\ResponseDefinitions;
13+
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
1314

1415
/**
1516
* @psalm-suppress PropertyNotSetInConstructor
@@ -93,14 +94,14 @@ public function __construct() {
9394
public function getColumnsArray(): array {
9495

9596
$columnSettings = $this->getColumnsSettingsArray();
96-
usort($columnSettings, function ($a, $b) {
97-
return $a['order'] - $b['order'];
97+
usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
98+
return $a[ViewColumnInformation::KEY_ORDER] - $b[ViewColumnInformation::KEY_ORDER];
9899
});
99-
return array_column($columnSettings, 'columnId');
100+
return array_column($columnSettings, ViewColumnInformation::KEY_ID);
100101
}
101102

102103
/**
103-
* @return array<array{columnId: int, order: int}>
104+
* @return array<ViewColumnInformation>
104105
*/
105106
public function getColumnsSettingsArray(): array {
106107
$columns = $this->getArray($this->getColumns());
@@ -109,15 +110,12 @@ public function getColumnsSettingsArray(): array {
109110
}
110111

111112
if (is_array(reset($columns))) {
112-
return $columns;
113+
return array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns);
113114
}
114115

115116
$result = [];
116117
foreach ($columns as $index => $columnId) {
117-
$result[] = [
118-
'columnId' => $columnId,
119-
'order' => (int)$index + 1
120-
];
118+
$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
121119
}
122120
return $result;
123121
}
@@ -160,10 +158,6 @@ public function setColumnsArray(array $array):void {
160158
$this->setColumns(\json_encode($array));
161159
}
162160

163-
public function setColumnsSettingsArray(array $array): void {
164-
$this->setColumnSettings(\json_encode($array));
165-
}
166-
167161
public function setSortArray(array $array):void {
168162
$this->setSort(\json_encode($array));
169163
}
@@ -211,6 +205,7 @@ public function jsonSerialize(): array {
211205
* @return int[]
212206
*/
213207
public function getColumnIds(): array {
214-
return array_column($this->getColumnsSettingsArray(), 'columnId');
208+
$columns = $this->getColumnsSettingsArray();
209+
return array_map(static fn (ViewColumnInformation $column): int => $column[ViewColumnInformation::KEY_ID], $columns);
215210
}
216211
}

‎lib/Service/TableTemplateService.php

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCA\Tables\Errors\InternalError;
1414
use OCA\Tables\Errors\NotFoundError;
1515
use OCA\Tables\Errors\PermissionError;
16+
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
1617
use OCP\AppFramework\Db\DoesNotExistException;
1718
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
1819
use OCP\DB\Exception;
@@ -466,11 +467,8 @@ private function makeVacationRequests(Table $table):void {
466467
[
467468
'title' => $this->l->t('Create Vacation Request'),
468469
'emoji' => '️➕',
469-
'columnSettings' => json_encode(array_map(function ($columnId, $index) {
470-
return [
471-
'columnId' => $columnId,
472-
'order' => $index
473-
];
470+
'columnSettings' => json_encode(array_map(function (int $columnId, int $index): ViewColumnInformation {
471+
return new ViewColumnInformation($columnId, $index);
474472
}, [$columns['employee']->getId(), $columns['from']->getId(), $columns['to']->getId(), $columns['workingDays']->getId(), $columns['dateRequest']->getId()], array_keys([$columns['employee']->getId(), $columns['from']->getId(), $columns['to']->getId(), $columns['workingDays']->getId(), $columns['dateRequest']->getId()]))),
475473
'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_AT, 'mode' => 'ASC']]),
476474
'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name'], ['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]),
@@ -480,11 +478,8 @@ private function makeVacationRequests(Table $table):void {
480478
[
481479
'title' => $this->l->t('Open Request'),
482480
'emoji' => '️📝',
483-
'columnSettings' => json_encode(array_map(function ($column, $index) {
484-
return [
485-
'columnId' => $column->getId(),
486-
'order' => $index
487-
];
481+
'columnSettings' => json_encode(array_map(function (Column $column, int $index): ViewColumnInformation {
482+
return new ViewColumnInformation($column->getId(), $index);
488483
}, array_values($columns), array_keys(array_values($columns)))),
489484
'sort' => json_encode([['columnId' => $columns['from']->getId(), 'mode' => 'ASC']]),
490485
'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]),
@@ -494,11 +489,8 @@ private function makeVacationRequests(Table $table):void {
494489
[
495490
'title' => $this->l->t('Request Status'),
496491
'emoji' => '️❓',
497-
'columnSettings' => json_encode(array_map(function ($column, $index) {
498-
return [
499-
'columnId' => $column->getId(),
500-
'order' => $index
501-
];
492+
'columnSettings' => json_encode(array_map(function (Column $column, int $index): ViewColumnInformation {
493+
return new ViewColumnInformation($column->getId(), $index);
502494
}, array_values($columns), array_keys(array_values($columns)))),
503495
'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]),
504496
'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name']]]),
@@ -508,11 +500,8 @@ private function makeVacationRequests(Table $table):void {
508500
[
509501
'title' => $this->l->t('Closed requests'),
510502
'emoji' => '️✅',
511-
'columnSettings' => json_encode(array_map(function ($column, $index) {
512-
return [
513-
'columnId' => $column->getId(),
514-
'order' => $index
515-
];
503+
'columnSettings' => json_encode(array_map(function (Column $column, int $index): ViewColumnInformation {
504+
return new ViewColumnInformation($column->getId(), $index);
516505
}, array_values($columns), array_keys(array_values($columns)))),
517506
'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]),
518507
'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@checked']], [['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]),
@@ -797,7 +786,14 @@ private function makeStartupTable(Table $table):void {
797786
$this->createView($table, [
798787
'title' => $this->l->t('Check yourself!'),
799788
'emoji' => '🏁',
800-
'columnSettings' => json_encode([['columnId' => $columns['what']->getId(), 'order' => 0], ['columnId' => $columns['how']->getId(), 'order' => 1], ['columnId' => $columns['ease']->getId(), 'order' => 2], ['columnId' => $columns['done']->getId(), 'order' => 3]]),
789+
'columnSettings' => json_encode(
790+
[
791+
new ViewColumnInformation($columns['what']->getId(), order: 0),
792+
new ViewColumnInformation($columns['how']->getId(), order: 1),
793+
new ViewColumnInformation($columns['ease']->getId(), order: 2),
794+
new ViewColumnInformation($columns['done']->getId(), order: 3),
795+
]
796+
),
801797
'filter' => json_encode([[['columnId' => $columns['done']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]),
802798
]);
803799
}

‎lib/Service/ValueObject/ViewColumnInformation.php

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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+
* @template-implements ArrayAccess<string, mixed>
16+
*/
17+
class ViewColumnInformation implements ArrayAccess, JsonSerializable {
18+
public const KEY_ID = 'columnId';
19+
public const KEY_ORDER = 'order';
20+
21+
/** @var array{columndId?: int, order?: int} */
22+
protected array $data = [];
23+
protected const KEYS = [
24+
self::KEY_ID,
25+
self::KEY_ORDER,
26+
];
27+
28+
public function __construct(
29+
int $columnId,
30+
int $order,
31+
) {
32+
$this->offsetSet(self::KEY_ID, $columnId);
33+
$this->offsetSet(self::KEY_ORDER, $order);
34+
}
35+
36+
public static function fromArray(array $data): static {
37+
$vci = new static($data[self::KEY_ID], $data[self::KEY_ORDER]);
38+
foreach ($data as $key => $value) {
39+
$vci[$key] = $value;
40+
}
41+
return $vci;
42+
}
43+
44+
public function offsetExists(mixed $offset): bool {
45+
return in_array((string)$offset, self::KEYS);
46+
}
47+
48+
public function offsetGet(mixed $offset): mixed {
49+
return $this->data[$offset] ?? null;
50+
}
51+
52+
public function offsetSet(mixed $offset, mixed $value): void {
53+
if (!$this->offsetExists($offset)) {
54+
return;
55+
}
56+
$this->data[(string)$offset] = $value;
57+
}
58+
59+
public function offsetUnset(mixed $offset): void {
60+
if (!$this->offsetExists($offset)) {
61+
return;
62+
}
63+
unset($this->data[(string)$offset]);
64+
}
65+
66+
public function jsonSerialize(): array {
67+
return $this->data;
68+
}
69+
}

‎lib/Service/ViewService.php

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCA\Tables\Helper\UserHelper;
2525
use OCA\Tables\Model\Permissions;
2626
use OCA\Tables\ResponseDefinitions;
27+
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
2728
use OCP\AppFramework\Db\DoesNotExistException;
2829
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
2930
use OCP\EventDispatcher\IEventDispatcher;
@@ -272,11 +273,8 @@ public function update(int $id, array $data, ?string $userId = null, bool $skipT
272273
$this->logger->info('The old columns format is deprecated. Please use the new format with columnId and order properties.');
273274
$decodedValue = \json_decode($value, true);
274275
$value = [];
275-
foreach ($decodedValue as $order => $id) {
276-
$value[] = [
277-
'columnId' => $id,
278-
'order' => $order
279-
];
276+
foreach ($decodedValue as $order => $columnId) {
277+
$value[] = new ViewColumnInformation($columnId, order: $order);
280278
}
281279

282280
$value = \json_encode($value);
@@ -285,7 +283,8 @@ public function update(int $id, array $data, ?string $userId = null, bool $skipT
285283
if ($key === 'columnSettings' || $key === 'columns') {
286284
// we have to fetch the service here as ColumnService already depends on the ViewService, i.e. no DI
287285
$columnService = \OCP\Server::get(ColumnService::class);
288-
$columnIds = array_column(\json_decode($value, true), 'columnId');
286+
$rawColumnsArray = \json_decode($value, true);
287+
$columnIds = array_column($rawColumnsArray, ViewColumnInformation::KEY_ID);
289288

290289
$availableColumns = $columnService->findAllByManagedView($view, $userId);
291290
$availableColumns = array_map(static fn (Column $column) => $column->getId(), $availableColumns);
@@ -295,6 +294,14 @@ public function update(int $id, array $data, ?string $userId = null, bool $skipT
295294
}
296295
}
297296

297+
// ensure we have the correct format and expected values
298+
try {
299+
$columnsArray = array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $rawColumnsArray);
300+
$value = \json_encode($columnsArray);
301+
} catch (\Throwable $t) {
302+
throw new \InvalidArgumentException('Invalid column data provided', 400, $t);
303+
}
304+
298305
$key = 'columns';
299306
}
300307

@@ -543,8 +550,8 @@ function (array $filter) use ($columnId) {
543550
);
544551

545552
$columnSettings = $view->getColumnsSettingsArray();
546-
$columnSettings = array_filter($columnSettings, function (array $setting) use ($columnId): bool {
547-
return $setting['columnId'] !== $columnId;
553+
$columnSettings = array_filter($columnSettings, static function (ViewColumnInformation $setting) use ($columnId): bool {
554+
return $setting[ViewColumnInformation::KEY_ID] !== $columnId;
548555
});
549556
$columnSettings = array_values($columnSettings);
550557

@@ -587,16 +594,12 @@ public function search(string $term, int $limit = 100, int $offset = 0, ?string
587594
* @param string|null $userId The user ID performing the action
588595
* @return void
589596
* @throws InternalError
590-
* @throws PermissionError
591597
*/
592598
public function addColumnToView(View $view, Column $column, ?string $userId = null): void {
593599
try {
594600
$columnsSettings = $view->getColumnsSettingsArray();
595-
$nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, 'order')) + 1;
596-
$columnsSettings[] = [
597-
'columnId' => $column->getId(),
598-
'order' => $nextOrder
599-
];
601+
$nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, ViewColumnInformation::KEY_ORDER)) + 1;
602+
$columnsSettings[] = new ViewColumnInformation($column->getId(), $nextOrder);
600603
$this->update($view->getId(), ['columnSettings' => json_encode($columnsSettings)], $userId, true);
601604
} catch (Exception $e) {
602605
$this->logger->error($e->getMessage(), ['exception' => $e]);

‎tests/integration/features/bootstrap/FeatureContext.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,10 @@ public function applyColumnsToView(string $user, string $columnList, string $vie
900900
$columns = explode(',', $columnList);
901901
$columnSettings = array_map(function (string $columnAlias, int $index) {
902902
if (is_numeric($columnAlias)) {
903-
return (int)$columnAlias;
903+
return [
904+
'columnId' => (int)$columnAlias,
905+
'order' => $index
906+
];
904907
}
905908

906909
$col = $this->collectionManager->getByAlias('column', $columnAlias);

0 commit comments

Comments
 (0)