From 527c3e0b24eb79b8fb56c07c7e885d39f59f9fa9 Mon Sep 17 00:00:00 2001 From: Abdeali Chharchhoda Date: Wed, 24 Jul 2024 16:54:20 +0530 Subject: [PATCH] fix: changes as per review --- .../doctype/payment_entry/payment_entry.js | 21 +-- .../doctype/payment_entry/payment_entry.py | 173 ++++++++++-------- .../payment_request/payment_request.json | 10 +- .../payment_request/payment_request.py | 105 +++++------ erpnext/controllers/accounts_controller.py | 44 ++--- 5 files changed, 170 insertions(+), 183 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index 185a860da33..898d5684cf0 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -7,8 +7,6 @@ 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 = [ @@ -1027,8 +1025,6 @@ 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( @@ -1134,6 +1130,7 @@ frappe.ui.form.on("Payment Entry", { }); frm.refresh_fields(); + if (frappe.flags.allocate_payment_amount) frm.call("set_matched_payment_requests"); frm.events.set_total_allocated_amount(frm); }, @@ -1681,11 +1678,6 @@ 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", { @@ -1731,15 +1723,10 @@ frappe.ui.form.on("Payment Entry Reference", { allocated_amount: function (frm, cdt, cdn) { frm.events.set_total_allocated_amount(frm); - const row = locals[cdt][cdn]; + const row = frappe.get_doc(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; + if (row.payment_request || !row.reference_name || !row.reference_doctype || !row.allocated_amount) + return; frm.call("set_matched_payment_request", { row_idx: row.idx }); }, diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index ded28c91204..1823b9f410f 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -181,7 +181,7 @@ class PaymentEntry(AccountsController): self.set_total_in_words() def before_save(self): - self.check_payment_requests() + self.check_references_for_unset_payment_request() def on_submit(self): if self.difference_amount: @@ -190,9 +190,8 @@ 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_reference_advance_payment_status() + self.update_payment_requests() + self.update_references_advance_payment_status() self.set_status() def set_liability_account(self): @@ -267,33 +266,25 @@ 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_reference_advance_payment_status() + self.update_payment_requests(cancel=True) + self.update_references_advance_payment_status() self.set_status() - def set_payment_req_outstanding_amount(self, cancel=False): + def update_payment_requests(self, cancel=False): from erpnext.accounts.doctype.payment_request.payment_request import ( - update_payment_req_outstanding_amount, + update_payment_requests_as_per_pe_references, ) - update_payment_req_outstanding_amount(self, cancel=cancel) + update_payment_requests_as_per_pe_references(self.references, cancel=cancel) - def set_payment_req_status(self): - from erpnext.accounts.doctype.payment_request.payment_request import update_payment_req_status - - update_payment_req_status(self, None) - - # todo: need to optimize - def set_reference_advance_payment_status(self): + def update_references_advance_payment_status(self): advance_payment_doctypes = frappe.get_hooks("advance_payment_receivable_doctypes") + frappe.get_hooks( "advance_payment_payable_doctypes" ) for ref in self.get("references"): - ref_doc = frappe.get_doc(ref.reference_doctype, ref.reference_name) if ref.reference_doctype in advance_payment_doctypes: - # set advance payment status + ref_doc = frappe.get_doc(ref.reference_doctype, ref.reference_name) ref_doc.set_advance_payment_status() def update_outstanding_amounts(self): @@ -334,7 +325,7 @@ class PaymentEntry(AccountsController): if self.payment_type == "Internal Transfer": return - self.validate_allocated_amount_as_of_pr() + self.validate_allocated_amount_as_per_payment_request() if self.party_type in ("Customer", "Supplier"): self.validate_allocated_amount_with_latest_data() @@ -348,7 +339,7 @@ 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): + def validate_allocated_amount_as_per_payment_request(self): from erpnext.accounts.doctype.payment_request.payment_request import ( get_outstanding_amount_of_payment_entry_references as get_outstanding_amounts, ) @@ -358,9 +349,10 @@ class PaymentEntry(AccountsController): 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) - ) + msg=_( + "Row #{0}: Allocated Amount cannot be greater than Outstanding Amount of Payment Request {1}" + ).format(ref.idx, get_link_to_form("Payment Request", ref.payment_request)), + title=_("Invalid Allocated Amount"), ) def term_based_allocation_enabled_for_reference( @@ -1735,59 +1727,69 @@ class PaymentEntry(AccountsController): return current_tax_fraction - def check_payment_requests(self): + def check_references_for_unset_payment_request(self): if not self.references: return - not_set_count = sum(1 for row in self.references if not row.payment_request) + matched_payment_requests = get_matched_payment_requests_of_references( + [row 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"), + unset_pr_rows = {} + + for row in self.references: + if row.payment_request: + continue + + matched_pr = matched_payment_requests.get( + (row.reference_doctype, row.reference_name, row.allocated_amount) ) - frappe.msgprint(msg=msg, alert=True, indicator="orange") + if matched_pr: + unset_pr_rows[row.idx] = matched_pr + + if unset_pr_rows: + message = _("Matched Payment Requests found for references, but not set.

") + message += _("
View Details
") + + frappe.msgprint( + msg=message, + indicator="yellow", + ) - # todo: can be optimize @frappe.whitelist() def set_matched_payment_requests(self): if not self.references: return + matched_payment_requests = get_matched_payment_requests_of_references(self.references) + 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 + 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 + row.payment_request = matched_payment_requests.get( + (row.reference_doctype, row.reference_name, row.allocated_amount) ) if row.payment_request: matched_count += 1 - if matched_count == 0: + if not matched_count: 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, + msg=_("Setting {0} matched Payment Request(s)").format(matched_count), alert=True, ) @@ -1799,38 +1801,61 @@ class PaymentEntry(AccountsController): 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: + if ( + row.payment_request + or not row.reference_doctype + or not row.reference_name + or not row.allocated_amount + ): return - row.payment_request = get_matched_payment_request( - row.reference_doctype, row.reference_name, row.allocated_amount + matched_pr = get_matched_payment_requests_of_references([row]) + + if not matched_pr: + return + + row.payment_request = matched_pr[(row.reference_doctype, row.reference_name, row.allocated_amount)] + + frappe.msgprint( + msg=_("Setting matched Payment Request"), + alert=True, ) - if row.payment_request: - frappe.msgprint( - msg=_("Matched {0} is set").format(frappe.bold("Payment Request")), - alert=True, - ) +# FIXME: can be optimize and use query builder +def get_matched_payment_requests_of_references(references=None): + if not references: + return -# todo: can be optimize -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", + refs = [ + (row.reference_doctype, row.reference_name, row.allocated_amount) + for row in references + if row.reference_doctype and row.reference_name and row.allocated_amount + ] + + if not refs: + return + + all_matched_prs = frappe.db.sql( + """ + select name, reference_doctype, reference_name, outstanding_amount + from `tabPayment Request` + where (reference_doctype, reference_name, outstanding_amount) in %s + and status != 'Paid' and docstatus = 1 + """, + (refs,), + as_dict=True, ) - if len(payment_requests) == 1: - return payment_requests[0] + single_matched_prs = {} + for pr in all_matched_prs: + key = (pr.reference_doctype, pr.reference_name, pr.outstanding_amount) + if key in single_matched_prs: + single_matched_prs[key].append(pr.name) + else: + single_matched_prs[key] = [pr.name] - return None + return {key: names[0] for key, names in single_matched_prs.items() if len(names) == 1} def validate_inclusive_tax(tax, doc): diff --git a/erpnext/accounts/doctype/payment_request/payment_request.json b/erpnext/accounts/doctype/payment_request/payment_request.json index 46723df1061..50cc12aa4ea 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.json +++ b/erpnext/accounts/doctype/payment_request/payment_request.json @@ -134,7 +134,8 @@ "no_copy": 1, "options": "reference_doctype", "print_hide": 1, - "read_only": 1 + "read_only": 1, + "search_index": 1 }, { "fieldname": "transaction_details", @@ -147,7 +148,8 @@ "fieldtype": "Currency", "label": "Amount", "non_negative": 1, - "options": "currency" + "options": "currency", + "reqd": 1 }, { "default": "0", @@ -403,7 +405,7 @@ "read_only": 1 }, { - "depends_on": "eval:doc.docstatus==1", + "depends_on": "eval: doc.docstatus === 1", "fieldname": "outstanding_amount", "fieldtype": "Currency", "label": "Outstanding Amount", @@ -415,7 +417,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2024-07-20 17:54:33.064658", + "modified": "2024-07-23 19:02:07.754296", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Request", diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index 77b4e0ea43c..eb1fadb1fea 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -103,7 +103,10 @@ class PaymentRequest(Document): def validate_payment_request_amount(self): if self.grand_total == 0: - frappe.throw(_("Total Payment Request cannot be zero")) + frappe.throw( + _("{0} cannot be zero").format(self.get_label_from_fieldname("grand_total")), + title=_("Invalid Amount"), + ) existing_payment_request_amount = flt( get_existing_payment_request_amount(self.reference_doctype, self.reference_name) @@ -267,7 +270,7 @@ class PaymentRequest(Document): ) def set_as_paid(self): - if self.payment_channel == "Phone" and self.status != "Paid": + if self.payment_channel == "Phone": self.db_set({"status": "Paid", "outstanding_amount": 0}) else: @@ -279,8 +282,6 @@ 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) @@ -297,8 +298,8 @@ class PaymentRequest(Document): bank_amount = self.outstanding_amount if party_account_currency == ref_doc.company_currency and party_account_currency != self.currency: - total = ref_doc.get("rounded_total") or ref_doc.grand_total - base_total = ref_doc.get("base_rounded_total") or ref_doc.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 = flt(self.outstanding_amount / total * base_total, self.precision("grand_total")) else: party_amount = self.outstanding_amount @@ -314,7 +315,6 @@ class PaymentRequest(Document): payment_entry.update( { "mode_of_payment": self.mode_of_payment, - "reference_no": self.name, "reference_date": nowdate(), "remarks": "Payment Entry against {} {} via Payment Request {}".format( self.reference_doctype, self.reference_name, self.name @@ -323,7 +323,7 @@ class PaymentRequest(Document): ) # Add reference of Payment Request - payment_entry.get("references")[0].payment_request = self.name + payment_entry.references[0].payment_request = self.name # Update dimensions payment_entry.update( @@ -333,9 +333,6 @@ class PaymentRequest(Document): } ) - 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)}) @@ -420,12 +417,11 @@ class PaymentRequest(Document): return create_stripe_subscription(gateway_controller, data) def update_reference_advance_payment_status(self): - ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name) advance_payment_doctypes = frappe.get_hooks("advance_payment_receivable_doctypes") + frappe.get_hooks( "advance_payment_payable_doctypes" ) if self.reference_doctype in advance_payment_doctypes: - # set advance payment status + ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name) ref_doc.set_advance_payment_status() @@ -475,6 +471,9 @@ def make_payment_request(**args): if existing_payment_request_amount: grand_total -= existing_payment_request_amount + if not grand_total: + frappe.throw(_("Payment Request is already created")) + if draft_payment_request: frappe.db.set_value( "Payment Request", draft_payment_request, "grand_total", grand_total, update_modified=False @@ -571,29 +570,22 @@ def get_amount(ref_doc, payment_account=None): return grand_total -def get_existing_payment_request_amount(ref_dt, ref_dn, only_paid=False): +def get_existing_payment_request_amount(ref_dt, ref_dn): """ - Return the total amount of Payment Requests against a reference document. \n - If `only_paid` is True, it will return the total amount of paid Payment Requests. \n - Else, it will return the total amount of all Payment Requests. + Return the total amount of Payment Requests against a reference document. """ PR = frappe.qb.DocType("Payment Request") - if only_paid: - select = Sum(PR.grand_total - PR.outstanding_amount) - else: - select = Sum(PR.grand_total) - response = ( frappe.qb.from_(PR) - .select(select) + .select(Sum(PR.grand_total)) .where(PR.reference_doctype == ref_dt) .where(PR.reference_name == ref_dn) .where(PR.docstatus == 1) .run() ) - return response[0][0] or 0 + return response[0][0] if response else 0 def get_gateway_details(args): # nosemgrep @@ -635,40 +627,13 @@ def make_payment_entry(docname): return doc.create_payment_entry(submit=False).as_dict() -def update_payment_req_outstanding_amount(pe_doc, cancel=False): - outstanding_amounts = get_outstanding_amount_of_payment_entry_references(pe_doc.references) +def update_payment_requests_as_per_pe_references(references=None, cancel=False): + if not references: + return - 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 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) - ) - - frappe.db.set_value( - "Payment Request", - ref.payment_request, - "outstanding_amount", - new_outstanding_amount, - ) - - -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)]}, + filters={"name": ["in", get_referenced_payment_requests(references)]}, fields=[ "name", "grand_total", @@ -679,24 +644,40 @@ def update_payment_req_status(pe_doc, method): payment_requests = {pr.name: pr for pr in payment_requests} - for ref in pe_doc.references: + for ref in references: if not ref.payment_request: continue payment_request = payment_requests[ref.payment_request] - if payment_request["outstanding_amount"] == payment_request["grand_total"]: + # update outstanding amount + new_outstanding_amount = ( + payment_request["outstanding_amount"] + ref.allocated_amount + if cancel + else payment_request["outstanding_amount"] - ref.allocated_amount + ) + + if not cancel and new_outstanding_amount < 0: + frappe.throw( + msg=_( + "The allocated amount is greater than the outstanding amount of Payment Request {0}" + ).format(ref.payment_request), + title=_("Invalid Allocated Amount"), + ) + + # update status + if new_outstanding_amount == payment_request["grand_total"]: status = "Initiated" if payment_request["payment_request_type"] == "Outward" else "Requested" - elif payment_request["outstanding_amount"] == 0: + elif new_outstanding_amount == 0: status = "Paid" - elif payment_request["outstanding_amount"] > 0: + elif new_outstanding_amount > 0: status = "Partially Paid" + # update database frappe.db.set_value( "Payment Request", ref.payment_request, - "status", - status, + {"outstanding_amount": new_outstanding_amount, "status": status}, ) @@ -724,7 +705,7 @@ def get_dummy_message(doc): {%- else %}

Hello,

{% endif %}

{{ _("Requesting payment against {0} {1} for amount {2}").format(doc.doctype, - doc.name, doc.get_formatted("grand_total")) }}

+ doc.name, doc.get_formatted("grand_total")) }}

{{ _("Make Payment") }} diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 831a8c51bba..228d986300b 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1943,35 +1943,27 @@ class AccountsController(TransactionBase): self.set_advance_payment_status() - # todo: need to optimize - # todo: modularize def set_advance_payment_status(self): - from erpnext.accounts.doctype.payment_request.payment_request import ( - get_existing_payment_request_amount as get_paid_amount, - ) - new_status = None - available_payment_requests = frappe.db.count( - "Payment Request", - {"reference_doctype": self.doctype, "reference_name": self.name, "docstatus": 1}, - ) - total_amount = self.get("rounded_total") or self.grand_total - paid_amount = get_paid_amount(self.doctype, self.name, only_paid=True) - if paid_amount == total_amount: - new_status = "Fully Paid" - elif paid_amount > 0 and paid_amount < total_amount: - new_status = "Partially Paid" - elif self.doctype in frappe.get_hooks("advance_payment_receivable_doctypes"): - if not available_payment_requests: - new_status = "Not Requested" - elif paid_amount == 0 or paid_amount == 0.0: - new_status = "Requested" - elif self.doctype in frappe.get_hooks("advance_payment_payable_doctypes"): - if not available_payment_requests: - new_status = "Not Initiated" - elif paid_amount == 0 or paid_amount == 0.0: - new_status = "Initiated" + paid_amount = frappe.get_value( + doctype="Payment Request", + filters={ + "reference_doctype": self.doctype, + "reference_name": self.name, + "docstatus": 1, + }, + fieldname="sum(grand_total - outstanding_amount)", + ) + + if not paid_amount: + if self.doctype in frappe.get_hooks("advance_payment_receivable_doctypes"): + new_status = "Not Requested" if paid_amount is None else "Requested" + elif self.doctype in frappe.get_hooks("advance_payment_payable_doctypes"): + new_status = "Not Initiated" if paid_amount is None else "Initiated" + else: + total_amount = self.get("rounded_total") or self.get("grand_total") + new_status = "Fully Paid" if paid_amount == total_amount else "Partially Paid" if new_status == self.advance_payment_status: return