Update payment forms (and tests)

Fields now more self-contained, the bankcard form is much simpler.

oscar/apps/payment/forms.py

@@ -18,14 +18,26 @@ Bankcard = get_class('payment.utils', 'Bankcard')
 
 class BankcardNumberField(forms.CharField):
 
+    def __init__(self, *args, **kwargs):
+        _kwargs = {
+            'max_length': 20,
+            'widget': forms.TextInput(attrs={'autocomplete': 'off'}),
+            'label': _("Card number")
+        }
+        _kwargs.update(kwargs)
+        super(BankcardNumberField, self).__init__(*args, **_kwargs)
+
     def clean(self, value):
-        """Check if given CC number is valid and one of the
-           card types we accept"""
+        """
+        Check if given CC number is valid and one of the
+        card types we accept
+        """
         non_decimal = re.compile(r'\D+')
         value = non_decimal.sub('', value.strip())
 
         if value and not bankcards.luhn(value):
-            raise forms.ValidationError(_("Please enter a valid credit card number."))
+            raise forms.ValidationError(
+                _("Please enter a valid credit card number."))
         return super(BankcardNumberField, self).clean(value)
 
 
@@ -62,14 +74,16 @@ class BankcardMonthField(forms.MultiValueField):
             errors.update(kwargs['error_messages'])
 
         fields = (
-            forms.ChoiceField(choices=self.month_choices(),
-                              error_messages={'invalid': errors['invalid_month']}),
-            forms.ChoiceField(choices=self.year_choices(),
-                              error_messages={'invalid': errors['invalid_year']}),
+            forms.ChoiceField(
+                choices=self.month_choices(),
+                error_messages={'invalid': errors['invalid_month']}),
+            forms.ChoiceField(
+                choices=self.year_choices(),
+                error_messages={'invalid': errors['invalid_year']}),
         )
-
         super(BankcardMonthField, self).__init__(fields, *args, **kwargs)
-        self.widget = BankcardMonthWidget(widgets = [fields[0].widget, fields[1].widget])
+        self.widget = BankcardMonthWidget(
+            widgets=[fields[0].widget, fields[1].widget])
 
     def month_choices(self):
         return []
@@ -79,19 +93,29 @@ class BankcardMonthField(forms.MultiValueField):
 
 
 class BankcardExpiryMonthField(BankcardMonthField):
-    """
-    Expiry month
-    """
+
+    def __init__(self, *args, **kwargs):
+        today = date.today()
+        _kwargs = {
+            'required': True,
+            'label': _("Valid to"),
+            'initial': ["%.2d" % today.month, today.year]
+        }
+        _kwargs.update(kwargs)
+        super(BankcardExpiryMonthField, self).__init__(*args, **_kwargs)
+
     def month_choices(self):
         return [("%.2d" % x, "%.2d" % x) for x in xrange(1, 13)]
 
     def year_choices(self):
-        return [(x, x) for x in xrange(date.today().year, date.today().year+self.num_years)]
+        return [(x, x) for x in xrange(date.today().year,
+                                       date.today().year + self.num_years)]
 
     def clean(self, value):
         expiry_date = super(BankcardExpiryMonthField, self).clean(value)
         if date.today() > expiry_date:
-            raise forms.ValidationError(_("The expiration date you entered is in the past."))
+            raise forms.ValidationError(
+                _("The expiration date you entered is in the past."))
         return expiry_date
 
     def compress(self, data_list):
@@ -111,20 +135,32 @@ class BankcardExpiryMonthField(BankcardMonthField):
 
 
 class BankcardStartingMonthField(BankcardMonthField):
+
+    def __init__(self, *args, **kwargs):
+        _kwargs = {
+            'required': False,
+            'label': _("Valid from"),
+        }
+        _kwargs.update(kwargs)
+        super(BankcardStartingMonthField, self).__init__(*args, **_kwargs)
+
     def month_choices(self):
         months = [("%.2d" % x, "%.2d" % x) for x in xrange(1, 13)]
         months.insert(0, ("", "--"))
         return months
 
     def year_choices(self):
-        years = [(x, x) for x in xrange(date.today().year - self.num_years, date.today().year + 1)]
+        today = date.today()
+        years = [(x, x) for x in xrange(today.year - self.num_years,
+                                        today.year + 1)]
         years.insert(0, ("", "--"))
         return years
 
     def clean(self, value):
         starting_date = super(BankcardMonthField, self).clean(value)
         if starting_date and date.today() < starting_date:
-            raise forms.ValidationError(_("The starting date you entered is in the future."))
+            raise forms.ValidationError(
+                _("The starting date you entered is in the future."))
         return starting_date
 
     def compress(self, data_list):
@@ -141,32 +177,35 @@ class BankcardStartingMonthField(BankcardMonthField):
         return None
 
 
-class BankcardForm(forms.ModelForm):
-    number = BankcardNumberField(max_length=20, widget=forms.TextInput(attrs={'autocomplete':'off'}),
-                                 label=_("Card number"))
-    # Name is not normally needed by payment gateways so we hide it by default
-    name = forms.CharField(max_length=128, label=_("Name on card"),
-                           widget=forms.widgets.HiddenInput, required=False)
-    cvv_number = forms.RegexField(
-        required=True, label=_("CVV Number"),
-        regex=r'^\d{3,4}$',
-        widget=forms.TextInput(attrs={'size': '5'}),
-        help_text=_("This is the 3 or 4 digit security number on the back of your bankcard"))
-    start_month = BankcardStartingMonthField(label=_("Valid from"), required=False)
-    expiry_month = BankcardExpiryMonthField(required=True, label=_("Valid to"))
+class BankcardCCVField(forms.RegexField):
 
     def __init__(self, *args, **kwargs):
-        if 'initial' not in kwargs:
-            # Set the initial expiry month to be current month
-            today = date.today()
-            kwargs['initial'] = {'expiry_month': ["%.2d" % today.month,
-                                                  today.year]}
-        super(BankcardForm, self).__init__(*args, **kwargs)
+        _kwargs = {
+            'required': True,
+            'label': _("CCV number"),
+            'widget': forms.TextInput(attrs={'size': '5'}),
+            'error_message': _("Please enter a 3 or 4 digit number"),
+            'help_text': _("This is the 3 or 4 digit security number "
+                           "on the back of your bankcard")
+        }
+        _kwargs.update(kwargs)
+        super(BankcardCCVField, self).__init__(
+            r'^\d{3,4}$', *args, **_kwargs)
+
+    def clean(self, value):
+        value = value.strip()
+        return super(BankcardCCVField, self).clean(value)
+
+
+class BankcardForm(forms.ModelForm):
+    number = BankcardNumberField()
+    cvv_number = BankcardCCVField()
+    start_month = BankcardStartingMonthField()
+    expiry_month = BankcardExpiryMonthField()
 
     class Meta:
         model = BankcardModel
-        exclude = ('user', 'partner_reference')
-        fields = ('number', 'name', 'start_month', 'expiry_month', 'cvv_number')
+        fields = ('number', 'start_month', 'expiry_month', 'cvv_number')
 
     def get_bankcard_obj(self):
         """
@@ -186,7 +225,7 @@ class BankcardForm(forms.ModelForm):
 class BillingAddressForm(AbstractAddressForm):
 
     def __init__(self, *args, **kwargs):
-        super(BillingAddressForm,self ).__init__(*args, **kwargs)
+        super(BillingAddressForm, self).__init__(*args, **kwargs)
         self.set_country_queryset()
 
     def set_country_queryset(self):

tests/unit/payment/form_tests.py

@@ -1,12 +1,110 @@
+import datetime
+
 from django.test import TestCase
+from django.forms import ValidationError
 
 from oscar.apps.payment import forms
 
 
-class BankcardNumberFieldTest(TestCase):
+class TestBankcardNumberField(TestCase):
 
     def setUp(self):
         self.field = forms.BankcardNumberField()
 
-    def test_spaces_are_stipped(self):
-        self.assertEquals('4111111111111111', self.field.clean('  4111 1111 1111 1111'))
+    def test_strips_non_digits(self):
+        self.assertEquals(
+            '4111111111111111', self.field.clean('  4111 1111 1111 1111'))
+
+    def test_rejects_numbers_which_dont_pass_luhn(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean('1234123412341234')
+
+
+class TestStartingMonthField(TestCase):
+
+    def setUp(self):
+        self.field = forms.BankcardStartingMonthField()
+
+    def test_returns_a_date(self):
+        start_date = self.field.clean(['01', '2010'])
+        self.assertTrue(isinstance(start_date, datetime.date))
+
+    def test_rejects_invalid_months(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean(['00', '2010'])
+
+    def test_rejects_invalid_years(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean(['01', '201'])
+
+    def test_rejects_months_in_the_future(self):
+        today = datetime.date.today()
+        with self.assertRaises(ValidationError):
+            self.field.clean(['01', today.year + 1])
+
+    def test_returns_the_first_day_of_month(self):
+        start_date = self.field.clean(['01', '2010'])
+        self.assertEquals(1, start_date.day)
+
+
+
+class TestExpiryMonthField(TestCase):
+
+    def setUp(self):
+        self.field = forms.BankcardExpiryMonthField()
+
+    def test_returns_a_date(self):
+        today = datetime.date.today()
+        end_date = self.field.clean(['01', today.year + 1])
+        self.assertTrue(isinstance(end_date, datetime.date))
+
+    def test_rejects_invalid_months(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean(['00', '2010'])
+
+    def test_rejects_invalid_years(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean(['01', '201'])
+
+    def test_rejects_months_in_the_past(self):
+        today = datetime.date.today()
+        with self.assertRaises(ValidationError):
+            self.field.clean(['01', today.year - 1])
+
+    def test_returns_last_day_of_month(self):
+        today = datetime.date.today()
+        end_date = self.field.clean(['01', today.year + 1])
+        self.assertEquals(31, end_date.day)
+
+    def test_defaults_to_current_month(self):
+        today = datetime.date.today()
+        self.assertEquals(["%.2d" % today.month, today.year],
+                          self.field.initial)
+
+
+class TestCCVField(TestCase):
+    """CCV field"""
+
+    def setUp(self):
+        self.field = forms.BankcardCCVField()
+
+    def test_is_required_by_default(self):
+        with self.assertRaises(ValidationError):
+            self.field.clean("")
+
+    def test_only_permits_3_or_4_digit_numbers(self):
+        invalid = ['00', '12a', '12345']
+        for sample in invalid:
+            with self.assertRaises(ValidationError):
+                self.field.clean(sample)
+        valid = ['123', '  123   ', '1235']
+        for sample in valid:
+            self.field.clean(sample)
+
+    def test_has_meaningful_error_message(self):
+        try:
+            self.field.clean("asdf")
+        except ValidationError, e:
+            self.assertEquals("Please enter a 3 or 4 digit number",
+                              e.messages[0])
+