diff --git a/erpnext/controllers/subcontracting_controller.py b/erpnext/controllers/subcontracting_controller.py index 468137a1cf1..a78b45f4d86 100644 --- a/erpnext/controllers/subcontracting_controller.py +++ b/erpnext/controllers/subcontracting_controller.py @@ -151,7 +151,7 @@ class SubcontractingController(StockController): ).format(item.idx, get_link_to_form("Item", item.item_code)) ) - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item"): + if not item.get("secondary_item_type") and not item.get("valuation_type"): if not is_sub_contracted_item: frappe.throw( _("Row {0}: Item {1} must be a subcontracted item.").format(item.idx, item.item_name) @@ -1248,10 +1248,10 @@ class SubcontractingController(StockController): total_amt = sum( flt(item.amount) for item in self.get("items") - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item") + if not item.get("secondary_item_type") and not item.get("valuation_type") ) for item in self.items: - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item"): + if not item.get("secondary_item_type") and not item.get("valuation_type"): item.additional_cost_per_qty = ( (item.amount * self.total_additional_costs) / total_amt ) / item.qty @@ -1259,15 +1259,15 @@ class SubcontractingController(StockController): total_qty = sum( flt(item.qty) for item in self.get("items") - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item") + if not item.get("secondary_item_type") and not item.get("valuation_type") ) additional_cost_per_qty = self.total_additional_costs / total_qty for item in self.items: - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item"): + if not item.get("secondary_item_type") and not item.get("valuation_type"): item.additional_cost_per_qty = additional_cost_per_qty else: for item in self.items: - if not item.get("secondary_item_type") and not item.get("is_legacy_scrap_item"): + if not item.get("secondary_item_type") and not item.get("valuation_type"): item.additional_cost_per_qty = 0 @frappe.whitelist() diff --git a/erpnext/controllers/subcontracting_inward_controller.py b/erpnext/controllers/subcontracting_inward_controller.py index 96b63f365b2..7cf6ca5e40f 100644 --- a/erpnext/controllers/subcontracting_inward_controller.py +++ b/erpnext/controllers/subcontracting_inward_controller.py @@ -243,7 +243,7 @@ class SubcontractingInwardController: for item in self.get("items") if not item.is_finished_item and not item.secondary_item_type - and not item.is_legacy_scrap_item + and not item.valuation_type and frappe.get_cached_value("Item", item.item_code, "is_customer_provided_item") ] @@ -380,7 +380,7 @@ class SubcontractingInwardController: if self.purpose in ["Subcontracting Delivery", "Subcontracting Return", "Manufacture"]: for item in self.items: if ( - item.is_finished_item or item.secondary_item_type or item.is_legacy_scrap_item + item.is_finished_item or item.secondary_item_type or item.valuation_type ) and item.valuation_rate == 0: item.allow_zero_valuation_rate = 1 @@ -480,7 +480,7 @@ class SubcontractingInwardController: self.validate_delivery_on_save() else: for item in self.items: - if not item.secondary_item_type and not item.is_legacy_scrap_item: + if not item.secondary_item_type and not item.valuation_type: delivered_qty, returned_qty = frappe.get_value( "Subcontracting Inward Order Item", item.scio_detail, @@ -550,7 +550,7 @@ class SubcontractingInwardController: bold( frappe.get_cached_value( "Subcontracting Inward Order Item" - if not item.secondary_item_type and not item.is_legacy_scrap_item + if not item.secondary_item_type and not item.valuation_type else "Subcontracting Inward Order Secondary Item", item.scio_detail, "stock_uom", @@ -602,7 +602,7 @@ class SubcontractingInwardController: ) for item in [item for item in self.items if not item.is_finished_item]: - if item.secondary_item_type or item.is_legacy_scrap_item: + if item.secondary_item_type or item.valuation_type: scio_secondary_item = frappe.get_value( "Subcontracting Inward Order Secondary Item", { @@ -661,7 +661,7 @@ class SubcontractingInwardController: for item in self.items: doctype = ( "Subcontracting Inward Order Item" - if not item.secondary_item_type and not item.is_legacy_scrap_item + if not item.secondary_item_type and not item.valuation_type else "Subcontracting Inward Order Secondary Item" ) qty_map[doctype][item.scio_detail] += ( @@ -802,7 +802,7 @@ class SubcontractingInwardController: items = [ item for item in self.items - if not item.is_finished_item and not item.secondary_item_type and not item.is_legacy_scrap_item + if not item.is_finished_item and not item.secondary_item_type and not item.valuation_type ] if not items: return @@ -913,7 +913,7 @@ class SubcontractingInwardController: def update_inward_order_secondary_items(self): if (scio := self.subcontracting_inward_order) and self.purpose == "Manufacture": secondary_items_list = [ - item for item in self.items if item.secondary_item_type or item.is_legacy_scrap_item + item for item in self.items if item.secondary_item_type or item.valuation_type ] secondary_items = defaultdict(float) diff --git a/erpnext/manufacturing/doctype/bom/bom.js b/erpnext/manufacturing/doctype/bom/bom.js index 86b8e2e83df..7a3fe82e2c7 100644 --- a/erpnext/manufacturing/doctype/bom/bom.js +++ b/erpnext/manufacturing/doctype/bom/bom.js @@ -767,6 +767,7 @@ var get_bom_material_detail = function (doc, cdt, cdn, secondary_items) { conversion_factor: d.conversion_factor, sourced_by_supplier: d.sourced_by_supplier, do_not_explode: d.do_not_explode, + source_warehouse: d.source_warehouse || doc.default_source_warehouse, fetch_rate: !secondary_items, }, callback: function (r) { @@ -777,6 +778,10 @@ var get_bom_material_detail = function (doc, cdt, cdn, secondary_items) { doc = locals[doc.doctype][doc.name]; erpnext.bom.calculate_rm_cost(doc); erpnext.bom.calculate_total(doc); + + if (secondary_items && d.valuation_type === "Valuation Rate") { + erpnext.bom.fetch_secondary_item_cost(doc, cdt, cdn); + } }, freeze: true, }); @@ -790,11 +795,10 @@ cur_frm.cscript.qty = function (doc) { cur_frm.cscript.rate = function (doc, cdt, cdn) { var d = locals[cdt][cdn]; - const is_secondary_item = cdt == "BOM Secondary Item"; if (d.bom_no) { frappe.msgprint(__("You cannot change the rate if BOM is mentioned against any Item.")); - get_bom_material_detail(doc, cdt, cdn, is_secondary_item); + get_bom_material_detail(doc, cdt, cdn, false); } else { erpnext.bom.calculate_rm_cost(doc); erpnext.bom.calculate_total(doc); @@ -957,6 +961,9 @@ frappe.ui.form.on("BOM Item", { do_not_explode: function (frm, cdt, cdn) { get_bom_material_detail(frm.doc, cdt, cdn, false); }, + source_warehouse: function (frm, cdt, cdn) { + get_bom_material_detail(frm.doc, cdt, cdn, false); + }, }); frappe.ui.form.on("BOM Item", "qty", function (frm, cdt, cdn) { @@ -1029,11 +1036,48 @@ frappe.tour["BOM"] = [ ]; frappe.ui.form.on("BOM Secondary Item", { - item_code(frm, cdt, cdn) { - const { item_code } = locals[cdt][cdn]; + valuation_type(frm, cdt, cdn) { + const row = locals[cdt][cdn]; + if (row.valuation_type !== "% of FG Cost") { + frappe.model.set_value(cdt, cdn, "cost_allocation_per", 0); + } + if (row.valuation_type === "Valuation Rate") { + erpnext.bom.fetch_secondary_item_cost(frm.doc, cdt, cdn); + } else if (row.valuation_type !== "Manual") { + frappe.model.set_value(cdt, cdn, { cost: 0, base_cost: 0 }); + } }, }); +erpnext.bom.fetch_secondary_item_cost = function (doc, cdt, cdn) { + const row = locals[cdt][cdn]; + if (!row.item_code) return; + + frappe.call({ + doc: doc, + method: "get_bom_material_detail", + args: { + company: doc.company, + item_code: row.item_code, + uom: row.uom, + stock_uom: row.stock_uom, + conversion_factor: row.conversion_factor, + warehouse: doc.default_target_warehouse, + set_rate_based_on_warehouse: 1, + force_valuation_rate: 1, + fetch_rate: 1, + bom_no: "", + }, + callback(r) { + const cost = flt(r.message.rate) * flt(row.stock_qty); + frappe.model.set_value(cdt, cdn, { + cost: cost, + base_cost: cost * flt(doc.conversion_rate || 1), + }); + }, + }); +}; + function trigger_process_loss_qty_prompt(frm, cdt, cdn, item_code) { frappe.prompt( { diff --git a/erpnext/manufacturing/doctype/bom/bom.json b/erpnext/manufacturing/doctype/bom/bom.json index 9205528ec9e..f2c4fd183cf 100644 --- a/erpnext/manufacturing/doctype/bom/bom.json +++ b/erpnext/manufacturing/doctype/bom/bom.json @@ -402,7 +402,7 @@ { "fetch_from": "item.description", "fieldname": "description", - "fieldtype": "Small Text", + "fieldtype": "Text Editor", "label": "Item Description", "read_only": 1 }, @@ -771,7 +771,7 @@ "image_field": "image", "is_submittable": 1, "links": [], - "modified": "2026-08-21 23:11:39.133941", + "modified": "2026-08-23 15:20:11.032436", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM", diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 811697ce5dc..8034bf2f714 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -156,7 +156,7 @@ class BOM(WebsiteGenerator): currency: DF.Link default_source_warehouse: DF.Link | None default_target_warehouse: DF.Link | None - description: DF.SmallText | None + description: DF.TextEditor | None exploded_items: DF.Table[BOMExplosionItem] fg_based_operating_cost: DF.Check has_variants: DF.Check @@ -341,7 +341,7 @@ class BOM(WebsiteGenerator): self.validate_semi_finished_goods() self.validate_batch_split_operations() self.validate_secondary_items() - self.set_fg_cost_allocation() + self.validate_secondary_items_cost() self.validate_total_cost_allocation() def set_operation_finished_goods(self): @@ -427,8 +427,20 @@ class BOM(WebsiteGenerator): ) def validate_secondary_items(self): + seen_items = set() for item in self.secondary_items: - if not item.is_legacy and item.item_code == self.item: + # every consumer merges secondary rows by item and type, so duplicates cannot + # keep their own quantities, percentages or valuation mode + key = (item.item_code, item.secondary_item_type or "") + if key in seen_items: + frappe.throw( + _( + "Row #{0}: Item {1} is already added with the same Type in the Secondary Items table." + ).format(item.idx, get_link_to_form("Item", item.item_code)) + ) + seen_items.add(key) + + if item.valuation_type != "Valuation Rate" and item.item_code == self.item: frappe.throw( _( "Row #{0}: Finished Good Item {1} cannot be added in the Secondary Items table." @@ -519,13 +531,25 @@ class BOM(WebsiteGenerator): def set_fg_cost_allocation(self): total_secondary_items_per = 0 + own_cost = 0 for item in self.secondary_items: + if item.valuation_type in ("Valuation Rate", "Manual"): + item.cost_allocation_per = 0 + own_cost += flt(item.cost) total_secondary_items_per += item.cost_allocation_per if self.cost_allocation_per == 100 and total_secondary_items_per: self.cost_allocation_per -= total_secondary_items_per - self.cost_allocation = self.raw_material_cost * (self.cost_allocation_per / 100) + self.cost_allocation = (self.raw_material_cost - own_cost) * (self.cost_allocation_per / 100) + + def validate_secondary_items_cost(self): + if flt(self.secondary_items_cost) > flt(self.raw_material_cost): + frappe.throw( + _("The cost of the secondary items cannot exceed the raw material cost of {0}.").format( + frappe.bold(flt(self.raw_material_cost)) + ) + ) def validate_total_cost_allocation(self): total_cost_allocation_per = self.cost_allocation_per @@ -600,6 +624,7 @@ class BOM(WebsiteGenerator): "conversion_factor": item.conversion_factor, "sourced_by_supplier": item.sourced_by_supplier, "do_not_explode": item.do_not_explode, + "source_warehouse": item.source_warehouse or self.default_source_warehouse, "fetch_rate": True, } ) @@ -1118,7 +1143,8 @@ class BOM(WebsiteGenerator): def has_scrap_items(self): return any( - d.get("secondary_item_type") == "Scrap" or d.get("is_legacy") for d in self.get("secondary_items") + d.get("secondary_item_type") == "Scrap" or d.get("valuation_type") == "Valuation Rate" + for d in self.get("secondary_items") ) def validate_bom_currency(self, item): @@ -1240,7 +1266,9 @@ def _get_price_list_item_rate(args, bom_doc): def get_valuation_rate(data): """ - 1) Get average valuation rate from all warehouses + 1) Get average valuation rate from the scoping warehouse if one is passed + (source warehouse for raw materials, default target warehouse for secondary + items), else from all warehouses 2) If no value, get last valuation rate from SLE 3) If no value, get valuation rate from Item """ @@ -1279,8 +1307,12 @@ def _get_avg_valuation_rate_from_bins(item_code, company, data): .where((bin_table.item_code == item_code) & (wh_table.company == company)) ) + warehouse = data.get("source_warehouse") if data.get("set_rate_based_on_warehouse") and data.get("warehouse"): - item_valuation = item_valuation.where(bin_table.warehouse == data.get("warehouse")) + warehouse = data.get("warehouse") + + if warehouse: + item_valuation = item_valuation.where(bin_table.warehouse == warehouse) return item_valuation.run(as_dict=True)[0].get("valuation_rate") @@ -1498,18 +1530,18 @@ def _add_exploded_item_columns(query, t, bom, amount_col, stock_item_condition): def _add_secondary_item_columns(query, t, stock_item_condition): - # non-grouped columns are constant per grouped item_code -> Max() keeps the GROUP BY valid on - # postgres while returning the same value MySQL picked arbitrarily. + # grouped by (item_code, secondary_item_type), which the BOM keeps unique, so every Max() + # below returns the single grouped row's own value while keeping the GROUP BY valid on + # postgres. query = query.select( Max(t.item_doc.description).as_("description"), Max(t.bom_item.cost_allocation_per).as_("cost_allocation_per"), Max(t.bom_item.process_loss_per).as_("process_loss_per"), - Max(t.bom_item.secondary_item_type).as_("secondary_item_type"), + t.bom_item.secondary_item_type, Max(t.bom_item.name).as_("name"), - Max(t.bom_item.is_legacy).as_("is_legacy"), ).where(stock_item_condition) - return query, [t.bom_item.item_code] + return query, [t.bom_item.item_code, t.bom_item.secondary_item_type] def _add_normal_item_columns(query, t, amount_col, stock_item_condition, track_semi_finished_goods): @@ -1549,6 +1581,9 @@ def _add_normal_item_columns(query, t, amount_col, stock_item_condition, track_s def _add_bom_item_to_dict(item_dict, item, company, opts): key = item.item_code + if opts.fetch_secondary_items: + key = (item.item_code, item.secondary_item_type or "") + if item.operation_row_id: key = (item.item_code, item.operation_row_id) diff --git a/erpnext/manufacturing/doctype/bom/services/costing.py b/erpnext/manufacturing/doctype/bom/services/costing.py index b72d1b8909d..b477f2ec405 100644 --- a/erpnext/manufacturing/doctype/bom/services/costing.py +++ b/erpnext/manufacturing/doctype/bom/services/costing.py @@ -36,7 +36,12 @@ class BOMCostingService: return flt(rate) * flt(self.doc.plc_conversion_rate or 1) / (self.doc.conversion_rate or 1) def _raw_material_rate(self, arg, notify): - from erpnext.manufacturing.doctype.bom.bom import get_bom_item_rate + from erpnext.manufacturing.doctype.bom.bom import get_bom_item_rate, get_valuation_rate + + # Valuation Rate secondary items ignore the BOM's rm_cost_as_per method: bin-average + # valuation like the raw materials, scoped to the default target warehouse when set. + if arg.get("force_valuation_rate"): + return get_valuation_rate(arg) # Customer Provided parts and Supplier sourced parts will have zero rate if frappe.db.get_value("Item", arg["item_code"], "is_customer_provided_item") or arg.get( @@ -142,6 +147,7 @@ class BOMCostingService: self.calculate_op_cost(update_hour_rate) self.calculate_rm_cost(save=save_updates) self.calculate_secondary_items_costs(save=save_updates) + self.doc.set_fg_cost_allocation() if save_updates: # not via doc event, table is not regenerated and needs updation self.calculate_exploded_cost() @@ -248,6 +254,7 @@ class BOMCostingService: "conversion_factor": d.conversion_factor, "sourced_by_supplier": d.sourced_by_supplier, "is_phantom_item": d.is_phantom_item, + "source_warehouse": d.source_warehouse or self.doc.default_source_warehouse, } def _set_item_amounts(self, d): @@ -261,24 +268,56 @@ class BOMCostingService: ) def calculate_secondary_items_costs(self, save=False): - """Fetch RM rate as per today's valuation rate and calculate totals""" + """Valuation Rate and Manual rows carry their own cost, deducted from the raw + material cost; the % of FG Cost rows split the remainder by their percentage.""" total_sm_cost = 0 base_total_sm_cost = 0 precision = self.doc.precision("raw_material_cost") + allocation_basis = flt(self.doc.raw_material_cost) - self._set_own_cost_secondary_items( + precision, save + ) for d in self.doc.get("secondary_items"): - if not d.is_legacy: - d.cost = flt(self.doc.raw_material_cost * (d.cost_allocation_per / 100), precision) + if d.valuation_type not in ("Valuation Rate", "Manual"): + d.cost = flt(allocation_basis * (d.cost_allocation_per / 100), precision) d.base_cost = flt(d.cost * self.doc.conversion_rate, precision) - - total_sm_cost += d.cost - base_total_sm_cost += d.base_cost if save: d.db_update() + total_sm_cost += d.cost + base_total_sm_cost += d.base_cost + self.doc.secondary_items_cost = total_sm_cost self.doc.base_secondary_items_cost = base_total_sm_cost + def _set_own_cost_secondary_items(self, precision, save) -> float: + """Cost of the rows valued on their own: fetched for Valuation Rate, kept for Manual.""" + total = 0.0 + for d in self.doc.get("secondary_items"): + if d.valuation_type == "Valuation Rate": + rate = self.get_rm_rate(self._secondary_item_rate_args(d)) + d.cost = flt(flt(rate) * flt(d.stock_qty), precision) + elif d.valuation_type == "Manual": + d.cost = flt(d.cost, precision) + else: + continue + + d.base_cost = flt(d.cost * self.doc.conversion_rate, precision) + total += d.cost + if save: + d.db_update() + + return total + + def _secondary_item_rate_args(self, d): + return { + "item_code": d.item_code, + "company": self.doc.company, + "warehouse": self.doc.default_target_warehouse, + "set_rate_based_on_warehouse": 1, + "force_valuation_rate": 1, + } + def calculate_exploded_cost(self): "Set exploded row cost from it's parent BOM." rm_rate_map = self.get_rm_rate_map() diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 515708f6d80..8f3c8b39168 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -543,7 +543,7 @@ class TestBOM(ERPNextTestSuite): fg_item_non_whole, fg_item_whole, bom_item = create_process_loss_bom_items() bom_doc = create_bom_with_process_loss_item( - fg_item_non_whole, bom_item, scrap_qty=2, scrap_rate=0, process_loss_percentage=110 + fg_item_non_whole, bom_item, scrap_qty=2, process_loss_percentage=110 ) # PL can't be > 100 self.assertRaises(frappe.ValidationError, bom_doc.submit) @@ -570,12 +570,224 @@ class TestBOM(ERPNextTestSuite): "secondary_item_type": "Additional Finished Good", "qty": 1, "cost_allocation_per": 10, + "valuation_type": "% of FG Cost", }, ) # FG item of the BOM cannot also be a secondary item self.assertRaises(frappe.ValidationError, bom_doc.save) + @timeout + def test_duplicate_secondary_item_not_allowed(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 1, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 1, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 1, + "cost_allocation_per": 10, + "valuation_type": "% of FG Cost", + }, + ) + self.assertRaises(frappe.ValidationError, bom_doc.save) + + # the same item with a different type is a distinct secondary output + bom_doc.secondary_items[1].secondary_item_type = "By-Product" + bom_doc.save() + + @timeout + def test_secondary_item_manual_cost(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + by_product = make_item(properties={"is_stock_item": 1}).name + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 10, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": by_product, + "secondary_item_type": "By-Product", + "qty": 1, + "valuation_type": "Manual", + "cost": 150, + }, + ) + bom_doc.save() + + row = bom_doc.secondary_items[0] + self.assertEqual(row.cost, 150) + self.assertEqual(row.cost_allocation_per, 0) + self.assertEqual(bom_doc.secondary_items_cost, 150) + self.assertEqual(bom_doc.total_cost, 850) + self.assertEqual(bom_doc.cost_allocation, 850) + + # a manual cost above the raw material cost would turn the finished good negative + bom_doc.secondary_items[0].cost = 1100 + self.assertRaises(frappe.ValidationError, bom_doc.save) + + @timeout + def test_secondary_item_valuation_rate_method(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + by_product = make_item(properties={"is_stock_item": 1}).name + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 10, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 2, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.append( + "secondary_items", + { + "item_code": by_product, + "secondary_item_type": "By-Product", + "qty": 1, + "cost_allocation_per": 10, + "valuation_type": "% of FG Cost", + }, + ) + bom_doc.save() + + scrap_row = bom_doc.secondary_items[0] + self.assertEqual(scrap_row.cost, 100) + self.assertEqual(scrap_row.cost_allocation_per, 0) + + # the by-product's percentage applies to the cost net of the valuation rate rows + self.assertEqual(bom_doc.raw_material_cost, 1000) + self.assertEqual(bom_doc.secondary_items[1].cost, 90) + self.assertEqual(bom_doc.cost_allocation_per, 90) + self.assertEqual(bom_doc.cost_allocation, 810) + self.assertEqual(bom_doc.secondary_items_cost, 190) + self.assertEqual(bom_doc.total_cost, 810) + + @timeout + def test_rm_rate_scoped_to_source_warehouse(self): + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + make_stock_entry(item_code=rm_item, target="_Test Warehouse 1 - _TC", qty=10, basic_rate=50) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 1, "source_warehouse": "_Test Warehouse - _TC"}) + bom_doc.save() + self.assertEqual(bom_doc.items[0].rate, 100) + + bom_doc.items[0].source_warehouse = None + bom_doc.save() + self.assertEqual(bom_doc.items[0].rate, 75) + + bom_doc.default_source_warehouse = "_Test Warehouse 1 - _TC" + bom_doc.save() + self.assertEqual(bom_doc.items[0].rate, 50) + + @timeout + def test_secondary_item_rate_scoped_to_target_warehouse(self): + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1}).name + + make_stock_entry(item_code=scrap_item, target="_Test Warehouse - _TC", qty=10, basic_rate=40) + make_stock_entry(item_code=scrap_item, target="_Test Warehouse 1 - _TC", qty=10, basic_rate=20) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.default_target_warehouse = "_Test Warehouse - _TC" + bom_doc.append("items", {"item_code": rm_item, "qty": 10, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 1, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.save() + self.assertEqual(bom_doc.secondary_items[0].cost, 40) + + bom_doc.default_target_warehouse = None + bom_doc.save() + self.assertEqual(bom_doc.secondary_items[0].cost, 30) + + @timeout + def test_secondary_item_valuation_rate_refreshed_on_update_cost(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 10, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 2, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.save() + bom_doc.submit() + + frappe.db.set_value("Item", scrap_item, "valuation_rate", 80) + bom_doc.update_cost() + bom_doc.reload() + + self.assertEqual(bom_doc.secondary_items[0].cost, 160) + self.assertEqual(bom_doc.total_cost, 840) + self.assertEqual(bom_doc.cost_allocation, 840) + @timeout def test_bom_item_query(self): query = partial( @@ -1276,7 +1488,7 @@ def reset_item_valuation_rate(item_code, warehouse_list=None, qty=None, rate=Non def create_bom_with_process_loss_item( - fg_item, bom_item, scrap_qty=0, scrap_rate=0, fg_qty=2, process_loss_percentage=0, company=None + fg_item, bom_item, scrap_qty=0, fg_qty=2, process_loss_percentage=0, company=None ): bom_doc = frappe.new_doc("BOM") bom_doc.item = fg_item.item_code @@ -1298,11 +1510,11 @@ def create_bom_with_process_loss_item( "secondary_items", { "item_code": fg_item.item_code, + "secondary_item_type": "Scrap", "qty": scrap_qty, "stock_qty": scrap_qty, "uom": fg_item.stock_uom, "stock_uom": fg_item.stock_uom, - "rate": scrap_rate, }, ) diff --git a/erpnext/manufacturing/doctype/bom/test_records.json b/erpnext/manufacturing/doctype/bom/test_records.json index 2386fd0f38b..bd1684fd1f3 100644 --- a/erpnext/manufacturing/doctype/bom/test_records.json +++ b/erpnext/manufacturing/doctype/bom/test_records.json @@ -38,15 +38,13 @@ { "secondary_items":[ { - "amount": 2000.0, "doctype": "BOM Secondary Item", "item_code": "_Test Item Home Desktop 100", "parentfield": "secondary_items", "stock_qty": 1.0, - "rate": 2000.0, "stock_uom": "_Test UOM", "secondary_item_type": "Scrap", - "is_legacy": 1 + "valuation_type": "Valuation Rate" } ], "items": [ diff --git a/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.json b/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.json index d3ad50b169f..9f740752a72 100644 --- a/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.json +++ b/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.json @@ -6,9 +6,6 @@ "editable_grid": 1, "engine": "InnoDB", "field_order": [ - "rate", - "column_break_gres", - "is_legacy", "section_break_sbnk", "item_code", "item_name", @@ -25,6 +22,7 @@ "column_break_wsra", "image_nygv", "section_break_ielf", + "valuation_type", "cost_allocation_per", "process_loss_per", "column_break_gtbl", @@ -34,13 +32,12 @@ ], "fields": [ { - "depends_on": "eval:!doc.is_legacy", "fieldname": "secondary_item_type", "fieldtype": "Select", "in_list_view": 1, "label": "Type", - "mandatory_depends_on": "eval:!doc.is_legacy", - "options": "\nCo-Product\nBy-Product\nScrap\nAdditional Finished Good" + "options": "\nCo-Product\nBy-Product\nScrap\nAdditional Finished Good", + "reqd": 1 }, { "fieldname": "item_code", @@ -63,10 +60,9 @@ "fieldname": "cost", "fieldtype": "Currency", "label": "Cost", - "no_copy": 1, "non_negative": 1, "options": "currency", - "read_only": 1, + "read_only_depends_on": "eval:doc.valuation_type != 'Manual'", "reqd": 1 }, { @@ -103,7 +99,6 @@ "reqd": 1 }, { - "depends_on": "eval:!doc.is_legacy", "fieldname": "section_break_ielf", "fieldtype": "Section Break" }, @@ -143,6 +138,7 @@ }, { "default": "0", + "depends_on": "eval:doc.valuation_type == '% of FG Cost'", "fieldname": "cost_allocation_per", "fieldtype": "Percent", "label": "Cost Allocation %", @@ -175,33 +171,20 @@ "fieldtype": "Currency", "hidden": 1, "label": "Base Cost (Company Currency)", - "no_copy": 1, "non_negative": 1, "options": "Company:company:default_currency", "read_only": 1, "reqd": 1 }, { - "fieldname": "column_break_gres", - "fieldtype": "Column Break" - }, - { - "default": "0", - "depends_on": "is_legacy", - "fieldname": "is_legacy", - "fieldtype": "Check", - "label": "Is Legacy", - "no_copy": 1, - "read_only": 1 - }, - { - "depends_on": "eval:doc.is_legacy", - "fieldname": "rate", - "fieldtype": "Currency", - "label": "Rate", - "no_copy": 1, - "non_negative": 1, - "read_only": 1 + "default": "Valuation Rate", + "description": "Valuation Rate and Manual value this item on its own and deduct that cost from the raw material cost, like the pre-v16 scrap items. % of FG Cost allocates a percentage of the remaining raw material cost.", + "fieldname": "valuation_type", + "fieldtype": "Select", + "label": "Valuation Type", + "options": "Valuation Rate\n% of FG Cost\nManual", + "reqd": 1, + "show_description_on_click": 1 }, { "default": "0", diff --git a/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.py b/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.py index 5f0a89249ac..ec2f72247ac 100644 --- a/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.py +++ b/erpnext/manufacturing/doctype/bom_secondary_item/bom_secondary_item.py @@ -20,7 +20,6 @@ class BOMSecondaryItem(Document): cost_allocation_per: DF.Percent description: DF.TextEditor | None image: DF.AttachImage | None - is_legacy: DF.Check item_code: DF.Link item_name: DF.Data | None parent: DF.Data @@ -29,11 +28,11 @@ class BOMSecondaryItem(Document): process_loss_per: DF.Percent process_loss_qty: DF.Float qty: DF.Float - rate: DF.Currency secondary_item_type: DF.Literal["", "Co-Product", "By-Product", "Scrap", "Additional Finished Good"] stock_qty: DF.Float stock_uom: DF.Link | None uom: DF.Link + valuation_type: DF.Literal["Valuation Rate", "% of FG Cost", "Manual"] # end: auto-generated types pass diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 8455e64f2ee..76187dacd4d 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -307,8 +307,9 @@ class JobCard(Document): fetch_exploded=0, fetch_secondary_items=1, ) - for item_code, values in items_dict.items(): - self.append_secondary_item(item_code, frappe._dict(values)) + for values in items_dict.values(): + values = frappe._dict(values) + self.append_secondary_item(values.item_code, values) def append_secondary_item(self, item_code, values): secondary_item = { @@ -320,11 +321,10 @@ class JobCard(Document): "bom_secondary_item": values.name, } - if not values.is_legacy: - secondary_item["stock_qty"] -= flt( - secondary_item["stock_qty"] * (values.process_loss_per / 100), - self.precision("for_quantity"), - ) + secondary_item["stock_qty"] -= flt( + secondary_item["stock_qty"] * (flt(values.process_loss_per) / 100), + self.precision("for_quantity"), + ) self.append("secondary_items", secondary_item) @@ -1881,7 +1881,7 @@ class JobCard(Document): add_additional_cost(ste.stock_entry, wo_doc, self) ManufactureStockEntry(ste.stock_entry).add_secondary_items_from_job_card() for row in ste.stock_entry.items: - if (row.secondary_item_type or row.is_legacy_scrap_item) and not row.t_warehouse: + if (row.secondary_item_type or row.valuation_type) and not row.t_warehouse: row.t_warehouse = self.target_warehouse diff --git a/erpnext/manufacturing/doctype/job_card/test_job_card.py b/erpnext/manufacturing/doctype/job_card/test_job_card.py index f8330fea5aa..f3021611a68 100644 --- a/erpnext/manufacturing/doctype/job_card/test_job_card.py +++ b/erpnext/manufacturing/doctype/job_card/test_job_card.py @@ -1550,6 +1550,7 @@ class TestJobCard(ERPNextTestSuite): "qty": 1, "process_loss_per": 10, "cost_allocation_per": 5, + "valuation_type": "% of FG Cost", "secondary_item_type": "Scrap", }, ) @@ -2985,7 +2986,7 @@ class TestJobCard(ERPNextTestSuite): frappe.db.set_value( "Stock Entry Detail", s.items[3].name, - {"secondary_item_type": None, "is_legacy_scrap_item": 1}, + {"secondary_item_type": None, "valuation_type": "Valuation Rate"}, ) from erpnext.stock.doctype.stock_entry.services.manufacturing import ManufactureStockEntry @@ -2994,6 +2995,88 @@ class TestJobCard(ERPNextTestSuite): used_secondary_items = ManufactureStockEntry(stock_entry).get_used_secondary_items() self.assertEqual(used_secondary_items[("_Test Item", "Scrap")], 2) + def test_secondary_items_from_multiple_boms_stay_separate(self): + """Rows linked to different BOM rows keep their own quantity and valuation mode.""" + from erpnext.stock.doctype.item.test_item import make_item + + secondary_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + + bom_links = [] + for cost_allocation_per in (0, 10): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 1, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": secondary_item, + "secondary_item_type": "Scrap", + "qty": 1, + "cost_allocation_per": cost_allocation_per, + "valuation_type": "% of FG Cost" if cost_allocation_per else "Valuation Rate", + }, + ) + bom_doc.save() + bom_doc.submit() + bom_links.append(bom_doc.secondary_items[0].name) + + for row in frappe.get_doc("BOM", self.work_order.bom_no).items: + make_stock_entry( + item_code=row.item_code, + target="_Test Warehouse - _TC", + qty=10, + basic_rate=100, + ) + + job_card = frappe.get_last_doc("Job Card", {"work_order": self.work_order.name}) + job_card.append( + "secondary_items", + { + "item_code": secondary_item, + "stock_qty": 2, + "secondary_item_type": "Scrap", + "bom_secondary_item": bom_links[0], + }, + ) + job_card.append( + "secondary_items", + { + "item_code": secondary_item, + "stock_qty": 3, + "secondary_item_type": "Scrap", + "bom_secondary_item": bom_links[1], + }, + ) + job_card.append( + "time_logs", + { + "from_time": "2009-01-01 12:06:25", + "to_time": "2009-01-01 12:37:25", + "completed_qty": job_card.for_quantity, + }, + ) + job_card.save() + job_card.submit() + + from erpnext.manufacturing.doctype.work_order.mapper import ( + make_stock_entry as make_stock_entry_for_wo, + ) + + s = frappe.get_doc(make_stock_entry_for_wo(self.work_order.name, "Manufacture")) + + rows = {d.bom_secondary_item: d for d in s.items if d.item_code == secondary_item} + self.assertEqual(len(rows), 2) + self.assertEqual(rows[bom_links[0]].qty, 2) + self.assertEqual(rows[bom_links[0]].valuation_type, "Valuation Rate") + self.assertEqual(rows[bom_links[1]].qty, 3) + self.assertEqual(rows[bom_links[1]].valuation_type, "% of FG Cost") + @ERPNextTestSuite.change_settings( "Manufacturing Settings", {"overproduction_percentage_for_work_order": 100} ) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index d6dc5d4d195..1a84d63b486 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -3631,6 +3631,7 @@ def make_bom(**args): "stock_uom": item_doc.stock_uom, "qty": args.scrap_qty or 1, "cost_allocation_per": args.scrap_cost_allocation_per or 10, + "valuation_type": "% of FG Cost", "process_loss_per": args.scrap_process_loss_per or 10, }, ) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 2bc48e3e18e..b24d8a4b024 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -1132,7 +1132,7 @@ class TestWorkOrder(ERPNextTestSuite): stock_entry = frappe.get_doc(make_stock_entry(wo_order.name, "Manufacture", 10)) for row in stock_entry.items: - if row.secondary_item_type or row.is_legacy_scrap_item: + if row.secondary_item_type or row.valuation_type: self.assertEqual(row.qty, 1) # Partial Job Card 1 with qty 10 @@ -1144,7 +1144,7 @@ class TestWorkOrder(ERPNextTestSuite): stock_entry = frappe.get_doc(make_stock_entry(wo_order.name, "Manufacture", 10)) for row in stock_entry.items: - if row.secondary_item_type or row.is_legacy_scrap_item: + if row.secondary_item_type or row.valuation_type: self.assertEqual(row.qty, 2) # Partial Job Card 2 with qty 10 @@ -2924,7 +2924,7 @@ class TestWorkOrder(ERPNextTestSuite): self.assertTrue(se_doc.additional_costs) secondary_items = [] for item in se_doc.items: - if item.secondary_item_type or item.is_legacy_scrap_item: + if item.secondary_item_type or item.valuation_type: secondary_items.append(item.item_code) self.assertEqual( @@ -5529,6 +5529,7 @@ class TestWorkOrder(ERPNextTestSuite): "item_name": scrap_item, "qty": 3, "cost_allocation_per": 25, + "valuation_type": "% of FG Cost", "process_loss_per": 0, }, ) @@ -5577,6 +5578,7 @@ class TestWorkOrder(ERPNextTestSuite): "item_name": scrap_item, "qty": 3, "cost_allocation_per": 25, + "valuation_type": "% of FG Cost", "process_loss_per": 0, }, ) @@ -5911,7 +5913,15 @@ def prepare_boms_for_sub_assembly_test(): do_not_submit=True, ) - bom.append("secondary_items", {"item_code": "Test Final Scrap Item 1", "qty": 1, "is_legacy": 1}) + bom.append( + "secondary_items", + { + "item_code": "Test Final Scrap Item 1", + "secondary_item_type": "Scrap", + "qty": 1, + "valuation_type": "Valuation Rate", + }, + ) bom.submit() @@ -5924,7 +5934,15 @@ def prepare_boms_for_sub_assembly_test(): do_not_submit=True, ) - bom.append("secondary_items", {"item_code": "Test Final Scrap Item 2", "qty": 1, "is_legacy": 1}) + bom.append( + "secondary_items", + { + "item_code": "Test Final Scrap Item 2", + "secondary_item_type": "Scrap", + "qty": 1, + "valuation_type": "Valuation Rate", + }, + ) bom.submit() diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 5aadf2179d3..0524902f139 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -219,7 +219,7 @@ class WorkOrder(Document): .where( (parent.work_order == self.name) & (parent.docstatus == 1) - & ((child.secondary_item_type != "") | (child.is_legacy_scrap_item == 1)) + & ((child.secondary_item_type != "") | (Coalesce(child.valuation_type, "") != "")) ) .select( child.item_code, diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 4f75c4f1a3c..15cc77fc900 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -517,3 +517,4 @@ erpnext.patches.v16_0.repair_work_order_material_transfer erpnext.patches.v16_0.remove_frappe_crm_custom_fields erpnext.patches.v16_0.add_batch_split_stock_entry_type erpnext.patches.v16_0.add_transaction_roles_to_sms_settings +erpnext.patches.v16_0.set_secondary_item_valuation_type diff --git a/erpnext/patches/v16_0/co_by_product_patch.py b/erpnext/patches/v16_0/co_by_product_patch.py index 5baca92b31a..e89c8def117 100644 --- a/erpnext/patches/v16_0/co_by_product_patch.py +++ b/erpnext/patches/v16_0/co_by_product_patch.py @@ -22,8 +22,10 @@ def copy_doctypes(): def insert_into_bom(): - fields = ["item_code", "item_name", "stock_uom", "stock_qty", "rate"] - data = frappe.get_all("BOM Scrap Item", {"docstatus": ("<", 2)}, ["parent", *fields]) + fields = ["item_code", "item_name", "stock_uom", "stock_qty"] + data = frappe.get_all( + "BOM Scrap Item", {"docstatus": ("<", 2)}, ["parent", *fields, "amount", "base_amount"] + ) grouped_data = defaultdict(list) for item in data: grouped_data[item.parent].append(item) @@ -40,8 +42,10 @@ def insert_into_bom(): "uom": item.stock_uom, "conversion_factor": 1, "qty": item.stock_qty, - "is_legacy": 1, + "valuation_type": "Valuation Rate", "secondary_item_type": "Scrap", + "cost": item.amount, + "base_cost": item.base_amount, } ) secondary_item.insert() @@ -100,12 +104,21 @@ def bulk_insert(parent_doctype, old_doctype, new_doctype, old_fields, new_fields def rename_fields(): rename_field("BOM", "scrap_material_cost", "secondary_items_cost") rename_field("BOM", "base_scrap_material_cost", "base_secondary_items_cost") - rename_field("Stock Entry Detail", "is_scrap_item", "is_legacy_scrap_item") + set_valuation_type("Stock Entry Detail", "is_scrap_item") rename_field( "Manufacturing Settings", "set_op_cost_and_scrap_from_sub_assemblies", "set_op_cost_and_secondary_items_from_sub_assemblies", ) rename_field("Selling Settings", "deliver_scrap_items", "deliver_secondary_items") - rename_field("Subcontracting Receipt Item", "is_scrap_item", "is_legacy_scrap_item") + set_valuation_type("Subcontracting Receipt Item", "is_scrap_item") rename_field("Subcontracting Receipt Item", "scrap_cost_per_qty", "secondary_items_cost_per_qty") + + +def set_valuation_type(doctype, legacy_field): + """The legacy scrap flag becomes the Valuation Rate method.""" + if not frappe.db.has_column(doctype, legacy_field): + return + + table = frappe.qb.DocType(doctype) + frappe.qb.update(table).set(table.valuation_type, "Valuation Rate").where(table[legacy_field] == 1).run() diff --git a/erpnext/patches/v16_0/set_secondary_item_valuation_type.py b/erpnext/patches/v16_0/set_secondary_item_valuation_type.py new file mode 100644 index 00000000000..a7ad240558d --- /dev/null +++ b/erpnext/patches/v16_0/set_secondary_item_valuation_type.py @@ -0,0 +1,79 @@ +import frappe +from frappe.utils import flt + + +def execute(): + """Set valuation_type on sites that migrated before the field existed. + + Fresh migrations get it from co_by_product_patch; the legacy columns never + existed there, so every step below is a no-op. + """ + for doctype, legacy_fields in ( + ("BOM Secondary Item", ["is_legacy", "use_valuation_rate"]), + ("Stock Entry Detail", ["is_legacy_scrap_item", "use_valuation_rate"]), + ("Subcontracting Receipt Item", ["is_legacy_scrap_item", "use_valuation_rate"]), + ): + set_valuation_rate_method(doctype, legacy_fields) + + set_percentage_method() + backfill_cost_from_rate() + + +def set_valuation_rate_method(doctype, legacy_fields): + table = frappe.qb.DocType(doctype) + for field in legacy_fields: + if not frappe.db.has_column(doctype, field): + continue + + frappe.qb.update(table).set(table.valuation_type, "Valuation Rate").where(table[field] == 1).run() + + +def set_percentage_method(): + """Rows created by the percentage system before the method field existed. + + Only BOM rows need this: the field is mandatory there, and the costing treats + the percentage method as the default for everything else.""" + rows = frappe.get_all("BOM Secondary Item", filters={"valuation_type": ("is", "not set")}, pluck="name") + if not rows: + return + + frappe.db.set_value( + "BOM Secondary Item", + {"name": ("in", rows)}, + "valuation_type", + "% of FG Cost", + update_modified=False, + ) + + +def backfill_cost_from_rate(): + """Earlier v16 builds stored the migrated scrap rate on the removed rate field.""" + if not frappe.db.has_column("BOM Secondary Item", "rate"): + return + + table = frappe.qb.DocType("BOM Secondary Item") + rows = ( + frappe.qb.from_(table) + .select(table.name, table.parent, table.rate, table.stock_qty) + .where((table.valuation_type == "Valuation Rate") & (table.cost == 0) & (table.rate > 0)) + ).run(as_dict=True) + if not rows: + return + + conversion_rates = dict( + frappe.get_all( + "BOM", + filters={"name": ("in", {row.parent for row in rows})}, + fields=["name", "conversion_rate"], + as_list=True, + ) + ) + + for row in rows: + cost = flt(row.rate) * flt(row.stock_qty) + frappe.db.set_value( + "BOM Secondary Item", + row.name, + {"cost": cost, "base_cost": cost * flt(conversion_rates.get(row.parent) or 1)}, + update_modified=False, + ) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index b7a5fd9b2b7..280b3088633 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -24,7 +24,7 @@ erpnext.stock.is_incoming_qi_purpose = (purpose) => erpnext.stock.row_requires_quality_inspection = (purpose, row) => { if ( erpnext.stock.secondary_item_purposes.includes(purpose) && - (row.secondary_item_type || row.is_legacy_scrap_item) + (row.secondary_item_type || row.valuation_type) ) return false; if (purpose === "Manufacture") return !!row.is_finished_item; diff --git a/erpnext/stock/doctype/quality_inspection/quality_inspection.py b/erpnext/stock/doctype/quality_inspection/quality_inspection.py index 743d065ba27..88a6e573608 100644 --- a/erpnext/stock/doctype/quality_inspection/quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/quality_inspection.py @@ -446,7 +446,7 @@ def item_query(doctype: Any, txt: str | None, searchfield: Any, start: int, page "and", ["items.secondary_item_type", "is", "not set"], "and", - ["items.is_legacy_scrap_item", "=", 0], + ["items.valuation_type", "is", "not set"], ] ) if purpose == "Manufacture": diff --git a/erpnext/stock/doctype/stock_entry/services/batch_split.py b/erpnext/stock/doctype/stock_entry/services/batch_split.py index d8eae6888ae..beaf7c84460 100644 --- a/erpnext/stock/doctype/stock_entry/services/batch_split.py +++ b/erpnext/stock/doctype/stock_entry/services/batch_split.py @@ -47,7 +47,7 @@ class BatchSplitFinishedGood: fg_rows = [ row for row in self.doc.items - if row.is_finished_item and not row.secondary_item_type and not row.is_legacy_scrap_item + if row.is_finished_item and not row.secondary_item_type and not row.valuation_type ] if len(fg_rows) != 1: @@ -128,7 +128,7 @@ class BatchSplitFinishedGood: if row.is_finished_item or not row.s_warehouse: return False - if row.secondary_item_type or row.is_legacy_scrap_item: + if row.secondary_item_type or row.valuation_type: return False return bool(frappe.get_cached_value("Item", row.item_code, "has_batch_no")) diff --git a/erpnext/stock/doctype/stock_entry/services/disassemble.py b/erpnext/stock/doctype/stock_entry/services/disassemble.py index 4c31de36c97..0b31822141d 100644 --- a/erpnext/stock/doctype/stock_entry/services/disassemble.py +++ b/erpnext/stock/doctype/stock_entry/services/disassemble.py @@ -230,7 +230,7 @@ class DisassembleStockEntry(BaseStockEntry): "t_warehouse": t_warehouse, "is_finished_item": source_row.is_finished_item, "secondary_item_type": source_row.secondary_item_type, - "is_legacy_scrap_item": source_row.is_legacy_scrap_item, + "valuation_type": source_row.valuation_type, "bom_secondary_item": source_row.bom_secondary_item, "bom_no": source_row.bom_no, "use_serial_batch_fields": 1 if (source_row.batch_no or source_row.serial_no) else 0, @@ -284,7 +284,7 @@ class DisassembleStockEntry(BaseStockEntry): for field in fields: item_args[field] = row.get(field) - item_args["is_legacy_scrap_item"] = row.get("is_legacy") + item_args["valuation_type"] = row.get("valuation_type") item_args["s_warehouse"] = self.doc.from_warehouse item_args["uom"] = item_args.get("uom") or item_args.get("stock_uom") item_args["bom_secondary_item"] = row.get("name") @@ -330,7 +330,7 @@ class DisassembleStockEntry(BaseStockEntry): SED.conversion_factor, SED.is_finished_item, SED.secondary_item_type, - SED.is_legacy_scrap_item, + SED.valuation_type, SED.bom_secondary_item, SED.batch_no, SED.serial_no, @@ -397,7 +397,7 @@ class DisassembleStockEntry(BaseStockEntry): SED.stock_uom, SED.is_finished_item, SED.secondary_item_type, - SED.is_legacy_scrap_item, + SED.valuation_type, SED.bom_secondary_item, SED.batch_no, SED.serial_no, diff --git a/erpnext/stock/doctype/stock_entry/services/gl_composer.py b/erpnext/stock/doctype/stock_entry/services/gl_composer.py index 6b20416b3b5..0a9cbba0b0b 100644 --- a/erpnext/stock/doctype/stock_entry/services/gl_composer.py +++ b/erpnext/stock/doctype/stock_entry/services/gl_composer.py @@ -102,11 +102,7 @@ class StockEntryGLComposer(BaseStockGLComposer): if not item.t_warehouse or item.s_warehouse: return 0.0 - if ( - item.get("is_finished_item") - or item.get("secondary_item_type") - or item.get("is_legacy_scrap_item") - ): + if item.get("is_finished_item") or item.get("secondary_item_type") or item.get("valuation_type"): return 0.0 if get_valuation_method(item.item_code, self.doc.company) != "Standard Cost": diff --git a/erpnext/stock/doctype/stock_entry/services/manufacturing.py b/erpnext/stock/doctype/stock_entry/services/manufacturing.py index bfba22f1d44..82c52fd4edc 100644 --- a/erpnext/stock/doctype/stock_entry/services/manufacturing.py +++ b/erpnext/stock/doctype/stock_entry/services/manufacturing.py @@ -3,7 +3,7 @@ from collections import defaultdict import frappe from frappe import _, bold -from frappe.query_builder.functions import Max, Min, Sum +from frappe.query_builder.functions import Coalesce, Max, Min, NullIf, Sum from frappe.utils import ceil, cint, flt, get_link_to_form from erpnext.manufacturing.doctype.bom.bom import add_additional_cost @@ -40,7 +40,7 @@ class BaseManufactureStockEntry(BaseStockEntry): not row.s_warehouse and self.doc.from_warehouse and not row.is_finished_item - and not row.is_legacy_scrap_item + and not row.valuation_type and not row.secondary_item_type ): row.s_warehouse = self.doc.from_warehouse @@ -49,7 +49,7 @@ class BaseManufactureStockEntry(BaseStockEntry): elif ( not row.t_warehouse and self.doc.to_warehouse - and (row.is_finished_item or row.is_legacy_scrap_item or row.secondary_item_type) + and (row.is_finished_item or row.valuation_type or row.secondary_item_type) ): row.t_warehouse = self.doc.to_warehouse row.s_warehouse = None @@ -98,9 +98,12 @@ class BaseManufactureStockEntry(BaseStockEntry): secondary_items = get_secondary_items(self.doc.bom_no, self.doc.work_order) for row in secondary_items: item_args = self.get_item_dict(row) - item_args["is_legacy_scrap_item"] = bool(row.get("is_legacy")) + item_args["valuation_type"] = row.get("valuation_type") item_args["secondary_item_type"] = row.secondary_item_type item_args["bom_secondary_item"] = row.name + if row.get("valuation_type") == "Manual": + item_args["set_basic_rate_manually"] = 1 + item_args["basic_rate"] = flt(row.get("manual_rate")) if row.secondary_item_type == "Scrap" and self.wo_doc and self.wo_doc.get("scrap_warehouse"): item_args["t_warehouse"] = self.wo_doc.scrap_warehouse @@ -868,6 +871,7 @@ class ManufactureStockEntry(BaseManufactureStockEntry): return secondary_items = self.get_secondary_items_from_job_card() + bom_rows = self.get_bom_secondary_item_details(secondary_items) for row in secondary_items: if row.stock_qty <= 0: continue @@ -877,11 +881,31 @@ class ManufactureStockEntry(BaseManufactureStockEntry): row.transfer_qty = row.qty row.s_warehouse = None row.t_warehouse = row.warehouse or self.doc.to_warehouse - row.is_legacy_scrap_item = row.is_legacy + bom_row = bom_rows.get(row.bom_secondary_item, frappe._dict()) + row.valuation_type = bom_row.get("valuation_type") + if row.valuation_type == "Manual": + row.set_basic_rate_manually = 1 + row.basic_rate = ( + flt(bom_row.cost) / flt(bom_row.stock_qty) if flt(bom_row.get("stock_qty")) else 0 + ) row.secondary_item_type = row.get("secondary_item_type") self.doc.append("items", row) + def get_bom_secondary_item_details(self, secondary_items) -> dict: + names = [row.bom_secondary_item for row in secondary_items if row.bom_secondary_item] + if not names: + return {} + + return { + row.name: row + for row in frappe.get_all( + "BOM Secondary Item", + filters={"name": ("in", names)}, + fields=["name", "valuation_type", "cost", "stock_qty"], + ) + } + def get_secondary_items_from_job_card(self): if not self.wo_doc.operations: return [] @@ -898,17 +922,14 @@ class ManufactureStockEntry(BaseManufactureStockEntry): def _adjust_secondary_item_qtys(self, secondary_items, used_secondary_items, pending_qty): for row in secondary_items: - key = (row.item_code, row.secondary_item_type or "") - row.stock_qty -= flt(used_secondary_items.get(key)) + row.stock_qty -= flt(used_secondary_items.get(get_secondary_item_key(row))) row.stock_qty = row.stock_qty * flt(self.doc.fg_completed_qty) / flt(pending_qty) def get_used_secondary_items(self): data = self._query_used_secondary_items() used_secondary_items = defaultdict(float) for row in data: - secondary_item_type = row.secondary_item_type or ("Scrap" if row.is_legacy_scrap_item else "") - key = (row.item_code, secondary_item_type) - used_secondary_items[key] += row.qty + used_secondary_items[get_secondary_item_key(row)] += row.qty return used_secondary_items def _query_used_secondary_items(self): @@ -918,10 +939,16 @@ class ManufactureStockEntry(BaseManufactureStockEntry): frappe.qb.from_(se) .inner_join(sed) .on(sed.parent == se.name) - .select(sed.item_code, sed.secondary_item_type, sed.is_legacy_scrap_item, sed.qty) + .select( + sed.item_code, + sed.secondary_item_type, + sed.valuation_type, + sed.qty, + sed.bom_secondary_item, + ) .where( (se.work_order == self.doc.work_order) - & ((sed.secondary_item_type.isnotnull()) | (sed.is_legacy_scrap_item == 1)) + & ((sed.secondary_item_type.isnotnull()) | (Coalesce(sed.valuation_type, "") != "")) & (se.docstatus == 1) & (se.purpose.isin(["Repack", "Manufacture"])) ) @@ -1183,7 +1210,7 @@ def get_bom_items(bom_no, use_multi_level_bom=None, qty=None, fetch_secondary_it table_name = "BOM Explosion Item" if use_multi_level_bom else "BOM Item" items = _run_bom_items_query(bom_no, table_name, qty) - return _deduplicate_bom_items(items) + return _deduplicate_bom_items(items, by_type=fetch_secondary_items) def _run_bom_items_query(bom_no, table_name, qty): @@ -1199,7 +1226,6 @@ def _run_bom_items_query(bom_no, table_name, qty): doctype.stock_uom, doctype.description, (doctype.stock_qty / bom_doc.quantity.as_("qty") * qty).as_("qty"), - doctype.rate.as_("basic_rate"), ) .where((bom_doc.name == bom_no) & (bom_doc.docstatus == 1)) .orderby(doctype.idx) @@ -1215,9 +1241,11 @@ def _add_bom_table_specific_fields(query, doctype, table_name): doctype.uom, doctype.process_loss_per, doctype.secondary_item_type, - doctype.is_legacy, + doctype.valuation_type, doctype.conversion_factor, + (doctype.cost / NullIf(doctype.stock_qty, 0)).as_("manual_rate"), ) + query = query.select(doctype.rate.as_("basic_rate")) if table_name == "BOM Item": return query.select( doctype.allow_alternative_item, doctype.uom, doctype.conversion_factor, doctype.bom_no @@ -1225,13 +1253,14 @@ def _add_bom_table_specific_fields(query, doctype, table_name): return query -def _deduplicate_bom_items(items): +def _deduplicate_bom_items(items, by_type=False): item_dict = {} for item in items: - if item.item_code in item_dict: - item_dict[item.item_code].qty += item.qty + key = (item.item_code, item.get("secondary_item_type") or "") if by_type else item.item_code + if key in item_dict: + item_dict[key].qty += item.qty else: - item_dict[item.item_code] = item + item_dict[key] = item return list(item_dict.values()) @@ -1261,6 +1290,21 @@ def get_secondary_items_from_sub_assemblies(bom_no): return items +def get_secondary_item_key(row): + """Identity of a secondary output: its BOM row when linked, else (item, type). + + Grouping only by (item, type) would merge rows that different BOMs of the same work + order produce, and one BOM row's percentage or valuation mode would then govern the + other BOMs' quantities too.""" + if row.get("bom_secondary_item"): + return row.bom_secondary_item + + return ( + row.item_code, + row.secondary_item_type or ("Scrap" if row.get("valuation_type") == "Valuation Rate" else ""), + ) + + def get_secondary_items_from_job_card(work_order, jc_name=None): job_card = frappe.qb.DocType("Job Card") job_card_secondary_item = frappe.qb.DocType("Job Card Secondary Item") @@ -1270,12 +1314,12 @@ def get_secondary_items_from_job_card(work_order, jc_name=None): .select( Sum(job_card_secondary_item.stock_qty).as_("stock_qty"), job_card_secondary_item.item_code, - # stock_uom and the secondary-item BOM link are constant per grouped - # (item_code, secondary_item_type) -> Max() returns their single value. item_name and - # description are editable per line, so they come from a representative line below. + # stock_uom is constant per grouped item_code -> Max() returns its single value. + # item_name and description are editable per line, so they come from a + # representative line below. Max(job_card_secondary_item.stock_uom).as_("stock_uom"), job_card_secondary_item.secondary_item_type, - Max(job_card_secondary_item.bom_secondary_item).as_("bom_secondary_item"), + job_card_secondary_item.bom_secondary_item, ) .join(job_card_secondary_item) .on(job_card_secondary_item.parent == job_card.name) @@ -1284,7 +1328,11 @@ def get_secondary_items_from_job_card(work_order, jc_name=None): & (job_card.work_order == work_order) & (job_card.docstatus == 1) ) - .groupby(job_card_secondary_item.item_code, job_card_secondary_item.secondary_item_type) + .groupby( + job_card_secondary_item.item_code, + job_card_secondary_item.secondary_item_type, + job_card_secondary_item.bom_secondary_item, + ) .orderby(Min(job_card_secondary_item.idx)) ) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.js b/erpnext/stock/doctype/stock_entry/stock_entry.js index 498c8bf6fe2..1bbcf569f60 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.js +++ b/erpnext/stock/doctype/stock_entry/stock_entry.js @@ -289,6 +289,13 @@ frappe.ui.form.on("Stock Entry", { frm.trigger("get_items_from_transit_entry"); frm.trigger("toggle_warehouse_fields"); frm.trigger("toggle_weight_per_piece"); + + // only BOM-less rows are editable, and they cannot allocate a BOM percentage; + // read-only rows from a BOM still display their stored % of FG Cost + frm.fields_dict.items.grid.update_docfield_property("valuation_type", "options", [ + "Valuation Rate", + "Manual", + ]); erpnext.toggle_serial_batch_fields(frm); if (!frm.doc.docstatus && !frm.doc.subcontracting_inward_order) { @@ -1021,6 +1028,29 @@ frappe.ui.form.on("Stock Entry Detail", { ); }, + secondary_item_type(frm, cdt, cdn) { + const row = locals[cdt][cdn]; + if (row.bom_secondary_item) return; + + if (!row.secondary_item_type) { + if (row.valuation_type) { + frappe.model.set_value(cdt, cdn, { valuation_type: "", set_basic_rate_manually: 0 }); + } + return; + } + + if (!row.valuation_type) { + frappe.model.set_value(cdt, cdn, "valuation_type", "Valuation Rate"); + } + }, + + valuation_type(frm, cdt, cdn) { + const row = locals[cdt][cdn]; + if (!row.secondary_item_type || row.bom_secondary_item) return; + + frappe.model.set_value(cdt, cdn, "set_basic_rate_manually", row.valuation_type === "Manual" ? 1 : 0); + }, + conversion_factor(frm, cdt, cdn) { frm.events.set_basic_rate(frm, cdt, cdn); }, diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 44936d4ad94..57df17602a2 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -83,7 +83,10 @@ def is_costed_out_of_finished_item(row) -> bool: A secondary item that is not linked to a BOM has no cost allocation of its own, so it is valued the way the legacy scrap item was: its cost is deducted from the finished good. """ - return bool(row.is_legacy_scrap_item or (row.secondary_item_type and not row.bom_secondary_item)) + return bool( + row.valuation_type in ("Valuation Rate", "Manual") + or (row.secondary_item_type and not row.bom_secondary_item) + ) class StockEntry(StockController, SubcontractingInwardController): @@ -318,6 +321,7 @@ class StockEntry(StockController, SubcontractingInwardController): if self.purpose in ("Manufacture", "Repack"): self.mark_finished_and_secondary_items() + self.set_bomless_secondary_valuation_types() if not self.job_card: self.validate_finished_goods() else: @@ -599,9 +603,16 @@ class StockEntry(StockController, SubcontractingInwardController): secondary_items_cost_basis = self.get_secondary_items_cost_basis(outgoing_items_cost) zero_valuation_items = [] - finished_items_last = sorted(self.get("items"), key=lambda row: cint(row.is_finished_item)) + # Own-cost rows first: their value is deducted from the basis the percentage + # allocated rows and the finished good split, so it must be known before those. + finished_items_last = sorted( + self.get("items"), + key=lambda row: (cint(row.is_finished_item), cint(not is_costed_out_of_finished_item(row))), + ) for d in finished_items_last: if d.s_warehouse or d.set_basic_rate_manually: + if d.set_basic_rate_manually: + d.basic_amount = flt(flt(d.transfer_qty) * flt(d.basic_rate), d.precision("basic_amount")) continue # Zero-qty secondary items carry no inventory value; skip rate calculation @@ -690,34 +701,43 @@ class StockEntry(StockController, SubcontractingInwardController): if self.bom_no: d.basic_rate *= bom_cost_allocation_per / 100 + elif is_costed_out_of_finished_item(d): + # Recomputed every time: a rate fetched before the target warehouse was set + # must not stick to the row. + d.basic_rate = self.get_row_valuation_rate(d, raise_error_if_no_rate) + has_derived_rate = True elif d.secondary_item_type and d.bom_secondary_item: cost_allocation_per = flt( frappe.get_value("BOM Secondary Item", d.bom_secondary_item, "cost_allocation_per") ) if flt(d.transfer_qty): - d.basic_rate = (secondary_items_cost_basis * (cost_allocation_per / 100)) / d.transfer_qty + allocation_basis = secondary_items_cost_basis - self.get_costed_out_items_cost() + d.basic_rate = (allocation_basis * (cost_allocation_per / 100)) / d.transfer_qty has_derived_rate = True # A rate of zero that was derived rather than left unset is a real cost. Falling back to # the item's valuation here would value free inputs, or an unallocated row, as output. if not d.basic_rate and not d.allow_zero_valuation_rate and not has_derived_rate: - d.basic_rate = get_valuation_rate( - d.item_code, - d.t_warehouse, - self.doctype, - self.name, - d.allow_zero_valuation_rate, - currency=erpnext.get_company_currency(self.company), - company=self.company, - raise_error_if_no_rate=raise_error_if_no_rate, - batch_no=d.batch_no, - serial_and_batch_bundle=d.serial_and_batch_bundle, - ) + d.basic_rate = self.get_row_valuation_rate(d, raise_error_if_no_rate) # do not round off basic rate to avoid precision loss d.basic_rate = flt(d.basic_rate) d.basic_amount = flt(flt(d.transfer_qty) * flt(d.basic_rate), d.precision("basic_amount")) + def get_row_valuation_rate(self, d, raise_error_if_no_rate): + return get_valuation_rate( + d.item_code, + d.t_warehouse, + self.doctype, + self.name, + d.allow_zero_valuation_rate, + currency=erpnext.get_company_currency(self.company), + company=self.company, + raise_error_if_no_rate=raise_error_if_no_rate, + batch_no=d.batch_no, + serial_and_batch_bundle=d.serial_and_batch_bundle, + ) + def _notify_zero_valuation_rate(self, items): if len(items) > 1: message = _( @@ -766,6 +786,8 @@ class StockEntry(StockController, SubcontractingInwardController): ) def get_basic_rate_for_repacked_items(self, finished_item_qty, outgoing_items_cost): + outgoing_items_cost -= self.get_costed_out_items_cost() + finished_items = [ d.item_code for d in self.get("items") if d.is_finished_item and not d.set_basic_rate_manually ] @@ -783,13 +805,41 @@ class StockEntry(StockController, SubcontractingInwardController): ) return flt(outgoing_items_cost / total_fg_qty) + def set_bomless_secondary_valuation_types(self): + """Secondary rows without a BOM link choose their own costing: valuation rate or manual. + + There is no percentage to allocate without a BOM row, so % of FG Cost is rejected.""" + for d in self.get("items"): + if d.bom_secondary_item: + continue + + if not d.secondary_item_type: + if d.valuation_type: + d.valuation_type = "" + d.set_basic_rate_manually = 0 + continue + + if d.valuation_type == "% of FG Cost": + frappe.throw( + _( + "Row #{0}: % of FG Cost needs a BOM secondary item. Choose Valuation Rate or Manual for {1}." + ).format(d.idx, frappe.bold(d.item_code)) + ) + + if not d.valuation_type: + d.valuation_type = "Valuation Rate" + + d.set_basic_rate_manually = cint(d.valuation_type == "Manual") + + def get_costed_out_items_cost(self) -> float: + """Total value of the rows that are deducted from the cost the other incoming rows split.""" + return sum(flt(d.basic_amount) for d in self.get("items") if is_costed_out_of_finished_item(d)) + def get_basic_rate_for_manufactured_item( self, finished_item_qty, outgoing_items_cost=0, has_consumption_basis=False ) -> float: settings = frappe.get_single("Manufacturing Settings") - scrap_items_cost = sum( - [flt(d.basic_amount) for d in self.get("items") if is_costed_out_of_finished_item(d)] - ) + scrap_items_cost = self.get_costed_out_items_cost() if settings.material_consumption: outgoing_items_cost = self._get_rm_cost_for_manufacture( @@ -816,7 +866,7 @@ class StockEntry(StockController, SubcontractingInwardController): def _validate_no_raw_materials_in_manufacture_entry(self, settings): for item in self.items: - if not item.is_finished_item and not item.secondary_item_type and not item.is_legacy_scrap_item: + if not item.is_finished_item and not item.secondary_item_type and not item.valuation_type: label = frappe.get_meta(settings.doctype).get_translated_label( "get_rm_cost_from_consumption_entry" ) @@ -956,7 +1006,7 @@ class StockEntry(StockController, SubcontractingInwardController): for d in self.items: if d.t_warehouse and not d.s_warehouse: - if d.secondary_item_type or d.is_legacy_scrap_item: + if d.secondary_item_type or d.valuation_type: d.is_finished_item = 0 elif self.purpose == "Repack" or d.item_code == finished_item: d.is_finished_item = 1 diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 5bd70a59240..bc3f9ae9fbd 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -1243,8 +1243,8 @@ class TestStockEntry(ERPNextTestSuite): rm_cost += d.amount fg_cost = next(filter(lambda x: x.item_code == "_Test FG Item", s.get("items"))).amount secondary_item_cost = next( - filter(lambda x: x.secondary_item_type or x.is_legacy_scrap_item, s.get("items")) - ).amount + x.amount for x in s.get("items") if x.secondary_item_type or x.valuation_type + ) self.assertEqual(fg_cost, flt(rm_cost - secondary_item_cost, 2)) @@ -1325,6 +1325,356 @@ class TestStockEntry(ERPNextTestSuite): self.assertRaises(frappe.ValidationError, ste.submit) + def test_manufacture_entry_with_valuation_rate_secondary_item(self): + from erpnext.manufacturing.doctype.work_order.mapper import ( + make_stock_entry as _make_stock_entry, + ) + + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + by_product = make_item(properties={"is_stock_item": 1}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append( + "items", + {"item_code": rm_item, "qty": 10, "rate": 100.0, "source_warehouse": "_Test Warehouse - _TC"}, + ) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 2, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.append( + "secondary_items", + { + "item_code": by_product, + "secondary_item_type": "By-Product", + "qty": 1, + "cost_allocation_per": 10, + "valuation_type": "% of FG Cost", + }, + ) + bom_doc.save() + bom_doc.submit() + + work_order = frappe.new_doc("Work Order") + work_order.update( + { + "company": "_Test Company", + "fg_warehouse": "_Test Warehouse 1 - _TC", + "production_item": fg_item, + "bom_no": bom_doc.name, + "qty": 1.0, + "stock_uom": frappe.db.get_value("Item", fg_item, "stock_uom"), + "skip_transfer": 1, + } + ) + work_order.get_items_and_operations_from_bom() + work_order.submit() + + entry = frappe.get_doc(_make_stock_entry(work_order.name, "Manufacture", 1)) + entry.insert() + + rm_cost = sum(d.basic_amount for d in entry.items if d.s_warehouse) + self.assertEqual(rm_cost, 1000) + + # valuation rate row is valued at its valuation rate and deducted from the + # basis; the percentage rows and the finished good split the remainder + scrap_row = next(d for d in entry.items if d.valuation_type == "Valuation Rate") + self.assertEqual(scrap_row.basic_rate, 50) + self.assertEqual(scrap_row.basic_amount, 100) + + by_product_row = next(d for d in entry.items if d.secondary_item_type == "By-Product") + self.assertEqual(by_product_row.basic_amount, 90) + + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 810) + + incoming_cost = sum(d.basic_amount for d in entry.items if not d.s_warehouse) + self.assertEqual(incoming_cost, rm_cost) + + # a stale rate, e.g. fetched before the target warehouse was set, must not stick + scrap_row.basic_rate = 999 + entry.save() + scrap_row = next(d for d in entry.items if d.valuation_type == "Valuation Rate") + self.assertEqual(scrap_row.basic_rate, 50) + + def test_manufacture_entry_with_same_item_secondary_types(self): + from erpnext.manufacturing.doctype.work_order.mapper import ( + make_stock_entry as _make_stock_entry, + ) + + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + secondary_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append( + "items", + {"item_code": rm_item, "qty": 10, "rate": 100.0, "source_warehouse": "_Test Warehouse - _TC"}, + ) + bom_doc.append( + "secondary_items", + { + "item_code": secondary_item, + "secondary_item_type": "Scrap", + "qty": 2, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.append( + "secondary_items", + { + "item_code": secondary_item, + "secondary_item_type": "By-Product", + "qty": 1, + "cost_allocation_per": 10, + "valuation_type": "% of FG Cost", + }, + ) + bom_doc.save() + bom_doc.submit() + + work_order = frappe.new_doc("Work Order") + work_order.update( + { + "company": "_Test Company", + "fg_warehouse": "_Test Warehouse 1 - _TC", + "production_item": fg_item, + "bom_no": bom_doc.name, + "qty": 1.0, + "stock_uom": frappe.db.get_value("Item", fg_item, "stock_uom"), + "skip_transfer": 1, + } + ) + work_order.get_items_and_operations_from_bom() + work_order.submit() + + entry = frappe.get_doc(_make_stock_entry(work_order.name, "Manufacture", 1)) + entry.insert() + + # both rows of the same item keep their own type and costing mode + secondary_rows = [d for d in entry.items if d.item_code == secondary_item] + self.assertEqual(len(secondary_rows), 2) + + scrap_row = next(d for d in secondary_rows if d.valuation_type == "Valuation Rate") + self.assertEqual(scrap_row.basic_amount, 100) + + by_product_row = next(d for d in secondary_rows if d.valuation_type != "Valuation Rate") + self.assertEqual(by_product_row.secondary_item_type, "By-Product") + self.assertEqual(by_product_row.basic_amount, 90) + + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 810) + + def test_manufacture_entry_with_manual_secondary_item(self): + from erpnext.manufacturing.doctype.work_order.mapper import ( + make_stock_entry as _make_stock_entry, + ) + + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + by_product = make_item(properties={"is_stock_item": 1}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append( + "items", + {"item_code": rm_item, "qty": 10, "rate": 100.0, "source_warehouse": "_Test Warehouse - _TC"}, + ) + bom_doc.append( + "secondary_items", + { + "item_code": by_product, + "secondary_item_type": "By-Product", + "qty": 2, + "valuation_type": "Manual", + "cost": 120, + }, + ) + bom_doc.save() + bom_doc.submit() + + work_order = frappe.new_doc("Work Order") + work_order.update( + { + "company": "_Test Company", + "fg_warehouse": "_Test Warehouse 1 - _TC", + "production_item": fg_item, + "bom_no": bom_doc.name, + "qty": 1.0, + "stock_uom": frappe.db.get_value("Item", fg_item, "stock_uom"), + "skip_transfer": 1, + } + ) + work_order.get_items_and_operations_from_bom() + work_order.submit() + + entry = frappe.get_doc(_make_stock_entry(work_order.name, "Manufacture", 1)) + entry.insert() + + # the manual row starts at the BOM cost per unit and is deducted from the FG + manual_row = next(d for d in entry.items if d.valuation_type == "Manual") + self.assertEqual(manual_row.set_basic_rate_manually, 1) + self.assertEqual(manual_row.basic_rate, 60) + self.assertEqual(manual_row.basic_amount, 120) + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 880) + + # the user's own rate reprices the row and the finished good + manual_row.basic_rate = 100 + entry.save() + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 800) + + # a manual cost above the consumed cost would turn the finished good negative + manual_row = next(d for d in entry.items if d.valuation_type == "Manual") + manual_row.basic_rate = 600 + self.assertRaises(frappe.ValidationError, entry.save) + + def test_repack_entry_with_valuation_rate_secondary_item(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 100}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + bom_doc = frappe.new_doc("BOM") + bom_doc.item = fg_item + bom_doc.quantity = 1 + bom_doc.company = "_Test Company" + bom_doc.currency = "INR" + bom_doc.append("items", {"item_code": rm_item, "qty": 10, "rate": 100.0}) + bom_doc.append( + "secondary_items", + { + "item_code": scrap_item, + "secondary_item_type": "Scrap", + "qty": 2, + "valuation_type": "Valuation Rate", + }, + ) + bom_doc.save() + bom_doc.submit() + + entry = frappe.new_doc("Stock Entry") + entry.company = "_Test Company" + entry.purpose = "Repack" + entry.set_stock_entry_type() + entry.from_bom = 1 + entry.bom_no = bom_doc.name + entry.fg_completed_qty = 1 + entry.from_warehouse = "_Test Warehouse - _TC" + entry.to_warehouse = "_Test Warehouse 1 - _TC" + entry.get_items() + entry.insert() + + # the repacked good absorbs the consumed cost net of the own-cost rows + scrap_row = next(d for d in entry.items if d.valuation_type == "Valuation Rate") + self.assertEqual(scrap_row.basic_amount, 100) + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 900) + + outgoing = sum(d.basic_amount for d in entry.items if d.s_warehouse) + incoming = sum(d.basic_amount for d in entry.items if not d.s_warehouse) + self.assertEqual(incoming, outgoing) + + def test_bomless_manufacture_entry_secondary_valuation_types(self): + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1}).name + scrap_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 50}).name + manual_item = make_item(properties={"is_stock_item": 1}).name + + make_stock_entry(item_code=rm_item, target="_Test Warehouse - _TC", qty=10, basic_rate=100) + + def row(item_code, qty, **kwargs): + stock_uom = frappe.db.get_value("Item", item_code, "stock_uom") + return { + "item_code": item_code, + "qty": qty, + "transfer_qty": qty, + "uom": stock_uom, + "stock_uom": stock_uom, + "conversion_factor": 1, + **kwargs, + } + + entry = frappe.new_doc("Stock Entry") + entry.company = "_Test Company" + entry.purpose = "Manufacture" + entry.set_stock_entry_type() + entry.fg_completed_qty = 1 + entry.append("items", row(rm_item, 10, s_warehouse="_Test Warehouse - _TC")) + entry.append("items", row(fg_item, 1, t_warehouse="_Test Warehouse 1 - _TC", is_finished_item=1)) + entry.append( + "items", + row(scrap_item, 2, t_warehouse="_Test Warehouse 1 - _TC", secondary_item_type="Scrap"), + ) + entry.append( + "items", + row( + manual_item, + 1, + t_warehouse="_Test Warehouse 1 - _TC", + secondary_item_type="By-Product", + valuation_type="Manual", + basic_rate=70, + ), + ) + entry.insert() + + # without a BOM link, the valuation type defaults to Valuation Rate + scrap_row = next(d for d in entry.items if d.item_code == scrap_item) + self.assertEqual(scrap_row.valuation_type, "Valuation Rate") + self.assertEqual(scrap_row.basic_rate, 50) + + # a manual row keeps the user's rate + manual_row = next(d for d in entry.items if d.item_code == manual_item) + self.assertTrue(manual_row.set_basic_rate_manually) + self.assertEqual(manual_row.basic_amount, 70) + + # both are deducted from the finished good + fg_row = next(d for d in entry.items if d.is_finished_item) + self.assertEqual(fg_row.basic_amount, 830) + + # there is no percentage to allocate without a BOM row + manual_row.valuation_type = "% of FG Cost" + self.assertRaises(frappe.ValidationError, entry.save) + + def test_valuation_rate_lookup_without_voucher_no(self): + from erpnext.stock.stock_ledger import get_valuation_rate + + item = make_item(properties={"is_stock_item": 1}).name + make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=5, basic_rate=77) + + # unsaved documents pass no voucher_no; the lookup must still find the last SLE + rate = get_valuation_rate( + item, "_Test Warehouse - _TC", "Stock Entry", None, raise_error_if_no_rate=False + ) + self.assertEqual(rate, 77) + def test_quality_check_for_secondary_item(self): from erpnext.manufacturing.doctype.work_order.mapper import ( make_stock_entry as _make_stock_entry, @@ -1363,7 +1713,7 @@ class TestStockEntry(ERPNextTestSuite): basic_rate=row.basic_rate or 100, ) - if row.secondary_item_type or row.is_legacy_scrap_item: + if row.secondary_item_type or row.valuation_type: row.item_code = secondary_item row.uom = frappe.db.get_value("Item", secondary_item, "stock_uom") row.stock_uom = frappe.db.get_value("Item", secondary_item, "stock_uom") @@ -1372,15 +1722,11 @@ class TestStockEntry(ERPNextTestSuite): stock_entry.save() self.assertTrue( - [ - row.item_code - for row in stock_entry.items - if row.secondary_item_type or row.is_legacy_scrap_item - ] + [row.item_code for row in stock_entry.items if row.secondary_item_type or row.valuation_type] ) for row in stock_entry.items: - if not row.secondary_item_type and not row.is_legacy_scrap_item: + if not row.secondary_item_type and not row.valuation_type: qc = frappe.get_doc( { "doctype": "Quality Inspection", @@ -1400,7 +1746,7 @@ class TestStockEntry(ERPNextTestSuite): stock_entry.reload() stock_entry.submit() for row in stock_entry.items: - if row.secondary_item_type or row.is_legacy_scrap_item: + if row.secondary_item_type or row.valuation_type: self.assertFalse(row.quality_inspection) else: self.assertTrue(row.quality_inspection) @@ -3132,6 +3478,7 @@ class TestStockEntry(ERPNextTestSuite): "qty": 5, "cost_allocation_per": 25, "process_loss_per": 0, + "valuation_type": "% of FG Cost", }, ) bom.insert() @@ -3193,6 +3540,7 @@ class TestStockEntry(ERPNextTestSuite): "qty": 5, "cost_allocation_per": 0, "process_loss_per": 0, + "valuation_type": "% of FG Cost", }, ) bom.insert() @@ -3249,6 +3597,7 @@ class TestStockEntry(ERPNextTestSuite): "qty": 5, "cost_allocation_per": 25, "process_loss_per": 0, + "valuation_type": "% of FG Cost", }, ) bom.insert() diff --git a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json index 396d68487b6..c4bed814be6 100644 --- a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json +++ b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.json @@ -18,7 +18,7 @@ "item_name", "col_break2", "is_finished_item", - "is_legacy_scrap_item", + "valuation_type", "secondary_item_type", "quality_inspection", "subcontracted_item", @@ -572,7 +572,7 @@ }, { "default": "0", - "depends_on": "eval:!doc.is_legacy_scrap_item && !doc.secondary_item_type", + "depends_on": "eval:!doc.valuation_type && !doc.secondary_item_type", "fieldname": "is_finished_item", "fieldtype": "Check", "label": "Is Finished Item", @@ -674,11 +674,12 @@ "set_only_once": 1 }, { - "depends_on": "eval:parent.purpose == \"Manufacture\" && doc.t_warehouse && !doc.is_finished_item && !doc.is_legacy_scrap_item", + "depends_on": "eval:parent.purpose == \"Manufacture\" && doc.t_warehouse && !doc.is_finished_item", "fieldname": "secondary_item_type", "fieldtype": "Select", "label": "Type", - "options": "\nCo-Product\nBy-Product\nScrap\nAdditional Finished Good" + "options": "\nCo-Product\nBy-Product\nScrap\nAdditional Finished Good", + "read_only_depends_on": "eval:doc.bom_secondary_item" }, { "fieldname": "bom_secondary_item", @@ -688,12 +689,12 @@ "read_only": 1 }, { - "default": "0", - "depends_on": "is_legacy_scrap_item", - "fieldname": "is_legacy_scrap_item", - "fieldtype": "Check", - "label": "Is Legacy Scrap Item", - "read_only": 1 + "depends_on": "secondary_item_type", + "fieldname": "valuation_type", + "fieldtype": "Select", + "label": "Valuation Type", + "options": "\nValuation Rate\n% of FG Cost\nManual", + "read_only_depends_on": "eval:!doc.secondary_item_type || doc.bom_secondary_item" } ], "grid_page_length": 50, diff --git a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py index 4e690d4d8ec..5d3cbbd630d 100644 --- a/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py +++ b/erpnext/stock/doctype/stock_entry_detail/stock_entry_detail.py @@ -46,7 +46,6 @@ class StockEntryDetail(Document): has_item_scanned: DF.Check image: DF.Attach | None is_finished_item: DF.Check - is_legacy_scrap_item: DF.Check item_code: DF.Link item_group: DF.Data | None item_name: DF.Data | None @@ -81,6 +80,7 @@ class StockEntryDetail(Document): transferred_qty: DF.Float secondary_item_type: DF.Literal["", "Co-Product", "By-Product", "Scrap", "Additional Finished Good"] uom: DF.Link + valuation_type: DF.Literal["", "Valuation Rate", "% of FG Cost", "Manual"] use_serial_batch_fields: DF.Check valuation_rate: DF.Currency # end: auto-generated types diff --git a/erpnext/stock/services/quality_inspection_service.py b/erpnext/stock/services/quality_inspection_service.py index b9fab051038..10c53b9495e 100644 --- a/erpnext/stock/services/quality_inspection_service.py +++ b/erpnext/stock/services/quality_inspection_service.py @@ -55,7 +55,7 @@ SECONDARY_ITEM_PURPOSES = ("Manufacture", "Repack", "Disassemble") def is_inspection_exempt_secondary_row(doc, row) -> bool: """Whether the row is a secondary item on a document that produces secondary items.""" - if not (row.get("secondary_item_type") or row.get("is_legacy_scrap_item")): + if not (row.get("secondary_item_type") or row.get("valuation_type")): return False if doc.doctype == "Stock Entry": @@ -66,9 +66,7 @@ def is_inspection_exempt_secondary_row(doc, row) -> bool: def stock_entry_row_requires_inspection(purpose, row): """Check if this Stock Entry row need a Quality Inspection.""" - if purpose in SECONDARY_ITEM_PURPOSES and ( - row.get("secondary_item_type") or row.get("is_legacy_scrap_item") - ): + if purpose in SECONDARY_ITEM_PURPOSES and (row.get("secondary_item_type") or row.get("valuation_type")): return False if purpose == "Manufacture": return bool(row.is_finished_item) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index e132a9524e6..f5e95256782 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -2183,9 +2183,11 @@ def get_valuation_rate( & (table.warehouse == warehouse) & (table.batch_no == batch_no) & (table.is_cancelled == 0) - & ((table.voucher_no != voucher_no) | (table.voucher_type != voucher_type)) ) ) + if voucher_no: + # Comparing against a None voucher_no yields NULL, which filters out every row + query = query.where((table.voucher_no != voucher_no) | (table.voucher_type != voucher_type)) last_valuation_rate = query.run() if last_valuation_rate and last_valuation_rate[0][0] is not None: @@ -2211,7 +2213,7 @@ def get_valuation_rate( # Get valuation rate from last sle for the same item and warehouse sle_entry = frappe.qb.DocType("Stock Ledger Entry") - if last_valuation_rate := ( + last_sle_query = ( frappe.qb.from_(sle_entry) .select(sle_entry.valuation_rate) .where( @@ -2219,12 +2221,18 @@ def get_valuation_rate( & (sle_entry.warehouse == warehouse) & (sle_entry.valuation_rate >= 0) & (sle_entry.is_cancelled == 0) - & ~((sle_entry.voucher_no == voucher_no) & (sle_entry.voucher_type == voucher_type)) ) .orderby(sle_entry.posting_datetime, order=frappe.qb.desc) .orderby(sle_entry.creation, order=frappe.qb.desc) .limit(1) - ).run(): + ) + if voucher_no: + # Comparing against a None voucher_no yields NULL, which filters out every row + last_sle_query = last_sle_query.where( + ~((sle_entry.voucher_no == voucher_no) & (sle_entry.voucher_type == voucher_type)) + ) + + if last_valuation_rate := last_sle_query.run(): return flt(last_valuation_rate[0][0]) if fallbacks: diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py index 2171fd7d65e..248f959162c 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py @@ -147,11 +147,13 @@ class SubcontractingReceipt(SubcontractingController): super().validate() - if self.is_new() and self.get("_action") == "save" and not frappe.in_test: - self.get_secondary_items() - self.set_missing_values() + # after set_missing_values, so the secondary rates are computed from the same + # calculated per-qty costs the Get Secondary Items button uses + if self.is_new() and self.get("_action") == "save" and not frappe.in_test: + self.get_secondary_items(recalculate_rate=True) + if self.get("_action") == "submit": self.validate_secondary_items() self.validate_accepted_warehouse() @@ -379,74 +381,140 @@ class SubcontractingReceipt(SubcontractingController): for item in list(self.items): if item.bom: - bom = frappe.get_doc("BOM", item.bom) - for secondary_item in bom.secondary_items: - per_unit = secondary_item.stock_qty / bom.quantity - received_qty = flt(item.received_qty * per_unit, item.precision("received_qty")) - qty = flt( - item.received_qty * (per_unit - (secondary_item.process_loss_qty / bom.quantity)), - item.precision("qty"), - ) - if not secondary_item.is_legacy: - lcv_cost_per_qty = ( - flt(item.landed_cost_voucher_amount) / flt(item.qty) if flt(item.qty) else 0.0 - ) - fg_item_cost = ( - flt(item.rm_cost_per_qty) - + flt(item.secondary_items_cost_per_qty) - + flt(item.additional_cost_per_qty) - + flt(lcv_cost_per_qty) - + flt(item.service_cost_per_qty) - ) * flt(item.received_qty) - rate = ( - (item.amount if self.is_new() else fg_item_cost) - * (secondary_item.cost_allocation_per / 100) - ) / qty - else: - rate = ( - get_valuation_rate( - secondary_item.item_code, - self.set_warehouse, - self.doctype, - self.name, - currency=erpnext.get_company_currency(self.company), - company=self.company, - ) - or secondary_item.rate - ) - - self.append( - "items", - { - "secondary_item_type": secondary_item.secondary_item_type, - "is_legacy_scrap_item": secondary_item.is_legacy, - "reference_name": item.name, - "item_code": secondary_item.item_code, - "item_name": secondary_item.item_name, - "qty": received_qty - if not secondary_item.is_legacy - else flt(item.qty) * (flt(secondary_item.stock_qty) / flt(bom.quantity)), - "received_qty": received_qty, - "process_loss_qty": received_qty - qty, - "stock_uom": secondary_item.stock_uom, - "rate": rate, - "rm_cost_per_qty": 0, - "service_cost_per_qty": 0, - "additional_cost_per_qty": 0, - "secondary_items_cost_per_qty": 0, - "amount": qty * rate, - "warehouse": self.set_warehouse, - "rejected_warehouse": self.rejected_warehouse, - }, - ) + self.add_secondary_items_of_fg_row(item) if recalculate_rate: self.calculate_additional_costs() self.calculate_items_qty_and_amount() + def calculate_percentage_secondary_rows(self, percentage_rows, secondary_items_cost_map): + allocation_map = {} + names = [row.bom_secondary_item for row in percentage_rows if row.bom_secondary_item] + if names: + allocation_map = dict( + frappe.get_all( + "BOM Secondary Item", + filters={"name": ("in", names)}, + fields=["name", "cost_allocation_per"], + as_list=True, + ) + ) + + fg_rows = {row.name: row for row in self.get("items") if row.bom} + for item in percentage_rows: + qty = flt(item.received_qty) - flt(item.process_loss_qty) + fg_row = fg_rows.get(item.reference_name) + if qty and fg_row and item.bom_secondary_item in allocation_map: + item.rate = self.get_percentage_secondary_rate( + fg_row, + flt(allocation_map[item.bom_secondary_item]), + qty, + secondary_items_cost_map.get(item.reference_name, 0), + ) + item.amount = qty * flt(item.rate) + + def get_secondary_item_valuation_rate(self, item): + """Valuation at the row's warehouse; keeps the stored rate when none is found.""" + return ( + get_valuation_rate( + item.item_code, + item.warehouse or self.set_warehouse, + self.doctype, + self.name, + currency=erpnext.get_company_currency(self.company), + company=self.company, + ) + or item.rate + ) + + def add_secondary_items_of_fg_row(self, item): + """Own-cost rows first: the percentage rows allocate from the cost net of them.""" + bom = frappe.get_doc("BOM", item.bom) + warehouse = self.set_warehouse or item.warehouse + + own_cost = 0.0 + percentage_rows = [] + for secondary_item in bom.secondary_items: + if secondary_item.valuation_type in ("Valuation Rate", "Manual"): + row = self.append_secondary_item(item, bom, secondary_item, warehouse) + own_cost += flt(row.qty) * flt(row.rate) + else: + percentage_rows.append(secondary_item) + + for secondary_item in percentage_rows: + self.append_secondary_item(item, bom, secondary_item, warehouse, own_cost) + + def append_secondary_item(self, item, bom, secondary_item, warehouse, own_cost=0.0): + per_unit = secondary_item.stock_qty / bom.quantity + received_qty = flt(item.received_qty * per_unit, item.precision("received_qty")) + qty = flt( + item.received_qty * (per_unit - (secondary_item.process_loss_qty / bom.quantity)), + item.precision("qty"), + ) + rate = self.get_secondary_item_rate(item, secondary_item, warehouse, qty, own_cost) + + return self.append( + "items", + { + "secondary_item_type": secondary_item.secondary_item_type, + "valuation_type": secondary_item.valuation_type, + "bom_secondary_item": secondary_item.name, + "reference_name": item.name, + "item_code": secondary_item.item_code, + "item_name": secondary_item.item_name, + "qty": received_qty + if secondary_item.valuation_type not in ("Valuation Rate", "Manual") + else flt(item.qty) * (flt(secondary_item.stock_qty) / flt(bom.quantity)), + "received_qty": received_qty, + "process_loss_qty": received_qty - qty, + "stock_uom": secondary_item.stock_uom, + "rate": rate, + "rm_cost_per_qty": 0, + "service_cost_per_qty": 0, + "additional_cost_per_qty": 0, + "secondary_items_cost_per_qty": 0, + "amount": qty * rate, + "warehouse": warehouse, + "rejected_warehouse": self.rejected_warehouse, + }, + ) + + def get_secondary_item_rate(self, item, secondary_item, warehouse, qty, own_cost): + if secondary_item.valuation_type == "Manual": + if not flt(secondary_item.stock_qty): + return 0 + return flt(secondary_item.cost) / flt(secondary_item.stock_qty) + + if secondary_item.valuation_type == "Valuation Rate": + rate = get_valuation_rate( + secondary_item.item_code, + warehouse, + self.doctype, + self.name, + currency=erpnext.get_company_currency(self.company), + company=self.company, + ) + if not rate and secondary_item.stock_qty: + rate = flt(secondary_item.cost) / flt(secondary_item.stock_qty) + return rate + + return self.get_percentage_secondary_rate(item, secondary_item.cost_allocation_per, qty, own_cost) + + def get_percentage_secondary_rate(self, fg_row, cost_allocation_per, qty, own_cost): + lcv_cost_per_qty = ( + flt(fg_row.landed_cost_voucher_amount) / flt(fg_row.qty) if flt(fg_row.qty) else 0.0 + ) + fg_item_cost = ( + flt(fg_row.rm_cost_per_qty) + + flt(fg_row.additional_cost_per_qty) + + flt(lcv_cost_per_qty) + + flt(fg_row.service_cost_per_qty) + ) * flt(fg_row.received_qty) - flt(own_cost) + return (fg_item_cost * (cost_allocation_per / 100)) / qty + def remove_secondary_items(self): for item in list(self.items): - if item.secondary_item_type or item.is_legacy_scrap_item: + if item.secondary_item_type or item.valuation_type: self.remove(item) else: item.secondary_items_cost_per_qty = 0 @@ -503,24 +571,31 @@ class SubcontractingReceipt(SubcontractingController): else: rm_cost_map[item.reference_name] = item.amount + # own-cost rows first: they are deducted from the finished good, and the + # percentage rows reprice from the basis net of them secondary_items_cost_map = {} + percentage_rows = [] for item in self.get("items") or []: - if item.secondary_item_type or item.is_legacy_scrap_item: - qty = ( - flt(item.qty) - if item.is_legacy_scrap_item - else (flt(item.received_qty) - flt(item.process_loss_qty)) - ) - item.amount = qty * flt(item.rate) + if not (item.secondary_item_type or item.valuation_type): + continue - if item.reference_name in secondary_items_cost_map: - secondary_items_cost_map[item.reference_name] += item.amount - else: - secondary_items_cost_map[item.reference_name] = item.amount + if item.valuation_type in ("Valuation Rate", "Manual"): + if item.valuation_type == "Valuation Rate": + # Recomputed every time: a rate fetched against another warehouse + # must not stick to the row when the warehouse changes. + item.rate = self.get_secondary_item_valuation_rate(item) + item.amount = flt(item.qty) * flt(item.rate) + secondary_items_cost_map[item.reference_name] = ( + secondary_items_cost_map.get(item.reference_name, 0) + item.amount + ) + else: + percentage_rows.append(item) + + self.calculate_percentage_secondary_rows(percentage_rows, secondary_items_cost_map) total_qty = total_amount = 0 for item in self.get("items") or []: - if not item.secondary_item_type and not item.is_legacy_scrap_item: + if not item.secondary_item_type and not item.valuation_type: if item.qty: if item.name in rm_cost_map: item.rm_supp_cost = rm_cost_map[item.name] @@ -542,6 +617,7 @@ class SubcontractingReceipt(SubcontractingController): + flt(item.service_cost_per_qty) + flt(item.additional_cost_per_qty) + flt(lcv_cost_per_qty) + - flt(item.secondary_items_cost_per_qty) ) if item.bom: @@ -563,7 +639,7 @@ class SubcontractingReceipt(SubcontractingController): def validate_secondary_items(self): for item in self.items: - if item.secondary_item_type or item.is_legacy_scrap_item: + if item.secondary_item_type or item.valuation_type: if not item.qty: frappe.throw( _("Row #{0}: Secondary Item Qty cannot be zero").format(item.idx), diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py index b480a726b47..09fb084a4ca 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt/test_subcontracting_receipt.py @@ -1192,11 +1192,33 @@ class TestSubcontractingReceipt(ERPNextTestSuite): "secondary_items", { "item_code": item, + "secondary_item_type": "Scrap", "stock_qty": 1 * (idx + 1), - "rate": 10 * (idx + 1), - "is_legacy": 1, + "valuation_type": "Valuation Rate", }, ) + manual_item = make_item(properties={"is_stock_item": 1}).name + bom.append( + "secondary_items", + { + "item_code": manual_item, + "secondary_item_type": "By-Product", + "stock_qty": 1, + "valuation_type": "Manual", + "cost": 30, + }, + ) + percentage_item = make_item(properties={"is_stock_item": 1}).name + bom.append( + "secondary_items", + { + "item_code": percentage_item, + "secondary_item_type": "Co-Product", + "stock_qty": 1, + "valuation_type": "% of FG Cost", + "cost_allocation_per": 10, + }, + ) bom.save() bom.submit() @@ -1220,10 +1242,72 @@ class TestSubcontractingReceipt(ERPNextTestSuite): scr.get_secondary_items() scr_secondary_items = set( - [item.item_code for item in scr.items if item.secondary_item_type or item.is_legacy_scrap_item] + [item.item_code for item in scr.items if item.secondary_item_type or item.valuation_type] + ) + self.assertEqual(len(scr.items), 5) # 1 FG Item + 4 Secondary Items + self.assertEqual(scr_secondary_items, {*secondary_items, manual_item, percentage_item}) + + # without a document level warehouse the rows fall back to the FG row's warehouse + scr.set_warehouse = None + scr.get_secondary_items() + fg_warehouse = next(item.warehouse for item in scr.items if item.bom) + for item in scr.items: + if item.secondary_item_type or item.valuation_type: + self.assertEqual(item.warehouse, fg_warehouse) + + # the percentage row allocates from the cost net of the own-cost rows, so the + # received value equals the consumed value + scr.save() + fg_row = next(item for item in scr.items if item.bom) + fg_gross = ( + flt(fg_row.rm_cost_per_qty) + + flt(fg_row.service_cost_per_qty) + + flt(fg_row.additional_cost_per_qty) + ) * flt(fg_row.received_qty) + secondary_total = sum( + flt(row.amount) for row in scr.items if row.secondary_item_type or row.valuation_type + ) + self.assertAlmostEqual(flt(fg_row.amount) + secondary_total, fg_gross, places=2) + + # valuation rate rows are repriced at their warehouse on every save + make_stock_entry(item_code=secondary_item_1, target="_Test Warehouse - _TC", qty=5, basic_rate=40) + vr_row = next(item for item in scr.items if item.item_code == secondary_item_1) + vr_row.warehouse = "_Test Warehouse - _TC" + scr.save() + vr_row = next(item for item in scr.items if item.item_code == secondary_item_1) + self.assertEqual(vr_row.rate, 40) + + # the manual row starts at the BOM cost per unit and takes the user's own rate + manual_row = next(item for item in scr.items if item.item_code == manual_item) + self.assertEqual(manual_row.rate, 30) + manual_row.rate = 45 + scr.save() + manual_row = next(item for item in scr.items if item.item_code == manual_item) + self.assertEqual(manual_row.rate, 45) + self.assertEqual(manual_row.amount, 45 * manual_row.qty) + + # percentage rows reprice on save when the own-cost basis changes + own_total = sum( + flt(row.amount) for row in scr.items if row.valuation_type in ("Valuation Rate", "Manual") + ) + percentage_row = next(item for item in scr.items if item.item_code == percentage_item) + self.assertAlmostEqual(flt(percentage_row.amount), (fg_gross - own_total) * 0.10, places=2) + + # the finished good's rate is its cost allocation share of the net cost + fg_row = next(item for item in scr.items if item.bom) + self.assertTrue(fg_row.secondary_items_cost_per_qty > 0) + fg_percent = flt(frappe.get_value("BOM", fg_row.bom, "cost_allocation_per")) / 100 + self.assertAlmostEqual( + flt(fg_row.rate), + ( + flt(fg_row.rm_cost_per_qty) + + flt(fg_row.service_cost_per_qty) + + flt(fg_row.additional_cost_per_qty) + - flt(fg_row.secondary_items_cost_per_qty) + ) + * fg_percent, + places=2, ) - self.assertEqual(len(scr.items), 3) # 1 FG Item + 2 Scrap Items - self.assertEqual(scr_secondary_items, set(secondary_items)) scr.submit() diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.json b/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.json index 6ba81c05c15..61dce1d4e7b 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.json +++ b/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.json @@ -8,7 +8,8 @@ "engine": "InnoDB", "field_order": [ "item_code", - "is_legacy_scrap_item", + "valuation_type", + "bom_secondary_item", "secondary_item_type", "column_break_2", "item_name", @@ -166,12 +167,12 @@ "label": "Accepted Qty", "no_copy": 1, "print_width": "100px", - "read_only_depends_on": "eval:doc.secondary_item_type || doc.is_legacy_scrap_item", + "read_only_depends_on": "eval:doc.secondary_item_type || doc.valuation_type", "width": "100px" }, { "columns": 1, - "depends_on": "eval:!parent.is_return && !doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!parent.is_return && !doc.secondary_item_type && !doc.valuation_type", "fieldname": "rejected_qty", "fieldtype": "Float", "in_list_view": 1, @@ -179,7 +180,7 @@ "no_copy": 1, "print_hide": 1, "print_width": "100px", - "read_only_depends_on": "eval:doc.secondary_item_type || doc.is_legacy_scrap_item", + "read_only_depends_on": "eval:doc.secondary_item_type || doc.valuation_type", "width": "100px" }, { @@ -220,9 +221,10 @@ "fieldtype": "Currency", "in_list_view": 1, "label": "Rate", + "non_negative": 1, "options": "Company:company:default_currency", "print_width": "100px", - "read_only": 1, + "read_only_depends_on": "eval:doc.valuation_type != 'Manual'", "width": "100px" }, { @@ -239,7 +241,7 @@ }, { "default": "0", - "depends_on": "eval:!doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.secondary_item_type && !doc.valuation_type", "fieldname": "rm_cost_per_qty", "fieldtype": "Currency", "label": "Raw Material Cost Per Qty", @@ -249,7 +251,7 @@ }, { "default": "0", - "depends_on": "eval:!doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.secondary_item_type && !doc.valuation_type", "fieldname": "service_cost_per_qty", "fieldtype": "Currency", "label": "Service Cost Per Qty", @@ -259,7 +261,7 @@ }, { "default": "0", - "depends_on": "eval:!doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.secondary_item_type && !doc.valuation_type", "fieldname": "additional_cost_per_qty", "fieldtype": "Currency", "label": "Additional Cost Per Qty", @@ -283,7 +285,7 @@ "width": "100px" }, { - "depends_on": "eval: !parent.is_return && !doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval: !parent.is_return && !doc.secondary_item_type && !doc.valuation_type", "fieldname": "rejected_warehouse", "fieldtype": "Link", "ignore_user_permissions": 1, @@ -295,7 +297,7 @@ "width": "100px" }, { - "depends_on": "eval:!doc.__islocal && !doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.__islocal && !doc.secondary_item_type && !doc.valuation_type", "fieldname": "quality_inspection", "fieldtype": "Link", "label": "Quality Inspection", @@ -377,7 +379,7 @@ "no_copy": 1, "options": "BOM", "print_hide": 1, - "read_only_depends_on": "eval:doc.secondary_item_type || doc.is_legacy_scrap_item" + "read_only_depends_on": "eval:doc.secondary_item_type || doc.valuation_type" }, { "fetch_from": "item_code.brand", @@ -504,7 +506,7 @@ "print_hide": 1 }, { - "depends_on": "eval:(doc.use_serial_batch_fields === 0 || doc.docstatus === 1) && !doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:(doc.use_serial_batch_fields === 0 || doc.docstatus === 1) && !doc.secondary_item_type && !doc.valuation_type", "fieldname": "rejected_serial_and_batch_bundle", "fieldtype": "Link", "label": "Rejected Serial and Batch Bundle", @@ -587,7 +589,7 @@ "label": "Add Serial / Batch Bundle" }, { - "depends_on": "eval:doc.use_serial_batch_fields === 0 && !doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:doc.use_serial_batch_fields === 0 && !doc.secondary_item_type && !doc.valuation_type", "fieldname": "add_serial_batch_for_rejected_qty", "fieldtype": "Button", "label": "Add Serial / Batch No (Rejected Qty)" @@ -601,7 +603,7 @@ "search_index": 1 }, { - "depends_on": "eval:!doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.secondary_item_type && !doc.valuation_type", "fieldname": "landed_cost_voucher_amount", "fieldtype": "Currency", "label": "Landed Cost Voucher Amount", @@ -629,7 +631,7 @@ }, { "default": "0", - "depends_on": "eval:!doc.secondary_item_type && !doc.is_legacy_scrap_item", + "depends_on": "eval:!doc.secondary_item_type && !doc.valuation_type", "fieldname": "secondary_items_cost_per_qty", "fieldtype": "Currency", "label": "Secondary Items Cost Per Qty", @@ -639,11 +641,18 @@ "read_only": 1 }, { - "default": "0", - "depends_on": "is_legacy_scrap_item", - "fieldname": "is_legacy_scrap_item", - "fieldtype": "Check", - "label": "Is Legacy Scrap Item", + "depends_on": "valuation_type", + "fieldname": "valuation_type", + "fieldtype": "Select", + "label": "Valuation Type", + "options": "\nValuation Rate\n% of FG Cost\nManual", + "read_only": 1 + }, + { + "fieldname": "bom_secondary_item", + "fieldtype": "Data", + "hidden": 1, + "label": "BOM Secondary Item", "read_only": 1 }, { diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.py b/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.py index 47cfd9a1648..46c710afb48 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt_item/subcontracting_receipt_item.py @@ -17,6 +17,7 @@ class SubcontractingReceiptItem(Document): additional_cost_per_qty: DF.Currency amount: DF.Currency batch_no: DF.Link | None + bom_secondary_item: DF.Data | None bom: DF.Link | None brand: DF.Link | None conversion_factor: DF.Float @@ -25,7 +26,6 @@ class SubcontractingReceiptItem(Document): expense_account: DF.Link | None image: DF.Attach | None include_exploded_items: DF.Check - is_legacy_scrap_item: DF.Check item_code: DF.Link item_name: DF.Data | None job_card: DF.Link | None @@ -64,6 +64,7 @@ class SubcontractingReceiptItem(Document): subcontracting_receipt_item: DF.Data | None secondary_item_type: DF.Literal["", "Co-Product", "By-Product", "Scrap", "Additional Finished Good"] use_serial_batch_fields: DF.Check + valuation_type: DF.Literal["", "Valuation Rate", "% of FG Cost", "Manual"] warehouse: DF.Link | None # end: auto-generated types