diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 6865731f15d..49f27d4de73 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -3384,6 +3384,7 @@ class TestSalesInvoice(ERPNextTestSuite): si.posting_date = getdate() si.submit() + @IntegrationTestCase.change_settings("Accounts Settings", {"over_billing_allowance": 0}) def test_over_billing_case_against_delivery_note(self): """ Test a case where duplicating the item with qty = 1 in the invoice @@ -3391,24 +3392,23 @@ class TestSalesInvoice(ERPNextTestSuite): """ from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note - over_billing_allowance = frappe.db.get_single_value("Accounts Settings", "over_billing_allowance") - frappe.db.set_single_value("Accounts Settings", "over_billing_allowance", 0) - dn = create_delivery_note() dn.submit() si = make_sales_invoice(dn.name) - # make a copy of first item and add it to invoice item_copy = frappe.copy_doc(si.items[0]) + si.save() + + si.items = [] # Clear existing items si.append("items", item_copy) si.save() + si.append("items", item_copy) with self.assertRaises(frappe.ValidationError) as err: - si.submit() + si.save() self.assertTrue("cannot overbill" in str(err.exception).lower()) - - frappe.db.set_single_value("Accounts Settings", "over_billing_allowance", over_billing_allowance) + dn.cancel() @IntegrationTestCase.change_settings( "Accounts Settings", diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 04fb045dac4..daef99b25a9 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -2043,69 +2043,48 @@ class AccountsController(TransactionBase): def validate_multiple_billing(self, ref_dt, item_ref_dn, based_on): from erpnext.controllers.status_updater import get_allowance_for - item_allowance = {} - global_qty_allowance, global_amount_allowance = None, None + ref_wise_billed_amount = self.get_reference_wise_billed_amt(ref_dt, item_ref_dn, based_on) - role_allowed_to_over_bill = frappe.get_cached_value( - "Accounts Settings", None, "role_allowed_to_over_bill" - ) - user_roles = frappe.get_roles() + if not ref_wise_billed_amount: + return total_overbilled_amt = 0.0 + overbilled_items = [] + precision = self.precision(based_on, "items") + precision_allowance = 1 / (10**precision) - reference_names = [d.get(item_ref_dn) for d in self.get("items") if d.get(item_ref_dn)] - reference_details = self.get_billing_reference_details(reference_names, ref_dt + " Item", based_on) + role_allowed_to_overbill = frappe.get_single_value("Accounts Settings", "role_allowed_to_over_bill") + is_overbilling_allowed = role_allowed_to_overbill in frappe.get_roles() - for item in self.get("items"): - if not item.get(item_ref_dn): - continue + for row in ref_wise_billed_amount.values(): + total_billed_amt = row.billed_amt + allowance = get_allowance_for(row.item_code, {}, None, None, "amount")[0] - ref_amt = flt(reference_details.get(item.get(item_ref_dn)), self.precision(based_on, item)) - based_on_amt = flt(item.get(based_on)) - - if not ref_amt: - if based_on_amt: # Skip warning for free items - frappe.msgprint( - _( - "System will not check over billing since amount for Item {0} in {1} is zero" - ).format(item.item_code, ref_dt), - title=_("Warning"), - indicator="orange", - ) - continue - - already_billed = self.get_billed_amount_for_item(item, item_ref_dn, based_on) - - total_billed_amt = flt(flt(already_billed) + based_on_amt, self.precision(based_on, item)) - - allowance, item_allowance, global_qty_allowance, global_amount_allowance = get_allowance_for( - item.item_code, item_allowance, global_qty_allowance, global_amount_allowance, "amount" - ) - - max_allowed_amt = flt(ref_amt * (100 + allowance) / 100) + max_allowed_amt = flt(row.ref_amt * (100 + allowance) / 100) if total_billed_amt < 0 and max_allowed_amt < 0: # while making debit note against purchase return entry(purchase receipt) getting overbill error - total_billed_amt = abs(total_billed_amt) - max_allowed_amt = abs(max_allowed_amt) + total_billed_amt, max_allowed_amt = abs(total_billed_amt), abs(max_allowed_amt) overbill_amt = total_billed_amt - max_allowed_amt + row["max_allowed_amt"] = max_allowed_amt total_overbilled_amt += overbill_amt - if overbill_amt > 0.01 and role_allowed_to_over_bill not in user_roles: - if self.doctype != "Purchase Invoice": - self.throw_overbill_exception(item, max_allowed_amt) - elif not cint( + if overbill_amt > precision_allowance and not is_overbilling_allowed: + if self.doctype != "Purchase Invoice" or not cint( frappe.db.get_single_value( "Buying Settings", "bill_for_rejected_quantity_in_purchase_invoice" ) ): - self.throw_overbill_exception(item, max_allowed_amt) + overbilled_items.append(row) - if role_allowed_to_over_bill in user_roles and total_overbilled_amt > 0.1: + if overbilled_items: + self.throw_overbill_exception(overbilled_items, precision) + + if is_overbilling_allowed and total_overbilled_amt > 0.1: frappe.msgprint( _("Overbilling of {} ignored because you have {} role.").format( - total_overbilled_amt, role_allowed_to_over_bill + total_overbilled_amt, role_allowed_to_overbill ), indicator="orange", alert=True, @@ -2121,55 +2100,88 @@ class AccountsController(TransactionBase): ) ) - def get_billed_amount_for_item(self, item, item_ref_dn, based_on): + def get_reference_wise_billed_amt(self, ref_dt, item_ref_dn, based_on): """ Returns Sum of Amount of Sales/Purchase Invoice Items that are linked to `item_ref_dn` (`dn_detail` / `pr_detail`) that are submitted OR not submitted but are under current invoice """ + reference_names = [d.get(item_ref_dn) for d in self.items if d.get(item_ref_dn)] - from frappe.query_builder import Criterion - from frappe.query_builder.functions import Sum + if not reference_names: + return - item_doctype = frappe.qb.DocType(item.doctype) + ref_wise_billed_amount = {} + precision = self.precision(based_on, "items") + reference_details = self.get_billing_reference_details(reference_names, ref_dt + " Item", based_on) + already_billed = self.get_already_billed_amount(reference_names, item_ref_dn, based_on) + + for item in self.items: + key = item.get(item_ref_dn) + if not key: + continue + + ref_amt = flt(reference_details.get(key), precision) + current_amount = flt(item.get(based_on), precision) + + if not ref_amt: + if current_amount: # Skip warning for free items + frappe.msgprint( + _( + "System will not check over billing since amount for Item {0} in {1} is zero" + ).format(item.item_code, ref_dt), + title=_("Warning"), + indicator="orange", + ) + continue + + ref_wise_billed_amount.setdefault( + key, + frappe._dict(item_code=item.item_code, billed_amt=0.0, ref_amt=ref_amt, rows=[]), + ) + + ref_wise_billed_amount[key]["rows"].append(item.idx) + ref_wise_billed_amount[key]["ref_amt"] = ref_amt + ref_wise_billed_amount[key]["billed_amt"] += current_amount + if key in already_billed: + ref_wise_billed_amount[key]["billed_amt"] += flt(already_billed.pop(key, 0), precision) + + return ref_wise_billed_amount + + def get_already_billed_amount(self, reference_names, item_ref_dn, based_on): + item_doctype = frappe.qb.DocType(self.items[0].doctype) based_on_field = frappe.qb.Field(based_on) join_field = frappe.qb.Field(item_ref_dn) - result = ( - frappe.qb.from_(item_doctype) - .select(Sum(based_on_field)) - .where(join_field == item.get(item_ref_dn)) - .where( - Criterion.any( - [ # select all items from other invoices OR current invoices - Criterion.all( - [ # for selecting items from other invoices - item_doctype.docstatus == 1, - item_doctype.parent != self.name, - ] - ), - Criterion.all( - [ # for selecting items from current invoice, that are linked to same reference - item_doctype.docstatus == 0, - item_doctype.parent == self.name, - item_doctype.name != item.name, - ] - ), - ] - ) - ) - ).run() - - return result[0][0] if result else 0 - - def throw_overbill_exception(self, item, max_allowed_amt): - frappe.throw( - _( - "Cannot overbill for Item {0} in row {1} more than {2}. To allow over-billing, please set allowance in Accounts Settings" - ).format(item.item_code, item.idx, max_allowed_amt) + return frappe._dict( + ( + frappe.qb.from_(item_doctype) + .select(join_field, Sum(based_on_field)) + .where(join_field.isin(reference_names)) + .where((item_doctype.docstatus == 1) & (item_doctype.parent != self.name)) + .groupby(join_field) + ).run() ) + def throw_overbill_exception(self, overbilled_items, precision): + message = ( + _("

Cannot overbill for the following Items:

") + + "" + ) + message += _("

To allow over-billing, please set allowance in Accounts Settings.

") + + frappe.throw(_(message)) + def get_company_default(self, fieldname, ignore_validation=False): from erpnext.accounts.utils import get_company_default