From a852dc1f11db26bf744f1657a2fc0dfdc7c8a137 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Thu, 17 Nov 2022 11:00:01 +0100 Subject: [PATCH 1/5] feat: validate repost item valuation against accounts freeze date (cherry picked from commit 61f05132dbd0cd3d0ed76e04f2d45a8a6ebf1c66) # Conflicts: # erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py --- .../repost_item_valuation.py | 19 ++++- .../test_repost_item_valuation.py | 80 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 14f5e548ecc..97027082843 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -5,7 +5,7 @@ import frappe from frappe import _ from frappe.exceptions import QueryDeadlockError, QueryTimeoutError from frappe.model.document import Document -from frappe.utils import cint, get_link_to_form, get_weekday, now, nowtime +from frappe.utils import cint, get_link_to_form, get_weekday, getdate, now, nowtime from frappe.utils.user import get_users_with_role from rq.timeouts import JobTimeoutException @@ -25,6 +25,21 @@ class RepostItemValuation(Document): self.set_status(write=False) self.reset_field_values() self.set_company() + self.validate_accounts_freeze() + + def validate_accounts_freeze(self): + acc_settings = frappe.db.get_value( + 'Accounts Settings', + 'Accounts Settings', + ['acc_frozen_upto', 'frozen_accounts_modifier'], + as_dict=1 + ) + if not acc_settings.acc_frozen_upto: + return + if acc_settings.frozen_accounts_modifier and self.owner in get_users_with_role(acc_settings.frozen_accounts_modifier): + return + if getdate(self.posting_date) <= getdate(acc_settings.acc_frozen_upto): + frappe.throw(_("You cannot repost item valuation before {}").format(acc_settings.acc_frozen_upto)) def reset_field_values(self): if self.based_on == "Transaction": @@ -235,7 +250,7 @@ def _get_directly_dependent_vouchers(doc): def notify_error_to_stock_managers(doc, traceback): recipients = get_users_with_role("Stock Manager") if not recipients: - get_users_with_role("System Manager") + recipients = get_users_with_role("System Manager") subject = _("Error while reposting item valuation") message = ( diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index edd2553d5d1..62ed035df95 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -272,3 +272,83 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): [{"credit": 50, "debit": 0}], gle_filters={"account": "Stock In Hand - TCP1"}, ) +<<<<<<< HEAD +======= + + def test_duplicate_ple_on_repost(self): + from erpnext.accounts import utils + + # lower numbers to simplify test + orig_chunk_size = utils.GL_REPOSTING_CHUNK + utils.GL_REPOSTING_CHUNK = 2 + self.addCleanup(setattr, utils, "GL_REPOSTING_CHUNK", orig_chunk_size) + + rate = 100 + item = self.make_item() + item.valuation_rate = 90 + item.allow_negative_stock = 1 + item.save() + + company = "_Test Company with perpetual inventory" + + # consume non-existing stock + sinv = create_sales_invoice( + company=company, + posting_date=today(), + debit_to="Debtors - TCP1", + income_account="Sales - TCP1", + expense_account="Cost of Goods Sold - TCP1", + warehouse="Stores - TCP1", + update_stock=1, + currency="INR", + item_code=item.name, + cost_center="Main - TCP1", + qty=1, + rate=rate, + ) + + # backdated receipt triggers repost + make_stock_entry( + item=item.name, + company=company, + qty=5, + rate=rate, + target="Stores - TCP1", + posting_date=add_to_date(today(), days=-1), + ) + + ple_entries = frappe.db.get_list( + "Payment Ledger Entry", + filters={"voucher_type": sinv.doctype, "voucher_no": sinv.name, "delinked": 0}, + ) + + # assert successful deduplication on PLE + self.assertEqual(len(ple_entries), 1) + + # outstanding should not be affected + sinv.reload() + self.assertEqual(sinv.outstanding_amount, 100) + + def test_account_freeze_validation(self): + today = nowdate() + + riv = frappe.get_doc( + doctype="Repost Item Valuation", + item_code="_Test Item", + warehouse="_Test Warehouse - _TC", + based_on="Item and Warehouse", + posting_date=today, + posting_time="00:01:00", + ) + riv.flags.dont_run_in_test = True # keep it queued + + accounts_settings = frappe.get_doc("Accounts Settings") + accounts_settings.acc_frozen_upto = today + accounts_settings.frozen_accounts_modifier = '' + accounts_settings.save() + + self.assertRaises(frappe.ValidationError, riv.save) + + accounts_settings.acc_frozen_upto = '' + accounts_settings.save() +>>>>>>> 61f05132db (feat: validate repost item valuation against accounts freeze date) From 6992e727cf283fa1e55934e21b08de033742cc34 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Thu, 17 Nov 2022 11:00:34 +0100 Subject: [PATCH 2/5] chore: pre-commit (cherry picked from commit be15419bd5d3481dd83c7c4fedbb010d3e0e13d9) # Conflicts: # erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py --- .../repost_item_valuation.py | 16 ++++++++++------ .../test_repost_item_valuation.py | 7 ++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 97027082843..62d0d94a92e 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -29,17 +29,21 @@ class RepostItemValuation(Document): def validate_accounts_freeze(self): acc_settings = frappe.db.get_value( - 'Accounts Settings', - 'Accounts Settings', - ['acc_frozen_upto', 'frozen_accounts_modifier'], - as_dict=1 + "Accounts Settings", + "Accounts Settings", + ["acc_frozen_upto", "frozen_accounts_modifier"], + as_dict=1, ) if not acc_settings.acc_frozen_upto: return - if acc_settings.frozen_accounts_modifier and self.owner in get_users_with_role(acc_settings.frozen_accounts_modifier): + if acc_settings.frozen_accounts_modifier and self.owner in get_users_with_role( + acc_settings.frozen_accounts_modifier + ): return if getdate(self.posting_date) <= getdate(acc_settings.acc_frozen_upto): - frappe.throw(_("You cannot repost item valuation before {}").format(acc_settings.acc_frozen_upto)) + frappe.throw( + _("You cannot repost item valuation before {}").format(acc_settings.acc_frozen_upto) + ) def reset_field_values(self): if self.based_on == "Transaction": diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 62ed035df95..7758b32d98c 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -344,11 +344,16 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): accounts_settings = frappe.get_doc("Accounts Settings") accounts_settings.acc_frozen_upto = today - accounts_settings.frozen_accounts_modifier = '' + accounts_settings.frozen_accounts_modifier = "" accounts_settings.save() self.assertRaises(frappe.ValidationError, riv.save) +<<<<<<< HEAD accounts_settings.acc_frozen_upto = '' accounts_settings.save() >>>>>>> 61f05132db (feat: validate repost item valuation against accounts freeze date) +======= + accounts_settings.acc_frozen_upto = "" + accounts_settings.save() +>>>>>>> be15419bd5 (chore: pre-commit) From 7d6e2f979fe4575e9efda97e67f49ba1619d90a1 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Wed, 23 Nov 2022 18:59:15 +0530 Subject: [PATCH 3/5] fix: check for session user rather than owner (cherry picked from commit b482e3876dccb2547330b95c1c8d4f2cf7039918) --- .../repost_item_valuation/repost_item_valuation.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 62d0d94a92e..9e0ff07a642 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -36,11 +36,12 @@ class RepostItemValuation(Document): ) if not acc_settings.acc_frozen_upto: return - if acc_settings.frozen_accounts_modifier and self.owner in get_users_with_role( - acc_settings.frozen_accounts_modifier - ): - return if getdate(self.posting_date) <= getdate(acc_settings.acc_frozen_upto): + if acc_settings.frozen_accounts_modifier and frappe.session.user in get_users_with_role( + acc_settings.frozen_accounts_modifier + ): + frappe.msgprint(_("Caution: This might alter frozen accounts.")) + return frappe.throw( _("You cannot repost item valuation before {}").format(acc_settings.acc_frozen_upto) ) From 198a64d5746d0a6911002cfe6b61cf147fa258ee Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Wed, 23 Nov 2022 19:04:11 +0530 Subject: [PATCH 4/5] chore: pre-commit (cherry picked from commit 88a0aa4077b6cd96800a813e5ba163b573d01f47) --- .../doctype/repost_item_valuation/repost_item_valuation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 9e0ff07a642..10e4ba055a1 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -37,8 +37,9 @@ class RepostItemValuation(Document): if not acc_settings.acc_frozen_upto: return if getdate(self.posting_date) <= getdate(acc_settings.acc_frozen_upto): - if acc_settings.frozen_accounts_modifier and frappe.session.user in get_users_with_role( + if ( acc_settings.frozen_accounts_modifier + and frappe.session.user in get_users_with_role(acc_settings.frozen_accounts_modifier) ): frappe.msgprint(_("Caution: This might alter frozen accounts.")) return From 6bdf143084ce02eb2e08b7c61922541fd48ca5b1 Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Mon, 1 May 2023 18:26:51 +0530 Subject: [PATCH 5/5] fix: conflicts --- .../test_repost_item_valuation.py | 63 ------------------- 1 file changed, 63 deletions(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 7758b32d98c..b9e2ed1beb9 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -272,62 +272,6 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): [{"credit": 50, "debit": 0}], gle_filters={"account": "Stock In Hand - TCP1"}, ) -<<<<<<< HEAD -======= - - def test_duplicate_ple_on_repost(self): - from erpnext.accounts import utils - - # lower numbers to simplify test - orig_chunk_size = utils.GL_REPOSTING_CHUNK - utils.GL_REPOSTING_CHUNK = 2 - self.addCleanup(setattr, utils, "GL_REPOSTING_CHUNK", orig_chunk_size) - - rate = 100 - item = self.make_item() - item.valuation_rate = 90 - item.allow_negative_stock = 1 - item.save() - - company = "_Test Company with perpetual inventory" - - # consume non-existing stock - sinv = create_sales_invoice( - company=company, - posting_date=today(), - debit_to="Debtors - TCP1", - income_account="Sales - TCP1", - expense_account="Cost of Goods Sold - TCP1", - warehouse="Stores - TCP1", - update_stock=1, - currency="INR", - item_code=item.name, - cost_center="Main - TCP1", - qty=1, - rate=rate, - ) - - # backdated receipt triggers repost - make_stock_entry( - item=item.name, - company=company, - qty=5, - rate=rate, - target="Stores - TCP1", - posting_date=add_to_date(today(), days=-1), - ) - - ple_entries = frappe.db.get_list( - "Payment Ledger Entry", - filters={"voucher_type": sinv.doctype, "voucher_no": sinv.name, "delinked": 0}, - ) - - # assert successful deduplication on PLE - self.assertEqual(len(ple_entries), 1) - - # outstanding should not be affected - sinv.reload() - self.assertEqual(sinv.outstanding_amount, 100) def test_account_freeze_validation(self): today = nowdate() @@ -348,12 +292,5 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): accounts_settings.save() self.assertRaises(frappe.ValidationError, riv.save) - -<<<<<<< HEAD - accounts_settings.acc_frozen_upto = '' - accounts_settings.save() ->>>>>>> 61f05132db (feat: validate repost item valuation against accounts freeze date) -======= accounts_settings.acc_frozen_upto = "" accounts_settings.save() ->>>>>>> be15419bd5 (chore: pre-commit)