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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions cypress/e2e/view-filtering-selection.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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')

Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/Api1Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>}|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<int>}|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<Http::STATUS_OK, TablesView, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_NOT_FOUND|Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: View updated
Expand Down
4 changes: 4 additions & 0 deletions lib/Db/ColumnTypes/SuperColumnQB.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ 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':
return $formattedCellValue . ' != :' . $searchValuePlaceHolder;
case 'is-greater-than':
return $formattedCellValue . ' > :' . $searchValuePlaceHolder;
case 'is-greater-than-or-equal':
Expand Down
4 changes: 2 additions & 2 deletions lib/Db/LegacyRowMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private function buildFilterByColumnType($qb, array $filter, string $filterId):
/**
* @param (float|int|string)[][] $filterGroup
*
* @psalm-param list<array{columnId: int, operator: 'begins-with'|'contains'|'ends-with'|'is-empty'|'is-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal', value: float|int|string}> $filterGroup
* @psalm-param list<array{columnId: int, operator: 'begins-with'|'contains'|'does-not-contain'|'ends-with'|'is-empty'|'is-equal'|'is-not-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal', value: float|int|string}> $filterGroup
*/
private function getInnerFilterExpressions(IQueryBuilder $qb, array $filterGroup, int $groupIndex): array {
$innerFilterExpressions = [];
Expand All @@ -114,7 +114,7 @@ private function getInnerFilterExpressions(IQueryBuilder $qb, array $filterGroup
/**
* @param (float|int|string)[][][] $filters
*
* @psalm-param non-empty-list<list<array{columnId: int, operator: 'begins-with'|'contains'|'ends-with'|'is-empty'|'is-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal', value: float|int|string}>> $filters
* @psalm-param non-empty-list<list<array{columnId: int, operator: 'begins-with'|'contains'|'does-not-contain'|'ends-with'|'is-empty'|'is-equal'|'is-not-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal', value: float|int|string}>> $filters
*/
private function getFilterGroups(IQueryBuilder $qb, array $filters): array {
$filterGroups = [];
Expand Down
79 changes: 69 additions & 10 deletions lib/Db/Row2Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Comment on lines +185 to +188

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What is the reason behind this change on the Mapper level (and the other switches to InternalError in this class)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be fair, those changes may be slightly out of the scope of this PR and were more of a 'refactor-while-passing-by'. I added them primarily to make the functions more robust. By catching and logging DoesNotExistException and then throwing a controlled InternalError, the idea is to prevent unhandled exceptions from crashing the application and to make debugging easier.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK

}

/**
Expand Down Expand Up @@ -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
Expand All @@ -410,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':
Expand Down Expand Up @@ -442,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(
Expand All @@ -455,6 +465,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((string)($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') {
Expand All @@ -464,6 +512,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));
Expand Down Expand Up @@ -547,6 +604,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':
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public function getSortArray(): array {

/**
* @psalm-suppress MismatchingDocblockReturnType
* @return list<list<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}>>
* @return list<list<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}>>
*/
public function getFilterArray():array {
$filters = $this->getArray($this->getFilter());
Expand Down
2 changes: 1 addition & 1 deletion lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* columns: list<int>,
* columnSettings:list<array{columnId: int, order: int, readonly: bool}>,
* sort: list<array{columnId: int, mode: 'ASC'|'DESC'}>,
* filter: list<list<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}>>,
* filter: list<list<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}>>,
* isShared: bool,
* favorite: bool,
* onSharePermissions: ?array{
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/RowService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Loading
Loading