Merge lp:~wallyworld/launchpad/delete-bugtasks-1324 into lp:launchpad

[Ian Booth]

Add a bugtask delete method to the IBugTask API and also export to the web service.

Add a bugtask delete method to the IBugTask API and also export to the web service.

A followup branch will add an entry to the bug activity log when a task is deleted and an email will be sent to the bug supervisor of the pillar for which the task is deleted.

== Preimplementation ==

Talked with lifeless and wgrant. lifeless was concerned about deleting a bugtask if it were a conjoined master/slave. wgrant said this doesn't matter due to recent changes to allow such 'orphaned' bugtasks.

== Implementation ==

The delete implementation was done in a destroySelf method on BugTask. This fits with how other entities are deleted in Launchpad (even though my preference is for the lifecycle of entities to be managed by a service based interface rather than the entiy itself). In terms of security, a new Launchpad permission is introduced - "launchpad.Delete". I feel it is better more semantically correct to use this new permission rather than just piggyback onto the launchpad.Edit permission.
A security adaptor is provided to check the necessary role based permissions. A new interface is introduced to allow the security and permissions to be wired up that that everything works in both the server side and web service api - IBugTaskDelete.

There are also other business rules which stop a bugtask from being deleted eg the last bugtask cannot be deleted. These rules are checked in the canBeDeleted() method and a CannotDeleteBugtask exception is raised in such cases.

A feature flag is used to hide this operation - 'disclosure.delete_bugtask.enabled'. During testing, a problem was found with the feature flag infrastructure. The StormFeatureRulesSource.setAllRules() method needs to call flush(), or else the rules passed in when the fixture is set up are not guaranteed to be in the database when required. A case in point: the check_permission method uses a @block_implicit_flushes decorator.

== Demo and QA ==

An example script to invoke the api using launchpadlib:

from launchpadlib.launchpad import Launchpad

def test_delete_bugtask(bug_num, pillar_name):
    lp = Launchpad.login_with(
        'testing', service_root=uris.LPNET_SERVICE_ROOT, version='devel')
    bug = lp.bugs[bug_num]
    bugtasks = bug.bug_tasks
    if len(bugtasks) == 1:
        print "This bug has no extra bugtasks to be deleted."
    else:
        for bugtask in bugtasks:
            if bugtask.target.name == pillar_name:
                bugtask.lp_delete()
                print "deleted task for %s on bug %d." % (pillar_name, bug_num)

== Tests ==

Add a new testcase TestBugTaskDeletion with some tests:

test_cannot_delete_if_not_logged_in
test_unauthorised_cannot_delete
test_pillar_owner_can_delete
test_bug_supervisor_can_delete
test_task_reporter_can_delete
test_cannot_delete_only_bugtask
test_delete_bugtask
test_delete_default_bugtask
test_bug_heat_updated

Add a testcase for the web service interface: TestWebService

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/permissions.zcml
  lib/lp/bugs/configure.zcml
  lib/lp/bugs/security.py
  lib/lp/bugs/interfaces/bugtask.py
  lib/lp/bugs/model/bugtask.py
  lib/lp/bugs/model/tests/test_bugtask.py
  lib/lp/services/features/flags.py
  lib/lp/services/features/rulesource.py

[Данило Шеган (danilo)]

Looks pretty good: very nice to see this happening.

I do have a few observations/questions that I'd like addressed first.

If concerns were raised about conjoined bug tasks, I suppose it would be nice to have a test for that, though I believe nothing would really be affected (a slave would lose a master and vice versa, but that's about it).

Next, I wonder if there was a good reason not to use launchpad.Admin privilege instead? I.e. the way you implement Delete for BugTask seems limited to checking permissions on the context. Real admins (like LOSAs) will not be able to delete bug tasks, if I am reading it correctly.

I also don't feel like checking for feature flags belongs in the permissions code: you don't have a permission because feature is not enabled? Just sounds wrong to me, no big deal otherwise. Tied to that, perhaps tests should ensure UnauthorizedError is raised when delete is attempted instead of using check_permission as well?

Also, I'd prefer if tests which contain logic for when both feature flag is on and off are split into two separate tests each.

Also, is adding a store.flush() really necessary to the rulesource.py? If you need it for tests, maybe it's not necessary for production (and would introduce a query or two that we could avoid, if updates to a single object happen before and after flags are modified). Not a big deal though.

Thanks for the lint fixes :)

review: Needs Fixing

[Ian Booth (wallyworld)]

Thanks for the thorough review :-)

>
> If concerns were raised about conjoined bug tasks, I suppose it would be nice
> to have a test for that, though I believe nothing would really be affected (a
> slave would lose a master and vice versa, but that's about it).
>

As discussed on irc, I think we are ok here. wgrant showed me a demo on qas where the code and ui didn't care if a conjoined pair is "broken"

> Next, I wonder if there was a good reason not to use launchpad.Admin privilege
> instead? I.e. the way you implement Delete for BugTask seems limited to
> checking permissions on the context. Real admins (like LOSAs) will not be able
> to delete bug tasks, if I am reading it correctly.
>

I think this is a good point to raise and we need to add support for launchpad.Admin to be able to delete bug tasks.

> I also don't feel like checking for feature flags belongs in the permissions
> code: you don't have a permission because feature is not enabled? Just sounds
> wrong to me, no big deal otherwise. Tied to that, perhaps tests should ensure
> UnauthorizedError is raised when delete is attempted instead of using
> check_permission as well?
>

My understanding of the requirements is that if the fflag is off, then it is to be treated as if the user doesn't have permission and the implementation does achieve that goal. If I am wrong, I'll change it.

> Also, I'd prefer if tests which contain logic for when both feature flag is on
> and off are split into two separate tests each.
>

As discussed on irc, the fflag will (hopefully) be temporary and when it's removed, the tests will be updated. So it seems ok to me to leave as is. It seems easier to reduce the setup duplication and just remove the fflag bit later on.

> Also, is adding a store.flush() really necessary to the rulesource.py? If you
> need it for tests, maybe it's not necessary for production (and would
> introduce a query or two that we could avoid, if updates to a single object
> happen before and after flags are modified). Not a big deal though.
>

It is needed for tests for sure. I don't think we use that same code path in prod so I don't believe it's an issue there. And even if we do, all we are doing is writing to the db sooner than is otherwise the case.

[Ian Booth (wallyworld)]

>
> > Next, I wonder if there was a good reason not to use launchpad.Admin
> privilege
> > instead? I.e. the way you implement Delete for BugTask seems limited to
> > checking permissions on the context. Real admins (like LOSAs) will not be
> able
> > to delete bug tasks, if I am reading it correctly.
> >
>
> I think this is a good point to raise and we need to add support for
> launchpad.Admin to be able to delete bug tasks.
>

I've added a security adapter to allow lp admins to also delete bug tasks.

[Данило Шеган (danilo)]

Since I don't see you on IRC and I might be leaving soon, here's what I typed there:
 * I don't think the way you introduced security adapter is sufficient: in my reading, it modifies the launchpad.Admin privilege for BugTask to match the launchpad.Delete privilege, but does not really allow admins to do the delete
 * your test is not really testing what I thought it should (i.e. log in as <email address hidden> and not someone holding a contextual launchpad.Admin permission [like target owner])
 * (or better yet, login_celebrity('admin') from the same module login is in)

Note that I might be wrong and your test would pass even with the permissions as-is because admins are always allowed. Which would indicate that we have plenty of permission definitions which check for is_admin needlessly.

If I am not very responsive, feel free to find a different reviewer: I wouldn't want this branch to be held back because of me! :)

[Ian Booth (wallyworld)]

> Since I don't see you on IRC and I might be leaving soon, here's what I typed
> there:
> * I don't think the way you introduced security adapter is sufficient: in my
> reading, it modifies the launchpad.Admin privilege for BugTask to match the
> launchpad.Delete privilege, but does not really allow admins to do the delete
> * your test is not really testing what I thought it should (i.e. log in as
> <email address hidden> and not someone holding a contextual launchpad.Admin
> permission [like target owner])
> * (or better yet, login_celebrity('admin') from the same module login is in)
>

You are right - my test was flawed and the security adaptor wasn't quite right. They're both fixed now.

[Curtis Hovey (sinzui)]

Thank you for including Danilo's suggestion. This looks good to land.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/permissions.zcml'
--- lib/canonical/launchpad/permissions.zcml	2010-12-10 23:10:09 +0000
+++ lib/canonical/launchpad/permissions.zcml	2011-10-19 22:37:25 +0000
@@ -27,6 +27,9 @@
     id="launchpad.Append" title="Adding something" access_level="write" />
 
   <permission
+    id="launchpad.Delete" title="Deleting something" access_level="write" />
+
+  <permission
     id="launchpad.Moderate" title="Moderate something" access_level="write" />
 
   <permission

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-10-05 18:02:45 +0000
+++ lib/lp/bugs/configure.zcml	2011-10-19 22:37:25 +0000
@@ -236,6 +236,9 @@
                     findSimilarBugs
                     getContributorInfo"/>
             <require
+                permission="launchpad.Delete"
+                interface="lp.bugs.interfaces.bugtask.IBugTaskDelete"/>
+            <require
                 permission="launchpad.Edit"
                 attributes="
                     setStatusFromDebbugs

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-10-11 11:11:10 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-10-19 22:37:25 +0000
@@ -17,6 +17,7 @@
     'BugTaskStatus',
     'BugTaskStatusSearch',
     'BugTaskStatusSearchDisplay',
+    'CannotDeleteBugtask',
     'DB_INCOMPLETE_BUGTASK_STATUSES',
     'DB_UNRESOLVED_BUGTASK_STATUSES',
     'DEFAULT_SEARCH_BUGTASK_STATUSES_FOR_DISPLAY',
@@ -24,6 +25,7 @@
     'IAddBugTaskForm',
     'IAddBugTaskWithProductCreationForm',
     'IBugTask',
+    'IBugTaskDelete',
     'IBugTaskDelta',
     'IBugTaskSearch',
     'IBugTaskSet',
@@ -58,6 +60,7 @@
     call_with,
     error_status,
     export_as_webservice_entry,
+    export_destructor_operation,
     export_read_operation,
     export_write_operation,
     exported,
@@ -435,6 +438,15 @@
     for item in DEFAULT_SEARCH_BUGTASK_STATUSES]
 
 
+@error_status(httplib.BAD_REQUEST)
+class CannotDeleteBugtask(Exception):
+    """The bugtask cannot be deleted.
+
+    Raised when a user tries to delete a bugtask but the deletion cannot
+    proceed because of a model constraint or other business rule violation.
+    """
+
+
 @error_status(httplib.UNAUTHORIZED)
 class UserCannotEditBugTaskStatus(Unauthorized):
     """User not permitted to change status.
@@ -482,7 +494,23 @@
     in a search for related bug tasks"""
 
 
-class IBugTask(IHasDateCreated, IHasBug):
+class IBugTaskDelete(Interface):
+    """An interface for operations allowed with the Delete permission."""
+    @export_destructor_operation()
+    @call_with(who=REQUEST_USER)
+    @operation_for_version('devel')
+    def delete(who):
+        """Delete this bugtask.
+
+        :param who: the user who is removing the bugtask.
+        :raises: CannotDeleteBugtask if the bugtask cannot be deleted due to a
+            business rule or other model constraint.
+        :raises: Unauthorized if the user does not have permission
+            to delete the bugtask.
+        """
+
+
+class IBugTask(IHasDateCreated, IHasBug, IBugTaskDelete):
     """A bug needing fixing in a particular product or package."""
     export_as_webservice_entry()
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-10-19 20:19:27 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-10-19 22:37:25 +0000
@@ -114,6 +114,7 @@
     BugTaskSearchParams,
     BugTaskStatus,
     BugTaskStatusSearch,
+    CannotDeleteBugtask,
     DB_INCOMPLETE_BUGTASK_STATUSES,
     DB_UNRESOLVED_BUGTASK_STATUSES,
     get_bugtask_status,
@@ -620,6 +621,28 @@
         """
         return self._status in RESOLVED_BUGTASK_STATUSES
 
+    def canBeDeleted(self):
+        num_bugtasks = Store.of(self).find(
+            BugTask, bug=self.bug).count()
+
+        return num_bugtasks > 1
+
+    def delete(self, who=None):
+        """See `IBugTask`."""
+        if who is None:
+            who = getUtility(ILaunchBag).user
+
+        if not self.canBeDeleted():
+            raise CannotDeleteBugtask(
+                "Cannot delete bugtask: %s" % self.title)
+        bug = self.bug
+        target = self.target
+        self.destroySelf()
+        del get_property_cache(bug).bugtasks
+
+        # When a task is deleted the bug's heat needs to be recalculated.
+        target.recalculateBugHeatCache()
+
     def findSimilarBugs(self, user, limit=10):
         """See `IBugTask`."""
         if self.product is not None:

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-10-18 21:11:23 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-10-19 22:37:25 +0000
@@ -4,11 +4,16 @@
 __metaclass__ = type
 
 from datetime import timedelta
+import transaction
 import unittest
 
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
+<<<<<<< TREE
 from testtools.testcase import ExpectedException
+=======
+from lazr.restfulclient.errors import Unauthorized
+>>>>>>> MERGE-SOURCE
 from testtools.matchers import Equals
 from zope.component import getUtility
 from zope.event import notify
@@ -21,8 +26,13 @@
     any,
     not_equals,
     )
+from canonical.launchpad.webapp.authorization import (
+    check_permission,
+    clear_cache,
+    )
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.testing.layers import (
+    AppServerLayer,
     DatabaseFunctionalLayer,
     LaunchpadZopelessLayer,
     )
@@ -34,6 +44,7 @@
     BugTaskImportance,
     BugTaskSearchParams,
     BugTaskStatus,
+    CannotDeleteBugtask,
     DB_UNRESOLVED_BUGTASK_STATUSES,
     IBugTaskSet,
     RESOLVED_BUGTASK_STATUSES,
@@ -61,12 +72,14 @@
     )
 from lp.registry.interfaces.product import IProductSet
 from lp.registry.interfaces.projectgroup import IProjectGroupSet
+from lp.services.features.testing import FeatureFixture
 from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.testing import (
     ANONYMOUS,
     EventRecorder,
     feature_flags,
     login,
+    login_celebrity,
     login_person,
     logout,
     normalize_whitespace,
@@ -75,6 +88,7 @@
     StormStatementRecorder,
     TestCase,
     TestCaseWithFactory,
+    ws_object,
     )
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.fakemethod import FakeMethod
@@ -1456,6 +1470,119 @@
             bug.default_bugtask.pillar.displayname, result['pillar_name'])
 
 
+class TestBugTaskDeletion(TestCaseWithFactory):
+    """Test the different cases that makes a bugtask deletable or not."""
+
+    layer = DatabaseFunctionalLayer
+
+    flags = {u"disclosure.delete_bugtask.enabled": u"on"}
+
+    def test_cannot_delete_if_not_logged_in(self):
+        # You cannot delete a bug task if not logged in.
+        bug = self.factory.makeBug()
+        with FeatureFixture(self.flags):
+            self.assertFalse(
+                check_permission('launchpad.Delete', bug.default_bugtask))
+
+    def test_unauthorised_cannot_delete(self):
+        # Unauthorised users cannot delete a bug task.
+        bug = self.factory.makeBug()
+        unauthorised = self.factory.makePerson()
+        login_person(unauthorised)
+        with FeatureFixture(self.flags):
+            self.assertFalse(
+                check_permission('launchpad.Delete', bug.default_bugtask))
+
+    def test_admin_can_delete(self):
+        # With the feature flag on, an admin can delete a bug task.
+        bug = self.factory.makeBug()
+        login_celebrity('admin')
+        with FeatureFixture(self.flags):
+            self.assertTrue(
+                check_permission('launchpad.Admin', bug.default_bugtask))
+        # Admins can also the task even without the feature flag.
+        clear_cache()
+        self.assertTrue(
+            check_permission('launchpad.Admin', bug.default_bugtask))
+
+    def test_pillar_owner_can_delete(self):
+        # With the feature flag on, the pillar owner can delete a bug task.
+        bug = self.factory.makeBug()
+        login_person(bug.default_bugtask.pillar.owner)
+        with FeatureFixture(self.flags):
+            self.assertTrue(
+                check_permission('launchpad.Delete', bug.default_bugtask))
+        # They can't delete the task without the feature flag.
+        clear_cache()
+        self.assertFalse(
+            check_permission('launchpad.Delete', bug.default_bugtask))
+
+    def test_bug_supervisor_can_delete(self):
+        # With the feature flag on, the bug supervisor can delete a bug task.
+        bug_supervisor = self.factory.makePerson()
+        product = self.factory.makeProduct(bug_supervisor=bug_supervisor)
+        bug = self.factory.makeBug(product=product)
+        login_person(bug_supervisor)
+        with FeatureFixture(self.flags):
+            self.assertTrue(
+                check_permission('launchpad.Delete', bug.default_bugtask))
+        # They can't delete the task without the feature flag.
+        clear_cache()
+        self.assertFalse(
+            check_permission('launchpad.Delete', bug.default_bugtask))
+
+    def test_task_reporter_can_delete(self):
+        # With the feature flag on, the bug task reporter can delete bug task.
+        bug = self.factory.makeBug()
+        login_person(bug.default_bugtask.owner)
+        with FeatureFixture(self.flags):
+            self.assertTrue(
+                check_permission('launchpad.Delete', bug.default_bugtask))
+        # They can't delete the task without the feature flag.
+        clear_cache()
+        self.assertFalse(
+            check_permission('launchpad.Delete', bug.default_bugtask))
+
+    def test_cannot_delete_only_bugtask(self):
+        # The only bugtask cannot be deleted.
+        bug = self.factory.makeBug()
+        bugtask = bug.default_bugtask
+        login_person(bugtask.owner)
+        with FeatureFixture(self.flags):
+            self.assertRaises(CannotDeleteBugtask, bugtask.delete)
+
+    def test_delete_bugtask(self):
+        # A bugtask can be deleted.
+        bugtask = self.factory.makeBugTask()
+        bug = bugtask.bug
+        login_person(bugtask.owner)
+        with FeatureFixture(self.flags):
+            bugtask.delete()
+        self.assertEqual([bug.default_bugtask], bug.bugtasks)
+
+    def test_delete_default_bugtask(self):
+        # The default bugtask can be deleted.
+        bugtask = self.factory.makeBugTask()
+        bug = bugtask.bug
+        login_person(bug.default_bugtask.owner)
+        with FeatureFixture(self.flags):
+            bug.default_bugtask.delete()
+        self.assertEqual([bugtask], bug.bugtasks)
+        self.assertEqual(bugtask, bug.default_bugtask)
+
+    def test_bug_heat_updated(self):
+        # Test that the bug heat is updated when a bugtask is deleted.
+        bug = self.factory.makeBug()
+        distro = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(distribution=distro)
+        login_person(distro.owner)
+        dsp_task = bug.addTask(bug.owner, dsp)
+        self.assertTrue(dsp.total_bug_heat > 0)
+        with FeatureFixture(self.flags):
+            dsp_task.delete()
+        self.assertTrue(dsp.total_bug_heat == 0)
+
+
 class TestConjoinedBugTasks(TestCaseWithFactory):
     """Tests for conjoined bug task functionality."""
 
@@ -2323,3 +2450,33 @@
             "package in which the bug has not yet been reported."
             % d.displayname,
             validate_new_target, task.bug, d)
+
+
+class TestWebservice(TestCaseWithFactory):
+    """Tests for the webservice."""
+
+    layer = AppServerLayer
+
+    def test_delete_bugtask(self):
+        """Test that a bugtask can be deleted with the feature flag on."""
+        product = self.factory.makeProduct()
+        owner = self.factory.makePerson()
+        db_bugtask = self.factory.makeBugTask(target=product, owner=owner)
+        db_bug = db_bugtask.bug
+        transaction.commit()
+        logout()
+
+        # It will fail without feature flag enabled.
+        launchpad = self.factory.makeLaunchpadService(owner)
+        bugtask = ws_object(launchpad, db_bugtask)
+        self.assertRaises(Unauthorized, bugtask.lp_delete)
+
+        flags = {u"disclosure.delete_bugtask.enabled": u"on"}
+        with FeatureFixture(flags):
+            launchpad = self.factory.makeLaunchpadService(owner)
+            bugtask = ws_object(launchpad, db_bugtask)
+            bugtask.lp_delete()
+            transaction.commit()
+        # Check the delete really worked.
+        with person_logged_in(removeSecurityProxy(db_bug).owner):
+            self.assertEqual([db_bug.default_bugtask], db_bug.bugtasks)

=== modified file 'lib/lp/bugs/security.py'
--- lib/lp/bugs/security.py	2011-09-07 21:53:15 +0000
+++ lib/lp/bugs/security.py	2011-10-19 22:37:25 +0000
@@ -6,8 +6,11 @@
 __metaclass__ = type
 __all__ = []
 
+from zope.component import getUtility
+
 from canonical.launchpad.interfaces.launchpad import IHasBug
 from lp.services.messages.interfaces.message import IMessage
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.security import (
     AnonymousAuthorization,
     AuthorizationBase,
@@ -19,9 +22,13 @@
 from lp.bugs.interfaces.bugnomination import IBugNomination
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
 from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
+from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
+from lp.bugs.interfaces.bugtask import IBugTaskDelete
 from lp.bugs.interfaces.bugtracker import IBugTracker
 from lp.bugs.interfaces.bugwatch import IBugWatch
 from lp.bugs.interfaces.structuralsubscription import IStructuralSubscription
+from lp.registry.interfaces.role import IHasOwner
+from lp.services.features import getFeatureFlag
 
 
 class EditBugNominationStatus(AuthorizationBase):
@@ -47,6 +54,50 @@
         return self.obj.bug.userCanView(user)
 
 
+class DeleteBugTask(AuthorizationBase):
+    permission = 'launchpad.Delete'
+    usedfor = IBugTaskDelete
+
+    def checkAuthenticated(self, user):
+        """Check that a user may delete a bugtask.
+
+        A user may delete a bugtask if:
+         - The disclosure.delete_bugtask.enabled feature flag is enabled,
+         and they are:
+         - project maintainer
+         - task creator
+         - bug supervisor
+        """
+        if user is None:
+            return False
+
+        # Admins can always delete bugtasks.
+        if user.inTeam(getUtility(ILaunchpadCelebrities).admin):
+            return True
+
+        delete_allowed = bool(getFeatureFlag(
+            'disclosure.delete_bugtask.enabled'))
+        if not delete_allowed:
+            return False
+
+        bugtask = self.obj
+        owner = None
+        if IHasOwner.providedBy(bugtask.pillar):
+            owner = bugtask.pillar.owner
+        bugsupervisor = None
+        if IHasBugSupervisor.providedBy(bugtask.pillar):
+            bugsupervisor = bugtask.pillar.bug_supervisor
+        return (
+            user.inTeam(owner) or
+            user.inTeam(bugsupervisor) or
+            user.inTeam(bugtask.owner))
+
+
+class AdminDeleteBugTask(DeleteBugTask):
+    """Launchpad admins can also delete bug tasks."""
+    permission = 'launchpad.Admin'
+
+
 class PublicToAllOrPrivateToExplicitSubscribersForBugTask(AuthorizationBase):
     permission = 'launchpad.View'
     usedfor = IHasBug

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-10-14 05:00:27 +0000
+++ lib/lp/services/features/flags.py	2011-10-19 22:37:25 +0000
@@ -143,6 +143,10 @@
      ('Enables the auto subscribing and unsubscribing of users as a bug '
       'transitions between public, private and security related states.'),
      ''),
+    ('disclosure.delete_bugtask.enabled',
+     'boolean',
+     ('Enables bugtasks to be deleted by authorised users.'),
+     ''),
     ('bugs.autoconfirm.enabled_distribution_names',
      'space delimited',
      ('Enables auto-confirming bugtasks for distributions (and their '

=== modified file 'lib/lp/services/features/rulesource.py'
--- lib/lp/services/features/rulesource.py	2011-04-27 02:27:20 +0000
+++ lib/lp/services/features/rulesource.py	2011-10-19 22:37:25 +0000
@@ -87,7 +87,8 @@
     def parseRules(self, text_form):
         """Return a list of tuples for the parsed form of the text input.
 
-        For each non-blank line gives back a tuple of (flag, scope, priority, value).
+        For each non-blank line gives back a tuple of
+        (flag, scope, priority, value).
 
         Returns a list rather than a generator so that you see any syntax
         errors immediately.
@@ -139,8 +140,8 @@
 
         :param new_rules: List of (name, scope, priority, value) tuples.
         """
-        # XXX: would be slightly better to only update rules as necessary so we keep
-        # timestamps, and to avoid the direct sql etc -- mbp 20100924
+        # XXX: would be slightly better to only update rules as necessary so
+        # we keep timestamps, and to avoid the direct sql etc -- mbp 20100924
         store = getFeatureStore()
         store.execute('DELETE FROM FeatureFlag')
         for (flag, scope, priority, value) in new_rules:
@@ -149,6 +150,7 @@
                 flag=unicode(flag),
                 value=value,
                 priority=priority))
+        store.flush()
 
 
 class NullFeatureRuleSource(FeatureRuleSource):