DataFilter: refactor the whole class(es)

application/views/helpers/FormDataFilter.php

@@ -20,8 +20,6 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
 
     private $fieldName;
 
-    private $addTo;
-
     private $cachedColumnSelect;
 
     private $query;
@@ -50,19 +48,13 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
             unset($attribs['columns']);
         }
 
-        if (array_key_exists('addTo', $attribs)) {
-            $this->addTo = $attribs['addTo'];
-            unset($attribs['addTo']);
-        }
-
         // TODO: check for columns in attribs, preserve & remove them from the
         // array use attribs? class etc? disabled?
         // override _getInfo?
-
         $this->fieldName = $name;
-        // $this->fieldName = $id;
+
         if ($value === null) {
-            $value = Filter::matchAll();
+            $value = $this->emptyExpression();
         } elseif (is_string($value)) {
             $value = Filter::fromQueryString($value);
         }
@@ -72,16 +64,19 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
 
     protected function renderFilter(Filter $filter, $level = 0)
     {
-        if ($level === 0 && $filter->isChain() && $filter->isEmpty()) {
+        if ($level === 0 && (
-            return '<ul class="filter-expression filter-root"><li class="active">'
+            ($filter->isChain() && $filter->isEmpty())
-            . $this->renderNewFilter()
+            || $filter->isExpression())) {
-            . '</li></ul>';
+            $pre = '<ul class="filter-expression filter-root"><li class="active">';
+            $post = '</li></ul>';
+        } else {
+            $pre = $post = '';
         }
 
         if ($filter instanceof FilterChain) {
-            return $this->renderFilterChain($filter, $level);
+            return $pre . $this->renderFilterChain($filter, $level) . $post;
         } elseif ($filter instanceof FilterExpression) {
-            return $this->renderFilterExpression($filter, $level);
+            return $pre . $this->renderFilterExpression($filter, $level) . $post;
         } else {
             throw new ProgrammingError('Got a Filter being neither expression nor chain');
         }
@@ -101,14 +96,9 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
                 . '</li>';
         }
 
-        if ($this->addTo && $this->addTo == $filter->getId()) {
+        return $this->beginChain($filter)
-            $parts[] = '<li style="background: #ffb">'
+            . implode('', $parts)
-                . $this->renderNewFilter()
+            . $this->endChain($filter);
-                // . $this->cancelLink()
-                . '</li>';
-        }
-
-        return $this->beginChain($filter) . implode('', $parts) . $this->endChain($filter);
     }
 
     protected function beginChain(FilterChain $filter)
@@ -117,7 +107,7 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
 
         $list = $filter->isEmpty() ? '' : '<ul' . $root . '>' . "\n";
 
-        return '<div class="filter-chain'
+        return '<li><div class="filter-chain'
              . '"><span class="handle"> </span>'
              . $this->selectOperator($filter)
              . $this->removeLink($filter)
@@ -129,7 +119,7 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
     protected function endChain(FilterChain $filter)
     {
         $list = $filter->isEmpty() ? '' : "</ul>\n";
-        return $list . "</div>\n";
+        return $list . "</div></li>\n";
     }
 
     protected function beginExpression(FilterExpression $filter)
@@ -143,28 +133,6 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
         return "</div>\n";
     }
 
-    protected function eventuallyAddTo($html, Filter $filter, $level)
-    {
-        if ($this->addTo && $this->addTo === $filter->getId()) {
-            // $filter->replaceWith(Filter::matchAll(clone($filter)));
-            // $and = $filt = Filter::matchAll(clone($filter));
-            // return $this->renderFilterChain($filter, $level);
-
-            $html .= preg_replace(
-                //'/ class="autosubmit"/',
-                '/ class="/',
-                ' class="autofocus',
-                $this->selectOperator()
-            ) . '<ul><li>'
-            . $html
-            . '</li><li class="active">'
-            . $this->renderNewFilter() /*.$this->cancelLink()*/
-            . '</li></ul>';
-        }
-
-        return $html;
-    }
-
     protected function filterExpressionHtml(FilterExpression $filter, $level)
     {
         return $this->selectColumn($filter)
@@ -177,11 +145,8 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
     protected function renderFilterExpression(FilterExpression $filter, $level)
     {
         return $this->beginExpression($filter)
-             . $this->eventuallyAddTo(
+             . $this->filterExpressionHtml($filter, $level)
-                 $this->filterExpressionHtml($filter, $level),
+             . $this->endExpression($filter);
-                 $filter,
-                 $level
-             ) . $this->endExpression($filter);
     }
 
     protected function element(Filter $filter = null)
@@ -252,9 +217,7 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
 
     protected function renderNewFilter()
     {
-        return $this->selectColumn()
+        return $this->renderFilterExpression($this->emptyExpression(), 0);
-              . $this->selectSign()
-              . $this->element();
     }
 
     protected function arrayForSelect($array, $flip = false)
@@ -278,11 +241,7 @@ class Zend_View_Helper_FormDataFilter extends Zend_View_Helper_FormElement
         $prefix = $this->fieldName . '[id_';
         $suffix = '][' . $field . ']';
 
-        if ($filter === null) {
+        return $prefix . $filter->getId() . $suffix;
-            return $prefix . 'new_' . ($this->addTo ?: '0') . $suffix;
-        } else {
-            return $prefix . $filter->getId() . $suffix;
-        }
     }
 
     protected function selectOperator(Filter $filter = null)

library/Director/Web/Form/Element/DataFilter.php

@@ -32,10 +32,6 @@ class DataFilter extends FormElement
                 $value = $this->arrayToFilter($value);
             }
 
-            if ($value->isEmpty()) {
-                $value = Filter::matchAll(Filter::expression('', '=', ''));
-            }
-
         } catch (Exception $e) {
             $value = null;
             // TODO: getFile, getLine
@@ -45,77 +41,40 @@ class DataFilter extends FormElement
         }
     }
 
+    /**
+     * This method transforms filter form data into a filter
+     * and reacts on pressed buttons
+     */
     protected function arrayToFilter($array)
     {
         if ($array === null) {
+            return null;
             return Filter::matchAll();
         }
 
-        $firstKey = key($array);
+        $filter = null;
-        if (! in_array($firstKey, array('id_1', 'id_new_0'))) {
-            die('FCK: ' . key($array));
-        }
-
-        $entry = array_shift($array);
-        $filter = $this->entryToFilter($entry);
-        if ($firstKey === 'id_new_0') {
-            $this->setAttrib('addTo', '0');
-        }
-   
-        $remove = $strip = null;
-
-        // TODO: This is for the first entry, duplicates code and has debug info
-        $filterId = $this->idToFilterId($firstKey);
-        switch ($this->entryAction($entry)) {
-            case 'cancel':
-                $remove = $filterId;
-                echo "cancel";
-                break;
-
-            case 'minus':
-                $strip = $filterId;
-                echo "minus";
-                break;
-
-            case 'plus':
-                $this->setAttrib('addTo', $filterId);
-                echo "plus";
-                break;
-        }
-
         foreach ($array as $id => $entry) {
-            // TODO: addTo from FilterEditor
-
-            $sub = $this->entryToFilter($entry);
             $filterId = $this->idToFilterId($id);
-
+            $sub = $this->entryToFilter($entry);
-            switch ($this->entryAction($entry)) {
+            $this->checkEntryForActions($filterId, $entry);
-                case 'cancel':
-                    $remove = $filterId;
-                    break;
-
-                case 'minus':
-                    $strip = $filterId;
-                    break;
-
-                case 'plus':
-                    $this->setAttrib('addTo', $filterId);
-                    break;
-            }
-
             $parentId = $this->parentIdFor($filterId);
-            $filter->getById($parentId)->addFilter($sub);
+
+            if ($filter === null) {
+                $filter = $sub;
+            } else {
+                $filter->getById($parentId)->addFilter($sub);
+            }
         }
 
-        if ($remove) {
+        if ($remove = $this->getAttrib('removeFilter')) {
             if ($filter->getById($remove)->isRootNode()) {
-                $filter = Filter::matchAll();
+                $filter = $this->emptyExpression();
             } else {
                 $filter->removeId($remove);
             }
         }
 
-        if ($strip) {
+        if ($strip = $this->getAttrib('stripFilter')) {
             $subId = $strip . '-1';
             if ($filter->getId() === $strip) {
                 $filter = $filter->getById($strip . '-1');
@@ -124,9 +83,45 @@ class DataFilter extends FormElement
             }
         }
 
+        if ($addTo = $this->getAttrib('addTo')) {
+            $parent = $filter->getById($addTo);
+
+            if ($parent->isChain()) {
+                if ($parent->isEmpty()) {
+                    $parent->addFilter($this->emptyExpression());
+                } elseif ($parent->getOperatorName() === 'NOT') {
+                    $andNot = Filter::matchAll();
+                    foreach ($parent->filters() as $sub) {
+                        $andNot->addFilter(clone($sub));
+                    }
+                    $clone->addFilter($this->emptyExpression());
+                    $filter->replaceById(
+                        $parent->getId(),
+                        Filter::not($andNot)
+                    );
+                } else {
+                    $parent->addFilter($this->emptyExpression());
+                }
+            } else {
+                $replacement = Filter::matchAll(clone($parent));
+                if ($parent->isRootNode()) {
+                    $filter = $replacement;
+                } else {
+                    $filter->replaceById($parent->getId(), $replacement);
+                }
+            }
+
+            $this->setAttrib('addTo', null);
+        }
+
         return $filter;
     }
 
+    protected function emptyExpression()
+    {
+        return Filter::expression('', '=', '');
+    }
+
     protected function parentIdFor($id)
     {
         if (false === ($pos = strrpos($id, '-'))) {
@@ -145,41 +140,72 @@ class DataFilter extends FormElement
         return $m[2];
     }
 
+    protected function checkEntryForActions($filterId, $entry)
+    {
+        switch ($this->entryAction($entry)) {
+            case 'cancel':
+                $this->setAttrib('removeFilter', $filterId);
+                break;
+
+            case 'minus':
+                $this->setAttrib('stripFilter', $filterId);
+                break;
+
+            case 'plus':
+            case 'angle-double-right':
+                $this->setAttrib('addTo', $filterId);
+                break;
+        }
+    }
+
+    /**
+     * Transforms a single submitted form component from an array
+     * into a Filter object
+     *
+     * @param Array $entry The array as submitted through the form
+     *
+     * @return Filter
+     */
     protected function entryToFilter($entry)
     {
         if (array_key_exists('operator', $entry)) {
             return Filter::chain($entry['operator']);
         } else {
-            if ($entry['sign'] === 'true') {
+            return $this->entryToFilterExpression($entry);
-                return Filter::expression(
+        }
-                    $entry['column'],
+    }
-                    '=',
+
-                    true
+    protected function entryToFilterExpression($entry)
-                );
+    {
-            } elseif ($entry['sign'] === 'in') {
+        if ($entry['sign'] === 'true') {
-                if (array_key_exists('value', $entry)) {
+            return Filter::expression(
-                    if (is_array($entry['value'])) {
+                $entry['column'],
-                        $value = array_filter($entry['value'], 'strlen');
+                '=',
-                    } elseif (empty($entry['value'])) {
+                true
-                        $value = array();
+            );
-                    } else {
+        } elseif ($entry['sign'] === 'in') {
-                        $value = array($entry['value']);
+            if (array_key_exists('value', $entry)) {
-                    }
+                if (is_array($entry['value'])) {
-                } else {
+                    $value = array_filter($entry['value'], 'strlen');
+                } elseif (empty($entry['value'])) {
                     $value = array();
+                } else {
+                    $value = array($entry['value']);
                 }
-                return Filter::expression(
-                    $entry['column'],
-                    '=',
-                    $value
-                );
             } else {
-                return Filter::expression(
+                $value = array();
-                    $entry['column'],
-                    $entry['sign'],
-                    array_key_exists('value', $entry) ? $entry['value'] : null
-                );
             }
+            return Filter::expression(
+                $entry['column'],
+                '=',
+                $value
+            );
+        } else {
+            return Filter::expression(
+                $entry['column'],
+                $entry['sign'],
+                array_key_exists('value', $entry) ? $entry['value'] : null
+            );
         }
     }