From b0aaaa8c75cbfa68f855277eb4dcc05e66d55452 Mon Sep 17 00:00:00 2001
From: foppe <fjdh@disroot.org>
Date: Sun, 14 Jun 2026 12:43:51 +0200
Subject: [PATCH] diag(gocardless): summarise transaction on mandate-creation
 failure

When create_mandate() cannot persist a GoCardless Mandate, future payments stop
reusing it and re-prompt the payer (#89). Frappe already captures the traceback,
but the failing transaction is buried in a locals dump. Add a readable summary
line (mandate id, reference document, customer, gocardless_customer) above the
traceback so the Error Log entry is actionable on its own.

Test drives a forced create_mandate() failure and asserts the summary is logged.
No behavioural change.
---
 .../test_gocardless_mandate.py                | 75 ++++++++++++++++++-
 .../pages/gocardless_confirmation.py          | 14 +++-
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/payments/payment_gateways/doctype/gocardless_mandate/test_gocardless_mandate.py b/payments/payment_gateways/doctype/gocardless_mandate/test_gocardless_mandate.py
index 0c1952a16..649f574e0 100644
--- a/payments/payment_gateways/doctype/gocardless_mandate/test_gocardless_mandate.py
+++ b/payments/payment_gateways/doctype/gocardless_mandate/test_gocardless_mandate.py
@@ -1,8 +1,77 @@
 # Copyright (c) 2018, Frappe Technologies and Contributors
 # See license.txt
 
-import unittest
+import frappe
+from frappe.tests.utils import FrappeTestCase
 
+from payments.templates.pages.gocardless_confirmation import create_mandate
 
-class TestGoCardlessMandate(unittest.TestCase):
-	pass
+
+def _make_payment_request(customer_name):
+	"""A Customer -> Sales Invoice -> Payment Request chain, as the GoCardless
+	confirmation flow expects, so create_mandate() can resolve the reference."""
+	customer = frappe.get_doc({"doctype": "Customer", "customer_name": customer_name}).insert(
+		ignore_permissions=True
+	)
+	si = frappe.get_doc(
+		{
+			"doctype": "Sales Invoice",
+			"company": "_Test Company",
+			"customer": customer.name,
+			"currency": "INR",
+			"items": [{"item_code": "Consulting", "qty": 1, "rate": 100}],
+		}
+	)
+	si.flags.ignore_mandatory = True
+	si.insert(ignore_permissions=True)
+	pr = frappe.get_doc(
+		{
+			"doctype": "Payment Request",
+			"payment_request_type": "Inward",
+			"reference_doctype": "Sales Invoice",
+			"reference_name": si.name,
+			"party_type": "Customer",
+			"party": customer.name,
+			"grand_total": 100,
+			"currency": "INR",
+			"email_to": "x@example.com",
+		}
+	)
+	pr.flags.ignore_mandatory = True
+	pr.flags.ignore_validate = True
+	pr.insert(ignore_permissions=True)
+	return customer, pr
+
+
+class TestGoCardlessMandate(FrappeTestCase):
+	def test_failed_mandate_creation_logs_transaction_context(self):
+		"""When create_mandate() can't persist a mandate, the Error Log entry must
+		spell out the transaction (reference doc, customer, mandate id) in a readable
+		summary so the failure is actionable on its own -- the bare traceback Frappe
+		captures by default buries those values in a locals dump (#89 diagnosis)."""
+		customer, pr = _make_payment_request("_Test GC Diag Customer")
+
+		# Force a failure inside create_mandate's insert: gocardless_customer is reqd,
+		# so an empty value raises MandatoryError, which create_mandate swallows.
+		create_mandate(
+			{
+				"mandate": "MD-DIAG-1",
+				"customer": "",
+				"reference_doctype": "Payment Request",
+				"reference_docname": pr.name,
+			}
+		)
+		self.assertFalse(frappe.db.exists("GoCardless Mandate", "MD-DIAG-1"))
+
+		log = frappe.get_all(
+			"Error Log",
+			filters={"method": ("like", "%nable to create mandate%")},
+			fields=["error"],
+			order_by="creation desc",
+			limit=1,
+		)
+		self.assertTrue(log, "expected an Error Log entry for the failed mandate creation")
+		error = log[0]["error"]
+		# The readable summary line the enrichment adds (distinct from a locals dump).
+		self.assertIn(f"Could not save GoCardless Mandate MD-DIAG-1 for Payment Request {pr.name}", error)
+		self.assertIn(f"customer: {customer.customer_name}", error)
diff --git a/payments/templates/pages/gocardless_confirmation.py b/payments/templates/pages/gocardless_confirmation.py
index 024206412..fd26ebf3c 100644
--- a/payments/templates/pages/gocardless_confirmation.py
+++ b/payments/templates/pages/gocardless_confirmation.py
@@ -102,4 +102,16 @@ def create_mandate(data):
 			).insert(ignore_permissions=True)
 
 		except Exception:
-			frappe.log_error("Gocardless: Unable to create mandate")
+			# Persisting the mandate is what lets future payments reuse it instead of
+			# re-prompting the payer (#89). Summarise which transaction/customer failed
+			# so the Error Log entry is actionable on its own, above the traceback.
+			frappe.log_error(
+				title="GoCardless: Unable to create mandate",
+				message=(
+					f"Could not save GoCardless Mandate {mandate} for "
+					f"{data.get('reference_doctype')} {data.get('reference_docname')} "
+					f"(customer: {erpnext_customer.customer_name if erpnext_customer else None}, "
+					f"gocardless_customer: {data.get('customer')}).\n\n"
+					f"{frappe.get_traceback(with_context=True)}"
+				),
+			)