launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/nomminate-driver-permissions-1 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #901332 in Launchpad itself: "AttributeError: 'SourcePackage' object has no attribute 'personHasDriverRights'  nominating a bug"
  https://bugs.launchpad.net/launchpad/+bug/901332

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/nomminate-driver-permissions-1/+merge/84850

Allow driver permission checks on source packages.

    Launchpad bug: https://bugs.launchpad.net/bugs/901332
    Pre-implementation: No one

SourcePackage does implement all of the IHasDrivers interface.
The personHasDriverRights that is used for permission checks to work with
the object is missing. It is not possible to nominate a fix
for a source package at the moment because a regression caused to an
effort to implement a single consistent set of permission rules in
the bugs domain.

--------------------------------------------------------------------

RULES

    * personHasDriverRights on SourcePackage,
      consider using the HasDriversMixin
    * ADDENDUM: DSP uses HasDriversMixin to implement personHasDriverRights,
      but permissions are not specified for callsites to use it.


QA

    * Visit https://bugs.qastaging.launchpad.net/ubuntu/oneiric/+source/linux/+bug/861296/+nominate
    * Verify the page displays
      (The page will probably say your do not have permission to nominate)
    * Visit https://bugs.qastaging.launchpad.net/ubuntu/+source/linux/+bug/861296/+nominate
    * Verify the page displays
      (The page will probably say your do not have permission to nominate

LINT

    lib/lp/registry/configure.zcml
    lib/lp/registry/model/sourcepackage.py
    lib/lp/registry/tests/test_distributionsourcepackage.py
    lib/lp/registry/tests/test_sourcepackage.py


TEST

    ./bin/test -vvc -t river lp.registry.tests.test_sourcepackage
    ./bin/test -vvc -t river lp.registry.tests.test_distributionsourcepackage


IMPLEMENTATION

Added the HasDriversMixin to SourcePackage to complete the implementation
of IHasDrivers.
    lib/lp/registry/model/sourcepackage.py
    lib/lp/registry/tests/test_sourcepackage.py

Added personHasDriverRights to ZCML after I discovered that adding a
comparable test for personHasDriverRights did errored even though
the methods is defined.
    lib/lp/registry/configure.zcml
    lib/lp/registry/tests/test_distributionsourcepackage.py
-- 
https://code.launchpad.net/~sinzui/launchpad/nomminate-driver-permissions-1/+merge/84850
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/nomminate-driver-permissions-1 into lp:launchpad.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-11-28 03:51:37 +0000
+++ lib/lp/registry/configure.zcml	2011-12-07 21:41:27 +0000
@@ -487,6 +487,7 @@
                 latest_overall_publication
                 name
                 official_bug_tags
+                personHasDriverRights
                 publishing_history
                 releases
                 sourcepackagename

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2011-11-23 03:09:55 +0000
+++ lib/lp/registry/model/sourcepackage.py	2011-12-07 21:41:27 +0000
@@ -66,6 +66,7 @@
     ISourcePackage,
     ISourcePackageFactory,
     )
+from lp.registry.model.hasdrivers import HasDriversMixin
 from lp.registry.model.packaging import Packaging
 from lp.registry.model.suitesourcepackage import SuiteSourcePackage
 from lp.soyuz.enums import (
@@ -195,7 +196,8 @@
 
 class SourcePackage(BugTargetBase, HasBugHeatMixin, HasCodeImportsMixin,
                     HasTranslationImportsMixin, HasTranslationTemplatesMixin,
-                    HasBranchesMixin, HasMergeProposalsMixin):
+                    HasBranchesMixin, HasMergeProposalsMixin,
+                    HasDriversMixin):
     """A source package, e.g. apache2, in a distroseries.
 
     This object is not a true database object, but rather attempts to

=== modified file 'lib/lp/registry/tests/test_distributionsourcepackage.py'
--- lib/lp/registry/tests/test_distributionsourcepackage.py	2011-11-23 03:43:53 +0000
+++ lib/lp/registry/tests/test_distributionsourcepackage.py	2011-12-07 21:41:27 +0000
@@ -153,6 +153,14 @@
         self.assertNotEqual([], distribution.drivers)
         self.assertEqual(dsp.drivers, distribution.drivers)
 
+    def test_personHasDriverRights(self):
+        # A distribution driver has driver permissions on a DSP.
+        distribution = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=distribution)
+        driver = distribution.drivers[0]
+        self.assertTrue(dsp.personHasDriverRights(driver))
+
 
 class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py	2011-11-23 03:43:53 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py	2011-12-07 21:41:27 +0000
@@ -502,6 +502,14 @@
         self.assertNotEqual([], distroseries.drivers)
         self.assertEqual(sourcepackage.drivers, distroseries.drivers)
 
+    def test_personHasDriverRights(self):
+        # A distroseries driver has driver permissions on source packages.
+        distroseries = self.factory.makeDistroSeries()
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=distroseries)
+        driver = distroseries.drivers[0]
+        self.assertTrue(sourcepackage.personHasDriverRights(driver))
+
 
 class TestSourcePackageWebService(WebServiceTestCase):