launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01023
[Merge] lp:~michael.nelson/launchpad/635005-difference-details-1 into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/635005-difference-details-1 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#635005 DistroSeriesDifference detailed dropdown is missing
https://bugs.launchpad.net/bugs/635005
Overview
========
This is the first branch to address bug 635005 - adding a template snippet with details about a DistroSeriesDifference.
It continues the work that began (and landed behind a feature flag) at:
https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
Details
=======
The branch:
* Adds a traversal for a DistroSeriesDifference object, +listing-distroseries-extra, similar to that used on pages like:
https://edge.launchpad.net/~michael.nelson/+archive/pocketsphinx/+packages?field.name_filter=&field.status_filter=&field.series_filter=
for the extra details dropdown.
* The above required a new getter method, IDistroSeriesDifferenceSource.getByDistroSeriesAndName()
* Testing the view required updating a soyuz helper method that creates binaries with summaries so that the summary for the source is displayed.
* Testing the view also required adding a new factory method makePackageDiff().
I've put the template tests in a unit test, I hope that's ok. If you think it's necessary, I can add a story with the next branch that shows the non-js behaviour (ie. this page will display when users click on the first column of the list shown at: http://launchpadlibrarian.net/55200786/627295-ui-tweaks.png)
To demo:
========
You'll need to run the following in a harness:
http://pastebin.ubuntu.com/493653/
and then you can see the raw snippet at:
https://launchpad.dev/ubuntu/hoary/+difference/foo/+listing-distroseries-extra
(in retrospect, that script could be shortened and used the new helpers in the branch that I created since creating the script).
To test:
========
bin/test -vv -m test_distroseriesdifference_views -m test_sourcepackage_views -m test_distroseriesdifference -m test_publishing
--
https://code.launchpad.net/~michael.nelson/launchpad/635005-difference-details-1/+merge/35408
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/635005-difference-details-1 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2010-08-31 10:00:44 +0000
+++ lib/lp/registry/browser/configure.zcml 2010-09-14 14:03:47 +0000
@@ -149,6 +149,17 @@
class="lp.registry.browser.distroseries.DistroSeriesLocalDifferences"
template="../templates/distroseries-localdifferences.pt"
permission="zope.Public"/>
+ <browser:url
+ for="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"
+ path_expression="string:+difference/${difference/source_package_name}"
+ rootsite="mainsite"
+ attribute_to_parent="derived_series"/>
+ <browser:page
+ name="+listing-distroseries-extra"
+ for="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"
+ class="lp.registry.browser.distroseriesdifference.DistroSeriesDifferenceView"
+ template="../templates/distroseriesdifference-listing-extra.pt"
+ permission="zope.Public"/>
<browser:menus
classes="
DistroSeriesFacets
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-09-08 07:53:06 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-09-14 14:03:47 +0000
@@ -147,6 +147,11 @@
def traverse_queue(self, id):
return getUtility(IPackageUploadSet).get(id)
+ @stepthrough('+difference')
+ def traverse_difference(self, name):
+ dsd_source = getUtility(IDistroSeriesDifferenceSource)
+ return dsd_source.getByDistroSeriesAndName(self.context, name)
+
class DistroSeriesBreadcrumb(Breadcrumb):
"""Builds a breadcrumb for an `IDistroSeries`."""
=== added file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py 2010-09-14 14:03:47 +0000
@@ -0,0 +1,29 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Browser views for DistroSeriesDifferences."""
+
+__metaclass__ = type
+__all__ = [
+ 'DistroSeriesDifferenceView',
+ ]
+
+from canonical.launchpad.webapp.publisher import LaunchpadView
+
+
+class DistroSeriesDifferenceView(LaunchpadView):
+
+ @property
+ def summary(self):
+ """Return the summary of the related source package."""
+ source_pub = None
+ if self.context.source_pub is not None:
+ source_pub = self.context.source_pub
+ elif self.context.parent_source_pub is not None:
+ source_pub = self.context.parent_source_pub
+
+ if source_pub is not None:
+ return source_pub.meta_sourcepackage.summary
+ else:
+ return None
+
=== added file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-14 14:03:47 +0000
@@ -0,0 +1,172 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Unit tests for the DistroSeriesDifference views."""
+
+from __future__ import with_statement
+
+__metaclass__ = type
+
+from BeautifulSoup import BeautifulSoup
+from zope.component import getUtility
+
+from canonical.testing import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
+from lp.registry.enum import DistroSeriesDifferenceType
+from lp.registry.interfaces.distroseriesdifference import IDistroSeriesDifferenceSource
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+from lp.testing import (
+ celebrity_logged_in,
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.testing.views import create_initialized_view
+
+
+class DistroSeriesDifferenceTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def addSummaryToDifference(self, distro_series_difference):
+ """Helper that adds binaries with summary info to the source pubs."""
+ distro_series = distro_series_difference.derived_series
+ source_package_name_str = distro_series_difference.source_package_name.name
+ stp = SoyuzTestPublisher()
+
+ if distro_series_difference.difference_type == (
+ DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES):
+ source_pub = distro_series_difference.parent_source_pub
+ else:
+ source_pub = distro_series_difference.source_pub
+
+ stp.makeSourcePackageSummaryData(source_pub)
+ stp.updateDistroSeriesPackageCache(source_pub.distroseries)
+
+ # updateDistroSeriesPackageCache reconnects the db, so the
+ # objects need to be reloaded.
+ dsd_source = getUtility(IDistroSeriesDifferenceSource)
+ ds_diff = dsd_source.getByDistroSeriesAndName(
+ distro_series, source_package_name_str)
+ return ds_diff
+
+ def test_summary_for_source_pub(self):
+ # For packages unique to the derived series (or different
+ # versions) the summary is based on the derived source pub.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+ ds_diff = self.addSummaryToDifference(ds_diff)
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+
+ self.assertIsNot(None, view.summary)
+ self.assertEqual(
+ ds_diff.source_pub.meta_sourcepackage.summary, view.summary)
+
+ def test_summary_for_missing_difference(self):
+ # For packages only in the parent series, the summary is based
+ # on the parent publication.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ difference_type=(
+ DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
+ ds_diff = self.addSummaryToDifference(ds_diff)
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+
+ self.assertIsNot(None, view.summary)
+ self.assertEqual(
+ ds_diff.parent_source_pub.meta_sourcepackage.summary,
+ view.summary)
+
+ def test_summary_no_pubs(self):
+ # If the difference has been resolved by removing packages then
+ # there will not be a summary.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ difference_type=(
+ DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
+ with celebrity_logged_in('admin'):
+ ds_diff.parent_source_pub.status = PackagePublishingStatus.DELETED
+ ds_diff.update()
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+
+ self.assertIs(None, ds_diff.parent_source_pub)
+ self.assertIs(None, ds_diff.source_pub)
+ self.assertIs(None, view.summary)
+
+
+class DistroSeriesDifferenceTemplateTestCase(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def number_of_request_diff_texts(self, html):
+ """Check that the html doesn't include the request diff text."""
+ soup = BeautifulSoup(html)
+ return len(soup.findAll('dd', 'request-derived-diff'))
+
+ def contains_one_link_to_diff(self, html, package_diff):
+ """Return whether the html contains a link to the diff content."""
+ soup = BeautifulSoup(html)
+ return 1 == len(soup.findAll(
+ 'a', href=package_diff.diff_content.http_url))
+
+ def test_both_request_diff_texts_rendered(self):
+ # An unlinked description of a potential diff is displayed when
+ # no diff is present.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+ # Both diffs present simple text repr. of proposed diff.
+ self.assertEqual(2, self.number_of_request_diff_texts(view()))
+
+ def test_source_diff_rendering_diff(self):
+ # A linked description of the diff is displayed when
+ # it is present.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.derived_series.owner):
+ ds_diff.package_diff = self.factory.makePackageDiff()
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+ # The text for the parent diff remains, but the source package
+ # diff is now a link.
+ self.assertEqual(1, self.number_of_request_diff_texts(view()))
+ self.assertTrue(
+ self.contains_one_link_to_diff(view(), ds_diff.package_diff))
+
+ def test_source_diff_rendering_no_source(self):
+ # If there is no source pub for this difference, then we don't
+ # display even the request for a diff.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ difference_type=
+ (DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+ self.assertEqual(1, self.number_of_request_diff_texts(view()))
+
+ def test_parent_source_diff_rendering_diff(self):
+ # A linked description of the diff is displayed when
+ # it is present.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.derived_series.owner):
+ ds_diff.parent_package_diff = self.factory.makePackageDiff()
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+ # The text for the source diff remains, but the parent package
+ # diff is now a link.
+ self.assertEqual(1, self.number_of_request_diff_texts(view()))
+ self.assertTrue(
+ self.contains_one_link_to_diff(
+ view(), ds_diff.parent_package_diff))
+
+ def test_parent_source_diff_rendering_no_source(self):
+ # If there is no source pub for this difference, then we don't
+ # display even the request for a diff.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ difference_type=
+ (DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES))
+
+ view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
+ self.assertEqual(1, self.number_of_request_diff_texts(view()))
=== modified file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py'
--- lib/lp/registry/browser/tests/test_sourcepackage_views.py 2010-09-02 11:34:34 +0000
+++ lib/lp/registry/browser/tests/test_sourcepackage_views.py 2010-09-14 14:03:47 +0000
@@ -61,7 +61,7 @@
def test_get_register_upstream_url_summary(self):
test_publisher = SoyuzTestPublisher()
- test_data = test_publisher.makeSourcePackageWithBinaryPackageRelease()
+ test_data = test_publisher.makeSourcePackageSummaryData()
source_package_name = (
test_data['source_package'].sourcepackagename.name)
distroseries_id = test_data['distroseries'].id
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-09-10 13:29:42 +0000
+++ lib/lp/registry/configure.zcml 2010-09-14 14:03:47 +0000
@@ -114,7 +114,8 @@
<allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferencePublic"/>
<require
permission="launchpad.Edit"
- interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceEdit"/>
+ interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceEdit"
+ set_attributes="package_diff parent_package_diff"/>
</class>
<!-- DistroSeriesDifferenceComment -->
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-10 09:28:45 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-14 14:03:47 +0000
@@ -177,3 +177,13 @@
:type status: `DistroSeriesDifferenceStatus`.
:return: A result set of differences.
"""
+
+ def getByDistroSeriesAndName(distro_series, source_package_name):
+ """Returns a single difference matching the series and name.
+
+ :param distro_series: The derived distribution series which is to be
+ searched for differences.
+ :type distro_series: `IDistroSeries`.
+ :param source_package_name: The name of the package difference.
+ :type source_package_name: unicode.
+ """
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-10 14:44:15 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-14 14:03:47 +0000
@@ -42,6 +42,7 @@
)
from lp.registry.model.distroseriesdifferencecomment import (
DistroSeriesDifferenceComment)
+from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.propertycache import (
cachedproperty,
IPropertyCacheManager,
@@ -126,6 +127,16 @@
DistroSeriesDifference.difference_type == difference_type,
DistroSeriesDifference.status.is_in(status))
+ @staticmethod
+ def getByDistroSeriesAndName(distro_series, source_package_name):
+ """See `IDistroSeriesDifferenceSource`."""
+ return IStore(DistroSeriesDifference).find(
+ DistroSeriesDifference,
+ DistroSeriesDifference.derived_series == distro_series,
+ DistroSeriesDifference.source_package_name == (
+ SourcePackageName.id),
+ SourcePackageName.name == source_package_name).one()
+
@cachedproperty
def source_pub(self):
"""See `IDistroSeriesDifference`."""
=== modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt'
--- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-08-09 23:05:53 +0000
+++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-09-14 14:03:47 +0000
@@ -6,7 +6,7 @@
>>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
>>> test_publisher = SoyuzTestPublisher()
>>> login('admin@xxxxxxxxxxxxx')
- >>> test_data = test_publisher.makeSourcePackageWithBinaryPackageRelease()
+ >>> test_data = test_publisher.makeSourcePackageSummaryData()
>>> test_publisher.updateDistroSeriesPackageCache(
... test_data['distroseries'])
>>> logout()
=== added file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-14 14:03:47 +0000
@@ -0,0 +1,34 @@
+<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";
+ i18n:domain="launchpad">
+ <dl>
+ <dt>Description:</dt>
+ <dd><tal:description replace="view/summary" /></dd>
+ <dt>Last common version:</dt>
+ <dd>Not implemented.</dd>
+ <dt>Package differences:</dt>
+ <tal:source-diff-option condition="context/source_pub">
+ <dd tal:condition="context/package_diff"
+ tal:content="structure context/package_diff/fmt:link">
+ <a>link to a diff</a></dd>
+ <dd tal:condition="not: context/package_diff" class="request-derived-diff">
+ Base version to derived <span
+ tal:replace="context/source_version">1.2.3</span>
+ </dd>
+ </tal:source-diff-option>
+
+ <tal:parent-diff-option condition="context/parent_source_pub">
+ <dd tal:condition="context/parent_package_diff"
+ tal:content="structure context/parent_package_diff/fmt:link">
+ <a>link to a diff</a></dd>
+ <dd tal:condition="not: context/parent_package_diff" class="request-derived-diff">
+ Base version to parent <span
+ tal:replace="context/parent_source_version">1.2.3</span>
+ </dd>
+ </tal:parent-diff-option>
+ </dl>
+ <h2>Comments:</h2>
+
+</tal:root>
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-10 14:44:15 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-14 14:03:47 +0000
@@ -454,6 +454,17 @@
self.assertContentEqual(diffs['normal'] + diffs['ignored'], result)
+ def test_getByDistroSeriesAndName(self):
+ # An individual difference is obtained using the name.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str='fooname')
+
+ dsd_source = getUtility(IDistroSeriesDifferenceSource)
+ result = dsd_source.getByDistroSeriesAndName(
+ ds_diff.derived_series, 'fooname')
+
+ self.assertEqual(ds_diff, result)
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py 2010-08-30 19:06:34 +0000
+++ lib/lp/soyuz/tests/test_publishing.py 2010-09-14 14:03:47 +0000
@@ -494,45 +494,47 @@
return source
- def makeSourcePackageWithBinaryPackageRelease(self):
+ def makeSourcePackageSummaryData(self, source_pub=None):
"""Make test data for SourcePackage.summary.
The distroseries that is returned from this method needs to be
passed into updateDistroseriesPackageCache() so that
SourcePackage.summary can be populated.
"""
- distribution = self.factory.makeDistribution(
- name='youbuntu', displayname='Youbuntu')
- distroseries = self.factory.makeDistroRelease(name='busy',
- distribution=distribution)
- source_package_name = self.factory.makeSourcePackageName(
- name='bonkers')
- source_package = self.factory.makeSourcePackage(
- sourcepackagename=source_package_name,
- distroseries=distroseries)
- component = self.factory.makeComponent('multiverse')
+ if source_pub is None:
+ distribution = self.factory.makeDistribution(
+ name='youbuntu', displayname='Youbuntu')
+ distroseries = self.factory.makeDistroRelease(name='busy',
+ distribution=distribution)
+ source_package_name = self.factory.makeSourcePackageName(
+ name='bonkers')
+ source_package = self.factory.makeSourcePackage(
+ sourcepackagename=source_package_name,
+ distroseries=distroseries)
+ component = self.factory.makeComponent('multiverse')
+ source_pub = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=source_package_name,
+ distroseries=distroseries,
+ component=component)
+
das = self.factory.makeDistroArchSeries(
- distroseries=distroseries)
- spph = self.factory.makeSourcePackagePublishingHistory(
- sourcepackagename=source_package_name,
- distroseries=distroseries,
- component=component)
+ distroseries=source_pub.distroseries)
for name in ('flubber-bin', 'flubber-lib'):
binary_package_name = self.factory.makeBinaryPackageName(name)
build = self.factory.makeBinaryPackageBuild(
- source_package_release=spph.sourcepackagerelease,
+ source_package_release=source_pub.sourcepackagerelease,
archive=self.factory.makeArchive(),
distroarchseries=das)
bpr = self.factory.makeBinaryPackageRelease(
binarypackagename=binary_package_name,
summary='summary for %s' % name,
- build=build, component=component)
+ build=build, component=source_pub.component)
bpph = self.factory.makeBinaryPackagePublishingHistory(
binarypackagerelease=bpr, distroarchseries=das)
return dict(
- distroseries=distroseries,
- source_package=source_package)
+ distroseries=source_pub.distroseries,
+ source_package=source_pub.meta_sourcepackage)
def updateDistroSeriesPackageCache(
self, distroseries, restore_db_connection='launchpad'):
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-13 08:19:39 +0000
+++ lib/lp/testing/factory.py 2010-09-14 14:03:47 +0000
@@ -220,6 +220,7 @@
ArchivePurpose,
BinaryPackageFileType,
BinaryPackageFormat,
+ PackageDiffStatus,
PackagePublishingPriority,
PackagePublishingStatus,
PackageUploadStatus,
@@ -1798,6 +1799,22 @@
expires=expires, restricted=restricted)
return library_file_alias
+ def makePackageDiff(self, from_spr=None, to_spr=None):
+ """Make a completed package diff."""
+ if from_spr is None:
+ from_spr = self.makeSourcePackageRelease()
+ if to_spr is None:
+ to_spr = self.makeSourcePackageRelease()
+
+ diff = from_spr.requestDiffTo(
+ from_spr.creator, to_spr)
+
+ naked_diff = removeSecurityProxy(diff)
+ naked_diff.status = PackageDiffStatus.COMPLETED
+ naked_diff.diff_content = self.makeLibraryFileAlias()
+ naked_diff.date_fulfilled = UTC_NOW
+ return diff
+
def makeDistribution(self, name=None, displayname=None, owner=None,
members=None, title=None, aliases=None):
"""Make a new distribution."""
Follow ups
