diff --git a/erpnext/controllers/subcontracting_inward_controller.py b/erpnext/controllers/subcontracting_inward_controller.py index 4907f2d8484..892e5767adc 100644 --- a/erpnext/controllers/subcontracting_inward_controller.py +++ b/erpnext/controllers/subcontracting_inward_controller.py @@ -4,6 +4,7 @@ import frappe from frappe import _, bold from frappe.query_builder import Case from frappe.utils import flt, get_link_to_form +from pypika.terms import ValueWrapper from erpnext.stock.serial_batch_bundle import get_serial_batch_list_from_item @@ -161,6 +162,23 @@ class SubcontractingInwardController: customer_warehouse = frappe.get_cached_value( "Subcontracting Inward Order", self.subcontracting_inward_order, "customer_warehouse" ) + work_order_items = frappe.get_all( + "Work Order Item", + {"parent": self.work_order, "docstatus": 1, "is_customer_provided_item": 1}, + ["item_code", "transferred_qty", "required_qty", "stock_reserved_qty"], + ) + wo_item_dict = frappe._dict( + { + wo_item.item_code: frappe._dict( + { + "transferred_qty": wo_item.transferred_qty, + "required_qty": wo_item.required_qty, + "stock_reserved_qty": wo_item.stock_reserved_qty, + } + ) + for wo_item in work_order_items + } + ) item_codes = [] for item in self.items: if not frappe.get_cached_value("Item", item.item_code, "is_customer_provided_item"): @@ -183,23 +201,6 @@ class SubcontractingInwardController: ) ) else: - work_order_items = frappe.get_all( - "Work Order Item", - {"parent": self.work_order, "docstatus": 1, "is_customer_provided_item": 1}, - ["item_code", "transferred_qty", "required_qty", "stock_reserved_qty"], - ) - wo_item_dict = frappe._dict( - { - wo_item.item_code: frappe._dict( - { - "transferred_qty": wo_item.transferred_qty, - "required_qty": wo_item.required_qty, - "stock_reserved_qty": wo_item.stock_reserved_qty, - } - ) - for wo_item in work_order_items - } - ) if wo_item := wo_item_dict.get(item.item_code): if wo_item.transferred_qty + item.transfer_qty > max( wo_item.required_qty, wo_item.stock_reserved_qty @@ -230,7 +231,7 @@ class SubcontractingInwardController: ): frappe.throw( _( - "Target Warehouse for Finished Good must be same as Finished Good Warehouse {1} in Work Order {2} linked to the Subcontracting Inward Order." + "Target Warehouse for Finished Good must be same as Finished Good Warehouse {0} in Work Order {1} linked to the Subcontracting Inward Order." ).format( get_link_to_form("Warehouse", fg_warehouse), get_link_to_form("Work Order", self.work_order), @@ -246,126 +247,133 @@ class SubcontractingInwardController: and frappe.get_cached_value("Item", item.item_code, "is_customer_provided_item") ] + if frappe.get_cached_value("Work Order", self.work_order, "skip_transfer"): + self._validate_manufacture_consumption_against_scio(items) + else: + self._validate_manufacture_consumption_against_work_order(items) + + def _validate_manufacture_consumption_against_scio(self, items): customer_warehouse = frappe.get_cached_value( "Subcontracting Inward Order", self.subcontracting_inward_order, "customer_warehouse" ) - if frappe.get_cached_value("Work Order", self.work_order, "skip_transfer"): - table = frappe.qb.DocType("Subcontracting Inward Order Received Item") - query = ( - frappe.qb.from_(table) - .select( - table.rm_item_code, - (table.received_qty - table.returned_qty).as_("total_qty"), - table.consumed_qty, - table.name, - ) - .where( - (table.docstatus == 1) - & (table.parent == self.subcontracting_inward_order) - & ( - table.reference_name - == frappe.get_cached_value( - "Work Order", self.work_order, "subcontracting_inward_order_item" - ) - ) - & (table.rm_item_code.isin([item.item_code for item in items])) - ) + table = frappe.qb.DocType("Subcontracting Inward Order Received Item") + query = ( + frappe.qb.from_(table) + .select( + table.rm_item_code, + table.consumed_qty, + (table.received_qty - table.returned_qty).as_("available_qty"), ) - rm_item_dict = frappe._dict( - { - d.rm_item_code: frappe._dict( - {"name": d.name, "total_qty": d.total_qty, "qty": d.consumed_qty} + .where( + (table.docstatus == 1) + & (table.parent == self.subcontracting_inward_order) + & ( + table.reference_name + == frappe.get_cached_value( + "Work Order", self.work_order, "subcontracting_inward_order_item" ) - for d in query.run(as_dict=True) - } + ) + & (table.rm_item_code.isin([item.item_code for item in items])) + ) + ) + lookup = { + d.rm_item_code: frappe._dict(consumed_qty=d.consumed_qty, available_qty=d.available_qty) + for d in query.run(as_dict=True) + } + + def on_missing(item): + frappe.throw( + _( + "Row #{0}: Customer Provided Item {1} is not a part of Subcontracting Inward Order {2}" + ).format( + item.idx, + get_link_to_form("Item", item.item_code), + get_link_to_form("Subcontracting Inward Order", self.subcontracting_inward_order), + ) ) - item_codes = [] - for item in items: - if rm := rm_item_dict.get(item.item_code): - if rm.qty + item.transfer_qty > rm.total_qty: - frappe.throw( - _( - "Row #{0}: Customer Provided Item {1} exceeds quantity available through Subcontracting Inward Order" - ).format(item.idx, get_link_to_form("Item", item.item_code), item.transfer_qty) - ) - elif item.s_warehouse != customer_warehouse: - frappe.throw( - _( - "Row #{0}: For Customer Provided Item {1}, Source Warehouse must be {2}" - ).format( - item.idx, - get_link_to_form("Item", item.item_code), - get_link_to_form("Warehouse", customer_warehouse), - ) - ) - elif item.item_code in item_codes: - frappe.throw( - _( - "Row #{0}: Customer Provided Item {1} cannot be added multiple times in the Subcontracting Inward process." - ).format( - item.idx, - get_link_to_form("Item", item.item_code), - ) - ) - else: - item_codes.append(item.item_code) - else: + def on_overconsumption(item): + frappe.throw( + _( + "Row #{0}: Customer Provided Item {1} exceeds quantity available through Subcontracting Inward Order" + ).format(item.idx, get_link_to_form("Item", item.item_code)) + ) + + def check_source_warehouse(item): + if item.s_warehouse != customer_warehouse: + frappe.throw( + _("Row #{0}: For Customer Provided Item {1}, Source Warehouse must be {2}").format( + item.idx, + get_link_to_form("Item", item.item_code), + get_link_to_form("Warehouse", customer_warehouse), + ) + ) + + self._validate_customer_provided_consumption( + items, lookup, on_missing, on_overconsumption, check_source_warehouse + ) + + def _validate_manufacture_consumption_against_work_order(self, items): + work_order_items = frappe.get_all( + "Work Order Item", + {"parent": self.work_order, "docstatus": 1, "is_customer_provided_item": 1}, + ["item_code", "transferred_qty", "consumed_qty"], + ) + lookup = { + wo_item.item_code: frappe._dict( + consumed_qty=wo_item.consumed_qty, available_qty=wo_item.transferred_qty + ) + for wo_item in work_order_items + } + + def on_missing(item): + frappe.throw( + _("Row #{0}: Customer Provided Item {1} is not a part of Work Order {2}").format( + item.idx, + get_link_to_form("Item", item.item_code), + get_link_to_form("Work Order", self.work_order), + ) + ) + + def on_overconsumption(item): + frappe.throw( + _( + "Row #{0}: Overconsumption of Customer Provided Item {1} against Work Order {2} is not allowed in the Subcontracting Inward process." + ).format( + item.idx, + get_link_to_form("Item", item.item_code), + get_link_to_form("Work Order", self.work_order), + ) + ) + + self._validate_customer_provided_consumption(items, lookup, on_missing, on_overconsumption) + + def _validate_customer_provided_consumption( + self, items, lookup, on_missing, on_overconsumption, extra_check=None + ): + """Shared per-item guard for the skip-transfer and transfer manufacture paths. + + `lookup` maps item_code -> {consumed_qty, available_qty}; the branch-specific + throw messages are supplied as callbacks. `extra_check` runs an extra per-item + validation (the source-warehouse check on the skip-transfer path). + """ + seen = [] + for item in items: + record = lookup.get(item.item_code) + if not record: + on_missing(item) + elif record.consumed_qty + item.transfer_qty > record.available_qty: + on_overconsumption(item) + else: + if extra_check: + extra_check(item) + if item.item_code in seen: frappe.throw( _( - "Row #{0}: Customer Provided Item {1} is not a part of Subcontracting Inward Order {2}" - ).format( - item.idx, - get_link_to_form("Item", item.item_code), - get_link_to_form("Subcontracting Inward Order", self.subcontracting_inward_order), - ) - ) - else: - work_order_items = frappe.get_all( - "Work Order Item", - {"parent": self.work_order, "docstatus": 1, "is_customer_provided_item": 1}, - ["item_code", "transferred_qty", "consumed_qty"], - ) - wo_item_dict = frappe._dict( - { - wo_item.item_code: frappe._dict( - {"transferred_qty": wo_item.transferred_qty, "consumed_qty": wo_item.consumed_qty} - ) - for wo_item in work_order_items - } - ) - item_codes = [] - for item in items: - if wo_item := wo_item_dict.get(item.item_code): - if wo_item.consumed_qty + item.transfer_qty > wo_item.transferred_qty: - frappe.throw( - _( - "Row #{0}: Overconsumption of Customer Provided Item {1} against Work Order {2} is not allowed in the Subcontracting Inward process." - ).format( - item.idx, - get_link_to_form("Item", item.item_code), - get_link_to_form("Work Order", self.work_order), - ) - ) - elif item.item_code in item_codes: - frappe.throw( - _( - "Row #{0}: Customer Provided Item {1} cannot be added multiple times in the Subcontracting Inward process." - ).format( - item.idx, - get_link_to_form("Item", item.item_code), - ) - ) - else: - item_codes.append(item.item_code) - else: - frappe.throw( - _("Row #{0}: Customer Provided Item {1} is not a part of Work Order {2}").format( - item.idx, - get_link_to_form("Item", item.item_code), - get_link_to_form("Work Order", self.work_order), - ) + "Row #{0}: Customer Provided Item {1} cannot be added multiple times in the Subcontracting Inward process." + ).format(item.idx, get_link_to_form("Item", item.item_code)) ) + seen.append(item.item_code) def set_allow_zero_valuation_rate(self): if self.subcontracting_inward_order: @@ -500,8 +508,6 @@ class SubcontractingInwardController: ) ) - from pypika.terms import ValueWrapper - table = frappe.qb.DocType("Subcontracting Inward Order Item") query = ( frappe.qb.from_(table) @@ -646,10 +652,9 @@ class SubcontractingInwardController: "Work Order", self.work_order, "subcontracting_inward_order_item" ) ): - if scio_item_name: - frappe.get_doc( - "Subcontracting Inward Order Item", scio_item_name - ).update_manufacturing_qty_fields() + frappe.get_doc( + "Subcontracting Inward Order Item", scio_item_name + ).update_manufacturing_qty_fields() elif self.purpose in ["Subcontracting Delivery", "Subcontracting Return"]: fieldname = "delivered_qty" if self.purpose == "Subcontracting Delivery" else "returned_qty" qty_map = defaultdict(lambda: defaultdict(float)) @@ -700,6 +705,13 @@ class SubcontractingInwardController: def update_inward_order_received_items_for_raw_materials_receipt(self): data = frappe._dict() + next_received_idx = ( + frappe.db.count( + "Subcontracting Inward Order Received Item", + {"parent": self.subcontracting_inward_order}, + ) + + 1 + ) for item in self.items: if item.scio_detail: data[item.scio_detail] = frappe._dict( @@ -711,11 +723,7 @@ class SubcontractingInwardController: parent=self.subcontracting_inward_order, parenttype="Subcontracting Inward Order", parentfield="received_items", - idx=frappe.db.count( - "Subcontracting Inward Order Received Item", - {"parent": self.subcontracting_inward_order}, - ) - + 1, + idx=next_received_idx, rm_item_code=item.item_code, stock_uom=item.stock_uom, warehouse=item.t_warehouse, @@ -734,6 +742,7 @@ class SubcontractingInwardController: scio_rm.flags.skip_docstatus_validation = True scio_rm.insert() scio_rm.submit() + next_received_idx += 1 item.db_set("scio_detail", scio_rm.name) if data: @@ -795,6 +804,9 @@ class SubcontractingInwardController: 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 items: + return + item_code_wh = frappe._dict( { ( @@ -862,6 +874,13 @@ class SubcontractingInwardController: ) main_item_code = next(fg for fg in self.items if fg.is_finished_item).item_code + next_received_idx = ( + frappe.db.count( + "Subcontracting Inward Order Received Item", + {"parent": self.subcontracting_inward_order}, + ) + + 1 + ) for extra_item in [ item for item in items @@ -874,11 +893,7 @@ class SubcontractingInwardController: parent=self.subcontracting_inward_order, parenttype="Subcontracting Inward Order", parentfield="received_items", - idx=frappe.db.count( - "Subcontracting Inward Order Received Item", - {"parent": self.subcontracting_inward_order}, - ) - + 1, + idx=next_received_idx, main_item_code=main_item_code, rm_item_code=extra_item.item_code, stock_uom=extra_item.stock_uom, @@ -893,6 +908,7 @@ class SubcontractingInwardController: doc.flags.skip_docstatus_validation = True doc.insert() doc.submit() + next_received_idx += 1 def update_inward_order_secondary_items(self): if (scio := self.subcontracting_inward_order) and self.purpose == "Manufacture": @@ -955,6 +971,9 @@ class SubcontractingInwardController: ) fg_item_code = next(fg for fg in self.items if fg.is_finished_item).item_code + next_secondary_idx = ( + frappe.db.count("Subcontracting Inward Order Secondary Item", {"parent": scio}) + 1 + ) for secondary_item in [ item for item in secondary_items_list @@ -965,8 +984,7 @@ class SubcontractingInwardController: parent=scio, parenttype="Subcontracting Inward Order", parentfield="secondary_items", - idx=frappe.db.count("Subcontracting Inward Order Secondary Item", {"parent": scio}) - + 1, + idx=next_secondary_idx, item_code=secondary_item.item_code, fg_item_code=fg_item_code, stock_uom=secondary_item.stock_uom, @@ -981,6 +999,7 @@ class SubcontractingInwardController: doc.flags.skip_docstatus_validation = True doc.insert() doc.submit() + next_secondary_idx += 1 def cancel_stock_reservation_entries_for_inward(self): if self.purpose == "Receive from Customer": @@ -1151,7 +1170,7 @@ def get_fg_reference_names( "Subcontracting Inward Order Item", limit_start=start, limit_page_length=page_len, - filters={"parent": filters.get("parent"), "item_code": ("like", "%%%s%%" % txt), "docstatus": 1}, + filters={"parent": filters.get("parent"), "item_code": ("like", f"%{txt}%"), "docstatus": 1}, fields=["name", "item_code", "delivery_warehouse"], as_list=True, order_by="idx", diff --git a/erpnext/subcontracting/doctype/subcontracting_inward_order/test_subcontracting_inward_order.py b/erpnext/subcontracting/doctype/subcontracting_inward_order/test_subcontracting_inward_order.py index 4e8c62a62f4..e7c5da910a4 100644 --- a/erpnext/subcontracting/doctype/subcontracting_inward_order/test_subcontracting_inward_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_inward_order/test_subcontracting_inward_order.py @@ -330,6 +330,31 @@ class IntegrationTestSubcontractingInwardOrder(ERPNextTestSuite): self.assertEqual(scio.items[0].delivered_qty, 2) self.assertEqual(scio.items[0].returned_qty, 1) + def test_manufacture_consumption_validates_against_work_order(self): + """Cover the non-skip-transfer manufacture path, where consumption is validated + against the Work Order's transferred quantity (the Work Order branch of + validate_manufacture).""" + so, scio = create_so_scio() + frappe.new_doc("Stock Entry").update(scio.make_rm_stock_entry_inward()).submit() + + scio.reload() + wo = frappe.get_doc("Work Order", scio.make_work_order()[0]) + wo.wip_warehouse = "Work In Progress - _TC" + next( + item for item in wo.required_items if item.item_code == "Self RM" + ).source_warehouse = "Stores - _TC" + wo.submit() + + frappe.new_doc("Stock Entry").update( + make_stock_entry_from_wo(wo.name, "Material Transfer for Manufacture") + ).submit() + + manufacture = frappe.new_doc("Stock Entry").update(make_stock_entry_from_wo(wo.name, "Manufacture")) + manufacture.submit() + + scio.reload() + self.assertEqual(scio.items[0].produced_qty, 5) + @ERPNextTestSuite.change_settings("Selling Settings", {"allow_delivery_of_overproduced_qty": 1}) @ERPNextTestSuite.change_settings( "Manufacturing Settings", {"overproduction_percentage_for_work_order": 20}