← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~brian-murray/launchpad/cherry-pick-bug-636412 into lp:~launchpad-pqm/launchpad/production-devel

 

Brian Murray has proposed merging lp:~brian-murray/launchpad/cherry-pick-bug-636412 into lp:~launchpad-pqm/launchpad/production-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Bug 636412 is about +subscribe for distributions with a bug supervisor set, Ubuntu, oops'ing for people who don't have permission to subscribe to Ubuntu.  This has been fixed on edge and has passed qa.  This is also something that should be fixed on production and this branch contains a cherry pick of the fix for bug 636412.
-- 
https://code.launchpad.net/~brian-murray/launchpad/cherry-pick-bug-636412/+merge/36066
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~brian-murray/launchpad/cherry-pick-bug-636412 into lp:~launchpad-pqm/launchpad/production-devel.
=== modified file 'lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt'
--- lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt	2010-05-21 04:38:42 +0000
+++ lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt	2010-09-20 20:23:49 +0000
@@ -194,3 +194,33 @@
     >>> anon_browser.open('http://launchpad.dev/bzr')
     >>> print anon_browser.title
     Bazaar Version Control System in Launchpad
+
+== Distribution with a bug supervisor ==
+
+If a distribution has a bug supervisor only that team or members of it can
+subscribe to all of the distribution's bugs.
+
+First, check the page content for a distribution without a bug supervisor.
+
+    >>> browser.open('http://bugs.launchpad.dev/ubuntu/+subscribe')
+    >>> text_contents = extract_text(find_main_content(browser.contents))
+    >>> "You can choose to receive an e-mail every time" in text_contents
+    True
+
+Set a bug supervisor for Ubuntu.
+
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> ubuntu = removeSecurityProxy(getUtility(IDistributionSet).get(1))
+    >>> guadamen = getUtility(IPersonSet).getByName('guadamen')
+    >>> ubuntu.bug_supervisor = guadamen
+    >>> flush_database_updates()
+    >>> logout()
+
+Second, check that the page content for a distribution with a bug supervisor
+contains a message about not being able to subscribe.
+
+    >>> browser.open('http://bugs.launchpad.dev/ubuntu/+subscribe')
+    >>> text_contents = extract_text(find_main_content(browser.contents))
+    >>> "You are unable to subscribe to bug reports about" in text_contents
+    True

=== modified file 'lib/lp/registry/browser/structuralsubscription.py'
--- lib/lp/registry/browser/structuralsubscription.py	2010-08-24 10:45:57 +0000
+++ lib/lp/registry/browser/structuralsubscription.py	2010-09-20 20:23:49 +0000
@@ -172,6 +172,10 @@
         """Return True, if the current user is subscribed."""
         return self.isSubscribed(self.user)
 
+    def userCanAlter(self):
+        if self.context.userCanAlterBugSubscription(self.user, self.user):
+            return True
+
     @action(u'Save these changes', name='save')
     def save_action(self, action, data):
         """Process the subscriptions submitted by the user."""

=== modified file 'lib/lp/registry/templates/structural-subscriptions-manage.pt'
--- lib/lp/registry/templates/structural-subscriptions-manage.pt	2009-12-05 18:37:28 +0000
+++ lib/lp/registry/templates/structural-subscriptions-manage.pt	2010-09-20 20:23:49 +0000
@@ -8,6 +8,20 @@
 >
 <body>
   <div metal:fill-slot="main">
+    <tal:no_permissions condition="not: view/userCanAlter|nothing">
+    <p>
+      You are unable to subscribe to bug reports about <span
+        tal:replace="context/title">this item</span> as it generates
+      a high amount of bug activity which results in more e-mails than
+      most users can handle.
+    </p>
+    <p>
+      If you really want to subscribe to <span
+        tal:replace="context/title">this item</span> bug mail than
+      please contact its bug supervisor.
+    </p>
+    </tal:no_permissions>
+    <tal:has_permissions condition="view/userCanAlter|nothing">
     <p>
       You can choose to receive an e-mail every time someone reports or
       changes a public bug associated with 
@@ -19,6 +33,7 @@
       time.
     </p>
     <div metal:use-macro="context/@@launchpad_form/form" />
+  </tal:has_permissions>
   </div>
   <div metal:fill-slot="side">
     <div tal:replace="structure context/@@+portlet-malone-bugmail-filtering-faq"/>

=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py	2010-09-17 00:53:33 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py	2010-09-20 20:23:49 +0000
@@ -7,6 +7,7 @@
 __all__ = [
     'close_bugs',
     'close_bugs_for_queue_item',
+    'close_bugs_for_sourcepackagerelease',
     'close_bugs_for_sourcepublication',
     'get_bugs_from_changes_file',
     'ProcessAccepted',
@@ -16,6 +17,7 @@
 import sys
 
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.webapp.errorlog import (
@@ -165,6 +167,14 @@
 
     janitor = getUtility(ILaunchpadCelebrities).janitor
     for bug in bugs_to_close:
+        # We need to remove the security proxy here because the bug
+        # might be private and if this code is called via someone using
+        # the +queue page they will get an OOPS.  Ideally, we should
+        # migrate this code to the Job system though, but that's a lot
+        # of work.  If you don't do that and you're changing stuff in
+        # here, BE CAREFUL with the unproxied bug object and look at
+        # what you're doing with it that might violate security.
+        bug = removeSecurityProxy(bug)
         edited_task = bug.setStatus(
             target=source_release.sourcepackage,
             status=BugTaskStatus.FIXRELEASED,

=== modified file 'lib/lp/soyuz/scripts/tests/test_queue.py'
--- lib/lp/soyuz/scripts/tests/test_queue.py	2010-08-27 12:21:25 +0000
+++ lib/lp/soyuz/scripts/tests/test_queue.py	2010-09-20 20:23:49 +0000
@@ -3,6 +3,8 @@
 
 """queue tool base class tests."""
 
+from __future__ import with_statement
+
 __metaclass__ = type
 __all__ = [
     'upload_bar_source',
@@ -12,6 +14,7 @@
 import hashlib
 import os
 import shutil
+from StringIO import StringIO
 import tempfile
 from unittest import (
     TestCase,
@@ -19,6 +22,7 @@
     )
 
 from zope.component import getUtility
+from zope.security.interfaces import ForbiddenAttribute
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
@@ -33,7 +37,10 @@
     fillLibrarianFile,
     )
 from canonical.librarian.utils import filechunks
-from canonical.testing import LaunchpadZopelessLayer
+from canonical.testing import (
+    DatabaseFunctionalLayer, 
+    LaunchpadZopelessLayer,
+    )
 from lp.archiveuploader.nascentupload import NascentUpload
 from lp.archiveuploader.tests import (
     datadir,
@@ -42,7 +49,10 @@
     mock_logger_quiet,
     )
 from lp.bugs.interfaces.bug import IBugSet
-from lp.bugs.interfaces.bugtask import IBugTaskSet
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    IBugTaskSet,
+    )
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -56,6 +66,9 @@
 from lp.soyuz.interfaces.archive import (
     IArchiveSet,
     )
+from lp.soyuz.scripts.processaccepted import (
+    close_bugs_for_sourcepackagerelease,
+    )
 from lp.soyuz.interfaces.queue import (
     IPackageUploadSet,
     )
@@ -64,6 +77,11 @@
     CommandRunnerError,
     name_queue_map,
     )
+from lp.testing import (
+    celebrity_logged_in,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 
 
 class TestQueueBase(TestCase):
@@ -926,6 +944,36 @@
             'override binary pmount', component_name='partner')
 
 
+class TestQueuePageClosingBugs(TestCaseWithFactory):
+    # The distroseries +queue page can close bug when accepting
+    # packages.  Unit tests for that belong here.
+
+    layer = DatabaseFunctionalLayer
+
+    def test_close_bugs_for_sourcepackagerelease_with_private_bug(self):
+        # lp.soyuz.scripts.processaccepted.close_bugs_for_sourcepackagerelease
+        # should work with private bugs where the person using the queue
+        # page doesn't have access to it.
+        changes_file_template = "Format: 1.7\nLaunchpad-bugs-fixed: %s\n"
+        # changelog_entry is required for an assertion inside the function
+        # we're testing.
+        spr = self.factory.makeSourcePackageRelease(changelog_entry="blah")
+        archive_admin = self.factory.makePerson()
+        bug = self.factory.makeBug(private=True)
+        bug_task = self.factory.makeBugTask(target=spr.sourcepackage, bug=bug)
+        changes = StringIO(changes_file_template % bug.id)
+
+        with person_logged_in(archive_admin):
+            # The archive admin user can't normally see this bug.
+            self.assertRaises(ForbiddenAttribute, bug, 'status')
+            # But the bug closure should work.
+            close_bugs_for_sourcepackagerelease(spr, changes)
+
+        # Verify it was closed.
+        with celebrity_logged_in("admin"):
+            self.assertEqual(bug_task.status, BugTaskStatus.FIXRELEASED)
+
+
 class TestQueueToolInJail(TestQueueBase):
     layer = LaunchpadZopelessLayer
     dbuser = config.uploadqueue.dbuser

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-09-10 03:20:48 +0000
+++ lib/lp/testing/factory.py	2010-09-20 20:23:49 +0000
@@ -2537,6 +2537,7 @@
                                  source_package_recipe_build=None,
                                  dscsigningkey=None,
                                  user_defined_fields=None,
+                                 changelog_entry=None,
                                  homepage=None):
         """Make a `SourcePackageRelease`."""
         if distroseries is None:
@@ -2594,7 +2595,7 @@
             build_conflicts_indep=build_conflicts_indep,
             architecturehintlist=architecturehintlist,
             changelog=None,
-            changelog_entry=None,
+            changelog_entry=changelog_entry,
             dsc=None,
             copyright=self.getUniqueString(),
             dscsigningkey=dscsigningkey,


Follow ups