launchpad-reviewers team mailing list archive

[Paul Hummer <paul@xxxxxxxxxxxxxxxxxxxx>]

Paul Hummer has proposed merging lp:~rockstar/launchpad/fix-queue-permissions into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch fixes a few stupid things I did with the original models, and Tim/Ian pointed them out to me.
-- 
https://code.launchpad.net/~rockstar/launchpad/fix-queue-permissions/+merge/39278
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/fix-queue-permissions into lp:launchpad.

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-10-21 01:42:14 +0000
+++ lib/canonical/launchpad/security.py	2010-10-25 14:56:16 +0000
@@ -67,6 +67,7 @@
     user_has_special_branch_access,
     )
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
 from lp.code.interfaces.codeimport import ICodeImport
 from lp.code.interfaces.codeimportjob import (
     ICodeImportJobSet,
@@ -1162,6 +1163,18 @@
         return user.in_bazaar_experts or user.in_buildd_admin
 
 
+class EditBranchMergeQueue(AuthorizationBase):
+    """Control who can edit a BranchMergeQueue.
+
+    Access is granted only to the owner of the queue.
+    """
+    permission = 'launchpad.Edit'
+    usedfor = IBranchMergeQueue
+
+    def checkAuthenticated(self, user):
+        return user.isOwner(self.obj)
+
+
 class AdminDistributionTranslations(AuthorizationBase):
     """Class for deciding who can administer distribution translations.
 

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-10-21 00:49:21 +0000
+++ lib/lp/code/configure.zcml	2010-10-25 14:56:16 +0000
@@ -23,8 +23,20 @@
       name="code" />
 
   <!-- Branch Merge Queues -->
+  <securedutility
+     component="lp.code.model.branchmergequeue.BranchMergeQueue"
+     provides="lp.code.interfaces.branchmergequeue.IBranchMergeQueueSource">
+    <allow interface="lp.code.interfaces.branchmergequeue.IBranchMergeQueueSource"/>
+
+  </securedutility>
+
   <class class="lp.code.model.branchmergequeue.BranchMergeQueue">
-    <allow interface="lp.code.interfaces.branchmergequeue.IBranchMergeQueue" />
+    <require permission="zope.Public"
+             attributes="registrant owner name description configuration
+                         date_created branches" />
+    <require permission="launchpad.Edit"
+             attributes="setMergeQueueConfig"
+             set_attributes="owner name description configuration" />
   </class>
 
   <class class="lp.code.model.codereviewvote.CodeReviewVoteReference">

=== modified file 'lib/lp/code/model/branchmergequeue.py'
--- lib/lp/code/model/branchmergequeue.py	2010-10-18 21:20:28 +0000
+++ lib/lp/code/model/branchmergequeue.py	2010-10-25 14:56:16 +0000
@@ -21,6 +21,7 @@
     )
 
 from canonical.database.datetimecol import UtcDateTimeCol
+from canonical.launchpad.interfaces.lpstorm import IMasterStore
 from lp.code.errors import InvalidMergeQueueConfig
 from lp.code.interfaces.branchmergequeue import (
     IBranchMergeQueue,
@@ -69,6 +70,8 @@
     def new(cls, name, owner, registrant, description=None,
             configuration=None):
         """See `IBranchMergeQueueSource`."""
+        store = IMasterStore(BranchMergeQueue)
+
         queue = cls()
         queue.name = name
         queue.owner = owner
@@ -76,4 +79,5 @@
         queue.description = description
         queue.configuration = configuration
 
+        store.add(queue)
         return queue

=== modified file 'lib/lp/code/model/tests/test_branchmergequeue.py'
--- lib/lp/code/model/tests/test_branchmergequeue.py	2010-10-20 17:48:51 +0000
+++ lib/lp/code/model/tests/test_branchmergequeue.py	2010-10-25 14:56:16 +0000
@@ -7,8 +7,6 @@
 
 import simplejson
 
-import transaction
-
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.webapp.testing import verifyObject
 from canonical.testing.layers import (
@@ -86,17 +84,20 @@
         config = unicode(simplejson.dumps({
             'test': 'make test'}))
 
-        queue.setMergeQueueConfig(config)
+        with person_logged_in(queue.owner):
+            queue.setMergeQueueConfig(config)
 
         self.assertEqual(queue.configuration, config)
 
     def test_setMergeQueueConfig_invalid_json(self):
         """Test that invalid json can't be set as the config."""
         queue = self.factory.makeBranchMergeQueue()
-        self.assertRaises(
-            InvalidMergeQueueConfig,
-            queue.setMergeQueueConfig,
-            'abc')
+
+        with person_logged_in(queue.owner):
+            self.assertRaises(
+                InvalidMergeQueueConfig,
+                queue.setMergeQueueConfig,
+                'abc')
 
 
 class TestWebservice(TestCaseWithFactory):
@@ -123,7 +124,6 @@
                 branch2.addToQueue(db_queue)
             launchpad = launchpadlib_for('test', db_queue.owner,
                 service_root="http://api.launchpad.dev:8085";)
-            transaction.commit()
 
         queuer = ws_object(launchpad, queuer)
         queue = ws_object(launchpad, db_queue)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-10-25 12:11:43 +0000
+++ lib/lp/testing/factory.py	2010-10-25 14:56:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 aanonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=F0401
@@ -133,6 +133,7 @@
     RevisionControlSystems,
     )
 from lp.code.errors import UnknownBranchTypeError
+from lp.code.interfaces.branchmergequeue import IBranchMergeQueueSource
 from lp.code.interfaces.branchnamespace import get_branch_namespace
 from lp.code.interfaces.branchtarget import IBranchTarget
 from lp.code.interfaces.codeimport import ICodeImportSet
@@ -152,7 +153,6 @@
     PreviewDiff,
     StaticDiff,
     )
-from lp.code.model.branchmergequeue import BranchMergeQueue
 from lp.codehosting.codeimport.worker import CodeImportSourceDetails
 from lp.hardwaredb.interfaces.hwdb import (
     HWSubmissionFormat,
@@ -1114,7 +1114,7 @@
             configuration = unicode(simplejson.dumps({
                 self.getUniqueString('key'): self.getUniqueString('value')}))
 
-        queue = BranchMergeQueue.new(
+        queue = getUtility(IBranchMergeQueueSource).new(
             name, registrant, owner, description, configuration)
         return queue