Skip to content

Commit

Permalink
Use a static array to store the conditions required by each admin
Browse files Browse the repository at this point in the history
  • Loading branch information
@phansys
phansys committed Mar 26, 2021
1 parent 9907175 commit b27e3e9
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 28 deletions.
67 changes: 41 additions & 26 deletions src/Filter/Filter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ abstract class Filter extends BaseFilter
protected $active = false;

/**
* @var Orx
* Holds an array of `orX` expressions used by each admin when the condition
* equals the value on `FilterInterface::CONDITION_OR`, using the admin code
* as index.
*
* @var array<string, Orx>
*/
private $orExpression;
private static $orExpressionsByAdmin = [];

/**
* NEXT_MAJOR change $query type for Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQueryInterface.
Expand Down Expand Up @@ -58,11 +62,6 @@ public function isActive()
return $this->active;
}

final public function setOrExpression(Orx $orx): void
{
$this->orExpression = $orx;
}

/**
* @param mixed[] $data
*
Expand Down Expand Up @@ -107,8 +106,7 @@ protected function applyWhere(BaseProxyQueryInterface $query, $parameter)
}

if (self::CONDITION_OR === $this->getCondition()) {
$condition = $this->getOrExpression($query);
$condition->add($parameter);
$this->addOrParameter($query, $parameter);
} else {
$query->getQueryBuilder()->andWhere($parameter);
}
Expand Down Expand Up @@ -155,52 +153,69 @@ protected function getNewParameterName(BaseProxyQueryInterface $query)
}

/**
* Returns the reference to the `Orx` expression used in the `where` clause.
* Adds the parameter to the corresponding `Orx` expression used in the `where` clause.
* If it doesn't exist, a new one is created.
* This method uses a marker (":sonata_admin_datagrid_filter_query_marker") in
* This method groups the filter "OR" conditions based on the "admin_code" option. It this
* option is not set, it uses a marker (":sonata_admin_datagrid_filter_query_marker") in
* the resulting DQL in order to identify the corresponding "WHERE (...)" condition
* group each time it is required.
* It allows to get queries like "WHERE previous_condition = previous_value AND (filter_1 = value OR filter_2 = value OR ...)",
* where the logical "OR" operators added by the filters are grouped inside a condition,
* instead of having unfolded "WHERE ..." clauses like "WHERE previous_condition = previous_value OR filter_1 = value OR filter_2 = value OR ...",
* which will produce undesired results.
*
* TODO: Remove the logic related to the ":sonata_admin_datagrid_filter_query_marker" marker when
* the constraint for "sonata-project/admin-bundle" guarantees that the "admin_code" option is set.
*
* @param mixed $parameter
*/
private function getOrExpression(BaseProxyQueryInterface $query): Orx
private function addOrParameter(BaseProxyQueryInterface $query, $parameter): void
{
if (null !== $this->orExpression) {
return $this->orExpression;
$adminCode = $this->getOption('admin_code');
$orExpression = self::$orExpressionsByAdmin[$adminCode] ?? null;
if ($orExpression instanceof Orx) {
$orExpression->add($parameter);

return;
}

$qb = $query->getQueryBuilder();
$where = $qb->getDQLPart('where');

if ($where) {
// Search for the ":sonata_admin_datagrid_filter_query_marker" marker in order to
// get the `Orx` expression.
if (null === $adminCode && null !== $where) {
foreach ($where->getParts() as $expression) {
if (!$expression instanceof Orx) {
continue;
}

$expressionParts = $expression->getParts();

// Search for the ":sonata_admin_datagrid_filter_query_marker" marker in order to
// get the `Orx` expression.
if (isset($expressionParts[0]) && \is_string($expressionParts[0]) &&
0 === strpos($expressionParts[0], ':sonata_admin_datagrid_filter_query_marker')
) {
$this->orExpression = $expression;
$expression->add($parameter);

return $this->orExpression;
return;
}
}
}

// Create a new `Orx` expression, adding it to the `where` clause.
$this->orExpression = $qb->expr()->orX();
// Place ":sonata_admin_datagrid_filter_query_marker" parameter as marker for the `Orx` expression.
$this->orExpression->add($qb->expr()->isNull(':sonata_admin_datagrid_filter_query_marker'));
$qb->setParameter('sonata_admin_datagrid_filter_query_marker', 'sonata_admin.datagrid.filter_query.marker');
$qb->andWhere($this->orExpression);
// Create a new `Orx` expression.
$orExpression = $qb->expr()->orX();

if (null === $adminCode) {
// Add the ":sonata_admin_datagrid_filter_query_marker" parameter as marker for the `Orx` expression.
$orExpression->add($qb->expr()->isNull(':sonata_admin_datagrid_filter_query_marker'));
$qb->setParameter('sonata_admin_datagrid_filter_query_marker', 'sonata_admin.datagrid.filter_query.marker');
} else {
self::$orExpressionsByAdmin[$adminCode] = $orExpression;
}

$orExpression->add($parameter);

return $this->orExpression;
// Add the `Orx` expression to the `where` clause.
$qb->andWhere($orExpression);
}
}
67 changes: 65 additions & 2 deletions tests/Filter/FilterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@

namespace Sonata\DoctrineORMAdminBundle\Tests\Filter;

use PHPUnit\Framework\TestCase;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Query\Expr;
use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
use Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQuery;
use Sonata\DoctrineORMAdminBundle\Filter\Filter;
use Sonata\DoctrineORMAdminBundle\Filter\StringFilter;
use Sonata\DoctrineORMAdminBundle\Tests\Filter\FilterTestCase;

final class FilterTest extends TestCase
final class FilterTest extends FilterTestCase
{
/**
* @var Filter
Expand Down Expand Up @@ -58,6 +62,65 @@ public function testIsActive(): void
$this->assertFalse($this->filter->isActive());
}

/**
* @dataProvider orExpressionProvider
*/
public function testOrExpression(string $expected, array $filterOptions = []): void
{
$allowEmpty = false;
$value = 'sonata';

$expressionBuilder = new Expr();
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->method('getExpressionBuilder')->willReturn($expressionBuilder);
$queryBuilder = new TestQueryBuilder($entityManager);

$queryBuilder->select('e')
->from('MyEntity', 'e')
->where($queryBuilder->expr()->eq(1, 2))
->andWhere(
$queryBuilder->expr()->orX(
$queryBuilder->expr()->eq(3, 4),
$queryBuilder->expr()->eq(5, 6)
)
);

$proxyQuery = new ProxyQuery($queryBuilder);
$this->assertSameQuery([], $proxyQuery);

$filter1 = new StringFilter();
$filter1->setCondition(Filter::CONDITION_OR);
$filter1->initialize('field_name', $filterOptions);

$filter1->filter($proxyQuery, 'alias', 'project', ['value' => $value]);

$filter2 = new StringFilter();
$filter2->setCondition(Filter::CONDITION_OR);
$filter2->initialize('field_name', $filterOptions);

$filter2->filter($proxyQuery, 'alias', 'version', ['value' => '3.x']);

$this->assertSame($expected, $queryBuilder->getDQL());
}

public function orExpressionProvider(): iterable
{
yield 'Using "admin_code" option' => [
'SELECT e FROM MyEntity e WHERE 1 = 2 AND (3 = 4 OR 5 = 6) AND (alias.project LIKE :field_name_0 OR alias.version LIKE :field_name_1)',
[
'allow_empty' => false,
'admin_code' => 'my_admin',
],
];

yield 'Missing "admin_code" option, fallback to DQL marker' => [
'SELECT e FROM MyEntity e WHERE 1 = 2 AND (3 = 4 OR 5 = 6) AND (:sonata_admin_datagrid_filter_query_marker IS NULL OR alias.project LIKE :field_name_0 OR alias.version LIKE :field_name_1)',
[
'allow_empty' => false,
],
];
}

private function createFilter(): Filter
{
return new class() extends Filter {
Expand Down

0 comments on commit b27e3e9

Please sign in to comment.