delete form fields so validation doesn't fire and move validation aro…

…und (bug 693466)
jbalogh · Oct 11, 2011 · 9367d34 · 9367d34
1 parent 41b457a
commit 9367d34
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 30 deletions.
diff --git a/apps/devhub/forms.py b/apps/devhub/forms.py
@@ -966,34 +966,35 @@ def __init__(self, *args, **kw):
         self.fields['free'].queryset = (self.extra['amo_user'].addons
                                         .exclude(pk=self.addon.pk)
                                         .filter(premium_type=amo.ADDON_FREE))
-        # For the wizard, we need fields not shown to be optional
-        for field in self.extra.get('not_required', []):
-            self.fields[field].required = False
+        # For the wizard, we need to remove some fields.
+        for field in self.extra.get('exclude', []):
+            del self.fields[field]
 
     def clean_paypal_id(self):
-        check_paypal_id(self.cleaned_data['paypal_id'])
-        return self.cleaned_data['paypal_id']
+        paypal_id = self.cleaned_data['paypal_id']
+        # Check it's a valid paypal id.
+        check_paypal_id(paypal_id)
 
-    def clean(self):
-        if (self.addon.paypal_id
-            and self.addon.paypal_id != self.cleaned_data['paypal_id']
+        if (self.addon.paypal_id and self.addon.paypal_id != paypal_id
             and self.addon.premium
-            and self.addon.premium.paypal_permissions_token):
+            and self.addon.premium.has_permissions_token()):
             # If a user changes their paypal id, then we need
-            # to nuke the token.
+            # to nuke the token, but don't do this when it's is blank.
             self.addon.premium.paypal_permissions_token = ''
             self.addon.premium.save()
-            raise forms.ValidationError(
-                        _('The PayPal third-party refund '
-                          'token has been removed.'))
+            raise forms.ValidationError(_('The PayPal third-party refund '
+                                          'token has been removed.'))
 
-        if (not self.addon.premium or not
-            self.addon.premium.has_permissions_token()):
+        return paypal_id
+
+    def clean(self):
+        if (not self.addon.premium or
+            not self.addon.premium.has_permissions_token()):
             # L10n: We require PayPal users to have a third party token.
             raise forms.ValidationError(
                         _('No PayPal third-party refund token set up.'))
-        if (self.addon.premium and not
-            self.addon.premium.has_valid_permissions_token()):
+        if (self.addon.premium and
+            not self.addon.premium.has_valid_permissions_token()):
             # L10n: We require PayPal users to have a third party token.
             raise forms.ValidationError(
                         _('The PayPal third-party refund '
@@ -1011,12 +1012,12 @@ def clean_free(self):
         return self.cleaned_data['free']
 
     def save(self):
-        if self.cleaned_data['paypal_id']:
+        if 'paypal_id' in self.cleaned_data:
             self.addon.paypal_id = self.cleaned_data['paypal_id']
             self.addon.support_email = self.cleaned_data['support_email']
             self.addon.save()
 
-        if self.cleaned_data['price']:
+        if 'price' in self.cleaned_data:
             premium = self.addon.premium
             if not premium:
                 premium = AddonPremium()

diff --git a/apps/devhub/tests/test_forms.py b/apps/devhub/tests/test_forms.py
@@ -124,36 +124,47 @@ def test_preview_size(self):
 
 @mock.patch('market.models.AddonPremium.has_valid_permissions_token',
             lambda z: True)
+@mock.patch('devhub.forms.check_paypal_id', lambda z: True)
 class TestPremiumForm(amo.tests.TestCase):
     fixtures = ['base/addon_3615', 'base/users']
 
-    def complete(self, data):
+    def complete(self, data, exclude):
         return forms.PremiumForm(data, extra={
             'addon': Addon.objects.get(pk=3615),
             'amo_user': UserProfile.objects.get(pk=999),
-            'not_required': ['price']})
+            'exclude': exclude})
 
+    @mock.patch('devhub.forms.check_paypal_id', lambda z: True)
     def test_remove_token(self):
         addon = Addon.objects.get(pk=3615)
         addon.update(paypal_id='')
         ap = AddonPremium.objects.create(paypal_permissions_token='1',
                                          addon=addon)
         data = {'support_email': 'foo@bar.com', 'paypal_id': 'foo@bar.com'}
-        form = self.complete(data)
-        form.is_valid()
+        form = self.complete(data, ['price'])
+        assert form.is_valid()
         form.save()
         # Do not remove the token, we had no paypal_id.
         assert AddonPremium.objects.get(pk=ap.pk).paypal_permissions_token
 
         data['paypal_id'] = 'fooa@bar.com'
-        form = self.complete(data)
+        form = self.complete(data, ['price'])
         # Remove the token and fail the form.
         assert not form.is_valid()
         assert not AddonPremium.objects.get(pk=ap.pk).paypal_permissions_token
 
         AddonPremium.objects.get(pk=ap.pk).update(paypal_permissions_token='a')
         data['paypal_id'] = 'foo@bar.com'
-        form = self.complete(data)
+        form = self.complete(data, ['price'])
         # Do not remove the token the token.
         assert form.is_valid()
         assert AddonPremium.objects.get(pk=ap.pk).paypal_permissions_token
+
+    def test_no_paypal_id(self):
+        addon = Addon.objects.get(pk=3615)
+        addon.update(paypal_id='some@id.com')
+        AddonPremium.objects.create(paypal_permissions_token='1',
+                                    addon=addon)
+        form = self.complete({}, ['paypal_id', 'support_email'])
+        assert not form.is_valid()
+        eq_(['price'], form.errors.keys())
diff --git a/apps/devhub/views.py b/apps/devhub/views.py
@@ -1274,7 +1274,7 @@ def marketplace_paypal(request, addon_id, addon):
     form = forms.PremiumForm(request.POST or None,
                              extra={'addon': addon,
                                     'amo_user': request.amo_user,
-                                    'not_required': ['price']})
+                                    'exclude': ['price']})
     if form.is_valid():
         form.save()
         return redirect('devhub.market.2', addon.slug)
@@ -1290,8 +1290,8 @@ def marketplace_pricing(request, addon_id, addon):
     form = forms.PremiumForm(request.POST or None,
                              extra={'addon': addon,
                                     'amo_user': request.amo_user,
-                                    'not_required': ['paypal_id',
-                                                     'support_email']})
+                                    'exclude': ['paypal_id',
+                                                'support_email']})
     if form.is_valid():
         form.save()
         if not (request.amo_user.addons.exclude(pk=addon.pk)
@@ -1308,8 +1308,8 @@ def marketplace_upsell(request, addon_id, addon):
     form = forms.PremiumForm(request.POST or None,
                              extra={'addon': addon,
                                     'amo_user': request.amo_user,
-                                    'not_required': ['price', 'paypal_id',
-                                                     'support_email']})
+                                    'exclude': ['price', 'paypal_id',
+                                                'support_email']})
     if form.is_valid():
         form.save()
         return redirect('devhub.market.4', addon.slug)