From 95d0dd7a1cf6be6b7da41e1404184217eb93f64a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 16 Dec 2021 12:12:04 -0800 Subject: [PATCH] Fix for colliding facet types bug, closes #625 Refs #830 --- datasette/facets.py | 74 ++++++++++++++++++++++------------------ datasette/views/table.py | 9 ++++- docs/plugin_hooks.rst | 6 ++-- tests/test_facets.py | 30 ++++++++-------- 4 files changed, 67 insertions(+), 52 deletions(-) diff --git a/datasette/facets.py b/datasette/facets.py index 51fccb01..a1bb4a5f 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -193,7 +193,7 @@ class ColumnFacet(Facet): return suggested_facets async def facet_results(self): - facet_results = {} + facet_results = [] facets_timed_out = [] qs_pairs = self.get_querystring_pairs() @@ -221,16 +221,18 @@ class ColumnFacet(Facet): custom_time_limit=self.ds.setting("facet_time_limit_ms"), ) facet_results_values = [] - facet_results[column] = { - "name": column, - "type": self.type, - "hideable": source != "metadata", - "toggle_url": self.ds.urls.path( - path_with_removed_args(self.request, {"_facet": column}) - ), - "results": facet_results_values, - "truncated": len(facet_rows_results) > facet_size, - } + facet_results.append( + { + "name": column, + "type": self.type, + "hideable": source != "metadata", + "toggle_url": self.ds.urls.path( + path_with_removed_args(self.request, {"_facet": column}) + ), + "results": facet_results_values, + "truncated": len(facet_rows_results) > facet_size, + } + ) facet_rows = facet_rows_results.rows[:facet_size] if self.table: # Attempt to expand foreign keys into labels @@ -352,7 +354,7 @@ class ArrayFacet(Facet): async def facet_results(self): # self.configs should be a plain list of columns - facet_results = {} + facet_results = [] facets_timed_out = [] facet_size = self.get_facet_size() @@ -392,16 +394,20 @@ class ArrayFacet(Facet): custom_time_limit=self.ds.setting("facet_time_limit_ms"), ) facet_results_values = [] - facet_results[column] = { - "name": column, - "type": self.type, - "results": facet_results_values, - "hideable": source != "metadata", - "toggle_url": self.ds.urls.path( - path_with_removed_args(self.request, {"_facet_array": column}) - ), - "truncated": len(facet_rows_results) > facet_size, - } + facet_results.append( + { + "name": column, + "type": self.type, + "results": facet_results_values, + "hideable": source != "metadata", + "toggle_url": self.ds.urls.path( + path_with_removed_args( + self.request, {"_facet_array": column} + ) + ), + "truncated": len(facet_rows_results) > facet_size, + } + ) facet_rows = facet_rows_results.rows[:facet_size] pairs = self.get_querystring_pairs() for row in facet_rows: @@ -480,7 +486,7 @@ class DateFacet(Facet): return suggested_facets async def facet_results(self): - facet_results = {} + facet_results = [] facets_timed_out = [] args = dict(self.get_querystring_pairs()) facet_size = self.get_facet_size() @@ -507,16 +513,18 @@ class DateFacet(Facet): custom_time_limit=self.ds.setting("facet_time_limit_ms"), ) facet_results_values = [] - facet_results[column] = { - "name": column, - "type": self.type, - "results": facet_results_values, - "hideable": source != "metadata", - "toggle_url": path_with_removed_args( - self.request, {"_facet_date": column} - ), - "truncated": len(facet_rows_results) > facet_size, - } + facet_results.append( + { + "name": column, + "type": self.type, + "results": facet_results_values, + "hideable": source != "metadata", + "toggle_url": path_with_removed_args( + self.request, {"_facet_date": column} + ), + "truncated": len(facet_rows_results) > facet_size, + } + ) facet_rows = facet_rows_results.rows[:facet_size] for row in facet_rows: selected = str(args.get(f"{column}__date")) == str(row["value"]) diff --git a/datasette/views/table.py b/datasette/views/table.py index f294ffb1..3d0e27cb 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -754,7 +754,14 @@ class TableView(RowTableShared): instance_facet_results, instance_facets_timed_out, ) = await facet.facet_results() - facet_results.update(instance_facet_results) + for facet_info in instance_facet_results: + base_key = facet_info["name"] + key = base_key + i = 1 + while key in facet_results: + i += 1 + key = f"{base_key}_{i}" + facet_results[key] = facet_info facets_timed_out.extend(instance_facets_timed_out) # Figure out columns and rows for the query diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 23f19e38..4a7c36c3 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -668,7 +668,7 @@ Each Facet subclass implements a new type of facet operation. The class should l async def facet_results(self): # This should execute the facet operation and return results, again # using self.sql and self.params as the starting point - facet_results = {} + facet_results = [] facets_timed_out = [] facet_size = self.get_facet_size() # Do some calculations here... @@ -683,11 +683,11 @@ Each Facet subclass implements a new type of facet operation. The class should l "toggle_url": self.ds.absolute_url(self.request, toggle_path), "selected": selected, }) - facet_results[column] = { + facet_results.append({ "name": column, "results": facet_results_values, "truncated": len(facet_rows_results) > facet_size, - } + }) except QueryInterrupted: facets_timed_out.append(column) diff --git a/tests/test_facets.py b/tests/test_facets.py index 5b1aa935..a99979d3 100644 --- a/tests/test_facets.py +++ b/tests/test_facets.py @@ -107,8 +107,8 @@ async def test_column_facet_results(app_client): ) buckets, timed_out = await facet.facet_results() assert [] == timed_out - assert { - "_city_id": { + assert [ + { "name": "_city_id", "type": "column", "hideable": True, @@ -145,7 +145,7 @@ async def test_column_facet_results(app_client): ], "truncated": False, } - } == buckets + ] == buckets @pytest.mark.asyncio @@ -159,8 +159,8 @@ async def test_column_facet_results_column_starts_with_underscore(app_client): ) buckets, timed_out = await facet.facet_results() assert [] == timed_out - assert buckets == { - "_neighborhood": { + assert buckets == [ + { "name": "_neighborhood", "type": "column", "hideable": True, @@ -267,7 +267,7 @@ async def test_column_facet_results_column_starts_with_underscore(app_client): ], "truncated": False, } - } + ] @pytest.mark.asyncio @@ -282,8 +282,8 @@ async def test_column_facet_from_metadata_cannot_be_hidden(app_client): ) buckets, timed_out = await facet.facet_results() assert [] == timed_out - assert { - "_city_id": { + assert [ + { "name": "_city_id", "type": "column", "hideable": False, @@ -320,7 +320,7 @@ async def test_column_facet_from_metadata_cannot_be_hidden(app_client): ], "truncated": False, } - } == buckets + ] == buckets @pytest.mark.asyncio @@ -369,8 +369,8 @@ async def test_array_facet_results(app_client): ) buckets, timed_out = await facet.facet_results() assert [] == timed_out - assert { - "tags": { + assert [ + { "name": "tags", "type": "array", "results": [ @@ -400,7 +400,7 @@ async def test_array_facet_results(app_client): "toggle_url": "/", "truncated": False, } - } == buckets + ] == buckets @pytest.mark.asyncio @@ -471,8 +471,8 @@ async def test_date_facet_results(app_client): ) buckets, timed_out = await facet.facet_results() assert [] == timed_out - assert { - "created": { + assert [ + { "name": "created", "type": "date", "results": [ @@ -509,7 +509,7 @@ async def test_date_facet_results(app_client): "toggle_url": "/", "truncated": False, } - } == buckets + ] == buckets @pytest.mark.asyncio