[MPI 2] Add Manual Payment Records
This PR:
- Adds the ManualPaymentRecord model
- Implements a payment flow for manual payments, through a combination of:
- changes to
payment_ticketandpayment_completion - implementation of
prepare_intenton the ManualPaymentRecord model
- changes to
- Supports the ability for the user to change their payment reference by re-submitting
payment_completion
Key Questions
- When do we create a RegistrationPayment?
- currently I'm creating it in the
payment_ticket, but it could also be created upon the submission ofpayment_completion(which is the implementation I had initially) - Creating it in the
ticketmakes it possible for organizers to mark users as paid who haven't yet submitted their payment reference (although we could also handle this in thecapture_paymentlogic, which is introduced in the next PR) -
PaymentIntent#update_status_and_charges, as well asRegistration#record_paymentget a little messier if we create the RegPayment upon payment_completion, as we need to add a more conditional clauses for if a manual payment already exists - but there are probably ways to get around this
- currently I'm creating it in the
MPI 1.5 Considerations
This PR introduces an inconsistency in the relationship between a PaymentIntent ("WCA" implied whenever I write it like this) and its associated PaymentRecord - in Stripe or PayPal, a PaymentIntent would be associated with the PaymentRecord which represents the "parent" object for the overall payment lifecycle - a Stripe Payment Intent, or a PayPal Order.
However, in this PR, we are associating the PaymentIntent with the equivalent of a "charge" record[1], and because our code relies on the assumption that there is an underlying provider record for the PaymentIntent to be associated with, we are creating the PaymentRecord at the same time as creating the PaymentIntent. This is awkward - we should either:
- create the PaymentRecord at a later stage; only when the "charge" takes place
- create an intermediary PaymentRecord which corresponds to a PaymentIntent
I'm partial to (1) because (2):
- creates an unnecessary record in our database
- makes us rely on the assumption that there will always be some sort of "parent"-equivalent payment record - or force us to improvise one if there isn't. (more on this shortly)
The change (1) requires is us allowing the creation of PaymentIntents without associated payment records. This can be done by:
- making the
belongs_to: :payment_recordassociationoptional: true, and - changing the
wca_status_consistencyvalidation to allow there to be no associated payment record only if thewca_statusiscreated
I have pushed a WIP version of this to the PR.
I had previously thought that this would also require us to implement some generalized charge-surfacing logic in the PaymentIntent - but inspection of our current payment flow reveals that this is handled in the find_payment_from_request-style logic in the provider account models. If needed we can refactor/expand on these - and perhaps moving forward with this implementation will reveal that need.
We may also need to refactor around registrations_controller:448 - as this relies explicitly on the assumption that a PI will have an underlying PaymentRecord representing that PI
[1] We could consider some internal terminology for this - "payment event", or "payment attempt"
What do PaymentRecords represent?
I see our payment process as follows:
- Provider gives us an update (payment completion/webhook)
- We internalise that provider logic (determine & persist PaymentRecord updates)
- We translate provider-specific logic into general WCA payments logic (PaymentIntent updates)
- The generalised WCA payments logic triggers WCA business logic (create RegistrationPayments)
Based on this understanding, I think the value of PaymentRecords lies in their being as flexible as possible - we should try to mimic provider structures/logic, not try to force conformity to our payments model. Conformity to our payments model is "the provider does Some Stuff (:TM), and we can abstract that into a certain set of properties on the PaymentIntent that our business logic can act from".
Refactor opportunities
-
remote(inretrieve_remote) ->provider- eg,retrieve_provider_recordor similar -
find_payment_from_request(in the ConnectedProviderAccount models - eg, ConnectedStripeAccount) ->find_payment_intent(as this seems to be what it is actually doing -
MPI PR Overview
I'm busy reorganizing PR's and it's a lot to keep track of - here's the breakdown:
Canonical PR's:
- MPI 1: #12408 Added ManualPaymentIntegration
- MPI 2: This PR
- MPI 3: IN PROGRESS - Adds the user-facing frontend for manual payments
- MPI 4: TODO - RegAdmin table changes - displaying payment reference info in the table, and adding the ability to capture manual payments (also adds a route for bulk capturing manual registration payments)
- MPI 5: TODO - RegEdit changes - correctly display manual payments in Payment History, and allow the organizer to mark them as paid/unpaid as necessary (also adds a route for uncapturing registration payments)
Historical / Reference PR's:
Any of these still open will be closed as/when they are no longer needed.
- #12159 - has the full commit history for backend changes, as part of that history was squashed when creating MPI 2 (useful for the commit history)
- #12278 - contains all frontend changes, as well as the full commit history from ALL MPI changes until that point (I branched off parent branches, instead of rebasing onto them). In the process of being split up into MPI 3/4/5, then I'll close it
Future PR's:
- MPI 100: TODO - Enable Live Auto Accept for manual payments
- MPI 101: TODO - Expose payment capture endpoints to 3rd parties via API