Merge pull request #40643 from blaggacao/fix/adv-pay-status

fix(pr): set adv. pay. status base on PR stati

erpnext/accounts/doctype/payment_entry/payment_entry.py

@@ -188,6 +188,7 @@ class PaymentEntry(AccountsController):
 		self.update_outstanding_amounts()
 		self.update_advance_paid()
 		self.update_payment_schedule()
+		self.set_payment_req_status()
 		self.set_status()
 
 	def set_liability_account(self):

erpnext/accounts/doctype/payment_gateway_account/test_payment_gateway_account.py

@@ -5,6 +5,8 @@ import unittest
 
 # test_records = frappe.get_test_records('Payment Gateway Account')
 
+test_ignore = ["Payment Gateway"]
+
 
 class TestPaymentGatewayAccount(unittest.TestCase):
 	pass

erpnext/accounts/doctype/payment_request/payment_request.py

@@ -149,35 +149,37 @@ class PaymentRequest(Document):
 					).format(self.grand_total, amount)
 				)
 
-	def on_submit(self):
-		if self.payment_request_type == "Outward":
-			self.db_set("status", "Initiated")
-			return
-		elif self.payment_request_type == "Inward":
-			self.db_set("status", "Requested")
-
-		send_mail = self.payment_gateway_validation() if self.payment_gateway else None
+	def on_change(self):
 		ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name)
-
-		if (
-			hasattr(ref_doc, "order_type") and ref_doc.order_type == "Shopping Cart"
-		) or self.flags.mute_email:
-			send_mail = False
-
-		if send_mail and self.payment_channel != "Phone":
-			self.set_payment_request_url()
-			self.send_email()
-			self.make_communication_entry()
-
-		elif self.payment_channel == "Phone":
-			self.request_phone_payment()
-
 		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.set_total_advance_paid()
+			ref_doc.set_advance_payment_status()
+
+	def on_submit(self):
+		if self.payment_request_type == "Outward":
+			self.db_set("status", "Initiated")
+		elif self.payment_request_type == "Inward":
+			self.db_set("status", "Requested")
+
+		if self.payment_request_type == "Inward":
+			send_mail = self.payment_gateway_validation() if self.payment_gateway else None
+			ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name)
+
+			if (
+				hasattr(ref_doc, "order_type") and ref_doc.order_type == "Shopping Cart"
+			) or self.flags.mute_email:
+				send_mail = False
+
+			if send_mail and self.payment_channel != "Phone":
+				self.set_payment_request_url()
+				self.send_email()
+				self.make_communication_entry()
+
+			elif self.payment_channel == "Phone":
+				self.request_phone_payment()
 
 	def request_phone_payment(self):
 		controller = _get_payment_gateway_controller(self.payment_gateway)
@@ -217,14 +219,6 @@ class PaymentRequest(Document):
 		self.check_if_payment_entry_exists()
 		self.set_as_cancelled()
 
-		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.set_total_advance_paid()
-
 	def make_invoice(self):
 		ref_doc = frappe.get_doc(self.reference_doctype, self.reference_name)
 		if hasattr(ref_doc, "order_type") and ref_doc.order_type == "Shopping Cart":

erpnext/buying/doctype/purchase_order/test_purchase_order.py

@@ -1127,10 +1127,17 @@ class TestPurchaseOrder(FrappeTestCase):
 		po = create_purchase_order()
 		self.assertEqual(frappe.db.get_value(po.doctype, po.name, "advance_payment_status"), "Not Initiated")
 
-		pr = make_payment_request(dt=po.doctype, dn=po.name, submit_doc=True, return_doc=True)
+		pr = make_payment_request(
+			dt=po.doctype, dn=po.name, submit_doc=True, return_doc=True, payment_request_type="Outward"
+		)
+
+		po.reload()
 		self.assertEqual(frappe.db.get_value(po.doctype, po.name, "advance_payment_status"), "Initiated")
 
 		pe = get_payment_entry(po.doctype, po.name).save().submit()
+
+		pr.reload()
+		self.assertEqual(pr.status, "Paid")
 		self.assertEqual(frappe.db.get_value(po.doctype, po.name, "advance_payment_status"), "Fully Paid")
 
 		pe.reload()

erpnext/controllers/accounts_controller.py

@@ -1924,32 +1924,43 @@ class AccountsController(TransactionBase):
 
 			self.db_set("advance_paid", advance_paid)
 
-		self.set_advance_payment_status(advance_paid, order_total)
+		self.set_advance_payment_status()
 
-	def set_advance_payment_status(self, advance_paid: float | None = None, order_total: float | None = None):
+	def set_advance_payment_status(self):
 		new_status = None
-		# if money is paid set the paid states
-		if advance_paid:
-			new_status = "Partially Paid" if advance_paid < order_total else "Fully Paid"
 
-		if not new_status:
-			prs = frappe.db.count(
-				"Payment Request",
-				{
-					"reference_doctype": self.doctype,
-					"reference_name": self.name,
-					"docstatus": 1,
-				},
-			)
-			if self.doctype in frappe.get_hooks("advance_payment_receivable_doctypes"):
-				new_status = "Requested" if prs else "Not Requested"
-			if self.doctype in frappe.get_hooks("advance_payment_payable_doctypes"):
-				new_status = "Initiated" if prs else "Not Initiated"
+		stati = frappe.get_list(
+			"Payment Request",
+			{
+				"reference_doctype": self.doctype,
+				"reference_name": self.name,
+				"docstatus": 1,
+			},
+			pluck="status",
+		)
+		if self.doctype in frappe.get_hooks("advance_payment_receivable_doctypes"):
+			if not stati:
+				new_status = "Not Requested"
+			elif "Requested" in stati or "Failed" in stati:
+				new_status = "Requested"
+			elif "Partially Paid" in stati:
+				new_status = "Partially Paid"
+			elif "Paid" in stati:
+				new_status = "Fully Paid"
+		if self.doctype in frappe.get_hooks("advance_payment_payable_doctypes"):
+			if not stati:
+				new_status = "Not Initiated"
+			elif "Initiated" in stati or "Failed" in stati or "Payment Ordered" in stati:
+				new_status = "Initiated"
+			elif "Partially Paid" in stati:
+				new_status = "Partially Paid"
+			elif "Paid" in stati:
+				new_status = "Fully Paid"
 
 		if new_status == self.advance_payment_status:
 			return
 
-		self.db_set("advance_payment_status", new_status)
+		self.db_set("advance_payment_status", new_status, update_modified=False)
 		self.set_status(update=True)
 		self.notify_update()
 

erpnext/hooks.py

@@ -349,7 +349,6 @@ doc_events = {
 	"Payment Entry": {
 		"on_submit": [
 			"erpnext.regional.create_transaction_log",
-			"erpnext.accounts.doctype.payment_request.payment_request.update_payment_req_status",
 			"erpnext.accounts.doctype.dunning.dunning.resolve_dunning",
 		],
 		"on_cancel": ["erpnext.accounts.doctype.dunning.dunning.resolve_dunning"],

erpnext/selling/doctype/sales_order/test_sales_order.py

@@ -2,6 +2,7 @@
 # License: GNU General Public License v3. See license.txt
 
 import json
+from unittest.mock import patch
 
 import frappe
 import frappe.permissions
@@ -1956,10 +1957,48 @@ class TestSalesOrder(FrappeTestCase):
 				self.assertEqual(so.items[0].rate, scenario.get("expected_rate"))
 				self.assertEqual(so.packed_items[0].rate, scenario.get("expected_rate"))
 
-	def test_sales_order_advance_payment_status(self):
+	@patch(
+		# this also shadows one (1) call to _get_payment_gateway_controller
+		"erpnext.accounts.doctype.payment_request.payment_request.PaymentRequest.get_payment_url",
+		return_value=None,
+	)
+	def test_sales_order_advance_payment_status(self, mocked_get_payment_url):
 		from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry
 		from erpnext.accounts.doctype.payment_request.payment_request import make_payment_request
 
+		# Flow progressing to SI with payment entries "moved" from SO to SI
+		so = make_sales_order(qty=1, rate=100, do_not_submit=True)
+		# no-op; for optical consistency with how a webshop SO would look like
+		so.order_type = "Shopping Cart"
+		so.submit()
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested")
+
+		pr = make_payment_request(
+			dt=so.doctype, dn=so.name, order_type="Shopping Cart", submit_doc=True, return_doc=True
+		)
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested")
+
+		pe = pr.set_as_paid()
+		pr.reload()  # status updated
+		pe.reload()  # references moved to Sales Invoice
+		self.assertEqual(pr.status, "Paid")
+		self.assertEqual(pe.references[0].reference_doctype, "Sales Invoice")
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Fully Paid")
+
+		pe.cancel()
+		pr.reload()
+		self.assertEqual(pr.status, "Paid")  # TODO: this might be a bug
+		so.reload()  # reload
+		# regardless, since the references have already "handed-over" to SI,
+		# the SO keeps its historical state at the time of hand over
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Fully Paid")
+
+		pr.cancel()
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested"
+		)  # TODO: this might be a bug; handover has happened
+
+		# Flow NOT progressing to SI with payment entries NOT "moved"
 		so = make_sales_order(qty=1, rate=100)
 		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested")
 
@@ -1971,11 +2010,15 @@ class TestSalesOrder(FrappeTestCase):
 
 		pe.reload()
 		pe.cancel()
-		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested")
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested"
+		)  # here: reset
 
 		pr.reload()
 		pr.cancel()
-		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested")
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested"
+		)  # here: reset
 
 	def test_pick_list_without_rejected_materials(self):
 		serial_and_batch_item = make_item(