refactor: cr/dr note should be standalone even when created from another invoice

[ruthra-kumar]

Credit/Debit Notes should post ledger against itself(standalone), even if they are created against a parent invoice. This way, they can be reconciled against a different invoice from same customer/supplier.

[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.

[barredterra]

Hi Ruthra, I wanted to review this and here's what I did:

1. Create a Credit Note against an existing, overdue Sales Invoice
2. Create a new Sales Invoice

I observed two things:

• The old Sales Invoice, that was returned, didn't get marked as "Credit Note Issued". Is this on purpose?
• The Credit Note did not become available for reconciliation with the new invoice. Neither pre-submit via "Fetch advance payments" nor post-submit via Payment Reconciliation.

[ruthra-kumar]

@barredterra Thanks for reviewing this.

Due to other priority tasks, this change has been in the backburner. Will revist it probably by this weekend.

• The old Sales Invoice, that was returned, didn't get marked as "Credit Note Issued". Is this on purpose?

No. But, I think this can addressed. Most probably some refactor required in the set_status method.

• The Credit Note did not become available for reconciliation with the new invoice. Neither pre-submit via "Fetch advance payments" nor post-submit via Payment Reconciliation.

I did notice this when I initially made the change. Payment Reconciliation tool filters Cr/Dr Notes that have its return_against field set. So, even though ledger entries are posted against itself, the tool ignore them.

erpnext/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py

Lines 176 to 178 in b4db5e9

	return_invoices = [
	x for x in self.return_invoices if x.return_against == None or x.return_against == ""
	]

[barredterra]

Linking this discussion her, for reference: #36170

[dj12djdjs]

@ruthra-kumar I think #36049 needs to ship with this PR. Without it, the negative outstanding won't be reconcilable via Payment Reconciliation

[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.

[codecov]

Codecov Report

Merging #35330 (60eee56) into develop (e64b004) will increase coverage by 0.49%.
Report is 150 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35330      +/-   ##
===========================================
+ Coverage    64.70%   65.19%   +0.49%     
===========================================
  Files          791      792       +1     
  Lines        61572    61854     +282     
===========================================
+ Hits         39838    40327     +489     
+ Misses       21734    21527     -207     

Files Changed	Coverage Δ
...e/payment_reconciliation/payment_reconciliation.py	93.68% <100.00%> (+5.59%)	⬆️
...ounts/doctype/purchase_invoice/purchase_invoice.py	84.04% <100.00%> (-0.15%)	⬇️
...xt/accounts/doctype/sales_invoice/sales_invoice.py	83.79% <100.00%> (-0.09%)	⬇️

... and 44 files with indirect coverage changes

[ruthra-kumar]

PR is in working state now.

Changes to existing design:

1. Credit/Debit Notes by default will be available in Payment Reconciliation tool.
2. Since Credit/Debit notes are posting ledger entries against itself, they will have -ve outstanding and not the parent invoice.

[ruthra-kumar]

I think #36049 needs to ship with this PR. Without it, the negative outstanding won't be reconcilable via Payment Reconciliation

@dj12djdjs Due to

Since Credit/Debit notes are posting ledger entries against itself, they will have -ve outstanding and not the parent invoice.

Invoice will not have -ve outstanding and will not be reported by reconciliation tool. Rather the Cr/Dr note will show up there.
So, no need for #36049

[ruthra-kumar]

@barredterra

The old Sales Invoice, that was returned, didn't get marked as "Credit Note Issued". Is this on purpose?

With this change, we might be breaking the requirement for this status. An Invoice requires,

1. outstanding_amount to be -ve
2. return_against of a Cr note to reference this
3. It itself should not be a Return.

to get Credit Note Issued status.

Due to,

Since Credit/Debit notes are posting ledger entries against itself, they will have -ve outstanding and not the parent invoice.

[1] will not be met, so Credit Note Issued will never be set.

We can simply remove [1] from the requirement, then it breaks the users expection that if Credit note is issued, then outstanding will be -ve. But, i think this is unavoidable and should be expected going forward.

Thoughts?

[barredterra]

Makes sense 👍