473 - Refactor and change out the donation flow to use Stripe.SetupIntents #479

dimitur2204 · 2023-03-23T14:19:21Z

Motivation and context

Changed out the donation flow to now use the Stripe.SetupIntent in order to solve a problem that occured with the PaymentIntent and recurring donations, now when we do not use the CheckoutSession.

This is an overview of the new flow in a simplified version (recurring donations will be part of a future PR):

Changes

Added a new stripe.module that is concerned with all functionality regarding communication with stripe and using the stripe client. Provides better separation of concerns.

Changed out how the donations are created. With the new implementation they are created directly in the Donation.succeeded state after the PaymentIntent was confirmed successfully and a charge was made, but this is subject to change if needed. The creation of the Donations is now reliant on the metadata provided on the Setup/Payment Intent.

Changed out some methods into services that made more sense for them to be in.

Removed old checkout session implementation.

Testing

I have only adjusted current tests and removed old ones, that were testing parts that do not exist now.

New endpoints

POST /setup-intent - Creates a new SetupIntent
POST /setup-intent/:id - Updates SetupIntent
POST /setup-intent/:id/finalize - Finalizes SetupIntent by creating a Customer, attaching a PaymentMethod and returning a confirmed PaymentIntent

All endpoints concerning communication with Stripe have been moved to the stripe.controller

Potential problems

There might be some confusion between the StripeModule that comes from golevelup and our new Stripe module.

The donations being instantiated in status Donation.succeeded means that failed and possibly donations that might take a long time to process for some reason might be problematic. However, all the tests I did with many different Stripe test cards are passing!

* added error logging to stripe client * added type for stripe config * removed unneded import --------- Co-authored-by: quantum-grit <quantum-grit@gmail.com>

* added: explicit version for stripe api client * added: maxNetworkRetries 2 to stripe client * added: full stripe error log --------- Co-authored-by: quantum-grit <quantum-grit@gmail.com>

Co-authored-by: quantum-grit <quantum-grit@gmail.com>

…ow-recurring-payments-setup-intent

github-actions · 2023-03-23T14:19:40Z

✅ Tests will run for this PR. Once they succeed it can be merged.

kachar · 2023-04-03T08:56:53Z

@dimitur2204 @igoychev The PR looks awesome to me. Can't wait to test it on staging

igoychev · 2023-04-03T10:34:00Z

@dimitur2204 @igoychev The PR looks awesome to me. Can't wait to test it on staging

Yep, we are just pushing out the Expense Reports and Bank Imports and this one will come next!

igoychev

Great improvement and also puts things in good order!

Please see a few comments that need adjustments and we are good to go.

igoychev · 2023-04-04T09:11:23Z

apps/api/src/paypal/paypal.service.spec.ts

+      imports: [ConfigModule, HttpModule],
+      providers: [
+        PaypalService,
+        {
+          provide: CampaignService,
+          useValue: {},
+        },
+        {
+          provide: DonationsService,
+          useValue: {},
+        },
+      ],


The DonationsService depends on StripeClient and including it here makes Paypal depend on Stripe too, which is not a good practice.
It looks like if we move createSubscriptionDonation from DonationsService to the StripeService will resolve the situation.

Also wen resolving the dependencies, it will be better to add DonationsModule in the imports[] section, so that there is no need to include its services in the providers section.

igoychev · 2023-04-04T09:17:39Z

apps/api/src/stripe/stripe.module.ts

+  imports: [
+    StripeClientModule.forRootAsync(StripeClientModule, {
+      inject: [ConfigService],
+      useFactory: StripeConfigFactory.useFactory,
+    }),


you might need to add the ConfigModule in imports[] section, to indicate who is providing the ConfigService

igoychev · 2023-04-04T09:19:51Z

apps/api/src/stripe/stripe.service.ts

+      customer: customer.id,
+    })
+    const paymentIntent = await this.stripeClient.paymentIntents.create({
+      amount: Math.round(Number(setupIntent.metadata.amount)),


could you share what is the need of the round here? asking if it could create rounding errors with the amount

igoychev · 2023-04-04T09:22:13Z

apps/api/src/prisma/prisma-client.mock.ts

-import { DeepMockProxy } from 'jest-mock-extended/lib/mjs/Mock'
+import { mockDeep, mockReset, DeepMockProxy } from 'jest-mock-extended'



nice side improvement!

igoychev · 2023-04-04T09:26:08Z

apps/api/src/donations/events/stripe-payment.service.ts

-  @StripeWebhookHandler('payment_intent.canceled')
-  async handlePaymentIntentCancelled(event: Stripe.Event) {
-    const paymentIntent: Stripe.PaymentIntent = event.data.object as Stripe.PaymentIntent
-    Logger.log(
-      '[ handlePaymentIntentCancelled ]',
-      paymentIntent,
-      paymentIntent.metadata as DonationMetadata,
-    )


The need of handling payment_intent.canceled was for recording if the person didn't complete the payment after his initial desire to donate.
Could you share how we are recording that information after this handler is removed?

igoychev · 2023-04-04T09:28:15Z

apps/api/src/donations/events/stripe-payment.service.spec.ts

    const mockedupdateDonationPayment = jest
-      .spyOn(campaignService, 'updateDonationPayment')
+      .spyOn(donationService, 'createDonation')


[consistency] mockedupdateDonationPayment needs to become mockedcreateDonation

igoychev · 2023-04-04T09:30:36Z

apps/api/src/donations/events/stripe-payment.service.spec.ts

    const mockedupdateDonationPayment = jest
-      .spyOn(campaignService, 'updateDonationPayment')
+      .spyOn(donationService, 'createDonation')
      .mockImplementation(() => Promise.resolve(''))
-      .mockName('updateDonationPayment')
+      .mockName('createDonation')


The test is for cancelling the subscription, yet it seems to expect a createDonation function to be called?

igoychev · 2023-04-04T09:48:55Z

apps/api/src/donations/donations.service.ts

+    const product = await this.stripeClient.products.create({
+      name: `Donation of ${subscriptionPaymentDto.amount}`,
+      description: `Donation of ${subscriptionPaymentDto.amount} to campaign ${subscriptionPaymentDto.campaignId} by person ${person.email}`,
+    })


As per Stripe documentation the product should be the name of the campaign - see 3rd bullet point here: https://stripe.com/docs/products-prices/how-products-and-prices-work#what-is-a-product

In that regard the product name and description should contain only campaign related information and should not include the price or person details, so that we don't create too many products for each client and each price. Also if easy, we could also pass the campaign url as parameter to the create product function.

igoychev · 2023-04-04T09:57:28Z

apps/api/src/donations/donations.service.ts

+    const invoice = await this.stripeClient.invoices.retrieve(
+      subscription.latest_invoice as string,
+      {
+        expand: ['payment_intent'],
+      },
+    )


just noticed that we can skip the invoice.retrieve request if we pass pass the "expand: ['latest_invoice.payment_intent']," to the subscription.create function so that it is returned there

igoychev · 2024-03-04T08:02:09Z

closing without merge, as the code changed quite a lot after this pull request

dimitur2204 and others added 25 commits March 23, 2023 15:10

473 - remove the use of emails as extCustomerIds

3897f3b

473 - add createSubscriptionDonation method

533025d

473 - add endpoint for subscription donation

57a7869

473 - update donation to now take dynamic amount and currency

f1180e9

473 - add default incomplete subscription method

d74742d

add a @Roles to the create-payment endpoint (podkrepi-bg#469)

234e7f9

Added error loging to stripe client (podkrepi-bg#470)

1e5a4b7

* added error logging to stripe client * added type for stripe config * removed unneded import --------- Co-authored-by: quantum-grit <quantum-grit@gmail.com>

add version to stripe client (podkrepi-bg#471)

018929c

* added: explicit version for stripe api client * added: maxNetworkRetries 2 to stripe client * added: full stripe error log --------- Co-authored-by: quantum-grit <quantum-grit@gmail.com>

upgrading prisma to remove openssl1.1-compat (podkrepi-bg#472)

18fc356

Co-authored-by: quantum-grit <quantum-grit@gmail.com>

Add openssl explicitly for prisma

5f00dfb

473 - separete stripe into it's own module

77baff7

473 - add create setup intent endpoint

2697d7d

473 - fix stripe module imports

a2d33cf

473 - add update setup intent endpoint

dabd220

473 - add finalization of setup intent

e32222d

473 - cleanup donation service from old implementation

cddf10d

473 - fix setup intent updating of campaignId metadata

e65cff7

473 - change out flow to use setup intent metadata

d692c03

473 - remove unneeded paymentIntent.created hook

798960b

473 - fix getPaymentDataFromCharge customerId assignment

fb6955e

473 - remove creation of wish from the charge.succeeded hook

cfa2ad9

473 - move create donation to the donationService

8db3f77

473 - remove unneeded endpoint

ae0d0f6

473 - fix anonymous donations

7347068

Merge remote-tracking branch 'origin/master' into 473-new-donation-fl…

2ed3800

…ow-recurring-payments-setup-intent

dimitur2204 added 3 commits March 23, 2023 15:43

473 - fix stripe payment service tests

ea68458

473 - fix strip webhook tests

d3827e2

473 - fix stripe and paypal module tests

b7f9feb

dimitur2204 self-assigned this Mar 24, 2023

473 - fix jest-mock-extended import

756c197

dimitur2204 marked this pull request as ready for review March 24, 2023 10:06

dimitur2204 added the run tests Allows running the tests workflows for forked repos label Mar 24, 2023

dimitur2204 requested review from igoychev and slavcho March 24, 2023 10:06

github-actions bot removed the run tests Allows running the tests workflows for forked repos label Mar 24, 2023

dimitur2204 changed the base branch from master to 480-epic-new-donation-flow-recurring-donations-support March 24, 2023 10:19

473 - remove on_session setup intent option

d037416

dimitur2204 added the run tests Allows running the tests workflows for forked repos label Mar 24, 2023

github-actions bot removed the run tests Allows running the tests workflows for forked repos label Mar 24, 2023

dimitur2204 mentioned this pull request Mar 24, 2023

1393 - Refactor and change out the donation flow to use Stripe.SetupIntents podkrepi-bg/frontend#1396

Merged

dimitur2204 requested a review from kachar March 24, 2023 10:33

kachar approved these changes Apr 3, 2023

View reviewed changes

igoychev requested changes Apr 4, 2023

View reviewed changes

igoychev closed this Mar 4, 2024

sashko9807 mentioned this pull request Apr 29, 2024

refactor(stripe): Use PaymentIntent and subscriptions instead of Checkout sessions #627

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

473 - Refactor and change out the donation flow to use Stripe.SetupIntents #479

473 - Refactor and change out the donation flow to use Stripe.SetupIntents #479

dimitur2204 commented Mar 23, 2023 •

edited

Loading

github-actions bot commented Mar 23, 2023 •

edited

Loading

kachar commented Apr 3, 2023

igoychev commented Apr 3, 2023

igoychev left a comment

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev Apr 4, 2023

igoychev commented Mar 4, 2024

		import { DeepMockProxy } from 'jest-mock-extended/lib/mjs/Mock'
		import { mockDeep, mockReset, DeepMockProxy } from 'jest-mock-extended'

473 - Refactor and change out the donation flow to use Stripe.SetupIntents #479

473 - Refactor and change out the donation flow to use Stripe.SetupIntents #479

Conversation

dimitur2204 commented Mar 23, 2023 • edited Loading

Motivation and context

Changes

Testing

New endpoints

Potential problems

github-actions bot commented Mar 23, 2023 • edited Loading

kachar commented Apr 3, 2023

igoychev commented Apr 3, 2023

igoychev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igoychev commented Mar 4, 2024

dimitur2204 commented Mar 23, 2023 •

edited

Loading

github-actions bot commented Mar 23, 2023 •

edited

Loading