From da191390a5eb4da803c8ac732fb46aad60920519 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 14:40:24 +0530 Subject: [PATCH 1/3] fix: support for multiple order by in add_conditions --- frappe/database/query.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 8d8a767370..ba19ab6356 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -108,11 +108,14 @@ def change_orderby(order: str): tuple: field, order """ order = order.split() - if order[1].lower() == "asc": - orderby, order = order[0], Order.asc - return orderby, order - orderby, order = order[0], Order.desc - return orderby, order + + try: + if order[1].lower() == "asc": + return order[0], Order.asc + except IndexError: + pass + + return order[0], Order.desc OPERATOR_MAP = { @@ -175,10 +178,13 @@ class Query: """ if kwargs.get("orderby"): orderby = kwargs.get("orderby") - order = kwargs.get("order") if kwargs.get("order") else Order.desc if isinstance(orderby, str) and len(orderby.split()) > 1: - orderby, order = change_orderby(orderby) - conditions = conditions.orderby(orderby, order=order) + for ordby in orderby.split(","): + if ordby := ordby.strip(): + orderby, order = change_orderby(ordby) + conditions = conditions.orderby(orderby, order=order) + else: + conditions = conditions.orderby(orderby, order=kwargs.get("order") or Order.desc) if kwargs.get("limit"): conditions = conditions.limit(kwargs.get("limit")) From c51635702716c64e9c3947176520828adaab27d9 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 17:18:13 +0530 Subject: [PATCH 2/3] test(get_value): test for multiple order bys --- frappe/tests/test_db.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 5b469cd5db..b83c290268 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -87,6 +87,13 @@ class TestDB(unittest.TestCase): frappe.db.get_values("User", filters=[["name", "=", "Administrator"]], fieldname="email"), ) + # test multiple orderby's + delimiter = '"' if frappe.db.db_type == "postgres" else "`" + self.assertIn( + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format(deli=delimiter), + frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0) + ) + def test_get_value_limits(self): # check both dict and list style filters From 6405b0510a39488d7d51b3e23a575eddf790e365 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 20 Apr 2022 17:32:09 +0530 Subject: [PATCH 3/3] chore: fix linter --- frappe/database/query.py | 2 +- frappe/tests/test_db.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ba19ab6356..136f5c86b6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -294,7 +294,7 @@ class Query: table: str, fields: Union[List, Tuple], filters: Union[Dict[str, Union[str, int]], str, int] = None, - **kwargs + **kwargs, ): criterion = self.build_conditions(table, filters, **kwargs) if isinstance(fields, (list, tuple)): diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index b83c290268..6cba55c425 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -90,8 +90,10 @@ class TestDB(unittest.TestCase): # test multiple orderby's delimiter = '"' if frappe.db.db_type == "postgres" else "`" self.assertIn( - "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format(deli=delimiter), - frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0) + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format( + deli=delimiter + ), + frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0), ) def test_get_value_limits(self):