launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/delete-milestone-all-bugs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #899123 in Launchpad itself: "IntegrityError raised deleting a series"
  https://bugs.launchpad.net/launchpad/+bug/899123

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/delete-milestone-all-bugs/+merge/125924

Milestone:+delete made the assumption that whoever was deleting the milestone had the permissions to see every bugtask that referenced that milestone. I have extended BugTaskSearch to ignore privacy for this one call site and made use of rSP to make certain that we can unreference all bugtasks before we delete the milestone.
-- 
https://code.launchpad.net/~stevenk/launchpad/delete-milestone-all-bugs/+merge/125924
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/delete-milestone-all-bugs into lp:launchpad.

=== modified file 'lib/lp/bugs/interfaces/bugtasksearch.py'
--- lib/lp/bugs/interfaces/bugtasksearch.py	2012-09-17 16:13:40 +0000
+++ lib/lp/bugs/interfaces/bugtasksearch.py	2012-09-24 05:27:03 +0000
@@ -176,7 +176,7 @@
                  created_since=None, exclude_conjoined_tasks=False, cve=None,
                  upstream_target=None, milestone_dateexpected_before=None,
                  milestone_dateexpected_after=None, created_before=None,
-                 information_type=None):
+                 information_type=None, ignore_privacy=False):
 
         self.bug = bug
         self.searchtext = searchtext
@@ -236,6 +236,7 @@
             self.information_type = set((information_type,))
         else:
             self.information_type = None
+        self.ignore_privacy = ignore_privacy
 
     def setProduct(self, product):
         """Set the upstream context on which to filter the search."""

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-09-21 02:03:56 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-09-24 05:27:03 +0000
@@ -700,10 +700,11 @@
             extra_clauses.append(
                 Milestone.dateexpected <= dateexpected_before)
 
-    clause, decorator = _get_bug_privacy_filter_with_decorator(params.user)
-    if clause:
-        extra_clauses.append(clause)
-        decorators.append(decorator)
+    if not params.ignore_privacy:
+        clause, decorator = _get_bug_privacy_filter_with_decorator(params.user)
+        if clause:
+            extra_clauses.append(clause)
+            decorators.append(decorator)
 
     hw_clause = _build_hardware_related_clause(params)
     if hw_clause is not None:

=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py	2012-08-07 02:31:56 +0000
+++ lib/lp/registry/browser/__init__.py	2012-09-24 05:27:03 +0000
@@ -22,6 +22,7 @@
 
 from storm.store import Store
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.browser.folder import ExportedFolder
 from lp.app.browser.launchpadform import (
@@ -157,13 +158,14 @@
         """The context's URL."""
         return canonical_url(self.context)
 
-    def _getBugtasks(self, target):
+    def _getBugtasks(self, target, ignore_privacy=False):
         """Return the list `IBugTask`s associated with the target."""
         if IProductSeries.providedBy(target):
             params = BugTaskSearchParams(user=self.user)
             params.setProductSeries(target)
         else:
-            params = BugTaskSearchParams(milestone=target, user=self.user)
+            params = BugTaskSearchParams(
+                milestone=target, user=self.user, ignore_privacy=ignore_privacy)
         bugtasks = getUtility(IBugTaskSet).search(params)
         return list(bugtasks)
 
@@ -232,11 +234,15 @@
     def _deleteMilestone(self, milestone):
         """Delete a milestone and unlink related objects."""
         self._unsubscribe_structure(milestone)
-        for bugtask in self._getBugtasks(milestone):
-            if bugtask.conjoined_master is not None:
-                Store.of(bugtask).remove(bugtask.conjoined_master)
+        # We need to remove the milestone from every bug, even those the
+        # current user can't see/change, otherwise we can't delete the
+        # milestone, since it's still referenced.
+        for bugtask in self._getBugtasks(milestone, ignore_privacy=True):
+            nb = removeSecurityProxy(bugtask)
+            if nb.conjoined_master is not None:
+                Store.of(bugtask).remove(nb.conjoined_master)
             else:
-                bugtask.milestone = None
+                nb.milestone = None
         for spec in self._getSpecifications(milestone):
             spec.milestone = None
         self._deleteRelease(milestone.product_release)

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2012-09-18 19:41:02 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2012-09-24 05:27:03 +0000
@@ -14,6 +14,10 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
+from lp.registry.interfaces.accesspolicy import (
+    IAccessPolicyGrantSource,
+    IAccessPolicySource,
+    )
 from lp.registry.interfaces.person import TeamMembershipPolicy
 from lp.registry.model.milestonetag import ProjectGroupMilestoneTag
 from lp.services.config import config
@@ -184,6 +188,26 @@
             BugTaskSearchParams(user=None))
         self.assertEqual(0, tasks.count())
 
+    def test_delete_all_bugtasks(self):
+        # When a milestone is deleted, all bugtasks are deleted.
+        milestone = self.factory.makeMilestone()
+        self.factory.makeBug(milestone=milestone)
+        self.factory.makeBug(
+            milestone=milestone, information_type=InformationType.USERDATA)
+        # Remove the APG the product owner has so he can't see the private bug.
+        ap = getUtility(IAccessPolicySource).find(
+            [(milestone.product, InformationType.USERDATA)]).one()
+        getUtility(IAccessPolicyGrantSource).revoke(
+            [(ap, milestone.product.owner)])
+        form = {
+            'field.actions.delete': 'Delete Milestone',
+            }
+        with person_logged_in(milestone.product.owner):
+            view = create_initialized_view(milestone, '+delete', form=form)
+        self.assertEqual([], view.errors)
+        tasks = milestone.product.development_focus.searchTasks(
+            BugTaskSearchParams(user=None))
+        self.assertEqual(0, tasks.count())
 
 class TestQueryCountBase(TestCaseWithFactory):