Merge lp:~salgado/launchpad/team-page-tweaks into lp:launchpad

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

= Summary =

Fix a bunch of trivial bugs on the team-index page.

== Proposed fix ==

Fix
Bug 423080
Bug 423085
Bug 423091
Bug 423681

As part of it I moved the polls portlet into a separate file and got rid
of a now-unused menu link from CommonLinksMixin

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/templates/team-index.pt
  lib/lp/registry/templates/team-portlet-polls.pt
  lib/lp/registry/stories/foaf/xx-team-home.txt
  lib/lp/registry/browser/person.py
  lib/lp/registry/templates/person-related-software-navlinks.pt
  lib/lp/soyuz/templates/person-portlet-ppas.pt
  lib/lp/registry/templates/person-macros.pt
  lib/lp/registry/templates/team-portlet-membership.pt

== Pylint notices ==

lib/lp/registry/browser/person.py
    118: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)
    119: [F0401] Unable to import 'lazr.config' (No module named config)
    120: [F0401] Unable to import 'lazr.restful.interface' (No module
named restful)

[Curtis Hovey (sinzui)]

Hi Guilherme.

I really appreciate these fixes. I looked at the UI and we agreed that the top two portlets are guaranteed to display so we can place them in their own <div class="yui-g"> to make them align.

I recorded our IRC conversation inline.

> === modified file 'lib/lp/registry/browser/person.py'
> --- lib/lp/registry/browser/person.py 2009-09-08 14:07:32 +0000
> +++ lib/lp/registry/browser/person.py 2009-09-09 18:50:13 +0000
...

> @@ -884,11 +878,15 @@
> return Link(target, text, icon='info')
>
> def ppa(self):
> + # XXX: salgado, 2009-09-09, bug=426899: Need to conditionally disable
> + # this link.
> target = '+ppa-packages'
> text = 'PPA Packages'
> return Link(target, text, icon='info')
>
> def projects(self):
> + # XXX: salgado, 2009-09-09, bug=426900: Need to conditionally disable
> + # this link.

We agreed this is harder to do than the time we have allotted to fix these
issues.

...

> === modified file 'lib/lp/registry/stories/foaf/xx-team-home.txt'
> --- lib/lp/registry/stories/foaf/xx-team-home.txt 2009-09-01 00:49:34 +0000
> +++ lib/lp/registry/stories/foaf/xx-team-home.txt 2009-09-09 18:50:13 +0000

...

> @@ -60,9 +62,10 @@
> Show polls
>
> >>> browser.open('http://launchpad.dev/~launchpad')
> - >>> print extract_text(find_tag_by_id(browser.contents, 'polls'))
> - Polls
> - No polls created.
> + >>> from BeautifulSoup import BeautifulSoup, SoupStrainer
> + >>> print BeautifulSoup(
> + ... browser.contents, parseOnlyThese=SoupStrainer(id='polls'))
> + <BLANKLINE>

We agreed that this is the better way to test:

    >>> print find_tag_by_id(browser.contents, 'polls')
    None

review: Approve

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-09-09 09:24:47 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-09-09 18:50:13 +0000
@@ -1042,6 +1042,12 @@
             template="../templates/team-index.pt"/>
         <browser:page
             for="lp.registry.interfaces.person.ITeam"
+            class="lp.registry.browser.person.TeamIndexView"
+            permission="zope.Public"
+            name="+portlet-polls"
+            template="../templates/team-portlet-polls.pt"/>
+        <browser:page
+            for="lp.registry.interfaces.person.ITeam"
             permission="zope.Public"
             class="lp.registry.browser.team.TeamMapView"
             name="+map"
 
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2009-09-08 14:07:32 +0000
+++ lib/lp/registry/browser/person.py	2009-09-09 18:50:13 +0000
@@ -853,12 +853,6 @@
         text = 'Change home page'
         return Link(target, text, icon='edit')
 
-    def related_projects(self):
-        target = '+related-software#projects'
-        text = 'List related projects'
-        summary = 'Projects %s is involved with' % self.context.displayname
-        return Link(target, text, summary, icon='product')
-
     @enabled_with_permission('launchpad.Edit')
     def activate_ppa(self):
         target = "+activate-ppa"
@@ -867,14 +861,14 @@
                    'Package Archive and create a new PPA.')
         return Link(target, text, summary, icon='add')
 
-    def summary(self):
+    def related_software_summary(self):
         target = '+related-software'
         text = 'Summary'
         return Link(target, text, icon='info')
 
     def maintained(self):
         target = '+maintained-packages'
-        text = 'Maintained Packages'
+        text = 'Maintained packages'
         enabled = bool(self.person.getLatestMaintainedPackages())
         return Link(target, text, enabled=enabled, icon='info')
 
@@ -884,11 +878,15 @@
         return Link(target, text, icon='info')
 
     def ppa(self):
+        # XXX: salgado, 2009-09-09, bug=426899: Need to conditionally disable
+        # this link.
         target = '+ppa-packages'
         text = 'PPA Packages'
         return Link(target, text, icon='info')
 
     def projects(self):
+        # XXX: salgado, 2009-09-09, bug=426900: Need to conditionally disable
+        # this link.
         target = '+related-projects'
         text = 'Related Projects'
         return Link(target, text, icon='info')
@@ -903,7 +901,7 @@
              'editircnicknames', 'editjabberids', 'editpassword',
              'editsshkeys', 'editpgpkeys', 'editlocation', 'memberships',
              'mentoringoffers', 'codesofconduct', 'karma',
-             'administer', 'related_projects', 'activate_ppa', 'maintained',
+             'administer', 'projects', 'activate_ppa', 'maintained',
              'view_ppa_subscriptions']
 
     @enabled_with_permission('launchpad.Edit')
@@ -1080,7 +1078,8 @@
 
     usedfor = IPersonRelatedSoftwareMenu
     facet = 'overview'
-    links = ('summary', 'maintained', 'uploaded', 'ppa', 'projects')
+    links = ('related_software_summary', 'maintained', 'uploaded', 'ppa',
+             'projects')
 
     @property
     def person(self):
@@ -1297,8 +1296,7 @@
              'editemail', 'configure_mailing_list', 'moderate_mailing_list',
              'editlanguages', 'map', 'polls',
              'add_poll', 'join', 'leave', 'add_my_teams', 'mentorships',
-             'reassign', 'related_projects',
-             'activate_ppa', 'maintained']
+             'reassign', 'projects', 'activate_ppa', 'maintained', 'ppa']
 
 
 class TeamOverviewNavigationMenu(NavigationMenu, TeamMenuMixin):
@@ -2780,6 +2778,10 @@
         else:
             raise AssertionError('Unknown group to contact.')
 
+    def should_show_polls_portlet(self):
+        menu = TeamOverviewMenu(self.context)
+        return self.hasCurrentPolls() or menu.add_poll().enabled
+
     def hasCurrentPolls(self):
         """Return True if this team has any non-closed polls."""
         assert self.context.isTeam()
 
=== modified file 'lib/lp/registry/stories/foaf/xx-team-home.txt'
--- lib/lp/registry/stories/foaf/xx-team-home.txt	2009-09-01 00:49:34 +0000
+++ lib/lp/registry/stories/foaf/xx-team-home.txt	2009-09-09 18:50:13 +0000
@@ -36,8 +36,12 @@
     2005-06-06
 
     >>> print extract_text(
+    ...     find_tag_by_id(browser.contents, 'subscription-policy'))
+    Subscription policy:
+    Moderated Team
+
+    >>> print extract_text(
     ...     find_tag_by_id(browser.contents, 'membership-summary'))
-    This is a Moderated Team.
     10 active members, 1 invited members, 2 proposed members...
 
     >>> print extract_text(
@@ -49,10 +53,8 @@
     ...     find_tag_by_id(browser.contents, 'contact-languages'))
     Languages:
     English
-    Set preferred languages
 
-The polls portlet only shows a link to view all polls if current
-polls exist.
+The polls portlet is only shown if current polls exist.
 
     >>> print extract_text(find_tag_by_id(browser.contents, 'polls'))
     Polls
@@ -60,9 +62,10 @@
     Show polls
 
     >>> browser.open('http://launchpad.dev/~launchpad')
-    >>> print extract_text(find_tag_by_id(browser.contents, 'polls'))
-    Polls
-    No polls created.
+    >>> from BeautifulSoup import BeautifulSoup, SoupStrainer
+    >>> print BeautifulSoup(
+    ...     browser.contents, parseOnlyThese=SoupStrainer(id='polls'))
+    <BLANKLINE>
 
 The subteam-of portlet is not shown if the team is not a subteam.
 
@@ -129,6 +132,7 @@
     Email:
     Contact this team's email address
     [e-mail]
+    Set contact address
 
 Member can contact their team even if the team does not have a contact
 address:
 
=== modified file 'lib/lp/registry/templates/person-macros.pt'
--- lib/lp/registry/templates/person-macros.pt	2009-09-01 00:32:01 +0000
+++ lib/lp/registry/templates/person-macros.pt	2009-09-09 18:50:13 +0000
@@ -35,8 +35,7 @@
           <div style="white-space: nowrap">
             <img src="/@@/mail" alt="mail" />
             <tal:email replace="email">[e-mail]</tal:email>
-            <a class="sprite edit"
-               tal:attributes="href overview_menu/editemail/fmt:url"></a>
+            <a tal:replace="structure overview_menu/editemail/fmt:icon"></a>
           </div>
         </tal:emails>
         <a tal:condition="not: view/visible_email_addresses"
@@ -65,9 +64,15 @@
         <dt>Languages:</dt>
         <dd>
           <tal:languages content="view/languages">English</tal:languages>
-          <a tal:attributes="href overview_menu/editlanguages/fmt:url"
-            class="sprite edit">
-          <span class="invisible-link">Set preferred languages</span></a>
+          <a tal:replace="structure overview_menu/editlanguages/fmt:icon" />
+        </dd>
+      </dl>
+      <dl id="subscription-policy">
+        <dt>Subscription policy:</dt>
+        <dd>
+          <span tal:replace="context/subscriptionpolicy/title" />
+          <img src="/@@/maybe"
+            tal:attributes="title context/subscriptionpolicy/description" />
         </dd>
       </dl>
 
 
=== modified file 'lib/lp/registry/templates/person-related-software-navlinks.pt'
--- lib/lp/registry/templates/person-related-software-navlinks.pt	2009-09-08 14:37:17 +0000
+++ lib/lp/registry/templates/person-related-software-navlinks.pt	2009-09-09 18:50:13 +0000
@@ -6,7 +6,8 @@
 
     <ul id="navlinks" class="horizontal">
       <li>
-        <a tal:replace="structure view/menu:navigation/summary/render"/></li>
+        <a tal:replace="structure
+          view/menu:navigation/related_software_summary/render"/></li>
       <li>
         <a tal:replace="structure view/menu:navigation/maintained/render"/></li>
       <li>
 
=== modified file 'lib/lp/registry/templates/team-index.pt'
--- lib/lp/registry/templates/team-index.pt	2009-08-28 00:57:32 +0000
+++ lib/lp/registry/templates/team-index.pt	2009-09-09 18:50:13 +0000
@@ -43,54 +43,8 @@
   </ul>
 
   <tal:menu replace="structure view/@@+global-actions" />
-
-  <div id="polls" class="portlet">
-    <h2>Polls</h2>
-    <p tal:condition="not: view/hasCurrentPolls">
-      No polls created.
-    </p>
-
-    <ul tal:condition="view/hasCurrentPolls">
-      <li tal:repeat="poll view/openpolls">
-        <a tal:attributes="href poll/fmt:url">
-          <span tal:replace="poll/title" />
-        </a> - closes
-        <span
-          tal:attributes="title poll/datecloses/fmt:datetime"
-          tal:content="poll/datecloses/fmt:displaydate" />.
-
-        <tal:block define="user request/lp:person" condition="user">
-          <tal:block condition="python: poll.personVoted(user)">
-            You have
-            <span tal:replace="poll/closesIn/fmt:approximateduration" />
-            to change your vote if you wish.
-          </tal:block>
-
-          <tal:block condition="python: not poll.personVoted(user)">
-            You have
-            <span tal:replace="poll/closesIn/fmt:approximateduration" />
-            left to vote in this poll.
-          </tal:block>
-        </tal:block>
-
-      </li>
-
-      <li tal:condition="view/userIsOwner"
-          tal:repeat="poll view/notyetopenedpolls">
-        <a tal:attributes="href poll/fmt:url">
-          <span tal:replace="poll/title" />
-        </a> - opens
-        <span
-          tal:attributes="title poll/dateopens/fmt:datetime"
-          tal:content="poll/dateopens/fmt:displaydate" />
-      </li>
-    </ul>
-
-    <a tal:condition="view/hasCurrentPolls"
-       tal:replace="structure overview_menu/polls/fmt:link" />
-    <a tal:replace="structure overview_menu/add_poll/fmt:link" />
-
-  </div>
+  <tal:polls replace="structure context/@@+portlet-polls" />
+
 </div>
 
 <!-- heading slot. -->
@@ -111,6 +65,20 @@
       class="description"
       tal:content="structure context/teamdescription/fmt:text-to-html"
     />
+    <ul class="horizontal">
+      <li
+        tal:define="link context/menu:overview/projects"
+        tal:condition="link/enabled"
+        tal:content="structure link/fmt:link" />
+      <li
+        tal:define="link context/menu:overview/maintained"
+        tal:condition="link/enabled"
+        tal:content="structure link/fmt:link" />
+      <li
+        tal:define="link context/menu:overview/ppa"
+        tal:condition="link/enabled"
+        tal:content="structure link/fmt:link" />
+    </ul>
 </div>
 
 <!-- main slot. -->
@@ -120,16 +88,16 @@
   <div class="yui-g">
     <!-- First portlet column. -->
     <div class="first yui-u">
-      <div tal:content="structure context/@@+portlet-membership" />
+      <metal:details use-macro="context/@@+person-macros/team-details" />
       <div tal:content="structure context/@@+portlet-ppas" />
-      <metal:subteam-of use-macro="context/@@+person-macros/subteam-of" />
+      <div tal:content="structure context/@@+portlet-mailinglist" />
     </div>
 
     <!-- Second portlet column. -->
     <div class="yui-u">
-      <metal:details use-macro="context/@@+person-macros/team-details" />
+      <div tal:content="structure context/@@+portlet-membership" />
       <div tal:content="structure context/@@+portlet-related-projects" />
-      <div tal:content="structure context/@@+portlet-mailinglist" />
+      <metal:subteam-of use-macro="context/@@+person-macros/subteam-of" />
     </div>
   </div>
 
 
=== modified file 'lib/lp/registry/templates/team-portlet-membership.pt'
--- lib/lp/registry/templates/team-portlet-membership.pt	2009-09-03 17:20:42 +0000
+++ lib/lp/registry/templates/team-portlet-membership.pt	2009-09-09 18:53:47 +0000
@@ -17,13 +17,6 @@
   <div id="membership-summary" class="portletBody portletContent"
     style="margin-bottom: 1.5em;">
     <div id="membership-summary">
-      This is a
-      <span>
-        <strong id="subscription-policy"
-                tal:content="context/subscriptionpolicy/title" />.
-        <img src="/@@/maybe"
-          tal:attributes="title context/subscriptionpolicy/description" />
-      </span>
       <div>
         <img src="/@@/team" alt="team" />
         <strong><tal:active content="context/all_member_count" /></strong>
@@ -128,4 +121,5 @@
     </tr>
   </table>
   </tal:can-view>
+</div>
 </tal:root>
 
=== added file 'lib/lp/registry/templates/team-portlet-polls.pt'
--- lib/lp/registry/templates/team-portlet-polls.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/team-portlet-polls.pt	2009-09-09 18:50:13 +0000
@@ -0,0 +1,56 @@
+<tal:root
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  omit-tag="">
+
+  <div id="polls" class="portlet"
+       tal:define="overview_menu context/menu:overview"
+       tal:condition="view/should_show_polls_portlet">
+    <h2>Polls</h2>
+    <p tal:condition="not: view/hasCurrentPolls">
+      No polls created.
+    </p>
+
+    <ul tal:condition="view/hasCurrentPolls">
+      <li tal:repeat="poll view/openpolls">
+        <a tal:attributes="href poll/fmt:url">
+          <span tal:replace="poll/title" />
+        </a> - closes
+        <span
+          tal:attributes="title poll/datecloses/fmt:datetime"
+          tal:content="poll/datecloses/fmt:displaydate" />.
+
+        <tal:block define="user request/lp:person" condition="user">
+          <tal:block condition="python: poll.personVoted(user)">
+            You have
+            <span tal:replace="poll/closesIn/fmt:approximateduration" />
+            to change your vote if you wish.
+          </tal:block>
+
+          <tal:block condition="python: not poll.personVoted(user)">
+            You have
+            <span tal:replace="poll/closesIn/fmt:approximateduration" />
+            left to vote in this poll.
+          </tal:block>
+        </tal:block>
+
+      </li>
+
+      <li tal:condition="view/userIsOwner"
+          tal:repeat="poll view/notyetopenedpolls">
+        <a tal:attributes="href poll/fmt:url">
+          <span tal:replace="poll/title" />
+        </a> - opens
+        <span
+          tal:attributes="title poll/dateopens/fmt:datetime"
+          tal:content="poll/dateopens/fmt:displaydate" />
+      </li>
+    </ul>
+
+    <a tal:condition="view/hasCurrentPolls"
+       tal:replace="structure overview_menu/polls/fmt:link" />
+    <a tal:replace="structure overview_menu/add_poll/fmt:link" />
+
+  </div>
+</tal:root>
 
=== modified file 'lib/lp/soyuz/templates/person-portlet-ppas.pt'
--- lib/lp/soyuz/templates/person-portlet-ppas.pt	2009-08-21 15:16:48 +0000
+++ lib/lp/soyuz/templates/person-portlet-ppas.pt	2009-09-09 18:50:13 +0000
@@ -15,10 +15,6 @@
               tal:condition="link/enabled"
               tal:content="structure link/fmt:icon-link" />
 
-          <li tal:define="link context/menu:overview/maintained"
-              tal:condition="link/enabled"
-              tal:replace="structure link/fmt:icon-link" />
-
           <tal:is-person condition="not: context/is_team">
             <li tal:define="link context/menu:overview/view_ppa_subscriptions"
                 tal:condition="link/enabled"