fix: Recalculate payment amounts instead of refetching the schedule.

[bosue]

Fixes #38030.

Reverts #35007 while still fixing the issue reported there.

Reverts #35285 while still fixing the issue reported there.

Also reverts #36440. Unsure whether allocate_payment_based_on_payment_terms is useful at all – either you have a schedule and allocate accordingly or you don’t. But even if turned off, we need the payment schedule to be correct. Otherwise we needed to clear the schedule altogether.

Last mostly correct commit was #34872, but we need to doublecheck if every single of the later reported cases are still covered.

Definitely needs more test cases and optimally some feedback by @ruthra-kumar, @GursheenK, and/or @deepeshgarg007.

[codecov]

Codecov Report

Merging #38044 (c084a06) into develop (8d9f391) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head c084a06 differs from pull request most recent head 479a824. Consider uploading reports for the commit 479a824 to get more accurate results

@@             Coverage Diff             @@
##           develop   #38044      +/-   ##
===========================================
- Coverage    67.39%   67.39%   -0.01%     
===========================================
  Files          757      757              
  Lines        60379    60376       -3     
===========================================
- Hits         40692    40689       -3     
  Misses       19687    19687              

Files	Coverage Δ
erpnext/controllers/accounts_controller.py	84.81% <100.00%> (-0.03%)	⬇️

[bosue]

Marked as draft as we need additional tests. First I will reinstate Gursheen‘s test from #36440.

Besides that, please review.

Also consider #38031: There may be a point in removing allocate_payment_based_on_payment_terms altogether. You either have a Payment Schedule and make sure payments are allocated accordingly. Or you don‘t.

And #38029: When the Sales Order already has a Payment Schedule, it‘s hard to imagine why you wouldn‘t preserve it in the Sales Invoice.

Simplifying the logic here would reduce complexity both in code and in the UI:

1. If the Sales Order has a Payment Schedule, always fetch it in the Sales Invoice
2. If it doesn‘t, always fetch the customer’s default template.
3. If you end up with an invalid Payment Schedule, always fix it.
4. If you end up with a valid Payment Schedule, always use it.
5. Otherwise go ahead without a Payment Schedule.

[bosue]

Gursheen‘s test still fails. Need to look into what exactly is expected there and why, and add more testing.
The above questions aren‘t diminished by that, so nonetheless pls review.

[stale]

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

[bosue]

I still think both my assessment and the solution was correct, but apologize for putting you guys off with a premature PR. Will only reopen once fully investigated.