From d802d7397313a857fca97c8f7f7a190a11fa5e5f Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Jun 2021 14:08:07 +0530 Subject: [PATCH 1/5] fix: Consider Website Item Groups in Item group page product listing - Passed an argument to query engine to know when query is for item group page - If for item group page, get data with regards to website item group table - This query should be fast since there's one filter and that shortens the table beforehand - This data is merged with the results from the Item master (results only considering item attributes and field filters) - The combined data is then sorted as per weightage Co-authored-by: Gavin D'souza --- .../setup/doctype/item_group/item_group.py | 2 +- erpnext/shopping_cart/product_query.py | 32 ++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py index db31d6d699..1c72cebfa9 100644 --- a/erpnext/setup/doctype/item_group/item_group.py +++ b/erpnext/setup/doctype/item_group/item_group.py @@ -91,7 +91,7 @@ class ItemGroup(NestedSet, WebsiteGenerator): field_filters['item_group'] = self.name engine = ProductQuery() - context.items = engine.query(attribute_filters, field_filters, search, start) + context.items = engine.query(attribute_filters, field_filters, search, start, item_group=self.name) filter_engine = ProductFiltersBuilder(self.name) diff --git a/erpnext/shopping_cart/product_query.py b/erpnext/shopping_cart/product_query.py index dd94c26bc6..bb31220447 100644 --- a/erpnext/shopping_cart/product_query.py +++ b/erpnext/shopping_cart/product_query.py @@ -22,13 +22,14 @@ class ProductQuery: self.settings = frappe.get_doc("Products Settings") self.cart_settings = frappe.get_doc("Shopping Cart Settings") self.page_length = self.settings.products_per_page or 20 - self.fields = ['name', 'item_name', 'item_code', 'website_image', 'variant_of', 'has_variants', 'item_group', 'image', 'web_long_description', 'description', 'route'] + self.fields = ['name', 'item_name', 'item_code', 'website_image', 'variant_of', 'has_variants', + 'item_group', 'image', 'web_long_description', 'description', 'route', 'weightage'] self.filters = [] self.or_filters = [['show_in_website', '=', 1]] if not self.settings.get('hide_variants'): self.or_filters.append(['show_variant_in_website', '=', 1]) - def query(self, attributes=None, fields=None, search_term=None, start=0): + def query(self, attributes=None, fields=None, search_term=None, start=0, item_group=None): """Summary Args: @@ -44,6 +45,15 @@ class ProductQuery: if search_term: self.build_search_filters(search_term) result = [] + website_item_groups = [] + + # if from item group page consider website item group table + if item_group: + website_item_groups = frappe.db.get_all( + "Item", + fields=self.fields + ["`tabWebsite Item Group`.parent as wig_parent"], + filters=[["Website Item Group", "item_group", "=", item_group]] + ) if attributes: all_items = [] @@ -61,12 +71,10 @@ class ProductQuery: ], or_filters=self.or_filters, start=start, - limit=self.page_length, - order_by="weightage desc" + limit=self.page_length ) items_dict = {item.name: item for item in items} - # TODO: Replace Variants by their parent templates all_items.append(set(items_dict.keys())) @@ -78,14 +86,22 @@ class ProductQuery: filters=self.filters, or_filters=self.or_filters, start=start, - limit=self.page_length, - order_by="weightage desc" + limit=self.page_length ) + # Combine results having context of website item groups into item results + if item_group and website_item_groups: + items_list = {row.name for row in result} + for row in website_item_groups: + if row.wig_parent not in items_list: + result.append(row) + + result = sorted(result, key=lambda x: x.get("weightage"), reverse=True) + for item in result: product_info = get_product_info_for_website(item.item_code, skip_quotation_creation=True).get('product_info') if product_info: - item.formatted_price = product_info['price'].get('formatted_price') if product_info['price'] else None + item.formatted_price = product_info.get('price', {}).get('formatted_price') return result From 1b9b72d12eac988f03b1feda17f6524c96ab5b72 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Jun 2021 16:03:24 +0530 Subject: [PATCH 2/5] fix: Filters did not consider Website Item Group --- erpnext/shopping_cart/filters.py | 21 +++++++++++++++------ erpnext/shopping_cart/product_query.py | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/erpnext/shopping_cart/filters.py b/erpnext/shopping_cart/filters.py index 6c63d8759b..979afd3c13 100644 --- a/erpnext/shopping_cart/filters.py +++ b/erpnext/shopping_cart/filters.py @@ -22,12 +22,15 @@ class ProductFiltersBuilder: filter_data = [] for df in fields: - filters = {} + filters, or_filters = {}, [] if df.fieldtype == "Link": if self.item_group: - filters['item_group'] = self.item_group + or_filters.extend([ + ["item_group", "=", self.item_group], + ["Website Item Group", "item_group", "=", self.item_group] + ]) - values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, distinct="True", pluck=df.fieldname) + values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, or_filters=or_filters, distinct="True", pluck=df.fieldname, debug=1) else: doctype = df.get_link_doctype() @@ -44,7 +47,9 @@ class ProductFiltersBuilder: values = [d.name for d in frappe.get_all(doctype, filters)] # Remove None - values = values.remove(None) if None in values else values + if None in values: + values.remove(None) + if values: filter_data.append([df, values]) @@ -61,14 +66,18 @@ class ProductFiltersBuilder: for attr_doc in attribute_docs: selected_attributes = [] for attr in attr_doc.item_attribute_values: + or_filters = [] filters= [ ["Item Variant Attribute", "attribute", "=", attr.parent], ["Item Variant Attribute", "attribute_value", "=", attr.attribute_value] ] if self.item_group: - filters.append(["item_group", "=", self.item_group]) + or_filters.extend([ + ["item_group", "=", self.item_group], + ["Website Item Group", "item_group", "=", self.item_group] + ]) - if frappe.db.get_all("Item", filters, limit=1): + if frappe.db.get_all("Item", filters, or_filters=or_filters, limit=1): selected_attributes.append(attr) if selected_attributes: diff --git a/erpnext/shopping_cart/product_query.py b/erpnext/shopping_cart/product_query.py index bb31220447..0b05f68ae9 100644 --- a/erpnext/shopping_cart/product_query.py +++ b/erpnext/shopping_cart/product_query.py @@ -101,7 +101,7 @@ class ProductQuery: for item in result: product_info = get_product_info_for_website(item.item_code, skip_quotation_creation=True).get('product_info') if product_info: - item.formatted_price = product_info.get('price', {}).get('formatted_price') + item.formatted_price = (product_info.get('price') or {}).get('formatted_price') return result From f913e0dd05b41921d8ad8c6f0fa1620bb1adc545 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Jun 2021 20:06:11 +0530 Subject: [PATCH 3/5] fix: Consider Table Multiselect fields in Query engine - Since table multiselect fields were not handled, the query tried searching for this child field in item master - This broke the query - On trying to reload or go back to all-products page with field filters that are table mutiselect, page breaks --- erpnext/shopping_cart/product_query.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/erpnext/shopping_cart/product_query.py b/erpnext/shopping_cart/product_query.py index 0b05f68ae9..cd4a176921 100644 --- a/erpnext/shopping_cart/product_query.py +++ b/erpnext/shopping_cart/product_query.py @@ -115,6 +115,17 @@ class ProductQuery: if not values: continue + # handle multiselect fields in filter addition + meta = frappe.get_meta('Item', cached=True) + df = meta.get_field(field) + if df.fieldtype == 'Table MultiSelect': + child_doctype = df.options + child_meta = frappe.get_meta(child_doctype, cached=True) + fields = child_meta.get("fields", { "fieldtype": "Link", "in_list_view": 1 }) + if fields: + self.filters.append([child_doctype, fields[0].fieldname, 'IN', values]) + continue + if isinstance(values, list): # If value is a list use `IN` query self.filters.append([field, 'IN', values]) From 078826d510aef7d1a1a0e3bce63c451f1d47e727 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Jun 2021 20:12:59 +0530 Subject: [PATCH 4/5] fix: Sider --- erpnext/shopping_cart/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/shopping_cart/filters.py b/erpnext/shopping_cart/filters.py index 979afd3c13..9f06d20bde 100644 --- a/erpnext/shopping_cart/filters.py +++ b/erpnext/shopping_cart/filters.py @@ -30,7 +30,7 @@ class ProductFiltersBuilder: ["Website Item Group", "item_group", "=", self.item_group] ]) - values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, or_filters=or_filters, distinct="True", pluck=df.fieldname, debug=1) + values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, or_filters=or_filters, distinct="True", pluck=df.fieldname, debug=1) else: doctype = df.get_link_doctype() From a4f5dcaa9ab610c10de24a1bc0c835cd615d6a09 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Jun 2021 22:38:10 +0530 Subject: [PATCH 5/5] chore: Test for Item visibility in multiple item group pages --- .../test_product_configurator.py | 63 +++++++++++++++++++ erpnext/shopping_cart/filters.py | 2 +- erpnext/shopping_cart/product_query.py | 6 +- 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/erpnext/portal/product_configurator/test_product_configurator.py b/erpnext/portal/product_configurator/test_product_configurator.py index 3521e7e8bf..daaba67173 100644 --- a/erpnext/portal/product_configurator/test_product_configurator.py +++ b/erpnext/portal/product_configurator/test_product_configurator.py @@ -43,6 +43,30 @@ class TestProductConfigurator(unittest.TestCase): "show_variant_in_website": 1 }).insert() + def create_regular_web_item(self, name, item_group=None): + if not frappe.db.exists('Item', name): + doc = frappe.get_doc({ + "description": name, + "item_code": name, + "item_name": name, + "doctype": "Item", + "is_stock_item": 1, + "item_group": item_group or "_Test Item Group", + "stock_uom": "_Test UOM", + "item_defaults": [{ + "company": "_Test Company", + "default_warehouse": "_Test Warehouse - _TC", + "expense_account": "_Test Account Cost for Goods Sold - _TC", + "buying_cost_center": "_Test Cost Center - _TC", + "selling_cost_center": "_Test Cost Center - _TC", + "income_account": "Sales - _TC" + }], + "show_in_website": 1 + }).insert() + else: + doc = frappe.get_doc("Item", name) + return doc + def test_product_list(self): template_items = frappe.get_all('Item', {'show_in_website': 1}) variant_items = frappe.get_all('Item', {'show_variant_in_website': 1}) @@ -79,3 +103,42 @@ class TestProductConfigurator(unittest.TestCase): 'Test Size': ['2XL'] }) self.assertEqual(len(items), 1) + + def test_products_in_multiple_item_groups(self): + """Check if product is visible on multiple item group pages barring its own.""" + from erpnext.shopping_cart.product_query import ProductQuery + + if not frappe.db.exists("Item Group", {"name": "Tech Items"}): + item_group_doc = frappe.get_doc({ + "doctype": "Item Group", + "item_group_name": "Tech Items", + "parent_item_group": "All Item Groups", + "show_in_website": 1 + }).insert() + else: + item_group_doc = frappe.get_doc("Item Group", "Tech Items") + + doc = self.create_regular_web_item("Portal Item", item_group="Tech Items") + if not frappe.db.exists("Website Item Group", {"parent": "Portal Item"}): + doc.append("website_item_groups", { + "item_group": "_Test Item Group Desktops" + }) + doc.save() + + # check if item is visible in its own Item Group's page + engine = ProductQuery() + items = engine.query({}, {"item_group": "Tech Items"}, None, start=0, item_group="Tech Items") + self.assertEqual(len(items), 1) + self.assertEqual(items[0].item_code, "Portal Item") + + # check if item is visible in configured foreign Item Group's page + engine = ProductQuery() + items = engine.query({}, {"item_group": "_Test Item Group Desktops"}, None, start=0, item_group="_Test Item Group Desktops") + item_codes = [row.item_code for row in items] + + self.assertIn(len(items), [2, 3]) + self.assertIn("Portal Item", item_codes) + + # teardown + doc.delete() + item_group_doc.delete() \ No newline at end of file diff --git a/erpnext/shopping_cart/filters.py b/erpnext/shopping_cart/filters.py index 9f06d20bde..7dfa09e2d6 100644 --- a/erpnext/shopping_cart/filters.py +++ b/erpnext/shopping_cart/filters.py @@ -30,7 +30,7 @@ class ProductFiltersBuilder: ["Website Item Group", "item_group", "=", self.item_group] ]) - values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, or_filters=or_filters, distinct="True", pluck=df.fieldname, debug=1) + values = frappe.get_all("Item", fields=[df.fieldname], filters=filters, or_filters=or_filters, distinct="True", pluck=df.fieldname) else: doctype = df.get_link_doctype() diff --git a/erpnext/shopping_cart/product_query.py b/erpnext/shopping_cart/product_query.py index cd4a176921..d96d803416 100644 --- a/erpnext/shopping_cart/product_query.py +++ b/erpnext/shopping_cart/product_query.py @@ -121,12 +121,10 @@ class ProductQuery: if df.fieldtype == 'Table MultiSelect': child_doctype = df.options child_meta = frappe.get_meta(child_doctype, cached=True) - fields = child_meta.get("fields", { "fieldtype": "Link", "in_list_view": 1 }) + fields = child_meta.get("fields") if fields: self.filters.append([child_doctype, fields[0].fieldname, 'IN', values]) - continue - - if isinstance(values, list): + elif isinstance(values, list): # If value is a list use `IN` query self.filters.append([field, 'IN', values]) else: