fix(gocardless): make mandate reuse work under a Customer naming series#232
Open
0spinboson wants to merge 3 commits into
Open
fix(gocardless): make mandate reuse work under a Customer naming series#2320spinboson wants to merge 3 commits into
0spinboson wants to merge 3 commits into
Conversation
When create_mandate() cannot persist a GoCardless Mandate, future payments stop reusing it and re-prompt the payer (frappe#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.
The GoCardless Mandate.customer field is a Link -> Customer custom field, so it must hold the Customer docname. create_mandate() stored customer_name (display) and check_mandate_validity() looked it up by the same display value. Under a Customer naming series the docname differs from customer_name, so: - the insert fails Link validation and is silently swallowed -> no mandate row - even if a row existed, the lookup keyed on the wrong value The first payment still succeeds (it charges via the redirect-flow mandate id directly), so the breakage only surfaces on the *second* payment as a re-prompt. Store and look the mandate up by the Customer docname (carried through as data['customer']). Extract get_registered_mandate() for the lookup, which also collapses the previous exists()+get_value() double query. Verified end-to-end against the GoCardless sandbox API under a naming series. Refs frappe#89
gocardless_checkout.check_mandate() looked the payer up with
get_doc('Customer', payer_name), but Payment Request fills payer_name with
customer_name (the display name). Under a Customer naming series that is not the
Customer docname, so the lookup raises DoesNotExistError and the first
subscription fails before a mandate can be created.
Extract get_payer_customer() to resolve by docname, then fall back to
customer_name, so the redirect flow is created regardless of Customer naming.
Refs frappe#89
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
GoCardless Mandate.customeris aLink -> Customercustom field, so it musthold the Customer docname. Two code paths instead used the Customer display
name (
customer_name):gocardless_confirmation.create_mandate()storedcustomer_namein themandate's
customerfield, andGoCardlessSettings.check_mandate_validity()looked the mandate up by the same display value.
gocardless_checkout.check_mandate()resolved the payer withget_doc("Customer", payer_name), wherepayer_nameis thecustomer_namethat Payment Request puts in the checkout URL.
On sites where the Customer naming rule is "Customer Name", the docname equals
customer_name, so this works by coincidence. On sites using a Customer namingseries, the docname differs from
customer_name, and:LinkValidationErrorand is silently swallowed, sono mandate row is stored;
DoesNotExistError.Because the first charge goes through the redirect-flow mandate id directly, the
first payment still succeeds; the breakage only surfaces on the next payment as
a repeated mandate prompt, which is the behaviour reported in
#89.
What
data["customer"].GoCardlessSettings.get_registered_mandate()for the lookup, whichalso collapses the previous
exists()+get_value()double query.gocardless_checkout.get_payer_customer(), which resolves the payerby docname and falls back to
customer_name, so the redirect flow is createdregardless of the Customer naming rule.
Verified end-to-end against the GoCardless sandbox API under a Customer naming
series: the mandate is now persisted and reused on the second payment. Tests
cover the mandate keying, the lookup helper, and the payer resolution.
This resolves #89 for sites using a Customer naming series. Sites
named by customer_name were already unaffected.
Note
Builds on #231 (its diagnostic logging commit is included here as the base of
this branch) and should be merged after it; once #231 lands, this PR's diff
reduces to the two fix commits.
Refs #89