Skip to content

Commit

Permalink
Move custom clean errors to specific fields
Browse files Browse the repository at this point in the history
Move the additional_limitation and rqeuires_study_review Validation
errors to the specific fields, not the NON_FIELD_ERRORS. This provides
better behavior on the forms.
  • Loading branch information
amstilp committed Mar 19, 2024
1 parent 483e15c commit 9a262ac
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 18 deletions.
7 changes: 5 additions & 2 deletions primed/cdsa/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,18 @@ def get_absolute_url(self):
def clean(self):
super().clean()
# Checks for fields only allowed for primary agreements.
errors = {}
if not self.is_primary:
if self.additional_limitations:
raise ValidationError(
errors["additional_limitations"] = ValidationError(
"Additional limitations are only allowed for primary agreements."
)
if self.requires_study_review:
raise ValidationError(
errors["requires_study_review"] = ValidationError(
"requires_study_review can only be True for primary agreements."
)
if errors:
raise ValidationError(errors)

def get_agreement_group(self):
return self.study
Expand Down
17 changes: 10 additions & 7 deletions primed/cdsa/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""Tests for the `cdsa` app."""

from anvil_consortium_manager.tests.factories import WorkspaceFactory
from django.core.exceptions import NON_FIELD_ERRORS
from django.test import TestCase

from primed.duo.models import DataUseModifier
Expand Down Expand Up @@ -448,9 +447,11 @@ def test_invalid_component_with_additional_limitations(self):
}
form = self.form_class(data=form_data)
self.assertFalse(form.is_valid())
self.assertIn(NON_FIELD_ERRORS, form.errors)
self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1)
self.assertIn("only allowed for primary", form.errors[NON_FIELD_ERRORS][0])
self.assertIn("additional_limitations", form.errors)
self.assertEqual(len(form.errors["additional_limitations"]), 1)
self.assertIn(
"only allowed for primary", form.errors["additional_limitations"][0]
)

def test_valid_primary_with_requires_study_review_true(self):
"""Form is valid with necessary input."""
Expand All @@ -473,9 +474,11 @@ def test_invalid_component_with_requires_study_review_true(self):
}
form = self.form_class(data=form_data)
self.assertFalse(form.is_valid())
self.assertIn(NON_FIELD_ERRORS, form.errors)
self.assertEqual(len(form.errors[NON_FIELD_ERRORS]), 1)
self.assertIn("can only be True for primary", form.errors[NON_FIELD_ERRORS][0])
self.assertIn("requires_study_review", form.errors)
self.assertEqual(len(form.errors["requires_study_review"]), 1)
self.assertIn(
"can only be True for primary", form.errors["requires_study_review"][0]
)


class NonDataAffiliateAgreementFormTest(TestCase):
Expand Down
16 changes: 14 additions & 2 deletions primed/cdsa/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,13 @@ def test_clean_additional_limitations_not_primary(self):
)
with self.assertRaises(ValidationError) as e:
instance.clean()
self.assertIn("only allowed for primary agreements", e.exception.message)
self.assertEqual(len(e.exception.error_dict), 1)
self.assertIn("additional_limitations", e.exception.error_dict)
self.assertEqual(len(e.exception.error_dict["additional_limitations"]), 1)
self.assertIn(
"only allowed for primary agreements",
e.exception.error_dict["additional_limitations"][0].message,
)

def test_str_method(self):
"""The custom __str__ method returns the correct string."""
Expand Down Expand Up @@ -574,7 +580,13 @@ def test_requires_study_review_not_primary(self):
)
with self.assertRaises(ValidationError) as e:
instance.clean()
self.assertIn("can only be True for primary", e.exception.message)
self.assertEqual(len(e.exception.error_dict), 1)
self.assertIn("requires_study_review", e.exception.error_dict)
self.assertEqual(len(e.exception.error_dict["requires_study_review"]), 1)
self.assertIn(
"can only be True for primary",
e.exception.error_dict["requires_study_review"][0].message,
)


class NonDataAffiliateAgreementTest(TestCase):
Expand Down
14 changes: 7 additions & 7 deletions primed/cdsa/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission
from django.contrib.messages import get_messages
from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied
from django.core.exceptions import PermissionDenied
from django.http import Http404
from django.shortcuts import resolve_url
from django.test import RequestFactory, TestCase, override_settings
Expand Down Expand Up @@ -2956,11 +2956,11 @@ def test_cannot_create_component_with_requires_study_review(self):
self.assertFalse(formset.is_valid())
self.assertFalse(formset.forms[0].is_valid())
self.assertEqual(len(formset.forms[0].errors), 1)
self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors)
self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1)
self.assertIn("requires_study_review", formset.forms[0].errors)
self.assertEqual(len(formset.forms[0].errors["requires_study_review"]), 1)
self.assertIn(
"can only be True for primary",
formset.forms[0].errors[NON_FIELD_ERRORS][0],
formset.forms[0].errors["requires_study_review"][0],
)

def test_can_create_primary_with_additional_limitations(self):
Expand Down Expand Up @@ -3054,11 +3054,11 @@ def test_cannot_create_component_with_additional_limitations(self):
self.assertFalse(formset.is_valid())
self.assertFalse(formset.forms[0].is_valid())
self.assertEqual(len(formset.forms[0].errors), 1)
self.assertIn(NON_FIELD_ERRORS, formset.forms[0].errors)
self.assertEqual(len(formset.forms[0].errors[NON_FIELD_ERRORS]), 1)
self.assertIn("additional_limitations", formset.forms[0].errors)
self.assertEqual(len(formset.forms[0].errors["additional_limitations"]), 1)
self.assertIn(
"only allowed for primary",
formset.forms[0].errors[NON_FIELD_ERRORS][0],
formset.forms[0].errors["additional_limitations"][0],
)

def test_error_missing_cc_id(self):
Expand Down

0 comments on commit 9a262ac

Please sign in to comment.