From 0b659730ac2c2a15d4c177371ea15893eb4fed53 Mon Sep 17 00:00:00 2001
From: Jakub Gryko <87192257+jgryko5@users.noreply.github.com>
Date: Tue, 13 Jul 2021 09:56:29 +0200
Subject: [PATCH 1/2] Added ability to sort by more than one column
---
datasette/static/app.css | 6 +-
datasette/static/sort_utils.js | 142 +++++++++++++++++
datasette/templates/__sorting_example.html | 30 ++++
datasette/templates/_table.html | 5 +-
datasette/utils/__init__.py | 43 +++++
datasette/views/table.py | 175 ++++++++++++---------
package-lock.json | 12 +-
tests/fixtures.py | 2 +
tests/test_api.py | 20 +++
tests/test_html.py | 4 +-
tests/test_utils.py | 55 +++++++
11 files changed, 411 insertions(+), 83 deletions(-)
create mode 100644 datasette/static/sort_utils.js
create mode 100644 datasette/templates/__sorting_example.html
diff --git a/datasette/static/app.css b/datasette/static/app.css
index ad517c98c4..f6ba74d6d4 100644
--- a/datasette/static/app.css
+++ b/datasette/static/app.css
@@ -519,7 +519,7 @@ input[type="search"]::-webkit-search-results-decoration {
display: none;
}
-form input[type=submit], form button[type=button] {
+form input[type=submit], form button[type=button], .btn-primary, .btn-secondary{
font-weight: 400;
cursor: pointer;
text-align: center;
@@ -532,14 +532,14 @@ form input[type=submit], form button[type=button] {
border-radius: .25rem;
}
-form input[type=submit] {
+form input[type=submit], .btn-primary{
color: #fff;
background-color: #007bff;
border-color: #007bff;
-webkit-appearance: button;
}
-form button[type=button] {
+form button[type=button]{
color: #007bff;
background-color: #fff;
border-color: #007bff;
diff --git a/datasette/static/sort_utils.js b/datasette/static/sort_utils.js
new file mode 100644
index 0000000000..2f7006dabe
--- /dev/null
+++ b/datasette/static/sort_utils.js
@@ -0,0 +1,142 @@
+function sort() {
+ /*
+ Generate url with sorting parameters according to the
+ sorting queue
+ */
+ let sq = window.sorting_queue;
+ if (!sq.length) {
+ if (!document.getElementById(`alert_cols`))
+ document
+ .getElementById("sort_utils")
+ .insertAdjacentHTML(
+ "afterend",
+ `You need to select some columns to sort!
`
+ );
+ return;
+ }
+ var sort_url = new URLSearchParams(window.search);
+ for (let option of sq) {
+ sort_url.append(
+ `_sort${option.direction === "dsc" ? "_desc" : ""}`,
+ option.name
+ );
+ }
+ //Taken from table.js - line 37
+ window.location.href = sort_url ? "?" + sort_url : location.pathname;
+}
+
+function toggleSortMenu() {
+ /*
+ Function used to toggle the visibility of the sorting menu
+ */
+ let menu = document.getElementById("sort_menu");
+ let btn = document.getElementById("toggle_sort_menu");
+ if (menu.style.display == "none") {
+ menu.style.display = "inline-block";
+ menu.classList.add("anim-scale-in");
+ //Taken from table.js - lines 79-85
+ document.addEventListener("click", function (ev) {
+ var target = ev.target;
+ while (target && target != menu && target != btn) {
+ target = target.parentNode;
+ }
+ if (!target) {
+ menu.style.display = "none";
+ }
+ });
+ } else {
+ menu.style.display = "none";
+ }
+}
+
+function populateSortMenu() {
+ window.sorting_queue = [];
+ var sort_url = new URLSearchParams(window.location.search);
+ /*
+ Clearing all of the checkboxes and selecting them according to url parameters.
+ */
+ var all_checkboxes = document.querySelectorAll(`input[type=checkbox]`);
+ for (let checkbox of all_checkboxes) {
+ checkbox.checked = false;
+ }
+ params = [];
+ sort_url.forEach(function (value, key) {
+ params.push({
+ name: key,
+ value: value,
+ });
+ modifySortingQueue(value, key.includes("_desc") ? "dsc" : "asc");
+ });
+ if (!params.length) return;
+ for (let param of params) {
+ let chb = document.getElementsByName(param.value)[0];
+ chb.checked = true;
+ if (param.name.includes("_desc")) var rdb = `${param.value}_dsc`;
+ else var rdb = `${param.value}_asc`;
+ document.getElementById(rdb).checked = true;
+ }
+}
+
+function modifySortingQueue(column, type = undefined) {
+ /*
+ Function that runs every time a checkbox is clicked.
+ If it does not exist in the queue, it is added.
+ If it exists, it is removed from the queue.
+ */
+ let sq = window.sorting_queue;
+ var s_option = sq.find((condition) => condition["name"] === column);
+ if (!s_option) {
+ var type =
+ document.querySelector(`input[name="${column}_direction"]:checked`)
+ .value || type;
+ if (!type) type = "asc";
+ sq.push({
+ name: column,
+ direction: type,
+ selected: true,
+ });
+ } else
+ sq.splice(
+ sq.findIndex(function (e) {
+ return e === s_option;
+ }),
+ 1
+ );
+ displaySortingDescription();
+}
+
+function displaySortingDescription() {
+ /*
+ Function that generates a human description of sorting
+ based on selected options in the sorting queue.
+ */
+ let sq = window.sorting_queue;
+ var s_option_p = document.getElementById("selected_options");
+ var hd = [];
+ for (let condition of sq) {
+ if (condition.selected)
+ hd.push(
+ `${condition.name}${condition.direction === "dsc" ? " descending" : ""}`
+ );
+ }
+ s_option_p.innerHTML = hd.join(", ");
+}
+
+function modifySortingDirection(column) {
+ /*
+ Every time a radio button is clicked,
+ the corresponding value in the sorting queue is modified.
+ */
+ let sq = window.sorting_queue;
+ var s_option = sq.find((condition) => condition["name"] === column);
+ if (!s_option) return;
+ var type = document.querySelector(
+ `input[name="${column}_direction"]:checked`
+ ).value;
+ sq[
+ sq.findIndex(function (e) {
+ return e === s_option;
+ })
+ ].direction = type;
+ displaySortingDescription();
+}
diff --git a/datasette/templates/__sorting_example.html b/datasette/templates/__sorting_example.html
new file mode 100644
index 0000000000..3f7f4119ea
--- /dev/null
+++ b/datasette/templates/__sorting_example.html
@@ -0,0 +1,30 @@
+
+
+
+
+
+
\ No newline at end of file
diff --git a/datasette/templates/_table.html b/datasette/templates/_table.html
index d765937eb5..93213fe31e 100644
--- a/datasette/templates/_table.html
+++ b/datasette/templates/_table.html
@@ -1,4 +1,5 @@
{% if display_rows %}
+{% include "__sorting_example.html" %}
@@ -8,10 +9,10 @@
{% if not column.sortable %}
{{ column.name }}
{% else %}
- {% if column.name == sort %}
+ {% if column.name in sort %}
{{ column.name }} ▼
{% else %}
- {{ column.name }}{% if column.name == sort_desc %} ▲{% endif %}
+ {{ column.name }}{% if column.name in sort_desc %} ▲{% endif %}
{% endif %}
{% endif %}
diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py
index aec5a55b98..dad62cb864 100644
--- a/datasette/utils/__init__.py
+++ b/datasette/utils/__init__.py
@@ -68,6 +68,24 @@ async def await_me_maybe(value):
return value
+def check_nulls(list, values):
+ """Takes a Python list, escapes its values and converts to '{column_name} is null'"""
+ value_list = ""
+ for item in list:
+ if not values[item]:
+ value_list += f"{escape_sqlite(item)} is null and"
+ return value_list[:-4]
+
+
+def check_not_nulls(list, values):
+ """Takes a Python list, escapes its values and converts to '{column_name} is not null'"""
+ value_list = ""
+ for item in list:
+ if not values[item]:
+ value_list += f"{escape_sqlite(item)} is not null and"
+ return value_list[:-4]
+
+
def urlsafe_components(token):
"""Splits token on commas and URL decodes each component"""
return [urllib.parse.unquote_plus(b) for b in token.split(",")]
@@ -116,6 +134,27 @@ def compound_keys_after_sql(pks, start_index=0):
return "({})".format("\n or\n".join(or_clauses))
+def compound_sort_sql(sort, start_index=0):
+ #
+ # A modified version of compound_keys_after_sql supporting sorting by multiple columns in any direction
+ #
+ or_clauses = []
+ pks_left = sort[:]
+ while pks_left:
+ and_clauses = []
+ last = pks_left[-1]
+ rest = pks_left[:-1]
+ for i, pk in enumerate(rest):
+ and_clauses.append(f"{escape_sqlite(pk.name)} = :p{i + start_index}")
+ and_clauses.append(
+ f"{escape_sqlite(last.name)} {'>' if last.direction=='asc' else '<'} :p{len(rest) + start_index}"
+ )
+ or_clauses.append(f"({' and '.join(and_clauses)})")
+ pks_left.pop()
+ or_clauses.reverse()
+ return "({})".format("\n or\n".join(or_clauses))
+
+
class CustomJSONEncoder(json.JSONEncoder):
def default(self, obj):
if isinstance(obj, sqlite3.Row):
@@ -1076,3 +1115,7 @@ def method(self, *args, **kwargs):
class StartupError(Exception):
pass
+
+
+SortingOrder = namedtuple("SortingOrder", ["name", "direction"])
+"""An Object storing a particular sorting condition."""
diff --git a/datasette/views/table.py b/datasette/views/table.py
index 876a0c8151..10292ac403 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -22,6 +22,10 @@
to_css_class,
urlsafe_components,
value_as_boolean,
+ check_nulls,
+ check_not_nulls,
+ compound_sort_sql,
+ SortingOrder,
)
from datasette.utils.asgi import BadRequest, NotFound
from datasette.filters import Filters
@@ -387,10 +391,12 @@ async def data(
nofacet = True
# Ensure we don't drop anything with an empty value e.g. ?name__exact=
- args = MultiParams(
- urllib.parse.parse_qs(request.query_string, keep_blank_values=True)
+ query_params = urllib.parse.parse_qsl(
+ request.query_string, keep_blank_values=True
)
+ args = MultiParams(query_params)
+
# Special args start with _ and do not contain a __
# That's so if there is a column that starts with _
# it can still be queried using ?_col__exact=blah
@@ -543,29 +549,35 @@ async def data(
sortable_columns = await self.sortable_columns_for_table(
database, table, use_rowid
)
-
- # Allow for custom sort order
- sort = special_args.get("_sort")
- sort_desc = special_args.get("_sort_desc")
-
- if not sort and not sort_desc:
- sort = table_metadata.get("sort")
- sort_desc = table_metadata.get("sort_desc")
-
- if sort and sort_desc:
- raise DatasetteError("Cannot use _sort and _sort_desc at the same time")
-
- if sort:
- if sort not in sortable_columns:
- raise DatasetteError(f"Cannot sort table by {sort}")
-
- order_by = escape_sqlite(sort)
-
- if sort_desc:
- if sort_desc not in sortable_columns:
- raise DatasetteError(f"Cannot sort table by {sort_desc}")
-
- order_by = f"{escape_sqlite(sort_desc)} desc"
+ #
+ # Getting sorting conditions from url and saving them in a list
+ #
+ sort = [
+ SortingOrder(escape_sqlite(value), "asc" if name == "_sort" else "desc")
+ for name, value in query_params
+ if name == "_sort" or name == "_sort_desc"
+ ]
+ sort_asc = []
+ sort_desc = []
+ if not sort:
+ if table_metadata.get("sort"):
+ sort.append(SortingOrder(table_metadata.get("sort"), "asc"))
+ if table_metadata.get("sort_desc"):
+ sort.append(SortingOrder(table_metadata.get("sort_desc"), "desc"))
+ for item in sort:
+ if item.direction == "asc":
+ sort_asc.append(item.name)
+ elif item.direction == "desc":
+ sort_desc.append(item.name)
+ order_by_list = []
+ for condition in sort:
+ if condition.name not in sortable_columns:
+ raise DatasetteError(f"Cannot sort table by {condition.name}")
+ order_by_list.append(
+ f"{escape_sqlite(condition.name)}{' desc' if condition.direction == 'desc' else ''}"
+ )
+ if order_by_list:
+ order_by = ",".join(order_by_list)
from_sql = "from {table_name} {where}".format(
table_name=escape_sqlite(table),
@@ -581,18 +593,23 @@ async def data(
_next = _next or special_args.get("_next")
offset = ""
if _next:
+ next_params_not_parsed = _next.split(",")
if is_view:
# _next is an offset
offset = f" offset {int(_next)}"
else:
components = urlsafe_components(_next)
# If a sort order is applied, the first of these is the sort value
- if sort or sort_desc:
- sort_value = components[0]
- # Special case for if non-urlencoded first token was $null
- if _next.split(",")[0] == "$null":
- sort_value = None
- components = components[1:]
+ if sort:
+ sort_values = dict()
+ next_sorting_values = components[: len(sort)]
+ for index, item in enumerate(sort):
+ sv = next_sorting_values[index]
+ if sv == "$null" and next_params_not_parsed[index] == "$null":
+ sort_values[item.name] = None
+ else:
+ sort_values[item.name] = sv
+ components = components[len(sort) :]
# Figure out the SQL for next-based-on-primary-key first
next_by_pk_clauses = []
@@ -602,50 +619,58 @@ async def data(
else:
# Apply the tie-breaker based on primary keys
if len(components) == len(pks):
- param_len = len(params)
+ param_len = len(params) + len(sort)
next_by_pk_clauses.append(
compound_keys_after_sql(pks, param_len)
)
for i, pk_value in enumerate(components):
params[f"p{param_len + i}"] = pk_value
-
# Now add the sort SQL, which may incorporate next_by_pk_clauses
- if sort or sort_desc:
- if sort_value is None:
+ if sort:
+ if None in sort_values.values():
if sort_desc:
# Just items where column is null ordered by pk
where_clauses.append(
- "({column} is null and {next_clauses})".format(
- column=escape_sqlite(sort_desc),
+ "({column_null} and {next_clauses})".format(
+ column_null=check_nulls(sort_desc, sort_values),
next_clauses=" and ".join(next_by_pk_clauses),
)
)
+
else:
where_clauses.append(
- "({column} is not null or ({column} is null and {next_clauses}))".format(
- column=escape_sqlite(sort),
+ "({column_not_null} or ({column_null} and {next_clauses}))".format(
+ column_not_null=check_not_nulls(
+ sort_asc, sort_values
+ ),
+ column_null=check_nulls(sort_asc, sort_values),
next_clauses=" and ".join(next_by_pk_clauses),
)
)
else:
+
+ pagination = []
+ pagination.append(compound_sort_sql(sort, 0))
+ ties = []
+ for index, item in enumerate(sort):
+ ties.append(f"{item.name} = :p{index}")
where_clauses.append(
- "({column} {op} :p{p}{extra_desc_only} or ({column} = :p{p} and {next_clauses}))".format(
- column=escape_sqlite(sort or sort_desc),
- op=">" if sort else "<",
- p=len(params),
+ "({pagination}{extra_desc_only} or ({ties} and {next_clauses}))".format(
+ ties=" and ".join(ties),
+ pagination="".join(pagination),
extra_desc_only=""
- if sort
+ if sort_asc
else " or {column2} is null".format(
- column2=escape_sqlite(sort or sort_desc)
+ column2=escape_sqlite(",".join(sort_desc))
),
next_clauses=" and ".join(next_by_pk_clauses),
)
)
- params[f"p{len(params)}"] = sort_value
+ for index, item in enumerate(sort):
+ params[f"p{index}"] = sort_values[item.name]
order_by = f"{order_by}, {order_by_pks}"
else:
where_clauses.extend(next_by_pk_clauses)
-
where_clause = ""
if where_clauses:
where_clause = f"where {' and '.join(where_clauses)} "
@@ -807,28 +832,35 @@ async def data(
# Pagination next link
next_value = None
next_url = None
+ added_args = []
if 0 < page_size < len(rows):
if is_view:
next_value = int(_next or 0) + page_size
else:
next_value = path_from_row_pks(rows[-2], pks, use_rowid)
# If there's a sort or sort_desc, add that value as a prefix
- if (sort or sort_desc) and not is_view:
- prefix = rows[-2][sort or sort_desc]
- if isinstance(prefix, dict) and "value" in prefix:
- prefix = prefix["value"]
- if prefix is None:
- prefix = "$null"
- else:
- prefix = urllib.parse.quote_plus(str(prefix))
+ if sort and not is_view:
+ prefix = []
+ for condition in sort:
+ item = rows[-2][condition.name]
+ if item:
+ if isinstance(item, dict) and "value" in item:
+ item = item["value"]
+ item = urllib.parse.quote_plus(str(item))
+ prefix.append(item)
+ else:
+ prefix.append("$null")
+
+ prefix = ",".join(prefix)
next_value = f"{prefix},{next_value}"
- added_args = {"_next": next_value}
- if sort:
- added_args["_sort"] = sort
- else:
- added_args["_sort_desc"] = sort_desc
+ added_args.append(("_next", next_value))
+ for item in sort:
+ if item.direction == "asc":
+ added_args.append(("_sort", item.name))
+ else:
+ added_args.append(("_sort_desc", item.name))
else:
- added_args = {"_next": next_value}
+ added_args.append(("_next", next_value))
next_url = self.ds.absolute_url(
request, path_with_replaced_args(request, added_args)
)
@@ -850,11 +882,14 @@ async def data(
human_description_en = filters.human_description_en(
extra=extra_human_descriptions
)
-
- if sort or sort_desc:
- sorted_by = "sorted by {}{}".format(
- (sort or sort_desc), " descending" if sort_desc else ""
- )
+ sorting_conditions = []
+ for item in sort:
+ if item.direction == "asc":
+ sorting_conditions.append(item.name)
+ else:
+ sorting_conditions.append(f"{item.name} descending")
+ if sorting_conditions:
+ sorted_by = f"sorted by {', '.join(sorting_conditions)}"
human_description_en = " ".join(
[b for b in [human_description_en, sorted_by] if b]
)
@@ -886,11 +921,11 @@ async def extra_template():
# if no sort specified AND table has a single primary key,
# set sort to that so arrow is displayed
- if not sort and not sort_desc:
+ if not sort:
if 1 == len(pks):
- sort = pks[0]
+ sort_asc.append(pks[0])
elif use_rowid:
- sort = "rowid"
+ sort_asc.append("rowid")
async def table_actions():
links = []
@@ -928,7 +963,7 @@ async def table_actions():
"path_with_removed_args": path_with_removed_args,
"append_querystring": append_querystring,
"request": request,
- "sort": sort,
+ "sort": sort_asc,
"sort_desc": sort_desc,
"disable_sort": is_view,
"custom_table_templates": [
diff --git a/package-lock.json b/package-lock.json
index 06623e6f84..3c12cca706 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -9,9 +9,9 @@
}
},
"node_modules/prettier": {
- "version": "2.2.1",
- "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.2.1.tgz",
- "integrity": "sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q==",
+ "version": "2.3.2",
+ "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.3.2.tgz",
+ "integrity": "sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ==",
"dev": true,
"bin": {
"prettier": "bin-prettier.js"
@@ -23,9 +23,9 @@
},
"dependencies": {
"prettier": {
- "version": "2.2.1",
- "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.2.1.tgz",
- "integrity": "sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q==",
+ "version": "2.3.2",
+ "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.3.2.tgz",
+ "integrity": "sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ==",
"dev": true
}
}
diff --git a/tests/fixtures.py b/tests/fixtures.py
index dce9487691..96b7bfe6b4 100644
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@@ -318,6 +318,8 @@ def generate_sortable_rows(num):
},
"sortable": {
"sortable_columns": [
+ "pk1",
+ "pk2",
"sortable",
"sortable_with_nulls",
"sortable_with_nulls_2",
diff --git a/tests/test_api.py b/tests/test_api.py
index 0049d76d84..6d34cd39df 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -950,6 +950,24 @@ def test_paginate_compound_keys_with_extra_filters(app_client):
),
# text column contains '$null' - ensure it doesn't confuse pagination:
("_sort=text", lambda row: row["text"], "sorted by text"),
+ # test cases for sorting by multiple columns
+ (
+ "_sort_desc=sortable&_sort=pk2",
+ lambda row: (
+ -row["sortable"],
+ row["pk2"],
+ ),
+ "sorted by sortable descending, pk2",
+ ),
+ (
+ "_sort=pk2&_sort_desc=sortable&_sort=pk1",
+ lambda row: (
+ row["pk2"],
+ -row["sortable"],
+ row["pk1"],
+ ),
+ "sorted by pk2, sortable descending, pk1",
+ ),
],
)
def test_sortable(app_client, query_string, sort_key, human_description_en):
@@ -993,10 +1011,12 @@ def test_sortable_argument_errors(app_client):
assert "Cannot sort table by badcolumn" == response.json["error"]
response = app_client.get("/fixtures/sortable.json?_sort_desc=badcolumn2")
assert "Cannot sort table by badcolumn2" == response.json["error"]
+ """ Test not needed anymore - Datasette should now support sorting by many conditions
response = app_client.get(
"/fixtures/sortable.json?_sort=sortable_with_nulls&_sort_desc=sortable"
)
assert "Cannot use _sort and _sort_desc at the same time" == response.json["error"]
+ """
def test_sortable_columns_metadata(app_client):
diff --git a/tests/test_html.py b/tests/test_html.py
index aee6bce193..510e3a0907 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -397,7 +397,7 @@ def test_sort_links(app_client):
"data-column-not-null": "0",
"data-is-pk": "1",
},
- "a_href": None,
+ "a_href": "/fixtures/sortable?_sort=pk1",
},
{
"attrs": {
@@ -408,7 +408,7 @@ def test_sort_links(app_client):
"data-column-not-null": "0",
"data-is-pk": "1",
},
- "a_href": None,
+ "a_href": "/fixtures/sortable?_sort=pk2",
},
{
"attrs": {
diff --git a/tests/test_utils.py b/tests/test_utils.py
index be3daf2eb8..78d15506b8 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -349,6 +349,61 @@ def test_compound_keys_after_sql():
)
+def test_compound_sort_sql():
+ assert """
+((a > :p0)
+ or
+(a = :p0 and b > :p1)
+ or
+(a = :p0 and b = :p1 and c < :p2))
+ """.strip() == utils.compound_sort_sql(
+ [
+ utils.SortingOrder("a", "asc"),
+ utils.SortingOrder("b", "asc"),
+ utils.SortingOrder("c", "dsc"),
+ ]
+ )
+ assert """
+((a < :p0)
+ or
+(a = :p0 and b < :p1)
+ or
+(a = :p0 and b = :p1 and c > :p2))
+ """.strip() == utils.compound_sort_sql(
+ [
+ utils.SortingOrder("a", "dsc"),
+ utils.SortingOrder("b", "dsc"),
+ utils.SortingOrder("c", "asc"),
+ ]
+ )
+ assert """
+((a < :p0)
+ or
+(a = :p0 and b > :p1)
+ or
+(a = :p0 and b = :p1 and c < :p2))
+ """.strip() == utils.compound_sort_sql(
+ [
+ utils.SortingOrder("a", "dsc"),
+ utils.SortingOrder("b", "asc"),
+ utils.SortingOrder("c", "dsc"),
+ ]
+ )
+ assert """
+((a < :p0)
+ or
+(a = :p0 and b < :p1)
+ or
+(a = :p0 and b = :p1 and c < :p2))
+ """.strip() == utils.compound_sort_sql(
+ [
+ utils.SortingOrder("a", "dsc"),
+ utils.SortingOrder("b", "dsc"),
+ utils.SortingOrder("c", "dsc"),
+ ]
+ )
+
+
async def table_exists(table):
return table == "exists.csv"
From 739697660382e4d2974619b4a5605baef87d233a Mon Sep 17 00:00:00 2001
From: Jakub Gryko <87192257+jgryko5@users.noreply.github.com>
Date: Mon, 19 Jul 2021 12:43:40 +0200
Subject: [PATCH 2/2] Making suggested changes
---
datasette/static/app.css | 2 +-
datasette/static/sort_utils.js | 30 +++++++++++-----------
datasette/templates/__sorting_example.html | 2 +-
datasette/utils/__init__.py | 7 ++---
datasette/views/table.py | 11 +++-----
package-lock.json | 12 ++++-----
tests/test_api.py | 6 -----
7 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/datasette/static/app.css b/datasette/static/app.css
index f6ba74d6d4..c0d9783d63 100644
--- a/datasette/static/app.css
+++ b/datasette/static/app.css
@@ -519,7 +519,7 @@ input[type="search"]::-webkit-search-results-decoration {
display: none;
}
-form input[type=submit], form button[type=button], .btn-primary, .btn-secondary{
+form input[type=submit], form button[type=button], .btn-primary, .btn-secondary {
font-weight: 400;
cursor: pointer;
text-align: center;
diff --git a/datasette/static/sort_utils.js b/datasette/static/sort_utils.js
index 2f7006dabe..e2ac8c7ce3 100644
--- a/datasette/static/sort_utils.js
+++ b/datasette/static/sort_utils.js
@@ -31,22 +31,22 @@ function toggleSortMenu() {
*/
let menu = document.getElementById("sort_menu");
let btn = document.getElementById("toggle_sort_menu");
- if (menu.style.display == "none") {
- menu.style.display = "inline-block";
- menu.classList.add("anim-scale-in");
- //Taken from table.js - lines 79-85
- document.addEventListener("click", function (ev) {
- var target = ev.target;
- while (target && target != menu && target != btn) {
- target = target.parentNode;
- }
- if (!target) {
- menu.style.display = "none";
- }
- });
- } else {
- menu.style.display = "none";
+ if (menu.style.display != "none") {
+ menu.style.dispaly = "none";
+ return;
}
+ menu.style.display = "inline-block";
+ menu.classList.add("anim-scale-in");
+ //Taken from table.js - lines 79-85
+ document.addEventListener("click", function (ev) {
+ var target = ev.target;
+ while (target && target != menu && target != btn) {
+ target = target.parentNode;
+ }
+ if (!target) {
+ menu.style.display = "none";
+ }
+ });
}
function populateSortMenu() {
diff --git a/datasette/templates/__sorting_example.html b/datasette/templates/__sorting_example.html
index 3f7f4119ea..29067c2b82 100644
--- a/datasette/templates/__sorting_example.html
+++ b/datasette/templates/__sorting_example.html
@@ -27,4 +27,4 @@
-
\ No newline at end of file
+
diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py
index dad62cb864..1ebc345a06 100644
--- a/datasette/utils/__init__.py
+++ b/datasette/utils/__init__.py
@@ -141,11 +141,12 @@ def compound_sort_sql(sort, start_index=0):
or_clauses = []
pks_left = sort[:]
while pks_left:
- and_clauses = []
last = pks_left[-1]
rest = pks_left[:-1]
- for i, pk in enumerate(rest):
- and_clauses.append(f"{escape_sqlite(pk.name)} = :p{i + start_index}")
+ and_clauses = [
+ f"{escape_sqlite(pk.name)} = :p{i + start_index}"
+ for i, pk in enumerate(rest)
+ ]
and_clauses.append(
f"{escape_sqlite(last.name)} {'>' if last.direction=='asc' else '<'} :p{len(rest) + start_index}"
)
diff --git a/datasette/views/table.py b/datasette/views/table.py
index 10292ac403..2133e004a5 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -553,22 +553,17 @@ async def data(
# Getting sorting conditions from url and saving them in a list
#
sort = [
- SortingOrder(escape_sqlite(value), "asc" if name == "_sort" else "desc")
+ SortingOrder(value, "asc" if name == "_sort" else "desc")
for name, value in query_params
if name == "_sort" or name == "_sort_desc"
]
- sort_asc = []
- sort_desc = []
if not sort:
if table_metadata.get("sort"):
sort.append(SortingOrder(table_metadata.get("sort"), "asc"))
if table_metadata.get("sort_desc"):
sort.append(SortingOrder(table_metadata.get("sort_desc"), "desc"))
- for item in sort:
- if item.direction == "asc":
- sort_asc.append(item.name)
- elif item.direction == "desc":
- sort_desc.append(item.name)
+ sort_asc = [item.name for item in sort if item.direction == "asc"]
+ sort_desc = [item.name for item in sort if item.direction == "desc"]
order_by_list = []
for condition in sort:
if condition.name not in sortable_columns:
diff --git a/package-lock.json b/package-lock.json
index 3c12cca706..06623e6f84 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -9,9 +9,9 @@
}
},
"node_modules/prettier": {
- "version": "2.3.2",
- "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.3.2.tgz",
- "integrity": "sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ==",
+ "version": "2.2.1",
+ "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.2.1.tgz",
+ "integrity": "sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q==",
"dev": true,
"bin": {
"prettier": "bin-prettier.js"
@@ -23,9 +23,9 @@
},
"dependencies": {
"prettier": {
- "version": "2.3.2",
- "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.3.2.tgz",
- "integrity": "sha512-lnJzDfJ66zkMy58OL5/NY5zp70S7Nz6KqcKkXYzn2tMVrNxvbqaBpg7H3qHaLxCJ5lNMsGuM8+ohS7cZrthdLQ==",
+ "version": "2.2.1",
+ "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.2.1.tgz",
+ "integrity": "sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q==",
"dev": true
}
}
diff --git a/tests/test_api.py b/tests/test_api.py
index 6d34cd39df..2d78b57df7 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1011,12 +1011,6 @@ def test_sortable_argument_errors(app_client):
assert "Cannot sort table by badcolumn" == response.json["error"]
response = app_client.get("/fixtures/sortable.json?_sort_desc=badcolumn2")
assert "Cannot sort table by badcolumn2" == response.json["error"]
- """ Test not needed anymore - Datasette should now support sorting by many conditions
- response = app_client.get(
- "/fixtures/sortable.json?_sort=sortable_with_nulls&_sort_desc=sortable"
- )
- assert "Cannot use _sort and _sort_desc at the same time" == response.json["error"]
- """
def test_sortable_columns_metadata(app_client):