Merge lp:~sinzui/launchpad/prefill-homepageurl-0 into lp:launchpad

[Curtis Hovey]

This is my branch to prefill the homepageurl when registering projects
for packages.

    lp:~sinzui/launchpad/prefill-homepageurl-0
    Diff size: 101
    Launchpad bug: https://bugs.launchpad.net/bugs/621778
    Test command: ./bin/test -vv -t TestSourcePackageViewHelpers
    Pre-implementation: jelmer
    Target release: 10.12

Prefill the homepageurl when registering projects for packages
--------------------------------------------------------------

This is the final branch needed to support prefilling the project's
homepageurl with the homepage url from the source package release.
All the data, forms, and helpers are in place. The helpers need to
pass the homepageurl to the form.

Rules
-----

    * Update get_register_upstream_url() in browser/sourcepackage.py to
      include the homepageurl field.

QA
--

    * Visit https://qastaging.launchpad.net/ubuntu/+source/maven-plugin-tools
    * Choose to register a new project
    * Verify that the homepageurl is
      http://maven.apache.org/plugins/maven-javadoc-plugin/

Lint
----

Linting changed files:
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/registry/browser/tests/test_sourcepackage_views.py

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

Updated get_register_upstream_url to add the homepageurl using the
SP's currentrelease. The value cannot be None because that is serialised
as 'None' which causes a form error since it is not a valid url.
Refactored the setup for a test so that an SP with publishing history is
available to test the homepage.
    lib/lp/registry/browser/sourcepackage.py
    lib/lp/registry/browser/tests/test_sourcepackage_views.py

[Māris Fogels (mars)]

Hi Curtis,

This looks like an excellent change, r=mars.

You may have an opportunity here to refactor the tests for readability. I found the assertEquals() statements hard to read - a custom method like assertInQueryURL() may be easier to read, and also reduce the dependence on the entire query data structure:

  self.assertInQueryURL('field.homepageurl', 'http://eg.dom/bonkers')

Maris

review: Approve

=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py	2010-11-23 23:22:27 +0000
+++ lib/lp/registry/browser/sourcepackage.py	2010-11-24 13:28:47 +0000
@@ -115,6 +115,11 @@
     distroseries_string = "%s/%s" % (
         source_package.distroseries.distribution.name,
         source_package.distroseries.name)
+    current_release = source_package.currentrelease
+    if current_release is not None and current_release.homepage is not None:
+        homepage = current_release.homepage
+    else:
+        homepage = ''
     params = {
         '_return_url': canonical_url(source_package),
         'field.source_package_name': source_package.sourcepackagename.name,
@@ -122,6 +127,7 @@
         'field.name': source_package.name,
         'field.displayname': displayname,
         'field.title': displayname,
+        'field.homepageurl': homepage,
         'field.__visited_steps__': ProjectAddStepOne.step_name,
         'field.actions.continue': 'Continue',
         }

=== modified file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py'
--- lib/lp/registry/browser/tests/test_sourcepackage_views.py	2010-11-09 23:29:12 +0000
+++ lib/lp/registry/browser/tests/test_sourcepackage_views.py	2010-11-24 13:28:47 +0000
@@ -10,6 +10,7 @@
 
 from zope.component import getUtility
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -36,38 +37,7 @@
 
     layer = DatabaseFunctionalLayer
 
-    def test_get_register_upstream_url_displayname(self):
-        distroseries = self.factory.makeDistroRelease(
-            distribution=self.factory.makeDistribution(name='zoobuntu'),
-            name='walrus')
-        source_package = self.factory.makeSourcePackage(
-            distroseries=distroseries,
-            sourcepackagename='python-super-package')
-        url = get_register_upstream_url(source_package)
-        expected_base = '/projects/+new'
-        expected_params = [
-            ('_return_url',
-             'http://launchpad.dev/zoobuntu/walrus/'
-             '+source/python-super-package'),
-            ('field.__visited_steps__', 'projectaddstep1'),
-            ('field.actions.continue', 'Continue'),
-            # The sourcepackagename 'python-super-package' is split on
-            # the hyphens, and each word is capitalized.
-            ('field.displayname', 'Python Super Package'),
-            ('field.distroseries', 'zoobuntu/walrus'),
-            ('field.name', 'python-super-package'),
-            # The summary is missing, since the source package doesn't
-            # have a binary package release, and parse_qsl() excludes
-            # empty params.
-            ('field.source_package_name', 'python-super-package'),
-            ('field.title', 'Python Super Package'),
-            ]
-        base, query = urllib.splitquery(url)
-        params = cgi.parse_qsl(query)
-        self.assertEqual((expected_base, expected_params),
-                         (base, params))
-
-    def test_get_register_upstream_url_summary(self):
+    def _makePublishedSourcePackage(self):
         test_publisher = SoyuzTestPublisher()
         test_data = test_publisher.makeSourcePackageSummaryData()
         source_package_name = (
@@ -79,26 +49,57 @@
         # updateDistroSeriesPackageCache reconnects the db, so the
         # objects need to be reloaded.
         distroseries = getUtility(IDistroSeriesSet).get(distroseries_id)
-        source_package = distroseries.getSourcePackage(source_package_name)
+        return distroseries.getSourcePackage(source_package_name)
+
+    def assertInQueryString(self, url, field, value):
+        base, query = urllib.splitquery(url)
+        params = cgi.parse_qsl(query)
+        self.assertTrue((field, value) in params)
+
+    def test_get_register_upstream_url_fields(self):
+        distroseries = self.factory.makeDistroRelease(
+            distribution=self.factory.makeDistribution(name='zoobuntu'),
+            name='walrus')
+        source_package = self.factory.makeSourcePackage(
+            distroseries=distroseries,
+            sourcepackagename='python-super-package')
         url = get_register_upstream_url(source_package)
-        expected_base = '/projects/+new'
+        base, query = urllib.splitquery(url)
+        self.assertEqual('/projects/+new', base)
+        params = cgi.parse_qsl(query)
         expected_params = [
             ('_return_url',
-             'http://launchpad.dev/youbuntu/busy/+source/bonkers'),
+             'http://launchpad.dev/zoobuntu/walrus/'
+             '+source/python-super-package'),
             ('field.__visited_steps__', 'projectaddstep1'),
             ('field.actions.continue', 'Continue'),
-            ('field.displayname', 'Bonkers'),
-            ('field.distroseries', 'youbuntu/busy'),
-            ('field.name', 'bonkers'),
-            ('field.source_package_name', 'bonkers'),
-            ('field.summary', 'summary for flubber-bin\n'
-                              + 'summary for flubber-lib'),
-            ('field.title', 'Bonkers'),
+            ('field.displayname', 'Python Super Package'),
+            ('field.distroseries', 'zoobuntu/walrus'),
+            ('field.name', 'python-super-package'),
+            ('field.source_package_name', 'python-super-package'),
+            ('field.title', 'Python Super Package'),
             ]
-        base, query = urllib.splitquery(url)
-        params = cgi.parse_qsl(query)
-        self.assertEqual((expected_base, expected_params),
-                         (base, params))
+        self.assertEqual(expected_params, params)
+
+    def test_get_register_upstream_url_displayname(self):
+        # The sourcepackagename 'python-super-package' is split on
+        # the hyphens, and each word is capitalized.
+        distroseries = self.factory.makeDistroRelease(
+            distribution=self.factory.makeDistribution(name='zoobuntu'),
+            name='walrus')
+        source_package = self.factory.makeSourcePackage(
+            distroseries=distroseries,
+            sourcepackagename='python-super-package')
+        url = get_register_upstream_url(source_package)
+        self.assertInQueryString(
+            url, 'field.displayname', 'Python Super Package')
+
+    def test_get_register_upstream_url_summary(self):
+        source_package = self._makePublishedSourcePackage()
+        url = get_register_upstream_url(source_package)
+        self.assertInQueryString(
+            url, 'field.summary',
+            'summary for flubber-bin\nsummary for flubber-lib')
 
     def test_get_register_upstream_url_summary_duplicates(self):
 
@@ -130,28 +131,22 @@
             distroseries=FakeDistroSeries(
                 name='walrus',
                 distribution=FakeDistribution(name='zoobuntu')),
-            releases=[releases])
+            releases=[releases],
+            currentrelease=Faker(homepage=None),
+            )
+        url = get_register_upstream_url(source_package)
+        self.assertInQueryString(
+            url, 'field.summary',
+            'summary for bar\nsummary for baz\nsummary for foo')
 
+    def test_get_register_upstream_url_homepage(self):
+        source_package = self._makePublishedSourcePackage()
+        # SourcePackageReleases cannot be modified by users.
+        removeSecurityProxy(
+            source_package.currentrelease).homepage = 'http://eg.dom/bonkers'
         url = get_register_upstream_url(source_package)
-        expected_base = '/projects/+new'
-        expected_params = [
-            ('_return_url',
-             'http://launchpad.dev/zoobuntu/walrus/+source/foo'),
-            ('field.__visited_steps__', 'projectaddstep1'),
-            ('field.actions.continue', 'Continue'),
-            ('field.displayname', 'Foo'),
-            ('field.distroseries', 'zoobuntu/walrus'),
-            ('field.name', 'foo'),
-            ('field.source_package_name', 'foo'),
-            ('field.summary', 'summary for bar\n'
-                              + 'summary for baz\n'
-                              + 'summary for foo'),
-            ('field.title', 'Foo'),
-            ]
-        base, query = urllib.splitquery(url)
-        params = cgi.parse_qsl(query)
-        self.assertEqual((expected_base, expected_params),
-                         (base, params))
+        self.assertInQueryString(
+            url, 'field.homepageurl', 'http://eg.dom/bonkers')
 
 
 class TestSourcePackageUpstreamConnectionsView(TestCaseWithFactory):

=== modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt'
--- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2010-11-11 21:48:20 +0000
+++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2010-11-24 13:28:47 +0000
@@ -67,11 +67,12 @@
     The Hoary Hedgehog Release (active development) ...
       0.1-2  release (main) ... weeks ago
 
+
 Register a project from a source package
 ----------------------------------------
 
-If an upstream project doesn't already exist in Launchpad, it can
-be registered with data from the source package prefilling the first
+No Privileges Person can register a project for a package, and Launchpad
+will use the data from the source package to prefill the first
 step of the multistep form.
 
     >>> user_browser.open(
@@ -79,16 +80,6 @@
     >>> user_browser.getControl(
     ...     'Register the upstream project').selected = True
     >>> user_browser.getControl("Link to Upstream Project").click()
-    >>> print user_browser.url.replace('&', '\n&')
-    http://launchpad.dev/projects/+new?_return_url=http...%2Bsource%2Fbonkers
-    &field.__visited_steps__=projectaddstep1
-    &field.actions.continue=Continue
-    &field.displayname=Bonkers
-    &field.distroseries=youbuntu%2Fbusy
-    &field.name=bonkers
-    &field.source_package_name=bonkers
-    &field.summary=summary+for+flubber-bin%0Asummary+for+flubber-lib
-    &field.title=Bonkers
     >>> print user_browser.getControl(name='field.name').value
     bonkers
     >>> print user_browser.getControl(name='field.displayname').value
@@ -102,9 +93,9 @@
     ...     find_tag_by_id(user_browser.contents, 'step-title'))
     Step 2 (of 2): Check for duplicate projects
 
-If the user selects "Choose another upstream project" and then finds out
-that the project doesn't exist, there is a also a link on the
-+edit-packaging page to register the project.
+When No Privileges Person selects "Choose another upstream project" and
+then finds out that the project doesn't exist, he uses the
+"Link to Upstream Project" button to register the project.
 
     >>> user_browser.open(
     ...     'http://launchpad.dev/youbuntu/busy/+source/bonkers/')
@@ -115,16 +106,6 @@
     http://launchpad.dev/youbuntu/busy/+source/bonkers/+edit-packaging
 
     >>> user_browser.getLink("Register the upstream project").click()
-    >>> print user_browser.url.replace('&', '\n&')
-    http://launchpad.dev/projects/+new?_return_url=http...%2Bsource%2Fbonkers
-    &field.__visited_steps__=projectaddstep1
-    &field.actions.continue=Continue
-    &field.displayname=Bonkers
-    &field.distroseries=youbuntu%2Fbusy
-    &field.name=bonkers
-    &field.source_package_name=bonkers
-    &field.summary=summary+for+flubber-bin%0Asummary+for+flubber-lib
-    &field.title=Bonkers
     >>> print user_browser.getControl(name='field.name').value
     bonkers
     >>> print user_browser.getControl(name='field.displayname').value
@@ -138,8 +119,7 @@
     ...     find_tag_by_id(user_browser.contents, 'step-title'))
     Step 2 (of 2): Check for duplicate projects
 
-If there are no problems with the prefilled data, then the license
-just needs to be selected. The user will then be redirected back
+After No Privileges Person select the licenses, he user is redirected back
 to the source package page and an informational message will be displayed.
 
     >>> user_browser.getControl(name='field.licenses').value = ['BSD']