From 1701125fca1eb45581301adfad69d7d0505a6f9a Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 29 Jun 2021 11:22:27 +0530 Subject: [PATCH] chore: Cleanup Query Engine and Product query API - Resolved merge conflicts in item_group.py - Separate api.py file for product listing backend api - Brought back ORM in query engine, handled missing cases (website item groups, etc) - Return results from API in a descriptive manner, helps keep sanity in JS - On toggling views store view preference in localStorage --- erpnext/e_commerce/api.py | 49 ++++++++ .../doctype/website_item/website_item.py | 49 +------- .../test_product_configurator.py | 7 +- erpnext/e_commerce/product_query.py | 107 ++++++++++-------- erpnext/e_commerce/product_view.js | 23 ++-- .../setup/doctype/item_group/item_group.py | 20 ---- erpnext/www/all-products/index.js | 2 +- 7 files changed, 135 insertions(+), 122 deletions(-) create mode 100644 erpnext/e_commerce/api.py diff --git a/erpnext/e_commerce/api.py b/erpnext/e_commerce/api.py new file mode 100644 index 00000000000..081e8a95a68 --- /dev/null +++ b/erpnext/e_commerce/api.py @@ -0,0 +1,49 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from frappe.utils import cint + +from erpnext.e_commerce.product_query import ProductQuery +from erpnext.e_commerce.filters import ProductFiltersBuilder +from erpnext.setup.doctype.item_group.item_group import get_child_groups + +@frappe.whitelist(allow_guest=True) +def get_product_filter_data(): + """Get pre-rendered filtered products and discount filters on load.""" + if frappe.form_dict: + search = frappe.form_dict.search + field_filters = frappe.parse_json(frappe.form_dict.field_filters) + attribute_filters = frappe.parse_json(frappe.form_dict.attribute_filters) + start = cint(frappe.parse_json(frappe.form_dict.start)) if frappe.form_dict.start else 0 + item_group = frappe.form_dict.item_group + else: + search, attribute_filters, item_group = None, None, None + field_filters = {} + start = 0 + + sub_categories = [] + if item_group: + field_filters['item_group'] = item_group + sub_categories = get_child_groups(item_group) + + engine = ProductQuery() + result = engine.query(attribute_filters, field_filters, search_term=search, + start=start, item_group=item_group) + + # discount filter data + filters = {} + discounts = result["discounts"] + + if discounts: + filter_engine = ProductFiltersBuilder() + filters["discount_filters"] = filter_engine.get_discount_filters(discounts) + + return { + "items": result["items"] or [], + "filters": filters, + "settings": engine.settings, + "sub_categories": sub_categories, + "items_count": result["items_count"] + } \ No newline at end of file diff --git a/erpnext/e_commerce/doctype/website_item/website_item.py b/erpnext/e_commerce/doctype/website_item/website_item.py index 5a0363c2876..c13704f6479 100644 --- a/erpnext/e_commerce/doctype/website_item/website_item.py +++ b/erpnext/e_commerce/doctype/website_item/website_item.py @@ -2,11 +2,9 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -from __future__ import unicode_literals import frappe import json import itertools -from six import string_types from frappe import _ from frappe.website.website_generator import WebsiteGenerator @@ -16,13 +14,12 @@ from frappe.website.doctype.website_slideshow.website_slideshow import get_slide from erpnext.setup.doctype.item_group.item_group import (get_parent_item_groups, invalidate_cache_for) from erpnext.e_commerce.doctype.item_review.item_review import get_item_reviews -# SEARCH +# SEARCH from erpnext.e_commerce.website_item_indexing import ( - insert_item_to_index, - update_index_for_item, + insert_item_to_index, + update_index_for_item, delete_item_from_index ) -# ----- class WebsiteItem(WebsiteGenerator): website = frappe._dict( @@ -398,7 +395,7 @@ def make_website_item(doc, save=True): if not doc: return - if isinstance(doc, string_types): + if isinstance(doc, str): doc = json.loads(doc) if frappe.db.exists("Website Item", {"item_code": doc.get("item_code")}): @@ -420,7 +417,7 @@ def make_website_item(doc, save=True): # Add to search cache insert_item_to_index(website_item) - + return [website_item.name, website_item.web_item_name] def on_doctype_update(): @@ -443,38 +440,4 @@ def check_if_user_is_customer(user=None): customer = link.link_name break - return True if customer else False - -@frappe.whitelist(allow_guest=True) -def get_product_filter_data(): - """Get pre-rendered filtered products and discount filters on load.""" - from erpnext.e_commerce.product_query import ProductQuery - from erpnext.e_commerce.filters import ProductFiltersBuilder - from erpnext.setup.doctype.item_group.item_group import get_child_groups - - if frappe.form_dict: - search = frappe.form_dict.search - field_filters = frappe.parse_json(frappe.form_dict.field_filters) - attribute_filters = frappe.parse_json(frappe.form_dict.attribute_filters) - start = cint(frappe.parse_json(frappe.form_dict.start)) if frappe.form_dict.start else 0 - item_group = frappe.form_dict.item_group - else: - search, attribute_filters, item_group = None, None, None - field_filters = {} - start = 0 - - sub_categories = [] - if item_group: - field_filters['item_group'] = item_group - sub_categories = get_child_groups(item_group) - - engine = ProductQuery() - items, discounts = engine.query(attribute_filters, field_filters, search_term=search, start=start) - - # discount filter data - filters = {} - if discounts: - filter_engine = ProductFiltersBuilder() - filters["discount_filters"] = filter_engine.get_discount_filters(discounts) - - return items or [], filters, engine.settings, sub_categories \ No newline at end of file + return True if customer else False \ No newline at end of file diff --git a/erpnext/e_commerce/product_configurator/test_product_configurator.py b/erpnext/e_commerce/product_configurator/test_product_configurator.py index daaba671736..34ca9d96ef3 100644 --- a/erpnext/e_commerce/product_configurator/test_product_configurator.py +++ b/erpnext/e_commerce/product_configurator/test_product_configurator.py @@ -127,13 +127,16 @@ class TestProductConfigurator(unittest.TestCase): # 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") + result = engine.query({}, {"item_group": "Tech Items"}, None, start=0, item_group="Tech Items") + items = result["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") + result = engine.query({}, {"item_group": "_Test Item Group Desktops"}, None, start=0, item_group="_Test Item Group Desktops") + items = result["items"] item_codes = [row.item_code for row in items] self.assertIn(len(items), [2, 3]) diff --git a/erpnext/e_commerce/product_query.py b/erpnext/e_commerce/product_query.py index c7ce72a39b9..3c03f7e4065 100644 --- a/erpnext/e_commerce/product_query.py +++ b/erpnext/e_commerce/product_query.py @@ -15,16 +15,14 @@ class ProductQuery: page_length (Int): Length of page for the query settings (Document): E Commerce Settings DocType """ - def __init__(self): self.settings = frappe.get_doc("E Commerce Settings") self.page_length = self.settings.products_per_page or 20 - self.fields = ['wi.web_item_name', 'wi.name', 'wi.item_name', 'wi.item_code', 'wi.website_image', 'wi.variant_of', - 'wi.has_variants', 'wi.item_group', 'wi.image', 'wi.web_long_description', 'wi.description', - 'wi.route', 'wi.website_warehouse', 'wi.ranking'] - self.conditions = "" - self.or_conditions = "" - self.substitutions = [] + self.fields = ['web_item_name', 'name', 'item_name', 'item_code', 'website_image', + 'variant_of', 'has_variants', 'item_group', 'image', 'web_long_description', + 'description', 'route', 'website_warehouse', 'ranking'] + self.filters = [["published", "=", 1]] + self.or_filters = [] def query(self, attributes=None, fields=None, search_term=None, start=0, item_group=None): """Summary @@ -44,7 +42,7 @@ class ProductQuery: # if from item group page consider website item group table if item_group: website_item_groups = frappe.db.get_all( - "Item", + "Website Item", fields=self.fields + ["`tabWebsite Item Group`.parent as wig_parent"], filters=[["Website Item Group", "item_group", "=", item_group]] ) @@ -54,13 +52,13 @@ class ProductQuery: if search_term: self.build_search_filters(search_term) if self.settings.hide_variants: - self.conditions += " and wi.variant_of IS NULL" + self.filters.append(["variant_of", "is", "not set"]) + count = 0 if attributes: - result = self.query_items_with_attributes(attributes, start) + result, count = self.query_items_with_attributes(attributes, start) else: - result = self.query_items(self.conditions, self.or_conditions, - self.substitutions, start=start) + result, count = self.query_items(start=start) # Combine results having context of website item groups into item results if item_group and website_item_groups: @@ -93,7 +91,11 @@ class ProductQuery: discount_percent = frappe.utils.flt(fields["discount"][0]) result = [row for row in result if row.get("discount_percent") and row.discount_percent >= discount_percent] - return result, discounts + return { + "items": result, + "items_count": count, + "discounts": discounts + } def get_price_discount_info(self, item, price_object, discount_list): """Modify item object and add price details.""" @@ -119,50 +121,52 @@ class ProductQuery: elif not frappe.db.get_value("Item", item.item_code, "is_stock_item"): item.in_stock = "green" # non-stock item will always be available - def query_items(self, conditions, or_conditions, substitutions, start=0, with_attributes=False): + def query_items(self, start=0): """Build a query to fetch Website Items based on field filters.""" - self.query_fields = ", ".join(self.fields) + count = frappe.db.get_all( + "Website Item", + fields=["count(*) as count"], + filters=self.filters, + or_filters=self.or_filters, + limit_start=start)[0].get("count") - attribute_table = ", `tabItem Variant Attribute` iva" if with_attributes else "" + items = frappe.db.get_all( + "Website Item", + fields=self.fields, + filters=self.filters, + or_filters=self.or_filters, + limit_page_length=self.page_length, + limit_start=start) - return frappe.db.sql(f""" - select distinct {self.query_fields} - from - `tabWebsite Item` wi {attribute_table} - where - wi.published = 1 - {conditions} - {or_conditions} - limit {self.page_length} offset {start} - """, - tuple(substitutions), - as_dict=1) + return items, count or 0 def query_items_with_attributes(self, attributes, start=0): """Build a query to fetch Website Items based on field & attribute filters.""" all_items = [] - self.conditions += " and iva.parent = wi.item_code" + item_codes = [] for attribute, values in attributes.items(): if not isinstance(values, list): values = [values] - conditions_copy = self.conditions - substitutions_copy = self.substitutions.copy() + # get items that have selected attribute & value + item_code_list = frappe.db.get_all( + "Item", + fields=["item_code"], + filters=[ + ["published_in_website", "=", 1], + ["Item Variant Attribute", "attribute", "=", attribute], + ["Item Variant Attribute", "attribute_value", "in", values] + ]) + item_codes.append({x.item_code for x in item_code_list}) - conditions_copy += " and iva.attribute = '{0}' and iva.attribute_value in ({1})" \ - .format(attribute, (", ").join(['%s'] * len(values))) - substitutions_copy.extend(values) + if item_codes: + item_codes = list(set.intersection(*item_codes)) + self.filters.append(["item_code", "in", item_codes]) - items = self.query_items(conditions_copy, self.or_conditions, substitutions_copy, - start=start, with_attributes=True) + items, count = self.query_items(start=start) - items_dict = {item.name: item for item in items} - - all_items.append(set(items_dict.keys())) - - result = [items_dict.get(item) for item in set.intersection(*all_items)] - return result + return items, count def build_fields_filters(self, filters): """Build filters for field values @@ -174,13 +178,22 @@ class ProductQuery: if not values or field == "discount": continue - if isinstance(values, list): + # 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") + if fields: + self.filters.append([child_doctype, fields[0].fieldname, 'IN', values]) + elif isinstance(values, list): # If value is a list use `IN` query - self.conditions += " and wi.{0} in ({1})".format(field, (', ').join(['%s'] * len(values))) - self.substitutions.extend(values) + self.filters.append([field, "in", values]) else: # `=` will be faster than `IN` for most cases - self.conditions += " and wi.{0} = '{1}'".format(field, values) + self.filters.append([field, "=", values]) def build_search_filters(self, search_term): """Query search term in specified fields @@ -203,4 +216,4 @@ class ProductQuery: # Build or filters for query search = '%{}%'.format(search_term) for field in search_fields: - self.or_conditions += " or {0} like '{1}'".format(field, search) + self.or_filters.append([field, "like", search]) diff --git a/erpnext/e_commerce/product_view.js b/erpnext/e_commerce/product_view.js index 5ebf44ec5c7..67fc91395b6 100644 --- a/erpnext/e_commerce/product_view.js +++ b/erpnext/e_commerce/product_view.js @@ -32,28 +32,31 @@ erpnext.ProductView = class { this.disable_view_toggler(true); frappe.call({ - method: 'erpnext.e_commerce.doctype.website_item.website_item.get_product_filter_data', + method: 'erpnext.e_commerce.api.get_product_filter_data', args: args, callback: function(result) { if (!result.exc && result && result.message) { - if (me.item_group && result.message[3].length) { - me.render_item_sub_categories(result.message[3]); + if (me.item_group && result.message["sub_categories"].length) { + me.render_item_sub_categories(result.message["sub_categories"]); } - if (!result.message[0].length) { + if (!result.message["items"].length) { // if result has no items or result is empty me.render_no_products_section(); + + me.bind_filters(); + me.restore_filters_state(); } else { - me.render_filters(result.message[1]); + me.render_filters(result.message["filters"]); // Render views - me.render_list_view(result.message[0], result.message[2]); - me.render_grid_view(result.message[0], result.message[2]); - me.products = result.message[0]; + me.render_list_view(result.message["items"], result.message["settings"]); + me.render_grid_view(result.message["items"], result.message["settings"]); + me.products = result.message["items"]; } // Bottom paging - me.add_paging_section(result.message[2]); + me.add_paging_section(result.message["settings"]); } else { me.render_no_products_section(); } @@ -190,6 +193,7 @@ erpnext.ProductView = class { $("#products-grid-area").addClass("hidden"); $("#products-list-area").removeClass("hidden"); + localStorage.setItem("product_view", "List View"); }); $("#image-view").click(function() { @@ -200,6 +204,7 @@ erpnext.ProductView = class { $("#products-list-area").addClass("hidden"); $("#products-grid-area").removeClass("hidden"); + localStorage.setItem("product_view", "Grid View"); }); } diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py index 959b74b34fc..d4f40676fc3 100644 --- a/erpnext/setup/doctype/item_group/item_group.py +++ b/erpnext/setup/doctype/item_group/item_group.py @@ -70,26 +70,6 @@ class ItemGroup(NestedSet, WebsiteGenerator): context.page_length = cint(frappe.db.get_single_value('E Commerce Settings', 'products_per_page')) or 6 context.search_link = '/product_search' - if frappe.form_dict: - search = frappe.form_dict.search - field_filters = frappe.parse_json(frappe.form_dict.field_filters) - attribute_filters = frappe.parse_json(frappe.form_dict.attribute_filters) - start = frappe.parse_json(frappe.form_dict.start) - else: - search = None - attribute_filters = None - field_filters = {} - start = 0 - - if not field_filters: - field_filters = {} - - # Ensure the query remains within current item group - field_filters['item_group'] = self.name - - engine = ProductQuery() - context.items = engine.query(attribute_filters, field_filters, search, start, item_group=self.name) - filter_engine = ProductFiltersBuilder(self.name) context.field_filters = filter_engine.get_field_filters() diff --git a/erpnext/www/all-products/index.js b/erpnext/www/all-products/index.js index e38514a32a0..db2dec046dd 100644 --- a/erpnext/www/all-products/index.js +++ b/erpnext/www/all-products/index.js @@ -5,7 +5,7 @@ $(() => { let is_item_group_page = $(".item-group-content").data("item-group"); this.item_group = is_item_group_page || null; - let view_type = "List View"; + let view_type = localStorage.getItem("product_view") || "List View"; // Render Product Views, Filters & Search frappe.require('/assets/js/e-commerce.min.js', function() {