ajax search results tweaks

2011-10-18 17:00:41 -07:00 · 2011-10-18 17:00:41 -07:00 · 59a47a57c7
--- a/apps/amo/middleware.py
+++ b/apps/amo/middleware.py
@ -183,6 +183,16 @@ class TimingMiddleware(object):
 pjax_log = commonware.log.getLogger('z.timer')


+class PjaxMiddleware(object):
+
+    def process_request(self, request):
+        request.PJAX = bool(request.META.get('HTTP_X_PJAX'))
+
+    def process_response(self, request, response):
+        patch_vary_headers(response, ['X-PJAX'])
+        return response
+
+
 class LazyPjaxMiddleware(object):

    def process_request(self, request):
--- a/apps/amo/templates/amo/impala/paginator.html
+++ b/apps/amo/templates/amo/impala/paginator.html
@ -15,10 +15,10 @@
         title="{{ _('Jump to first page') }}"
         {% if not pager.has_previous() %}class="disabled"{% endif %}>
        &#x25C2;&#x25C2;</a>
-      <a href="{{ pager.url|urlparams(page=pager.previous_page_number()) }}"
+      <a href="{{ pager.url|urlparams(page=pager.previous_page_number()) if pager.has_previous() else '#' }}"
         class="button prev{% if not pager.has_previous() %} disabled{% endif %}">
        &#x25C2; {{ _('Previous') }}</a>
-      <a href="{{ pager.url|urlparams(page=pager.next_page_number()) }}"
+      <a href="{{ pager.url|urlparams(page=pager.next_page_number()) if pager.has_next() else '#' }}"
         class="button next{% if not pager.has_next() %} disabled{% endif %}">
        {{ _('Next') }} &#x25B8;</a>
      <a href="{{ pager.url|urlparams(page=pager.paginator.num_pages) }}"
--- a/apps/amo/tests/test_middleware.py
+++ b/apps/amo/tests/test_middleware.py
@ -12,7 +12,7 @@ from pyquery import PyQuery as pq
 from test_utils import RequestFactory

 import amo.tests
-from amo.middleware import LazyPjaxMiddleware
+from amo.middleware import PjaxMiddleware, LazyPjaxMiddleware
 from amo.urlresolvers import reverse
 from zadmin.models import Config, _config_cache

@ -22,11 +22,11 @@ class TestMiddleware(amo.tests.TestCase):
    def test_no_vary_cookie(self):
        # We don't break good usage of Vary.
        response = test.Client().get('/')
-        eq_(response['Vary'], 'Accept-Language, User-Agent, X-Mobile')
+        eq_(response['Vary'], 'Accept-Language, User-Agent, X-PJAX, X-Mobile')

        # But we do prevent Vary: Cookie.
        response = test.Client().get('/', follow=True)
-        eq_(response['Vary'], 'X-Mobile, User-Agent')
+        eq_(response['Vary'], 'X-PJAX, X-Mobile, User-Agent')


 def test_redirect_with_unicode_get():
@ -76,6 +76,38 @@ def test_hide_password_middleware():
    eq_(request.POST['password2'], '******')


+class TestPjaxMiddleware(amo.tests.TestCase):
+
+    def setUp(self):
+        self.factory = RequestFactory()
+
+    def check(self, xpjax, pjax):
+        request = self.factory.get('/')
+        if xpjax:
+            request.META['HTTP_X_PJAX'] = xpjax
+        PjaxMiddleware().process_request(request)
+        eq_(request.PJAX, pjax)
+
+    def test_xpjax(self):
+        self.check(xpjax='true', pjax=True)
+        self.check(xpjax='1', pjax=True)
+        self.check(xpjax='0', pjax=True)
+
+    def test_no_xpjax(self):
+        self.check(xpjax=None, pjax=False)
+
+    def test_vary(self):
+        request = self.factory.get('/')
+        response = http.HttpResponse()
+        r = PjaxMiddleware().process_response(request, response)
+        assert r is response
+        eq_(response['Vary'], 'X-PJAX')
+
+        response['Vary'] = 'User-Agent'
+        PjaxMiddleware().process_response(request, response)
+        eq_(response['Vary'], 'User-Agent, X-PJAX')
+
+
 class TestLazyPjaxMiddleware(amo.tests.TestCase):

    def setUp(self):
--- a/apps/search/context_processors.py
+++ b/apps/search/context_processors.py
@ -0,0 +1,2 @@
+def is_pjax(request):
+    return {'PJAX': request.PJAX}
--- a/apps/search/templates/search/results.html
+++ b/apps/search/templates/search/results.html
@ -1,4 +1,4 @@
-{% extends "base_ajax.html" if is_pjax else "impala/base.html" %}
+{% extends "base_ajax.html" if PJAX else "impala/base.html" %}

 {% block bodyclass %}
  {{ 'pjax' if waffle.switch('ajax-search') }} {{ super() }}
@ -51,7 +51,7 @@
 {% endmacro %}

 {% block content %}
-  {% if is_pjax %}
+  {% if PJAX %}
    {% include "search/results_inner.html" %}
  {% else %}
    <section id="search-facets" class="secondary" role="complementary">
--- a/apps/search/tests/test_views.py
+++ b/apps/search/tests/test_views.py
@ -191,7 +191,6 @@ class TestESSearch(amo.tests.ESTestCase):
        for view in self.search_views:
            r = self.client.get(reverse(view))
            eq_(r.status_code, 200)
-            eq_(r.context['is_pjax'], None)

            for var in expected_context_vars[view]:
                assert var in r.context, (
@ -207,7 +206,6 @@ class TestESSearch(amo.tests.ESTestCase):
        for view in self.search_views:
            r = self.client.get(reverse(view), HTTP_X_PJAX=True)
            eq_(r.status_code, 200)
-            eq_(r.context['is_pjax'], True)

            doc = pq(r.content)
            eq_(doc('html').length, 0)
--- a/apps/search/views.py
+++ b/apps/search/views.py
@ -419,9 +419,10 @@ def app_search(request, template=None):
    query = form.cleaned_data
    qs = (Webapp.search().query(or_=name_query(query['q']))
          .filter(type=amo.ADDON_WEBAPP, status=amo.STATUS_PUBLIC,
-                  is_disabled=False)
-          .facet(tags={'terms': {'field': 'tag'}},
-                 categories={'terms': {'field': 'category', 'size': 100}}))
+                  is_disabled=False))
+    if not request.PJAX:
+        qs = qs.facet(tags={'terms': {'field': 'tag'}},
+                      categories={'terms': {'field': 'category', 'size': 100}})
    if query.get('tag'):
        qs = qs.filter(tag=query['tag'])
    if query.get('cat'):
@ -435,17 +436,15 @@ def app_search(request, template=None):
        qs = qs.order_by(mapping[query['sort']])

    pager = amo.utils.paginate(request, qs)
-    facets = pager.object_list.facets

    ctx = {
-        'is_pjax': request.META.get('HTTP_X_PJAX'),
        'pager': pager,
        'query': query,
-        'form': form,
        'sorting': sort_sidebar(request, query, form),
        'sort_opts': form.fields['sort'].choices,
    }
-    if not ctx['is_pjax']:
+    if not request.PJAX:
+        facets = pager.object_list.facets
        ctx.update({
            'categories': category_sidebar(request, query, facets),
            'tags': tag_sidebar(request, query, facets),
@ -478,12 +477,13 @@ def search(request, tag_name=None, template=None):

    qs = (Addon.search()
          .filter(status__in=amo.REVIEWED_STATUSES, is_disabled=False,
-                  app=APP.id)
-          .facet(tags={'terms': {'field': 'tag'}},
-                 platforms={'terms': {'field': 'platform'}},
-                 appversions={'terms':
-                              {'field': 'appversion.%s.max' % APP.id}},
-                 categories={'terms': {'field': 'category', 'size': 100}}))
+                  app=APP.id))
+    if not request.PJAX:
+        qs = qs.facet(tags={'terms': {'field': 'tag'}},
+                      platforms={'terms': {'field': 'platform'}},
+                      appversions={'terms':
+                                   {'field': 'appversion.%s.max' % APP.id}},
+                      categories={'terms': {'field': 'category', 'size': 100}})
    if query.get('q'):
        qs = qs.query(or_=name_query(query['q']))
    if tag_name or query.get('tag'):
@ -526,15 +526,13 @@ def search(request, tag_name=None, template=None):
    pager = amo.utils.paginate(request, qs)

    ctx = {
-        'is_pjax': request.META.get('HTTP_X_PJAX'),
        'pager': pager,
        'query': query,
-        'form': form,
        'sort_opts': sort,
        'extra_sort_opts': extra_sort,
        'sorting': sort_sidebar(request, query, form),
    }
-    if not ctx['is_pjax']:
+    if not request.PJAX:
        facets = pager.object_list.facets
        ctx.update({
            'categories': category_sidebar(request, query, facets),
--- a/media/css/impala/search.less
+++ b/media/css/impala/search.less
@ -100,7 +100,7 @@
            }
            &.active {
                &:after {
-                    -moz-transform: rotate(90deg);
+                    .transform(rotate(90deg));
                }
                li a {
                    display: block;
@ -129,7 +129,7 @@
                left: 5px;
            }
            &.active:after {
-                -moz-transform: rotate(-90deg);
+                .transform(rotate(-90deg));
            }
        }
    }
--- a/media/js/impala/search.js
+++ b/media/js/impala/search.js
@ -82,7 +82,7 @@ function initSearchPjax(container) {
        $wrapper.removeClass('loading').find('.updating').remove();

        // Scroll up.
-        $('html').animate({scrollTop: 0}, 200);
+        $('html, body').animate({scrollTop: 0}, 200);
    }

    function turnPages(e) {
--- a/settings.py
+++ b/settings.py
@ -244,6 +244,7 @@ TEMPLATE_CONTEXT_PROCESSORS = (
    'amo.context_processors.i18n',
    'amo.context_processors.global_settings',
    'amo.context_processors.static_url',
+    'search.context_processors.is_pjax',
    'webapps.context_processors.is_webapps',
    'jingo_minify.helpers.build_ids',
 )
@ -282,6 +283,7 @@ MIDDLEWARE_CLASSES = (
    # Mobile detection should happen in Zeus.
    'mobility.middleware.DetectMobileMiddleware',
    'mobility.middleware.XMobileMiddleware',
+    'amo.middleware.PjaxMiddleware',
    # Disabled until ready:
    # 'amo.middleware.LazyPjaxMiddleware',
    'amo.middleware.RemoveSlashMiddleware',