From 362ecf564a01b321d43c22c5e50ee3a5c7a422b9 Mon Sep 17 00:00:00 2001
From: Dave Dash <dd+github@davedash.com>
Date: Tue, 7 Dec 2010 16:47:23 -0800
Subject: [PATCH] Fixes for device/manufacture faceting.

---
 .../dashboard/templates/dashboard/devices.html | 17 +++++++++++------
 .../dashboard/templates/dashboard/locales.html |  2 +-
 .../templates/dashboard/manufacturers.html     | 13 +++++++++----
 apps/feedback/helpers.py                       |  6 +++---
 apps/search/client.py                          |  6 +++++-
 apps/search/forms.py                           | 18 ++++++++++++++----
 apps/search/templates/search/search.html       | 10 +++++-----
 apps/search/tests.py                           | 17 +++++++++++++++++
 8 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/apps/dashboard/templates/dashboard/devices.html b/apps/dashboard/templates/dashboard/devices.html
index d154e395..6a6c586f 100644
--- a/apps/dashboard/templates/dashboard/devices.html
+++ b/apps/dashboard/templates/dashboard/devices.html
@@ -1,9 +1,11 @@
 {% macro device_item(device, count=0, selected=False) %}
 <li>
-  <input id="device_{{ device }}" name="device" value="{{ device }}"
+  <input id="device_{{ device }}" name="device"
+         value="{{ device or 'Unknown' }}"
+         {{ 'disabled' if defaults['manufacturer'] and not device }}
          type="checkbox" {{ 'checked' if selected else '' }} />
   <label for="device_{{ device }}" class="bar {{ 'selected' if selected else '' }}" data-value="{{ count }}">
-    <strong>{{ device }}</strong>
+    <strong>{{ device or _('Unknown') }}</strong>
   </label>
 </li>
 {% endmacro %}
@@ -13,10 +15,13 @@
   {{ filter_box_toggle(_('Devices')) }}
 
   <ul class="bars" data-total="{{ total }}">
-
-    {% for item in devices %}
-      {{ device_item(item.device, item.count) }}
-    {% endfor %}
+    {% if defaults['device'] %}
+      {{ device_item(defaults['device'], selected=True) }}
+    {% else %}
+      {% for item in devices[:15] %}
+        {{ device_item(item.device, item.count) }}
+      {% endfor %}
+    {% endif %}
   </ul>
 </div>
 {% endif %}
diff --git a/apps/dashboard/templates/dashboard/locales.html b/apps/dashboard/templates/dashboard/locales.html
index ec8c370e..507fda85 100644
--- a/apps/dashboard/templates/dashboard/locales.html
+++ b/apps/dashboard/templates/dashboard/locales.html
@@ -18,7 +18,7 @@
     {% if defaults['locale'] %}
       {{ locale_item(defaults['locale'], selected=True) }}
     {% else %}
-      {% for item in locales %}
+      {% for item in locales[:15] %}
       {{ locale_item(item.locale, item.count) }}
       {% endfor %}
     {% endif %}
diff --git a/apps/dashboard/templates/dashboard/manufacturers.html b/apps/dashboard/templates/dashboard/manufacturers.html
index 6b3dde3c..b1cf22dd 100644
--- a/apps/dashboard/templates/dashboard/manufacturers.html
+++ b/apps/dashboard/templates/dashboard/manufacturers.html
@@ -1,6 +1,7 @@
 {% macro brand_item(manufacturer, count=0, selected=False) %}
 <li>
-  <input id="brand_{{ manufacturer }}" name="brand" value="{{ manufacturer }}"
+  <input id="brand_{{ manufacturer }}" name="manufacturer"
+         value="{{ manufacturer or 'Unknown' }}"
          type="checkbox" {{ 'checked' if selected else '' }} />
   <label for="brand_{{ manufacturer }}"
          class="bar {{ 'selected' if selected else '' }}"
@@ -15,9 +16,13 @@
   {{ filter_box_toggle(_('Manufacturer')) }}
 
   <ul class="bars" data-total="{{ total }}">
-    {% for item in manufacturers %}
-      {{ brand_item(item.manufacturer, item.count) }}
-    {% endfor %}
+    {% if defaults['manufacturer'] %}
+      {{ brand_item(defaults['manufacturer'], selected=True) }}
+    {% else %}
+      {% for item in manufacturers %}
+        {{ brand_item(item.manufacturer, item.count) }}
+      {% endfor %}
+    {% endif %}
   </ul>
 </div>
 {% endif %}
diff --git a/apps/feedback/helpers.py b/apps/feedback/helpers.py
index 3cbda737..72a53d0b 100644
--- a/apps/feedback/helpers.py
+++ b/apps/feedback/helpers.py
@@ -13,7 +13,7 @@ def os_name(os):
 
 
 @register.function
-def locale_name(locale, native=False, default=_lazy('unknown')):
+def locale_name(locale, native=False, default=_lazy('Unknown')):
     """Convert a locale code into a human readable locale name."""
     if locale in product_details.languages:
         return product_details.languages[locale][
@@ -31,10 +31,10 @@ def smiley(style, page=None):
     """
     if not style in ('happy', 'sad'):
         return ''
-    if style == 'happy': # positive smiley
+    if style == 'happy':  # positive smiley
         character = '&#9786;'
         title = _('Praise')
-    else: # negative smiley
+    else:  # negative smiley
         character = '&#9785;'
         title = _('Issue')
     return jinja2.Markup(
diff --git a/apps/search/client.py b/apps/search/client.py
index 8224cfdc..1c0fcc36 100644
--- a/apps/search/client.py
+++ b/apps/search/client.py
@@ -59,7 +59,11 @@ def extract_filters(kwargs):
         metas['type'] = kwargs['type']
 
     for meta in ('os', 'manufacturer', 'device'):
-        if kwargs.get(meta):
+        val = kwargs.get(meta)
+        if val and val.lower() == 'unknown':
+            # In this situation 'unknown' usually means empty.
+            metas[meta] = crc32('')
+        elif val:
             metas[meta] = crc32(kwargs[meta])
 
     if kwargs.get('locale'):
diff --git a/apps/search/forms.py b/apps/search/forms.py
index b0566934..b641513d 100644
--- a/apps/search/forms.py
+++ b/apps/search/forms.py
@@ -6,8 +6,9 @@
 from product_details import product_details
 from tower import ugettext_lazy as _lazy
 
-from feedback import FIREFOX, MOBILE, OS_USAGE, OPINION_TYPES
+from feedback import FIREFOX, MOBILE, OS_USAGE
 from feedback.version_compare import simplify_version, version_int
+from input import KNOWN_DEVICES, KNOWN_MANUFACTURERS
 from input.fields import DateInput, SearchInput
 from input.utils import uniquifier
 
@@ -34,17 +35,23 @@
 SENTIMENT_CHOICES = [('', _lazy('-- all --', 'sentiment_choice')),
     ('happy', _lazy('Praise')),
     ('sad', _lazy('Issues')),
-    ('suggestions', _lazy('Suggestions'))
+    ('suggestions', _lazy('Suggestions')),
 ]
-SENTIMENTS = ('happy', 'sad', 'suggestions')
 
+SENTIMENTS = ('happy', 'sad', 'suggestions')
 
 OS_CHOICES = ([('', _lazy('-- all --', 'os_choice'))] +
               [(o.short, o.pretty) for o in OS_USAGE])
 
+MANUFACTURER_CHOICES = [('Unknown', _lazy('Unknown'))] + [(m, m) for m in
+                                                          KNOWN_MANUFACTURERS]
+
+DEVICE_CHOICES = [('Unknown', _lazy('Unknown'))] + [(d, d) for d in
+                                                    KNOWN_DEVICES]
+
 LOCALE_CHOICES = [
     ('', _lazy('-- all --', 'locale_choice')),
-    ('unknown', _lazy('unknown')),
+    ('Unknown', _lazy('Unknown')),
 ] + [(lang, lang) for lang in sorted(product_details.languages)]
 
 
@@ -61,6 +68,9 @@ class ReporterSearchForm(forms.Form):
                                choices=LOCALE_CHOICES)
     os = forms.ChoiceField(required=False, label=_lazy('OS:'),
                            choices=OS_CHOICES)
+    manufacturer = forms.ChoiceField(required=False,
+                                     choices=MANUFACTURER_CHOICES)
+    device = forms.ChoiceField(required=False, choices=DEVICE_CHOICES)
     date_start = forms.DateField(required=False, widget=DateInput(
         attrs={'class': 'datepicker'}), label=_lazy('Date range:'))
     date_end = forms.DateField(required=False, widget=DateInput(
diff --git a/apps/search/templates/search/search.html b/apps/search/templates/search/search.html
index 4d8632ee..5ab4ec0b 100644
--- a/apps/search/templates/search/search.html
+++ b/apps/search/templates/search/search.html
@@ -31,12 +31,12 @@ <h3>{{ _('Product') }}</h3>
     {# TODO URL provided box #}
 
     {{ platforms_block(demo.os, opinion_count, defaults=defaults) }}
+    {% if product == 'mobile' %}
+      {{ manufacturer_block(demo.manufacturer, opinion_count,
+                            defaults=defaults) }}
 
-    {{ manufacturer_block(demo.manufacturer, opinion_count,
-                          defaults=defaults) }}
-
-    {{ device_block(demo.device, opinion_count, defaults=defaults) }}
-
+      {{ device_block(demo.device, opinion_count, defaults=defaults) }}
+    {% endif %}
     {{ locales_block(demo.locale, opinion_count, defaults=defaults) }}
 
   </form>
diff --git a/apps/search/tests.py b/apps/search/tests.py
index 94ff594a..eeed8d3a 100644
--- a/apps/search/tests.py
+++ b/apps/search/tests.py
@@ -143,12 +143,29 @@ class SearchViewTest(SphinxTestCase):
     def setUp(self):
         # add more opinions so we can test things.
         populate(100, 'desktop')
+        populate(100)
         super(SearchViewTest, self).setUp()
 
     def test_pagination_max(self):
         r = search_request(page=700)
         self.failUnlessEqual(r.status_code, 200)
 
+    def test_filters(self):
+        """
+        Make sure Manufacturer and Device filters don't show up on search for
+        desktop.
+        """
+        r = search_request()
+        doc = pq(r.content)
+        filters = doc('.filter h3 a')
+        assert not any(['Manufacturer' in f.text_content() for f in filters])
+        assert not any(['Device' in f.text_content() for f in filters])
+        r = search_request(product='mobile')
+        doc = pq(r.content)
+        filters = doc('.filter h3 a')
+        assert any(['Manufacturer' in f.text_content() for f in filters])
+        assert any(['Device' in f.text_content() for f in filters])
+
     def compare_2_pages(self, page1, page2):
         r = search_request(page=page1)
         doc = pq(r.content)