From 26bd3053d190df07e8b75e0e86203050047b25cf Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 4 Feb 2022 17:34:56 +0530 Subject: [PATCH 1/5] perf: Weed out disabled variants via sql query instead of pythonic looping separately - If the number of variants are large (almost 2lakhs), the query to get variants and attribute data takes time - If the no.of disabled attributes is large as well, the list comprehension weeding out disabled variants takes forever - We dont need to loop over the variants data so many times - Avoid any `if a in list(b)` is best when the iterables have tremendous data --- .../variant_selector/item_variants_cache.py | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/erpnext/e_commerce/variant_selector/item_variants_cache.py b/erpnext/e_commerce/variant_selector/item_variants_cache.py index bb6b3ef37f..9b22255d9a 100644 --- a/erpnext/e_commerce/variant_selector/item_variants_cache.py +++ b/erpnext/e_commerce/variant_selector/item_variants_cache.py @@ -66,25 +66,39 @@ class ItemVariantsCacheManager: ) ] - # join with Website Item - item_variants_data = frappe.get_all( - 'Item Variant Attribute', - {'variant_of': parent_item_code}, - ['parent', 'attribute', 'attribute_value'], - order_by='name', - as_list=1 + # Get Variants and tehir Attributes that are not disabled + iva = frappe.qb.DocType("Item Variant Attribute") + item = frappe.qb.DocType("Item") + query = ( + frappe.qb.from_(iva) + .join(item).on(item.name == iva.parent) + .select( + iva.parent, iva.attribute, iva.attribute_value + ).where( + (iva.variant_of == parent_item_code) + & (item.disabled == 0) + ).orderby(iva.name) ) + item_variants_data = query.run() - disabled_items = set( - [i.name for i in frappe.db.get_all('Item', {'disabled': 1})] - ) + # item_variants_data = frappe.get_all( + # 'Item Variant Attribute', + # {'variant_of': parent_item_code}, + # ['parent', 'attribute', 'attribute_value'], + # order_by='name', + # as_list=1 + # ) + + # disabled_items = set( + # [i.name for i in frappe.db.get_all('Item', {'disabled': 1})] + # ) attribute_value_item_map = frappe._dict() item_attribute_value_map = frappe._dict() # dont consider variants that are disabled # pull all other variants - item_variants_data = [r for r in item_variants_data if r[0] not in disabled_items] + # item_variants_data = [r for r in item_variants_data if r[0] not in disabled_items] for row in item_variants_data: item_code, attribute, attribute_value = row From a64228741d065f7ac33b3208d3a704616250f925 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 8 Feb 2022 11:15:19 +0530 Subject: [PATCH 2/5] fix: Trim spaces from attributes (multi-variant creation) & explicit method for building cache - Multiple Item Variants creation fails due to extra spaces in attributes from popup. Clean them before passing to server side - Mention explicit method to build variants cache to avoid ambiguity between old method path (pre-refactor) --- erpnext/e_commerce/variant_selector/item_variants_cache.py | 5 ++++- erpnext/stock/doctype/item/item.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/erpnext/e_commerce/variant_selector/item_variants_cache.py b/erpnext/e_commerce/variant_selector/item_variants_cache.py index 9b22255d9a..3aefc446c2 100644 --- a/erpnext/e_commerce/variant_selector/item_variants_cache.py +++ b/erpnext/e_commerce/variant_selector/item_variants_cache.py @@ -138,4 +138,7 @@ def build_cache(item_code): def enqueue_build_cache(item_code): if frappe.cache().hget('item_cache_build_in_progress', item_code): return - frappe.enqueue(build_cache, item_code=item_code, queue='long') + frappe.enqueue( + "erpnext.e_commerce.variant_selector.item_variants_cache.build_cache", + item_code=item_code, queue='long' + ) diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js index 2a30ca11fb..dfc09181ca 100644 --- a/erpnext/stock/doctype/item/item.js +++ b/erpnext/stock/doctype/item/item.js @@ -545,7 +545,7 @@ $.extend(erpnext.item, { let selected_attributes = {}; me.multiple_variant_dialog.$wrapper.find('.form-column').each((i, col) => { if(i===0) return; - let attribute_name = $(col).find('label').html(); + let attribute_name = $(col).find('label').html().trim(); selected_attributes[attribute_name] = []; let checked_opts = $(col).find('.checkbox input'); checked_opts.each((i, opt) => { From 4f5a0b8941101f759f2d1af33d952a1bfdfd3cf4 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 8 Feb 2022 12:02:02 +0530 Subject: [PATCH 3/5] chore: Fix flaky test `test_exact_match_with_price` - Clear cart settings in cache to avoid stale values --- erpnext/e_commerce/variant_selector/test_variant_selector.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/e_commerce/variant_selector/test_variant_selector.py b/erpnext/e_commerce/variant_selector/test_variant_selector.py index b83961e6e1..4d907c6221 100644 --- a/erpnext/e_commerce/variant_selector/test_variant_selector.py +++ b/erpnext/e_commerce/variant_selector/test_variant_selector.py @@ -104,6 +104,8 @@ class TestVariantSelector(ERPNextTestCase): }) make_web_item_price(item_code="Test-Tshirt-Temp-S-R", price_list_rate=100) + + frappe.local.shopping_cart_settings = None # clear cached settings values next_values = get_next_attribute_and_values( "Test-Tshirt-Temp", selected_attributes={"Test Size": "Small", "Test Colour": "Red"} From 29c576e144489072c992e9b5bdfe4c9359639ef8 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 16 Feb 2022 12:41:39 +0530 Subject: [PATCH 4/5] chore: Remove commented out code --- .../variant_selector/item_variants_cache.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/erpnext/e_commerce/variant_selector/item_variants_cache.py b/erpnext/e_commerce/variant_selector/item_variants_cache.py index 3aefc446c2..3107c019e6 100644 --- a/erpnext/e_commerce/variant_selector/item_variants_cache.py +++ b/erpnext/e_commerce/variant_selector/item_variants_cache.py @@ -81,25 +81,9 @@ class ItemVariantsCacheManager: ) item_variants_data = query.run() - # item_variants_data = frappe.get_all( - # 'Item Variant Attribute', - # {'variant_of': parent_item_code}, - # ['parent', 'attribute', 'attribute_value'], - # order_by='name', - # as_list=1 - # ) - - # disabled_items = set( - # [i.name for i in frappe.db.get_all('Item', {'disabled': 1})] - # ) - attribute_value_item_map = frappe._dict() item_attribute_value_map = frappe._dict() - # dont consider variants that are disabled - # pull all other variants - # item_variants_data = [r for r in item_variants_data if r[0] not in disabled_items] - for row in item_variants_data: item_code, attribute, attribute_value = row # (attr, value) => [item1, item2] From a26183e205effa11d1fae7a3d6cb96c7db100e07 Mon Sep 17 00:00:00 2001 From: Kenneth Sequeira <33246109+kennethsequeira@users.noreply.github.com> Date: Wed, 16 Feb 2022 13:02:36 +0530 Subject: [PATCH 5/5] fix: add supported currencies (#29805) --- .../doctype/gocardless_settings/gocardless_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/erpnext_integrations/doctype/gocardless_settings/gocardless_settings.py b/erpnext/erpnext_integrations/doctype/gocardless_settings/gocardless_settings.py index a8119ac86c..f02f76e18b 100644 --- a/erpnext/erpnext_integrations/doctype/gocardless_settings/gocardless_settings.py +++ b/erpnext/erpnext_integrations/doctype/gocardless_settings/gocardless_settings.py @@ -13,7 +13,7 @@ from frappe.utils import call_hook_method, cint, flt, get_url class GoCardlessSettings(Document): - supported_currencies = ["EUR", "DKK", "GBP", "SEK"] + supported_currencies = ["EUR", "DKK", "GBP", "SEK", "AUD", "NZD", "CAD", "USD"] def validate(self): self.initialize_client() @@ -80,7 +80,7 @@ class GoCardlessSettings(Document): def validate_transaction_currency(self, currency): if currency not in self.supported_currencies: - frappe.throw(_("Please select another payment method. Stripe does not support transactions in currency '{0}'").format(currency)) + frappe.throw(_("Please select another payment method. Go Cardless does not support transactions in currency '{0}'").format(currency)) def get_payment_url(self, **kwargs): return get_url("./integrations/gocardless_checkout?{0}".format(urlencode(kwargs)))