launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04011
[Merge] lp:~jtv/launchpad/bug-798521 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-798521 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #798521 in Launchpad itself: "distroseries-queue.pt logic should be in view"
https://bugs.launchpad.net/launchpad/+bug/798521
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-798521/+merge/65297
= Summary =
This cleans up some technical debt related to the +queue page, in penance for my first adding to it in a hurry.
== Implementation details ==
Here's a quick run-through of what I changed (a bit of a grab-bag, but it's all connected):
Added a Storm class for the PackagesetSources linking table, which was previously hacked into the python ad-hoc.
Batch-fetched any SourcePackageReleases associated with the PackageUploads on the page.
Batch-fetched any SourcePackageNames associated with the SourcePackageReleases.
Batch-fetched any Packagesets associated with the SourcePackages.
Extended CompletePackageUpload to be less like a "PackageUpload with some data pre-fetched" and more like a "view class for PackageUploads."
Moved some small-scale TAL logic into CompletePackageUpload (but not so much as to split the <table> structure across TAL and python).
Unit-tested CompletePackageUpload.
Modernized the PackagesetSet unit test, replacing its setUp with specific factory calls.
== Tests ==
{{{
./bin/test -vvc soyuz.browser.tests.test_queue
./bin/test -vvc soyuz.tests.test_packageset
./bin/test -vvc soyuz -t stories
}}}
== Demo and Q/A ==
The +queue page should still work, and be no less efficient. (Hopefully it will be slightly more efficient).
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/browser/tests/test_queue.py
lib/lp/soyuz/templates/distroseries-queue.pt
lib/lp/soyuz/model/packagesetsources.py
lib/lp/soyuz/browser/queue.py
lib/lp/soyuz/interfaces/packageset.py
lib/lp/soyuz/model/packageset.py
lib/lp/soyuz/tests/test_packageset.py
lib/lp/registry/model/distroseriesdifference.py
--
https://code.launchpad.net/~jtv/launchpad/bug-798521/+merge/65297
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-798521 into lp:launchpad.
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-06-14 13:47:51 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-06-21 05:48:36 +0000
@@ -97,6 +97,7 @@
DistroSeriesSourcePackageRelease,
)
from lp.soyuz.model.packageset import Packageset
+from lp.soyuz.model.packagesetsources import PackagesetSources
from lp.soyuz.model.publishing import SourcePackagePublishingHistory
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -205,7 +206,6 @@
if len(dsds) == 0:
return {}
- PackagesetSources = Table("PackageSetSources")
FlatPackagesetInclusion = Table("FlatPackagesetInclusion")
tables = IStore(Packageset).using(
@@ -213,18 +213,17 @@
PackagesetSources, FlatPackagesetInclusion)
results = tables.find(
(DistroSeriesDifference.id, Packageset),
- Column("packageset", PackagesetSources) == (
- Column("child", FlatPackagesetInclusion)),
+ PackagesetSources.packageset == Column(
+ "child", FlatPackagesetInclusion),
Packageset.distroseries_id == (
DistroSeriesDifference.parent_series_id if in_parent else
DistroSeriesDifference.derived_series_id),
Column("parent", FlatPackagesetInclusion) == Packageset.id,
- Column("sourcepackagename", PackagesetSources) == (
+ PackagesetSources.sourcepackagename == (
DistroSeriesDifference.source_package_name_id),
DistroSeriesDifference.id.is_in(dsd.id for dsd in dsds))
results = results.order_by(
- Column("sourcepackagename", PackagesetSources),
- Packageset.name)
+ PackagesetSources.sourcepackagename, Packageset.name)
grouped = defaultdict(list)
for dsd_id, packageset in results:
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2011-06-16 10:44:00 +0000
+++ lib/lp/soyuz/browser/queue.py 2011-06-21 05:48:36 +0000
@@ -9,6 +9,7 @@
'QueueItemsView',
]
+import cgi
import operator
from lazr.delegates import delegates
@@ -22,6 +23,7 @@
NotFoundError,
UnexpectedFormData,
)
+from lp.services.database.bulk import load_related
from lp.soyuz.enums import (
PackagePublishingPriority,
PackageUploadStatus,
@@ -176,6 +178,18 @@
# Listify to avoid repeated queries.
return list(old_binary_packages)
+ def getPackagesetsForSourceFiles(self, source_files):
+ """Get the `Packagesets` belonging to `source_files`.
+
+ :param source_files: A sequence of `SourcePackageReleaseFile`s.
+ """
+ sprs = set(
+ source_file.sourcepackagerelease for source_file in source_files)
+ if None in sprs:
+ sprs.remove(None)
+ return getUtility(IPackagesetSet).getForPackages(
+ self.context, [spr.sourcepackagename_id for spr in sprs])
+
def decoratedQueueBatch(self):
"""Return the current batch, converted to decorated objects.
@@ -183,6 +197,9 @@
CompletePackageUpload. This avoids many additional SQL queries
in the +queue template.
"""
+ # Avoid circular imports.
+ from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+
uploads = list(self.batchnav.currentBatch())
if len(uploads) == 0:
@@ -199,6 +216,9 @@
source_file_set = getUtility(ISourcePackageReleaseFileSet)
source_files = source_file_set.getByPackageUploadIDs(upload_ids)
+ load_related(
+ SourcePackageRelease, source_files, ['sourcepackagerelease_id'])
+
# Get a dictionary of lists of binary files keyed by upload ID.
package_upload_builds_dict = self.builds_dict(
upload_ids, binary_files)
@@ -217,9 +237,11 @@
self.old_binary_packages = self.calculateOldBinaries(
binary_package_names)
+ package_sets = self.getPackagesetsForSourceFiles(source_files)
+
return [
CompletePackageUpload(
- item, build_upload_files, source_upload_files)
+ item, build_upload_files, source_upload_files, package_sets)
for item in uploads]
def is_new(self, binarypackagerelease):
@@ -415,8 +437,9 @@
class CompletePackageUpload:
"""A decorated `PackageUpload` including sources, builds and packages.
- Some properties of PackageUpload are cached here to reduce the number
- of queries that the +queue template has to make.
+ This acts effectively as a view for package uploads. Some properties of
+ the class are cached here to reduce the number of queries that the +queue
+ template has to make. Others are added here exclusively.
"""
# These need to be predeclared to avoid delegates taking them over.
# Would be nice if there was a way of allowing writes to just work
@@ -433,7 +456,7 @@
delegates(IPackageUpload)
def __init__(self, packageupload, build_upload_files,
- source_upload_files):
+ source_upload_files, package_sets):
self.pocket = packageupload.pocket
self.date_created = packageupload.date_created
self.context = packageupload
@@ -459,6 +482,12 @@
else:
self.sourcepackagerelease = None
+ if self.contains_source:
+ self.package_sets = package_sets.get(
+ self.sourcepackagerelease.sourcepackagenameID, [])
+ else:
+ self.package_sets = []
+
@property
def pending_delayed_copy(self):
"""Whether the context is a delayed-copy pending processing."""
@@ -478,11 +507,87 @@
return self.context.changesfile
@property
- def package_sets(self):
- assert self.sourcepackagerelease, \
- "Can only be used on a source upload."
- return ' '.join(sorted(ps.name for ps in
- getUtility(IPackagesetSet).setsIncludingSource(
- self.sourcepackagerelease.sourcepackagename,
- distroseries=self.distroseries,
- direct_inclusion=True)))
+ def display_package_sets(self):
+ """Package sets, if any, for display on the +queue page."""
+ if self.contains_source:
+ return ' '.join(sorted(
+ packageset.name for packageset in self.package_sets))
+ else:
+ return ""
+
+ @property
+ def display_component(self):
+ """Component name, if any, for display on the +queue page."""
+ if self.contains_source:
+ return self.component_name.lower()
+ else:
+ return ""
+
+ @property
+ def display_section(self):
+ """Section name, if any, for display on the +queue page."""
+ if self.contains_source:
+ return self.section_name.lower()
+ else:
+ return ""
+
+ @property
+ def display_priority(self):
+ """Priority name, if any, for display on the +queue page."""
+ if self.contains_source:
+ return self.sourcepackagerelease.urgency.name.lower()
+ else:
+ return ""
+
+ def composeIcon(self, alt, icon, title=None):
+ """Compose an icon for the package's icon list."""
+ # These should really be sprites!
+ if title is None:
+ title = alt
+ return '<img alt="[%s]" src="/@@/%s" title="%s" />' % (
+ cgi.escape(alt, quote=True),
+ icon,
+ cgi.escape(title, quote=True),
+ )
+
+ def composeIconList(self):
+ """List icons that should be shown for this upload."""
+ ddtp = "Debian Description Translation Project Indexes"
+ potential_icons = [
+ (self.contains_source, ("Source", 'package-source')),
+ (self.contains_build, ("Build", 'package-binary', "Binary")),
+ (self.contains_translation, ("Translation", 'translation-file')),
+ (self.contains_installer, ("Installer", 'ubuntu-icon')),
+ (self.contains_upgrader, ("Upgrader", 'ubuntu-icon')),
+ (self.contains_ddtp, (ddtp, 'ubuntu-icon')),
+ ]
+ return [
+ self.composeIcon(*details)
+ for condition, details in potential_icons
+ if condition]
+
+ def composeNameAndChangesLink(self):
+ """Compose HTML: upload name and link to changes file."""
+ raw_displayname = self.displayname
+ displayname = cgi.escape(raw_displayname)
+ if self.pending_delayed_copy or self.changesfile is None:
+ return displayname
+ else:
+ return '<a href="%s" title="Changes file for %s">%s</a>' % (
+ self.changesfile.http_url,
+ cgi.escape(self.displayname, quote=True),
+ displayname)
+
+ @property
+ def icons_and_name(self):
+ """Icon list and name, linked to changes file if appropriate."""
+ iconlist_id = "queue%d-iconlist" % self.id
+ icons = self.composeIconList()
+ link = self.composeNameAndChangesLink()
+ return """
+ <div id="%s">
+ %s
+ %s
+ (%s)
+ </div>
+ """ % (iconlist_id, '\n'.join(icons), link, self.displayarchs)
=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py 2011-06-16 13:21:14 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py 2011-06-21 05:48:36 +0000
@@ -5,6 +5,8 @@
__metaclass__ = type
+import cgi
+from lxml import html
import transaction
from zope.component import (
getUtility,
@@ -12,8 +14,12 @@
)
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.testing.layers import LaunchpadFunctionalLayer
+from canonical.testing.layers import (
+ LaunchpadFunctionalLayer,
+ LaunchpadZopelessLayer,
+ )
from lp.archiveuploader.tests import datadir
+from lp.soyuz.browser.queue import CompletePackageUpload
from lp.soyuz.enums import PackageUploadStatus
from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
from lp.soyuz.interfaces.queue import IPackageUploadSet
@@ -208,8 +214,8 @@
upload.distroseries.main_archive)
with person_logged_in(queue_admin):
view = self.makeView(upload.distroseries, queue_admin)
- html = view()
- self.assertIn(upload.package_name, html)
+ html_text = view()
+ self.assertIn(upload.package_name, html_text)
def test_view_renders_build_upload(self):
login(ADMIN_EMAIL)
@@ -218,8 +224,8 @@
upload.distroseries.main_archive)
with person_logged_in(queue_admin):
view = self.makeView(upload.distroseries, queue_admin)
- html = view()
- self.assertIn(upload.package_name, html)
+ html_text = view()
+ self.assertIn(upload.package_name, html_text)
def test_view_renders_copy_upload(self):
login(ADMIN_EMAIL)
@@ -228,5 +234,166 @@
upload.distroseries.main_archive)
with person_logged_in(queue_admin):
view = self.makeView(upload.distroseries, queue_admin)
- html = view()
- self.assertIn(upload.package_name, html)
+ html_text = view()
+ self.assertIn(upload.package_name, html_text)
+
+
+class TestCompletePackageUpload(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def makeCompletePackageUpload(self, upload=None, build_upload_files=None,
+ source_upload_files=None,
+ package_sets=None):
+ if upload is None:
+ upload = self.factory.makeSourcePackageUpload()
+ if build_upload_files is None:
+ build_upload_files = {}
+ if source_upload_files is None:
+ source_upload_files = {}
+ if package_sets is None:
+ package_sets = {}
+ return CompletePackageUpload(
+ upload, build_upload_files, source_upload_files, package_sets)
+
+ def mapPackageSets(self, upload, package_sets=None):
+ if package_sets is None:
+ package_sets = [self.factory.makePackageset(
+ distroseries=upload.distroseries)]
+ spn = upload.sourcepackagerelease.sourcepackagename
+ return {spn.id: package_sets}
+
+ def test_display_package_sets_returns_source_upload_packagesets(self):
+ upload = self.factory.makeSourcePackageUpload()
+ package_sets = self.mapPackageSets(upload)
+ complete_upload = self.makeCompletePackageUpload(
+ upload, package_sets=package_sets)
+ self.assertEqual(
+ package_sets.values()[0][0].name,
+ complete_upload.display_package_sets)
+
+ def test_display_package_sets_returns_empty_for_non_source_upload(self):
+ upload = self.factory.makeBuildPackageUpload()
+ complete_upload = self.makeCompletePackageUpload(
+ upload, package_sets=self.mapPackageSets(upload))
+ self.assertEqual("", complete_upload.display_package_sets)
+
+ def test_display_package_sets_sorts_by_name(self):
+ complete_upload = self.makeCompletePackageUpload()
+ distroseries = complete_upload.distroseries
+ complete_upload.package_sets = [
+ self.factory.makePackageset(distroseries=distroseries, name=name)
+ for name in [u'ccc', u'aaa', u'bbb']]
+ self.assertEqual("aaa bbb ccc", complete_upload.display_package_sets)
+
+ def test_display_component_returns_source_upload_component_name(self):
+ complete_upload = self.makeCompletePackageUpload()
+ self.assertEqual(
+ complete_upload.sourcepackagerelease.component.name.lower(),
+ complete_upload.display_component)
+
+ def test_display_component_returns_empty_for_non_source_upload(self):
+ complete_upload = self.makeCompletePackageUpload(
+ self.factory.makeBuildPackageUpload())
+ self.assertEqual('', complete_upload.display_component)
+
+ def test_display_section_returns_source_upload_section_name(self):
+ complete_upload = self.makeCompletePackageUpload()
+ self.assertEqual(
+ complete_upload.sourcepackagerelease.section.name.lower(),
+ complete_upload.display_section)
+
+ def test_display_section_returns_empty_for_non_source_upload(self):
+ complete_upload = self.makeCompletePackageUpload(
+ self.factory.makeBuildPackageUpload())
+ self.assertEqual('', complete_upload.display_section)
+
+ def test_display_priority_returns_source_upload_priority(self):
+ complete_upload = self.makeCompletePackageUpload()
+ self.assertEqual(
+ complete_upload.sourcepackagerelease.urgency.name.lower(),
+ complete_upload.display_priority)
+
+ def test_display_priority_returns_empty_for_non_source_upload(self):
+ complete_upload = self.makeCompletePackageUpload(
+ self.factory.makeBuildPackageUpload())
+ self.assertEqual('', complete_upload.display_priority)
+
+ def test_composeIcon_produces_image_tag(self):
+ alt = self.factory.getUniqueString()
+ icon = self.factory.getUniqueString() + ".png"
+ title = self.factory.getUniqueString()
+ html_text = self.makeCompletePackageUpload().composeIcon(
+ alt, icon, title)
+ img = html.fromstring(html_text)
+ self.assertEqual("img", img.tag)
+ self.assertEqual("[%s]" % alt, img.get("alt"))
+ self.assertEqual("/@@/" + icon, img.get("src"))
+ self.assertEqual(title, img.get("title"))
+
+ def test_composeIcon_title_defaults_to_alt_text(self):
+ alt = self.factory.getUniqueString()
+ icon = self.factory.getUniqueString() + ".png"
+ html_text = self.makeCompletePackageUpload().composeIcon(alt, icon)
+ img = html.fromstring(html_text)
+ self.assertEqual(alt, img.get("title"))
+
+ def test_composeIcon_escapes_alt_and_title(self):
+ alt = 'alt"&'
+ icon = self.factory.getUniqueString() + ".png"
+ title = 'title"&'
+ html_text = self.makeCompletePackageUpload().composeIcon(
+ alt, icon, title)
+ img = html.fromstring(html_text)
+ self.assertEqual("[%s]" % alt, img.get("alt"))
+ self.assertEqual(title, img.get("title"))
+
+ def test_composeIconList_produces_icons(self):
+ icons = self.makeCompletePackageUpload().composeIconList()
+ self.assertNotEqual([], icons)
+ self.assertEqual('img', html.fromstring(icons[0]).tag)
+
+ def test_composeIconList_produces_icons_conditionally(self):
+ complete_upload = self.makeCompletePackageUpload()
+ base_count = len(complete_upload.composeIconList())
+ complete_upload.contains_build = True
+ new_count = len(complete_upload.composeIconList())
+ self.assertEqual(base_count + 1, new_count)
+
+ def test_composeNameAndChangesLink_does_not_link_if_no_changes_file(self):
+ upload = self.factory.makeCopyJobPackageUpload()
+ complete_upload = self.makeCompletePackageUpload(upload)
+ self.assertEqual(
+ complete_upload.displayname,
+ complete_upload.composeNameAndChangesLink())
+
+ def test_composeNameAndChangesLink_links_to_changes_file(self):
+ complete_upload = self.makeCompletePackageUpload()
+ link = html.fromstring(complete_upload.composeNameAndChangesLink())
+ self.assertEqual(
+ complete_upload.changesfile.http_url, link.get("href"))
+
+ def test_composeNameAndChangesLink_escapes_nonlinked_display_name(self):
+ filename = 'name"&name'
+ upload = self.factory.makeCustomPackageUpload(filename=filename)
+ # Stop nameAndChangesLink from producing a link.
+ upload.changesfile = None
+ complete_upload = self.makeCompletePackageUpload(upload)
+ self.assertIn(
+ cgi.escape(filename), complete_upload.composeNameAndChangesLink())
+
+ def test_composeNameAndChangesLink_escapes_name_in_link(self):
+ filename = 'name"&name'
+ upload = self.factory.makeCustomPackageUpload(filename=filename)
+ complete_upload = self.makeCompletePackageUpload(upload)
+ link = html.fromstring(complete_upload.composeNameAndChangesLink())
+ self.assertIn(filename, link.get("title"))
+ self.assertEqual(filename, link.text)
+
+ def test_icons_and_name_composes_icons_and_link_and_archs(self):
+ complete_upload = self.makeCompletePackageUpload()
+ icons_and_name = html.fromstring(complete_upload.icons_and_name)
+ self.assertNotEqual(None, icons_and_name.find("img"))
+ self.assertNotEqual(None, icons_and_name.find("a"))
+ self.assertIn(
+ complete_upload.displayarchs, ' '.join(icons_and_name.itertext()))
=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py 2011-06-16 18:51:45 +0000
+++ lib/lp/soyuz/interfaces/packageset.py 2011-06-21 05:48:36 +0000
@@ -445,6 +445,17 @@
:return: An iterable collection of `IPackageset` instances.
"""
+ def getForPackages(distroseries, sourcepackagename_ids):
+ """Get `Packagesets` that directly contain the given packages.
+
+ :param distroseries: `DistroSeries` to look in. Only packagesets for
+ this series will be returned.
+ :param sourcepackagename_ids: A sequence of `SourcePackageName` ids.
+ Only packagesets for these package names will be returned.
+ :return: A dict mapping `SourcePackageName` ids to lists of their
+ respective packagesets, in no particular order.
+ """
+
@operation_parameters(
sourcepackagename=TextLine(
title=_('Source package name'), required=True),
=== modified file 'lib/lp/soyuz/model/packageset.py'
--- lib/lp/soyuz/model/packageset.py 2011-03-22 11:30:13 +0000
+++ lib/lp/soyuz/model/packageset.py 2011-06-21 05:48:36 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -36,6 +36,7 @@
NoSuchPackageSet,
)
from lp.soyuz.model.packagesetgroup import PackagesetGroup
+from lp.soyuz.model.packagesetsources import PackagesetSources
def _order_result_set(result_set):
@@ -427,23 +428,36 @@
source_name = getUtility(ISourcePackageNameSet)[source_name]
return source_name
+ def getForPackages(self, distroseries, sourcepackagename_ids):
+ """See `IPackagesetSet`."""
+ tuples = IStore(Packageset).find(
+ (PackagesetSources.sourcepackagename_id, Packageset),
+ Packageset.id == PackagesetSources.packageset_id,
+ Packageset.distroseries == distroseries,
+ PackagesetSources.sourcepackagename_id.is_in(
+ sourcepackagename_ids))
+ packagesets_by_package = {}
+ for package, packageset in tuples:
+ packagesets_by_package.setdefault(package, []).append(packageset)
+ return packagesets_by_package
+
def setsIncludingSource(self, sourcepackagename, distroseries=None,
direct_inclusion=False):
"""See `IPackagesetSet`."""
sourcepackagename = self._nameToSourcePackageName(sourcepackagename)
- if direct_inclusion == False:
+ if direct_inclusion:
+ query = '''
+ SELECT pss.packageset FROM packagesetsources pss
+ WHERE pss.sourcepackagename = ?
+ '''
+ else:
query = '''
SELECT fpsi.parent
FROM packagesetsources pss, flatpackagesetinclusion fpsi
WHERE pss.sourcepackagename = ?
AND pss.packageset = fpsi.child
'''
- else:
- query = '''
- SELECT pss.packageset FROM packagesetsources pss
- WHERE pss.sourcepackagename = ?
- '''
store = IStore(Packageset)
psets = SQL(query, (sourcepackagename.id,))
clauses = [Packageset.id.is_in(psets)]
=== added file 'lib/lp/soyuz/model/packagesetsources.py'
--- lib/lp/soyuz/model/packagesetsources.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/model/packagesetsources.py 2011-06-21 05:48:36 +0000
@@ -0,0 +1,40 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""The `PackagesetSources` linking table.
+
+This table associates `Packageset`s with `SourcePackageName`s.
+"""
+
+__metaclass__ = type
+__all__ = [
+ 'PackagesetSources',
+ ]
+
+from storm.locals import (
+ Int,
+ Reference,
+ Storm,
+ )
+
+
+class PackagesetSources(Storm):
+ """Linking table: which packages are in a package set?"""
+ # This table is largely managed from Packageset, but also directly
+ # accessed from other places.
+
+ __storm_table__ = 'PackagesetSources'
+
+ # There's a vestigial id as well, a holdover from the SQLObject
+ # days. Nobody seems to use it. The only key that matters is
+ # (packageset, sourcepackagename).
+ __storm_primary__ = (
+ 'packageset_id',
+ 'sourcepackagename_id',
+ )
+
+ packageset_id = Int(name='packageset')
+ packageset = Reference(packageset_id, 'Packageset.id')
+ sourcepackagename_id = Int(name='sourcepackagename')
+ sourcepackagename = Reference(
+ sourcepackagename_id, 'SourcePackageName.id')
=== modified file 'lib/lp/soyuz/templates/distroseries-queue.pt'
--- lib/lp/soyuz/templates/distroseries-queue.pt 2011-06-16 13:21:14 +0000
+++ lib/lp/soyuz/templates/distroseries-queue.pt 2011-06-21 05:48:36 +0000
@@ -89,33 +89,30 @@
<input type="checkbox" name="QUEUE_ID"
tal:attributes="value packageupload/id"/>
</td>
- <td style="padding-top: 5px">
- <metal:iconlist use-macro="template/macros/package-iconlist"/>
+ <td
+ style="padding-top: 5px"
+ tal:content="structure packageupload/icons_and_name">
</td>
<td style="padding-top: 5px"
tal:content="packageupload/displayversion">2.0.17-6
</td>
<tal:is_source define="is_source packageupload/contains_source">
- <td style="padding-top: 5px">
- <tal:component condition="is_source"
- content="packageupload/sourcepackagerelease/component/name/lower">
- </tal:component>
- </td>
- <td style="padding-top: 5px">
- <tal:section condition="is_source"
- content="packageupload/sourcepackagerelease/section/name/lower">
- </tal:section>
- </td>
- <td style="padding-top: 5px">
- <tal:priority condition="is_source"
- content="packageupload/sourcepackagerelease/urgency/name/lower">
- </tal:priority>
- </td>
- <td style="padding-top: 5px">
- <tal:packagesets condition="is_source"
- content="packageupload/package_sets">
- </tal:packagesets>
+ <td
+ style="padding-top: 5px"
+ tal:content="packageupload/display_component">
+ </td>
+ <td
+ style="padding-top: 5px"
+ tal:content="packageupload/display_section">
+ </td>
+ <td
+ style="padding-top: 5px"
+ tal:content="packageupload/display_priority">
+ </td>
+ <td
+ style="padding-top: 5px"
+ tal:content="packageupload/display_package_sets">
</td>
</tal:is_source>
<td style="padding-top: 5px"
@@ -232,7 +229,9 @@
style="display:none">
<td/>
<td tal:condition="view/availableActions"/>
- <td tal:define="libraryfilealias file/libraryfile">
+ <td
+ tal:define="libraryfilealias file/libraryfile"
+ tal:condition="libraryfilealias">
<metal:file use-macro="template/macros/package-file"/>
</td>
<td colspan="6"/>
@@ -298,59 +297,15 @@
:libraryfilealias: A LibraryFileAlias to link to. If it is expired,
no link will be created.
</tal:comment>
- <tal:not-none condition="libraryfilealias">
- <tal:unexpired tal:condition="libraryfilealias/content">
- <a tal:attributes="href libraryfilealias/http_url">
- <tal:filename replace="libraryfilealias/filename"/>
- </a>
- (<span tal:replace="libraryfilealias/content/filesize/fmt:bytes" />)
- </tal:unexpired>
- <tal:expired tal:condition="not:libraryfilealias/content">
- <span tal:content="libraryfilealias/filename"/>
- </tal:expired>
- </tal:not-none>
-</metal:macro>
-
-<metal:macro define-macro="package-iconlist">
- <tal:comment replace="nothing">
- This macro expects the following variables defined:
- :packageupload: A PackageUpload record for which we display icons.
- </tal:comment>
-
- <div tal:attributes="id string:queue${packageupload/id}-iconlist">
- <img tal:condition="packageupload/contains_source"
- alt="[Source]" src="/@@/package-source" title="Source"/>
- <img tal:condition="packageupload/contains_build"
- alt="[Build]" src="/@@/package-binary" title="Binary"/>
- <img tal:condition="packageupload/contains_translation"
- alt="[Translation]" src="/@@/translation-file"
- title="Translation"/>
- <img tal:condition="packageupload/contains_installer"
- alt="[Installer]" src="/@@/ubuntu-icon"
- title="Installer"/>
- <img tal:condition="packageupload/contains_upgrader"
- alt="[Upgrader]" src="/@@/ubuntu-icon"
- title="Upgrader"/>
- <img tal:condition="packageupload/contains_ddtp"
- alt="[Debian Description Translation Project Indexes]"
- src="/@@/ubuntu-icon"
- title="Debian Description Translation Project Indexes"/>
- <tal:not-delayed condition="not: packageupload/pending_delayed_copy">
- <a tal:condition="packageupload/changesfile"
- tal:content="packageupload/displayname"
- tal:attributes="
- href packageupload/changesfile/http_url;
- title string:Changes file for ${packageupload/displayname};">
- </a>
- <tal:no-changes-file
- condition="not: packageupload/changesfile"
- replace="packageupload/displayname"/>
- </tal:not-delayed>
- <tal:pending_delayed_copy_title
- condition="packageupload/pending_delayed_copy"
- replace="packageupload/displayname" />
- <tal:arches replace="string: (${packageupload/displayarchs})"/>
- </div>
+ <tal:unexpired tal:condition="libraryfilealias/content">
+ <a tal:attributes="href libraryfilealias/http_url">
+ <tal:filename replace="libraryfilealias/filename"/>
+ </a>
+ (<span tal:replace="libraryfilealias/content/filesize/fmt:bytes" />)
+ </tal:unexpired>
+ <tal:expired tal:condition="not:libraryfilealias/content">
+ <span tal:content="libraryfilealias/filename"/>
+ </tal:expired>
</metal:macro>
</metal:macros>
=== modified file 'lib/lp/soyuz/tests/test_packageset.py'
--- lib/lp/soyuz/tests/test_packageset.py 2011-03-22 11:30:13 +0000
+++ lib/lp/soyuz/tests/test_packageset.py 2011-06-21 05:48:36 +0000
@@ -1,11 +1,11 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Packageset features."""
from zope.component import getUtility
-from canonical.testing.layers import LaunchpadZopelessLayer
+from canonical.testing.layers import ZopelessDatabaseLayer
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.series import SeriesStatus
from lp.soyuz.interfaces.packageset import (
@@ -17,128 +17,154 @@
class TestPackagesetSet(TestCaseWithFactory):
- layer = LaunchpadZopelessLayer
-
- def setUp(self):
- """Setup a distribution with multiple distroseries."""
- super(TestPackagesetSet, self).setUp()
- self.distribution = getUtility(IDistributionSet).getByName(
- 'ubuntu')
- self.distroseries_current = self.distribution.currentseries
- self.distroseries_experimental = self.factory.makeDistroRelease(
- distribution = self.distribution, name="experimental",
+ layer = ZopelessDatabaseLayer
+
+ def getUbuntu(self):
+ """Get the Ubuntu `Distribution`."""
+ return getUtility(IDistributionSet).getByName('ubuntu')
+
+ def makeExperimentalSeries(self):
+ """Create an experimental Ubuntu `DistroSeries`."""
+ return self.factory.makeDistroSeries(
+ distribution=self.getUbuntu(), name="experimental",
status=SeriesStatus.EXPERIMENTAL)
- self.person1 = self.factory.makePerson(
- name='hacker', displayname=u'Happy Hacker')
-
- self.packageset_set = getUtility(IPackagesetSet)
-
def test_new_defaults_to_current_distroseries(self):
# If the distroseries is not provided, the current development
# distroseries will be assumed.
- packageset = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1)
-
+ packageset = getUtility(IPackagesetSet).new(
+ self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
+ self.factory.makePerson())
self.failUnlessEqual(
- self.distroseries_current, packageset.distroseries)
+ self.getUbuntu().currentseries, packageset.distroseries)
def test_new_with_specified_distroseries(self):
# A distroseries can be provided when creating a package set.
- packageset = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1,
- distroseries=self.distroseries_experimental)
-
- self.failUnlessEqual(
- self.distroseries_experimental, packageset.distroseries)
+ experimental_series = self.makeExperimentalSeries()
+ packageset = getUtility(IPackagesetSet).new(
+ self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
+ self.factory.makePerson(), distroseries=experimental_series)
+ self.failUnlessEqual(experimental_series, packageset.distroseries)
def test_new_creates_new_packageset_group(self):
# Creating a new packageset should also create a new packageset
# group with the same owner.
- packageset = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1,
- distroseries=self.distroseries_experimental)
-
- self.failUnlessEqual(
- self.person1, packageset.packagesetgroup.owner)
+ owner = self.factory.makePerson()
+ experimental_series = self.makeExperimentalSeries()
+ packageset = getUtility(IPackagesetSet).new(
+ self.factory.getUniqueUnicode(), self.factory.getUniqueUnicode(),
+ owner, distroseries=experimental_series)
+ self.failUnlessEqual(owner, packageset.packagesetgroup.owner)
def test_new_duplicate_name_for_same_distroseries(self):
# Creating a packageset with a duplicate name for the
# given distroseries will fail.
- packageset = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1,
- distroseries=self.distroseries_experimental)
-
- self.failUnlessRaises(
- DuplicatePackagesetName, self.packageset_set.new,
- u'kernel', u'A packageset with a duplicate name', self.person1,
- distroseries=self.distroseries_experimental)
+ distroseries = self.factory.makeDistroSeries()
+ name = self.factory.getUniqueUnicode()
+ self.factory.makePackageset(name, distroseries=distroseries)
+ self.assertRaises(
+ DuplicatePackagesetName, getUtility(IPackagesetSet).new,
+ name, self.factory.getUniqueUnicode(), self.factory.makePerson(),
+ distroseries=distroseries)
def test_new_duplicate_name_for_different_distroseries(self):
# Creating a packageset with a duplicate name but for a different
# series is no problem.
- packageset = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1)
-
- packageset2 = self.packageset_set.new(
- u'kernel', u'A packageset with a duplicate name', self.person1,
- distroseries=self.distroseries_experimental)
- self.assertEqual(packageset.name, packageset2.name)
+ name = self.factory.getUniqueUnicode()
+ packageset1 = self.factory.makePackageset(name)
+ packageset2 = getUtility(IPackagesetSet).new(
+ name, self.factory.getUniqueUnicode(), self.factory.makePerson(),
+ distroseries=self.factory.makeDistroSeries())
+ self.assertEqual(packageset1.name, packageset2.name)
def test_new_related_packageset(self):
# Creating a new package set while specifying a `related_set` should
# have the effect that the former ends up in the same group as the
# latter.
- pset1 = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1)
-
- pset2 = self.packageset_set.new(
- u'kernel', u'A related package set.', self.person1,
- distroseries=self.distroseries_experimental, related_set=pset1)
+ name = self.factory.getUniqueUnicode()
+ pset1 = self.factory.makePackageset(name)
+ pset2 = self.factory.makePackageset(
+ name, distroseries=self.makeExperimentalSeries(),
+ related_set=pset1)
self.assertEqual(pset1.packagesetgroup, pset2.packagesetgroup)
def test_get_by_name_in_current_distroseries(self):
# IPackagesetSet.getByName() will return the package set in the
# current distroseries if the optional `distroseries` parameter is
# omitted.
- pset1 = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1)
- pset2 = self.packageset_set.new(
- u'kernel', u'A related package set.', self.person1,
- distroseries=self.distroseries_experimental, related_set=pset1)
- pset_found = getUtility(IPackagesetSet).getByName('kernel')
- self.assertEqual(pset1, pset_found)
+ name = self.factory.getUniqueUnicode()
+ pset1 = self.factory.makePackageset(name)
+ self.factory.makePackageset(
+ name, distroseries=self.makeExperimentalSeries(),
+ related_set=pset1)
+ self.assertEqual(pset1, getUtility(IPackagesetSet).getByName(name))
def test_get_by_name_in_specified_distroseries(self):
# IPackagesetSet.getByName() will return the package set in the
# specified distroseries.
- pset1 = self.packageset_set.new(
- u'kernel', u'Contains all OS kernel packages', self.person1)
- pset2 = self.packageset_set.new(
- u'kernel', u'A related package set.', self.person1,
- distroseries=self.distroseries_experimental, related_set=pset1)
+ name = self.factory.getUniqueUnicode()
+ experimental_series = self.makeExperimentalSeries()
+ pset1 = self.factory.makePackageset(name)
+ pset2 = self.factory.makePackageset(
+ name, distroseries=experimental_series, related_set=pset1)
pset_found = getUtility(IPackagesetSet).getByName(
- 'kernel', distroseries=self.distroseries_experimental)
+ name, distroseries=experimental_series)
self.assertEqual(pset2, pset_found)
def test_get_by_distroseries(self):
# IPackagesetSet.getBySeries() will return those package sets
# associated with the given distroseries.
- pset1 = self.packageset_set.new(
- u'timmy', u'Timmy Mallett', self.person1)
- pset2 = self.packageset_set.new(
- u'savile', u'Jimmy Savile', self.person1)
- self.packageset_set.new(
- u'hoskins', u'Bob Hoskins', self.person1,
- distroseries=self.distroseries_experimental)
+ package_sets_for_current_ubuntu = [
+ self.factory.makePackageset() for counter in xrange(2)]
+ self.factory.makePackageset(
+ distroseries=self.makeExperimentalSeries())
self.assertContentEqual(
- [pset1, pset2],
- self.packageset_set.getBySeries(self.distroseries_current))
+ package_sets_for_current_ubuntu,
+ getUtility(IPackagesetSet).getBySeries(
+ self.getUbuntu().currentseries))
+
+ def test_getForPackages_returns_packagesets(self):
+ # getForPackages finds package sets for given source package
+ # names in a distroseries, and maps them by
+ # SourcePackageName.id.
+ series = self.factory.makeDistroSeries()
+ packageset = self.factory.makePackageset(distroseries=series)
+ package = self.factory.makeSourcePackageName()
+ packageset.addSources([package.name])
+ self.assertEqual(
+ {package.id: [packageset]},
+ getUtility(IPackagesetSet).getForPackages(series, [package.id]))
+
+ def test_getForPackages_filters_by_distroseries(self):
+ # getForPackages does not return packagesets for different
+ # distroseries.
+ series = self.factory.makeDistroSeries()
+ other_series = self.factory.makeDistroSeries()
+ packageset = self.factory.makePackageset(distroseries=series)
+ package = self.factory.makeSourcePackageName()
+ packageset.addSources([package.name])
+ self.assertEqual(
+ {},
+ getUtility(IPackagesetSet).getForPackages(
+ other_series, [package.id]))
+
+ def test_getForPackages_filters_by_sourcepackagename(self):
+ # getForPackages does not return packagesets for different
+ # source package names.
+ series = self.factory.makeDistroSeries()
+ packageset = self.factory.makePackageset(distroseries=series)
+ package = self.factory.makeSourcePackageName()
+ other_package = self.factory.makeSourcePackageName()
+ packageset.addSources([package.name])
+ self.assertEqual(
+ {},
+ getUtility(IPackagesetSet).getForPackages(
+ series, [other_package.id]))
class TestPackageset(TestCaseWithFactory):
- layer = LaunchpadZopelessLayer
+ layer = ZopelessDatabaseLayer
def setUp(self):
"""Setup a distribution with multiple distroseries."""
@@ -147,10 +173,10 @@
'ubuntu')
self.distroseries_current = self.distribution.currentseries
self.distroseries_experimental = self.factory.makeDistroRelease(
- distribution = self.distribution, name="experimental",
+ distribution=self.distribution, name="experimental",
status=SeriesStatus.EXPERIMENTAL)
self.distroseries_experimental2 = self.factory.makeDistroRelease(
- distribution = self.distribution, name="experimental2",
+ distribution=self.distribution, name="experimental2",
status=SeriesStatus.EXPERIMENTAL)
self.person1 = self.factory.makePerson(
Follow ups
