diff --git a/lib/Db/LegacyRowMapper.php b/lib/Db/LegacyRowMapper.php index 7e2c35182..02caa88cd 100644 --- a/lib/Db/LegacyRowMapper.php +++ b/lib/Db/LegacyRowMapper.php @@ -14,6 +14,7 @@ use OCA\Tables\Db\ColumnTypes\SuperColumnQB; use OCA\Tables\Db\ColumnTypes\TextColumnQB; use OCA\Tables\Errors\InternalError; +use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Helper\UserHelper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -30,39 +31,23 @@ /** @template-extends QBMapper */ class LegacyRowMapper extends QBMapper { protected string $table = 'tables_rows'; - protected TextColumnQB $textColumnQB; - protected SelectionColumnQB $selectionColumnQB; - protected NumberColumnQB $numberColumnQB; - protected DatetimeColumnQB $datetimeColumnQB; - protected SuperColumnQB $genericColumnQB; - protected ColumnMapper $columnMapper; - protected LoggerInterface $logger; - protected UserHelper $userHelper; - protected Row2Mapper $rowMapper; protected int $platform; public function __construct( IDBConnection $db, - LoggerInterface $logger, - TextColumnQB $textColumnQB, - SelectionColumnQB $selectionColumnQB, - NumberColumnQB $numberColumnQB, - DatetimeColumnQB $datetimeColumnQB, - SuperColumnQB $columnQB, - ColumnMapper $columnMapper, - UserHelper $userHelper, - Row2Mapper $rowMapper) { + private LoggerInterface $logger, + private TextColumnQB $textColumnQB, + private SelectionColumnQB $selectionColumnQB, + private NumberColumnQB $numberColumnQB, + private DatetimeColumnQB $datetimeColumnQB, + private SuperColumnQB $genericColumnQB, + private ColumnMapper $columnMapper, + private ColumnsHelper $columnsHelper, + private UserHelper $userHelper, + private Row2Mapper $rowMapper, + ) { parent::__construct($db, $this->table, LegacyRow::class); - $this->logger = $logger; - $this->textColumnQB = $textColumnQB; - $this->numberColumnQB = $numberColumnQB; - $this->selectionColumnQB = $selectionColumnQB; - $this->datetimeColumnQB = $datetimeColumnQB; - $this->genericColumnQB = $columnQB; - $this->columnMapper = $columnMapper; - $this->userHelper = $userHelper; - $this->rowMapper = $rowMapper; $this->setPlatform(); } @@ -134,31 +119,6 @@ private function getFilterGroups(IQueryBuilder $qb, array $filters): array { return $filterGroups; } - private function resolveSearchValue(string $unresolvedSearchValue, string $userId): string { - switch (ltrim($unresolvedSearchValue, '@')) { - case 'me': return $userId; - case 'my-name': return $this->userHelper->getUserDisplayName($userId); - case 'checked': return 'true'; - case 'unchecked': return 'false'; - case 'stars-0': return '0'; - case 'stars-1': return '1'; - case 'stars-2': return '2'; - case 'stars-3': return '3'; - case 'stars-4': return '4'; - case 'stars-5': return '5'; - case 'datetime-date-today': return date('Y-m-d') ? date('Y-m-d') : ''; - case 'datetime-date-start-of-year': return date('Y-01-01') ? date('Y-01-01') : ''; - case 'datetime-date-start-of-month': return date('Y-m-01') ? date('Y-m-01') : ''; - case 'datetime-date-start-of-week': - $day = date('w'); - $result = date('m-d-Y', strtotime('-'.$day.' days')); - return $result ?: ''; - case 'datetime-time-now': return date('H:i'); - case 'datetime-now': return date('Y-m-d H:i') ? date('Y-m-d H:i') : ''; - default: return $unresolvedSearchValue; - } - } - /** * @param (int|string)[][] $sortArray * @@ -262,7 +222,7 @@ private function addFilterToQuery(IQueryBuilder $qb, View $view, array $neededCo $filter['columnType'] = $neededColumnTypes[$filter['columnId']]; // TODO move resolution for magic fields to service layer if(str_starts_with((string) $filter['value'], '@')) { - $filter['value'] = $this->resolveSearchValue((string) $filter['value'], $userId); + $filter['value'] = $this->columnsHelper->resolveSearchValue((string) $filter['value'], $userId); } } } diff --git a/lib/Db/Row2.php b/lib/Db/Row2.php index b64a6abb0..f9394aefa 100644 --- a/lib/Db/Row2.php +++ b/lib/Db/Row2.php @@ -9,6 +9,7 @@ use JsonSerializable; use OCA\Tables\Model\Public\Row; +use OCA\Tables\Model\RowDataInput; use OCA\Tables\ResponseDefinitions; /** @@ -73,10 +74,10 @@ public function getData(): ?array { } /** - * @param list $data + * @param RowDataInput|list $data * @return void */ - public function setData(array $data): void { + public function setData(array|RowDataInput $data): void { foreach ($data as $cell) { $this->insertOrUpdateCell($cell); } diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index a3a80ec09..1bdea71b8 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -267,7 +267,7 @@ private function replacePlaceholderValues(array &$filters, string $userId): void foreach ($filters as &$filterGroup) { foreach ($filterGroup as &$filter) { if (substr($filter['value'], 0, 1) === '@') { - $filter['value'] = $this->resolveSearchValue($filter['value'], $userId); + $filter['value'] = $this->columnsHelper->resolveSearchValue($filter['value'], $userId); } } } @@ -476,35 +476,6 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col } } - /** @noinspection DuplicatedCode */ - private function resolveSearchValue(string $placeholder, string $userId): string { - if (substr($placeholder, 0, 14) === '@selection-id-') { - return substr($placeholder, 14); - } - switch (ltrim($placeholder, '@')) { - case 'me': return $userId; - case 'my-name': return $this->userHelper->getUserDisplayName($userId); - case 'checked': return 'true'; - case 'unchecked': return 'false'; - case 'stars-0': return '0'; - case 'stars-1': return '1'; - case 'stars-2': return '2'; - case 'stars-3': return '3'; - case 'stars-4': return '4'; - case 'stars-5': return '5'; - case 'datetime-date-today': return date('Y-m-d') ? date('Y-m-d') : ''; - case 'datetime-date-start-of-year': return date('Y-01-01') ? date('Y-01-01') : ''; - case 'datetime-date-start-of-month': return date('Y-m-01') ? date('Y-m-01') : ''; - case 'datetime-date-start-of-week': - $day = date('w'); - $result = date('m-d-Y', strtotime('-'.$day.' days')); - return $result ?: ''; - case 'datetime-time-now': return date('H:i'); - case 'datetime-now': return date('Y-m-d H:i') ? date('Y-m-d H:i') : ''; - default: return $placeholder; - } - } - /** * @param IResult $result * @param RowSleeve[] $sleeves diff --git a/lib/Helper/ColumnsHelper.php b/lib/Helper/ColumnsHelper.php index ccc637b27..f7b74aff8 100644 --- a/lib/Helper/ColumnsHelper.php +++ b/lib/Helper/ColumnsHelper.php @@ -7,13 +7,48 @@ namespace OCA\Tables\Helper; +use OCA\Tables\Db\Column; + class ColumnsHelper { public array $columns = [ - 'text', - 'number', - 'datetime', - 'selection', - 'usergroup' + Column::TYPE_TEXT, + Column::TYPE_NUMBER, + Column::TYPE_DATETIME, + Column::TYPE_SELECTION, + Column::TYPE_USERGROUP, ]; + + public function __construct( + private UserHelper $userHelper, + ) { + } + + public function resolveSearchValue(string $placeholder, string $userId): string { + if (str_starts_with($placeholder, '@selection-id-')) { + return substr($placeholder, 14); + } + switch (ltrim($placeholder, '@')) { + case 'me': return $userId; + case 'my-name': return $this->userHelper->getUserDisplayName($userId); + case 'checked': return 'true'; + case 'unchecked': return 'false'; + case 'stars-0': return '0'; + case 'stars-1': return '1'; + case 'stars-2': return '2'; + case 'stars-3': return '3'; + case 'stars-4': return '4'; + case 'stars-5': return '5'; + case 'datetime-date-today': return date('Y-m-d') ? date('Y-m-d') : ''; + case 'datetime-date-start-of-year': return date('Y-01-01') ? date('Y-01-01') : ''; + case 'datetime-date-start-of-month': return date('Y-m-01') ? date('Y-m-01') : ''; + case 'datetime-date-start-of-week': + $day = date('w'); + $result = date('m-d-Y', strtotime('-'.$day.' days')); + return $result ?: ''; + case 'datetime-time-now': return date('H:i'); + case 'datetime-now': return date('Y-m-d H:i') ? date('Y-m-d H:i') : ''; + default: return $placeholder; + } + } } diff --git a/lib/Model/RowDataInput.php b/lib/Model/RowDataInput.php index 137bf3e2a..56d145ea5 100644 --- a/lib/Model/RowDataInput.php +++ b/lib/Model/RowDataInput.php @@ -71,4 +71,12 @@ public function valid(): bool { public function rewind(): void { reset($this->data); } + + public static function fromArray(array $data): self { + $newRowData = new RowDataInput(); + foreach ($data as $value) { + $newRowData->add((int)$value['columnId'], $value['value']); + } + return $newRowData; + } } diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 18eeafeef..28a5f46fa 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -20,6 +20,7 @@ use OCA\Tables\Event\RowAddedEvent; use OCA\Tables\Event\RowDeletedEvent; use OCA\Tables\Event\RowUpdatedEvent; +use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Model\RowDataInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; @@ -36,30 +37,21 @@ * @psalm-import-type TablesRow from ResponseDefinitions */ class RowService extends SuperService { - private ColumnMapper $columnMapper; - private ViewMapper $viewMapper; - private TableMapper $tableMapper; - private Row2Mapper $row2Mapper; private array $tmpRows = []; // holds already loaded rows as a small cache - protected IEventDispatcher $eventDispatcher; - public function __construct( - PermissionsService $permissionsService, LoggerInterface $logger, ?string $userId, - ColumnMapper $columnMapper, - ViewMapper $viewMapper, - TableMapper $tableMapper, - Row2Mapper $row2Mapper, - IEventDispatcher $eventDispatcher + PermissionsService $permissionsService, + private ColumnMapper $columnMapper, + private ViewMapper $viewMapper, + private TableMapper $tableMapper, + private Row2Mapper $row2Mapper, + private IEventDispatcher $eventDispatcher, + private ColumnsHelper $columnsHelper, ) { parent::__construct($logger, $userId, $permissionsService); - $this->columnMapper = $columnMapper; - $this->viewMapper = $viewMapper; - $this->tableMapper = $tableMapper; - $this->row2Mapper = $row2Mapper; - $this->eventDispatcher = $eventDispatcher; + } /** @@ -174,6 +166,8 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data): R throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); } + $view = null; + if ($viewId) { try { $view = $this->viewMapper->find($viewId); @@ -215,9 +209,10 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data): R throw new InternalError('Cannot create row without table or view in context'); } + $data = $data instanceof RowDataInput ? $data : RowDataInput::fromArray($data); $data = $this->cleanupData($data, $columns, $tableId, $viewId); + $data = $this->enhanceWithViewDefaults($view, $data); - // perf $tableId = $tableId ?? $view->getTableId(); $row2 = new Row2(); $row2->setTableId($tableId); @@ -227,7 +222,7 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data): R $this->eventDispatcher->dispatchTyped(new RowAddedEvent($insertedRow)); - return $insertedRow; + return $this->filterRowResult($view, $insertedRow); } catch (InternalError|Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); @@ -235,16 +230,37 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data): R } /** - * @param RowDataInput|list $data - * @param Column[] $columns - * @param int|null $tableId - * @param int|null $viewId - * @return list + * When inserting rows into views we try to prefill columns that are not accessible by reasonable defaults * + * This might not work in all cases, but for single filter rules this is the sanest to ensure the row is actually part of the view + */ + private function enhanceWithViewDefaults(?View $view, RowDataInput $data): RowDataInput { + if ($view === null) { + return $data; + } + + $filters = $view->getFilterArray(); + if ($filterRule = array_shift($filters)) { + foreach ($filterRule as $filter) { + if (in_array($filter['columnId'], $view->getColumnsArray())) { + continue; + } + + if (!in_array($filter['operator'], ['is-equal'])) { + continue; + } + + $data->add($filter['columnId'], $this->columnsHelper->resolveSearchValue((string)$filter['value'], $this->userId)); + } + } + return $data; + } + + /** * @throws InternalError */ - private function cleanupData(RowDataInput|array $data, array $columns, ?int $tableId, ?int $viewId): array { - $out = []; + private function cleanupData(RowDataInput $data, array $columns, ?int $tableId, ?int $viewId): RowDataInput { + $out = new RowDataInput(); foreach ($data as $entry) { $column = $this->getColumnFromColumnsArray((int) $entry['columnId'], $columns); @@ -262,10 +278,7 @@ private function cleanupData(RowDataInput|array $data, array $columns, ?int $tab } // parse given value to respect the column type value format - $out[] = [ - 'columnId' => (int) $entry['columnId'], - 'value' => $this->parseValueByColumnType($column, $entry['value']) - ]; + $out->add((int) $entry['columnId'], $this->parseValueByColumnType($column, $entry['value'])); } return $out; } @@ -273,7 +286,7 @@ private function cleanupData(RowDataInput|array $data, array $columns, ?int $tab /** * @param Column $column * @param string|array|int|float|bool|null $value - * @return string|int|float|null + * @return array|string|int|float|null */ private function parseValueByColumnType(Column $column, $value = null) { try { @@ -413,6 +426,7 @@ public function updateSet( } $previousData = $item->getData(); + $data = RowDataInput::fromArray($data); $data = $this->cleanupData($data, $columns, $item->getTableId(), $viewId); foreach ($data as $entry) { diff --git a/tests/unit/Service/LegacyRowMapperTest.php b/tests/unit/Service/LegacyRowMapperTest.php index e567e864e..bce9a926c 100644 --- a/tests/unit/Service/LegacyRowMapperTest.php +++ b/tests/unit/Service/LegacyRowMapperTest.php @@ -12,6 +12,7 @@ use OCA\Tables\Db\ColumnTypes\SelectionColumnQB; use OCA\Tables\Db\ColumnTypes\SuperColumnQB; use OCA\Tables\Db\ColumnTypes\TextColumnQB; +use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Helper\UserHelper; use OCP\IDBConnection; use OCP\Server; @@ -120,7 +121,8 @@ public function testMigrateLegacyRow() { $row2Mapper = $this->createMock(Row2Mapper::class); $logger = $this->createMock(LoggerInterface::class); $userHelper = $this->createMock(UserHelper::class); - $legacyRowMapper = new LegacyRowMapper($dbConnection, $logger, $textColumnQb, $selectionColumnQb, $numberColumnQb, $datetimeColumnQb, $superColumnQb, $columnMapper, $userHelper, $row2Mapper); + $columnsHelper = $this->createMock(ColumnsHelper::class); + $legacyRowMapper = new LegacyRowMapper($dbConnection, $logger, $textColumnQb, $selectionColumnQb, $numberColumnQb, $datetimeColumnQb, $superColumnQb, $columnMapper, $columnsHelper, $userHelper, $row2Mapper); $legacyRow = new LegacyRow(); $legacyRow->setId(5);