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

upgrade and add dj stripe #40

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

upgrade and add dj stripe #40

wants to merge 23 commits into from

Conversation

hhartwell
Copy link

Template

This is a template. While not all sections are necessary, depending on the size and complexity of the PR,
more information is better.

@ mention of reviewers

...

A brief description of the purpose of the changes contained in this PR.

...

Known issues to be addressed in a separate PR

...

Hand testing

  • add checklist here

Any relevant files for testing

link to any relevant files (or drag and drop into github)

Misc. comments

...

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • Ready to merge

tests/integration/test_payment_processing.py Show resolved Hide resolved
testproject/urls.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
.github/workflows/pythonpackage.yml Outdated Show resolved Hide resolved
@hhartwell hhartwell changed the title [WIP] upgrade and add dj stripe upgrade and add dj stripe Jan 11, 2024
README.md Outdated
@@ -181,3 +181,48 @@ class TestExceptionsViewSet(APIView):
| command | description|
| :--- | :----: |
| `upload_file <source> <destination>` | uses `django-storages` settings to upload a file |

### djstripe
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
### djstripe
### Payment helpers ([dj-stripe](https://dj-stripe.dev/))

README.md Outdated
#### Create and charge a payment intent
```py
from ckc.stripe.utils.payments import create_payment_intent, confirm_payment_intent
#for manual control
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#for manual control
# for manual control

README.md Outdated
@@ -181,3 +181,48 @@ class TestExceptionsViewSet(APIView):
| command | description|
Copy link
Collaborator

Choose a reason for hiding this comment

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

the ./manage.py commands are meant to go at the end of the README btw

Comment on lines +17 to +25
# 'customer',
# 'stripe_id',
# 'card_brand',
# 'card_last4',
# 'card_exp_month',
# 'card_exp_year',
# 'is_default',
# 'created',
# 'modified',
Copy link
Collaborator

Choose a reason for hiding this comment

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

pull, or useful?


)
except stripe.error.CardError:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

oo, should we really let this pass? I think we should let this be raised, and handle it with ValidationError wrappers at a certain point?

try:
    create_payment_intent(...)
except stripe.error.CardError:
    raise ValidationError("There was a problem with your card? Please try a different card.")

@@ -0,0 +1,39 @@
import stripe
Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually think we don't need this utils/ subdir? payments.py, and subscriptions.py could live right in ckc.stripe

@returns stripe.Price

"""
stripe_product = stripe.Product.create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

any error handling needed in this function? we can have some fun soon trying to cause all kinds of problems and handle them nicely .. then never have to handle them again!

setup.cfg Outdated
@@ -60,3 +61,7 @@ license_files =
python_requires = >= 3.6
packages = find:
zip_safe: False

[options.extras_require]
stripe =
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make this optional, lots of other projects don't use stripe

So we can install django-ckc without also installing stripe, unless we do django-ckc[stripe] or something

Comment on lines 19 to 22
def setUp(self):
self.user = User.objects.create_user(username="test", password="test")
self.client.force_authenticate(user=self.user)
return super().setUp()
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's do a setUpTestData (runs only once for a class, re-used between functions)

Can throw customer, created = Customer.get_or_create(subscriber=self.user) in it as well

example:

    @classmethod
    def setUpTestData(cls):
        cls.user = User.objects.create_user(username="test", password="test")
        cls.client.force_authenticate(user=self.user)
        cls.customer, cls.created = Customer.get_or_create(subscriber=cls.user)

if cls.client isn't available, you may have to do that force_authenticate step in the setUp function instead

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.

2 participants