Merge lp:~edwin-grubbs/launchpad/bug-234051-bug-project-info into lp:launchpad/db-devel

[Edwin Grubbs]

Added pillar info below the bug portlet on the person index page.

[Edwin Grubbs (edwin-grubbs)]

Summary
-------

In the "Assigned bugs" portlet (person-portlet-currentfocus.pt), I added
a table to display the badges like other portlets do. I also added an
additional line that lists all the pillars that a given bug affects.

I also removed the $distribution/+allpackages page as decided in bug
196322.

I removed the italics from the "registered" css class as requested
in bug 465524 to fix the "Latest memberships" portlet.

Here is a screenshot for the UI review.
https://chinstrap.canonical.com/~egrubbs/bugs_with_pillar_info_and_latest_memberships_without_italics.png

Implementation details
----------------------

I converted the bugtask-listing-detailed.pt page into a macro so that
the show_extra_details variable could be passed in.
    lib/lp/bugs/templates/bugtask-macros-listing.pt

Tests
-----

The only test changes that were necessary were to remove some for
the deleted +allpackages page.

Demo and Q/A
------------

* Open http://launchpad.dev/~name16

[Barry Warsaw (barry)]

> In the "Assigned bugs" portlet (person-portlet-currentfocus.pt), I added
> a table to display the badges like other portlets do. I also added an
> additional line that lists all the pillars that a given bug affects.

This looks great. The one question I have is why all the pillars are not
linked? Looking at the screenshot I see that e.g. for bug #9 Ubuntu is
linked, but not alsa-utils, Evolution, or Mozilla Thunderbird. How hard would
it be to link all of those?

> I also removed the $distribution/+allpackages page as decided in bug
> 196322.
>
> I removed the italics from the "registered" css class as requested
> in bug 465524 to fix the "Latest memberships" portlet.

Yay!

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css 2009-11-04 17:35:50 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css 2009-11-06 17:30:56 +0000
> @@ -424,10 +424,6 @@
> .registered {
> font-size: 85%;
> color: #666;
> - font-style: italic;
> - }
> -li .registered {
> - font-style: normal;

There's a subtle change here. Not only are you deleting the italics style
from .registered (good) but you're also adding the font-size and color styles
to li.registered. That might be okay, but is it what you intended?

> }
> .description {
> clear: both;

=== renamed file 'lib/lp/bugs/templates/bugtask-listing-detailed.pt' => 'lib/lp/bugs/templates/bugtask-macros-listing.pt'
--- lib/lp/bugs/templates/bugtask-listing-detailed.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bugtask-macros-listing.pt 2009-11-06 17:30:56 +0000
> @@ -1,28 +1,38 @@
> -<tr xmlns:tal="http://xml.zope.org/namespaces/tal">
> - <td class="icon left" tal:content="structure context/image:icon" />
> - <td>
> - <div>
> - #<span tal:replace="context/bug/id">4</span>
> - <a tal:attributes="href context/fmt:url"
> - tal:content="context/bug/title">Bug Title Here
> - </a>
> - <tal:badges replace="structure context/image:badges" />
> - </div>
> - <div class="lesser" tal:condition="hide_extra_details|nothing">
> - <tal:affected_pillars define="also_in context/other_affected_pillars"
> - condition="also_in">
> - also in
> - <tal:per_pillar repeat="pillar also_in">
> - <b tal:content="pillar/displayname" />,
> - </tal:per_pillar>
> - </tal:affected_pillars>
> - reported
> - <span
> - tal:attributes="title context/datecreated/fmt:datetime"
> - tal:content="context/datecreated/fmt:displaydate">2005-10-05</span>
> - by
> - <a tal:attributes="href context/owner/fmt:url"
> - tal:content="context/owner/displayname">Foo Bar</a>
> - </div>
> - </td>
> -</tr>
> +<metal:header
> + xmlns:metal="http://xml.zope.org/namespaces/metal"
> + xmlns:tal="http://xml.zope.org/namespaces/tal"
> + define-macro="detailed">
> + <tal:comment replace="nothing">
> + Variables to be defined for this macro.
> + :bugtask: (Required) BugTask that will be displayed.
> + :show_extra_details: (Optional) Boolean determining whether
> + to display a second line of in...

review: Approve (code ui*)

[Martin Albisetti (beuno)]

+1 on barry's comments, and I would also add some bottom padding to after each item (not a lot, maybe 0.5-1em).

review: Approve (ui)

[Curtis Hovey (sinzui)]

Hi Edwin.

I looked at the screenshot. I think alignment of the badges is hard to scan. I know that badges are often aligned to the right and playout in a fixed order so that the milestone badges always align. Consider your own profile page where most bugs will have a milestone, and a few will have branches.

review: Needs Information (ui)

[Edwin Grubbs (edwin-grubbs)]

Hi Barry,

Thanks for the review. In addition to your comments that I addressed
below, I added the padding that Martin asked for and the change in
badge alignment that Curtis asked for.

On Fri, Nov 6, 2009 at 12:45 PM, Barry Warsaw <email address hidden> wrote:
> Review: Approve code ui*
>> In the "Assigned bugs" portlet (person-portlet-currentfocus.pt), I added
>> a table to display the badges like other portlets do. I also added an
>> additional line that lists all the pillars that a given bug affects.
>
> This looks great.  The one question I have is why all the pillars are not
> linked?  Looking at the screenshot I see that e.g. for bug #9 Ubuntu is
> linked, but not alsa-utils, Evolution, or Mozilla Thunderbird.  How hard would
> it be to link all of those?

Not hard. I'll do that.

>> I also removed the $distribution/+allpackages page as decided in bug
>> 196322.
>>
>> I removed the italics from the "registered" css class as requested
>> in bug 465524 to fix the "Latest memberships" portlet.
>
> Yay!
>
> === modified file 'lib/canonical/launchpad/icing/style-3-0.css'
> --- lib/canonical/launchpad/icing/style-3-0.css 2009-11-04 17:35:50 +0000
> +++ lib/canonical/launchpad/icing/style-3-0.css 2009-11-06 17:30:56 +0000
>> @@ -424,10 +424,6 @@
>>  .registered {
>>      font-size: 85%;
>>      color: #666;
>> -    font-style: italic;
>> -    }
>> -li .registered {
>> -    font-style: normal;
>
> There's a subtle change here.  Not only are you deleting the italics style
> from .registered (good) but you're also adding the font-size and color styles
> to li.registered.  That might be okay, but is it what you intended?

The "li .registered" selector doesn't eliminate all the styles from
the ".registered", just the ones that are explicitely overridden
(font-style), so elements with the "registered" class inside an <li>
were already getting their font-size and color set as it is now.

>>      }
>>  .description {
>>      clear: both;
>
> === renamed file 'lib/lp/bugs/templates/bugtask-listing-detailed.pt' => 'lib/lp/bugs/templates/bugtask-macros-listing.pt'
> --- lib/lp/bugs/templates/bugtask-listing-detailed.pt   2009-07-17 17:59:07 +0000
> +++ lib/lp/bugs/templates/bugtask-macros-listing.pt     2009-11-06 17:30:56 +0000
>> @@ -1,28 +1,38 @@
>> -<tr xmlns:tal="http://xml.zope.org/namespaces/tal">
>> -  <td class="icon left" tal:content="structure context/image:icon" />
>> -  <td>
>> -    <div>
>> -      #<span tal:replace="context/bug/id">4</span>
>> -      <a tal:attributes="href context/fmt:url"
>> -         tal:content="context/bug/title">Bug Title Here
>> -      </a>
>> -      <tal:badges replace="structure context/image:badges" />
>> -    </div>
>> -    <div class="lesser" tal:condition="hide_extra_details|nothing">
>> -      <tal:affected_pillars define="also_in context/other_affected_pillars"
>> -                           condition="also_in">
>> -        also in
>> -          <tal:per_pillar repeat="pillar also_in">
>> -            <b tal:content="pillar/displayname" />,
>> -          </tal:per_pillar>
>> -        </tal:affected_pillars>
>> -      reported
>> -      <span
>> -        tal:attributes="title context/datecreated/fmt:datet...

[Curtis Hovey (sinzui)]

Thank you very much for fixing some many irksome problems. I know it was trying to reconcile all these little issues. I really appreciate this fix.

review: Approve (ui)

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2009-11-04 17:35:50 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2009-11-06 22:51:12 +0000
@@ -424,10 +424,6 @@
 .registered {
     font-size: 85%;
     color: #666;
-    font-style: italic;
-    }
-li .registered {
-    font-style: normal;
     }
 .description {
     clear: both;
 
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2009-10-30 12:09:16 +0000
+++ lib/lp/bugs/browser/configure.zcml	2009-11-06 22:51:12 +0000
@@ -487,8 +487,8 @@
             template="../templates/bugtask-tasks-and-nominations-table-row.pt"/>
         <browser:page
             for="lp.bugs.interfaces.bugtask.IBugTask"
-            name="+listing-detailed"
-            template="../templates/bugtask-listing-detailed.pt"
+            name="+bugtask-macros-listing"
+            template="../templates/bugtask-macros-listing.pt"
             permission="zope.Public"/>
         <browser:page
             name="+edit"
 
=== renamed file 'lib/lp/bugs/templates/bugtask-listing-detailed.pt' => 'lib/lp/bugs/templates/bugtask-macros-listing.pt'
--- lib/lp/bugs/templates/bugtask-listing-detailed.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bugtask-macros-listing.pt	2009-11-06 22:51:12 +0000
@@ -1,28 +1,39 @@
-<tr xmlns:tal="http://xml.zope.org/namespaces/tal">
-  <td class="icon left" tal:content="structure context/image:icon" />
-  <td>
-    <div>
-      #<span tal:replace="context/bug/id">4</span>
-      <a tal:attributes="href context/fmt:url"
-         tal:content="context/bug/title">Bug Title Here
-      </a>
-      <tal:badges replace="structure context/image:badges" />
-    </div>
-    <div class="lesser" tal:condition="hide_extra_details|nothing">
-      <tal:affected_pillars define="also_in context/other_affected_pillars"
-                           condition="also_in">
-        also in 
-          <tal:per_pillar repeat="pillar also_in">
-            <b tal:content="pillar/displayname" />,
-          </tal:per_pillar>
-        </tal:affected_pillars>
-      reported
-      <span
-        tal:attributes="title context/datecreated/fmt:datetime"
-        tal:content="context/datecreated/fmt:displaydate">2005-10-05</span>
-      by
-      <a tal:attributes="href context/owner/fmt:url"
-         tal:content="context/owner/displayname">Foo Bar</a>
-    </div>
-  </td>
-</tr>
+<metal:header
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  define-macro="detailed">
+  <tal:comment replace="nothing">
+    Variables to be defined for this macro.
+      :bugtask: (Required) BugTask that will be displayed.
+      :show_extra_details: (Optional) Boolean determining whether
+          to display a second line of info below the bug title.
+  </tal:comment>
+  <tr>
+    <td class="icon left" tal:content="structure bugtask/image:icon" />
+    <td>
+        #<span tal:replace="bugtask/bug/id">4</span>
+        <a tal:attributes="href bugtask/fmt:url"
+          tal:content="bugtask/bug/title">Bug Title Here
+        </a>
+    </td>
+    <td style="text-align: right">
+      <tal:badges replace="structure bugtask/image:badges" />
+    </td>
+  </tr>
+  <tr tal:condition="show_extra_details|nothing">
+    <td colspan="3" style="padding-bottom: 0.5em">
+      <div class="registered">
+        <tal:affected_pillars define="also_in bugtask/other_affected_pillars">
+          in <a tal:attributes="href bugtask/pillar/fmt:url"
+                tal:content="bugtask/pillar/displayname"
+              /><tal:comma condition="also_in">,</tal:comma>
+            <tal:per_pillar repeat="pillar also_in">
+              <a tal:attributes="href pillar/fmt:url"
+                 tal:content="pillar/displayname"
+                 /><tal:comma condition="not: repeat/pillar/end">,</tal:comma>
+            </tal:per_pillar>
+          </tal:affected_pillars>
+      </div>
+    </td>
+  </tr>
+</metal:header>
 
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-10-26 19:47:59 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-11-06 22:51:12 +0000
@@ -1770,13 +1770,6 @@
             DistributionNavigation"/>
     <browser:page
         for="lp.registry.interfaces.distribution.IDistribution"
-        class="lp.registry.browser.distribution.DistributionAllPackagesView"
-        permission="zope.Public"
-        name="+allpackages"
-        facet="overview"
-        template="../templates/distribution-allpackages.pt"/>
-    <browser:page
-        for="lp.registry.interfaces.distribution.IDistribution"
         facet="overview"
         class="lp.registry.browser.distribution.DistributionArchiveMirrorsView"
         permission="zope.Public"
 
=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2009-10-20 14:18:10 +0000
+++ lib/lp/registry/browser/distribution.py	2009-11-06 22:51:12 +0000
@@ -8,7 +8,6 @@
 __all__ = [
     'DerivativeDistributionOverviewMenu',
     'DistributionAddView',
-    'DistributionAllPackagesView',
     'DistributionArchiveMirrorsRSSView',
     'DistributionArchiveMirrorsView',
     'DistributionArchivesView',
@@ -296,7 +295,7 @@
 
     usedfor = IDistribution
     facet = 'overview'
-    links = ['edit', 'branding', 'driver', 'search', 'allpkgs', 'members',
+    links = ['edit', 'branding', 'driver', 'search', 'members',
              'mirror_admin', 'reassign', 'addseries', 'series', 'milestones',
              'top_contributors',
              'builds', 'cdimage_mirrors', 'archive_mirrors',
@@ -360,10 +359,6 @@
         enabled = self._userCanSeeNonPublicMirrorListings()
         return Link('+unofficialmirrors', text, enabled=enabled, icon='info')
 
-    def allpkgs(self):
-        text = 'List all packages'
-        return Link('+allpackages', text, icon='info')
-
     @enabled_with_permission('launchpad.Edit')
     def members(self):
         text = 'Change members team'
@@ -717,16 +712,6 @@
             distribution=self.context)
 
 
-class DistributionAllPackagesView(LaunchpadView):
-    """A view to show all the packages in a distribution."""
-
-    def initialize(self):
-        results = self.context.getSourcePackageCaches()
-        self.batchnav = BatchNavigator(results, self.request)
-
-    label = 'All packages'
-
-
 class DistributionSetActionNavigationMenu(RegistryCollectionActionMenuBase):
     """Action menu for `DistributionSetView`."""
 
 
=== removed file 'lib/lp/registry/stories/distribution/xx-distribution-all-packages.txt'
--- lib/lp/registry/stories/distribution/xx-distribution-all-packages.txt	2009-09-18 15:24:30 +0000
+++ lib/lp/registry/stories/distribution/xx-distribution-all-packages.txt	1970-01-01 00:00:00 +0000
@@ -1,40 +0,0 @@
-
-  >>> print http(r"""
-  ... GET /ubuntu/+allpackages HTTP/1.1
-  ... """)
-  HTTP/1.1 200 Ok
-  ...
-  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
-  <BLANKLINE>
-  ...
-      <title>All packages...</title>
-  ...
-          <strong>1</strong>
-          &rarr;
-          <strong>5</strong>
-          of
-          10 results
-  ...
-        <a href="...">alsa-utils</a>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-        <a href="...">cnews</a>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-        <a href="...">commercialpackage</a>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-        <a href="...">evolution</a>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-  <BLANKLINE>
-        <a href="...">foobar</a>
-  ...
-
 
=== removed file 'lib/lp/registry/templates/distribution-allpackages.pt'
--- lib/lp/registry/templates/distribution-allpackages.pt	2009-09-15 01:17:46 +0000
+++ lib/lp/registry/templates/distribution-allpackages.pt	1970-01-01 00:00:00 +0000
@@ -1,25 +0,0 @@
-<html
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  metal:use-macro="view/macro:page/main_only"
-  i18n:domain="launchpad">
-  <body>
-    <div metal:fill-slot="main"
-         tal:define="batch view/batchnav/currentBatch">
-      <tal:navigation replace="structure view/batchnav/@@+navigation-links-upper" />
-
-      <p>
-      <tal:per_item repeat="result batch">
-        <a tal:content="result/name"
-           tal:attributes="href result/distributionsourcepackage/fmt:url"
-           >apache2</a>
-      </tal:per_item>
-      </p>
-
-      <tal:navigation replace="structure view/batchnav/@@+navigation-links-lower" />
-
-    </div>
-  </body>
-</html>
 
=== modified file 'lib/lp/registry/templates/person-portlet-currentfocus.pt'
--- lib/lp/registry/templates/person-portlet-currentfocus.pt	2009-09-11 19:59:54 +0000
+++ lib/lp/registry/templates/person-portlet-currentfocus.pt	2009-11-06 22:51:12 +0000
@@ -20,8 +20,8 @@
 
         <table>
           <tr tal:repeat="bugtask bugtasks">
-            <tal:define-vars define="hide_extra_details string:true">
-              <tal:task tal:replace="structure bugtask/@@+listing-detailed" />
+            <tal:define-vars define="show_extra_details python:True">
+              <metal:task use-macro="bugtask/@@+bugtask-macros-listing/detailed" />
             </tal:define-vars>
           </tr>
         </table>