Merge lp:~stevenk/launchpad/use-userHasPriviledges into lp:launchpad

[Steve Kowalik]

[r=wgrant][bug=885692] Add userHasPrivileges for IBugTarget contexts so that more than the bug supervisor can set the extra fields.

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.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-11-22 23:29:08 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-11-24 00:54: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 privileged 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-24 00:54: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 privileged users
+---------------------------------
+
+Privileged 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.
+Privileged 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-24 00:54:26 +0000
@@ -1346,8 +1346,12 @@
         else:
             return None
 
-    def userHasPrivileges(self, user):
-        """See `IBugTask`."""
+    @classmethod
+    def userHasPrivilegesContext(cls, context, user):
+        """Does the user have privileges for the given context?
+
+        :return: a boolean.
+        """
         if not user:
             return False
         role = IPersonRoles(user)
@@ -1365,12 +1369,12 @@
         # Otherwise, if you're a member of the pillar owner, drivers, or the
         # bug supervisor, you can change bug details.
         return (
-            role.isOwner(self.pillar) or role.isOneOfDrivers(self.pillar) or
-            role.isBugSupervisor(self.pillar) or
-            (self.distroseries is not None and
-                role.isDriver(self.distroseries)) or
-            (self.productseries is not None and
-                role.isDriver(self.productseries)))
+            role.isOwner(context.pillar) or role.isOneOfDrivers(context) or
+            role.isBugSupervisor(context.pillar))
+
+    def userHasPrivileges(self, user):
+        """See `IBugTask`."""
+        return self.userHasPrivilegesContext(self.target, user)
 
     def __repr__(self):
         return "<BugTask for bug %s on %r>" % (self.bugID, self.target)

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-11-18 05:22:34 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-11-24 00:54:26 +0000
@@ -51,6 +51,7 @@
 from lp.bugs.model.bugtask import (
     bug_target_from_key,
     bug_target_to_key,
+    BugTask,
     BugTaskSet,
     build_tag_search_clause,
     IllegalTarget,
@@ -2592,12 +2593,12 @@
             self.assertEqual([db_bug.default_bugtask], db_bug.bugtasks)
 
 
-class TestBugTaskUserHasPriviliges(TestCaseWithFactory):
+class TestBugTaskUserHasPrivileges(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestBugTaskUserHasPriviliges, self).setUp()
+        super(TestBugTaskUserHasPrivileges, self).setUp()
         self.celebrities = getUtility(ILaunchpadCelebrities)
 
     def test_admin_is_allowed(self):
@@ -2655,3 +2656,32 @@
         bugtask = self.factory.makeBugTask()
         self.assertFalse(
             bugtask.userHasPrivileges(self.factory.makePerson()))
+
+
+class TestBugTaskUserHasPrivilegesContext(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def assert_userHasPrivilegesContext(self, obj):
+        self.assertFalse(
+            BugTask.userHasPrivilegesContext(obj, self.factory.makePerson()))
+
+    def test_distribution(self):
+        distribution = self.factory.makeDistribution()
+        self.assert_userHasPrivilegesContext(distribution)
+
+    def test_distributionsourcepackage(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        self.assert_userHasPrivilegesContext(dsp)
+
+    def test_product(self):
+        product = self.factory.makeProduct()
+        self.assert_userHasPrivilegesContext(product)
+
+    def test_productseries(self):
+        productseries = self.factory.makeProductSeries()
+        self.assert_userHasPrivilegesContext(productseries)
+
+    def test_sourcepackage(self):
+        source = self.factory.makeSourcePackage()
+        self.assert_userHasPrivilegesContext(source)

=== modified file 'lib/lp/bugs/stories/bug-release-management/xx-bug-release-management.txt'
--- lib/lp/bugs/stories/bug-release-management/xx-bug-release-management.txt	2011-08-08 18:14:38 +0000
+++ lib/lp/bugs/stories/bug-release-management/xx-bug-release-management.txt	2011-11-24 00:54:26 +0000
@@ -146,7 +146,7 @@
     ...     (dsp.name, bug_task.bug.id))
 
     >>> admin_browser.getControl("Hydro").selected = True
-    >>> admin_browser.getControl("Nominate").click()
+    >>> admin_browser.getControl("Target").click()
 
     >>> for tag in find_tags_by_class(admin_browser.contents, 'message'):
     ...     print tag

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-11-22 00:02:48 +0000
+++ lib/lp/registry/configure.zcml	2011-11-24 00:54:26 +0000
@@ -481,6 +481,7 @@
                 displayname
                 distribution
                 distro
+                drivers
                 findRelatedArchivePublications
                 findRelatedArchives
                 getMergeProposals

=== modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py'
--- lib/lp/registry/interfaces/distributionsourcepackage.py	2011-10-04 21:43:11 +0000
+++ lib/lp/registry/interfaces/distributionsourcepackage.py	2011-11-24 00:54:26 +0000
@@ -45,13 +45,14 @@
     IHasMergeProposals,
     )
 from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.role import IHasDrivers
 from lp.soyuz.enums import ArchivePurpose
 
 
 class IDistributionSourcePackage(IBugTarget, IHasBranches, IHasMergeProposals,
                                  IHasOfficialBugTags,
                                  IStructuralSubscriptionTarget,
-                                 IQuestionTarget):
+                                 IQuestionTarget, IHasDrivers):
     """Represents a source package in a distribution.
 
     Create IDistributionSourcePackages by invoking
@@ -122,6 +123,8 @@
         "Number of translations matching the distribution and "
         "sourcepackagename of the IDistributionSourcePackage.")
 
+    drivers = Attribute("The drivers for the distribution.")
+
     def getReleasesAndPublishingHistory():
         """Return a list of all releases of this source package in this
         distribution and their corresponding publishing history.

=== modified file 'lib/lp/registry/interfaces/sourcepackage.py'
--- lib/lp/registry/interfaces/sourcepackage.py	2011-06-02 20:53:43 +0000
+++ lib/lp/registry/interfaces/sourcepackage.py	2011-11-24 00:54:26 +0000
@@ -58,6 +58,7 @@
     IHasMergeProposals,
     )
 from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.interfaces.role import IHasDrivers
 from lp.soyuz.interfaces.component import IComponent
 from lp.translations.interfaces.hastranslationtemplates import (
     IHasTranslationTemplates,
@@ -69,7 +70,8 @@
 
 class ISourcePackagePublic(IBugTarget, IHasBranches, IHasMergeProposals,
                            IHasOfficialBugTags, IHasCodeImports,
-                           IHasTranslationImports, IHasTranslationTemplates):
+                           IHasTranslationImports, IHasTranslationTemplates,
+                           IHasDrivers):
     """Public attributes for SourcePackage."""
 
     id = Attribute("ID")
@@ -170,6 +172,9 @@
     distribution_sourcepackage = Attribute(
         "The IDistributionSourcePackage for this source package.")
 
+    drivers = Attribute(
+        "The drivers for the distroseries for this source package.")
+
     def __getitem__(version):
         """Return the source package release with the given version in this
         distro series, or None."""

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2011-11-10 13:55:16 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2011-11-24 00:54:26 +0000
@@ -63,6 +63,7 @@
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.hasdrivers import HasDriversMixin
 from lp.registry.model.karma import KarmaTotalCache
 from lp.registry.model.packaging import Packaging
 from lp.registry.model.sourcepackage import (
@@ -131,7 +132,8 @@
                                 HasBranchesMixin,
                                 HasCustomLanguageCodesMixin,
                                 HasMergeProposalsMixin,
-                                HasBugHeatMixin):
+                                HasBugHeatMixin,
+                                HasDriversMixin):
     """This is a "Magic Distribution Source Package". It is not an
     SQLObject, but instead it represents a source package with a particular
     name in a particular distribution. You can then ask it all sorts of
@@ -219,6 +221,11 @@
         return self._get(
             self.distribution, self.sourcepackagename) is not None
 
+    @property
+    def drivers(self):
+        """See `IHasDrivers`."""
+        return self.distribution.drivers
+
     def delete(self):
         """See `DistributionSourcePackage`."""
         dsp_in_db = self._self_in_database

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2011-09-28 03:28:50 +0000
+++ lib/lp/registry/model/sourcepackage.py	2011-11-24 00:54:26 +0000
@@ -528,6 +528,11 @@
         """See `IHasBugs`."""
         return self.distribution_sourcepackage.max_bug_heat
 
+    @property
+    def drivers(self):
+        """See `IHasDrivers`."""
+        return self.distroseries.drivers
+
     def createBug(self, bug_params):
         """See canonical.launchpad.interfaces.IBugTarget."""
         # We don't currently support opening a new bug directly on an

=== modified file 'lib/lp/registry/tests/test_distributionsourcepackage.py'
--- lib/lp/registry/tests/test_distributionsourcepackage.py	2011-11-10 20:23:30 +0000
+++ lib/lp/registry/tests/test_distributionsourcepackage.py	2011-11-24 00:54:26 +0000
@@ -145,6 +145,14 @@
         dsp = self.factory.makeDistributionSourcePackage(with_db=False)
         self.assertFalse(dsp.is_official)
 
+    def test_drivers_are_distributions(self):
+        # DSP.drivers returns the drivers for the distribution.
+        distribution = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=distribution)
+        self.assertNotEqual([], distribution.drivers)
+        self.assertEqual(dsp.drivers, distribution.drivers)
+
 
 class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/tests/test_sourcepackage.py'
--- lib/lp/registry/tests/test_sourcepackage.py	2011-09-19 22:43:14 +0000
+++ lib/lp/registry/tests/test_sourcepackage.py	2011-11-24 00:54:26 +0000
@@ -494,6 +494,14 @@
         self.assertEqual(
             expected, sourcepackage.getSharingDetailPermissions())
 
+    def test_drivers_are_distroseries(self):
+        # SP.drivers returns the drivers for the distroseries.
+        distroseries = self.factory.makeDistroSeries()
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=distroseries)
+        self.assertNotEqual([], distroseries.drivers)
+        self.assertEqual(sourcepackage.drivers, distroseries.drivers)
+
 
 class TestSourcePackageWebService(WebServiceTestCase):