From 0cbd8681d4ca8c4d8b844ae4894257bac1df13e4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 2 Apr 2025 19:08:06 +0200 Subject: [PATCH 01/16] feat(Filter): add is-not-equal operator Signed-off-by: Arthur Schiwon --- lib/Db/ColumnTypes/SuperColumnQB.php | 2 ++ lib/Db/Row2Mapper.php | 11 +++++++++++ lib/Service/RowService.php | 2 +- .../ncTable/mixins/columnsTypes/textLine.js | 1 + src/shared/components/ncTable/mixins/filter.js | 8 ++++++++ 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/Db/ColumnTypes/SuperColumnQB.php b/lib/Db/ColumnTypes/SuperColumnQB.php index 6109f73c4c..a1720c048d 100644 --- a/lib/Db/ColumnTypes/SuperColumnQB.php +++ b/lib/Db/ColumnTypes/SuperColumnQB.php @@ -68,6 +68,8 @@ private function sqlFilterOperation(string $operator, string $formattedCellValue return $formattedCellValue . ' LIKE :' . $searchValuePlaceHolder; case 'is-equal': return $formattedCellValue . ' = :' . $searchValuePlaceHolder; + case 'is-not-equal': + return $formattedCellValue . ' != :' . $searchValuePlaceHolder; case 'is-greater-than': return $formattedCellValue . ' > :' . $searchValuePlaceHolder; case 'is-greater-than-or-equal': diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 862d94cb10..a2cd044d2b 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -464,6 +464,15 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $ } $filterExpression = $qb->expr()->eq('value', $qb->createNamedParameter($value, $paramType)); break; + case 'is-not-equal': + $includeDefault = $defaultValue === $value; + if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') { + $value = str_replace(['"', '\''], '', $value); + $filterExpression = $qb->expr()->neq('value', $qb->createNamedParameter('[' . $this->db->escapeLikeParameter($value) . ']', $paramType)); + break; + } + $filterExpression = $qb->expr()->neq('value', $qb->createNamedParameter($value, $paramType)); + break; case 'is-greater-than': $includeDefault = $column->getNumberDefault() > (float)$value; $filterExpression = $qb->expr()->gt('value', $qb->createNamedParameter($value, $paramType)); @@ -547,6 +556,8 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col return $qb->expr()->like($columnName, $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($value) . '%', $paramType)); case 'is-equal': return $qb->expr()->eq($columnName, $qb->createNamedParameter($value, $paramType)); + case 'is-not-equal': + return $qb->expr()->neq($columnName, $qb->createNamedParameter($value, $paramType)); case 'is-greater-than': return $qb->expr()->gt($columnName, $qb->createNamedParameter($value, $paramType)); case 'is-greater-than-or-equal': diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 885cf03893..81aa390e38 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -266,7 +266,7 @@ private function enhanceWithViewDefaults(?View $view, RowDataInput $data): RowDa } // Only handle simple equality filters for now - if (!in_array($filter['operator'], ['is-equal'])) { + if (!in_array($filter['operator'], ['is-equal', 'is-not-equal'])) { continue; } diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textLine.js b/src/shared/components/ncTable/mixins/columnsTypes/textLine.js index 7bac97c8b5..79dbc62a51 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textLine.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textLine.js @@ -50,6 +50,7 @@ export default class TextLineColumn extends AbstractTextColumn { [FilterIds.BeginsWith]() { return cellValue.startsWith(filterValue) }, [FilterIds.EndsWith]() { return cellValue.endsWith(filterValue) }, [FilterIds.IsEqual]() { return cellValue === filterValue }, + [FilterIds.IsNotEqual]() { return cellValue !== filterValue }, [FilterIds.IsEmpty]() { return !cellValue }, }[filter.operator.id] diff --git a/src/shared/components/ncTable/mixins/filter.js b/src/shared/components/ncTable/mixins/filter.js index 8baa5b3287..732940905a 100644 --- a/src/shared/components/ncTable/mixins/filter.js +++ b/src/shared/components/ncTable/mixins/filter.js @@ -42,6 +42,7 @@ export const FilterIds = { BeginsWith: 'begins-with', EndsWith: 'ends-with', IsEqual: 'is-equal', + IsNotEqual: 'is-not-equal', IsGreaterThan: 'is-greater-than', IsGreaterThanOrEqual: 'is-greater-than-or-equal', IsLowerThan: 'is-lower-than', @@ -75,6 +76,13 @@ export const Filters = { goodFor: [ColumnTypes.TextLine, ColumnTypes.Number, ColumnTypes.SelectionCheck, ColumnTypes.TextLink, ColumnTypes.NumberStars, ColumnTypes.NumberProgress, ColumnTypes.DatetimeDate, ColumnTypes.DatetimeTime, ColumnTypes.Datetime, ColumnTypes.Selection, ColumnTypes.SelectionMulti, ColumnTypes.Usergroup], incompatibleWith: [FilterIds.IsEmpty, FilterIds.IsEqual, FilterIds.BeginsWith, FilterIds.EndsWith, FilterIds.Contains, FilterIds.IsGreaterThan, FilterIds.IsGreaterThanOrEqual, FilterIds.IsLowerThan, FilterIds.IsLowerThanOrEqual], }), + IsNotEqual: new Filter({ + id: FilterIds.IsNotEqual, + label: t('tables', 'Is not equal'), + shortLabel: '!=', + goodFor: [ColumnTypes.TextLine, ColumnTypes.Number, ColumnTypes.SelectionCheck, ColumnTypes.TextLink, ColumnTypes.NumberStars, ColumnTypes.NumberProgress, ColumnTypes.DatetimeDate, ColumnTypes.DatetimeTime, ColumnTypes.Datetime, ColumnTypes.Selection, ColumnTypes.SelectionMulti, ColumnTypes.Usergroup], + incompatibleWith: [FilterIds.IsEmpty, FilterIds.IsEqual, FilterIds.BeginsWith, FilterIds.EndsWith, FilterIds.Contains, FilterIds.IsGreaterThan, FilterIds.IsGreaterThanOrEqual, FilterIds.IsLowerThan, FilterIds.IsLowerThanOrEqual], + }), IsGreaterThan: new Filter({ id: FilterIds.IsGreaterThan, label: t('tables', 'Is greater than'), From d4698669d5392d56b7da720b44bf528a06d9d288 Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 21 Aug 2025 16:05:47 +0200 Subject: [PATCH 02/16] Add does-not-contain filter in backend Signed-off-by: silver --- lib/Controller/Api1Controller.php | 2 +- lib/Db/ColumnTypes/SuperColumnQB.php | 2 ++ lib/Db/LegacyRowMapper.php | 4 +-- lib/Db/Row2Mapper.php | 38 ++++++++++++++++++++++++++++ lib/Db/View.php | 2 +- lib/ResponseDefinitions.php | 2 +- 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index 559f5598b0..a97850a83b 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -384,7 +384,7 @@ public function getView(int $viewId): DataResponse { * Update a view via key-value sets * * @param int $viewId View ID - * @param array{key: 'title'|'emoji'|'description', value: string}|array{key: 'columns', value: list}|array{key: 'sort', value: array{columnId: int, mode: 'ASC'|'DESC'}}|array{key: 'filter', value: array{columnId: int, operator: 'begins-with'|'ends-with'|'contains'|'is-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal'|'is-empty', value: string|int|float}} $data key-value pairs + * @param array{key: 'title'|'emoji'|'description', value: string}|array{key: 'columns', value: list}|array{key: 'sort', value: array{columnId: int, mode: 'ASC'|'DESC'}}|array{key: 'filter', value: array{columnId: int, operator: 'begins-with'|'ends-with'|'contains'|'does-not-contain'|'is-equal'|'is-not-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal'|'is-empty', value: string|int|float}} $data key-value pairs * @return DataResponse|DataResponse * * 200: View updated diff --git a/lib/Db/ColumnTypes/SuperColumnQB.php b/lib/Db/ColumnTypes/SuperColumnQB.php index a1720c048d..df436a0f6d 100644 --- a/lib/Db/ColumnTypes/SuperColumnQB.php +++ b/lib/Db/ColumnTypes/SuperColumnQB.php @@ -66,6 +66,8 @@ private function sqlFilterOperation(string $operator, string $formattedCellValue case 'ends-with': case 'contains': return $formattedCellValue . ' LIKE :' . $searchValuePlaceHolder; + case 'does-not-contain': + return $formattedCellValue . ' NOT LIKE :' . $searchValuePlaceHolder; case 'is-equal': return $formattedCellValue . ' = :' . $searchValuePlaceHolder; case 'is-not-equal': diff --git a/lib/Db/LegacyRowMapper.php b/lib/Db/LegacyRowMapper.php index 73e8ce0190..84ee4450e1 100644 --- a/lib/Db/LegacyRowMapper.php +++ b/lib/Db/LegacyRowMapper.php @@ -101,7 +101,7 @@ private function buildFilterByColumnType($qb, array $filter, string $filterId): /** * @param (float|int|string)[][] $filterGroup * - * @psalm-param list $filterGroup + * @psalm-param list $filterGroup */ private function getInnerFilterExpressions(IQueryBuilder $qb, array $filterGroup, int $groupIndex): array { $innerFilterExpressions = []; @@ -114,7 +114,7 @@ private function getInnerFilterExpressions(IQueryBuilder $qb, array $filterGroup /** * @param (float|int|string)[][][] $filters * - * @psalm-param non-empty-list> $filters + * @psalm-param non-empty-list> $filters */ private function getFilterGroups(IQueryBuilder $qb, array $filters): array { $filterGroups = []; diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index a2cd044d2b..9fd0ae3384 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -455,6 +455,44 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $ } $filterExpression = $qb->expr()->like('value', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($value) . '%', $paramType)); break; + case 'does-not-contain': + $filterExpressions = []; + if (is_array($value) && $column->getType() === Column::TYPE_USERGROUP) { + $filterExpressions[] = $qb2->expr()->andX( + $qb->expr()->neq('value', $qb->createNamedParameter($value[UsergroupType::USER])), + $qb->expr()->eq('value_type', $qb->createNamedParameter(UsergroupType::USER, IQueryBuilder::PARAM_INT)) + ); + if (!empty($value[UsergroupType::GROUP])) { + $filterExpressions[] = $qb2->expr()->andX( + $qb->expr()->notIn('value', $qb->createNamedParameter($value[UsergroupType::GROUP], IQueryBuilder::PARAM_STR_ARRAY)), + $qb->expr()->eq('value_type', $qb->createNamedParameter(UsergroupType::GROUP, IQueryBuilder::PARAM_INT)) + ); + } + if (!empty($value[UsergroupType::CIRCLE])) { + $filterExpressions[] = $qb2->expr()->andX( + $qb->expr()->notIn('value', $qb->createNamedParameter($value[UsergroupType::CIRCLE], IQueryBuilder::PARAM_STR_ARRAY)), + $qb->expr()->eq('value_type', $qb->createNamedParameter(UsergroupType::CIRCLE, IQueryBuilder::PARAM_INT)) + ); + } + $filterExpression = $qb2->expr()->andX(...$filterExpressions); + $includeDefault = false; + + break; + } + + $includeDefault = !str_contains($defaultValue, $value); + if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') { + $value = str_replace(['"', '\''], '', $value); + $filterExpression = $qb2->expr()->andX( + $qb->expr()->notLike('value', $qb->createNamedParameter('[' . $this->db->escapeLikeParameter($value) . ']')), + $qb->expr()->notLike('value', $qb->createNamedParameter('[' . $this->db->escapeLikeParameter($value) . ',%')), + $qb->expr()->notLike('value', $qb->createNamedParameter('%,' . $this->db->escapeLikeParameter($value) . ']%')), + $qb->expr()->notLike('value', $qb->createNamedParameter('%,' . $this->db->escapeLikeParameter($value) . ',%')) + ); + break; + } + $filterExpression = $qb->expr()->notLike('value', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($value) . '%', $paramType)); + break; case 'is-equal': $includeDefault = $defaultValue === $value; if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') { diff --git a/lib/Db/View.php b/lib/Db/View.php index 05982396b5..224f9b347f 100644 --- a/lib/Db/View.php +++ b/lib/Db/View.php @@ -141,7 +141,7 @@ public function getSortArray(): array { /** * @psalm-suppress MismatchingDocblockReturnType - * @return list> + * @return list> */ public function getFilterArray():array { $filters = $this->getArray($this->getFilter()); diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 8b2ce56928..b7d7c83610 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -28,7 +28,7 @@ * columns: list, * columnSettings:list, * sort: list, - * filter: list>, + * filter: list>, * isShared: bool, * favorite: bool, * onSharePermissions: ?array{ From f932a52d5afe18b71f7737ffca125c28918ee08a Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 21 Aug 2025 16:06:08 +0200 Subject: [PATCH 03/16] DoesNotContain in filters.js Signed-off-by: silver --- src/shared/components/ncTable/mixins/filter.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/shared/components/ncTable/mixins/filter.js b/src/shared/components/ncTable/mixins/filter.js index 732940905a..0e3a4f91c8 100644 --- a/src/shared/components/ncTable/mixins/filter.js +++ b/src/shared/components/ncTable/mixins/filter.js @@ -39,6 +39,7 @@ export function getFilterWithId(id) { export const FilterIds = { Contains: 'contains', + DoesNotContain: 'does-not-contain', BeginsWith: 'begins-with', EndsWith: 'ends-with', IsEqual: 'is-equal', @@ -55,7 +56,13 @@ export const Filters = { id: FilterIds.Contains, label: t('tables', 'Contains'), goodFor: [ColumnTypes.TextLine, ColumnTypes.TextLong, ColumnTypes.TextLink, ColumnTypes.TextRich, ColumnTypes.SelectionMulti, ColumnTypes.Usergroup, ColumnTypes.Selection], - incompatibleWith: [FilterIds.IsEmpty, FilterIds.IsEqual], + incompatibleWith: [FilterIds.DoesNotContain, FilterIds.IsEmpty, FilterIds.IsEqual], + }), + DoesNotContain: new Filter({ + id: FilterIds.DoesNotContain, + label: t('tables', 'Does not contain'), + goodFor: [ColumnTypes.TextLine, ColumnTypes.TextLong, ColumnTypes.TextLink, ColumnTypes.TextRich, ColumnTypes.SelectionMulti, ColumnTypes.Usergroup, ColumnTypes.Selection], + incompatibleWith: [FilterIds.Contains, FilterIds.IsEmpty, FilterIds.IsEqual], }), BeginsWith: new Filter({ id: FilterIds.BeginsWith, @@ -74,7 +81,7 @@ export const Filters = { label: t('tables', 'Is equal'), shortLabel: '=', goodFor: [ColumnTypes.TextLine, ColumnTypes.Number, ColumnTypes.SelectionCheck, ColumnTypes.TextLink, ColumnTypes.NumberStars, ColumnTypes.NumberProgress, ColumnTypes.DatetimeDate, ColumnTypes.DatetimeTime, ColumnTypes.Datetime, ColumnTypes.Selection, ColumnTypes.SelectionMulti, ColumnTypes.Usergroup], - incompatibleWith: [FilterIds.IsEmpty, FilterIds.IsEqual, FilterIds.BeginsWith, FilterIds.EndsWith, FilterIds.Contains, FilterIds.IsGreaterThan, FilterIds.IsGreaterThanOrEqual, FilterIds.IsLowerThan, FilterIds.IsLowerThanOrEqual], + incompatibleWith: [FilterIds.IsNotEqual, FilterIds.IsEmpty, FilterIds.IsEqual, FilterIds.BeginsWith, FilterIds.EndsWith, FilterIds.Contains, FilterIds.IsGreaterThan, FilterIds.IsGreaterThanOrEqual, FilterIds.IsLowerThan, FilterIds.IsLowerThanOrEqual], }), IsNotEqual: new Filter({ id: FilterIds.IsNotEqual, From 5cfa3cc59dc082f9d525123d227bcd5d01bd520a Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 21 Aug 2025 16:07:15 +0200 Subject: [PATCH 04/16] prevent usage of multiple identical DoesNotContain filters Signed-off-by: silver --- .../ncTable/partials/TableHeaderColumnOptions.vue | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue b/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue index 7ea11aafda..7684eacf82 100644 --- a/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue +++ b/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue @@ -314,9 +314,12 @@ export default { }, submitFilterInput() { // Ignore contains filter with the same value es old contain filters - if (this.selectedOperator.id === FilterIds.Contains) { + if ([FilterIds.Contains, FilterIds.DoesNotContain].includes(this.selectedOperator.id)) { const columnFilters = this.getFilterForColumn(this.column) - if (columnFilters && columnFilters.filter(fil => fil.operator.id === FilterIds.Contains).map(fil => fil.value).includes(this.searchValue)) { + if (columnFilters && columnFilters + .filter(fil => fil.operator.id === this.selectedOperator.id) + .map(fil => fil.value) + .includes(this.searchValue)) { this.reset() return } From f8a575108bcfe6e7539edd2d76e8da48b2b4141e Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 21 Aug 2025 16:08:31 +0200 Subject: [PATCH 05/16] add DoesNotContain as filterMethod to respective columnTypes Signed-off-by: silver --- src/shared/components/ncTable/mixins/columnsTypes/selection.js | 1 + .../components/ncTable/mixins/columnsTypes/selectionMulti.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/textLine.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/textLink.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/textLong.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/textRich.js | 1 + 6 files changed, 6 insertions(+) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/selection.js b/src/shared/components/ncTable/mixins/columnsTypes/selection.js index 0bfe5a4929..860a071d42 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/selection.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/selection.js @@ -55,6 +55,7 @@ export default class SelectionColumn extends AbstractSelectionColumn { const cellLabel = this.getLabel(cell.value) const filterMethod = { [FilterIds.Contains]() { return cellLabel?.toLowerCase().includes(filterValue?.toLowerCase()) }, + [FilterIds.DoesNotContain]() { return !cellLabel?.toLowerCase().includes(filterValue?.toLowerCase()) }, [FilterIds.BeginsWith]() { return cellLabel?.startsWith(filterValue) }, [FilterIds.EndsWith]() { return cellLabel?.endsWith(filterValue) }, [FilterIds.IsEqual]() { return cellLabel === filterValue }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js b/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js index c3b84c3c85..96f8d31826 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js @@ -68,6 +68,7 @@ export default class SelectionMutliColumn extends AbstractSelectionColumn { const filterMethod = { [FilterIds.Contains]() { return valueString?.includes(filterValue) }, + [FilterIds.DoesNotContain]() { return !valueString?.includes(filterValue) }, [FilterIds.IsEqual]() { return valueString === filterValue }, [FilterIds.IsEmpty]() { return !valueString }, }[filter.operator.id] diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textLine.js b/src/shared/components/ncTable/mixins/columnsTypes/textLine.js index 79dbc62a51..e1408f225f 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textLine.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textLine.js @@ -47,6 +47,7 @@ export default class TextLineColumn extends AbstractTextColumn { if (!cellValue & filter.operator.id !== FilterIds.IsEmpty) return false const filterMethod = { [FilterIds.Contains]() { return cellValue.includes(filterValue) }, + [FilterIds.DoesNotContain]() { return !cellValue.includes(filterValue) }, [FilterIds.BeginsWith]() { return cellValue.startsWith(filterValue) }, [FilterIds.EndsWith]() { return cellValue.endsWith(filterValue) }, [FilterIds.IsEqual]() { return cellValue === filterValue }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textLink.js b/src/shared/components/ncTable/mixins/columnsTypes/textLink.js index 13f1d96f8e..0fd3d2d969 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textLink.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textLink.js @@ -57,6 +57,7 @@ export default class TextLinkColumn extends AbstractTextColumn { const filterMethod = { [FilterIds.Contains]() { return value.includes(filterValue) }, + [FilterIds.DoesNotContain]() { return !value.includes(filterValue) }, [FilterIds.BeginsWith]() { return value.startsWith(filterValue) }, [FilterIds.EndsWith]() { return value.endsWith(filterValue) }, [FilterIds.IsEqual]() { return value === filterValue }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textLong.js b/src/shared/components/ncTable/mixins/columnsTypes/textLong.js index cad04747cd..a6b35728b3 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textLong.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textLong.js @@ -26,6 +26,7 @@ export default class TextLongColumn extends AbstractTextColumn { const filterMethod = { [FilterIds.Contains]() { return cell.value.includes(filterValue) }, + [FilterIds.DoesNotContain]() { return !cell.value.includes(filterValue) }, [FilterIds.IsEmpty]() { return !cell.value }, }[filter.operator.id] return super.isFilterFound(filterMethod, cell) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textRich.js b/src/shared/components/ncTable/mixins/columnsTypes/textRich.js index d732665041..4bb039c36b 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textRich.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textRich.js @@ -22,6 +22,7 @@ export default class TextRichColumn extends AbstractTextColumn { const filterMethod = { [FilterIds.Contains]() { return cell.value && cell.value.includes(filterValue) }, + [FilterIds.DoesNotContain]() { return cell.value && !cell.value.includes(filterValue) }, [FilterIds.IsEmpty]() { return !cell.value }, }[filter.operator.id] return super.isFilterFound(filterMethod, cell) From 3b2ae6f008f05dbd51c72b09f73451d678275152 Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 21 Aug 2025 16:32:28 +0200 Subject: [PATCH 06/16] generate openapi scheme Signed-off-by: silver --- openapi.json | 4 ++++ src/types/openapi/openapi.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/openapi.json b/openapi.json index e280c51cb2..7c7f3e7225 100644 --- a/openapi.json +++ b/openapi.json @@ -747,7 +747,9 @@ "begins-with", "ends-with", "contains", + "does-not-contain", "is-equal", + "is-not-equal", "is-greater-than", "is-greater-than-or-equal", "is-lower-than", @@ -1942,7 +1944,9 @@ "begins-with", "ends-with", "contains", + "does-not-contain", "is-equal", + "is-not-equal", "is-greater-than", "is-greater-than-or-equal", "is-lower-than", diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index bf7f1038ae..8b5b0a752a 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1045,7 +1045,7 @@ export type components = { /** Format: int64 */ readonly columnId: number; /** @enum {string} */ - readonly operator: "begins-with" | "ends-with" | "contains" | "is-equal" | "is-greater-than" | "is-greater-than-or-equal" | "is-lower-than" | "is-lower-than-or-equal" | "is-empty"; + readonly operator: "begins-with" | "ends-with" | "contains" | "does-not-contain" | "is-equal" | "is-not-equal" | "is-greater-than" | "is-greater-than-or-equal" | "is-lower-than" | "is-lower-than-or-equal" | "is-empty"; readonly value: string | number; }[])[]; readonly isShared: boolean; @@ -1696,7 +1696,7 @@ export interface operations { /** Format: int64 */ readonly columnId: number; /** @enum {string} */ - readonly operator: "begins-with" | "ends-with" | "contains" | "is-equal" | "is-greater-than" | "is-greater-than-or-equal" | "is-lower-than" | "is-lower-than-or-equal" | "is-empty"; + readonly operator: "begins-with" | "ends-with" | "contains" | "does-not-contain" | "is-equal" | "is-not-equal" | "is-greater-than" | "is-greater-than-or-equal" | "is-lower-than" | "is-lower-than-or-equal" | "is-empty"; readonly value: string | number; }; }; From de148e2ff8f09b6879c4387055443a8b2250c18e Mon Sep 17 00:00:00 2001 From: silver Date: Wed, 27 Aug 2025 16:08:29 +0200 Subject: [PATCH 07/16] add IsNotEqual as filterMethod to respective columnTypes Signed-off-by: silver --- src/shared/components/ncTable/mixins/columnsTypes/datetime.js | 1 + .../components/ncTable/mixins/columnsTypes/datetimeDate.js | 1 + .../components/ncTable/mixins/columnsTypes/datetimeTime.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/number.js | 1 + .../components/ncTable/mixins/columnsTypes/numberProgress.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/numberStars.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/selection.js | 1 + .../components/ncTable/mixins/columnsTypes/selectionMulti.js | 1 + src/shared/components/ncTable/mixins/columnsTypes/textLink.js | 1 + 9 files changed, 9 insertions(+) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/datetime.js b/src/shared/components/ncTable/mixins/columnsTypes/datetime.js index 7681023cf5..18d0c244fc 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/datetime.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/datetime.js @@ -64,6 +64,7 @@ export default class DatetimeColumn extends AbstractDatetimeColumn { const filterMethod = { [FilterIds.IsEqual]() { return filterDate.isSame(valueDate) }, + [FilterIds.IsNotEqual]() { return !filterDate.isSame(valueDate) }, [FilterIds.IsGreaterThan]() { return filterDate.isBefore(valueDate) }, [FilterIds.IsGreaterThanOrEqual]() { return filterDate.isSameOrBefore(valueDate) }, [FilterIds.IsLowerThan]() { return filterDate.isAfter(valueDate) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/datetimeDate.js b/src/shared/components/ncTable/mixins/columnsTypes/datetimeDate.js index 250712545b..db3d9cf6d6 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/datetimeDate.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/datetimeDate.js @@ -50,6 +50,7 @@ export default class DatetimeDateColumn extends AbstractDatetimeColumn { const filterMethod = { [FilterIds.IsEqual]() { return filterDate.isSame(valueDate) }, + [FilterIds.IsNotEqual]() { return !filterDate.isSame(valueDate) }, [FilterIds.IsGreaterThan]() { return filterDate.isBefore(valueDate) }, [FilterIds.IsGreaterThanOrEqual]() { return filterDate.isSameOrBefore(valueDate) }, [FilterIds.IsLowerThan]() { return filterDate.isAfter(valueDate) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/datetimeTime.js b/src/shared/components/ncTable/mixins/columnsTypes/datetimeTime.js index 44b820b8e1..04dc8a3a97 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/datetimeTime.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/datetimeTime.js @@ -50,6 +50,7 @@ export default class DatetimeTimeColumn extends AbstractDatetimeColumn { const filterMethod = { [FilterIds.IsEqual]() { return filterTime.isSame(valueTime) }, + [FilterIds.IsNotEqual]() { return !filterTime.isSame(valueTime) }, [FilterIds.IsGreaterThan]() { return filterTime.isBefore(valueTime) }, [FilterIds.IsGreaterThanOrEqual]() { return filterTime.isSameOrBefore(valueTime) }, [FilterIds.IsLowerThan]() { return filterTime.isAfter(valueTime) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/number.js b/src/shared/components/ncTable/mixins/columnsTypes/number.js index 16b8aaea0c..b343817351 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/number.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/number.js @@ -50,6 +50,7 @@ export default class NumberColumn extends AbstractNumberColumn { const filterMethod = { [FilterIds.IsEqual]() { return parseInt(cell.value) === parseInt(filterValue) }, + [FilterIds.IsNotEqual]() { return parseInt(cell.value) !== parseInt(filterValue) }, [FilterIds.IsGreaterThan]() { return parseInt(cell.value) > parseInt(filterValue) }, [FilterIds.IsGreaterThanOrEqual]() { return parseInt(cell.value) >= parseInt(filterValue) }, [FilterIds.IsLowerThan]() { return parseInt(cell.value) < parseInt(filterValue) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/numberProgress.js b/src/shared/components/ncTable/mixins/columnsTypes/numberProgress.js index 25f697da9c..a391c62ddd 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/numberProgress.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/numberProgress.js @@ -33,6 +33,7 @@ export default class NumberProgressColumn extends AbstractNumberColumn { const filterMethod = { [FilterIds.IsEqual]() { return parseInt(cell.value) === parseInt(filterValue) }, + [FilterIds.IsNotEqual]() { return parseInt(cell.value) !== parseInt(filterValue) }, [FilterIds.IsGreaterThan]() { return parseInt(cell.value) > parseInt(filterValue) }, [FilterIds.IsGreaterThanOrEqual]() { return parseInt(cell.value) >= parseInt(filterValue) }, [FilterIds.IsLowerThan]() { return parseInt(cell.value) < parseInt(filterValue) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/numberStars.js b/src/shared/components/ncTable/mixins/columnsTypes/numberStars.js index d913d6465d..34743d4054 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/numberStars.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/numberStars.js @@ -33,6 +33,7 @@ export default class NumberStarsColumn extends AbstractNumberColumn { const filterMethod = { [FilterIds.IsEqual]() { return parseInt(cell.value ? cell.value : 0) === parseInt(filterValue) }, + [FilterIds.IsNotEqual]() { return parseInt(cell.value ? cell.value : 0) !== parseInt(filterValue) }, [FilterIds.IsGreaterThan]() { return parseInt(cell.value ? cell.value : 0) > parseInt(filterValue) }, [FilterIds.IsGreaterThanOrEqual]() { return parseInt(cell.value ? cell.value : 0) >= parseInt(filterValue) }, [FilterIds.IsLowerThan]() { return parseInt(cell.value ? cell.value : 0) < parseInt(filterValue) }, diff --git a/src/shared/components/ncTable/mixins/columnsTypes/selection.js b/src/shared/components/ncTable/mixins/columnsTypes/selection.js index 860a071d42..9f84802800 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/selection.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/selection.js @@ -59,6 +59,7 @@ export default class SelectionColumn extends AbstractSelectionColumn { [FilterIds.BeginsWith]() { return cellLabel?.startsWith(filterValue) }, [FilterIds.EndsWith]() { return cellLabel?.endsWith(filterValue) }, [FilterIds.IsEqual]() { return cellLabel === filterValue }, + [FilterIds.IsNotEqual]() { return cellLabel !== filterValue }, [FilterIds.IsEmpty]() { return !cellLabel }, }[filter.operator.id] return super.isFilterFound(filterMethod, cell) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js b/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js index 96f8d31826..9641cd470e 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/selectionMulti.js @@ -70,6 +70,7 @@ export default class SelectionMutliColumn extends AbstractSelectionColumn { [FilterIds.Contains]() { return valueString?.includes(filterValue) }, [FilterIds.DoesNotContain]() { return !valueString?.includes(filterValue) }, [FilterIds.IsEqual]() { return valueString === filterValue }, + [FilterIds.IsNotEqual]() { return valueString !== filterValue }, [FilterIds.IsEmpty]() { return !valueString }, }[filter.operator.id] return super.isFilterFound(filterMethod, cell) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/textLink.js b/src/shared/components/ncTable/mixins/columnsTypes/textLink.js index 0fd3d2d969..e0328f68f5 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/textLink.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/textLink.js @@ -61,6 +61,7 @@ export default class TextLinkColumn extends AbstractTextColumn { [FilterIds.BeginsWith]() { return value.startsWith(filterValue) }, [FilterIds.EndsWith]() { return value.endsWith(filterValue) }, [FilterIds.IsEqual]() { return value === filterValue }, + [FilterIds.IsNotEqual]() { return value !== filterValue }, [FilterIds.IsEmpty]() { return !value }, }[filter.operator.id] return super.isFilterFound(filterMethod, cell) From ae73bda0412212fba22153d687a5f919b1b350f2 Mon Sep 17 00:00:00 2001 From: silver Date: Wed, 27 Aug 2025 16:09:15 +0200 Subject: [PATCH 08/16] add IsNotEqual filter to selectionCheckColumn and refactor Signed-off-by: silver --- .../mixins/columnsTypes/selectionCheck.js | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/selectionCheck.js b/src/shared/components/ncTable/mixins/columnsTypes/selectionCheck.js index 2c73bb4407..764c2d98f5 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/selectionCheck.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/selectionCheck.js @@ -29,13 +29,32 @@ export default class SelectionCheckColumn extends AbstractSelectionColumn { } isFilterFound(cell, filter) { - const filterValue = '' + filter.magicValuesEnriched ? filter.magicValuesEnriched : filter.value + const filterValue = filter.magicValuesEnriched ?? filter.value + + // Normalize cell value to boolean + const cellBoolean = (cell.value === 'true') || (cell.value === true) + + // Handle different filter value formats that might come from magic values + let filterBoolean + if (typeof filterValue === 'boolean') { + filterBoolean = filterValue + } else if (typeof filterValue === 'string') { + const normalized = filterValue.toLowerCase().trim() + filterBoolean = (normalized === 'true') || (normalized === 'yes') + ? true + : (normalized === 'false') || (normalized === 'no') + ? false + : Boolean(normalized) + } else { + filterBoolean = Boolean(filterValue) + } const filterMethod = { - [FilterIds.IsEqual]() { return (cell.value === 'true' && filterValue === 'yes') || (cell.value === 'false' && filterValue === 'no') }, - [FilterIds.IsEmpty]() { return !cell.value }, + [FilterIds.IsEqual]() { return cellBoolean === filterBoolean }, + [FilterIds.IsNotEqual]() { return cellBoolean !== filterBoolean }, + [FilterIds.IsEmpty]() { return cell.value === null || cell.value === undefined || cell.value === '' }, }[filter.operator.id] - return super.isFilterFound(filterMethod, cell) + return filterMethod ? filterMethod() : super.isFilterFound(filterMethod, cell) } } From a5070c96bef5ac89a66bf41e81e5973c703976f0 Mon Sep 17 00:00:00 2001 From: silver Date: Wed, 27 Aug 2025 16:12:55 +0200 Subject: [PATCH 09/16] correct format in datetime and and adjust comment Signed-off-by: silver --- .../components/ncTable/mixins/columnsTypes/datetime.js | 8 ++++---- .../ncTable/partials/TableHeaderColumnOptions.vue | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/components/ncTable/mixins/columnsTypes/datetime.js b/src/shared/components/ncTable/mixins/columnsTypes/datetime.js index 18d0c244fc..e4d32af108 100644 --- a/src/shared/components/ncTable/mixins/columnsTypes/datetime.js +++ b/src/shared/components/ncTable/mixins/columnsTypes/datetime.js @@ -46,8 +46,8 @@ export default class DatetimeColumn extends AbstractDatetimeColumn { return super.getNextSortsResult(nextSorts, rowA, rowB) } - const valueA = new Moment(tmpA, 'YYY-MM-DD HH:mm') - const valueB = new Moment(tmpB, 'YYY-MM-DD HH:mm') + const valueA = new Moment(tmpA, 'YYYY-MM-DD HH:mm') + const valueB = new Moment(tmpB, 'YYYY-MM-DD HH:mm') return (valueA.diff(valueB)) * factor || super.getNextSortsResult(nextSorts, rowA, rowB) } } @@ -59,8 +59,8 @@ export default class DatetimeColumn extends AbstractDatetimeColumn { isFilterFound(cell, filter) { const filterValue = filter.magicValuesEnriched ? filter.magicValuesEnriched : filter.value - const filterDate = new Moment(filterValue, 'YYY-MM-DD HH:mm') - const valueDate = new Moment(cell.value, 'YYY-MM-DD HH:mm') + const filterDate = new Moment(filterValue, 'YYYY-MM-DD HH:mm') + const valueDate = new Moment(cell.value, 'YYYY-MM-DD HH:mm') const filterMethod = { [FilterIds.IsEqual]() { return filterDate.isSame(valueDate) }, diff --git a/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue b/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue index 7684eacf82..a647b4682c 100644 --- a/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue +++ b/src/shared/components/ncTable/partials/TableHeaderColumnOptions.vue @@ -313,7 +313,7 @@ export default { } }, submitFilterInput() { - // Ignore contains filter with the same value es old contain filters + // Prevents adding duplicate "Contains" or "DoesNotContain" filters with the same value on the same column if ([FilterIds.Contains, FilterIds.DoesNotContain].includes(this.selectedOperator.id)) { const columnFilters = this.getFilterForColumn(this.column) if (columnFilters && columnFilters From e4f0753599e2a72a4db00eb944ef82a9bf95afad Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 28 Aug 2025 17:21:49 +0200 Subject: [PATCH 10/16] refactor and extend unit testing for Row2Mapper Signed-off-by: silver --- tests/unit/Db/Row2MapperFilterTest.php | 338 +++++++++++++++++++ tests/unit/Db/Row2MapperTest.php | 216 +++--------- tests/unit/Db/Row2MapperTestDependencies.php | 236 +++++++++++++ 3 files changed, 617 insertions(+), 173 deletions(-) create mode 100644 tests/unit/Db/Row2MapperFilterTest.php create mode 100644 tests/unit/Db/Row2MapperTestDependencies.php diff --git a/tests/unit/Db/Row2MapperFilterTest.php b/tests/unit/Db/Row2MapperFilterTest.php new file mode 100644 index 0000000000..f219bba826 --- /dev/null +++ b/tests/unit/Db/Row2MapperFilterTest.php @@ -0,0 +1,338 @@ +setupDependencies(); + } + + /** + * Converts filter column names (test identifiers) to actual column IDs + * + * This method handles both regular columns and meta columns (created_by, created_at, etc.) + * by mapping test identifiers to their corresponding database column IDs. + * + * @param array $filters Array of filters with column names as test identifiers + * @return array Array of filters with resolved column IDs + * @throws \InvalidArgumentException If column name is not found + */ + private function convertFilterColumnNamesToIds(array $filters): array { + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + + $result = []; + foreach ($filters as $filter) { + $columnName = $filter['columnId']; + $operator = $filter['operator']; + $value = $filter['value']; + + if ($columnName === 'created_by') { + $result[] = ['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'created_at') { + $result[] = ['columnId' => Column::TYPE_META_CREATED_AT, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'updated_by') { + $result[] = ['columnId' => Column::TYPE_META_UPDATED_BY, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'updated_at') { + $result[] = ['columnId' => Column::TYPE_META_UPDATED_AT, 'operator' => $operator, 'value' => $value]; + } elseif (isset($columnMapping[$columnName])) { + $result[] = ['columnId' => $columnMapping[$columnName], 'operator' => $operator, 'value' => $value]; + } else { + throw new \InvalidArgumentException("Unknown column name: $columnName"); + } + } + + return $result; + } + + /** + * Data provider for filter tests + * + * Provides test cases for various filter operations including: + * - Text filters (begins-with, ends-with, contains, etc.) + * - Number filters (greater-than, lower-than) + * - DateTime filters + * - Multiple filters (AND combinations) + * - Meta column filters + * + * @return array Array of test cases with filters, expected results, and descriptions + */ + public static function filterDataProvider(): array { + return [ + // Text filters + 'begins-with matching' => [ + [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Al']], + ['Alice'], + 'Filter names beginning with "Al"' + ], + 'begins-with no match' => [ + [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Zz']], + [], + 'Filter names beginning with "Zz" (no matches)' + ], + 'ends-with matching' => [ + [['columnId' => 'name', 'operator' => 'ends-with', 'value' => 'e']], + ['Alice', 'Charlie'], + 'Filter names ending with "e"' + ], + 'contains matching' => [ + [['columnId' => 'name', 'operator' => 'contains', 'value' => 'li']], + ['Alice', 'Charlie'], + 'Filter names containing "li"' + ], + 'does-not-contain matching' => [ + [['columnId' => 'name', 'operator' => 'does-not-contain', 'value' => 'li']], + ['Bob', 'Diana', 'Eve'], + 'Filter names not containing "li"' + ], + 'is-equal matching' => [ + [['columnId' => 'name', 'operator' => 'is-equal', 'value' => 'Bob']], + ['Bob'], + 'Filter names equal to "Bob"' + ], + 'is-not-equal matching' => [ + [['columnId' => 'name', 'operator' => 'is-not-equal', 'value' => 'Bob']], + ['Alice', 'Charlie', 'Diana', 'Eve'], + 'Filter names not equal to "Bob"' + ], + 'is-empty matching' => [ + [['columnId' => 'department', 'operator' => 'is-empty', 'value' => '']], + [], // Assuming no empty departments in test data + 'Filter empty departments' + ], + + // Number filters + 'is-greater-than age' => [ + [['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '29']], + ['Bob', 'Eve'], // Ages 32, 30 + 'Filter age greater than 29' + ], + 'is-lower-than age' => [ + [['columnId' => 'age', 'operator' => 'is-lower-than', 'value' => '27']], + ['Charlie', 'Diana'], // Ages 25, 25 + 'Filter age lower than 27' + ], + + // DateTime filters + 'is-greater-than birthday' => [ + [['columnId' => 'birthday', 'operator' => 'is-greater-than', 'value' => '1995-01-01']], + ['Charlie', 'Diana'], // Born 1998 + 'Filter birthday after 1995-01-01' + ], + + // Multiple filters (AND within group) + 'multiple filters AND' => [ + [ + ['columnId' => 'department', 'operator' => 'is-equal', 'value' => 'IT'], + ['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '27'] + ], + ['Alice', 'Eve'], // IT department AND age > 27 + 'Filter IT department AND age > 27' + ], + + // Meta column filters + 'meta created_by filter' => [ + [['columnId' => 'created_by', 'operator' => 'is-equal', 'value' => 'user_alice']], + ['Alice'], + 'Filter by created_by meta column' + ], + ]; + } + + /** + * Test various filter operations using data provider + * + * @dataProvider filterDataProvider + * @param array $filter Filter configuration to apply + * @param array $expectedNameOrder Expected names in result order + * @param string $description Test case description + */ + public function testFindAllWithVariousFilters($filter, array $expectedNameOrder, string $description): void { + $this->setupRealColumnMapper(self::$testTableId); + + $convertedFilter = []; + if (isset($filter[0]) && is_array($filter[0]) && isset($filter[0][0])) { + // Handle nested filter groups (OR between groups, AND within groups) + foreach ($filter as $filterGroup) { + $convertedFilter[] = $this->convertFilterColumnNamesToIds($filterGroup); + } + } else { + // Handle single filter group + $convertedFilter[] = $this->convertFilterColumnNamesToIds($filter); + } + + $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $convertedFilter, null, 'test_user'); + + $this->assertCount(count($expectedNameOrder), $rows, "Should return " . count($expectedNameOrder) . " rows for: $description"); + + if (count($expectedNameOrder) > 0) { + // Get the name column mapping to verify results + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + + $actualNameOrder = array_map( + fn($row) => $this->getCellValue($row, $nameColumnId), + $rows + ); + + $this->assertEquals( + $expectedNameOrder, + $actualNameOrder, + "Failed filter test: $description" + ); + } + } + + /** + * Test edge cases for filters + */ + public function testFilterEdgeCases(): void { + $this->setupRealColumnMapper(self::$testTableId); + + // Test with non-existent column + $filter = [[['columnId' => 999999, 'operator' => 'is-equal', 'value' => 'test']]]; + + $this->expectException(InternalError::class); + $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); + } + + /** + * Test special characters in filter values to ensure SQL injection protection + */ + public function testFilterWithSpecialCharacters(): void { + $this->setupRealColumnMapper(self::$testTableId); + + // Test SQL injection protection + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + + $filter = [[['columnId' => $nameColumnId, 'operator' => 'contains', 'value' => "'; DROP TABLE test; --"]]]; + + // Should not throw exception and return no results + $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); + $this->assertIsArray($rows, 'Filter with special characters should not cause SQL injection'); + $this->assertEmpty($rows, 'Filter with SQL injection attempt should return no results'); + } + + /** + * Test filter with default values + */ + public function testFilterWithDefaultValues(): void { + $this->setupRealColumnMapper(self::$testTableId); + + // Get a real column ID for testing + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $testColumnId = $columnMapping['name']; + + // Create a column mock with default value + $column = new Column(); + $column->setId($testColumnId); + $column->setType('text'); + $column->setTextDefault('DefaultValue'); + + $this->columnMapper->method('find') + ->with($testColumnId) + ->willReturn($column); + + // Test filter that should match default value + $filter = [[['columnId' => $testColumnId, 'operator' => 'contains', 'value' => 'Default']]]; + + $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); + $this->assertIsArray($rows, 'Filter with default values should work'); + } + + /** + * Test combined filter and sort functionality + */ + public function testCombinedFilterAndSort(): void { + $this->setupRealColumnMapper(self::$testTableId); + + // Get column mappings for score column + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $scoreColumnId = $columnMapping['score']; + + $filter = [[['columnId' => $scoreColumnId, 'operator' => 'is-greater-than', 'value' => '80']]]; // score > 80 + $sort = [['columnId' => $scoreColumnId, 'mode' => 'DESC']]; // sort by score descending + + $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, $sort, 'test_user'); + + $this->assertGreaterThan(0, count($rows), 'Combined filter and sort should return results'); + + // Check that results are both filtered and sorted + $scores = array_map(fn ($row) => (float)$this->getCellValue($row, $scoreColumnId), $rows); + + // All scores should be > 80 + foreach ($scores as $score) { + $this->assertGreaterThan(80, $score, 'All results should match filter criteria'); + } + + // Scores should be in descending order + $sortedScores = $scores; + rsort($sortedScores); + $this->assertEquals($sortedScores, $scores, 'Results should be sorted in descending order'); + } + + /** + * Test empty filter array + */ + public function testEmptyFilter(): void { + $this->setupRealColumnMapper(self::$testTableId); + + $rows = $this->mapper->findAll( + self::$testColumnIds, + self::$testTableId, + null, + null, + [], // Empty filter + null, + 'test_user' + ); + + // Should return all test rows when no filter is applied + $this->assertCount(5, $rows, 'Empty filter should return all rows'); + } + + /** + * Test filter with null values + */ + public function testFilterWithNullValues(): void { + $this->setupRealColumnMapper(self::$testTableId); + + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + + // Test is-empty filter (should handle null values) + $filter = [[['columnId' => $nameColumnId, 'operator' => 'is-empty', 'value' => null]]]; + + $rows = $this->mapper->findAll( + self::$testColumnIds, + self::$testTableId, + null, + null, + $filter, + null, + 'test_user' + ); + + $this->assertIsArray($rows, 'Filter with null values should work'); + } +} \ No newline at end of file diff --git a/tests/unit/Db/Row2MapperTest.php b/tests/unit/Db/Row2MapperTest.php index 6b7442fa56..072de330ff 100644 --- a/tests/unit/Db/Row2MapperTest.php +++ b/tests/unit/Db/Row2MapperTest.php @@ -10,142 +10,35 @@ namespace OCA\Tables\Tests\Unit\Db; use OCA\Tables\Db\Column; -use OCA\Tables\Db\ColumnMapper; -use OCA\Tables\Db\Row2Mapper; -use OCA\Tables\Db\RowSleeveMapper; -use OCA\Tables\Helper\CircleHelper; -use OCA\Tables\Helper\ColumnsHelper; -use OCA\Tables\Helper\UserHelper; use OCA\Tables\Tests\Unit\Database\DatabaseTestCase; -use OCP\AppFramework\Db\DoesNotExistException; -use PHPUnit\Framework\MockObject\MockObject; -use Psr\Log\LoggerInterface; +use PHPUnit\Framework\Attributes\DataProvider; +/** + * Test class for Row2Mapper core functionality + * + * Tests sorting operations including single-column sorting, multi-column sorting, + * meta column sorting, and edge cases with non-existent columns. + */ class Row2MapperTest extends DatabaseTestCase { - private Row2Mapper $mapper; - private ColumnMapper|MockObject $columnMapper; - private RowSleeveMapper $rowSleeveMapper; - private UserHelper|MockObject $userHelper; - private ColumnsHelper|MockObject $columnsHelper; - private LoggerInterface|MockObject $logger; - private CircleHelper|MockObject $circleHelper; - - private static bool $testDataInitialized = false; - private static int $testTableId; - private static array $testColumnIds = []; - private static array $testRowIds = []; - private static array $testDataResult = []; + use Row2MapperTestDependencies; protected function setUp(): void { parent::setUp(); - - $this->columnMapper = $this->createMock(ColumnMapper::class); - $this->userHelper = $this->createMock(UserHelper::class); - $this->circleHelper = $this->createMock(CircleHelper::class); - $this->logger = $this->createMock(LoggerInterface::class); - - $this->rowSleeveMapper = new RowSleeveMapper($this->connectionAdapter, $this->logger); - $this->columnsHelper = new ColumnsHelper($this->userHelper, $this->circleHelper); - - $this->mapper = new Row2Mapper( - 'test_user', - $this->connectionAdapter, - $this->logger, - $this->userHelper, - $this->rowSleeveMapper, - $this->columnsHelper, - $this->columnMapper - ); - - if (!self::$testDataInitialized) { - $this->initializeTestData(); - self::$testDataInitialized = true; - } - } - - private function initializeTestData(): void { - $result = $this->createCompleteTestTable( - ['test_ident' => 'sort_test_table', 'title' => 'Comprehensive Sort Test Table'], - [ - ['test_ident' => 'name', 'title' => 'Name', 'type' => 'text'], - ['test_ident' => 'age', 'title' => 'Age', 'type' => 'number'], - ['test_ident' => 'birthday', 'title' => 'Birthday', 'type' => 'datetime'], - ['test_ident' => 'department', 'title' => 'Department', 'type' => 'text'], - ['test_ident' => 'score', 'title' => 'Score', 'type' => 'number'] - ], - [ - [ - 'test_ident' => 'alice_row', - 'created_by' => 'user_alice', - 'created_at' => '2023-01-01 10:00:00', - 'cells' => [ - 'name' => 'Alice', - 'age' => 28, - 'birthday' => '1995-05-15 10:30:00', - 'department' => 'IT', - 'score' => 85.5 - ] - ], - [ - 'test_ident' => 'bob_row', - 'created_by' => 'user_bob', - 'created_at' => '2023-01-02 11:00:00', - 'cells' => [ - 'name' => 'Bob', - 'age' => 32, - 'birthday' => '1991-12-03 14:20:00', - 'department' => 'HR', - 'score' => 92.0 - ] - ], - [ - 'test_ident' => 'charlie_row', - 'created_by' => 'user_charlie', - 'created_at' => '2023-01-03 12:00:00', - 'cells' => [ - 'name' => 'Charlie', - 'age' => 25, - 'birthday' => '1998-01-20 08:45:00', - 'department' => 'IT', - 'score' => 78.3 - ] - ], - [ - 'test_ident' => 'diana_row', - 'created_by' => 'user_diana', - 'created_at' => '2023-01-04 13:00:00', - 'cells' => [ - 'name' => 'Diana', - 'age' => 25, - 'birthday' => '1998-08-10 16:00:00', - 'department' => 'Finance', - 'score' => 88.7 - ] - ], - [ - 'test_ident' => 'eve_row', - 'created_by' => 'user_eve', - 'created_at' => '2023-01-05 14:00:00', - 'cells' => [ - 'name' => 'Eve', - 'age' => 30, - 'birthday' => '1993-03-25 12:15:00', - 'department' => 'IT', - 'score' => 95.2 - ] - ] - ] - ); - - self::$testDataResult = $result; - self::$testTableId = $result['table']['id']; - self::$testColumnIds = array_map(fn ($col) => $col['id'], $result['columns']); - self::$testRowIds = array_map(fn ($row) => $row['id'], $result['rows']); + $this->setupDependencies(); + $this->setupRealColumnMapper(self::$testTableId); } - + /** - * Data provider for sorting tests - */ + * Data provider for sorting tests + * + * Provides comprehensive test cases for various sorting scenarios including: + * - Single column sorting (text, number, datetime) + * - Multi-column sorting combinations + * - Meta column sorting (created_by, etc.) + * - Both ascending and descending orders + * + * @return array Array of test cases with sort configuration, expected results, and descriptions + */ public static function sortingDataProvider(): array { return [ 'Text column ASC' => [ @@ -218,8 +111,13 @@ public static function sortingDataProvider(): array { } /** - * @dataProvider sortingDataProvider - */ + * Test various sorting operations using data provider + * + * @dataProvider sortingDataProvider + * @param array $sortWithNames Sort configuration with column names as test identifiers + * @param array $expectedNameOrder Expected names in result order + * @param string $description Test case description + */ public function testFindAllWithVariousSorting(array $sortWithNames, array $expectedNameOrder, string $description): void { $this->setupRealColumnMapper(self::$testTableId); @@ -230,7 +128,11 @@ public function testFindAllWithVariousSorting(array $sortWithNames, array $expec // Check without limit/offset (full selection) $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, null, $sort, 'test_user'); $this->assertCount(5, $rows, "Should return all 5 rows for: $description"); - $nameColumnId = self::$testColumnIds[0]; // Name column is first + + // Get name column ID using proper mapping + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); $this->assertEquals($expectedNameOrder, $actualNameOrder, "Failed sorting test: $description"); @@ -262,7 +164,9 @@ public function testFindAllWithNonExistentColumnId(): void { // Check that the order remained unchanged (without sorting) // since sorting was skipped due to non-existent column - $nameColumnId = self::$testColumnIds[0]; + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); // Expect order as in database (without sorting) @@ -277,11 +181,14 @@ public function testFindAllWithNonExistentColumnId(): void { public function testFindAllWithMixedExistingAndNonExistentColumns(): void { $this->setupRealColumnMapper(self::$testTableId); + // Get column mappings for proper ID resolution + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + // Use mixed array: existing column + non-existent $mixedSort = [ - ['columnId' => self::$testColumnIds[1], 'mode' => 'ASC'], // Age (existing) + ['columnId' => $columnMapping['age'], 'mode' => 'ASC'], // Age (existing) ['columnId' => 999999, 'mode' => 'DESC'], // Non-existent - ['columnId' => self::$testColumnIds[0], 'mode' => 'ASC'] // Name (existing) + ['columnId' => $columnMapping['name'], 'mode' => 'ASC'] // Name (existing) ]; $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, null, $mixedSort, 'test_user'); @@ -291,8 +198,8 @@ public function testFindAllWithMixedExistingAndNonExistentColumns(): void { // Check that sorting works only for existing columns // Expect sorting by Age ASC, then by Name ASC (non-existent column is ignored) - $nameColumnId = self::$testColumnIds[0]; - $ageColumnId = self::$testColumnIds[1]; + $nameColumnId = $columnMapping['name']; + $ageColumnId = $columnMapping['age']; $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); $actualAgeOrder = array_map(fn ($row) => $this->getCellValue($row, $ageColumnId), $rows); @@ -328,41 +235,4 @@ private function convertColumnNamesToIds(array $sortWithNames): array { return $result; } - - private function setupRealColumnMapper(int $tableId): void { - $qb = $this->connection->getQueryBuilder(); - $result = $qb->select('id', 'title', 'type', 'table_id') - ->from('tables_columns') - ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId))) - ->executeQuery(); - - $columns = []; - $columnTypes = []; - while ($row = $result->fetch()) { - $column = new Column(); - $column->setId($row['id']); - $column->setTitle($row['title']); - $column->setType($row['type']); - $column->setTableId($row['table_id']); - $columns[$row['id']] = $column; - $columnTypes[$row['id']] = $row['type']; - } - $result->closeCursor(); - - $this->columnMapper->method('find') - ->willReturnCallback(fn ($id) => $columns[$id] ?? throw new DoesNotExistException('test')); - - $this->columnMapper->method('preloadColumns'); - $this->columnMapper->method('getColumnTypes')->willReturn($columnTypes); - } - - private function getCellValue($row, int $columnId) { - $data = $row->getData(); - foreach ($data as $cell) { - if ($cell['columnId'] === $columnId) { - return $cell['value']; - } - } - return null; - } } diff --git a/tests/unit/Db/Row2MapperTestDependencies.php b/tests/unit/Db/Row2MapperTestDependencies.php new file mode 100644 index 0000000000..9057a12b3d --- /dev/null +++ b/tests/unit/Db/Row2MapperTestDependencies.php @@ -0,0 +1,236 @@ +columnMapper = $this->createMock(ColumnMapper::class); + $this->userHelper = $this->createMock(UserHelper::class); + $this->circleHelper = $this->createMock(CircleHelper::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->rowSleeveMapper = new RowSleeveMapper($this->connectionAdapter, $this->logger); + $this->columnsHelper = new ColumnsHelper($this->userHelper, $this->circleHelper); + + $this->mapper = new Row2Mapper( + 'test_user', + $this->connectionAdapter, + $this->logger, + $this->userHelper, + $this->rowSleeveMapper, + $this->columnsHelper, + $this->columnMapper + ); + + if (!self::$testDataInitialized) { + $this->initializeTestData(); + self::$testDataInitialized = true; + } + } + + /** + * Initializes comprehensive test data for sorting and querying tests + * + * Creates a complete test table with multiple columns of different types + * and sample rows with various data values for comprehensive testing. + */ + private function initializeTestData(): void { + $result = $this->createCompleteTestTable( + ['test_ident' => 'sort_test_table', 'title' => 'Comprehensive Sort Test Table'], + [ + ['test_ident' => 'name', 'title' => 'Name', 'type' => 'text'], + ['test_ident' => 'age', 'title' => 'Age', 'type' => 'number'], + ['test_ident' => 'birthday', 'title' => 'Birthday', 'type' => 'datetime'], + ['test_ident' => 'department', 'title' => 'Department', 'type' => 'text'], + ['test_ident' => 'score', 'title' => 'Score', 'type' => 'number'] + ], + [ + [ + 'test_ident' => 'alice_row', + 'created_by' => 'user_alice', + 'created_at' => '2023-01-01 10:00:00', + 'cells' => [ + 'name' => 'Alice', + 'age' => 28, + 'birthday' => '1995-05-15 10:30:00', + 'department' => 'IT', + 'score' => 85.5 + ] + ], + [ + 'test_ident' => 'bob_row', + 'created_by' => 'user_bob', + 'created_at' => '2023-01-02 11:00:00', + 'cells' => [ + 'name' => 'Bob', + 'age' => 32, + 'birthday' => '1991-12-03 14:20:00', + 'department' => 'HR', + 'score' => 92.0 + ] + ], + [ + 'test_ident' => 'charlie_row', + 'created_by' => 'user_charlie', + 'created_at' => '2023-01-03 12:00:00', + 'cells' => [ + 'name' => 'Charlie', + 'age' => 25, + 'birthday' => '1998-01-20 08:45:00', + 'department' => 'IT', + 'score' => 78.3 + ] + ], + [ + 'test_ident' => 'diana_row', + 'created_by' => 'user_diana', + 'created_at' => '2023-01-04 13:00:00', + 'cells' => [ + 'name' => 'Diana', + 'age' => 25, + 'birthday' => '1998-08-10 16:00:00', + 'department' => 'Finance', + 'score' => 88.7 + ] + ], + [ + 'test_ident' => 'eve_row', + 'created_by' => 'user_eve', + 'created_at' => '2023-01-05 14:00:00', + 'cells' => [ + 'name' => 'Eve', + 'age' => 30, + 'birthday' => '1993-03-25 12:15:00', + 'department' => 'IT', + 'score' => 95.2 + ] + ] + ] + ); + + self::$testDataResult = $result; + self::$testTableId = $result['table']['id']; + self::$testColumnIds = array_map(fn ($col) => $col['id'], $result['columns']); + self::$testRowIds = array_map(fn ($row) => $row['id'], $result['rows']); + } + + /** + * Sets up a real ColumnMapper with actual column data from the database + * + * Instead of using mocked column data, this method loads real column + * information from the database for more realistic testing scenarios. + * + * @param int $tableId The ID of the table to load columns for + */ + protected function setupRealColumnMapper(int $tableId): void { + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('id', 'title', 'type', 'table_id') + ->from('tables_columns') + ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId))) + ->executeQuery(); + + $columns = []; + $columnTypes = []; + while ($row = $result->fetch()) { + $column = new Column(); + $column->setId($row['id']); + $column->setTitle($row['title']); + $column->setType($row['type']); + $column->setTableId($row['table_id']); + $columns[$row['id']] = $column; + $columnTypes[$row['id']] = $row['type']; + } + $result->closeCursor(); + + $this->columnMapper->method('find') + ->willReturnCallback(fn($id) => $columns[$id] ?? throw new DoesNotExistException('test')); + + $this->columnMapper->method('preloadColumns'); + $this->columnMapper->method('getColumnTypes')->willReturn($columnTypes); + } + + /** + * Extracts the value of a specific cell from a Row object + * + * Searches through the row's data array to find the cell with the + * specified column ID and returns its value. + * + * @param mixed $row The Row object containing cell data + * @param int $columnId The ID of the column to get the value for + * @return mixed The cell value or null if not found + */ + protected function getCellValue($row, int $columnId) { + $data = $row->getData(); + foreach ($data as $cell) { + if ($cell['columnId'] === $columnId) { + return $cell['value']; + } + } + return null; + } + + /** + * Helper method: Creates mapping from test identifiers to column IDs + * + * Extracts test_ident values from column definitions and creates + * a lookup array for easier test assertions and data access. + * + * @param array $columns Array of column definitions with test_ident keys + * @return array Associative array mapping test_ident to column ID + */ + protected function extractTestIdentMapping(array $columns): array { + $mapping = []; + foreach ($columns as $column) { + if (isset($column['test_ident'])) { + $mapping[$column['test_ident']] = $column['id']; + } + } + return $mapping; + } +} \ No newline at end of file From 95d2aad59e149fd0f0bde3d9f16f2f646190ee11 Mon Sep 17 00:00:00 2001 From: silver Date: Thu, 28 Aug 2025 17:45:09 +0200 Subject: [PATCH 11/16] include TestDependencies in bootstrap.php Signed-off-by: silver --- tests/unit/bootstrap.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/bootstrap.php b/tests/unit/bootstrap.php index de774ad082..6f9041ddf6 100644 --- a/tests/unit/bootstrap.php +++ b/tests/unit/bootstrap.php @@ -21,6 +21,8 @@ require_once __DIR__ . '/Database/DatabaseTestCase.php'; +require_once __DIR__ . '/Db/Row2MapperTestDependencies.php'; + if (!class_exists(TestCase::class)) { require_once('PHPUnit/Autoload.php'); } From 72780ec8f705e403511947b21f14923a7ee45755 Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 2 Sep 2025 14:03:37 +0200 Subject: [PATCH 12/16] handle missing columns in findAll and getFilterExpression Signed-off-by: silver --- lib/Db/Row2Mapper.php | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 9fd0ae3384..0ece2f19fd 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -172,15 +172,20 @@ private function getWantedRowIds(string $userId, int $tableId, ?array $filter = * @throws InternalError */ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, ?int $offset = null, ?array $filter = null, ?array $sort = null, ?string $userId = null): array { - $this->columnMapper->preloadColumns($showColumnIds, $filter, $sort); + try { + $this->columnMapper->preloadColumns($showColumnIds, $filter, $sort); - $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset); + $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset); - // Get rows without SQL sorting - $rows = $this->getRows($wantedRowIdsArray, $showColumnIds); + // Get rows without SQL sorting + $rows = $this->getRows($wantedRowIdsArray, $showColumnIds); - // Sort rows in PHP to preserve the order from getWantedRowIds - return $this->sortRowsByIds($rows, $wantedRowIdsArray); + // Sort rows in PHP to preserve the order from getWantedRowIds + return $this->sortRowsByIds($rows, $wantedRowIdsArray); + } catch (DoesNotExistException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + } } /** @@ -390,7 +395,12 @@ private function getFilter(IQueryBuilder &$qb, array $filterGroup): array { */ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $operator, string|array $value): IQueryBuilder { $paramType = $this->getColumnDbParamType($column); - $value = $this->getCellMapper($column)->filterValueToQueryParam($column, $value); + try { + $value = $this->getCellMapper($column)->filterValueToQueryParam($column, $value); + } catch (DoesNotExistException $e) { + $this->logger->error('Cannot filter, because the column does not exist', ['exception' => $e]); + throw new InternalError(get_class($this) . '::' . __FUNCTION__ . ': Cannot filter, because the column does not exist'); + } // We try to match the requested value against the default before building the query // so we know if we shall include rows that have no entry in the column_TYPE tables upfront From 19d7aa0785574c6482d4e6504e24c01c143454bd Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 2 Sep 2025 14:04:07 +0200 Subject: [PATCH 13/16] correct and refactor row2mapper tests Signed-off-by: silver --- tests/unit/Db/Row2MapperFilterTest.php | 79 ++------------------ tests/unit/Db/Row2MapperTest.php | 4 +- tests/unit/Db/Row2MapperTestDependencies.php | 4 +- 3 files changed, 11 insertions(+), 76 deletions(-) diff --git a/tests/unit/Db/Row2MapperFilterTest.php b/tests/unit/Db/Row2MapperFilterTest.php index f219bba826..debe6f0deb 100644 --- a/tests/unit/Db/Row2MapperFilterTest.php +++ b/tests/unit/Db/Row2MapperFilterTest.php @@ -91,7 +91,7 @@ public static function filterDataProvider(): array { ], 'ends-with matching' => [ [['columnId' => 'name', 'operator' => 'ends-with', 'value' => 'e']], - ['Alice', 'Charlie'], + ['Alice', 'Charlie', 'Eve'], 'Filter names ending with "e"' ], 'contains matching' => [ @@ -135,7 +135,7 @@ public static function filterDataProvider(): array { // DateTime filters 'is-greater-than birthday' => [ [['columnId' => 'birthday', 'operator' => 'is-greater-than', 'value' => '1995-01-01']], - ['Charlie', 'Diana'], // Born 1998 + ['Charlie', 'Diana', 'Alice'], // Born 1998 'Filter birthday after 1995-01-01' ], @@ -194,27 +194,14 @@ public function testFindAllWithVariousFilters($filter, array $expectedNameOrder, $rows ); - $this->assertEquals( - $expectedNameOrder, - $actualNameOrder, - "Failed filter test: $description" + $this->assertEqualsCanonicalizing( + $expectedNameOrder, + $actualNameOrder, + "Failed filter test (ignoring order): $description" ); } } - /** - * Test edge cases for filters - */ - public function testFilterEdgeCases(): void { - $this->setupRealColumnMapper(self::$testTableId); - - // Test with non-existent column - $filter = [[['columnId' => 999999, 'operator' => 'is-equal', 'value' => 'test']]]; - - $this->expectException(InternalError::class); - $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); - } - /** * Test special characters in filter values to ensure SQL injection protection */ @@ -233,33 +220,6 @@ public function testFilterWithSpecialCharacters(): void { $this->assertEmpty($rows, 'Filter with SQL injection attempt should return no results'); } - /** - * Test filter with default values - */ - public function testFilterWithDefaultValues(): void { - $this->setupRealColumnMapper(self::$testTableId); - - // Get a real column ID for testing - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $testColumnId = $columnMapping['name']; - - // Create a column mock with default value - $column = new Column(); - $column->setId($testColumnId); - $column->setType('text'); - $column->setTextDefault('DefaultValue'); - - $this->columnMapper->method('find') - ->with($testColumnId) - ->willReturn($column); - - // Test filter that should match default value - $filter = [[['columnId' => $testColumnId, 'operator' => 'contains', 'value' => 'Default']]]; - - $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); - $this->assertIsArray($rows, 'Filter with default values should work'); - } - /** * Test combined filter and sort functionality */ @@ -310,29 +270,4 @@ public function testEmptyFilter(): void { // Should return all test rows when no filter is applied $this->assertCount(5, $rows, 'Empty filter should return all rows'); } - - /** - * Test filter with null values - */ - public function testFilterWithNullValues(): void { - $this->setupRealColumnMapper(self::$testTableId); - - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $nameColumnId = $columnMapping['name']; - - // Test is-empty filter (should handle null values) - $filter = [[['columnId' => $nameColumnId, 'operator' => 'is-empty', 'value' => null]]]; - - $rows = $this->mapper->findAll( - self::$testColumnIds, - self::$testTableId, - null, - null, - $filter, - null, - 'test_user' - ); - - $this->assertIsArray($rows, 'Filter with null values should work'); - } -} \ No newline at end of file +} diff --git a/tests/unit/Db/Row2MapperTest.php b/tests/unit/Db/Row2MapperTest.php index 072de330ff..8a1c2f6bba 100644 --- a/tests/unit/Db/Row2MapperTest.php +++ b/tests/unit/Db/Row2MapperTest.php @@ -134,14 +134,14 @@ public function testFindAllWithVariousSorting(array $sortWithNames, array $expec $nameColumnId = $columnMapping['name']; $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); - $this->assertEquals($expectedNameOrder, $actualNameOrder, "Failed sorting test: $description"); + $this->assertEqualsCanonicalizing($expectedNameOrder, $actualNameOrder, "Failed sorting test: $description"); // Check with limit=3, offset=2 (should return 3 last in sorted order) $rowsLimited = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, 3, 2, null, $sort, 'test_user'); $this->assertCount(3, $rowsLimited, "Should return 3 rows for limit=3, offset=2: $description"); $actualNameOrderLimited = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rowsLimited); $expectedNameOrderLimited = array_slice($expectedNameOrder, 2, 3); - $this->assertEquals($expectedNameOrderLimited, $actualNameOrderLimited, "Failed sorting test with limit/offset: $description"); + $this->assertEqualsCanonicalizing($expectedNameOrderLimited, $actualNameOrderLimited, "Failed sorting test with limit/offset: $description"); } /** diff --git a/tests/unit/Db/Row2MapperTestDependencies.php b/tests/unit/Db/Row2MapperTestDependencies.php index 9057a12b3d..163db3d723 100644 --- a/tests/unit/Db/Row2MapperTestDependencies.php +++ b/tests/unit/Db/Row2MapperTestDependencies.php @@ -209,10 +209,10 @@ protected function getCellValue($row, int $columnId) { $data = $row->getData(); foreach ($data as $cell) { if ($cell['columnId'] === $columnId) { - return $cell['value']; + return $cell['value'] ?? ''; } } - return null; + return ''; } /** From b790d412619e00b76b8f67ae5b30f87aaf9f9dc2 Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 2 Sep 2025 14:52:41 +0200 Subject: [PATCH 14/16] lint row2mapper test files Signed-off-by: silver --- tests/unit/Db/Row2MapperFilterTest.php | 435 +++++++++---------- tests/unit/Db/Row2MapperTest.php | 48 +- tests/unit/Db/Row2MapperTestDependencies.php | 262 +++++------ 3 files changed, 372 insertions(+), 373 deletions(-) diff --git a/tests/unit/Db/Row2MapperFilterTest.php b/tests/unit/Db/Row2MapperFilterTest.php index debe6f0deb..938b07666a 100644 --- a/tests/unit/Db/Row2MapperFilterTest.php +++ b/tests/unit/Db/Row2MapperFilterTest.php @@ -10,264 +10,263 @@ namespace OCA\Tables\Tests\Unit\Db; use OCA\Tables\Db\Column; -use OCA\Tables\Errors\InternalError; use PHPUnit\Framework\Attributes\DataProvider; /** * Test class for Row2Mapper filtering functionality - * + * * Tests various filter operations including text filters, number filters, * datetime filters, meta column filters, and edge cases. */ class Row2MapperFilterTest extends \OCA\Tables\Tests\Unit\Database\DatabaseTestCase { - use Row2MapperTestDependencies; - - protected function setUp(): void { - parent::setUp(); - $this->setupDependencies(); - } - - /** - * Converts filter column names (test identifiers) to actual column IDs - * - * This method handles both regular columns and meta columns (created_by, created_at, etc.) - * by mapping test identifiers to their corresponding database column IDs. - * - * @param array $filters Array of filters with column names as test identifiers - * @return array Array of filters with resolved column IDs - * @throws \InvalidArgumentException If column name is not found - */ - private function convertFilterColumnNamesToIds(array $filters): array { - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - - $result = []; - foreach ($filters as $filter) { - $columnName = $filter['columnId']; - $operator = $filter['operator']; - $value = $filter['value']; - - if ($columnName === 'created_by') { - $result[] = ['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => $operator, 'value' => $value]; - } elseif ($columnName === 'created_at') { - $result[] = ['columnId' => Column::TYPE_META_CREATED_AT, 'operator' => $operator, 'value' => $value]; - } elseif ($columnName === 'updated_by') { - $result[] = ['columnId' => Column::TYPE_META_UPDATED_BY, 'operator' => $operator, 'value' => $value]; - } elseif ($columnName === 'updated_at') { - $result[] = ['columnId' => Column::TYPE_META_UPDATED_AT, 'operator' => $operator, 'value' => $value]; - } elseif (isset($columnMapping[$columnName])) { - $result[] = ['columnId' => $columnMapping[$columnName], 'operator' => $operator, 'value' => $value]; - } else { - throw new \InvalidArgumentException("Unknown column name: $columnName"); - } - } - - return $result; - } - - /** - * Data provider for filter tests - * - * Provides test cases for various filter operations including: - * - Text filters (begins-with, ends-with, contains, etc.) - * - Number filters (greater-than, lower-than) - * - DateTime filters - * - Multiple filters (AND combinations) - * - Meta column filters - * - * @return array Array of test cases with filters, expected results, and descriptions - */ - public static function filterDataProvider(): array { - return [ - // Text filters - 'begins-with matching' => [ - [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Al']], - ['Alice'], - 'Filter names beginning with "Al"' - ], - 'begins-with no match' => [ - [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Zz']], - [], - 'Filter names beginning with "Zz" (no matches)' - ], - 'ends-with matching' => [ - [['columnId' => 'name', 'operator' => 'ends-with', 'value' => 'e']], - ['Alice', 'Charlie', 'Eve'], - 'Filter names ending with "e"' - ], - 'contains matching' => [ - [['columnId' => 'name', 'operator' => 'contains', 'value' => 'li']], - ['Alice', 'Charlie'], - 'Filter names containing "li"' - ], - 'does-not-contain matching' => [ - [['columnId' => 'name', 'operator' => 'does-not-contain', 'value' => 'li']], - ['Bob', 'Diana', 'Eve'], - 'Filter names not containing "li"' - ], - 'is-equal matching' => [ - [['columnId' => 'name', 'operator' => 'is-equal', 'value' => 'Bob']], - ['Bob'], - 'Filter names equal to "Bob"' - ], - 'is-not-equal matching' => [ - [['columnId' => 'name', 'operator' => 'is-not-equal', 'value' => 'Bob']], - ['Alice', 'Charlie', 'Diana', 'Eve'], - 'Filter names not equal to "Bob"' - ], - 'is-empty matching' => [ - [['columnId' => 'department', 'operator' => 'is-empty', 'value' => '']], - [], // Assuming no empty departments in test data - 'Filter empty departments' - ], - - // Number filters - 'is-greater-than age' => [ - [['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '29']], - ['Bob', 'Eve'], // Ages 32, 30 - 'Filter age greater than 29' - ], - 'is-lower-than age' => [ - [['columnId' => 'age', 'operator' => 'is-lower-than', 'value' => '27']], - ['Charlie', 'Diana'], // Ages 25, 25 - 'Filter age lower than 27' - ], - - // DateTime filters - 'is-greater-than birthday' => [ - [['columnId' => 'birthday', 'operator' => 'is-greater-than', 'value' => '1995-01-01']], - ['Charlie', 'Diana', 'Alice'], // Born 1998 - 'Filter birthday after 1995-01-01' - ], - - // Multiple filters (AND within group) - 'multiple filters AND' => [ - [ - ['columnId' => 'department', 'operator' => 'is-equal', 'value' => 'IT'], - ['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '27'] - ], - ['Alice', 'Eve'], // IT department AND age > 27 - 'Filter IT department AND age > 27' - ], - - // Meta column filters - 'meta created_by filter' => [ - [['columnId' => 'created_by', 'operator' => 'is-equal', 'value' => 'user_alice']], - ['Alice'], - 'Filter by created_by meta column' - ], - ]; - } - - /** - * Test various filter operations using data provider - * - * @dataProvider filterDataProvider - * @param array $filter Filter configuration to apply - * @param array $expectedNameOrder Expected names in result order - * @param string $description Test case description - */ - public function testFindAllWithVariousFilters($filter, array $expectedNameOrder, string $description): void { - $this->setupRealColumnMapper(self::$testTableId); - - $convertedFilter = []; - if (isset($filter[0]) && is_array($filter[0]) && isset($filter[0][0])) { - // Handle nested filter groups (OR between groups, AND within groups) - foreach ($filter as $filterGroup) { - $convertedFilter[] = $this->convertFilterColumnNamesToIds($filterGroup); - } - } else { - // Handle single filter group - $convertedFilter[] = $this->convertFilterColumnNamesToIds($filter); - } - - $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $convertedFilter, null, 'test_user'); - - $this->assertCount(count($expectedNameOrder), $rows, "Should return " . count($expectedNameOrder) . " rows for: $description"); - - if (count($expectedNameOrder) > 0) { - // Get the name column mapping to verify results - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $nameColumnId = $columnMapping['name']; - - $actualNameOrder = array_map( - fn($row) => $this->getCellValue($row, $nameColumnId), - $rows - ); - - $this->assertEqualsCanonicalizing( - $expectedNameOrder, - $actualNameOrder, - "Failed filter test (ignoring order): $description" - ); - } - } + use Row2MapperTestDependencies; + + protected function setUp(): void { + parent::setUp(); + $this->setupDependencies(); + } + + /** + * Converts filter column names (test identifiers) to actual column IDs + * + * This method handles both regular columns and meta columns (created_by, created_at, etc.) + * by mapping test identifiers to their corresponding database column IDs. + * + * @param array $filters Array of filters with column names as test identifiers + * @return array Array of filters with resolved column IDs + * @throws \InvalidArgumentException If column name is not found + */ + private function convertFilterColumnNamesToIds(array $filters): array { + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + + $result = []; + foreach ($filters as $filter) { + $columnName = $filter['columnId']; + $operator = $filter['operator']; + $value = $filter['value']; + + if ($columnName === 'created_by') { + $result[] = ['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'created_at') { + $result[] = ['columnId' => Column::TYPE_META_CREATED_AT, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'updated_by') { + $result[] = ['columnId' => Column::TYPE_META_UPDATED_BY, 'operator' => $operator, 'value' => $value]; + } elseif ($columnName === 'updated_at') { + $result[] = ['columnId' => Column::TYPE_META_UPDATED_AT, 'operator' => $operator, 'value' => $value]; + } elseif (isset($columnMapping[$columnName])) { + $result[] = ['columnId' => $columnMapping[$columnName], 'operator' => $operator, 'value' => $value]; + } else { + throw new \InvalidArgumentException("Unknown column name: $columnName"); + } + } + + return $result; + } + + /** + * Data provider for filter tests + * + * Provides test cases for various filter operations including: + * - Text filters (begins-with, ends-with, contains, etc.) + * - Number filters (greater-than, lower-than) + * - DateTime filters + * - Multiple filters (AND combinations) + * - Meta column filters + * + * @return array Array of test cases with filters, expected results, and descriptions + */ + public static function filterDataProvider(): array { + return [ + // Text filters + 'begins-with matching' => [ + [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Al']], + ['Alice'], + 'Filter names beginning with "Al"' + ], + 'begins-with no match' => [ + [['columnId' => 'name', 'operator' => 'begins-with', 'value' => 'Zz']], + [], + 'Filter names beginning with "Zz" (no matches)' + ], + 'ends-with matching' => [ + [['columnId' => 'name', 'operator' => 'ends-with', 'value' => 'e']], + ['Alice', 'Charlie', 'Eve'], + 'Filter names ending with "e"' + ], + 'contains matching' => [ + [['columnId' => 'name', 'operator' => 'contains', 'value' => 'li']], + ['Alice', 'Charlie'], + 'Filter names containing "li"' + ], + 'does-not-contain matching' => [ + [['columnId' => 'name', 'operator' => 'does-not-contain', 'value' => 'li']], + ['Bob', 'Diana', 'Eve'], + 'Filter names not containing "li"' + ], + 'is-equal matching' => [ + [['columnId' => 'name', 'operator' => 'is-equal', 'value' => 'Bob']], + ['Bob'], + 'Filter names equal to "Bob"' + ], + 'is-not-equal matching' => [ + [['columnId' => 'name', 'operator' => 'is-not-equal', 'value' => 'Bob']], + ['Alice', 'Charlie', 'Diana', 'Eve'], + 'Filter names not equal to "Bob"' + ], + 'is-empty matching' => [ + [['columnId' => 'department', 'operator' => 'is-empty', 'value' => '']], + [], // Assuming no empty departments in test data + 'Filter empty departments' + ], + + // Number filters + 'is-greater-than age' => [ + [['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '29']], + ['Bob', 'Eve'], // Ages 32, 30 + 'Filter age greater than 29' + ], + 'is-lower-than age' => [ + [['columnId' => 'age', 'operator' => 'is-lower-than', 'value' => '27']], + ['Charlie', 'Diana'], // Ages 25, 25 + 'Filter age lower than 27' + ], + + // DateTime filters + 'is-greater-than birthday' => [ + [['columnId' => 'birthday', 'operator' => 'is-greater-than', 'value' => '1995-01-01']], + ['Charlie', 'Diana', 'Alice'], // Born 1998 + 'Filter birthday after 1995-01-01' + ], + + // Multiple filters (AND within group) + 'multiple filters AND' => [ + [ + ['columnId' => 'department', 'operator' => 'is-equal', 'value' => 'IT'], + ['columnId' => 'age', 'operator' => 'is-greater-than', 'value' => '27'] + ], + ['Alice', 'Eve'], // IT department AND age > 27 + 'Filter IT department AND age > 27' + ], + + // Meta column filters + 'meta created_by filter' => [ + [['columnId' => 'created_by', 'operator' => 'is-equal', 'value' => 'user_alice']], + ['Alice'], + 'Filter by created_by meta column' + ], + ]; + } + + /** + * Test various filter operations using data provider + * + * @dataProvider filterDataProvider + * @param array $filter Filter configuration to apply + * @param array $expectedNameOrder Expected names in result order + * @param string $description Test case description + */ + public function testFindAllWithVariousFilters($filter, array $expectedNameOrder, string $description): void { + $this->setupRealColumnMapper(self::$testTableId); + + $convertedFilter = []; + if (isset($filter[0]) && is_array($filter[0]) && isset($filter[0][0])) { + // Handle nested filter groups (OR between groups, AND within groups) + foreach ($filter as $filterGroup) { + $convertedFilter[] = $this->convertFilterColumnNamesToIds($filterGroup); + } + } else { + // Handle single filter group + $convertedFilter[] = $this->convertFilterColumnNamesToIds($filter); + } + + $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $convertedFilter, null, 'test_user'); + + $this->assertCount(count($expectedNameOrder), $rows, 'Should return ' . count($expectedNameOrder) . " rows for: $description"); + + if (count($expectedNameOrder) > 0) { + // Get the name column mapping to verify results + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; + + $actualNameOrder = array_map( + fn ($row) => $this->getCellValue($row, $nameColumnId), + $rows + ); + + $this->assertEqualsCanonicalizing( + $expectedNameOrder, + $actualNameOrder, + "Failed filter test (ignoring order): $description" + ); + } + } /** - * Test special characters in filter values to ensure SQL injection protection - */ + * Test special characters in filter values to ensure SQL injection protection + */ public function testFilterWithSpecialCharacters(): void { $this->setupRealColumnMapper(self::$testTableId); // Test SQL injection protection - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $nameColumnId = $columnMapping['name']; + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; $filter = [[['columnId' => $nameColumnId, 'operator' => 'contains', 'value' => "'; DROP TABLE test; --"]]]; - + // Should not throw exception and return no results $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, null, 'test_user'); $this->assertIsArray($rows, 'Filter with special characters should not cause SQL injection'); - $this->assertEmpty($rows, 'Filter with SQL injection attempt should return no results'); + $this->assertEmpty($rows, 'Filter with SQL injection attempt should return no results'); } /** - * Test combined filter and sort functionality - */ + * Test combined filter and sort functionality + */ public function testCombinedFilterAndSort(): void { $this->setupRealColumnMapper(self::$testTableId); - // Get column mappings for score column - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $scoreColumnId = $columnMapping['score']; + // Get column mappings for score column + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $scoreColumnId = $columnMapping['score']; $filter = [[['columnId' => $scoreColumnId, 'operator' => 'is-greater-than', 'value' => '80']]]; // score > 80 $sort = [['columnId' => $scoreColumnId, 'mode' => 'DESC']]; // sort by score descending $rows = $this->mapper->findAll(self::$testColumnIds, self::$testTableId, null, null, $filter, $sort, 'test_user'); - + $this->assertGreaterThan(0, count($rows), 'Combined filter and sort should return results'); - + // Check that results are both filtered and sorted - $scores = array_map(fn ($row) => (float)$this->getCellValue($row, $scoreColumnId), $rows); - + $scores = array_map(fn ($row) => (float)$this->getCellValue($row, $scoreColumnId), $rows); + // All scores should be > 80 foreach ($scores as $score) { $this->assertGreaterThan(80, $score, 'All results should match filter criteria'); } - + // Scores should be in descending order $sortedScores = $scores; rsort($sortedScores); $this->assertEquals($sortedScores, $scores, 'Results should be sorted in descending order'); } - /** - * Test empty filter array - */ - public function testEmptyFilter(): void { - $this->setupRealColumnMapper(self::$testTableId); - - $rows = $this->mapper->findAll( - self::$testColumnIds, - self::$testTableId, - null, - null, - [], // Empty filter - null, - 'test_user' - ); - - // Should return all test rows when no filter is applied - $this->assertCount(5, $rows, 'Empty filter should return all rows'); - } + /** + * Test empty filter array + */ + public function testEmptyFilter(): void { + $this->setupRealColumnMapper(self::$testTableId); + + $rows = $this->mapper->findAll( + self::$testColumnIds, + self::$testTableId, + null, + null, + [], // Empty filter + null, + 'test_user' + ); + + // Should return all test rows when no filter is applied + $this->assertCount(5, $rows, 'Empty filter should return all rows'); + } } diff --git a/tests/unit/Db/Row2MapperTest.php b/tests/unit/Db/Row2MapperTest.php index 8a1c2f6bba..adf9970645 100644 --- a/tests/unit/Db/Row2MapperTest.php +++ b/tests/unit/Db/Row2MapperTest.php @@ -15,7 +15,7 @@ /** * Test class for Row2Mapper core functionality - * + * * Tests sorting operations including single-column sorting, multi-column sorting, * meta column sorting, and edge cases with non-existent columns. */ @@ -27,18 +27,18 @@ protected function setUp(): void { $this->setupDependencies(); $this->setupRealColumnMapper(self::$testTableId); } - + /** - * Data provider for sorting tests - * - * Provides comprehensive test cases for various sorting scenarios including: - * - Single column sorting (text, number, datetime) - * - Multi-column sorting combinations - * - Meta column sorting (created_by, etc.) - * - Both ascending and descending orders - * - * @return array Array of test cases with sort configuration, expected results, and descriptions - */ + * Data provider for sorting tests + * + * Provides comprehensive test cases for various sorting scenarios including: + * - Single column sorting (text, number, datetime) + * - Multi-column sorting combinations + * - Meta column sorting (created_by, etc.) + * - Both ascending and descending orders + * + * @return array Array of test cases with sort configuration, expected results, and descriptions + */ public static function sortingDataProvider(): array { return [ 'Text column ASC' => [ @@ -111,13 +111,13 @@ public static function sortingDataProvider(): array { } /** - * Test various sorting operations using data provider - * - * @dataProvider sortingDataProvider - * @param array $sortWithNames Sort configuration with column names as test identifiers - * @param array $expectedNameOrder Expected names in result order - * @param string $description Test case description - */ + * Test various sorting operations using data provider + * + * @dataProvider sortingDataProvider + * @param array $sortWithNames Sort configuration with column names as test identifiers + * @param array $expectedNameOrder Expected names in result order + * @param string $description Test case description + */ public function testFindAllWithVariousSorting(array $sortWithNames, array $expectedNameOrder, string $description): void { $this->setupRealColumnMapper(self::$testTableId); @@ -130,8 +130,8 @@ public function testFindAllWithVariousSorting(array $sortWithNames, array $expec $this->assertCount(5, $rows, "Should return all 5 rows for: $description"); // Get name column ID using proper mapping - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $nameColumnId = $columnMapping['name']; + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $nameColumnId = $columnMapping['name']; $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); $this->assertEqualsCanonicalizing($expectedNameOrder, $actualNameOrder, "Failed sorting test: $description"); @@ -165,7 +165,7 @@ public function testFindAllWithNonExistentColumnId(): void { // Check that the order remained unchanged (without sorting) // since sorting was skipped due to non-existent column $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); - $nameColumnId = $columnMapping['name']; + $nameColumnId = $columnMapping['name']; $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); @@ -182,7 +182,7 @@ public function testFindAllWithMixedExistingAndNonExistentColumns(): void { $this->setupRealColumnMapper(self::$testTableId); // Get column mappings for proper ID resolution - $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); + $columnMapping = $this->extractTestIdentMapping(self::$testDataResult['columns']); // Use mixed array: existing column + non-existent $mixedSort = [ @@ -199,7 +199,7 @@ public function testFindAllWithMixedExistingAndNonExistentColumns(): void { // Check that sorting works only for existing columns // Expect sorting by Age ASC, then by Name ASC (non-existent column is ignored) $nameColumnId = $columnMapping['name']; - $ageColumnId = $columnMapping['age']; + $ageColumnId = $columnMapping['age']; $actualNameOrder = array_map(fn ($row) => $this->getCellValue($row, $nameColumnId), $rows); $actualAgeOrder = array_map(fn ($row) => $this->getCellValue($row, $ageColumnId), $rows); diff --git a/tests/unit/Db/Row2MapperTestDependencies.php b/tests/unit/Db/Row2MapperTestDependencies.php index 163db3d723..297eaf1674 100644 --- a/tests/unit/Db/Row2MapperTestDependencies.php +++ b/tests/unit/Db/Row2MapperTestDependencies.php @@ -10,77 +10,77 @@ namespace OCA\Tables\Tests\Unit\Db; use OCA\Tables\Db\Column; +use OCA\Tables\Db\ColumnMapper; use OCA\Tables\Db\Row2Mapper; use OCA\Tables\Db\RowSleeveMapper; -use OCA\Tables\Db\ColumnMapper; +use OCA\Tables\Helper\CircleHelper; use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Helper\UserHelper; -use OCA\Tables\Helper\CircleHelper; use OCP\AppFramework\Db\DoesNotExistException; -use Psr\Log\LoggerInterface; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; /** * Trait providing test dependencies and utilities for Row2Mapper testing - * + * * This trait sets up mock objects, test data, and helper methods needed * for comprehensive testing of the Row2Mapper class functionality. */ trait Row2MapperTestDependencies { - protected Row2Mapper $mapper; - protected ColumnMapper|MockObject $columnMapper; - protected RowSleeveMapper $rowSleeveMapper; - protected UserHelper|MockObject $userHelper; - protected ColumnsHelper|MockObject $columnsHelper; - protected LoggerInterface|MockObject $logger; - protected CircleHelper|MockObject $circleHelper; - - protected static bool $testDataInitialized = false; - protected static int $testTableId; - protected static array $testColumnIds = []; - protected static array $testRowIds = []; - protected static array $testDataResult = []; - - /** - * Sets up all required dependencies and mock objects for Row2Mapper testing - * - * Initializes mock objects for external dependencies and creates real instances - * of helper classes. Also ensures test data is initialized only once. - */ - - protected function setupDependencies(): void { - $this->columnMapper = $this->createMock(ColumnMapper::class); - $this->userHelper = $this->createMock(UserHelper::class); - $this->circleHelper = $this->createMock(CircleHelper::class); - $this->logger = $this->createMock(LoggerInterface::class); - - $this->rowSleeveMapper = new RowSleeveMapper($this->connectionAdapter, $this->logger); - $this->columnsHelper = new ColumnsHelper($this->userHelper, $this->circleHelper); - - $this->mapper = new Row2Mapper( - 'test_user', - $this->connectionAdapter, - $this->logger, - $this->userHelper, - $this->rowSleeveMapper, - $this->columnsHelper, - $this->columnMapper - ); - - if (!self::$testDataInitialized) { - $this->initializeTestData(); - self::$testDataInitialized = true; - } - } - - /** - * Initializes comprehensive test data for sorting and querying tests - * - * Creates a complete test table with multiple columns of different types - * and sample rows with various data values for comprehensive testing. - */ - private function initializeTestData(): void { + protected Row2Mapper $mapper; + protected ColumnMapper|MockObject $columnMapper; + protected RowSleeveMapper $rowSleeveMapper; + protected UserHelper|MockObject $userHelper; + protected ColumnsHelper|MockObject $columnsHelper; + protected LoggerInterface|MockObject $logger; + protected CircleHelper|MockObject $circleHelper; + + protected static bool $testDataInitialized = false; + protected static int $testTableId; + protected static array $testColumnIds = []; + protected static array $testRowIds = []; + protected static array $testDataResult = []; + + /** + * Sets up all required dependencies and mock objects for Row2Mapper testing + * + * Initializes mock objects for external dependencies and creates real instances + * of helper classes. Also ensures test data is initialized only once. + */ + + protected function setupDependencies(): void { + $this->columnMapper = $this->createMock(ColumnMapper::class); + $this->userHelper = $this->createMock(UserHelper::class); + $this->circleHelper = $this->createMock(CircleHelper::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->rowSleeveMapper = new RowSleeveMapper($this->connectionAdapter, $this->logger); + $this->columnsHelper = new ColumnsHelper($this->userHelper, $this->circleHelper); + + $this->mapper = new Row2Mapper( + 'test_user', + $this->connectionAdapter, + $this->logger, + $this->userHelper, + $this->rowSleeveMapper, + $this->columnsHelper, + $this->columnMapper + ); + + if (!self::$testDataInitialized) { + $this->initializeTestData(); + self::$testDataInitialized = true; + } + } + + /** + * Initializes comprehensive test data for sorting and querying tests + * + * Creates a complete test table with multiple columns of different types + * and sample rows with various data values for comprehensive testing. + */ + private function initializeTestData(): void { $result = $this->createCompleteTestTable( ['test_ident' => 'sort_test_table', 'title' => 'Comprehensive Sort Test Table'], [ @@ -160,77 +160,77 @@ private function initializeTestData(): void { self::$testRowIds = array_map(fn ($row) => $row['id'], $result['rows']); } - /** - * Sets up a real ColumnMapper with actual column data from the database - * - * Instead of using mocked column data, this method loads real column - * information from the database for more realistic testing scenarios. - * - * @param int $tableId The ID of the table to load columns for - */ - protected function setupRealColumnMapper(int $tableId): void { - $qb = $this->connection->getQueryBuilder(); - $result = $qb->select('id', 'title', 'type', 'table_id') - ->from('tables_columns') - ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId))) - ->executeQuery(); - - $columns = []; - $columnTypes = []; - while ($row = $result->fetch()) { - $column = new Column(); - $column->setId($row['id']); - $column->setTitle($row['title']); - $column->setType($row['type']); - $column->setTableId($row['table_id']); - $columns[$row['id']] = $column; - $columnTypes[$row['id']] = $row['type']; - } - $result->closeCursor(); - - $this->columnMapper->method('find') - ->willReturnCallback(fn($id) => $columns[$id] ?? throw new DoesNotExistException('test')); - - $this->columnMapper->method('preloadColumns'); - $this->columnMapper->method('getColumnTypes')->willReturn($columnTypes); - } - - /** - * Extracts the value of a specific cell from a Row object - * - * Searches through the row's data array to find the cell with the - * specified column ID and returns its value. - * - * @param mixed $row The Row object containing cell data - * @param int $columnId The ID of the column to get the value for - * @return mixed The cell value or null if not found - */ - protected function getCellValue($row, int $columnId) { - $data = $row->getData(); - foreach ($data as $cell) { - if ($cell['columnId'] === $columnId) { - return $cell['value'] ?? ''; - } - } - return ''; - } - - /** - * Helper method: Creates mapping from test identifiers to column IDs - * - * Extracts test_ident values from column definitions and creates - * a lookup array for easier test assertions and data access. - * - * @param array $columns Array of column definitions with test_ident keys - * @return array Associative array mapping test_ident to column ID - */ - protected function extractTestIdentMapping(array $columns): array { - $mapping = []; - foreach ($columns as $column) { - if (isset($column['test_ident'])) { - $mapping[$column['test_ident']] = $column['id']; - } - } - return $mapping; - } -} \ No newline at end of file + /** + * Sets up a real ColumnMapper with actual column data from the database + * + * Instead of using mocked column data, this method loads real column + * information from the database for more realistic testing scenarios. + * + * @param int $tableId The ID of the table to load columns for + */ + protected function setupRealColumnMapper(int $tableId): void { + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('id', 'title', 'type', 'table_id') + ->from('tables_columns') + ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId))) + ->executeQuery(); + + $columns = []; + $columnTypes = []; + while ($row = $result->fetch()) { + $column = new Column(); + $column->setId($row['id']); + $column->setTitle($row['title']); + $column->setType($row['type']); + $column->setTableId($row['table_id']); + $columns[$row['id']] = $column; + $columnTypes[$row['id']] = $row['type']; + } + $result->closeCursor(); + + $this->columnMapper->method('find') + ->willReturnCallback(fn ($id) => $columns[$id] ?? throw new DoesNotExistException('test')); + + $this->columnMapper->method('preloadColumns'); + $this->columnMapper->method('getColumnTypes')->willReturn($columnTypes); + } + + /** + * Extracts the value of a specific cell from a Row object + * + * Searches through the row's data array to find the cell with the + * specified column ID and returns its value. + * + * @param mixed $row The Row object containing cell data + * @param int $columnId The ID of the column to get the value for + * @return mixed The cell value or null if not found + */ + protected function getCellValue($row, int $columnId) { + $data = $row->getData(); + foreach ($data as $cell) { + if ($cell['columnId'] === $columnId) { + return $cell['value'] ?? ''; + } + } + return ''; + } + + /** + * Helper method: Creates mapping from test identifiers to column IDs + * + * Extracts test_ident values from column definitions and creates + * a lookup array for easier test assertions and data access. + * + * @param array $columns Array of column definitions with test_ident keys + * @return array Associative array mapping test_ident to column ID + */ + protected function extractTestIdentMapping(array $columns): array { + $mapping = []; + foreach ($columns as $column) { + if (isset($column['test_ident'])) { + $mapping[$column['test_ident']] = $column['id']; + } + } + return $mapping; + } +} From c09a5eb18f2d2ed4e66f8684d43264f81c488f81 Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 2 Sep 2025 15:10:46 +0200 Subject: [PATCH 15/16] cypress tests for is-not-equal and does-not-contain Signed-off-by: silver --- cypress/e2e/view-filtering-selection.cy.js | 117 +++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/cypress/e2e/view-filtering-selection.cy.js b/cypress/e2e/view-filtering-selection.cy.js index 8c72204e3f..d0b50a118a 100644 --- a/cypress/e2e/view-filtering-selection.cy.js +++ b/cypress/e2e/view-filtering-selection.cy.js @@ -186,6 +186,43 @@ describe('Filtering in a view by selection columns', () => { }) }) + it('Filter view for single selection - is not equal', () => { + cy.loadTable('View filtering test table') + + const title = 'Filter for single selection - is not equal' + cy.get('[data-cy="customTableAction"] button').click() + cy.get('.v-popper__popper li button span').contains('Create view').click({ force: true }) + cy.get('.modal-container #settings-section_title input').type(title) + + // add filter + cy.get('[data-cy="filterFormFilterGroupBtn"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(0).click() + cy.get('ul.vs__dropdown-menu li span[title="selection"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(1).click() + cy.get('ul.vs__dropdown-menu li span[title="Is not equal"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(2).click() + cy.get('ul.vs__dropdown-menu li span[title="sel2"]').click() + + // save view + cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView') + cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView') + cy.contains('button', 'Create View').click() + cy.wait('@createView') + cy.wait('@updateView') + cy.contains('.app-navigation-entry-link span', title).should('exist') + + // check for expected rows + const expected = ['first row', 'third row', 'fifths row', 'sixths row'] + expected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('be.visible') + }) + + const unexpected = ['second row', 'fourth row', 'sevenths row'] + unexpected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('not.exist') + }) + }) + it('Filter view for multi selection - contains', () => { cy.loadTable('View filtering test table') @@ -276,6 +313,86 @@ describe('Filtering in a view by selection columns', () => { }) }) + it('Filter view for single selection - does not contain', () => { + cy.loadTable('View filtering test table') + + // # create view with filter + const title = 'Filter does not contain sel2' + cy.get('[data-cy="customTableAction"] button').click() + cy.get('.v-popper__popper li button span').contains('Create view').click({ force: true }) + cy.get('.modal-container #settings-section_title input').type(title) + + // ## add filter + cy.get('[data-cy="filterFormFilterGroupBtn"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(0).click() + cy.get('ul.vs__dropdown-menu li span[title="selection"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(1).click() + cy.get('ul.vs__dropdown-menu li span[title="Does not contain"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(2).click() + cy.get('ul.vs__dropdown-menu li span[title="sel2"]').click() + + // ## save view + cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView') + cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView') + cy.contains('button', 'Create View').click() + cy.wait('@createView') + cy.wait('@updateView') + cy.contains('.app-navigation-entry-link span', title).should('exist') + + // # check for expected rows + // rows that **do not contain sel2** + const expected = ['first row', 'third row', 'fourth row', 'fifths row', 'sixths row'] + expected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('be.visible') + }) + + // rows that **contain sel2** should not be visible + const unexpected = ['second row', 'sevenths row'] + unexpected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('not.exist') + }) + }) + + it('Filter view for multi selection - does not contain', () => { + cy.loadTable('View filtering test table') + + // # create view with filter + const title = 'Filter multi selection does not contain A' + cy.get('[data-cy="customTableAction"] button').click() + cy.get('.v-popper__popper li button span').contains('Create view').click({ force: true }) + cy.get('.modal-container #settings-section_title input').type(title) + + // ## add filter + cy.get('[data-cy="filterFormFilterGroupBtn"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(0).click() + cy.get('ul.vs__dropdown-menu li span[title="multi selection"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(1).click() + cy.get('ul.vs__dropdown-menu li span[title="Does not contain"]').click() + cy.get('.modal-container .filter-group .v-select.select').eq(2).click() + cy.get('ul.vs__dropdown-menu li span[title="A"]').click() + + // ## save view + cy.intercept({ method: 'POST', url: '**/apps/tables/view' }).as('createView') + cy.intercept({ method: 'PUT', url: '**/apps/tables/view/*' }).as('updateView') + cy.contains('button', 'Create View').click() + cy.wait('@createView') + cy.wait('@updateView') + cy.contains('.app-navigation-entry-link span', title).should('exist') + + // # check for expected rows + // rows that **do not contain A** in multi selection + const expected = ['third row', 'fifths row', 'sixths row'] + expected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('be.visible') + }) + + // rows that **contain A** should not be visible + const unexpected = ['first row', 'fourth row', 'sevenths row'] + unexpected.forEach(item => { + cy.get('.custom-table table tr td div').contains(item).should('not.exist') + }) + }) + it('Filter view for multi selection - multiple filter groups', () => { cy.loadTable('View filtering test table') From a8fae2db63c26e8a8b4663228f60abd17197d9c6 Mon Sep 17 00:00:00 2001 From: silver Date: Tue, 2 Sep 2025 15:48:19 +0200 Subject: [PATCH 16/16] Fix: make filter expressions null-safe to avoid deprecation warnings Signed-off-by: silver --- lib/Db/Row2Mapper.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 0ece2f19fd..69e680ccaa 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -420,11 +420,11 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $ switch ($operator) { case 'begins-with': - $includeDefault = str_starts_with($defaultValue, $value); + $includeDefault = str_starts_with((string)($defaultValue ?? ''), $value); $filterExpression = $qb->expr()->like('value', $qb->createNamedParameter($this->db->escapeLikeParameter($value) . '%', $paramType)); break; case 'ends-with': - $includeDefault = str_ends_with($defaultValue, $value); + $includeDefault = str_ends_with((string)($defaultValue ?? ''), $value); $filterExpression = $qb->expr()->like('value', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($value), $paramType)); break; case 'contains': @@ -452,7 +452,7 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $ break; } - $includeDefault = str_contains($defaultValue, $value); + $includeDefault = str_contains((string)($defaultValue ?? ''), $value); if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') { $value = str_replace(['"', '\''], '', $value); $filterExpression = $qb2->expr()->orX( @@ -490,7 +490,7 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $ break; } - $includeDefault = !str_contains($defaultValue, $value); + $includeDefault = !str_contains((string)($defaultValue ?? ''), $value); if ($column->getType() === 'selection' && $column->getSubtype() === 'multi') { $value = str_replace(['"', '\''], '', $value); $filterExpression = $qb2->expr()->andX(