Skip to content

fix: handle zero net pay in payroll entry while linking journal entry.#4183

Open
krishna-254 wants to merge 3 commits intofrappe:developfrom
krishna-254:fix/payroll-entry-net-pay-zero
Open

fix: handle zero net pay in payroll entry while linking journal entry.#4183
krishna-254 wants to merge 3 commits intofrappe:developfrom
krishna-254:fix/payroll-entry-net-pay-zero

Conversation

@krishna-254
Copy link
Copy Markdown
Contributor

@krishna-254 krishna-254 commented Feb 26, 2026

fixes #3972
Depends on frappe/erpnext#53002

  • Allow row with zero credit and debit when its reference type is Payroll Entry in journal entry.
  • the journal will be now linked allowing it to be easily change on change in payroll entry.
  • Added test case for this case.
image

Summary by CodeRabbit

  • Bug Fixes

    • Stricter validation on journal entry rows to prevent rows with both debit and credit as zero (with expected exceptions for certain voucher types), surfacing clearer errors during save/submit.
    • Payroll accounting now includes payroll-payable rows even when amounts are zero, ensuring complete accounting records.
  • Tests

    • Added coverage for payroll entries with zero amounts to prevent regressions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 26, 2026

Walkthrough

Adds an override mapping to use a new HRMSJournalEntry class for Journal Entry doctypes, introducing custom debit/credit validation that exempts payroll rows and Exchange Gain Or Loss multi-currency vouchers. Adjusts payroll entry accounting to append payroll-payable rows when amount is zero for payable entries. Adds a test covering payroll entries with zero net pay and related journal entry creation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary fix: handling zero net pay in payroll entry while linking journal entries.
Linked Issues check ✅ Passed All code changes directly address issue #3972: zero-amount journal entry rows are now allowed when reference_type is Payroll Entry, preserving the Journal Entry link and enabling proper cancellation cascading.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #3972: overriding Journal Entry validation, modifying Payroll Entry row appending logic, and adding corresponding test coverage.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
hrms/payroll/doctype/payroll_entry/test_payroll_entry.py (1)

1109-1115: Assert the zero debit/credit payroll-payable row explicitly.

The current check confirms linkage, but not the exact zero-amount row behavior introduced by this fix. Please assert debit == 0 and credit == 0 on the payroll-payable reference row.

✅ Suggested test tightening
-		journal_entry_name = frappe.db.get_value(
-			"Journal Entry Account",
-			{"reference_type": "Payroll Entry", "reference_name": payroll_entry.name},
-			"parent",
-		)
-		self.assertIsNotNone(journal_entry_name, "Journal Entry should be created for zero net pay")
+		payable_row = frappe.db.get_value(
+			"Journal Entry Account",
+			{
+				"reference_type": "Payroll Entry",
+				"reference_name": payroll_entry.name,
+				"account": company.default_payroll_payable_account,
+			},
+			["parent", "debit", "credit"],
+			as_dict=True,
+		)
+		self.assertIsNotNone(payable_row, "Payroll payable row should be linked to Payroll Entry")
+		self.assertEqual(flt(payable_row.debit), 0)
+		self.assertEqual(flt(payable_row.credit), 0)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hrms/payroll/doctype/payroll_entry/test_payroll_entry.py` around lines 1109 -
1115, Update the test in test_payroll_entry.py to not only assert that a Journal
Entry was linked (journal_entry_name) but also fetch the specific "Journal Entry
Account" row for the payroll-payable reference and assert its debit and credit
are zero; you can locate the row via the same query keys {"reference_type":
"Payroll Entry", "reference_name": payroll_entry.name} (use frappe.db.get_value
to retrieve "debit, credit" or frappe.get_doc on the parent Journal Entry and
find the child row) and add assertions self.assertEqual(debit, 0) and
self.assertEqual(credit, 0) to confirm the zero-amount payroll-payable row.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hrms/payroll/doctype/payroll_entry/payroll_entry.py`:
- Around line 855-856: The current condition (if amt or (amt >= 0 and account ==
self.payroll_payable_account):) can append zero-amount earnings/deductions when
their account equals self.payroll_payable_account; change the check so
zero-value rows are appended only when they are explicit payroll-payable rows —
e.g. replace the condition with an explicit test like: if amt != 0 or (amt == 0
and account == self.payroll_payable_account and row.get('is_payroll_payable')):
(ensure the code that builds row sets row['is_payroll_payable'] for genuine
payable entries), referencing amt, account, self.payroll_payable_account and
accounts.append(row).

---

Nitpick comments:
In `@hrms/payroll/doctype/payroll_entry/test_payroll_entry.py`:
- Around line 1109-1115: Update the test in test_payroll_entry.py to not only
assert that a Journal Entry was linked (journal_entry_name) but also fetch the
specific "Journal Entry Account" row for the payroll-payable reference and
assert its debit and credit are zero; you can locate the row via the same query
keys {"reference_type": "Payroll Entry", "reference_name": payroll_entry.name}
(use frappe.db.get_value to retrieve "debit, credit" or frappe.get_doc on the
parent Journal Entry and find the child row) and add assertions
self.assertEqual(debit, 0) and self.assertEqual(credit, 0) to confirm the
zero-amount payroll-payable row.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 804c915 and 052214f.

📒 Files selected for processing (4)
  • hrms/hooks.py
  • hrms/overrides/journal_entry.py
  • hrms/payroll/doctype/payroll_entry/payroll_entry.py
  • hrms/payroll/doctype/payroll_entry/test_payroll_entry.py

@github-actions
Copy link
Copy Markdown

This pull request is being marked as inactive because of no recent activity.
If your PR hasn't been reviewed, it's likely because it doesn't fullfill the contribution guidelines. Please read them carefully and fix the pull request. When you are sure all items are checked, please ping relevant codeowner in the comment. Be nice, they have a lot on their plate too.

It will be closed in 3 days if no further activity occurs.
Thank you for contributing!

@github-actions
Copy link
Copy Markdown

This pull request is being marked as inactive because of no recent activity.
If your PR hasn't been reviewed, it's likely because it doesn't fullfill the contribution guidelines. Please read them carefully and fix the pull request. When you are sure all items are checked, please ping relevant codeowner in the comment. Be nice, they have a lot on their plate too.

It will be closed in 3 days if no further activity occurs.
Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Payroll Entry with 0 Net Pay Creates Unlinked Journal Entry

3 participants