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

Autofetch Payment Terms clears Terms Template #38030

Open
bosue opened this issue Nov 9, 2023 · 1 comment
Open

Autofetch Payment Terms clears Terms Template #38030

bosue opened this issue Nov 9, 2023 · 1 comment
Labels

Comments

@bosue
Copy link
Contributor

bosue commented Nov 9, 2023

Information about bug

With Autofetch Payment Terms being enabled in Accounts Settings, saving a Sales Invoice clears the Payment Terms Template:

RPReplay_Final1699568332.mov

Consequence is that the “Allocate Payment Based On Payment Terms” setting that isn’t copied over to the transaction, gets lost and won’t be adhered to in partial payments.

With Autofetch Payment Terms being disabled, the Payment Terms Template is properly preserved:

RPReplay_Final1699568297.mov

Module

accounts, selling

Version

Frappe / ERPNext: dev

Installation method

None

Relevant log output / Stack trace / Full Error Message.

No response

@bosue
Copy link
Contributor Author

bosue commented Nov 11, 2023

This bug was introduced with #35285.

The lines added will (try to) fetch payment terms from the Sales Order, even if payment_terms_template and payment_schedule are already set. I think we need to be more precise as to which validation should be skipped and which shouldn‘t. Unfortunately no tests seems to have caught one or the other.

A better fix for the issue described in #35285 should be leaving out these altogether superfluous validations:

-               if not (
-                       automatically_fetch_payment_terms
-                       and allocate_payment_based_on_payment_terms
-                       and self.linked_order_has_payment_terms(po_or_so, fieldname, doctype)
-               ):
+               if self.get("payment_schedule"):
                         [recalculate amounts as per payment schedule]

This step is about recalculating schedule amounts, particularly in the case of a partial invoice or any other change in the total amount.

This step is not about (re)fetching the payment schedule itself. So all we need to know is if there‘s a payment schedule now – yes or no.

allocate_payment_based_on_payment_terms is irrelevant, too, as we want the payment schedule to be correct(ed) either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant