fix: distribution of additional costs in mfg stock entry (#27629)

* refactor: remove unnecessary list comprehensions

* fix: correct cost distribution logic

While apportioning costs same condition should be present on both sides
so total value is representative of all items to be apportioned.

Here while calculating incoming_items_cost only FG items are considered,
but while apportioning all items with to_warehouse are considered.

Solution: only apportion additional cost on FG items

* test: test cost distribution

* fix: patch for additional cost

fix(patch): consider PCV while patching

- consider Period closing voucher while patching
- recomute rates for SLE of affected stock entries

consider only FG/scrap item SLEs for recomputation of rates

* fix: remove client side logic for addn cost

All of this is done in python code hence removed client side code.
This commit is contained in:
Ankush Menat 2021-09-30 09:58:59 +05:30 committed by GitHub
parent 2a1e202e07
commit 4685ed5a8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 145 additions and 64 deletions

View File

@ -311,3 +311,4 @@ erpnext.patches.v13_0.gst_fields_for_pos_invoice
erpnext.patches.v13_0.create_accounting_dimensions_in_pos_doctypes
erpnext.patches.v13_0.create_custom_field_for_finance_book
erpnext.patches.v13_0.modify_invalid_gain_loss_gl_entries
erpnext.patches.v13_0.fix_additional_cost_in_mfg_stock_entry

View File

@ -0,0 +1,76 @@
from typing import List, NewType
import frappe
StockEntryCode = NewType("StockEntryCode", str)
def execute():
stock_entry_codes = find_broken_stock_entries()
for stock_entry_code in stock_entry_codes:
patched_stock_entry = patch_additional_cost(stock_entry_code)
create_repost_item_valuation(patched_stock_entry)
def find_broken_stock_entries() -> List[StockEntryCode]:
period_closing_date = frappe.db.get_value(
"Period Closing Voucher", {"docstatus": 1}, "posting_date", order_by="posting_date desc"
)
stock_entries_to_patch = frappe.db.sql(
"""
select se.name, sum(sed.additional_cost) as item_additional_cost, se.total_additional_costs
from `tabStock Entry` se
join `tabStock Entry Detail` sed
on sed.parent = se.name
where
se.docstatus = 1 and
se.posting_date > %s
group by
sed.parent
having
item_additional_cost != se.total_additional_costs
""",
period_closing_date,
as_dict=True,
)
return [d.name for d in stock_entries_to_patch]
def patch_additional_cost(code: StockEntryCode):
stock_entry = frappe.get_doc("Stock Entry", code)
stock_entry.distribute_additional_costs()
stock_entry.update_valuation_rate()
stock_entry.set_total_incoming_outgoing_value()
stock_entry.set_total_amount()
stock_entry.db_update()
for item in stock_entry.items:
item.db_update()
return stock_entry
def create_repost_item_valuation(stock_entry):
from erpnext.controllers.stock_controller import create_repost_item_valuation_entry
# turn on recalculate flag so reposting corrects the incoming/outgoing rates.
frappe.db.set_value(
"Stock Ledger Entry",
{"voucher_no": stock_entry.name, "actual_qty": (">", 0)},
"recalculate_rate",
1,
update_modified=False,
)
create_repost_item_valuation_entry(
args=frappe._dict(
{
"posting_date": stock_entry.posting_date,
"posting_time": stock_entry.posting_time,
"voucher_type": stock_entry.doctype,
"voucher_no": stock_entry.name,
"company": stock_entry.company,
}
)
)

View File

@ -548,44 +548,7 @@ frappe.ui.form.on('Stock Entry', {
calculate_basic_amount: function(frm, item) {
item.basic_amount = flt(flt(item.transfer_qty) * flt(item.basic_rate),
precision("basic_amount", item));
frm.events.calculate_amount(frm);
},
calculate_amount: function(frm) {
frm.events.calculate_total_additional_costs(frm);
let total_basic_amount = 0;
if (in_list(["Repack", "Manufacture"], frm.doc.purpose)) {
total_basic_amount = frappe.utils.sum(
(frm.doc.items || []).map(function(i) {
return i.is_finished_item ? flt(i.basic_amount) : 0;
})
);
} else {
total_basic_amount = frappe.utils.sum(
(frm.doc.items || []).map(function(i) {
return i.t_warehouse ? flt(i.basic_amount) : 0;
})
);
}
for (let i in frm.doc.items) {
let item = frm.doc.items[i];
if (((in_list(["Repack", "Manufacture"], frm.doc.purpose) && item.is_finished_item) || item.t_warehouse) && total_basic_amount) {
item.additional_cost = (flt(item.basic_amount) / total_basic_amount) * frm.doc.total_additional_costs;
} else {
item.additional_cost = 0;
}
item.amount = flt(item.basic_amount + flt(item.additional_cost), precision("amount", item));
if (flt(item.transfer_qty)) {
item.valuation_rate = flt(flt(item.basic_rate) + (flt(item.additional_cost) / flt(item.transfer_qty)),
precision("valuation_rate", item));
}
}
refresh_field('items');
},
calculate_total_additional_costs: function(frm) {
@ -781,11 +744,6 @@ frappe.ui.form.on('Landed Cost Taxes and Charges', {
amount: function(frm, cdt, cdn) {
frm.events.set_base_amount(frm, cdt, cdn);
// Adding this check because same table in used in LCV
// This causes an error if you try to post an LCV immediately after a Stock Entry
if (frm.doc.doctype == 'Stock Entry') {
frm.events.calculate_amount(frm);
}
},
expense_account: function(frm, cdt, cdn) {

View File

@ -555,22 +555,27 @@ class StockEntry(StockController):
def distribute_additional_costs(self):
# If no incoming items, set additional costs blank
if not any([d.item_code for d in self.items if d.t_warehouse]):
if not any(d.item_code for d in self.items if d.t_warehouse):
self.additional_costs = []
self.total_additional_costs = sum([flt(t.base_amount) for t in self.get("additional_costs")])
self.total_additional_costs = sum(flt(t.base_amount) for t in self.get("additional_costs"))
if self.purpose in ("Repack", "Manufacture"):
incoming_items_cost = sum([flt(t.basic_amount) for t in self.get("items") if t.is_finished_item])
incoming_items_cost = sum(flt(t.basic_amount) for t in self.get("items") if t.is_finished_item)
else:
incoming_items_cost = sum([flt(t.basic_amount) for t in self.get("items") if t.t_warehouse])
incoming_items_cost = sum(flt(t.basic_amount) for t in self.get("items") if t.t_warehouse)
if incoming_items_cost:
for d in self.get("items"):
if (self.purpose in ("Repack", "Manufacture") and d.is_finished_item) or d.t_warehouse:
d.additional_cost = (flt(d.basic_amount) / incoming_items_cost) * self.total_additional_costs
else:
d.additional_cost = 0
if not incoming_items_cost:
return
for d in self.get("items"):
if self.purpose in ("Repack", "Manufacture") and not d.is_finished_item:
d.additional_cost = 0
continue
elif not d.t_warehouse:
d.additional_cost = 0
continue
d.additional_cost = (flt(d.basic_amount) / incoming_items_cost) * self.total_additional_costs
def update_valuation_rate(self):
for d in self.get("items"):
@ -805,7 +810,11 @@ class StockEntry(StockController):
def get_gl_entries(self, warehouse_account):
gl_entries = super(StockEntry, self).get_gl_entries(warehouse_account)
total_basic_amount = sum([flt(t.basic_amount) for t in self.get("items") if t.t_warehouse])
if self.purpose in ("Repack", "Manufacture"):
total_basic_amount = sum(flt(t.basic_amount) for t in self.get("items") if t.is_finished_item)
else:
total_basic_amount = sum(flt(t.basic_amount) for t in self.get("items") if t.t_warehouse)
divide_based_on = total_basic_amount
if self.get("additional_costs") and not total_basic_amount:
@ -816,20 +825,24 @@ class StockEntry(StockController):
for t in self.get("additional_costs"):
for d in self.get("items"):
if d.t_warehouse:
item_account_wise_additional_cost.setdefault((d.item_code, d.name), {})
item_account_wise_additional_cost[(d.item_code, d.name)].setdefault(t.expense_account, {
"amount": 0.0,
"base_amount": 0.0
})
if self.purpose in ("Repack", "Manufacture") and not d.is_finished_item:
continue
elif not d.t_warehouse:
continue
multiply_based_on = d.basic_amount if total_basic_amount else d.qty
item_account_wise_additional_cost.setdefault((d.item_code, d.name), {})
item_account_wise_additional_cost[(d.item_code, d.name)].setdefault(t.expense_account, {
"amount": 0.0,
"base_amount": 0.0
})
item_account_wise_additional_cost[(d.item_code, d.name)][t.expense_account]["amount"] += \
flt(t.amount * multiply_based_on) / divide_based_on
multiply_based_on = d.basic_amount if total_basic_amount else d.qty
item_account_wise_additional_cost[(d.item_code, d.name)][t.expense_account]["base_amount"] += \
flt(t.base_amount * multiply_based_on) / divide_based_on
item_account_wise_additional_cost[(d.item_code, d.name)][t.expense_account]["amount"] += \
flt(t.amount * multiply_based_on) / divide_based_on
item_account_wise_additional_cost[(d.item_code, d.name)][t.expense_account]["base_amount"] += \
flt(t.base_amount * multiply_based_on) / divide_based_on
if item_account_wise_additional_cost:
for d in self.get("items"):

View File

@ -837,6 +837,39 @@ class TestStockEntry(unittest.TestCase):
frappe.db.set_default("allow_negative_stock", 0)
def test_additional_cost_distribution_manufacture(self):
se = frappe.get_doc(
doctype="Stock Entry",
purpose="Manufacture",
additional_costs=[frappe._dict(base_amount=100)],
items=[
frappe._dict(item_code="RM", basic_amount=10),
frappe._dict(item_code="FG", basic_amount=20, t_warehouse="X", is_finished_item=1),
frappe._dict(item_code="scrap", basic_amount=30, t_warehouse="X")
],
)
se.distribute_additional_costs()
distributed_costs = [d.additional_cost for d in se.items]
self.assertEqual([0.0, 100.0, 0.0], distributed_costs)
def test_additional_cost_distribution_non_manufacture(self):
se = frappe.get_doc(
doctype="Stock Entry",
purpose="Material Receipt",
additional_costs=[frappe._dict(base_amount=100)],
items=[
frappe._dict(item_code="RECEIVED_1", basic_amount=20, t_warehouse="X"),
frappe._dict(item_code="RECEIVED_2", basic_amount=30, t_warehouse="X")
],
)
se.distribute_additional_costs()
distributed_costs = [d.additional_cost for d in se.items]
self.assertEqual([40.0, 60.0], distributed_costs)
def make_serialized_item(**args):
args = frappe._dict(args)
se = frappe.copy_doc(test_records[0])