fix: Discount Filtes & Filter behaviour

- Client: Maintain state where listing is re-rendered due filter trigger
- Client: Handle binding/restoring discount filters separately on filter trigger
- Client: Placeholder Image for search results
- If any filter is checked, query and display items from page 1
- Query Engine: Smaller functions and handle discount filter properly
- Added index on item group and brand for Website item

erpnext/e_commerce/api.py

@@ -18,11 +18,17 @@ def get_product_filter_data():
 		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
+		from_filters = frappe.parse_json(frappe.form_dict.from_filters)
 	else:
-		search, attribute_filters, item_group = None, None, None
+		search, attribute_filters, item_group, from_filters = None, None, None, None
 		field_filters = {}
 		start = 0
 
+	if from_filters:
+		# if filter is checked, go to start
+		# and show filtered items from page 1
+		start = 0
+
 	sub_categories = []
 	if item_group:
 		field_filters['item_group'] = item_group

erpnext/e_commerce/doctype/website_item/website_item.json

@@ -318,7 +318,7 @@
  "image_field": "image",
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2021-07-08 12:22:23.466598",
+ "modified": "2021-07-08 19:25:15.115746",
  "modified_by": "Administrator",
  "module": "E-commerce",
  "name": "Website Item",

erpnext/e_commerce/doctype/website_item/website_item.py

@@ -400,6 +400,9 @@ def on_doctype_update():
 	# since route is a Text column, it needs a length for indexing
 	frappe.db.add_index("Website Item", ["route(500)"])
 
+	frappe.db.add_index("Website Item", ["item_group"])
+	frappe.db.add_index("Website Item", ["brand"])
+
 def check_if_user_is_customer(user=None):
 	from frappe.contacts.doctype.contact.contact import get_contact_name
 

erpnext/e_commerce/product_query.py

@@ -21,15 +21,15 @@ class ProductQuery:
 	def __init__(self):
 		self.settings = frappe.get_doc("E Commerce Settings")
 		self.page_length = self.settings.products_per_page or 20
+
+		self.or_filters = []
+		self.filters = [["published", "=", 1]]
 		self.fields = ['web_item_name', 'name', 'item_name', 'item_code', 'website_image',
 			'variant_of', 'has_variants', 'item_group', 'image', 'web_long_description',
 			'short_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
-
+		"""
 		Args:
 			attributes (dict, optional): Item Attribute filters
 			fields (dict, optional): Field level filters
@@ -37,18 +37,13 @@ class ProductQuery:
 			start (int, optional): Page start
 
 		Returns:
-			list: List of results with set fields
+			dict: Dict containing items, item count & discount range
 		"""
-		result, discount_list = [], []
-		website_item_groups = []
+		# track if discounts included in field filters
+		self.filter_with_discount = bool(fields.get("discount"))
+		result, discount_list, website_item_groups, count = [], [], [], 0
 
-		# if from item group page consider website item group table
-		if item_group:
-			website_item_groups = frappe.db.get_all(
-				"Website Item",
-				fields=self.fields + ["`tabWebsite Item Group`.parent as wig_parent"],
-				filters=[["Website Item Group", "item_group", "=", item_group]]
-			)
+		website_item_groups = self.get_website_item_group_results(item_group, website_item_groups)
 
 		if fields:
 			self.build_fields_filters(fields)
@@ -57,33 +52,23 @@ class ProductQuery:
 		if self.settings.hide_variants:
 			self.filters.append(["variant_of", "is", "not set"])
 
-		count = 0
+		# query results
 		if attributes:
 			result, count = self.query_items_with_attributes(attributes, start)
 		else:
 			result, count = self.query_items(start=start)
 
-		# add price and availability info in results
-		for item in result:
-			product_info = get_product_info_for_website(item.item_code, skip_quotation_creation=True).get('product_info')
+		result = self.combine_web_item_group_results(item_group, result, website_item_groups)
 
-			if product_info and product_info['price']:
-				self.get_price_discount_info(item, product_info['price'], discount_list)
-
-			if self.settings.show_stock_availability:
-				self.get_stock_availability(item)
-
-			item.wished = False
-			if frappe.db.exists("Wishlist Item", {"item_code": item.item_code, "parent": frappe.session.user}):
-				item.wished = True
+		# sort combined results by ranking
+		result = sorted(result, key=lambda x: x.get("ranking"), reverse=True)
+		result, discount_list = self.add_display_details(result, discount_list)
 
 		discounts = []
 		if discount_list:
 			discounts = [min(discount_list), max(discount_list)]
 
-		if fields and "discount" in fields:
-			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]
+		result = self.filter_results_by_discount(fields, result)
 
 		return {
 			"items": result,
@@ -91,30 +76,6 @@ class ProductQuery:
 			"discounts": discounts
 		}
 
-	def get_price_discount_info(self, item, price_object, discount_list):
-		"""Modify item object and add price details."""
-		item.formatted_mrp = price_object.get('formatted_mrp')
-		item.formatted_price = price_object.get('formatted_price')
-
-		if price_object.get('discount_percent'):
-			item.discount_percent = flt(price_object.discount_percent)
-			discount_list.append(price_object.discount_percent)
-
-		if item.formatted_mrp:
-			item.discount = price_object.get('formatted_discount_percent') or \
-				price_object.get('formatted_discount_rate')
-		item.price = price_object.get('price_list_rate')
-
-	def get_stock_availability(self, item):
-		"""Modify item object and add stock details."""
-		if item.get("website_warehouse"):
-			stock_qty = frappe.utils.flt(
-				frappe.db.get_value("Bin", {"item_code": item.item_code, "warehouse": item.get("website_warehouse")},
-					"actual_qty"))
-			item.in_stock = "green" if stock_qty else "red"
-		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, start=0):
 		"""Build a query to fetch Website Items based on field filters."""
 		# MySQL does not support offset without limit,
@@ -129,12 +90,18 @@ class ProductQuery:
 			order_by="ranking desc")
 		count = len(count_items)
 
+		# If discounts included, return all rows.
+		# Slice after filtering rows with discount (See `filter_results_by_discount`).
+		# Slicing before hand will miss discounted items on the 3rd or 4th page.
+		# Discounts are fetched on computing Pricing Rules so we cannot query them directly.
+		page_length = 184467440737095516 if self.filter_with_discount else self.page_length
+
 		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_page_length=page_length,
 			limit_start=start,
 			order_by="ranking desc")
 
@@ -215,3 +182,77 @@ class ProductQuery:
 		search = '%{}%'.format(search_term)
 		for field in search_fields:
 			self.or_filters.append([field, "like", search])
+
+	def get_website_item_group_results(self, item_group, website_item_groups):
+		"""Get Web Items for Item Group Page via Website Item Groups."""
+		if item_group:
+			website_item_groups = frappe.db.get_all(
+				"Website Item",
+				fields=self.fields + ["`tabWebsite Item Group`.parent as wig_parent"],
+				filters=[["Website Item Group", "item_group", "=", item_group]]
+			)
+		return website_item_groups
+
+	def add_display_details(self, result, discount_list):
+		"""Add price and availability details in result."""
+		for item in result:
+			product_info = get_product_info_for_website(item.item_code, skip_quotation_creation=True).get('product_info')
+
+			if product_info and product_info['price']:
+				# update/mutate item and discount_list objects
+				self.get_price_discount_info(item, product_info['price'], discount_list)
+
+			if self.settings.show_stock_availability:
+				self.get_stock_availability(item)
+
+			item.wished = False
+			if frappe.db.exists("Wishlist Item", {"item_code": item.item_code, "parent": frappe.session.user}):
+				item.wished = True
+
+		return result, discount_list
+
+	def get_price_discount_info(self, item, price_object, discount_list):
+		"""Modify item object and add price details."""
+		fields = ["formatted_mrp", "formatted_price", "price_list_rate"]
+		for field in fields:
+			item[field] = price_object.get(field)
+
+		if price_object.get('discount_percent'):
+			item.discount_percent = flt(price_object.discount_percent)
+			discount_list.append(price_object.discount_percent)
+
+		if item.formatted_mrp:
+			item.discount = price_object.get('formatted_discount_percent') or \
+				price_object.get('formatted_discount_rate')
+
+	def get_stock_availability(self, item):
+		"""Modify item object and add stock details."""
+		if item.get("website_warehouse"):
+			stock_qty = frappe.utils.flt(
+				frappe.db.get_value("Bin", {"item_code": item.item_code, "warehouse": item.get("website_warehouse")},
+					"actual_qty"))
+			item.in_stock = "green" if stock_qty else "red"
+		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 combine_web_item_group_results(self, item_group, result, website_item_groups):
+		"""Combine results with 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)
+
+		return result
+
+	def filter_results_by_discount(self, fields, result):
+		if fields and fields.get("discount"):
+			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]
+
+		if self.filter_with_discount:
+			# no limit was added to results while querying
+			# slice results manually
+			result[:self.page_length]
+
+		return result

erpnext/e_commerce/product_search.js

@@ -210,9 +210,10 @@ erpnext.ProductSearch = class {
 		let search_results = data.message.results;
 
 		search_results.forEach((res) => {
+			let thumbnail = res.thumbnail || '/assets/erpnext/images/ui-states/cart-empty-state.png';
 			html += `
 				<div class="dropdown-item" style="display: flex;">
-					<img class="item-thumb col-2" src=${res.thumbnail || 'img/placeholder.png'} />
+					<img class="item-thumb col-2" src=${thumbnail} />
 					<div class="col-9" style="white-space: normal;">
 						<a href="/${res.route}">${res.web_item_name}</a><br>
 						<span class="brand-line">${res.brand ? "by " + res.brand : ""}</span>

erpnext/e_commerce/product_view.js

@@ -27,6 +27,7 @@ erpnext.ProductView =  class {
 	get_item_filter_data(from_filters=false) {
 		// Get and render all Product related views
 		let me = this;
+		this.from_filters = from_filters;
 		let args = this.get_query_filters();
 
 		this.disable_view_toggler(true);
@@ -36,6 +37,7 @@ erpnext.ProductView =  class {
 			args: args,
 			callback: function(result) {
 				if (!result.exc && result && result.message) {
+					// Sub Category results are independent of Items
 					if (me.item_group && result.message["sub_categories"].length) {
 						me.render_item_sub_categories(result.message["sub_categories"]);
 					}
@@ -45,7 +47,7 @@ erpnext.ProductView =  class {
 						me.render_no_products_section();
 					} else {
 						// Add discount filters
-						me.get_discount_filter_html(result.message["filters"].discount_filters);
+						me.re_render_discount_filters(result.message["filters"].discount_filters);
 
 						// Render views
 						me.render_list_view(result.message["items"], result.message["settings"]);
@@ -129,7 +131,8 @@ erpnext.ProductView =  class {
 			field_filters: field_filters,
 			attribute_filters: attribute_filters,
 			item_group: this.item_group,
-			start: filters.start || null
+			start: filters.start || null,
+			from_filters: this.from_filters || false
 		};
 	}
 
@@ -221,10 +224,14 @@ erpnext.ProductView =  class {
 	}
 
 	bind_paging_action() {
+		let me = this;
 		$('.btn-prev, .btn-next').click((e) => {
 			const $btn = $(e.target);
+			me.from_filters = false;
+
 			$btn.prop('disabled', true);
 			const start = $btn.data('start');
+
 			let query_params = frappe.utils.get_query_params();
 			query_params.start = start;
 			let path = window.location.pathname + '?' + frappe.utils.get_url_from_dict(query_params);
@@ -232,6 +239,18 @@ erpnext.ProductView =  class {
 		});
 	}
 
+	re_render_discount_filters(filter_data) {
+		this.get_discount_filter_html(filter_data);
+		if (this.from_filters) {
+			// Bind filter action if triggered via filters
+			// if not from filter action, page load will bind actions
+			this.bind_discount_filter_action();
+		}
+		// discount filters are rendered with Items (later)
+		// unlike the other filters
+		this.restore_discount_filter();
+	}
+
 	get_discount_filter_html(filter_data) {
 		$("#discount-filters").remove();
 		if (filter_data) {
@@ -266,12 +285,56 @@ erpnext.ProductView =  class {
 		}
 	}
 
+	restore_discount_filter() {
+		const filters = frappe.utils.get_query_params();
+		let field_filters = filters.field_filters;
+		if (!field_filters) return;
+
+		field_filters = JSON.parse(field_filters);
+
+		if (field_filters && field_filters["discount"]) {
+			const values = field_filters["discount"];
+			const selector = values.map(value => {
+				return `input[data-filter-name="discount"][data-filter-value="${value}"]`;
+			}).join(',');
+			$(selector).prop('checked', true);
+			this.field_filters = field_filters;
+		}
+	}
+
+	bind_discount_filter_action() {
+		let me = this;
+		$('.discount-filter').on('change', (e) => {
+			const $checkbox = $(e.target);
+			const is_checked = $checkbox.is(':checked');
+
+			const {
+				filterValue: filter_value
+			} = $checkbox.data();
+
+			delete this.field_filters["discount"];
+
+			if (is_checked) {
+				this.field_filters["discount"] = []
+				this.field_filters["discount"].push(filter_value);
+			}
+
+			if (this.field_filters["discount"].length === 0) {
+				delete this.field_filters["discount"];
+			}
+
+			me.change_route_with_filters();
+		})
+	}
+
 	bind_filters() {
 		let me = this;
 		this.field_filters = {};
 		this.attribute_filters = {};
 
 		$('.product-filter').on('change', (e) => {
+			me.from_filters = true;
+
 			const $checkbox = $(e.target);
 			const is_checked = $checkbox.is(':checked');
 
@@ -317,21 +380,31 @@ erpnext.ProductView =  class {
 				}
 			}
 
-			let route_params = frappe.utils.get_query_params();
-			const query_string = me.get_query_string({
-				start: me.if_key_exists(route_params.start) || 0,
-				field_filters: JSON.stringify(me.if_key_exists(this.field_filters)),
-				attribute_filters: JSON.stringify(me.if_key_exists(this.attribute_filters)),
-			});
-			window.history.pushState('filters', '', `${location.pathname}?` + query_string);
-
-			$('.page_content input').prop('disabled', true);
-
-			me.make(true);
-			$('.page_content input').prop('disabled', false);
+			me.change_route_with_filters();
 		});
 	}
 
+	change_route_with_filters() {
+		let route_params = frappe.utils.get_query_params();
+
+		let start = this.if_key_exists(route_params.start) || 0;
+		if (this.from_filters) {
+			start = 0; // show items from first page if new filters are triggered
+		}
+
+		const query_string = this.get_query_string({
+			start: start,
+			field_filters: JSON.stringify(this.if_key_exists(this.field_filters)),
+			attribute_filters: JSON.stringify(this.if_key_exists(this.attribute_filters)),
+		});
+		window.history.pushState('filters', '', `${location.pathname}?` + query_string);
+
+		$('.page_content input').prop('disabled', true);
+
+		this.make(true);
+		$('.page_content input').prop('disabled', false);
+	}
+
 	restore_filters_state() {
 		const filters = frappe.utils.get_query_params();
 		let {field_filters, attribute_filters} = filters;