Shiloh/brotherton-erpnext
1
1
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor: Cache Item Reviews and other review feedback

Browse Source 
- `get_doc` -> `get_values` and `db.sql` -> `db.delete` in Wishlist Item deletion
- cache first page of Item Reviews and burst cache on addition and deletion of reviews
- Update redisearch docs link in E Commerce Settings
- Removed unused cint import
- Broke setting attribute context into smaller functions and code cleanup
- Minor recommended items padding tweak
- Item reviews form dict now uses website item as key
- Customer reviews rendered from UI style consistency
- Stock status consistency in listing and full page
- Handle no price in variant dialog for matched item
This commit is contained in:
marination 2021-08-25 13:09:35 +05:30
parent 9aeb211142
commit 6b2b9dcee2
14 changed files with 218 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json
View File

@ -329,7 +329,7 @@
"fieldname": "redisearch_warning",
"fieldtype": "HTML",
"label": "Redisearch Warning",
"options": "<p class=\"alert alert-warning\">Redisearch module not loaded. If you want to use advanced product search features, refer documentation <a class=\"alert-link\" href=\"https://frappeframework.com/docs/user/en/installation\" target=\"_blank\">here</a>.</p>"
"options": "<p class=\"alert alert-warning\">Redisearch is not loaded. If you want to use the advanced product search feature, refer <a class=\"alert-link\" href=\"https://docs.erpnext.com/docs/v13/user/manual/en/setting-up/articles/installing-redisearch\" target=\"_blank\">here</a>.</p>"
},
{
"default": "0",
@ -379,7 +379,7 @@
"index_web_pages_for_search": 1,
"issingle": 1,
"links": [],
"modified": "2021-08-24 13:40:15.294696",
"modified": "2021-08-24 21:10:45.669526",
"modified_by": "Administrator",
"module": "E-commerce",
"name": "E Commerce Settings",

2
erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py
View File

@ -3,7 +3,7 @@
# For license information, please see license.txt
import frappe
from frappe.utils import cint, comma_and
from frappe.utils import comma_and
from frappe import _, msgprint
from frappe.model.document import Document
from frappe.utils import unique

99
erpnext/e_commerce/doctype/item_review/item_review.py
View File

@ -15,44 +15,94 @@ class UnverifiedReviewer(frappe.ValidationError):
pass
class ItemReview(Document):
pass
def after_insert(self):
# regenerate cache on review creation
reviews_dict = get_queried_reviews(self.website_item)
set_reviews_in_cache(self.website_item, reviews_dict)
def after_delete(self):
# regenerate cache on review deletion
reviews_dict = get_queried_reviews(self.website_item)
set_reviews_in_cache(self.website_item, reviews_dict)
@frappe.whitelist()
def get_item_reviews(web_item, start, end, data=None):
def get_item_reviews(web_item, start=0, end=10, data=None):
"Get Website Item Review Data."
start, end = cint(start), cint(end)
settings = get_shopping_cart_settings()
# Get cached reviews for first page (start=0)
# avoid cache when page is different
from_cache = not bool(start)
if not data:
data = frappe._dict()
settings = get_shopping_cart_settings()
if settings and settings.get("enable_reviews"):
data.reviews = frappe.db.get_all("Item Review", filters={"website_item": web_item},
fields=["*"], limit_start=cint(start), limit_page_length=cint(end))
reviews_cache = frappe.cache().hget("item_reviews", web_item)
if from_cache and reviews_cache:
data = reviews_cache
else:
data = get_queried_reviews(web_item, start, end, data)
if from_cache:
set_reviews_in_cache(web_item, data)
rating_data = frappe.db.get_all("Item Review", filters={"website_item": web_item},
fields=["avg(rating) as average, count(*) as total"])[0]
data.average_rating = flt(rating_data.average, 1)
data.average_whole_rating = flt(data.average_rating, 0)
return data
# get % of reviews per rating
reviews_per_rating = []
for i in range(1,6):
count = frappe.db.get_all("Item Review", filters={"website_item": web_item, "rating": i},
fields=["count(*) as count"])[0].count
def get_queried_reviews(web_item, start=0, end=10, data=None):
"""
Query Website Item wise reviews and cache if needed.
Cache stores only first page of reviews i.e. 10 reviews maximum.
Returns:
dict: Containing reviews, average ratings, % of reviews per rating and total reviews.
"""
if not data:
data = frappe._dict()
percent = flt((count / rating_data.total or 1) * 100, 0) if count else 0
reviews_per_rating.append(percent)
data.reviews = frappe.db.get_all(
"Item Review",
filters={"website_item": web_item},
fields=["*"],
limit_start=start,
limit_page_length=end
)
data.reviews_per_rating = reviews_per_rating
data.total_reviews = rating_data.total
rating_data = frappe.db.get_all(
"Item Review",
filters={"website_item": web_item},
fields=["avg(rating) as average, count(*) as total"]
)[0]
return data
data.average_rating = flt(rating_data.average, 1)
data.average_whole_rating = flt(data.average_rating, 0)
# get % of reviews per rating
reviews_per_rating = []
for i in range(1,6):
count = frappe.db.get_all(
"Item Review",
filters={"website_item": web_item, "rating": i},
fields=["count(*) as count"]
)[0].count
percent = flt((count / rating_data.total or 1) * 100, 0) if count else 0
reviews_per_rating.append(percent)
data.reviews_per_rating = reviews_per_rating
data.total_reviews = rating_data.total
return data
def set_reviews_in_cache(web_item, reviews_dict):
frappe.cache().hset("item_reviews", web_item, reviews_dict)
@frappe.whitelist()
def add_item_review(web_item, title, rating, comment=None):
""" Add an Item Review by a user if non-existent. """
if frappe.session.user == "Guest":
frappe.throw(_("You are not verified to write a review yet. Please contact us for verification."),
exc=UnverifiedReviewer)
# guest user should not reach here ideally in the case they do via an API, throw error
frappe.throw(_("You are not verified to write a review yet."), exc=UnverifiedReviewer)
if not frappe.db.exists("Item Review", {"user": frappe.session.user, "website_item": web_item}):
doc = frappe.get_doc({
@ -88,5 +138,6 @@ def get_customer(silent=False):
elif silent:
return None
else:
frappe.throw(_("You are not verified to write a review yet. Please contact us for verification."),
exc=UnverifiedReviewer)
# should not reach here unless via an API
frappe.throw(_("You are not a verified customer yet. Please contact us to proceed."),
exc=UnverifiedReviewer)

129
erpnext/e_commerce/doctype/website_item/website_item.py
View File

@ -204,7 +204,9 @@ class WebsiteItem(WebsiteGenerator):
self.get_product_details_section(context)
if settings.enable_reviews:
get_item_reviews(self.name, 0, 4, context)
reviews_data = get_item_reviews(self.name)
context.update(reviews_data)
context.reviews = context.reviews[:4]
context.wished = False
if frappe.db.exists("Wishlist Item", {"item_code": self.item_code, "parent": frappe.session.user}):
@ -219,32 +221,38 @@ class WebsiteItem(WebsiteGenerator):
return context
def set_variant_context(self, context):
if self.has_variants:
context.no_cache = True
if not self.has_variants:
return
# load variants
# also used in set_attribute_context
context.variants = frappe.get_all(
"Item",
filters={"variant_of": self.item_code, "published_in_website": 1},
order_by="name asc")
context.no_cache = True
variant = frappe.form_dict.variant
variant = frappe.form_dict.variant
if not variant and context.variants:
# the case when the item is opened for the first time from its list
variant = context.variants[0]
# load variants
# also used in set_attribute_context
context.variants = frappe.get_all(
"Item",
filters={
"variant_of": self.item_code,
"published_in_website": 1
},
order_by="name asc")
if variant:
context.variant = frappe.get_doc("Item", variant)
# the case when the item is opened for the first time from its list
if not variant and context.variants:
variant = context.variants[0]
for fieldname in ("website_image", "website_image_alt", "web_long_description", "description",
"website_specifications"):
if context.variant.get(fieldname):
value = context.variant.get(fieldname)
if isinstance(value, list):
value = [d.as_dict() for d in value]
if variant:
context.variant = frappe.get_doc("Item", variant)
fields = ("website_image", "website_image_alt", "web_long_description", "description",
"website_specifications")
context[fieldname] = value
for fieldname in fields:
if context.variant.get(fieldname):
value = context.variant.get(fieldname)
if isinstance(value, list):
value = [d.as_dict() for d in value]
context[fieldname] = value
if self.slideshow:
if context.variant and context.variant.slideshow:
@ -253,48 +261,57 @@ class WebsiteItem(WebsiteGenerator):
context.update(get_slideshow(self))
def set_attribute_context(self, context):
if self.has_variants:
attribute_values_available = {}
context.attribute_values = {}
context.selected_attributes = {}
if not self.has_variants:
return
# load attributes
for v in context.variants:
v.attributes = frappe.get_all("Item Variant Attribute",
fields=["attribute", "attribute_value"],
filters={"parent": v.name})
# make a map for easier access in templates
v.attribute_map = frappe._dict({})
for attr in v.attributes:
v.attribute_map[attr.attribute] = attr.attribute_value
attribute_values_available = {}
context.attribute_values = {}
context.selected_attributes = {}
for attr in v.attributes:
values = attribute_values_available.setdefault(attr.attribute, [])
if attr.attribute_value not in values:
values.append(attr.attribute_value)
# load attributes
self.set_selected_attributes(context.variants, context, attribute_values_available)
if v.name == context.variant.name:
context.selected_attributes[attr.attribute] = attr.attribute_value
# filter attributes, order based on attribute table
item = frappe.get_cached_doc("Item", self.item_code)
self.set_attribute_values(item.attributes, context, attribute_values_available)
# filter attributes, order based on attribute table
item = frappe.get_cached_doc("Item", self.item_code)
for attr in item.attributes:
values = context.attribute_values.setdefault(attr.attribute, [])
context.variant_info = json.dumps(context.variants)
if cint(frappe.db.get_value("Item Attribute", attr.attribute, "numeric_values")):
for val in sorted(attribute_values_available.get(attr.attribute, []), key=flt):
values.append(val)
def set_selected_attributes(self, variants, context, attribute_values_available):
for variant in variants:
variant.attributes = frappe.get_all(
"Item Variant Attribute",
filters={"parent": variant.name},
fields=["attribute", "attribute_value as value"])
else:
# get list of values defined (for sequence)
for attr_value in frappe.db.get_all("Item Attribute Value",
fields=["attribute_value"],
filters={"parent": attr.attribute}, order_by="idx asc"):
# make an attribute-value map for easier access in templates
variant.attribute_map = frappe._dict(
{ attr.attribute : attr.value for attr in variant.attributes}
)
if attr_value.attribute_value in attribute_values_available.get(attr.attribute, []):
values.append(attr_value.attribute_value)
for attr in variant.attributes:
values = attribute_values_available.setdefault(attr.attribute, [])
if attr.value not in values:
values.append(attr.value)
context.variant_info = json.dumps(context.variants)
if variant.name == context.variant.name:
context.selected_attributes[attr.attribute] = attr.value
def set_attribute_values(self, attributes, context, attribute_values_available):
for attr in attributes:
values = context.attribute_values.setdefault(attr.attribute, [])
if cint(frappe.db.get_value("Item Attribute", attr.attribute, "numeric_values")):
for val in sorted(attribute_values_available.get(attr.attribute, []), key=flt):
values.append(val)
else:
# get list of values defined (for sequence)
for attr_value in frappe.db.get_all("Item Attribute Value",
fields=["attribute_value"],
filters={"parent": attr.attribute}, order_by="idx asc"):
if attr_value.attribute_value in attribute_values_available.get(attr.attribute, []):
values.append(attr_value.attribute_value)
def set_disabled_attributes(self, context):
"""Disable selection options of attribute combinations that do not result in a variant"""

23
erpnext/e_commerce/doctype/wishlist/wishlist.py
View File

@ -50,16 +50,19 @@ def add_to_wishlist(item_code):
@frappe.whitelist()
def remove_from_wishlist(item_code):
if frappe.db.exists("Wishlist Item", {"item_code": item_code, "parent": frappe.session.user}):
frappe.db.sql("""
DELETE
FROM `tabWishlist Item`
WHERE
item_code=%(item_code)s
and parent='%(user)s'
""" % {"item_code": frappe.db.escape(item_code), "user": frappe.session.user})
frappe.db.delete(
"Wishlist Item",
{
"item_code": item_code,
"parent": frappe.session.user
}
)
frappe.db.commit()
wishlist = frappe.get_doc("Wishlist", frappe.session.user)
wishlist_items = frappe.db.get_values(
"Wishlist Item",
filters={"parent": frappe.session.user}
)
if hasattr(frappe.local, "cookie_manager"):
frappe.local.cookie_manager.set_cookie("wish_count", str(len(wishlist.items)))
frappe.local.cookie_manager.set_cookie("wish_count", str(len(wishlist_items)))

23
erpnext/e_commerce/product_data_engine/query.py
View File

@ -2,9 +2,10 @@
# License: GNU General Public License v3. See license.txt
import frappe
from frappe.utils import flt
from erpnext.e_commerce.shopping_cart.product_info import get_product_info_for_website
from erpnext.e_commerce.doctype.item_review.item_review import get_customer
from frappe.utils import flt
from erpnext.utilities.product import get_non_stock_item_status
from erpnext.e_commerce.shopping_cart.product_info import get_product_info_for_website
@ -235,12 +236,22 @@ class ProductQuery:
def get_stock_availability(self, item):
"""Modify item object and add stock details."""
item.in_stock = False
warehouse = item.get("website_warehouse")
is_stock_item = frappe.get_cached_value("Item", item.item_code, "is_stock_item")
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 = bool(stock_qty)
if not is_stock_item:
if warehouse:
# product bundle case
item.in_stock = get_non_stock_item_status(item.item_code, "website_warehouse")
else:
item.in_stock = True
elif warehouse:
# stock item and has warehouse
actual_qty = frappe.db.get_value(
"Bin",
{"item_code": item.item_code,"warehouse": item.get("website_warehouse")},
"actual_qty")
item.in_stock = bool(flt(actual_qty))
def get_cart_items(self):
customer = get_customer(silent=True)

2
erpnext/e_commerce/shopping_cart/product_info.py
View File

@ -24,7 +24,7 @@ def get_product_info_for_website(item_code, skip_quotation_creation=False):
selling_price_list = cart_quotation.get("selling_price_list") if cart_quotation else _set_price_list(cart_settings, None)
price = []
price = {}
if cart_settings.show_price:
is_guest = frappe.session.user == "Guest"
# Show Price if logged in.

19
erpnext/public/scss/shopping_cart.scss
View File

@ -188,13 +188,6 @@ body.product-page {
font-weight: 600;
}
.out-of-stock {
font-weight: 500;
font-size: 14px;
line-height: 20px;
color: #F47A7A;
}
.item-card {
padding: var(--padding-sm);
min-width: 300px;
@ -450,6 +443,10 @@ body.product-page {
.r-item-image {
width: 40%;
.product-image {
padding: 2px 15px;
}
.no-image-r-item {
display: flex; justify-content: center;
background-color: var(--gray-200);
@ -464,7 +461,6 @@ body.product-page {
.r-item-info {
font-size: 14px;
padding-left: 8px;
padding-right: 0;
width: 60%;
@ -1322,6 +1318,13 @@ body.product-page {
font-weight: 500;
}
.out-of-stock {
font-weight: 500;
font-size: 14px;
line-height: 20px;
color: #F47A7A;
}
.mt-minus-2 {
margin-top: -2rem;
}

4
erpnext/templates/generators/item/item_add_to_cart.html
View File

@ -35,8 +35,8 @@
{% if cart_settings.show_stock_availability %}
<div class="mt-2">
{% if product_info.in_stock == 0 %}
<span class="text-danger no-stock">
{{ _('Not in stock') }}
<span class="no-stock out-of-stock">
{{ _('Out of stock') }}
</span>
{% elif product_info.in_stock == 1 %}
<span class="in-green has-stock">

2
erpnext/templates/generators/item/item_configure.js
View File

@ -214,7 +214,7 @@ class ItemConfigure {
? `<div class="alert alert-success d-flex justify-content-between align-items-center" role="alert">
<div><div>
${one_item}
${product_info && product_info.price
${product_info && product_info.price && !$.isEmptyObject()
? '(' + product_info.price.formatted_price_sales_uom + ')'
: ''
}

2
erpnext/templates/generators/item/item_reviews.html
View File

@ -29,7 +29,7 @@
{% if total_reviews > 4 %}
<div class="mt-6 mb-6"style="color: var(--primary);">
<a href="/customer_reviews?item_code={{ doc.item_code }}">{{ _("View all reviews") }}</a>
<a href="/customer_reviews?web_item={{ doc.name }}">{{ _("View all reviews") }}</a>
</div>
{% endif %}

26
erpnext/templates/pages/customer_reviews.js
View File

@ -96,23 +96,25 @@ $(() => {
reviews.forEach((review) => {
$content.append(`
<div class="mb-3 review">
<div style="display: flex;">
<div class="d-flex">
<p class="mr-4 user-review-title">
<span>${__(review.review_title)}</span>
</p>
<div class="rating">
${me.get_review_stars(review.rating)}
</div>
<p class="ml-4 user-review-title">
<span>${__(review.review_title)}</span>
</p>
</div>
<div class="review-signature">
<span class="reviewer">${__(review.customer)}</span>
<span>${__(review.published_on)}</span>
</div>
<div class="product-description mb-4 mt-4">
<div class="product-description mb-4">
<p>
${__(review.comment)}
</p>
</div>
<div class="review-signature mb-2">
<span class="reviewer">${__(review.customer)}</span>
<span class="indicator grey" style="--text-on-gray: var(--gray-300);"></span>
<span class="reviewer">${__(review.published_on)}</span>
</div>
</div>
`);
});
@ -122,9 +124,11 @@ $(() => {
let stars = ``;
for (let i = 1; i < 6; i++) {
let fill_class = i <= rating ? 'star-click' : '';
stars += `<svg class="icon icon-md ${fill_class}">
stars += `
<svg class="icon icon-sm ${fill_class}">
<use href="#icon-star"></use>
</svg>`;
</svg>
`;
}
return stars;
}

8
erpnext/templates/pages/customer_reviews.py
View File

@ -10,10 +10,10 @@ def get_context(context):
context.full_page = True
context.reviews = None
if frappe.form_dict and frappe.form_dict.get("item_code"):
context.item_code = frappe.form_dict.get("item_code")
context.web_item = frappe.db.get_value("Website Item", {"item_code": context.item_code}, "name")
if frappe.form_dict and frappe.form_dict.get("web_item"):
context.web_item = frappe.form_dict.get("web_item")
context.user_is_customer = check_if_user_is_customer()
context.enable_reviews = get_shopping_cart_settings().enable_reviews
if context.enable_reviews:
get_item_reviews(context.web_item, 0, 10, context)
reviews_data = get_item_reviews(context.web_item)
context.update(reviews_data)

4
erpnext/utilities/product.py
View File

@ -139,7 +139,7 @@ def get_non_stock_item_status(item_code, item_warehouse_field):
# if item is a product bundle, check if its bundle items are in stock
if frappe.db.exists("Product Bundle", item_code):
items = frappe.get_doc("Product Bundle", item_code).get_all_children()
bundle_warehouse = frappe.db.get_value('Item', item_code, item_warehouse_field)
return all([get_web_item_qty_in_stock(d.item_code, item_warehouse_field, bundle_warehouse).in_stock for d in items])
bundle_warehouse = frappe.db.get_value("Website Item", {"item_code": item_code}, item_warehouse_field)
return all(get_web_item_qty_in_stock(d.item_code, item_warehouse_field, bundle_warehouse).in_stock for d in items)
else:
return 1