Merge lp:~gmb/launchpad/include-bnl-bug-651108 into lp:launchpad

[Graham Binns]

A feature-flagged bug_notififcation_level field has been added to BugSubscriptionSubscribeSelfView.

This branch adds a feature-flagged bug_notification_level field to BugSubscriptionSubscribeSelf View. This is the first step in allowing users to chose what level their direct subscriptions work at.

== lib/lp/bugs/browser/bugsubscription.py ==

 - I've added the bug_notification_level field and feature-flagged it.
 - I've updated the subscribe_action() and _handleSubscribe() methods to deal with bug_notification_levels correctly.

== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==

 - I've added a test to show that when the malone.advanced-subscriptions.enabled flag is turned on, the bug_notification_level field appears in the UI.

== lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt ==

 - I've added a unit test to show that when the advanced subscriptions features are turned on the bug_notification_level field is used to create a subscription at the appropriate level.

[Brad Crittenden (bac)]

Hi Graham,

Thanks for this branch. I find it particularly interesting as I've not yet done nor reviewed an feature flag branches.

Please run the super new utilities/update-copyright script before landing. I think it only updates the files you've touched, some of which need it.

At line 144 of browser/bugsubscription.py a local variable is assigned but not used. I wonder why lint doesn't catch it? Speaking of, there are some lint issues that appear to be real.

I find the multiple tests in setUpWidgets to be confusing. Please factor out the test for is_subscribed and nest the remaining tests. It should simplify things a bit.

This comment is confusing/misleading as it implies the user can toggle the features:

183 + # When a user subscribes to a bug using the advanced features on
184 + # the Bug +subscribe page, the bug notification level they...

I think this version is more readable and accurate:

183 + # When a user subscribes to a bug with the advanced features on
184 + # the Bug +subscribe page enabled, the bug notification level they...

It is odd the current diff in the MP is showing a conflict in tales-cache.txt but none exists.

review: Approve (code)

[Brad Crittenden (bac)]

Actually, I need to retract my approval because the diff I am shown in the MP is inaccurate. Merging your branch into devel I see a diff of 898 lines where the diff in the MP only shows 283 lines. So assuming mine is correct, I've only reviewed about a 1/4 of your changes.

review: Needs Information (code)

[Brad Crittenden (bac)]

OK, so I just failed to see the pre-req branch. This aspect of it looks good. You *do* have a conflict in the pre-req branch.

review: Approve (code)

[Graham Binns (gmb)]

On Friday, October 22, 2010, Brad Crittenden <email address hidden> wrote:
> Review: Approve code
> OK, so I just failed to see the pre-req branch.  This aspect of it looks good.  You *do* have a conflict in the pre-req branch.

Thaks for the review. I'll make the changes you suggest. Note that the
prerequisite branch is just a slight modification of one of mars'S
branches, so I may have to just extract the code I need (the test
helper) in order to be able to land this branch.

--
Graham Binns
http://grahambinns.com

[lp:~gmb/launchpad/include-bnl-bug-651108]

11684. By Graham Binns on 2010-10-22

Review changes for brad.

11685. By Graham Binns on 2010-10-22

Reverted all but the FeatureFixture helper.

11686. By Graham Binns on 2010-10-22

De-linted.

11687. By Graham Binns on 2010-10-22

Slight code tweak.

11688. By Graham Binns on 2010-11-01

Merged mainline.

=== modified file 'lib/canonical/launchpad/doc/profiling.txt'
--- lib/canonical/launchpad/doc/profiling.txt	2010-11-01 11:25:43 +0000
+++ lib/canonical/launchpad/doc/profiling.txt	2010-08-26 15:57:56 +0000
@@ -100,10 +100,11 @@
 
 The feature has two modes.
 
--   It can be configured to optionally profile requests.  To turn this on you
-    must add a feature flag that sets the 'request_profiling' flag to 'on'.
-    This flag can be set directly in the database or using bin/harness.  See
-    https://dev.launchpad.net/LEP/FeatureFlags for the details.
+-   It can be configured to optionally profile requests.  To turn this on, in
+    ``launchpad-lazr.conf`` (e.g.,
+    ``configs/development/launchpad-lazr.conf``) , in the ``[profiling]``
+    section, set ``profiling_allowed: True``.  As of this writing, this
+    is the default value for development.
 
     Once it is turned on, you can insert /++profile++/ in the URL to get
     basic instructions on how to use the feature.  You use the
@@ -122,19 +123,6 @@
     lp/services/profile is hooked up properly.  This is intended to be a
     smoke test.  The unit tests verify further functionality.
 
-    # XXX mars 2010-09-23
-    # We will temporarily turn on the profiling feature so that this test
-    # still passes. Profiling is currently controlled using feature flags
-    # instead of config values. This fixture can be removed once profiling for
-    # developers is active by default once again.
-    >>> from lp.services.features.testing import FeatureFixture
-    >>> fixture = FeatureFixture({'request_profiling': 'on'})
-    >>> fixture.setUp()
-
-    >>> from lp.services.features import getFeatureFlag
-    >>> getFeatureFlag('request_profiling')
-    u'on'
-
     >>> response = http('GET /++profile++ HTTP/1.0')
     >>> '<h1>Profiling Information</h1>' in response.getBody()
     True
@@ -226,7 +214,3 @@
     >>> shutil.rmtree(pagetests_profile_dir)
     >>> shutil.rmtree(profile_dir)
     >>> old_config = config.pop('memory_profile')
-
-
-    # Turn profiling off
-    >>> fixture.cleanUp()

=== added file 'lib/lp/app/configure.zcml'
--- lib/lp/app/configure.zcml	1970-01-01 00:00:00 +0000
+++ lib/lp/app/configure.zcml	2010-09-29 12:39:28 +0000
@@ -0,0 +1,14 @@
+<!-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+     GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<configure
+    xmlns="http://namespaces.zope.org/zope"
+    xmlns:browser="http://namespaces.zope.org/browser"
+    xmlns:i18n="http://namespaces.zope.org/i18n"
+    xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
+    xmlns:lp="http://namespaces.canonical.com/lp"
+    i18n_domain="launchpad">
+    <include
+        package=".browser"/>
+</configure>

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2010-10-19 10:29:00 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2010-11-01 11:25:46 +0000
@@ -18,8 +18,6 @@
 from zope import formlib
 from zope.app.form import CustomWidgetFactory
 from zope.app.form.browser.itemswidgets import RadioWidget
-from zope.app.form.interfaces import IInputWidget
-from zope.app.form.utility import setUpWidget
 from zope.schema import Choice
 from zope.schema.vocabulary import (
     SimpleTerm,
@@ -30,7 +28,6 @@
 from canonical.launchpad.webapp import (
     action,
     canonical_url,
-    custom_widget,
     LaunchpadFormView,
     LaunchpadView,
     )
@@ -39,7 +36,6 @@
 from canonical.launchpad.webapp.menu import structured
 from lp.bugs.browser.bug import BugViewMixin
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
-from lp.registry.enum import BugNotificationLevel
 from lp.services import features
 from lp.services.propertycache import cachedproperty
 
@@ -85,10 +81,13 @@
     """A view to handle the +subscribe page for a bug."""
 
     schema = IBugSubscription
-    # XXX 2010-10-18 gmb bug=651108:
-    #     field_names is currently empty because we don't use any of
-    #     them.  This will be amended in a subsequent branch.
-    field_names = []
+
+    @property
+    def field_names(self):
+        if self._use_advanced_features:
+            return ['bug_notification_level']
+        else:
+            return []
 
     @property
     def next_url(self):
@@ -102,7 +101,7 @@
         # far as referrers are concerned so that we can handle privacy
         # issues properly.
         ignored_referrer_urls = (
-            'localhost', self.request.getURL(),  context_url)
+            'localhost', self.request.getURL(), context_url)
         if referer and referer not in ignored_referrer_urls:
             next_url = referer
         elif self._redirecting_to_bug_list:
@@ -112,7 +111,6 @@
         return next_url
 
     cancel_url = next_url
-    _redirecting_to_bug_list = False
 
     @cachedproperty
     def _subscribers_for_current_user(self):
@@ -128,10 +126,21 @@
         self._subscriber_count_for_current_user = person_count
         return persons_for_user.values()
 
+    @cachedproperty
+    def _use_advanced_features(self):
+        """Return True if advanced subscriptions features are enabled."""
+        return features.getFeatureFlag(
+            'malone.advanced-subscriptions.enabled')
+
+    def initialize(self):
+        """See `LaunchpadFormView`."""
+        self._subscriber_count_for_current_user = 0
+        self._redirecting_to_bug_list = False
+        super(BugSubscriptionSubscribeSelfView, self).initialize()
+
     def setUpFields(self):
         """See `LaunchpadFormView`."""
         super(BugSubscriptionSubscribeSelfView, self).setUpFields()
-        bug = self.context.bug
         if self.user is None:
             return
 
@@ -162,10 +171,41 @@
             __name__='subscription', title=_("Subscription options"),
             vocabulary=subscription_vocabulary, required=True,
             default=default_subscription_value)
-        self.form_fields += formlib.form.Fields(subscription_field)
+
+        if self._use_advanced_features:
+            # We need to pop the bug_notification_level field out of
+            # form_fields so that we can put the subscription_field first in
+            # the list. formlib.form.Fields doesn't have an insert() method.
+            bug_notification_level_field = self.form_fields.select(
+                'bug_notification_level')
+            self.form_fields = self.form_fields.omit('bug_notification_level')
+            self.form_fields += formlib.form.Fields(subscription_field)
+            self.form_fields += bug_notification_level_field
+            self.form_fields['bug_notification_level'].custom_widget = (
+                CustomWidgetFactory(RadioWidget))
+        else:
+            self.form_fields += formlib.form.Fields(subscription_field)
         self.form_fields['subscription'].custom_widget = CustomWidgetFactory(
             RadioWidget)
 
+    def setUpWidgets(self):
+        """See `LaunchpadFormView`."""
+        super(BugSubscriptionSubscribeSelfView, self).setUpWidgets()
+        if self._use_advanced_features:
+            if self.user_is_subscribed:
+                self.widgets['bug_notification_level'].visible = False
+            else:
+                if self._subscriber_count_for_current_user == 0:
+                    # We hide the subscription widget if the user isn't
+                    # subscribed, since we know who the subscriber is and we
+                    # don't need to present them with a single radio button.
+                    self.widgets['subscription'].visible = False
+                else:
+                    # We show the subscription widget when the user is
+                    # subscribed via a team, because they can either
+                    # subscribe theirself or unsubscribe their team.
+                    self.widgets['subscription'].visible = True
+
     @cachedproperty
     def user_is_subscribed(self):
         """Is the user subscribed to this bug?"""
@@ -195,14 +235,18 @@
         subscription_person = self.widgets['subscription'].getInputValue()
         if (not self.user_is_subscribed and
             (subscription_person == self.user)):
-            self._handleSubscribe()
+            if self._use_advanced_features:
+                bug_notification_level = data['bug_notification_level']
+            else:
+                bug_notification_level = None
+            self._handleSubscribe(bug_notification_level)
         else:
             self._handleUnsubscribe(subscription_person)
         self.request.response.redirect(self.next_url)
 
-    def _handleSubscribe(self):
+    def _handleSubscribe(self, level=None):
         """Handle a subscribe request."""
-        self.context.bug.subscribe(self.user, self.user)
+        self.context.bug.subscribe(self.user, self.user, level=level)
         self.request.response.addNotification(
             "You have been subscribed to this bug.")
 

=== added file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2010-11-01 11:25:46 +0000
@@ -0,0 +1,65 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for BugSubscription views."""
+
+__metaclass__ = type
+
+from storm.store import Store
+
+from canonical.launchpad.ftests import LaunchpadFormHarness
+from canonical.testing.layers import LaunchpadFunctionalLayer
+
+from lp.bugs.browser.bugsubscription import BugSubscriptionSubscribeSelfView
+from lp.bugs.model.bugsubscription import BugSubscription
+from lp.registry.enum import BugNotificationLevel
+from lp.services.features.testing import FeatureFixture
+from lp.testing import person_logged_in, TestCaseWithFactory
+
+
+class BugSubscriptionAdvancedFeaturesTestCase(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(BugSubscriptionAdvancedFeaturesTestCase, self).setUp()
+        self.useFixture(
+            FeatureFixture({
+                'malone.advanced-subscriptions.enabled': 'on'}))
+
+    def _getBugSubscriptionForUserAndBug(self, user, bug):
+        """Return the BugSubscription for a given user, bug combination."""
+        store = Store.of(bug)
+        return store.find(
+            BugSubscription,
+            BugSubscription.person == user,
+            BugSubscription.bug == bug).one()
+
+    def test_subscribe_uses_bug_notification_level(self):
+        # When a user subscribes to a bug using the advanced features on
+        # the Bug +subscribe page, the bug notification level they
+        # choose is taken into account.
+        bug = self.factory.makeBug()
+        # We unsubscribe the bug's owner because if we don't there will
+        # be two COMMENTS-level subscribers.
+        with person_logged_in(bug.owner):
+            bug.unsubscribe(bug.owner, bug.owner)
+
+        for level in BugNotificationLevel.items:
+            person = self.factory.makePerson()
+            with person_logged_in(person):
+                harness = LaunchpadFormHarness(
+                    bug.default_bugtask, BugSubscriptionSubscribeSelfView)
+                form_data = {
+                    'field.subscription': person.name,
+                    'field.bug_notification_level': level.name,
+                    }
+                harness.submit('continue', form_data)
+
+            subscription = self._getBugSubscriptionForUserAndBug(
+                person, bug)
+            self.assertEqual(
+                level, subscription.bug_notification_level,
+                "Bug notification level of subscription should be %s, is "
+                "actually %s." % (
+                    level.name, subscription.bug_notification_level.name))

=== added file 'lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt	2010-11-01 11:25:46 +0000
@@ -0,0 +1,42 @@
+Advanced personal subscriptions
+-------------------------------
+
+There are advanced features for bug subscriptions. These require a
+feature flag to be turned on for them to be accessible.
+
+    >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+    >>> feature_store = getFeatureStore()
+    >>> ignore = feature_store.add(FeatureFlag(
+    ...     scope=u'default',
+    ...     flag=u'malone.advanced-subscriptions.enabled',
+    ...     value=u'on', priority=1))
+    >>> feature_store.flush()
+
+Now when a user visits the +subscribe page of a bug they are given the
+option to subscribe to the bug at a given BugNotificationLevel.
+
+    >>> from canonical.launchpad.webapp import canonical_url
+    >>> from lp.testing.sampledata import USER_EMAIL
+    >>> login(USER_EMAIL)
+    >>> bug = factory.makeBug()
+    >>> logout()
+    >>> user_browser.open(
+    ...     canonical_url(bug.default_bugtask, view_name='+subscribe'))
+    >>> bug_notification_level_control = user_browser.getControl(
+    ...     name='field.bug_notification_level')
+    >>> for control in bug_notification_level_control.controls:
+    ...     print control.optionValue
+    NOTHING
+    LIFECYCLE
+    METADATA
+    COMMENTS
+
+The user can now subscribe to the bug at any of the given notification
+levels. In this case, they want to subscribe to just metadata updates:
+
+    >>> bug_notification_level_control.getControl('Details').click()
+    >>> user_browser.getControl('Continue').click()
+
+    >>> for message in find_tags_by_class(user_browser.contents, 'message'):
+    ...     print extract_text(message)
+    You have been subscribed to this bug.

=== modified file 'lib/lp/services/features/testing.py'
--- lib/lp/services/features/testing.py	2010-11-01 11:25:43 +0000
+++ lib/lp/services/features/testing.py	2010-11-01 11:25:46 +0000
@@ -66,4 +66,3 @@
         controller = FeatureController(lambda _: True, rule_source)
         per_thread.features = controller
         self.addCleanup(setattr, per_thread, 'features', original_controller)
-

=== modified file 'lib/lp/services/memcache/doc/tales-cache.txt'
--- lib/lp/services/memcache/doc/tales-cache.txt	2010-11-01 11:25:43 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt	2010-11-01 11:25:46 +0000
@@ -348,23 +348,17 @@
 Memcache in templates can be disabled entirely by setting the memcache flag to
 'disabled'.
 
-<<<<<<< TREE
     >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
     >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+    >>> from lp.services.features.webapp import ScopesFromRequest
+    >>> from lp.services.features.flags import FeatureController
     >>> from lp.services.features import per_thread
-    >>> from lp.services.features.flags import FeatureController
-    >>> from lp.services.features.webapp import ScopesFromRequest
     >>> ignore = getFeatureStore().add(FeatureFlag(
     ...     scope=u'default', flag=u'memcache', value=u'disabled',
     ...     priority=1))
     >>> empty_request = LaunchpadTestRequest()
     >>> per_thread.features = FeatureController(
     ...     ScopesFromRequest(empty_request).lookup)
-=======
-    >>> from lp.services.features.testing import FeatureFixture
-    >>> fixture = FeatureFixture({'memcache': 'disabled'})
-    >>> fixture.setUp()
->>>>>>> MERGE-SOURCE
 
 And now what cached before will not cache.
 
@@ -381,6 +375,3 @@
     <div>
         <span>second</span>
     </div>
-
-    # Clean up our custom flag settings.
-    >>> fixture.cleanUp()

=== modified file 'lib/lp/services/profile/profile.py'
--- lib/lp/services/profile/profile.py	2010-11-01 11:25:43 +0000
+++ lib/lp/services/profile/profile.py	2010-08-27 14:20:28 +0000
@@ -30,15 +30,12 @@
     memory,
     resident,
     )
-from lp.services import features
 
 
 class ProfilingOops(Exception):
     """Fake exception used to log OOPS information when profiling pages."""
 
 
-# This variable holds all of the data about an active profile run for the
-# duration of the request.
 _profilers = threading.local()
 
 
@@ -49,13 +46,8 @@
     If profiling is enabled, start a profiler for this thread. If memory
     profiling is requested, save the VSS and RSS.
     """
-    if features.getFeatureFlag('request_profiling'):
-        # Set a flag so that the end_request hook knows that is has some work
-        # to do.
-        _profilers.active = True
-    else:
+    if not config.profiling.profiling_allowed:
         return
-
     actions = get_desired_profile_actions(event.request)
     if config.profiling.profile_all_requests:
         actions.add('log')
@@ -79,7 +71,7 @@
 @adapter(IEndRequestEvent)
 def end_request(event):
     """If profiling is turned on, save profile data for the request."""
-    if not getattr(_profilers, 'active', False):
+    if not config.profiling.profiling_allowed:
         return
     try:
         actions = _profilers.actions

=== modified file 'lib/lp/services/profile/tests.py'
--- lib/lp/services/profile/tests.py	2010-11-01 11:25:43 +0000
+++ lib/lp/services/profile/tests.py	2010-08-26 22:10:56 +0000
@@ -14,6 +14,7 @@
 import time
 import unittest
 
+from lp.testing import TestCase
 from bzrlib.lsprof import BzrProfiler
 from zope.app.publication.interfaces import EndRequestEvent
 from zope.component import getSiteManager
@@ -25,11 +26,7 @@
     )
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.webapp.interfaces import StartRequestEvent
-from canonical.testing import layers
 from lp.services.profile import profile
-from lp.services.features.testing import FeatureFixture
-from lp.testing import TestCase
-
 
 EXAMPLE_HTML_START = '''\
 <html><head><title>Random!</title></head>
@@ -46,8 +43,6 @@
 
 class BaseTest(TestCase):
 
-    layer = layers.DatabaseFunctionalLayer
-
     def _get_request(self, path='/'):
         """Return a test request for the given path."""
         return LaunchpadTestRequest(PATH_INFO=path)
@@ -63,19 +58,13 @@
                 getattr(profile._profilers, name, None), None,
                 'Profiler state (%s) is dirty; %s.' % (name, message))
 
-    def turnOnProfiling(self):
-        """Turn on the request_profiling feature flag."""
-        self.useFixture(FeatureFixture({'request_profiling': 'on'}))
-
-    def turnOffProfiling(self):
-        """Turn off the request_profiling feature flag."""
-        self.useFixture(FeatureFixture({'request_profiling': None}))
-
     def pushProfilingConfig(
-        self, profile_all_requests='False', memory_profile_log=''):
+        self, profiling_allowed='False', profile_all_requests='False',
+        memory_profile_log=''):
         """This is a convenience for setting profile configs."""
         self.pushConfig(
             'profiling',
+            profiling_allowed=profiling_allowed,
             profile_all_requests=profile_all_requests,
             memory_profile_log=memory_profile_log)
 
@@ -97,12 +86,11 @@
                 delattr(profile._profilers, name)
         TestCase.tearDown(self)
 
-    def test_feature_flag_stops_profiling(self):
-        """The ``request_profiling`` feature flag should disable all
+    def test_config_stops_profiling(self):
+        """The ``profiling_allowed`` configuration should disable all
         profiling, even if it is requested"""
-        self.turnOffProfiling()
         self.pushProfilingConfig(
-            profile_all_requests='True',
+            profiling_allowed='False', profile_all_requests='True',
             memory_profile_log='.')
         profile.start_request(self._get_start_event('/++profile++show,log'))
         self.assertCleanProfilerState('config was ignored')
@@ -110,7 +98,7 @@
     def test_optional_profiling_without_marked_request_has_no_profile(self):
         """Even if profiling is allowed, it does not happen with a normal
         request."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         profile.start_request(self._get_start_event('/'))
         self.assertEqual(profile._profilers.actions, set())
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
@@ -120,7 +108,7 @@
     def test_optional_profiling_with_show_request_starts_profiling(self):
         """If profiling is allowed and a request with the "show" marker
         URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         profile.start_request(self._get_start_event('/++profile++show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -131,7 +119,7 @@
     def test_optional_profiling_with_log_request_starts_profiling(self):
         """If profiling is allowed and a request with the "log" marker
         URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         profile.start_request(self._get_start_event('/++profile++log/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -142,7 +130,7 @@
     def test_optional_profiling_with_combined_request_starts_profiling(self):
         """If profiling is allowed and a request with the "log" and
         "show" marker URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         profile.start_request(self._get_start_event('/++profile++log,show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -153,7 +141,7 @@
     def test_optional_profiling_with_reversed_request_starts_profiling(self):
         """If profiling is allowed and a request with the "show" and
         "log" marker URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         # The fact that this is reversed from the previous request is the only
         # difference from the previous test.  Also, it doesn't have a
         # trailing slash. :-P
@@ -166,8 +154,8 @@
 
     def test_forced_profiling_registers_action(self):
         """profile_all_requests should register as a log action"""
-        self.turnOnProfiling()
-        self.pushProfilingConfig(profile_all_requests='True')
+        self.pushProfilingConfig(
+            profiling_allowed='True', profile_all_requests='True')
         profile.start_request(self._get_start_event('/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -179,7 +167,7 @@
         """If profiling is allowed and a request with the marker URL segment
         is made incorrectly, profiling does not start and help is an action.
         """
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         profile.start_request(self._get_start_event('/++profile++/'))
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
         self.assertIs(
@@ -191,8 +179,8 @@
         """If profiling is forced and a request with the marker URL segment
         is made incorrectly, profiling starts and help is an action.
         """
-        self.turnOnProfiling()
-        self.pushProfilingConfig(profile_all_requests='True')
+        self.pushProfilingConfig(
+            profiling_allowed='True', profile_all_requests='True')
         profile.start_request(self._get_start_event('/++profile++/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -201,8 +189,8 @@
         self.assertEquals(profile._profilers.actions, set(('help', 'log')))
 
     def test_memory_profile_start(self):
-        self.turnOnProfiling()
-        self.pushProfilingConfig(memory_profile_log='.')
+        self.pushProfilingConfig(
+            profiling_allowed='True', memory_profile_log='.')
         profile.start_request(self._get_start_event('/'))
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
         self.assertIsInstance(profile._profilers.memory_profile_start, tuple)
@@ -210,8 +198,8 @@
         self.assertEqual(profile._profilers.actions, set())
 
     def test_combo_memory_and_profile_start(self):
-        self.turnOnProfiling()
-        self.pushProfilingConfig(memory_profile_log='.')
+        self.pushProfilingConfig(
+            profiling_allowed='True', memory_profile_log='.')
         profile.start_request(self._get_start_event('/++profile++show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIsInstance(profile._profilers.memory_profile_start, tuple)
@@ -279,11 +267,10 @@
     #########################################################################
     # Tests
     def test_config_stops_profiling(self):
-        """The ``request_profiling`` feature should disable all
+        """The ``profiling_allowed`` configuration should disable all
         profiling, even if it is requested"""
-        self.turnOffProfiling()
         self.pushProfilingConfig(
-            profile_all_requests='True',
+            profiling_allowed='False', profile_all_requests='True',
             memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show,log')
         self.assertIs(getattr(request, 'oops', None), None)
@@ -295,7 +282,7 @@
     def test_optional_profiling_without_marked_request_has_no_profile(self):
         """Even if profiling is allowed, it does not happen with a normal
         request."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/')
         self.assertIs(getattr(request, 'oops', None), None)
         self.assertEqual(self.getAddedResponse(request), '')
@@ -306,7 +293,7 @@
     def test_optional_profiling_with_show_request_profiles(self):
         """If profiling is allowed and a request with the "show" marker
         URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++show/')
         self.assertIsInstance(request.oops, ErrorReport)
         self.assertIn('Top Inline Time', self.getAddedResponse(request))
@@ -317,7 +304,7 @@
     def test_optional_profiling_with_log_request_profiles(self):
         """If profiling is allowed and a request with the "log" marker
         URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++log/')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -332,7 +319,7 @@
     def test_optional_profiling_with_combined_request_profiles(self):
         """If profiling is allowed and a request with the "log" and
         "show" marker URL segment is made, profiling starts."""
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++log,show')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -346,8 +333,8 @@
 
     def test_forced_profiling_logs(self):
         """profile_all_requests should register as a log action"""
-        self.turnOnProfiling()
-        self.pushProfilingConfig(profile_all_requests='True')
+        self.pushProfilingConfig(
+            profiling_allowed='True', profile_all_requests='True')
         request = self.endRequest('/')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -364,7 +351,7 @@
         """If profiling is allowed and a request with the marker URL segment
         is made incorrectly, profiling does not start and help is an action.
         """
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++')
         self.assertIs(getattr(request, 'oops', None), None)
         response = self.getAddedResponse(request)
@@ -378,8 +365,8 @@
         """If profiling is forced and a request with the marker URL segment
         is made incorrectly, profiling starts and help is an action.
         """
-        self.turnOnProfiling()
-        self.pushProfilingConfig(profile_all_requests='True')
+        self.pushProfilingConfig(
+            profiling_allowed='True', profile_all_requests='True')
         request = self.endRequest('/++profile++')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -395,8 +382,9 @@
 
     def test_memory_profile(self):
         "Does the memory profile work?"
-        self.turnOnProfiling()
-        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
+        self.pushProfilingConfig(
+            profiling_allowed='True',
+            memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/')
         self.assertIs(getattr(request, 'oops', None), None)
         self.assertEqual(self.getAddedResponse(request), '')
@@ -412,8 +400,9 @@
 
     def test_memory_profile_with_non_defaults(self):
         "Does the memory profile work with an oops and pageid?"
-        self.turnOnProfiling()
-        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
+        self.pushProfilingConfig(
+            profiling_allowed='True',
+            memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show/no-such-file',
                                   KeyError(), pageid='Foo')
         log = self.getMemoryLog()
@@ -424,8 +413,9 @@
         self.assertCleanProfilerState()
 
     def test_combo_memory_and_profile(self):
-        self.turnOnProfiling()
-        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
+        self.pushProfilingConfig(
+            profiling_allowed='True',
+            memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show/')
         self.assertIsInstance(request.oops, ErrorReport)
         self.assertIn('Top Inline Time', self.getAddedResponse(request))
@@ -434,7 +424,7 @@
         self.assertCleanProfilerState()
 
     def test_profiling_oops_is_informational(self):
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++show/')
         response = self.getAddedResponse(request)
         self.assertIsInstance(request.oops, ErrorReport)
@@ -443,7 +433,7 @@
         self.assertCleanProfilerState()
 
     def test_real_oops_trumps_profiling_oops(self):
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++show/no-such-file',
                                   KeyError('foo'))
         self.assertIsInstance(request.oops, ErrorReport)
@@ -454,7 +444,7 @@
         self.assertCleanProfilerState()
 
     def test_oopsid_is_in_profile_filename(self):
-        self.turnOnProfiling()
+        self.pushProfilingConfig(profiling_allowed='True')
         request = self.endRequest('/++profile++log/')
         self.assertIn("-" + request.oopsid + "-", self.getProfilePaths()[0])
         self.assertCleanProfilerState()