Blank foreign key labels now show as hyphens, closes #1086

2020-11-11 15:37:37 -08:00 · 2020-11-11 15:37:37 -08:00 · 2a981e2ac1
commit 2a981e2ac1
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@ -156,9 +156,9 @@
                <ul class="tight-bullets">
                    {% for facet_value in facet_info.results %}
                        {% if not facet_value.selected %}
-                            <li><a href="{{ facet_value.toggle_url }}">{{ (facet_value.label if facet_value.label is not none else "_") }}</a> {{ "{:,}".format(facet_value.count) }}</li>
+                            <li><a href="{{ facet_value.toggle_url }}">{{ (facet_value.label | string()) or "-" }}</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>
+                            <li>{{ facet_value.label or "-" }} &middot; {{ "{:,}".format(facet_value.count) }} <a href="{{ facet_value.toggle_url }}" class="cross">&#x2716;</a></li>
                        {% endif %}
                    {% endfor %}
                    {% if facet_info.truncated %}
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@ -195,7 +195,7 @@ class RowTableShared(DataView):
                            table=urllib.parse.quote_plus(other_table),
                            link_id=urllib.parse.quote_plus(str(value)),
                            id=str(jinja2.escape(value)),
-                            label=str(jinja2.escape(label)),
+                            label=str(jinja2.escape(label)) or "-",
                        )
                    )
                elif value in ("", None):
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@ -386,8 +386,10 @@ CREATE INDEX idx_compound_three_primary_keys_content ON compound_three_primary_k
 CREATE TABLE foreign_key_references (
  pk varchar(30) primary key,
  foreign_key_with_label varchar(30),
+  foreign_key_with_blank_label varchar(30),
  foreign_key_with_no_label varchar(30),
  FOREIGN KEY (foreign_key_with_label) REFERENCES simple_primary_key(id),
+  FOREIGN KEY (foreign_key_with_blank_label) REFERENCES simple_primary_key(id),
  FOREIGN KEY (foreign_key_with_no_label) REFERENCES primary_key_multiple_columns(id)
 );

@ -622,8 +624,8 @@ INSERT INTO simple_primary_key VALUES (4, 'RENDER_CELL_DEMO');
 INSERT INTO primary_key_multiple_columns VALUES (1, 'hey', 'world');
 INSERT INTO primary_key_multiple_columns_explicit_label VALUES (1, 'hey', 'world2');

-INSERT INTO foreign_key_references VALUES (1, 1, 1);
-INSERT INTO foreign_key_references VALUES (2, null, null);
+INSERT INTO foreign_key_references VALUES (1, 1, 3, 1);
+INSERT INTO foreign_key_references VALUES (2, null, null, null);

 INSERT INTO complex_foreign_keys VALUES (1, 1, 2, 1);
 INSERT INTO custom_foreign_key_label VALUES (1, 1);
--- a/tests/test_api.py
+++ b/tests/test_api.py
@ -49,21 +49,21 @@ def test_homepage_sort_by_relationships(app_client):
    tables = [
        t["name"] for t in response.json["fixtures"]["tables_and_views_truncated"]
    ]
-    assert [
+    assert tables == [
        "simple_primary_key",
+        "foreign_key_references",
        "complex_foreign_keys",
        "roadside_attraction_characteristics",
        "searchable_tags",
-        "foreign_key_references",
-    ] == tables
+    ]


 def test_database_page(app_client):
    response = app_client.get("/fixtures.json")
    assert response.status == 200
    data = response.json
-    assert "fixtures" == data["database"]
-    assert [
+    assert data["database"] == "fixtures"
+    assert data["tables"] == [
        {
            "name": "123_starts_with_digits",
            "columns": ["content"],
@ -232,7 +232,12 @@ def test_database_page(app_client):
        },
        {
            "name": "foreign_key_references",
-            "columns": ["pk", "foreign_key_with_label", "foreign_key_with_no_label"],
+            "columns": [
+                "pk",
+                "foreign_key_with_label",
+                "foreign_key_with_blank_label",
+                "foreign_key_with_no_label",
+            ],
            "primary_keys": ["pk"],
            "count": 2,
            "hidden": False,
@ -245,6 +250,11 @@ def test_database_page(app_client):
                        "column": "foreign_key_with_no_label",
                        "other_column": "id",
                    },
+                    {
+                        "other_table": "simple_primary_key",
+                        "column": "foreign_key_with_blank_label",
+                        "other_column": "id",
+                    },
                    {
                        "other_table": "simple_primary_key",
                        "column": "foreign_key_with_label",
@ -403,6 +413,11 @@ def test_database_page(app_client):
            "fts_table": None,
            "foreign_keys": {
                "incoming": [
+                    {
+                        "other_table": "foreign_key_references",
+                        "column": "id",
+                        "other_column": "foreign_key_with_blank_label",
+                    },
                    {
                        "other_table": "foreign_key_references",
                        "column": "id",
@ -548,7 +563,7 @@ def test_database_page(app_client):
            "foreign_keys": {"incoming": [], "outgoing": []},
            "private": False,
        },
-    ] == data["tables"]
+    ]


 def test_no_files_uses_memory_database(app_client_no_files):
@ -1203,32 +1218,38 @@ def test_row_foreign_key_tables(app_client):
        "/fixtures/simple_primary_key/1.json?_extras=foreign_key_tables"
    )
    assert response.status == 200
-    assert [
+    assert response.json["foreign_key_tables"] == [
        {
-            "column": "id",
-            "count": 1,
-            "other_column": "foreign_key_with_label",
            "other_table": "foreign_key_references",
-        },
-        {
-            "column": "id",
-            "count": 1,
-            "other_column": "f3",
-            "other_table": "complex_foreign_keys",
-        },
-        {
            "column": "id",
+            "other_column": "foreign_key_with_blank_label",
            "count": 0,
-            "other_column": "f2",
-            "other_table": "complex_foreign_keys",
        },
        {
+            "other_table": "foreign_key_references",
            "column": "id",
+            "other_column": "foreign_key_with_label",
            "count": 1,
-            "other_column": "f1",
-            "other_table": "complex_foreign_keys",
        },
-    ] == response.json["foreign_key_tables"]
+        {
+            "other_table": "complex_foreign_keys",
+            "column": "id",
+            "other_column": "f3",
+            "count": 1,
+        },
+        {
+            "other_table": "complex_foreign_keys",
+            "column": "id",
+            "other_column": "f2",
+            "count": 0,
+        },
+        {
+            "other_table": "complex_foreign_keys",
+            "column": "id",
+            "other_column": "f1",
+            "count": 1,
+        },
+    ]


 def test_unit_filters(app_client):
@ -1593,13 +1614,14 @@ def test_expand_label(app_client):
        "/fixtures/foreign_key_references.json?_shape=object"
        "&_label=foreign_key_with_label&_size=1"
    )
-    assert {
+    assert response.json == {
        "1": {
            "pk": "1",
            "foreign_key_with_label": {"value": "1", "label": "hello"},
+            "foreign_key_with_blank_label": "3",
            "foreign_key_with_no_label": "1",
        }
-    } == response.json
+    }


@pytest.mark.parametrize(
@ -1790,11 +1812,13 @@ def test_null_foreign_keys_are_not_expanded(app_client):
        {
            "pk": "1",
            "foreign_key_with_label": {"value": "1", "label": "hello"},
+            "foreign_key_with_blank_label": {"value": "3", "label": ""},
            "foreign_key_with_no_label": {"value": "1", "label": "1"},
        },
        {
            "pk": "2",
            "foreign_key_with_label": None,
+            "foreign_key_with_blank_label": None,
            "foreign_key_with_no_label": None,
        },
    ] == response.json
--- a/tests/test_csv.py
+++ b/tests/test_csv.py
@ -42,9 +42,9 @@ pk,created,planet_int,on_earth,state,city_id,city_id_label,neighborhood,tags,com
 )

 EXPECTED_TABLE_WITH_NULLABLE_LABELS_CSV = """
-pk,foreign_key_with_label,foreign_key_with_label_label,foreign_key_with_no_label,foreign_key_with_no_label_label
-1,1,hello,1,1
-2,,,,
+pk,foreign_key_with_label,foreign_key_with_label_label,foreign_key_with_blank_label,foreign_key_with_blank_label_label,foreign_key_with_no_label,foreign_key_with_no_label_label
+1,1,hello,3,,1,1
+2,,,,,,
 """.lstrip().replace(
    "\n", "\r\n"
 )
--- a/tests/test_html.py
+++ b/tests/test_html.py
@ -464,7 +464,7 @@ def test_facet_display(app_client):
                ],
            }
        )
-    assert [
+    assert actual == [
        {
            "name": "city_id",
            "items": [
@ -520,7 +520,7 @@ def test_facet_display(app_client):
                },
            ],
        },
-    ] == actual
+    ]


 def test_facets_persist_through_filter_form(app_client):
@ -801,37 +801,47 @@ def test_table_html_foreign_key_links(app_client):
    response = app_client.get("/fixtures/foreign_key_references")
    assert response.status == 200
    table = Soup(response.body, "html.parser").find("table")
-    expected = [
+    actual = [[str(td) for td in tr.select("td")] for tr in table.select("tbody tr")]
+    assert actual == [
        [
            '<td class="col-pk type-pk"><a href="/fixtures/foreign_key_references/1">1</a></td>',
            '<td class="col-foreign_key_with_label type-str"><a href="/fixtures/simple_primary_key/1">hello</a>\xa0<em>1</em></td>',
+            '<td class="col-foreign_key_with_blank_label type-str"><a href="/fixtures/simple_primary_key/3">-</a>\xa0<em>3</em></td>',
            '<td class="col-foreign_key_with_no_label type-str"><a href="/fixtures/primary_key_multiple_columns/1">1</a></td>',
        ],
        [
            '<td class="col-pk type-pk"><a href="/fixtures/foreign_key_references/2">2</a></td>',
            '<td class="col-foreign_key_with_label type-none">\xa0</td>',
+            '<td class="col-foreign_key_with_blank_label type-none">\xa0</td>',
            '<td class="col-foreign_key_with_no_label type-none">\xa0</td>',
        ],
    ]
-    assert expected == [
-        [str(td) for td in tr.select("td")] for tr in table.select("tbody tr")
-    ]
+
+
+def test_table_html_foreign_key_facets(app_client):
+    response = app_client.get(
+        "/fixtures/foreign_key_references?_facet=foreign_key_with_blank_label"
+    )
+    assert response.status == 200
+    assert (
+        '<li><a href="http://localhost/fixtures/foreign_key_references?_facet=foreign_key_with_blank_label&amp;foreign_key_with_blank_label=3">'
+        "-</a> 1</li>"
+    ) in response.text


 def test_table_html_disable_foreign_key_links_with_labels(app_client):
    response = app_client.get("/fixtures/foreign_key_references?_labels=off&_size=1")
    assert response.status == 200
    table = Soup(response.body, "html.parser").find("table")
-    expected = [
+    actual = [[str(td) for td in tr.select("td")] for tr in table.select("tbody tr")]
+    assert actual == [
        [
            '<td class="col-pk type-pk"><a href="/fixtures/foreign_key_references/1">1</a></td>',
            '<td class="col-foreign_key_with_label type-str">1</td>',
+            '<td class="col-foreign_key_with_blank_label type-str">3</td>',
            '<td class="col-foreign_key_with_no_label type-str">1</td>',
        ]
    ]
-    assert expected == [
-        [str(td) for td in tr.select("td")] for tr in table.select("tbody tr")
-    ]


 def test_table_html_foreign_key_custom_label_column(app_client):