From 552c46db98b65b2c49e0a5abc0536bc5e9be5b84 Mon Sep 17 00:00:00 2001 From: Abdeali Chharchhoda Date: Mon, 22 Jul 2024 11:25:01 +0530 Subject: [PATCH] fix: multiple issues in Payment Request --- .../doctype/payment_entry/payment_entry.js | 36 +++- .../doctype/payment_entry/payment_entry.py | 132 +++++++++++++- .../payment_entry_reference.json | 11 +- .../payment_entry_reference.py | 1 + .../payment_request/payment_request.js | 4 +- .../payment_request/payment_request.json | 13 +- .../payment_request/payment_request.py | 170 ++++++++++++------ 7 files changed, 300 insertions(+), 67 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index 519ce9769ee..185a860da33 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -7,6 +7,8 @@ cur_frm.cscript.tax_table = "Advance Taxes and Charges"; erpnext.accounts.taxes.setup_tax_validations("Payment Entry"); erpnext.accounts.taxes.setup_tax_filters("Advance Taxes and Charges"); +const FAULTY_VALUES = ["", null, undefined, 0]; + frappe.ui.form.on("Payment Entry", { onload: function (frm) { frm.ignore_doctypes_on_cancel_all = [ @@ -166,6 +168,19 @@ frappe.ui.form.on("Payment Entry", { }; }); + frm.set_query("payment_request", "references", function (doc, cdt, cdn) { + const row = locals[cdt][cdn]; + const filters = { + docstatus: 1, + status: ["!=", "Paid"], + reference_doctype: row.reference_doctype, + reference_name: row.reference_name, + }; + return { + filters: filters, + }; + }); + frm.set_query("sales_taxes_and_charges_template", function () { return { filters: { @@ -1012,6 +1027,8 @@ frappe.ui.form.on("Payment Entry", { total_negative_outstanding - total_positive_outstanding ); } + + frm.events.set_matched_payment_requests(frm); } frm.events.allocate_party_amount_against_ref_docs( @@ -1664,6 +1681,11 @@ frappe.ui.form.on("Payment Entry", { return current_tax_amount; }, + + set_matched_payment_requests: async function (frm) { + await frappe.after_ajax(); + frm.call("set_matched_payment_requests"); + }, }); frappe.ui.form.on("Payment Entry Reference", { @@ -1706,8 +1728,20 @@ frappe.ui.form.on("Payment Entry Reference", { } }, - allocated_amount: function (frm) { + allocated_amount: function (frm, cdt, cdn) { frm.events.set_total_allocated_amount(frm); + + const row = locals[cdt][cdn]; + + // if payment_request already set then return + if (row.payment_request) return; + + const references = [row.reference_name, row.reference_doctype, row.allocated_amount]; + + // if any of the reference fields are faulty, it returns + if (FAULTY_VALUES.some((el) => references.includes(el))) return; + + frm.call("set_matched_payment_request", { row_idx: row.idx }); }, references_remove: function (frm) { diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index a09ca2155f3..1eff3264bb5 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -8,7 +8,7 @@ from functools import reduce import frappe from frappe import ValidationError, _, qb, scrub, throw from frappe.utils import cint, comma_or, flt, getdate, nowdate -from frappe.utils.data import comma_and, fmt_money +from frappe.utils.data import comma_and, fmt_money, get_link_to_form from pypika import Case from pypika.functions import Coalesce, Sum @@ -180,6 +180,9 @@ class PaymentEntry(AccountsController): self.set_status() self.set_total_in_words() + def before_save(self): + self.check_payment_requests() + def on_submit(self): if self.difference_amount: frappe.throw(_("Difference Amount must be zero")) @@ -187,6 +190,7 @@ class PaymentEntry(AccountsController): self.update_outstanding_amounts() self.update_advance_paid() self.update_payment_schedule() + self.set_payment_req_outstanding_amount() self.set_payment_req_status() self.set_status() @@ -262,9 +266,17 @@ class PaymentEntry(AccountsController): self.update_advance_paid() self.delink_advance_entry_references() self.update_payment_schedule(cancel=1) + self.set_payment_req_outstanding_amount(cancel=True) self.set_payment_req_status() self.set_status() + def set_payment_req_outstanding_amount(self, cancel=False): + from erpnext.accounts.doctype.payment_request.payment_request import ( + update_payment_req_outstanding_amount, + ) + + update_payment_req_outstanding_amount(self, cancel=cancel) + def set_payment_req_status(self): from erpnext.accounts.doctype.payment_request.payment_request import update_payment_req_status @@ -308,6 +320,8 @@ class PaymentEntry(AccountsController): if self.payment_type == "Internal Transfer": return + self.validate_allocated_amount_as_of_pr() + if self.party_type in ("Customer", "Supplier"): self.validate_allocated_amount_with_latest_data() else: @@ -320,6 +334,21 @@ class PaymentEntry(AccountsController): if flt(d.allocated_amount) < 0 and flt(d.allocated_amount) < flt(d.outstanding_amount): frappe.throw(fail_message.format(d.idx)) + def validate_allocated_amount_as_of_pr(self): + from erpnext.accounts.doctype.payment_request.payment_request import ( + get_outstanding_amount_of_payment_entry_references as get_outstanding_amounts, + ) + + outstanding_amounts = get_outstanding_amounts(self.references) + + for ref in self.references: + if ref.payment_request and ref.allocated_amount > outstanding_amounts[ref.payment_request]: + frappe.throw( + _("Allocated Amount cannot be greater than Outstanding Amount of {0}").format( + get_link_to_form("Payment Request", ref.payment_request) + ) + ) + def term_based_allocation_enabled_for_reference( self, reference_doctype: str, reference_name: str ) -> bool: @@ -1692,6 +1721,103 @@ class PaymentEntry(AccountsController): return current_tax_fraction + def check_payment_requests(self): + if not self.references: + return + + not_set_count = sum(1 for row in self.references if not row.payment_request) + + if not_set_count == 0: + return + elif not_set_count == 1: + msg = _("{0} {1} is not set in {2}").format( + not_set_count, + frappe.bold("Payment Request"), + frappe.bold("Payment References"), + ) + else: + msg = _("{0} {1} are not set in {2}").format( + not_set_count, + frappe.bold("Payment Request"), + frappe.bold("Payment References"), + ) + + frappe.msgprint(msg=msg, alert=True, indicator="orange") + + # todo: can be optimized + @frappe.whitelist() + def set_matched_payment_requests(self): + if not self.references: + return + + matched_count = 0 + + for row in self.references: + if row.payment_request or ( + not row.reference_doctype or not row.reference_name or not row.allocated_amount + ): + continue + + row.payment_request = get_matched_payment_request( + row.reference_doctype, row.reference_name, row.allocated_amount + ) + + if row.payment_request: + matched_count += 1 + + if matched_count == 0: + return + elif matched_count == 1: + msg = _("{0} matched {1} is set").format(matched_count, frappe.bold("Payment Request")) + else: + msg = _("{0} matched {1} are set").format(matched_count, frappe.bold("Payment Request")) + + frappe.msgprint( + msg=msg, + alert=True, + ) + + @frappe.whitelist() + def set_matched_payment_request(self, row_idx): + row = next((row for row in self.references if row.idx == row_idx), None) + + if not row: + frappe.throw(_("Row #{0} not found").format(row_idx), title=_("Row Not Found")) + + # if payment entry already set then do not set it again + if row.payment_request: + return + + row.payment_request = get_matched_payment_request( + row.reference_doctype, row.reference_name, row.allocated_amount + ) + + if row.payment_request: + frappe.msgprint( + msg=_("Matched {0} is set").format(frappe.bold("Payment Request")), + alert=True, + ) + + +# todo: can be optimized +def get_matched_payment_request(reference_doctype, reference_name, outstanding_amount): + payment_requests = frappe.get_all( + doctype="Payment Request", + filters={ + "reference_doctype": reference_doctype, + "reference_name": reference_name, + "outstanding_amount": outstanding_amount, + "status": ["!=", "Paid"], + "docstatus": 1, + }, + pluck="name", + ) + + if len(payment_requests) == 1: + return payment_requests[0] + + return None + def validate_inclusive_tax(tax, doc): def _on_previous_row_error(row_range): @@ -1723,6 +1849,7 @@ def validate_inclusive_tax(tax, doc): frappe.throw(_("Valuation type charges can not be marked as Inclusive")) +# todo: modify its test @frappe.whitelist() def get_outstanding_reference_documents(args, validate=False): if isinstance(args, str): @@ -1880,6 +2007,9 @@ def get_outstanding_reference_documents(args, validate=False): ) ) + frappe.log("Data") + frappe.log(data) + return data diff --git a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json index 352ece24f06..7fce86c99d9 100644 --- a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json +++ b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json @@ -18,7 +18,8 @@ "allocated_amount", "exchange_rate", "exchange_gain_loss", - "account" + "account", + "payment_request" ], "fields": [ { @@ -120,12 +121,18 @@ "fieldname": "payment_type", "fieldtype": "Data", "label": "Payment Type" + }, + { + "fieldname": "payment_request", + "fieldtype": "Link", + "label": "Payment Request", + "options": "Payment Request" } ], "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2024-04-05 09:44:08.310593", + "modified": "2024-07-20 17:57:32.866780", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Entry Reference", diff --git a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py index 4a027b4ee32..68d819d0840 100644 --- a/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py +++ b/erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py @@ -25,6 +25,7 @@ class PaymentEntryReference(Document): parent: DF.Data parentfield: DF.Data parenttype: DF.Data + payment_request: DF.Link | None payment_term: DF.Link | None payment_type: DF.Data | None reference_doctype: DF.Link diff --git a/erpnext/accounts/doctype/payment_request/payment_request.js b/erpnext/accounts/doctype/payment_request/payment_request.js index f12facfbf5a..44313e5c0d2 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.js +++ b/erpnext/accounts/doctype/payment_request/payment_request.js @@ -52,8 +52,8 @@ frappe.ui.form.on("Payment Request", "refresh", function (frm) { } if ( - (!frm.doc.payment_gateway_account || frm.doc.payment_request_type == "Outward") && - frm.doc.status == "Initiated" + frm.doc.payment_request_type == "Outward" && + ["Initiated", "Partially Paid"].includes(frm.doc.status) ) { frm.add_custom_button(__("Create Payment Entry"), function () { frappe.call({ diff --git a/erpnext/accounts/doctype/payment_request/payment_request.json b/erpnext/accounts/doctype/payment_request/payment_request.json index 7674712374c..46723df1061 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.json +++ b/erpnext/accounts/doctype/payment_request/payment_request.json @@ -21,6 +21,7 @@ "grand_total", "is_a_subscription", "column_break_18", + "outstanding_amount", "currency", "subscription_section", "subscription_plans", @@ -400,13 +401,21 @@ "no_copy": 1, "print_hide": 1, "read_only": 1 + }, + { + "depends_on": "eval:doc.docstatus==1", + "fieldname": "outstanding_amount", + "fieldtype": "Currency", + "label": "Outstanding Amount", + "non_negative": 1, + "read_only": 1 } ], "in_create": 1, "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2024-06-20 13:54:55.245774", + "modified": "2024-07-20 17:54:33.064658", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Request", @@ -444,4 +453,4 @@ "sort_field": "creation", "sort_order": "DESC", "states": [] -} +} \ No newline at end of file diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index 814b56f79bc..9afb5fb000b 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -49,6 +49,7 @@ class PaymentRequest(Document): cost_center: DF.Link | None currency: DF.Link | None email_to: DF.Data | None + failed_reason: DF.Data | None grand_total: DF.Currency iban: DF.ReadOnly | None is_a_subscription: DF.Check @@ -57,16 +58,17 @@ class PaymentRequest(Document): mode_of_payment: DF.Link | None mute_email: DF.Check naming_series: DF.Literal["ACC-PRQ-.YYYY.-"] + outstanding_amount: DF.Currency party: DF.DynamicLink | None party_type: DF.Link | None payment_account: DF.ReadOnly | None - payment_channel: DF.Literal["", "Email", "Phone"] + payment_channel: DF.Literal["", "Email", "Phone", "Other"] payment_gateway: DF.ReadOnly | None payment_gateway_account: DF.Link | None payment_order: DF.Link | None payment_request_type: DF.Literal["Outward", "Inward"] payment_url: DF.Data | None - print_format: DF.Literal + print_format: DF.Literal[None] project: DF.Link | None reference_doctype: DF.Link | None reference_name: DF.DynamicLink | None @@ -100,6 +102,9 @@ class PaymentRequest(Document): frappe.throw(_("To create a Payment Request reference document is required")) def validate_payment_request_amount(self): + if self.grand_total == 0: + frappe.throw(_("Total Payment Request cannot be zero")) + existing_payment_request_amount = flt( get_existing_payment_request_amount(self.reference_doctype, self.reference_name) ) @@ -159,6 +164,8 @@ class PaymentRequest(Document): ref_doc.set_advance_payment_status() def before_submit(self): + self.outstanding_amount = self.grand_total + if self.payment_request_type == "Outward": self.status = "Initiated" elif self.payment_request_type == "Inward": @@ -265,7 +272,9 @@ class PaymentRequest(Document): ) def set_as_paid(self): - if self.payment_channel == "Phone": + self.db_set("status", "Paid") + + if self.payment_channel == "Phone" and self.status != "Paid": self.db_set("status", "Paid") else: @@ -277,6 +286,8 @@ class PaymentRequest(Document): def create_payment_entry(self, submit=True): """create entry""" + if self.payment_channel == "Phone": + frappe.throw(_("Payment Entry cannot be created for Phone Payment")) frappe.flags.ignore_account_permission = True ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name) @@ -290,11 +301,14 @@ class PaymentRequest(Document): party_account_currency = ref_doc.get("party_account_currency") or get_account_currency(party_account) - bank_amount = self.grand_total + bank_amount = self.outstanding_amount + if party_account_currency == ref_doc.company_currency and party_account_currency != self.currency: - party_amount = ref_doc.get("base_rounded_total") or ref_doc.get("base_grand_total") + total = ref_doc.get("rounded_total") or ref_doc.get("grand_total") + base_total = ref_doc.get("base_rounded_total") or ref_doc.get("base_grand_total") + party_amount = round(self.outstanding / total * base_total, self.precision("grand_total")) else: - party_amount = self.grand_total + party_amount = self.outstanding_amount payment_entry = get_payment_entry( self.reference_doctype, @@ -315,6 +329,9 @@ class PaymentRequest(Document): } ) + # Add reference of Payment Request + payment_entry.get("references")[0].payment_request = self.name + # Update dimensions payment_entry.update( { @@ -323,13 +340,8 @@ class PaymentRequest(Document): } ) - if party_account_currency == ref_doc.company_currency and party_account_currency != self.currency: - amount = payment_entry.base_paid_amount - else: - amount = self.grand_total - - payment_entry.received_amount = amount - payment_entry.get("references")[0].allocated_amount = amount + payment_entry.received_amount = self.outstanding_amount + payment_entry.get("references")[0].allocated_amount = self.outstanding_amount for dimension in get_accounting_dimensions(): payment_entry.update({dimension: self.get(dimension)}) @@ -535,7 +547,6 @@ def get_amount(ref_doc, payment_account=None): dt = ref_doc.doctype if dt in ["Sales Order", "Purchase Order"]: grand_total = flt(ref_doc.rounded_total) or flt(ref_doc.grand_total) - grand_total -= get_paid_amount_against_order(dt, ref_doc.name) elif dt in ["Sales Invoice", "Purchase Invoice"]: if not ref_doc.get("is_pos"): if ref_doc.party_account_currency == ref_doc.currency: @@ -560,24 +571,20 @@ def get_amount(ref_doc, payment_account=None): def get_existing_payment_request_amount(ref_dt, ref_dn): """ - Get the existing payment request which are unpaid or partially paid for payment channel other than Phone - and get the summation of existing paid payment request for Phone payment channel. + Get the total amount of `Paid` / `Partially Paid` payment requests against a document. """ - existing_payment_request_amount = frappe.db.sql( - """ - select sum(grand_total) - from `tabPayment Request` - where - reference_doctype = %s - and reference_name = %s - and docstatus = 1 - and (status != 'Paid' - or (payment_channel = 'Phone' - and status = 'Paid')) - """, - (ref_dt, ref_dn), + PR = frappe.qb.DocType("Payment Request") + + response = ( + frappe.qb.from_(PR) + .select(Sum(PR.grand_total - PR.outstanding_amount)) + .where(PR.reference_doctype == ref_dt) + .where(PR.reference_name == ref_dn) + .where(PR.docstatus == 1) + .run() ) - return flt(existing_payment_request_amount[0][0]) if existing_payment_request_amount else 0 + + return response[0][0] or 0 def get_gateway_details(args): # nosemgrep @@ -619,41 +626,86 @@ def make_payment_entry(docname): return doc.create_payment_entry(submit=False).as_dict() -def update_payment_req_status(doc, method): - from erpnext.accounts.doctype.payment_entry.payment_entry import get_reference_details +def update_payment_req_outstanding_amount(pe_doc, cancel=False): + outstanding_amounts = get_outstanding_amount_of_payment_entry_references(pe_doc.references) - for ref in doc.references: - payment_request_name = frappe.db.get_value( - "Payment Request", - { - "reference_doctype": ref.reference_doctype, - "reference_name": ref.reference_name, - "docstatus": 1, - }, + for ref in pe_doc.references: + if not ref.payment_request: + continue + + old_outstanding_amount = outstanding_amounts[ref.payment_request] + + new_outstanding_amount = ( + old_outstanding_amount + ref.allocated_amount + if cancel + else old_outstanding_amount - ref.allocated_amount ) - if payment_request_name: - ref_details = get_reference_details( - ref.reference_doctype, - ref.reference_name, - doc.party_account_currency, - doc.party_type, - doc.party, + if not cancel and new_outstanding_amount < 0: + frappe.throw( + _( + "The allocated amount is greater than the outstanding amount of Payment Request {0}" + ).format(ref.payment_request) ) - pay_req_doc = frappe.get_doc("Payment Request", payment_request_name) - status = pay_req_doc.status - if status != "Paid" and not ref_details.outstanding_amount: - status = "Paid" - elif status != "Partially Paid" and ref_details.outstanding_amount != ref_details.total_amount: - status = "Partially Paid" - elif ref_details.outstanding_amount == ref_details.total_amount: - if pay_req_doc.payment_request_type == "Outward": - status = "Initiated" - elif pay_req_doc.payment_request_type == "Inward": - status = "Requested" + frappe.db.set_value( + "Payment Request", + ref.payment_request, + "outstanding_amount", + new_outstanding_amount, + ) - pay_req_doc.db_set("status", status) + +def update_payment_req_status(pe_doc, method): + payment_requests = frappe.get_all( + "Payment Request", + filters={"name": ["in", get_referenced_payment_requests(pe_doc.references)]}, + fields=[ + "name", + "grand_total", + "outstanding_amount", + "payment_request_type", + ], + ) + + payment_requests = {pr.name: pr for pr in payment_requests} + + for ref in pe_doc.references: + if not ref.payment_request: + continue + + payment_request = payment_requests[ref.payment_request] + + if payment_request["outstanding_amount"] == payment_request["grand_total"]: + status = "Initiated" if payment_request["payment_request_type"] == "Outward" else "Requested" + elif payment_request["outstanding_amount"] == 0: + status = "Paid" + elif payment_request["outstanding_amount"] > 0: + status = "Partially Paid" + + frappe.db.set_value( + "Payment Request", + ref.payment_request, + "status", + status, + ) + + +def get_outstanding_amount_of_payment_entry_references(references: list) -> dict: + payment_requests = get_referenced_payment_requests(references) + + return dict( + frappe.get_all( + "Payment Request", + filters={"name": ["in", payment_requests]}, + fields=["name", "outstanding_amount"], + as_list=True, + ) + ) + + +def get_referenced_payment_requests(references: list) -> set: + return {row.payment_request for row in references if row.payment_request} def get_dummy_message(doc):