From 5607762c0fdc7496f36136a461318b5a80a5cdba Mon Sep 17 00:00:00 2001 From: Andrew McLeod Date: Fri, 16 Nov 2018 11:08:39 +0000 Subject: [PATCH 1/2] fix/feat: Allow extension of barcode types (without validation) Currently, it is difficult to add new custom barcode types for two reasons, both of which relate to validate_barcode in item.py: - There is a bug where barcode types with a space in, such as Code 128, are split in two (so barcode_type is checked against 'Code' and '128' rather than 'Code 128'). This is fixed by splitting the Options field against a newline, instead of spaces. - All barcodes are validated against the stdnum.ean library. This only handles EAN-8, EAN-13 and UPC-12 barcodes and any other barcode will fail. Barcodes with no type will continue to not be checked. Barcodes with the default barcode_types of EAN, UPC will continue to be checked. The non-default barcode_types of EAN-13 and EAN-8 will also be checked. The barcode_type is cast to upper case before this check is made so ean, upc, ean-13 and ean-8 will also be validated. This allows people to add their own barcode types, such as Code 128 and QR codes. Users can add custom validation of these barcodes using the usual hooks, but they cannot remove the standard validation. --- erpnext/stock/doctype/item/item.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 17cf044074..294e26d80f 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -502,7 +502,7 @@ class Item(WebsiteGenerator): from stdnum import ean if len(self.barcodes) > 0: for item_barcode in self.barcodes: - options = frappe.get_meta("Item Barcode").get_options("barcode_type").split() + options = frappe.get_meta("Item Barcode").get_options("barcode_type").split('\n') if item_barcode.barcode: duplicate = frappe.db.sql( """select parent from `tabItem Barcode` where barcode = %s and parent != %s""", (item_barcode.barcode, self.name)) @@ -511,7 +511,7 @@ class Item(WebsiteGenerator): item_barcode.barcode, duplicate[0][0])) item_barcode.barcode_type = "" if item_barcode.barcode_type not in options else item_barcode.barcode_type - if item_barcode.barcode_type: + if item_barcode.barcode_type and item_barcode.barcode_type.upper() in ('EAN', 'UPC-A', 'EAN-13', 'EAN-8'): if not ean.is_valid(item_barcode.barcode): frappe.throw(_("Barcode {0} is not a valid {1} code").format( item_barcode.barcode, item_barcode.barcode_type)) From f99a68a69507173f3cdfc814cb1010cc7cdb7773 Mon Sep 17 00:00:00 2001 From: Andrew McLeod Date: Fri, 16 Nov 2018 13:55:55 +0000 Subject: [PATCH 2/2] feat: Added unit testing for Item Barcodes in item.py Adds three different barcodes and barcodes types to a test item and checks that they are added correctly. Adds a barcode that already exists, and checks a DuplicateEntryError is raised. Adds an invalid EAN barcode and checks InvalidBarcode (a subclass of ValidationError) is raised. --- erpnext/stock/doctype/item/item.py | 8 +++- erpnext/stock/doctype/item/test_item.py | 61 ++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 294e26d80f..566b6386fe 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -32,6 +32,10 @@ class StockExistsForTemplate(frappe.ValidationError): pass +class InvalidBarcode(frappe.ValidationError): + pass + + class Item(WebsiteGenerator): website = frappe._dict( page_title_field="item_name", @@ -508,13 +512,13 @@ class Item(WebsiteGenerator): """select parent from `tabItem Barcode` where barcode = %s and parent != %s""", (item_barcode.barcode, self.name)) if duplicate: frappe.throw(_("Barcode {0} already used in Item {1}").format( - item_barcode.barcode, duplicate[0][0])) + item_barcode.barcode, duplicate[0][0]), frappe.DuplicateEntryError) item_barcode.barcode_type = "" if item_barcode.barcode_type not in options else item_barcode.barcode_type if item_barcode.barcode_type and item_barcode.barcode_type.upper() in ('EAN', 'UPC-A', 'EAN-13', 'EAN-8'): if not ean.is_valid(item_barcode.barcode): frappe.throw(_("Barcode {0} is not a valid {1} code").format( - item_barcode.barcode, item_barcode.barcode_type)) + item_barcode.barcode, item_barcode.barcode_type), InvalidBarcode) def validate_warehouse_for_reorder(self): '''Validate Reorder level table for duplicate and conditional mandatory''' diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 7ef4f8cecc..24292f7b4f 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -8,7 +8,7 @@ import frappe from frappe.test_runner import make_test_objects from erpnext.controllers.item_variant import (create_variant, ItemVariantExistsError, InvalidItemAttributeValueError, get_variant) -from erpnext.stock.doctype.item.item import StockExistsForTemplate +from erpnext.stock.doctype.item.item import StockExistsForTemplate, InvalidBarcode from erpnext.stock.doctype.item.item import get_uom_conv_factor from frappe.model.rename_doc import rename_doc from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry @@ -305,6 +305,65 @@ class TestItem(unittest.TestCase): item_doc.has_variants = 1 self.assertRaises(StockExistsForTemplate, item_doc.save) + def test_add_item_barcode(self): + # Clean up + frappe.db.sql("""delete from `tabItem Barcode`""") + item_code = "Test Item Barcode" + if frappe.db.exists("Item", item_code): + frappe.delete_doc("Item", item_code) + + # Create new item and add barcodes + barcode_properties_list = [ + { + "barcode": "0012345678905", + "barcode_type": "EAN" + }, + { + "barcode": "012345678905", + "barcode_type": "UAN" + }, + { + "barcode": "ARBITRARY_TEXT", + } + ] + create_item(item_code) + for barcode_properties in barcode_properties_list: + item_doc = frappe.get_doc('Item', item_code) + new_barcode = item_doc.append('barcodes') + new_barcode.update(barcode_properties) + item_doc.save() + + # Check values saved correctly + barcodes = frappe.get_list( + 'Item Barcode', + fields=['barcode', 'barcode_type'], + filters={'parent': item_code}) + + for barcode_properties in barcode_properties_list: + barcode_to_find = barcode_properties['barcode'] + matching_barcodes = [ + x for x in barcodes + if x['barcode'] == barcode_to_find + ] + self.assertEqual(len(matching_barcodes), 1) + details = matching_barcodes[0] + + for key, value in iteritems(barcode_properties): + self.assertEqual(value, details.get(key)) + + # Add barcode again - should cause DuplicateEntryError + item_doc = frappe.get_doc('Item', item_code) + new_barcode = item_doc.append('barcodes') + new_barcode.update(barcode_properties_list[0]) + self.assertRaises(frappe.DuplicateEntryError, item_doc.save) + + # Add invalid barcode - should cause InvalidBarcode + item_doc = frappe.get_doc('Item', item_code) + new_barcode = item_doc.append('barcodes') + new_barcode.barcode = '9999999999999' + new_barcode.barcode_type = 'EAN' + self.assertRaises(InvalidBarcode, item_doc.save) + def set_item_variant_settings(fields): doc = frappe.get_doc('Item Variant Settings') doc.set('fields', fields)