Merge pull request #39886 from barredterra/refactor-sales-invoice-item
refactor(Sales Invoice): move methods into child row controller
diff --git a/erpnext/accounts/doctype/pos_invoice_item/pos_invoice_item.py b/erpnext/accounts/doctype/pos_invoice_item/pos_invoice_item.py
index c24db1d..e058f82 100644
--- a/erpnext/accounts/doctype/pos_invoice_item/pos_invoice_item.py
+++ b/erpnext/accounts/doctype/pos_invoice_item/pos_invoice_item.py
@@ -3,10 +3,10 @@
# import frappe
-from frappe.model.document import Document
+from erpnext.accounts.doctype.sales_invoice_item.sales_invoice_item import SalesInvoiceItem
-class POSInvoiceItem(Document):
+class POSInvoiceItem(SalesInvoiceItem):
# begin: auto-generated types
# This code is auto-generated. Do not modify anything in this block.
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
index 3352e0d..e2cbf5e 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
@@ -8,6 +8,7 @@
from frappe.model.mapper import get_mapped_doc
from frappe.model.utils import get_fetch_values
from frappe.utils import add_days, cint, cstr, flt, formatdate, get_link_to_form, getdate, nowdate
+from frappe.utils.data import comma_and
import erpnext
from erpnext.accounts.deferred_revenue import validate_service_stop_date
@@ -27,7 +28,6 @@
from erpnext.accounts.utils import cancel_exchange_gain_loss_journal, get_account_currency
from erpnext.assets.doctype.asset.depreciation import (
depreciate_asset,
- get_disposal_account_and_cost_center,
get_gl_entries_on_asset_disposal,
get_gl_entries_on_asset_regain,
reset_depreciation_schedule,
@@ -39,7 +39,6 @@
from erpnext.projects.doctype.timesheet.timesheet import get_projectwise_timesheet_data
from erpnext.setup.doctype.company.company import update_company_current_month_sales
from erpnext.stock.doctype.delivery_note.delivery_note import update_billed_amount_based_on_so
-from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no, get_serial_nos
form_grid_templates = {"items": "templates/form_grid/item_grid.html"}
@@ -297,11 +296,13 @@
if cint(self.is_pos):
self.validate_pos()
+ self.validate_dropship_item()
+
if cint(self.update_stock):
- self.validate_dropship_item()
self.validate_warehouse()
self.update_current_stock()
- self.validate_delivery_note()
+
+ self.validate_delivery_note()
# validate service stop date to lie in between start and end date
validate_service_stop_date(self)
@@ -379,13 +380,7 @@
def validate_item_cost_centers(self):
for item in self.items:
- cost_center_company = frappe.get_cached_value("Cost Center", item.cost_center, "company")
- if cost_center_company != self.company:
- frappe.throw(
- _("Row #{0}: Cost Center {1} does not belong to company {2}").format(
- frappe.bold(item.idx), frappe.bold(item.cost_center), frappe.bold(self.company)
- )
- )
+ item.validate_cost_center(self.company)
def validate_income_account(self):
for item in self.get("items"):
@@ -601,43 +596,48 @@
self.delete_auto_created_batches()
def update_status_updater_args(self):
- if cint(self.update_stock):
- self.status_updater.append(
- {
- "source_dt": "Sales Invoice Item",
- "target_dt": "Sales Order Item",
- "target_parent_dt": "Sales Order",
- "target_parent_field": "per_delivered",
- "target_field": "delivered_qty",
- "target_ref_field": "qty",
- "source_field": "qty",
- "join_field": "so_detail",
- "percent_join_field": "sales_order",
- "status_field": "delivery_status",
- "keyword": "Delivered",
- "second_source_dt": "Delivery Note Item",
- "second_source_field": "qty",
- "second_join_field": "so_detail",
- "overflow_type": "delivery",
- "extra_cond": """ and exists(select name from `tabSales Invoice`
- where name=`tabSales Invoice Item`.parent and update_stock = 1)""",
- }
- )
- if cint(self.is_return):
- self.status_updater.append(
- {
- "source_dt": "Sales Invoice Item",
- "target_dt": "Sales Order Item",
- "join_field": "so_detail",
- "target_field": "returned_qty",
- "target_parent_dt": "Sales Order",
- "source_field": "-1 * qty",
- "second_source_dt": "Delivery Note Item",
- "second_source_field": "-1 * qty",
- "second_join_field": "so_detail",
- "extra_cond": """ and exists (select name from `tabSales Invoice` where name=`tabSales Invoice Item`.parent and update_stock=1 and is_return=1)""",
- }
- )
+ if not cint(self.update_stock):
+ return
+
+ self.status_updater.append(
+ {
+ "source_dt": "Sales Invoice Item",
+ "target_dt": "Sales Order Item",
+ "target_parent_dt": "Sales Order",
+ "target_parent_field": "per_delivered",
+ "target_field": "delivered_qty",
+ "target_ref_field": "qty",
+ "source_field": "qty",
+ "join_field": "so_detail",
+ "percent_join_field": "sales_order",
+ "status_field": "delivery_status",
+ "keyword": "Delivered",
+ "second_source_dt": "Delivery Note Item",
+ "second_source_field": "qty",
+ "second_join_field": "so_detail",
+ "overflow_type": "delivery",
+ "extra_cond": """ and exists(select name from `tabSales Invoice`
+ where name=`tabSales Invoice Item`.parent and update_stock = 1)""",
+ }
+ )
+
+ if not cint(self.is_return):
+ return
+
+ self.status_updater.append(
+ {
+ "source_dt": "Sales Invoice Item",
+ "target_dt": "Sales Order Item",
+ "join_field": "so_detail",
+ "target_field": "returned_qty",
+ "target_parent_dt": "Sales Order",
+ "source_field": "-1 * qty",
+ "second_source_dt": "Delivery Note Item",
+ "second_source_field": "-1 * qty",
+ "second_join_field": "so_detail",
+ "extra_cond": """ and exists (select name from `tabSales Invoice` where name=`tabSales Invoice Item`.parent and update_stock=1 and is_return=1)""",
+ }
+ )
def check_credit_limit(self):
from erpnext.selling.doctype.customer.customer import check_credit_limit
@@ -662,13 +662,8 @@
def unlink_sales_invoice_from_timesheets(self):
for row in self.timesheets:
timesheet = frappe.get_doc("Timesheet", row.time_sheet)
- for time_log in timesheet.time_logs:
- if time_log.sales_invoice == self.name:
- time_log.sales_invoice = None
- timesheet.calculate_total_amounts()
- timesheet.calculate_percentage_billed()
+ timesheet.unlink_sales_invoice(self.name)
timesheet.flags.ignore_validate_update_after_submit = True
- timesheet.set_status()
timesheet.db_update_all()
@frappe.whitelist()
@@ -1011,12 +1006,17 @@
frappe.throw(_("Warehouse required for stock Item {0}").format(d.item_code))
def validate_delivery_note(self):
- for d in self.get("items"):
- if d.delivery_note:
- msgprint(
- _("Stock cannot be updated against Delivery Note {0}").format(d.delivery_note),
- raise_exception=1,
- )
+ """If items are linked with a delivery note, stock cannot be updated again."""
+ if not cint(self.update_stock):
+ return
+
+ notes = [item.delivery_note for item in self.items if item.delivery_note]
+ if notes:
+ frappe.throw(
+ _("Stock cannot be updated against the following Delivery Notes: {0}").format(
+ comma_and(notes)
+ ),
+ )
def validate_write_off_account(self):
if flt(self.write_off_amount) and not self.write_off_account:
@@ -1030,29 +1030,23 @@
msgprint(_("Please enter Account for Change Amount"), raise_exception=1)
def validate_dropship_item(self):
- for item in self.items:
- if item.sales_order:
- if frappe.db.get_value("Sales Order Item", item.so_detail, "delivered_by_supplier"):
- frappe.throw(_("Could not update stock, invoice contains drop shipping item."))
+ """If items are drop shipped, stock cannot be updated."""
+ if not cint(self.update_stock):
+ return
+
+ if any(item.delivered_by_supplier for item in self.items):
+ frappe.throw(
+ _(
+ "Stock cannot be updated because the invoice contains a drop shipping item. Please disable 'Update Stock' or remove the drop shipping item."
+ ),
+ )
def update_current_stock(self):
- for d in self.get("items"):
- if d.item_code and d.warehouse:
- bin = frappe.db.sql(
- "select actual_qty from `tabBin` where item_code = %s and warehouse = %s",
- (d.item_code, d.warehouse),
- as_dict=1,
- )
- d.actual_qty = bin and flt(bin[0]["actual_qty"]) or 0
+ for item in self.items:
+ item.set_actual_qty()
- for d in self.get("packed_items"):
- bin = frappe.db.sql(
- "select actual_qty, projected_qty from `tabBin` where item_code = %s and warehouse = %s",
- (d.item_code, d.warehouse),
- as_dict=1,
- )
- d.actual_qty = bin and flt(bin[0]["actual_qty"]) or 0
- d.projected_qty = bin and flt(bin[0]["projected_qty"]) or 0
+ for packed_item in self.packed_items:
+ packed_item.set_actual_and_projected_qty()
def update_packing_list(self):
if cint(self.update_stock) == 1:
@@ -1127,17 +1121,8 @@
return warehouse
def set_income_account_for_fixed_assets(self):
- disposal_account = depreciation_cost_center = None
- for d in self.get("items"):
- if d.is_fixed_asset:
- if not disposal_account:
- disposal_account, depreciation_cost_center = get_disposal_account_and_cost_center(
- self.company
- )
-
- d.income_account = disposal_account
- if not d.cost_center:
- d.cost_center = depreciation_cost_center
+ for item in self.items:
+ item.set_income_account_for_fixed_asset(self.company)
def check_prev_docstatus(self):
for d in self.get("items"):
@@ -1510,47 +1495,46 @@
)
if not skip_change_gl_entries:
- self.make_gle_for_change_amount(gl_entries)
+ gl_entries.extend(self.get_gle_for_change_amount())
- def make_gle_for_change_amount(self, gl_entries):
- if self.change_amount:
- if self.account_for_change_amount:
- gl_entries.append(
- self.get_gl_dict(
- {
- "account": self.debit_to,
- "party_type": "Customer",
- "party": self.customer,
- "against": self.account_for_change_amount,
- "debit": flt(self.base_change_amount),
- "debit_in_account_currency": flt(self.base_change_amount)
- if self.party_account_currency == self.company_currency
- else flt(self.change_amount),
- "against_voucher": self.return_against
- if cint(self.is_return) and self.return_against
- else self.name,
- "against_voucher_type": self.doctype,
- "cost_center": self.cost_center,
- "project": self.project,
- },
- self.party_account_currency,
- item=self,
- )
- )
+ def get_gle_for_change_amount(self) -> list[dict]:
+ if not self.change_amount:
+ return []
- gl_entries.append(
- self.get_gl_dict(
- {
- "account": self.account_for_change_amount,
- "against": self.customer,
- "credit": self.base_change_amount,
- "cost_center": self.cost_center,
- },
- item=self,
- )
- )
- else:
- frappe.throw(_("Select change amount account"), title=_("Mandatory Field"))
+ if not self.account_for_change_amount:
+ frappe.throw(_("Please set Account for Change Amount"), title=_("Mandatory Field"))
+
+ return [
+ self.get_gl_dict(
+ {
+ "account": self.debit_to,
+ "party_type": "Customer",
+ "party": self.customer,
+ "against": self.account_for_change_amount,
+ "debit": flt(self.base_change_amount),
+ "debit_in_account_currency": flt(self.base_change_amount)
+ if self.party_account_currency == self.company_currency
+ else flt(self.change_amount),
+ "against_voucher": self.return_against
+ if cint(self.is_return) and self.return_against
+ else self.name,
+ "against_voucher_type": self.doctype,
+ "cost_center": self.cost_center,
+ "project": self.project,
+ },
+ self.party_account_currency,
+ item=self,
+ ),
+ self.get_gl_dict(
+ {
+ "account": self.account_for_change_amount,
+ "against": self.customer,
+ "credit": self.base_change_amount,
+ "cost_center": self.cost_center,
+ },
+ item=self,
+ ),
+ ]
def make_write_off_gl_entry(self, gl_entries):
# write off entries, applicable if only pos
@@ -1659,48 +1643,9 @@
"""
validate serial number agains Delivery Note and Sales Invoice
"""
- self.set_serial_no_against_delivery_note()
- self.validate_serial_against_delivery_note()
-
- def set_serial_no_against_delivery_note(self):
for item in self.items:
- if item.serial_no and item.delivery_note and item.qty != len(get_serial_nos(item.serial_no)):
- item.serial_no = get_delivery_note_serial_no(item.item_code, item.qty, item.delivery_note)
-
- def validate_serial_against_delivery_note(self):
- """
- validate if the serial numbers in Sales Invoice Items are same as in
- Delivery Note Item
- """
-
- for item in self.items:
- if not item.delivery_note or not item.dn_detail:
- continue
-
- serial_nos = frappe.db.get_value("Delivery Note Item", item.dn_detail, "serial_no") or ""
- dn_serial_nos = set(get_serial_nos(serial_nos))
-
- serial_nos = item.serial_no or ""
- si_serial_nos = set(get_serial_nos(serial_nos))
- serial_no_diff = si_serial_nos - dn_serial_nos
-
- if serial_no_diff:
- dn_link = frappe.utils.get_link_to_form("Delivery Note", item.delivery_note)
- serial_no_msg = ", ".join(frappe.bold(d) for d in serial_no_diff)
-
- msg = _("Row #{0}: The following Serial Nos are not present in Delivery Note {1}:").format(
- item.idx, dn_link
- )
- msg += " " + serial_no_msg
-
- frappe.throw(msg=msg, title=_("Serial Nos Mismatch"))
-
- if item.serial_no and cint(item.qty) != len(si_serial_nos):
- frappe.throw(
- _("Row #{0}: {1} Serial numbers required for Item {2}. You have provided {3}.").format(
- item.idx, item.qty, item.item_code, len(si_serial_nos)
- )
- )
+ item.set_serial_no_against_delivery_note()
+ item.validate_serial_against_delivery_note()
def update_project(self):
if self.project:
diff --git a/erpnext/accounts/doctype/sales_invoice_item/sales_invoice_item.py b/erpnext/accounts/doctype/sales_invoice_item/sales_invoice_item.py
index 9be1b42..f92a7a8 100644
--- a/erpnext/accounts/doctype/sales_invoice_item/sales_invoice_item.py
+++ b/erpnext/accounts/doctype/sales_invoice_item/sales_invoice_item.py
@@ -2,7 +2,13 @@
# License: GNU General Public License v3. See license.txt
+import frappe
+from frappe import _
from frappe.model.document import Document
+from frappe.utils.data import cint
+
+from erpnext.assets.doctype.asset.depreciation import get_disposal_account_and_cost_center
+from erpnext.stock.doctype.serial_no.serial_no import get_delivery_note_serial_no, get_serial_nos
class SalesInvoiceItem(Document):
@@ -92,4 +98,67 @@
weight_uom: DF.Link | None
# end: auto-generated types
- pass
+ def validate_cost_center(self, company: str):
+ cost_center_company = frappe.get_cached_value("Cost Center", self.cost_center, "company")
+ if cost_center_company != company:
+ frappe.throw(
+ _("Row #{0}: Cost Center {1} does not belong to company {2}").format(
+ frappe.bold(self.idx), frappe.bold(self.cost_center), frappe.bold(company)
+ )
+ )
+
+ def set_actual_qty(self):
+ if self.item_code and self.warehouse:
+ self.actual_qty = (
+ frappe.db.get_value(
+ "Bin", {"item_code": self.item_code, "warehouse": self.warehouse}, "actual_qty"
+ )
+ or 0
+ )
+
+ def set_income_account_for_fixed_asset(self, company: str):
+ """Set income account for fixed asset item based on company's disposal account and cost center."""
+ if not self.is_fixed_asset:
+ return
+
+ disposal_account, depreciation_cost_center = get_disposal_account_and_cost_center(company)
+
+ self.income_account = disposal_account
+ if not self.cost_center:
+ self.cost_center = depreciation_cost_center
+
+ def set_serial_no_against_delivery_note(self):
+ """Set serial no based on delivery note."""
+ if self.serial_no and self.delivery_note and self.qty != len(get_serial_nos(self.serial_no)):
+ self.serial_no = get_delivery_note_serial_no(self.item_code, self.qty, self.delivery_note)
+
+ def validate_serial_against_delivery_note(self):
+ """Ensure the serial numbers in this Sales Invoice Item are same as in the linked Delivery Note."""
+ if not self.delivery_note or not self.dn_detail:
+ return
+
+ serial_nos = frappe.db.get_value("Delivery Note Item", self.dn_detail, "serial_no") or ""
+ dn_serial_nos = set(get_serial_nos(serial_nos))
+
+ serial_nos = self.serial_no or ""
+ si_serial_nos = set(get_serial_nos(serial_nos))
+ serial_no_diff = si_serial_nos - dn_serial_nos
+
+ if serial_no_diff:
+ dn_link = frappe.utils.get_link_to_form("Delivery Note", self.delivery_note)
+ msg = (
+ _("Row #{0}: The following serial numbers are not present in Delivery Note {1}:").format(
+ self.idx, dn_link
+ )
+ + " "
+ + ", ".join(frappe.bold(d) for d in serial_no_diff)
+ )
+
+ frappe.throw(msg=msg, title=_("Serial Nos Mismatch"))
+
+ if self.serial_no and cint(self.qty) != len(si_serial_nos):
+ frappe.throw(
+ _(
+ "Row #{0}: {1} serial numbers are required for Item {2}. You have provided {3} serial numbers."
+ ).format(self.idx, self.qty, self.item_code, len(si_serial_nos))
+ )
diff --git a/erpnext/projects/doctype/timesheet/timesheet.py b/erpnext/projects/doctype/timesheet/timesheet.py
index e26d04a..d701496 100644
--- a/erpnext/projects/doctype/timesheet/timesheet.py
+++ b/erpnext/projects/doctype/timesheet/timesheet.py
@@ -256,6 +256,16 @@
if not ts_detail.is_billable:
ts_detail.billing_rate = 0.0
+ def unlink_sales_invoice(self, sales_invoice: str):
+ """Remove link to Sales Invoice from all time logs."""
+ for time_log in self.time_logs:
+ if time_log.sales_invoice == sales_invoice:
+ time_log.sales_invoice = None
+
+ self.calculate_total_amounts()
+ self.calculate_percentage_billed()
+ self.set_status()
+
@frappe.whitelist()
def get_projectwise_timesheet_data(project=None, parent=None, from_time=None, to_time=None):
diff --git a/erpnext/stock/doctype/packed_item/packed_item.py b/erpnext/stock/doctype/packed_item/packed_item.py
index c5fed0d..d81ceaf 100644
--- a/erpnext/stock/doctype/packed_item/packed_item.py
+++ b/erpnext/stock/doctype/packed_item/packed_item.py
@@ -51,7 +51,16 @@
warehouse: DF.Link | None
# end: auto-generated types
- pass
+ def set_actual_and_projected_qty(self):
+ "Set actual and projected qty based on warehouse and item_code"
+ _bin = frappe.db.get_value(
+ "Bin",
+ {"item_code": self.item_code, "warehouse": self.warehouse},
+ ["actual_qty", "projected_qty"],
+ as_dict=True,
+ )
+ self.actual_qty = _bin.actual_qty if _bin else 0
+ self.projected_qty = _bin.projected_qty if _bin else 0
def make_packing_list(doc):