Fix choices attribute Django check for empty values (#11)

* Add tests for choices attribute check

* Fix handling of empty choices values

This was accidentally returning the warning without paramaters filled in
for empty values, instead of actually skipping them.

Author: PiDelport

tests/models.py

@@ -84,6 +84,26 @@ class LocationTimeZoneChoices(models.Model):
     )
 
 
+class LocationTimeZoneChoicesWithEmpty(models.Model):
+    timezone = TimeZoneField(
+        verbose_name=_('timezone'),
+        max_length=64,
+        null=True,
+        blank=True,
+        choices=[('', 'No time zone')] + list(PRETTY_ALL_TIMEZONES_CHOICES),
+    )
+
+
+class LocationTimeZoneBadChoices(models.Model):
+    timezone = TimeZoneField(
+        verbose_name=_('timezone'),
+        max_length=64,
+        null=True,
+        blank=True,
+        choices=[('Bad/Worse', 'Bad Choice')],
+    )
+
+
 class TZWithGoodStringDefault(models.Model):
     """Test should validate that"""
     timezone = TimeZoneField(

tests/test_choices.py

@@ -8,9 +8,11 @@
 import re
 
 # Django
+from django.core import checks
 from django.test import TestCase
 
 # App
+from tests import models
 from timezone_utils.choices import (ALL_TIMEZONES_CHOICES,
                                     COMMON_TIMEZONES_CHOICES,
                                     GROUPED_ALL_TIMEZONES_CHOICES,
@@ -170,3 +172,21 @@ def test_get_choices_values_ungrouped(self):
         values = map(itemgetter(0), choices)
         for value in values:
             self.assertIn(value, pytz.all_timezones)
+
+    def test_check_choices_attribute_good(self):
+        self.assertEqual(models.LocationTimeZoneChoices.check(), [])
+        # Don't warn for empty values:
+        self.assertEqual(models.LocationTimeZoneChoicesWithEmpty.check(), [])
+
+    def test_check_choices_attribute_bad(self):
+        self.assertEqual(models.LocationTimeZoneBadChoices.check(), [
+            checks.Warning(
+                msg=(
+                    "'choices' contains an invalid time zone value 'Bad/Worse' "
+                    "which was not found as a supported time zone by pytz "
+                    "{version}.".format(version=pytz.__version__)
+                ),
+                hint='Values must be found in pytz.all_timezones.',
+                obj=models.LocationTimeZoneBadChoices._meta.get_field('timezone'),
+            ),
+        ])

timezone_utils/fields.py

@@ -195,10 +195,11 @@ def _check_choices_attribute(self):   # pragma: no cover
                                     )
                                 })
 
-                            # Return the warning
-                            return [
-                                checks.Warning(**warning_params)
-                            ]
+                                # Return the warning
+                                return [
+                                    checks.Warning(**warning_params)
+                                ]
+
                 elif option_key not in pytz.all_timezones:
                     # Make sure we don't raise this error on empty
                     #   values
@@ -211,10 +212,10 @@ def _check_choices_attribute(self):   # pragma: no cover
                             )
                         })
 
-                    # Return the warning
-                    return [
-                        checks.Warning(**warning_params)
-                    ]
+                        # Return the warning
+                        return [
+                            checks.Warning(**warning_params)
+                        ]
 
         # When no error, return an empty list
         return []