From 20bc65c97b8b09c2633d594b538db9b63d120be2 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 9 Aug 2021 13:25:13 +0530 Subject: [PATCH] test: Website Item (Item full page) - Tests for website item. Desk tests and portal tests - Allow guests on method `get_offer_details` - Fetch stock details only if `show_stock_availability` is enabled - Validation for duplicate web items on item merge - Separate method for `validate_properties_before_merge` - Common error title and exception `DataValidationError` for merge validations on Item --- .../test_e_commerce_settings.py | 9 + .../doctype/website_item/test_website_item.py | 348 +++++++++++++++++- .../doctype/website_item/website_item.py | 29 +- .../doctype/website_offer/website_offer.py | 2 +- erpnext/e_commerce/product_grid.js | 2 +- erpnext/e_commerce/product_list.js | 2 +- .../e_commerce/shopping_cart/product_info.py | 12 +- .../setup/doctype/item_group/item_group.py | 6 +- erpnext/stock/doctype/item/item.py | 49 ++- .../generators/item/item_configure.js | 2 +- 10 files changed, 416 insertions(+), 45 deletions(-) diff --git a/erpnext/e_commerce/doctype/e_commerce_settings/test_e_commerce_settings.py b/erpnext/e_commerce/doctype/e_commerce_settings/test_e_commerce_settings.py index 798529b222e..9257b7d75e3 100644 --- a/erpnext/e_commerce/doctype/e_commerce_settings/test_e_commerce_settings.py +++ b/erpnext/e_commerce/doctype/e_commerce_settings/test_e_commerce_settings.py @@ -38,4 +38,13 @@ class TestECommerceSettings(unittest.TestCase): frappe.db.sql("update `tabTax Rule` set use_for_shopping_cart = 1") +def setup_e_commerce_settings(values_dict): + "Accepts a dict of values that updates E Commerce Settings." + if not values_dict: + return + + doc = frappe.get_doc("E Commerce Settings", "E Commerce Settings") + doc.update(values_dict) + doc.save() + test_dependencies = ["Tax Rule"] diff --git a/erpnext/e_commerce/doctype/website_item/test_website_item.py b/erpnext/e_commerce/doctype/website_item/test_website_item.py index 1f2cff025cb..9fc674ffcc4 100644 --- a/erpnext/e_commerce/doctype/website_item/test_website_item.py +++ b/erpnext/e_commerce/doctype/website_item/test_website_item.py @@ -5,22 +5,54 @@ from __future__ import unicode_literals import frappe import unittest +from erpnext.stock.doctype.item.item import DataValidationError from erpnext.stock.doctype.item.test_item import make_item from erpnext.e_commerce.doctype.website_item.website_item import make_website_item +from erpnext.controllers.item_variant import create_variant +from erpnext.e_commerce.doctype.e_commerce_settings.test_e_commerce_settings import setup_e_commerce_settings +from erpnext.e_commerce.shopping_cart.product_info import get_product_info_for_website + +WEBITEM_DESK_TESTS = ("test_website_item_desk_item_sync", "test_publish_variant_and_template") +WEBITEM_PRICE_TESTS = ('test_website_item_price_for_logged_in_user', 'test_website_item_price_for_guest_user') class TestWebsiteItem(unittest.TestCase): @classmethod def setUpClass(cls): - make_item("Test Web Item", { - "has_variant": 1, - "variant_based_on": "Item Attribute", - "attributes": [ - { - "attribute": "Test Size" - } - ] + setup_e_commerce_settings({ + "company": "_Test Company", + "enabled": 1, + "default_customer_group": "_Test Customer Group", + "price_list": "_Test Price List India" }) + def setUp(self): + if self._testMethodName in WEBITEM_DESK_TESTS: + make_item("Test Web Item", { + "has_variant": 1, + "variant_based_on": "Item Attribute", + "attributes": [ + { + "attribute": "Test Size" + } + ] + }) + elif self._testMethodName in WEBITEM_PRICE_TESTS: + self.create_regular_web_item() + make_web_item_price(item_code="Test Mobile Phone") + make_web_pricing_rule( + title="Test Pricing Rule for Test Mobile Phone", + item_code="Test Mobile Phone", + selling=1) + + def tearDown(self): + if self._testMethodName in WEBITEM_DESK_TESTS: + frappe.get_doc("Item", "Test Web Item").delete() + elif self._testMethodName in WEBITEM_PRICE_TESTS: + frappe.delete_doc("Pricing Rule", "Test Pricing Rule for Test Mobile Phone") + frappe.get_cached_doc("Item Price", {"item_code": "Test Mobile Phone"}).delete() + frappe.get_cached_doc("Website Item", {"item_code": "Test Mobile Phone"}).delete() + + def test_index_creation(self): "Check if index is getting created in db." from erpnext.e_commerce.doctype.website_item.website_item import on_doctype_update @@ -37,22 +69,30 @@ class TestWebsiteItem(unittest.TestCase): def test_website_item_desk_item_sync(self): "Check creation/updation/deletion of Website Item and its impact on Item master." web_item = None - item = make_item("Test Web Item") + item = make_item("Test Web Item") # will return item if exists try: web_item = make_website_item(item, save=False) web_item.save() except Exception: - self.fail(f"Error while creating website item for {item.item_code}") + self.fail(f"Error while creating website item for {item}") # check if website item was created self.assertTrue(bool(web_item)) + self.assertTrue(bool(web_item.route)) item.reload() - # check if item was back updated self.assertEqual(web_item.published, 1) - self.assertEqual(item.published_in_website, 1) + self.assertEqual(item.published_in_website, 1) # check if item was back updated self.assertEqual(web_item.item_group, item.item_group) + # check if changing item data changes it in website item + item.item_name = "Test Web Item 1" + item.stock_uom = "Unit" + item.save() + web_item.reload() + self.assertEqual(web_item.item_name, item.item_name) + self.assertEqual(web_item.stock_uom, item.stock_uom) + # check if disabling item unpublished website item item.disabled = 1 item.save() @@ -64,5 +104,287 @@ class TestWebsiteItem(unittest.TestCase): item.reload() self.assertEqual(item.published_in_website, 0) + def test_publish_variant_and_template(self): + "Check if template is published on publishing variant." + template = frappe.get_doc("Item", "Test Web Item") + variant = create_variant("Test Web Item", {"Test Size": "Large"}) + variant.save() + + # check if template is not published + self.assertIsNone(frappe.db.exists("Website Item", {"item_code": variant.variant_of})) + + variant_web_item = make_website_item(variant, save=False) + variant_web_item.save() + + # check if template is published + try: + template_web_item = frappe.get_doc("Website Item", {"item_code": variant.variant_of}) + except DoesNotExistError: + self.fail(f"Template of {variant.item_code}, {variant.variant_of} not published") + + # teardown + variant_web_item.delete() + template_web_item.delete() + variant.delete() + + def test_impact_on_merging_items(self): + "Check if merging items is blocked if old and new items both have website items" + first_item = make_item("Test First Item") + second_item = make_item("Test Second Item") + + first_web_item = make_website_item(first_item, save=False) + first_web_item.save() + second_web_item = make_website_item(second_item, save=False) + second_web_item.save() + + with self.assertRaises(DataValidationError): + frappe.rename_doc("Item", "Test First Item", "Test Second Item", merge=True) + # tear down - item.delete() \ No newline at end of file + second_web_item.delete() + first_web_item.delete() + second_item.delete() + first_item.delete() + + # Website Item Portal Tests Begin + + def test_website_item_breadcrumbs(self): + "Check if breadcrumbs include homepage, product listing navigation page, parent item group(s) and item group." + from erpnext.setup.doctype.item_group.item_group import get_parent_item_groups + + item_code = "Test Breadcrumb Item" + item = make_item(item_code, { + "item_group": "_Test Item Group B - 1", + }) + + if not frappe.db.exists("Website Item", {"item_code": item_code}): + web_item = make_website_item(item, save=False) + web_item.save() + else: + web_item = frappe.get_cached_doc("Website Item", {"item_code": item_code}) + + frappe.db.set_value("Item Group", "_Test Item Group B - 1", "show_in_website", 1) + frappe.db.set_value("Item Group", "_Test Item Group B", "show_in_website", 1) + + breadcrumbs = get_parent_item_groups(item.item_group) + + self.assertEqual(breadcrumbs[0]["name"], "Home") + self.assertEqual(breadcrumbs[1]["name"], "Shop by Category") + self.assertEqual(breadcrumbs[2]["name"], "_Test Item Group B") # parent item group + self.assertEqual(breadcrumbs[3]["name"], "_Test Item Group B - 1") + + # tear down + web_item.delete() + item.delete() + + def test_website_item_price_for_logged_in_user(self): + "Check if price details are fetched correctly while logged in." + item_code = "Test Mobile Phone" + + # show price in e commerce settings + setup_e_commerce_settings({"show_price": 1}) + + # price and pricing rule added via setUp + + # check if price and slashed price is fetched correctly + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertTrue(bool(data.product_info["price"])) + + price_object = data.product_info["price"] + self.assertEqual(price_object.get("discount_percent"), 10) + self.assertEqual(price_object.get("price_list_rate"), 900) + self.assertEqual(price_object.get("formatted_mrp"), "₹ 1,000.00") + self.assertEqual(price_object.get("formatted_price"), "₹ 900.00") + self.assertEqual(price_object.get("formatted_discount_percent"), "10%") + + # disable show price + setup_e_commerce_settings({"show_price": 0}) + + # price should not be fetched + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertFalse(bool(data.product_info["price"])) + + # tear down + frappe.set_user("Administrator") + + def test_website_item_price_for_guest_user(self): + "Check if price details are fetched correctly for guest user." + item_code = "Test Mobile Phone" + + # show price for guest user in e commerce settings + setup_e_commerce_settings({ + "show_price": 1, + "hide_price_for_guest": 0 + }) + + # price and pricing rule added via setUp + + # switch to guest user + frappe.set_user("Guest") + + # price should be fetched + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertTrue(bool(data.product_info["price"])) + + price_object = data.product_info["price"] + self.assertEqual(price_object.get("discount_percent"), 10) + self.assertEqual(price_object.get("price_list_rate"), 900) + + # hide price for guest user + frappe.set_user("Administrator") + setup_e_commerce_settings({"hide_price_for_guest": 1}) + frappe.set_user("Guest") + + # price should not be fetched + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertFalse(bool(data.product_info["price"])) + + # tear down + frappe.set_user("Administrator") + + def test_website_item_stock_when_out_of_stock(self): + """ + Check if stock details are fetched correctly for empty inventory when: + 1) Showing stock availability enabled: + - Warehouse unset + - Warehouse set + 2) Showing stock availability disabled + """ + item_code = "Test Mobile Phone" + self.create_regular_web_item() + setup_e_commerce_settings({"show_stock_availability": 1}) + + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + + # check if stock details are fetched and item not in stock without warehouse set + self.assertFalse(bool(data.product_info["in_stock"])) + self.assertFalse(bool(data.product_info["stock_qty"])) + + # set warehouse + frappe.db.set_value("Website Item", {"item_code": item_code}, "website_warehouse", "_Test Warehouse - _TC") + + # check if stock details are fetched and item not in stock with warehouse set + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertFalse(bool(data.product_info["in_stock"])) + self.assertEqual(data.product_info["stock_qty"][0][0], 0) + + # disable show stock availability + setup_e_commerce_settings({"show_stock_availability": 0}) + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + + # check if stock detail attributes are not fetched if stock availability is hidden + self.assertIsNone(data.product_info.get("in_stock")) + self.assertIsNone(data.product_info.get("stock_qty")) + self.assertIsNone(data.product_info.get("show_stock_qty")) + + # tear down + frappe.get_cached_doc("Website Item", {"item_code": "Test Mobile Phone"}).delete() + + def test_website_item_stock_when_in_stock(self): + """ + Check if stock details are fetched correctly for available inventory when: + 1) Showing stock availability enabled: + - Warehouse set + - Warehouse unset + 2) Showing stock availability disabled + """ + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + item_code = "Test Mobile Phone" + self.create_regular_web_item() + setup_e_commerce_settings({"show_stock_availability": 1}) + + # set warehouse + frappe.db.set_value("Website Item", {"item_code": item_code}, "website_warehouse", "_Test Warehouse - _TC") + + # stock up item + stock_entry = make_stock_entry(item_code=item_code, target="_Test Warehouse - _TC", qty=2, rate=100) + + # check if stock details are fetched and item is in stock with warehouse set + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertTrue(bool(data.product_info["in_stock"])) + self.assertEqual(data.product_info["stock_qty"][0][0], 2) + + # unset warehouse + frappe.db.set_value("Website Item", {"item_code": item_code}, "website_warehouse", "") + + # check if stock details are fetched and item not in stock without warehouse set + # (even though it has stock in some warehouse) + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + self.assertFalse(bool(data.product_info["in_stock"])) + self.assertFalse(bool(data.product_info["stock_qty"])) + + # disable show stock availability + setup_e_commerce_settings({"show_stock_availability": 0}) + frappe.local.shopping_cart_settings = None + data = get_product_info_for_website(item_code, skip_quotation_creation=True) + + # check if stock detail attributes are not fetched if stock availability is hidden + self.assertIsNone(data.product_info.get("in_stock")) + self.assertIsNone(data.product_info.get("stock_qty")) + self.assertIsNone(data.product_info.get("show_stock_qty")) + + # tear down + stock_entry.cancel() + frappe.get_cached_doc("Website Item", {"item_code": "Test Mobile Phone"}).delete() + + + def create_regular_web_item(self): + "Create Regular Item and Website Item." + item_code = "Test Mobile Phone" + item = make_item(item_code) + + if not frappe.db.exists("Website Item", {"item_code": item_code}): + web_item = make_website_item(item, save=False) + web_item.save() + +def make_web_item_price(**kwargs): + item_code = kwargs.get("item_code") + if not item_code: + return + + if not frappe.db.exists("Item Price", {"item_code": item_code}): + item_price = frappe.get_doc({ + "doctype": "Item Price", + "item_code": item_code, + "price_list": kwargs.get("price_list") or "_Test Price List India", + "price_list_rate": kwargs.get("price_list_rate") or 1000 + }) + item_price.insert() + else: + item_price = frappe.get_cached_doc("Item Price", {"item_code": item_code}) + + return item_price + +def make_web_pricing_rule(**kwargs): + title = kwargs.get("title") + if not title: + return + + if not frappe.db.exists("Pricing Rule", title): + pricing_rule = frappe.get_doc({ + "doctype": "Pricing Rule", + "title": title, + "apply_on": kwargs.get("apply_on") or "Item Code", + "items": [{ + "item_code": kwargs.get("item_code") + }], + "selling": kwargs.get("selling") or 0, + "buying": kwargs.get("buying") or 0, + "rate_or_discount": kwargs.get("rate_or_discount") or "Discount Percentage", + "discount_percentage": kwargs.get("discount_percentage") or 10, + "company": kwargs.get("company") or "_Test Company", + "currency": kwargs.get("currency") or "INR", + "for_price_list": kwargs.get("price_list") or "_Test Price List India" + }) + pricing_rule.insert() + else: + pricing_rule = frappe.get_doc("Pricing Rule", {"title": title}) + + return pricing_rule \ 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 3d6d3100c96..b6c70106ea7 100644 --- a/erpnext/e_commerce/doctype/website_item/website_item.py +++ b/erpnext/e_commerce/doctype/website_item/website_item.py @@ -63,7 +63,6 @@ class WebsiteItem(WebsiteGenerator): def on_trash(self): super(WebsiteItem, self).on_trash() - # Delete Item from search index delete_item_from_index(self) self.publish_unpublish_desk_item(publish=False) @@ -85,7 +84,7 @@ class WebsiteItem(WebsiteGenerator): 'route')) + '/' + self.scrub((self.item_name if self.item_name else self.item_code) + '-' + random_string(5)) def update_template_item(self): - """Set Show in Website for Template Item if True for its Variant""" + """Publish Template Item if Variant is published.""" if self.variant_of: if self.published: # show template @@ -109,9 +108,15 @@ class WebsiteItem(WebsiteGenerator): return # find if website image url exists as public - file_doc = frappe.get_all("File", filters={ - "file_url": self.website_image - }, fields=["name", "is_private"], order_by="is_private asc", limit_page_length=1) + file_doc = frappe.get_all( + "File", + filters={ + "file_url": self.website_image + }, + fields=["name", "is_private"], + order_by="is_private asc", + limit_page_length=1 + ) if file_doc: file_doc = file_doc[0] @@ -184,7 +189,7 @@ class WebsiteItem(WebsiteGenerator): context.show_search = True context.search_link = '/search' - context.parents = get_parent_item_groups(self.item_group, from_item=True) + context.parents = get_parent_item_groups(self.item_group, from_item=True) # breadcumbs self.attributes = frappe.get_all("Item Variant Attribute", fields=["attribute", "attribute_value"], filters={"parent": self.item_code}) @@ -204,7 +209,7 @@ class WebsiteItem(WebsiteGenerator): context.recommended_items = None settings = context.shopping_cart.cart_settings - if settings.enable_recommendations: + if settings and settings.enable_recommendations: context.recommended_items = self.get_recommended_items(settings) return context @@ -215,8 +220,9 @@ class WebsiteItem(WebsiteGenerator): # load variants # also used in set_attribute_context - context.variants = frappe.get_all("Item", - filters={"variant_of": self.name, "published_in_website": 1}, + context.variants = frappe.get_all( + "Item", + filters={"variant_of": self.item_code, "published_in_website": 1}, order_by="name asc") variant = frappe.form_dict.variant @@ -267,7 +273,8 @@ class WebsiteItem(WebsiteGenerator): context.selected_attributes[attr.attribute] = attr.attribute_value # filter attributes, order based on attribute table - for attr in self.attributes: + item = frappe.get_cached_doc("Item", self.item_code) + for attr in item.attributes: values = context.attribute_values.setdefault(attr.attribute, []) if cint(frappe.db.get_value("Item Attribute", attr.attribute, "numeric_values")): @@ -345,7 +352,7 @@ class WebsiteItem(WebsiteGenerator): context.metatags.description = safe_description[:300] - context.metatags.title = self.item_name or self.item_code + context.metatags.title = self.web_item_name or self.item_name or self.item_code context.metatags['og:type'] = 'product' context.metatags['og:site_name'] = 'ERPNext' diff --git a/erpnext/e_commerce/doctype/website_offer/website_offer.py b/erpnext/e_commerce/doctype/website_offer/website_offer.py index 59d580eea76..e446f85d65f 100644 --- a/erpnext/e_commerce/doctype/website_offer/website_offer.py +++ b/erpnext/e_commerce/doctype/website_offer/website_offer.py @@ -9,6 +9,6 @@ from frappe.model.document import Document class WebsiteOffer(Document): pass -@frappe.whitelist() +@frappe.whitelist(allow_guest=True) def get_offer_details(offer_id): return frappe.db.get_value('Website Offer', {'name': offer_id}, ['offer_details']) diff --git a/erpnext/e_commerce/product_grid.js b/erpnext/e_commerce/product_grid.js index b46e0d983a3..d5ed649774c 100644 --- a/erpnext/e_commerce/product_grid.js +++ b/erpnext/e_commerce/product_grid.js @@ -144,7 +144,7 @@ erpnext.ProductGrid = class { } get_stock_availability(item, settings) { - if (!item.has_variants && !item.in_stock && settings.show_stock_availability) { + if (settings.show_stock_availability && !item.has_variants && !item.in_stock) { return `${ __("Out of stock") }`; } return ``; diff --git a/erpnext/e_commerce/product_list.js b/erpnext/e_commerce/product_list.js index 822a9efe193..ec877371702 100644 --- a/erpnext/e_commerce/product_list.js +++ b/erpnext/e_commerce/product_list.js @@ -125,7 +125,7 @@ erpnext.ProductList = class { } get_stock_availability(item, settings) { - if (!item.has_variants && !item.in_stock && settings.show_stock_availability) { + if (settings.show_stock_availability && !item.has_variants && !item.in_stock) { return `
${ __("Out of stock") } diff --git a/erpnext/e_commerce/shopping_cart/product_info.py b/erpnext/e_commerce/shopping_cart/product_info.py index c03082681da..e4cf089b67e 100644 --- a/erpnext/e_commerce/shopping_cart/product_info.py +++ b/erpnext/e_commerce/shopping_cart/product_info.py @@ -36,18 +36,22 @@ def get_product_info_for_website(item_code, skip_quotation_creation=False): cart_settings.company ) - stock_status = get_web_item_qty_in_stock(item_code, "website_warehouse") + stock_status = None + if cart_settings.show_stock_availability: + stock_status = get_web_item_qty_in_stock(item_code, "website_warehouse") product_info = { "price": price, - "stock_qty": stock_status.stock_qty, - "in_stock": stock_status.in_stock if stock_status.is_stock_item else get_non_stock_item_status(item_code, "website_warehouse"), "qty": 0, "uom": frappe.db.get_value("Item", item_code, "stock_uom"), - "show_stock_qty": show_quantity_in_website(), "sales_uom": frappe.db.get_value("Item", item_code, "sales_uom") } + if stock_status: + product_info["stock_qty"] = stock_status.stock_qty + product_info["in_stock"] = stock_status.in_stock if stock_status.is_stock_item else get_non_stock_item_status(item_code, "website_warehouse") + product_info["show_stock_qty"] = show_quantity_in_website() + if product_info["price"]: if frappe.session.user != "Guest": item = cart_quotation.get({"item_code": item_code}) if cart_quotation else None diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py index e7d10c7f23d..3bdc56dfe29 100644 --- a/erpnext/setup/doctype/item_group/item_group.py +++ b/erpnext/setup/doctype/item_group/item_group.py @@ -140,17 +140,17 @@ def get_item_for_list_in_html(context): def get_parent_item_groups(item_group_name, from_item=False): - base_nav_page = {"name": frappe._("Shop by Category"), "route":"/shop-by-category"} + base_nav_page = {"name": _("Shop by Category"), "route":"/shop-by-category"} if from_item and frappe.request.environ.get("HTTP_REFERER"): # base page after 'Home' will vary on Item page last_page = frappe.request.environ["HTTP_REFERER"].split('/')[-1] if last_page and last_page in ("shop-by-category", "all-products"): base_nav_page_title = " ".join(last_page.split("-")).title() - base_nav_page = {"name": frappe._(base_nav_page_title), "route":"/"+last_page} + base_nav_page = {"name": _(base_nav_page_title), "route":"/"+last_page} base_parents = [ - {"name": frappe._("Home"), "route":"/"}, + {"name": _("Home"), "route":"/"}, base_nav_page, ] diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 6be862afb69..f162f284fc4 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -29,6 +29,8 @@ class StockExistsForTemplate(frappe.ValidationError): class InvalidBarcode(frappe.ValidationError): pass +class DataValidationError(frappe.ValidationError): + pass class Item(Document): def onload(self): @@ -374,15 +376,8 @@ class Item(Document): frappe.db.set_value("Item", old_name, "item_name", new_name) if merge: - # Validate properties before merging - if not frappe.db.exists("Item", new_name): - frappe.throw(_("Item {0} does not exist").format(new_name)) - - field_list = ["stock_uom", "is_stock_item", "has_serial_no", "has_batch_no"] - new_properties = [cstr(d) for d in frappe.db.get_value("Item", new_name, field_list)] - if new_properties != [cstr(self.get(fld)) for fld in field_list]: - frappe.throw(_("To merge, following properties must be same for both items") - + ": \n" + ", ".join([self.meta.get_label(fld) for fld in field_list])) + self.validate_properties_before_merge(new_name) + self.validate_duplicate_website_item_before_merge(old_name, new_name) def after_rename(self, old_name, new_name, merge): if merge: @@ -429,7 +424,41 @@ class Item(Document): msg += _("Note: To merge the items, create a separate Stock Reconciliation for the old item {0}").format( frappe.bold(old_name)) - frappe.throw(_(msg), title=_("Merge not allowed")) + frappe.throw(_(msg), title=_("Cannot Merge"), exc=DataValidationError) + + def validate_properties_before_merge(self, new_name): + # Validate properties before merging + if not frappe.db.exists("Item", new_name): + frappe.throw(_("Item {0} does not exist").format(new_name)) + + field_list = ["stock_uom", "is_stock_item", "has_serial_no", "has_batch_no"] + new_properties = [cstr(d) for d in frappe.db.get_value("Item", new_name, field_list)] + + if new_properties != [cstr(self.get(field)) for field in field_list]: + msg = _("To merge, following properties must be same for both items") + msg += ": \n" + ", ".join([self.meta.get_label(fld) for fld in field_list]) + frappe.throw(msg, title=_("Cannot Merge"), exc=DataValidationError) + + def validate_duplicate_website_item_before_merge(self, old_name, new_name): + """ + Block merge if both old and new items have website items against them. + This is to avoid duplicate website items after merging. + """ + web_items = frappe.get_all( + "Website Item", + filters={ + "item_code": ["in", [old_name, new_name]] + }, + fields=["item_code", "name"]) + + if len(web_items) <= 1: + return + + old_web_item = [d.get("name") for d in web_items if d.get("item_code") == old_name][0] + web_item_link = get_link_to_form("Website Item", old_web_item) + + msg = f"Please delete linked Website Item {frappe.bold(web_item_link)} before merging {old_name} and {new_name}" + frappe.throw(_(msg), title=_("Cannot Merge"), exc=DataValidationError) def set_last_purchase_rate(self, new_name): last_purchase_rate = get_last_purchase_details(new_name).get("base_net_rate", 0) diff --git a/erpnext/templates/generators/item/item_configure.js b/erpnext/templates/generators/item/item_configure.js index d7b8d328c35..f47650a27e2 100644 --- a/erpnext/templates/generators/item/item_configure.js +++ b/erpnext/templates/generators/item/item_configure.js @@ -201,7 +201,7 @@ class ItemConfigure { ${frappe.utils.icon('assets', 'md')} - ${__("Add to Cart")}s + ${__("Add to Cart")} ` : '';