Launchpad itself

Merge lp:~sinzui/launchpad/question-title-0 into lp:launchpad

  1. question-title-0
  2. Merge into devel
Proposed by Curtis Hovey
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: 11500
Proposed branch: lp:~sinzui/launchpad/question-title-0
Merge into: lp:launchpad
Diff against target: 83 lines (+58/-1)
2 files modified
lib/lp/answers/browser/question.py (+5/-1)
lib/lp/answers/browser/tests/test_question.py (+53/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/question-title-0
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+34555@code.launchpad.net

Description of the change

This is my branch to prevent users from entering long question summaries.

    lp:~sinzui/launchpad/question-title-0
    Diff size: 84
    Launchpad bug:
          https://bugs.launchpad.net/bugs/583623
    Test command: ./bin/test -vv -t TestQuestionAddView
    Pre-implementation: no one
    Target release: 10.09

Prevent users from entering long question summaries
----------------------------------------------------

OOPS-1601O1172 was caused when the user pasted an error log into the title
field. The field should restrict the title to a reasonable

Discovering a reasonable length took some time, looking at question titles
in the db, and at bug titles (because we want bugs to be convertible to
questions). Changing the field definition has a secondary complication--we
would need to update all the existing questions that have titles that are
too long. Since we really want comments to work on way in Launchpad, a
better approach is to ensure the view does not let the user provide too
much information.

Rules
-----

The simplest fix for this issue is to use displayMaxWidth=250 in the
QuestionAddView to tell the browser to ignore characters after 250.
     custom_widget('title', TextWidget, displayWidth=40, displayMaxWidth=250)

But in the case where users are posting from a non-lp form (I am speculating)
we can add a constraint to the view to return a field error too. This is also
easier to test than browser compliance.

QA
--

    * On staging, as a question with more than 250 characters.
    * Verify the page says the question summary cannot exceed 250 characters.

Lint
----

Linting changed files:
  lib/lp/answers/browser/question.py
  lib/lp/answers/browser/tests/test_question.py

Test
----

I renamed the existing module to test_views since it it is loading doctests
for question and faq views.

    * lib/lp/answers/browser/tests/test_question.py
      * Added tests for the max length of the title field.

Implementation
--------------

    * lib/lp/answers/browser/question.py
      * Added a validation rule to report an error if the question title is
        exceeds 250 characters.
      * Added displayMaxWidth to the html element so that users can see that
        the excessive content is ignored.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Curtis,

This looks good.

-Edwin

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/answers/browser/question.py'
2--- lib/lp/answers/browser/question.py 2010-08-24 10:45:57 +0000
3+++ lib/lp/answers/browser/question.py 2010-09-03 17:08:51 +0000
4@@ -539,7 +539,7 @@
5 # The fields displayed on the search page.
6 search_field_names = ['language', 'title']
7
8- custom_widget('title', TextWidget, displayWidth=40)
9+ custom_widget('title', TextWidget, displayWidth=40, displayMaxWidth=250)
10
11 search_template = ViewPageTemplateFile(
12 '../templates/question-add-search.pt')
13@@ -601,6 +601,10 @@
14 if 'title' not in data:
15 self.setFieldError(
16 'title', _('You must enter a summary of your problem.'))
17+ else:
18+ if len(data['title']) > 250:
19+ self.setFieldError(
20+ 'title', _('The summary cannot exceed 250 characters.'))
21 if self.widgets.get('description'):
22 if 'description' not in data:
23 self.setFieldError(
24
25=== added file 'lib/lp/answers/browser/tests/test_question.py'
26--- lib/lp/answers/browser/tests/test_question.py 1970-01-01 00:00:00 +0000
27+++ lib/lp/answers/browser/tests/test_question.py 2010-09-03 17:08:51 +0000
28@@ -0,0 +1,53 @@
29+# Copyright 2010 Canonical Ltd. This software is licensed under the
30+# GNU Affero General Public License version 3 (see the file LICENSE).
31+
32+"""Tests for the question module."""
33+
34+__metaclass__ = type
35+
36+__all__ = []
37+
38+from canonical.testing import DatabaseFunctionalLayer
39+from lp.answers.publisher import AnswersLayer
40+from lp.testing import (
41+ login_person,
42+ TestCaseWithFactory,
43+ )
44+from lp.testing.views import create_initialized_view
45+
46+
47+class TestQuestionAddView(TestCaseWithFactory):
48+ """Verify the behavior of the QuestionAddView."""
49+
50+ layer = DatabaseFunctionalLayer
51+
52+ def setUp(self):
53+ super(TestQuestionAddView, self).setUp()
54+ self.question_target = self.factory.makeProduct()
55+ self.user = self.factory.makePerson()
56+ login_person(self.user)
57+
58+ def getSearchForm(self, title, language='en'):
59+ return {
60+ 'field.title': title,
61+ 'field.language': language,
62+ 'field.actions.continue': 'Continue',
63+ }
64+
65+ def test_question_title_within_max_display_width(self):
66+ # Titles (summary in the view) less than 250 characters are accepted.
67+ form = self.getSearchForm('123456789 ' * 10)
68+ view = create_initialized_view(
69+ self.question_target, name='+addquestion', layer=AnswersLayer,
70+ form=form, principal=self.user)
71+ self.assertEqual([], view.errors)
72+
73+ def test_question_title_exceeds_max_display_width(self):
74+ # Titles (summary in the view) cannot exceed 250 characters.
75+ form = self.getSearchForm('123456789 ' * 26)
76+ view = create_initialized_view(
77+ self.question_target, name='+addquestion', layer=AnswersLayer,
78+ form=form, principal=self.user)
79+ self.assertEqual(1, len(view.errors))
80+ self.assertEqual(
81+ 'The summary cannot exceed 250 characters.', view.errors[0])
82
83=== renamed file 'lib/lp/answers/browser/tests/test_question.py' => 'lib/lp/answers/browser/tests/test_views.py'