django-paypal icon indicating copy to clipboard operation
django-paypal copied to clipboard

pdt functionality in a decorator

Open shezi opened this issue 14 years ago • 12 comments

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.

shezi avatar Mar 25 '11 12:03 shezi

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?

mij avatar Dec 07 '11 20:12 mij

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 avatar Dec 07 '11 21:12 shezi

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

mij avatar Dec 09 '11 18:12 mij

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

dcramer avatar Dec 09 '11 20:12 dcramer

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.

mij avatar Dec 10 '11 17:12 mij

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

shezi avatar Dec 12 '11 09:12 shezi

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 avatar Dec 12 '11 14:12 mij

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 avatar Dec 16 '11 01:12 mij

ping shezi: tests?

mij avatar Mar 14 '12 12:03 mij

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

shezi avatar Mar 14 '12 12:03 shezi

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.

shezi avatar Mar 18 '12 12:03 shezi

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.

ianlewis avatar May 01 '12 07:05 ianlewis