fix: add validation for tax account types

[rehanrehman389]

Problem:
Currently tax account validation only exists on the frontend through link field filters. This can be bypassed through:

• API calls
• Data imports

[coderabbitai]

📝 Walkthrough

Walkthrough

A new validation method validate_tax_account_type() has been added to the AccountsController class. This method validates that tax accounts have the correct account type based on the transaction context. It determines whether a document is a Sales transaction (invoices, orders, quotations, delivery notes) or a Purchase transaction, then validates that each tax entry's account type matches the allowed types for that transaction category. The validation is invoked during the standard validation flow alongside existing tax validations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify the logic that distinguishes between Sales and Purchase transactions is correct
• Confirm the allowed account type sets for each transaction category are accurate and complete
• Review error message clarity and user guidance for when validation fails
• Ensure the validation is placed at an appropriate point in the validation flow and doesn't duplicate existing checks

Suggested reviewers

• ruthra-kumar

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding validation for tax account types.
Description check	✅ Passed	The description explains the problem being solved: tax account validation exists only on frontend and can be bypassed via API calls and data imports.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc78109 and 9c17bd2.

📒 Files selected for processing (1)

• erpnext/controllers/accounts_controller.py (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Patch Test
• GitHub Check: Python Unit Tests (3)
• GitHub Check: Python Unit Tests (2)
• GitHub Check: Python Unit Tests (4)
• GitHub Check: Python Unit Tests (1)
• GitHub Check: Summary

🔇 Additional comments (2)

erpnext/controllers/accounts_controller.py (2)

252-252: LGTM: Logical placement of the new validation.

The new tax account type validation is appropriately placed alongside the existing validate_tax_account_company() check, keeping related tax validations grouped together.

---

1234-1234: Consider validating accounts with missing account_type.

The condition if account_type and account_type not in valid_types skips validation when account_type is None or empty. While this may be intentional to handle accounts without a type, it could also be a validation gap.

Please verify: Should accounts used for taxes be required to have an account_type set? If so, consider adding a validation for missing account_type:

 		if account_type and account_type not in valid_types:
+			# existing validation
+		elif not account_type:
+			frappe.throw(
+				_("Row #{0}: Account {1} must have an Account Type set for use in taxes.").format(
+					d.idx, frappe.bold(d.account_head)
+				),
+				title=_("Missing Account Type")
+			)

If allowing accounts without a type is intentional, this comment can be ignored.

[coderabbitai]

⚠️ 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.

[diptanilsaha]

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

[coderabbitai]

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!

[rehanrehman389]

@diptanilsaha #50350