Suggestion: Refactor and consolidate event handlers
Problem
Right now, our connect event handlers are being called with Stripe.Event as their first argument, but most of the other factors in how their being called are kind of all around the place.
Some require user_id (a Stripe.Account id) as the second argument, some inferr it, some get is as part of the object that's being handled, etc.
The thing is, we store the Stripe.Event as a local record, StripeEvent, every time before it gets handled. The local StripeEvent has the following info which ought to be enough for us to handle any event:
-
object_id- id of the object stored in the event. For platform events, this is enough to retrieve the object from the stripe API. -
user_id- id of the connect account associated with the event object, withobject_id, enough to retrieve the object from stripe API in the case of a connect event -
type- the type of the event. enough to figure out which handler should handle it -
endpoint-"platform"or"connect"- enough to figure out which handler module ought to be used to handle the event.
This means that, instead of passing in the Stripe.Event into our handlers, we ought to be able to pass in CodeCorps.StripeEvent instead and not deal with the problematic code in EventHandler.call_handler.
We can use the opportunity to consolidate our handler modules.
- Move all handlers of platform events under
StripeService.PlatformEvents. - Move all handlers of connect events under
StripeService.ConnectEvents. - For platform events, each handler should call their respective service module with just an
id_from_stripefor that object. Everything else can be inferred after retrieving the object from the Stripe API - For connect events, handler should call their respective service with
id_from_stripeanduser_id, which should then be enough to retrieve the object from stripe and read the rest from there. This means that our event handlers will all have the identical interface within their endpoint group. - I would also consider renaming our
XService.createfunctions (those that simply fetch the record from stripe and insert it locally), tocreate_from_stripeor evenimport_from_stripe, since this is what they in fact do.
Platform event handlers
-
customer.source.updated- currently passesStripe.Event.data.object.idinto service, which can beStripeEvent.object_id` -
customer.updated- same ascustomer.source.updated
Connect event handlers
-
account.updated- currently passesStripe.Event.data.object.idinto service, which can beStripeEvent.object_id. For the sake of connect events having the same interface, we should consider passing inStripeEvent.user_idas a second argument (the connect account id), even though it has the same value asobject_id` in this case. -
charge.succeeded- currently passesStripe.Event.data.object.idinto service, which can beStripeEvent.object_id. Also passes inuser_id, which we've explicitly merged into the stripe object. If we switch toStripeEvent, we can just simply pass inStripeEvent.user_id` -
customer.subscription.deleted- currently passesStripe.Event.data.object.idinto service, which can beStripeEvent.object_id. Also passes incustomer, so we can infer the account and then fetch the record. If we pass inStripeEvent.user_id` instead, we would not have to do all that. -
customer.subscription.updated- same ascustomer.subscription.deleted -
invoice.payment.succeeded- same ascustomer.subscription.deleted
@JoshSmith Could use your thoughts on this. Seems like having a unified interface for handling our connect/platform events would be helpful.