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-1282] Include setup intent context #2001

Merged
merged 2 commits into from Mar 27, 2024
Merged

[MBL-1282] Include setup intent context #2001

merged 2 commits into from Mar 27, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Mar 26, 2024

πŸ“² What

Include the setup intent context in setup intents.

πŸ‘€ See

Jira

βœ… Acceptance criteria

  • Adding new payment methods still works in pledge context and in settings.

@ifosli ifosli marked this pull request as ready for review March 26, 2024 22:37
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, one nit

clientSecretSignal = AppEnvironment.current.apiService
.createStripeSetupIntent(input: CreateSetupIntentInput(projectId: project.graphID))
.createStripeSetupIntent(
input: CreateSetupIntentInput(projectId: project.graphID, context: setupIntentContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I believe paymentSheetType should never be .setupIntent in the .latePledge context, but this is harmless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll just leave it for now, and we can clean it up later if we need to! Specifically, this is for the "Add card" option on the payment sheet; it's not about scheduling a specific payment or anything like that. I don't know enough about the other calls to know if we use a different method to add new cards for late pledges, so I'd rather leave this for now, and we can clean it up later once we have all the backend calls in place, if it makes sense to do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm totally fine with that. The way it's set up is confusing anyways!

@ifosli ifosli merged commit def5fa8 into main Mar 27, 2024
5 checks passed
@ifosli ifosli deleted the setupIntentContext branch March 27, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants