Payment System Enchancements: Generalizing invoices and payments

[SaiFi0102]

Problem

Journal Entry's advance payment balance is calculated from Journal Entry Account which can result in an invalid unallocated amount. When a Payment Entry or reverse Journal Entry is made against a Journal Entry that debits Creditors or credits Debtors (makes a payment), the unallocated amount is not updated.

To fix this problem of being able to reverse payable/receivable entry I had to modify the payment system such that Journal Entry's balance is calculated from the GL Entries made by and against the Journal Entry. To do that I had to change and generalize payable/receivable entries from voucher documents.

Modifications

Voucher Documents (the ones that credit/debit payables/receivables):

• Previously all voucher documents other than Journal Entry were setting against_voucher to reference themselves. In my opinion Journal Entry follows a more logical scheme as it sets against_voucher as empty rather than referencing itself when not referencing another voucher.
• Made GL Entry:against_voucher's values consistent with all voucher documents such that against_voucher is set as empty when making a party payable/receivable entry and is set to reference another voucher only when adjusting a previously made payable/receivable GL Entry (for payments and returns etc). This simplifies and generalizes queries for calculating invoices and unallocated payments.
• Added a generalized utility function get_balance_on_voucher that calculates a balance for a (voucher_type, voucher_no, account, party_type, party) tuple.

GL Entry and General Ledger:

• Indexed GL Entry:against_voucher_type since invoice/payment queries filter through this field often.
• Calling update_outstanding_amt from save_gl_entries and delete_gl_entries instead of on_update_with_args so that one query is required to update outstanding_amount rather than calling a query for each and every GL Entry.
• Added validation for GL Entry so that either party_type and party both must be set or both must be unset. Same for against_voucher and against_voucher_type

Journal Entry:

• Changed logic of calculating balance/unallocated payments such that GL Entry balance is queried to get the balance for the whole (Journal Entry, Account, Party Type, Party) tuple instead of individual Journal Entry Accounts. This prevents overallocating the balance of Journal Entry and prevents losing information about it's outstanding amount.
• Modified update_reference_in_journal_entry so that it updates references in multiple Journal Entry Accounts if the first entry's amount is less than the allocated_amount
• Removed is_advance field from Journal Entry Account as it becomes impossible to get is_advance field when calculating advance payments from GL Entries. And also because in my opinion an unallocated payment should always be allocatable from Advance Payments in Purchase/Sales Invoice.
• Removed reference_due_date from Journal Entry Account since it wasn't being used for anything.
• Force set exchange_rate for entries referencing Purchase Invoice, Sales Invoice or Journal Entry.
• (Journal Entry, Account, Party Type, Party) tuple's exchange rate is calculated as the average of all exchange rates in a (Journal Entry, Account, Party Type, Party) tuple.
• I tried to make exchange_rate read-only when referencing voucher documents, however, frappe doesn't allow setting a specific row in a child table as read-only, so if the user changes exchange rate manually for such entries, he/she could see errors for debit != credit!
• Some bug fixes: setting remarks as empty string to prevent a copied value, remarks for Purchase Order using debit amount instead of credit amount

Payment Entry:

• Get negative invoices only when Receiving from Payable or Paying to Receivable
• Reload reference details before validating allocated amounts. Print details if allocated_amount > outstanding_amount since the client may not show the updated outstanding amount.
• Some bug fixes: auto set paid_amount instead of received_amount and vice-versa when receiving currency and paying currency is different, replaced an incorrect payment_type == 'Internal Transfer' with 'Receive', display depends on for exchange_rate field when it's not 1

Payment Reconciliation:

• Using utils functions instead of it's own query to get advance payments
• Include remarks
• Only auto set allocated amount if not manually set
• Updated get_advance_payment_entries to filter by against_account for Bank/Cash account field

Exchange Rate fields

• Set all exchange_rate field's precision to 9

purchase_invoice.json modified date

• Set modifed_date since it wasn't updated in Allow cost center in entry of balance sheet accounts #14972

What's Left?

Writing a patch:

• Change against_voucher and against_voucher_type to this PR's standard
• What else should be patched?

Setting read-only on specific child table row field

• If the user changes exchange rate in Journal Entry Account referencing a voucher document, the server reloads the exchange rate from the database so there may be an validation error that debit != credit. But the user will not see the updated exchange rate. In this case, the exchange rate should be read only

Validation fail due to old outstanding balance not updated in client:

• Payment Entry reloads outstanding amount and then validates if allocated amount <= outstanding amount. If it fails the server-side script will print a validation error (with outstanding and allocated amounts), however, it will not update the UI's outstanding_amount and probably confusing the user.
• Maybe there should be a feature to push changes forcefully to the client

Possibly create a child table for Journal Entry Outstanding Balances

• If the GL Entry queries to calculate outstanding balances on Journal Entries proves to be expensive, it could be made more efficient by creating a child table like Journal Entry Outstanding Balances that is updated using the update_outstanding_amt function called from make_gl_entries and querying from that child table instead of GL Entry.

Bank Statment Transaction Entry

• I ignored Bank Statment Transaction Entry's use of reconcile_against_document because I have no idea how it works.
• Would it require changes to use the new reconcile_against_document?

Use precision based threshold to check if monetary value > 0 or < 0

• I saw multiple absolute values used to check if a monetary value is significantly greater than 0.
• Some values: > 0.005, >= 0.005, > 0.009
• I suggest replacing them all with values like unless there is a specific reason for using 0.005:
• • >= 1.0 / (10 ** (precision("field")))
• • < 1.0 / (10 ** (precision("field")))
• It would allow currencies with high precision like cryptocurrencies to function properly

[rmehta]

@SaiFi0102 thanks for taking this on.

But a good explanation via screenshots will be very helpful. It is really hard to review 67 files without an overall understanding of what you are trying to achieve.

Can you ping me separately via email so we can discuss this via a call?

[SaiFi0102]

Hey @rmehta. Sure I'll post some screenshots. And yeah it'd be nice to discuss this over a call, however, I'll have to get back to you in a week since I'm giving exams, so yeah I'll email you! By the way for some reason GitHub is showing commits from other users since I merged from staging rather than develop (since develop is now v12). So I'm closing this and opening another pull request #15542. You will see that I have modified 26 files, not 67!