-
Notifications
You must be signed in to change notification settings - Fork 206
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
pdt functionality in a decorator #6
base: master
Are you sure you want to change the base?
Conversation
I think this patch is essential. Basically, django-paypal currently sees PayPal callbacks as dead-ends (store and forget), whereas this allows to react on them. For example, one can enable user permissions based on a received payment. Any comment, authors? |
I'm glad you like it. That's what I use this patch for: adding credits to a user account after the payment was received. Before, my functionality would have to be added directly into the django-paypal namespace. With the decorator, I can use my own view function (and possibly even different ones for different payment buttons) to handle my logic. |
shezi: as an improvement, you should also update README.md with an example snippets for using the decorator. |
I'd feel much more comfortable merging this in if it had proper tests to go along with it. I haven't actually used the project in a while, so simple code reviewing can't really suffice from me |
The pull diff appears large, but most of it is the "pdt" view code turned into a decorator. As far as I experienced, there is only a little bug in decorators.py:49, where "item_check_callable" (from the view code) is undefined in the decorator call. |
I'll work on it this week and try to put some tests up. |
shezi: as you are doing that, consider adding a flag to the wrapped function that says if the PDT is new or not. As mostly the decorator will be used to react to a payment, it makes a lot of sense for it to have this information. pdt_new = False # one
try:
pdt_obj = PayPalPDT.objects.get(txn_id=txn_id)
pdt_new = True # two
...
kwargs.update({'pdt_active': pdt_active, 'pdt_failed': failed, 'pdt_obj': pdt_obj, 'pdt_new': pdt_new }) # three |
Further modifications to the decorator. I am commenting here rather than making a new pull so the repository will get correct code directly:
|
ping shezi: tests? |
Uh, yeah, forgot about those, sorry. I'll put them on my todo list and make some soon! |
Just a short notice: I won't get around to working on this request in the next four weeks. Sorry. I'll try and finish it afterwards. |
Rather than a decorator, pdt should probably be implemented as a class based view which calls a hook method that you can override at the end of pdt verification. |
I've refactored the PDT callback functionality into a decorator. This way, one can simply add the @pdt decoration to a view function and handle the PDT callback with custom logic.