Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MBL-1391] Payment Intent StripeIntentContextType and CheckoutID #2057

Merged
merged 9 commits into from
May 14, 2024

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented May 13, 2024

📲 What

Updates the call to CreatePaymentIntent to include the StripeIntentContextType and the Checkout ID.

  • We added the context somewhat recently for the SetupIntent calls. Same idea here.

🤔 Why

We need this information to store additional metadata in the payment intent when it's created for on-session payments. This is helpful when the backend tries to debug any issues during checkout.

🛠 How

Checkout ID

  • Update GraphAPI to expose the new property
  • Update CreatePaymentIntentInput
  • Pass the id returned from CreateCheckoutMutation to .createPaymentIntent call sites
    • This is already available to us. Just need to pass it around accordingly
  • Make sure to encode the id so the backend receives it in base 64.

Context

  • Adds paymentIntentContext (defined in GraphAPI model) to CreatePaymentIntentInput
  • Updates .createPaymentIntent call sites

👀 See

No UI changes

♿️ Accessibility

No accessibility changes needed

🏎 Performance

Same

✅ Acceptance criteria

  • No changes to normal Crowdfunding checkout flow
  • No changes to Late Pledge checkout flow
  • No changes to creating Setup Intents and Payment Intents

⏰ Next Steps

  • Update the Late pledge feature flag to block late pledges on app versions that don't pass in the additional metadata (i.e. older app versions)
  • Note this for regression testing in the upcoming release since this touches pieces of both checkout flows.

@scottkicks scottkicks self-assigned this May 13, 2024
@scottkicks scottkicks force-pushed the scott/mbl-1391/late-pledge-payment-intent-context branch from 163fe42 to b6017d4 Compare May 13, 2024 19:40
@scottkicks scottkicks marked this pull request as ready for review May 13, 2024 20:30
@scottkicks scottkicks requested review from a team and amy-at-kickstarter and removed request for a team May 13, 2024 20:30
) {
self.projectId = projectId
self.amountDollars = amountDollars
self.digitalMarketingAttributed = digitalMarketingAttributed
self.paymentIntentContext = paymentIntentContext
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, but this type seems redundant to GraphAPI.CreatePaymentIntentInput

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@scottkicks scottkicks changed the title [MBL-1391] Payment Intent StripeIntentContextType [MBL-1391] Payment Intent StripeIntentContextType and CheckoutID May 14, 2024
@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@scottkicks scottkicks force-pushed the scott/mbl-1391/late-pledge-payment-intent-context branch from 90830ff to 8362cfa Compare May 14, 2024 18:48
@@ -334,6 +337,7 @@ public final class PledgePaymentMethodsViewModel: PledgePaymentMethodsViewModelT
)
clientSecretSignal = stripeIntentService.createPaymentIntent(
for: project.graphID,
checkoutId: checkoutId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this one already graph-encoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope!

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM, assuming that everywhere you needed the encoded GraphQL ID is correct

@scottkicks scottkicks merged commit 881f728 into main May 14, 2024
5 checks passed
@scottkicks scottkicks deleted the scott/mbl-1391/late-pledge-payment-intent-context branch May 14, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants