launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/lptestrequest into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #758649 in Launchpad itself: "LaunchpadTestRequest should not always set features to NullFeatureController"
  https://bugs.launchpad.net/launchpad/+bug/758649

For more details, see:
https://code.launchpad.net/~bac/launchpad/lptestrequest/+merge/57484

= Summary =

LaunchpadTestRequest always uses NullFeatureController which causes all
feature flags in a page template rendering to be off.

== Proposed fix ==

In LTR self.features should be the current relevant feature controller
if one exists otherwise the NullFeatureController.

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

bin/test -vvt test_subscription_links

This test previously had a work-around to fix the problem locally.  That
work-around has been removed and the test still works.

== Demo and Q/A ==

N/A

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/webapp/servers.py
  lib/lp/registry/browser/tests/test_subscription_links.py
-- 
https://code.launchpad.net/~bac/launchpad/lptestrequest/+merge/57484
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/lptestrequest into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2011-03-28 02:49:22 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2011-04-13 13:11:16 +0000
@@ -108,6 +108,7 @@
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr.interfaces.feed import IFeed
 from lp.app.errors import UnexpectedFormData
+from lp.services.features import get_relevant_feature_controller
 from lp.services.features.flags import NullFeatureController
 from lp.services.propertycache import cachedproperty
 from lp.testopenid.interfaces.server import ITestOpenIDApplication
@@ -869,7 +870,9 @@
         self.needs_json = False
         # stub out the FeatureController that would normally be provided by
         # the publication mechanism
-        self.features = NullFeatureController()
+        # self.features = NullFeatureController()
+        self.features = (get_relevant_feature_controller() or
+                         NullFeatureController())
 
     @property
     def uuid(self):

=== modified file 'lib/lp/registry/browser/tests/test_subscription_links.py'
--- lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-12 17:45:17 +0000
+++ lib/lp/registry/browser/tests/test_subscription_links.py	2011-04-13 13:11:16 +0000
@@ -12,14 +12,10 @@
 from canonical.launchpad.webapp.interaction import ANONYMOUS
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.webapp.publisher import canonical_url
-from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.testing.pages import first_tag_by_class
 from canonical.testing.layers import DatabaseFunctionalLayer
 
 from lp.registry.interfaces.person import IPersonSet
-from lp.services.features import (
-    get_relevant_feature_controller,
-    )
 from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     celebrity_logged_in,
@@ -101,13 +97,9 @@
                 self.contents = view.render()
 
     def create_view(self, user):
-        request = LaunchpadTestRequest(
-            PATH_INFO='/', HTTP_COOKIE='', QUERY_STRING='')
-        request.features = get_relevant_feature_controller()
         return create_initialized_view(
             self.target, self.view, principal=user,
-            rootsite=self.rootsite,
-            request=request, current_request=False)
+            rootsite=self.rootsite, current_request=False)
 
     def test_subscribe_link_feature_flag_off_owner(self):
         self._create_scenario(self.target.owner, None)