Skip to content
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.

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

fix: correct bank entry calculation with deductions and prevent multiple loan repayment deductions #2347

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

Nihantra-Patel
Copy link
Contributor

@Nihantra-Patel Nihantra-Patel commented Oct 28, 2024

Introduced via #2283 & #2324 and loan flow.

Issue:

  • When deductions were present, the bank entry calculation did not work as expected.
  • total_loan_repayment was being subtracted multiple times for each salary detail, leading to an incorrect salary_slip_total in the bank entry.

Resolution:

  • Adjusted the get_salary_slip_details function to ensure total_loan_repayment is fetched only once per unique salary slip.
  • In the make_bank_entry function, we separated the total_loan_repayment deduction from the main loop, ensuring it’s only subtracted once per salary slip, after calculating the total of earnings and deductions.

@ruchamahabal
Copy link
Member

Can you add a test to ensure this doesn't break?

@Nihantra-Patel
Copy link
Contributor Author

Can you add a test to ensure this doesn't break?

added in previous PR: https://github.com/frappe/hrms/pull/2283/files#diff-cf6a883b62bb2053bba145dc2c7501552ea94f0b170188dd3a8d8ce1771028c1

@ruchamahabal
Copy link
Member

ruchamahabal commented Oct 28, 2024

added in previous PR: #2283 (files)

The test is mostly of no use since it doesn't assert the loan amount deduction from salary slip total. Else, the above error could have been caught with the test

Copy link
Member

@ruchamahabal ruchamahabal left a comment

Choose a reason for hiding this comment

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

Thanks for being patient with the review 🙈 :D

@ruchamahabal ruchamahabal merged commit cdf1e90 into frappe:develop Oct 28, 2024
7 checks passed
ruchamahabal added a commit that referenced this pull request Oct 28, 2024
…2347

fix: correct bank entry calculation with deductions and prevent multiple loan repayment deductions (backport #2347)
@ruchamahabal
Copy link
Member

@Mergifyio backport version-15

Copy link
Contributor

mergify bot commented Oct 29, 2024

backport version-15

✅ Backports have been created

ruchamahabal added a commit that referenced this pull request Oct 29, 2024
…ple loan repayment deductions (backport #2347) (#2356)

Co-authored-by: Nihantra Patel <nihantra@frappe.io>
Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request Oct 29, 2024
## [15.33.1](v15.33.0...v15.33.1) (2024-10-29)

### Bug Fixes

* correct bank entry calculation with deductions and prevent multiple loan repayment deductions (backport [#2347](#2347)) ([#2356](#2356)) ([372e552](372e552))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants