launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00906
[Merge] lp:~michael.nelson/launchpad/distro-series-difference-browser2 into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/distro-series-difference-browser2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#627295 Add browser code for DistroSeriesDifferences
https://bugs.launchpad.net/bugs/627295
Overview
========
This branch adds the initial UI for displaying differences between a derived series and its parent.
The page:
https://bugs.edge.launchpad.net/soyuz/+bug/627295/+attachment/1553364/+files/distro-series-differences.png
The UI mockup:
https://dev.launchpad.net/LEP/DerivativeDistributions?action=AttachFile&do=get&target=derived-series-diffs_uiround2.png
Details
=======
The view is behind a feature flag, so the url will redirect back to the distro series when the soyuz.derived-series-ui.enabled feature flag is not set.
This branch just does the initial non-js functionality. A following branch will add the remaining non-js functionality (the search bar). I'll also create a bug for the text of the popup help.
To test:
========
bin/test -vv -m test_series_views -m test_distroseriesdifference
To demo locally
===============
Run the following in bin/iharness:
http://pastebin.ubuntu.com/489730/
--
https://code.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/distro-series-difference-browser2 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2010-08-27 04:33:52 +0000
+++ lib/lp/registry/browser/configure.zcml 2010-09-07 11:24:46 +0000
@@ -143,6 +143,12 @@
for="lp.registry.interfaces.distroseries.IDistroSeries"
class="canonical.launchpad.browser.AskAQuestionButtonView"
permission="zope.Public"/>
+ <browser:page
+ name="+localpackagediffs"
+ for="lp.registry.interfaces.distroseries.IDistroSeries"
+ class="lp.registry.browser.distroseries.DistroSeriesLocalDifferences"
+ template="../templates/distroseries-localdifferences.pt"
+ permission="zope.Public"/>
<browser:menus
classes="
DistroSeriesFacets
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-09-03 15:02:39 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-09-07 11:24:46 +0000
@@ -11,6 +11,7 @@
'DistroSeriesBreadcrumb',
'DistroSeriesEditView',
'DistroSeriesFacets',
+ 'DistroSeriesLocalDifferences',
'DistroSeriesPackageSearchView',
'DistroSeriesPackagesView',
'DistroSeriesNavigation',
@@ -70,7 +71,10 @@
StructuralSubscriptionTargetTraversalMixin,
)
from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.distroseriesdifference import (
+ IDistroSeriesDifferenceSource)
from lp.registry.interfaces.series import SeriesStatus
+from lp.services.features.flags import FeatureController
from lp.services.propertycache import cachedproperty
from lp.services.worlddata.interfaces.country import ICountry
from lp.services.worlddata.interfaces.language import ILanguageSet
@@ -525,3 +529,37 @@
navigator = BatchNavigator(packages, self.request, size=20)
navigator.setHeadings('package', 'packages')
return navigator
+
+
+class DistroSeriesLocalDifferences(LaunchpadView):
+ """Present differences betteen a derived series and its parent."""
+
+ page_title = 'Local package differences'
+
+ def initialize(self):
+ """Redirect to the derived series if the feature is not enabled."""
+ def in_scope(value):
+ return True
+
+ feature_controller = FeatureController(in_scope)
+ if feature_controller.getFlag('soyuz.derived-series-ui.enabled') != 'on':
+ self.request.response.redirect(
+ canonical_url(self.context))
+ return
+ super(DistroSeriesLocalDifferences, self).initialize()
+
+ @property
+ def label(self):
+ return (
+ "Source package differences between '%s' and "
+ "parent series '%s'" % (
+ self.context.displayname,
+ self.context.parent_series.displayname,
+ ))
+
+ @cachedproperty
+ def cached_differences(self):
+ """Return a batch navigator of potentially filtered results."""
+ differences = getUtility(IDistroSeriesDifferenceSource).getForDistroSeries(
+ self.context)
+ return BatchNavigator(differences, self.request)
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2010-08-31 15:14:01 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2010-09-07 11:24:46 +0000
@@ -3,14 +3,23 @@
__metaclass__ = type
+from BeautifulSoup import BeautifulSoup
from storm.zope.interfaces import IResultSet
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+import unittest
+
from canonical.config import config
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.webapp.batching import BatchNavigator
+from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing import LaunchpadZopelessLayer
+from lp.registry.enum import (
+ DistroSeriesDifferenceStatus,
+ DistroSeriesDifferenceType,
+ )
+from lp.services.features.model import FeatureFlag, getFeatureStore
from lp.testing import TestCaseWithFactory
from lp.testing.views import create_initialized_view
@@ -44,6 +53,123 @@
self.assertEqual(view.needs_linking, None)
+class DistroSeriesLocalPackageDiffsTestCase(TestCaseWithFactory):
+ """Test the distroseries +localpackagediffs view."""
+
+ layer = LaunchpadZopelessLayer
+
+ def makeDerivedSeries(self, derived_name=None, parent_name=None):
+ # Helper that creates a derived distro series.
+ parent = self.factory.makeDistroSeries(name=parent_name)
+ derived_series = self.factory.makeDistroSeries(
+ name=derived_name, parent_series=parent)
+ return derived_series
+
+ def setDerivedSeriesUIFeatureFlag(self):
+ # Helper to set the feature flag enabling the derived series ui.
+ ignore = getFeatureStore().add(FeatureFlag(
+ scope=u'default', flag=u'soyuz.derived-series-ui.enabled',
+ value=u'on', priority=1))
+
+ def test_view_redirects_without_feature_flag(self):
+ # If the feature flag soyuz.derived-series-ui.enabled is not set the
+ # view simply redirects to the derived series.
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+
+ # self.assertIs(
+ # None, getFeatureFlag('soyuz.derived-series-ui.enabled'))
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ response = view.request.response
+ self.assertEqual(302, response.getStatus())
+ self.assertEqual(
+ canonical_url(derived_series), response.getHeader('location'))
+
+ def test_label(self):
+ # The view label includes the names of both series.
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ self.assertEqual(
+ "Source package differences between 'Derilucid' and "
+ "parent series 'Lucid'",
+ view.label)
+
+ def test_batch_includes_needing_attention_only(self):
+ # The differences attribute includes differences needing
+ # attention only.
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+ current_difference = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series)
+ old_difference = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ status=DistroSeriesDifferenceStatus.RESOLVED)
+
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ self.assertContentEqual(
+ [current_difference], view.cached_differences.batch)
+
+ def test_batch_includes_different_versions_only(self):
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+ different_versions_diff = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series)
+ unique_diff = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series,
+ difference_type=(
+ DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES))
+
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ self.assertContentEqual(
+ [different_versions_diff], view.cached_differences.batch)
+
+ def test_template_includes_help_link(self):
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+
+ self.setDerivedSeriesUIFeatureFlag()
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ soup = BeautifulSoup(view())
+ help_links = soup.findAll(
+ 'a', href='/+help/soyuz/derived-series-syncing.html')
+ self.assertEqual(1, len(help_links))
+
+ def test_diff_row_includes_last_comment_only(self):
+ # The most recent comment is rendered for each difference.
+ derived_series = self.makeDerivedSeries(
+ parent_name='lucid', derived_name='derilucid')
+ difference = self.factory.makeDistroSeriesDifference(
+ derived_series=derived_series)
+ difference.addComment(difference.owner, "Earlier comment")
+ difference.addComment(difference.owner, "Latest comment")
+
+ self.setDerivedSeriesUIFeatureFlag()
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ # Find all the rows within the body of the table
+ # listing the differences.
+ soup = BeautifulSoup(view())
+ diff_table = soup.find('table', {'class': 'listing'})
+ rows = diff_table.tbody.findAll('tr')
+
+ self.assertEqual(1, len(rows))
+ self.assertIn("Latest comment", unicode(rows[0]))
+ self.assertNotIn("Earlier comment", unicode(rows[0]))
+
+
class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):
"""Test the series.milestone_batch_navigator attribute."""
@@ -61,6 +187,7 @@
product = self.factory.makeProduct()
for name in ('a', 'b', 'c', 'd'):
product.development_focus.newMilestone(name)
+
view = create_initialized_view(
product.development_focus, name='+index')
self._check_milestone_batch_navigator(view)
@@ -84,3 +211,7 @@
for item in view.milestone_batch_navigator.currentBatch()]
self.assertEqual(expected, milestone_names)
config.pop('default-batch-size')
+
+
+def test_suite():
+ return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-01 12:23:01 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-07 11:24:46 +0000
@@ -10,6 +10,7 @@
]
from lazr.enum import DBItem
+from storm.expr import Desc
from storm.locals import (
Int,
Reference,
@@ -200,4 +201,4 @@
comments = IStore(DSDComment).find(
DistroSeriesDifferenceComment,
DSDComment.distro_series_difference == self)
- return comments.order_by(DSDComment.id)
+ return comments.order_by(Desc(DSDComment.id))
=== added file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-07 11:24:46 +0000
@@ -0,0 +1,79 @@
+<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>
+ <tal:heading metal:fill-slot="heading">
+ <h1 tal:content="view/label">Package differences between ...</h1>
+ </tal:heading>
+
+ <div class="top-portlet" metal:fill-slot="main"
+ tal:define="differences view/cached_differences;
+ series_name context/displayname;
+ parent_name context/parent_series/displayname;">
+ <p>Source packages shown here are present in both
+ <tal:replace replace="series_name">Derilucid</tal:replace>
+ and the parent series,
+ <tal:replace replace="context/parent_series/fullseriesname">Ubuntu Lucid
+ </tal:replace>, but are different somehow. Changes could be in
+ either or both series so check the versions (and the diff if
+ necessary) before syncing the
+ <tal:replace replace="parent_name">Lucid
+ </tal:replace> version
+ (<a href="/+help/soyuz/derived-series-syncing.html" target="help">Read
+ more about syncing from the parent series</a>).
+ </p>
+
+ <div tal:condition="differences/batch">
+ <tal:navigation_top
+ replace="structure differences/@@+navigation-links-upper" />
+ <table class="listing">
+ <thead>
+ <tr>
+ <th><tal:replace replace="parent_name" /> package</th>
+ <th><tal:replace replace="series_name" /> version</th>
+ <th>Last uploaded by</th>
+ <th>Latest comment</th>
+ </tr>
+ </thead>
+ <tbody>
+ <tal:difference repeat="difference differences/batch">
+ <tr tal:define="parent_source_pub difference/parent_source_pub;
+ source_pub difference/source_pub;
+ signer source_pub/uploader/fmt:link|string:no signer;">
+ <td><a tal:attributes="href parent_source_pub/sourcepackagerelease/fmt:url">
+ <tal:replace
+ replace="parent_source_pub/sourcepackagerelease/title"/></a>
+ </td>
+ <td><a tal:attributes="href source_pub/sourcepackagerelease/fmt:url">
+ <tal:replace
+ replace="source_pub/sourcepackagerelease/version"/></a>
+ </td>
+ <td><a tal:replace="structure signer">Steph Smith</a>
+ <span tal:attributes="title difference/source_pub/datepublished/fmt:datetime"
+ tal:content="difference/source_pub/datepublished/fmt:approximatedate">2005-09-16</span>
+ </td>
+ <td>
+ <tal:comment tal:define="comment python:difference.getComments().first();"
+ tal:condition="comment">
+ <span tal:replace="structure
+ comment/message/owner/name">joesmith</span>:
+ <span tal:replace="comment/comment">I'm on this.</span>
+ (<span
+ tal:replace="comment/message/datecreated/fmt:approximatedate">2005-09-16</span>)
+ </tal:comment>
+ </td>
+ </tr>
+
+ </tal:difference>
+ </tbody>
+ </table>
+ </div>
+
+ </div>
+
+ </body>
+</html>
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-01 12:23:01 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-07 11:24:46 +0000
@@ -254,7 +254,8 @@
self.assertEqual(ds_diff, dsd_comment.distro_series_difference)
def test_getComments(self):
- # All comments for this difference are returned.
+ # All comments for this difference are returned with the
+ # most recent comment first.
ds_diff = self.factory.makeDistroSeriesDifference()
with person_logged_in(ds_diff.owner):
@@ -264,7 +265,7 @@
ds_diff.owner, "Wait until version 2.1")
self.assertEqual(
- [dsd_comment, dsd_comment_2], list(ds_diff.getComments()))
+ [dsd_comment_2, dsd_comment], list(ds_diff.getComments()))
def test_addComment_not_public(self):
# Comments cannot be added with launchpad.View.
=== added file 'lib/lp/soyuz/help/derived-series-syncing.html'
--- lib/lp/soyuz/help/derived-series-syncing.html 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/help/derived-series-syncing.html 2010-09-07 11:24:46 +0000
@@ -0,0 +1,46 @@
+<html>
+ <head>
+ <title>Syncing software from a parent series</title>
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssreset/reset.css" />
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssfonts/fonts.css" />
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssbase/base.css" />
+ </head>
+ <body>
+ <h1>Syncing software from a parent series</h1>
+
+ <p>
+ A nice introduction text written by the great
+ </p>
+
+ <p>
+ <strong>Important:</strong> Syncing a package from the parent
+ series will overwrite ....
+ </p>
+
+ <h2>Checking the differences</h2>
+
+ <p>
+ Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
+ eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim
+ ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
+ aliquip ex ea commodo consequat. Duis aute irure dolor in
+ reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
+ pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
+ culpa qui officia deserunt mollit anim id est laborum.
+ </p>
+
+ <p>
+ <strong>Step 1:</strong>
+ Sed ut perspiciatis unde omnis iste natus error sit voluptatem
+ accusantium doloremque laudantium, totam rem aperiam, eaque ipsa
+ quae ab illo inventore veritatis et quasi architecto beatae vitae
+ dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit
+ aspernatur aut odit aut fugit, sed quia consequuntur magni dolores
+ eos qui ratione voluptatem sequi nesciunt.
+ </p>
+
+ </body>
+</html>
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-03 16:43:11 +0000
+++ lib/lp/testing/factory.py 2010-09-07 11:24:46 +0000
@@ -1873,7 +1873,8 @@
source_pub = self.makeSourcePackagePublishingHistory(
distroseries=derived_series,
version=versions.get('derived'),
- sourcepackagename=source_package_name)
+ sourcepackagename=source_package_name,
+ status = PackagePublishingStatus.PUBLISHED)
if difference_type is not (
DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES):
@@ -1881,7 +1882,8 @@
source_pub = self.makeSourcePackagePublishingHistory(
distroseries=derived_series.parent_series,
version=versions.get('parent'),
- sourcepackagename=source_package_name)
+ sourcepackagename=source_package_name,
+ status = PackagePublishingStatus.PUBLISHED)
return getUtility(IDistroSeriesDifferenceSource).new(
derived_series, source_package_name, difference_type,
Follow ups
