launchpad-reviewers team mailing list archive

[Alvaro Crespo Serrano <mp+477422@xxxxxxxxxxxxxxxxxx>]

Alvaro Crespo Serrano has proposed merging ~alvarocs/launchpad:fix-bug-summary-field into launchpad:master.

Commit message:
Add character limit to bug titles summary field.




Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~alvarocs/launchpad/+git/launchpad/+merge/477422

Fix for the bug reported in https://bugs.launchpad.net/launchpad/+bug/2086655. Launchpad did not have character limit for bug titles. This could make the send-bug-notifications get stuck and block bug notifications from being sent.
- Implemented a character limit of 200 for bug title "summary" field in class IBugView & class IBugAddForm for when adding or editing a bug.
- Validator checks in class FileBugViewBase.
- Corresponding unit tests.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~alvarocs/launchpad:fix-bug-summary-field into launchpad:master.

diff --git a/lib/lp/bugs/browser/bugtarget.py b/lib/lp/bugs/browser/bugtarget.py
index cb8c16d..199309d 100644
--- a/lib/lp/bugs/browser/bugtarget.py
+++ b/lib/lp/bugs/browser/bugtarget.py
@@ -410,16 +410,32 @@ class FileBugViewBase(LaunchpadFormView):
         # The comment field is only required if filing a new bug.
         if self.submit_bug_action.submitted():
             comment = data.get("comment")
+            title = data.get("title", "").strip()
             # The widget only exposes the error message. The private
             # attr contains the real error.
-            widget_error = self.widgets.get("comment")._error
-            if widget_error and isinstance(widget_error.errors, TooLong):
+            widget_error_title = self.widgets.get("title")._error
+            widget_error_comment = self.widgets.get("comment")._error
+            if widget_error_title and isinstance(
+                widget_error_title.errors, TooLong
+            ):
+                self.setFieldError(
+                    "title",
+                    "The summary is too long. Please, provide a one-line "
+                    "summary of the problem.",
+                )
+            elif not title or widget_error_title is not None:
+                self.setFieldError(
+                    "title", "Provide a one-line summary of the problem."
+                )
+            if widget_error_comment and isinstance(
+                widget_error_comment.errors, TooLong
+            ):
                 self.setFieldError(
                     "comment",
                     "The description is too long. If you have lots of "
                     "text to add, attach a file to the bug instead.",
                 )
-            elif not comment or widget_error is not None:
+            elif not comment or widget_error_comment is not None:
                 self.setFieldError(
                     "comment", "Provide details about the issue."
                 )
diff --git a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
index d6f0013..769e610 100644
--- a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
+++ b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
@@ -8,7 +8,7 @@ from lazr.restful.interfaces import IJSONRequestCache
 from testscenarios import WithScenarios, load_tests_apply_scenarios
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
-from zope.schema.interfaces import TooLong, TooShort
+from zope.schema.interfaces import RequiredMissing, TooLong, TooShort
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import PUBLIC_INFORMATION_TYPES, InformationType
@@ -338,6 +338,44 @@ class FileBugViewMixin:
 class TestFileBugViewBase(FileBugViewMixin, TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
+    def test_submit_title_empty_error(self):
+        # The title cannot be an empty string.
+        form = self.get_form(title="")
+        view = self.create_initialized_view(form=form)
+        self.assertEqual(2, len(view.errors))
+        self.assertEqual(
+            "Provide a one-line summary of the problem.",
+            view.getFieldError("title"),
+        )
+
+    def test_submit_title_withespace_only_error(self):
+        # The title cannot be a whitespace only string.
+        form = self.get_form(title=" ")
+        view = self.create_initialized_view(form=form)
+        self.assertEqual(2, len(view.errors))
+        self.assertIsInstance(view.errors[0].errors, RequiredMissing)
+        self.assertEqual(
+            "Provide a one-line summary of the problem.", view.errors[1]
+        )
+
+    def test_submit_title_too_large_error(self):
+        # The title cannot exceed the max length of 200.
+        title = "x" * 201
+        form = self.get_form(title=title)
+        view = self.create_initialized_view(form=form)
+        self.assertEqual(2, len(view.errors))
+        self.assertIsInstance(view.errors[0].errors, TooLong)
+        message_start = "The summary is too long. Please, provide a "
+        "one-line summary of the problem."
+        self.assertTrue(view.getFieldError("title").startswith(message_start))
+
+    def test_submit_title_max(self):
+        # The title can be as large as 200.
+        form = self.get_form(title="x" * 200)
+        view = self.create_initialized_view(form=form)
+        self.assertEqual(0, len(view.errors))
+        self.assertTrue(view.added_bug is not None)
+
     def test_submit_comment_empty_error(self):
         # The comment cannot be an empty string.
         form = self.get_form(comment="")
diff --git a/lib/lp/bugs/interfaces/bug.py b/lib/lp/bugs/interfaces/bug.py
index 0f31c6a..5135dee 100644
--- a/lib/lp/bugs/interfaces/bug.py
+++ b/lib/lp/bugs/interfaces/bug.py
@@ -224,6 +224,8 @@ class IBugView(Interface):
             title=_("Summary"),
             required=True,
             description=_("""A one-line summary of the problem."""),
+            min_length=1,
+            max_length=200,
         )
     )
     description = exported(
@@ -1437,7 +1439,9 @@ class IBugAddForm(IBug):
         ),
         vocabulary="BinaryAndSourcePackageName",
     )
-    title = Title(title=_("Summary"), required=True)
+    title = Title(
+        title=_("Summary"), min_length=1, max_length=200, required=True
+    )
     distribution = Choice(
         title=_("Linux Distribution"),
         required=True,
diff --git a/lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.rst b/lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.rst
index 2af51fd..d06f46c 100644
--- a/lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.rst
+++ b/lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.rst
@@ -71,7 +71,7 @@ will be displayed as well.
     ...     print(message.decode_contents())
     ...
     There is 1 error.
-    Required input is missing.
+    Provide a one-line summary of the problem.
 
 With both values set, the bug is created.