Skip to content

Commit

Permalink
fix: handle distinct for fieldname (#25511) (#25515)
Browse files Browse the repository at this point in the history
`distinct count(fieldname)` is supported well but `count(distinct fieldname)` fails if fieldname contains full field with table name included. This PR just adds basic handling for it.

Needs to be rewritten entirely in QB __some day__.

(cherry picked from commit 8a7beeb)

# Conflicts:
#	frappe/model/db_query.py

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
mergify[bot] and ankush committed Mar 19, 2024
1 parent 6c8cc5c commit 9168b0f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
4 changes: 3 additions & 1 deletion frappe/model/db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,9 @@ def extract_tables(self):

if table_name.lower().startswith("group_concat("):
table_name = table_name[13:]
if not table_name[0] == "`":
if table_name.lower().startswith("distinct"):
table_name = table_name[8:].strip()
if table_name[0] != "`":
table_name = f"`{table_name}`"
if (
table_name not in self.query_tables
Expand Down
2 changes: 1 addition & 1 deletion frappe/public/js/frappe/list/list_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
if (this.count_without_children) {
str = __("{0} of {1} ({2} rows with children)", [
count_without_children,
this.total_count,
count_str,
current_count,
]);
}
Expand Down
16 changes: 16 additions & 0 deletions frappe/tests/test_db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from frappe.model.db_query import DatabaseQuery, get_between_date_filter
from frappe.permissions import add_user_permission, clear_user_permissions_for_doctype
from frappe.query_builder import Column
from frappe.tests.test_query_builder import db_type_is, run_only_if
from frappe.tests.utils import FrappeTestCase
from frappe.utils.testutils import add_custom_field, clear_custom_fields

Expand Down Expand Up @@ -1182,6 +1183,7 @@ def test_ambiguous_linked_tables(self):


class TestReportView(FrappeTestCase):
@run_only_if(db_type_is.MARIADB) # TODO: postgres name casting is messed up
def test_get_count(self):
frappe.local.request = frappe._dict()
frappe.local.request.method = "GET"
Expand Down Expand Up @@ -1241,6 +1243,20 @@ def test_get_count(self):
self.assertIsInstance(count, int)
self.assertLessEqual(count, limit)

# test with distinct
limit = 2
frappe.local.form_dict = frappe._dict(
{
"doctype": "DocType",
"fields": [],
"distinct": "true",
"limit": limit,
}
)
count = execute_cmd("frappe.desk.reportview.get_count")
self.assertIsInstance(count, int)
self.assertLessEqual(count, limit)

def test_reportview_get(self):
user = frappe.get_doc("User", "test@example.com")
add_child_table_to_blog_post()
Expand Down

0 comments on commit 9168b0f

Please sign in to comment.