fix(payment_entry): merge GL entries with similar account heads based on setting

[khushi8112]

Closes: #48790

[coderabbitai]

📝 Walkthrough

Walkthrough

PaymentEntry.make_gl_entries now reads the Accounts Settings value merge_similar_account_heads and passes it as a merge_entries argument into process_gl_map and make_gl_entries calls, enabling optional consolidation of GL entries when multiple deductions use the same account head. The cancel path and existing exchange gain/loss journal calls remain unchanged. Tests were added/updated to verify merged vs. separate GL entries when the setting is enabled or disabled, and an existing multi-currency test was adjusted to enable the merge setting before assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implementation conflicts with the linked issue #48790. The issue requests separate GL entries per row (like Journal Entry), but the PR implements optional merging based on a setting, which contradicts the requirement.	Clarify the intended behavior: either implement mandatory per-row GL entries matching Journal Entry behavior, or update the issue to reflect the optional merging approach with appropriate justification.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Out of Scope Changes check	❓ Inconclusive	Changes are mostly in-scope, but enabling the merge setting in test_purchase_invoice_advance_taxes appears tangential to the Payment Entry GL merge feature implementation.	Verify whether the purchase invoice test change is necessary for the Payment Entry functionality or if it should be in a separate PR addressing broader GL entry merging.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: enabling GL entry merging based on an Accounts Settings flag in Payment Entry.
Description check	✅ Passed	The description references the closed issue #48790, which is directly related to the changeset regarding GL entry handling in Payment Entry.

✨ Finishing touches

• 📝 Generate docstrings

---

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: 0

🧹 Nitpick comments (1)

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py (1)

1503-1504: Consider using the @IntegrationTestCase.change_settings decorator for proper test isolation.

Setting merge_similar_account_heads directly without cleanup can leave the setting modified after the test completes, potentially affecting other tests. The decorator pattern (already used in this file at lines 512, 1294, 1497, etc.) automatically restores the original value after the test.

Suggested approach using decorator

Remove lines 1503-1504 and modify the decorator at line 1497-1499:

 	@IntegrationTestCase.change_settings(
-		"Accounts Settings", {"unlink_payment_on_cancellation_of_invoice": 1}
+		"Accounts Settings", {"unlink_payment_on_cancellation_of_invoice": 1, "merge_similar_account_heads": 1}
 	)
 	def test_purchase_invoice_advance_taxes(self):
 		from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
 
-		frappe.db.set_single_value("Accounts Settings", "merge_similar_account_heads", 1)
-
 		company = "_Test Company"

Alternatively, if the setting must be modified within the test body, restore it afterward:

+		original_value = frappe.db.get_single_value("Accounts Settings", "merge_similar_account_heads")
 		frappe.db.set_single_value("Accounts Settings", "merge_similar_account_heads", 1)
 
 		company = "_Test Company"
 		# ... rest of test ...
+		
+		# Cleanup
+		frappe.db.set_single_value("Accounts Settings", "merge_similar_account_heads", original_value)

📜 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 7baa75f and b8b5575.

📒 Files selected for processing (1)

• erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: ljain112
Repo: frappe/erpnext PR: 49963
File: erpnext/accounts/doctype/journal_entry/journal_entry.py:0-0
Timestamp: 2025-10-09T06:59:10.528Z
Learning: In erpnext/accounts/doctype/journal_entry/journal_entry.py, the validate() method calls validate_multi_currency() before apply_tax_withholding(). The validate_multi_currency() method internally calls set_exchange_rate(), so exchange rates are properly set on all account rows before TDS calculations occur in apply_tax_withholding().

Learnt from: vorasmit
Repo: frappe/erpnext PR: 51099
File: erpnext/patches/v16_0/migrate_tax_withholding_data.py:904-910
Timestamp: 2025-12-23T11:28:49.128Z
Learning: In the old Tax Withholding system (before the Tax Withholding Entry refactor), only one TCS account per Sales Invoice was supported. When reviewing migration code in erpnext/patches/v16_0/migrate_tax_withholding_data.py, the query selecting TCS accounts from GL Entries doesn't need explicit aggregation because historical data will only have one TCS account per invoice.

📚 Learning: 2025-12-23T11:28:49.128Z

Learnt from: vorasmit
Repo: frappe/erpnext PR: 51099
File: erpnext/patches/v16_0/migrate_tax_withholding_data.py:904-910
Timestamp: 2025-12-23T11:28:49.128Z
Learning: In the old Tax Withholding system (before the Tax Withholding Entry refactor), only one TCS account per Sales Invoice was supported. When reviewing migration code in erpnext/patches/v16_0/migrate_tax_withholding_data.py, the query selecting TCS accounts from GL Entries doesn't need explicit aggregation because historical data will only have one TCS account per invoice.

Applied to files:

• erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

📚 Learning: 2025-12-16T05:33:58.723Z

Learnt from: Abdeali099
Repo: frappe/erpnext PR: 51078
File: erpnext/accounts/doctype/financial_report_template/financial_report_engine.py:486-491
Timestamp: 2025-12-16T05:33:58.723Z
Learning: In ERPNext/Frappe codebase, query.run(as_dict=True) returns frappe._dict objects that support both dict-style access (obj["key"]) and attribute-style access (obj.key). Therefore, attribute access on query results is valid and will not raise AttributeError. When reviewing Python code, prefer attribute access (obj.key) for readability where the key is known to exist, but ensure existence checks or fallback handling if there is any doubt about missing keys.

Applied to files:

• erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

⏰ 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: Python Unit Tests (1)
• GitHub Check: Python Unit Tests (4)
• GitHub Check: Patch Test
• GitHub Check: Python Unit Tests (2)
• GitHub Check: Python Unit Tests (3)
• GitHub Check: Summary

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.05%. Comparing base (e1f9adf) to head (b8b5575).
⚠️ Report is 174 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #51453      +/-   ##
===========================================
+ Coverage    79.04%   79.05%   +0.01%     
===========================================
  Files         1179     1179              
  Lines       121265   121407     +142     
===========================================
+ Hits         95853    95981     +128     
- Misses       25412    25426      +14     

Files with missing lines	Coverage Δ
...xt/accounts/doctype/payment_entry/payment_entry.py	77.87% <100.00%> (+0.01%)	⬆️
...counts/doctype/payment_entry/test_payment_entry.py	99.67% <100.00%> (+<0.01%)	⬆️
.../doctype/purchase_invoice/test_purchase_invoice.py	97.79% <100.00%> (+<0.01%)	⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.