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

@@ -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"]
+	}

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, 
+	insert_item_to_index,
-	update_index_for_item, 
+	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
+	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

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])

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',
+		self.fields = ['web_item_name', 'name', 'item_name', 'item_code', 'website_image',
-			'wi.has_variants', 'wi.item_group', 'wi.image', 'wi.web_long_description', 'wi.description',
+			'variant_of', 'has_variants', 'item_group', 'image', 'web_long_description',
-			'wi.route', 'wi.website_warehouse', 'wi.ranking']
+			'description', 'route', 'website_warehouse', 'ranking']
-		self.conditions = ""
+		self.filters = [["published", "=", 1]]
-		self.or_conditions = ""
+		self.or_filters = []
-		self.substitutions = []
 
 	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,
+			result, count = self.query_items(start=start)
-				self.substitutions, 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"""
+		return items, count or 0
-			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)
 
 	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
+			# get items that have selected attribute & value
-			substitutions_copy = self.substitutions.copy()
+			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})" \
+		if item_codes:
-				.format(attribute, (", ").join(['%s'] * len(values)))
+			item_codes = list(set.intersection(*item_codes))
-			substitutions_copy.extend(values)
+			self.filters.append(["item_code", "in", item_codes])
 
-			items = self.query_items(conditions_copy, self.or_conditions, substitutions_copy,
+		items, count = self.query_items(start=start)
-				start=start, with_attributes=True)
 
-			items_dict = {item.name: item for item in items}
+		return items, count
-
-			all_items.append(set(items_dict.keys()))
-
-		result = [items_dict.get(item) for item in set.intersection(*all_items)]
-		return result
 
 	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.filters.append([field, "in", values])
-				self.substitutions.extend(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])

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) {
+					if (me.item_group && result.message["sub_categories"].length) {
-						me.render_item_sub_categories(result.message[3]);
+						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_list_view(result.message["items"], result.message["settings"]);
-						me.render_grid_view(result.message[0], result.message[2]);
+						me.render_grid_view(result.message["items"], result.message["settings"]);
-						me.products = result.message[0];
+						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");
 		});
 	}
 

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()

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() {