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

Update for 2015-10-16 API Version #178

Closed
paltman opened this issue Nov 12, 2015 · 5 comments
Closed

Update for 2015-10-16 API Version #178

paltman opened this issue Nov 12, 2015 · 5 comments
Assignees
Milestone

Comments

@paltman
Copy link
Member

paltman commented Nov 12, 2015

Upgrading DSP to 2015-10-16 going to be a pretty massive upgrade and likely require lots of model changes and rethinking as we are currently only on 2012-11-07.

The bulk of the logic is currently contained as methods on models in models.py. It's already a pretty large module and it's a little overwhelming to think about updating this module with all the updated bits in stripe since November 7, 2012.

All of the models in pinax-stripe are designed to be non-editable. They are merely a cache of the data that is in Stripe. The methods are wrappers around making API calls and updating the cached data (mostly).

I think to make this project easier to maintain and for new developers to reason about, we need to separate the business logic from the storage of the API data.

Some options:

  • Proxy Models for containing all the logic leaving the models doing the storage without any logic.
  • Service Layer where we pull out the methods off the models and create functions that perform actions. This could be something like creating an actions module with functional areas that map to areas of the API as submodules (e.g. actions/subscriptions.py, actions/transfers.py, etc.).

I think these two options are largely the same, with the exception that the Proxy Model will have the data already and the Service Layer you'd need to pass in the required data. I think my preference is the Service Layer right now.

I'd love to hear feedback from the community on ideas for addressing this issue.

@paltman paltman added this to the 2.0 milestone Nov 12, 2015
@paltman
Copy link
Member Author

paltman commented Nov 15, 2015

@brosner I'd especially love to know your thoughts on this.

@jtauber Your input is something I'd also love to hear if you have anything to add.

@paltman
Copy link
Member Author

paltman commented Nov 15, 2015

One thing I really like about the separation, especially in creation of a service layer, is we can more rapidly maintain 1:1 the model-cache layer with objects in the API as they are just simple bags of data.

Likewise, have functions that serve as the API for views/forms and external projects/apps, can more easily be maintained and constructed and to some degree provide some stability in API changes that might occur going forward.

Furthermore, as Stripe continues to grow, having these modules broken out could ease having distinct areas that different maintainers might surface (e.g. someone using ACH functionality in their project might be better able and have interest in maintaining the actions/ach.py module).

@brosner
Copy link
Member

brosner commented Nov 17, 2015

I like the direction. No opposition from me. Defining narrow interfaces is a good plan in my mind. I don't have a particular preference over which direction you go. Proxy models feel more like idomatic Django, but there isn't much value in exposing the underlying data since you said it is treated as a local cache. If I were to approach this problem, I would want to identify the common tasks being performed and wrap them up in an interface. This way the interface implemetation can change based on the underlying Stripe API. Different versions of the interface can crop up to support newer features, but still enabling us to maintain compatibility with older APIs (based on some deprecation policy we can establish).

At this point code is key. We can discuss design until the end of time so it's best to have code we can reason about.

@paltman paltman self-assigned this Nov 17, 2015
@paltman
Copy link
Member Author

paltman commented Nov 19, 2015

Going to refactor the views to deal with multiple subscriptions and right now just going for something RESTful:

/subscriptions/                       # list subscriptions
/subscriptions/create/                # subscribe to a plan
/subscriptions/(?P<pk>\d+)/delete/    # cancel
/subscriptions/(?P<pk>\d+)/update/    # change plan

/payment-methods/                     # list current payments methods
/payment-methods/create/              # add a new payment method
/payment-methods/(?P<pk>\d+)/delete/  # delete a payment method
/payment-methods/(?P<pk>\d+)/update/  # update a payment method

/invoices/                            # list all invoices (payment history)
/invoices/(?P<pk>\d+)/                # view details of the invoice

I am certain once we have this working we will want something a little more polished so the the UX is more fluid.

@paltman paltman mentioned this issue Nov 21, 2015
@paltman
Copy link
Member Author

paltman commented Nov 22, 2015

#192 closes this issue

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

No branches or pull requests

2 participants