Merge lp:~jcsackett/launchpad/mirrors-tell-where-they-live-652134 into lp:launchpad

[j.c.sackett]

Updates imported branches on products to display upstream information and present as EXTERNAL codehosting_usage.

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

[Gavin Panella (allenap)]

Looks good, only one comment:

+ # Test that the correct information is shown for an import

I think the style here should be more declarative (if that's the right
term), so something like:

  The correct information is shown for an import.

It's probably worth explaining what's being tested rather than only
saying "correct information", unless it's easier to read the test than
to describe. Something like:

  The page for an imports of an external branch includes an
  explanation of where it came from and what it can be now be used
  for.

review: Approve

[j.c.sackett (jcsackett)]

> Looks good, only one comment:
>
> + # Test that the correct information is shown for an import
>
> I think the style here should be more declarative (if that's the right
> term), so something like:
>
> The correct information is shown for an import.
>
> It's probably worth explaining what's being tested rather than only
> saying "correct information", unless it's easier to read the test than
> to describe. Something like:
>
> The page for an imports of an external branch includes an
> explanation of where it came from and what it can be now be used
> for.

I have replaced the test comment with a better one per your suggestion. Thanks!

=== modified file 'lib/lp/code/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py	2010-10-08 16:38:43 +0000
+++ lib/lp/code/browser/tests/test_product.py	2010-10-15 20:41:30 +0000
@@ -31,6 +31,7 @@
     BrowserTestCase,
     login,
     login_person,
+    logout,
     TestCaseWithFactory,
     time_counter,
     )
@@ -157,6 +158,27 @@
 class TestProductCodeIndexServiceUsages(ProductTestBase, BrowserTestCase):
     """Tests for the product code page, especially the usage messasges."""
 
+    def test_external_imported(self):
+        # A product with an imported development focus branch should say so,
+        # and should display the upstream information along with the LP info.
+        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 the master branch 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):
         # A mirrored branch says code is hosted externally, and displays
         # upstream data.

=== modified file 'lib/lp/code/stories/branches/xx-product-branches.txt'
--- lib/lp/code/stories/branches/xx-product-branches.txt	2010-09-28 19:25:54 +0000
+++ lib/lp/code/stories/branches/xx-product-branches.txt	2010-10-15 20:41:30 +0000
@@ -93,9 +93,16 @@
     >>> browser.open('http://code.launchpad.dev/evolution')
     >>> summary = get_summary(browser)
     >>> print extract_text(get_summary(browser))
-    You can get a copy of the development focus branch using the
-    command:
-       bzr branch lp://dev/evolution
+    The Evolution Groupware Application hosts its code
+    externally.
+    You can learn more at the project's
+    web page.
+    Launchpad imports the master branch and you can create branches from
+    it.
+    You can
+    get a copy of the development focus branch
+    using the command:
+    bzr branch lp://dev/evolution
 
     >>> links = summary.findAll('a')
 

=== 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-15 20:41:30 +0000
@@ -19,14 +19,26 @@
   </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:has-import-data condition="branch/code_import">
+        <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>.
+        </tal:has-import-data>
+        <tal:no-import-data condition="not: branch/code_import">
+        <tal:project_title replace="context/title" /> hosts its code
+          externally.
+        </tal:no-import-data>
+      </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 +47,11 @@
       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 the master branch 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-15 20:41:30 +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