launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/urifield-with-leading-space into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #956339 in Launchpad itself: "  IntegrityError: new row for relation "codeimport" violates check constraint "valid_vcs_details" "
  https://bugs.launchpad.net/launchpad/+bug/956339

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/urifield-with-leading-space/+merge/128855

URIWidget is now a subclass of StrippedTextWidget, which means it will strip leading and trailing whitespace. This fixes the root cause for the OOPS, since the database validation function valid_absolute_url() insists unstripped text is invalid, but URIField's validation method strips URIs before validation.

I've cleaned up some whitespace and pylint garbage to force this branch to net-negative, and have cleaned up a very weird copyright header that was completely lowercased.
-- 
https://code.launchpad.net/~stevenk/launchpad/urifield-with-leading-space/+merge/128855
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/urifield-with-leading-space into lp:launchpad.

=== modified file 'lib/lp/app/widgets/textwidgets.py'
--- lib/lp/app/widgets/textwidgets.py	2012-06-29 08:40:05 +0000
+++ lib/lp/app/widgets/textwidgets.py	2012-10-10 03:25:24 +0000
@@ -144,7 +144,7 @@
         return value.astimezone(tz).strftime('%Y-%m-%d %H:%M:%S')
 
 
-class URIWidget(TextWidget):
+class URIWidget(StrippedTextWidget):
     """A widget that represents a URI."""
 
     displayWidth = 44
@@ -157,7 +157,7 @@
     def _toFieldValue(self, input):
         if isinstance(input, list):
             raise UnexpectedFormData('Only a single value is expected')
-        return TextWidget._toFieldValue(self, input)
+        return super(URIWidget, self)._toFieldValue(input)
 
 
 class URIComponentWidget(LowerCaseTextWidget):

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2012-10-08 04:59:10 +0000
+++ lib/lp/registry/browser/productseries.py	2012-10-10 03:25:24 +0000
@@ -1104,8 +1104,7 @@
         return branch
 
 
-class ProductSeriesLinkBranchView(ReturnToReferrerMixin,
-                                  ProductSeriesView,
+class ProductSeriesLinkBranchView(ReturnToReferrerMixin, ProductSeriesView,
                                   LaunchpadEditFormView):
     """View to set the bazaar branch for a product series."""
 
@@ -1118,10 +1117,7 @@
         return 'Link an existing branch to %s %s series' % (
             self.context.product.displayname, self.context.name)
 
-    @property
-    def page_title(self):
-        """The page title."""
-        return self.label
+    page_title = label
 
     @action(_('Update'), name='update')
     def update_action(self, action, data):

=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py	2012-08-14 13:43:24 +0000
+++ lib/lp/services/fields/__init__.py	2012-10-10 03:25:24 +0000
@@ -1,7 +1,5 @@
-# copyright 2009-2010 canonical ltd.  this software is licensed under the
-# gnu affero general public license version 3 (see the file LICENSE).
-
-# pylint: disable-msg=E0211,E0213,W0401
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 __all__ = [

=== modified file 'lib/lp/services/fields/doc/uri-field.txt'
--- lib/lp/services/fields/doc/uri-field.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/services/fields/doc/uri-field.txt	2012-10-10 03:25:24 +0000
@@ -202,8 +202,12 @@
 
 Multiple values will cause an UnexpectedFormData exception:
 
-  >>> widget._toFieldValue(['http://launchpad.net',
-  ...                       'http://ubuntu.com'])
+  >>> widget._toFieldValue(['http://launchpad.net', 'http://ubuntu.com'])
   Traceback (most recent call last):
     ...
   UnexpectedFormData: Only a single value is expected
+
+Values with leading and trailing whitespace are stripped.
+
+   >>> widget._toFieldValue('  http://www.ubuntu.com/   ')
+   u'http://www.ubuntu.com/'