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

Bill Users for Paid Accounts #61

Merged
merged 7 commits into from Aug 7, 2019

Conversation

stephenprater
Copy link
Collaborator

What changes does this PR introduce?

This enables us to bill users through Stripe for Paid Accounts

Any background context you want to provide?

There are a few major parts to this PR:

  1. Upload Plan data to stripe consistently
  2. Add "foreign keys" to models to maintain their connection to stripe.
  3. Allow users to update their records via the API
  4. Keep those changes in sync with Stripe
  5. Respond to Stripe initiated changes via webhook and keep those changes in sync with our tables.

As an outgrowth of 3/4/5 I discovered that our use of SQLAlchemy made the models both hard to test in isolation and likely to result in a lot of specious calls to stripe. We needed better controls of the change management than SQLAlchemy's (coarse) callbacks offered, so we need to track changes ourselves. About half of our interaction methods need to either flush or commit to the DB, but it's it's confusing to have a lot of database state management mixed in with business logic.

To avoid that, I created an Interactor mixin that let's us manage that kind of housekeeping a little more decoratively.

Where should the reviewer start?

I tried to order these commits in a logical order so it made sense when you read them instead of a single 5,000 line diff - so you should be able to start at the top.

Has this been manually tested? How?

Tests pass! This one is definitely going to need pushbutton.

What value does this provide to our end users?

Let's them pay us for our awesome service, keeping us all employed and our service running!

What GIF best describes this PR or how it makes you feel?

making it rain

@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from 4b7d9cb to 37ed306 Compare July 12, 2019 01:00
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 12, 2019
@stephenprater
Copy link
Collaborator Author

Copy link
Collaborator

@AndrewScibek AndrewScibek left a comment

Choose a reason for hiding this comment

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

I started to review it and then started to think that it seems weird to include all our billing logic in this repo. There is nothing to hide and idk where else you would put it but seems like something that isnt needed in the repo. Thoughts?

Edit:
After discussion today it seems we will indeed split this work away from holepunch at a later date. It is not needed immediately but could make our life easier down the road

app/jobs/email.py Show resolved Hide resolved
app/utils/general.py Show resolved Hide resolved
@stephenprater
Copy link
Collaborator Author

@CypherpunkArmory CypherpunkArmory deleted a comment Jul 16, 2019
@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from ef06667 to 679653d Compare July 16, 2019 23:37
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 16, 2019
@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from 679653d to 1401093 Compare July 22, 2019 18:14
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 22, 2019
@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from 3fe48c8 to 7866ea0 Compare July 22, 2019 18:19
@stephenprater stephenprater requested review from AndrewScibek and removed request for MatthewTighe July 22, 2019 18:19
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 22, 2019
@AndrewScibek AndrewScibek force-pushed the 483-create-billing-on-plan-change branch from 3c86f7f to 00ecd5c Compare July 25, 2019 18:35
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 25, 2019
@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from 00ecd5c to 5031062 Compare July 25, 2019 21:30
@CypherpunkArmory CypherpunkArmory deleted a comment Jul 25, 2019
@AndrewScibek AndrewScibek removed their request for review July 30, 2019 03:36
@stephenprater stephenprater force-pushed the 483-create-billing-on-plan-change branch from a5b8a6d to c33b2bc Compare August 7, 2019 03:34
@CypherpunkArmory CypherpunkArmory deleted a comment Aug 7, 2019
@stephenprater stephenprater merged commit cf27dd0 into master Aug 7, 2019
@stephenprater stephenprater deleted the 483-create-billing-on-plan-change branch August 7, 2019 03:40
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.

None yet

2 participants