From 4caaab32d14be1ad6cf5e7ae51105728f9cc2566 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 18 Jul 2022 17:59:42 +0530 Subject: [PATCH 1/7] perf: Optimization of gl entry processing logic in period closing voucher --- erpnext/accounts/doctype/gl_entry/gl_entry.py | 4 +- .../period_closing_voucher.json | 24 ++- .../period_closing_voucher.py | 201 ++++++++++-------- .../test_period_closing_voucher.py | 3 +- erpnext/accounts/general_ledger.py | 5 +- 5 files changed, 141 insertions(+), 96 deletions(-) diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.py b/erpnext/accounts/doctype/gl_entry/gl_entry.py index 9f716568cc0..1987c8340d4 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.py +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.py @@ -42,7 +42,7 @@ class GLEntry(Document): self.validate_and_set_fiscal_year() self.pl_must_have_cost_center() - if not self.flags.from_repost: + if not self.flags.from_repost and self.voucher_type != "Period Closing Voucher": self.check_mandatory() self.validate_cost_center() self.check_pl_account() @@ -51,7 +51,7 @@ class GLEntry(Document): def on_update(self): adv_adj = self.flags.adv_adj - if not self.flags.from_repost: + if not self.flags.from_repost and self.voucher_type != "Period Closing Voucher": self.validate_account_details(adv_adj) self.validate_dimensions_for_pl_and_bs() self.validate_allowed_dimensions() diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json index 84c941ecc10..95742640c96 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json @@ -10,10 +10,11 @@ "fiscal_year", "amended_from", "company", - "cost_center_wise_pnl", "column_break1", "closing_account_head", - "remarks" + "remarks", + "status", + "error_message" ], "fields": [ { @@ -86,17 +87,26 @@ "reqd": 1 }, { - "default": "0", - "fieldname": "cost_center_wise_pnl", - "fieldtype": "Check", - "label": "Book Cost Center Wise Profit/Loss" + "depends_on": "eval:doc.docstatus!=0", + "fieldname": "status", + "fieldtype": "Select", + "label": "GL Entry Processing Status", + "options": "In Progress\nCompleted\nFailed", + "read_only": 1 + }, + { + "depends_on": "eval:doc.status=='Failed'", + "fieldname": "error_message", + "fieldtype": "Text", + "label": "Error Message", + "read_only": 1 } ], "icon": "fa fa-file-text", "idx": 1, "is_submittable": 1, "links": [], - "modified": "2021-05-20 15:27:37.210458", + "modified": "2022-07-15 14:51:04.714154", "modified_by": "Administrator", "module": "Accounts", "name": "Period Closing Voucher", diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py index 5a86376199c..7022a9e7144 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py @@ -20,13 +20,26 @@ class PeriodClosingVoucher(AccountsController): self.validate_posting_date() def on_submit(self): + self.status = "In Progress" self.make_gl_entries() def on_cancel(self): + self.status = "In Progress" self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry") - from erpnext.accounts.general_ledger import make_reverse_gl_entries - - make_reverse_gl_entries(voucher_type="Period Closing Voucher", voucher_no=self.name) + gle_count = frappe.db.count( + "GL Entry", + { + "voucher_type": "Period Closing Voucher", + "voucher_no": self.name, + "is_cancelled": 0 + } + ) + if gle_count > 5000: + frappe.enqueue(make_reverse_gl_entries, voucher_type="Period Closing Voucher", + voucher_no=self.name, queue="long") + frappe.msgprint(_("The GL Entries will be cancelled in the background, it can take a few minutes."), alert=True) + else: + make_reverse_gl_entries(voucher_type="Period Closing Voucher", voucher_no=self.name) def validate_account_head(self): closing_account_type = frappe.db.get_value("Account", self.closing_account_head, "root_type") @@ -63,116 +76,138 @@ class PeriodClosingVoucher(AccountsController): pce[0][0], self.posting_date ) ) - + def make_gl_entries(self): gl_entries = self.get_gl_entries() if gl_entries: - from erpnext.accounts.general_ledger import make_gl_entries - - make_gl_entries(gl_entries) + if len(gl_entries) > 5000: + frappe.enqueue(process_gl_entries, gl_entries=gl_entries, queue="long") + frappe.msgprint(_("The GL Entries will be processed in the background, it can take a few minutes."), alert=True) + else: + process_gl_entries(gl_entries) def get_gl_entries(self): gl_entries = [] - pl_accounts = self.get_pl_balances() - for acc in pl_accounts: + # pl account + for acc in self.get_pl_balances_based_on_dimensions(group_by_account=True): + if flt(acc.bal_in_company_currency): + gl_entries.append(self.get_gle_for_pl_account(acc)) + + # closing liability account + for acc in self.get_pl_balances_based_on_dimensions(group_by_account=False): if flt(acc.bal_in_company_currency): - gl_entries.append( - self.get_gl_dict( - { - "account": acc.account, - "cost_center": acc.cost_center, - "finance_book": acc.finance_book, - "account_currency": acc.account_currency, - "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) < 0 - else 0, - "debit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) < 0 - else 0, - "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) > 0 - else 0, - "credit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) > 0 - else 0, - }, - item=acc, - ) - ) - - if gl_entries: - gle_for_net_pl_bal = self.get_pnl_gl_entry(pl_accounts) - gl_entries += gle_for_net_pl_bal + gl_entries.append(self.get_gle_for_closing_account(acc)) return gl_entries + + def get_gle_for_pl_account(self, acc): + gl_entry = self.get_gl_dict( + { + "account": acc.account, + "cost_center": acc.cost_center, + "finance_book": acc.finance_book, + "account_currency": acc.account_currency, + "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) + if flt(acc.bal_in_account_currency) < 0 + else 0, + "debit": abs(flt(acc.bal_in_company_currency)) + if flt(acc.bal_in_company_currency) < 0 + else 0, + "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) + if flt(acc.bal_in_account_currency) > 0 + else 0, + "credit": abs(flt(acc.bal_in_company_currency)) + if flt(acc.bal_in_company_currency) > 0 + else 0, + }, + item=acc, + ) + self.update_default_dimensions(gl_entry, acc) + return gl_entry + + def get_gle_for_closing_account(self, acc): + gl_entry = self.get_gl_dict( + { + "account": self.closing_account_head, + "cost_center": acc.cost_center, + "finance_book": acc.finance_book, + "account_currency": acc.account_currency, + "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) + if flt(acc.bal_in_account_currency) > 0 + else 0, + "debit": abs(flt(acc.bal_in_company_currency)) + if flt(acc.bal_in_company_currency) > 0 + else 0, + "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) + if flt(acc.bal_in_account_currency) < 0 + else 0, + "credit": abs(flt(acc.bal_in_company_currency)) + if flt(acc.bal_in_company_currency) < 0 + else 0, + }, + item=acc, + ) + self.update_default_dimensions(gl_entry, acc) + return gl_entry - def get_pnl_gl_entry(self, pl_accounts): - company_cost_center = frappe.db.get_value("Company", self.company, "cost_center") - gl_entries = [] - - for acc in pl_accounts: - if flt(acc.bal_in_company_currency): - cost_center = acc.cost_center if self.cost_center_wise_pnl else company_cost_center - gl_entry = self.get_gl_dict( - { - "account": self.closing_account_head, - "cost_center": cost_center, - "finance_book": acc.finance_book, - "account_currency": acc.account_currency, - "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) > 0 - else 0, - "debit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) > 0 - else 0, - "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) < 0 - else 0, - "credit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) < 0 - else 0, - }, - item=acc, - ) - - self.update_default_dimensions(gl_entry) - - gl_entries.append(gl_entry) - - return gl_entries - - def update_default_dimensions(self, gl_entry): + def update_default_dimensions(self, gl_entry, acc): if not self.accounting_dimensions: self.accounting_dimensions = get_accounting_dimensions() - _, default_dimensions = get_dimensions() for dimension in self.accounting_dimensions: - gl_entry.update({dimension: default_dimensions.get(self.company, {}).get(dimension)}) + gl_entry.update({dimension: acc.get(dimension)}) - def get_pl_balances(self): + def get_pl_balances_based_on_dimensions(self, group_by_account=False): """Get balance for dimension-wise pl accounts""" dimension_fields = ["t1.cost_center", "t1.finance_book"] - + self.accounting_dimensions = get_accounting_dimensions() for dimension in self.accounting_dimensions: dimension_fields.append("t1.{0}".format(dimension)) - return frappe.db.sql( - """ + if group_by_account: + dimension_fields.append("t1.account") + + return frappe.db.sql(""" select - t1.account, t2.account_currency, {dimension_fields}, + t2.account_currency, + {dimension_fields}, sum(t1.debit_in_account_currency) - sum(t1.credit_in_account_currency) as bal_in_account_currency, sum(t1.debit) - sum(t1.credit) as bal_in_company_currency from `tabGL Entry` t1, `tabAccount` t2 - where t1.is_cancelled = 0 and t1.account = t2.name and t2.report_type = 'Profit and Loss' - and t2.docstatus < 2 and t2.company = %s - and t1.posting_date between %s and %s - group by t1.account, {dimension_fields} + where + t1.is_cancelled = 0 + and t1.account = t2.name + and t2.report_type = 'Profit and Loss' + and t2.docstatus < 2 + and t2.company = %s + and t1.posting_date between %s and %s + group by {dimension_fields} """.format( dimension_fields=", ".join(dimension_fields) ), (self.company, self.get("year_start_date"), self.posting_date), as_dict=1, ) + +def process_gl_entries(gl_entries): + from erpnext.accounts.general_ledger import make_gl_entries + try: + make_gl_entries(gl_entries, merge_entries=False) + frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Completed") + except Exception as e: + frappe.db.rollback() + frappe.log_error(e) + frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed") + +def make_reverse_gl_entries(voucher_type, voucher_no): + from erpnext.accounts.general_ledger import make_reverse_gl_entries + try: + make_reverse_gl_entries(voucher_type=voucher_type, voucher_no=voucher_no) + frappe.db.set_value("Period Closing Voucher", voucher_no, "status", "Completed") + except Exception as e: + frappe.db.rollback() + frappe.log_error(e) + frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed") \ No newline at end of file diff --git a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py index 3b938ea1ca4..1fb003f3c63 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py @@ -49,7 +49,7 @@ class TestPeriodClosingVoucher(unittest.TestCase): expected_gle = ( ("Cost of Goods Sold - TPC", 0.0, 600.0), - (surplus_account, 600.0, 400.0), + (surplus_account, 200.0, 0.0), ("Sales - TPC", 400.0, 0.0), ) @@ -59,7 +59,6 @@ class TestPeriodClosingVoucher(unittest.TestCase): """, (pcv.name), ) - self.assertEqual(pcv_gle, expected_gle) def test_cost_center_wise_posting(self): diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 76ef3abb6f8..eed87174ac2 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -168,6 +168,7 @@ def get_cost_center_allocation_data(company, posting_date): def merge_similar_entries(gl_map, precision=None): merged_gl_map = [] accounting_dimensions = get_accounting_dimensions() + for entry in gl_map: # if there is already an entry in this account then just add it # to that entry @@ -290,7 +291,6 @@ def save_entries(gl_map, adv_adj, update_outstanding, from_repost=False): for entry in gl_map: make_entry(entry, adv_adj, update_outstanding, from_repost) - def make_entry(args, adv_adj, update_outstanding, from_repost=False): gle = frappe.new_doc("GL Entry") gle.update(args) @@ -298,9 +298,10 @@ def make_entry(args, adv_adj, update_outstanding, from_repost=False): gle.flags.from_repost = from_repost gle.flags.adv_adj = adv_adj gle.flags.update_outstanding = update_outstanding or "Yes" + gle.flags.notify_update = False gle.submit() - if not from_repost: + if not from_repost and gle.voucher_type != "Period Closing Voucher": validate_expense_against_budget(args) From 914a388ee3c89333860171cce526f688d42867c3 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 19 Jul 2022 12:32:54 +0530 Subject: [PATCH 2/7] test: Added test for PCV cancellation --- .../period_closing_voucher.py | 77 ++++++++++--------- .../test_period_closing_voucher.py | 11 ++- erpnext/accounts/general_ledger.py | 3 +- 3 files changed, 53 insertions(+), 38 deletions(-) diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py index 7022a9e7144..385d64a6333 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py @@ -8,7 +8,6 @@ from frappe.utils import flt from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( get_accounting_dimensions, - get_dimensions, ) from erpnext.accounts.utils import get_account_currency from erpnext.controllers.accounts_controller import AccountsController @@ -27,17 +26,19 @@ class PeriodClosingVoucher(AccountsController): self.status = "In Progress" self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry") gle_count = frappe.db.count( - "GL Entry", - { - "voucher_type": "Period Closing Voucher", - "voucher_no": self.name, - "is_cancelled": 0 - } + "GL Entry", + {"voucher_type": "Period Closing Voucher", "voucher_no": self.name, "is_cancelled": 0}, ) if gle_count > 5000: - frappe.enqueue(make_reverse_gl_entries, voucher_type="Period Closing Voucher", - voucher_no=self.name, queue="long") - frappe.msgprint(_("The GL Entries will be cancelled in the background, it can take a few minutes."), alert=True) + frappe.enqueue( + make_reverse_gl_entries, + voucher_type="Period Closing Voucher", + voucher_no=self.name, + queue="long", + ) + frappe.msgprint( + _("The GL Entries will be cancelled in the background, it can take a few minutes."), alert=True + ) else: make_reverse_gl_entries(voucher_type="Period Closing Voucher", voucher_no=self.name) @@ -76,13 +77,16 @@ class PeriodClosingVoucher(AccountsController): pce[0][0], self.posting_date ) ) - + def make_gl_entries(self): gl_entries = self.get_gl_entries() if gl_entries: if len(gl_entries) > 5000: frappe.enqueue(process_gl_entries, gl_entries=gl_entries, queue="long") - frappe.msgprint(_("The GL Entries will be processed in the background, it can take a few minutes."), alert=True) + frappe.msgprint( + _("The GL Entries will be processed in the background, it can take a few minutes."), + alert=True, + ) else: process_gl_entries(gl_entries) @@ -91,7 +95,7 @@ class PeriodClosingVoucher(AccountsController): # pl account for acc in self.get_pl_balances_based_on_dimensions(group_by_account=True): - if flt(acc.bal_in_company_currency): + if flt(acc.bal_in_company_currency): gl_entries.append(self.get_gle_for_pl_account(acc)) # closing liability account @@ -100,7 +104,7 @@ class PeriodClosingVoucher(AccountsController): gl_entries.append(self.get_gle_for_closing_account(acc)) return gl_entries - + def get_gle_for_pl_account(self, acc): gl_entry = self.get_gl_dict( { @@ -109,23 +113,19 @@ class PeriodClosingVoucher(AccountsController): "finance_book": acc.finance_book, "account_currency": acc.account_currency, "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) < 0 - else 0, - "debit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) < 0 - else 0, + if flt(acc.bal_in_account_currency) < 0 + else 0, + "debit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) < 0 else 0, "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) - if flt(acc.bal_in_account_currency) > 0 - else 0, - "credit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) > 0 - else 0, + if flt(acc.bal_in_account_currency) > 0 + else 0, + "credit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) > 0 else 0, }, item=acc, ) self.update_default_dimensions(gl_entry, acc) return gl_entry - + def get_gle_for_closing_account(self, acc): gl_entry = self.get_gl_dict( { @@ -136,15 +136,11 @@ class PeriodClosingVoucher(AccountsController): "debit_in_account_currency": abs(flt(acc.bal_in_account_currency)) if flt(acc.bal_in_account_currency) > 0 else 0, - "debit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) > 0 - else 0, + "debit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) > 0 else 0, "credit_in_account_currency": abs(flt(acc.bal_in_account_currency)) if flt(acc.bal_in_account_currency) < 0 else 0, - "credit": abs(flt(acc.bal_in_company_currency)) - if flt(acc.bal_in_company_currency) < 0 - else 0, + "credit": abs(flt(acc.bal_in_company_currency)) if flt(acc.bal_in_company_currency) < 0 else 0, }, item=acc, ) @@ -162,7 +158,7 @@ class PeriodClosingVoucher(AccountsController): """Get balance for dimension-wise pl accounts""" dimension_fields = ["t1.cost_center", "t1.finance_book"] - + self.accounting_dimensions = get_accounting_dimensions() for dimension in self.accounting_dimensions: dimension_fields.append("t1.{0}".format(dimension)) @@ -170,7 +166,8 @@ class PeriodClosingVoucher(AccountsController): if group_by_account: dimension_fields.append("t1.account") - return frappe.db.sql(""" + return frappe.db.sql( + """ select t2.account_currency, {dimension_fields}, @@ -192,22 +189,30 @@ class PeriodClosingVoucher(AccountsController): as_dict=1, ) + def process_gl_entries(gl_entries): from erpnext.accounts.general_ledger import make_gl_entries + try: make_gl_entries(gl_entries, merge_entries=False) - frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Completed") + frappe.db.set_value( + "Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Completed" + ) except Exception as e: frappe.db.rollback() frappe.log_error(e) - frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed") + frappe.db.set_value( + "Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed" + ) + def make_reverse_gl_entries(voucher_type, voucher_no): from erpnext.accounts.general_ledger import make_reverse_gl_entries + try: make_reverse_gl_entries(voucher_type=voucher_type, voucher_no=voucher_no) frappe.db.set_value("Period Closing Voucher", voucher_no, "status", "Completed") except Exception as e: frappe.db.rollback() frappe.log_error(e) - frappe.db.set_value("Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed") \ No newline at end of file + frappe.db.set_value("Period Closing Voucher", voucher_no, "status", "Failed") diff --git a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py index 1fb003f3c63..9f810f1826d 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py @@ -92,7 +92,6 @@ class TestPeriodClosingVoucher(unittest.TestCase): ) pcv = self.make_period_closing_voucher(submit=False) - pcv.cost_center_wise_pnl = 1 pcv.save() pcv.submit() surplus_account = pcv.closing_account_head @@ -115,6 +114,16 @@ class TestPeriodClosingVoucher(unittest.TestCase): self.assertEqual(pcv_gle, expected_gle) + pcv.reload() + pcv.cancel() + + self.assertFalse( + frappe.db.get_value( + "GL Entry", + {"voucher_type": "Period Closing Voucher", "voucher_no": pcv.name, "is_cancelled": 0}, + ) + ) + def test_period_closing_with_finance_book_entries(self): frappe.db.sql("delete from `tabGL Entry` where company='Test PCV Company'") diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index eed87174ac2..16072f331f1 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -168,7 +168,7 @@ def get_cost_center_allocation_data(company, posting_date): def merge_similar_entries(gl_map, precision=None): merged_gl_map = [] accounting_dimensions = get_accounting_dimensions() - + for entry in gl_map: # if there is already an entry in this account then just add it # to that entry @@ -291,6 +291,7 @@ def save_entries(gl_map, adv_adj, update_outstanding, from_repost=False): for entry in gl_map: make_entry(entry, adv_adj, update_outstanding, from_repost) + def make_entry(args, adv_adj, update_outstanding, from_repost=False): gle = frappe.new_doc("GL Entry") gle.update(args) From 69b906438d5a7c2a2b12419fdb0d02869a1e1507 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 19 Jul 2022 15:05:56 +0530 Subject: [PATCH 3/7] fix: set status on submit/cancel --- .../doctype/period_closing_voucher/period_closing_voucher.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py index 385d64a6333..3b890eef25f 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py @@ -19,11 +19,11 @@ class PeriodClosingVoucher(AccountsController): self.validate_posting_date() def on_submit(self): - self.status = "In Progress" + self.db_set("status", "In Progress") self.make_gl_entries() def on_cancel(self): - self.status = "In Progress" + self.db_set("status", "In Progress") self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry") gle_count = frappe.db.count( "GL Entry", From 516be870dfe4beb9046512143dab299314ca670c Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 20 Jul 2022 10:58:20 +0530 Subject: [PATCH 4/7] fix: Renamed status field to gle_processing_status --- .../period_closing_voucher.json | 8 ++++---- .../period_closing_voucher/period_closing_voucher.py | 12 ++++++------ .../test_period_closing_voucher.py | 2 ++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json index 95742640c96..54a76b34196 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.json @@ -13,7 +13,7 @@ "column_break1", "closing_account_head", "remarks", - "status", + "gle_processing_status", "error_message" ], "fields": [ @@ -88,14 +88,14 @@ }, { "depends_on": "eval:doc.docstatus!=0", - "fieldname": "status", + "fieldname": "gle_processing_status", "fieldtype": "Select", "label": "GL Entry Processing Status", "options": "In Progress\nCompleted\nFailed", "read_only": 1 }, { - "depends_on": "eval:doc.status=='Failed'", + "depends_on": "eval:doc.gle_processing_status=='Failed'", "fieldname": "error_message", "fieldtype": "Text", "label": "Error Message", @@ -106,7 +106,7 @@ "idx": 1, "is_submittable": 1, "links": [], - "modified": "2022-07-15 14:51:04.714154", + "modified": "2022-07-20 14:51:04.714154", "modified_by": "Administrator", "module": "Accounts", "name": "Period Closing Voucher", diff --git a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py index 3b890eef25f..866a94d04b1 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py @@ -19,11 +19,11 @@ class PeriodClosingVoucher(AccountsController): self.validate_posting_date() def on_submit(self): - self.db_set("status", "In Progress") + self.db_set("gle_processing_status", "In Progress") self.make_gl_entries() def on_cancel(self): - self.db_set("status", "In Progress") + self.db_set("gle_processing_status", "In Progress") self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry") gle_count = frappe.db.count( "GL Entry", @@ -196,13 +196,13 @@ def process_gl_entries(gl_entries): try: make_gl_entries(gl_entries, merge_entries=False) frappe.db.set_value( - "Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Completed" + "Period Closing Voucher", gl_entries[0].get("voucher_no"), "gle_processing_status", "Completed" ) except Exception as e: frappe.db.rollback() frappe.log_error(e) frappe.db.set_value( - "Period Closing Voucher", gl_entries[0].get("voucher_no"), "status", "Failed" + "Period Closing Voucher", gl_entries[0].get("voucher_no"), "gle_processing_status", "Failed" ) @@ -211,8 +211,8 @@ def make_reverse_gl_entries(voucher_type, voucher_no): try: make_reverse_gl_entries(voucher_type=voucher_type, voucher_no=voucher_no) - frappe.db.set_value("Period Closing Voucher", voucher_no, "status", "Completed") + frappe.db.set_value("Period Closing Voucher", voucher_no, "gle_processing_status", "Completed") except Exception as e: frappe.db.rollback() frappe.log_error(e) - frappe.db.set_value("Period Closing Voucher", voucher_no, "status", "Failed") + frappe.db.set_value("Period Closing Voucher", voucher_no, "gle_processing_status", "Failed") diff --git a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py index 9f810f1826d..3dca58856be 100644 --- a/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py +++ b/erpnext/accounts/doctype/period_closing_voucher/test_period_closing_voucher.py @@ -59,6 +59,8 @@ class TestPeriodClosingVoucher(unittest.TestCase): """, (pcv.name), ) + pcv.reload() + self.assertEqual(pcv.gle_processing_status, "Completed") self.assertEqual(pcv_gle, expected_gle) def test_cost_center_wise_posting(self): From 6c29146c9183878a7a08db84cdc8e05a64c88455 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 20 Jul 2022 10:25:10 +0530 Subject: [PATCH 5/7] fix: Removed 'Allow Monthly Depreciation' checkbox --- erpnext/assets/doctype/asset/asset.json | 10 +--- erpnext/assets/doctype/asset/asset.py | 56 +++---------------- erpnext/patches.txt | 1 + ..._and_frequency_for_monthly_depreciation.py | 14 +++++ 4 files changed, 24 insertions(+), 57 deletions(-) create mode 100644 erpnext/patches/v13_0/fix_number_and_frequency_for_monthly_depreciation.py diff --git a/erpnext/assets/doctype/asset/asset.json b/erpnext/assets/doctype/asset/asset.json index 6e6bbf1cd29..991df4eada6 100644 --- a/erpnext/assets/doctype/asset/asset.json +++ b/erpnext/assets/doctype/asset/asset.json @@ -40,7 +40,6 @@ "purchase_date", "section_break_23", "calculate_depreciation", - "allow_monthly_depreciation", "column_break_33", "opening_accumulated_depreciation", "number_of_depreciations_booked", @@ -456,13 +455,6 @@ "fieldname": "dimension_col_break", "fieldtype": "Column Break" }, - { - "default": "0", - "depends_on": "calculate_depreciation", - "fieldname": "allow_monthly_depreciation", - "fieldtype": "Check", - "label": "Allow Monthly Depreciation" - }, { "collapsible": 1, "collapsible_depends_on": "is_existing_asset", @@ -518,7 +510,7 @@ "link_fieldname": "asset" } ], - "modified": "2022-01-30 20:19:24.680027", + "modified": "2022-07-20 10:15:12.887372", "modified_by": "Administrator", "module": "Assets", "name": "Asset", diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index a880c2f3911..a22d70dd630 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -343,51 +343,13 @@ class Asset(AccountsController): skip_row = True if depreciation_amount > 0: - # With monthly depreciation, each depreciation is divided by months remaining until next date - if self.allow_monthly_depreciation: - # month range is 1 to 12 - # In pro rata case, for first and last depreciation, month range would be different - month_range = ( - months - if (has_pro_rata and n == 0) - or (has_pro_rata and n == cint(number_of_pending_depreciations) - 1) - else finance_book.frequency_of_depreciation - ) - - for r in range(month_range): - if has_pro_rata and n == 0: - # For first entry of monthly depr - if r == 0: - days_until_first_depr = date_diff(monthly_schedule_date, self.available_for_use_date) - per_day_amt = depreciation_amount / days - depreciation_amount_for_current_month = per_day_amt * days_until_first_depr - depreciation_amount -= depreciation_amount_for_current_month - date = monthly_schedule_date - amount = depreciation_amount_for_current_month - else: - date = add_months(monthly_schedule_date, r) - amount = depreciation_amount / (month_range - 1) - elif (has_pro_rata and n == cint(number_of_pending_depreciations) - 1) and r == cint( - month_range - ) - 1: - # For last entry of monthly depr - date = last_schedule_date - amount = depreciation_amount / month_range - else: - date = add_months(monthly_schedule_date, r) - amount = depreciation_amount / month_range - - self._add_depreciation_row( - date, amount, finance_book.depreciation_method, finance_book.finance_book, finance_book.idx - ) - else: - self._add_depreciation_row( - schedule_date, - depreciation_amount, - finance_book.depreciation_method, - finance_book.finance_book, - finance_book.idx, - ) + self._add_depreciation_row( + schedule_date, + depreciation_amount, + finance_book.depreciation_method, + finance_book.finance_book, + finance_book.idx, + ) def _add_depreciation_row( self, schedule_date, depreciation_amount, depreciation_method, finance_book, finance_book_id @@ -854,10 +816,8 @@ class Asset(AccountsController): return args.get("rate_of_depreciation") value = flt(args.get("expected_value_after_useful_life")) / flt(self.gross_purchase_amount) - depreciation_rate = math.pow(value, 1.0 / flt(args.get("total_number_of_depreciations"), 2)) - - return 100 * (1 - flt(depreciation_rate, float_precision)) + return flt((100 * (1 - depreciation_rate)), float_precision) def get_pro_rata_amt(self, row, depreciation_amount, from_date, to_date): days = date_diff(to_date, from_date) diff --git a/erpnext/patches.txt b/erpnext/patches.txt index c3476f9b8f8..34122a5d0ee 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -347,3 +347,4 @@ erpnext.patches.v14_0.copy_is_subcontracted_value_to_is_old_subcontracting_flow erpnext.patches.v14_0.migrate_gl_to_payment_ledger erpnext.patches.v14_0.crm_ux_cleanup erpnext.patches.v14_0.remove_india_localisation # 14-07-2022 +erpnext.patches.v13_0.fix_number_and_frequency_for_monthly_depreciation \ No newline at end of file diff --git a/erpnext/patches/v13_0/fix_number_and_frequency_for_monthly_depreciation.py b/erpnext/patches/v13_0/fix_number_and_frequency_for_monthly_depreciation.py new file mode 100644 index 00000000000..e827058df0c --- /dev/null +++ b/erpnext/patches/v13_0/fix_number_and_frequency_for_monthly_depreciation.py @@ -0,0 +1,14 @@ +import frappe + + +def execute(): + assets = frappe.get_all("Asset", filters={"allow_monthly_depreciation": 1}) + + for d in assets: + print(d.name) + asset_doc = frappe.get_doc("Asset", d.name) + for i in asset_doc.get("finance_books"): + if i.frequency_of_depreciation != 1: + i.total_number_of_depreciations *= i.frequency_of_depreciation + i.frequency_of_depreciation = 1 + i.db_update() From b26438ccf7bad8592520acb6305f148d0f82c230 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 20 Jul 2022 11:19:51 +0530 Subject: [PATCH 6/7] test: Fixed test for WDV method depreciation schedule --- erpnext/assets/doctype/asset/test_asset.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index 95abaa122f2..34d374c9e8b 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -721,12 +721,12 @@ class TestDepreciationMethods(AssetSetup): ) expected_schedules = [ - ["2022-02-28", 645.0, 645.0], - ["2022-03-31", 1206.8, 1851.8], - ["2022-04-30", 1051.12, 2902.92], - ["2022-05-31", 915.52, 3818.44], - ["2022-06-30", 797.42, 4615.86], - ["2022-07-15", 384.14, 5000.0], + ["2022-02-28", 647.25, 647.25], + ["2022-03-31", 1210.71, 1857.96], + ["2022-04-30", 1053.99, 2911.95], + ["2022-05-31", 917.55, 3829.5], + ["2022-06-30", 798.77, 4628.27], + ["2022-07-15", 371.73, 5000.0], ] schedules = [ @@ -737,7 +737,6 @@ class TestDepreciationMethods(AssetSetup): ] for d in asset.get("schedules") ] - self.assertEqual(schedules, expected_schedules) From e1fa723eef4c501d0a6114862874b3c3ae91faf0 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 20 Jul 2022 15:19:09 +0530 Subject: [PATCH 7/7] fix: set args to empty list if None to avoid enumerate error --- erpnext/stock/stock_ledger.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 80256c3e672..fd1aece7b15 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -208,6 +208,8 @@ def repost_future_sle( via_landed_cost_voucher=False, doc=None, ): + if not args: + args = [] # set args to empty list if None to avoid enumerate error items_to_be_repost = get_items_to_be_repost( voucher_type=voucher_type, voucher_no=voucher_no, doc=doc @@ -303,7 +305,7 @@ def get_items_to_be_repost(voucher_type=None, voucher_no=None, doc=None): group_by="item_code, warehouse", ) - return items_to_be_repost + return items_to_be_repost or [] def get_distinct_item_warehouse(args=None, doc=None):