Skip to content

Commit

Permalink
[back] fix: validate score and score_max in ComparisonCriteriaScore f…
Browse files Browse the repository at this point in the history
…orm (#1996)
GresilleSiffle authored Aug 1, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 89795e0 commit d8a5cf1
Showing 2 changed files with 88 additions and 23 deletions.
16 changes: 13 additions & 3 deletions backend/tournesol/models/comparisons.py
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
import uuid

import computed_property
from django.core.exceptions import ValidationError
from django.core.validators import MinValueValidator
from django.db import models
from django.db.models import F, ObjectDoesNotExist, Q
@@ -37,13 +38,13 @@ class Meta:
related_name="comparisons"
)
entity_1 = models.ForeignKey(
'Entity',
"Entity",
on_delete=models.CASCADE,
related_name="comparisons_entity_1",
help_text="Left entity to compare",
)
entity_2 = models.ForeignKey(
'Entity',
"Entity",
on_delete=models.CASCADE,
related_name="comparisons_entity_2",
help_text="Right entity to compare",
@@ -147,7 +148,7 @@ class Meta:
def __str__(self):
return f"{self.comparison}/{self.criteria}/{self.score}"

def save(self, *args, **kwargs):
def _validate_score_max(self):
if self.score_max is None:
raise TypeError("The value of score_max cannot be None.")

@@ -159,4 +160,13 @@ def save(self, *args, **kwargs):
f"The absolute value of the score {self.score} given to the criterion "
f"{self.criteria} can't be greater than the value of score_max {self.score_max}."
)

def clean(self):
try:
self._validate_score_max()
except (TypeError, ValueError) as err:
raise ValidationError({"score_max": err.args[0]}) from err

def save(self, *args, **kwargs):
self._validate_score_max()
return super().save(*args, **kwargs)
95 changes: 75 additions & 20 deletions backend/tournesol/tests/test_model_comparisoncriteriascore.py
Original file line number Diff line number Diff line change
@@ -2,6 +2,8 @@
All test cases of the `ComparisonCriteriaScore` model.
"""

from unittest.mock import Mock

from django.core.exceptions import ValidationError
from django.test import TestCase

@@ -23,7 +25,7 @@ def setUp(self):
self.user = UserFactory(username=self._user)
self.comparison = ComparisonFactory(poll=self.poll, user=self.user)

def test_validators_score_max(self):
def test_score_max_validators(self):
score = ComparisonCriteriaScore(
comparison=self.comparison,
criteria=self.poll.main_criteria,
@@ -43,8 +45,8 @@ def test_validators_score_max(self):
score.score_max = 1
score.clean_fields()

def test_save_validate_score(self):
score = ComparisonCriteriaScore(
def test_validate_score_max(self):
score_test = ComparisonCriteriaScore(
comparison=self.comparison,
criteria=self.poll.main_criteria,
score=11,
@@ -53,27 +55,26 @@ def test_save_validate_score(self):

# The score cannot be greater than score_max.
with self.assertRaises(ValueError):
score.save()
score_test._validate_score_max()

score.score = -11
score_test.score = -11
# The absolute value of the score cannot be greater than score_max.
with self.assertRaises(ValueError):
score.save()
score_test._validate_score_max()

# The score can be zero.
score.score = 0
score.save()
score_test.score = 0
score_test._validate_score_max()

# The score can be equal to the score_max.
score.score = score.score_max
score.save()
score_test.score = score_test.score_max
score_test._validate_score_max()

# The absolute value of the score can be lesser than score_max.
score.score = -1
score.save()
score_test.score = -1
score_test._validate_score_max()

def test_save_validate_score_max(self):
score = ComparisonCriteriaScore(
score_max_test = ComparisonCriteriaScore(
comparison=self.comparison,
criteria=self.poll.main_criteria,
score=5,
@@ -82,17 +83,71 @@ def test_save_validate_score_max(self):

# score_max cannot be None.
with self.assertRaises(TypeError):
score.save()
score_max_test._validate_score_max()

score.score_max = 0
score_max_test.score_max = 0
# score_max cannot be zero.
with self.assertRaises(ValueError):
score.save()
score_max_test._validate_score_max()

score.score_max = -10
score_max_test.score_max = -10
# score_max cannot be negative.
with self.assertRaises(ValueError):
score.save()
score_max_test._validate_score_max()

score_max_test.score_max = 10
score_max_test._validate_score_max()

def test_clean_calls_validate_score_max(self):
"""
The method `clean` should call the method `_validate_score_max` and
transform its exceptions into `ValidationError`.
"""
score = ComparisonCriteriaScore(
comparison=self.comparison,
criteria=self.poll.main_criteria,
score=10,
score_max=10,
)

score.score_max = 10
score._validate_score_max = Mock()
score.clean()
score._validate_score_max.assert_called_once()

# The exceptions raised by _validate_score_max should be transformed
# into `ValidationError` by the method `clean`.
expected_exceptions = [TypeError("oops"), ValueError("oops")]
for exception in expected_exceptions:
score._validate_score_max = Mock(side_effect=exception)

with self.assertRaises(ValidationError):
score.clean()

# All other exceptions should not be transformed.
score._validate_score_max = Mock(side_effect=KeyError)
with self.assertRaises(KeyError):
score.clean()

def test_save_calls_validate_score_max(self):
"""
The method `save` should call the method `_validate_score_max`.
"""
score = ComparisonCriteriaScore(
comparison=self.comparison,
criteria=self.poll.main_criteria,
score=10,
score_max=10,
)

score._validate_score_max = Mock()
score.save()
score._validate_score_max.assert_called_once()

# All exceptions raised by _validate_score_max should not be
# transformed by `save`.
expected_exceptions = [KeyError, TypeError, ValueError]
for exception in expected_exceptions:
score._validate_score_max = Mock(side_effect=exception)

with self.assertRaises(exception):
score.save()

0 comments on commit d8a5cf1

Please sign in to comment.