Browse Source

Cleanup search orderBy functionality, refs #493

Jordi Boggiano 9 years ago
parent
commit
b8b3c058e6

+ 4 - 0
app/config/config_test.yml

@@ -1,6 +1,10 @@
 imports:
     - { resource: config_dev.yml }
 
+parameters:
+    database_name: %database_name_test%
+    redis_dsn: %redis_dsn_test%
+
 framework:
     test: ~
     session:

+ 2 - 0
app/config/defaults.yml

@@ -2,3 +2,5 @@ parameters:
     packagist_host: ~
     packagist_metadata_dir: "%kernel.cache_dir%/composer-packages-build"
     session_save_path: %kernel.cache_dir%/sessions
+    database_name_test: packagist_test
+    redis_dsn_test: redis://127.0.0.1/14

+ 2 - 0
app/config/parameters.yml.dist

@@ -4,6 +4,7 @@ parameters:
     database_name: packagist
     database_user: root
     database_password:
+    database_name_test: packagist_test
 
     mailer_transport:
     mailer_host: localhost
@@ -17,6 +18,7 @@ parameters:
     # router.request_context.scheme: https
 
     redis_dsn: redis://localhost/1
+    redis_dsn_test: redis://127.0.0.1/14
 
     locale: en
 

+ 170 - 169
src/Packagist/WebBundle/Controller/WebController.php

@@ -56,7 +56,7 @@ class WebController extends Controller
      */
     public function indexAction()
     {
-        return array('page' => 'home', 'searchForm' => $this->createSearchForm()->createView());
+        return array('page' => 'home');
     }
 
     /**
@@ -80,7 +80,6 @@ class WebController extends Controller
 
         $data['packages'] = $this->setupPager($packages, $page);
         $data['meta'] = $this->getPackagesMetadata($data['packages']);
-        $data['searchForm'] = $this->createSearchForm()->createView();
 
         return $data;
     }
@@ -117,7 +116,6 @@ class WebController extends Controller
             'newlyReleased' => $newReleases,
             'random' => $random,
             'popular' => $popular,
-            'searchForm' => $this->createSearchForm()->createView(),
         );
 
         return $data;
@@ -161,7 +159,6 @@ class WebController extends Controller
 
         $data = array(
             'packages' => $packages,
-            'searchForm' => $this->createSearchForm()->createView(),
         );
         $data['meta'] = $this->getPackagesMetadata($data['packages']);
 
@@ -231,125 +228,24 @@ class WebController extends Controller
         return new JsonResponse(array('packageNames' => $names));
     }
 
-    /**
-     * Initializes the pager for a query.
-     *
-     * @param \Doctrine\ORM\QueryBuilder $query Query for packages
-     * @param int                        $page  Pagenumber to retrieve.
-     * @return \Pagerfanta\Pagerfanta
-     */
-    protected function setupPager($query, $page)
-    {
-        $paginator = new Pagerfanta(new DoctrineORMAdapter($query, true));
-        $paginator->setMaxPerPage(15);
-        $paginator->setCurrentPage($page, false, true);
-
-        return $paginator;
-    }
-
-    /**
-     * @param array $orderBys
-     *
-     * @return array
-     */
-    protected function getFilteredOrderedBys(array $orderBys)
+    public function searchFormAction(Request $req)
     {
-        if ($orderBys) {
-            $allowedSorts = array(
-                'downloads' => 1,
-                'favers' => 1
-            );
-
-            $allowedOrders = array(
-                'asc' => 1,
-                'desc' => 1,
-            );
-
-            $filteredOrderBys = array();
-
-            foreach ($orderBys as $orderBy) {
-                if (isset($orderBy['sort'])
-                    && isset($allowedSorts[$orderBy['sort']])
-                    && isset($orderBy['order'])
-                    && isset($allowedOrders[$orderBy['order']])) {
-                    $filteredOrderBys[] = $orderBy;
-                }
-            }
-        } else {
-            $filteredOrderBys = array();
-        }
+        $form = $this->createForm(new SearchQueryType, new SearchQuery);
 
-        return $filteredOrderBys;
-    }
+        $filteredOrderBys = $this->getFilteredOrderedBys($req);
+        $normalizedOrderBys = $this->getNormalizedOrderBys($filteredOrderBys);
 
-    /**
-     * @param array $orderBys
-     *
-     * @return array
-     */
-    protected function getNormalizedOrderBys(array $orderBys)
-    {
-        $normalizedOrderBys = array();
+        $this->computeSearchQuery($req, $filteredOrderBys);
 
-        foreach ($orderBys as $sort) {
-            $normalizedOrderBys[$sort['sort']] = $sort['order'];
+        if ($req->query->has('search_query')) {
+            $form->bind($req);
         }
 
-        return $normalizedOrderBys;
-    }
-
-    protected function getOrderBysViewModel(Request $req, $normalizedOrderBys)
-    {
-        $makeDefaultArrow = function ($sort) use ($normalizedOrderBys) {
-            if (isset($normalizedOrderBys[$sort])) {
-                if (strtolower($normalizedOrderBys[$sort]) === 'asc') {
-                    $val = 'icon-arrow-up';
-                } else {
-                    $val = 'icon-arrow-down';
-                }
-            } else {
-                $val = '';
-            }
-
-            return $val;
-        };
-
-        $makeDefaultHref = function ($sort) use ($req, $normalizedOrderBys) {
-            if (isset($normalizedOrderBys[$sort])) {
-                if (strtolower($normalizedOrderBys[$sort]) === 'asc') {
-                    $order = 'desc';
-                } else {
-                    $order = 'asc';
-                }
-            } else {
-                $order = 'desc';
-            }
-
-            return '?' . http_build_query(array(
-                'q' => $req->query->get('q') === null ? '' : $req->query->get('q'),
-                'orderBys' => array(
-                    array(
-                        'sort' => $sort,
-                        'order' => $order
-                    )
-                )
-            ));
-        };
-
-        return array(
-            'downloads' => array(
-                'title' => 'Clic to sort by downloads desc',
-                'class' => 'icon-download',
-                'arrowClass' => $makeDefaultArrow('downloads'),
-                'href' => $makeDefaultHref('downloads')
-            ),
-            'favers' => array(
-                'title' => 'Clic to sort by favorites desc',
-                'class' => 'icon-star',
-                'arrowClass' => $makeDefaultArrow('favers'),
-                'href' => $makeDefaultHref('favers')
-            ),
-        );
+        $orderBysViewModel = $this->getOrderBysViewModel($req, $normalizedOrderBys);
+        return $this->render('PackagistWebBundle:Web:searchForm.html.twig', array(
+            'searchForm' => $form->createView(),
+            'orderBys' => $orderBysViewModel
+        ));
     }
 
     /**
@@ -358,36 +254,12 @@ class WebController extends Controller
      */
     public function searchAction(Request $req)
     {
-        $form = $this->createSearchForm();
-
-        $orderBys = $req->query->get('orderBys', array());
+        $form = $this->createForm(new SearchQueryType, new SearchQuery);
 
-        $filteredOrderBys = $this->getFilteredOrderedBys($orderBys);
+        $filteredOrderBys = $this->getFilteredOrderedBys($req);
         $normalizedOrderBys = $this->getNormalizedOrderBys($filteredOrderBys);
 
-        if ($req->getRequestFormat() !== 'json' && !$req->isXmlHttpRequest()) {
-            $orderBysViewModel = $this->getOrderBysViewModel($req, $normalizedOrderBys);
-        }
-
-        // transform q=search shortcut
-        if ($req->query->has('q') || $req->query->has('orderBys')) {
-            $searchQuery = array();
-
-            $q = $req->query->get('q');
-
-            if ($q !== null) {
-                $searchQuery['query'] = $q;
-            }
-
-            if (!empty($filteredOrderBys)) {
-                $searchQuery['orderBys'] = $filteredOrderBys;
-            }
-
-            $req->query->set(
-                'search_query',
-                $searchQuery
-            );
-        }
+        $this->computeSearchQuery($req, $filteredOrderBys);
 
         $typeFilter = $req->query->get('type');
         $tagsFilter = $req->query->get('tags');
@@ -544,8 +416,6 @@ class WebController extends Controller
             return $this->render('PackagistWebBundle:Web:search.html.twig', array(
                 'packages' => $paginator,
                 'meta' => $metadata,
-                'searchForm' => $form->createView(),
-                'orderBys' => $orderBysViewModel
             ));
         } elseif ($req->getRequestFormat() === 'json') {
             return JsonResponse::create(array(
@@ -553,10 +423,7 @@ class WebController extends Controller
             ), 400)->setCallback($req->query->get('callback'));
         }
 
-        return $this->render('PackagistWebBundle:Web:search.html.twig', array(
-            'searchForm' => $form->createView(),
-            'orderBys' => $orderBysViewModel
-        ));
+        return $this->render('PackagistWebBundle:Web:search.html.twig');
     }
 
     /**
@@ -590,7 +457,7 @@ class WebController extends Controller
             }
         }
 
-        return array('form' => $form->createView(), 'page' => 'submit', 'searchForm' => $this->createSearchForm()->createView());
+        return array('form' => $form->createView(), 'page' => 'submit');
     }
 
     /**
@@ -673,8 +540,7 @@ class WebController extends Controller
             'packages' => $packages,
             'meta' => $this->getPackagesMetadata($packages),
             'vendor' => $vendor,
-            'paginate' => false,
-            'searchForm' => $this->createSearchForm()->createView()
+            'paginate' => false
         );
     }
 
@@ -720,8 +586,7 @@ class WebController extends Controller
             'name' => $name,
             'packages' => $providers,
             'meta' => $this->getPackagesMetadata($providers),
-            'paginate' => false,
-            'searchForm' => $this->createSearchForm()->createView()
+            'paginate' => false
         ));
     }
 
@@ -822,7 +687,6 @@ class WebController extends Controller
         } catch (ConnectionException $e) {
         }
 
-        $data['searchForm'] = $this->createSearchForm()->createView();
         if ($maintainerForm = $this->createAddMaintainerForm($package)) {
             $data['addMaintainerForm'] = $maintainerForm->createView();
         }
@@ -1131,7 +995,6 @@ class WebController extends Controller
             }
         }
 
-        $data['searchForm'] = $this->createSearchForm()->createView();
         return $data;
     }
 
@@ -1187,7 +1050,6 @@ class WebController extends Controller
             }
         }
 
-        $data['searchForm'] = $this->createSearchForm()->createView();
         return $data;
     }
 
@@ -1295,15 +1157,6 @@ class WebController extends Controller
         return new RedirectResponse('http://getcomposer.org/', 301);
     }
 
-    public function render($view, array $parameters = array(), Response $response = null)
-    {
-        if (!isset($parameters['searchForm'])) {
-            $parameters['searchForm'] = $this->createSearchForm()->createView();
-        }
-
-        return parent::render($view, $parameters, $response);
-    }
-
     private function createAddMaintainerForm($package)
     {
         if (!$user = $this->getUser()) {
@@ -1356,8 +1209,156 @@ class WebController extends Controller
         return $this->createFormBuilder(array())->getForm();
     }
 
-    private function createSearchForm()
+    /**
+     * Initializes the pager for a query.
+     *
+     * @param \Doctrine\ORM\QueryBuilder $query Query for packages
+     * @param int                        $page  Pagenumber to retrieve.
+     * @return \Pagerfanta\Pagerfanta
+     */
+    protected function setupPager($query, $page)
+    {
+        $paginator = new Pagerfanta(new DoctrineORMAdapter($query, true));
+        $paginator->setMaxPerPage(15);
+        $paginator->setCurrentPage($page, false, true);
+
+        return $paginator;
+    }
+
+    /**
+     * @param array $orderBys
+     *
+     * @return array
+     */
+    protected function getFilteredOrderedBys(Request $req)
+    {
+        $orderBys = $req->query->get('orderBys', array());
+        if (!$orderBys) {
+            $orderBys = $req->query->get('search_query');
+            $orderBys = isset($orderBys['orderBys']) ? $orderBys['orderBys'] : array();
+        }
+
+        if ($orderBys) {
+            $allowedSorts = array(
+                'downloads' => 1,
+                'favers' => 1
+            );
+
+            $allowedOrders = array(
+                'asc' => 1,
+                'desc' => 1,
+            );
+
+            $filteredOrderBys = array();
+
+            foreach ($orderBys as $orderBy) {
+                if (isset($orderBy['sort'])
+                    && isset($allowedSorts[$orderBy['sort']])
+                    && isset($orderBy['order'])
+                    && isset($allowedOrders[$orderBy['order']])) {
+                    $filteredOrderBys[] = $orderBy;
+                }
+            }
+        } else {
+            $filteredOrderBys = array();
+        }
+
+        return $filteredOrderBys;
+    }
+
+    /**
+     * @param array $orderBys
+     *
+     * @return array
+     */
+    protected function getNormalizedOrderBys(array $orderBys)
+    {
+        $normalizedOrderBys = array();
+
+        foreach ($orderBys as $sort) {
+            $normalizedOrderBys[$sort['sort']] = $sort['order'];
+        }
+
+        return $normalizedOrderBys;
+    }
+
+    protected function getOrderBysViewModel(Request $req, $normalizedOrderBys)
     {
-        return $this->createForm(new SearchQueryType, new SearchQuery);
+        $makeDefaultArrow = function ($sort) use ($normalizedOrderBys) {
+            if (isset($normalizedOrderBys[$sort])) {
+                if (strtolower($normalizedOrderBys[$sort]) === 'asc') {
+                    $val = 'icon-arrow-up';
+                } else {
+                    $val = 'icon-arrow-down';
+                }
+            } else {
+                $val = '';
+            }
+
+            return $val;
+        };
+
+        $makeDefaultHref = function ($sort) use ($req, $normalizedOrderBys) {
+            if (isset($normalizedOrderBys[$sort])) {
+                if (strtolower($normalizedOrderBys[$sort]) === 'asc') {
+                    $order = 'desc';
+                } else {
+                    $order = 'asc';
+                }
+            } else {
+                $order = 'desc';
+            }
+
+            $query = $req->query->get('search_query');
+            $query = isset($query['query']) ? $query['query'] : '';
+
+            return '?' . http_build_query(array(
+                'q' => $query,
+                'orderBys' => array(
+                    array(
+                        'sort' => $sort,
+                        'order' => $order
+                    )
+                )
+            ));
+        };
+
+        return array(
+            'downloads' => array(
+                'title' => 'Sort by downloads',
+                'class' => 'icon-download',
+                'arrowClass' => $makeDefaultArrow('downloads'),
+                'href' => $makeDefaultHref('downloads')
+            ),
+            'favers' => array(
+                'title' => 'Sort by favorites',
+                'class' => 'icon-star',
+                'arrowClass' => $makeDefaultArrow('favers'),
+                'href' => $makeDefaultHref('favers')
+            ),
+        );
+    }
+
+    private function computeSearchQuery(Request $req, array $filteredOrderBys)
+    {
+        // transform q=search shortcut
+        if ($req->query->has('q') || $req->query->has('orderBys')) {
+            $searchQuery = array();
+
+            $q = $req->query->get('q');
+
+            if ($q !== null) {
+                $searchQuery['query'] = $q;
+            }
+
+            if (!empty($filteredOrderBys)) {
+                $searchQuery['orderBys'] = $filteredOrderBys;
+            }
+
+            $req->query->set(
+                'search_query',
+                $searchQuery
+            );
+        }
     }
 }

+ 11 - 4
src/Packagist/WebBundle/Resources/public/css/main.css

@@ -446,7 +446,7 @@ form ul {
 }
 
 .sortable #search_query_query {
-  width: 737px;
+  width: 777px;
   display: inline;
 }
 .sortable .no-js #search_query_query {
@@ -457,16 +457,23 @@ form ul {
   display: none;
 }
 
+.sortable .icon.inactive {
+  color: #555;
+}
+
 .sortable #order-bys-wrapper {
   margin-left: 0.7em;
   display: inline;
   font-size: 24px;
   position: relative;
-  top: 4px;
+  top: 6px;
+}
+
+.sortable .order-by-group .icon:first-child {
+  margin-right: 5px;
 }
 
 .sortable #order-bys-wrapper a {
-  width: 46px;
   margin-right: 7px;
   display: inline-block;
 }
@@ -809,4 +816,4 @@ pre {
   @page { margin: 0.5cm; }
   p, h2, h3 { orphans: 3; widows: 3; }
   h2, h3{ page-break-after: avoid; }
-}
+}

+ 3 - 0
src/Packagist/WebBundle/Resources/public/js/search.js

@@ -18,6 +18,9 @@
         list.html(newList.html());
         list.removeClass('hidden');
         list.find('ul.packages li:first').addClass('selected');
+        $('.order-by-group').attr('href', function (index, current) {
+            return current.replace(/q=.*?&/, 'q=' + encodeURIComponent($('input[type="search"]', form).val()) + '&')
+        });
 
         searching = false;
 

+ 1 - 1
src/Packagist/WebBundle/Resources/views/Web/search.html.twig

@@ -5,7 +5,7 @@
 
 {% block search %}
     <div class="box clearfix">
-        {% include "PackagistWebBundle:Web:searchForm.html.twig" %}
+        {{ render(controller('PackagistWebBundle:Web:searchForm', { req: app.request })) }}
 
         <div class="search-list {% if packages is not defined %}hidden{% endif %}">
             {% if packages is defined %}

+ 15 - 9
src/Packagist/WebBundle/Resources/views/Web/searchForm.html.twig

@@ -3,19 +3,25 @@
     <div class="{% if orderBys is defined %}sortable{% endif %}">
         {{ form_errors(searchForm.query) }}
         {{ form_widget(searchForm.query, {'attr': {'autocomplete': 'off', 'autofocus': 'autofocus', 'placeholder': 'Search packages...', 'tabindex': 1}}) }}
-        {% if orderBys is defined %}
-            <div id="order-bys-wrapper">
-                {% for sort,param in orderBys %}
-                    <a title="{{ param.title }}" href="{{ param.href }}">
-                        <i class="icon {{ param.class }}"></i>
+        {% set hasActiveOrderBy = false %}
+        {% spaceless %}
+        <div id="order-bys-wrapper">
+            {% for sort, param in orderBys %}
+                <a title="{{ param.title }}" href="{{ param.href }}" class="order-by-group">
+                    <i class="icon {{ param.class }}{% if param.arrowClass is empty %} inactive{% endif %}"></i>
+                    {% if param.arrowClass is not empty %}
                         <i class="icon {{ param.arrowClass }}"></i>
-                    </a>
-                {% endfor %}
+                        {% set hasActiveOrderBy = true %}
+                    {% endif %}
+                </a>
+            {% endfor %}
+            {% if hasActiveOrderBy %}
                 <a title="Clear order bys" href="?q={{ searchForm.vars.value.query }}" class="clear">
                     <i class="icon icon-remove-circle"></i>
                 </a>
-            </div>
-        {% endif  %}
+            {% endif %}
+        </div>
+        {% endspaceless %}
         <div
             class="hidden">
             {{ form_widget(searchForm.orderBys) }}

+ 4 - 6
src/Packagist/WebBundle/Resources/views/layout.html.twig

@@ -74,13 +74,11 @@
                 {% endfor %}
 
                 {% block search %}
-                    {% if searchForm is defined %}
-                        <div class="box">
-                            {% include "PackagistWebBundle:Web:searchForm.html.twig" %}
-                            <div class="search-list hidden">
-                            </div>
+                    <div class="box">
+                        {{ render(controller('PackagistWebBundle:Web:searchForm', { req: app.request })) }}
+                        <div class="search-list hidden">
                         </div>
-                    {% endif %}
+                    </div>
                 {% endblock %}
 
                 {% block content %}

+ 6 - 6
src/Packagist/WebBundle/Tests/Controller/WebControllerTest.php

@@ -249,10 +249,10 @@ class WebControllerTest extends WebTestCase
 
         $kernelRootDir = $container->getParameter('kernel.root_dir');
 
-        $this->executeCommand($kernelRootDir . '/console doctrine:database:drop --env=test --force', false);
-        $this->executeCommand($kernelRootDir . '/console doctrine:database:create --env=test');
-        $this->executeCommand($kernelRootDir . '/console doctrine:schema:create --env=test');
-        $this->executeCommand($kernelRootDir . '/console redis:flushall --env=test -n');
+        $this->executeCommand('php '.$kernelRootDir . '/console doctrine:database:drop --env=test --force', false);
+        $this->executeCommand('php '.$kernelRootDir . '/console doctrine:database:create --env=test');
+        $this->executeCommand('php '.$kernelRootDir . '/console doctrine:schema:create --env=test');
+        $this->executeCommand('php '.$kernelRootDir . '/console redis:flushall --env=test -n');
 
         $lock = $container->getParameter('kernel.cache_dir').'/composer-indexer.lock';
 
@@ -293,7 +293,7 @@ class WebControllerTest extends WebTestCase
 
         $onBeforeIndex($container, $twigPackage, $packagistPackage, $symfonyPackage);
 
-        $this->executeCommand($kernelRootDir . '/console packagist:index --env=test --force');
+        $this->executeCommand('php '.$kernelRootDir . '/console packagist:index --env=test --force');
 
         $client->request('GET', '/search.json?q=' . $orderBysQryStrPart);
 
@@ -444,4 +444,4 @@ class WebControllerTest extends WebTestCase
             'total' => count($results)
         );
     }
-}
+}