launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/mirrors-tell-where-they-live-652134 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #652134 Imported projects do not tell me where upstream is
  https://bugs.launchpad.net/bugs/652134


Summary
=======

Updates presentation of products with an imported development focus branch to
reflect a codehosting_usage of EXTERNAL, not NOT_APPLICABLE, and to show 
the appropriate upstream data.

Proposed fix
============

Update the product code index page to show the upstream information if the
development focus branch is imported.

Pre-implementation notes
========================

Spoke with Curtis Hovey.

Implementation details
======================

Largely as in Proposed; however, the branch also corrected the
codehosting_usage enum to display a branch_type of IMPORTED as a
ServiceUsage of EXTERNAL. Previously it was NOT_APPLICABLE.

Tests
=====

bin/test -t code.*external_imported

Demo and Q/A
============

In launchpad.dev, add the gnome-terminal/imported branch as the focus branch
for gnome-terminal; gnome-terminal should show the upstream links for the
branch.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/tests/test_product.py
  lib/lp/code/templates/product-branch-summary.pt
  lib/lp/registry/model/product.py

-- 
https://code.launchpad.net/~jcsackett/launchpad/mirrors-tell-where-they-live-652134/+merge/37925
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/mirrors-tell-where-they-live-652134 into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/browser/tests/test_product.py	2010-10-08 00:28:45 +0000
@@ -31,6 +31,7 @@
     BrowserTestCase,
     login,
     login_person,
+    logout,
     TestCaseWithFactory,
     time_counter,
     )
@@ -157,6 +158,26 @@
 class TestProductCodeIndexServiceUsages(ProductTestBase, BrowserTestCase):
     """Tests for the product code page, especially the usage messasges."""
 
+    def test_external_import(self):
+        # Test that the correct information is shown for an import
+        product = self.factory.makeProduct()
+        code_import = self.factory.makeProductCodeImport(
+            svn_branch_url='http://svn.example.org/branch')
+        login_person(product.owner)
+        product.development_focus.branch = code_import.branch
+        logout()
+        self.assertEqual(ServiceUsage.EXTERNAL, product.codehosting_usage)
+        browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
+        login(ANONYMOUS)
+        content = find_tag_by_id(browser.contents, 'external')
+        text = extract_text(content)
+        expected = ("%(product_title)s hosts its code at %(branch_url)s. "
+            "Launchpad imports code from there and you can create "
+            "branches from it." % dict(
+                product_title=product.title,
+                branch_url=code_import.url))
+        self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
+
     def test_external_mirrored(self):
         # Test that the correct URL is displayed for a mirrored branch.
         product, branch = self.makeProductAndDevelopmentFocusBranch(

=== modified file 'lib/lp/code/templates/product-branch-summary.pt'
--- lib/lp/code/templates/product-branch-summary.pt	2010-09-28 19:25:54 +0000
+++ lib/lp/code/templates/product-branch-summary.pt	2010-10-08 00:28:45 +0000
@@ -19,14 +19,20 @@
   </div>
 
   <div id="external"
-       tal:condition="context/codehosting_usage/enumvalue:EXTERNAL">
-    <p>
-      <strong>
+       tal:condition="context/codehosting_usage/enumvalue:EXTERNAL"
+       tal:define="branch view/branch">
+    <strong>
+      <p tal:condition="not: branch/branch_type/enumvalue:IMPORTED">
         <tal:project_title replace="context/title" /> hosts its code at
         <a tal:attributes="href view/mirror_location"
            tal:content="view/mirror_location"></a>.
-      </strong>
-    </p>
+      </p>
+      <p tal:condition="branch/branch_type/enumvalue:IMPORTED">
+        <tal:project_title replace="context/title" /> hosts its code at
+        <a tal:attributes="href branch/code_import/url"
+           tal:content="branch/code_import/url"></a>.
+      </p>
+    </strong>
     <p tal:condition="context/homepageurl">
       You can learn more at the project's
       <a tal:attributes="href context/homepageurl">web page</a>.
@@ -35,6 +41,10 @@
       Launchpad has a mirror of the master branch and you can create branches
       from it.
     </p>
+    <p tal:condition="view/branch/branch_type/enumvalue:IMPORTED">
+      Launchpad imports code from there and you can create
+      branches from it.
+    </p>
     <p tal:condition="view/branch/branch_type/enumvalue:REMOTE">
       Launchpad does not have a copy of the remote branch.
     </p>

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-10-03 15:30:06 +0000
+++ lib/lp/registry/model/product.py	2010-10-08 00:28:45 +0000
@@ -405,7 +405,9 @@
         elif self.development_focus.branch.branch_type == BranchType.HOSTED:
             return ServiceUsage.LAUNCHPAD
         elif self.development_focus.branch.branch_type in (
-            BranchType.MIRRORED, BranchType.REMOTE):
+            BranchType.MIRRORED,
+            BranchType.REMOTE,
+            BranchType.IMPORTED):
             return ServiceUsage.EXTERNAL
         return ServiceUsage.NOT_APPLICABLE