From 398fa03fff887922ff85235d6576612d29b4f3ef Mon Sep 17 00:00:00 2001
From: Peter Gribanov <info@peter-gribanov.ru>
Date: Tue, 22 Sep 2020 10:00:27 +0300
Subject: [PATCH] delegate filter query by empty value to filters

extract code from Datagrid::buildPager() into separate methods to improve readability

add upgrade info

add example

invert check to fail fast
---
 UPGRADE-3.x.md            |  24 +++++++++
 src/Datagrid/Datagrid.php | 109 +++++++++++++++++++++++---------------
 2 files changed, 90 insertions(+), 43 deletions(-)

diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
index f6f0783777d..96f534c340c 100644
--- a/UPGRADE-3.x.md
+++ b/UPGRADE-3.x.md
@@ -7,6 +7,30 @@ This interface has been deprecated without replacement.
 
 `ModelManagerInterface::getDefaultSortValues()` won't be used anymore.
 
+### Empty values in datagrid filters
+
+Now empty values are passed to datagrid filters. If you have custom datagrid filters, add empty string check to them.
+
+```php
+->add('with_open_comments', CallbackFilter::class, [
+    'callback' => static function(ProxyQueryInterface $queryBuilder, string $alias, string $field, array $value): bool {
+        if (!$value['value']) {
+            return false;
+        }
+
+        $queryBuilder
+            ->leftJoin(sprintf('%s.comments', $alias), 'c')
+            ->andWhere('c.moderation = :moderation')
+            ->setParameter('moderation', CommentModeration::APPROVED);
+
+        return true;
+    },
+    'field_type' => CheckboxType::class
+]);
+```
+
+The `!$value['value']` check is required to not filtering by `''` if you didn't used the filter.
+
 UPGRADE FROM 3.77 to 3.78
 =========================
 
diff --git a/src/Datagrid/Datagrid.php b/src/Datagrid/Datagrid.php
index 5062462d029..997132ced2c 100644
--- a/src/Datagrid/Datagrid.php
+++ b/src/Datagrid/Datagrid.php
@@ -146,50 +146,11 @@ static function ($value) {
         $this->form = $this->formBuilder->getForm();
         $this->form->submit($this->values);
 
-        $data = $this->form->getData();
-
-        foreach ($this->getFilters() as $name => $filter) {
-            $this->values[$name] = $this->values[$name] ?? null;
-            $filterFormName = $filter->getFormName();
-            if (isset($this->values[$filterFormName]['value']) && '' !== $this->values[$filterFormName]['value']) {
-                $filter->apply($this->query, $data[$filterFormName]);
-            }
-        }
-
-        if (isset($this->values['_sort_by'])) {
-            if (!$this->values['_sort_by'] instanceof FieldDescriptionInterface) {
-                throw new UnexpectedTypeException($this->values['_sort_by'], FieldDescriptionInterface::class);
-            }
-
-            if ($this->values['_sort_by']->isSortable()) {
-                $this->query->setSortBy($this->values['_sort_by']->getSortParentAssociationMapping(), $this->values['_sort_by']->getSortFieldMapping());
-
-                $this->values['_sort_order'] = $this->values['_sort_order'] ?? 'ASC';
-                $this->query->setSortOrder($this->values['_sort_order']);
-            }
-        }
-
-        $maxPerPage = 25;
-        if (isset($this->values['_per_page'])) {
-            if (isset($this->values['_per_page']['value'])) {
-                $maxPerPage = $this->values['_per_page']['value'];
-            } else {
-                $maxPerPage = $this->values['_per_page'];
-            }
-        }
-        $this->pager->setMaxPerPage($maxPerPage);
-
-        $page = 1;
-        if (isset($this->values['_page'])) {
-            if (isset($this->values['_page']['value'])) {
-                $page = $this->values['_page']['value'];
-            } else {
-                $page = $this->values['_page'];
-            }
-        }
-
-        $this->pager->setPage($page);
+        $this->applyFilters($this->form->getData() ?? []);
+        $this->applySorting();
 
+        $this->pager->setMaxPerPage($this->getMaxPerPage(25));
+        $this->pager->setPage($this->getPage(1));
         $this->pager->setQuery($this->query);
         $this->pager->init();
 
@@ -330,6 +291,68 @@ public function getPaginationParameters(int $page): array
         return ['filter' => $values];
     }
 
+    private function applyFilters(array $data): void
+    {
+        foreach ($this->getFilters() as $name => $filter) {
+            $this->values[$name] = $this->values[$name] ?? null;
+            $filterFormName = $filter->getFormName();
+            if (isset($this->values[$filterFormName]['type'], $this->values[$filterFormName]['value']) &&
+                ('' !== $this->values[$filterFormName]['type'] || '' !== $this->values[$filterFormName]['value'])
+            ) {
+                $filter->apply($this->query, $data[$filterFormName]);
+            }
+        }
+    }
+
+    private function applySorting(): void
+    {
+        if (!isset($this->values['_sort_by'])) {
+            return;
+        }
+
+        if (!$this->values['_sort_by'] instanceof FieldDescriptionInterface) {
+            throw new UnexpectedTypeException($this->values['_sort_by'], FieldDescriptionInterface::class);
+        }
+
+        if (!$this->values['_sort_by']->isSortable()) {
+            return;
+        }
+
+        $this->query->setSortBy(
+            $this->values['_sort_by']->getSortParentAssociationMapping(),
+            $this->values['_sort_by']->getSortFieldMapping()
+        );
+
+        $this->values['_sort_order'] = $this->values['_sort_order'] ?? 'ASC';
+        $this->query->setSortOrder($this->values['_sort_order']);
+    }
+
+    private function getMaxPerPage(int $default): int
+    {
+        if (!isset($this->values['_per_page'])) {
+            return $default;
+        }
+
+        if (isset($this->values['_per_page']['value'])) {
+            return (int) $this->values['_per_page']['value'];
+        }
+
+        return (int) $this->values['_per_page'];
+    }
+
+    private function getPage(int $default): int
+    {
+        if (!isset($this->values['_page'])) {
+            return $default;
+        }
+
+        if (isset($this->values['_page']['value'])) {
+            return (int) $this->values['_page']['value'];
+        }
+
+        return (int) $this->values['_page'];
+    }
+
     private function isFieldAlreadySorted(FieldDescriptionInterface $fieldDescription): bool
     {
         $values = $this->getValues();