Return client error status code for invalid form submissions (#849)

* Returns client error status code for invalid form submissions

* fix flaky test

bookmarks/tests/test_bookmark_edit_view.py

@@ -26,6 +26,11 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
         }
         return {**form_data, **overrides}
 
+    def test_should_render_successfully(self):
+        bookmark = self.setup_bookmark()
+        response = self.client.get(reverse("bookmarks:edit", args=[bookmark.id]))
+        self.assertEqual(response.status_code, 200)
+
     def test_should_edit_bookmark(self):
         bookmark = self.setup_bookmark()
         form_data = self.create_form_data({"id": bookmark.id})
@@ -46,6 +51,14 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertEqual(tags[0].name, "editedtag1")
         self.assertEqual(tags[1].name, "editedtag2")
 
+    def test_should_return_422_with_invalid_form(self):
+        bookmark = self.setup_bookmark()
+        form_data = self.create_form_data({"id": bookmark.id, "url": ""})
+        response = self.client.post(
+            reverse("bookmarks:edit", args=[bookmark.id]), form_data
+        )
+        self.assertEqual(response.status_code, 422)
+
     def test_should_edit_unread_state(self):
         bookmark = self.setup_bookmark()
 

bookmarks/tests/test_bookmark_new_view.py

@@ -46,6 +46,11 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
         self.assertEqual(tags[0].name, "tag1")
         self.assertEqual(tags[1].name, "tag2")
 
+    def test_should_return_422_with_invalid_form(self):
+        form_data = self.create_form_data({"url": ""})
+        response = self.client.post(reverse("bookmarks:new"), form_data)
+        self.assertEqual(response.status_code, 422)
+
     def test_should_create_new_unread_bookmark(self):
         form_data = self.create_form_data({"unread": True})
 

bookmarks/tests/test_password_change_view.py

@@ -43,6 +43,7 @@ class PasswordChangeViewTestCase(TestCase, BookmarkFactoryMixin):
 
         response = self.client.post(reverse("change_password"), form_data)
 
+        self.assertEqual(response.status_code, 422)
         self.assertIn("old_password", response.context_data["form"].errors)
 
     def test_should_return_error_for_mismatching_new_password(self):
@@ -54,4 +55,5 @@ class PasswordChangeViewTestCase(TestCase, BookmarkFactoryMixin):
 
         response = self.client.post(reverse("change_password"), form_data)
 
+        self.assertEqual(response.status_code, 422)
         self.assertIn("new_password2", response.context_data["form"].errors)

bookmarks/tests/test_settings_general_view.py

@@ -22,6 +22,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
         if not overrides:
             overrides = {}
         form_data = {
+            "update_profile": "",
             "theme": UserProfile.THEME_AUTO,
             "bookmark_date_display": UserProfile.BOOKMARK_DATE_DISPLAY_RELATIVE,
             "bookmark_description_display": UserProfile.BOOKMARK_DESCRIPTION_DISPLAY_INLINE,
@@ -195,6 +196,12 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
 
         self.assertSuccessMessage(html, "Profile updated")
 
+    def test_update_profile_with_invalid_form_returns_422(self):
+        form_data = self.create_profile_form_data({"items_per_page": "-1"})
+        response = self.client.post(reverse("bookmarks:settings.update"), form_data)
+
+        self.assertEqual(response.status_code, 422)
+
     def test_update_profile_should_not_be_called_without_respective_form_action(self):
         form_data = {
             "theme": UserProfile.THEME_DARK,
@@ -217,7 +224,6 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
             # Enabling favicons schedules update
             form_data = self.create_profile_form_data(
                 {
-                    "update_profile": "",
                     "enable_favicons": True,
                 }
             )
@@ -331,7 +337,6 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin):
             # Enabling favicons schedules update
             form_data = self.create_profile_form_data(
                 {
-                    "update_profile": "",
                     "enable_preview_images": True,
                 }
             )

bookmarks/tests/test_singlefile_service.py

@@ -1,3 +1,4 @@
+import secrets
 import gzip
 import os
 import subprocess
@@ -9,9 +10,10 @@ from bookmarks.services import singlefile
 
 
 class SingleFileServiceTestCase(TestCase):
-    html_content = "<html><body><h1>Hello, World!</h1></body></html>"
+    def setUp(self):
-    html_filepath = "temp.html.gz"
+        self.html_content = "<html><body><h1>Hello, World!</h1></body></html>"
-    temp_html_filepath = "temp.html.gz.tmp"
+        self.html_filepath = secrets.token_hex(8) + ".html.gz"
+        self.temp_html_filepath = self.html_filepath + ".tmp"
 
     def tearDown(self):
         if os.path.exists(self.html_filepath):

bookmarks/views/bookmarks.py

@@ -150,7 +150,6 @@ def convert_tag_string(tag_string: str):
 
 @login_required
 def new(request):
-    status = 200
     initial_url = request.GET.get("url")
     initial_title = request.GET.get("title")
     initial_description = request.GET.get("description")
@@ -169,8 +168,6 @@ def new(request):
                 return HttpResponseRedirect(reverse("bookmarks:close"))
             else:
                 return HttpResponseRedirect(reverse("bookmarks:index"))
-        else:
-            status = 422
     else:
         form = BookmarkForm()
         if initial_url:
@@ -186,6 +183,7 @@ def new(request):
         if initial_mark_unread:
             form.initial["unread"] = "true"
 
+    status = 422 if request.method == "POST" and not form.is_valid() else 200
     context = {
         "form": form,
         "auto_close": initial_auto_close,
@@ -216,9 +214,10 @@ def edit(request, bookmark_id: int):
 
     form.initial["tag_string"] = build_tag_string(bookmark.tag_names, " ")
 
+    status = 422 if request.method == "POST" and not form.is_valid() else 200
     context = {"form": form, "bookmark_id": bookmark_id, "return_url": return_url}
 
-    return render(request, "bookmarks/edit.html", context)
+    return render(request, "bookmarks/edit.html", context, status=status)
 
 
 def remove(request, bookmark_id: int):

siteroot/urls.py

@@ -40,6 +40,13 @@ class LinkdingLoginView(auth_views.LoginView):
         return response
 
 
+class LinkdingPasswordChangeView(auth_views.PasswordChangeView):
+    def form_invalid(self, form):
+        response = super().form_invalid(form)
+        response.status_code = 422
+        return response
+
+
 urlpatterns = [
     path("admin/", linkding_admin_site.urls),
     path(
@@ -50,7 +57,7 @@ urlpatterns = [
     path("logout/", auth_views.LogoutView.as_view(), name="logout"),
     path(
         "change-password/",
-        auth_views.PasswordChangeView.as_view(),
+        LinkdingPasswordChangeView.as_view(),
         name="change_password",
     ),
     path(