← Back to team overview

launchpad-reviewers team mailing list archive

lp:~michael.nelson/launchpad/distro-series-difference-browser2-for-devel into lp:launchpad/devel

 

Michael Nelson has proposed merging lp:~michael.nelson/launchpad/distro-series-difference-browser2-for-devel into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This is just landing changes in devel that have already been approved for db-devel (now that, post LP 10.09, db-devel has been merged into devel).

The previous MP is here:
https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
-- 
https://code.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2-for-devel/+merge/34991
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/distro-series-difference-browser2-for-devel into lp:launchpad/devel.
=== 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-09 15:02:49 +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-09 15:02:49 +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,36 @@
         navigator = BatchNavigator(packages, self.request, size=20)
         navigator.setHeadings('package', 'packages')
         return navigator
+
+
+class DistroSeriesLocalDifferences(LaunchpadView):
+    """Present differences between 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-09 15:02:49 +0000
@@ -3,14 +3,24 @@
 
 __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.flags import FeatureController
+from lp.services.features.model import FeatureFlag, getFeatureStore
 from lp.testing import TestCaseWithFactory
 from lp.testing.views import create_initialized_view
 
@@ -44,6 +54,134 @@
         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 getDerivedSeriesUIFeatureFlag(self, flag):
+        """Helper to return the given flag leaving tests more readable."""
+        def in_scope(value):
+            return True
+
+        feature_controller = FeatureController(in_scope)
+        return feature_controller.getFlag(flag)
+
+    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, self.getDerivedSeriesUIFeatureFlag(
+                '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):
+        # The view contains differences of type DIFFERENT_VERSIONS only.
+        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):
+        # The help link for popup help is included.
+        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 +199,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 +223,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-09 15:02:49 +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-09 15:02:49 +0000
@@ -0,0 +1,86 @@
+<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>Source</th>
+              <th><tal:replace replace="parent_name" /> version</th>
+              <th><tal:replace replace="series_name" /> version</th>
+              <th>Last uploaded</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/sourcepackagerelease/uploader/fmt:link|nothing;">
+              <td><span
+                      tal:replace="parent_source_pub/source_package_name">Foo</span>
+              </td>
+              <td><a tal:attributes="href parent_source_pub/sourcepackagerelease/fmt:url">
+                    <tal:replace
+                    replace="parent_source_pub/sourcepackagerelease/version"/></a>
+              </td>
+              <td><a tal:attributes="href source_pub/sourcepackagerelease/fmt:url">
+                    <tal:replace
+                    replace="source_pub/sourcepackagerelease/version"/></a>
+              </td>
+              <td>
+                  <span tal:attributes="title difference/source_pub/datepublished/fmt:datetime"
+                        tal:content="difference/source_pub/datepublished/fmt:approximatedate">2005-09-16</span>
+                  <tal:signer condition="signer">
+                   by <a tal:replace="structure signer">Steph Smith</a>
+                  </tal:signer>
+              </td>
+              <td>
+                <tal:comment tal:define="comment python:difference.getComments().first();"
+                             tal:condition="comment">
+                  <span tal:replace="comment/comment/fmt:shorten/50">I'm on this.</span>
+                  <br /><span class="greyed-out greylink"><span
+                      tal:replace="comment/message/datecreated/fmt:approximatedate">2005-09-16</span>
+                  by <a tal:replace="structure
+                      comment/message/owner/fmt:link">joesmith</a></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-09 15:02:49 +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-09 15:02:49 +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-09 14:11:43 +0000
+++ lib/lp/testing/factory.py	2010-09-09 15:02:49 +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,