launchpad-reviewers team mailing list archive

["Francis J. Lacoste" <francis.lacoste@xxxxxxxxxx>]

Francis J. Lacoste has proposed merging lp:~flacoste/launchpad/bug-797088 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~flacoste/launchpad/bug-797088/+merge/64711

= Summary =

There was a bug in the implementation of the permission checker for setting
official package branches. It always called checkUpload with the RELEASE
pocket.

== Proposed fix ==

We don't care about pockets, so use verifyUpload() instead which doesn't do
the pocket check.

== Pre-implementation notes ==

* I discussed the policy requirements around UDD with lifeless and maxb.
* I validated the fix with bigjools.

== Implementation details ==

None really, it's pretty straighforward.

== Tests ==

./bin/test -m lp.registry -vvt TestSourcePackageSecurity


== Demo and Q/A ==

Check on staging that setBranch can be called over API for SUPPORTED and
CURRENT Ubuntu distro series.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/lp/registry/tests/test_sourcepackage.py
-- 
https://code.launchpad.net/~flacoste/launchpad/bug-797088/+merge/64711
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~flacoste/launchpad/bug-797088 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-06-06 20:56:43 +0000
+++ lib/canonical/launchpad/security.py	2011-06-15 16:29:26 +0000
@@ -2608,8 +2608,8 @@
 
         # checkUpload() returns the reason the user can't upload
         # or None if they are allowed.
-        reason = distribution.main_archive.checkUpload(
-            user.person, self.obj.distroseries, self.obj.sourcepackagename,
-            component=None, pocket=PackagePublishingPocket.RELEASE,
-            strict_component=False)
+        reason = distribution.main_archive.verifyUpload(
+            user.person, distroseries=self.obj.distroseries,
+            sourcepackagename=self.obj.sourcepackagename,
+            component=None, strict_component=False)
         return reason is None

=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py	2011-06-03 18:47:39 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py	2011-06-15 16:29:26 +0000
@@ -531,7 +531,7 @@
                 "Distribution owner should have launchpad.Edit on source "
                 "packages.")
 
-    def test_uploader_have_launchpad_edit(self):
+    def test_uploader_has_launchpad_edit(self):
         sourcepackage = self.factory.makeSourcePackage()
         uploader = self.factory.makePerson()
         archive = sourcepackage.get_default_archive()
@@ -543,6 +543,51 @@
                 "Uploader to the package should have launchpad.Edit on "
                 "source packages.")
 
+    def test_uploader_has_launchpad_edit_on_obsolete_series(self):
+        obsolete_series = self.factory.makeDistroRelease(
+            status=SeriesStatus.OBSOLETE)
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=obsolete_series)
+        uploader = self.factory.makePerson()
+        archive = sourcepackage.get_default_archive()
+        with person_logged_in(sourcepackage.distribution.main_archive.owner):
+            archive.newPackageUploader(uploader, sourcepackage.name)
+        with person_logged_in(uploader):
+            self.failUnless(
+                checkPermission('launchpad.Edit', sourcepackage),
+                "Uploader to the package should have launchpad.Edit on "
+                "source packages in an OBSOLETE series.")
+
+    def test_uploader_have_launchpad_edit_on_current_series(self):
+        current_series = self.factory.makeDistroRelease(
+            status=SeriesStatus.CURRENT)
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=current_series)
+        uploader = self.factory.makePerson()
+        archive = sourcepackage.get_default_archive()
+        with person_logged_in(sourcepackage.distribution.main_archive.owner):
+            archive.newPackageUploader(uploader, sourcepackage.name)
+        with person_logged_in(uploader):
+            self.failUnless(
+                checkPermission('launchpad.Edit', sourcepackage),
+                "Uploader to the package should have launchpad.Edit on "
+                "source packages in a CURRENT series.")
+
+    def test_uploader_have_launchpad_edit_on_supported_series(self):
+        supported_series = self.factory.makeDistroRelease(
+            status=SeriesStatus.SUPPORTED)
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=supported_series)
+        uploader = self.factory.makePerson()
+        archive = sourcepackage.get_default_archive()
+        with person_logged_in(sourcepackage.distribution.main_archive.owner):
+            archive.newPackageUploader(uploader, sourcepackage.name)
+        with person_logged_in(uploader):
+            self.failUnless(
+                checkPermission('launchpad.Edit', sourcepackage),
+                "Uploader to the package should have launchpad.Edit on "
+                "source packages in a SUPPORTED series.")
+
     def test_john_doe_cannot_edit(self):
         sourcepackage = self.factory.makeSourcePackage()
         john_doe = self.factory.makePerson()