pdt functionality in a decorator #6

Open
wants to merge 1 commit into
from

Projects

None yet

4 participants

@shezi
shezi commented Mar 25, 2011

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.

@mij
mij commented Dec 7, 2011

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?

@shezi
shezi commented Dec 7, 2011

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.

@mij
mij commented Dec 9, 2011

shezi: as an improvement, you should also update README.md with an example snippets for using the decorator.

@dcramer
Owner
dcramer commented Dec 9, 2011

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

@mij
mij commented Dec 10, 2011

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.
item_check_callable is an optional user callback for custom validation of the transaction. Removing it fixes the issue.

@shezi
shezi commented Dec 12, 2011

I'll work on it this week and try to put some tests up.

@mij
mij commented Dec 12, 2011

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.
This boils down to:

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
@mij
mij commented Dec 16, 2011

Further modifications to the decorator. I am commenting here rather than making a new pull so the repository will get correct code directly:

  1. do not call wrapped function transparently for POST
  2. pass explicit "pdt_duplicate" variable to tell the wrapped function if this is not the first time we see this payment ID
@require_GET
def aux(request, *args, **kwargs):
    pdt_obj = None
    pdt_active = False
    txn_id = request.GET.get('tx')
    failed = False
    pdt_duplicate = False
    if txn_id is not None:
        pdt_active = True
        # If an existing transaction with the id tx exists: use it
        try:
            pdt_obj = PayPalPDT.objects.get(txn_id=txn_id)
            pdt_duplicate = True
        except PayPalPDT.DoesNotExist:
            # This is a new transaction so we continue processing PDT request
            pass

        if pdt_obj is None:
            form = PayPalPDTForm(request.GET)
            if form.is_valid():
                try:
                    pdt_obj = form.save(commit=False)
                except Exception, e:
                    error = repr(e)
                    failed = True
            else:
                error = form.errors
                failed = True

            if failed:
                pdt_obj = PayPalPDT()
                pdt_obj.set_flag("Invalid form. %s" % error)

            pdt_obj.initialize(request)

            if not failed:
                # The PDT object gets saved during verify
                pdt_obj.verify()
    else:
        pass # we ignore any PDT requests that don't have a transaction id
@mij
mij commented Mar 14, 2012

ping shezi: tests?

@shezi
shezi commented Mar 14, 2012

Uh, yeah, forgot about those, sorry. I'll put them on my todo list and make some soon!

@shezi
shezi commented Mar 18, 2012

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.

@IanLewis
IanLewis commented May 1, 2012

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment