mirror
/
wagtail
kopia lustrzana https://github.com/wagtail/wagtail
1
0
Forkuj 0
Kod Zgłoszenia Packages Projekty Wydania Wiki Aktywność

Improve form validation in search promotions form 

Move the individual form validation to the SearchPromotionForm class
instead of the formset.

Do not assume external_link_url and the other fields are available in
cleaned_data, which can cause a crash if the input is invalid.

Attach validation errors to the most relevant section instead of bunging
everything as a non-field error.
pull/11950/merge
Sage Abdullah 2024-11-21 15:47:11 +00:00 zatwierdzone przez Matt Westcott
rodzic f177c02da8
commit 63da8b2733
2 zmienionych plików z 322 dodań i 32 usunięć
Pokaż statystyki Ściągnij plik aktualizacji Ściągnij plik porównania Expand all files Collapse all files

68
wagtail/contrib/search_promotions/forms.py
Wyświetl plik

@ -25,6 +25,49 @@ class SearchPromotionForm(forms.ModelForm):
super().__init__(*args, **kwargs)
self.fields["page"].widget = AdminPageChooser()
def clean(self):
cleaned_data = super().clean()
# Use the raw value instead of from form.cleaned_data so we don't
# consider an invalid field as empty. For example, leaving the
# page field empty and entering an invalid external_link_url
# shouldn't raise an error about needing to enter a page or URL,
# since the user *has* entered (or tried to enter) a URL.
page = self["page"].value()
external_link_url = self["external_link_url"].value()
external_link_text = self["external_link_text"].value()
# Must supply a page or external_link_url (but not both)
if page is None:
if external_link_url:
# if an external_link_url is supplied,
# then external_link_text is also required
if not external_link_text:
self.add_error(
"external_link_text",
forms.ValidationError(
_(
"You must enter an external link text if you enter an external link URL."
)
),
)
else:
self.add_error(
None,
forms.ValidationError(
_("You must recommend a page OR an external link.")
),
)
elif external_link_url:
self.add_error(
None,
forms.ValidationError(
_("Please only select a page OR enter an external link.")
),
)
return cleaned_data
class Meta:
model = SearchPromotion
fields = (
@ -73,31 +116,6 @@ class SearchPromotionsFormSet(SearchPromotionsFormSetBase):
non_empty_forms = 0
for i in range(0, self.total_form_count()):
form = self.forms[i]
page = form.cleaned_data["page"]
external_link_url = form.cleaned_data["external_link_url"]
external_link_text = form.cleaned_data["external_link_text"]
# only a page or external_link_url can be supplied
if page is None:
if external_link_url:
# if an external_link_url then external_link_text is also required
if not external_link_text:
raise forms.ValidationError(
_(
"You must enter an external link text if you enter an external link URL."
)
)
else:
raise forms.ValidationError(
_("You must recommend a page OR an external link.")
)
else:
if external_link_url:
raise forms.ValidationError(
_("Please only select a page OR enter an external link.")
)
if self.can_delete and self._should_delete_form(form):
non_deleted_forms -= 1
if not (form.instance.id is None and not form.has_changed()):

286
wagtail/contrib/search_promotions/tests.py
Wyświetl plik

@ -491,6 +491,33 @@ class TestSearchPromotionsAddView(WagtailTestUtils, TestCase):
# Check that the search pick was created
self.assertTrue(Query.get("test").editors_picks.filter(page_id=1).exists())
def test_post_with_invalid_page(self):
# Submit
post_data = {
"query_string": "test",
"editors_picks-TOTAL_FORMS": 1,
"editors_picks-INITIAL_FORMS": 0,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-page": 9999999999,
"editors_picks-0-description": "Hello",
}
response = self.client.post(reverse("wagtailsearchpromotions:add"), post_data)
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
0,
"page",
"Select a valid choice. "
"That choice is not one of the available choices.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 0, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_with_external_link(self):
# Submit
post_data = {
@ -550,14 +577,16 @@ class TestSearchPromotionsAddView(WagtailTestUtils, TestCase):
}
response = self.client.post(reverse("wagtailsearchpromotions:add"), post_data)
# User should be given an error
# User should be given an error on a specific form in the formset
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
None,
0,
None,
"Please only select a page OR enter an external link.",
)
# Should not raise an error on the top-level formset
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_missing_recommendation(self):
post_data = {
@ -571,14 +600,42 @@ class TestSearchPromotionsAddView(WagtailTestUtils, TestCase):
}
response = self.client.post(reverse("wagtailsearchpromotions:add"), post_data)
# User should be given an error
# User should be given an error on a specific form in the formset
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
None,
0,
None,
"You must recommend a page OR an external link.",
)
# Should not raise an error on the top-level formset
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_invalid_external_link(self):
post_data = {
"query_string": "test",
"editors_picks-TOTAL_FORMS": 1,
"editors_picks-INITIAL_FORMS": 0,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-external_link_url": "notalink",
"editors_picks-0-external_link_text": "Wagtail",
"editors_picks-0-description": "Hello",
}
response = self.client.post(reverse("wagtailsearchpromotions:add"), post_data)
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
0,
"external_link_url",
"Enter a valid URL.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 0, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_missing_external_text(self):
post_data = {
@ -592,15 +649,19 @@ class TestSearchPromotionsAddView(WagtailTestUtils, TestCase):
}
response = self.client.post(reverse("wagtailsearchpromotions:add"), post_data)
# User should be given an error
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
None,
None,
0,
"external_link_text",
"You must enter an external link text if you enter an external link URL.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 0, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
class TestSearchPromotionsEditView(WagtailTestUtils, TestCase):
def setUp(self):
@ -657,6 +718,42 @@ class TestSearchPromotionsEditView(WagtailTestUtils, TestCase):
"Description has changed",
)
def test_post_with_invalid_page(self):
# Submit
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-page": 1,
"editors_picks-0-description": "Description has changed", # Change
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 1,
"editors_picks-1-page": 9214599,
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
1,
"page",
"Select a valid choice. "
"That choice is not one of the available choices.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 0, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], 1, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_reorder(self):
# Check order before reordering
self.assertEqual(Query.get("Hello").editors_picks.all()[0], self.search_pick)
@ -698,6 +795,45 @@ class TestSearchPromotionsEditView(WagtailTestUtils, TestCase):
self.assertEqual(Query.get("Hello").editors_picks.all()[0], self.search_pick_2)
self.assertEqual(Query.get("Hello").editors_picks.all()[1], self.search_pick)
def test_post_with_external_link(self):
# Submit
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 1, # Change
"editors_picks-0-external_link_url": "https://wagtail.org",
"editors_picks-0-external_link_text": "Wagtail",
"editors_picks-0-description": "Root page",
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 0, # Change
"editors_picks-1-external_link_url": "https://djangoproject.com",
"editors_picks-1-external_link_text": "Django",
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be redirected back to the index
self.assertRedirects(response, reverse("wagtailsearchpromotions:index"))
# Check that the search pick was created
self.assertTrue(
Query.get("Hello")
.editors_picks.filter(external_link_url="https://wagtail.org")
.exists()
)
self.assertTrue(
Query.get("Hello")
.editors_picks.filter(external_link_url="https://djangoproject.com")
.exists()
)
def test_post_delete_recommendation(self):
# Submit
post_data = {
@ -762,6 +898,142 @@ class TestSearchPromotionsEditView(WagtailTestUtils, TestCase):
"Please specify at least one recommendation for this search term.",
)
def test_post_with_page_and_external_link(self):
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-page": 1,
"editors_picks-0-description": "Description has changed", # Change
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 1,
"editors_picks-1-page": 2,
"editors_picks-1-external_link_url": "https://wagtail.org",
"editors_picks-1-external_link_text": "Wagtail",
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be given an error on a specific form in the formset
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
1,
None,
"Please only select a page OR enter an external link.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], 0, None, [])
def test_post_missing_recommendation(self):
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-description": "Description has changed", # Change
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 1,
"editors_picks-1-external_link_url": "https://wagtail.org",
"editors_picks-1-external_link_text": "Wagtail",
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be given an error on a specific form in the formset
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
0,
None,
"You must recommend a page OR an external link.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], 1, None, [])
def test_post_invalid_external_link(self):
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-page": 1,
"editors_picks-0-description": "Description has changed", # Change
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 1,
"editors_picks-1-external_link_url": "notalink",
"editors_picks-1-external_link_text": "Wagtail",
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
1,
"external_link_url",
"Enter a valid URL.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 1, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
def test_post_missing_external_text(self):
post_data = {
"query_string": "Hello",
"editors_picks-TOTAL_FORMS": 2,
"editors_picks-INITIAL_FORMS": 2,
"editors_picks-MAX_NUM_FORMS": 1000,
"editors_picks-0-id": self.search_pick.id,
"editors_picks-0-DELETE": "",
"editors_picks-0-ORDER": 0,
"editors_picks-0-page": 1,
"editors_picks-0-description": "Description has changed", # Change
"editors_picks-1-id": self.search_pick_2.id,
"editors_picks-1-DELETE": "",
"editors_picks-1-ORDER": 1,
"editors_picks-1-external_link_url": "https://wagtail.org",
"editors_picks-1-description": "Homepage",
}
response = self.client.post(
reverse("wagtailsearchpromotions:edit", args=(self.query.id,)), post_data
)
# User should be given an error on the specific field in the form
self.assertEqual(response.status_code, 200)
self.assertFormSetError(
response.context["searchpicks_formset"],
1,
"external_link_text",
"You must enter an external link text if you enter an external link URL.",
)
# Should not raise an error anywhere else
self.assertFormSetError(response.context["searchpicks_formset"], 1, None, [])
self.assertFormSetError(response.context["searchpicks_formset"], None, None, [])
class TestSearchPromotionsDeleteView(WagtailTestUtils, TestCase):
def setUp(self):