model.Service: move validation logic in validators

Author: deronnax

appointment/models.py

@@ -64,14 +64,14 @@ class Service(models.Model):
     Version: 1.1.0
     Since: 1.0.0
     """
-    name = models.CharField(max_length=100)
+    name = models.CharField(max_length=100, blank=False)
     description = models.TextField(blank=True, null=True)
-    duration = models.DurationField()
+    duration = models.DurationField(validators=[MinValueValidator(datetime.timedelta(seconds=1))])
     price = models.DecimalField(max_digits=6, decimal_places=2, validators=[MinValueValidator(0)])
     down_payment = models.DecimalField(max_digits=6, decimal_places=2, default=0, validators=[MinValueValidator(0)])
     image = models.ImageField(upload_to='services/', blank=True, null=True)
     currency = models.CharField(max_length=3, default='USD', validators=[MaxLengthValidator(3), MinLengthValidator(3)])
-    background_color = models.CharField(max_length=50, null=True, blank=True, default="")
+    background_color = models.CharField(max_length=50, null=True, blank=True, default=generate_rgb_color)
     reschedule_limit = models.PositiveIntegerField(
         default=0,
         help_text=_("Maximum number of times an appointment can be rescheduled.")
@@ -88,22 +88,6 @@ class Service(models.Model):
     def __str__(self):
         return self.name
 
-    def save(self, *args, **kwargs):
-        # duration shouldn't be negative or equal to 0
-        if self.duration <= datetime.timedelta(seconds=0):
-            raise ValidationError(_("Duration must be greater than 0"))
-        # name shouldn't be empty
-        if self.name == "":
-            raise ValidationError(_("Name cannot be empty"))
-        # price shouldn't be negative
-        if self.price < 0:
-            raise ValidationError(_("Price cannot be negative"))
-        if self.down_payment < 0:
-            raise ValidationError(_("Down payment cannot be negative"))
-        if self.background_color == "":
-            self.background_color = generate_rgb_color()
-        return super().save(*args, **kwargs)
-
     def to_dict(self):
         return {
             "id": self.id,

appointment/tests/models/test_model_service.py

@@ -137,8 +137,8 @@ def test_invalid_service_currency_length(self):
 
     def test_service_duration_zero(self):
         """A service cannot be created with a duration of zero."""
-        with self.assertRaises(ValidationError):
-            Service.objects.create(name="Test Service", duration=timedelta(), price=100)
+        service = Service(name="Test Service", duration=timedelta(0), price=100)
+        self.assertRaises(ValidationError, service.full_clean)
 
     def test_price_and_down_payment_same(self):
         """A service can be created with a price and down payment of the same value."""
@@ -148,30 +148,31 @@ def test_price_and_down_payment_same(self):
     def test_service_with_no_name(self):
         """A service cannot be created with no name."""
         with self.assertRaises(ValidationError):
-            Service.objects.create(name="", duration=timedelta(hours=1), price=100)
+            Service.objects.create(name="", duration=timedelta(hours=1), price=100).full_clean()
 
     def test_service_with_invalid_duration(self):
         """Service should not be created with a negative or zero duration."""
-        with self.assertRaises(ValidationError):
-            Service.objects.create(name="Invalid Duration Service", duration=timedelta(seconds=-1), price=50)
-        with self.assertRaises(ValidationError):
-            Service.objects.create(name="Zero Duration Service", duration=timedelta(seconds=0), price=50)
+        service = Service(name="Invalid Duration Service", duration=timedelta(seconds=-1), price=50)
+        self.assertRaises(ValidationError, service.full_clean)
+        service = Service(name="Zero Duration Service", duration=timedelta(seconds=0), price=50)
+        self.assertRaises(ValidationError, service.full_clean)
 
     def test_service_with_empty_name(self):
         """Service should not be created with an empty name."""
-        with self.assertRaises(ValidationError):
-            Service.objects.create(name="", duration=timedelta(hours=1), price=50)
+        service = Service.objects.create(name="", duration=timedelta(hours=1), price=50)
+        self.assertRaises(ValidationError, service.full_clean)
 
     def test_service_with_negative_price(self):
         """Service should not be created with a negative price."""
-        with self.assertRaises(ValidationError):
-            Service.objects.create(name="Negative Price Service", duration=timedelta(hours=1), price=-1)
+        service = Service(name="Negative Price Service", duration=timedelta(hours=1), price=-1)
+        self.assertRaises(ValidationError, service.full_clean)
 
     def test_service_with_negative_down_payment(self):
         """Service should not have a negative down payment."""
         with self.assertRaises(ValidationError):
-            Service.objects.create(name="Service with Negative Down Payment", duration=timedelta(hours=1), price=50,
+            service = Service(name="Service with Negative Down Payment", duration=timedelta(hours=1), price=50,
                                    down_payment=-1)
+            service.full_clean()
 
     def test_service_auto_generate_background_color(self):
         """Service should auto-generate a background color if none is provided."""