At the moment, it is difficult to use Payment Entry with custom doctypes as references because all of the validation logic is currently hardcoded. I am proposing to refactor Payment Entry to remove these doctype-specific references.
I am still relatively new to frappe, but I believe my approach would be to define a controller class that any custom doctype could inherit. This controller class would define methods specifying necessary validation parameters, which individual doctypes could then override with their own particular conditions.
I am writing now to solicit feedback, especially from the maintainers. Is there anything here I might not be considering?
Exactly. Right now, payment_entry.py contains a lot of code that looks like this:
if d.reference_doctype in ("Sales Invoice", "Purchase Invoice", "Expense Claim", "Fees"):
if self.party_type == "Customer":
ref_party_account = get_party_account_based_on_invoice_discounting(d.reference_name) or ref_doc.debit_to
elif self.party_type == "Student":
ref_party_account = ref_doc.receivable_account
elif self.party_type=="Supplier":
ref_party_account = ref_doc.credit_to
My proposal is to make the code in payment_entry generic (and thus compatible with any doctype), shifting any specific validation that needs to be done to helper methods in the reference doctype itself.
I could imagine the public developers telegram group may be a good place to get some more feedback and advice on how to technically approach this endeavor.
Yes do I want to refactor Payment Entry/Journal Entry to be generalized enough to be able to select any DocType that has Payable/Receivable entries, however, there are a few problems to solve before that can be achieved.
Non-Standard Fieldnames
Fieldnames are non-standard: credit_to, debit_to, customer, supplier, employee that requires the developer to use a lot of if/else/case conditions to get the correct fieldnames for different DocTypes. To generalize these fields I had the idea of allowing DocFields of type “Alias” that can allow the DocType to define fieldnames like party_account that can act as an alias to either credit_to or debit_to and party as an alias to either customer, supplier and so on. So the Alias DocFields will do the job of overridable methods but in an even more dynamic way. Either we can implement Alias DocFields OR we can just rename the fields to more standard fieldnames.
Non-Standard GL Entries
I have solved this problem already in my Payment System Enhancement PR. The problem was that Invoice’s Party Receivable/Payable GL Entries had against_voucher pointing to itself while Journal Entry way of making Payable/Receivable GL Entries was leaving against_voucher empty. This made the queries complicated with multiple conditions and specialized to specific DocTypes.
@peterg It’s nice to see someone taking interest in this area. If you are working on it, we can definitely discuss this, we could share our ideas. I have been messing around with the accounting module for quite some time now to share my knowledge :).
Hi @saifi0102, That’s great! I appreciate the offer to be involved, and I will be glad to benefit from your experience.
Can you say more about the Alias DocFields? I’ve not come across that mechanism up to this point. Is this an already existing field type, or would it need to be created? How would it be more dynamic than a well-defined interface?
The advantage (in my opinion) of the overridable interface approach is the clarity of its semantics. At least as I’ve always understood it, adding complexity to a data model in order to facilitate interaction between different types of objects is an undesirable practice in MVC design. It makes good integration testing more complicated, and it makes the code harder to understand.
In the end, though, this comes down to the philosophy of the architecture, so perhaps @rmehta could chime in. Is there a set of principals defining how doctypes should interact with each other?
No it’s not a feature yet, it’s just an idea right now.
For example you want to get the party of some invoice/voucher. The only option you have right now is to check what DocType the voucher is, if it is a Sales Invoice the party field would be doc.customer, if it’s a Purchase Invoice the party field would be doc.supplier.
It would be much easier to have an interface function or attribute that you can use to get the party … like doc.get_party() or doc.get_billing_party(). The alias DocField would help you do just that, doc.party would return the value of doc.customer if in Sales Invoice or doc.supplier if in Purchase Invoice. The DocField would have the definition of where to get the value from.
The interface method doc.get_party() will not tell you where it got the value from, for that you would have to add another interface method like doc.get_party_fieldname(). This solves the problem, but I find that the idea of an Alias type DocField would make it much more cleaner. You could get the value from doc.party and you could get which fieldname it is getting the value from by something like doc.meta.get_source_fieldname('party').
I agree with that. To solve the problem that you stated, you will definitely have to create a controller that defines how Payment Entry, Journal Entry and other accounting DocTypes should interact with with voucher documents. All I am saying is that you can achieve that with more beauty if you incorporate Alias DocFields.
I think we’re just going to have to disagree about whether or introducing “alias” fields is clean. To my mind, adding complexity to a data model in order to define controller-level operations is always a bad practice. Especially for a project with ERPNext’s complexity, clean semantics is well worth 2-3 extra lines of code.
In any case, I hope a maintainer will chime in to provide some direction here. I’ve searched around but haven’t been able to find design specifications by the team to provide guidance on these kinds of questions.