launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/projectgroup-branches-652156 into lp:launchpad/devel.

Requested reviews:
  Guilherme Salgado (salgado): ui
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #652156 project group does not show the unknown code hosting message
  https://bugs.launchpad.net/bugs/652156


Summary
=======

Updates the projectgroup branch listing page to display a message similar to other codehosting usage messages for other pillars (e.g. "<Product> does not use Launchpad for codehosting.")

Right now if no products are configured to have branches on Launchpad (the condition to set codehosting_usage to LAUNCHPAD), the project group simply shows an empty table with a message about using bazaar; in line with the bridging the gap tasks, it should simply state the the project group isn't using launchpad for codehosting, and be done with it.

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

Detect when there are no branches and display the appropriate message instead of using the branch-listing portlet.

Preimplementation talk
======================

Spoke with Edwin Grubbs. We brought up the possiblity of linking out to all the products for the group so someone could configure the products, but to do so sanely (i.e. only show the links for products the viewer can configure) requires doing database checks for every product; this could lead to death by SQL timeouts.

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

Largely as in proposed. A new property, has_branches, was added to projectgroups to check if there are any branches for the projectgroup's products. If this returns false, the message is shown.

Tests
=====

bin/test -vvct TestProjectBranches
bin/test -vvct TestProjectBranchListing

Screenshots
===========

The old appearance is available at: 
http://people.canonical.com/~jc/images/withoutbranches-old.png 

The new appearance is available at: 
http://people.canonical.com/~jc/images/withoutbranches-new.png 

The appearance with any branches (in either case) is available at: 
http://people.canonical.com/~jc/images/withbranches.png

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/tests/test_branchlisting.py
  lib/lp/code/templates/project-branches.pt
  lib/lp/code/tests/test_project.py
  lib/lp/registry/interfaces/projectgroup.py
  lib/lp/registry/model/projectgroup.py

-- 
https://code.launchpad.net/~jcsackett/launchpad/projectgroup-branches-652156/+merge/38355
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/projectgroup-branches-652156 into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2010-08-24 02:21:50 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2010-10-13 18:43:40 +0000
@@ -421,5 +421,33 @@
         self.assertIs(None, branches)
 
 
+class TestProjectBranchListing(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestProjectBranchListing, self).setUp()
+        self.project = self.factory.makeProject()
+        self.product = self.factory.makeProduct(project=self.project)
+    
+    def test_no_branches_gets_message_not_listing(self):
+        # If there are no product branches on the project's products, then
+        # the view shows the no code hosting message instead of a listing.
+        browser = self.getUserBrowser(
+            canonical_url(self.project, rootsite='code'))
+        expected_text = ("None of %s's projects are using Launchpad to host "
+                         "code." % self.project.displayname)
+        no_branch_div = find_tag_by_id(browser.contents, "no-branchtable")
+        text = extract_text(no_branch_div)
+
+    def test_branches_get_listing(self):
+        # If a product has a branch, then the project view has a branch
+        # listing.
+        branch = self.factory.makeProductBranch(product=self.product)
+        browser = self.getUserBrowser(
+            canonical_url(self.project, rootsite='code'))
+        table = find_tag_by_id(browser.contents, "branchtable")
+        self.assertIsNot(None, table)
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/code/templates/project-branches.pt'
--- lib/lp/code/templates/project-branches.pt	2009-09-17 00:27:40 +0000
+++ lib/lp/code/templates/project-branches.pt	2010-10-13 18:43:40 +0000
@@ -17,8 +17,17 @@
              tal:condition="link/enabled"
              tal:content="structure link/render" />
       </div>
-
-      <tal:branchlisting content="structure branches/@@+branch-listing" />
+      <tal:no-branches
+        condition="not:context/has_branches">
+        <div id="no-branchtable">
+          <strong>None of <tal:project replace="context/displayname"/>'s
+          projects are using Launchpad to host their code.</strong>
+        </div>
+      </tal:no-branches>
+      <tal:has-branches
+        condition="context/has_branches">
+        <tal:branchlisting content="structure branches/@@+branch-listing" />
+      </tal:has-branches>
     </div>
 
   </body>

=== added file 'lib/lp/code/tests/test_project.py'
--- lib/lp/code/tests/test_project.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/tests/test_project.py	2010-10-13 18:43:40 +0000
@@ -0,0 +1,36 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for product views."""
+
+__metaclass__ = type
+
+import unittest 
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+
+
+class TestProjectBranches(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestProjectBranches, self).setUp()
+        self.project = self.factory.makeProject()
+        self.product = self.factory.makeProduct(project=self.project)
+    
+    def test_has_branches_with_no_branches(self):
+        # If there are no product branches on the project's products, then
+        # has branches returns False.
+        self.assertFalse(self.project.has_branches())
+
+    def test_has_branches_with_branches(self):
+        # If a product has a branch, then the product's project returns
+        # true for has_branches.
+        self.factory.makeProductBranch(product=self.product)
+        self.assertTrue(self.project.has_branches())
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
--- lib/lp/registry/interfaces/projectgroup.py	2010-10-07 13:26:53 +0000
+++ lib/lp/registry/interfaces/projectgroup.py	2010-10-13 18:43:40 +0000
@@ -325,6 +325,10 @@
         """Return a boolean showing the existance of translatables products.
         """
 
+    def has_branches(self):
+        """Return a boolean showing the existance of products with branches.
+        """
+        
     def hasProducts():
         """Returns True if a project has products associated with it, False
         otherwise.

=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py	2010-10-07 13:26:53 +0000
+++ lib/lp/registry/model/projectgroup.py	2010-10-13 18:43:40 +0000
@@ -222,6 +222,10 @@
         """See `IProjectGroup`."""
         return self.translatables().count() != 0
 
+    def has_branches(self):
+        """ See `IProjectGroup`."""
+        return self.getBranches().count() != 0
+
     def _getBaseQueryAndClauseTablesForQueryingSprints(self):
         query = """
             Product.project = %s