From cbb5ffb6feff3e6b47a3d815ab986e90b66b4db3 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 24 Feb 2022 15:58:12 +0530 Subject: [PATCH 01/25] fix: Multi-currency bank reconciliation fixes --- .../bank_reconciliation_tool/bank_reconciliation_tool.py | 2 +- .../doctype/bank_transaction/bank_transaction.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index 4211bd0169d..11c94f705dc 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -231,7 +231,7 @@ def reconcile_vouchers(bank_transaction_name, vouchers): }), transaction.currency, company_account) if total_amount > transaction.unallocated_amount: - frappe.throw(_("The Sum Total of Amounts of All Selected Vouchers Should be Less than the Unallocated Amount of the Bank Transaction")) + frappe.throw(_("The sum total of amounts of all selected vouchers should be less than the unallocated amount of the bank transaction")) account = frappe.db.get_value("Bank Account", transaction.bank_account, "account") for voucher in vouchers: diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 51e1d6e9a00..655a1c22656 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -109,8 +109,13 @@ def get_paid_amount(payment_entry, currency, bank_account): paid_amount_field = "paid_amount" if payment_entry.payment_document == 'Payment Entry': doc = frappe.get_doc("Payment Entry", payment_entry.payment_entry) - paid_amount_field = ("base_paid_amount" - if doc.paid_to_account_currency == currency else "paid_amount") + + if doc.payment_type == 'Receive': + paid_amount_field = ("received_amount" + if doc.paid_to_account_currency == currency else "base_received_amount") + elif doc.payment_type == 'Pay': + paid_amount_field = ("paid_amount" + if doc.paid_to_account_currency == currency else "base_paid_amount") return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, paid_amount_field) From abe580e8b0fc6f5fae720c4ab4713eb2e239abf6 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 1 Mar 2022 17:37:38 +0530 Subject: [PATCH 02/25] fix: Nil and Exempted values in GSTR-3B Report --- .../doctype/gstr_3b_report/gstr_3b_report.py | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py index cb79cf82866..d5ff88c7c90 100644 --- a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py +++ b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py @@ -127,7 +127,8 @@ class GSTR3BReport(Document): def get_inward_nil_exempt(self, state): inward_nil_exempt = frappe.db.sql(""" - SELECT p.place_of_supply, sum(i.base_amount) as base_amount, i.is_nil_exempt, i.is_non_gst + SELECT p.place_of_supply, p.supplier_address, + i.base_amount, i.is_nil_exempt, i.is_non_gst FROM `tabPurchase Invoice` p , `tabPurchase Invoice Item` i WHERE p.docstatus = 1 and p.name = i.parent and p.is_opening = 'No' @@ -135,7 +136,7 @@ class GSTR3BReport(Document): and (i.is_nil_exempt = 1 or i.is_non_gst = 1 or p.gst_category = 'Registered Composition') and month(p.posting_date) = %s and year(p.posting_date) = %s and p.company = %s and p.company_gstin = %s - GROUP BY p.place_of_supply, i.is_nil_exempt, i.is_non_gst""", + """, (self.month_no, self.year, self.company, self.gst_details.get("gstin")), as_dict=1) inward_nil_exempt_details = { @@ -149,18 +150,24 @@ class GSTR3BReport(Document): } } + address_state_map = get_address_state_map() + for d in inward_nil_exempt: - if d.place_of_supply: - if (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ - and state == d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["gst"]["intra"] += d.base_amount - elif (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ - and state != d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["gst"]["inter"] += d.base_amount - elif d.is_non_gst == 1 and state == d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["non_gst"]["intra"] += d.base_amount - elif d.is_non_gst == 1 and state != d.place_of_supply.split("-")[1]: - inward_nil_exempt_details["non_gst"]["inter"] += d.base_amount + if not d.place_of_supply: + d.place_of_supply = "00-" + cstr(state) + + supplier_state = address_state_map.get(d.supplier_address) or state + + if (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ + and cstr(supplier_state) == cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["gst"]["intra"] += d.base_amount + elif (d.is_nil_exempt == 1 or d.get('gst_category') == 'Registered Composition') \ + and cstr(supplier_state) != cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["gst"]["inter"] += d.base_amount + elif d.is_non_gst == 1 and cstr(supplier_state) == cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["non_gst"]["intra"] += d.base_amount + elif d.is_non_gst == 1 and cstr(supplier_state) != cstr(d.place_of_supply.split("-")[1]): + inward_nil_exempt_details["non_gst"]["inter"] += d.base_amount return inward_nil_exempt_details @@ -419,6 +426,11 @@ class GSTR3BReport(Document): return ",".join(missing_field_invoices) +def get_address_state_map(): + return frappe._dict( + frappe.get_all('Address', fields=['name', 'gst_state'], as_list=1) + ) + def get_json(template): file_path = os.path.join(os.path.dirname(__file__), '{template}.json'.format(template=template)) with open(file_path, 'r') as f: From 395b15058caedbcd6bc461d9d397a563bd2e981b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 10:50:03 +0530 Subject: [PATCH 03/25] fix: Sales and Purchase retrun optimization --- erpnext/controllers/sales_and_purchase_return.py | 13 ++++++++----- .../stock/doctype/delivery_note/delivery_note.py | 3 +++ .../doctype/purchase_receipt/purchase_receipt.py | 3 +++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 8c3aab442bb..2b094e3626c 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -208,7 +208,7 @@ def get_already_returned_items(doc): return items -def get_returned_qty_map_for_row(row_name, doctype): +def get_returned_qty_map_for_row(return_against, supplier, row_name, doctype): child_doctype = doctype + " Item" reference_field = "dn_detail" if doctype == "Delivery Note" else frappe.scrub(child_doctype) @@ -226,9 +226,12 @@ def get_returned_qty_map_for_row(row_name, doctype): if doctype == "Purchase Receipt": fields += ["sum(abs(`tab{0}`.received_stock_qty)) as received_stock_qty".format(child_doctype)] + # Used retrun against and supplier and is_retrun because there is an index added for it data = frappe.db.get_list(doctype, fields = fields, filters = [ + [doctype, "return_against", "=", return_against], + [doctype, "supplier", "=", supplier], [doctype, "docstatus", "=", 1], [doctype, "is_return", "=", 1], [child_doctype, reference_field, "=", row_name] @@ -307,7 +310,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.serial_no = '\n'.join(serial_nos) if doctype == "Purchase Receipt": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.received_qty = -1 * flt(source_doc.received_qty - (returned_qty_map.get('received_qty') or 0)) target_doc.rejected_qty = -1 * flt(source_doc.rejected_qty - (returned_qty_map.get('rejected_qty') or 0)) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) @@ -321,7 +324,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.purchase_receipt_item = source_doc.name elif doctype == "Purchase Invoice": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.received_qty = -1 * flt(source_doc.received_qty - (returned_qty_map.get('received_qty') or 0)) target_doc.rejected_qty = -1 * flt(source_doc.rejected_qty - (returned_qty_map.get('rejected_qty') or 0)) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) @@ -335,7 +338,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.purchase_invoice_item = source_doc.name elif doctype == "Delivery Note": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) @@ -348,7 +351,7 @@ def make_return_doc(doctype, source_name, target_doc=None): if default_warehouse_for_sales_return: target_doc.warehouse = default_warehouse_for_sales_return elif doctype == "Sales Invoice" or doctype == "POS Invoice": - returned_qty_map = get_returned_qty_map_for_row(source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 2a4d63954a7..06439b64e4e 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -798,3 +798,6 @@ def make_inter_company_transaction(doctype, source_name, target_doc=None): }, target_doc, set_missing_values) return doclist + +def on_doctype_update(): + frappe.db.add_index("Delivery Note", ["customer", "is_return", "return_against"]) diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 33e40c85f1a..52f10ea9c83 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -926,3 +926,6 @@ def get_item_account_wise_additional_cost(purchase_document): account.base_amount * item.get(based_on_field) / total_item_cost return item_account_wise_cost + +def on_doctype_update(): + frappe.db.add_index("Purchase Receipt", ["supplier", "is_return", "return_against"]) From 5d66cc4c4a3a91af0d306f24c0b35f69a9b2966e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 12:22:37 +0530 Subject: [PATCH 04/25] fix: Add cost center in loan document --- erpnext/loan_management/doctype/loan/loan.json | 15 ++++++++++++++- erpnext/loan_management/doctype/loan/loan.py | 8 ++++++++ .../loan_interest_accrual.py | 7 ++++--- erpnext/patches.txt | 3 ++- .../patches/v13_0/add_cost_center_in_loans.py | 16 ++++++++++++++++ 5 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 erpnext/patches/v13_0/add_cost_center_in_loans.py diff --git a/erpnext/loan_management/doctype/loan/loan.json b/erpnext/loan_management/doctype/loan/loan.json index 196f36f0f46..ef78a640aa0 100644 --- a/erpnext/loan_management/doctype/loan/loan.json +++ b/erpnext/loan_management/doctype/loan/loan.json @@ -32,6 +32,8 @@ "monthly_repayment_amount", "repayment_start_date", "is_term_loan", + "accounting_dimensions_section", + "cost_center", "account_info", "mode_of_payment", "disbursement_account", @@ -366,12 +368,23 @@ "options": "Account", "read_only": 1, "reqd": 1 + }, + { + "fieldname": "accounting_dimensions_section", + "fieldtype": "Section Break", + "label": "Accounting Dimensions" + }, + { + "fieldname": "cost_center", + "fieldtype": "Link", + "label": "Cost Center", + "options": "Cost Center" } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-01-25 16:29:16.325501", + "modified": "2022-03-10 11:50:31.957360", "modified_by": "Administrator", "module": "Loan Management", "name": "Loan", diff --git a/erpnext/loan_management/doctype/loan/loan.py b/erpnext/loan_management/doctype/loan/loan.py index b798e088b4f..0fe9947472b 100644 --- a/erpnext/loan_management/doctype/loan/loan.py +++ b/erpnext/loan_management/doctype/loan/loan.py @@ -25,6 +25,7 @@ class Loan(AccountsController): self.set_loan_amount() self.validate_loan_amount() self.set_missing_fields() + self.validate_cost_center() self.validate_accounts() self.check_sanctioned_amount_limit() self.validate_repay_from_salary() @@ -45,6 +46,13 @@ class Loan(AccountsController): frappe.throw(_("Account {0} does not belongs to company {1}").format(frappe.bold(self.get(fieldname)), frappe.bold(self.company))) + def validate_cost_center(self): + if not self.cost_center and self.rate_of_interest != 0: + self.cost_center = frappe.db.get_value('Company', self.company, 'cost_center') + + if not self.cost_center: + frappe.throw(_('Cost center is mandatory for loans having rate of interest greater than 0')) + def on_submit(self): self.link_loan_security_pledge() diff --git a/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py b/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py index 1c800a06da0..f6a3ededcbe 100644 --- a/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py +++ b/erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py @@ -6,7 +6,6 @@ import frappe from frappe import _ from frappe.utils import add_days, cint, date_diff, flt, get_datetime, getdate, nowdate -import erpnext from erpnext.accounts.general_ledger import make_gl_entries from erpnext.controllers.accounts_controller import AccountsController @@ -41,6 +40,8 @@ class LoanInterestAccrual(AccountsController): def make_gl_entries(self, cancel=0, adv_adj=0): gle_map = [] + cost_center = frappe.db.get_value('Loan', self.loan, 'cost_center') + if self.interest_amount: gle_map.append( self.get_gl_dict({ @@ -54,7 +55,7 @@ class LoanInterestAccrual(AccountsController): "against_voucher": self.loan, "remarks": _("Interest accrued from {0} to {1} against loan: {2}").format( self.last_accrual_date, self.posting_date, self.loan), - "cost_center": erpnext.get_default_cost_center(self.company), + "cost_center": cost_center, "posting_date": self.posting_date }) ) @@ -69,7 +70,7 @@ class LoanInterestAccrual(AccountsController): "against_voucher": self.loan, "remarks": ("Interest accrued from {0} to {1} against loan: {2}").format( self.last_accrual_date, self.posting_date, self.loan), - "cost_center": erpnext.get_default_cost_center(self.company), + "cost_center": cost_center, "posting_date": self.posting_date }) ) diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 13f0e7b872e..ebda8058e02 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -357,4 +357,5 @@ erpnext.patches.v13_0.set_work_order_qty_in_so_from_mr erpnext.patches.v13_0.update_accounts_in_loan_docs erpnext.patches.v14_0.update_batch_valuation_flag erpnext.patches.v14_0.delete_non_profit_doctypes -erpnext.patches.v14_0.update_employee_advance_status \ No newline at end of file +erpnext.patches.v14_0.update_employee_advance_status +erpnext.patches.v13_0.add_cost_center_in_loans \ No newline at end of file diff --git a/erpnext/patches/v13_0/add_cost_center_in_loans.py b/erpnext/patches/v13_0/add_cost_center_in_loans.py new file mode 100644 index 00000000000..25e1722a4ff --- /dev/null +++ b/erpnext/patches/v13_0/add_cost_center_in_loans.py @@ -0,0 +1,16 @@ +import frappe + + +def execute(): + frappe.reload_doc('loan_management', 'doctype', 'loan') + loan = frappe.qb.DocType('Loan') + + for company in frappe.get_all('Company', pluck='name'): + default_cost_center = frappe.db.get_value('Company', company, 'cost_center') + frappe.qb.update( + loan + ).set( + loan.cost_center, default_cost_center + ).where( + loan.company == company + ).run() \ No newline at end of file From e9d458b822e7436632d0d53fdc4a068267876a0a Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 12:29:17 +0530 Subject: [PATCH 05/25] fix: Update party type --- erpnext/controllers/sales_and_purchase_return.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 2b094e3626c..21666336e03 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -208,10 +208,15 @@ def get_already_returned_items(doc): return items -def get_returned_qty_map_for_row(return_against, supplier, row_name, doctype): +def get_returned_qty_map_for_row(return_against, party, row_name, doctype): child_doctype = doctype + " Item" reference_field = "dn_detail" if doctype == "Delivery Note" else frappe.scrub(child_doctype) + if doctype in ('Purchase Receipt', 'Purchase Invoice'): + party_type = 'supplier' + else: + party_type = 'customer' + fields = [ "sum(abs(`tab{0}`.qty)) as qty".format(child_doctype), "sum(abs(`tab{0}`.stock_qty)) as stock_qty".format(child_doctype) @@ -231,7 +236,7 @@ def get_returned_qty_map_for_row(return_against, supplier, row_name, doctype): fields = fields, filters = [ [doctype, "return_against", "=", return_against], - [doctype, "supplier", "=", supplier], + [doctype, party_type, "=", party], [doctype, "docstatus", "=", 1], [doctype, "is_return", "=", 1], [child_doctype, reference_field, "=", row_name] @@ -338,7 +343,7 @@ def make_return_doc(doctype, source_name, target_doc=None): target_doc.purchase_invoice_item = source_doc.name elif doctype == "Delivery Note": - returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.customer, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) @@ -351,7 +356,7 @@ def make_return_doc(doctype, source_name, target_doc=None): if default_warehouse_for_sales_return: target_doc.warehouse = default_warehouse_for_sales_return elif doctype == "Sales Invoice" or doctype == "POS Invoice": - returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.supplier, source_doc.name, doctype) + returned_qty_map = get_returned_qty_map_for_row(source_parent.name, source_parent.customer, source_doc.name, doctype) target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get('qty') or 0)) target_doc.stock_qty = -1 * flt(source_doc.stock_qty - (returned_qty_map.get('stock_qty') or 0)) From a5befb6bf82546deaf9ea21da2eb828aca1bb51e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 14:18:54 +0530 Subject: [PATCH 06/25] fix: Update modified timestamp --- erpnext/stock/doctype/delivery_note/delivery_note.json | 2 +- erpnext/stock/doctype/purchase_receipt/purchase_receipt.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.json b/erpnext/stock/doctype/delivery_note/delivery_note.json index 55a4c956a67..7ebc4eed751 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.json +++ b/erpnext/stock/doctype/delivery_note/delivery_note.json @@ -1315,7 +1315,7 @@ "idx": 146, "is_submittable": 1, "links": [], - "modified": "2021-10-09 14:29:13.428984", + "modified": "2022-03-10 14:29:13.428984", "modified_by": "Administrator", "module": "Stock", "name": "Delivery Note", diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json index b54a90eed35..6d4b4a19bd2 100755 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.json @@ -1165,7 +1165,7 @@ "idx": 261, "is_submittable": 1, "links": [], - "modified": "2022-02-01 11:40:52.690984", + "modified": "2022-03-10 11:40:52.690984", "modified_by": "Administrator", "module": "Stock", "name": "Purchase Receipt", From 6794148c04d6cfdb3ac9b4df38e0957f35334c5a Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Thu, 10 Mar 2022 10:17:29 +0100 Subject: [PATCH 07/25] fix(lead): reload contact before updating links (#29966) * fix(lead): reload contact before updading links Contact might have changed since it was created. * refactor: reload contact after insert --- erpnext/crm/doctype/lead/lead.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/crm/doctype/lead/lead.py b/erpnext/crm/doctype/lead/lead.py index c31b068a43f..33ec5521522 100644 --- a/erpnext/crm/doctype/lead/lead.py +++ b/erpnext/crm/doctype/lead/lead.py @@ -214,6 +214,7 @@ class Lead(SellingController): }) contact.insert(ignore_permissions=True) + contact.reload() # load changes by hooks on contact return contact From 412645597567df133dd91a6cf44db93207575052 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 15:43:09 +0530 Subject: [PATCH 08/25] fix: dont reset UOM in MR on every get_item_detail call (#30164) --- erpnext/stock/doctype/material_request/material_request.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/stock/doctype/material_request/material_request.js b/erpnext/stock/doctype/material_request/material_request.js index 5f53be0869e..e68b0abfb94 100644 --- a/erpnext/stock/doctype/material_request/material_request.js +++ b/erpnext/stock/doctype/material_request/material_request.js @@ -214,6 +214,7 @@ frappe.ui.form.on('Material Request', { material_request_type: frm.doc.material_request_type, plc_conversion_rate: 1, rate: item.rate, + uom: item.uom, conversion_factor: item.conversion_factor }, overwrite_warehouse: overwrite_warehouse @@ -392,6 +393,7 @@ frappe.ui.form.on("Material Request Item", { item_code: function(frm, doctype, name) { const item = locals[doctype][name]; item.rate = 0; + item.uom = ''; set_schedule_date(frm); frm.events.get_item_data(frm, item, true); }, From b4d4ae6aa3f258357a974e0a76247b3e752bdbf2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 16:09:26 +0530 Subject: [PATCH 09/25] test: refactor item merge test and disable commits --- erpnext/stock/doctype/item/test_item.py | 19 ++++++++++--------- .../repost_item_valuation.py | 3 ++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index d7671b1d714..d57308b2afa 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -371,23 +371,24 @@ class TestItem(FrappeTestCase): variant.save() def test_item_merging(self): - create_item("Test Item for Merging 1") - create_item("Test Item for Merging 2") + old = create_item(frappe.generate_hash(length=20)).name + new = create_item(frappe.generate_hash(length=20)).name - make_stock_entry(item_code="Test Item for Merging 1", target="_Test Warehouse - _TC", + make_stock_entry(item_code=old, target="_Test Warehouse - _TC", qty=1, rate=100) - make_stock_entry(item_code="Test Item for Merging 2", target="_Test Warehouse 1 - _TC", + make_stock_entry(item_code=old, target="_Test Warehouse 1 - _TC", + qty=1, rate=100) + make_stock_entry(item_code=new, target="_Test Warehouse 1 - _TC", qty=1, rate=100) - frappe.rename_doc("Item", "Test Item for Merging 1", "Test Item for Merging 2", merge=True) + frappe.rename_doc("Item", old, new, merge=True) - self.assertFalse(frappe.db.exists("Item", "Test Item for Merging 1")) + self.assertFalse(frappe.db.exists("Item", old)) self.assertTrue(frappe.db.get_value("Bin", - {"item_code": "Test Item for Merging 2", "warehouse": "_Test Warehouse - _TC"})) - + {"item_code": new, "warehouse": "_Test Warehouse - _TC"})) self.assertTrue(frappe.db.get_value("Bin", - {"item_code": "Test Item for Merging 2", "warehouse": "_Test Warehouse 1 - _TC"})) + {"item_code": new, "warehouse": "_Test Warehouse 1 - _TC"})) def test_item_merging_with_product_bundle(self): from erpnext.selling.doctype.product_bundle.test_product_bundle import make_product_bundle 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 977d470995e..f4d52ad73d1 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -118,7 +118,8 @@ def repost(doc): doc.set_status('Failed') raise finally: - frappe.db.commit() + if not frappe.flags.in_test: + frappe.db.commit() def repost_sl_entries(doc): if doc.based_on == 'Transaction': From 73901aad6f88c06cfb6dab8da133fba4175bd692 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 16:30:29 +0530 Subject: [PATCH 10/25] fix: handle duplicate bins during item merge renames --- erpnext/stock/doctype/item/item.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 32c72fd2f64..8ede95539b3 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -392,6 +392,7 @@ class Item(Document): self.validate_properties_before_merge(new_name) self.validate_duplicate_product_bundles_before_merge(old_name, new_name) self.validate_duplicate_website_item_before_merge(old_name, new_name) + self.delete_old_bins(old_name) def after_rename(self, old_name, new_name, merge): if merge: @@ -420,6 +421,9 @@ class Item(Document): frappe.db.set_value(dt, d.name, "item_wise_tax_detail", json.dumps(item_wise_tax_detail), update_modified=False) + def delete_old_bins(self, old_name): + frappe.db.delete("Bin", {"item_code": old_name}) + def validate_duplicate_item_in_stock_reconciliation(self, old_name, new_name): records = frappe.db.sql(""" SELECT parent, COUNT(*) as records FROM `tabStock Reconciliation Item` @@ -500,11 +504,11 @@ class Item(Document): existing_allow_negative_stock = frappe.db.get_value("Stock Settings", None, "allow_negative_stock") frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1) - repost_stock_for_warehouses = frappe.db.sql_list("""select distinct warehouse - from tabBin where item_code=%s""", new_name) + repost_stock_for_warehouses = frappe.get_all("Stock Ledger Entry", + "warehouse", filters={"item_code": new_name}, pluck="warehouse", distinct=True) # Delete all existing bins to avoid duplicate bins for the same item and warehouse - frappe.db.sql("delete from `tabBin` where item_code=%s", new_name) + frappe.db.delete("Bin", {"item_code": new_name}) for warehouse in repost_stock_for_warehouses: repost_stock(new_name, warehouse) From 18e2a33a9be4d4efea13ce5711343413b31358b8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 16:44:05 +0530 Subject: [PATCH 11/25] fix: fetch new fields in bom from routing --- erpnext/manufacturing/doctype/bom/bom.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 37d2b9ff978..c0fb63f8280 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -192,7 +192,8 @@ class BOM(WebsiteGenerator): if self.routing: self.set("operations", []) fields = ["sequence_id", "operation", "workstation", "description", - "time_in_mins", "batch_size", "operating_cost", "idx", "hour_rate"] + "time_in_mins", "batch_size", "operating_cost", "idx", "hour_rate", + "set_cost_based_on_bom_qty", "fixed_time"] for row in frappe.get_all("BOM Operation", fields = fields, filters = {'parenttype': 'Routing', 'parent': self.routing}, order_by="sequence_id, idx"): From 362102e802bb501312a39bb21810336de26696b9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 16:49:29 +0530 Subject: [PATCH 12/25] fix: dont hardcode hour rate precision --- erpnext/manufacturing/doctype/bom/bom.py | 2 +- .../manufacturing/doctype/bom_operation/bom_operation.json | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index c0fb63f8280..a8ce1d7642c 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -198,7 +198,7 @@ class BOM(WebsiteGenerator): for row in frappe.get_all("BOM Operation", fields = fields, filters = {'parenttype': 'Routing', 'parent': self.routing}, order_by="sequence_id, idx"): child = self.append('operations', row) - child.hour_rate = flt(row.hour_rate / self.conversion_rate, 2) + child.hour_rate = flt(row.hour_rate / self.conversion_rate, child.precision("hour_rate")) def set_bom_material_details(self): for item in self.get("items"): diff --git a/erpnext/manufacturing/doctype/bom_operation/bom_operation.json b/erpnext/manufacturing/doctype/bom_operation/bom_operation.json index c7be7efc9e1..341f9692c45 100644 --- a/erpnext/manufacturing/doctype/bom_operation/bom_operation.json +++ b/erpnext/manufacturing/doctype/bom_operation/bom_operation.json @@ -66,7 +66,8 @@ "label": "Hour Rate", "oldfieldname": "hour_rate", "oldfieldtype": "Currency", - "options": "currency" + "options": "currency", + "precision": "2" }, { "description": "In minutes", @@ -186,7 +187,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2021-12-15 03:00:00.473173", + "modified": "2022-03-10 06:19:08.462027", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM Operation", From 7dd10367f49b9f67def80aa0daa612848f00092c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 17:07:57 +0530 Subject: [PATCH 13/25] fix: only update valuation rate if not None --- erpnext/stock/stock_ledger.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index ba1081f4dce..353bfa452b6 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -838,11 +838,13 @@ class update_entries_after(object): for warehouse, data in self.data.items(): bin_name = get_or_make_bin(self.item_code, warehouse) - frappe.db.set_value('Bin', bin_name, { - "valuation_rate": data.valuation_rate, + updated_values = { "actual_qty": data.qty_after_transaction, "stock_value": data.stock_value - }) + } + if data.valuation_rate is not None: + updated_values["valuation_rate"] = data.valuation_rate + frappe.db.set_value('Bin', bin_name, updated_values) def get_previous_sle_of_current_voucher(args, exclude_current_voucher=False): From 6c54be0dcd83cc021844017f2ceeed58a88ca920 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 17:37:29 +0530 Subject: [PATCH 14/25] test: flaky MR report test all test records are on same day so sorting was random in report rows. --- ...test_requested_items_to_order_and_receive.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/erpnext/buying/report/requested_items_to_order_and_receive/test_requested_items_to_order_and_receive.py b/erpnext/buying/report/requested_items_to_order_and_receive/test_requested_items_to_order_and_receive.py index f3c751c5c3c..a533da00e3a 100644 --- a/erpnext/buying/report/requested_items_to_order_and_receive/test_requested_items_to_order_and_receive.py +++ b/erpnext/buying/report/requested_items_to_order_and_receive/test_requested_items_to_order_and_receive.py @@ -17,8 +17,8 @@ class TestRequestedItemsToOrderAndReceive(FrappeTestCase): def setUp(self) -> None: create_item("Test MR Report Item") self.setup_material_request() # to order and receive - self.setup_material_request(order=True) # to receive (ordered) - self.setup_material_request(order=True, receive=True) # complete (ordered & received) + self.setup_material_request(order=True, days=1) # to receive (ordered) + self.setup_material_request(order=True, receive=True, days=2) # complete (ordered & received) self.filters = frappe._dict( company="_Test Company", from_date=today(), to_date=add_days(today(), 30), @@ -32,9 +32,8 @@ class TestRequestedItemsToOrderAndReceive(FrappeTestCase): data = get_data(self.filters) self.assertEqual(len(data), 2) # MRs today should be fetched - self.filters.from_date = add_days(today(), 1) - data = get_data(self.filters) - self.assertEqual(len(data), 0) # MRs today should not be fetched as from date is tomorrow + data = get_data(self.filters.update({"from_date": add_days(today(), 10)})) + self.assertEqual(len(data), 0) # MRs today should not be fetched as from date is in future def test_ordered_received_material_requests(self): data = get_data(self.filters) @@ -44,19 +43,19 @@ class TestRequestedItemsToOrderAndReceive(FrappeTestCase): self.assertEqual(data[0].ordered_qty, 0.0) self.assertEqual(data[1].ordered_qty, 57.0) - def setup_material_request(self, order=False, receive=False): + def setup_material_request(self, order=False, receive=False, days=0): po = None test_records = frappe.get_test_records('Material Request') mr = frappe.copy_doc(test_records[0]) - mr.transaction_date = today() - mr.schedule_date = add_days(today(), 1) + mr.transaction_date = add_days(today(), days) + mr.schedule_date = add_days(mr.transaction_date, 1) for row in mr.items: row.item_code = "Test MR Report Item" row.item_name = "Test MR Report Item" row.description = "Test MR Report Item" row.uom = "Nos" - row.schedule_date = add_days(today(), 1) + row.schedule_date = mr.schedule_date mr.submit() if order or receive: From 472fe8e3191764d281b54c0c49c83a88b26deed9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 10 Mar 2022 17:39:55 +0530 Subject: [PATCH 15/25] test: submit PR directly --- .../stock/doctype/purchase_receipt/test_purchase_receipt.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index fa28f2252de..0017fa7ee11 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -769,8 +769,7 @@ class TestPurchaseReceipt(FrappeTestCase): update_purchase_receipt_status, ) - pr = make_purchase_receipt(do_not_submit=True) - pr.submit() + pr = make_purchase_receipt() update_purchase_receipt_status(pr.name, "Closed") self.assertEqual( From d596e0e4dff86cf8cde640bd18a54ee159276a4c Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Mar 2022 20:56:36 +0530 Subject: [PATCH 16/25] fix: Shipping rule application fixes --- .../doctype/shipping_rule/shipping_rule.py | 3 +-- erpnext/controllers/taxes_and_totals.py | 5 ++++- .../public/js/controllers/taxes_and_totals.js | 4 +++- .../doctype/sales_order/test_sales_order.py | 22 +++++++++++++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/shipping_rule/shipping_rule.py b/erpnext/accounts/doctype/shipping_rule/shipping_rule.py index 792e7d21a78..7e5129911e4 100644 --- a/erpnext/accounts/doctype/shipping_rule/shipping_rule.py +++ b/erpnext/accounts/doctype/shipping_rule/shipping_rule.py @@ -71,8 +71,7 @@ class ShippingRule(Document): if doc.currency != doc.company_currency: shipping_amount = flt(shipping_amount / doc.conversion_rate, 2) - if shipping_amount: - self.add_shipping_rule_to_tax_table(doc, shipping_amount) + self.add_shipping_rule_to_tax_table(doc, shipping_amount) def get_shipping_amount_from_rules(self, value): for condition in self.get("conditions"): diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index a1bb6670c42..d362cdde110 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -37,6 +37,8 @@ class calculate_taxes_and_totals(object): self.set_discount_amount() self.apply_discount_amount() + self.calculate_shipping_charges() + if self.doc.doctype in ["Sales Invoice", "Purchase Invoice"]: self.calculate_total_advance() @@ -50,7 +52,6 @@ class calculate_taxes_and_totals(object): self.initialize_taxes() self.determine_exclusive_rate() self.calculate_net_total() - self.calculate_shipping_charges() self.calculate_taxes() self.manipulate_grand_total_for_inclusive_tax() self.calculate_totals() @@ -276,6 +277,8 @@ class calculate_taxes_and_totals(object): shipping_rule = frappe.get_doc("Shipping Rule", self.doc.shipping_rule) shipping_rule.apply(self.doc) + self._calculate() + def calculate_taxes(self): rounding_adjustment_computed = self.doc.get('is_consolidated') and self.doc.get('rounding_adjustment') if not rounding_adjustment_computed: diff --git a/erpnext/public/js/controllers/taxes_and_totals.js b/erpnext/public/js/controllers/taxes_and_totals.js index ae0e2a3f6f8..9fec43b74ef 100644 --- a/erpnext/public/js/controllers/taxes_and_totals.js +++ b/erpnext/public/js/controllers/taxes_and_totals.js @@ -39,6 +39,8 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { this._calculate_taxes_and_totals(); this.calculate_discount_amount(); + this.calculate_shipping_charges(); + // Advance calculation applicable to Sales /Purchase Invoice if(in_list(["Sales Invoice", "POS Invoice", "Purchase Invoice"], this.frm.doc.doctype) && this.frm.doc.docstatus < 2 && !this.frm.doc.is_return) { @@ -81,7 +83,6 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { this.initialize_taxes(); this.determine_exclusive_rate(); this.calculate_net_total(); - this.calculate_shipping_charges(); this.calculate_taxes(); this.manipulate_grand_total_for_inclusive_tax(); this.calculate_totals(); @@ -275,6 +276,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { frappe.model.round_floats_in(this.frm.doc, ["total", "base_total", "net_total", "base_net_total"]); if (frappe.meta.get_docfield(this.frm.doc.doctype, "shipping_rule", this.frm.doc.name)) { this.shipping_rule(); + this._calculate_taxes_and_totals(); } } diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index b5284793e13..86a08828b26 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -1477,6 +1477,28 @@ class TestSalesOrder(FrappeTestCase): self.assertEqual(so.items[0].work_order_qty, wo.produced_qty) self.assertEqual(mr.status, "Manufactured") + def test_sales_order_with_shipping_rule(self): + from erpnext.accounts.doctype.shipping_rule.test_shipping_rule import create_shipping_rule + shipping_rule = create_shipping_rule(shipping_rule_type = "Selling", shipping_rule_name = "Shipping Rule - Sales Invoice Test") + sales_order = make_sales_order(do_not_save=True) + sales_order.shipping_rule = shipping_rule.name + + sales_order.items[0].qty = 1 + sales_order.save() + self.assertEqual(sales_order.taxes[0].tax_amount, 50) + + sales_order.items[0].qty = 2 + sales_order.save() + self.assertEqual(sales_order.taxes[0].tax_amount, 100) + + sales_order.items[0].qty = 3 + sales_order.save() + self.assertEqual(sales_order.taxes[0].tax_amount, 200) + + sales_order.items[0].qty = 21 + sales_order.save() + self.assertEqual(sales_order.taxes[0].tax_amount, 0) + def automatically_fetch_payment_terms(enable=1): accounts_settings = frappe.get_doc("Accounts Settings") accounts_settings.automatically_fetch_payment_terms = enable From d9c91748f4e8eb37ec1c76ee5718c38de8ce9f65 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Tue, 15 Feb 2022 16:35:04 +0530 Subject: [PATCH 17/25] fix: if an item code is too long, truncate before setting BOM name --- erpnext/manufacturing/doctype/bom/bom.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index a8ce1d7642c..3c325037e52 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -121,7 +121,21 @@ class BOM(WebsiteGenerator): else: idx = 1 - name = 'BOM-' + self.item + ('-%.3i' % idx) + prefix = self.doctype + suffix = "%.3i" % idx + bom_name = prefix + "-" + self.item + "-" + suffix + + if len(bom_name) <= 140: + name = bom_name + else: + # since max characters for name is 140, remove enough characters from the + # item name to fit the prefix, suffix and the separators + truncated_length = 140 - (len(prefix) + len(suffix) + 2) + truncated_item_name = self.item[:truncated_length] + # if a partial word is found after truncate, remove the extra characters + truncated_item_name = truncated_item_name.rsplit(" ", 1)[0] + name = prefix + "-" + truncated_item_name + "-" + suffix + if frappe.db.exists("BOM", name): conflicting_bom = frappe.get_doc("BOM", name) From e2c99e02a95a87021786a0666e97e174a3f65a44 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 16 Feb 2022 12:35:34 +0530 Subject: [PATCH 18/25] test: bom for item_code that is >VARCHAR_LEN --- erpnext/manufacturing/doctype/bom/test_bom.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 3cc91b341c6..e472388d368 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -432,6 +432,15 @@ class TestBOM(FrappeTestCase): self.assertEqual(bom.transfer_material_against, "Work Order") bom.delete() + def test_bom_name_length(self): + """ test >140 char names""" + bom_tree = { + "x" * 140 : { + " ".join(["abc"] * 35): {} + } + } + create_nested_bom(bom_tree, prefix="") + def get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) From 7f2670941ca7f2e41a37a8ea3ed186a0fa04b57c Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 16 Feb 2022 15:55:25 +0530 Subject: [PATCH 19/25] fix: improve bom autoname logic --- erpnext/manufacturing/doctype/bom/bom.py | 38 ++++++++++++++---------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 3c325037e52..2c342c0d691 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -2,6 +2,7 @@ # License: GNU General Public License v3. See license.txt import functools +import re from collections import deque from operator import itemgetter from typing import List @@ -103,27 +104,34 @@ class BOM(WebsiteGenerator): ) def autoname(self): - names = frappe.db.sql_list("""select name from `tabBOM` where item=%s""", self.item) + existing_boms = frappe.get_all("BOM", filters={"item": self.item}) + if existing_boms: + existing_bom_names = [bom.name for bom in existing_boms] - if names: - # name can be BOM/ITEM/001, BOM/ITEM/001-1, BOM-ITEM-001, BOM-ITEM-001-1 + # split by "/" and "-" + delimiters = ["/", "-"] + pattern = "|".join(map(re.escape, delimiters)) + bom_parts = [re.split(pattern, bom_name) for bom_name in existing_bom_names] - # split by item - names = [name.split(self.item, 1) for name in names] - names = [d[-1][1:] for d in filter(lambda x: len(x) > 1 and x[-1], names)] + # filter out BOMs that do not follow the following formats: + # - BOM/ITEM/001 + # - BOM/ITEM/001-1 + # - BOM-ITEM-001 + # - BOM-ITEM-001-1 + valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) - # split by (-) if cancelled - if names: - names = [cint(name.split('-')[-1]) for name in names] - idx = max(names) + 1 + # extract the current index from the BOM parts + if valid_bom_parts: + indexes = [cint(part[-1]) for part in valid_bom_parts] + index = max(indexes) + 1 else: - idx = 1 + index = 1 else: - idx = 1 + index = 1 prefix = self.doctype - suffix = "%.3i" % idx - bom_name = prefix + "-" + self.item + "-" + suffix + suffix = "%.3i" % index # convert index to string (1 -> "001") + bom_name = f"{prefix}-{self.item}-{suffix}" if len(bom_name) <= 140: name = bom_name @@ -134,7 +142,7 @@ class BOM(WebsiteGenerator): truncated_item_name = self.item[:truncated_length] # if a partial word is found after truncate, remove the extra characters truncated_item_name = truncated_item_name.rsplit(" ", 1)[0] - name = prefix + "-" + truncated_item_name + "-" + suffix + name = f"{prefix}-{truncated_item_name}-{suffix}" if frappe.db.exists("BOM", name): conflicting_bom = frappe.get_doc("BOM", name) From 6b58d534030df956f9983c003741ad69263aa287 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Feb 2022 11:36:44 +0530 Subject: [PATCH 20/25] refactor: split versioning code for testability --- erpnext/manufacturing/doctype/bom/bom.py | 46 ++++++++++--------- erpnext/manufacturing/doctype/bom/test_bom.py | 21 +++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 2c342c0d691..817b8e986e8 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -104,28 +104,9 @@ class BOM(WebsiteGenerator): ) def autoname(self): - existing_boms = frappe.get_all("BOM", filters={"item": self.item}) + existing_boms = frappe.get_all("BOM", filters={"item": self.item}, pluck="name") if existing_boms: - existing_bom_names = [bom.name for bom in existing_boms] - - # split by "/" and "-" - delimiters = ["/", "-"] - pattern = "|".join(map(re.escape, delimiters)) - bom_parts = [re.split(pattern, bom_name) for bom_name in existing_bom_names] - - # filter out BOMs that do not follow the following formats: - # - BOM/ITEM/001 - # - BOM/ITEM/001-1 - # - BOM-ITEM-001 - # - BOM-ITEM-001-1 - valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) - - # extract the current index from the BOM parts - if valid_bom_parts: - indexes = [cint(part[-1]) for part in valid_bom_parts] - index = max(indexes) + 1 - else: - index = 1 + index = self.get_next_version_index(existing_boms) else: index = 1 @@ -156,6 +137,29 @@ class BOM(WebsiteGenerator): self.name = name + @staticmethod + def get_next_version_index(existing_boms: List[str]) -> int: + # split by "/" and "-" + delimiters = ["/", "-"] + pattern = "|".join(map(re.escape, delimiters)) + bom_parts = [re.split(pattern, bom_name) for bom_name in existing_boms] + + # filter out BOMs that do not follow the following formats: + # - BOM/ITEM/001 + # - BOM/ITEM/001-1 + # - BOM-ITEM-001 + # - BOM-ITEM-001-1 + valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) + + # extract the current index from the BOM parts + if valid_bom_parts: + indexes = [cint(part[-1]) for part in valid_bom_parts] + index = max(indexes) + 1 + else: + index = 1 + + return index + def validate(self): self.route = frappe.scrub(self.name).replace('_', '-') diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index e472388d368..346870d1d1c 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -441,6 +441,27 @@ class TestBOM(FrappeTestCase): } create_nested_bom(bom_tree, prefix="") + def test_version_index(self): + + bom = frappe.new_doc("BOM") + + version_index_test_cases = [ + (1, []), + (1, ["BOM#XYZ"]), + (2, ["BOM/ITEM/001"]), + (2, ["BOM/ITEM/001", "BOM/ITEM/001-1"]), + (2, ["BOM-ITEM-001",]), + (2, ["BOM-ITEM-001", "BOM-ITEM-001-1"]), + (3, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-001-1"]), + (4, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-003"]), + (2, ["BOM-ITEM-001", "BOM-ITEM-001-1", "BOM-ITEM-001-2"]), + ] + + for expected_index, existing_boms in version_index_test_cases: + with self.subTest(): + self.assertEqual(expected_index, bom.get_next_version_index(existing_boms), + msg=f"Incorrect index for {existing_boms}") + def get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) From 67d8a7ba86c3131b19b4077055524f69d334314e Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Wed, 2 Mar 2022 15:25:06 +0530 Subject: [PATCH 21/25] fix: cancelled document check --- erpnext/manufacturing/doctype/bom/bom.py | 15 +++++++++------ erpnext/manufacturing/doctype/bom/test_bom.py | 7 ++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 817b8e986e8..a025ff740c6 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -104,7 +104,13 @@ class BOM(WebsiteGenerator): ) def autoname(self): - existing_boms = frappe.get_all("BOM", filters={"item": self.item}, pluck="name") + # ignore amended documents while calculating current index + existing_boms = frappe.get_all( + "BOM", + filters={"item": self.item, "amended_from": ["is", "not set"]}, + pluck="name" + ) + if existing_boms: index = self.get_next_version_index(existing_boms) else: @@ -144,15 +150,12 @@ class BOM(WebsiteGenerator): pattern = "|".join(map(re.escape, delimiters)) bom_parts = [re.split(pattern, bom_name) for bom_name in existing_boms] - # filter out BOMs that do not follow the following formats: - # - BOM/ITEM/001 - # - BOM/ITEM/001-1 - # - BOM-ITEM-001 - # - BOM-ITEM-001-1 + # filter out BOMs that do not follow the following formats: BOM/ITEM/001, BOM-ITEM-001 valid_bom_parts = list(filter(lambda x: len(x) > 1 and x[-1], bom_parts)) # extract the current index from the BOM parts if valid_bom_parts: + # handle cancelled and submitted documents indexes = [cint(part[-1]) for part in valid_bom_parts] index = max(indexes) + 1 else: diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 346870d1d1c..6f9dff454e2 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -449,12 +449,9 @@ class TestBOM(FrappeTestCase): (1, []), (1, ["BOM#XYZ"]), (2, ["BOM/ITEM/001"]), - (2, ["BOM/ITEM/001", "BOM/ITEM/001-1"]), - (2, ["BOM-ITEM-001",]), - (2, ["BOM-ITEM-001", "BOM-ITEM-001-1"]), - (3, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-001-1"]), + (2, ["BOM-ITEM-001"]), + (3, ["BOM-ITEM-001", "BOM-ITEM-002"]), (4, ["BOM-ITEM-001", "BOM-ITEM-002", "BOM-ITEM-003"]), - (2, ["BOM-ITEM-001", "BOM-ITEM-001-1", "BOM-ITEM-001-2"]), ] for expected_index, existing_boms in version_index_test_cases: From 94d0f8d4e79aa87d66f73bc7056cf5faf9114588 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 16:48:53 +0530 Subject: [PATCH 22/25] test: actual bom naming test --- erpnext/manufacturing/doctype/bom/test_bom.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 6f9dff454e2..4417123178c 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -459,6 +459,42 @@ class TestBOM(FrappeTestCase): self.assertEqual(expected_index, bom.get_next_version_index(existing_boms), msg=f"Incorrect index for {existing_boms}") + def test_bom_versioning(self): + bom_tree = { + frappe.generate_hash(length=10) : { + frappe.generate_hash(length=10): {} + } + } + bom = create_nested_bom(bom_tree, prefix="") + self.assertEqual(int(bom.name.split("-")[-1]), 1) + original_bom_name = bom.name + + bom.cancel() + bom.reload() + self.assertEqual(bom.name, original_bom_name) + + # create a new amendment + amendment = frappe.copy_doc(bom) + amendment.docstatus = 0 + amendment.amended_from = bom.name + + amendment.save() + amendment.submit() + amendment.reload() + + self.assertNotEqual(amendment.name, bom.name) + # `origname-001-1` version + self.assertEqual(int(amendment.name.split("-")[-1]), 1) + self.assertEqual(int(amendment.name.split("-")[-2]), 1) + + # create a new version + version = frappe.copy_doc(amendment) + version.docstatus = 0 + version.amended_from = None + version.save() + self.assertNotEqual(amendment.name, version.name) + self.assertEqual(int(version.name.split("-")[-1]), 2) + def get_default_bom(item_code="_Test FG Item 2"): return frappe.db.get_value("BOM", {"item": item_code, "is_active": 1, "is_default": 1}) From b085e96a1265aebddb895ea4a5e8df6b5f9e72bc Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 11 Mar 2022 18:28:50 +0530 Subject: [PATCH 23/25] revert: "Merge pull request #29290 from s-aga-r/fix/delivery-note/billed-amount" (#29782) (#29807) * Revert "Merge pull request #29290 from s-aga-r/fix/delivery-note/billed-amount" This reverts commit 038f94955006c88209f9df28e3a785c59a4ddb28, reversing changes made to c7b491843476bca89be02851ccafb7e409876609. * fix: linter (cherry picked from commit 7fa46f77e0bdbc516b3c0cb0fb20594ee7fa398b) # Conflicts: # erpnext/patches.txt Co-authored-by: Sagar Sharma --- .../doctype/sales_invoice/sales_invoice.py | 6 +++--- .../stock/doctype/delivery_note/delivery_note.py | 16 ++++------------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 573da276a2a..862ac81ff38 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -1255,14 +1255,14 @@ class SalesInvoice(SellingController): def update_billing_status_in_dn(self, update_modified=True): updated_delivery_notes = [] for d in self.get("items"): - if d.so_detail: - updated_delivery_notes += update_billed_amount_based_on_so(d.so_detail, update_modified) - elif d.dn_detail: + if d.dn_detail: billed_amt = frappe.db.sql("""select sum(amount) from `tabSales Invoice Item` where dn_detail=%s and docstatus=1""", d.dn_detail) billed_amt = billed_amt and billed_amt[0][0] or 0 frappe.db.set_value("Delivery Note Item", d.dn_detail, "billed_amt", billed_amt, update_modified=update_modified) updated_delivery_notes.append(d.delivery_note) + elif d.so_detail: + updated_delivery_notes += update_billed_amount_based_on_so(d.so_detail, update_modified) for dn in set(updated_delivery_notes): frappe.get_doc("Delivery Note", dn).update_billing_percentage(update_modified=update_modified) diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 2a4d63954a7..fbcc8038aa0 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -342,25 +342,21 @@ def update_billed_amount_based_on_so(so_detail, update_modified=True): from frappe.query_builder.functions import Sum # Billed against Sales Order directly - si = frappe.qb.DocType("Sales Invoice").as_("si") si_item = frappe.qb.DocType("Sales Invoice Item").as_("si_item") sum_amount = Sum(si_item.amount).as_("amount") - billed_against_so = frappe.qb.from_(si).from_(si_item).select(sum_amount).where( - (si_item.parent == si.name) & + billed_against_so = frappe.qb.from_(si_item).select(sum_amount).where( (si_item.so_detail == so_detail) & ((si_item.dn_detail.isnull()) | (si_item.dn_detail == '')) & - (si_item.docstatus == 1) & - (si.update_stock == 0) + (si_item.docstatus == 1) ).run() billed_against_so = billed_against_so and billed_against_so[0][0] or 0 # Get all Delivery Note Item rows against the Sales Order Item row - dn = frappe.qb.DocType("Delivery Note").as_("dn") dn_item = frappe.qb.DocType("Delivery Note Item").as_("dn_item") - dn_details = frappe.qb.from_(dn).from_(dn_item).select(dn_item.name, dn_item.amount, dn_item.si_detail, dn_item.parent, dn_item.stock_qty, dn_item.returned_qty).where( + dn_details = frappe.qb.from_(dn).from_(dn_item).select(dn_item.name, dn_item.amount, dn_item.si_detail, dn_item.parent).where( (dn.name == dn_item.parent) & (dn_item.so_detail == so_detail) & (dn.docstatus == 1) & @@ -385,11 +381,7 @@ def update_billed_amount_based_on_so(so_detail, update_modified=True): # Distribute billed amount directly against SO between DNs based on FIFO if billed_against_so and billed_amt_agianst_dn < dnd.amount: - if dnd.returned_qty: - pending_to_bill = flt(dnd.amount) * (dnd.stock_qty - dnd.returned_qty) / dnd.stock_qty - else: - pending_to_bill = flt(dnd.amount) - pending_to_bill -= billed_amt_agianst_dn + pending_to_bill = flt(dnd.amount) - billed_amt_agianst_dn if pending_to_bill <= billed_against_so: billed_amt_agianst_dn += pending_to_bill billed_against_so -= pending_to_bill From 1c37d2711abebe7db43c89c49869ce63e533e2a7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 19:01:26 +0530 Subject: [PATCH 24/25] test: standalone SI creates and attaches serial nos --- .../accounts/doctype/sales_invoice/test_sales_invoice.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 6d929e4386f..271e02a50c6 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -2615,6 +2615,14 @@ class TestSalesInvoice(unittest.TestCase): frappe.db.set_value('Accounts Settings', None, 'acc_frozen_upto', None) + def test_standalone_serial_no_return(self): + si = create_sales_invoice(item_code="_Test Serialized Item With Series", update_stock=True, is_return=True, qty=-1) + si.submit() + self.assertTrue(si.items[0].serial_no) + + return si + + def get_sales_invoice_for_e_invoice(): si = make_sales_invoice_for_ewaybill() si.naming_series = 'INV-2020-.#####' From 1a256c62c422c518ba074cec6b275482d8de7d38 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 11 Mar 2022 19:06:07 +0530 Subject: [PATCH 25/25] fix: attach sr no si standalone credit note --- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 9 ++++++++- .../accounts/doctype/sales_invoice/test_sales_invoice.py | 4 +--- erpnext/stock/doctype/delivery_note/delivery_note.py | 8 +++++++- .../stock/doctype/delivery_note/test_delivery_note.py | 5 +++++ erpnext/stock/doctype/serial_no/serial_no.py | 7 +++++-- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 862ac81ff38..54217fba83a 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -42,7 +42,11 @@ from erpnext.projects.doctype.timesheet.timesheet import get_projectwise_timeshe from erpnext.setup.doctype.company.company import update_company_current_month_sales from erpnext.stock.doctype.batch.batch import set_batch_nos from erpnext.stock.doctype.delivery_note.delivery_note import update_billed_amount_based_on_so -from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no, get_serial_nos +from erpnext.stock.doctype.serial_no.serial_no import ( + get_delivery_note_serial_no, + get_serial_nos, + update_serial_nos_after_submit, +) from erpnext.stock.utils import calculate_mapped_packed_items_return form_grid_templates = { @@ -226,6 +230,9 @@ class SalesInvoice(SellingController): # because updating reserved qty in bin depends upon updated delivered qty in SO if self.update_stock == 1: self.update_stock_ledger() + if self.is_return and self.update_stock: + update_serial_nos_after_submit(self, "items") + # this sequence because outstanding may get -ve self.make_gl_entries() diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 271e02a50c6..62c3508ab00 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -2617,11 +2617,9 @@ class TestSalesInvoice(unittest.TestCase): def test_standalone_serial_no_return(self): si = create_sales_invoice(item_code="_Test Serialized Item With Series", update_stock=True, is_return=True, qty=-1) - si.submit() + si.reload() self.assertTrue(si.items[0].serial_no) - return si - def get_sales_invoice_for_e_invoice(): si = make_sales_invoice_for_ewaybill() diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 314e4ba433a..75ccd86153e 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -13,7 +13,10 @@ from frappe.utils import cint, flt from erpnext.controllers.accounts_controller import get_taxes_and_charges from erpnext.controllers.selling_controller import SellingController from erpnext.stock.doctype.batch.batch import set_batch_nos -from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no +from erpnext.stock.doctype.serial_no.serial_no import ( + get_delivery_note_serial_no, + update_serial_nos_after_submit, +) from erpnext.stock.utils import calculate_mapped_packed_items_return form_grid_templates = { @@ -220,6 +223,9 @@ class DeliveryNote(SellingController): # Updating stock ledger should always be called after updating prevdoc status, # because updating reserved qty in bin depends upon updated delivered qty in SO self.update_stock_ledger() + if self.is_return: + update_serial_nos_after_submit(self, "items") + self.make_gl_entries() self.repost_future_sle_and_gle() diff --git a/erpnext/stock/doctype/delivery_note/test_delivery_note.py b/erpnext/stock/doctype/delivery_note/test_delivery_note.py index 16c892128a6..fc3dce1ee90 100644 --- a/erpnext/stock/doctype/delivery_note/test_delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/test_delivery_note.py @@ -822,6 +822,11 @@ class TestDeliveryNote(FrappeTestCase): automatically_fetch_payment_terms(enable=0) + def test_standalone_serial_no_return(self): + dn = create_delivery_note(item_code="_Test Serialized Item With Series", is_return=True, qty=-1) + dn.reload() + self.assertTrue(dn.items[0].serial_no) + def create_return_delivery_note(**args): args = frappe._dict(args) from erpnext.controllers.sales_and_purchase_return import make_return_doc diff --git a/erpnext/stock/doctype/serial_no/serial_no.py b/erpnext/stock/doctype/serial_no/serial_no.py index ee08e38f33c..bf62f50c971 100644 --- a/erpnext/stock/doctype/serial_no/serial_no.py +++ b/erpnext/stock/doctype/serial_no/serial_no.py @@ -394,7 +394,7 @@ def update_serial_nos(sle, item_det): if not sle.is_cancelled and not sle.serial_no and cint(sle.actual_qty) > 0 \ and item_det.has_serial_no == 1 and item_det.serial_no_series: serial_nos = get_auto_serial_nos(item_det.serial_no_series, sle.actual_qty) - frappe.db.set(sle, "serial_no", serial_nos) + sle.db_set("serial_no", serial_nos) validate_serial_no(sle, item_det) if sle.serial_no: auto_make_serial_nos(sle) @@ -516,13 +516,16 @@ def update_serial_nos_after_submit(controller, parentfield): if controller.doctype == "Stock Entry": warehouse = d.t_warehouse qty = d.transfer_qty + elif controller.doctype in ("Sales Invoice", "Delivery Note"): + warehouse = d.warehouse + qty = d.stock_qty else: warehouse = d.warehouse qty = (d.qty if controller.doctype == "Stock Reconciliation" else d.stock_qty) for sle in stock_ledger_entries: if sle.voucher_detail_no==d.name: - if not accepted_serial_nos_updated and qty and abs(sle.actual_qty)==qty \ + if not accepted_serial_nos_updated and qty and abs(sle.actual_qty) == abs(qty) \ and sle.warehouse == warehouse and sle.serial_no != d.serial_no: d.serial_no = sle.serial_no frappe.db.set_value(d.doctype, d.name, "serial_no", sle.serial_no)