Implement validate_unique in a more elegant way and solve a validatio… (#179)

* Implement validate_unique in a more elegant way and solve a validation bug in the original implementation

* add test case for validate_unique

* add test case for validate_unique

* add test case for validate_unique

* update validate_unique to a more readable implementation

* update validate_unique to a more readable implementation

* update validate_unique to a more readable implementation

* Add tests for validate_unique. Thanks @rsichny

Author: tinylambda

django_celery_beat/models.py

@@ -314,21 +314,24 @@ class Meta:
 
     def validate_unique(self, *args, **kwargs):
         super(PeriodicTask, self).validate_unique(*args, **kwargs)
-        if not self.interval and not self.crontab and not self.solar:
+
+        schedule_types = ['interval', 'crontab', 'solar']
+        selected_schedule_types = [s for s in schedule_types
+                                   if getattr(self, s)]
+
+        if len(selected_schedule_types) == 0:
             raise ValidationError({
                 'interval': [
                     'One of interval, crontab, or solar must be set.'
                 ]
             })
+
         err_msg = 'Only one of interval, crontab, or solar must be set'
-        if (self.interval and self.crontab) or (self.crontab and self.solar):
-            raise ValidationError({
-                'crontab': [err_msg]
-            })
-        if self.interval or self.solar:
-            raise ValidationError({
-                'solar': [err_msg]
-            })
+        if len(selected_schedule_types) > 1:
+            error_info = {}
+            for selected_schedule_type in selected_schedule_types:
+                error_info[selected_schedule_type] = [err_msg]
+            raise ValidationError(error_info)
 
     def save(self, *args, **kwargs):
         self.exchange = self.exchange or None

t/unit/test_admin.py

@@ -1,9 +1,15 @@
 from __future__ import absolute_import, unicode_literals
 
+from itertools import combinations
 from django.test import TestCase
 
 from django_celery_beat.admin import PeriodicTaskAdmin
-from django_celery_beat.models import PeriodicTask
+from django_celery_beat.models import \
+    PeriodicTask, \
+    CrontabSchedule, \
+    IntervalSchedule, \
+    SolarSchedule
+from django.core.exceptions import ValidationError
 
 
 class ActionsTests(TestCase):
@@ -51,3 +57,22 @@ def test_toggle_action_all_disabled(self):
         self.assertTrue(e1)
         self.assertTrue(e2)
         self.assertTrue(e3)
+
+    def test_validate_unique_raises_if_schedule_not_set(self):
+        with self.assertRaises(ValidationError):
+            PeriodicTask().validate_unique()
+
+    def test_validate_unique_raises_for_multiple_schedules(self):
+        schedules = [
+            ('crontab', CrontabSchedule()),
+            ('interval', IntervalSchedule()),
+            ('solar', SolarSchedule()),
+        ]
+        for options in combinations(schedules, 2):
+            with self.assertRaises(ValidationError):
+                PeriodicTask(**dict(options)).validate_unique()
+
+    def test_validate_unique_not_raises(self):
+        PeriodicTask(crontab=CrontabSchedule()).validate_unique()
+        PeriodicTask(interval=IntervalSchedule()).validate_unique()
+        PeriodicTask(solar=SolarSchedule()).validate_unique()