From 1e912db3bb43afcff98947e46149c1c848287c0e Mon Sep 17 00:00:00 2001 From: noahjacob Date: Mon, 26 Apr 2021 15:46:18 +0530 Subject: [PATCH 01/39] feat: Added check box to combine items --- .../production_plan/production_plan.js | 18 +- .../production_plan/production_plan.json | 22 +- .../production_plan/production_plan.py | 43 +- .../production_plan_item.json | 950 ++++-------------- .../__init__.py | 0 .../production_plan_item_reference.json | 52 + .../production_plan_item_reference.py | 10 + .../doctype/work_order/work_order.py | 27 +- 8 files changed, 355 insertions(+), 767 deletions(-) create mode 100644 erpnext/manufacturing/doctype/production_plan_item_reference/__init__.py create mode 100644 erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json create mode 100644 erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.py diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 288c1d0cd6..39b8c94bc1 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -211,16 +211,30 @@ frappe.ui.form.on('Production Plan', { }); }, - get_items: function(frm) { + get_items: function (frm) { + frm.clear_table('prod_plan_ref'); + frappe.call({ method: "get_items", freeze: true, doc: frm.doc, - callback: function() { + callback: function () { refresh_field('po_items'); } }); }, + combine_items: function (frm) { + frm.clear_table('po_items'); + frm.clear_table('prod_plan_ref'); + + frappe.call({ + method: "get_items", + freeze: true, + doc: frm.doc, + }); + + + }, get_items_for_mr: function(frm) { if (!frm.doc.for_warehouse) { diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index f11470086a..5c73992d1b 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -28,7 +28,10 @@ "material_requests", "select_items_to_manufacture_section", "get_items", + "combine_items", "po_items", + "section_break_25", + "prod_plan_ref", "material_request_planning", "include_non_stock_items", "include_subcontracted_items", @@ -316,13 +319,30 @@ "fieldname": "include_safety_stock", "fieldtype": "Check", "label": "Include Safety Stock in Required Qty Calculation" + }, + { + "fieldname": "prod_plan_ref", + "fieldtype": "Table", + "hidden": 1, + "label": "Production Plan Item Reference", + "options": "Production Plan Item Reference" + }, + { + "default": "0", + "fieldname": "combine_items", + "fieldtype": "Check", + "label": "Consolidate Items" + }, + { + "fieldname": "section_break_25", + "fieldtype": "Section Break" } ], "icon": "fa fa-calendar", "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-03-08 11:17:25.470147", + "modified": "2021-04-26 14:11:43.564957", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index a3e23a6897..b2bc21fb6d 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -96,8 +96,10 @@ class ProductionPlan(Document): @frappe.whitelist() def get_items(self): + self.set('po_items', []) if self.get_items_from == "Sales Order": - self.get_so_items() + self.get_so_items() + elif self.get_items_from == "Material Request": self.get_mr_items() @@ -165,10 +167,24 @@ class ProductionPlan(Document): self.calculate_total_planned_qty() def add_items(self, items): - self.set('po_items', []) + refs = {} for data in items: item_details = get_item_details(data.item_code) + if self.combine_items: + if item_details.bom_no in refs.keys(): + refs[item_details.bom_no]['qty'] = refs[item_details.bom_no]['qty'] + data.pending_qty + refs[item_details.bom_no]['so'].append(data.parent) + refs[item_details.bom_no]['so_items'].append(data.name) + refs[item_details.bom_no]['planned_qty'].append(data.pending_qty) + continue + else: + refs[item_details.bom_no] = {'qty': data.pending_qty, 'ref': data.name} + refs[item_details.bom_no]['so'] = [data.parent] + refs[item_details.bom_no]['so_items'] = [data.name] + refs[item_details.bom_no]['planned_qty'] = [data.pending_qty] + pi = self.append('po_items', { + 'name': data.name, 'include_exploded_items': 1, 'warehouse': data.warehouse, 'item_code': data.item_code, @@ -185,11 +201,32 @@ class ProductionPlan(Document): pi.sales_order = data.parent pi.sales_order_item = data.name pi.description = data.description - + + elif self.get_items_from == "Material Request": pi.material_request = data.parent pi.material_request_item = data.name pi.description = data.description + + if refs: + for d in self.po_items: + d.planned_qty = refs[d.bom_no]['qty'] + d.pending_qty = refs[d.bom_no]['qty'] + d.sales_order = '' + self.add_pp_ref(refs) + + def add_pp_ref(self, refs): + for r in refs: + idx = 0 + for so in refs[r]['so']: + self.append('prod_plan_ref', { + 'item_ref': refs[r]['ref'], + 'sales_order': so, + 'sales_order_item':refs[r]['so_items'][idx], + 'qty':refs[r]['planned_qty'][idx] + }) + idx+=1 + def calculate_total_planned_qty(self): self.total_planned_qty = 0 diff --git a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json index d0dce53437..9ff1717e70 100644 --- a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json +++ b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json @@ -1,792 +1,222 @@ { - "allow_copy": 0, - "allow_events_in_timeline": 0, - "allow_guest_to_view": 0, - "allow_import": 0, - "allow_rename": 0, - "autoname": "hash", - "beta": 0, - "creation": "2013-02-22 01:27:49", - "custom": 0, - "docstatus": 0, - "doctype": "DocType", - "editable_grid": 1, + "actions": [], + "autoname": "hash", + "creation": "2013-02-22 01:27:49", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "include_exploded_items", + "item_code", + "bom_no", + "planned_qty", + "column_break_6", + "make_work_order_for_sub_assembly_items", + "warehouse", + "planned_start_date", + "section_break_9", + "pending_qty", + "ordered_qty", + "produced_qty", + "column_break_17", + "description", + "stock_uom", + "reference_section", + "sales_order", + "sales_order_item", + "column_break_19", + "material_request", + "material_request_item", + "product_bundle_item" + ], "fields": [ { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 2, - "fetch_if_empty": 0, - "fieldname": "include_exploded_items", - "fieldtype": "Check", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Include Exploded Items", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "columns": 2, + "default": "0", + "fieldname": "include_exploded_items", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Include Exploded Items" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 2, - "fetch_if_empty": 0, - "fieldname": "item_code", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Item Code", - "length": 0, - "no_copy": 0, - "oldfieldname": "item_code", - "oldfieldtype": "Link", - "options": "Item", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "150px", - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "columns": 2, + "fieldname": "item_code", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Item Code", + "oldfieldname": "item_code", + "oldfieldtype": "Link", + "options": "Item", + "print_width": "150px", + "reqd": 1, "width": "150px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 2, - "fetch_if_empty": 0, - "fieldname": "bom_no", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "BOM No", - "length": 0, - "no_copy": 0, - "oldfieldname": "bom_no", - "oldfieldtype": "Link", - "options": "BOM", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "100px", - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "columns": 2, + "fieldname": "bom_no", + "fieldtype": "Link", + "in_list_view": 1, + "label": "BOM No", + "oldfieldname": "bom_no", + "oldfieldtype": "Link", + "options": "BOM", + "print_width": "100px", + "reqd": 1, "width": "100px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "planned_qty", - "fieldtype": "Float", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Planned Qty", - "length": 0, - "no_copy": 0, - "oldfieldname": "planned_qty", - "oldfieldtype": "Currency", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "100px", - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "fieldname": "planned_qty", + "fieldtype": "Float", + "in_list_view": 1, + "label": "Planned Qty", + "oldfieldname": "planned_qty", + "oldfieldtype": "Currency", + "print_width": "100px", + "reqd": 1, "width": "100px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "column_break_6", - "fieldtype": "Column Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "column_break_6", + "fieldtype": "Column Break" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "depends_on": "", - "description": "If enabled, system will create the work order for the exploded items against which BOM is available.", - "fetch_if_empty": 0, - "fieldname": "make_work_order_for_sub_assembly_items", - "fieldtype": "Check", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Make Work Order for Sub Assembly Items", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "default": "0", + "description": "If enabled, system will create the work order for the exploded items against which BOM is available.", + "fieldname": "make_work_order_for_sub_assembly_items", + "fieldtype": "Check", + "label": "Make Work Order for Sub Assembly Items" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "description": "", - "fetch_if_empty": 0, - "fieldname": "warehouse", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "For Warehouse", - "length": 0, - "no_copy": 0, - "options": "Warehouse", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "warehouse", + "fieldtype": "Link", + "in_list_view": 1, + "label": "For Warehouse", + "options": "Warehouse" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "default": "Today", - "fetch_if_empty": 0, - "fieldname": "planned_start_date", - "fieldtype": "Datetime", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Planned Start Date", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "default": "Today", + "fieldname": "planned_start_date", + "fieldtype": "Datetime", + "in_list_view": 1, + "label": "Planned Start Date", + "reqd": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "section_break_9", - "fieldtype": "Section Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Quantity and Description", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "section_break_9", + "fieldtype": "Section Break", + "label": "Quantity and Description" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "default": "0", - "fetch_if_empty": 0, - "fieldname": "pending_qty", - "fieldtype": "Float", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Pending Qty", - "length": 0, - "no_copy": 0, - "oldfieldname": "prevdoc_reqd_qty", - "oldfieldtype": "Currency", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "100px", - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "default": "0", + "fieldname": "pending_qty", + "fieldtype": "Float", + "label": "Pending Qty", + "oldfieldname": "prevdoc_reqd_qty", + "oldfieldtype": "Currency", + "print_width": "100px", + "read_only": 1, "width": "100px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "default": "0", - "fetch_if_empty": 0, - "fieldname": "ordered_qty", - "fieldtype": "Float", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Ordered Qty", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 1, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "default": "0", + "fieldname": "ordered_qty", + "fieldtype": "Float", + "label": "Ordered Qty", + "print_hide": 1, + "read_only": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "default": "0", - "fetch_if_empty": 0, - "fieldname": "produced_qty", - "fieldtype": "Float", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Produced Qty", - "length": 0, - "no_copy": 1, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "default": "0", + "fieldname": "produced_qty", + "fieldtype": "Float", + "label": "Produced Qty", + "no_copy": 1, + "read_only": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "column_break_17", - "fieldtype": "Column Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "column_break_17", + "fieldtype": "Column Break" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "description", - "fieldtype": "Text Editor", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Description", - "length": 0, - "no_copy": 0, - "oldfieldname": "description", - "oldfieldtype": "Text", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "200px", - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "fieldname": "description", + "fieldtype": "Text Editor", + "label": "Description", + "oldfieldname": "description", + "oldfieldtype": "Text", + "print_width": "200px", + "read_only": 1, "width": "200px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "stock_uom", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "UOM", - "length": 0, - "no_copy": 0, - "oldfieldname": "stock_uom", - "oldfieldtype": "Data", - "options": "UOM", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "print_width": "80px", - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0, + "fieldname": "stock_uom", + "fieldtype": "Link", + "label": "UOM", + "oldfieldname": "stock_uom", + "oldfieldtype": "Data", + "options": "UOM", + "print_width": "80px", + "read_only": 1, + "reqd": 1, "width": "80px" - }, + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "reference_section", - "fieldtype": "Section Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Reference", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "reference_section", + "fieldtype": "Section Break", + "label": "Reference" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "sales_order", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Sales Order", - "length": 0, - "no_copy": 0, - "oldfieldname": "source_docname", - "oldfieldtype": "Data", - "options": "Sales Order", - "permlevel": 0, - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "sales_order", + "fieldtype": "Link", + "label": "Sales Order", + "oldfieldname": "source_docname", + "oldfieldtype": "Data", + "options": "Sales Order", + "read_only": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "sales_order_item", - "fieldtype": "Data", - "hidden": 1, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Sales Order Item", - "length": 0, - "no_copy": 1, - "permlevel": 0, - "precision": "", - "print_hide": 1, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "sales_order_item", + "fieldtype": "Data", + "hidden": 1, + "label": "Sales Order Item", + "no_copy": 1, + "print_hide": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "column_break_19", - "fieldtype": "Column Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "column_break_19", + "fieldtype": "Column Break" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "material_request", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Material Request", - "length": 0, - "no_copy": 0, - "options": "Material Request", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "material_request", + "fieldtype": "Link", + "label": "Material Request", + "options": "Material Request", + "read_only": 1 + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "material_request_item", - "fieldtype": "Data", - "hidden": 1, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "material_request_item", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 - }, + "fieldname": "material_request_item", + "fieldtype": "Data", + "hidden": 1, + "label": "material_request_item" + }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fetch_if_empty": 0, - "fieldname": "product_bundle_item", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "label": "Product Bundle Item", - "length": 0, - "no_copy": 1, - "options": "Item", - "permlevel": 0, - "precision": "", - "print_hide": 1, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "fieldname": "product_bundle_item", + "fieldtype": "Link", + "label": "Product Bundle Item", + "no_copy": 1, + "options": "Item", + "print_hide": 1, + "read_only": 1 } - ], - "has_web_view": 0, - "hide_toolbar": 0, - "idx": 1, - "in_create": 0, - "is_submittable": 0, - "issingle": 0, - "istable": 1, - "max_attachments": 0, - "modified": "2019-04-08 23:09:57.199423", - "modified_by": "Administrator", - "module": "Manufacturing", - "name": "Production Plan Item", - "owner": "Administrator", - "permissions": [], - "quick_entry": 0, - "read_only": 0, - "show_name_in_global_search": 0, - "sort_order": "ASC", - "track_changes": 0, - "track_seen": 0, - "track_views": 0 + ], + "idx": 1, + "istable": 1, + "links": [], + "modified": "2021-04-22 12:10:01.102440", + "modified_by": "Administrator", + "module": "Manufacturing", + "name": "Production Plan Item", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "ASC" } \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/production_plan_item_reference/__init__.py b/erpnext/manufacturing/doctype/production_plan_item_reference/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json new file mode 100644 index 0000000000..19e813c5b3 --- /dev/null +++ b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json @@ -0,0 +1,52 @@ +{ + "actions": [], + "creation": "2021-04-22 10:32:58.896330", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "item_ref", + "sales_order", + "sales_order_item", + "qty" + ], + "fields": [ + { + "fieldname": "item_ref", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Item Reference" + }, + { + "fieldname": "sales_order", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Sales Order Reference", + "options": "Sales Order" + }, + { + "fieldname": "sales_order_item", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Sales Order Item" + }, + { + "fieldname": "qty", + "fieldtype": "Data", + "in_list_view": 1, + "label": "qty" + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2021-04-23 16:55:22.161418", + "modified_by": "Administrator", + "module": "Manufacturing", + "name": "Production Plan Item Reference", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "DESC", + "track_changes": 1 +} \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.py b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.py new file mode 100644 index 0000000000..51fbc3633b --- /dev/null +++ b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.py @@ -0,0 +1,10 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +from __future__ import unicode_literals +# import frappe +from frappe.model.document import Document + +class ProductionPlanItemReference(Document): + pass diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 8507f5eb34..bd286507ec 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -241,7 +241,13 @@ class WorkOrder(Document): if not self.fg_warehouse: frappe.throw(_("For Warehouse is required before Submit")) - self.update_work_order_qty_in_so() + prod_plan = frappe.get_doc('Production Plan', self.production_plan) + pp_ref = prod_plan.prod_plan_ref + if pp_ref: + self.update_work_order_qty_in_combined_so() + else: + self.update_work_order_qty_in_so() + self.update_reserved_qty_for_production() self.update_completed_qty_in_material_request() self.update_planned_qty() @@ -357,6 +363,25 @@ class WorkOrder(Document): work_order_qty = qty[0][0] if qty and qty[0][0] else 0 frappe.db.set_value('Sales Order Item', self.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) + + def update_work_order_qty_in_combined_so(self): + total_bundle_qty = 1 + if self.product_bundle_item: + total_bundle_qty = frappe.db.sql(""" select sum(qty) from + `tabProduct Bundle Item` where parent = %s""", (frappe.db.escape(self.product_bundle_item)))[0][0] + + if not total_bundle_qty: + # product bundle is 0 (product bundle allows 0 qty for items) + total_bundle_qty = 1 + + prod_plan = frappe.get_doc('Production Plan', self.production_plan) + pp_ref = prod_plan.prod_plan_ref + for p in pp_ref: + if p.item_ref == self.production_plan_item: + work_order_qty = int(p.qty) + frappe.db.set_value('Sales Order Item', + p.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) + def update_completed_qty_in_material_request(self): if self.material_request: From 82905166d988f162b6fa84a49ee63a0704bda661 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Wed, 28 Apr 2021 10:12:17 +0530 Subject: [PATCH 02/39] fix: Fixed updating sales order work qty after cancelling work order --- .../doctype/production_plan/production_plan.js | 1 - .../manufacturing/doctype/work_order/work_order.py | 12 +++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 39b8c94bc1..29c3d5b18e 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -224,7 +224,6 @@ frappe.ui.form.on('Production Plan', { }); }, combine_items: function (frm) { - frm.clear_table('po_items'); frm.clear_table('prod_plan_ref'); frappe.call({ diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index bd286507ec..87d57ad42c 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -258,7 +258,13 @@ class WorkOrder(Document): self.validate_cancel() frappe.db.set(self,'status', 'Cancelled') - self.update_work_order_qty_in_so() + prod_plan = frappe.get_doc('Production Plan', self.production_plan) + pp_ref = prod_plan.prod_plan_ref + if pp_ref: + self.update_work_order_qty_in_combined_so(cancel = True) + else: + self.update_work_order_qty_in_so() + self.delete_job_card() self.update_completed_qty_in_material_request() self.update_planned_qty() @@ -364,7 +370,7 @@ class WorkOrder(Document): frappe.db.set_value('Sales Order Item', self.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) - def update_work_order_qty_in_combined_so(self): + def update_work_order_qty_in_combined_so(self, cancel = None): total_bundle_qty = 1 if self.product_bundle_item: total_bundle_qty = frappe.db.sql(""" select sum(qty) from @@ -378,7 +384,7 @@ class WorkOrder(Document): pp_ref = prod_plan.prod_plan_ref for p in pp_ref: if p.item_ref == self.production_plan_item: - work_order_qty = int(p.qty) + work_order_qty = int(p.qty) if not cancel else 0 frappe.db.set_value('Sales Order Item', p.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) From b7ca9139042784530157d104126f3569bcd1d1d8 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Wed, 28 Apr 2021 19:21:36 +0530 Subject: [PATCH 03/39] fix: Added Item Reference field to link tables and update work_order_qty --- .../doctype/production_plan/production_plan.py | 1 + .../production_plan_item/production_plan_item.json | 11 +++++++++-- .../manufacturing/doctype/work_order/work_order.py | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index b2bc21fb6d..088089f87f 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -201,6 +201,7 @@ class ProductionPlan(Document): pi.sales_order = data.parent pi.sales_order_item = data.name pi.description = data.description + pi.item_reference = data.name elif self.get_items_from == "Material Request": diff --git a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json index 9ff1717e70..89ab7aa0a0 100644 --- a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json +++ b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json @@ -27,7 +27,8 @@ "column_break_19", "material_request", "material_request_item", - "product_bundle_item" + "product_bundle_item", + "item_reference" ], "fields": [ { @@ -206,12 +207,18 @@ "options": "Item", "print_hide": 1, "read_only": 1 + }, + { + "fieldname": "item_reference", + "fieldtype": "Data", + "hidden": 1, + "label": "Item Reference" } ], "idx": 1, "istable": 1, "links": [], - "modified": "2021-04-22 12:10:01.102440", + "modified": "2021-04-28 19:14:57.772123", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan Item", diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 87d57ad42c..d77c46fb03 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -382,8 +382,10 @@ class WorkOrder(Document): prod_plan = frappe.get_doc('Production Plan', self.production_plan) pp_ref = prod_plan.prod_plan_ref + pp_item = frappe.get_doc('Production Plan Item', self.production_plan_item) + item_ref = pp_item.item_reference for p in pp_ref: - if p.item_ref == self.production_plan_item: + if p.item_ref == item_ref: work_order_qty = int(p.qty) if not cancel else 0 frappe.db.set_value('Sales Order Item', p.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) From 56f697052cc01f3beab8a12d5b9978e2bf86ebbe Mon Sep 17 00:00:00 2001 From: noahjacob Date: Wed, 28 Apr 2021 19:25:22 +0530 Subject: [PATCH 04/39] test: added test case for combining items --- .../production_plan/test_production_plan.py | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 27335aa204..2f52ad9021 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -100,7 +100,7 @@ class TestProductionPlan(unittest.TestCase): def test_production_plan_sales_orders(self): item = 'Test Production Item 1' - so = make_sales_order(item_code=item, qty=5) + so = make_sales_order(item_code=item, qty=1) sales_order = so.name sales_order_item = so.items[0].name @@ -124,8 +124,8 @@ class TestProductionPlan(unittest.TestCase): wo_doc = frappe.get_doc('Work Order', work_order) wo_doc.update({ - 'wip_warehouse': '_Test Warehouse 1 - _TC', - 'fg_warehouse': '_Test Warehouse - _TC' + 'wip_warehouse': 'Work In Progress - _TC', + 'fg_warehouse': 'Finished Goods - _TC' }) wo_doc.submit() @@ -145,6 +145,57 @@ class TestProductionPlan(unittest.TestCase): self.assertEqual(sales_orders, []) + def test_production_plan_combine_items(self): + item = 'Test Production Item 1' + so = make_sales_order(item_code=item, qty=1) + sales_order = so.name + sales_order_item = so.items[0].name + + pln = frappe.new_doc('Production Plan') + pln.company = so.company + pln.get_items_from = 'Sales Order' + pln.append('sales_orders', { + 'sales_order': so.name, + 'sales_order_date': so.transaction_date, + 'customer': so.customer, + 'grand_total': so.grand_total + }) + so = make_sales_order(item_code=item, qty=2) + pln.append('sales_orders', { + 'sales_order': so.name, + 'sales_order_date': so.transaction_date, + 'customer': so.customer, + 'grand_total': so.grand_total + }) + pln.combine_items = 1 + pln.get_so_items() + pln.save() + pp = frappe.get_doc('Production Plan',pln.name) + for d in pp.prod_plan_ref: + d.item_ref = pp.po_items[0].name + pln.submit() + + self.assertTrue(pln.po_items[0].planned_qty,3) + + + pln.make_work_order() + + work_order = frappe.db.get_value('Work Order', {'production_plan_item': pln.po_items[0].name, + 'production_plan': pln.name,}, 'name') + + wo_doc = frappe.get_doc('Work Order', work_order) + wo_doc.update({ + 'wip_warehouse': 'Work In Progress - _TC', + }) + + wo_doc.submit() + for d in pln.prod_plan_ref: + so_wo_qty = frappe.db.get_value('Sales Order Item', d.sales_order_item, 'work_order_qty') + self.assertTrue(so_wo_qty,d.qty) + + + + def test_pp_to_mr_customer_provided(self): #Material Request from Production Plan for Customer Provided create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0) From 93c22ebbb98324153b6644475478c62ee15f8ff8 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Thu, 29 Apr 2021 12:57:41 +0530 Subject: [PATCH 05/39] refactor: created separate function to update work_order on cancel --- .../doctype/work_order/work_order.py | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index d77c46fb03..d9956e5bca 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -242,8 +242,8 @@ class WorkOrder(Document): frappe.throw(_("For Warehouse is required before Submit")) prod_plan = frappe.get_doc('Production Plan', self.production_plan) - pp_ref = prod_plan.prod_plan_ref - if pp_ref: + + if prod_plan.prod_plan_ref: self.update_work_order_qty_in_combined_so() else: self.update_work_order_qty_in_so() @@ -259,9 +259,9 @@ class WorkOrder(Document): frappe.db.set(self,'status', 'Cancelled') prod_plan = frappe.get_doc('Production Plan', self.production_plan) - pp_ref = prod_plan.prod_plan_ref - if pp_ref: - self.update_work_order_qty_in_combined_so(cancel = True) + + if prod_plan.prod_plan_ref: + self.update_work_order_combined_on_cancel() else: self.update_work_order_qty_in_so() @@ -370,7 +370,7 @@ class WorkOrder(Document): frappe.db.set_value('Sales Order Item', self.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) - def update_work_order_qty_in_combined_so(self, cancel = None): + def update_work_order_qty_in_combined_so(self): total_bundle_qty = 1 if self.product_bundle_item: total_bundle_qty = frappe.db.sql(""" select sum(qty) from @@ -381,14 +381,22 @@ class WorkOrder(Document): total_bundle_qty = 1 prod_plan = frappe.get_doc('Production Plan', self.production_plan) - pp_ref = prod_plan.prod_plan_ref pp_item = frappe.get_doc('Production Plan Item', self.production_plan_item) - item_ref = pp_item.item_reference - for p in pp_ref: - if p.item_ref == item_ref: - work_order_qty = int(p.qty) if not cancel else 0 + + for p in prod_plan.prod_plan_ref: + if p.item_ref == pp_item.item_reference: + work_order_qty = int(p.qty) frappe.db.set_value('Sales Order Item', p.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) + + def update_work_order_combined_on_cancel(self): + prod_plan = frappe.get_doc('Production Plan', self.production_plan) + pp_item = frappe.get_doc('Production Plan Item', self.production_plan_item) + + for p in prod_plan.prod_plan_ref: + if p.item_ref == pp_item.item_reference: + frappe.db.set_value('Sales Order Item', + p.sales_order_item, 'work_order_qty', 0.0) def update_completed_qty_in_material_request(self): From 90c667205adce829d9e0f9c4d74c4dd9effaa065 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Thu, 29 Apr 2021 12:58:35 +0530 Subject: [PATCH 06/39] test: added on_cancel test --- .../production_plan/test_production_plan.py | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 2f52ad9021..19b06bc8dd 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -148,8 +148,6 @@ class TestProductionPlan(unittest.TestCase): def test_production_plan_combine_items(self): item = 'Test Production Item 1' so = make_sales_order(item_code=item, qty=1) - sales_order = so.name - sales_order_item = so.items[0].name pln = frappe.new_doc('Production Plan') pln.company = so.company @@ -169,17 +167,13 @@ class TestProductionPlan(unittest.TestCase): }) pln.combine_items = 1 pln.get_so_items() - pln.save() - pp = frappe.get_doc('Production Plan',pln.name) - for d in pp.prod_plan_ref: - d.item_ref = pp.po_items[0].name + for d in pln.prod_plan_ref: + d.item_ref = pln.po_items[0].name pln.submit() - self.assertTrue(pln.po_items[0].planned_qty,3) - - - pln.make_work_order() + self.assertTrue(pln.po_items[0].planned_qty,3) + pln.make_work_order() work_order = frappe.db.get_value('Work Order', {'production_plan_item': pln.po_items[0].name, 'production_plan': pln.name,}, 'name') @@ -189,13 +183,19 @@ class TestProductionPlan(unittest.TestCase): }) wo_doc.submit() + so_items = [] for d in pln.prod_plan_ref: + so_items.append(d.sales_order_item) so_wo_qty = frappe.db.get_value('Sales Order Item', d.sales_order_item, 'work_order_qty') - self.assertTrue(so_wo_qty,d.qty) - - - - + self.assertEqual(so_wo_qty, d.qty) + wo_doc.cancel() + for s in so_items: + so_wo_qty = frappe.db.get_value('Sales Order Item', s, 'work_order_qty') + self.assertEqual(so_wo_qty, 0.0) + + lat_plan = frappe.get_doc('Production Plan',pln.name) + lat_plan.cancel() + def test_pp_to_mr_customer_provided(self): #Material Request from Production Plan for Customer Provided create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0) From 50f52dfbcbf3124c1f9468807f1f22e6df2cb10c Mon Sep 17 00:00:00 2001 From: noahjacob Date: Fri, 7 May 2021 17:18:48 +0530 Subject: [PATCH 07/39] refactor: variable names and refactored cancel function into submit function --- .../production_plan/production_plan.json | 18 +++++------ .../production_plan/production_plan.py | 16 +++++----- .../production_plan/test_production_plan.py | 17 +++++----- .../production_plan_item_reference.json | 16 +++++----- .../doctype/work_order/work_order.py | 32 ++++++------------- 5 files changed, 44 insertions(+), 55 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index 5c73992d1b..3041507caf 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -31,7 +31,7 @@ "combine_items", "po_items", "section_break_25", - "prod_plan_ref", + "prod_plan_references", "material_request_planning", "include_non_stock_items", "include_subcontracted_items", @@ -320,13 +320,6 @@ "fieldtype": "Check", "label": "Include Safety Stock in Required Qty Calculation" }, - { - "fieldname": "prod_plan_ref", - "fieldtype": "Table", - "hidden": 1, - "label": "Production Plan Item Reference", - "options": "Production Plan Item Reference" - }, { "default": "0", "fieldname": "combine_items", @@ -336,13 +329,20 @@ { "fieldname": "section_break_25", "fieldtype": "Section Break" + }, + { + "fieldname": "prod_plan_references", + "fieldtype": "Table", + "hidden": 1, + "label": "Production Plan Item Reference", + "options": "Production Plan Item Reference" } ], "icon": "fa fa-calendar", "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-04-26 14:11:43.564957", + "modified": "2021-05-07 16:56:00.255001", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 088089f87f..8d578fd935 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -172,7 +172,7 @@ class ProductionPlan(Document): item_details = get_item_details(data.item_code) if self.combine_items: if item_details.bom_no in refs.keys(): - refs[item_details.bom_no]['qty'] = refs[item_details.bom_no]['qty'] + data.pending_qty + refs[item_details.bom_no]['qty'] += data.pending_qty refs[item_details.bom_no]['so'].append(data.parent) refs[item_details.bom_no]['so_items'].append(data.name) refs[item_details.bom_no]['planned_qty'].append(data.pending_qty) @@ -217,15 +217,15 @@ class ProductionPlan(Document): self.add_pp_ref(refs) def add_pp_ref(self, refs): - for r in refs: + for bom_no in refs: idx = 0 - for so in refs[r]['so']: - self.append('prod_plan_ref', { - 'item_ref': refs[r]['ref'], + for so in refs[bom_no]['so']: + self.append('prod_plan_references', { + 'item_reference': refs[bom_no]['ref'], 'sales_order': so, - 'sales_order_item':refs[r]['so_items'][idx], - 'qty':refs[r]['planned_qty'][idx] - }) + 'sales_order_item':refs[bom_no]['so_items'][idx], + 'qty':refs[bom_no]['planned_qty'][idx] + }) idx+=1 diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 19b06bc8dd..ec5c5e0e13 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -167,8 +167,8 @@ class TestProductionPlan(unittest.TestCase): }) pln.combine_items = 1 pln.get_so_items() - for d in pln.prod_plan_ref: - d.item_ref = pln.po_items[0].name + for plan_reference in pln.prod_plan_references: + plan_reference.item_reference = pln.po_items[0].name pln.submit() self.assertTrue(pln.po_items[0].planned_qty,3) @@ -184,13 +184,14 @@ class TestProductionPlan(unittest.TestCase): wo_doc.submit() so_items = [] - for d in pln.prod_plan_ref: - so_items.append(d.sales_order_item) - so_wo_qty = frappe.db.get_value('Sales Order Item', d.sales_order_item, 'work_order_qty') - self.assertEqual(so_wo_qty, d.qty) + for plan_reference in pln.prod_plan_references: + so_items.append(plan_reference.sales_order_item) + so_wo_qty = frappe.db.get_value('Sales Order Item', plan_reference.sales_order_item, 'work_order_qty') + self.assertEqual(so_wo_qty, plan_reference.qty) + wo_doc.cancel() - for s in so_items: - so_wo_qty = frappe.db.get_value('Sales Order Item', s, 'work_order_qty') + for so_item in so_items: + so_wo_qty = frappe.db.get_value('Sales Order Item', so_item, 'work_order_qty') self.assertEqual(so_wo_qty, 0.0) lat_plan = frappe.get_doc('Production Plan',pln.name) diff --git a/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json index 19e813c5b3..84dee4ad28 100644 --- a/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json +++ b/erpnext/manufacturing/doctype/production_plan_item_reference/production_plan_item_reference.json @@ -5,18 +5,12 @@ "editable_grid": 1, "engine": "InnoDB", "field_order": [ - "item_ref", + "item_reference", "sales_order", "sales_order_item", "qty" ], "fields": [ - { - "fieldname": "item_ref", - "fieldtype": "Data", - "in_list_view": 1, - "label": "Item Reference" - }, { "fieldname": "sales_order", "fieldtype": "Link", @@ -35,12 +29,18 @@ "fieldtype": "Data", "in_list_view": 1, "label": "qty" + }, + { + "fieldname": "item_reference", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Item Reference" } ], "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2021-04-23 16:55:22.161418", + "modified": "2021-05-07 17:03:49.707487", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan Item Reference", diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index d9956e5bca..bb6450b775 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -240,10 +240,8 @@ class WorkOrder(Document): frappe.throw(_("Work-in-Progress Warehouse is required before Submit")) if not self.fg_warehouse: frappe.throw(_("For Warehouse is required before Submit")) - - prod_plan = frappe.get_doc('Production Plan', self.production_plan) - if prod_plan.prod_plan_ref: + if self.production_plan and frappe.db.exists('Production Plan Item Reference',{'parent':self.production_plan}): self.update_work_order_qty_in_combined_so() else: self.update_work_order_qty_in_so() @@ -256,12 +254,10 @@ class WorkOrder(Document): def on_cancel(self): self.validate_cancel() - frappe.db.set(self,'status', 'Cancelled') - prod_plan = frappe.get_doc('Production Plan', self.production_plan) - if prod_plan.prod_plan_ref: - self.update_work_order_combined_on_cancel() + if self.production_plan and frappe.db.exists('Production Plan Item Reference',{'parent':self.production_plan}): + self.update_work_order_qty_in_combined_so() else: self.update_work_order_qty_in_so() @@ -381,24 +377,16 @@ class WorkOrder(Document): total_bundle_qty = 1 prod_plan = frappe.get_doc('Production Plan', self.production_plan) - pp_item = frappe.get_doc('Production Plan Item', self.production_plan_item) + item_reference = frappe.get_value('Production Plan Item', self.production_plan_item,'item_reference') - for p in prod_plan.prod_plan_ref: - if p.item_ref == pp_item.item_reference: - work_order_qty = int(p.qty) + for plan_reference in prod_plan.prod_plan_references: + work_order_qty = 0.0 + if plan_reference.item_reference == item_reference: + if self.docstatus == 1: + work_order_qty = cint(plan_reference.qty) / total_bundle_qty frappe.db.set_value('Sales Order Item', - p.sales_order_item, 'work_order_qty', flt(work_order_qty/total_bundle_qty, 2)) + plan_reference.sales_order_item, 'work_order_qty', work_order_qty) - def update_work_order_combined_on_cancel(self): - prod_plan = frappe.get_doc('Production Plan', self.production_plan) - pp_item = frappe.get_doc('Production Plan Item', self.production_plan_item) - - for p in prod_plan.prod_plan_ref: - if p.item_ref == pp_item.item_reference: - frappe.db.set_value('Sales Order Item', - p.sales_order_item, 'work_order_qty', 0.0) - - def update_completed_qty_in_material_request(self): if self.material_request: frappe.get_doc("Material Request", self.material_request).update_completed_qty([self.material_request_item]) From 516c789127d8514368fc35392368b197534d5e0b Mon Sep 17 00:00:00 2001 From: noahjacob Date: Wed, 19 May 2021 13:29:30 +0530 Subject: [PATCH 08/39] refactor: variable names and suggested changes --- .../production_plan/production_plan.js | 6 +-- .../production_plan/production_plan.py | 51 ++++++++++--------- .../production_plan/test_production_plan.py | 16 +++--- .../doctype/work_order/work_order.py | 2 +- 4 files changed, 38 insertions(+), 37 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 29c3d5b18e..64d584118f 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -212,7 +212,7 @@ frappe.ui.form.on('Production Plan', { }, get_items: function (frm) { - frm.clear_table('prod_plan_ref'); + frm.clear_table('prod_plan_references'); frappe.call({ method: "get_items", @@ -224,15 +224,13 @@ frappe.ui.form.on('Production Plan', { }); }, combine_items: function (frm) { - frm.clear_table('prod_plan_ref'); + frm.clear_table('prod_plan_references'); frappe.call({ method: "get_items", freeze: true, doc: frm.doc, }); - - }, get_items_for_mr: function(frm) { diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 8d578fd935..46e047654b 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -171,20 +171,28 @@ class ProductionPlan(Document): for data in items: item_details = get_item_details(data.item_code) if self.combine_items: - if item_details.bom_no in refs.keys(): + if item_details.bom_no in refs: + refs[item_details.bom_no]['so_details'].append({ + 'sales_order': data.parent, + 'sales_order_item': data.name, + 'qty': data.pending_qty + }) refs[item_details.bom_no]['qty'] += data.pending_qty - refs[item_details.bom_no]['so'].append(data.parent) - refs[item_details.bom_no]['so_items'].append(data.name) - refs[item_details.bom_no]['planned_qty'].append(data.pending_qty) continue + else: - refs[item_details.bom_no] = {'qty': data.pending_qty, 'ref': data.name} - refs[item_details.bom_no]['so'] = [data.parent] - refs[item_details.bom_no]['so_items'] = [data.name] - refs[item_details.bom_no]['planned_qty'] = [data.pending_qty] - + refs[item_details.bom_no] = { + 'qty': data.pending_qty, + 'po_item_ref': data.name, + 'so_details': [] + } + refs[item_details.bom_no]['so_details'].append({ + 'sales_order': data.parent, + 'sales_order_item': data.name, + 'qty': data.pending_qty + }) + pi = self.append('po_items', { - 'name': data.name, 'include_exploded_items': 1, 'warehouse': data.warehouse, 'item_code': data.item_code, @@ -201,8 +209,6 @@ class ProductionPlan(Document): pi.sales_order = data.parent pi.sales_order_item = data.name pi.description = data.description - pi.item_reference = data.name - elif self.get_items_from == "Material Request": pi.material_request = data.parent @@ -210,24 +216,21 @@ class ProductionPlan(Document): pi.description = data.description if refs: - for d in self.po_items: - d.planned_qty = refs[d.bom_no]['qty'] - d.pending_qty = refs[d.bom_no]['qty'] - d.sales_order = '' + for po_item in self.po_items: + po_item.planned_qty = refs[po_item.bom_no]['qty'] + po_item.pending_qty = refs[po_item.bom_no]['qty'] + po_item.sales_order = '' self.add_pp_ref(refs) def add_pp_ref(self, refs): for bom_no in refs: - idx = 0 - for so in refs[bom_no]['so']: + for so_detail in refs[bom_no]['so_details']: self.append('prod_plan_references', { - 'item_reference': refs[bom_no]['ref'], - 'sales_order': so, - 'sales_order_item':refs[bom_no]['so_items'][idx], - 'qty':refs[bom_no]['planned_qty'][idx] + 'item_reference': refs[bom_no]['po_item_ref'], + 'sales_order': so_detail['sales_order'], + 'sales_order_item': so_detail['sales_order_item'], + 'qty': so_detail['qty'] }) - idx+=1 - def calculate_total_planned_qty(self): self.total_planned_qty = 0 diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index ec5c5e0e13..768f99eb43 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -166,16 +166,16 @@ class TestProductionPlan(unittest.TestCase): 'grand_total': so.grand_total }) pln.combine_items = 1 - pln.get_so_items() - for plan_reference in pln.prod_plan_references: - plan_reference.item_reference = pln.po_items[0].name + pln.get_items() pln.submit() - self.assertTrue(pln.po_items[0].planned_qty,3) + self.assertTrue(pln.po_items[0].planned_qty, 3) pln.make_work_order() - work_order = frappe.db.get_value('Work Order', {'production_plan_item': pln.po_items[0].name, - 'production_plan': pln.name,}, 'name') + work_order = frappe.db.get_value('Work Order', { + 'production_plan_item': pln.po_items[0].name, + 'production_plan': pln.name + }, 'name') wo_doc = frappe.get_doc('Work Order', work_order) wo_doc.update({ @@ -194,8 +194,8 @@ class TestProductionPlan(unittest.TestCase): so_wo_qty = frappe.db.get_value('Sales Order Item', so_item, 'work_order_qty') self.assertEqual(so_wo_qty, 0.0) - lat_plan = frappe.get_doc('Production Plan',pln.name) - lat_plan.cancel() + latest_plan = frappe.get_doc('Production Plan', pln.name) + latest_plan.cancel() def test_pp_to_mr_customer_provided(self): #Material Request from Production Plan for Customer Provided diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index bb6450b775..a154464a8b 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -377,7 +377,7 @@ class WorkOrder(Document): total_bundle_qty = 1 prod_plan = frappe.get_doc('Production Plan', self.production_plan) - item_reference = frappe.get_value('Production Plan Item', self.production_plan_item,'item_reference') + item_reference = frappe.get_value('Production Plan Item', self.production_plan_item, 'sales_order_item') for plan_reference in prod_plan.prod_plan_references: work_order_qty = 0.0 From 330353a5ced3d40dc2e699a4481234de205b1fd7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 22:07:45 +0530 Subject: [PATCH 09/39] refactor: use frappe.throw instread of recreating _msgprint was basically duplicating behvaiour of frappe.throw --- erpnext/stock/doctype/item/item.py | 25 ++++++------------- .../stock_reconciliation.py | 6 ++--- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index dbac79465e..174c87b48d 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1068,42 +1068,31 @@ def get_timeline_data(doctype, name): return out -def validate_end_of_life(item_code, end_of_life=None, disabled=None, verbose=1): +def validate_end_of_life(item_code, end_of_life=None, disabled=None): if (not end_of_life) or (disabled is None): end_of_life, disabled = frappe.db.get_value("Item", item_code, ["end_of_life", "disabled"]) if end_of_life and end_of_life != "0000-00-00" and getdate(end_of_life) <= now_datetime().date(): - msg = _("Item {0} has reached its end of life on {1}").format(item_code, formatdate(end_of_life)) - _msgprint(msg, verbose) + frappe.throw(_("Item {0} has reached its end of life on {1}").format(item_code, formatdate(end_of_life))) if disabled: - _msgprint(_("Item {0} is disabled").format(item_code), verbose) + frappe.throw(_("Item {0} is disabled").format(item_code)) -def validate_is_stock_item(item_code, is_stock_item=None, verbose=1): +def validate_is_stock_item(item_code, is_stock_item=None): if not is_stock_item: is_stock_item = frappe.db.get_value("Item", item_code, "is_stock_item") if is_stock_item != 1: - msg = _("Item {0} is not a stock Item").format(item_code) - - _msgprint(msg, verbose) + frappe.throw(_("Item {0} is not a stock Item").format(item_code)) -def validate_cancelled_item(item_code, docstatus=None, verbose=1): +def validate_cancelled_item(item_code, docstatus=None): if docstatus is None: docstatus = frappe.db.get_value("Item", item_code, "docstatus") if docstatus == 2: - msg = _("Item {0} is cancelled").format(item_code) - _msgprint(msg, verbose) - -def _msgprint(msg, verbose): - if verbose: - msgprint(msg, raise_exception=True) - else: - raise frappe.ValidationError(msg) - + frappe.throw(_("Item {0} is cancelled").format(item_code)) def get_last_purchase_details(item_code, doc_name=None, conversion_rate=1.0): """returns last purchase details in stock uom""" diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 7e216d6181..96b1cadaaf 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -167,8 +167,8 @@ class StockReconciliation(StockController): item = frappe.get_doc("Item", item_code) # end of life and stock item - validate_end_of_life(item_code, item.end_of_life, item.disabled, verbose=0) - validate_is_stock_item(item_code, item.is_stock_item, verbose=0) + validate_end_of_life(item_code, item.end_of_life, item.disabled) + validate_is_stock_item(item_code, item.is_stock_item) # item should not be serialized if item.has_serial_no and not row.serial_no and not item.serial_no_series: @@ -179,7 +179,7 @@ class StockReconciliation(StockController): raise frappe.ValidationError(_("Batch no is required for batched item {0}").format(item_code)) # docstatus should be < 2 - validate_cancelled_item(item_code, item.docstatus, verbose=0) + validate_cancelled_item(item_code, item.docstatus) except Exception as e: self.validation_messages.append(_("Row # ") + ("%d: " % (row.idx)) + cstr(e)) From 4a2dbd4885777e435282df7afe8631f157f7a0a8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 22:11:30 +0530 Subject: [PATCH 10/39] refactor: cleanup get_timeline_data, remove py2 --- erpnext/stock/doctype/item/item.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 174c87b48d..61d7e56d13 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -12,7 +12,7 @@ from erpnext.controllers.item_variant import (ItemVariantExistsError, copy_attributes_to_variant, get_variant, make_variant_item_code, validate_item_variant_attributes) from erpnext.setup.doctype.item_group.item_group import (get_parent_item_groups, invalidate_cache_for) from frappe import _, msgprint -from frappe.utils import (cint, cstr, flt, formatdate, get_timestamp, getdate, +from frappe.utils import (cint, cstr, flt, formatdate, getdate, now_datetime, random_string, strip, get_link_to_form, nowtime) from frappe.utils.html_utils import clean_html from frappe.website.doctype.website_slideshow.website_slideshow import \ @@ -21,8 +21,6 @@ from frappe.website.doctype.website_slideshow.website_slideshow import \ from frappe.website.render import clear_cache from frappe.website.website_generator import WebsiteGenerator -from six import iteritems - class DuplicateReorderRows(frappe.ValidationError): pass @@ -1054,18 +1052,15 @@ def make_item_price(item, price_list_name, item_price): }).insert() def get_timeline_data(doctype, name): - '''returns timeline data based on stock ledger entry''' - out = {} - items = dict(frappe.db.sql('''select posting_date, count(*) - from `tabStock Ledger Entry` where item_code=%s - and posting_date > date_sub(curdate(), interval 1 year) - group by posting_date''', name)) + """get timeline data based on Stock Ledger Entry. This is displayed as heatmap on the item page.""" - for date, count in iteritems(items): - timestamp = get_timestamp(date) - out.update({timestamp: count}) + items = frappe.db.sql("""select unix_timestamp(posting_date), count(*) + from `tabStock Ledger Entry` + where item_code=%s and posting_date > date_sub(curdate(), interval 1 year) + group by posting_date""", name) + + return dict(items) - return out def validate_end_of_life(item_code, end_of_life=None, disabled=None): From ad58a8164aeabfe0c87c54052ec5ba3db4c1ca56 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 22:58:22 +0530 Subject: [PATCH 11/39] refactor: code cleanup minor fixes for improving code quality --- erpnext/stock/doctype/item/item.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 61d7e56d13..0b92e27152 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -201,7 +201,7 @@ class Item(WebsiteGenerator): def make_route(self): if not self.route: return cstr(frappe.db.get_value('Item Group', self.item_group, - 'route')) + '/' + self.scrub((self.item_name if self.item_name else self.item_code) + '-' + random_string(5)) + 'route')) + '/' + self.scrub((self.item_name or self.item_code) + '-' + random_string(5)) def validate_website_image(self): if frappe.flags.in_import: @@ -256,7 +256,6 @@ class Item(WebsiteGenerator): "attached_to_name": self.name }) except frappe.DoesNotExistError: - pass # cleanup frappe.local.message_log.pop() From 0b4858d8e5d84723d82544721784d60e8541e3c2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 22:59:52 +0530 Subject: [PATCH 12/39] refactor: eliminate unnecessary loop, container casts --- erpnext/stock/doctype/item/item.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 0b92e27152..c41dd67727 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -539,10 +539,7 @@ class Item(WebsiteGenerator): def fill_customer_code(self): """ Append all the customer codes and insert into "customer_code" field of item table """ - cust_code = [] - for d in self.get('customer_items'): - cust_code.append(d.ref_code) - self.customer_code = ','.join(cust_code) + self.customer_code = ','.join(d.ref_code for d in self.get("customer_items", [])) def check_item_tax(self): """Check whether Tax Rate is not entered twice for same Tax Type""" @@ -755,7 +752,7 @@ class Item(WebsiteGenerator): template_item.save() def validate_item_defaults(self): - companies = list(set([row.company for row in self.item_defaults])) + companies = {row.company for row in self.item_defaults} if len(companies) != len(self.item_defaults): frappe.throw(_("Cannot set multiple Item Defaults for a company.")) From 83e6e2e68aec4ef9b6095652a83d19e44bf90e31 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:01:50 +0530 Subject: [PATCH 13/39] refactor: add guard clause for readability --- erpnext/stock/doctype/item/item.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index c41dd67727..b665eb8e46 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -953,20 +953,22 @@ class Item(WebsiteGenerator): d.variant_of = self.variant_of def cant_change(self): - if not self.get("__islocal"): - fields = ("has_serial_no", "is_stock_item", "valuation_method", "has_batch_no") + if self.get("__islocal"): + return - values = frappe.db.get_value("Item", self.name, fields, as_dict=True) - if not values.get('valuation_method') and self.get('valuation_method'): - values['valuation_method'] = frappe.db.get_single_value("Stock Settings", "valuation_method") or "FIFO" + fields = ("has_serial_no", "is_stock_item", "valuation_method", "has_batch_no") - if values: - for field in fields: - if cstr(self.get(field)) != cstr(values.get(field)): - if not self.check_if_linked_document_exists(field): - break # no linked document, allowed - else: - frappe.throw(_("As there are existing transactions against item {0}, you can not change the value of {1}").format(self.name, frappe.bold(self.meta.get_label(field)))) + values = frappe.db.get_value("Item", self.name, fields, as_dict=True) + if not values.get('valuation_method') and self.get('valuation_method'): + values['valuation_method'] = frappe.db.get_single_value("Stock Settings", "valuation_method") or "FIFO" + + if values: + for field in fields: + if cstr(self.get(field)) != cstr(values.get(field)): + if not self.check_if_linked_document_exists(field): + break # no linked document, allowed + else: + frappe.throw(_("As there are existing transactions against item {0}, you can not change the value of {1}").format(self.name, frappe.bold(self.meta.get_label(field)))) def check_if_linked_document_exists(self, field): linked_doctypes = ["Delivery Note Item", "Sales Invoice Item", "POS Invoice Item", "Purchase Receipt Item", From 931c886f92c34453f87b54e315b8f3614a10df48 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:06:16 +0530 Subject: [PATCH 14/39] fix: not checking all fields `break` will break out of the loop without checking remaining fields. --- erpnext/stock/doctype/item/item.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index b665eb8e46..d09a4aa0dc 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -965,9 +965,7 @@ class Item(WebsiteGenerator): if values: for field in fields: if cstr(self.get(field)) != cstr(values.get(field)): - if not self.check_if_linked_document_exists(field): - break # no linked document, allowed - else: + if self.check_if_linked_document_exists(field): frappe.throw(_("As there are existing transactions against item {0}, you can not change the value of {1}").format(self.name, frappe.bold(self.meta.get_label(field)))) def check_if_linked_document_exists(self, field): From 4b484d741d81834ad9749e9395b2510397b7ae09 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:10:54 +0530 Subject: [PATCH 15/39] refactor: use is_new() instead of __islocal Interface over implementation. --- erpnext/stock/doctype/item/item.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index d09a4aa0dc..7906923e6f 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -127,7 +127,7 @@ class Item(WebsiteGenerator): self.cant_change() self.update_show_in_website() - if not self.get("__islocal"): + if not self.is_new(): self.old_item_group = frappe.db.get_value(self.doctype, self.name, "item_group") self.old_website_item_groups = frappe.db.sql_list("""select item_group from `tabWebsite Item Group` @@ -807,7 +807,7 @@ class Item(WebsiteGenerator): frappe.throw(_("Item has variants.")) def validate_attributes_in_variants(self): - if not self.has_variants or self.get("__islocal"): + if not self.has_variants or self.is_new(): return old_doc = self.get_doc_before_save() @@ -895,7 +895,7 @@ class Item(WebsiteGenerator): frappe.throw(_("Variant Based On cannot be changed")) def validate_uom(self): - if not self.get("__islocal"): + if not self.is_new(): check_stock_uom_with_bin(self.name, self.stock_uom) if self.has_variants: for d in frappe.db.get_all("Item", filters={"variant_of": self.name}): @@ -953,7 +953,7 @@ class Item(WebsiteGenerator): d.variant_of = self.variant_of def cant_change(self): - if self.get("__islocal"): + if self.is_new(): return fields = ("has_serial_no", "is_stock_item", "valuation_method", "has_batch_no") From c229ac932288189366ec6dd57f74db6f34248b1f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:15:32 +0530 Subject: [PATCH 16/39] refactor: add guard clause for readability Both functions only execute based on a condition. In such cases condition should immediately exit the function, this is called "guard clause" and helps in readability (less indent, and easy to "exit" when reading the code. --- erpnext/stock/doctype/item/item.py | 94 ++++++++++++++++-------------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 7906923e6f..f7856be4ae 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -359,47 +359,49 @@ class Item(WebsiteGenerator): context.update(get_slideshow(self)) def set_attribute_context(self, context): - if self.has_variants: - attribute_values_available = {} - context.attribute_values = {} - context.selected_attributes = {} + if not self.has_variants: + return - # load attributes - for v in context.variants: - v.attributes = frappe.get_all("Item Variant Attribute", - fields=["attribute", "attribute_value"], - filters={"parent": v.name}) - # make a map for easier access in templates - v.attribute_map = frappe._dict({}) - for attr in v.attributes: - v.attribute_map[attr.attribute] = attr.attribute_value + attribute_values_available = {} + context.attribute_values = {} + context.selected_attributes = {} - for attr in v.attributes: - values = attribute_values_available.setdefault(attr.attribute, []) - if attr.attribute_value not in values: - values.append(attr.attribute_value) + # load attributes + for v in context.variants: + v.attributes = frappe.get_all("Item Variant Attribute", + fields=["attribute", "attribute_value"], + filters={"parent": v.name}) + # make a map for easier access in templates + v.attribute_map = frappe._dict({}) + for attr in v.attributes: + v.attribute_map[attr.attribute] = attr.attribute_value - if v.name == context.variant.name: - context.selected_attributes[attr.attribute] = attr.attribute_value + for attr in v.attributes: + values = attribute_values_available.setdefault(attr.attribute, []) + if attr.attribute_value not in values: + values.append(attr.attribute_value) - # filter attributes, order based on attribute table - for attr in self.attributes: - values = context.attribute_values.setdefault(attr.attribute, []) + if v.name == context.variant.name: + context.selected_attributes[attr.attribute] = attr.attribute_value - if cint(frappe.db.get_value("Item Attribute", attr.attribute, "numeric_values")): - for val in sorted(attribute_values_available.get(attr.attribute, []), key=flt): - values.append(val) + # filter attributes, order based on attribute table + for attr in self.attributes: + values = context.attribute_values.setdefault(attr.attribute, []) - else: - # get list of values defined (for sequence) - for attr_value in frappe.db.get_all("Item Attribute Value", - fields=["attribute_value"], - filters={"parent": attr.attribute}, order_by="idx asc"): + if cint(frappe.db.get_value("Item Attribute", attr.attribute, "numeric_values")): + for val in sorted(attribute_values_available.get(attr.attribute, []), key=flt): + values.append(val) - if attr_value.attribute_value in attribute_values_available.get(attr.attribute, []): - values.append(attr_value.attribute_value) + else: + # get list of values defined (for sequence) + for attr_value in frappe.db.get_all("Item Attribute Value", + fields=["attribute_value"], + filters={"parent": attr.attribute}, order_by="idx asc"): - context.variant_info = json.dumps(context.variants) + if attr_value.attribute_value in attribute_values_available.get(attr.attribute, []): + values.append(attr_value.attribute_value) + + context.variant_info = json.dumps(context.variants) def set_disabled_attributes(self, context): """Disable selection options of attribute combinations that do not result in a variant""" @@ -736,20 +738,22 @@ class Item(WebsiteGenerator): def update_template_item(self): """Set Show in Website for Template Item if True for its Variant""" - if self.variant_of: - if self.show_in_website: - self.show_variant_in_website = 1 - self.show_in_website = 0 + if not self.variant_of: + return - if self.show_variant_in_website: - # show template - template_item = frappe.get_doc("Item", self.variant_of) + if self.show_in_website: + self.show_variant_in_website = 1 + self.show_in_website = 0 - if not template_item.show_in_website: - template_item.show_in_website = 1 - template_item.flags.dont_update_variants = True - template_item.flags.ignore_permissions = True - template_item.save() + if self.show_variant_in_website: + # show template + template_item = frappe.get_doc("Item", self.variant_of) + + if not template_item.show_in_website: + template_item.show_in_website = 1 + template_item.flags.dont_update_variants = True + template_item.flags.ignore_permissions = True + template_item.save() def validate_item_defaults(self): companies = {row.company for row in self.item_defaults} From bf1b3b89d1cb07481006a94d78112c110be74f70 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Mon, 24 May 2021 17:03:15 +0530 Subject: [PATCH 17/39] refactor: updated conditional visibility of check box --- .../manufacturing/doctype/production_plan/production_plan.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index 3041507caf..1c0dde227c 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -322,6 +322,7 @@ }, { "default": "0", + "depends_on": "eval:doc.get_items_from == 'Sales Order'", "fieldname": "combine_items", "fieldtype": "Check", "label": "Consolidate Items" @@ -342,7 +343,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-05-07 16:56:00.255001", + "modified": "2021-05-24 16:59:03.643211", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", From 44c489223b855f837a279c65c989f30500ea70e8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:43:28 +0530 Subject: [PATCH 18/39] chore: remove py2 compat code --- erpnext/stock/doctype/item/item.py | 4 +--- erpnext/stock/doctype/item/test_item.py | 9 ++++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index f7856be4ae..a6f5160b5c 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1,8 +1,6 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt -from __future__ import unicode_literals - import itertools import json import erpnext diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index e0b89d8e45..c300132ad0 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -14,7 +14,6 @@ from erpnext.stock.doctype.item.item import get_uom_conv_factor from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details -from six import iteritems test_ignore = ["BOM"] test_dependencies = ["Warehouse", "Item Group", "Item Tax Template", "Brand"] @@ -98,7 +97,7 @@ class TestItem(unittest.TestCase): "ignore_pricing_rule": 1 }) - for key, value in iteritems(to_check): + for key, value in to_check.items(): self.assertEqual(value, details.get(key)) def test_item_tax_template(self): @@ -194,7 +193,7 @@ class TestItem(unittest.TestCase): "plc_conversion_rate": 1, "customer": "_Test Customer", }) - for key, value in iteritems(sales_item_check): + for key, value in sales_item_check.items(): self.assertEqual(value, sales_item_details.get(key)) purchase_item_check = { @@ -215,7 +214,7 @@ class TestItem(unittest.TestCase): "plc_conversion_rate": 1, "supplier": "_Test Supplier", }) - for key, value in iteritems(purchase_item_check): + for key, value in purchase_item_check.items(): self.assertEqual(value, purchase_item_details.get(key)) def test_item_attribute_change_after_variant(self): @@ -464,7 +463,7 @@ class TestItem(unittest.TestCase): self.assertEqual(len(matching_barcodes), 1) details = matching_barcodes[0] - for key, value in iteritems(barcode_properties): + for key, value in barcode_properties.items(): self.assertEqual(value, details.get(key)) # Add barcode again - should cause DuplicateEntryError From ccbde0efa07306710676d144fb7faf29635639db Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:52:00 +0530 Subject: [PATCH 19/39] refactor: use enumerate instead of trackign index also removed dead code --- erpnext/stock/doctype/item/item.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index a6f5160b5c..9a52fb4fc1 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -72,8 +72,6 @@ class Item(WebsiteGenerator): if not self.description: self.description = self.item_name - # if self.is_sales_item and not self.get('is_item_from_hub'): - # self.publish_in_hub = 1 def after_insert(self): '''set opening stock and item price''' @@ -1277,14 +1275,13 @@ def get_item_attribute(parent, attribute_value=''): filters = {'parent': parent, 'attribute_value': ("like", "%%%s%%" % attribute_value)}) def update_variants(variants, template, publish_progress=True): - count=0 - for d in variants: + total = len(variants) + for count, d in enumerate(variants, start=1): variant = frappe.get_doc("Item", d) copy_attributes_to_variant(template, variant) variant.save() - count+=1 if publish_progress: - frappe.publish_progress(count*100/len(variants), title = _("Updating Variants...")) + frappe.publish_progress(count / total * 100, title=_("Updating Variants...")) def on_doctype_update(): # since route is a Text column, it needs a length for indexing From 958d485ba49d47af46324395519d9683cbde4674 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 22 May 2021 23:58:38 +0530 Subject: [PATCH 20/39] refactor: msgprint(raise_exception)->frappe.throw --- erpnext/stock/doctype/item/item.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 9a52fb4fc1..e865bda5c1 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -516,7 +516,7 @@ class Item(WebsiteGenerator): def validate_item_type(self): if self.has_serial_no == 1 and self.is_stock_item == 0 and not self.is_fixed_asset: - msgprint(_("'Has Serial No' can not be 'Yes' for non-stock item"), raise_exception=1) + frappe.throw(_("'Has Serial No' can not be 'Yes' for non-stock item")) if self.has_serial_no == 0 and self.serial_no_series: self.serial_no_series = None @@ -1269,7 +1269,7 @@ def get_uom_conv_factor(uom, stock_uom): @frappe.whitelist() def get_item_attribute(parent, attribute_value=''): if not frappe.has_permission("Item"): - frappe.msgprint(_("No Permission"), raise_exception=1) + frappe.throw(_("No Permission")) return frappe.get_all("Item Attribute Value", fields = ["attribute_value"], filters = {'parent': parent, 'attribute_value': ("like", "%%%s%%" % attribute_value)}) From 297dc5e345b494c6f9cdba12653cdf45721a2af3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 00:17:08 +0530 Subject: [PATCH 21/39] perf: add basic optimisation for uom conversion --- erpnext/stock/doctype/item/item.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index e865bda5c1..2c862dcfb7 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1244,6 +1244,9 @@ def get_item_details(item_code, company=None): @frappe.whitelist() def get_uom_conv_factor(uom, stock_uom): + if uom == stock_uom: + return 1.0 + uoms = [uom, stock_uom] value = "" uom_details = frappe.db.sql("""select to_uom, from_uom, value from `tabUOM Conversion Factor`\ From 0d7f54c6c22578797f1e55eb4c29fbc452c591ce Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 00:30:27 +0530 Subject: [PATCH 22/39] refactor: simplify UOM conversion logic - Remove unnecessary sql query - Remove convoluted matching logic and be explcitiy while querying. - better variable names for understanding matching cases --- erpnext/stock/doctype/item/item.py | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 2c862dcfb7..ef855c7db5 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1247,27 +1247,22 @@ def get_uom_conv_factor(uom, stock_uom): if uom == stock_uom: return 1.0 - uoms = [uom, stock_uom] - value = "" - uom_details = frappe.db.sql("""select to_uom, from_uom, value from `tabUOM Conversion Factor`\ - where to_uom in ({0}) - """.format(', '.join([frappe.db.escape(i, percent=False) for i in uoms])), as_dict=True) + exact_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": stock_uom, "from_uom": uom}, ["value"], as_dict=1) + if exact_match: + return exact_match.value - for d in uom_details: - if d.from_uom == stock_uom and d.to_uom == uom: - value = 1/flt(d.value) - elif d.from_uom == uom and d.to_uom == stock_uom: - value = d.value + inverse_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": uom, "from_uom": stock_uom}, ["value"], as_dict=1) + if inverse_match: + return 1 / inverse_match.value - if not value: - uom_stock = frappe.db.get_value("UOM Conversion Factor", {"to_uom": stock_uom}, ["from_uom", "value"], as_dict=1) - uom_row = frappe.db.get_value("UOM Conversion Factor", {"to_uom": uom}, ["from_uom", "value"], as_dict=1) + # This attempts to try and get conversion from intermediate UOM. E.g. mg <=> g <=> kg + uom_stock = frappe.db.get_value("UOM Conversion Factor", {"to_uom": stock_uom}, ["from_uom", "value"], as_dict=1) + uom_row = frappe.db.get_value("UOM Conversion Factor", {"to_uom": uom}, ["from_uom", "value"], as_dict=1) - if uom_stock and uom_row: - if uom_stock.from_uom == uom_row.from_uom: - value = flt(uom_stock.value) * 1/flt(uom_row.value) + if uom_stock and uom_row: + if uom_stock.from_uom == uom_row.from_uom: + return flt(uom_stock.value) * 1/flt(uom_row.value) - return value @frappe.whitelist() def get_item_attribute(parent, attribute_value=''): From 019be66b7b1d137e686ca9b8189f638abdd5f47d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 01:12:47 +0530 Subject: [PATCH 23/39] fix: consider all UOMs for intermediate conversion - Using `get_value` will restrict intermediate UOM to first UOM that is found. - A self join is required to truly capture the required behaviour. - Add explanation and examples. --- erpnext/stock/doctype/item/item.py | 32 ++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index ef855c7db5..a5bc492422 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1244,24 +1244,40 @@ def get_item_details(item_code, company=None): @frappe.whitelist() def get_uom_conv_factor(uom, stock_uom): + """ Get UOM conversion factor from uom to stock_uom + e.g. uom = "Kg", stock_uom = "Gram" then returns 1000.0 + """ if uom == stock_uom: return 1.0 - exact_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": stock_uom, "from_uom": uom}, ["value"], as_dict=1) + from_uom, to_uom = uom, stock_uom # renaming for readability + + exact_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": to_uom, "from_uom": from_uom}, ["value"], as_dict=1) if exact_match: return exact_match.value - inverse_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": uom, "from_uom": stock_uom}, ["value"], as_dict=1) + inverse_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": from_uom, "from_uom": to_uom}, ["value"], as_dict=1) if inverse_match: return 1 / inverse_match.value - # This attempts to try and get conversion from intermediate UOM. E.g. mg <=> g <=> kg - uom_stock = frappe.db.get_value("UOM Conversion Factor", {"to_uom": stock_uom}, ["from_uom", "value"], as_dict=1) - uom_row = frappe.db.get_value("UOM Conversion Factor", {"to_uom": uom}, ["from_uom", "value"], as_dict=1) + # This attempts to try and get conversion from intermediate UOM. + # case: + # g -> mg = 1000 + # g -> kg = 0.001 + # therefore kg -> mg = 1000 / 0.001 = 1,000,000 + intermediate_match = frappe.db.sql(""" + select (first.value / second.value) as value + from `tabUOM Conversion Factor` first + join `tabUOM Conversion Factor` second + on first.from_uom = second.from_uom + where + first.to_uom = %(to_uom)s + and second.to_uom = %(from_uom)s + limit 1 + """, {"to_uom": to_uom, "from_uom": from_uom}, as_dict=1) - if uom_stock and uom_row: - if uom_stock.from_uom == uom_row.from_uom: - return flt(uom_stock.value) * 1/flt(uom_row.value) + if intermediate_match: + return intermediate_match[0].value @frappe.whitelist() From b9fa12d5721dddd00291fca1e9ef157527fb5905 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 12:26:01 +0530 Subject: [PATCH 24/39] test: add tests for uom conversion function --- erpnext/stock/doctype/item/test_item.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index c300132ad0..2366f06f6d 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -374,6 +374,14 @@ class TestItem(unittest.TestCase): self.assertEqual(item_doc.uoms[1].uom, "Kg") self.assertEqual(item_doc.uoms[1].conversion_factor, 1000) + def test_uom_conv_intermediate(self): + factor = get_uom_conv_factor("Pound", "Gram") + self.assertAlmostEqual(factor, 453.592, 3) + + def test_uom_conv_base_case(self): + factor = get_uom_conv_factor("m", "m") + self.assertEqual(factor, 1.0) + def test_item_variant_by_manufacturer(self): fields = [{'field_name': 'description'}, {'field_name': 'variant_based_on'}] set_item_variant_settings(fields) From f5a937bc45e2fe8dd4a18a3b804a86e3caa84cad Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 12:38:15 +0530 Subject: [PATCH 25/39] test: check index creation on item table --- erpnext/stock/doctype/item/test_item.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 2366f06f6d..d9d1e5a44d 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -487,6 +487,20 @@ class TestItem(unittest.TestCase): new_barcode.barcode_type = 'EAN' self.assertRaises(InvalidBarcode, item_doc.save) + def test_index_creation(self): + "check if index is getting created in db" + from erpnext.stock.doctype.item.item import on_doctype_update + on_doctype_update() + + indices = frappe.db.sql("show index from tabItem", as_dict=1) + expected_columns = {"item_code", "item_name", "item_group", "route"} + for index in indices: + expected_columns.discard(index.get("Column_name")) + + if expected_columns: + self.fail(f"Expected db index on these columns: {', '.join(expected_columns)}") + + def set_item_variant_settings(fields): doc = frappe.get_doc('Item Variant Settings') doc.set('fields', fields) From eb177328767c940857f46ca2345e972e9915eda2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 13:12:16 +0530 Subject: [PATCH 26/39] test: add test for item attribute completion --- erpnext/stock/doctype/item/item.py | 5 +++-- erpnext/stock/doctype/item/test_item.py | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index a5bc492422..ec46f60f2b 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1281,12 +1281,13 @@ def get_uom_conv_factor(uom, stock_uom): @frappe.whitelist() -def get_item_attribute(parent, attribute_value=''): +def get_item_attribute(parent, attribute_value=""): + """Used for providing auto-completions in child table.""" if not frappe.has_permission("Item"): frappe.throw(_("No Permission")) return frappe.get_all("Item Attribute Value", fields = ["attribute_value"], - filters = {'parent': parent, 'attribute_value': ("like", "%%%s%%" % attribute_value)}) + filters = {'parent': parent, 'attribute_value': ("like", f"%{attribute_value}%")}) def update_variants(variants, template, publish_progress=True): total = len(variants) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index d9d1e5a44d..7cd6050c02 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -10,13 +10,13 @@ from frappe.test_runner import make_test_objects from erpnext.controllers.item_variant import (create_variant, ItemVariantExistsError, InvalidItemAttributeValueError, get_variant) from erpnext.stock.doctype.item.item import StockExistsForTemplate, InvalidBarcode -from erpnext.stock.doctype.item.item import get_uom_conv_factor +from erpnext.stock.doctype.item.item import get_uom_conv_factor, get_item_attribute from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details test_ignore = ["BOM"] -test_dependencies = ["Warehouse", "Item Group", "Item Tax Template", "Brand"] +test_dependencies = ["Warehouse", "Item Group", "Item Tax Template", "Brand", "Item Attribute"] def make_item(item_code, properties=None): if frappe.db.exists("Item", item_code): @@ -500,6 +500,21 @@ class TestItem(unittest.TestCase): if expected_columns: self.fail(f"Expected db index on these columns: {', '.join(expected_columns)}") + def test_attribute_completions(self): + expected_attrs = [{'attribute_value': 'Small'}, + {'attribute_value': 'Extra Small'}, + {'attribute_value': 'Extra Large'}, + {'attribute_value': 'Large'}, + {'attribute_value': '2XL'}, + {'attribute_value': 'Medium'}] + + attrs = get_item_attribute("Test Size") + self.assertEqual(attrs, expected_attrs) + + attrs = get_item_attribute("Test Size", attribute_value="extra") + self.assertEqual(attrs, [{'attribute_value': 'Extra Small'}, {'attribute_value': 'Extra Large'}]) + + def set_item_variant_settings(fields): doc = frappe.get_doc('Item Variant Settings') From a11a8e8ab2e30eaa9f3b1cd80c27dc9ad8f13aeb Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 13:19:22 +0530 Subject: [PATCH 27/39] chore: add blame ignore file --- .git-blame-ignore-revs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 0000000000..be425ec2d9 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,12 @@ +# Since version 2.23 (released in August 2019), git-blame has a feature +# to ignore or bypass certain commits. +# +# This file contains a list of commits that are not likely what you +# are looking for in a blame, such as mass reformatting or renaming. +# You can set this file as a default ignore file for blame by running +# the following command. +# +# $ git config blame.ignoreRevsFile .git-blame-ignore-revs + +# This commit just changes spaces to tabs for indentation in some files +5f473611bd6ed57703716244a054d3fb5ba9cd23 From 4e360f805f5cb4f7ed500316aa97ca7e52b2f9bf Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 14:48:03 +0530 Subject: [PATCH 28/39] test: hoist defaults to function signature --- erpnext/stock/doctype/item/test_item.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 7cd6050c02..9694927914 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -530,23 +530,24 @@ def make_item_variant(): test_records = frappe.get_test_records('Item') -def create_item(item_code, is_stock_item=None, valuation_rate=0, warehouse=None, is_customer_provided_item=None, - customer=None, is_purchase_item=None, opening_stock=None, company=None): +def create_item(item_code, is_stock_item=1, valuation_rate=0, warehouse="_Test Warehouse - _TC", + is_customer_provided_item=None, customer=None, is_purchase_item=None, opening_stock=0, + company="_Test Company"): if not frappe.db.exists("Item", item_code): item = frappe.new_doc("Item") item.item_code = item_code item.item_name = item_code item.description = item_code item.item_group = "All Item Groups" - item.is_stock_item = is_stock_item or 1 - item.opening_stock = opening_stock or 0 - item.valuation_rate = valuation_rate or 0.0 + item.is_stock_item = is_stock_item + item.opening_stock = opening_stock + item.valuation_rate = valuation_rate item.is_purchase_item = is_purchase_item item.is_customer_provided_item = is_customer_provided_item item.customer = customer or '' item.append("item_defaults", { - "default_warehouse": warehouse or '_Test Warehouse - _TC', - "company": company or "_Test Company" + "default_warehouse": warehouse, + "company": company }) item.save() else: From fc54cf68ac33b213dd44821c10e01f84a6c4727a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 15:04:41 +0530 Subject: [PATCH 29/39] test: add tests for checking stock_uom with bin --- erpnext/stock/doctype/item/test_item.py | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 9694927914..8df12a3f16 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -514,6 +514,35 @@ class TestItem(unittest.TestCase): attrs = get_item_attribute("Test Size", attribute_value="extra") self.assertEqual(attrs, [{'attribute_value': 'Extra Small'}, {'attribute_value': 'Extra Large'}]) + def test_check_stock_uom_with_bin(self): + # this item has opening stock and stock_uom set in test_records. + item = frappe.get_doc("Item", "_Test Item") + item.stock_uom = "Gram" + self.assertRaises(frappe.ValidationError, item.save) + + def test_check_stock_uom_with_bin_no_sle(self): + from erpnext.stock.stock_balance import update_bin_qty + item = create_item("_Item with bin qty") + item.stock_uom = "Gram" + item.save() + + update_bin_qty(item.item_code, "_Test Warehouse - _TC", { + "reserved_qty": 10 + }) + + item.stock_uom = "Kilometer" + self.assertRaises(frappe.ValidationError, item.save) + + update_bin_qty(item.item_code, "_Test Warehouse - _TC", { + "reserved_qty": 0 + }) + + item.load_from_db() + item.stock_uom = "Kilometer" + try: + item.save() + except frappe.ValidationError as e: + self.fail(f"UoM change not allowed even though no SLE / BIN with positive qty exists: {e}") def set_item_variant_settings(fields): From 57266a7343edd1fb963d20db28593bed3f80ae50 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 14:21:24 +0530 Subject: [PATCH 30/39] refactor: check_stock_uom_with_bin --- erpnext/stock/doctype/item/item.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index ec46f60f2b..a0bd49543e 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -1183,27 +1183,25 @@ def check_stock_uom_with_bin(item, stock_uom): if stock_uom == frappe.db.get_value("Item", item, "stock_uom"): return - matched = True ref_uom = frappe.db.get_value("Stock Ledger Entry", {"item_code": item}, "stock_uom") if ref_uom: if cstr(ref_uom) != cstr(stock_uom): - matched = False - else: - bin_list = frappe.db.sql("select * from tabBin where item_code=%s", item, as_dict=1) - for bin in bin_list: - if (bin.reserved_qty > 0 or bin.ordered_qty > 0 or bin.indented_qty > 0 - or bin.planned_qty > 0) and cstr(bin.stock_uom) != cstr(stock_uom): - matched = False - break + frappe.throw(_("Default Unit of Measure for Item {0} cannot be changed directly because you have already made some transaction(s) with another UOM. You will need to create a new Item to use a different Default UOM.").format(item)) - if matched and bin_list: - frappe.db.sql("""update tabBin set stock_uom=%s where item_code=%s""", (stock_uom, item)) + bin_list = frappe.db.sql(""" + select * from tabBin where item_code = %s + and (reserved_qty > 0 or ordered_qty > 0 or indented_qty > 0 or planned_qty > 0) + and stock_uom != %s + """, (item, stock_uom), as_dict=1) + + if bin_list: + frappe.throw(_("Default Unit of Measure for Item {0} cannot be changed directly because you have already made some transaction(s) with another UOM. You need to either cancel the linked documents or create a new Item.").format(item)) + + # No SLE or documents against item. Bin UOM can be changed safely. + frappe.db.sql("""update tabBin set stock_uom=%s where item_code=%s""", (stock_uom, item)) - if not matched: - frappe.throw( - _("Default Unit of Measure for Item {0} cannot be changed directly because you have already made some transaction(s) with another UOM. You will need to create a new Item to use a different Default UOM.").format(item)) def get_item_defaults(item_code, company): item = frappe.get_cached_doc('Item', item_code) From e971b4592e3bb1894294f561a522f2b06336908b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 15:33:06 +0530 Subject: [PATCH 31/39] test: add test for is_stock_item --- erpnext/stock/doctype/item/test_item.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 8df12a3f16..d9c77efc24 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -10,7 +10,8 @@ from frappe.test_runner import make_test_objects from erpnext.controllers.item_variant import (create_variant, ItemVariantExistsError, InvalidItemAttributeValueError, get_variant) from erpnext.stock.doctype.item.item import StockExistsForTemplate, InvalidBarcode -from erpnext.stock.doctype.item.item import get_uom_conv_factor, get_item_attribute +from erpnext.stock.doctype.item.item import (get_uom_conv_factor, get_item_attribute, + validate_is_stock_item) from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details @@ -544,6 +545,13 @@ class TestItem(unittest.TestCase): except frappe.ValidationError as e: self.fail(f"UoM change not allowed even though no SLE / BIN with positive qty exists: {e}") + def test_validate_stock_item(self): + self.assertRaises(frappe.ValidationError, validate_is_stock_item, "_Test Non Stock Item") + + try: + validate_is_stock_item("_Test Item") + except frappe.ValidationError as e: + self.fail(f"stock item considered non-stock item: {e}") def set_item_variant_settings(fields): doc = frappe.get_doc('Item Variant Settings') From 42e057d079c1807393d376d347762e97100b6883 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 15:45:25 +0530 Subject: [PATCH 32/39] test: add test for get_timeline_data in item --- erpnext/stock/doctype/item/test_item.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index d9c77efc24..234a9132c2 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -11,7 +11,7 @@ from erpnext.controllers.item_variant import (create_variant, ItemVariantExistsE InvalidItemAttributeValueError, get_variant) from erpnext.stock.doctype.item.item import StockExistsForTemplate, InvalidBarcode from erpnext.stock.doctype.item.item import (get_uom_conv_factor, get_item_attribute, - validate_is_stock_item) + validate_is_stock_item, get_timeline_data) from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details @@ -488,6 +488,20 @@ class TestItem(unittest.TestCase): new_barcode.barcode_type = 'EAN' self.assertRaises(InvalidBarcode, item_doc.save) + def test_heatmap_data(self): + import time + data = get_timeline_data("Item", "_Test Item") + self.assertTrue(isinstance(data, dict)) + + now = time.time() + one_year_ago = now - 366 * 24 * 60 * 60 + + for timestamp, count in data.items(): + self.assertIsInstance(timestamp, int) + self.assertTrue(one_year_ago <= timestamp <= now) + self.assertIsInstance(count, int) + self.assertTrue(count >= 0) + def test_index_creation(self): "check if index is getting created in db" from erpnext.stock.doctype.item.item import on_doctype_update From 76dd6e904682a1bec1ff21d66c45e164fd26a47b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 16:19:48 +0530 Subject: [PATCH 33/39] test: contextmanager to change settings --- erpnext/tests/utils.py | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/erpnext/tests/utils.py b/erpnext/tests/utils.py index 16ecd5180b..11eb6afc1f 100644 --- a/erpnext/tests/utils.py +++ b/erpnext/tests/utils.py @@ -1,7 +1,8 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt -from __future__ import unicode_literals +import copy +from contextlib import contextmanager import frappe @@ -41,3 +42,38 @@ def create_test_contact_and_address(): contact.add_email("test_contact_customer@example.com", is_primary=True) contact.add_phone("+91 0000000000", is_primary_phone=True) contact.insert() + + +@contextmanager +def change_settings(doctype, settings_dict): + """ A context manager to ensure that settings are changed before running + function and restored after running it regardless of exceptions occured. + This is useful in tests where you want to make changes in a function but + don't retain those changes. + import and use as decorator to cover full function or using `with` statement. + + example: + @change_settings("Stock Settings", {"item_naming_by": "Naming Series"}) + def test_case(self): + ... + """ + + try: + settings = frappe.get_doc(doctype) + # remember setting + previous_settings = copy.deepcopy(settings_dict) + for key in previous_settings: + previous_settings[key] = getattr(settings, key) + + # change setting + for key, value in settings_dict.items(): + setattr(settings, key, value) + settings.save() + yield # yield control to calling function + + finally: + # restore settings + settings = frappe.get_doc(doctype) + for key, value in previous_settings.items(): + setattr(settings, key, value) + settings.save() From c15fef571fda7fa6bf2ae7f43380b29098775d87 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 16:26:37 +0530 Subject: [PATCH 34/39] test: item naming series behaviour --- erpnext/stock/doctype/item/test_item.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 234a9132c2..9adacdfb78 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -14,6 +14,7 @@ from erpnext.stock.doctype.item.item import (get_uom_conv_factor, get_item_attri validate_is_stock_item, get_timeline_data) from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details +from erpnext.tests.utils import change_settings test_ignore = ["BOM"] @@ -567,6 +568,13 @@ class TestItem(unittest.TestCase): except frappe.ValidationError as e: self.fail(f"stock item considered non-stock item: {e}") + @change_settings("Stock Settings", {"item_naming_by": "Naming Series"}) + def test_autoname_series(self): + item = frappe.new_doc("Item") + item.item_group = "All Item Groups" + item.save() # if item code saved without item_code then series worked + + def set_item_variant_settings(fields): doc = frappe.get_doc('Item Variant Settings') doc.set('fields', fields) From 3aed662f4690ad6fb5fda680aad4246103eded81 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 16:41:03 +0530 Subject: [PATCH 35/39] chore: translation / semgrep / sider fixes --- erpnext/stock/doctype/item/item.py | 6 +++--- erpnext/stock/doctype/item/test_item.py | 10 +++++----- .../stock_reconciliation/stock_reconciliation.py | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index a0bd49543e..dd815404fa 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -365,8 +365,8 @@ class Item(WebsiteGenerator): # load attributes for v in context.variants: v.attributes = frappe.get_all("Item Variant Attribute", - fields=["attribute", "attribute_value"], - filters={"parent": v.name}) + fields=["attribute", "attribute_value"], + filters={"parent": v.name}) # make a map for easier access in templates v.attribute_map = frappe._dict({}) for attr in v.attributes: @@ -1256,7 +1256,7 @@ def get_uom_conv_factor(uom, stock_uom): inverse_match = frappe.db.get_value("UOM Conversion Factor", {"to_uom": from_uom, "from_uom": to_uom}, ["value"], as_dict=1) if inverse_match: - return 1 / inverse_match.value + return 1 / inverse_match.value # This attempts to try and get conversion from intermediate UOM. # case: diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 9adacdfb78..406039dc58 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -518,11 +518,11 @@ class TestItem(unittest.TestCase): def test_attribute_completions(self): expected_attrs = [{'attribute_value': 'Small'}, - {'attribute_value': 'Extra Small'}, - {'attribute_value': 'Extra Large'}, - {'attribute_value': 'Large'}, - {'attribute_value': '2XL'}, - {'attribute_value': 'Medium'}] + {'attribute_value': 'Extra Small'}, + {'attribute_value': 'Extra Large'}, + {'attribute_value': 'Large'}, + {'attribute_value': '2XL'}, + {'attribute_value': 'Medium'}] attrs = get_item_attribute("Test Size") self.assertEqual(attrs, expected_attrs) diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 96b1cadaaf..b9f91906c6 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -96,7 +96,7 @@ class StockReconciliation(StockController): def validate_data(self): def _get_msg(row_num, msg): - return _("Row # {0}: ").format(row_num+1) + msg + return _("Row # {0}:").format(row_num+1) + " " + msg self.validation_messages = [] item_warehouse_combinations = [] @@ -182,7 +182,7 @@ class StockReconciliation(StockController): validate_cancelled_item(item_code, item.docstatus) except Exception as e: - self.validation_messages.append(_("Row # ") + ("%d: " % (row.idx)) + cstr(e)) + self.validation_messages.append(_("Row #") + " " + ("%d: " % (row.idx)) + cstr(e)) def update_stock_ledger(self): """ find difference between current and expected entries From 15f8a0fb22addd730b1ebfb43635cfb29f1ddb90 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 23 May 2021 18:10:21 +0530 Subject: [PATCH 36/39] test: fix flaky test --- erpnext/stock/doctype/item/test_item.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 406039dc58..c7467a5a0f 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -517,18 +517,15 @@ class TestItem(unittest.TestCase): self.fail(f"Expected db index on these columns: {', '.join(expected_columns)}") def test_attribute_completions(self): - expected_attrs = [{'attribute_value': 'Small'}, - {'attribute_value': 'Extra Small'}, - {'attribute_value': 'Extra Large'}, - {'attribute_value': 'Large'}, - {'attribute_value': '2XL'}, - {'attribute_value': 'Medium'}] + expected_attrs = {"Small", "Extra Small", "Extra Large", "Large", "2XL", "Medium"} attrs = get_item_attribute("Test Size") - self.assertEqual(attrs, expected_attrs) + received_attrs = {attr.attribute_value for attr in attrs} + self.assertEqual(received_attrs, expected_attrs) attrs = get_item_attribute("Test Size", attribute_value="extra") - self.assertEqual(attrs, [{'attribute_value': 'Extra Small'}, {'attribute_value': 'Extra Large'}]) + received_attrs = {attr.attribute_value for attr in attrs} + self.assertEqual(received_attrs, {"Extra Small", "Extra Large"}) def test_check_stock_uom_with_bin(self): # this item has opening stock and stock_uom set in test_records. From 73e41c0bd6e7984c468ca5c3feaac0d5920110d2 Mon Sep 17 00:00:00 2001 From: noahjacob Date: Tue, 25 May 2021 18:01:47 +0530 Subject: [PATCH 37/39] refactor: suggested changes --- erpnext/manufacturing/doctype/work_order/work_order.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index a154464a8b..2600790a59 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -383,7 +383,7 @@ class WorkOrder(Document): work_order_qty = 0.0 if plan_reference.item_reference == item_reference: if self.docstatus == 1: - work_order_qty = cint(plan_reference.qty) / total_bundle_qty + work_order_qty = flt(plan_reference.qty) / total_bundle_qty frappe.db.set_value('Sales Order Item', plan_reference.sales_order_item, 'work_order_qty', work_order_qty) From 10558344b0fa25b12f71fda9ca21bb8c5170cb83 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 27 May 2021 17:05:36 +0530 Subject: [PATCH 38/39] fix: timeout error in the repost item valuation --- erpnext/hooks.py | 4 +++- .../repost_item_valuation/repost_item_valuation.py | 13 ++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index 55169dffba..8ad77a1524 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -332,7 +332,9 @@ scheduler_events = { "erpnext.projects.doctype.project.project.collect_project_status", "erpnext.hr.doctype.shift_type.shift_type.process_auto_attendance_for_all_shifts", "erpnext.support.doctype.issue.issue.set_service_level_agreement_variance", - "erpnext.erpnext_integrations.connectors.shopify_connection.sync_old_orders", + "erpnext.erpnext_integrations.connectors.shopify_connection.sync_old_orders" + ], + "hourly_long": [ "erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.repost_entries" ], "daily": [ diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 5b626ea345..55f2ebb224 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -37,6 +37,9 @@ class RepostItemValuation(Document): self.db_set('status', status) def on_submit(self): + if not frappe.flags.in_test: + return + frappe.enqueue(repost, timeout=1800, queue='long', job_name='repost_sle', now=frappe.flags.in_test, doc=self) @@ -115,12 +118,6 @@ def notify_error_to_stock_managers(doc, traceback): frappe.sendmail(recipients=recipients, subject=subject, message=message) def repost_entries(): - job_log = frappe.get_all('Scheduled Job Log', fields = ['status', 'creation'], - filters = {'scheduled_job_type': 'repost_item_valuation.repost_entries'}, order_by='creation desc', limit=1) - - if job_log and job_log[0]['status'] == 'Start' and time_diff_in_hours(now(), job_log[0]['creation']) < 2: - return - riv_entries = get_repost_item_valuation_entries() for row in riv_entries: @@ -135,9 +132,7 @@ def repost_entries(): check_if_stock_and_account_balance_synced(today(), d.name) def get_repost_item_valuation_entries(): - date = add_to_date(now(), hours=-3) - return frappe.db.sql(""" SELECT name from `tabRepost Item Valuation` WHERE status != 'Completed' and creation <= %s and docstatus = 1 ORDER BY timestamp(posting_date, posting_time) asc, creation asc - """, date, as_dict=1) + """, now(), as_dict=1) From c59371ab0c500d9d863db334931d527e8face6eb Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Thu, 3 Jun 2021 20:02:58 +0530 Subject: [PATCH 39/39] fix: filter type for item query (#25942) --- erpnext/controllers/queries.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/erpnext/controllers/queries.py b/erpnext/controllers/queries.py index 638503edfa..81ac234e70 100644 --- a/erpnext/controllers/queries.py +++ b/erpnext/controllers/queries.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals import frappe import erpnext +import json from frappe.desk.reportview import get_match_cond, get_filters_cond from frappe.utils import nowdate, getdate from collections import defaultdict @@ -198,6 +199,9 @@ def tax_account_query(doctype, txt, searchfield, start, page_len, filters): def item_query(doctype, txt, searchfield, start, page_len, filters, as_dict=False): conditions = [] + if isinstance(filters, str): + filters = json.loads(filters) + #Get searchfields from meta and use in Item Link field query meta = frappe.get_meta("Item", cached=True) searchfields = meta.get_search_fields()