launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/fix-search-oops into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #827213 in Launchpad itself: "AttributeError: 'ConversionError' object has no attribute 'field_name' on +search page"
  https://bugs.launchpad.net/launchpad/+bug/827213

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/fix-search-oops/+merge/77429

Handle ConversionError in +search's validate().

Allow me to apologise for the test in advance -- it is quite gross, but since I'm testing invalid Unicode, I need to disable warnings.
-- 
https://code.launchpad.net/~stevenk/launchpad/fix-search-oops/+merge/77429
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/fix-search-oops into lp:launchpad.

=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py	2011-09-18 17:48:29 +0000
+++ lib/lp/app/browser/root.py	2011-09-29 02:14:31 +0000
@@ -14,6 +14,7 @@
 
 import feedparser
 from lazr.batchnavigator.z3batching import batch
+from zope.app.form.interfaces import ConversionError
 from zope.component import getUtility
 from zope.interface import Interface
 from zope.schema import TextLine
@@ -405,7 +406,12 @@
         """See `LaunchpadFormView`"""
         errors = list(self.errors)
         for error in errors:
-            if (error.field_name == 'text'
+            if isinstance(error, ConversionError):
+                self.setFieldError(
+                    'text', 'Can not convert your search term.')
+            elif isinstance(error, unicode):
+                continue
+            elif (error.field_name == 'text'
                 and isinstance(error.errors, TooLong)):
                 self.setFieldError(
                     'text', 'The search text cannot exceed 250 characters.')

=== modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt'
--- lib/lp/app/browser/tests/launchpad-search-pages.txt	2011-07-13 06:08:16 +0000
+++ lib/lp/app/browser/tests/launchpad-search-pages.txt	2011-09-29 02:14:31 +0000
@@ -39,9 +39,8 @@
     ...         search_param_list.append('%s=%s' % (name, value))
     ...     query_string = '&'.join(search_param_list)
     ...     request = LaunchpadTestRequest(
-    ...         SERVER_URL='http://launchpad.dev/+search',
-    ...         QUERY_STRING=query_string,
-    ...         form=form)
+    ...         SERVER_URL='https://launchpad.dev/+search',
+    ...         QUERY_STRING=query_string, form=form, PATH_INFO='/+search')
     ...     search_view = getMultiAdapter((root, request), name="+search")
     ...     search_view.initialize()
     ...     return search_view
@@ -504,12 +503,27 @@
 When an empty PageMatches object is returned by the GoogleSearchService to
 the view, there are no matches to show.
 
-    >>> search_view = getSearchView(
-    ...     form={'field.text': 'no-meaningful'})
+    >>> search_view = getSearchView(form={'field.text': 'no-meaningful'})
     >>> search_view.has_matches
     False
 
 
+Unintelligible searches
+-----------------------
+
+When a user searches for a malformed string, we don't OOPS, but show an
+error. Also disable warnings, since we are tossing around malformed Unicode.
+
+    >>> import warnings
+    >>> with warnings.catch_warnings():
+    ...     warnings.simplefilter('ignore')
+    ...     search_view = getSearchView(
+    ...         form={'field.text': '\xfe\xfckr\xfc'})
+    >>> html = search_view()
+    >>> 'Can not convert your search term' in html
+    True
+
+
 Bad Google response handling
 ----------------------------
 
@@ -518,8 +532,7 @@
 show a message explaining that the user can search again to find
 matching pages.
 
-    >>> search_view = getSearchView(
-    ...     form={'field.text': 'gnomebaker'})
+    >>> search_view = getSearchView(form={'field.text': 'gnomebaker'})
     >>> search_view.has_matches
     True
     >>> search_view.pillar.displayname
@@ -531,7 +544,7 @@
 link to try the search again
 
     >>> print search_view.url
-    http://launchpad.dev/+search?field.text=gnomebaker
+    https://launchpad.dev/+search?field.text=gnomebaker
 
 
 SearchFormView and SearchFormPrimaryView