Merge lp:~edwin-grubbs/launchpad/bug-663857-product-packages-timeout into lp:launchpad

[Edwin Grubbs]

[r=sinzui][ui=sinzui][bug=663857] Fixed timeout on $project/+packages page.

Summary
-------

This branch fixes the timeout on the $project/+packages page. It
restricts the results to series that are active or have packages. It
also batches the results.

I cleaned up the formatting, since a list of tables is fairly
confusing. I also sorted the results descending. If the series are named
by version number, the latest versions will appear at the top.

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

    lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/packaging-views.txt
lib/lp/registry/interfaces/product.py
lib/lp/registry/model/product.py
lib/lp/registry/templates/product-packages.pt

Tests
-----

./bin/test -vv -t 'packaging-views.txt|xx-product-package-pages.txt'

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

* Open https://code.edge.launchpad.net/obsolete-junk/+packages
  * The page should not timeout.

Lint
----

The one remaining lint item shouldn't be fixed.

Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/tests/packaging-views.txt
  lib/lp/registry/interfaces/product.py
  lib/lp/registry/model/product.py
  lib/lp/registry/templates/product-packages.pt

./lib/lp/registry/interfaces/product.py
     981: E301 expected 1 blank line, found 2

[Curtis Hovey (sinzui)]

Hi Edwin.

Thank you for improving the layout while address the timeout. This makes the page much easier to read.

review: Approve (code ui)

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-12-30 16:22:22 +0000
+++ lib/lp/registry/browser/product.py	2011-01-19 15:36:00 +0000
@@ -1117,25 +1117,10 @@
     page_title = label
 
     @cachedproperty
-    def series_packages(self):
-        """A hierarchy of product series, packaging and field data.
-
-        A dict of series and packagings. Each packaging is a dict of the
-        packaging and a hidden HTML field for forms:
-           [{series: <hoary>,
-             packagings: {
-                packaging: <packaging>,
-                field: '<input type=''hidden' ...>},
-                }]
-        """
-        packaged_series = []
-        for series in self.context.series:
-            packagings = []
-            for packaging in series.packagings:
-                packagings.append(packaging)
-            packaged_series.append(dict(
-                series=series, packagings=packagings))
-        return packaged_series
+    def series_batch(self):
+        """A batch of series that are active or have packages."""
+        return BatchNavigator(
+            self.context.active_or_packaged_series, self.request)
 
     @property
     def distro_packaging(self):
 
=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
--- lib/lp/registry/browser/tests/packaging-views.txt	2010-05-27 04:22:46 +0000
+++ lib/lp/registry/browser/tests/packaging-views.txt	2011-01-19 15:36:00 +0000
@@ -224,20 +224,19 @@
     >>> print view.label
     Linked packages
 
-The view provides the series_packages property that returns a list of
-dicts. Each dict as a series and a list of packages.
+The view provides the series_batch property.
 
     >>> def print_packages(view):
-    ...     for series_dict in view.series_packages:
-    ...         print series_dict['series'].name
-    ...         for package in series_dict['packagings']:
-    ...             print package.distroseries.name
+    ...     for series in view.series_batch.batch:
+    ...         print series.name
+    ...         for package in series.packagings:
+    ...             print '  Package:', package.distroseries.name
     >>> print_packages(view)
+    trunk
+    hotter
+      Package: grumpy
+      Package: hoary
     cold
-    hotter
-      grumpy
-      hoary
-    trunk
 
 The view provides the distro_packaging property that is a list of
 dictionaries for the distributions and their packaging.  The list is
@@ -255,11 +254,11 @@
     >>> view = create_initialized_view(
     ...     product, name='+packages', principal=a_user)
     >>> print_packages(view)
+    trunk
+    hotter
+      Package: grumpy
+      Package: hoary
     cold
-    hotter
-      grumpy
-      hoary
-    trunk
 
     # There are links to the +remove-packaging page.
     >>> table = find_tag_by_id(view.render(), 'packages-hotter')
@@ -270,8 +269,8 @@
     http://launchpad.dev/ubuntu/hoary/+source/thunderbird/+remove-packaging
 
     >>> [hoary_package] = [
-    ...     package for series_dict in view.series_packages
-    ...     for package in series_dict['packagings']
+    ...     package for series in view.series_batch.batch
+    ...     for package in series.packagings
     ...     if package.distroseries.name == 'hoary']
     >>> form = {'field.actions.unlink': 'Unlink'}
     >>> unlink_view = create_initialized_view(
@@ -279,13 +278,13 @@
     >>> unlink_view.errors
     []
 
-    # The view has to be reloaded since view.series_packages is cached.
+    # The view has to be reloaded since view.series_batch is cached.
     >>> view = create_initialized_view(product, name='+packages')
     >>> print_packages(view)
+    trunk
+    hotter
+      Package: grumpy
     cold
-    hotter
-      grumpy
-    trunk
 
 
 Distro series +packaging view
 
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2010-12-31 03:39:53 +0000
+++ lib/lp/registry/interfaces/product.py	2011-01-19 15:36:00 +0000
@@ -732,6 +732,9 @@
                 "Some bug trackers host multiple projects at the same URL "
                 "and require an identifier for the specific project.")))
 
+    active_or_packaged_series = Attribute(
+        _("Series that are active and/or have been packaged."))
+
     def getVersionSortedSeries(statuses=None, filter_statuses=None):
         """Return all the series sorted by the name field as a version.
 
 
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-12-02 16:13:51 +0000
+++ lib/lp/registry/model/product.py	2011-01-19 15:36:00 +0000
@@ -28,13 +28,17 @@
     SQLObjectNotFound,
     StringCol,
     )
-from storm.expr import NamedFunc
+from storm.expr import (
+    LeftJoin,
+    NamedFunc,
+    )
 from storm.locals import (
     And,
     Desc,
     Int,
     Join,
     Not,
+    Or,
     Select,
     SQL,
     Store,
@@ -48,9 +52,6 @@
     )
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.launchpad.components.decoratedresultset import (
-    DecoratedResultSet,
-    )
 from canonical.database.constants import UTC_NOW
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.database.enumcol import EnumCol
@@ -59,6 +60,9 @@
     SQLBase,
     sqlvalues,
     )
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.interfaces.launchpad import (
     IHasIcon,
     IHasLogo,
@@ -74,7 +78,6 @@
     IStoreSelector,
     MAIN_STORE,
     )
-from lp.registry.model.series import ACTIVE_STATUSES
 from lp.answers.interfaces.faqtarget import IFAQTarget
 from lp.answers.interfaces.questioncollection import (
     QUESTION_STATUS_DEFAULT_SEARCH,
@@ -145,6 +148,7 @@
 from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.hasdrivers import HasDriversMixin
 from lp.registry.model.karma import KarmaContextMixin
 from lp.registry.model.milestone import (
     HasMilestonesMixin,
@@ -156,7 +160,7 @@
 from lp.registry.model.productlicense import ProductLicense
 from lp.registry.model.productrelease import ProductRelease
 from lp.registry.model.productseries import ProductSeries
-from lp.registry.model.hasdrivers import HasDriversMixin
+from lp.registry.model.series import ACTIVE_STATUSES
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.registry.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin,
@@ -801,6 +805,22 @@
         orderBy='name')
 
     @property
+    def active_or_packaged_series(self):
+        store = Store.of(self)
+        tables = [
+            ProductSeries,
+            LeftJoin(Packaging, Packaging.productseries == ProductSeries.id),
+            ]
+        result = store.using(*tables).find(
+            ProductSeries,
+            ProductSeries.product == self,
+            Or(ProductSeries.status.is_in(ACTIVE_STATUSES),
+               Packaging.id != None))
+        result = result.order_by(Desc(ProductSeries.name))
+        result.config(distinct=True)
+        return result
+
+    @property
     def name_with_project(self):
         """See lib.canonical.launchpad.interfaces.IProduct"""
         if self.project and self.project.name != self.name:
@@ -1572,7 +1592,7 @@
 
     def getTranslatables(self):
         """See `IProductSet`"""
-        # XXX j.c.sackett 2010-11-19 bug=677532 It's less than ideal that 
+        # XXX j.c.sackett 2010-11-19 bug=677532 It's less than ideal that
         # this query is using _translations_usage, but there's no cleaner
         # way to deal with it. Once the bug above is resolved, this should
         # should be fixed to use translations_usage.
 
=== modified file 'lib/lp/registry/templates/product-packages.pt'
--- lib/lp/registry/templates/product-packages.pt	2010-03-03 18:35:08 +0000
+++ lib/lp/registry/templates/product-packages.pt	2011-01-19 15:36:00 +0000
@@ -10,28 +10,44 @@
 <body>
 
 <div metal:fill-slot="main">
-  <div class="top-portlet">
+  <div class="top-portlet"
+       tal:define="batch_nav view/series_batch">
     <h2>Packages by project series</h2>
 
-  <tal:series_dict repeat="series_dict view/series_packages">
-  <tal:series_packagings
-    define="series series_dict/series;
-            packagings series_dict/packagings">
+  <tal:multipage tal:condition="batch_nav/has_multiple_pages">
+    <tal:navigation
+        replace="structure batch_nav/@@+navigation-links-upper"/>
+  </tal:multipage>
+  <table class="listing">
+  <tr tal:repeat="series batch_nav/batch">
+  <td tal:define="packagings series/packagings">
+    <table style="width: 100%; margin: 0.5em 0 0 0">
+    <tr>
+    <td style="width: 33%">
     <h3>
       <a tal:attributes="href series/name">
         <span tal:replace="series/name">main</span> series
       </a>
     </h3>
+    </td>
 
-    <p tal:condition="not: packagings">
+    <tal:no-packages condition="not: packagings">
+      <td style="width: 33%; text-align: center">
       No packages linked to this series.
-    </p>
+      </td>
+      <td style="width: 33%; padding: 0 1.5em 0 0; text-align: right">
+        <a tal:condition="series/menu:overview/ubuntupkg/linked"
+           tal:replace="structure series/menu:overview/ubuntupkg/fmt:link" />
+      </td>
+    </tal:no-packages>
+    </tr>
+    </table>
 
     <tal:comment condition="nothing">
       This DIV is necessary for the table-actions:nth-child stylesheet.
     </tal:comment>
     <div>
-      <table class="listing"
+      <table class="listing" style="border: 0"
         tal:condition="packagings"
         tal:attributes="id series/name/fmt:css-id/packages-">
         <thead>
@@ -73,15 +89,19 @@
         </tbody>
       </table>
 
-      <ul class="table-actions"
-        tal:condition="series/menu:overview/ubuntupkg/linked">
-        <li>
+      <ul class="table-actions" tal:condition="packagings">
+        <li tal:condition="series/menu:overview/ubuntupkg/linked">
           <a tal:replace="structure series/menu:overview/ubuntupkg/fmt:link" />
         </li>
       </ul>
     </div>
-  </tal:series_packagings>
-  </tal:series_dict>
+  </td>
+  </tr>
+  </table>
+  <tal:multipage condition="batch_nav/has_multiple_pages">
+    <tal:navigation
+        replace="structure batch_nav/@@+navigation-links-lower"/>
+  </tal:multipage>
   </div>
 
   <div class="portlet">