From 4ab2ccd01fb1ef028111393df62c86b8188841cf Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 11 Mar 2024 15:28:56 +0530 Subject: [PATCH] fix!: get_count accepts splatted arguments --- frappe/core/doctype/rq_job/rq_job.py | 20 ++++++++----------- frappe/core/doctype/rq_job/test_rq_job.py | 6 +++--- frappe/core/doctype/rq_worker/rq_worker.py | 4 ++-- .../core/doctype/rq_worker/test_rq_worker.py | 4 ++-- frappe/desk/reportview.py | 4 ++-- frappe/tests/test_db_query.py | 18 ++++------------- 6 files changed, 21 insertions(+), 35 deletions(-) diff --git a/frappe/core/doctype/rq_job/rq_job.py b/frappe/core/doctype/rq_job/rq_job.py index 995ceda4c7..93ca331443 100644 --- a/frappe/core/doctype/rq_job/rq_job.py +++ b/frappe/core/doctype/rq_job/rq_job.py @@ -76,22 +76,18 @@ class RQJob(Document): return self._job_obj @staticmethod - def get_list(args): - start = cint(args.get("start")) - page_length = cint(args.get("page_length")) or 20 - - order_desc = "desc" in args.get("order_by", "") - - matched_job_ids = RQJob.get_matching_job_ids(args)[start : start + page_length] + def get_list(filters=None, start=0, page_length=20, order_by="modified desc"): + matched_job_ids = RQJob.get_matching_job_ids(filters=filters)[start : start + page_length] conn = get_redis_conn() jobs = [serialize_job(job) for job in Job.fetch_many(job_ids=matched_job_ids, connection=conn) if job] + order_desc = "desc" in order_by return sorted(jobs, key=lambda j: j.modified, reverse=order_desc) @staticmethod - def get_matching_job_ids(args) -> list[str]: - filters = make_filter_dict(args.get("filters")) + def get_matching_job_ids(filters) -> list[str]: + filters = make_filter_dict(filters or []) queues = _eval_filters(filters.get("queue"), QUEUES) statuses = _eval_filters(filters.get("status"), JOB_STATUSES) @@ -117,12 +113,12 @@ class RQJob(Document): frappe.msgprint(_("Job is not running."), title=_("Invalid Operation")) @staticmethod - def get_count(args) -> int: - return len(RQJob.get_matching_job_ids(args)) + def get_count(filters=None) -> int: + return len(RQJob.get_matching_job_ids(filters)) # None of these methods apply to virtual job doctype, overriden for sanity. @staticmethod - def get_stats(args): + def get_stats(): return {} def db_insert(self, *args, **kwargs): diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py index 57c857f7ab..938187142f 100644 --- a/frappe/core/doctype/rq_job/test_rq_job.py +++ b/frappe/core/doctype/rq_job/test_rq_job.py @@ -61,18 +61,18 @@ class TestRQJob(FrappeTestCase): def test_get_list_filtering(self): # Check failed job clearning and filtering remove_failed_jobs() - jobs = RQJob.get_list({"filters": [["RQ Job", "status", "=", "failed"]]}) + jobs = frappe.get_all("RQ Job", {"status": "failed"}) self.assertEqual(jobs, []) # Fail a job job = frappe.enqueue(method=self.BG_JOB, queue="short", fail=True) self.check_status(job, "failed") - jobs = RQJob.get_list({"filters": [["RQ Job", "status", "=", "failed"]]}) + jobs = frappe.get_all("RQ Job", {"status": "failed"}) self.assertEqual(len(jobs), 1) self.assertTrue(jobs[0].exc_info) # Assert that non-failed job still exists - non_failed_jobs = RQJob.get_list({"filters": [["RQ Job", "status", "!=", "failed"]]}) + non_failed_jobs = frappe.get_all("RQ Job", {"status": ("!=", "failed")}) self.assertGreaterEqual(len(non_failed_jobs), 1) # Create a slow job and check if it's stuck in "Started" diff --git a/frappe/core/doctype/rq_worker/rq_worker.py b/frappe/core/doctype/rq_worker/rq_worker.py index 47cdc7c088..2d5bc996e1 100644 --- a/frappe/core/doctype/rq_worker/rq_worker.py +++ b/frappe/core/doctype/rq_worker/rq_worker.py @@ -53,12 +53,12 @@ class RQWorker(Document): return [serialize_worker(worker) for worker in valid_workers] @staticmethod - def get_count(args) -> int: + def get_count() -> int: return len(get_workers()) # None of these methods apply to virtual workers, overriden for sanity. @staticmethod - def get_stats(args): + def get_stats(): return {} def db_insert(self, *args, **kwargs): diff --git a/frappe/core/doctype/rq_worker/test_rq_worker.py b/frappe/core/doctype/rq_worker/test_rq_worker.py index 5803b1a875..14c2b29de7 100644 --- a/frappe/core/doctype/rq_worker/test_rq_worker.py +++ b/frappe/core/doctype/rq_worker/test_rq_worker.py @@ -8,10 +8,10 @@ from frappe.tests.utils import FrappeTestCase class TestRQWorker(FrappeTestCase): def test_get_worker_list(self): - workers = RQWorker.get_list({}) + workers = RQWorker.get_list() self.assertGreaterEqual(len(workers), 1) self.assertTrue(any("short" in w.queue_type for w in workers)) def test_worker_serialization(self): - workers = RQWorker.get_list({}) + workers = RQWorker.get_list() frappe.get_doc("RQ Worker", workers[0].name) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 95405fdb6f..598648cc4c 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -51,7 +51,7 @@ def get_count() -> int: if is_virtual_doctype(args.doctype): controller = get_controller(args.doctype) - data = controller.get_count(args) + data = frappe.call(controller.get_count, args=args, **args) else: distinct = "distinct " if args.distinct == "true" else "" args.fields = [f"count({distinct}`tab{args.doctype}`.name) as total_count"] @@ -513,7 +513,7 @@ def get_sidebar_stats(stats, doctype, filters=None): if is_virtual_doctype(doctype): controller = get_controller(doctype) args = {"stats": stats, "filters": filters} - data = controller.get_stats(args) + data = frappe.call(controller.get_stats, args=args, **args) else: data = get_stats(stats, doctype, filters) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 25744079a1..4c0776fe7e 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1075,27 +1075,17 @@ class TestDBQuery(FrappeTestCase): class VirtualDocType: @staticmethod - def get_list(args): - ... + def get_list(args=None, limit_page_length=0, doctype=None): + self.assertEqual(args["filters"], [["Virtual DocType", "name", "=", "test"]]) + self.assertEqual(limit_page_length, 1) + self.assertEqual(doctype, "Virtual DocType") with patch( "frappe.controllers", new={frappe.local.site: {"Virtual DocType": VirtualDocType}}, ): - VirtualDocType.get_list = MagicMock() - frappe.get_all("Virtual DocType", filters={"name": "test"}, fields=["name"], limit=1) - call_args = VirtualDocType.get_list.call_args[0][0] - VirtualDocType.get_list.assert_called_once() - self.assertIsInstance(call_args, dict) - self.assertEqual(call_args["doctype"], "Virtual DocType") - self.assertEqual(call_args["filters"], [["Virtual DocType", "name", "=", "test"]]) - self.assertEqual(call_args["fields"], ["name"]) - self.assertEqual(call_args["limit_page_length"], 1) - self.assertEqual(call_args["limit_start"], 0) - self.assertEqual(call_args["order_by"], DefaultOrderBy) - def test_coalesce_with_in_ops(self): self.assertNotIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", "b"])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", None])}, run=0))