More Advanced Forms
Let’s look at some more advanced forms usage. We’ve helped our users to avoid blank list items, so now let’s help them avoid duplicate items.
Our validation constraint so far has been about preventing blank items, and as you may remember, it turned out we can enforce that very easily in the frontend. Avoiding duplicate items is less straightforward to do in the frontend (although not impossible, of course), so this chapter will lean more heavily on server-side validation, and bubbling errors from the backend back up to the UI.
This chapter goes into the more intricate details of Django’s forms framework, so you have my official permission to skim through it if you already know all about customising Django forms and how to display errors in the UI, or if you’re reading this book for the TDD rather than for the Django.
If you’re still learning Django, there’s good stuff in here! If you want to just skim-read, that’s OK too. Make sure you take a quick look at the aside on developer silliness, and the recap on testing views at the end.
Another FT for Duplicate Items
We add a second test method to ItemValidationTest,
and tell a little story about what we want to see happen
when a user tries to enter the same item twice into their to-do list:
def test_cannot_add_duplicate_items(self):
# Edith goes to the home page and starts a new list
self.browser.get(self.live_server_url)
self.get_item_input_box().send_keys("Buy wellies")
self.get_item_input_box().send_keys(Keys.ENTER)
self.wait_for_row_in_list_table("1: Buy wellies")
# She accidentally tries to enter a duplicate item
self.get_item_input_box().send_keys("Buy wellies")
self.get_item_input_box().send_keys(Keys.ENTER)
# She sees a helpful error message
self.wait_for(
lambda: self.assertEqual(
self.browser.find_element(By.CSS_SELECTOR, ".invalid-feedback").text,
"You've already got this in your list",
)
)Why have two test methods rather than extending one, or having a new file and class? It’s a judgement call. These two feel closely related; they’re both about validation on the same input field, so it feels right to keep them in the same file. On the other hand, they’re logically separate enough that it’s practical to keep them in different methods:
$ python src/manage.py test functional_tests.test_list_item_validation [...] selenium.common.exceptions.NoSuchElementException: Message: Unable to locate element: .invalid-feedback; [...] Ran 2 tests in 9.613s
OK, so we know the first of the two tests passes now. Is there a way to run just the failing one, I hear you ask? Why, yes indeed:
$ python src/manage.py test functional_tests.\ test_list_item_validation.ItemValidationTest.test_cannot_add_duplicate_items [...] selenium.common.exceptions.NoSuchElementException: Message: Unable to locate element: .invalid-feedback; [...]
In any case let’s commit it:
$ git commit -am"Ft for duplicate item validation"
Preventing Duplicates at the Model Layer
So, if we want to start to implement our actual objective for the chapter, let’s write a new test that checks that duplicate items in the same list raise an error:
def test_duplicate_items_are_invalid(self):
mylist = List.objects.create()
Item.objects.create(list=mylist, text="bla")
with self.assertRaises(ValidationError):
item = Item(list=mylist, text="bla")
item.full_clean()And, while it occurs to us, we add another test to make sure we don’t overdo it on our integrity constraints:
def test_CAN_save_same_item_to_different_lists(self):
list1 = List.objects.create()
list2 = List.objects.create()
Item.objects.create(list=list1, text="bla")
item = Item(list=list2, text="bla")
item.full_clean() # should not raiseI always like to put a little comment for tests which are checking that a particular use case should not raise an error; otherwise, it can be hard to see what’s being tested:
AssertionError: ValidationError not raised
If we want to get it deliberately wrong, we can do this:
class Item(models.Model):
text = models.TextField(default="", unique=True)
list = models.ForeignKey(List, default=None, on_delete=models.CASCADE)That lets us check that our second test really does pick up on this problem:
ERROR: test_CAN_save_same_item_to_different_lists (lists.tests.test_models.List
AndItemModelsTest.test_CAN_save_same_item_to_different_lists)
---------------------------------------------------------------------
Traceback (most recent call last):
File "...goat-book/src/lists/tests/test_models.py", line 59, in
test_CAN_save_same_item_to_different_lists
item.full_clean() # should not raise
[...]
django.core.exceptions.ValidationError: {'text': ['Item with this Text already
exists.']}
[...]
Just like ModelForm, models can use an inner class called Meta,
and that’s where we can implement a constraint
which says that an item must be unique for a particular list,
or in other words, that text and list must be unique together:
class Item(models.Model):
text = models.TextField(default="")
list = models.ForeignKey(List, default=None, on_delete=models.CASCADE)
class Meta:
unique_together = ("list", "text")And that passes:
Ran 24 tests in 0.024s OK
You might want to take a quick peek at the
Django docs on model Meta attributes
at this point.
Rewriting the Old Model Test
That long-winded model test did serendipitously help us find unexpected
bugs, but now it’s time to rewrite it. I wrote it in a very verbose style to
introduce the Django ORM, but in fact, we can get the same coverage from a
couple of much shorter tests.
Delete test_saving_and_retrieving_items and replace it with this:
class ListAndItemModelsTest(TestCase):
def test_default_text(self):
item = Item()
self.assertEqual(item.text, "")
def test_item_is_related_to_list(self):
mylist = List.objects.create()
item = Item()
item.list = mylist
item.save()
self.assertIn(item, mylist.item_set.all())
[...]That’s more than enough really—a check of the default values of attributes on a freshly initialized model object is enough to sense-check that we’ve probably set some fields up in models.py. The "item is related to list" test is a real "belt and braces" test to make sure that our foreign key relationship works.
While we’re at it, we can split this file out into tests for Item and tests
for List (there’s only one of the latter, test_get_absolute_url):
class ItemModelTest(TestCase):
def test_default_text(self):
[...]
class ListModelTest(TestCase):
def test_get_absolute_url(self):
[...]That’s neater and tidier:
$ python src/manage.py test lists [...] Ran 25 tests in 0.092s OK
Integrity Errors That Show Up on Save
A final aside before we move on. Do you remember the discussion in mentioned in [chapter_14_database_layer_validation] that some data integrity errors are picked up on save? It all depends on whether the integrity constraint is actually being enforced by the database.
Try running makemigrations and you’ll see
that Django wants to add the unique_together constraint to the database itself,
rather than just having it as an application-layer constraint:
$ python src/manage.py makemigrations
Migrations for 'lists':
src/lists/migrations/0005_alter_item_unique_together.py
~ Alter unique_together for item (1 constraint(s))
Now let’s run the migration:
$ python src/manage.py migrate
Now if we change our duplicates test to do a .save instead of a
.full_clean…
def test_duplicate_items_are_invalid(self):
mylist = List.objects.create()
Item.objects.create(list=mylist, text="bla")
with self.assertRaises(ValidationError):
item = Item(list=mylist, text="bla")
# item.full_clean()
item.save()It gives:
ERROR: test_duplicate_items_are_invalid (lists.tests.test_models.ItemModelTest.test_duplicate_items_are_invalid) [...] sqlite3.IntegrityError: UNIQUE constraint failed: lists_item.list_id, lists_item.text [...] django.db.utils.IntegrityError: UNIQUE constraint failed: lists_item.list_id, lists_item.text
You can see that the error bubbles up from SQLite, and it’s a different
error from the one we want, an IntegrityError instead of a ValidationError.
Let’s revert our changes to the test, and see them all passing again:
$ python src/manage.py test lists [...] Ran 25 tests in 0.092s OK
And now it’s time to commit our model-layer changes:
$ git status # should show changes to tests + models and new migration $ git add src/lists $ git diff --staged $ git commit -m "Implement duplicate item validation at model layer"
Experimenting with Duplicate Item Validation at the Views Layer
Let’s try running our FT, to see if that’s made any difference.
selenium.common.exceptions.NoSuchElementException: Message: Unable to locate element: .invalid-feedback; [...]
In case you didn’t see it as it flew past, the site is 500ing,[2], as in [integrity-error-unique-constraint] (feel free to try it out manually).
We need to be clearer on what we want to happen at the views level. Let’s write a unit test, to set out our expectations:
class ListViewTest(TestCase):
[...]
def test_for_invalid_input_nothing_saved_to_db(self):
[...]
def test_for_invalid_input_renders_list_template(self):
[...]
def test_for_invalid_input_shows_error_on_page(self):
[...]
def test_duplicate_item_validation_errors_end_up_on_lists_page(self):
list1 = List.objects.create()
Item.objects.create(list=list1, text="textey")
response = self.client.post(
f"/lists/{list1.id}/",
data={"text": "textey"},
)
expected_error = html.escape("You've already got this in your list")
self.assertContains(response, expected_error) (1)
self.assertTemplateUsed(response, "list.html") (2)
self.assertEqual(Item.objects.all().count(), 1) (3)| 1 | Here’s our main assertion, which is that we want to see a nice error message on the page |
| 2 | Here’s where we check that it’s landing on the normal list page. |
| 3 | And we double-check that we haven’t saved anything to the DB.[3] |
That test confirms that the IntegrityError is bubbling all the way up:
File "...goat-book/src/lists/views.py", line 28, in view_list
form.save(for_list=our_list)
~~~~~~~~~^^^^^^^^^^^^^^^^^^^
[...]
django.db.utils.IntegrityError: UNIQUE constraint failed: lists_item.list_id,
lists_item.text
We want to avoid integrity errors!
Ideally, we want the call to is_valid() to somehow notice
the duplication error before we even try to save.
But to do that, our form will need to know in advance what list it’s being used for.
Let’s put a skip on this test for now:
from unittest import skip
[...]
@skip
def test_duplicate_item_validation_errors_end_up_on_lists_page(self):A More Complex Form to Handle Uniqueness Validation
The form to create a new list only needs to know one thing, the new item text.
A form which validates that list items are unique
needs to know what list they’re in as well.
Just as we overrode the save method on our ItemForm,
this time we’ll override the constructor on our new form class
so that it knows what list it applies to.
Let’s duplicate our tests for the previous form, tweaking them slightly:
[...]
from lists.forms import (
DUPLICATE_ITEM_ERROR,
EMPTY_ITEM_ERROR,
ExistingListItemForm,
ItemForm,
)
[...]
class ExistingListItemFormTest(TestCase):
def test_form_renders_item_text_input(self):
list_ = List.objects.create()
form = ExistingListItemForm(for_list=list_) (1)
self.assertIn('placeholder="Enter a to-do item"', form.as_p())
def test_form_validation_for_blank_items(self):
list_ = List.objects.create()
form = ExistingListItemForm(for_list=list_, data={"text": ""})
self.assertFalse(form.is_valid())
self.assertEqual(form.errors["text"], [EMPTY_ITEM_ERROR])
def test_form_validation_for_duplicate_items(self):
list_ = List.objects.create()
Item.objects.create(list=list_, text="no twins!")
form = ExistingListItemForm(for_list=list_, data={"text": "no twins!"})
self.assertFalse(form.is_valid())
self.assertEqual(form.errors["text"], [DUPLICATE_ITEM_ERROR])| 1 | We’re specifying that our new ExistingListItemForm will take
an argument for_list= in its constructor,
to be able to specify which list the item is for. |
Next we iterate through a few TDD cycles until we get a form with a
custom constructor, which just ignores its for_list argument.
(I won’t show them all, but I’m sure you’ll do them, right? Remember, the Goat
sees all.)
DUPLICATE_ITEM_ERROR = "You've already got this in your list"
[...]
class ExistingListItemForm(forms.models.ModelForm):
def __init__(self, for_list, *args, **kwargs):
super().__init__(*args, **kwargs)At this point our error should be:
ValueError: ModelForm has no model class specified.
Then let’s see if making it inherit from our existing form helps:
class ExistingListItemForm(ItemForm):
def __init__(self, for_list, *args, **kwargs):
super().__init__(*args, **kwargs)Yes, that takes us down to just one failure:
FAIL: test_form_validation_for_duplicate_items (lists.tests.test_forms.Existing
ListItemFormTest.test_form_validation_for_duplicate_items)
[...]
self.assertFalse(form.is_valid())
AssertionError: True is not false
The next step requires a little knowledge of Django’s validation system, can read up on it in the Django docs on model validation and form validation.
We can customise validation for a field by implementing a clean_<fieldname>()
method, and raising a ValidationError if the field is invalid:
from django.core.exceptions import ValidationError
[...]
class ExistingListItemForm(ItemForm):
def __init__(self, for_list, *args, **kwargs):
super().__init__(*args, **kwargs)
self.instance.list = for_list
def clean_text(self):
text = self.cleaned_data["text"]
if self.instance.list.item_set.filter(text=text).exists():
raise forms.ValidationError(DUPLICATE_ITEM_ERROR)
return textThat makes the tests happy:
Found 29 test(s). [...] OK (skipped=1)
We’re there! A quick commit:
$ git diff $ git add src/lists/forms.py src/lists/tests/test_forms.py $ git commit -m "implement ExistingListItemForm, add DUPLICATE_ITEM_ERROR message"
Using the Existing List Item Form in the List View
Now let’s see if we can put this form to work in our view.
We remove the skip, and while we’re at it, we can use our new constant. Tidy.
from lists.forms import (
DUPLICATE_ITEM_ERROR,
EMPTY_ITEM_ERROR,
)
[...]
def test_duplicate_item_validation_errors_end_up_on_lists_page(self):
[...]
expected_error = html.escape(DUPLICATE_ITEM_ERROR)
self.assertContains(response, expected_error)
[...]We see our IntegrityError once again:
django.db.utils.IntegrityError: UNIQUE constraint failed: lists_item.list_id, lists_item.text
Our fix for this is to switch to using the new form class:
from lists.forms import ExistingListItemForm, ItemForm
[...]
def view_list(request, list_id):
our_list = List.objects.get(id=list_id)
form = ExistingListItemForm(for_list=our_list) (1)
if request.method == "POST":
form = ExistingListItemForm(for_list=our_list, data=request.POST) (1)
if form.is_valid():
form.save(for_list=our_list) (2)
[...]
[...]| 1 | We swap out ItemForm for ExistingListItemForm, and pass in the for_list=. |
| 2 | This is a bit annoying, we’re duplicating the for_list= argument.
This form should already know this! |
Customising the Save Method on Our New Form
Programming by wishful thinking as always,
let’s specify in our views.py that we wish we could call save()
without the duplicated argument:
@@ -25,6 +25,6 @@ def view_list(request, list_id):
if request.method == "POST":
form = ExistingListItemForm(for_list=our_list, data=request.POST)
if form.is_valid():
- form.save(for_list=our_list)
+ form.save()
return redirect(our_list)
return render(request, "list.html", {"list": our_list, "form": form})That gives us a failure as expected:
File "...goat-book/src/lists/views.py", line 28, in view_list
form.save()
~~~~~~~~~^^
TypeError: ItemForm.save() missing 1 required positional argument: 'for_list'
Let’s drop down to the forms level, and write another unit test for how we want our save method to work:
class ExistingListItemFormTest(TestCase):
[...]
def test_form_save(self):
mylist = List.objects.create()
form = ExistingListItemForm(for_list=mylist, data={"text": "hi"})
self.assertTrue(form.is_valid())
new_item = form.save()
self.assertEqual(new_item, Item.objects.get())
[...]We can make our form call the grandparent save method:
class ExistingListItemForm(ItemForm):
[...]
def save(self):
return forms.models.ModelForm.save(self) (1)| 1 | This manually calls the grandparent save().
Personal opinion here: I could have used super(),
but I prefer not to use super() when it requires arguments,
say, to get a grandparent.
I find Python 3’s super() with no args is awesome to get the immediate parent.
Anything else is too error-prone, and I find it ugly besides. YMMV. |
OK, how does that look? Yep, both the forms level and views level tests now pass.
$ python src/manage.py test lists [...] Ran 30 tests in 0.082s OK
Time to see what our FTs think!
The FTs Pick Up an Issue with Bootstrap Classes
Unfortunately, the FTs are telling us we’re not done:
$ python src/manage.py test functional_tests.test_list_item_validation [...] FAIL: test_cannot_add_duplicate_items [...] ---------------------------------------------------------------------- [...] AssertionError: '' != "You've already got this in your list" + You've already got this in your list
Let’s spin up the server with runserver, and try it out manually,
with DevTools open, to see what’s going on.
If you look through the HTML, you’ll see our error div is there,
with the correct error text, but it’s greyed out, indicating that it’s hidden,
as in Our Error Div is There But it’s Hidden:
I had to dig through the docs a little,[4]
but it turns out that Bootstrap requires form elements
with errors to have another custom class, is-invalid.
You can actually try this out in devtools! If you double-click, you can actually edit the HTML, and add the class, as in Hack it in Manually Yay:
Conditionally Customising CSS Classes For Invalid Forms
Speaking of hackery, I’m starting to get a bit nervous about the amount of hackery we’re doing in our forms now, but let’s try getting this to work by doing even more customisation on our forms.
We want this behaviour for both types of form really,
so it can go in the tests for the parent ItemForm class:
class ItemFormTest(TestCase):
def test_form_item_input_has_placeholder_and_css_classes(self):
[...]
def test_form_validation_for_blank_items(self):
[...]
def test_invalid_form_has_bootstrap_is_invalid_css_class(self):
form = ItemForm(data={"text": ""})
self.assertFalse(form.is_valid())
field = form.fields["text"]
self.assertEqual(
field.widget.attrs["class"], (1)
"form-control form-control-lg is-invalid",
)
def test_form_save_handles_saving_to_a_list(self):
[...]| 1 | Here’s where you can inspect the class attribute on the input field widget |
And here’s how we can make it work, by overriding the is_valid() method:
class ItemForm(forms.models.ModelForm):
class Meta:
[...]
def is_valid(self):
result = super().is_valid() (1)
if not result:
self.fields["text"].widget.attrs["class"] += " is-invalid" (2)
return result (3)
def save(self, for_list):
[...]| 1 | We make sure to call the parent is_valid() method first,
so we can do all the normal built-in validation. |
| 2 | Here’s how we add the extra CSS class to our widget |
| 3 | And we remember to return the result. |
It’s not too bad, but as I say I’m getting nervous about the amount of fiddly code in our forms classes. Let’s make a note on our scratchpad, and come back to it when our FT is passing paperhaps:
Speaking of our FT, let’s see how it does now?
$ python src/manage.py test functional_tests.test_list_item_validation
[...]
======================================================================
FAIL: test_cannot_add_empty_list_items (functional_tests.test_list_item_validat
ion.ItemValidationTest.test_cannot_add_empty_list_items)
---------------------------------------------------------------------
Traceback (most recent call last):
File "...goat-book/src/functional_tests/test_list_item_validation.py", line
47, in test_cannot_add_empty_list_items
self.wait_for_row_in_list_table("2: Make tea")
File "...goat-book/src/functional_tests/base.py", line 38, in
wait_for_row_in_list_table
self.assertIn(row_text, [row.text for row in rows])
AssertionError: '2: Make tea' not found in ['1: Make tea', '2: Purchase milk']
Ooops what happened here (The Cart is Before the Horse)?
A Little Digression on Queryset Ordering and String Representations
Something seems to be going wrong with the ordering of our list items. Trying to fix this by iterating against an FT is going to be slow, so let’s work at the unit test level.
We’ll add a test that checks that list items are ordered in the order they are inserted. You’ll have to forgive me if I jump straight to the right answer, using intuition borne of long experience, but I suspect that it might be sorting alphabetically based on list text instead (what else would it sort by after all?), so I’ll pick some text values designed to test that hypothesis:
class ListModelTest(TestCase):
def test_get_absolute_url(self):
[...]
def test_list_items_order(self):
list1 = List.objects.create()
item1 = Item.objects.create(list=list1, text="i1")
item2 = Item.objects.create(list=list1, text="item 2")
item3 = Item.objects.create(list=list1, text="3")
self.assertEqual(
list1.item_set.all(),
[item1, item2, item3],
)| FTs are a slow feedback loop. Switch to unit tests when you want to drill down on edge case bugs. |
That gives us a new failure, but it’s not very readable:
AssertionError: <QuerySet [<Item: Item object (3)>, <Item[40 chars]2)>]> != [<Item: Item object (1)>, <Item: Item obj[29 chars](3)>]
We need a better string representation for our Item model.
Let’s add another unit test:
| Ordinarily you would be wary of adding more failing tests when you already have some—it makes reading test output that much more complicated, and just generally makes you nervous. Will we ever get back to a working state? In this case, they’re all quite simple tests, so I’m not worried. |
class ItemModelTest(TestCase):
[...]
def test_string_representation(self):
item = Item(text="some text")
self.assertEqual(str(item), "some text")That gives us:
AssertionError: 'Item object (None)' != 'some text'
As well as the other two failures. Let’s start fixing them all now:
class Item(models.Model):
[...]
def __str__(self):
return self.textNow we’re down to one failure, and the ordering test has a more readable failure message:
AssertionError: <QuerySet [<Item: 3>, <Item: i1>, <Item: item 2>]> != [<Item: i1>, <Item: item 2>, <Item: 3>]
That confirms our suspicion that the ordering was alphabetical.
We can fix that in the class Meta:
class Item(models.Model):
[...]
class Meta:
ordering = ("id",)
unique_together = ("list", "text")Does that work?
AssertionError: <QuerySet [<Item: i1>, <Item: item 2>, <Item: 3>]> != [<Item: i1>, <Item: item 2>, <Item: 3>]
Urp? It has worked; you can see the items are in the same order, but the tests are confused.
I keep running into this problem actually—Django QuerySets don’t compare well with lists. We can fix it by converting the QuerySet to a list,[5] in our test:
self.assertEqual(
list(list1.item_set.all()),
[item1, item2, item3],
)That works; we get a fully passing unit test suite:
Ran 33 tests in 0.034s OK
We do need a migration for that ordering change though:
$ python src/manage.py makemigrations
Migrations for 'lists':
src/lists/migrations/0006_alter_item_options.py
~ Change Meta options on item
And as a final check, we rerun all the FTs:
$ python src/manage.py test functional_tests [...] --------------------------------------------------------------------- Ran 5 tests in 19.048s OK
Hooray! Time for a final commit:
git status git add src git commit -m "Add is-invalid css class, fix list item ordering"
On The Tradeoffs of Django Modelforms, and Frameworks in General
Let’s come back to our scratchpad item:
Let’s take a look at the current state of our forms classes. We’ve got a real mix of presentation logic, validation logic, and ORM/storage logic:
class ItemForm(forms.models.ModelForm):
class Meta:
model = Item
fields = ("text",)
widgets = {
"text": forms.widgets.TextInput(
attrs={
"placeholder": "Enter a to-do item", (1)
"class": "form-control form-control-lg", (1)
}
),
}
error_messages = {"text": {"required": EMPTY_ITEM_ERROR}}
def is_valid(self):
result = super().is_valid()
if not result:
self.fields["text"].widget.attrs["class"] += " is-invalid" (1)
return result
def save(self, for_list): (3)
self.instance.list = for_list
return super().save()
class ExistingListItemForm(ItemForm):
def __init__(self, for_list, *args, **kwargs):
super().__init__(*args, **kwargs)
self.instance.list = for_list (3)
def clean_text(self):
text = self.cleaned_data["text"]
if self.instance.list.item_set.filter(text=text).exists(): (2)
raise forms.ValidationError(DUPLICATE_ITEM_ERROR) (2)
return text
def save(self):
return forms.models.ModelForm.save(self) (3)| 1 | Presentation logic |
| 2 | Validation logic |
| 3 | ORM/storage logic |
I think what’s happened is that we’ve reached the limits of the Django forms framework’s sweet spot.
ModelForms can be great because they can do presentation, validation and database storage all in one go, so you can get a lot done without much code.
But once you want to customise the default behaviours for each of those things,
the code you do end up writing starts to get hard to understand.
Let’s see what things would look like if we tried to:
1. Moved the responsibility for presentation and the rendering of HTML back into the tamplate
2. Stop using ModelForm and do any database logic more explicitly,
with less magic
Moving Presentation Logic Back Into the Template
We’re talking about another refactor here, we want to move some functionality out of the form and into the template/views layer. How do we make sure we’ve got good test coverage?
-
We currently have some tests for the CSS classes including
is-invalidin test_forms.py -
We have some tests of some form attributes in test_views.py, eg the asserts on the input’s
name. -
And the FTs, ultimately, will tell us if things "really work" or not, including testing the interaction between our HTML, Bootstrap and the browser (eg, CSS visibility).
What we are learning is that the things we’re testing in test_forms.py will need to move.
| Lower-level tests are good for exploring an API, but they are tightly coupled to it. Higher level tests can enable more refactoring. |
Here’s one way to write that kind of test:
class ListViewTest(TestCase):
[...]
def test_for_invalid_input_shows_error_on_page(self):
[...]
def test_for_invalid_input_sets_is_invalid_class(self):
response = self.post_invalid_input()
parsed = lxml.html.fromstring(response.content)
[input] = parsed.cssselect("input[name=text]")
self.assertIn("is-invalid", input.get("class"))
def test_duplicate_item_validation_errors_end_up_on_lists_page(self):
[...]That’s green straight away:
Ran 34 tests in 0.040s OK
As always, it’s nice to deliberately break it, to see whether it has a nice failure message if nothing else. Let’s do that in forms.py:
@@ -24,7 +24,7 @@ class ItemForm(forms.models.ModelForm):
def is_valid(self):
result = super().is_valid()
if not result:
- self.fields["text"].widget.attrs["class"] += " is-invalid"
+ self.fields["text"].widget.attrs["class"] += " boo!"
return result
def save(self, for_list):Reassuringly, both our old test and the new one fail:
[...]
======================================================================
FAIL: test_invalid_form_has_bootstrap_is_invalid_css_class (lists.tests.test_fo
rms.ItemFormTest.test_invalid_form_has_bootstrap_is_invalid_css_class)
---------------------------------------------------------------------
Traceback (most recent call last):
File "...goat-book/src/lists/tests/test_forms.py", line 30, in
test_invalid_form_has_bootstrap_is_invalid_css_class
self.assertEqual(
~~~~~~~~~~~~~~~~^
field.widget.attrs["class"],
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"form-control form-control-lg is-invalid",
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
AssertionError: 'form-control form-control-lg boo!' != 'form-control
form-control-lg is-invalid'
- form-control form-control-lg boo!
? ^^^^
+ form-control form-control-lg is-invalid
? ^^^^^^^^^^
======================================================================
FAIL: test_for_invalid_input_sets_is_invalid_class (lists.tests.test_views.List
ViewTest.test_for_invalid_input_sets_is_invalid_class)
---------------------------------------------------------------------
Traceback (most recent call last):
File "...goat-book/src/lists/tests/test_views.py", line 129, in
test_for_invalid_input_sets_is_invalid_class
self.assertIn("is-invalid", input.get("class"))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'is-invalid' not found in 'form-control form-control-lg boo!'
---------------------------------------------------------------------
Ran 34 tests in 0.039s
FAILED (failures=2)
Let’s revert that and get back to passing.
So, rather than using the {{ form.text }} magic in our template,
let’s bring back our hand-crafted HTML.
It’ll be longer,
but at least all of our bootstrap classes will be in one place,
where we expect then, in the template:
@@ -16,10 +16,22 @@
<h1 class="display-1 mb-4">{% block header_text %}{% endblock %}</h1>
<form method="POST" action="{% block form_action %}{% endblock %}" >
- {{ form.text }}
{% csrf_token %}
+ <input (1)
+ id="id_text"
+ name="text"
+ class="form-control (2)
+ form-control-lg
+ {% if form.errors %}is-invalid{% endif %}"
+ placeholder="Enter a to-do item"
+ value="{{ form.text.value | default:'' }}" (3)
+ aria-describedby="id_text_feedback" (4)
+ required
+ />
{% if form.errors %}
- <div class="invalid-feedback">{{ form.errors.text }}</div>
+ <div id="id_text_feedback" class="invalid-feedback"> (4)
+ {{ form.errors.text.0 }} (5)
+ </div>
{% endif %}
</form>
</div>| 1 | Here’s our artisan <input> once again,
and the most important custom setting will be its class attributes. |
| 2 | As you can see, we can use conditionals even for providing additional class -es.[6] |
| 3 | The | default "filter" is a way to avoid the string "None"
from showing up as the value in our input field. |
| 4 | We add an id to the error message,
to be able to use aria-describedby on the input,
as recommended in the Bootstrap docs;
it makes the error message more accessible to screen readers. |
| 5 | If you just try to use form.errors.text you’ll see
that Django injects a <ul> list,
because the forms framework can report multiple errors for each field.
We know we’ve only got one, so we can use use form.errors.text.0. |
That passes.
Ran 34 tests in 0.034s OK
Out of curiosity, let’s try a deliberate failure here?
@@ -22,7 +22,7 @@
name="text"
class="form-control
form-control-lg
- {% if form.errors %}is-invalid{% endif %}"
+ {% if form.errors %}isnt-invalid{% endif %}"
placeholder="Enter a to-do item"
value="{{ form.text.value | default:'' }}"
aria-describedby="id_text_feedback"The failure looks like this:
self.assertIn("is-invalid", input.get("class"))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'is-invalid' not found in 'form-control\n
form-control-lg\n isnt-invalid'
Hmm that’s not idea actually. Let’s tweak our assert:
def test_for_invalid_input_sets_is_invalid_class(self):
response = self.post_invalid_input()
parsed = lxml.html.fromstring(response.content)
[input] = parsed.cssselect("input[name=text]")
self.assertIn("is-invalid", set(input.classes)) (1)| 1 | Rather than using get("class") which returns a raw string,
lxml can give us the classes as a list
(well, actually a special object, but one that we can turn into a set).
That’s more semantically correct, and gives a better error message: |
self.assertIn("is-invalid", set(input.classes))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'is-invalid' not found in {'form-control', 'isnt-invalid',
'form-control-lg'}
OK that’s good, we can revert the deliberate mistake in base.html.
Let’s do a quick FT run to check we’ve got it right:
$ python src/manage.py test functional_tests.test_list_item_validation Found 2 test(s). [...] OK
Good!
Tidying Up the Forms
Now let’s start tidying up our forms.
We can start by deleting the three presentation-layer tests from ItemFormTest,
@@ -10,28 +10,11 @@ from lists.models import Item, List
class ItemFormTest(TestCase):
- def test_form_item_input_has_placeholder_and_css_classes(self):
- form = ItemForm()
-
- rendered = form.as_p()
-
- self.assertIn('placeholder="Enter a to-do item"', rendered)
- self.assertIn('class="form-control form-control-lg"', rendered)
-
def test_form_validation_for_blank_items(self):
form = ItemForm(data={"text": ""})
self.assertFalse(form.is_valid())
self.assertEqual(form.errors["text"], [EMPTY_ITEM_ERROR])
- def test_invalid_form_has_bootstrap_is_invalid_css_class(self):
- form = ItemForm(data={"text": ""})
- self.assertFalse(form.is_valid())
- field = form.fields["text"]
- self.assertEqual(
- field.widget.attrs["class"],
- "form-control form-control-lg is-invalid",
- )
-
def test_form_save_handles_saving_to_a_list(self):
mylist = List.objects.create()
form = ItemForm(data={"text": "do me"})
@@ -42,11 +25,6 @@ class ItemFormTest(TestCase):
class ExistingListItemFormTest(TestCase):
- def test_form_renders_item_text_input(self):
- list_ = List.objects.create()
- form = ExistingListItemForm(for_list=list_)
- self.assertIn('placeholder="Enter a to-do item"', form.as_p())
-
def test_form_validation_for_blank_items(self):
list_ = List.objects.create()
form = ExistingListItemForm(for_list=list_, data={"text": ""})And now we can remove all that custom logic from the base ItemForm class:
@@ -11,22 +11,8 @@ class ItemForm(forms.models.ModelForm):
class Meta:
model = Item
fields = ("text",)
- widgets = {
- "text": forms.widgets.TextInput(
- attrs={
- "placeholder": "Enter a to-do item",
- "class": "form-control form-control-lg",
- }
- ),
- }
error_messages = {"text": {"required": EMPTY_ITEM_ERROR}}
- def is_valid(self):
- result = super().is_valid()
- if not result:
- self.fields["text"].widget.attrs["class"] += " is-invalid"
- return result
-
def save(self, for_list):
self.instance.list = for_list
return super().save()Deleting code yay!
At this point we should be down to 31 passing tests:
Ran 31 tests in 0.024s OK
Switching Back to Simple Forms
Now let’s change our forms away from being ModelForms and back to regular forms.
We’ll keep the save() methods for now,
but we’ll switch to using the ORM more explicitly,
rather than relying on the ModelForm magic:
@@ -7,27 +7,29 @@ EMPTY_ITEM_ERROR = "You can't have an empty list item"
DUPLICATE_ITEM_ERROR = "You've already got this in your list"
-class ItemForm(forms.models.ModelForm):
- class Meta:
- model = Item
- fields = ("text",)
- error_messages = {"text": {"required": EMPTY_ITEM_ERROR}}
+class ItemForm(forms.Form):
+ text = forms.CharField(
+ error_messages={"required": EMPTY_ITEM_ERROR},
+ required=True,
+ )
def save(self, for_list):
- self.instance.list = for_list
- return super().save()
+ return Item.objects.create(
+ list=for_list,
+ text=self.cleaned_data["text"],
+ )
class ExistingListItemForm(ItemForm):
def __init__(self, for_list, *args, **kwargs):
super().__init__(*args, **kwargs)
- self.instance.list = for_list
+ self._for_list = for_list
def clean_text(self):
text = self.cleaned_data["text"]
- if self.instance.list.item_set.filter(text=text).exists():
+ if self._for_list.item_set.filter(text=text).exists():
raise forms.ValidationError(DUPLICATE_ITEM_ERROR)
return text
def save(self):
- return forms.models.ModelForm.save(self)
+ return super().save(for_list=self._for_list)We should still have passing tests at this point:
Ran 31 tests in 0.026s OK
And we’re in a better place I think!
Wrapping Up: What We’ve Learned About Testing Django
We’re now at a point where our app looks a lot more like a "standard" Django app, and it implements the three common Django layers: models, forms, and views. We no longer have any "training wheels”-style tests, and our code looks pretty much like code we’d be happy to see in a real app.
We have one unit test file for each of our key source code files. Here’s a recap of the biggest (and highest-level) one, test_views
Next we’ll try to make our data validation more friendly by using a bit of client-side code. Uh-oh, you know what that means…
Comments