diff --git a/datasette/static/app.css b/datasette/static/app.css index 98c4a47c..4f2ddad8 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -231,3 +231,29 @@ a.not-underlined { .not-underlined .underlined { text-decoration: underline; } + +.facet-results { + display: flex; + flex-direction: row; + flex-wrap: wrap; +} +.facet-info { + width: 250px; + margin-right: 15px; +} +.facet-info li, +.facet-info ul { + margin: 0; + padding: 0; +} +.facet-info ul { + padding-left: 1.25em; + margin-bottom: 1em; +} +.facet-info-name a:link, +.facet-info-name a:visited, +.facet-info-name a:hover, +.facet-info-name a:focus, +.facet-info-name a:active { + text-decoration: none; +} diff --git a/datasette/templates/_rows_and_columns.html b/datasette/templates/_rows_and_columns.html index cbcbc6e3..c7a72253 100644 --- a/datasette/templates/_rows_and_columns.html +++ b/datasette/templates/_rows_and_columns.html @@ -7,9 +7,9 @@ {{ column.name }} {% else %} {% if column.name == sort %} - {{ column.name }} ▼ + {{ column.name }} ▼ {% else %} - {{ column.name }}{% if column.name == sort_desc %} ▲{% endif %} + {{ column.name }}{% if column.name == sort_desc %} ▲{% endif %} {% endif %} {% endif %} diff --git a/datasette/templates/table.html b/datasette/templates/table.html index ba582ba9..1ba61f87 100644 --- a/datasette/templates/table.html +++ b/datasette/templates/table.html @@ -81,6 +81,9 @@ {% endif %} + {% for facet in sorted_facet_results %} + + {% endfor %} @@ -91,14 +94,34 @@

This data as .json

-{% for facet_name, facet_values in facet_results.items() %} -

{{ facet_name }}

- -{% endfor %} + +{% endif %} {% include custom_rows_and_columns_templates %} diff --git a/datasette/utils.py b/datasette/utils.py index 118d9bd7..35950c68 100644 --- a/datasette/utils.py +++ b/datasette/utils.py @@ -149,10 +149,10 @@ def path_with_added_args(request, args, path=None): path = path or request.path if isinstance(args, dict): args = args.items() - arg_keys = set(a[0] for a in args) + args_to_remove = {k for k, v in args if v is None} current = [] for key, value in urllib.parse.parse_qsl(request.query_string): - if key not in arg_keys: + if key not in args_to_remove: current.append((key, value)) current.extend([ (key, value) @@ -165,6 +165,42 @@ def path_with_added_args(request, args, path=None): return path + query_string +def path_with_removed_args(request, args, path=None): + # args can be a dict or a set + path = path or request.path + current = [] + if isinstance(args, set): + def should_remove(key, value): + return key in args + elif isinstance(args, dict): + # Must match key AND value + def should_remove(key, value): + return args.get(key) == value + for key, value in urllib.parse.parse_qsl(request.query_string): + if not should_remove(key, value): + current.append((key, value)) + query_string = urllib.parse.urlencode(current) + if query_string: + query_string = '?{}'.format(query_string) + return path + query_string + + +def path_with_replaced_args(request, args, path=None): + path = path or request.path + if isinstance(args, dict): + args = args.items() + keys_to_replace = {p[0] for p in args} + current = [] + for key, value in urllib.parse.parse_qsl(request.query_string): + if key not in keys_to_replace: + current.append((key, value)) + current.extend([p for p in args if p[1] is not None]) + query_string = urllib.parse.urlencode(current) + if query_string: + query_string = '?{}'.format(query_string) + return path + query_string + + def path_with_ext(request, ext): path = request.path path += ext @@ -205,7 +241,6 @@ def make_dockerfile(files, metadata_file, extra_options, branch, template_dir, p for opt in extra_options.split(): cmd.append('"{}"'.format(opt)) - install_from = 'datasette' if branch: install = ['https://github.com/simonw/datasette/archive/{}.zip'.format( branch diff --git a/datasette/views/table.py b/datasette/views/table.py index 5369825c..ab66b880 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -13,6 +13,8 @@ from datasette.utils import ( is_url, path_from_row_pks, path_with_added_args, + path_with_removed_args, + path_with_replaced_args, to_css_class, urlsafe_components ) @@ -33,6 +35,54 @@ class RowTableShared(BaseView): sortable_columns.add("rowid") return sortable_columns + async def expand_foreign_keys(self, database, table, column, values): + "Returns dict mapping (column, value) -> label" + labeled_fks = {} + tables_info = self.ds.inspect()[database]["tables"] + table_info = tables_info.get(table) or {} + if not table_info: + return {} + foreign_keys = table_info["foreign_keys"]["outgoing"] + # Find the foreign_key for this column + try: + fk = [ + foreign_key for foreign_key in foreign_keys + if foreign_key["column"] == column + ][0] + except IndexError: + return {} + label_column = ( + # First look in metadata.json for this foreign key table: + self.table_metadata( + database, fk["other_table"] + ).get("label_column") + or tables_info.get(fk["other_table"], {}).get("label_column") + ) + if not label_column: + return {} + labeled_fks = {} + sql = ''' + select {other_column}, {label_column} + from {other_table} + where {other_column} in ({placeholders}) + '''.format( + other_column=escape_sqlite(fk["other_column"]), + label_column=escape_sqlite(label_column), + other_table=escape_sqlite(fk["other_table"]), + placeholders=", ".join(["?"] * len(set(values))), + ) + try: + results = await self.execute( + database, sql, list(set(values)) + ) + except sqlite3.OperationalError: + # Probably hit the timelimit + pass + else: + for id, value in results: + labeled_fks[(fk["column"], id)] = value + return labeled_fks + async def display_columns_and_rows( self, database, @@ -71,9 +121,13 @@ class RowTableShared(BaseView): continue ids_to_lookup = set([row[fk["column"]] for row in rows]) - sql = 'select "{other_column}", "{label_column}" from {other_table} where "{other_column}" in ({placeholders})'.format( - other_column=fk["other_column"], - label_column=label_column, + sql = ''' + select {other_column}, {label_column} + from {other_table} + where {other_column} in ({placeholders}) + '''.format( + other_column=escape_sqlite(fk["other_column"]), + label_column=escape_sqlite(label_column), other_table=escape_sqlite(fk["other_table"]), placeholders=", ".join(["?"] * len(ids_to_lookup)), ) @@ -198,11 +252,7 @@ class TableView(RowTableShared): "SELECT count(*) from sqlite_master WHERE type = 'view' and name=:n", {"n": table}, ) - )[ - 0 - ][ - 0 - ] + )[0][0] ) view_definition = None table_definition = None @@ -213,11 +263,7 @@ class TableView(RowTableShared): 'select sql from sqlite_master where name = :n and type="view"', {"n": table}, ) - )[ - 0 - ][ - 0 - ] + )[0][0] else: table_definition_rows = list( await self.execute( @@ -306,8 +352,8 @@ class TableView(RowTableShared): # Simple ?_search=xxx search = search_args["_search"] where_clauses.append( - "rowid in (select rowid from [{fts_table}] where [{fts_table}] match :search)".format( - fts_table=fts_table + "rowid in (select rowid from {fts_table} where {fts_table} match :search)".format( + fts_table=escape_sqlite(fts_table), ) ) search_descriptions.append('search matches "{}"'.format(search)) @@ -321,8 +367,10 @@ class TableView(RowTableShared): raise DatasetteError("Cannot search by that column", status=400) where_clauses.append( - "rowid in (select rowid from [{fts_table}] where [{search_col}] match :search_{i})".format( - fts_table=fts_table, search_col=search_col, i=i + "rowid in (select rowid from {fts_table} where {search_col} match :search_{i})".format( + fts_table=escape_sqlite(fts_table), + search_col=escape_sqlite(search_col), + i=i ) ) search_descriptions.append( @@ -488,34 +536,64 @@ class TableView(RowTableShared): ) # facets support + FACET_SIZE = 20 + metadata_facets = table_metadata.get("facets", []) + facets = metadata_facets[:] try: - facets = request.args["_facet"] + facets.extend(request.args["_facet"]) except KeyError: - facets = table_metadata.get("facets", []) + pass facet_results = {} for column in facets: facet_sql = """ select {col} as value, count(*) as count - {from_sql} - group by {col} order by count desc limit 20 + {from_sql} {and_or_where} {col} is not null + group by {col} order by count desc limit {limit} """.format( - col=escape_sqlite(column), from_sql=from_sql + col=escape_sqlite(column), + from_sql=from_sql, + and_or_where='and' if where_clauses else 'where', + limit=FACET_SIZE+1, ) try: facet_rows = await self.execute( - name, facet_sql, params, truncate=False, custom_time_limit=200 + name, facet_sql, params, + truncate=False, custom_time_limit=200 ) - facet_results[column] = [ - { + facet_results_values = [] + facet_results[column] = { + "name": column, + "results": facet_results_values, + "truncated": len(facet_rows) > FACET_SIZE, + } + facet_rows = facet_rows[:FACET_SIZE] + # Attempt to expand foreign keys into labels + values = [row["value"] for row in facet_rows] + expanded = (await self.expand_foreign_keys( + name, table, column, values + )) + for row in facet_rows: + selected = str(other_args.get(column)) == str(row["value"]) + if selected: + toggle_path = path_with_removed_args( + request, {column: str(row["value"])} + ) + else: + toggle_path = path_with_added_args( + request, {column: row["value"]} + ) + facet_results_values.append({ "value": row["value"], + "label": expanded.get( + (column, row["value"]), + row["value"] + ), "count": row["count"], "toggle_url": urllib.parse.urljoin( - request.url, - path_with_added_args(request, {column: row["value"]}), + request.url, toggle_path ), - } - for row in facet_rows - ] + "selected": selected, + }) except sqlite3.OperationalError: # Hit time limit pass @@ -551,7 +629,7 @@ class TableView(RowTableShared): else: added_args = {"_next": next_value} next_url = urllib.parse.urljoin( - request.url, path_with_added_args(request, added_args) + request.url, path_with_replaced_args(request, added_args) ) rows = rows[:page_size] @@ -565,6 +643,44 @@ class TableView(RowTableShared): # Almost certainly hit the timeout pass + # Detect suggested facets + FACET_LIMIT = 30 + suggested_facets = [] + for facet_column in columns: + if facet_column in facets: + continue + suggested_facet_sql = ''' + select distinct {column} {from_sql} + {and_or_where} {column} is not null + limit {limit} + '''.format( + column=escape_sqlite(facet_column), + from_sql=from_sql, + and_or_where='and' if where_clauses else 'where', + limit=FACET_LIMIT+1 + ) + distinct_values = None + try: + distinct_values = await self.execute( + name, suggested_facet_sql, params, + truncate=False, custom_time_limit=50 + ) + num_distinct_values = len(distinct_values) + if ( + num_distinct_values and + num_distinct_values > 1 and + num_distinct_values <= FACET_LIMIT and + num_distinct_values < filtered_table_rows_count + ): + suggested_facets.append({ + 'name': facet_column, + 'toggle_url': path_with_added_args( + request, {'_facet': facet_column} + ), + }) + except sqlite3.OperationalError: + pass + # human_description_en combines filters AND search, if provided human_description_en = filters.human_description_en(extra=search_descriptions) @@ -600,8 +716,15 @@ class TableView(RowTableShared): "display_columns": display_columns, "filter_columns": filter_columns, "display_rows": display_rows, + "sorted_facet_results": sorted( + facet_results.values(), + key=lambda f: (len(f["results"]), f["name"]), + reverse=True + ), + "facet_hideable": lambda facet: facet not in metadata_facets, "is_sortable": any(c["sortable"] for c in display_columns), - "path_with_added_args": path_with_added_args, + "path_with_replaced_args": path_with_replaced_args, + "path_with_removed_args": path_with_removed_args, "request": request, "sort": sort, "sort_desc": sort_desc, @@ -634,6 +757,7 @@ class TableView(RowTableShared): "units": units, "query": {"sql": sql, "params": params}, "facet_results": facet_results, + "suggested_facets": suggested_facets, "next": next_value and str(next_value) or None, "next_url": next_url, }, extra_template, ( @@ -736,8 +860,9 @@ class RowView(RowTableShared): sql = "select " + ", ".join( [ - '(select count(*) from {table} where "{column}"=:id)'.format( - table=escape_sqlite(fk["other_table"]), column=fk["other_column"] + '(select count(*) from {table} where {column}=:id)'.format( + table=escape_sqlite(fk["other_table"]), + column=escape_sqlite(fk["other_column"]), ) for fk in foreign_keys ] diff --git a/docs/facets.png b/docs/facets.png new file mode 100644 index 00000000..72d38043 Binary files /dev/null and b/docs/facets.png differ diff --git a/docs/facets.rst b/docs/facets.rst index 40499e55..519a5f9a 100644 --- a/docs/facets.rst +++ b/docs/facets.rst @@ -3,53 +3,87 @@ Facets ====== -This feature is currently under development, see `#255 `_ - Datasette facets can be used to add a faceted browse interface to any Datasette table. With facets, tables are displayed along with a summary showing the most common values in specified columns. These values can be selected to further filter the table. +.. image:: facets.png + Facets can be specified in two ways: using queryset parameters, or in ``metadata.json`` configuration for the table. Facets in querystrings ---------------------- -To turn on faceting for specific columns on a Datasette table view, add one or more ``_facet=COLUMN`` parameters to the URL. For example, if you want to turn on facets for the ``city`` and ``state`` columns, construct a URL that looks like this:: +To turn on faceting for specific columns on a Datasette table view, add one or more ``_facet=COLUMN`` parameters to the URL. For example, if you want to turn on facets for the ``city_id`` and ``state`` columns, construct a URL that looks like this:: - /dbname/tablename?_facet=state&_facet=city + /dbname/tablename?_facet=state&_facet=city_id This works for both the HTML interface and the ``.json`` view. When enabled, facets will cause a ``facet_results`` block to be added to the JSON output, looking something like this:: - "facet_results": { - "state": [ - { - "value": "CA", - "count": 10, - "toggle_url": "http://...&state=CA" - }, - { - "value": "MI", - "count": 4, - "toggle_url": "http://...&state=MI" - } - ], - "city": [ - { - "value": "San Francisco", - "count": 6, - "toggle_url": "http://...=San+Francisco" - }, - { - "value": "Detroit", - "count": 4, - "toggle_url": "http://...&city=Detroit" - }, - { - "value": "Los Angeles", - "count": 4, - "toggle_url": "http://...=Los+Angeles" - } - ] + { + "state": { + "name": "state", + "results": [ + { + "value": "CA", + "label": "CA", + "count": 10, + "toggle_url": "http://...?_facet=city_id&_facet=state&state=CA", + "selected": false + }, + { + "value": "MI", + "label": "MI", + "count": 4, + "toggle_url": "http://...?_facet=city_id&_facet=state&state=MI", + "selected": false + }, + { + "value": "MC", + "label": "MC", + "count": 1, + "toggle_url": "http://...?_facet=city_id&_facet=state&state=MC", + "selected": false + } + ], + "truncated": false + } + "city_id": { + "name": "city_id", + "results": [ + { + "value": 1, + "label": "San Francisco", + "count": 6, + "toggle_url": "http://...?_facet=city_id&_facet=state&city_id=1", + "selected": false + }, + { + "value": 2, + "label": "Los Angeles", + "count": 4, + "toggle_url": "http://...?_facet=city_id&_facet=state&city_id=2", + "selected": false + }, + { + "value": 3, + "label": "Detroit", + "count": 4, + "toggle_url": "http://...?_facet=city_id&_facet=state&city_id=3", + "selected": false + }, + { + "value": 4, + "label": "Memnonia", + "count": 1, + "toggle_url": "http://...?_facet=city_id&_facet=state&city_id=4", + "selected": false + } + ], + "truncated": false + } } +If Datasette detects that a column is a foreign key, the ``"label"`` property will be automatically derived from the detected label column on the referenced table. + Facets in metadata.json ----------------------- @@ -58,13 +92,29 @@ You can turn facets on by default for specific tables by adding them to a ``"fac Here's an example that turns on faceting by default for the ``qLegalStatus`` column in the ``Street_Tree_List`` table in the ``sf-trees`` database:: { - "databases": { - "sf-trees": { - "tables": { - "Street_Tree_List": { - "facets": ["qLegalStatus"] - } - } + "databases": { + "sf-trees": { + "tables": { + "Street_Tree_List": { + "facets": ["qLegalStatus"] } + } } + } } + +Facets defined in this way will always be shown in the interface and returned in the API, regardless of the ``_facet`` arguments passed to the view. + +Suggested facets +---------------- + +Datasette's table UI will suggest facets for the user to apply, based on the following criteria: + +For the currently filtered data are there any columns which, if applied as a facet... + +* Will return 20 or less unique options +* Will return more than one unique option +* Will return less unique options than the total number of filtered rows +* And the query used to evaluate this criteria can be completed in under 20ms + +That last point is particularly important: Datasette runs a query for every column that is displayed on a page, which could get expensive - so to avoid slow load times it sets a time limit of just 20ms for each of those queries. This means suggested facets are unlikely to appear for tables with millions of records in them. diff --git a/docs/full_text_search.png b/docs/full_text_search.png new file mode 100644 index 00000000..31f1e6a5 Binary files /dev/null and b/docs/full_text_search.png differ diff --git a/docs/full_text_search.rst b/docs/full_text_search.rst index e80a60c8..9ba8af24 100644 --- a/docs/full_text_search.rst +++ b/docs/full_text_search.rst @@ -5,6 +5,8 @@ Full-text search SQLite includes `a powerful mechanism for enabling full-text search `_ against SQLite records. Datasette can detect if a table has had full-text search configured for it in the underlying database and display a search interface for filtering that table. +.. image:: full_text_search.png + Datasette detects which tables have been configured for full-text search when it first inspects the database on startup (or via the ``datasette inspect`` command). You can visit the ``/-/inspect`` page on your Datasette instance to see the results of this inspection. Tables that have been configured for full-text search will have their ``fts_table`` property set to the name of another table (tables without full-text search will have this property set to ``null``). FTS versions diff --git a/tests/fixtures.py b/tests/fixtures.py index 2f2c32ff..5def4292 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -267,27 +267,41 @@ CREATE TABLE [select] ( ); INSERT INTO [select] VALUES ('group', 'having', 'and'); +CREATE TABLE facet_cities ( + id integer primary key, + name text +); +INSERT INTO facet_cities (id, name) VALUES + (1, 'San Francisco'), + (2, 'Los Angeles'), + (3, 'Detroit'), + (4, 'Memnonia') +; + CREATE TABLE facetable ( pk integer primary key, + planet_int integer, state text, - city text, - neighborhood text + city_id integer, + neighborhood text, + FOREIGN KEY ("city_id") REFERENCES [facet_cities](id) ); -INSERT INTO facetable (state, city, neighborhood) VALUES - ('CA', 'San Francisco', 'Mission'), - ('CA', 'San Francisco', 'Dogpatch'), - ('CA', 'San Francisco', 'SOMA'), - ('CA', 'San Francisco', 'Tenderloin'), - ('CA', 'San Francisco', 'Bernal Heights'), - ('CA', 'San Francisco', 'Hayes Valley'), - ('CA', 'Los Angeles', 'Hollywood'), - ('CA', 'Los Angeles', 'Downtown'), - ('CA', 'Los Angeles', 'Los Feliz'), - ('CA', 'Los Angeles', 'Koreatown'), - ('MI', 'Detroit', 'Downtown'), - ('MI', 'Detroit', 'Greektown'), - ('MI', 'Detroit', 'Corktown'), - ('MI', 'Detroit', 'Mexicantown') +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 simple_primary_key VALUES (1, 'hello'); diff --git a/tests/test_api.py b/tests/test_api.py index 3f91b075..2079ef21 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -19,15 +19,13 @@ def test_homepage(app_client): assert response.json.keys() == {'test_tables': 0}.keys() d = response.json['test_tables'] assert d['name'] == 'test_tables' - assert d['tables_count'] == 16 + assert d['tables_count'] == 17 def test_database_page(app_client): response = app_client.get('/test_tables.json', gather_request=False) data = response.json assert 'test_tables' == data['database'] - from pprint import pprint - pprint(data['tables']) assert [{ 'columns': ['content'], 'name': '123_starts_with_digits', @@ -105,10 +103,33 @@ def test_database_page(app_client): 'fts_table': None, 'primary_keys': ['pk'], }, { - 'columns': ['pk', 'state', 'city', 'neighborhood'], + 'columns': ['id', 'name'], + 'name': 'facet_cities', + 'count': 4, + 'foreign_keys': { + 'incoming': [{ + 'column': 'id', + 'other_column': 'city_id', + 'other_table': 'facetable', + }], + 'outgoing': [] + }, + 'fts_table': None, + 'hidden': False, + 'label_column': 'name', + 'primary_keys': ['id'], + }, { + 'columns': ['pk', 'planet_int', 'state', 'city_id', 'neighborhood'], 'name': 'facetable', - 'count': 14, - 'foreign_keys': {'incoming': [], 'outgoing': []}, + 'count': 15, + 'foreign_keys': { + 'incoming': [], + 'outgoing': [{ + 'column': 'city_id', + 'other_column': 'id', + 'other_table': 'facet_cities' + }], + }, 'fts_table': None, 'hidden': False, 'label_column': None, @@ -891,55 +912,141 @@ def test_page_size_matching_max_returned_rows(app_client_returend_rows_matches_p @pytest.mark.parametrize('path,expected_facet_results', [ ( - "/test_tables/facetable.json?_facet=state&_facet=city", + "/test_tables/facetable.json?_facet=state&_facet=city_id", { - "state": [ - { - "value": "CA", - "count": 10, - "toggle_url": "_facet=state&_facet=city&state=CA", - }, - { - "value": "MI", - "count": 4, - "toggle_url": "_facet=state&_facet=city&state=MI", - }, - ], - "city": [ - { - "value": "San Francisco", - "count": 6, - "toggle_url": "_facet=state&_facet=city&city=San+Francisco", - }, - { - "value": "Detroit", - "count": 4, - "toggle_url": "_facet=state&_facet=city&city=Detroit", - }, - { - "value": "Los Angeles", - "count": 4, - "toggle_url": "_facet=state&_facet=city&city=Los+Angeles", - }, - ], + "state": { + "name": "state", + "results": [ + { + "value": "CA", + "label": "CA", + "count": 10, + "toggle_url": "_facet=state&_facet=city_id&state=CA", + "selected": False, + }, + { + "value": "MI", + "label": "MI", + "count": 4, + "toggle_url": "_facet=state&_facet=city_id&state=MI", + "selected": False, + }, + { + "value": "MC", + "label": "MC", + "count": 1, + "toggle_url": "_facet=state&_facet=city_id&state=MC", + "selected": False, + } + ], + "truncated": False, + }, + "city_id": { + "name": "city_id", + "results": [ + { + "value": 1, + "label": "San Francisco", + "count": 6, + "toggle_url": "_facet=state&_facet=city_id&city_id=1", + "selected": False, + }, + { + "value": 2, + "label": "Los Angeles", + "count": 4, + "toggle_url": "_facet=state&_facet=city_id&city_id=2", + "selected": False, + }, + { + "value": 3, + "label": "Detroit", + "count": 4, + "toggle_url": "_facet=state&_facet=city_id&city_id=3", + "selected": False, + }, + { + "value": 4, + "label": "Memnonia", + "count": 1, + "toggle_url": "_facet=state&_facet=city_id&city_id=4", + "selected": False, + } + ], + "truncated": False, + } + } + ), ( + "/test_tables/facetable.json?_facet=state&_facet=city_id&state=MI", + { + "state": { + "name": "state", + "results": [ + { + "value": "MI", + "label": "MI", + "count": 4, + "selected": True, + "toggle_url": "_facet=state&_facet=city_id", + }, + ], + "truncated": False, + }, + "city_id": { + "name": "city_id", + "results": [ + { + "value": 3, + "label": "Detroit", + "count": 4, + "selected": False, + "toggle_url": "_facet=state&_facet=city_id&state=MI&city_id=3", + }, + ], + "truncated": False, + }, }, ), ( - "/test_tables/facetable.json?_facet=state&_facet=city&state=MI", + "/test_tables/facetable.json?_facet=planet_int", { - "state": [ - { - "value": "MI", - "count": 4, - "toggle_url": "_facet=state&_facet=city&state=MI", - }, - ], - "city": [ - { - "value": "Detroit", - "count": 4, - "toggle_url": "_facet=state&_facet=city&state=MI&city=Detroit", - }, - ], + "planet_int": { + "name": "planet_int", + "results": [ + { + "value": 1, + "label": 1, + "count": 14, + "selected": False, + "toggle_url": "_facet=planet_int&planet_int=1", + }, + { + "value": 2, + "label": 2, + "count": 1, + "selected": False, + "toggle_url": "_facet=planet_int&planet_int=2", + }, + ], + "truncated": False, + } + }, + ), ( + # planet_int is an integer field: + "/test_tables/facetable.json?_facet=planet_int&planet_int=1", + { + "planet_int": { + "name": "planet_int", + "results": [ + { + "value": 1, + "label": 1, + "count": 14, + "selected": True, + "toggle_url": "_facet=planet_int", + } + ], + "truncated": False, + }, }, ) ]) @@ -947,7 +1054,9 @@ def test_facets(app_client, path, expected_facet_results): response = app_client.get(path, gather_request=False) facet_results = response.json['facet_results'] # We only compare the querystring portion of the taggle_url - for facet_name, facet_values in facet_results.items(): - for facet_value in facet_values: + for facet_name, facet_info in facet_results.items(): + assert facet_name == facet_info["name"] + assert False is facet_info["truncated"] + for facet_value in facet_info["results"]: facet_value['toggle_url'] = facet_value['toggle_url'].split('?')[1] assert expected_facet_results == facet_results diff --git a/tests/test_html.py b/tests/test_html.py index 1b30fa95..3e94275f 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -150,6 +150,73 @@ def test_sort_by_desc_redirects(app_client): assert response.headers['Location'].endswith('?_sort_desc=sortable') +def test_sort_links(app_client): + response = app_client.get( + '/test_tables/sortable?_sort=sortable', + gather_request=False + ) + assert response.status == 200 + ths = Soup(response.body, 'html.parser').findAll('th') + attrs_and_link_attrs = [{ + 'attrs': th.attrs, + 'a_href': ( + th.find('a')['href'].split('/')[-1] + if th.find('a') + else None + ), + } for th in ths] + assert [ + { + "attrs": {"class": ["col-Link"], "scope": "col"}, + "a_href": None + }, + { + "attrs": {"class": ["col-pk1"], "scope": "col"}, + "a_href": None + }, + { + "attrs": {"class": ["col-pk2"], "scope": "col"}, + "a_href": None + }, + { + "attrs": {"class": ["col-content"], "scope": "col"}, + "a_href": None + }, + { + "attrs": {"class": ["col-sortable"], "scope": "col"}, + "a_href": "sortable?_sort_desc=sortable", + }, + { + "attrs": {"class": ["col-sortable_with_nulls"], "scope": "col"}, + "a_href": "sortable?_sort=sortable_with_nulls", + }, + { + "attrs": {"class": ["col-sortable_with_nulls_2"], "scope": "col"}, + "a_href": "sortable?_sort=sortable_with_nulls_2", + }, + { + "attrs": {"class": ["col-text"], "scope": "col"}, + "a_href": "sortable?_sort=text", + }, + ] == attrs_and_link_attrs + + +def test_facets_persist_through_filter_form(app_client): + response = app_client.get( + '/test_tables/facetable?_facet=planet_int&_facet=city_id', + gather_request=False + ) + assert response.status == 200 + inputs = Soup(response.body, 'html.parser').find('form').findAll('input') + hiddens = [i for i in inputs if i['type'] == 'hidden'] + assert [ + ('_facet', 'city_id'), + ('_facet', 'planet_int'), + ] == [ + (hidden['name'], hidden['value']) for hidden in hiddens + ] + + @pytest.mark.parametrize('path,expected_classes', [ ('/', ['index']), ('/test_tables', ['db', 'db-test_tables']), diff --git a/tests/test_utils.py b/tests/test_utils.py index 3f51f87d..e701aae6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -35,6 +35,9 @@ def test_urlsafe_components(path, expected): ('/?_facet=state&_facet=city&state=MI', ( ('city', 'Detroit'), ), '/?_facet=state&_facet=city&state=MI&city=Detroit'), + ('/?_facet=state&_facet=city', ( + ('_facet', 'planet_int'), + ), '/?_facet=state&_facet=city&_facet=planet_int'), ]) def test_path_with_added_args(path, added_args, expected): request = Request( @@ -45,6 +48,33 @@ def test_path_with_added_args(path, added_args, expected): assert expected == actual +@pytest.mark.parametrize('path,args,expected', [ + ('/foo?bar=1', {'bar'}, '/foo'), + ('/foo?bar=1&baz=2', {'bar'}, '/foo?baz=2'), + ('/foo?bar=1&bar=2&bar=3', {'bar': '2'}, '/foo?bar=1&bar=3'), +]) +def test_path_with_removed_args(path, args, expected): + request = Request( + path.encode('utf8'), + {}, '1.1', 'GET', None + ) + actual = utils.path_with_removed_args(request, args) + assert expected == actual + + +@pytest.mark.parametrize('path,args,expected', [ + ('/foo?bar=1', {'bar': 2}, '/foo?bar=2'), + ('/foo?bar=1&baz=2', {'bar': None}, '/foo?baz=2'), +]) +def test_path_with_replaced_args(path, args, expected): + request = Request( + path.encode('utf8'), + {}, '1.1', 'GET', None + ) + actual = utils.path_with_replaced_args(request, args) + assert expected == actual + + @pytest.mark.parametrize('row,pks,expected_path', [ ({'A': 'foo', 'B': 'bar'}, ['A', 'B'], 'foo,bar'), ({'A': 'f,o', 'B': 'bar'}, ['A', 'B'], 'f%2Co,bar'),