-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
A build of this branch is available at https://impress-org.github.io/givewp-next-gen/feature/stripe-gateway |
@JasonTheAdams please review this one. There is still more polish to do in regards to the full transaction cycle (including receipts) & the api in general - but those pieces will come soon after this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! Did a quick code review. I'll test this after the comments are addressed. 👍
src/NextGen/DonationForm/Blocks/DonationFormBlock/app/utilities/postData.ts
Outdated
Show resolved
Hide resolved
* A hook before the form is submitted. | ||
*/ | ||
beforeCreatePayment?(values: FormData): Promise<object> | Error; | ||
|
||
/** | ||
* A hook after the form is submitted. | ||
*/ | ||
afterCreatePayment?(response: object): Promise<void> | Error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more specific name we can give these? What are the gateways intended to do at these points? Perhaps this is fine. It's just not clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was a specific name lol. beforeCreatePayment()
fires before the gateway class method createPayment()
. afterCreatePayment()
gets fired after the gateway class method createPayment()
. This is where the gateway could return data to itself using ReturnToBrowser
/ json and do something with that response.
In the case for Stripe it needs to return a stripe intent status to the front-end and do something with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it is a specific as it can be. Right now the naming caters towards the event — that is, the name is clear that it's happening at a point in time and when. That doesn't really inform the gateway as what it ought to be doing at that time. I don't know if this is actually accurate, but something like, prepareForDonation
and finishDonation
— the point being that it gently guides the gateway as what it ought to be doing.
That said, perhaps what's being done at this event varies too much from gateway to gateway, in which case the current event-based naming is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotchya, I think I gravitated toward the event-based naming because it coincides with the backend api naming convention and was easier to conceptualize in what order they get triggered while writing the logic. I know what you're saying about guiding the intention of the methods but as you said, the intention could vary between gateways. Some Gateways might not even need to use these.
Some use cases I see for each:
beforeCreatePayment()
- sending data to
createPayment()
, validation, gateway object mounting, pre-fetching something/using custom route
afterCreatePayment()
- receiving data from
createPayment()
, gateway confirmation, redirecting (the framework will probably have a general way of redirecting to receipt as well).
src/NextGen/Gateways/Stripe/NextGenStripeGateway/NextGenStripeRepository.php
Outdated
Show resolved
Hide resolved
src/NextGen/Gateways/Stripe/NextGenStripeGateway/NextGenStripeRepository.php
Show resolved
Hide resolved
@JasonTheAdams comments addressed, give her a whirl 🧪 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes a donation with the new Stripe Elements and takes me to the confirmation page. Noice. 💯
A build of this branch is available at https://impress-org.github.io/givewp-next-gen/develop |
Description
To really put the Next Gen gateway api to the test, we need a tangible gateway. Since Stripe is usually the most involved in terms of front-end, we chose to scaffold a new Stripe gateway using the Payment Element api. The PR adds the new gateway and will provide enough functionality to complete a donation on the next gen form. There is still more polish to do in regards to the full transaction cycle, including receipts - but those pieces will come soon after this.
JS Gateway Updates
afterCreatePayment?(response: object): Promise<void> | Error
that can be used after creating the donation & returning fromcreatePayment($donation)
from the gateway.Affects
Visuals
Testing Instructions
npm run dev
Pre-review Checklist
@unreleased
tags included in DocBlocks