launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/use-userHasPriviledges into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #885692 in Launchpad itself: "bug supervisors have more power than maintainers and admins"
  https://bugs.launchpad.net/launchpad/+bug/885692

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/use-userHasPriviledges/+merge/82967

Write a class method on BugTask that checks the context (an IBugTarget) that a bug is being filed on. This allows us to extend the special rules to users such as the pillar owner or drivers.

Sadly, this branch is misnamed, and the method does not make use of IBugTask.userHasPrivilegdes() since we don't have a bugtask when the functions are called.
-- 
https://code.launchpad.net/~stevenk/launchpad/use-userHasPriviledges/+merge/82967
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/use-userHasPriviledges into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-11-18 04:06:16 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-11-22 03:34:26 +0000
@@ -131,6 +131,7 @@
 from lp.bugs.interfaces.bugtracker import IBugTracker
 from lp.bugs.interfaces.malone import IMaloneApplication
 from lp.bugs.interfaces.securitycontact import IHasSecurityContact
+from lp.bugs.model.bugtask import BugTask
 from lp.bugs.model.structuralsubscription import (
     get_structural_subscriptions_for_target,
     )
@@ -401,11 +402,10 @@
         # If the context is a project group we want to render the optional
         # fields since they will initially be hidden and later exposed if the
         # selected project supports them.
-        # XXX: StevenK 2011-11-18 bug=885692 This should make use of
-        # IBugTask.userHasPrivileges().
         include_extra_fields = IProjectGroup.providedBy(context)
-        if not include_extra_fields and IHasBugSupervisor.providedBy(context):
-            include_extra_fields = self.user.inTeam(context.bug_supervisor)
+        if not include_extra_fields:
+            include_extra_fields = BugTask.userHasPrivilegesContext(
+                context, self.user)
 
         if include_extra_fields:
             field_names.extend(
@@ -628,19 +628,16 @@
         if extra_data.private:
             params.private = extra_data.private
 
-        # XXX: StevenK 2011-11-18 bug=885692 This should make use of
-        # IBugTask.userHasPrivileges().
-        # Apply any extra options given by a bug supervisor.
-        if IHasBugSupervisor.providedBy(context):
-            if self.user.inTeam(context.bug_supervisor):
-                if 'assignee' in data:
-                    params.assignee = data['assignee']
-                if 'status' in data:
-                    params.status = data['status']
-                if 'importance' in data:
-                    params.importance = data['importance']
-                if 'milestone' in data:
-                    params.milestone = data['milestone']
+        # Apply any extra options given by priviliged users.
+        if BugTask.userHasPrivilegesContext(context, self.user):
+            if 'assignee' in data:
+                params.assignee = data['assignee']
+            if 'status' in data:
+                params.status = data['status']
+            if 'importance' in data:
+                params.importance = data['importance']
+            if 'milestone' in data:
+                params.milestone = data['milestone']
 
         self.added_bug = bug = context.createBug(params)
 

=== modified file 'lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt'
--- lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2011-02-11 15:14:37 +0000
+++ lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2011-11-22 03:34:26 +0000
@@ -697,23 +697,28 @@
     True
 
 
-Extra fields for bug supervisors
---------------------------------
-
-Bug supervisors are offered several extra options when filing bugs.
-
-    >>> person = factory.makePerson(name=u'bug-superdude')
-    >>> product = factory.makeProduct(owner=person)
+Extra fields for priviliged users
+---------------------------------
+
+Priviliged users are offered several extra options when filing bugs.
+
+    >>> owner = factory.makePerson(name=u'bug-superdude')
+    >>> person = factory.makePerson()
+    >>> product = factory.makeProduct(owner=owner)
 
     >>> login_person(person)
-
     >>> filebug_view = create_initialized_view(product, '+filebug')
     >>> normal_fields = set(filebug_view.field_names)
-    >>> product.setBugSupervisor(person, person)
+    >>> login_person(owner)
+    >>> filebug_view = create_initialized_view(product, '+filebug')
+    >>> owner_fields = set(filebug_view.field_names)
+    >>> product.setBugSupervisor(owner, owner)
     >>> supervisor_fields = set(filebug_view.field_names)
 
-Supervisors get all the same fields as normal users, plus a few extra.
+Priviliged users get all the same fields as normal users, plus a few extra.
 
+    >>> owner_fields == supervisor_fields
+    True
     >>> supervisor_fields.issuperset(normal_fields)
     True
 
@@ -734,7 +739,7 @@
 
     >>> bug_data = dict(
     ...     title=u'Extra Fields Bug', comment=u'Test description.',
-    ...     assignee=person, importance=BugTaskImportance.HIGH,
+    ...     assignee=owner, importance=BugTaskImportance.HIGH,
     ...     milestone=milestone, status=BugTaskStatus.TRIAGED)
     >>> print filebug_view.validate(bug_data)
     None

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-11-21 00:31:41 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-11-22 03:34:26 +0000
@@ -1346,6 +1346,20 @@
         else:
             return None
 
+    @classmethod
+    def userHasPrivilegesContext(cls, context, user):
+        """Does the user have priviliges for the given context?
+
+        :return: a boolean.
+        """
+        role = IPersonRoles(user)
+        if role.in_admin:
+            return True
+        pillar = context.pillar
+        return (
+            role.isOwner(pillar) or role.isOneOfDrivers(pillar) or
+            role.isBugSupervisor(pillar))
+
     def userHasPrivileges(self, user):
         """See `IBugTask`."""
         if not user: