fix: Properly create new rows in filtered views

Signed-off-by: Julius Knorr <jus@bitgrid.net>

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<LegacyRow> */
 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);
 					}
 				}
 			}

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<array{columnId: int, value: mixed}> $data
+	 * @param RowDataInput|list<array{columnId: int, value: mixed}> $data
 	 * @return void
 	 */
-	public function setData(array $data): void {
+	public function setData(array|RowDataInput $data): void {
 		foreach ($data as $cell) {
 			$this->insertOrUpdateCell($cell);
 		}

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

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;
+		}
+	}
 }

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;
+	}
 }

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,24 +222,45 @@ 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());
 		}
 	}
 
 	/**
-	 * @param RowDataInput|list<array{columnId: string|int, value: mixed}> $data
-	 * @param Column[] $columns
-	 * @param int|null $tableId
-	 * @param int|null $viewId
-	 * @return list<array{columnId: int, value: float|int|string|null}>
+	 * 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,18 +278,15 @@ 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;
 	}
 
 	/**
 	 * @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) {