Clean up code and add in condition that handles the scenario when Adm…

…inistrator sets a subscription plan's price to be 0.00. In which case, the paypal form object returned will be None and should be handled by django-subscription user accordingly.

subscription/admin.py

@@ -4,48 +4,58 @@
 
 from models import Subscription, UserSubscription, Transaction
 
-def _pricing(sub): return sub.get_pricing_display()
-def _trial(sub): return sub.get_trial_display()
+
+def _pricing(sub):
+    return sub.get_pricing_display()
+
+
+def _trial(sub):
+    return sub.get_trial_display()
+
 
 class SubscriptionAdmin(admin.ModelAdmin):
     list_display = ('name', _pricing, _trial)
 admin.site.register(Subscription, SubscriptionAdmin)
 
+
 def _subscription(trans):
-    print 22222, trans.subscription
     if trans.subscription != None:
         return u'<a href="/admin/subscription/subscription/%d/">%s</a>' % (
-            trans.subscription.pk, esc(trans.subscription) )
+            trans.subscription.pk, esc(trans.subscription))
 _subscription.allow_tags = True
 
+
 def _user(trans):
     if trans.user != None:
         return u'<a href="/admin/auth/user/%d/">%s</a>' % (
-            trans.user.pk, esc(trans.user) )
+            trans.user.pk, esc(trans.user))
 _user.allow_tags = True
 
+
 def _ipn(trans):
     if trans.ipn != None:
         return u'<a href="/admin/ipn/paypalipn/%d/">#%s</a>' % (
-            trans.ipn.pk, trans.ipn.pk )
+            trans.ipn.pk, trans.ipn.pk)
 _ipn.allow_tags = True
 
+
 class UserSubscriptionAdminForm(forms.ModelForm):
     class Meta:
         model = UserSubscription
     fix_group_membership = forms.fields.BooleanField(required=False)
     extend_subscription = forms.fields.BooleanField(required=False)
 
+
 class UserSubscriptionAdmin(admin.ModelAdmin):
-    list_display = ( '__unicode__', _user, _subscription, 'active', 'expires', 'valid' )
-    list_display_links = ( '__unicode__', )
+    list_display = ('__unicode__', _user, _subscription, 'active', 'expires', 'valid')
+    list_display_links = ('__unicode__',)
     list_filter = ('active', 'subscription', )
     date_hierarchy = 'expires'
     form = UserSubscriptionAdminForm
     fieldsets = (
-        (None, {'fields' : ('user', 'subscription', 'expires', 'active')}),
-        ('Actions', {'fields' : ('fix_group_membership', 'extend_subscription'),
-                     'classes' : ('collapse',)}),
+        (None, {'fields': ('user', 'subscription', 'expires', 'active')}),
+        ('Actions', {'fields': ('fix_group_membership', 'extend_subscription'),
+                     'classes': ('collapse',)}),
         )
 
     def save_model(self, request, obj, form, change):
@@ -56,19 +66,21 @@ def save_model(self, request, obj, form, change):
         obj.save()
 
     # action for Django-SVN or django-batch-admin app
-    actions = ( 'fix', 'extend', )
+    actions = ('fix', 'extend',)
 
     def fix(self, request, queryset):
         for us in queryset.all():
             us.fix()
     fix.short_description = 'Fix group membership'
 
     def extend(self, request, queryset):
-        for us in queryset.all(): us.extend()
+        for us in queryset.all():
+            us.extend()
     extend.short_description = 'Extend subscription'
 
 admin.site.register(UserSubscription, UserSubscriptionAdmin)
 
+
 class TransactionAdmin(admin.ModelAdmin):
     date_hierarchy = 'timestamp'
     list_display = ('timestamp', 'id', 'event', _subscription, _user, _ipn, 'amount', 'comment')

subscription/models.py

@@ -57,6 +57,7 @@ class Subscription(models.Model):
     group = models.ForeignKey(auth.models.Group, null=False, blank=False, unique=False)
 
     _PLURAL_UNITS = {
+        '0': ugettext_lazy('No trial'),
         'D': 'days',
         'W': 'weeks',
         'M': 'months',

subscription/views.py

@@ -1,25 +1,21 @@
-import datetime, decimal, urllib
+import urllib
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.contrib.sites.models import Site
-from django.core.exceptions import ObjectDoesNotExist
 from django.core.urlresolvers import reverse
-from django.db import transaction
-from django.http import HttpResponseRedirect
-from django.shortcuts import render, render_to_response, get_object_or_404
+from django.shortcuts import render, get_object_or_404
+from django.http import Http404
+from django.dispatch import Signal
+
 import django
 if django.VERSION <= (1, 5, 0):
-    from django.views.generic.list_detail import object_list
     from django.views.generic.simple import direct_to_template
-else:
-    from django.views.generic import TemplateView
-from django.dispatch import Signal
 
 _formclass = getattr(settings, 'SUBSCRIPTION_PAYPAL_FORM', 'paypal.standard.forms.PayPalPaymentsForm')
 _formclass_dot = _formclass.rindex('.')
 _formclass_module = __import__(_formclass[:_formclass_dot], {}, {}, [''])
-PayPalForm = getattr(_formclass_module, _formclass[_formclass_dot+1:])
+PayPalForm = getattr(_formclass_module, _formclass[_formclass_dot + 1:])
 
 from models import Subscription, UserSubscription
 
@@ -31,29 +27,41 @@
 else:
     cancel_url = 'https://www.paypal.com/cgi-bin/webscr?cmd=_subscr-find&alias=%s' % urllib.quote(settings.PAYPAL_RECEIVER_EMAIL)
 
+# Reference document for paypal html variables
 # https://cms.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_html_Appx_websitestandard_htmlvariables
 
+
 def _paypal_form_args(upgrade_subscription=False, **kwargs):
     "Return PayPal form arguments derived from kwargs."
     def _url(rel):
-        if not rel.startswith('/'): rel = '/'+rel
-        return 'http://%s%s' % ( Site.objects.get_current().domain, rel )
+        if not rel.startswith('/'):
+            rel = '/' + rel
+        return 'http://%s%s' % (Site.objects.get_current().domain, rel)
 
-    if upgrade_subscription: returl = reverse('subscription_change_done')
-    else: returl = reverse('subscription_done')
+    if upgrade_subscription:
+        returl = reverse('subscription_change_done')
+    else:
+        returl = reverse('subscription_done')
 
     rv = settings.SUBSCRIPTION_PAYPAL_SETTINGS.copy()
-    rv.update( notify_url = _url(reverse('paypal-ipn')),
-               return_url = _url(returl),
-               cancel_return = _url(reverse("subscription_cancel")),
-               **kwargs)
+    rv.update(notify_url=_url(reverse('paypal-ipn')),
+              return_url=_url(returl),
+              cancel_return=_url(reverse("subscription_cancel")),
+              **kwargs)
     return rv
 
-def _paypal_form(subscription, user, upgrade_subscription=False, **extra_args):
-    if not user.is_authenticated: return None
 
-    if subscription.recurrence_unit:
-        if not subscription.trial_unit:
+def _paypal_form(subscription, user, upgrade_subscription=False, **extra_args):
+    if not user.is_authenticated:
+        return None
+
+    if subscription.price <= 0:
+        # Handles the scenario when subscription price is set to 0 or negative
+        # value.  This means it is a "free plan" and should be handled
+        # appropriately by user of this library
+        return None
+    elif subscription.recurrence_unit:
+        if subscription.trial_unit == '0':
             trial = {}
         else:
             trial = {
@@ -65,36 +73,35 @@ def _paypal_form(subscription, user, upgrade_subscription=False, **extra_args):
         kwargs.update(trial)
         kwargs.update(extra_args)
         return PayPalForm(
-            initial = _paypal_form_args(
+                initial=_paypal_form_args(
                 cmd='_xclick-subscriptions',
-                item_name='%s: %s' % ( Site.objects.get_current().name,
-                                       subscription.name ),
-                item_number = subscription.id,
-                custom = user.id,
+                item_name='%s: %s' % (Site.objects.get_current().name, subscription.name),
+                item_number=subscription.id,
+                custom=user.id,
                 a3=subscription.price,
                 p3=subscription.recurrence_period,
                 t3=subscription.recurrence_unit,
-                src=1,                  # make payments recur
+                src=1,            # make payments recur
                 sra=1,            # reattempt payment on payment error
                 upgrade_subscription=upgrade_subscription,
-                modify=upgrade_subscription and 2 or 0, # subscription modification (upgrade/downgrade)
-                **kwargs),
-            button_type='subscribe'
+                # subscription modification (upgrade/downgrade)
+                modify=upgrade_subscription and 2 or 0, **kwargs),
+                button_type='subscribe'
             )
     else:
         return PayPalForm(
-            initial = _paypal_form_args(
-                item_name='%s: %s' % ( Site.objects.get_current().name,
-                                       subscription.name ),
-                item_number = subscription.id,
-                custom = user.id,
+                initial=_paypal_form_args(
+                item_name='%s: %s' % (Site.objects.get_current().name, subscription.name),
+                item_number=subscription.id,
+                custom=user.id,
                 amount=subscription.price))
 
+
 def subscription_list(request):
     return direct_to_template(
         request, template='subscription/subscription_list.html',
-        extra_context=dict(
-            object_list = Subscription.objects.all()))
+        extra_context=dict(object_list=Subscription.objects.all()))
+
 
 @login_required
 def subscription_detail(request, object_id, payment_method="standard"):
@@ -112,10 +119,8 @@ def subscription_detail(request, object_id, payment_method="standard"):
     if change_denied_reasons:
         form = None
     else:
-        extra_args = {}
         get_paypal_extra_args.send(sender=None, user=user, subscription=s, extra_args={})
-        form = _paypal_form(s, request.user,
-                            upgrade_subscription=(user is not None) and (user.subscription<>s))
+        form = _paypal_form(s, request.user, upgrade_subscription=(user is not None) and (user.subscription != s))
 
     try:
         s_us = request.user.usersubscription_set.get(subscription=s)
@@ -133,20 +138,20 @@ def subscription_detail(request, object_id, payment_method="standard"):
                 "returnurl": 'http://%s%s' % (domain, reverse('subscription_done'))}  # Express checkout return url
 
         data = {"item": item,
-                "payment_template": "payment.html",      # template name for payment
-                "confirm_template": "confirmation.html", # template name for confirmation
+                "payment_template": "payment.html",       # template name for payment
+                "confirm_template": "confirmation.html",  # template name for confirmation
                 "success_url": reverse('subscription_done')}              # redirect location after success
 
         o = PaymentMethodFactory.factory('WebsitePaymentsPro', data=data, request=request)
         # We return o.proceed() just because django-paypal's PayPalPro returns HttpResponse object
         return o.proceed()
 
     elif payment_method == 'standard':
-        template_vars = { 'object': s,
-                          'usersubscription': s_us,
-                          'change_denied_reasons': change_denied_reasons,
-                          'form': form,
-                          'cancel_url': cancel_url }
+        template_vars = {'object': s,
+                         'usersubscription': s_us,
+                         'change_denied_reasons': change_denied_reasons,
+                         'form': form,
+                         'cancel_url': cancel_url}
         template = 'subscription/subscription_detail.html'
         return render(request, template, template_vars)
     else: