launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/sync-greyedout-resolved-841934 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #841934 in Launchpad itself: "Sync checkbox is not greyed out for resolved/identical  versions"
  https://bugs.launchpad.net/launchpad/+bug/841934

For more details, see:
https://code.launchpad.net/~rvb/launchpad/sync-greyedout-resolved-841934/+merge/74975

This branch fixes canRequestSync so that it's not possible to sync resolved differences.  I choose to add an explicit check inside canRequestSync (as opposed to fixing isNewerThanParent) because checking the status is quicker than calling apt_pkg.VersionCompare.

Drive-by fix: It also removes an additional space that was present on the derivation portlet.

= Tests =

./bin/test -vvc test_distroseries test_canRequestSync_returns_False_if_DSD_is_resolved

= QA =

The resolved DSDs on this page should not be 'syncable' (the checkboxes should be disabled).
https://dogfood.launchpad.net/ubuntu/oneiric/+localpackagediffs?field.name_filter=&field.package_type=resolved&field.package_type-empty-marker=1

-- 
https://code.launchpad.net/~rvb/launchpad/sync-greyedout-resolved-841934/+merge/74975
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/sync-greyedout-resolved-841934 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-08-18 11:55:52 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-09-12 11:44:25 +0000
@@ -1066,10 +1066,12 @@
 
     def canRequestSync(self, dsd):
         """Does it make sense to request a sync for this difference?"""
-        # There are two conditions for this: it doesn't make sense to
-        # sync if the child's version of the package is newer than the
-        # parent's version, or if there is already a sync pending.
+        # There are three conditions for this: it doesn't make sense to
+        # sync if the dsd is resolved, if the child's version of the package
+        # is newer than the parent's version, or if there is already a sync
+        # pending.
         return (
+            not dsd.status == DistroSeriesDifferenceStatus.RESOLVED and
             not self.isNewerThanParent(dsd) and not self.pendingSync(dsd))
 
     def describeJobs(self, dsd):

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-08-19 06:57:33 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-09-12 11:44:25 +0000
@@ -1919,7 +1919,7 @@
         self.assertFalse(view.isNewerThanParent(dsd))
 
     def test_isNewerThanParent_is_False_for_equivalent_updates(self):
-        # Some non-identical version numbers compare as "equal."  If the
+        # Some non-identical version numbers compare as "equal".  If the
         # child and parent versions compare as equal, the child version
         # is not considered newer.
         dsd = self.factory.makeDistroSeriesDifference(
@@ -1962,6 +1962,14 @@
         view.hasPendingDSDUpdate = FakeMethod(result=True)
         self.assertTrue(view.canRequestSync(dsd))
 
+    def test_canRequestSync_returns_False_if_DSD_is_resolved(self):
+        dsd = self.factory.makeDistroSeriesDifference(
+            versions=dict(base='1.0', parent='1.1', derived='1.1'),
+            status=DistroSeriesDifferenceStatus.RESOLVED)
+        view = create_initialized_view(
+            dsd.derived_series, '+localpackagediffs')
+        self.assertFalse(view.canRequestSync(dsd))
+
     def test_describeJobs_returns_None_if_no_jobs(self):
         dsd = self.factory.makeDistroSeriesDifference()
         view = create_initialized_view(

=== modified file 'lib/lp/registry/templates/distroseries-portlet-derivation.pt'
--- lib/lp/registry/templates/distroseries-portlet-derivation.pt	2011-09-02 10:57:52 +0000
+++ lib/lp/registry/templates/distroseries-portlet-derivation.pt	2011-09-12 11:44:25 +0000
@@ -26,8 +26,7 @@
               123 packages
             </tal:differences_count>
             <a tal:attributes="href view/link_to_all_version_diffs">
-              with differences
-            </a>
+              with differences</a>
             <tal:needing_attention
                 condition="view/num_version_differences_needing_attention">
               (<tal:differences_count