launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-671539 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #671539 "registered branches" on teams' pages is useless
  https://bugs.launchpad.net/bugs/671539


= Summary =

Don't show the number of branches registered by teams as it is nonsense.

== Proposed fix ==

Make the link disabled if the person is a team and make the template be
conditional based on the enabled property.

Also removed some extraneous conditions in the page template.  Note at
the top 'menu' is used in a condition so it cannot be unset in the body
of the template.

Also changed the class on the <td>s for consistency of count alignment.

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.code xx-person-branches.txt

== Demo and Q/A ==

Look at https://launchpad.dev/~landscape-developers and
http://launchpad.net/~launchpad to see that the line is not displayed.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/stories/branches/xx-person-branches.txt
  lib/lp/code/templates/person-codesummary.pt
  lib/lp/code/browser/branchlisting.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-671539/+merge/40309
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-671539 into lp:launchpad/devel.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2010-10-27 08:51:05 +0000
+++ lib/lp/code/browser/branchlisting.py	2010-11-08 09:21:16 +0000
@@ -914,11 +914,16 @@
                 self.owned_branch_count, 'owned branch', 'owned branches'))
 
     def registered(self):
+        if not self.person.is_team:
+            enabled = True
+        else:
+            enabled = False
         return Link(
             '+registeredbranches',
             get_plural_text(
                 self.registered_branch_count,
-                'registered branch', 'registered branches'))
+                'registered branch', 'registered branches'),
+            enabled=enabled)
 
     def subscribed(self):
         return Link(

=== modified file 'lib/lp/code/stories/branches/xx-person-branches.txt'
--- lib/lp/code/stories/branches/xx-person-branches.txt	2010-10-18 20:59:08 +0000
+++ lib/lp/code/stories/branches/xx-person-branches.txt	2010-11-08 09:21:16 +0000
@@ -132,3 +132,21 @@
     2 registered branches
     1 subscribed branch
     0 active reviews
+
+
+Teams do not show registered branches
+=====================================
+
+For a team, showing registered branches does not make sense, so that
+line is omitted.
+
+    >>> browser.open('http://code.launchpad.dev/~landscape-developers')
+    >>> print_tag_with_id(
+    ...     browser.contents, 'portlet-person-codesummary')
+    1 owned branch
+    2 subscribed branches
+    0 active reviews
+
+    >>> browser.getLink('registered').click()
+    Traceback (most recent call last):
+    LinkNotFoundError

=== modified file 'lib/lp/code/templates/person-codesummary.pt'
--- lib/lp/code/templates/person-codesummary.pt	2010-10-15 01:48:05 +0000
+++ lib/lp/code/templates/person-codesummary.pt	2010-11-08 09:21:16 +0000
@@ -10,27 +10,21 @@
   <table>
     <tr class="code-links">
       <td class="code-count" tal:content="menu/owned_branch_count">100</td>
-      <td tal:condition="menu"
-          tal:content="structure menu/owned/render"
+      <td tal:content="structure menu/owned/render"
           />
     </tr>
-    <tr>
+    <tr class="code-links"
+        tal:condition="menu/registered/enabled">
       <td class="code-count" tal:content="menu/registered_branch_count">100</td>
-      <td tal:condition="menu"
-          tal:content="structure menu/registered/render"
-          />
+      <td tal:content="structure menu/registered/render" />
     </tr>
-    <tr>
+    <tr class="code-links">
       <td class="code-count" tal:content="menu/subscribed_branch_count">100</td>
-      <td tal:condition="menu"
-          tal:content="structure menu/subscribed/render"
-          />
+      <td tal:content="structure menu/subscribed/render" />
     </tr>
-    <tr id="merge-counts">
+    <tr class="code-links" id="merge-counts">
       <td class="code-count" tal:content="menu/active_review_count">5</td>
-      <td tal:condition="menu"
-          tal:content="structure menu/active_reviews/render"
-          />
+      <td tal:content="structure menu/active_reviews/render" />
     </tr>
   </table>
 </div>