Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/Analytics/AnalyticsDatasource.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,14 +374,19 @@ private function formatUsergroupValue(mixed $value): string {
}

private function formatRelationValue(Column $column, mixed $value): string {
if ($value === null || $value === '') {
$ids = $this->normalizeArrayValue($value);
if ($ids === []) {
return '';
}

$relationData = $this->relationService->getRelationData($column);
$valueId = (int)$value;
$labels = [];
foreach ($ids as $id) {
$valueId = (int)$id;
$labels[] = $relationData[$valueId]['label'] ?? (string)$id;
}

return $relationData[$valueId]['label'] ?? (string)$value;
return implode(', ', array_filter($labels, static fn (string $label): bool => $label !== ''));
}

private function parseDefaultValue(?string $value): mixed {
Expand Down
1 change: 1 addition & 0 deletions lib/Db/Column.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class Column extends EntitySuper implements JsonSerializable {
public const RELATION_TYPE = 'relationType';
public const RELATION_TARGET_ID = 'targetId';
public const RELATION_LABEL_COLUMN = 'labelColumn';
public const RELATION_ALLOW_MULTIPLE = 'allowMultiple';

protected ?string $uuid = null;
protected ?string $title = null;
Expand Down
45 changes: 42 additions & 3 deletions lib/Db/Row2Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,12 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
break;
}

if ($column->getType() === Column::TYPE_RELATION) {
$includeDefault = false;
$filterExpression = $qb->expr()->eq('value', $qb->createNamedParameter((int)$value, IQueryBuilder::PARAM_INT));
break;
}

$includeDefault = str_contains((string)($defaultValue ?? ''), (string)$value);
if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') {
$value = str_replace(['"', '\''], '', $value);
Expand Down Expand Up @@ -556,6 +562,9 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
$qb->expr()->notIn('sl3.id', $qb->createFunction($qb2->getSQL()))
);
}
if ($column->getType() === Column::TYPE_RELATION) {
return $this->getRelationExclusionFilter($qb, $qb2, $column, (int)$value);
}
$includeDefault = !str_contains((string)($defaultValue ?? ''), (string)$value);
if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') {
$value = str_replace(['"', '\''], '', $value);
Expand All @@ -576,6 +585,11 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
$filterExpression = $qb->expr()->eq('value', $qb->createNamedParameter('[' . $this->db->escapeLikeParameter($value) . ']', $paramType));
break;
}
if ($column->getType() === Column::TYPE_RELATION) {
$includeDefault = false;
$filterExpression = $qb->expr()->eq('value', $qb->createNamedParameter((int)$value, IQueryBuilder::PARAM_INT));
break;
}
$filterExpression = $qb->expr()->eq('value', $qb->createNamedParameter($value, $paramType));
break;
case 'is-not-equal':
Expand All @@ -585,6 +599,9 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
$filterExpression = $qb->expr()->neq('value', $qb->createNamedParameter('[' . $this->db->escapeLikeParameter($value) . ']', $paramType));
break;
}
if ($column->getType() === Column::TYPE_RELATION) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated with lines 565ff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — shared that logic in one place.

return $this->getRelationExclusionFilter($qb, $qb2, $column, (int)$value);
}
$filterExpression = $qb->expr()->neq('value', $qb->createNamedParameter($value, $paramType));
break;
case 'is-greater-than':
Expand Down Expand Up @@ -649,6 +666,23 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
);
}

/**
* Rows that do not have the given related id among their relation cell values.
*/
private function getRelationExclusionFilter(IQueryBuilder $qb, IQueryBuilder $qb2, Column $column, int $value): IQueryBuilder {
$qb2->andWhere($qb->expr()->eq('value', $qb->createNamedParameter($value, IQueryBuilder::PARAM_INT)));

return $this->db->getQueryBuilder()
->selectAlias('sl3.id', 'row_id')
->from('tables_row_sleeves', 'sl3')
->where(
$qb->expr()->eq('sl3.table_id', $qb->createNamedParameter($column->getTableId(), IQueryBuilder::PARAM_INT))
)
->andWhere(
$qb->expr()->notIn('sl3.id', $qb->createFunction($qb2->getSQL()))
);
}

/**
* @throws InternalError
*/
Expand Down Expand Up @@ -727,7 +761,7 @@ private function parseEntities(IResult $result, array $sleeves): array {
}

$rowValues = [];
$keyToColumnId = [];
$keyToColumn = [];
$keyToRowId = [];
$cellMapperCache = [];

Expand All @@ -752,12 +786,17 @@ private function parseEntities(IResult $result, array $sleeves): array {
} else {
$rowValues[$compositeKey] = $value;
}
$keyToColumnId[$compositeKey] = $rowData['column_id'];
$keyToColumn[$compositeKey] = $column;
$keyToRowId[$compositeKey] = $rowData['row_id'];
}

foreach ($rowValues as $compositeKey => $value) {
$rows[$keyToRowId[$compositeKey]]->addCell($keyToColumnId[$compositeKey], $value);
$column = $keyToColumn[$compositeKey];
$columnType = $column->getType();
if ($cellMapperCache[$columnType]->hasMultipleValues() && is_array($value)) {
$value = $cellMapperCache[$columnType]->formatAggregatedValues($column, $value);
}
$rows[$keyToRowId[$compositeKey]]->addCell($column->getId(), $value);
}

return array_values($rows);
Expand Down
10 changes: 10 additions & 0 deletions lib/Db/RowCellMapperSuper.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public function hasMultipleValues(): bool {
return false;
}

/**
* Shape aggregated multi-cell values for the API response.
*
* @param list<mixed> $values
* @return mixed
*/
public function formatAggregatedValues(Column $column, array $values): mixed {
return $values;
}

/**
* @throws Exception
*/
Expand Down
100 changes: 95 additions & 5 deletions lib/Db/RowCellRelationMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,113 @@

/** @template-extends RowCellMapperSuper<RowCellRelation, int|null, int|null> */
class RowCellRelationMapper extends RowCellMapperSuper {
private const DB_CHUNK_SIZE = 1_000;

protected string $table = 'tables_row_cells_relation';

public function __construct(IDBConnection $db) {
parent::__construct($db, $this->table, RowCellRelation::class);
}

/**
* @inheritDoc
* Relation values are stored as one cell row per related id (usergroup-style),
* so single- and multi-select columns share the same storage shape.
*/
public function hasMultipleValues(): bool {
return false;
return true;
}

/**
* @inheritDoc
*/
public function getDbParamType() {
return IQueryBuilder::PARAM_INT;
}

public function formatRowData(Column $column, array $row) {
$value = $row['value'];
if ($value === null || $value === '') {
return null;
}
return (int)$value;
}

/**
* @param list<int|null> $values
* @return list<int>|int|null
*/
public function formatAggregatedValues(Column $column, array $values): mixed {
$values = array_values(array_filter($values, static fn ($value) => $value !== null));
if (!(bool)($column->getCustomSettingsArray()[Column::RELATION_ALLOW_MULTIPLE] ?? false)) {
return $values[0] ?? null;
}
return $values;
}

public function applyDataToEntity(Column $column, RowCellSuper $cell, $data): void {
$cell->setValue($data === null || $data === '' ? null : (int)$data);
}

/**
* Keep only the first related value per row (lowest cell id).
* Used when allowMultiple is turned off on an existing column.
*/
public function truncateToSingleValuePerRow(int $columnId): void {
$qb = $this->db->getQueryBuilder();
$qb->select('id', 'row_id')
->from($this->table)
->where($qb->expr()->eq('column_id', $qb->createNamedParameter($columnId, IQueryBuilder::PARAM_INT)))
->orderBy('row_id', 'ASC')
->addOrderBy('id', 'ASC');

$result = $qb->executeQuery();
$seenRows = [];
$idsToDelete = [];
while ($row = $result->fetchAssociative()) {
$rowId = (int)$row['row_id'];
if (isset($seenRows[$rowId])) {
$idsToDelete[] = (int)$row['id'];
if (count($idsToDelete) >= self::DB_CHUNK_SIZE) {
$this->deleteByIds($idsToDelete);
$idsToDelete = [];
}
} else {
$seenRows[$rowId] = true;
}
}
$result->closeCursor();

if ($idsToDelete !== []) {
$this->deleteByIds($idsToDelete);
}
}

/**
* @param list<int> $ids
*/
private function deleteByIds(array $ids): void {
$deleteQb = $this->db->getQueryBuilder();
$deleteQb->delete($this->table)
->where($deleteQb->expr()->in('id', $deleteQb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
$deleteQb->executeStatement();
}

/**
* Whether any table row has no related value for this column.
*/
public function hasRowsWithoutValue(int $columnId, int $tableId): bool {
$qb = $this->db->getQueryBuilder();
$qb->select('sl.id')
->from('tables_row_sleeves', 'sl')
->leftJoin('sl', $this->table, 'c', $qb->expr()->andX(
$qb->expr()->eq('sl.id', 'c.row_id'),
$qb->expr()->eq('c.column_id', $qb->createNamedParameter($columnId, IQueryBuilder::PARAM_INT)),
$qb->expr()->isNotNull('c.value'),
))
->where($qb->expr()->eq('sl.table_id', $qb->createNamedParameter($tableId, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->isNull('c.id'))
->setMaxResults(1);

$result = $qb->executeQuery();
$hasEmpty = $result->fetchOne() !== false;
$result->closeCursor();
return $hasEmpty;
}
}
19 changes: 19 additions & 0 deletions lib/Service/ColumnService.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCA\Tables\Constants\ColumnType;
use OCA\Tables\Db\Column;
use OCA\Tables\Db\ColumnMapper;
use OCA\Tables\Db\RowCellRelationMapper;
use OCA\Tables\Db\Table;
use OCA\Tables\Db\TableMapper;
use OCA\Tables\Db\View;
Expand Down Expand Up @@ -54,6 +55,7 @@ public function __construct(
private readonly IL10N $l,
private readonly UserHelper $userHelper,
private readonly ColumnDtoValidator $columnDtoValidator,
private readonly RowCellRelationMapper $rowCellRelationMapper,
) {
parent::__construct($logger, $userId, $permissionsService);
}
Expand Down Expand Up @@ -374,6 +376,9 @@ public function update(
$this->columnDtoValidator->validate($columnDto);
$title = $this->normalizeTitle($columnDto->getTitle(), false);

$wasMandatory = (bool)$item->getMandatory();
$previousAllowMultiple = (bool)($item->getCustomSettingsArray()[Column::RELATION_ALLOW_MULTIPLE] ?? false);

if ($title !== null) {
$item->setTitle($title);
}
Expand Down Expand Up @@ -425,10 +430,24 @@ public function update(
$this->validateCustomSettings($columnDto->getCustomSettings());
$item->setCustomSettings($columnDto->getCustomSettings());

$willBeMandatory = $columnDto->isMandatory() !== null ? (bool)$columnDto->isMandatory() : $wasMandatory;
$newAllowMultiple = (bool)($item->getCustomSettingsArray()[Column::RELATION_ALLOW_MULTIPLE] ?? false);
if ($item->getType() === Column::TYPE_RELATION && $willBeMandatory && !$wasMandatory) {
if ($this->rowCellRelationMapper->hasRowsWithoutValue($item->getId(), $item->getTableId())) {
throw new BadRequestError(
'Cannot make this relation column mandatory while some rows have no related value.'
);
}
}

$this->updateMetadata($item, $userId);
try {
$updatedColumn = $this->mapper->update($item);

if ($updatedColumn->getType() === Column::TYPE_RELATION && $previousAllowMultiple && !$newAllowMultiple) {
$this->rowCellRelationMapper->truncateToSingleValuePerRow($updatedColumn->getId());
}

$this->activityManager->triggerEvent(
objectType: ActivityManager::TABLES_OBJECT_COLUMN,
object: $updatedColumn,
Expand Down
Loading