diff --git a/lib/BackgroundJob/ConvertViewColumnsFormat.php b/lib/BackgroundJob/ConvertViewColumnsFormat.php index eff22eee..e06adbdc 100644 --- a/lib/BackgroundJob/ConvertViewColumnsFormat.php +++ b/lib/BackgroundJob/ConvertViewColumnsFormat.php @@ -7,6 +7,7 @@ declare(strict_types=1); */ namespace OCA\Tables\BackgroundJob; +use OCA\Tables\Service\ValueObject\ViewColumnInformation; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\TimedJob; @@ -77,10 +78,7 @@ class ConvertViewColumnsFormat extends TimedJob { // Create new columns structure $newColumns = []; foreach ($columns as $order => $columnId) { - $newColumns[] = [ - 'columnId' => (int)$columnId, - 'order' => $order, - ]; + $newColumns[] = new ViewColumnInformation((int)$columnId, order: $order); } // Execute update query diff --git a/lib/Db/View.php b/lib/Db/View.php index 1a73c084..e8b4de16 100644 --- a/lib/Db/View.php +++ b/lib/Db/View.php @@ -1,5 +1,7 @@ getColumnsSettingsArray(); - usort($columnSettings, function ($a, $b) { - return $a['order'] - $b['order']; + usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) { + return $a->getOrder() - $b->getOrder(); }); - return array_column($columnSettings, 'columnId'); + return array_column($columnSettings, ViewColumnInformation::KEY_ID); } /** - * @return array + * @return array */ public function getColumnsSettingsArray(): array { $columns = $this->getArray($this->getColumns()); @@ -109,15 +112,12 @@ class View extends EntitySuper implements JsonSerializable { } if (is_array(reset($columns))) { - return $columns; + return array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns); } $result = []; foreach ($columns as $index => $columnId) { - $result[] = [ - 'columnId' => $columnId, - 'order' => (int)$index + 1 - ]; + $result[] = new ViewColumnInformation($columnId, order: (int)$index + 1); } return $result; } @@ -160,10 +160,6 @@ class View extends EntitySuper implements JsonSerializable { $this->setColumns(\json_encode($array)); } - public function setColumnsSettingsArray(array $array): void { - $this->setColumnSettings(\json_encode($array)); - } - public function setSortArray(array $array):void { $this->setSort(\json_encode($array)); } @@ -211,6 +207,7 @@ class View extends EntitySuper implements JsonSerializable { * @return int[] */ public function getColumnIds(): array { - return array_column($this->getColumnsSettingsArray(), 'columnId'); + $columns = $this->getColumnsSettingsArray(); + return array_map(static fn (ViewColumnInformation $column): int => $column[ViewColumnInformation::KEY_ID], $columns); } } diff --git a/lib/Service/TableTemplateService.php b/lib/Service/TableTemplateService.php index c613be7c..2bff78e6 100644 --- a/lib/Service/TableTemplateService.php +++ b/lib/Service/TableTemplateService.php @@ -1,5 +1,7 @@ $this->l->t('Create Vacation Request'), 'emoji' => '️➕', - 'columnSettings' => json_encode(array_map(function ($columnId, $index) { - return [ - 'columnId' => $columnId, - 'order' => $index - ]; - }, [$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()]))), - 'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_AT, 'mode' => 'ASC']]), + 'columnSettings' => $this->columnsToColumnSettingsJsonString($columns), 'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name'], ['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]), ] ); @@ -480,12 +477,7 @@ class TableTemplateService { [ 'title' => $this->l->t('Open Request'), 'emoji' => '️📝', - 'columnSettings' => json_encode(array_map(function ($column, $index) { - return [ - 'columnId' => $column->getId(), - 'order' => $index - ]; - }, array_values($columns), array_keys(array_values($columns)))), + 'columnSettings' => $this->columnsToColumnSettingsJsonString($columns), 'sort' => json_encode([['columnId' => $columns['from']->getId(), 'mode' => 'ASC']]), 'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]), ] @@ -494,12 +486,7 @@ class TableTemplateService { [ 'title' => $this->l->t('Request Status'), 'emoji' => '️❓', - 'columnSettings' => json_encode(array_map(function ($column, $index) { - return [ - 'columnId' => $column->getId(), - 'order' => $index - ]; - }, array_values($columns), array_keys(array_values($columns)))), + 'columnSettings' => $this->columnsToColumnSettingsJsonString($columns), 'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]), 'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name']]]), ] @@ -508,18 +495,30 @@ class TableTemplateService { [ 'title' => $this->l->t('Closed requests'), 'emoji' => '️✅', - 'columnSettings' => json_encode(array_map(function ($column, $index) { - return [ - 'columnId' => $column->getId(), - 'order' => $index - ]; - }, array_values($columns), array_keys(array_values($columns)))), + 'columnSettings' => $this->columnsToColumnSettingsJsonString($columns), 'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]), 'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@checked']], [['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]), ] ); } + /** + * @param array $columns + */ + private function columnsToColumnSettingsJsonString(array $columns): string { + $columns = array_filter($columns, static function ($item) { + return $item instanceof Column; + }); + return json_encode( + array_map( + static function (Column $column, int $index): ViewColumnInformation { + return new ViewColumnInformation($column->getId(), order: $index); + }, + array_values($columns), array_keys(array_values($columns)) + ) + ); + } + /** * @psalm-suppress PossiblyNullReference * @param Table $table @@ -797,7 +796,14 @@ class TableTemplateService { $this->createView($table, [ 'title' => $this->l->t('Check yourself!'), 'emoji' => '🏁', - '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]]), + 'columnSettings' => json_encode( + [ + new ViewColumnInformation($columns['what']->getId(), order: 0), + new ViewColumnInformation($columns['how']->getId(), order: 1), + new ViewColumnInformation($columns['ease']->getId(), order: 2), + new ViewColumnInformation($columns['done']->getId(), order: 3), + ] + ), 'filter' => json_encode([[['columnId' => $columns['done']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]), ]); } diff --git a/lib/Service/ValueObject/ViewColumnInformation.php b/lib/Service/ValueObject/ViewColumnInformation.php new file mode 100644 index 00000000..403877da --- /dev/null +++ b/lib/Service/ValueObject/ViewColumnInformation.php @@ -0,0 +1,73 @@ + + */ +class ViewColumnInformation implements ArrayAccess, JsonSerializable { + public const KEY_ID = 'columnId'; + public const KEY_ORDER = 'order'; + + /** @var array{columndId?: int, order?: int} */ + 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 getOrder(): int { + return $this->offsetGet(self::KEY_ORDER); + } + + public static function fromArray(array $data): static { + $vci = new static($data[self::KEY_ID], $data[self::KEY_ORDER]); + foreach ($data as $key => $value) { + $vci[$key] = $value; + } + return $vci; + } + + public function offsetExists(mixed $offset): bool { + return in_array((string)$offset, self::KEYS); + } + + public function offsetGet(mixed $offset): mixed { + return $this->data[$offset] ?? null; + } + + public function offsetSet(mixed $offset, mixed $value): void { + if (!$this->offsetExists($offset)) { + return; + } + $this->data[(string)$offset] = $value; + } + + public function offsetUnset(mixed $offset): void { + if (!$this->offsetExists($offset)) { + return; + } + unset($this->data[(string)$offset]); + } + + public function jsonSerialize(): array { + return $this->data; + } +} diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 792890ea..d9ae7bb6 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -24,6 +24,7 @@ use OCA\Tables\Event\ViewDeletedEvent; use OCA\Tables\Helper\UserHelper; use OCA\Tables\Model\Permissions; use OCA\Tables\ResponseDefinitions; +use OCA\Tables\Service\ValueObject\ViewColumnInformation; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\EventDispatcher\IEventDispatcher; @@ -272,11 +273,8 @@ class ViewService extends SuperService { $this->logger->info('The old columns format is deprecated. Please use the new format with columnId and order properties.'); $decodedValue = \json_decode($value, true); $value = []; - foreach ($decodedValue as $order => $id) { - $value[] = [ - 'columnId' => $id, - 'order' => $order - ]; + foreach ($decodedValue as $order => $columnId) { + $value[] = new ViewColumnInformation($columnId, order: $order); } $value = \json_encode($value); @@ -285,7 +283,8 @@ class ViewService extends SuperService { if ($key === 'columnSettings' || $key === 'columns') { // we have to fetch the service here as ColumnService already depends on the ViewService, i.e. no DI $columnService = \OCP\Server::get(ColumnService::class); - $columnIds = array_column(\json_decode($value, true), 'columnId'); + $rawColumnsArray = \json_decode($value, true); + $columnIds = array_column($rawColumnsArray, ViewColumnInformation::KEY_ID); $availableColumns = $columnService->findAllByManagedView($view, $userId); $availableColumns = array_map(static fn (Column $column) => $column->getId(), $availableColumns); @@ -295,6 +294,14 @@ class ViewService extends SuperService { } } + // ensure we have the correct format and expected values + try { + $columnsArray = array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $rawColumnsArray); + $value = \json_encode($columnsArray); + } catch (\Throwable $t) { + throw new \InvalidArgumentException('Invalid column data provided', 400, $t); + } + $key = 'columns'; } @@ -543,8 +550,8 @@ class ViewService extends SuperService { ); $columnSettings = $view->getColumnsSettingsArray(); - $columnSettings = array_filter($columnSettings, function (array $setting) use ($columnId): bool { - return $setting['columnId'] !== $columnId; + $columnSettings = array_filter($columnSettings, static function (ViewColumnInformation $setting) use ($columnId): bool { + return $setting[ViewColumnInformation::KEY_ID] !== $columnId; }); $columnSettings = array_values($columnSettings); @@ -587,16 +594,12 @@ class ViewService extends SuperService { * @param string|null $userId The user ID performing the action * @return void * @throws InternalError - * @throws PermissionError */ public function addColumnToView(View $view, Column $column, ?string $userId = null): void { try { $columnsSettings = $view->getColumnsSettingsArray(); - $nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, 'order')) + 1; - $columnsSettings[] = [ - 'columnId' => $column->getId(), - 'order' => $nextOrder - ]; + $nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, ViewColumnInformation::KEY_ORDER)) + 1; + $columnsSettings[] = new ViewColumnInformation($column->getId(), $nextOrder); $this->update($view->getId(), ['columnSettings' => json_encode($columnsSettings)], $userId, true); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index c2387023..bee2553c 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -900,7 +900,10 @@ class FeatureContext implements Context { $columns = explode(',', $columnList); $columnSettings = array_map(function (string $columnAlias, int $index) { if (is_numeric($columnAlias)) { - return (int)$columnAlias; + return [ + 'columnId' => (int)$columnAlias, + 'order' => $index + ]; } $col = $this->collectionManager->getByAlias('column', $columnAlias);