launchpad-reviewers team mailing list archive

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

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/initseries-perm-bug-796759 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #796759 in Launchpad itself: "+initseries page requires LP admin permissions. It doesn't work for drivers"
  https://bugs.launchpad.net/launchpad/+bug/796759

For more details, see:
https://code.launchpad.net/~rvb/launchpad/initseries-perm-bug-796759/+merge/65510

This branch changes the permission to access IDS:+initseries from launchpad.Admin to launchpad.Edit. This way admins and distroseries' drivers are able to initialize a series.

= Tests =

./bin/test -cvv test_distroseries test_initseries_access_admin
./bin/test -cvv test_distroseries test_initseries_access_driver
./bin/test -cvv test_distroseries test_initseries_access_simpleuser
./bin/test -cvv test_distroseries test_differences_init_link_series_driver

= Q/A =

As a distroseries's driver, make sure that:
- the link to +initseries is displayed on the series home page. 
- we can access IDS:+initseries
- we can initialize the series
-- 
https://code.launchpad.net/~rvb/launchpad/initseries-perm-bug-796759/+merge/65510
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/initseries-perm-bug-796759 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2011-06-14 12:17:00 +0000
+++ lib/lp/registry/browser/configure.zcml	2011-06-22 15:04:39 +0000
@@ -2016,7 +2016,7 @@
         for="lp.registry.interfaces.distroseries.IDistroSeries"
         class="lp.registry.browser.distroseries.DistroSeriesInitializeView"
         facet="overview"
-        permission="launchpad.Admin"
+        permission="launchpad.Edit"
         template="../templates/distroseries-initialize.pt">
     </browser:page>
 

=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-06-14 16:37:19 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-06-22 15:04:39 +0000
@@ -276,7 +276,7 @@
         text = 'Show uploads'
         return Link('+queue', text, icon='info')
 
-    @enabled_with_permission('launchpad.Admin')
+    @enabled_with_permission('launchpad.Edit')
     def initseries(self):
         enabled = (
              getFeatureFlag('soyuz.derived_series_ui.enabled') is not None and

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-06-15 08:04:39 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-06-22 15:04:39 +0000
@@ -31,6 +31,7 @@
 from canonical.database.constants import UTC_NOW
 from canonical.database.sqlbase import flush_database_caches
 from canonical.launchpad.testing.pages import find_tag_by_id
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.batching import BatchNavigator
 from canonical.launchpad.webapp.interaction import get_current_principal
 from canonical.launchpad.webapp.interfaces import BrowserNotificationLevel
@@ -79,6 +80,7 @@
 from lp.testing import (
     anonymous_logged_in,
     celebrity_logged_in,
+    login_celebrity,
     login_person,
     person_logged_in,
     StormStatementRecorder,
@@ -336,7 +338,7 @@
 
     def _assertInitSeriesLink(self, series, person, present=True):
         # Helper method to check the presence/absence of the link to
-        # +iniseries.
+        # +initseries.
         if person == 'admin':
             person = getUtility(ILaunchpadCelebrities).admin.teamowner
 
@@ -373,6 +375,17 @@
 
         self.assertInitSeriesLinkPresent(series, 'admin')
 
+    def test_differences_init_link_series_driver(self):
+        # The link to +initseries is displayed to the distroseries's
+        # drivers.
+        set_derived_series_ui_feature_flag(self)
+        distroseries = self.factory.makeDistroSeries()
+        driver = self.factory.makePerson()
+        with celebrity_logged_in('admin'):
+            distroseries.driver = driver
+
+        self.assertInitSeriesLinkPresent(distroseries, driver)
+
     def test_differences_init_link_not_admin(self):
         # The link to +initseries is not displayed to not admin users if the
         # feature flag is enabled.
@@ -589,6 +602,50 @@
                     u"This series is already being initialized."))
 
 
+class TestDistroSeriesInitializeViewAccess(TestCaseWithFactory):
+    """Test access to IDS.+initseries."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestDistroSeriesInitializeViewAccess,
+              self).setUp('foo.bar@xxxxxxxxxxxxx')
+        set_derived_series_ui_feature_flag(self)
+
+    def test_initseries_access_simpleuser(self):
+        # Unprivileged users cannot access +initseries.
+        distroseries = self.factory.makeDistroSeries()
+        view = create_initialized_view(distroseries, "+initseries")
+        login_person(self.factory.makePerson())
+
+        self.assertEqual(
+            False,
+            check_permission('launchpad.Edit', view))
+
+    def test_initseries_access_admin(self):
+        # Users with lp.Admin can access +initseries.
+        distroseries = self.factory.makeDistroSeries()
+        view = create_initialized_view(distroseries, "+initseries")
+        login_celebrity('admin')
+
+        self.assertEqual(
+            True,
+            check_permission('launchpad.Edit', view))
+
+    def test_initseries_access_driver(self):
+        # Distroseries drivers can access +initseries.
+        distroseries = self.factory.makeDistroSeries()
+        view = create_initialized_view(distroseries, "+initseries")
+        driver = self.factory.makePerson()
+        with celebrity_logged_in('admin'):
+            distroseries.driver = driver
+        login_person(driver)
+
+        self.assertEqual(
+            True,
+            check_permission('launchpad.Edit', view))
+
+
 class DistroSeriesDifferenceMixin:
     """A helper class for testing differences pages"""