From 25718f5cc758a30b6641e7c9be12f4386dc81a28 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sun, 1 Oct 2023 13:27:44 +0530 Subject: [PATCH 1/2] chore: rewrite query using query builder --- .../crm/report/lead_details/lead_details.py | 77 ++++++++----------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/erpnext/crm/report/lead_details/lead_details.py b/erpnext/crm/report/lead_details/lead_details.py index 7b8c43b2d6..98dfbec18b 100644 --- a/erpnext/crm/report/lead_details/lead_details.py +++ b/erpnext/crm/report/lead_details/lead_details.py @@ -4,6 +4,7 @@ import frappe from frappe import _ +from frappe.query_builder.functions import Concat_ws, Date def execute(filters=None): @@ -69,53 +70,41 @@ def get_columns(): def get_data(filters): - return frappe.db.sql( - """ - SELECT - `tabLead`.name, - `tabLead`.lead_name, - `tabLead`.status, - `tabLead`.lead_owner, - `tabLead`.territory, - `tabLead`.source, - `tabLead`.email_id, - `tabLead`.mobile_no, - `tabLead`.phone, - `tabLead`.owner, - `tabLead`.company, - concat_ws(', ', - trim(',' from `tabAddress`.address_line1), - trim(',' from tabAddress.address_line2) - ) AS address, - `tabAddress`.state, - `tabAddress`.pincode, - `tabAddress`.country - FROM - `tabLead` left join `tabDynamic Link` on ( - `tabLead`.name = `tabDynamic Link`.link_name and - `tabDynamic Link`.parenttype = 'Address') - left join `tabAddress` on ( - `tabAddress`.name=`tabDynamic Link`.parent) - WHERE - company = %(company)s - AND DATE(`tabLead`.creation) BETWEEN %(from_date)s AND %(to_date)s - {conditions} - ORDER BY - `tabLead`.creation asc """.format( - conditions=get_conditions(filters) - ), - filters, - as_dict=1, + lead = frappe.qb.DocType("Lead") + address = frappe.qb.DocType("Address") + dynamic_link = frappe.qb.DocType("Dynamic Link") + + query = ( + frappe.qb.from_(lead) + .left_join(dynamic_link) + .on((lead.name == dynamic_link.link_name) & (dynamic_link.parenttype == "Address")) + .left_join(address) + .on(address.name == dynamic_link.parent) + .select( + lead.name, + lead.lead_name, + lead.status, + lead.lead_owner, + lead.territory, + lead.source, + lead.email_id, + lead.mobile_no, + lead.phone, + lead.owner, + lead.company, + (Concat_ws(", ", address.address_line1, address.address_line2)).as_("address"), + address.state, + address.pincode, + address.country, + ) + .where(lead.company == filters.company) + .where(Date(lead.creation).between(filters.from_date, filters.to_date)) ) - -def get_conditions(filters): - conditions = [] - if filters.get("territory"): - conditions.append(" and `tabLead`.territory=%(territory)s") + query = query.where(lead.territory == filters.get("territory")) if filters.get("status"): - conditions.append(" and `tabLead`.status=%(status)s") + query = query.where(lead.status == filters.get("status")) - return " ".join(conditions) if conditions else "" + return query.run(as_dict=1) From fb51cae88b20274a836165a9b7aa70fe1a436ff4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 2 Oct 2023 16:16:31 +0530 Subject: [PATCH 2/2] chore: fix shopping cart tests --- .../shopping_cart/test_shopping_cart.py | 103 +++++++++++++----- 1 file changed, 77 insertions(+), 26 deletions(-) diff --git a/erpnext/e_commerce/shopping_cart/test_shopping_cart.py b/erpnext/e_commerce/shopping_cart/test_shopping_cart.py index 951039db4f..8210f9743d 100644 --- a/erpnext/e_commerce/shopping_cart/test_shopping_cart.py +++ b/erpnext/e_commerce/shopping_cart/test_shopping_cart.py @@ -17,7 +17,6 @@ from erpnext.e_commerce.shopping_cart.cart import ( request_for_quotation, update_cart, ) -from erpnext.tests.utils import create_test_contact_and_address class TestShoppingCart(unittest.TestCase): @@ -28,7 +27,6 @@ class TestShoppingCart(unittest.TestCase): def setUp(self): frappe.set_user("Administrator") - create_test_contact_and_address() self.enable_shopping_cart() if not frappe.db.exists("Website Item", {"item_code": "_Test Item"}): make_website_item(frappe.get_cached_doc("Item", "_Test Item")) @@ -46,48 +44,57 @@ class TestShoppingCart(unittest.TestCase): frappe.db.sql("delete from `tabTax Rule`") def test_get_cart_new_user(self): - self.login_as_new_user() - + self.login_as_customer( + "test_contact_two_customer@example.com", "_Test Contact 2 For _Test Customer" + ) + create_address_and_contact( + address_title="_Test Address for Customer 2", + first_name="_Test Contact for Customer 2", + email="test_contact_two_customer@example.com", + customer="_Test Customer 2", + ) # test if lead is created and quotation with new lead is fetched - quotation = _get_cart_quotation() + customer = frappe.get_doc("Customer", "_Test Customer 2") + quotation = _get_cart_quotation(party=customer) self.assertEqual(quotation.quotation_to, "Customer") self.assertEqual( quotation.contact_person, - frappe.db.get_value("Contact", dict(email_id="test_cart_user@example.com")), + frappe.db.get_value("Contact", dict(email_id="test_contact_two_customer@example.com")), ) self.assertEqual(quotation.contact_email, frappe.session.user) return quotation - def test_get_cart_customer(self): - def validate_quotation(): + def test_get_cart_customer(self, customer="_Test Customer 2"): + def validate_quotation(customer_name): # test if quotation with customer is fetched - quotation = _get_cart_quotation() + party = frappe.get_doc("Customer", customer_name) + quotation = _get_cart_quotation(party=party) self.assertEqual(quotation.quotation_to, "Customer") - self.assertEqual(quotation.party_name, "_Test Customer") + self.assertEqual(quotation.party_name, customer_name) self.assertEqual(quotation.contact_email, frappe.session.user) return quotation - self.login_as_customer( - "test_contact_two_customer@example.com", "_Test Contact 2 For _Test Customer" - ) - validate_quotation() - - self.login_as_customer() - quotation = validate_quotation() - + quotation = validate_quotation(customer) return quotation def test_add_to_cart(self): - self.login_as_customer() - + self.login_as_customer( + "test_contact_two_customer@example.com", "_Test Contact 2 For _Test Customer" + ) + create_address_and_contact( + address_title="_Test Address for Customer 2", + first_name="_Test Contact for Customer 2", + email="test_contact_two_customer@example.com", + customer="_Test Customer 2", + ) # clear existing quotations self.clear_existing_quotations() # add first item update_cart("_Test Item", 1) - quotation = self.test_get_cart_customer() + quotation = self.test_get_cart_customer("_Test Customer 2") self.assertEqual(quotation.get("items")[0].item_code, "_Test Item") self.assertEqual(quotation.get("items")[0].qty, 1) @@ -95,7 +102,7 @@ class TestShoppingCart(unittest.TestCase): # add second item update_cart("_Test Item 2", 1) - quotation = self.test_get_cart_customer() + quotation = self.test_get_cart_customer("_Test Customer 2") self.assertEqual(quotation.get("items")[1].item_code, "_Test Item 2") self.assertEqual(quotation.get("items")[1].qty, 1) self.assertEqual(quotation.get("items")[1].amount, 20) @@ -108,7 +115,7 @@ class TestShoppingCart(unittest.TestCase): # update first item update_cart("_Test Item", 5) - quotation = self.test_get_cart_customer() + quotation = self.test_get_cart_customer("_Test Customer 2") self.assertEqual(quotation.get("items")[0].item_code, "_Test Item") self.assertEqual(quotation.get("items")[0].qty, 5) self.assertEqual(quotation.get("items")[0].amount, 50) @@ -121,7 +128,7 @@ class TestShoppingCart(unittest.TestCase): # remove first item update_cart("_Test Item", 0) - quotation = self.test_get_cart_customer() + quotation = self.test_get_cart_customer("_Test Customer 2") self.assertEqual(quotation.get("items")[0].item_code, "_Test Item 2") self.assertEqual(quotation.get("items")[0].qty, 1) @@ -132,7 +139,17 @@ class TestShoppingCart(unittest.TestCase): @unittest.skip("Flaky in CI") def test_tax_rule(self): self.create_tax_rule() - self.login_as_customer() + + self.login_as_customer( + "test_contact_two_customer@example.com", "_Test Contact 2 For _Test Customer" + ) + create_address_and_contact( + address_title="_Test Address for Customer 2", + first_name="_Test Contact for Customer 2", + email="test_contact_two_customer@example.com", + customer="_Test Customer 2", + ) + quotation = self.create_quotation() from erpnext.accounts.party import set_taxes @@ -320,7 +337,7 @@ class TestShoppingCart(unittest.TestCase): if frappe.db.exists("User", email): return - frappe.get_doc( + user = frappe.get_doc( { "doctype": "User", "user_type": "Website User", @@ -330,6 +347,40 @@ class TestShoppingCart(unittest.TestCase): } ).insert(ignore_permissions=True) + user.add_roles("Customer") + + +def create_address_and_contact(**kwargs): + if not frappe.db.get_value("Address", {"address_title": kwargs.get("address_title")}): + frappe.get_doc( + { + "doctype": "Address", + "address_title": kwargs.get("address_title"), + "address_type": kwargs.get("address_type") or "Office", + "address_line1": kwargs.get("address_line1") or "Station Road", + "city": kwargs.get("city") or "_Test City", + "state": kwargs.get("state") or "Test State", + "country": kwargs.get("country") or "India", + "links": [ + {"link_doctype": "Customer", "link_name": kwargs.get("customer") or "_Test Customer"} + ], + } + ).insert() + + if not frappe.db.get_value("Contact", {"first_name": kwargs.get("first_name")}): + contact = frappe.get_doc( + { + "doctype": "Contact", + "first_name": kwargs.get("first_name"), + "links": [ + {"link_doctype": "Customer", "link_name": kwargs.get("customer") or "_Test Customer"} + ], + } + ) + contact.add_email(kwargs.get("email") or "test_contact_customer@example.com", is_primary=True) + contact.add_phone(kwargs.get("phone") or "+91 0000000000", is_primary_phone=True) + contact.insert() + test_dependencies = [ "Sales Taxes and Charges Template",