From c0811c4c74e4481b38a81daf2612d74c5b2f6dcc Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 8 Jul 2021 19:34:07 +0530 Subject: [PATCH] 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 | 8 +- .../doctype/website_item/website_item.json | 2 +- .../doctype/website_item/website_item.py | 3 + erpnext/e_commerce/product_query.py | 151 +++++++++++------- erpnext/e_commerce/product_search.js | 3 +- erpnext/e_commerce/product_view.js | 101 ++++++++++-- erpnext/www/all-products/img/placeholder.png | Bin 3985 -> 0 bytes 7 files changed, 196 insertions(+), 72 deletions(-) delete mode 100644 erpnext/www/all-products/img/placeholder.png diff --git a/erpnext/e_commerce/api.py b/erpnext/e_commerce/api.py index c6d27bd7b9..4f83f08ae7 100644 --- a/erpnext/e_commerce/api.py +++ b/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 diff --git a/erpnext/e_commerce/doctype/website_item/website_item.json b/erpnext/e_commerce/doctype/website_item/website_item.json index 9581539d2e..bf0f4f105d 100644 --- a/erpnext/e_commerce/doctype/website_item/website_item.json +++ b/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", diff --git a/erpnext/e_commerce/doctype/website_item/website_item.py b/erpnext/e_commerce/doctype/website_item/website_item.py index 8d3127e01f..3ff58fdf71 100644 --- a/erpnext/e_commerce/doctype/website_item/website_item.py +++ b/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 diff --git a/erpnext/e_commerce/product_query.py b/erpnext/e_commerce/product_query.py index b4d3bed2c1..804a7dec17 100644 --- a/erpnext/e_commerce/product_query.py +++ b/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 \ No newline at end of file diff --git a/erpnext/e_commerce/product_search.js b/erpnext/e_commerce/product_search.js index 8466b4f58c..b5ee083e00 100644 --- a/erpnext/e_commerce/product_search.js +++ b/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 += `
- +
${res.web_item_name}
${res.brand ? "by " + res.brand : ""} diff --git a/erpnext/e_commerce/product_view.js b/erpnext/e_commerce/product_view.js index 6b57aa9492..a70be44de4 100644 --- a/erpnext/e_commerce/product_view.js +++ b/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; diff --git a/erpnext/www/all-products/img/placeholder.png b/erpnext/www/all-products/img/placeholder.png deleted file mode 100644 index 9780ad8e0572d612ed6c12a510934d29e074da28..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 3985 zcmds)c~nzZ9>?zsU;-|Ht)kYDP@T3eEG`2A4T)O=cEAV*kljH62?AE36qH1*TA>A8 zT4V`OQIKE=E-|u%q@q$=Hh}~iWpS{HB*tjNBSId@yhmp`|8&lr_8fcWG=JQDbKm#g z`}w`^J?Gw>WCYM>+bpsH0L=FD-4qA_4wX1SutdM2_ATBAfNiMXruEx16xxBV+K@EI zwwHzmzmv`@*ZVpVTtZ{kuh>Vn4FBx$mcZ2ct9&;son^_7mwCmJ?T##58n?<}OT?Ye za&gu+A&0G(M-YzGlDE6$A>)x zBs3gnDe!$Q4Jfb#e!tfA?(IEW_N-o5#vc&YmR;fs#W&=waK*+;o`#Kwf<)>#zD$+I zcj>Rz%0G3NRlQg$>OY*>QdS$;>F!^_g0~%Fs2=(QL|!eb%#)o`Xo5GNVTpZe42c&b z@lvz?_E&0#I1sOa;!nTxXlq*CV4dNi7jQRSaPNPaDgKH}2`dQJba)N4SV+a?-Bn)O z*gS6MLg!Ex{K1rPioj{2aF4=;nLQ2Xt+a_^c_%d7xpg;P%yjjlUmxbyQ1es{M=PP2 zb-~r@ZSf~?le~Z%^8jM>OR5Qi@v37-GOGh_q(BU2>qlIEe5t*SCXkq1tIwXhpMAKa zUf@zJ!DUss%rV}A7Uan%6v_z3u7_$Qb?pMq;|zY^#!ibn;|%_+wvBAO+hT!4HoTaT z!H-I=9s#I5NA${Pi_c+izb4I^}qsfKoYt}zp( z^}?32?^o%YfPa>W$;`hV9wfnKqzBMDn4Bsf^BheP1vIJBpV(FG>5{Z&#@R~iJRK>P z3E$~;^<_U;yto~aPsXhsnD{uf>lt*;Pz}hmATIj^-0sNH?x&t6bgxwn?WA18$7l$7 z@8H(n`PrC}@h1IEsCW?Zz8pX9YMEH+Sr@hF!k!KZPvHiy;JkdYoc2bE`B`|5S0kZd z0&l?qMeMZzmGu9}tur`p5xRG&*cL#7ZfaCXNI-HW-3lsJBW>ne*Q}8_09tL10d{14 z{tQee{LwPTq$JG-xb`8W5~6V?O0Xi_*wZx16w`e`mLeV0MZof$WV$lF2Zs212eBuS zR*WUSi|U9jOa*ovpvMCox-6ifs3br@pIQJ21qEaPDQ$rziV1Okx4Fj}n~D6@^fG_{ zfBM0hrg;w|`%}oR$q7r?9QWI#^F?#6e}|muVuK5BFwg|Sler9K`9YJBNSoB*+NoaH z&V$HU{GVdQW8vFj*>6Cjr(wwxu;86gQ$+^WpYs8M(2RRZ9v(6i)j=aY&R2|^vY|JYaWHXWwdzY z#`b@8qCExC-EbmuRVvj7KZG+lU?QU!at+k?660 zbGK=OG_-$B7Dz6Z*tyBE!~L(n*cyJZ#r@mnQu4FSq?LwiSo;36B_k!rNPE_pyrt~| zpx`$=LQlLZ_PkWSv$Pzl^;yk!iYfhm6q4f6m;CvDPomEi)`KVyua?no5#a*K%uqp_ z0@rMJNzpNR8~Jhhu#fR(TE@J+<>l?VB8gpUId}9UX79+xtP@0dGi7+}l>4Kta7I%L zIRt1kx%)R=G(L^UpD-9VRfve&_8iO|9TO8<10!p9)0qkedQaoKcF&`Q3p|L-Jx9eq z#G8nc2G)a!T4C?0FQ5Mq<}A`pzQ0}M@btP)J)0h^prwozv-@?=7rV>i;R}^3&>KE{ zz<8zqXc`j58IKh!pKdWc6=v&o$Q`maqCCXI?~L|un#tm#JQ0BPESzh9W!}{TZ7sZk zLs4LD;R+5fT^!)`U60_vRUM*wF~He-ad2PhMenhI@X@IPZVJTX9e0NPC?g5zP4}YW zVase#kr-N+E9|f@%pUz-(Hm@}K8t8^Z05@!PY$jsOg0=MmbD=&OB%C@;VNq!abe0b z#eFXAs;>)`a7waHeljsn{0~;zd9k+uA6+~mF&+>edKXS9zQ6L8(T+WPt^IkP^~V$}Fb!t1BlfIz=UKJdmFqEbvI;$Fn}x z^4?91-z(t5sg$Qly`{#lTWZ47uF2K<#^6FtZ#rv3$xnF^%RCJ7xnL