← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~brian-murray/launchpad/bug-supervisor-nominate-for-release into lp:launchpad/devel

 

Brian Murray has proposed merging lp:~brian-murray/launchpad/bug-supervisor-nominate-for-release into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #114766 Only bug supervisor should be able to nominate a bug for a release
  https://bugs.launchpad.net/bugs/114766


Bug 114766 is about making bug nominations more useful, for nomination reviewers, by restricting the number ability to nominate bugs for a release from anyone to the bug supervisor group.  This is also something that the Ubuntu community is looking for as noted in bug 174375.  I've setup the branch so the that only projects or distributions that have a bug supervisor set are affected by this change.

lib/lp/bugs/browser/bug.py:
   restrict the enabling of the +nominate link to the bug supervisor team if set
lib/lp/bugs/model/bug.py:
    added check to ensure that nominator is a member of the correct team
lib/lp/bugs/configure.zcml:
    sorted permission attributes alphabetically
lib/lp/testing/factorypy:
     add in a bug supervisor parameter to makeDistribution (one already exists for makeProject)
lib/lp/bugs/stories/bug-release-management/:
    modified tests 30- and 40- not to use sample data
    60- added in a nomination that used to be in 30- or 40-

 I added in test_bugsupervisor_bugnomination.py:
    bin/test -cvvt test_bugsupervisor_bugnomination
I've also modified the tests in bug-release-management:
    bin/test -cvvt bug-release-management
-- 
https://code.launchpad.net/~brian-murray/launchpad/bug-supervisor-nominate-for-release/+merge/38733
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~brian-murray/launchpad/bug-supervisor-nominate-for-release into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2010-09-23 10:27:11 +0000
+++ lib/lp/bugs/browser/bug.py	2010-10-18 15:16:16 +0000
@@ -276,7 +276,13 @@
         else:
             text = 'Nominate for release'
 
-        return Link('+nominate', text, icon='milestone')
+        if target.bug_supervisor and (check_permission("launchpad.Driver", target) or
+            check_permission("launchpad.BugSupervisor", target)):
+            return Link('+nominate', text, icon='milestone')
+        elif target.bug_supervisor:
+            return Link('+nominate', text, enabled=False, icon='milestone')
+        else:
+            return Link('+nominate', text, icon='milestone')
 
     def addcomment(self):
         """Return the 'Comment or attach file' Link."""

=== modified file 'lib/lp/bugs/browser/tests/bug-nomination-views.txt'
--- lib/lp/bugs/browser/tests/bug-nomination-views.txt	2010-10-11 16:17:45 +0000
+++ lib/lp/bugs/browser/tests/bug-nomination-views.txt	2010-10-18 15:16:16 +0000
@@ -167,7 +167,7 @@
 
 The batch navigator used returns a BugTaskListingItem for each of the
 distribution's bugtasks that are nominated for this release, with a
-widget set up for accepting/declining the nomination. The widet is only
+widget set up for accepting/declining the nomination. The widget is only
 set up if the user has permission to approve the nomination.
 
     >>> from lp.bugs.interfaces.bug import CreateBugParams

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2010-10-18 01:06:18 +0000
+++ lib/lp/bugs/configure.zcml	2010-10-18 15:16:16 +0000
@@ -731,35 +731,35 @@
             <require
                 permission="launchpad.Edit"
                 attributes="
+                    addAttachment
+                    addChange
                     addChangeNotification
                     addCommentNotification
+                    addNomination
+                    addTask
                     addWatch
-                    removeWatch
+                    convertToQuestion
+                    expireNotifications
+                    findCvesInText
+                    linkAttachment
+                    linkBranch
                     linkCVE
                     linkCVEAndReturnNothing
-                    unlinkCVE
-                    findCvesInText
+                    linkHWSubmission
+                    linkMessage
+                    markAsDuplicate
+                    markUserAffected
                     newMessage
-                    linkMessage
-                    addAttachment
-                    unsubscribeFromDupes
-                    subscribe
-                    unsubscribe
-                    addNomination
-                    expireNotifications
-                    setStatus
+                    removeWatch
                     setPrivate
                     setSecurityRelated
-                    convertToQuestion
-                    markUserAffected
-                    addTask
-                    addChange
-                    markAsDuplicate
-                    linkHWSubmission
+                    setStatus
+                    subscribe
+                    unlinkBranch
+                    unlinkCVE
                     unlinkHWSubmission
-                    linkBranch
-                    unlinkBranch
-                    linkAttachment
+                    unsubscribe
+                    unsubscribeFromDupes
                     updateHeat"
                 set_attributes="
                     activity initial_message

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-10-15 16:09:18 +0000
+++ lib/lp/bugs/model/bug.py	2010-10-18 15:16:16 +0000
@@ -1540,6 +1540,20 @@
             assert IProductSeries.providedBy(target)
             productseries = target
 
+        admins = getUtility(ILaunchpadCelebrities).admin
+        if (distroseries and distroseries.distribution.bug_supervisor
+            and not owner.inTeam(distroseries.distribution.bug_supervisor)
+            and not owner.inTeam(distroseries.distribution.owner)
+            and not owner.inTeam(admins)):
+            raise NominationError(
+                "Only bug supervisors can nominate bugs.")
+        elif (productseries and productseries.bug_supervisor and not
+              owner.inTeam(productseries.bug_supervisor)
+              and not owner.inTeam(productseries.owner)
+              and not owner.inTeam(admins)):
+            raise NominationError(
+                "Only bug supervisors can nominate bugs.")
+
         nomination = BugNomination(
             owner=owner, bug=self, distroseries=distroseries,
             productseries=productseries)

=== modified file 'lib/lp/bugs/stories/bug-release-management/30-nominate-bug-for-distrorelease.txt'
--- lib/lp/bugs/stories/bug-release-management/30-nominate-bug-for-distrorelease.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/stories/bug-release-management/30-nominate-bug-for-distrorelease.txt	2010-10-18 15:16:16 +0000
@@ -2,51 +2,66 @@
 
 A bug can be nominated for a distribution release.
 
-    >>> user_browser.open(
-    ...     "http://launchpad.dev/distros/ubuntu/+source/mozilla-firefox/";
-    ...     "+bug/1/+nominate")
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> nominater = factory.makePerson(name='denominater',
+    ...     password='g00dpassword')
+    >>> poseidon = factory.makeDistribution(name='poseidon')
+    >>> dsp = factory.makeDistributionSourcePackage(distribution=poseidon)
+    >>> series = factory.makeDistroSeries(distribution=poseidon,
+    ...     name='aqua')
+    >>> series = factory.makeDistroSeries(distribution=poseidon,
+    ...     name='hydro')
+    >>> bug_task = factory.makeBugTask(target=dsp)
+    >>> nominater_browser = setupBrowser(
+    ...     auth='Basic %s:g00dpassword' %
+    ...     nominater.preferredemail.email)
+    >>> logout()
+    >>> nominater_browser.open(
+    ...     "http://launchpad.dev/poseidon/+source/%s/+bug/%s/+nominate"; %
+    ...     (dsp.name, bug_task.bug.id))
 
 Before we continue, we'll set up a second browser instance, to simulate
-no-priv accessing the site from another window. Working with the same
-form in different browser windows or tabs can sometimes trigger edge
-case errors, and we'll give an example of one shortly.
-
-    >>> nopriv_other_browser = setupBrowser(
-    ...     auth="Basic no-priv@xxxxxxxxxxxxx:test")
-    >>> nopriv_other_browser.open(
-    ...     "http://launchpad.dev/distros/ubuntu/+source/mozilla-firefox/";
-    ...     "+bug/1/+nominate")
-
-    >>> user_browser.getControl("Grumpy").selected = True
-    >>> nopriv_other_browser.getControl("Grumpy").selected = True
-
-    >>> user_browser.getControl("Submit").click()
-
-    >>> for tag in find_tags_by_class(user_browser.contents, 'message'):
+the nominater accessing the site from another window. Working with the same
+form in different browser windows or tabs can sometimes trigger edge case
+errors, and we'll give an example of one shortly.
+
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> nominater_other_browser = setupBrowser(
+    ...     auth='Basic %s:g00dpassword' %
+    ...     nominater.preferredemail.email)
+    >>> logout()
+    >>> nominater_other_browser.open(
+    ...     "http://launchpad.dev/poseidon/+source/%s/+bug/%s/+nominate"; %
+    ...     (dsp.name, bug_task.bug.id))
+    >>> nominater_browser.getControl("Aqua").selected = True
+    >>> nominater_browser.getControl("Submit").click()
+    >>> for tag in find_tags_by_class(nominater_browser.contents, 'message'):
     ...     print tag
-    <div...Added nominations for: Ubuntu Grumpy...
+    <div...Added nominations for: Poseidon Aqua...
 
-Now, if no-priv, having the form open in another browser window,
-accidentally nominates the bug for Grumpy a second time, an error is
+Now, if the nominater, having the form open in another browser window,
+accidentally nominates the bug for Aqua a second time, an error is
 raised.
 
-    >>> nopriv_other_browser.getControl("Submit").click()
+    >>> nominater_other_browser.getControl("Aqua").selected = True
+    >>> nominater_other_browser.getControl("Submit").click()
 
-    >>> for tag in find_tags_by_class(nopriv_other_browser.contents, 'message'):
+    >>> for tag in find_tags_by_class(nominater_other_browser.contents,
+    ...     'message'):
     ...     print tag.renderContents()
     There is 1 error.
-    This bug has already been nominated for these series: Grumpy
+    This bug has already been nominated for these series: Aqua
 
 When a nomination is submitted by a privileged user, it is immediately
 approved and targeted to the release.
 
     >>> admin_browser.open(
-    ...     "http://launchpad.dev/distros/ubuntu/+source/mozilla-firefox/";
-    ...     "+bug/1/+nominate")
+    ...     "http://launchpad.dev/poseidon/+source/%s/+bug/%s/+nominate"; %
+    ...     (dsp.name, bug_task.bug.id))
 
-    >>> admin_browser.getControl("Warty").selected = True
+    >>> admin_browser.getControl("Hydro").selected = True
     >>> admin_browser.getControl("Submit").click()
 
     >>> for tag in find_tags_by_class(admin_browser.contents, 'message'):
     ...     print tag
-    <div...Targeted bug to: Ubuntu Warty...
+    <div...Targeted bug to: Poseidon Hydro...

=== modified file 'lib/lp/bugs/stories/bug-release-management/40-nominate-bug-for-productseries.txt'
--- lib/lp/bugs/stories/bug-release-management/40-nominate-bug-for-productseries.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/stories/bug-release-management/40-nominate-bug-for-productseries.txt	2010-10-18 15:16:16 +0000
@@ -1,51 +1,63 @@
-=Nominating a bug for a product series =
+= Nominating a bug for a product series =
 
 A bug can be nominated for a product series.
 
-    >>> user_browser.open(
-    ...     "http://launchpad.dev/products/firefox/+bug/4/+nominate";)
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> nominater = factory.makePerson(name='nominater',
+    ...     password='g00dpassword')
+    >>> widget = factory.makeProduct(name='widget',
+    ...     official_malone = True)
+    >>> series = factory.makeProductSeries(product=widget,
+    ...     name='beta')
+    >>> bug = factory.makeBug(product=widget)
+    >>> nominater_browser = setupBrowser(
+    ...     auth='Basic %s:g00dpassword' %
+    ...     nominater.preferredemail.email)
+    >>> logout()
+    >>> nominater_browser.open(
+    ...     "http://launchpad.dev/widget/+bug/%s/+nominate"; % bug.id)
 
 Before we continue, we'll set up a second browser instance, to simulate
-no-priv accessing the site from another window. Working with the same
-form in different browser windows or tabs can sometimes trigger edge
-case errors, and we'll give an example of one shortly.
-
-    >>> nopriv_other_browser = setupBrowser(
-    ...     auth="Basic no-priv@xxxxxxxxxxxxx:test")
-    >>> nopriv_other_browser.open(
-    ...     "http://launchpad.dev/products/firefox/+bug/4/+nominate";)
-
-    >>> user_browser.getControl("1.0").selected = True
-    >>> nopriv_other_browser.getControl("1.0").selected = True
-
-    >>> user_browser.getControl("Submit").click()
-
-    >>> for tag in find_tags_by_class(
-    ...     user_browser.contents, 'informational message'):
+the nominater accessing the site from another window. Working with the same
+form in different browser windows or tabs can sometimes trigger edge case
+errors, and we'll give an example of one shortly.
+
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> nominater_other_browser = setupBrowser(
+    ...     auth='Basic %s:g00dpassword' %
+    ...     nominater.preferredemail.email)
+    >>> logout()
+    >>> nominater_other_browser.open(
+    ...     "http://launchpad.dev/widget/+bug/%s/+nominate"; % bug.id)
+
+    >>> nominater_browser.getControl("Beta").selected = True
+    >>> nominater_other_browser.getControl("Beta").selected = True
+    >>> nominater_browser.getControl("Submit").click()
+
+    >>> for tag in find_tags_by_class(nominater_browser.contents, 'message'):
     ...     print tag
-    <div...Added nominations for: Mozilla Firefox 1.0...
-
-Now, if no-priv, having the form open in another browser window,
-accidentally nominates the bug for firefox 1.0 a second time, an error
-is raised.
-
-    >>> nopriv_other_browser.getControl("Submit").click()
-
-    >>> for tag in find_tags_by_class(nopriv_other_browser.contents, 'message'):
+    <div...Added nominations for: Widget beta...
+
+Now, if the nominater, having the form open in another browser window,
+accidentally nominates the bug for Beta a second time, an error is raised.
+
+    >>> nominater_other_browser.getControl("Submit").click()
+
+    >>> for tag in find_tags_by_class(nominater_other_browser.contents,
+    ...     'message'):
     ...     print tag.renderContents()
     There is 1 error.
-    This bug has already been nominated for these series: 1.0
+    This bug has already been nominated for these series: Beta
 
 When a nomination is submitted by a privileged user, it is immediately
 approved and targeted to the release.
 
     >>> admin_browser.open(
-    ...     "http://launchpad.dev/products/firefox/+bug/4/+nominate";)
+    ...     "http://launchpad.dev/widget/+bug/%s/+nominate"; % bug.id)
 
     >>> admin_browser.getControl("Trunk").selected = True
     >>> admin_browser.getControl("Submit").click()
 
-    >>> for tag in find_tags_by_class(
-    ...     admin_browser.contents, 'informational message'):
+    >>> for tag in find_tags_by_class(admin_browser.contents, 'message'):
     ...     print tag
-    <div...Targeted bug to: Mozilla Firefox trunk...
+    <div...Targeted bug to: Widget trunk...

=== modified file 'lib/lp/bugs/stories/bug-release-management/60-defer-product-bug.txt'
--- lib/lp/bugs/stories/bug-release-management/60-defer-product-bug.txt	2010-06-11 21:51:48 +0000
+++ lib/lp/bugs/stories/bug-release-management/60-defer-product-bug.txt	2010-10-18 15:16:16 +0000
@@ -2,6 +2,11 @@
 product task is no longer editable. Instead the status is tracked
 in the series task.
 
+    >>> admin_browser.open(
+    ...     "http://launchpad.dev/products/firefox/+bug/4/+nominate";)
+    >>> admin_browser.getControl("Trunk").selected = True
+    >>> admin_browser.getControl("Submit").click()
+
     >>> user_browser.open('http://bugs.launchpad.dev/firefox/+bug/4')
     >>> firefox_edit_url = (
     ...     'http://bugs.launchpad.dev/firefox/+bug/4/+editstatus')

=== added file 'lib/lp/bugs/tests/test_bugsupervisor_bugnomination.py'
--- lib/lp/bugs/tests/test_bugsupervisor_bugnomination.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bugsupervisor_bugnomination.py	2010-10-18 15:16:16 +0000
@@ -0,0 +1,80 @@
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests related to bug nominations for an object with a bug supervisor."""
+
+__metaclass__ = type
+
+from canonical.launchpad.ftests import (
+    login,
+    logout,
+    )
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+from lp.bugs.interfaces.bugnomination import NominationError
+from lp.testing import TestCaseWithFactory
+
+
+class AddNominationTestMixin:
+    """Test case mixin for IBug.addNomination."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(AddNominationTestMixin, self).setUp()
+        login('foo.bar@xxxxxxxxxxxxx')
+        self.user = self.factory.makePerson(name='ordinary-user')
+        self.bug_supervisor = self.factory.makePerson(name='no-ordinary-user')
+        self.owner = self.factory.makePerson(name='extraordinary-user')
+        self.setUpTarget()
+
+    def tearDown(self):
+        logout()
+        super(AddNominationTestMixin, self).tearDown()
+
+    def test_user_addNominationFor_series(self):
+        # A bug may not be nominated for a series of a product with an
+        # existing task by just anyone.
+        self.assertRaises(NominationError,
+            self.bug.addNomination, self.user, self.series)
+
+    def test_bugsupervisor_addNominationFor_series(self):
+        # A bug may be nominated for a series of a product with an
+        # exisiting task by the product's bug supervisor.
+        self.bug.addNomination(self.bug_supervisor, self.series)
+        self.assertTrue(len(self.bug.getNominations()), 1)
+
+    def test_owner_addNominationFor_series(self):
+        # A bug may be nominated for a series of a product with an
+        # exisiting task by the product's owner.
+        self.bug.addNomination(self.owner, self.series)
+        self.assertTrue(len(self.bug.getNominations()), 1)
+
+
+class TestBugAddNominationProductSeries(
+    AddNominationTestMixin, TestCaseWithFactory):
+    """Test IBug.addNomination for IProductSeries nominations."""
+
+    def setUpTarget(self):
+        self.product = self.factory.makeProduct(official_malone = True,
+            bug_supervisor=self.bug_supervisor,
+            owner=self.owner)
+        self.series = self.factory.makeProductSeries(product=self.product)
+        self.bug = self.factory.makeBug(product=self.product)
+        self.milestone = self.factory.makeMilestone(productseries=self.series)
+
+
+class TestBugAddNominationDistroSeries(
+    AddNominationTestMixin, TestCaseWithFactory):
+    """Test IBug.addNomination for IDistroSeries nominations."""
+
+    def setUpTarget(self):
+        self.distro = self.factory.makeDistribution(
+            bug_supervisor=self.bug_supervisor,
+            owner=self.owner)
+        self.series = self.factory.makeDistroRelease(distribution=self.distro)
+        # The factory can't create a distro bug directly.
+        self.bug = self.factory.makeBug()
+        self.bug.addTask(self.bug_supervisor, self.distro)
+        self.milestone = self.factory.makeMilestone(
+            distribution=self.distro)

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/interfaces/distribution.py	2010-10-18 15:16:16 +0000
@@ -670,7 +670,7 @@
 
     def new(name, displayname, title, description, summary, domainname,
             members, owner, mugshot=None, logo=None, icon=None):
-        """Creaste a new distribution."""
+        """Create a new distribution."""
 
 
 class NoSuchDistribution(NameLookupFailed):

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-10-14 12:57:25 +0000
+++ lib/lp/testing/factory.py	2010-10-18 15:16:16 +0000
@@ -1861,7 +1861,8 @@
         return diff
 
     def makeDistribution(self, name=None, displayname=None, owner=None,
-                         members=None, title=None, aliases=None):
+                         members=None, title=None, aliases=None,
+                         bug_supervisor=None):
         """Make a new distribution."""
         if name is None:
             name = self.getUniqueString(prefix="distribution")
@@ -1881,6 +1882,9 @@
             members, owner)
         if aliases is not None:
             removeSecurityProxy(distro).setAliases(aliases)
+        if bug_supervisor is not None:
+            naked_distro = removeSecurityProxy(distro)
+            naked_distro.bug_supervisor = bug_supervisor
         return distro
 
     def makeDistroRelease(self, distribution=None, version=None,