Reposting fixes pre release (#24203)

* fix: finished item validation and rate

* fix: Check if stock and account balance in sync after reposting

* fix: validate stock accounts in journal entry

* fix: validate expense against budget
This commit is contained in:
Nabin Hait 2020-12-25 18:11:54 +05:30 committed by GitHub
parent fc01a838b5
commit 327f03566a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 205 additions and 152 deletions

View File

@ -6,14 +6,18 @@ import frappe, erpnext, json
from frappe.utils import cstr, flt, fmt_money, formatdate, getdate, nowdate, cint, get_link_to_form
from frappe import msgprint, _, scrub
from erpnext.controllers.accounts_controller import AccountsController
from erpnext.accounts.utils import get_balance_on, get_account_currency
from erpnext.accounts.utils import get_balance_on, get_stock_accounts, get_stock_and_account_balance, \
get_account_currency, check_if_stock_and_account_balance_synced
from erpnext.accounts.party import get_party_account
from erpnext.hr.doctype.expense_claim.expense_claim import update_reimbursed_amount
from erpnext.accounts.doctype.invoice_discounting.invoice_discounting import get_party_account_based_on_invoice_discounting
from erpnext.accounts.doctype.invoice_discounting.invoice_discounting \
import get_party_account_based_on_invoice_discounting
from erpnext.accounts.deferred_revenue import get_deferred_booking_accounts
from six import string_types, iteritems
class StockAccountInvalidTransaction(frappe.ValidationError): pass
class JournalEntry(AccountsController):
def __init__(self, *args, **kwargs):
super(JournalEntry, self).__init__(*args, **kwargs)
@ -46,6 +50,7 @@ class JournalEntry(AccountsController):
self.validate_empty_accounts_table()
self.set_account_and_party_balance()
self.validate_inter_company_accounts()
self.validate_stock_accounts()
if not self.title:
self.title = self.get_title()
@ -57,6 +62,8 @@ class JournalEntry(AccountsController):
self.update_expense_claim()
self.update_inter_company_jv()
self.update_invoice_discounting()
check_if_stock_and_account_balance_synced(self.posting_date,
self.company, self.doctype, self.name)
def on_cancel(self):
from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries
@ -95,6 +102,16 @@ class JournalEntry(AccountsController):
if account_currency == previous_account_currency:
if self.total_credit != doc.total_debit or self.total_debit != doc.total_credit:
frappe.throw(_("Total Credit/ Debit Amount should be same as linked Journal Entry"))
def validate_stock_accounts(self):
stock_accounts = get_stock_accounts(self.company, self.doctype, self.name)
for account in stock_accounts:
account_bal, stock_bal, warehouse_list = get_stock_and_account_balance(account,
self.posting_date, self.company)
if account_bal == stock_bal:
frappe.throw(_("Account: {0} can only be updated via Stock Transactions")
.format(account), StockAccountInvalidTransaction)
def update_inter_company_jv(self):
if self.voucher_type == "Inter Company Journal Entry" and self.inter_company_journal_entry_reference:

View File

@ -6,7 +6,7 @@ import unittest, frappe
from frappe.utils import flt, nowdate
from erpnext.accounts.doctype.account.test_account import get_inventory_account
from erpnext.exceptions import InvalidAccountCurrency
from erpnext.accounts.general_ledger import StockAccountInvalidTransaction
from erpnext.accounts.doctype.journal_entry.journal_entry import StockAccountInvalidTransaction
class TestJournalEntry(unittest.TestCase):
def test_journal_entry_with_against_jv(self):
@ -84,25 +84,31 @@ class TestJournalEntry(unittest.TestCase):
company = "_Test Company with perpetual inventory"
stock_account = get_inventory_account(company)
from erpnext.accounts.utils import get_stock_and_account_balance
account_bal, stock_bal, warehouse_list = get_stock_and_account_balance(stock_account, nowdate(), company)
diff = flt(account_bal) - flt(stock_bal)
if not diff:
diff = 100
jv = frappe.new_doc("Journal Entry")
jv.company = company
jv.posting_date = nowdate()
jv.append("accounts", {
"account": stock_account,
"cost_center": "Main - TCP1",
"debit_in_account_currency": 100
"debit_in_account_currency": 0 if diff > 0 else abs(diff),
"credit_in_account_currency": diff if diff > 0 else 0
})
jv.append("accounts", {
"account": "Stock Adjustment - TCP1",
"credit_in_account_currency": 100,
"cost_center": "Main - TCP1",
"debit_in_account_currency": diff if diff > 0 else 0,
"credit_in_account_currency": 0 if diff > 0 else abs(diff)
})
jv.insert()
from erpnext.accounts.utils import get_stock_and_account_balance
account_bal, stock_bal, warehouse_list = get_stock_and_account_balance(stock_account, nowdate(), company)
if account_bal == stock_bal:
self.assertRaises(StockAccountInvalidTransaction, jv.submit)
frappe.db.rollback()

View File

@ -5,15 +5,11 @@ from __future__ import unicode_literals
import frappe, erpnext
from frappe.utils import flt, cstr, cint, comma_and, today, getdate, formatdate, now
from frappe import _
from erpnext.accounts.utils import get_stock_and_account_balance
from frappe.model.meta import get_field_precision
from erpnext.accounts.doctype.budget.budget import validate_expense_against_budget
from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import get_accounting_dimensions
class ClosedAccountingPeriod(frappe.ValidationError): pass
class StockAccountInvalidTransaction(frappe.ValidationError): pass
class StockValueAndAccountBalanceOutOfSync(frappe.ValidationError): pass
def make_gl_entries(gl_map, cancel=False, adv_adj=False, merge_entries=True, update_outstanding='Yes', from_repost=False):
if gl_map:
@ -131,10 +127,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)
if not from_repost:
validate_account_for_perpetual_inventory(gl_map)
def make_entry(args, adv_adj, update_outstanding, from_repost=False):
gle = frappe.new_doc("GL Entry")
gle.update(args)
@ -144,63 +136,9 @@ def make_entry(args, adv_adj, update_outstanding, from_repost=False):
gle.run_method("on_update_with_args", adv_adj, update_outstanding, from_repost)
gle.submit()
# check against budget
if not from_repost:
validate_expense_against_budget(args)
def validate_account_for_perpetual_inventory(gl_map):
if cint(erpnext.is_perpetual_inventory_enabled(gl_map[0].company)):
account_list = [gl_entries.account for gl_entries in gl_map]
aii_accounts = [d.name for d in frappe.get_all("Account",
filters={'account_type': 'Stock', 'is_group': 0, 'company': gl_map[0].company})]
for account in account_list:
if account not in aii_accounts:
continue
# Always use current date to get stock and account balance as there can future entries for
# other items
account_bal, stock_bal, warehouse_list = get_stock_and_account_balance(account,
gl_map[0].posting_date, gl_map[0].company)
if gl_map[0].voucher_type=="Journal Entry":
# In case of Journal Entry, there are no corresponding SL entries,
# hence deducting currency amount
account_bal -= flt(gl_map[0].debit) - flt(gl_map[0].credit)
if account_bal == stock_bal:
frappe.throw(_("Account: {0} can only be updated via Stock Transactions")
.format(account), StockAccountInvalidTransaction)
elif abs(account_bal - stock_bal) > 0.1:
precision = get_field_precision(frappe.get_meta("GL Entry").get_field("debit"),
currency=frappe.get_cached_value('Company', gl_map[0].company, "default_currency"))
diff = flt(stock_bal - account_bal, precision)
error_reason = _("Stock Value ({0}) and Account Balance ({1}) are out of sync for account {2} and it's linked warehouses on {3}.").format(
stock_bal, account_bal, frappe.bold(account), gl_map[0].posting_date)
error_resolution = _("Please create adjustment Journal Entry for amount {0} ").format(frappe.bold(diff))
stock_adjustment_account = frappe.db.get_value("Company",gl_map[0].company,"stock_adjustment_account")
db_or_cr_warehouse_account =('credit_in_account_currency' if diff < 0 else 'debit_in_account_currency')
db_or_cr_stock_adjustment_account = ('debit_in_account_currency' if diff < 0 else 'credit_in_account_currency')
journal_entry_args = {
'accounts':[
{'account': account, db_or_cr_warehouse_account : abs(diff)},
{'account': stock_adjustment_account, db_or_cr_stock_adjustment_account : abs(diff)}
]
}
frappe.msgprint(msg="""{0}<br></br>{1}<br></br>""".format(error_reason, error_resolution),
raise_exception=StockValueAndAccountBalanceOutOfSync,
title=_('Values Out Of Sync'),
primary_action={
'label': _('Make Journal Entry'),
'client_action': 'erpnext.route_to_adjustment_jv',
'args': journal_entry_args
})
def validate_cwip_accounts(gl_map):
cwip_enabled = any([cint(ac.enable_cwip_accounting) for ac in frappe.db.get_all("Asset Category","enable_cwip_accounting")])

View File

@ -12,11 +12,12 @@ from frappe.utils import formatdate, get_number_format_info
from six import iteritems
# imported to enable erpnext.accounts.utils.get_account_currency
from erpnext.accounts.doctype.account.account import get_account_currency
from frappe.model.meta import get_field_precision
from erpnext.stock.utils import get_stock_value_on
from erpnext.stock import get_warehouse_account_map
class StockValueAndAccountBalanceOutOfSync(frappe.ValidationError): pass
class FiscalYearError(frappe.ValidationError): pass
@frappe.whitelist()
@ -585,24 +586,6 @@ def fix_total_debit_credit():
(dr_or_cr, dr_or_cr, '%s', '%s', '%s', dr_or_cr),
(d.diff, d.voucher_type, d.voucher_no))
def get_stock_and_account_balance(account=None, posting_date=None, company=None):
if not posting_date: posting_date = nowdate()
warehouse_account = get_warehouse_account_map(company)
account_balance = get_balance_on(account, posting_date, in_account_currency=False, ignore_account_permission=True)
related_warehouses = [wh for wh, wh_details in warehouse_account.items()
if wh_details.account == account and not wh_details.is_group]
total_stock_value = 0.0
for warehouse in related_warehouses:
value = get_stock_value_on(warehouse, posting_date)
total_stock_value += value
precision = frappe.get_precision("Journal Entry Account", "debit_in_account_currency")
return flt(account_balance, precision), flt(total_stock_value, precision), related_warehouses
def get_currency_precision():
precision = cint(frappe.db.get_default("currency_precision"))
if not precision:
@ -903,12 +886,6 @@ def get_coa(doctype, parent, is_root, chart=None):
return accounts
def get_stock_accounts(company):
return frappe.get_all("Account", filters = {
"account_type": "Stock",
"company": company
})
def update_gl_entries_after(posting_date, posting_time, for_warehouses=None, for_items=None,
warehouse_account=None, company=None):
def _delete_gl_entries(voucher_type, voucher_no):
@ -983,4 +960,90 @@ def compare_existing_and_expected_gle(existing_gle, expected_gle):
if not account_existed:
matched = False
break
return matched
return matched
def check_if_stock_and_account_balance_synced(posting_date, company, voucher_type=None, voucher_no=None):
if not cint(erpnext.is_perpetual_inventory_enabled(company)):
return
accounts = get_stock_accounts(company, voucher_type, voucher_no)
stock_adjustment_account = frappe.db.get_value("Company", company, "stock_adjustment_account")
for account in accounts:
account_bal, stock_bal, warehouse_list = get_stock_and_account_balance(account,
posting_date, company)
if abs(account_bal - stock_bal) > 0.1:
precision = get_field_precision(frappe.get_meta("GL Entry").get_field("debit"),
currency=frappe.get_cached_value('Company', company, "default_currency"))
diff = flt(stock_bal - account_bal, precision)
error_reason = _("Stock Value ({0}) and Account Balance ({1}) are out of sync for account {2} and it's linked warehouses as on {3}.").format(
stock_bal, account_bal, frappe.bold(account), posting_date)
error_resolution = _("Please create an adjustment Journal Entry for amount {0} on {1}")\
.format(frappe.bold(diff), frappe.bold(posting_date))
frappe.msgprint(
msg="""{0}<br></br>{1}<br></br>""".format(error_reason, error_resolution),
raise_exception=StockValueAndAccountBalanceOutOfSync,
title=_('Values Out Of Sync'),
primary_action={
'label': _('Make Journal Entry'),
'client_action': 'erpnext.route_to_adjustment_jv',
'args': get_journal_entry(account, stock_adjustment_account, diff)
})
def get_stock_accounts(company, voucher_type=None, voucher_no=None):
stock_accounts = [d.name for d in frappe.db.get_all("Account", {
"account_type": "Stock",
"company": company,
"is_group": 0
})]
if voucher_type and voucher_no:
if voucher_type == "Journal Entry":
stock_accounts = [d.account for d in frappe.db.get_all("Journal Entry Account", {
"parent": voucher_no,
"account": ["in", stock_accounts]
}, "account")]
else:
stock_accounts = [d.account for d in frappe.db.get_all("GL Entry", {
"voucher_type": voucher_type,
"voucher_no": voucher_no,
"account": ["in", stock_accounts]
}, "account")]
return stock_accounts
def get_stock_and_account_balance(account=None, posting_date=None, company=None):
if not posting_date: posting_date = nowdate()
warehouse_account = get_warehouse_account_map(company)
account_balance = get_balance_on(account, posting_date, in_account_currency=False, ignore_account_permission=True)
related_warehouses = [wh for wh, wh_details in warehouse_account.items()
if wh_details.account == account and not wh_details.is_group]
total_stock_value = 0.0
for warehouse in related_warehouses:
value = get_stock_value_on(warehouse, posting_date)
total_stock_value += value
precision = frappe.get_precision("Journal Entry Account", "debit_in_account_currency")
return flt(account_balance, precision), flt(total_stock_value, precision), related_warehouses
def get_journal_entry(account, stock_adjustment_account, amount):
db_or_cr_warehouse_account =('credit_in_account_currency' if amount < 0 else 'debit_in_account_currency')
db_or_cr_stock_adjustment_account = ('debit_in_account_currency' if amount < 0 else 'credit_in_account_currency')
return {
'accounts':[{
'account': account,
db_or_cr_warehouse_account: abs(amount)
}, {
'account': stock_adjustment_account,
db_or_cr_stock_adjustment_account : abs(amount)
}]
}

View File

@ -241,7 +241,7 @@ class BuyingController(StockController):
if rate > 0:
d.rate = rate
d.amount = flt(d.consumed_qty) * flt(d.rate)
d.amount = flt(flt(d.consumed_qty) * flt(d.rate), d.precision("amount"))
supplied_items_cost += flt(d.amount)
return supplied_items_cost

View File

@ -6,7 +6,7 @@ import frappe, erpnext
from frappe.utils import cint, flt, cstr, get_link_to_form, today, getdate
from frappe import _
import frappe.defaults
from erpnext.accounts.utils import get_fiscal_year
from erpnext.accounts.utils import get_fiscal_year, check_if_stock_and_account_balance_synced
from erpnext.accounts.general_ledger import make_gl_entries, make_reverse_gl_entries, process_gl_map
from erpnext.controllers.accounts_controller import AccountsController
from erpnext.stock.stock_ledger import get_valuation_rate
@ -402,6 +402,14 @@ class StockController(AccountsController):
if check_if_future_sle_exists(args):
create_repost_item_valuation_entry(args)
elif not is_reposting_pending():
check_if_stock_and_account_balance_synced(self.posting_date,
self.company, self.doctype, self.name)
def is_reposting_pending():
return frappe.db.exists("Repost Item Valuation",
{'docstatus': 1, 'status': ['in', ['Queued','In Progress']]})
def check_if_future_sle_exists(args):
sl_entries = frappe.db.get_all("Stock Ledger Entry",

View File

@ -408,7 +408,7 @@ class PurchaseReceipt(BuyingController):
if warehouse_with_no_account:
frappe.msgprint(_("No accounting entries for the following warehouses") + ": \n" +
"\n".join(warehouse_with_no_account))
return process_gl_map(gl_entries)
def get_asset_gl_entry(self, gl_entries):

View File

@ -5,11 +5,11 @@
from __future__ import unicode_literals
import frappe, erpnext
from frappe.model.document import Document
from frappe.utils import cint
from frappe.utils import cint, get_link_to_form
from erpnext.stock.stock_ledger import repost_future_sle
from erpnext.accounts.utils import update_gl_entries_after
from erpnext.accounts.utils import update_gl_entries_after, check_if_stock_and_account_balance_synced
from frappe.utils.user import get_users_with_role
from frappe import _
class RepostItemValuation(Document):
def validate(self):
self.set_status()
@ -51,12 +51,20 @@ def repost(doc):
repost_sl_entries(doc)
repost_gl_entries(doc)
check_if_stock_and_account_balance_synced(doc.posting_date, doc.company)
doc.set_status('Completed')
except Exception:
frappe.db.rollback()
traceback = frappe.get_traceback()
frappe.log_error(traceback)
frappe.db.set_value(doc.doctype, doc.name, 'error_log', traceback)
message = frappe.message_log.pop()
if traceback:
message += "<br>" + "Traceback: <br>" + traceback
frappe.db.set_value(doc.doctype, doc.name, 'error_log', message)
notify_error_to_stock_managers(doc)
doc.set_status('Failed')
raise
finally:
@ -86,4 +94,19 @@ def repost_gl_entries(doc):
warehouses = [doc.warehouse]
update_gl_entries_after(doc.posting_date, doc.posting_time,
warehouses, items, company=doc.company)
warehouses, items, company=doc.company)
def notify_error_to_stock_managers(doc, traceback):
recipients = get_users_with_role("Stock Manager")
if not recipients:
get_users_with_role("System Manager")
subject = _("Error while reposting item valuation")
message = (_("Hi,") + "<br>"
+ _("An error has been appeared while reposting item valuation via {0}")
.format(get_link_to_form(doc.doctype, doc.name)) + "<br>"
+ _("Please check the error message and take necessary actions to fix the error and then restart the reposting again.")
)
frappe.sendmail(recipients=recipients, subject=subject, message=message)

View File

@ -442,6 +442,7 @@ class StockEntry(StockController):
"""
# Set rate for outgoing items
outgoing_items_cost = self.set_rate_for_outgoing_items(reset_outgoing_rate)
finished_item_qty = sum([d.transfer_qty for d in self.items if d.is_finished_item])
# Set basic rate for incoming items
for d in self.get('items'):
@ -451,7 +452,7 @@ class StockEntry(StockController):
d.basic_rate = 0.0
elif d.is_finished_item:
if self.purpose == "Manufacture":
d.basic_rate = self.get_basic_rate_for_manufactured_item(d.transfer_qty, outgoing_items_cost)
d.basic_rate = self.get_basic_rate_for_manufactured_item(finished_item_qty, outgoing_items_cost)
elif self.purpose == "Repack":
d.basic_rate = self.get_basic_rate_for_repacked_items(d.transfer_qty, outgoing_items_cost)
@ -666,7 +667,7 @@ class StockEntry(StockController):
production_item, wo_qty = frappe.db.get_value("Work Order",
self.work_order, ["production_item", "qty"])
number_of_finished_items = 0
finished_items = []
for d in self.get('items'):
if d.is_finished_item:
if d.item_code != production_item:
@ -675,9 +676,9 @@ class StockEntry(StockController):
elif flt(d.transfer_qty) > flt(self.fg_completed_qty):
frappe.throw(_("Quantity in row {0} ({1}) must be same as manufactured quantity {2}"). \
format(d.idx, d.transfer_qty, self.fg_completed_qty))
number_of_finished_items += 1
finished_items.append(d.item_code)
if number_of_finished_items > 1:
if len(set(finished_items)) > 1:
frappe.throw(_("Multiple items cannot be marked as finished item"))
if self.purpose == "Manufacture":

View File

@ -179,22 +179,20 @@ class TestStockEntry(unittest.TestCase):
def test_material_transfer_gl_entry(self):
company = frappe.db.get_value('Warehouse', 'Stores - TCP1', 'company')
create_stock_reconciliation(qty=100, rate=100)
mtn = make_stock_entry(item_code="_Test Item", source="Stores - TCP1",
target="Finished Goods - TCP1", qty=45)
target="Finished Goods - TCP1", qty=45, company=company)
self.check_stock_ledger_entries("Stock Entry", mtn.name,
[["_Test Item", "Stores - TCP1", -45.0], ["_Test Item", "Finished Goods - TCP1", 45.0]])
stock_in_hand_account = get_inventory_account(mtn.company, mtn.get("items")[0].s_warehouse)
source_warehouse_account = get_inventory_account(mtn.company, mtn.get("items")[0].s_warehouse)
fixed_asset_account = get_inventory_account(mtn.company, mtn.get("items")[0].t_warehouse)
target_warehouse_account = get_inventory_account(mtn.company, mtn.get("items")[0].t_warehouse)
if stock_in_hand_account == fixed_asset_account:
if source_warehouse_account == target_warehouse_account:
# no gl entry as both source and target warehouse has linked to same account.
self.assertFalse(frappe.db.sql("""select * from `tabGL Entry`
where voucher_type='Stock Entry' and voucher_no=%s""", mtn.name))
where voucher_type='Stock Entry' and voucher_no=%s""", mtn.name, as_dict=1))
else:
stock_value_diff = abs(frappe.db.get_value("Stock Ledger Entry", {"voucher_type": "Stock Entry",
@ -202,8 +200,8 @@ class TestStockEntry(unittest.TestCase):
self.check_gl_entries("Stock Entry", mtn.name,
sorted([
[stock_in_hand_account, 0.0, stock_value_diff],
[fixed_asset_account, stock_value_diff, 0.0],
[source_warehouse_account, 0.0, stock_value_diff],
[target_warehouse_account, stock_value_diff, 0.0],
])
)
@ -754,37 +752,37 @@ class TestStockEntry(unittest.TestCase):
def test_total_basic_amount_zero(self):
se = frappe.get_doc({"doctype":"Stock Entry",
"purpose":"Material Receipt",
"stock_entry_type":"Material Receipt",
"posting_date": nowdate(),
"company":"_Test Company with perpetual inventory",
"items":[
{
"item_code":"_Test Item",
"description":"_Test Item",
"qty": 1,
"basic_rate": 0,
"uom":"Nos",
"t_warehouse": "Stores - TCP1",
"allow_zero_valuation_rate": 1,
"cost_center": "Main - TCP1"
},
{
"item_code":"_Test Item",
"description":"_Test Item",
"qty": 2,
"basic_rate": 0,
"uom":"Nos",
"t_warehouse": "Stores - TCP1",
"allow_zero_valuation_rate": 1,
"cost_center": "Main - TCP1"
},
],
"additional_costs":[
{"expense_account":"Miscellaneous Expenses - TCP1",
"amount":100,
"description": "miscellanous"}
]
"purpose":"Material Receipt",
"stock_entry_type":"Material Receipt",
"posting_date": nowdate(),
"company":"_Test Company with perpetual inventory",
"items":[
{
"item_code":"_Test Item",
"description":"_Test Item",
"qty": 1,
"basic_rate": 0,
"uom":"Nos",
"t_warehouse": "Stores - TCP1",
"allow_zero_valuation_rate": 1,
"cost_center": "Main - TCP1"
},
{
"item_code":"_Test Item",
"description":"_Test Item",
"qty": 2,
"basic_rate": 0,
"uom":"Nos",
"t_warehouse": "Stores - TCP1",
"allow_zero_valuation_rate": 1,
"cost_center": "Main - TCP1"
},
],
"additional_costs":[
{"expense_account":"Miscellaneous Expenses - TCP1",
"amount":100,
"description": "miscellanous"
}]
})
se.insert()
se.submit()

View File

@ -526,7 +526,7 @@
"index_web_pages_for_search": 1,
"istable": 1,
"links": [],
"modified": "2020-09-23 17:55:03.384138",
"modified": "2020-12-23 17:55:03.384138",
"modified_by": "Administrator",
"module": "Stock",
"name": "Stock Entry Detail",

View File

@ -57,8 +57,7 @@ def get_gl_data(report_filters, filters):
if report_filters.account:
stock_accounts = [report_filters.account]
else:
stock_accounts = [k.name
for k in get_stock_accounts(report_filters.company)]
stock_accounts = get_stock_accounts(report_filters.company)
filters.update({
"account": ("in", stock_accounts)