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
=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py 2010-08-24 10:45:57 +0000
+++ lib/lp/answers/browser/question.py 2010-09-03 17:08:51 +0000
@@ -539,7 +539,7 @@
539 # The fields displayed on the search page.539 # The fields displayed on the search page.
540 search_field_names = ['language', 'title']540 search_field_names = ['language', 'title']
541541
542 custom_widget('title', TextWidget, displayWidth=40)542 custom_widget('title', TextWidget, displayWidth=40, displayMaxWidth=250)
543543
544 search_template = ViewPageTemplateFile(544 search_template = ViewPageTemplateFile(
545 '../templates/question-add-search.pt')545 '../templates/question-add-search.pt')
@@ -601,6 +601,10 @@
601 if 'title' not in data:601 if 'title' not in data:
602 self.setFieldError(602 self.setFieldError(
603 'title', _('You must enter a summary of your problem.'))603 'title', _('You must enter a summary of your problem.'))
604 else:
605 if len(data['title']) > 250:
606 self.setFieldError(
607 'title', _('The summary cannot exceed 250 characters.'))
604 if self.widgets.get('description'):608 if self.widgets.get('description'):
605 if 'description' not in data:609 if 'description' not in data:
606 self.setFieldError(610 self.setFieldError(
607611
=== added file 'lib/lp/answers/browser/tests/test_question.py'
--- lib/lp/answers/browser/tests/test_question.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/browser/tests/test_question.py 2010-09-03 17:08:51 +0000
@@ -0,0 +1,53 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for the question module."""
5
6__metaclass__ = type
7
8__all__ = []
9
10from canonical.testing import DatabaseFunctionalLayer
11from lp.answers.publisher import AnswersLayer
12from lp.testing import (
13 login_person,
14 TestCaseWithFactory,
15 )
16from lp.testing.views import create_initialized_view
17
18
19class TestQuestionAddView(TestCaseWithFactory):
20 """Verify the behavior of the QuestionAddView."""
21
22 layer = DatabaseFunctionalLayer
23
24 def setUp(self):
25 super(TestQuestionAddView, self).setUp()
26 self.question_target = self.factory.makeProduct()
27 self.user = self.factory.makePerson()
28 login_person(self.user)
29
30 def getSearchForm(self, title, language='en'):
31 return {
32 'field.title': title,
33 'field.language': language,
34 'field.actions.continue': 'Continue',
35 }
36
37 def test_question_title_within_max_display_width(self):
38 # Titles (summary in the view) less than 250 characters are accepted.
39 form = self.getSearchForm('123456789 ' * 10)
40 view = create_initialized_view(
41 self.question_target, name='+addquestion', layer=AnswersLayer,
42 form=form, principal=self.user)
43 self.assertEqual([], view.errors)
44
45 def test_question_title_exceeds_max_display_width(self):
46 # Titles (summary in the view) cannot exceed 250 characters.
47 form = self.getSearchForm('123456789 ' * 26)
48 view = create_initialized_view(
49 self.question_target, name='+addquestion', layer=AnswersLayer,
50 form=form, principal=self.user)
51 self.assertEqual(1, len(view.errors))
52 self.assertEqual(
53 'The summary cannot exceed 250 characters.', view.errors[0])
054
=== renamed file 'lib/lp/answers/browser/tests/test_question.py' => 'lib/lp/answers/browser/tests/test_views.py'