Merge lp:~gmb/launchpad/fix-ff-for-ss-bug-677020 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Merged at revision: 11945
Proposed branch: lp:~gmb/launchpad/fix-ff-for-ss-bug-677020
Merge into: lp:launchpad
Diff against target: 50 lines (+17/-5)
2 files modified
lib/lp/bugs/browser/bugsubscription.py (+6/-5)
lib/lp/registry/browser/tests/test_structuralsubscription.py (+11/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/fix-ff-for-ss-bug-677020
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+41187@code.launchpad.net

Commit message

[r=adeuring][ui=none][bug=677020] The advanced subscription features are now flagged correctly on the StructuralSubscriptionView.

Description of the change

This branch ensures that the feature-flagging of the advanced subscriptions features works for the structural subscriptions form by adding a check for the feature flag to AdvancedSubscriptionMixin.setUpBugNotificationLevelField().

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

(17:24:50) abel: gmb: looks fine -- but what I don't understand is this: BugSubscriptionSubscribeSelfView.setUpFields() already calls _seUpBugNotificationLevelFiled only if the feature is enabled. Or am I missing something?
(17:25:39) gmb: abel: It's not BugSubscriptionSubscribeSelfView that's the problem, but I get your point. I'll remove that condition (the idea of my change is that whoever is using AdvancedSubscriptionMixin shouldn't have to remember to add the conditional in the first place.)
(17:26:09) abel: gmb: ah, OK, I see. so, r=me

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2010-11-12 11:02:58 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2010-11-19 09:07:59 +0000
@@ -133,6 +133,10 @@
133133
134 def _setUpBugNotificationLevelField(self):134 def _setUpBugNotificationLevelField(self):
135 """Set up the bug_notification_level field."""135 """Set up the bug_notification_level field."""
136 if not self._use_advanced_features:
137 # If advanced features are disabled, do nothing.
138 return
139
136 self.form_fields = self.form_fields.omit('bug_notification_level')140 self.form_fields = self.form_fields.omit('bug_notification_level')
137 self.form_fields += formlib.form.Fields(141 self.form_fields += formlib.form.Fields(
138 self._bug_notification_level_field)142 self._bug_notification_level_field)
@@ -263,11 +267,8 @@
263 if self.user is None:267 if self.user is None:
264 return268 return
265269
266 if self._use_advanced_features:270 self.form_fields += formlib.form.Fields(self._subscription_field)
267 self.form_fields += formlib.form.Fields(self._subscription_field)271 self._setUpBugNotificationLevelField()
268 self._setUpBugNotificationLevelField()
269 else:
270 self.form_fields += formlib.form.Fields(self._subscription_field)
271 self.form_fields['subscription'].custom_widget = CustomWidgetFactory(272 self.form_fields['subscription'].custom_widget = CustomWidgetFactory(
272 RadioWidget)273 RadioWidget)
273274
274275
=== modified file 'lib/lp/registry/browser/tests/test_structuralsubscription.py'
--- lib/lp/registry/browser/tests/test_structuralsubscription.py 2010-11-10 11:33:45 +0000
+++ lib/lp/registry/browser/tests/test_structuralsubscription.py 2010-11-19 09:07:59 +0000
@@ -249,6 +249,17 @@
249 harness.submit('save', form_data)249 harness.submit('save', form_data)
250 self.assertTrue(harness.hasErrors())250 self.assertTrue(harness.hasErrors())
251251
252 def test_extra_features_hidden_without_feature_flag(self):
253 # If the malone.advanced-subscriptions.enabled flag is turned
254 # off, the bug_notification_level field doesn't appear on the
255 # form.
256 person = self.factory.makePerson()
257 with person_logged_in(person):
258 harness = LaunchpadFormHarness(
259 self.target, StructuralSubscriptionView)
260 form_fields = harness.view.form_fields
261 self.assertIs(
262 None, form_fields.get('bug_notification_level'))
252263
253class TestProductSeriesAdvancedSubscriptionFeatures(264class TestProductSeriesAdvancedSubscriptionFeatures(
254 TestStructuralSubscriptionAdvancedFeaturesBase):265 TestStructuralSubscriptionAdvancedFeaturesBase):