From aeeb50f61b7b1bf8ccb61448f8b8253db457d350 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Wed, 20 Jun 2018 21:30:13 -0700
Subject: [PATCH] Correctly display facets with value of 0 - fixes #318

Also added comprehensive unit test for facet display HTML.
---
 datasette/templates/table.html |  2 +-
 tests/fixtures.py              | 35 +++++++-------
 tests/test_api.py              |  4 +-
 tests/test_csv.py              | 32 ++++++-------
 tests/test_html.py             | 83 ++++++++++++++++++++++++++++++++++
 5 files changed, 122 insertions(+), 34 deletions(-)

diff --git a/datasette/templates/table.html b/datasette/templates/table.html
index d09e6564..27cc1467 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -117,7 +117,7 @@
                 <ul>
                     {% for facet_value in facet_info.results %}
                         {% if not facet_value.selected %}
-                            <li><a href="{{ facet_value.toggle_url }}">{{ facet_value.label or "_" }}</a> {{ "{:,}".format(facet_value.count) }}</li>
+                            <li><a href="{{ facet_value.toggle_url }}">{{ (facet_value.label if facet_value.label is not none else "_") }}</a> {{ "{:,}".format(facet_value.count) }}</li>
                         {% else %}
                             <li>{{ facet_value.label }} &middot; {{ "{:,}".format(facet_value.count) }} <a href="{{ facet_value.toggle_url }}" class="cross">&#x2716;</a></li>
                         {% endif %}
diff --git a/tests/fixtures.py b/tests/fixtures.py
index ea0b4e35..600e4aad 100644
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@@ -332,27 +332,30 @@ INSERT INTO facet_cities (id, name) VALUES
 CREATE TABLE facetable (
     pk integer primary key,
     planet_int integer,
+    on_earth integer,
     state text,
     city_id integer,
     neighborhood text,
     FOREIGN KEY ("city_id") REFERENCES [facet_cities](id)
 );
-INSERT INTO facetable (planet_int, state, city_id, neighborhood) VALUES
-    (1, 'CA', 1, 'Mission'),
-    (1, 'CA', 1, 'Dogpatch'),
-    (1, 'CA', 1, 'SOMA'),
-    (1, 'CA', 1, 'Tenderloin'),
-    (1, 'CA', 1, 'Bernal Heights'),
-    (1, 'CA', 1, 'Hayes Valley'),
-    (1, 'CA', 2, 'Hollywood'),
-    (1, 'CA', 2, 'Downtown'),
-    (1, 'CA', 2, 'Los Feliz'),
-    (1, 'CA', 2, 'Koreatown'),
-    (1, 'MI', 3, 'Downtown'),
-    (1, 'MI', 3, 'Greektown'),
-    (1, 'MI', 3, 'Corktown'),
-    (1, 'MI', 3, 'Mexicantown'),
-    (2, 'MC', 4, 'Arcadia Planitia')
+INSERT INTO facetable
+    (planet_int, on_earth, state, city_id, neighborhood)
+VALUES
+    (1, 1, 'CA', 1, 'Mission'),
+    (1, 1, 'CA', 1, 'Dogpatch'),
+    (1, 1, 'CA', 1, 'SOMA'),
+    (1, 1, 'CA', 1, 'Tenderloin'),
+    (1, 1, 'CA', 1, 'Bernal Heights'),
+    (1, 1, 'CA', 1, 'Hayes Valley'),
+    (1, 1, 'CA', 2, 'Hollywood'),
+    (1, 1, 'CA', 2, 'Downtown'),
+    (1, 1, 'CA', 2, 'Los Feliz'),
+    (1, 1, 'CA', 2, 'Koreatown'),
+    (1, 1, 'MI', 3, 'Downtown'),
+    (1, 1, 'MI', 3, 'Greektown'),
+    (1, 1, 'MI', 3, 'Corktown'),
+    (1, 1, 'MI', 3, 'Mexicantown'),
+    (2, 0, 'MC', 4, 'Arcadia Planitia')
 ;
 
 INSERT INTO simple_primary_key VALUES (1, 'hello');
diff --git a/tests/test_api.py b/tests/test_api.py
index 2187deb5..eb6175a8 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -117,7 +117,7 @@ def test_database_page(app_client):
         'label_column': 'name',
         'primary_keys': ['id'],
     }, {
-        'columns': ['pk', 'planet_int', 'state', 'city_id', 'neighborhood'],
+        'columns': ['pk', 'planet_int', 'on_earth', 'state', 'city_id', 'neighborhood'],
         'name': 'facetable',
         'count': 15,
         'foreign_keys': {
@@ -1107,6 +1107,7 @@ def test_expand_labels(app_client):
         "2": {
             "pk": 2,
             "planet_int": 1,
+            "on_earth": 1,
             "state": "CA",
             "city_id": {
                 "value": 1,
@@ -1117,6 +1118,7 @@ def test_expand_labels(app_client):
         "13": {
             "pk": 13,
             "planet_int": 1,
+            "on_earth": 1,
             "state": "MI",
             "city_id": {
                 "value": 3,
diff --git a/tests/test_csv.py b/tests/test_csv.py
index fcbacc76..194de5b1 100644
--- a/tests/test_csv.py
+++ b/tests/test_csv.py
@@ -12,22 +12,22 @@ world
 '''.replace('\n', '\r\n')
 
 EXPECTED_TABLE_WITH_LABELS_CSV = '''
-pk,planet_int,state,city_id,city_id_label,neighborhood
-1,1,CA,1,San Francisco,Mission
-2,1,CA,1,San Francisco,Dogpatch
-3,1,CA,1,San Francisco,SOMA
-4,1,CA,1,San Francisco,Tenderloin
-5,1,CA,1,San Francisco,Bernal Heights
-6,1,CA,1,San Francisco,Hayes Valley
-7,1,CA,2,Los Angeles,Hollywood
-8,1,CA,2,Los Angeles,Downtown
-9,1,CA,2,Los Angeles,Los Feliz
-10,1,CA,2,Los Angeles,Koreatown
-11,1,MI,3,Detroit,Downtown
-12,1,MI,3,Detroit,Greektown
-13,1,MI,3,Detroit,Corktown
-14,1,MI,3,Detroit,Mexicantown
-15,2,MC,4,Memnonia,Arcadia Planitia
+pk,planet_int,on_earth,state,city_id,city_id_label,neighborhood
+1,1,1,CA,1,San Francisco,Mission
+2,1,1,CA,1,San Francisco,Dogpatch
+3,1,1,CA,1,San Francisco,SOMA
+4,1,1,CA,1,San Francisco,Tenderloin
+5,1,1,CA,1,San Francisco,Bernal Heights
+6,1,1,CA,1,San Francisco,Hayes Valley
+7,1,1,CA,2,Los Angeles,Hollywood
+8,1,1,CA,2,Los Angeles,Downtown
+9,1,1,CA,2,Los Angeles,Los Feliz
+10,1,1,CA,2,Los Angeles,Koreatown
+11,1,1,MI,3,Detroit,Downtown
+12,1,1,MI,3,Detroit,Greektown
+13,1,1,MI,3,Detroit,Corktown
+14,1,1,MI,3,Detroit,Mexicantown
+15,2,0,MC,4,Memnonia,Arcadia Planitia
 '''.lstrip().replace('\n', '\r\n')
 
 def test_table_csv(app_client):
diff --git a/tests/test_html.py b/tests/test_html.py
index 11c5fd43..3354f878 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -211,6 +211,89 @@ def test_sort_links(app_client):
     ] == attrs_and_link_attrs
 
 
+def test_facet_display(app_client):
+    response = app_client.get(
+        "/fixtures/facetable?_facet=planet_int&_facet=city_id&_facet=on_earth"
+    )
+    assert response.status == 200
+    soup = Soup(response.body, "html.parser")
+    divs = soup.find(
+        "div", {"class": "facet-results"}
+    ).findAll("div")
+    actual = []
+    for div in divs:
+        actual.append(
+            {
+                "name": div.find("strong").text,
+                "items": [
+                    {
+                        "name": a.text,
+                        "qs": a["href"].split("?")[-1],
+                        "count": int(str(a.parent).split("</a>")[1].split("<")[0]),
+                    }
+                    for a in div.find("ul").findAll("a")
+                ],
+            }
+        )
+    assert [
+        {
+            "name": "city_id",
+            "items": [
+                {
+                    "name": "San Francisco",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&city_id=1",
+                    "count": 6,
+                },
+                {
+                    "name": "Los Angeles",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&city_id=2",
+                    "count": 4,
+                },
+                {
+                    "name": "Detroit",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&city_id=3",
+                    "count": 4,
+                },
+                {
+                    "name": "Memnonia",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&city_id=4",
+                    "count": 1,
+                },
+            ],
+        },
+        {
+            "name": "planet_int",
+            "items": [
+                {
+                    "name": "1",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&planet_int=1",
+                    "count": 14,
+                },
+                {
+                    "name": "2",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&planet_int=2",
+                    "count": 1,
+                },
+            ],
+        },
+        {
+            "name": "on_earth",
+            "items": [
+                {
+                    "name": "1",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&on_earth=1",
+                    "count": 14,
+                },
+                {
+                    "name": "0",
+                    "qs": "_facet=planet_int&_facet=city_id&_facet=on_earth&on_earth=0",
+                    "count": 1,
+                },
+            ],
+        },
+    ] == actual
+
+
 def test_facets_persist_through_filter_form(app_client):
     response = app_client.get(
         '/fixtures/facetable?_facet=planet_int&_facet=city_id'