diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index 84929b5b4c2..a4f5eb4d92d 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -37,6 +37,7 @@ class Asset(AccountsController): self.validate_asset_values() self.validate_asset_and_reference() self.validate_item() + self.validate_cost_center() self.set_missing_values() self.prepare_depreciation_data() self.validate_gross_and_purchase_amount() @@ -96,6 +97,19 @@ class Asset(AccountsController): elif item.is_stock_item: frappe.throw(_("Item {0} must be a non-stock item").format(self.item_code)) + def validate_cost_center(self): + if not self.cost_center: return + + cost_center_company = frappe.db.get_value('Cost Center', self.cost_center, 'company') + if cost_center_company != self.company: + frappe.throw( + _("Selected Cost Center {} doesn't belongs to {}").format( + frappe.bold(self.cost_center), + frappe.bold(self.company) + ), + title=_("Invalid Cost Center") + ) + def validate_in_use_date(self): if not self.available_for_use_date: frappe.throw(_("Available for use date is required")) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index 0ddfb6c1c02..a15a1b6f388 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -1131,6 +1131,15 @@ class TestDepreciationBasics(AssetSetup): self.assertEqual(gle, expected_gle) self.assertEqual(asset.get("value_after_depreciation"), 0) + def test_asset_cost_center(self): + asset = create_asset(is_existing_asset = 1, do_not_save=1) + asset.cost_center = "Main - WP" + + self.assertRaises(frappe.ValidationError, asset.submit) + + asset.cost_center = "Main - _TC" + asset.submit() + def create_asset_data(): if not frappe.db.exists("Asset Category", "Computers"): create_asset_category() diff --git a/erpnext/manufacturing/doctype/bom/bom.json b/erpnext/manufacturing/doctype/bom/bom.json index 218ac64d8da..0b441969400 100644 --- a/erpnext/manufacturing/doctype/bom/bom.json +++ b/erpnext/manufacturing/doctype/bom/bom.json @@ -37,7 +37,6 @@ "inspection_required", "quality_inspection_template", "column_break_31", - "bom_level", "section_break_33", "items", "scrap_section", @@ -522,13 +521,6 @@ "fieldname": "column_break_31", "fieldtype": "Column Break" }, - { - "default": "0", - "fieldname": "bom_level", - "fieldtype": "Int", - "label": "BOM Level", - "read_only": 1 - }, { "fieldname": "section_break_33", "fieldtype": "Section Break", @@ -540,7 +532,7 @@ "image_field": "image", "is_submittable": 1, "links": [], - "modified": "2021-11-18 13:04:16.271975", + "modified": "2022-01-30 21:27:54.727298", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM", @@ -577,5 +569,6 @@ "show_name_in_global_search": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 0b5207ef859..b97dcab632f 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -156,8 +156,6 @@ class BOM(WebsiteGenerator): self.update_stock_qty() self.validate_scrap_items() self.update_cost(update_parent=False, from_child_bom=True, update_hour_rate = False, save=False) - self.set_bom_level() - def get_context(self, context): context.parents = [{'name': 'boms', 'title': _('All BOMs') }] @@ -704,7 +702,6 @@ class BOM(WebsiteGenerator): if not d.batch_size or d.batch_size <= 0: d.batch_size = 1 - def validate_scrap_items(self): for item in self.scrap_items: msg = "" @@ -735,20 +732,6 @@ class BOM(WebsiteGenerator): """Get a complete tree representation preserving order of child items.""" return BOMTree(self.name) - def set_bom_level(self, update=False): - levels = [] - - self.bom_level = 0 - for row in self.items: - if row.bom_no: - levels.append(frappe.get_cached_value("BOM", row.bom_no, "bom_level") or 0) - - if levels: - self.bom_level = max(levels) + 1 - - if update: - self.db_set("bom_level", self.bom_level) - def get_bom_item_rate(args, bom_doc): if bom_doc.rm_cost_as_per == 'Valuation Rate': rate = get_valuation_rate(args) * (args.get("conversion_factor") or 1) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 106777b82ef..f06624fe92c 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -560,9 +560,11 @@ class ProductionPlan(Document): get_sub_assembly_items(row.bom_no, bom_data, row.planned_qty) self.set_sub_assembly_items_based_on_level(row, bom_data, manufacturing_type) - def set_sub_assembly_items_based_on_level(self, row, bom_data, manufacturing_type=None): - bom_data = sorted(bom_data, key = lambda i: i.bom_level) + self.sub_assembly_items.sort(key= lambda d: d.bom_level, reverse=True) + for idx, row in enumerate(self.sub_assembly_items, start=1): + row.idx = idx + def set_sub_assembly_items_based_on_level(self, row, bom_data, manufacturing_type=None): for data in bom_data: data.qty = data.stock_qty data.production_plan_item = row.name @@ -1005,9 +1007,6 @@ def get_sub_assembly_items(bom_no, bom_data, to_produce_qty, indent=0): for d in data: if d.expandable: parent_item_code = frappe.get_cached_value("BOM", bom_no, "item") - bom_level = (frappe.get_cached_value("BOM", d.value, "bom_level") - if d.value else 0) - stock_qty = (d.stock_qty / d.parent_bom_qty) * flt(to_produce_qty) bom_data.append(frappe._dict({ 'parent_item_code': parent_item_code, @@ -1018,7 +1017,7 @@ def get_sub_assembly_items(bom_no, bom_data, to_produce_qty, indent=0): 'uom': d.stock_uom, 'bom_no': d.value, 'is_sub_contracted_item': d.is_sub_contracted_item, - 'bom_level': bom_level, + 'bom_level': indent, 'indent': indent, 'stock_qty': stock_qty })) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 2febc1e23c0..21a126b2a79 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -347,6 +347,45 @@ class TestProductionPlan(ERPNextTestCase): frappe.db.rollback() + def test_subassmebly_sorting(self): + """ Test subassembly sorting in case of multiple items with nested BOMs""" + from erpnext.manufacturing.doctype.bom.test_bom import create_nested_bom + + prefix = "_TestLevel_" + boms = { + "Assembly": { + "SubAssembly1": {"ChildPart1": {}, "ChildPart2": {},}, + "SubAssembly2": {"ChildPart3": {}}, + "SubAssembly3": {"SubSubAssy1": {"ChildPart4": {}}}, + "ChildPart5": {}, + "ChildPart6": {}, + "SubAssembly4": {"SubSubAssy2": {"ChildPart7": {}}}, + }, + "MegaDeepAssy": { + "SecretSubassy": {"SecretPart": {"VerySecret" : { "SuperSecret": {"Classified": {}}}},}, + # ^ assert that this is + # first item in subassy table + } + } + create_nested_bom(boms, prefix=prefix) + + items = [prefix + item_code for item_code in boms.keys()] + plan = create_production_plan(item_code=items[0], do_not_save=True) + plan.append("po_items", { + 'use_multi_level_bom': 1, + 'item_code': items[1], + 'bom_no': frappe.db.get_value('Item', items[1], 'default_bom'), + 'planned_qty': 1, + 'planned_start_date': now_datetime() + }) + plan.get_sub_assembly_items() + + bom_level_order = [d.bom_level for d in plan.sub_assembly_items] + self.assertEqual(bom_level_order, sorted(bom_level_order, reverse=True)) + # lowest most level of subassembly should be first + self.assertIn("SuperSecret", plan.sub_assembly_items[0].production_item) + + def create_production_plan(**args): args = frappe._dict(args) diff --git a/erpnext/manufacturing/doctype/production_plan_sub_assembly_item/production_plan_sub_assembly_item.json b/erpnext/manufacturing/doctype/production_plan_sub_assembly_item/production_plan_sub_assembly_item.json index 657ee35a852..45ea26c3a8a 100644 --- a/erpnext/manufacturing/doctype/production_plan_sub_assembly_item/production_plan_sub_assembly_item.json +++ b/erpnext/manufacturing/doctype/production_plan_sub_assembly_item/production_plan_sub_assembly_item.json @@ -102,7 +102,6 @@ }, { "columns": 1, - "fetch_from": "bom_no.bom_level", "fieldname": "bom_level", "fieldtype": "Int", "in_list_view": 1, @@ -189,7 +188,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2021-06-28 20:10:56.296410", + "modified": "2022-01-30 21:31:10.527559", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan Sub Assembly Item", @@ -198,5 +197,6 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py b/erpnext/manufacturing/report/bom_explorer/bom_explorer.py index 25de2e03797..19a80ab4076 100644 --- a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py +++ b/erpnext/manufacturing/report/bom_explorer/bom_explorer.py @@ -26,8 +26,7 @@ def get_exploded_items(bom, data, indent=0, qty=1): 'item_code': item.item_code, 'item_name': item.item_name, 'indent': indent, - 'bom_level': (frappe.get_cached_value("BOM", item.bom_no, "bom_level") - if item.bom_no else ""), + 'bom_level': indent, 'bom': item.bom_no, 'qty': item.qty * qty, 'uom': item.uom, @@ -73,7 +72,7 @@ def get_columns(): }, { "label": "BOM Level", - "fieldtype": "Data", + "fieldtype": "Int", "fieldname": "bom_level", "width": 100 }, diff --git a/erpnext/manufacturing/report/production_plan_summary/production_plan_summary.py b/erpnext/manufacturing/report/production_plan_summary/production_plan_summary.py index 55b1a3f2f9a..aaa231466fd 100644 --- a/erpnext/manufacturing/report/production_plan_summary/production_plan_summary.py +++ b/erpnext/manufacturing/report/production_plan_summary/production_plan_summary.py @@ -48,7 +48,7 @@ def get_production_plan_item_details(filters, data, order_details): "qty": row.planned_qty, "document_type": "Work Order", "document_name": work_order or "", - "bom_level": frappe.get_cached_value("BOM", row.bom_no, "bom_level"), + "bom_level": 0, "produced_qty": order_details.get((work_order, row.item_code), {}).get("produced_qty", 0), "pending_qty": flt(row.planned_qty) - flt(order_details.get((work_order, row.item_code), {}).get("produced_qty", 0)) }) diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 704b6696db1..bcc0c019bf1 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -1,4 +1,5 @@ erpnext.patches.v12_0.update_is_cancelled_field +erpnext.patches.v13_0.add_bin_unique_constraint erpnext.patches.v11_0.rename_production_order_to_work_order erpnext.patches.v11_0.refactor_naming_series erpnext.patches.v11_0.refactor_autoname_naming @@ -291,7 +292,6 @@ erpnext.patches.v13_0.set_training_event_attendance erpnext.patches.v13_0.rename_issue_status_hold_to_on_hold erpnext.patches.v13_0.bill_for_rejected_quantity_in_purchase_invoice erpnext.patches.v13_0.update_job_card_details -erpnext.patches.v13_0.update_level_in_bom #1234sswef erpnext.patches.v13_0.create_gst_payment_entry_fields #27-11-2021 erpnext.patches.v13_0.add_missing_fg_item_for_stock_entry erpnext.patches.v13_0.update_subscription_status_in_memberships diff --git a/erpnext/patches/v13_0/add_bin_unique_constraint.py b/erpnext/patches/v13_0/add_bin_unique_constraint.py new file mode 100644 index 00000000000..57fbaae9d8d --- /dev/null +++ b/erpnext/patches/v13_0/add_bin_unique_constraint.py @@ -0,0 +1,63 @@ +import frappe + +from erpnext.stock.stock_balance import ( + get_balance_qty_from_sle, + get_indented_qty, + get_ordered_qty, + get_planned_qty, + get_reserved_qty, +) +from erpnext.stock.utils import get_bin + + +def execute(): + delete_broken_bins() + delete_and_patch_duplicate_bins() + +def delete_broken_bins(): + # delete useless bins + frappe.db.sql("delete from `tabBin` where item_code is null or warehouse is null") + +def delete_and_patch_duplicate_bins(): + + duplicate_bins = frappe.db.sql(""" + SELECT + item_code, warehouse, count(*) as bin_count + FROM + tabBin + GROUP BY + item_code, warehouse + HAVING + bin_count > 1 + """, as_dict=1) + + for duplicate_bin in duplicate_bins: + item_code = duplicate_bin.item_code + warehouse = duplicate_bin.warehouse + existing_bins = frappe.get_list("Bin", + filters={ + "item_code": item_code, + "warehouse": warehouse + }, + fields=["name"], + order_by="creation",) + + # keep last one + existing_bins.pop() + + for broken_bin in existing_bins: + frappe.delete_doc("Bin", broken_bin.name) + + qty_dict = { + "reserved_qty": get_reserved_qty(item_code, warehouse), + "indented_qty": get_indented_qty(item_code, warehouse), + "ordered_qty": get_ordered_qty(item_code, warehouse), + "planned_qty": get_planned_qty(item_code, warehouse), + "actual_qty": get_balance_qty_from_sle(item_code, warehouse) + } + + bin = get_bin(item_code, warehouse) + bin.update(qty_dict) + bin.update_reserved_qty_for_production() + bin.update_reserved_qty_for_sub_contracting() + bin.db_update() diff --git a/erpnext/patches/v13_0/update_level_in_bom.py b/erpnext/patches/v13_0/update_level_in_bom.py deleted file mode 100644 index 499412ee270..00000000000 --- a/erpnext/patches/v13_0/update_level_in_bom.py +++ /dev/null @@ -1,31 +0,0 @@ -# Copyright (c) 2020, Frappe and Contributors -# License: GNU General Public License v3. See license.txt - - -import frappe - - -def execute(): - for document in ["bom", "bom_item", "bom_explosion_item"]: - frappe.reload_doc('manufacturing', 'doctype', document) - - frappe.db.sql(" update `tabBOM` set bom_level = 0 where docstatus = 1") - - bom_list = frappe.db.sql_list("""select name from `tabBOM` bom - where docstatus=1 and is_active=1 and not exists(select bom_no from `tabBOM Item` - where parent=bom.name and ifnull(bom_no, '')!='')""") - - count = 0 - while(count < len(bom_list)): - for parent_bom in get_parent_boms(bom_list[count]): - bom_doc = frappe.get_cached_doc("BOM", parent_bom) - bom_doc.set_bom_level(update=True) - bom_list.append(parent_bom) - count += 1 - -def get_parent_boms(bom_no): - return frappe.db.sql_list(""" - select distinct bom_item.parent from `tabBOM Item` bom_item - where bom_item.bom_no = %s and bom_item.docstatus=1 and bom_item.parenttype='BOM' - and exists(select bom.name from `tabBOM` bom where bom.name=bom_item.parent and bom.is_active=1) - """, bom_no) diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json index 8e79f0e5552..56dc71c57e1 100644 --- a/erpnext/stock/doctype/bin/bin.json +++ b/erpnext/stock/doctype/bin/bin.json @@ -33,6 +33,7 @@ "oldfieldtype": "Link", "options": "Warehouse", "read_only": 1, + "reqd": 1, "search_index": 1 }, { @@ -46,6 +47,7 @@ "oldfieldtype": "Link", "options": "Item", "read_only": 1, + "reqd": 1, "search_index": 1 }, { @@ -169,10 +171,11 @@ "idx": 1, "in_create": 1, "links": [], - "modified": "2021-03-30 23:09:39.572776", + "modified": "2022-01-30 17:04:54.715288", "modified_by": "Administrator", "module": "Stock", "name": "Bin", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -200,5 +203,6 @@ "quick_entry": 1, "search_fields": "item_code,warehouse", "sort_field": "modified", - "sort_order": "ASC" + "sort_order": "ASC", + "states": [] } \ No newline at end of file diff --git a/erpnext/stock/doctype/bin/bin.py b/erpnext/stock/doctype/bin/bin.py index 11ff359b483..1d874cd06fb 100644 --- a/erpnext/stock/doctype/bin/bin.py +++ b/erpnext/stock/doctype/bin/bin.py @@ -96,7 +96,7 @@ class Bin(Document): self.db_set('projected_qty', self.projected_qty) def on_doctype_update(): - frappe.db.add_index("Bin", ["item_code", "warehouse"]) + frappe.db.add_unique("Bin", ["item_code", "warehouse"], constraint_name="unique_item_warehouse") def update_stock(bin_name, args, allow_negative_stock=False, via_landed_cost_voucher=False): diff --git a/erpnext/stock/doctype/bin/test_bin.py b/erpnext/stock/doctype/bin/test_bin.py index 9c390d94b4e..250126c6b98 100644 --- a/erpnext/stock/doctype/bin/test_bin.py +++ b/erpnext/stock/doctype/bin/test_bin.py @@ -1,9 +1,36 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -import unittest +import frappe -# test_records = frappe.get_test_records('Bin') +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.stock.utils import _create_bin +from erpnext.tests.utils import ERPNextTestCase -class TestBin(unittest.TestCase): - pass + +class TestBin(ERPNextTestCase): + + + def test_concurrent_inserts(self): + """ Ensure no duplicates are possible in case of concurrent inserts""" + item_code = "_TestConcurrentBin" + make_item(item_code) + warehouse = "_Test Warehouse - _TC" + + bin1 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse) + bin1.insert() + + bin2 = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse) + with self.assertRaises(frappe.UniqueValidationError): + bin2.insert() + + # util method should handle it + bin = _create_bin(item_code, warehouse) + self.assertEqual(bin.item_code, item_code) + + frappe.db.rollback() + + def test_index_exists(self): + indexes = frappe.db.sql("show index from tabBin where Non_unique = 0", as_dict=1) + if not any(index.get("Key_name") == "unique_item_warehouse" for index in indexes): + self.fail(f"Expected unique index on item-warehouse") diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js index e346ea87214..da371d968c9 100644 --- a/erpnext/stock/doctype/item/item.js +++ b/erpnext/stock/doctype/item/item.js @@ -380,8 +380,7 @@ $.extend(erpnext.item, { // Show Stock Levels only if is_stock_item if (frm.doc.is_stock_item) { frappe.require('assets/js/item-dashboard.min.js', function() { - frm.dashboard.parent.find('.stock-levels').remove(); - const section = frm.dashboard.add_section('', __("Stock Levels"), 'stock-levels'); + const section = frm.dashboard.add_section('', __("Stock Levels")); erpnext.item.item_dashboard = new erpnext.stock.ItemDashboard({ parent: section, item_code: frm.doc.name, diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index ab106e036f1..a5bf2397411 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1674,6 +1674,8 @@ class StockEntry(StockController): for d in self.get("items"): item_code = d.get('original_item') or d.get('item_code') reserve_warehouse = item_wh.get(item_code) + if not (reserve_warehouse and item_code): + continue stock_bin = get_bin(item_code, reserve_warehouse) stock_bin.update_reserved_qty_for_sub_contracting() diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 4d77367a051..b8bdf39301e 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -177,13 +177,7 @@ def get_latest_stock_balance(): def get_bin(item_code, warehouse): bin = frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": warehouse}) if not bin: - bin_obj = frappe.get_doc({ - "doctype": "Bin", - "item_code": item_code, - "warehouse": warehouse, - }) - bin_obj.flags.ignore_permissions = 1 - bin_obj.insert() + bin_obj = _create_bin(item_code, warehouse) else: bin_obj = frappe.get_doc('Bin', bin, for_update=True) bin_obj.flags.ignore_permissions = True @@ -193,16 +187,24 @@ def get_or_make_bin(item_code: str , warehouse: str) -> str: bin_record = frappe.db.get_value('Bin', {'item_code': item_code, 'warehouse': warehouse}) if not bin_record: - bin_obj = frappe.get_doc({ - "doctype": "Bin", - "item_code": item_code, - "warehouse": warehouse, - }) + bin_obj = _create_bin(item_code, warehouse) + bin_record = bin_obj.name + return bin_record + +def _create_bin(item_code, warehouse): + """Create a bin and take care of concurrent inserts.""" + + bin_creation_savepoint = "create_bin" + try: + frappe.db.savepoint(bin_creation_savepoint) + bin_obj = frappe.get_doc(doctype="Bin", item_code=item_code, warehouse=warehouse) bin_obj.flags.ignore_permissions = 1 bin_obj.insert() - bin_record = bin_obj.name + except frappe.UniqueValidationError: + frappe.db.rollback(save_point=bin_creation_savepoint) # preserve transaction in postgres + bin_obj = frappe.get_last_doc("Bin", {"item_code": item_code, "warehouse": warehouse}) - return bin_record + return bin_obj def update_bin(args, allow_negative_stock=False, via_landed_cost_voucher=False): """WARNING: This function is deprecated. Inline this function instead of using it."""