launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-818032 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #818032 in Launchpad itself: "Remove the "Upgrade Packages" button"
  https://bugs.launchpad.net/launchpad/+bug/818032

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-818032/+merge/69791

= Summary =

Disable the Upgrade Packages button for now.  It's too dangerous and, in a few nasty corner cases, does not do what we want.


== Proposed fix ==

The mass-upgrade is behind the same feature flag as package sync.  The flag name mentions "sync" but not "upgrade."  Give the mass upgrade its own feature flag, which will be disabled by default.


== Pre-implementation notes ==

We may still want to enable the button for derived distributions later; for now, we can't have it in Ubuntu.  Removing the code (as I originally wanted to do) would cut pretty deep, and dispose of things we hope the able to use again.


== Tests ==

{{{
./bin/test -vvc lp.registry.browser.tests.test_distroseries
}}}


== Demo and Q/A ==

The DistroSeries:+localpackagediffs page will no longer have the Upgrade Packages button, even if you are privileged and do see the Sync button.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/tests/test_distroseries.py
  lib/lp/services/features/flags.py
  lib/lp/registry/browser/distroseries.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-818032/+merge/69791
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-818032 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-07-28 18:35:44 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-07-29 13:42:35 +0000
@@ -1190,7 +1190,7 @@
 
     def canUpgrade(self, action=None):
         """Should the form offer a packages upgrade?"""
-        if getFeatureFlag("soyuz.derived_series_sync.enabled") is None:
+        if getFeatureFlag("soyuz.derived_series_upgrade.enabled") is None:
             return False
         elif self.context.status not in UPGRADABLE_SERIES_STATUSES:
             # A feature freeze precludes blanket updates.

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-07-28 18:35:44 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-07-29 13:42:35 +0000
@@ -1386,10 +1386,17 @@
         self.assertContentEqual([dsd], view.getUpgrades())
 
     def enableDerivedSeriesSyncFeature(self):
+        """Enable the feature flag for derived-series sync."""
         self.useFixture(
             FeatureFixture(
                 {u'soyuz.derived_series_sync.enabled': u'on'}))
 
+    def enableDerivedSeriesUpgradeFeature(self):
+        """Enable the feature flag for derived-series upgrade."""
+        self.useFixture(
+            FeatureFixture(
+                {u'soyuz.derived_series_upgrade.enabled': u'on'}))
+
     @with_celebrity_logged_in("admin")
     def test_upgrades_offered_only_with_feature_flag(self):
         # The "Upgrade Packages" button will only be shown when a specific
@@ -1397,13 +1404,13 @@
         view = self.makeView()
         self.makePackageUpgrade(view.context)
         self.assertFalse(view.canUpgrade())
-        self.enableDerivedSeriesSyncFeature()
+        self.enableDerivedSeriesUpgradeFeature()
         self.assertTrue(view.canUpgrade())
 
     def test_upgrades_are_offered_if_appropriate(self):
         # The "Upgrade Packages" button will only be shown to privileged
         # users.
-        self.enableDerivedSeriesSyncFeature()
+        self.enableDerivedSeriesUpgradeFeature()
         dsd = self.makePackageUpgrade()
         view = self.makeView(dsd.derived_series)
         with celebrity_logged_in("admin"):
@@ -1417,7 +1424,7 @@
     def test_upgrades_offered_only_if_available(self):
         # If there are no upgrades, the "Upgrade Packages" button won't
         # be shown.
-        self.enableDerivedSeriesSyncFeature()
+        self.enableDerivedSeriesUpgradeFeature()
         view = self.makeView()
         self.assertFalse(view.canUpgrade())
         self.makePackageUpgrade(view.context)
@@ -1428,7 +1435,7 @@
         # There won't be an "Upgrade Packages" button once feature
         # freeze has occurred.  Mass updates would not make sense after
         # that point.
-        self.enableDerivedSeriesSyncFeature()
+        self.enableDerivedSeriesUpgradeFeature()
         upgradeable = {}
         for status in SeriesStatus.items:
             dsd = self.makePackageUpgrade()

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-07-21 03:13:36 +0000
+++ lib/lp/services/features/flags.py	2011-07-29 13:42:35 +0000
@@ -88,6 +88,10 @@
      'boolean',
      'Enables syncing of packages on derivative distributions pages.',
      ''),
+    ('soyuz.derived_series_upgrade.enabled',
+     'boolean',
+     'Enables mass-upgrade of packages on derivative distributions pages.',
+     ''),
     ('soyuz.derived_series_jobs.enabled',
      'boolean',
      "Compute package differences for derived distributions.",