diff --git a/datasette/facets.py b/datasette/facets.py index f74e2d01..94a1d83d 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -237,14 +237,17 @@ class ColumnFacet(Facet): else: expanded = {} for row in facet_rows: - selected = (column, str(row["value"])) in qs_pairs + column_qs = column + if column.startswith("_"): + column_qs = "{}__exact".format(column) + selected = (column_qs, str(row["value"])) in qs_pairs if selected: toggle_path = path_with_removed_args( - self.request, {column: str(row["value"])} + self.request, {column_qs: str(row["value"])} ) else: toggle_path = path_with_added_args( - self.request, {column: row["value"]} + self.request, {column_qs: row["value"]} ) facet_results_values.append( { diff --git a/tests/fixtures.py b/tests/fixtures.py index dc22c609..1a879126 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -355,12 +355,12 @@ METADATA = { "neighborhood_search": { "sql": textwrap.dedent( """ - select neighborhood, facet_cities.name, state + select _neighborhood, facet_cities.name, state from facetable join facet_cities on facetable.city_id = facet_cities.id - where neighborhood like '%' || :text || '%' - order by neighborhood; + where _neighborhood like '%' || :text || '%' + order by _neighborhood; """ ), "title": "Search neighborhoods", @@ -559,14 +559,14 @@ CREATE TABLE facetable ( on_earth integer, state text, city_id integer, - neighborhood text, + _neighborhood text, tags text, complex_array text, distinct_some_null, FOREIGN KEY ("city_id") REFERENCES [facet_cities](id) ); INSERT INTO facetable - (created, planet_int, on_earth, state, city_id, neighborhood, tags, complex_array, distinct_some_null) + (created, planet_int, on_earth, state, city_id, _neighborhood, tags, complex_array, distinct_some_null) VALUES ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Mission', '["tag1", "tag2"]', '[{"foo": "bar"}]', 'one'), ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Dogpatch', '["tag1", "tag3"]', '[]', 'two'), diff --git a/tests/test_api.py b/tests/test_api.py index 311ae464..43b52175 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -213,7 +213,7 @@ def test_database_page(app_client): "on_earth", "state", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", @@ -1241,7 +1241,9 @@ def test_table_filter_json_arraynotcontains(app_client): def test_table_filter_extra_where(app_client): - response = app_client.get("/fixtures/facetable.json?_where=neighborhood='Dogpatch'") + response = app_client.get( + "/fixtures/facetable.json?_where=_neighborhood='Dogpatch'" + ) assert [ [ 2, @@ -1259,14 +1261,16 @@ def test_table_filter_extra_where(app_client): def test_table_filter_extra_where_invalid(app_client): - response = app_client.get("/fixtures/facetable.json?_where=neighborhood=Dogpatch'") + response = app_client.get("/fixtures/facetable.json?_where=_neighborhood=Dogpatch'") assert 400 == response.status assert "Invalid SQL" == response.json["title"] def test_table_filter_extra_where_disabled_if_no_sql_allowed(): with make_app_client(metadata={"allow_sql": {}}) as client: - response = client.get("/fixtures/facetable.json?_where=neighborhood='Dogpatch'") + response = client.get( + "/fixtures/facetable.json?_where=_neighborhood='Dogpatch'" + ) assert 403 == response.status assert "_where= is not allowed" == response.json["error"] @@ -1696,7 +1700,7 @@ def test_suggested_facets(app_client): {"name": "on_earth", "querystring": "_facet=on_earth"}, {"name": "state", "querystring": "_facet=state"}, {"name": "city_id", "querystring": "_facet=city_id"}, - {"name": "neighborhood", "querystring": "_facet=neighborhood"}, + {"name": "_neighborhood", "querystring": "_facet=_neighborhood"}, {"name": "tags", "querystring": "_facet=tags"}, {"name": "complex_array", "querystring": "_facet=complex_array"}, {"name": "created", "querystring": "_facet_date=created"}, @@ -1752,7 +1756,7 @@ def test_nocount_nofacet_if_shape_is_object(app_client_with_trace): def test_expand_labels(app_client): response = app_client.get( "/fixtures/facetable.json?_shape=object&_labels=1&_size=2" - "&neighborhood__contains=c" + "&_neighborhood__contains=c" ) assert { "2": { @@ -1762,7 +1766,7 @@ def test_expand_labels(app_client): "on_earth": 1, "state": "CA", "city_id": {"value": 1, "label": "San Francisco"}, - "neighborhood": "Dogpatch", + "_neighborhood": "Dogpatch", "tags": '["tag1", "tag3"]', "complex_array": "[]", "distinct_some_null": "two", @@ -1774,7 +1778,7 @@ def test_expand_labels(app_client): "on_earth": 1, "state": "MI", "city_id": {"value": 3, "label": "Detroit"}, - "neighborhood": "Corktown", + "_neighborhood": "Corktown", "tags": "[]", "complex_array": "[]", "distinct_some_null": None, @@ -2125,7 +2129,7 @@ def test_http_options_request(app_client): "on_earth", "state", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", @@ -2152,7 +2156,7 @@ def test_http_options_request(app_client): "planet_int", "on_earth", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", diff --git a/tests/test_csv.py b/tests/test_csv.py index 5e9406e7..5902e9db 100644 --- a/tests/test_csv.py +++ b/tests/test_csv.py @@ -24,7 +24,7 @@ world ) EXPECTED_TABLE_WITH_LABELS_CSV = """ -pk,created,planet_int,on_earth,state,city_id,city_id_label,neighborhood,tags,complex_array,distinct_some_null +pk,created,planet_int,on_earth,state,city_id,city_id_label,_neighborhood,tags,complex_array,distinct_some_null 1,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Mission,"[""tag1"", ""tag2""]","[{""foo"": ""bar""}]",one 2,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Dogpatch,"[""tag1"", ""tag3""]",[],two 3,2019-01-14 08:00:00,1,1,CA,1,San Francisco,SOMA,[],[], diff --git a/tests/test_facets.py b/tests/test_facets.py index 22927512..01d8b8f5 100644 --- a/tests/test_facets.py +++ b/tests/test_facets.py @@ -23,7 +23,10 @@ async def test_column_facet_suggest(app_client): {"name": "on_earth", "toggle_url": "http://localhost/?_facet=on_earth"}, {"name": "state", "toggle_url": "http://localhost/?_facet=state"}, {"name": "city_id", "toggle_url": "http://localhost/?_facet=city_id"}, - {"name": "neighborhood", "toggle_url": "http://localhost/?_facet=neighborhood"}, + { + "name": "_neighborhood", + "toggle_url": "http://localhost/?_facet=_neighborhood", + }, {"name": "tags", "toggle_url": "http://localhost/?_facet=tags"}, { "name": "complex_array", @@ -56,8 +59,8 @@ async def test_column_facet_suggest_skip_if_already_selected(app_client): "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=city_id", }, { - "name": "neighborhood", - "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=neighborhood", + "name": "_neighborhood", + "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=_neighborhood", }, { "name": "tags", @@ -86,7 +89,7 @@ async def test_column_facet_suggest_skip_if_enabled_by_metadata(app_client): "planet_int", "on_earth", "state", - "neighborhood", + "_neighborhood", "tags", "complex_array", ] == suggestions @@ -144,6 +147,128 @@ async def test_column_facet_results(app_client): } == buckets +@pytest.mark.asyncio +async def test_column_facet_results_column_starts_with_underscore(app_client): + facet = ColumnFacet( + app_client.ds, + Request.fake("/?_facet=_neighborhood"), + database="fixtures", + sql="select * from facetable", + table="facetable", + ) + buckets, timed_out = await facet.facet_results() + assert [] == timed_out + assert buckets == { + "_neighborhood": { + "name": "_neighborhood", + "type": "column", + "hideable": True, + "toggle_url": "/", + "results": [ + { + "value": "Downtown", + "label": "Downtown", + "count": 2, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Downtown", + "selected": False, + }, + { + "value": "Arcadia Planitia", + "label": "Arcadia Planitia", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Arcadia+Planitia", + "selected": False, + }, + { + "value": "Bernal Heights", + "label": "Bernal Heights", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Bernal+Heights", + "selected": False, + }, + { + "value": "Corktown", + "label": "Corktown", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Corktown", + "selected": False, + }, + { + "value": "Dogpatch", + "label": "Dogpatch", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Dogpatch", + "selected": False, + }, + { + "value": "Greektown", + "label": "Greektown", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Greektown", + "selected": False, + }, + { + "value": "Hayes Valley", + "label": "Hayes Valley", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Hayes+Valley", + "selected": False, + }, + { + "value": "Hollywood", + "label": "Hollywood", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Hollywood", + "selected": False, + }, + { + "value": "Koreatown", + "label": "Koreatown", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Koreatown", + "selected": False, + }, + { + "value": "Los Feliz", + "label": "Los Feliz", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Los+Feliz", + "selected": False, + }, + { + "value": "Mexicantown", + "label": "Mexicantown", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Mexicantown", + "selected": False, + }, + { + "value": "Mission", + "label": "Mission", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Mission", + "selected": False, + }, + { + "value": "SOMA", + "label": "SOMA", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=SOMA", + "selected": False, + }, + { + "value": "Tenderloin", + "label": "Tenderloin", + "count": 1, + "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Tenderloin", + "selected": False, + }, + ], + "truncated": False, + } + } + + @pytest.mark.asyncio async def test_column_facet_from_metadata_cannot_be_hidden(app_client): facet = ColumnFacet( diff --git a/tests/test_html.py b/tests/test_html.py index 5f2ba2f1..1955e65b 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -235,7 +235,10 @@ def test_table_cell_truncation(): "Corkt…", "Mexic…", "Arcad…", - ] == [td.string for td in table.findAll("td", {"class": "col-neighborhood"})] + ] == [ + td.string + for td in table.findAll("td", {"class": "col-neighborhood-b352a7"}) + ] def test_row_page_does_not_truncate(): @@ -245,7 +248,8 @@ def test_row_page_does_not_truncate(): table = Soup(response.body, "html.parser").find("table") assert table["class"] == ["rows-and-columns"] assert ["Mission"] == [ - td.string for td in table.findAll("td", {"class": "col-neighborhood"}) + td.string + for td in table.findAll("td", {"class": "col-neighborhood-b352a7"}) ] @@ -1312,7 +1316,7 @@ def test_canned_query_show_hide_metadata_option( def test_extra_where_clauses(app_client): response = app_client.get( - "/fixtures/facetable?_where=neighborhood='Dogpatch'&_where=city_id=1" + "/fixtures/facetable?_where=_neighborhood='Dogpatch'&_where=city_id=1" ) soup = Soup(response.body, "html.parser") div = soup.select(".extra-wheres")[0] @@ -1320,12 +1324,12 @@ def test_extra_where_clauses(app_client): hrefs = [a["href"] for a in div.findAll("a")] assert [ "/fixtures/facetable?_where=city_id%3D1", - "/fixtures/facetable?_where=neighborhood%3D%27Dogpatch%27", + "/fixtures/facetable?_where=_neighborhood%3D%27Dogpatch%27", ] == hrefs # These should also be persisted as hidden fields inputs = soup.find("form").findAll("input") hiddens = [i for i in inputs if i["type"] == "hidden"] - assert [("_where", "neighborhood='Dogpatch'"), ("_where", "city_id=1")] == [ + assert [("_where", "_neighborhood='Dogpatch'"), ("_where", "city_id=1")] == [ (hidden["name"], hidden["value"]) for hidden in hiddens ] @@ -1634,11 +1638,11 @@ def test_base_url_affects_metadata_extra_css_urls(app_client_base_url_prefix): [ ( "/fixtures/neighborhood_search", - "/fixtures?sql=%0Aselect+neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+neighborhood%3B%0A&text=", + "/fixtures?sql=%0Aselect+_neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+_neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+_neighborhood%3B%0A&text=", ), ( "/fixtures/neighborhood_search?text=ber", - "/fixtures?sql=%0Aselect+neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+neighborhood%3B%0A&text=ber", + "/fixtures?sql=%0Aselect+_neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+_neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+_neighborhood%3B%0A&text=ber", ), ("/fixtures/pragma_cache_size", None), ( @@ -1716,23 +1720,23 @@ def test_navigation_menu_links( ( 5, # Default should show 2 facets - "/fixtures/facetable?_facet=neighborhood", + "/fixtures/facetable?_facet=_neighborhood", 2, True, - "/fixtures/facetable?_facet=neighborhood&_facet_size=max", + "/fixtures/facetable?_facet=_neighborhood&_facet_size=max", ), # _facet_size above max_returned_rows should show max_returned_rows (5) ( 5, - "/fixtures/facetable?_facet=neighborhood&_facet_size=50", + "/fixtures/facetable?_facet=_neighborhood&_facet_size=50", 5, True, - "/fixtures/facetable?_facet=neighborhood&_facet_size=max", + "/fixtures/facetable?_facet=_neighborhood&_facet_size=max", ), # If max_returned_rows is high enough, should return all ( 20, - "/fixtures/facetable?_facet=neighborhood&_facet_size=max", + "/fixtures/facetable?_facet=_neighborhood&_facet_size=max", 14, False, None, @@ -1741,7 +1745,7 @@ def test_navigation_menu_links( # _facet_size above max_returned_rows should show max_returned_rows (5) ( 5, - "/fixtures/facetable?_facet=neighborhood&_facet_size=max", + "/fixtures/facetable?_facet=_neighborhood&_facet_size=max", 5, True, None, @@ -1760,7 +1764,7 @@ def test_facet_more_links( ) as client: response = client.get(path) soup = Soup(response.body, "html.parser") - lis = soup.select("#facet-neighborhood ul li:not(.facet-truncated)") + lis = soup.select("#facet-neighborhood-b352a7 ul li:not(.facet-truncated)") facet_truncated = soup.select_one(".facet-truncated") assert len(lis) == expected_num_facets if not expected_ellipses: diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index ad829751..2d0cae7f 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -82,7 +82,7 @@ async def test_table_exists(db, tables, exists): "on_earth", "state", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", @@ -170,7 +170,7 @@ async def test_table_columns(db, table, expected): ), Column( cid=6, - name="neighborhood", + name="_neighborhood", type="text", notnull=0, default_value=None, diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 7dac8002..7ac6b173 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -437,7 +437,7 @@ def test_hook_register_output_renderer_all_parameters(app_client): "on_earth", "state", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", @@ -459,7 +459,7 @@ def test_hook_register_output_renderer_all_parameters(app_client): "", "", ], - "sql": "select pk, created, planet_int, on_earth, state, city_id, neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", + "sql": "select pk, created, planet_int, on_earth, state, city_id, _neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", "query_name": None, "database": "fixtures", "table": "facetable", @@ -526,12 +526,12 @@ def test_hook_register_output_renderer_can_render(app_client): "on_earth", "state", "city_id", - "neighborhood", + "_neighborhood", "tags", "complex_array", "distinct_some_null", ], - "sql": "select pk, created, planet_int, on_earth, state, city_id, neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", + "sql": "select pk, created, planet_int, on_earth, state, city_id, _neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", "query_name": None, "database": "fixtures", "table": "facetable",