From 27236b7e9ee7a325dfcf75c7588ad31747a04ec1 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 28 Oct 2021 16:45:23 +0530 Subject: [PATCH 01/16] fix: Remove RM Cost column as cost is not retrievable from Job card --- .../cost_of_poor_quality_report.py | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py index 0dcad448d7..c79ded6804 100644 --- a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py +++ b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py @@ -1,8 +1,6 @@ -# Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors +# 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 import _ from frappe.utils import flt @@ -31,7 +29,7 @@ def get_data(report_filters): for row in job_cards: row.operating_cost = flt(row.hour_rate) * (flt(row.total_time_in_mins) / 60.0) - update_raw_material_cost(row, report_filters) + # update_raw_material_cost(row, report_filters) data.append(row) return data @@ -47,11 +45,11 @@ def get_filters(report_filters, operations): return filters -def update_raw_material_cost(row, filters): - row.rm_cost = 0.0 - for data in frappe.get_all("Job Card Item", fields = ["amount"], - filters={"parent": row.name, "docstatus": 1}): - row.rm_cost += data.amount +# def update_raw_material_cost(row, filters): +# row.rm_cost = 0.0 +# for data in frappe.get_all("Job Card Item", fields = ["amount"], +# filters={"parent": row.name, "docstatus": 1}): +# row.rm_cost += data.amount def get_columns(filters): return [ @@ -60,7 +58,7 @@ def get_columns(filters): "fieldtype": "Link", "fieldname": "name", "options": "Job Card", - "width": "100" + "width": "120" }, { "label": _("Work Order"), @@ -112,18 +110,18 @@ def get_columns(filters): "label": _("Operating Cost"), "fieldtype": "Currency", "fieldname": "operating_cost", - "width": "100" - }, - { - "label": _("Raw Material Cost"), - "fieldtype": "Currency", - "fieldname": "rm_cost", - "width": "100" + "width": "150" }, + # { + # "label": _("Raw Material Cost"), + # "fieldtype": "Currency", + # "fieldname": "rm_cost", + # "width": "100" + # }, { "label": _("Total Time (in Mins)"), "fieldtype": "Float", "fieldname": "total_time_in_mins", - "width": "100" + "width": "150" } ] From 8502ccb5b2bd9306ce0ecdb49f4710a66c11860a Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 28 Oct 2021 16:53:45 +0530 Subject: [PATCH 02/16] chore: Add comment hinting to reason --- .../cost_of_poor_quality_report/cost_of_poor_quality_report.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py index c79ded6804..e6666f00bf 100644 --- a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py +++ b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py @@ -45,6 +45,8 @@ def get_filters(report_filters, operations): return filters +# Check PR #28123 as to why this is commented + # def update_raw_material_cost(row, filters): # row.rm_cost = 0.0 # for data in frappe.get_all("Job Card Item", fields = ["amount"], From 8d2abc4b86fe7d029f6e02eedb776ca0fbdbc35f Mon Sep 17 00:00:00 2001 From: Mohammed Yusuf Shaikh <49878143+mohammedyusufshaikh@users.noreply.github.com> Date: Wed, 24 Nov 2021 16:23:12 +0530 Subject: [PATCH 03/16] fix: fixes in work order doctype (#28217) * fix: fixes in work order doctype * fix: sider issues and disabled set only once property * fix: set default qty to manufacture * fix: dont manually collapse sections * fix: remove unnecessary messages * fix: make dependent fields read only Co-authored-by: Ankush Menat --- .../doctype/work_order/work_order.js | 2 +- .../doctype/work_order/work_order.json | 6 +- .../work_order_operation.json | 68 ++++++++----------- 3 files changed, 34 insertions(+), 42 deletions(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.js b/erpnext/manufacturing/doctype/work_order/work_order.js index bfce1b8cbe..5ffbb0374e 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.js +++ b/erpnext/manufacturing/doctype/work_order/work_order.js @@ -442,7 +442,7 @@ frappe.ui.form.on("Work Order", { additional_operating_cost: function(frm) { erpnext.work_order.calculate_cost(frm.doc); erpnext.work_order.calculate_total_cost(frm); - } + }, }); frappe.ui.form.on("Work Order Item", { diff --git a/erpnext/manufacturing/doctype/work_order/work_order.json b/erpnext/manufacturing/doctype/work_order/work_order.json index df7ee53b92..12cd58f418 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.json +++ b/erpnext/manufacturing/doctype/work_order/work_order.json @@ -326,6 +326,7 @@ "label": "Expected Delivery Date" }, { + "collapsible": 1, "fieldname": "operations_section", "fieldtype": "Section Break", "label": "Operations", @@ -337,7 +338,7 @@ "fieldname": "transfer_material_against", "fieldtype": "Select", "label": "Transfer Material Against", - "options": "\nWork Order\nJob Card" + "options": "Work Order\nJob Card" }, { "fieldname": "operations", @@ -573,8 +574,7 @@ "image_field": "image", "is_submittable": 1, "links": [], - "migration_hash": "a18118963f4fcdb7f9d326de5f4063ba", - "modified": "2021-10-29 15:12:32.203605", + "modified": "2021-11-08 17:36:07.016300", "modified_by": "Administrator", "module": "Manufacturing", "name": "Work Order", diff --git a/erpnext/manufacturing/doctype/work_order_operation/work_order_operation.json b/erpnext/manufacturing/doctype/work_order_operation/work_order_operation.json index f7b8787a0b..647c14b33d 100644 --- a/erpnext/manufacturing/doctype/work_order_operation/work_order_operation.json +++ b/erpnext/manufacturing/doctype/work_order_operation/work_order_operation.json @@ -6,27 +6,27 @@ "field_order": [ "details", "operation", - "bom", - "column_break_4", - "description", - "sequence_id", - "col_break1", - "completed_qty", "status", + "completed_qty", + "column_break_4", + "bom", "workstation", + "sequence_id", + "section_break_10", + "description", "estimated_time_and_cost", "planned_start_time", - "planned_end_time", - "column_break_10", - "time_in_mins", "hour_rate", + "time_in_mins", + "column_break_10", + "planned_end_time", "batch_size", "planned_operating_cost", "section_break_9", "actual_start_time", - "actual_end_time", - "column_break_11", "actual_operation_time", + "column_break_11", + "actual_end_time", "actual_operating_cost" ], "fields": [ @@ -42,7 +42,6 @@ "oldfieldname": "operation_no", "oldfieldtype": "Data", "options": "Operation", - "read_only": 1, "reqd": 1 }, { @@ -52,20 +51,14 @@ "label": "BOM", "no_copy": 1, "options": "BOM", - "print_hide": 1, - "read_only": 1 + "print_hide": 1 }, { "fieldname": "description", "fieldtype": "Text Editor", "label": "Operation Description", "oldfieldname": "opn_description", - "oldfieldtype": "Text", - "read_only": 1 - }, - { - "fieldname": "col_break1", - "fieldtype": "Column Break" + "oldfieldtype": "Text" }, { "columns": 1, @@ -74,19 +67,16 @@ "fieldtype": "Float", "in_list_view": 1, "label": "Completed Qty", - "no_copy": 1, - "read_only": 1 + "no_copy": 1 }, { "columns": 1, "default": "Pending", "fieldname": "status", "fieldtype": "Select", - "in_list_view": 1, "label": "Status", "no_copy": 1, - "options": "Pending\nWork in Progress\nCompleted", - "read_only": 1 + "options": "Pending\nWork in Progress\nCompleted" }, { "fieldname": "workstation", @@ -106,15 +96,13 @@ "fieldname": "planned_start_time", "fieldtype": "Datetime", "label": "Planned Start Time", - "no_copy": 1, - "read_only": 1 + "no_copy": 1 }, { "fieldname": "planned_end_time", "fieldtype": "Datetime", "label": "Planned End Time", - "no_copy": 1, - "read_only": 1 + "no_copy": 1 }, { "fieldname": "column_break_10", @@ -122,7 +110,7 @@ }, { "columns": 1, - "description": "in Minutes", + "description": "In Minutes", "fieldname": "time_in_mins", "fieldtype": "Float", "in_list_view": 1, @@ -152,6 +140,7 @@ "label": "Actual Time and Cost" }, { + "description": "Updated via 'Time Log' (In Minutes)", "fieldname": "actual_start_time", "fieldtype": "Datetime", "label": "Actual Start Time", @@ -159,7 +148,7 @@ "read_only": 1 }, { - "description": "Updated via 'Time Log'", + "description": "Updated via 'Time Log' (In Minutes)", "fieldname": "actual_end_time", "fieldtype": "Datetime", "label": "Actual End Time", @@ -171,7 +160,7 @@ "fieldtype": "Column Break" }, { - "description": "in Minutes\nUpdated via 'Time Log'", + "description": "Updated via 'Time Log' (In Minutes)", "fieldname": "actual_operation_time", "fieldtype": "Float", "label": "Actual Operation Time", @@ -190,25 +179,28 @@ { "fieldname": "batch_size", "fieldtype": "Int", - "label": "Batch Size", - "read_only": 1 + "label": "Batch Size" }, { "fieldname": "sequence_id", "fieldtype": "Int", + "hidden": 1, "label": "Sequence ID", - "print_hide": 1, - "read_only": 1 + "print_hide": 1 }, { "fieldname": "column_break_4", "fieldtype": "Column Break" + }, + { + "fieldname": "section_break_10", + "fieldtype": "Section Break" } ], "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2021-06-24 14:36:12.835543", + "modified": "2021-11-24 04:52:54.295168", "modified_by": "Administrator", "module": "Manufacturing", "name": "Work Order Operation", @@ -217,4 +209,4 @@ "sort_field": "modified", "sort_order": "DESC", "track_changes": 1 -} +} \ No newline at end of file From 42395af22ad3f55a7bbef64f1202768abca77042 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 25 Nov 2021 13:28:52 +0530 Subject: [PATCH 04/16] fix: Remove commented code --- .../cost_of_poor_quality_report.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py index e6666f00bf..77418235b0 100644 --- a/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py +++ b/erpnext/manufacturing/report/cost_of_poor_quality_report/cost_of_poor_quality_report.py @@ -29,7 +29,6 @@ def get_data(report_filters): for row in job_cards: row.operating_cost = flt(row.hour_rate) * (flt(row.total_time_in_mins) / 60.0) - # update_raw_material_cost(row, report_filters) data.append(row) return data @@ -45,14 +44,6 @@ def get_filters(report_filters, operations): return filters -# Check PR #28123 as to why this is commented - -# def update_raw_material_cost(row, filters): -# row.rm_cost = 0.0 -# for data in frappe.get_all("Job Card Item", fields = ["amount"], -# filters={"parent": row.name, "docstatus": 1}): -# row.rm_cost += data.amount - def get_columns(filters): return [ { @@ -114,12 +105,6 @@ def get_columns(filters): "fieldname": "operating_cost", "width": "150" }, - # { - # "label": _("Raw Material Cost"), - # "fieldtype": "Currency", - # "fieldname": "rm_cost", - # "width": "100" - # }, { "label": _("Total Time (in Mins)"), "fieldtype": "Float", From 6dc9b822bce20fc4520f6f5fe597a20c554ba025 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 28 Oct 2021 15:53:18 +0530 Subject: [PATCH 05/16] refactor: item-wh wise reposting by default --- erpnext/controllers/stock_controller.py | 38 ++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index aba15b47e3..6431388208 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -544,7 +544,7 @@ class StockController(AccountsController): "company": self.company }) if future_sle_exists(args): - create_repost_item_valuation_entry(args) + create_item_wise_repost_entries(voucher_type=self.doctype, voucher_no=self.name) @frappe.whitelist() def make_quality_inspections(doctype, docname, items): @@ -679,3 +679,39 @@ def create_repost_item_valuation_entry(args): repost_entry.flags.ignore_permissions = True repost_entry.save() repost_entry.submit() + + +def create_item_wise_repost_entries(voucher_type, voucher_no, allow_zero_rate=False): + """Using a voucher create repost item valuation records for all item-warehouse pairs.""" + + stock_ledger_entries = frappe.db.get_all("Stock Ledger Entry", + filters={"voucher_type": voucher_type, "voucher_no": voucher_no}, + fields=["item_code", "warehouse", "posting_date", "posting_time", "creation", "company"], + order_by="creation asc", + group_by="item_code, warehouse" + ) + distinct_item_warehouses = set() + + repost_entries = [] + + for sle in stock_ledger_entries: + item_wh = (sle.item_code, sle.warehouse) + if item_wh in distinct_item_warehouses: + continue + distinct_item_warehouses.add(item_wh) + + repost_entry = frappe.new_doc("Repost Item Valuation") + repost_entry.based_on = "Item and Warehouse" + repost_entry.voucher_type = voucher_type + repost_entry.voucher_no = voucher_no + + repost_entry.item_code = sle.item_code + repost_entry.warehouse = sle.warehouse + repost_entry.posting_date = sle.posting_date + repost_entry.posting_time = sle.posting_time + repost_entry.allow_zero_rate = allow_zero_rate + repost_entry.flags.ignore_links = True + repost_entry.submit() + repost_entries.append(repost_entry) + + return repost_entries From a36c249d3dcb413a4dff3d9552b6eaadf56d844e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 28 Oct 2021 16:03:37 +0530 Subject: [PATCH 06/16] test: item-wh repost creation --- erpnext/controllers/stock_controller.py | 1 - .../test_repost_item_valuation.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 6431388208..d241f3f7d0 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -691,7 +691,6 @@ def create_item_wise_repost_entries(voucher_type, voucher_no, allow_zero_rate=Fa group_by="item_code, warehouse" ) distinct_item_warehouses = set() - repost_entries = [] for sle in stock_ledger_entries: diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index c086f938b5..7abda61a83 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -5,6 +5,8 @@ import unittest import frappe +from erpnext.controllers.stock_controller import create_item_wise_repost_entries +from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt from erpnext.stock.doctype.repost_item_valuation.repost_item_valuation import ( in_configured_timeslot, ) @@ -70,3 +72,15 @@ class TestRepostItemValuation(unittest.TestCase): in_configured_timeslot(repost_settings, case.get("current_time")), msg=f"Exepcted false from : {case}", ) + + def test_create_item_wise_repost_item_valuation_entries(self): + pr = make_purchase_receipt(company="_Test Company with perpetual inventory", + warehouse = "Stores - TCP1", get_multiple_items = True) + + rivs = create_item_wise_repost_entries(pr.doctype, pr.name) + self.assertGreaterEqual(len(rivs), 2) + self.assertIn("_Test Item", [d.item_code for d in rivs]) + + for riv in rivs: + self.assertEqual(riv.company, "_Test Company with perpetual inventory") + self.assertEqual(riv.warehouse, "Stores - TCP1") From d220e08ba4c55c334a5586481dec192c4896ea13 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 28 Oct 2021 17:47:00 +0530 Subject: [PATCH 07/16] refactor: reuse get_items_to_be_repost function --- erpnext/controllers/stock_controller.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index d241f3f7d0..fe5d0c70cf 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -17,7 +17,7 @@ from erpnext.accounts.general_ledger import ( from erpnext.accounts.utils import get_fiscal_year from erpnext.controllers.accounts_controller import AccountsController from erpnext.stock import get_warehouse_account_map -from erpnext.stock.stock_ledger import get_valuation_rate +from erpnext.stock.stock_ledger import get_items_to_be_repost, get_valuation_rate class QualityInspectionRequiredError(frappe.ValidationError): pass @@ -684,12 +684,8 @@ def create_repost_item_valuation_entry(args): def create_item_wise_repost_entries(voucher_type, voucher_no, allow_zero_rate=False): """Using a voucher create repost item valuation records for all item-warehouse pairs.""" - stock_ledger_entries = frappe.db.get_all("Stock Ledger Entry", - filters={"voucher_type": voucher_type, "voucher_no": voucher_no}, - fields=["item_code", "warehouse", "posting_date", "posting_time", "creation", "company"], - order_by="creation asc", - group_by="item_code, warehouse" - ) + stock_ledger_entries = get_items_to_be_repost(voucher_type, voucher_no) + distinct_item_warehouses = set() repost_entries = [] From 45dd46be3d92201eb0195ae62fa5b2ba15616e5b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 2 Nov 2021 10:50:52 +0530 Subject: [PATCH 08/16] feat: option to select reposting method In current implementation selecting Item-Warehouse based reposting is better for few users, who don't use depenent SLEs but have frequent transactions involving same items. This change lets them switch to item-warehouse based reposting if required. Only use this if you understand technicalities of stock reposting. This is experimental but will become mainstream in coming days. --- erpnext/controllers/stock_controller.py | 7 ++++++- .../stock_reposting_settings.json | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index fe5d0c70cf..ca567fdc58 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -544,7 +544,12 @@ class StockController(AccountsController): "company": self.company }) if future_sle_exists(args): - create_item_wise_repost_entries(voucher_type=self.doctype, voucher_no=self.name) + item_based_reposting = cint(frappe.db.get_single_value("Stock Reposting Settings", "item_based_reposting")) + if item_based_reposting: + create_item_wise_repost_entries(voucher_type=self.doctype, voucher_no=self.name) + else: + create_repost_item_valuation_entry(args) + @frappe.whitelist() def make_quality_inspections(doctype, docname, items): diff --git a/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.json b/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.json index 2474059003..0facae8d3b 100644 --- a/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.json +++ b/erpnext/stock/doctype/stock_reposting_settings/stock_reposting_settings.json @@ -1,6 +1,7 @@ { "actions": [], "allow_rename": 1, + "beta": 1, "creation": "2021-10-01 10:56:30.814787", "doctype": "DocType", "editable_grid": 1, @@ -10,7 +11,8 @@ "limit_reposting_timeslot", "start_time", "end_time", - "limits_dont_apply_on" + "limits_dont_apply_on", + "item_based_reposting" ], "fields": [ { @@ -44,12 +46,18 @@ "fieldname": "limit_reposting_timeslot", "fieldtype": "Check", "label": "Limit timeslot for Stock Reposting" + }, + { + "default": "0", + "fieldname": "item_based_reposting", + "fieldtype": "Check", + "label": "Use Item based reposting" } ], "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2021-10-01 11:27:28.981594", + "modified": "2021-11-02 01:22:45.155841", "modified_by": "Administrator", "module": "Stock", "name": "Stock Reposting Settings", From a5a8c9104fbb5279e6b25c529b6b4c0fc7ccef97 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 2 Nov 2021 11:08:36 +0530 Subject: [PATCH 09/16] perf: index for item-sh on repost item valuation Item-WH based reposting requires querying existing similar repost. Assuming there is only 1 max extra entry with same params just indexing item-WH is sufficient to speed up the query. --- .../doctype/repost_item_valuation/repost_item_valuation.json | 4 ++-- .../doctype/repost_item_valuation/repost_item_valuation.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json index 3ff0f60b3e..794c15ef33 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json @@ -177,7 +177,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-11-18 02:18:10.524560", + "modified": "2021-11-24 02:18:10.524560", "modified_by": "Administrator", "module": "Stock", "name": "Repost Item Valuation", @@ -228,4 +228,4 @@ ], "sort_field": "modified", "sort_order": "DESC" -} \ No newline at end of file +} 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 59d191fa07..475f525157 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -58,6 +58,11 @@ class RepostItemValuation(Document): frappe.enqueue(repost, timeout=1800, queue='long', job_name='repost_sle', now=True, doc=self) + +def on_doctype_update(): + frappe.db.add_index("Repost Item Valuation", ["warehouse", "item_code"], "item_warehouse") + + def repost(doc): try: if not frappe.db.exists("Repost Item Valuation", doc.name): From 1d3842f03a8f90d2110e43008091ea761f83fcb4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 2 Nov 2021 15:22:39 +0530 Subject: [PATCH 10/16] fix: dont erase voucher_type and voucher_no for item_wh repost kept for tracability. --- .../doctype/repost_item_valuation/repost_item_valuation.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 475f525157..64cea1db34 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -27,15 +27,12 @@ class RepostItemValuation(Document): if self.based_on == 'Transaction': self.item_code = None self.warehouse = None - else: - self.voucher_type = None - self.voucher_no = None self.allow_negative_stock = self.allow_negative_stock or \ cint(frappe.db.get_single_value("Stock Settings", "allow_negative_stock")) def set_company(self): - if self.voucher_type and self.voucher_no: + if self.based_on == "Transaction": self.company = frappe.get_cached_value(self.voucher_type, self.voucher_no, "company") elif self.warehouse: self.company = frappe.get_cached_value("Warehouse", self.warehouse, "company") From 0d0e24a5f51e566dfa04364787dfb883a9b7ad30 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 2 Nov 2021 11:13:37 +0530 Subject: [PATCH 11/16] perf: skip unnecessary item-wh reposts Using basic idea that repost with older posting date will also take care of subsequent posting dates... When Item-WH reposts are queued: 1. If another repost with same item-wh but older posting date exists then skip current one. 2. If another repost with same item-wh but newer posting date exists then skip another one. --- .../repost_item_valuation.json | 2 +- .../repost_item_valuation.py | 58 +++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json index 794c15ef33..cd7e63b18b 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json @@ -64,7 +64,7 @@ "in_standard_filter": 1, "label": "Status", "no_copy": 1, - "options": "Queued\nIn Progress\nCompleted\nFailed", + "options": "Queued\nIn Progress\nCompleted\nSkipped\nFailed", "read_only": 1 }, { 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 64cea1db34..c0cb2c54b1 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -2,10 +2,21 @@ # For license information, please see license.txt +import datetime + import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import cint, get_link_to_form, get_weekday, now, nowtime, today +from frappe.utils import ( + cint, + get_datetime, + get_link_to_form, + get_time, + get_weekday, + now, + nowtime, + today, +) from frappe.utils.user import get_users_with_role from rq.timeouts import JobTimeoutException @@ -19,7 +30,7 @@ from erpnext.stock.stock_ledger import repost_future_sle class RepostItemValuation(Document): def validate(self): - self.set_status() + self.set_status(write=False) self.reset_field_values() self.set_company() @@ -37,12 +48,17 @@ class RepostItemValuation(Document): elif self.warehouse: self.company = frappe.get_cached_value("Warehouse", self.warehouse, "company") - def set_status(self, status=None): + def set_status(self, status=None, write=True): + status = status or self.status if not status: - status = 'Queued' - self.db_set('status', status) + self.status = 'Queued' + else: + self.status = status + if write: + self.db_set('status', self.status) def on_submit(self): + self.deduplicate_similar_repost() if not frappe.flags.in_test: return @@ -55,6 +71,35 @@ class RepostItemValuation(Document): frappe.enqueue(repost, timeout=1800, queue='long', job_name='repost_sle', now=True, doc=self) + def deduplicate_similar_repost(self): + """ Deduplicate similar reposts based on item-warehouse-posting combination.""" + if self.based_on != "Item and Warehouse": + return + + queued = frappe.db.get_value( + "Repost Item Valuation", + filters={ + "docstatus": 1, + "status": "Queued", + "item_code": self.item_code, + "warehouse": self.warehouse, + "based_on": self.based_on, + "name": ("!=", self.name) + }, + fieldname=["name", "posting_date", "posting_time"], + as_dict=True + ) + if not queued: + return + + posting_timestamp = datetime.datetime.combine(get_datetime(self.posting_date), get_time(self.posting_time)) + queued_timestamp = datetime.datetime.combine(get_datetime(queued.posting_date), get_time(queued.posting_time)) + + if posting_timestamp > queued_timestamp: + self.set_status("Skipped") + else: + frappe.db.set_value("Repost Item Valuation", queued.name, "status", "Skipped") + def on_doctype_update(): frappe.db.add_index("Repost Item Valuation", ["warehouse", "item_code"], "item_warehouse") @@ -136,7 +181,8 @@ def repost_entries(): for row in riv_entries: doc = frappe.get_doc('Repost Item Valuation', row.name) - repost(doc) + if doc.status in ('Queued', 'In Progress'): + repost(doc) riv_entries = get_repost_item_valuation_entries() if riv_entries: From 55631dd0d68fb3306bf285fa205e55e567b73837 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 2 Nov 2021 18:03:43 +0530 Subject: [PATCH 12/16] test: item-wh deduplication in reposting --- .../repost_item_valuation.py | 2 +- .../test_repost_item_valuation.py | 55 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) 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 c0cb2c54b1..ff490f8ecc 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -59,7 +59,7 @@ class RepostItemValuation(Document): def on_submit(self): self.deduplicate_similar_repost() - if not frappe.flags.in_test: + if not frappe.flags.in_test or self.flags.dont_run_in_test: return frappe.enqueue(repost, timeout=1800, queue='long', diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 7abda61a83..ea79572bc5 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -74,8 +74,11 @@ class TestRepostItemValuation(unittest.TestCase): ) def test_create_item_wise_repost_item_valuation_entries(self): - pr = make_purchase_receipt(company="_Test Company with perpetual inventory", - warehouse = "Stores - TCP1", get_multiple_items = True) + pr = make_purchase_receipt( + company="_Test Company with perpetual inventory", + warehouse="Stores - TCP1", + get_multiple_items=True, + ) rivs = create_item_wise_repost_entries(pr.doctype, pr.name) self.assertGreaterEqual(len(rivs), 2) @@ -84,3 +87,51 @@ class TestRepostItemValuation(unittest.TestCase): for riv in rivs: self.assertEqual(riv.company, "_Test Company with perpetual inventory") self.assertEqual(riv.warehouse, "Stores - TCP1") + + def test_deduplication(self): + def _assert_status(doc, status): + doc.load_from_db() + self.assertEqual(doc.status, status) + + riv_args = frappe._dict( + doctype="Repost Item Valuation", + item_code="_Test Item", + warehouse="_Test Warehouse - _TC", + based_on="Item and Warehouse", + voucher_type="Sales Invoice", + voucher_no="SI-1", + posting_date="2021-01-02", + posting_time="00:01:00", + ) + + # new repost without any duplicates + riv1 = frappe.get_doc(riv_args) + riv1.flags.dont_run_in_test = True + riv1.submit() + _assert_status(riv1, "Queued") + self.assertEqual(riv1.voucher_type, "Sales Invoice") # traceability + self.assertEqual(riv1.voucher_no, "SI-1") + + # newer than existing duplicate - riv1 + riv2 = frappe.get_doc(riv_args.update({"posting_date": "2021-01-03"})) + riv2.flags.dont_run_in_test = True + riv2.submit() + _assert_status(riv2, "Skipped") + + # older than exisitng duplicate - riv1 + riv3 = frappe.get_doc(riv_args.update({"posting_date": "2021-01-01"})) + riv3.flags.dont_run_in_test = True + riv3.submit() + _assert_status(riv3, "Queued") + _assert_status(riv1, "Skipped") + + # unrelated reposts, shouldn't do anything to others. + riv4 = frappe.get_doc(riv_args.update({"warehouse": "Stores - _TC"})) + riv4.flags.dont_run_in_test = True + riv4.submit() + _assert_status(riv4, "Queued") + _assert_status(riv3, "Queued") + + # to avoid breaking other tests accidentaly + riv4.set_status("Skipped") + riv3.set_status("Skipped") From ed94f0f3f26ecf71f9113d3d2d10aed05cf31265 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 24 Nov 2021 13:57:39 +0530 Subject: [PATCH 13/16] refactor: deduplicate during repost background job --- .../repost_item_valuation.py | 59 +++++++------------ .../test_repost_item_valuation.py | 3 + 2 files changed, 25 insertions(+), 37 deletions(-) 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 ff490f8ecc..965a32d92d 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -1,22 +1,10 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt - -import datetime - import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import ( - cint, - get_datetime, - get_link_to_form, - get_time, - get_weekday, - now, - nowtime, - today, -) +from frappe.utils import cint, get_link_to_form, get_weekday, now, nowtime, today from frappe.utils.user import get_users_with_role from rq.timeouts import JobTimeoutException @@ -58,7 +46,6 @@ class RepostItemValuation(Document): self.db_set('status', self.status) def on_submit(self): - self.deduplicate_similar_repost() if not frappe.flags.in_test or self.flags.dont_run_in_test: return @@ -76,30 +63,27 @@ class RepostItemValuation(Document): if self.based_on != "Item and Warehouse": return - queued = frappe.db.get_value( - "Repost Item Valuation", - filters={ - "docstatus": 1, - "status": "Queued", - "item_code": self.item_code, - "warehouse": self.warehouse, - "based_on": self.based_on, - "name": ("!=", self.name) - }, - fieldname=["name", "posting_date", "posting_time"], - as_dict=True - ) - if not queued: - return - - posting_timestamp = datetime.datetime.combine(get_datetime(self.posting_date), get_time(self.posting_time)) - queued_timestamp = datetime.datetime.combine(get_datetime(queued.posting_date), get_time(queued.posting_time)) - - if posting_timestamp > queued_timestamp: - self.set_status("Skipped") - else: - frappe.db.set_value("Repost Item Valuation", queued.name, "status", "Skipped") + filters = { + "item_code": self.item_code, + "warehouse": self.warehouse, + "name": self.name, + "posting_date": self.posting_date, + "posting_time": self.posting_time, + } + frappe.db.sql(""" + update `tabRepost Item Valuation` + set status = 'Skipped' + WHERE item_code = %(item_code)s + and warehouse = %(warehouse)s + and name != %(name)s + and TIMESTAMP(posting_date, posting_time) > TIMESTAMP(%(posting_date)s, %(posting_time)s) + and docstatus = 1 + and status = 'Queued' + and based_on = 'Item and Warehouse' + """, + filters + ) def on_doctype_update(): frappe.db.add_index("Repost Item Valuation", ["warehouse", "item_code"], "item_warehouse") @@ -182,6 +166,7 @@ def repost_entries(): for row in riv_entries: doc = frappe.get_doc('Repost Item Valuation', row.name) if doc.status in ('Queued', 'In Progress'): + doc.deduplicate_similar_repost() repost(doc) riv_entries = get_repost_item_valuation_entries() diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index ea79572bc5..de793163fd 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -116,12 +116,14 @@ class TestRepostItemValuation(unittest.TestCase): riv2 = frappe.get_doc(riv_args.update({"posting_date": "2021-01-03"})) riv2.flags.dont_run_in_test = True riv2.submit() + riv1.deduplicate_similar_repost() _assert_status(riv2, "Skipped") # older than exisitng duplicate - riv1 riv3 = frappe.get_doc(riv_args.update({"posting_date": "2021-01-01"})) riv3.flags.dont_run_in_test = True riv3.submit() + riv3.deduplicate_similar_repost() _assert_status(riv3, "Queued") _assert_status(riv1, "Skipped") @@ -129,6 +131,7 @@ class TestRepostItemValuation(unittest.TestCase): riv4 = frappe.get_doc(riv_args.update({"warehouse": "Stores - _TC"})) riv4.flags.dont_run_in_test = True riv4.submit() + riv4.deduplicate_similar_repost() _assert_status(riv4, "Queued") _assert_status(riv3, "Queued") From 0a2964dc826d6f027a20f8d16ab62a1a59238853 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 24 Nov 2021 15:55:31 +0530 Subject: [PATCH 14/16] fix: ignore permissions while creating reposts --- erpnext/controllers/stock_controller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index ca567fdc58..ae170945aa 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -711,6 +711,7 @@ def create_item_wise_repost_entries(voucher_type, voucher_no, allow_zero_rate=Fa repost_entry.posting_time = sle.posting_time repost_entry.allow_zero_rate = allow_zero_rate repost_entry.flags.ignore_links = True + repost_entry.flags.ignore_permissions = True repost_entry.submit() repost_entries.append(repost_entry) From 8b33358660fac5ee66c2b56e966faca1f6ba0522 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 25 Nov 2021 17:24:59 +0530 Subject: [PATCH 15/16] fix: patch failure due to new doctype --- .../v12_0/move_target_distribution_from_parent_to_child.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/patches/v12_0/move_target_distribution_from_parent_to_child.py b/erpnext/patches/v12_0/move_target_distribution_from_parent_to_child.py index 1e230a787c..36fe18d8b7 100644 --- a/erpnext/patches/v12_0/move_target_distribution_from_parent_to_child.py +++ b/erpnext/patches/v12_0/move_target_distribution_from_parent_to_child.py @@ -7,6 +7,7 @@ import frappe def execute(): frappe.reload_doc("setup", "doctype", "target_detail") + frappe.reload_doc("core", "doctype", "prepared_report") for d in ['Sales Person', 'Sales Partner', 'Territory']: frappe.db.sql(""" From 87f2dcfb597e45d9a7f970fbe4a644e1e70bca92 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 25 Nov 2021 19:38:44 +0530 Subject: [PATCH 16/16] fix: total stock summary UI glitch #28564 fix: total stock summary UI glitch --- .../total_stock_summary.js | 22 ++++--------------- .../total_stock_summary.py | 11 +++------- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/erpnext/stock/report/total_stock_summary/total_stock_summary.js b/erpnext/stock/report/total_stock_summary/total_stock_summary.js index 90648f1b24..88054aaea7 100644 --- a/erpnext/stock/report/total_stock_summary/total_stock_summary.js +++ b/erpnext/stock/report/total_stock_summary/total_stock_summary.js @@ -10,23 +10,8 @@ frappe.query_reports["Total Stock Summary"] = { "fieldtype": "Select", "width": "80", "reqd": 1, - "options": ["", "Warehouse", "Company"], - "change": function() { - let group_by = frappe.query_report.get_filter_value("group_by") - let company_filter = frappe.query_report.get_filter("company") - if (group_by == "Company") { - company_filter.df.reqd = 0; - company_filter.df.hidden = 1; - frappe.query_report.set_filter_value("company", ""); - company_filter.refresh(); - } - else { - company_filter.df.reqd = 1; - company_filter.df.hidden = 0; - company_filter.refresh(); - frappe.query_report.refresh(); - } - } + "options": ["Warehouse", "Company"], + "default": "Warehouse", }, { "fieldname": "company", @@ -34,8 +19,9 @@ frappe.query_reports["Total Stock Summary"] = { "fieldtype": "Link", "width": "80", "options": "Company", + "reqd": 1, "default": frappe.defaults.get_user_default("Company"), - "reqd": 1 + "depends_on": "eval: doc.group_by != 'Company'", }, ] } diff --git a/erpnext/stock/report/total_stock_summary/total_stock_summary.py b/erpnext/stock/report/total_stock_summary/total_stock_summary.py index 7e47b50b85..6f27558b88 100644 --- a/erpnext/stock/report/total_stock_summary/total_stock_summary.py +++ b/erpnext/stock/report/total_stock_summary/total_stock_summary.py @@ -7,8 +7,9 @@ from frappe import _ def execute(filters=None): - if not filters: filters = {} - validate_filters(filters) + + if not filters: + filters = {} columns = get_columns() stock = get_total_stock(filters) @@ -53,9 +54,3 @@ def get_total_stock(filters): ON warehouse.name = ledger.warehouse WHERE ledger.actual_qty != 0 %s""" % (columns, conditions)) - -def validate_filters(filters): - if filters.get("group_by") == 'Company' and \ - filters.get("company"): - - frappe.throw(_("Please set Company filter blank if Group By is 'Company'"))