From 3ad9393ff8aba8c1b2e4aacc4ef95a5ddf96d4f0 Mon Sep 17 00:00:00 2001 From: Marica Date: Fri, 23 Oct 2020 19:40:55 +0530 Subject: [PATCH] fix: SO to PO flow improvement (#23357) * fix: SO to PO flow improvement * fix: Dont map shipping_address - shipping_address is a text field in SO and link field in PO - Drop shipping case handles its mapping - normal case doesnt need to map * fix: Hide/Add rows depending on Against Default Supplier * fix: Removed Default Supplier Select field from popup - removed Default Supplier Select field from popup - only loop through suppliers of selected items if via default supplier - only check for items in selected items * fix: Sales Order Drop Shipping Test * fix: (translation)Multi line to single line strings Co-authored-by: Nabin Hait --- .../doctype/sales_order/sales_order.js | 161 +++++++++++------- .../doctype/sales_order/sales_order.py | 140 ++++++++------- .../doctype/sales_order/test_sales_order.py | 89 ++++------ 3 files changed, 216 insertions(+), 174 deletions(-) diff --git a/erpnext/selling/doctype/sales_order/sales_order.js b/erpnext/selling/doctype/sales_order/sales_order.js index 7b46fb6fca..989bd33e42 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.js +++ b/erpnext/selling/doctype/sales_order/sales_order.js @@ -162,7 +162,7 @@ erpnext.selling.SalesOrderController = erpnext.selling.SellingController.extend( // sales invoice if(flt(doc.per_billed, 6) < 100) { - this.frm.add_custom_button(__('Invoice'), () => me.make_sales_invoice(), __('Create')); + this.frm.add_custom_button(__('Sales Invoice'), () => me.make_sales_invoice(), __('Create')); } // material request @@ -554,19 +554,32 @@ erpnext.selling.SalesOrderController = erpnext.selling.SellingController.extend( }, make_purchase_order: function(){ + let pending_items = this.frm.doc.items.some((item) =>{ + let pending_qty = flt(item.stock_qty) - flt(item.ordered_qty); + return pending_qty > 0; + }) + if(!pending_items){ + frappe.throw({message: __("Purchase Order already created for all Sales Order items"), title: __("Note")}); + } + var me = this; var dialog = new frappe.ui.Dialog({ - title: __("For Supplier"), + title: __("Select Items"), fields: [ - {"fieldtype": "Link", "label": __("Supplier"), "fieldname": "supplier", "options":"Supplier", - "description": __("Leave the field empty to make purchase orders for all suppliers"), - "get_query": function () { - return { - query:"erpnext.selling.doctype.sales_order.sales_order.get_supplier", - filters: {'parent': me.frm.doc.name} - } - }}, - {fieldname: 'items_for_po', fieldtype: 'Table', label: 'Select Items', + { + "fieldtype": "Check", + "label": __("Against Default Supplier"), + "fieldname": "against_default_supplier", + "default": 0 + }, + { + "fieldtype": "Section Break", + "label": "", + "fieldname": "sec_break_dialog", + "hide_border": 1 + }, + { + fieldname: 'items_for_po', fieldtype: 'Table', label: 'Select Items', fields: [ { fieldtype:'Data', @@ -584,8 +597,8 @@ erpnext.selling.SalesOrderController = erpnext.selling.SellingController.extend( }, { fieldtype:'Float', - fieldname:'qty', - label: __('Quantity'), + fieldname:'pending_qty', + label: __('Pending Qty'), read_only: 1, in_list_view:1 }, @@ -594,60 +607,86 @@ erpnext.selling.SalesOrderController = erpnext.selling.SellingController.extend( read_only:1, fieldname:'uom', label: __('UOM'), + in_list_view:1, + }, + { + fieldtype:'Data', + fieldname:'supplier', + label: __('Supplier'), + read_only:1, in_list_view:1 - } + }, ], - data: cur_frm.doc.items, - get_data: function() { - return cur_frm.doc.items - } - }, - - {"fieldtype": "Button", "label": __('Create Purchase Order'), "fieldname": "make_purchase_order", "cssClass": "btn-primary"}, - ] - }); - - dialog.fields_dict.make_purchase_order.$input.click(function() { - var args = dialog.get_values(); - let selected_items = dialog.fields_dict.items_for_po.grid.get_selected_children() - if(selected_items.length == 0) { - frappe.throw({message: 'Please select Item form Table', title: __('Message'), indicator:'blue'}) - } - let selected_items_list = [] - for(let i in selected_items){ - selected_items_list.push(selected_items[i].item_code) - } - dialog.hide(); - return frappe.call({ - type: "GET", - method: "erpnext.selling.doctype.sales_order.sales_order.make_purchase_order", - args: { - "source_name": me.frm.doc.name, - "for_supplier": args.supplier, - "selected_items": selected_items_list - }, - freeze: true, - callback: function(r) { - if(!r.exc) { - // var args = dialog.get_values(); - if (args.supplier){ - var doc = frappe.model.sync(r.message); - frappe.set_route("Form", r.message.doctype, r.message.name); - } - else{ - frappe.route_options = { - "sales_order": me.frm.doc.name - } - frappe.set_route("List", "Purchase Order"); - } - } + data: me.frm.doc.items.map((item) =>{ + item.pending_qty = (flt(item.stock_qty) - flt(item.ordered_qty)) / flt(item.conversion_factor); + return item; + }).filter((item) => {return item.pending_qty > 0;}) } - }) + ], + primary_action_label: 'Create Purchase Order', + primary_action (args) { + if (!args) return; + let selected_items = dialog.fields_dict.items_for_po.grid.get_selected_children(); + if(selected_items.length == 0) { + frappe.throw({message: 'Please select Items from the Table', title: __('Items Required'), indicator:'blue'}) + } + + dialog.hide(); + + var method = args.against_default_supplier ? "make_purchase_order_for_default_supplier" : "make_purchase_order" + return frappe.call({ + type: "GET", + method: "erpnext.selling.doctype.sales_order.sales_order." + method, + args: { + "source_name": me.frm.doc.name, + "selected_items": selected_items + }, + freeze: true, + callback: function(r) { + if(!r.exc) { + if (!args.against_default_supplier) { + frappe.model.sync(r.message); + frappe.set_route("Form", r.message.doctype, r.message.name); + } + else { + frappe.route_options = { + "sales_order": me.frm.doc.name + } + frappe.set_route("List", "Purchase Order"); + } + } + } + }) + } }); - dialog.get_field("items_for_po").grid.only_sortable() - dialog.get_field("items_for_po").refresh() + + dialog.fields_dict["against_default_supplier"].df.onchange = () => { + console.log("yo"); + var against_default_supplier = dialog.get_value("against_default_supplier"); + var items_for_po = dialog.get_value("items_for_po"); + + if (against_default_supplier) { + let items_with_supplier = items_for_po.filter((item) => item.supplier) + + dialog.fields_dict["items_for_po"].df.data = items_with_supplier; + dialog.get_field("items_for_po").refresh(); + } else { + let pending_items = me.frm.doc.items.map((item) =>{ + item.pending_qty = (flt(item.stock_qty) - flt(item.ordered_qty)) / flt(item.conversion_factor); + return item; + }).filter((item) => {return item.pending_qty > 0;}); + + dialog.fields_dict["items_for_po"].df.data = pending_items; + dialog.get_field("items_for_po").refresh(); + } + } + + dialog.get_field("items_for_po").grid.only_sortable(); + dialog.get_field("items_for_po").refresh(); + dialog.wrapper.find('.grid-heading-row .grid-row-check').click(); dialog.show(); }, + hold_sales_order: function(){ var me = this; var d = new frappe.ui.Dialog({ diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index fe3fa82e84..ae227e0110 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -443,25 +443,19 @@ class SalesOrder(SellingController): for item in self.items: if item.ensure_delivery_based_on_produced_serial_no: if item.item_code in normal_items: - frappe.throw(_("Cannot ensure delivery by Serial No as \ - Item {0} is added with and without Ensure Delivery by \ - Serial No.").format(item.item_code)) + frappe.throw(_("Cannot ensure delivery by Serial No as Item {0} is added with and without Ensure Delivery by Serial No.").format(item.item_code)) if item.item_code not in reserved_items: if not frappe.get_cached_value("Item", item.item_code, "has_serial_no"): - frappe.throw(_("Item {0} has no Serial No. Only serilialized items \ - can have delivery based on Serial No").format(item.item_code)) + frappe.throw(_("Item {0} has no Serial No. Only serilialized items can have delivery based on Serial No").format(item.item_code)) if not frappe.db.exists("BOM", {"item": item.item_code, "is_active": 1}): - frappe.throw(_("No active BOM found for item {0}. Delivery by \ - Serial No cannot be ensured").format(item.item_code)) + frappe.throw(_("No active BOM found for item {0}. Delivery by Serial No cannot be ensured").format(item.item_code)) reserved_items.append(item.item_code) else: normal_items.append(item.item_code) if not item.ensure_delivery_based_on_produced_serial_no and \ item.item_code in reserved_items: - frappe.throw(_("Cannot ensure delivery by Serial No as \ - Item {0} is added with and without Ensure Delivery by \ - Serial No.").format(item.item_code)) + frappe.throw(_("Cannot ensure delivery by Serial No as Item {0} is added with and without Ensure Delivery by Serial No.").format(item.item_code)) def get_list_context(context=None): from erpnext.controllers.website_list_for_contact import get_list_context @@ -785,7 +779,7 @@ def get_events(start, end, filters=None): return data @frappe.whitelist() -def make_purchase_order(source_name, for_supplier=None, selected_items=[], target_doc=None): +def make_purchase_order_for_default_supplier(source_name, selected_items=[], target_doc=None): if isinstance(selected_items, string_types): selected_items = json.loads(selected_items) @@ -822,24 +816,21 @@ def make_purchase_order(source_name, for_supplier=None, selected_items=[], targe def update_item(source, target, source_parent): target.schedule_date = source.delivery_date - target.qty = flt(source.qty) - flt(source.ordered_qty) - target.stock_qty = (flt(source.qty) - flt(source.ordered_qty)) * flt(source.conversion_factor) + target.qty = flt(source.qty) - (flt(source.ordered_qty) / flt(source.conversion_factor)) + target.stock_qty = (flt(source.stock_qty) - flt(source.ordered_qty)) target.project = source_parent.project - suppliers =[] - if for_supplier: - suppliers.append(for_supplier) - else: - sales_order = frappe.get_doc("Sales Order", source_name) - for item in sales_order.items: - if item.supplier and item.supplier not in suppliers: - suppliers.append(item.supplier) + suppliers = [item.get('supplier') for item in selected_items if item.get('supplier') and item.get('supplier')] + suppliers = list(set(suppliers)) + + items_to_map = [item.get('item_code') for item in selected_items if item.get('item_code') and item.get('item_code')] + items_to_map = list(set(items_to_map)) if not suppliers: frappe.throw(_("Please set a Supplier against the Items to be considered in the Purchase Order.")) for supplier in suppliers: - po =frappe.get_list("Purchase Order", filters={"sales_order":source_name, "supplier":supplier, "docstatus": ("<", "2")}) + po = frappe.get_list("Purchase Order", filters={"sales_order":source_name, "supplier":supplier, "docstatus": ("<", "2")}) if len(po) == 0: doc = get_mapped_doc("Sales Order", source_name, { "Sales Order": { @@ -850,7 +841,8 @@ def make_purchase_order(source_name, for_supplier=None, selected_items=[], targe "contact_mobile", "contact_email", "contact_person", - "taxes_and_charges" + "taxes_and_charges", + "shipping_address" ], "validation": { "docstatus": ["=", 1] @@ -872,52 +864,82 @@ def make_purchase_order(source_name, for_supplier=None, selected_items=[], targe "item_tax_template" ], "postprocess": update_item, - "condition": lambda doc: doc.ordered_qty < doc.qty and doc.supplier == supplier and doc.item_code in selected_items + "condition": lambda doc: doc.ordered_qty < doc.stock_qty and doc.supplier == supplier and doc.item_code in items_to_map } }, target_doc, set_missing_values) - if not for_supplier: - doc.insert() + + doc.insert() else: suppliers =[] if suppliers: - if not for_supplier: - frappe.db.commit() + frappe.db.commit() return doc else: - frappe.msgprint(_("PO already created for all sales order items")) - + frappe.msgprint(_("Purchase Order already created for all Sales Order items")) @frappe.whitelist() -@frappe.validate_and_sanitize_search_inputs -def get_supplier(doctype, txt, searchfield, start, page_len, filters): - supp_master_name = frappe.defaults.get_user_default("supp_master_name") - if supp_master_name == "Supplier Name": - fields = ["name", "supplier_group"] - else: - fields = ["name", "supplier_name", "supplier_group"] - fields = ", ".join(fields) +def make_purchase_order(source_name, selected_items=[], target_doc=None): + if isinstance(selected_items, string_types): + selected_items = json.loads(selected_items) - return frappe.db.sql("""select {field} from `tabSupplier` - where docstatus < 2 - and ({key} like %(txt)s - or supplier_name like %(txt)s) - and name in (select supplier from `tabSales Order Item` where parent = %(parent)s) - and name not in (select supplier from `tabPurchase Order` po inner join `tabPurchase Order Item` poi - on po.name=poi.parent where po.docstatus<2 and poi.sales_order=%(parent)s) - order by - if(locate(%(_txt)s, name), locate(%(_txt)s, name), 99999), - if(locate(%(_txt)s, supplier_name), locate(%(_txt)s, supplier_name), 99999), - name, supplier_name - limit %(start)s, %(page_len)s """.format(**{ - 'field': fields, - 'key': frappe.db.escape(searchfield) - }), { - 'txt': "%%%s%%" % txt, - '_txt': txt.replace("%", ""), - 'start': start, - 'page_len': page_len, - 'parent': filters.get('parent') - }) + items_to_map = [item.get('item_code') for item in selected_items if item.get('item_code') and item.get('item_code')] + items_to_map = list(set(items_to_map)) + + def set_missing_values(source, target): + target.supplier = "" + target.apply_discount_on = "" + target.additional_discount_percentage = 0.0 + target.discount_amount = 0.0 + target.inter_company_order_reference = "" + target.customer = "" + target.customer_name = "" + target.run_method("set_missing_values") + target.run_method("calculate_taxes_and_totals") + + def update_item(source, target, source_parent): + target.schedule_date = source.delivery_date + target.qty = flt(source.qty) - (flt(source.ordered_qty) / flt(source.conversion_factor)) + target.stock_qty = (flt(source.stock_qty) - flt(source.ordered_qty)) + target.project = source_parent.project + + # po = frappe.get_list("Purchase Order", filters={"sales_order":source_name, "supplier":supplier, "docstatus": ("<", "2")}) + doc = get_mapped_doc("Sales Order", source_name, { + "Sales Order": { + "doctype": "Purchase Order", + "field_no_map": [ + "address_display", + "contact_display", + "contact_mobile", + "contact_email", + "contact_person", + "taxes_and_charges", + "shipping_address" + ], + "validation": { + "docstatus": ["=", 1] + } + }, + "Sales Order Item": { + "doctype": "Purchase Order Item", + "field_map": [ + ["name", "sales_order_item"], + ["parent", "sales_order"], + ["stock_uom", "stock_uom"], + ["uom", "uom"], + ["conversion_factor", "conversion_factor"], + ["delivery_date", "schedule_date"] + ], + "field_no_map": [ + "rate", + "price_list_rate", + "item_tax_template", + "supplier" + ], + "postprocess": update_item, + "condition": lambda doc: doc.ordered_qty < doc.stock_qty and doc.item_code in items_to_map + } + }, target_doc, set_missing_values) + return doc @frappe.whitelist() def make_work_orders(items, sales_order, company, project=None): diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 2f5f979bdf..9e25ed0c99 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -688,12 +688,12 @@ class TestSalesOrder(unittest.TestCase): frappe.db.set_value("Stock Settings", None, "auto_insert_price_list_rate_if_missing", 1) def test_drop_shipping(self): - from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order + from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order_for_default_supplier, \ + update_status as so_update_status from erpnext.buying.doctype.purchase_order.purchase_order import update_status - make_stock_entry(target="_Test Warehouse - _TC", qty=10, rate=100) + # make items po_item = make_item("_Test Item for Drop Shipping", {"is_stock_item": 1, "delivered_by_supplier": 1}) - dn_item = make_item("_Test Regular Item", {"is_stock_item": 1}) so_items = [ @@ -715,80 +715,61 @@ class TestSalesOrder(unittest.TestCase): ] if frappe.db.get_value("Item", "_Test Regular Item", "is_stock_item")==1: - make_stock_entry(item="_Test Regular Item", target="_Test Warehouse - _TC", qty=10, rate=100) + make_stock_entry(item="_Test Regular Item", target="_Test Warehouse - _TC", qty=2, rate=100) - #setuo existing qty from bin - bin = frappe.get_all("Bin", filters={"item_code": po_item.item_code, "warehouse": "_Test Warehouse - _TC"}, - fields=["ordered_qty", "reserved_qty"]) - - existing_ordered_qty = bin[0].ordered_qty if bin else 0.0 - existing_reserved_qty = bin[0].reserved_qty if bin else 0.0 - - bin = frappe.get_all("Bin", filters={"item_code": dn_item.item_code, - "warehouse": "_Test Warehouse - _TC"}, fields=["reserved_qty"]) - - existing_reserved_qty_for_dn_item = bin[0].reserved_qty if bin else 0.0 - - #create so, po and partial dn + #create so, po and dn so = make_sales_order(item_list=so_items, do_not_submit=True) so.submit() - po = make_purchase_order(so.name, '_Test Supplier', selected_items=[so_items[0]['item_code']]) + po = make_purchase_order_for_default_supplier(so.name, selected_items=[so_items[0]]) po.submit() - dn = create_dn_against_so(so.name, delivered_qty=1) + dn = create_dn_against_so(so.name, delivered_qty=2) self.assertEqual(so.customer, po.customer) self.assertEqual(po.items[0].sales_order, so.name) self.assertEqual(po.items[0].item_code, po_item.item_code) self.assertEqual(dn.items[0].item_code, dn_item.item_code) - - #test ordered_qty and reserved_qty - bin = frappe.get_all("Bin", filters={"item_code": po_item.item_code, "warehouse": "_Test Warehouse - _TC"}, - fields=["ordered_qty", "reserved_qty"]) - - ordered_qty = bin[0].ordered_qty if bin else 0.0 - reserved_qty = bin[0].reserved_qty if bin else 0.0 - - self.assertEqual(abs(flt(ordered_qty)), existing_ordered_qty) - self.assertEqual(abs(flt(reserved_qty)), existing_reserved_qty) - - reserved_qty = frappe.db.get_value("Bin", - {"item_code": dn_item.item_code, "warehouse": "_Test Warehouse - _TC"}, "reserved_qty") - - self.assertEqual(abs(flt(reserved_qty)), existing_reserved_qty_for_dn_item + 1) - #test po_item length self.assertEqual(len(po.items), 1) - #test per_delivered status + # test ordered_qty and reserved_qty for drop ship item + bin_po_item = frappe.get_all("Bin", filters={"item_code": po_item.item_code, "warehouse": "_Test Warehouse - _TC"}, + fields=["ordered_qty", "reserved_qty"]) + + ordered_qty = bin_po_item[0].ordered_qty if bin_po_item else 0.0 + reserved_qty = bin_po_item[0].reserved_qty if bin_po_item else 0.0 + + # drop ship PO should not impact bin, test the same + self.assertEqual(abs(flt(ordered_qty)), 0) + self.assertEqual(abs(flt(reserved_qty)), 0) + + # test per_delivered status update_status("Delivered", po.name) - self.assertEqual(flt(frappe.db.get_value("Sales Order", so.name, "per_delivered"), 2), 75.00) + self.assertEqual(flt(frappe.db.get_value("Sales Order", so.name, "per_delivered"), 2), 100.00) + po.load_from_db() - #test reserved qty after complete delivery - dn = create_dn_against_so(so.name, delivered_qty=1) - reserved_qty = frappe.db.get_value("Bin", - {"item_code": dn_item.item_code, "warehouse": "_Test Warehouse - _TC"}, "reserved_qty") - - self.assertEqual(abs(flt(reserved_qty)), existing_reserved_qty_for_dn_item) - - #test after closing so + # test after closing so so.db_set('status', "Closed") so.update_reserved_qty() - bin = frappe.get_all("Bin", filters={"item_code": po_item.item_code, "warehouse": "_Test Warehouse - _TC"}, + # test ordered_qty and reserved_qty for drop ship item after closing so + bin_po_item = frappe.get_all("Bin", filters={"item_code": po_item.item_code, "warehouse": "_Test Warehouse - _TC"}, fields=["ordered_qty", "reserved_qty"]) - ordered_qty = bin[0].ordered_qty if bin else 0.0 - reserved_qty = bin[0].reserved_qty if bin else 0.0 + ordered_qty = bin_po_item[0].ordered_qty if bin_po_item else 0.0 + reserved_qty = bin_po_item[0].reserved_qty if bin_po_item else 0.0 - self.assertEqual(abs(flt(ordered_qty)), existing_ordered_qty) - self.assertEqual(abs(flt(reserved_qty)), existing_reserved_qty) + self.assertEqual(abs(flt(ordered_qty)), 0) + self.assertEqual(abs(flt(reserved_qty)), 0) - reserved_qty = frappe.db.get_value("Bin", - {"item_code": dn_item.item_code, "warehouse": "_Test Warehouse - _TC"}, "reserved_qty") - - self.assertEqual(abs(flt(reserved_qty)), existing_reserved_qty_for_dn_item) + # teardown + so_update_status("Draft", so.name) + dn.load_from_db() + dn.cancel() + po.cancel() + so.load_from_db() + so.cancel() def test_reserved_qty_for_closing_so(self): bin = frappe.get_all("Bin", filters={"item_code": "_Test Item", "warehouse": "_Test Warehouse - _TC"},