Merge pull request #904 from davidbgk/891000-status-code-409

Return a different status code for "already purchased" (bug 891000)

apps/addons/decorators.py

@@ -6,8 +6,8 @@ from django.shortcuts import get_object_or_404
 
 import waffle
 
-from addons.models import Addon
 import commonware.log
+from addons.models import Addon
 
 log = commonware.log.getLogger('mkt.purchase')
 
@@ -90,17 +90,6 @@ def has_purchased_or_refunded(f):
     return wrapper
 
 
-def has_not_purchased(f):
-    """ The opposite of has_purchased. """
-    @functools.wraps(f)
-    def wrapper(request, addon, *args, **kw):
-        if addon.is_premium() and addon.has_purchased(request.amo_user):
-            log.info('Already purchased: %d' % addon.pk)
-            raise PermissionDenied
-        return f(request, addon, *args, **kw)
-    return wrapper
-
-
 def can_become_premium(f):
     """Check that the addon can become premium."""
     @functools.wraps(f)

apps/addons/tests/test_decorators.py

@@ -6,8 +6,8 @@ from nose.tools import eq_
 from test_utils import RequestFactory
 
 import amo.tests
-from addons.models import Addon
 from addons import decorators as dec
+from addons.models import Addon
 
 
 class TestAddonView(amo.tests.TestCase):
@@ -126,16 +126,3 @@ class TestPremiumDecorators(amo.tests.TestCase):
         self.addon.has_purchased.return_value = False
         with self.assertRaises(PermissionDenied):
             view(self.request, self.addon)
-
-    def test_has_not_purchased(self):
-        view = dec.has_not_purchased(self.func)
-        self.addon.is_premium.return_value = True
-        self.addon.has_purchased.return_value = False
-        eq_(view(self.request, self.addon), True)
-
-    def test_has_not_purchased_failure(self):
-        view = dec.has_not_purchased(self.func)
-        self.addon.is_premium.return_value = True
-        self.addon.has_purchased.return_value = True
-        with self.assertRaises(PermissionDenied):
-            view(self.request, self.addon)

docs/api/topics/payment.rst

@@ -289,8 +289,8 @@ Produces the JWT that is passed to `navigator.mozPay`_.
 
     :status 201: successfully completed.
     :status 401: not authenticated.
-    :status 403: app cannot be purchased. This could be because the app has
-        already been purchased.
+    :status 403: app cannot be purchased.
+    :status 409: app already purchased.
 
 .. _payment-status-label:
 

mkt/api/base.py

@@ -12,19 +12,20 @@ from django.http import HttpResponseNotFound
 
 import commonware.log
 from rest_framework.routers import Route, SimpleRouter
-from rest_framework.relations import HyperlinkedRelatedField, SlugRelatedField
+from rest_framework.relations import HyperlinkedRelatedField
 from rest_framework.viewsets import GenericViewSet
 from tastypie import fields, http
 from tastypie.bundle import Bundle
 from tastypie.exceptions import (ImmediateHttpResponse, NotFound,
                                  UnsupportedFormat)
 from tastypie.fields import ToOneField
+from tastypie.http import HttpConflict
 from tastypie.resources import ModelResource, Resource
 
 from access import acl
 from translations.fields import PurifiedField, TranslatedField
 
-from .exceptions import DeserializationError
+from .exceptions import AlreadyPurchased, DeserializationError
 from .http import HttpTooManyRequests
 from .serializers import Serializer
 
@@ -120,6 +121,9 @@ class Marketplace(object):
             # Reraise PermissionDenied as 403, otherwise you get 500.
             raise http_error(http.HttpForbidden, 'Permission denied.')
 
+        except AlreadyPurchased:
+            raise http_error(HttpConflict, 'Already purchased app.')
+
     def non_form_errors(self, error_list):
         """
         Raises passed field errors as an immediate HttpBadRequest response.

mkt/api/exceptions.py

@@ -4,3 +4,7 @@ from tastypie.exceptions import TastypieError
 class DeserializationError(TastypieError):
     def __init__(self, original=None):
         self.original = original
+
+
+class AlreadyPurchased(Exception):
+    pass

mkt/purchase/tests/test_webpay.py

@@ -17,6 +17,8 @@ import amo
 from amo.helpers import absolutify
 from amo.tests import TestCase
 from amo.urlresolvers import reverse
+from market.models import AddonPurchase
+from mkt.api.exceptions import AlreadyPurchased
 from stats.models import Contribution
 
 from utils import PurchaseTest
@@ -105,6 +107,14 @@ class TestPurchase(PurchaseTest):
                                 args=[self.addon.app_slug, uuid_]))
         eq_(data['status'], 'incomplete')
 
+    def test_status_for_already_purchased(self):
+        AddonPurchase.objects.create(addon=self.addon,
+                                     user=self.user,
+                                     type=amo.CONTRIB_PURCHASE)
+
+        with self.assertRaises(AlreadyPurchased):
+            self.client.post(self.prepare_pay)
+
     def test_pay_status_for_unknown_contrib(self):
         data = self.get(reverse('webpay.pay_status',
                                 args=[self.addon.app_slug, '<garbage>']))

mkt/purchase/webpay.py

@@ -14,8 +14,7 @@ import bleach
 import commonware.log
 from tower import ugettext as _
 
-from addons.decorators import (addon_view_factory, can_be_purchased,
-                               has_not_purchased)
+from addons.decorators import addon_view_factory, can_be_purchased
 import amo
 from amo.decorators import json_view, login_required, post_required, write
 from amo.helpers import absolutify
@@ -23,6 +22,7 @@ from amo.urlresolvers import reverse
 from lib.cef_loggers import app_pay_cef
 from lib.crypto.webpay import (InvalidSender, parse_from_webpay,
                                sign_webpay_jwt)
+from mkt.api.exceptions import AlreadyPurchased
 from mkt.webapps.models import Webapp
 from stats.models import ClientData, Contribution
 
@@ -71,9 +71,12 @@ def prepare_pay(request, addon):
 
 
 @can_be_purchased
-@has_not_purchased
 def _prepare_pay(request, addon):
     """Prepare a JWT to pass into navigator.pay()"""
+    if addon.is_premium() and addon.has_purchased(request.amo_user):
+        log.info('Already purchased: %d' % addon.pk)
+        raise AlreadyPurchased
+
     amount, currency, uuid_, contrib_for = start_purchase(request, addon)
     log.debug('Storing contrib for uuid: %s' % uuid_)
     Contribution.objects.create(addon_id=addon.id, amount=amount,

mkt/webpay/tests/test_resources.py

@@ -52,8 +52,11 @@ class TestPrepare(PurchaseTest, BaseOAuth):
     @patch('mkt.webapps.models.Webapp.has_purchased')
     def test_already_purchased(self, has_purchased):
         has_purchased.return_value = True
+        self.setup_base()
+        self.setup_package()
         res = self.client.post(self.list_url, data=json.dumps({'app': 337141}))
-        eq_(res.status_code, 403)
+        eq_(res.status_code, 409)
+        eq_(res.content, '{"reason": "Already purchased app."}')
 
     def _post(self):
         return self.client.post(self.list_url,