ios-oss icon indicating copy to clipboard operation
ios-oss copied to clipboard

[MBL-1393] Moves Stripe Intent Logic Into It's Own Service

Open scottkicks opened this issue 1 year ago • 2 comments

📲 What

Creates a new StripeIntentService to handle creating Payment and Setup Intents

🤔 Why

This logic is used in crowdfunding, late pledges, and account settings payment method flows. Creating a shared/reusable service makes things cleaner and easier to maintain.

Hopefully, it makes consolidating late pledges and crowdfunding easier as well.

🛠 How

Creates a pretty simple StripeIntentService to handle both intent types.

👀 See

No visible changes

✅ Acceptance criteria

  • [x] No changes to late pledge flow
  • [x] No changes to crowdfunding flow
  • [x] No changes to managing payment method in account settings

scottkicks avatar Apr 29 '24 19:04 scottkicks

@amy-at-kickstarter I added tests for StripeIntentService and also made it a class instead of a struct. My goal is to ensure that intent requests are made with the correct data and called only the expected number of times.

I tried a bit of a different pattern than what we use elsewhere in the app. One callout I have is that we have tests behind the actual API request already that make sure the correct data is being passed in, so what I have might be a bit overkill. Lmk what you think.

  1. This change helps us to not repeat the API call in multiple classes, which is great, but it still only encapsulates a single signal (the API request). What would it look like if we wrote a service that was a higher-order encapsulation of multiple signals, or signal transformations? What might take this further? One small idea that comes to mind is that it looks like we really just want the clientSecret from this API call, so you could probably fold at least a map operation into it.

I'm not sure about this. For one thing, we need the result of the Signal (the envelope with the client secret or the ErrorEnvelope), not just the client secret. I feel like keeping the .map in the calling method makes more sense so that it can handle the result appropriately.

scottkicks avatar May 07 '24 17:05 scottkicks

@amy-at-kickstarter, I did the following from our Slack discussion:

  • Removed the service from Environment and injected it only into the ViewModels needing it.
  • Updated MockStripeIntentService so that it uses apiService and keeps track of the number of requests made
  • Added test assertions in all relevant ViewModels that guarantee that we're only requesting the right intents and making those requests the correct number of times.

scottkicks avatar May 13 '24 19:05 scottkicks