Skip to content

fix: add validation for tax account types #50375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+25 −0
Open

fix: add validation for tax account types #50375

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions erpnext/controllers/accounts_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ def validate(self):
if self.meta.get_field("taxes_and_charges"):
self.validate_enabled_taxes_and_charges()
self.validate_tax_account_company()
self.validate_tax_account_type()

self.set_taxes_and_charges()

Expand Down Expand Up @@ -1218,6 +1219,30 @@ def validate_tax_account_company(self):
)
)

def validate_tax_account_type(self):
for d in self.get("taxes"):
if d.account_head:
account_type = frappe.get_cached_value("Account", d.account_head, "account_type")

if self.doctype in ["Sales Invoice", "Sales Order", "Quotation", "Delivery Note"]:
valid_types = ["Tax", "Chargeable", "Indirect Expense"]
transaction_type = "Sales"
else:
valid_types = ["Tax", "Chargeable", "Income Account", "Expenses Included In Valuation"]
transaction_type = "Purchase"
Comment on lines +1227 to +1232
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add POS Invoice to Sales document validation and verify else clause coverage.

Two potential issues:

  1. Missing POS Invoice: Line 1227 lists Sales documents but omits "POS Invoice", which is also a sales transaction. Without this, POS Invoices would incorrectly fall through to Purchase validation rules.

  2. Implicit Purchase assumption: The else clause (line 1230) assumes all non-Sales documents are Purchase transactions without explicit validation. This could cause incorrect validation if the method is called from unexpected document types.

Apply this diff to address both concerns:

-		if self.doctype in ["Sales Invoice", "Sales Order", "Quotation", "Delivery Note"]:
+		if self.doctype in ["Sales Invoice", "POS Invoice", "Sales Order", "Quotation", "Delivery Note"]:
 			valid_types = ["Tax", "Chargeable", "Indirect Expense"]
 			transaction_type = "Sales"
-		else:
+		elif self.doctype in ["Purchase Invoice", "Purchase Order", "Purchase Receipt"]:
 			valid_types = ["Tax", "Chargeable", "Income Account", "Expenses Included In Valuation"]
 			transaction_type = "Purchase"
+		else:
+			# Skip validation for document types that don't have tax validation requirements
+			return
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if self.doctype in ["Sales Invoice", "Sales Order", "Quotation", "Delivery Note"]:
valid_types = ["Tax", "Chargeable", "Indirect Expense"]
transaction_type = "Sales"
else:
valid_types = ["Tax", "Chargeable", "Income Account", "Expenses Included In Valuation"]
transaction_type = "Purchase"
if self.doctype in ["Sales Invoice", "POS Invoice", "Sales Order", "Quotation", "Delivery Note"]:
valid_types = ["Tax", "Chargeable", "Indirect Expense"]
transaction_type = "Sales"
elif self.doctype in ["Purchase Invoice", "Purchase Order", "Purchase Receipt"]:
valid_types = ["Tax", "Chargeable", "Income Account", "Expenses Included In Valuation"]
transaction_type = "Purchase"
else:
# Skip validation for document types that don't have tax validation requirements
return
🤖 Prompt for AI Agents 
In erpnext/controllers/accounts_controller.py around lines 1227 to 1232, the
sales-docs list omits "POS Invoice" and the else branch assumes every non-sales
document is a purchase; update the sales check to include "POS Invoice" and
replace the broad else with an explicit check for known Purchase document types
(e.g., "Purchase Invoice", "Purchase Order", "Purchase Receipt", etc.), setting
valid_types and transaction_type="Purchase" only for those, and raise or log a
clear error if the doctype is unexpected so the method doesn't silently apply
purchase rules to unknown documents.

Copy link
Member

@diptanilsaha diptanilsaha Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rehanrehman389 This appears to be legitimate. Please share your thoughts on it.

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

@rehanrehman389 rehanrehman389 Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diptanilsaha #50350


if account_type and account_type not in valid_types:
msg = (
_("Row #{0}: Account {1} of type {2} is not allowed for {3} taxes.").format(
d.idx, frappe.bold(d.account_head), frappe.bold(account_type), transaction_type
)
+ " "
)
msg += _("Please select an account with type: {0}").format(
", ".join([frappe.bold(t) for t in valid_types])
)
frappe.throw(msg, title=_("Invalid Tax Account"))

def get_gl_dict(self, args, account_currency=None, item=None):
"""this method populates the common properties of a gl entry record"""

Expand Down