launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/code-configuration-652142 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Summary
=======

The bulleted list of steps to take for a product with no branches implies that
a user can configure branches/codehosting for the product regardless of
whether or not they can.

This branch simply removes the messages about how to set up branches for the
product if the user doesn't have the permissions to configure code for the
product.

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

Adds a can_configure_branches checker to the branch views, and uses that to
filter the messages displayed to users, so users who can't configure things
aren't led to believe they can.

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

As above.

Tests
=====

bin/test -m lp.code.browser.tests.test_product

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

If you go to code.launchpad.dev/mega-money-maker logged in as
admin@xxxxxxxxxxxxx, you will see the bulleted list and a configuration
link.

If you logout and go to the same page, you will only see the message saying
that LP doesn't know where code is hosted, and that the product has no
branches.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

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

./lib/lp/code/templates/product-branch-summary.pt
      44: Line has trailing whitespace.
    
The trailing whitespace is a space needed in the displayed text on the webpage.

-- 
https://code.launchpad.net/~jcsackett/launchpad/code-configuration-652142/+merge/38015
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/code-configuration-652142 into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2010-09-28 22:21:51 +0000
+++ lib/lp/code/browser/branchlisting.py	2010-10-08 20:25:59 +0000
@@ -406,7 +406,8 @@
     @cachedproperty
     def official_package_links_map(self):
         """Return a map from branch id to a list of package links."""
-        links = self._query_optimiser.getOfficialSourcePackageLinksForBranches(
+        query_optimiser = self._query_optimiser
+        links = query_optimiser.getOfficialSourcePackageLinksForBranches(
             self._visible_branch_ids)
         result = {}
         for link in links:
@@ -1180,6 +1181,10 @@
                 'in this project.')
         return message % self.context.displayname
 
+    def can_configure_branches(self):
+        """Whether or not the user can configure branches."""
+        return check_permission("launchpad.Edit", self.context)
+
 
 class ProductBranchStatisticsView(BranchCountSummaryView,
                                   ProductBranchListingView):

=== 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 20:25:59 +0000
@@ -26,6 +26,7 @@
     BranchVisibilityRule,
     )
 from lp.code.interfaces.revision import IRevisionSet
+from lp.code.publisher import CodeLayer
 from lp.testing import (
     ANONYMOUS,
     BrowserTestCase,
@@ -34,7 +35,10 @@
     TestCaseWithFactory,
     time_counter,
     )
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+    create_view,
+    create_initialized_view,
+    )
 
 
 class ProductTestBase(TestCaseWithFactory):
@@ -294,5 +298,21 @@
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
 
+class TestCanConfigureBranches(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_cannot_configure_branches_product_no_edit_permission(self):
+        product = self.factory.makeProduct()
+        view = create_view(product, '+branches', layer=CodeLayer)
+        self.assertEqual(False, view.can_configure_branches())
+
+    def test_can_configure_branches_product_with_edit_permission(self):
+        product = self.factory.makeProduct()
+        login_person(product.owner)
+        view = create_view(product, '+branches', layer=CodeLayer)
+        self.assertEqual(True, view.can_configure_branches())
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== 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 20:25:59 +0000
@@ -42,21 +42,24 @@
 
   <tal:no-branches condition="not: view/branch_count">
     There are no branches for <tal:project-name replace="context/displayname"/>
-    in Launchpad.  You can change this by:
-
-    <ul class="bulleted" style="margin-top:1em;">
-
-      <li>activating code hosting directly on
-      Launchpad. (<a href="https://help.launchpad.net/Code/UploadingABranch";>read
-      more</a>)</li>
-
-      <li>asking Launchpad to mirror a Bazaar branch hosted
-      elsewhere. (<a href="https://help.launchpad.net/Code/MirroredBranches";>read
-      more</a>)</li>
-
-      <li>asking Launchpad to import code from Git, Subversion, or CVS into a
-      Bazaar branch. (<a href="https://help.launchpad.net/VcsImports";>read more</a>)</li>
-    </ul>
+    in Launchpad. 
+    <tal:can-configure condition="view/can_configure_branches">
+      You can change this by:
+
+      <ul class="bulleted" style="margin-top:1em;">
+
+        <li>activating code hosting directly on
+        Launchpad. (<a href="https://help.launchpad.net/Code/UploadingABranch";>read
+        more</a>)</li>
+
+        <li>asking Launchpad to mirror a Bazaar branch hosted
+        elsewhere. (<a href="https://help.launchpad.net/Code/MirroredBranches";>read
+        more</a>)</li>
+
+        <li>asking Launchpad to import code from Git, Subversion, or CVS into a
+        Bazaar branch. (<a href="https://help.launchpad.net/VcsImports";>read more</a>)</li>
+      </ul>
+    </tal:can-configure>
   </tal:no-branches>
 
   <tal:has-branches condition="view/branch_count">