launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/refactor-advanced-subscriptions-code-bug-670997 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #670997 Refactor the advanced subscriptions code into a mixin for ease-of-use
  https://bugs.launchpad.net/bugs/670997


This branch refactors the advanced subscriptions code out into a mixin so that it can be easily used by the structural subscriptions UI when the time comes.

I've made no test changes since the code in the mixin is covered by the tests for BugSubscriptionsSubscribeSelfView.
-- 
https://code.launchpad.net/~gmb/launchpad/refactor-advanced-subscriptions-code-bug-670997/+merge/40195
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/refactor-advanced-subscriptions-code-bug-670997 into lp:launchpad/devel.

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2010-11-03 13:44:35 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2010-11-05 15:24:59 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'AdvancedSubscriptionMixin',
     'BugPortletDuplicateSubcribersContents',
     'BugPortletSubcribersContents',
     'BugSubscriptionAddView',
@@ -77,8 +78,69 @@
     page_title = label
 
 
+class AdvancedSubscriptionMixin:
+    """A mixin of advanced subscription code for views.
+
+    In order to use this mixin in a view the view must:
+     - Define a current_user_subscription property which returns the
+       current BugSubscription or StructuralSubscription for request.user.
+     - Define a dict, _bug_notification_level_descriptions, which maps
+       BugNotificationLevel values to string descriptions for the
+       current context (see `BugSubscriptionSubscribeSelfView` for an
+       example).
+     - Update the view's setUpFields() to call
+       _setUpBugNotificationLevelField().
+    """
+
+    @cachedproperty
+    def _use_advanced_features(self):
+        """Return True if advanced subscriptions features are enabled."""
+        return features.getFeatureFlag(
+            'malone.advanced-subscriptions.enabled')
+
+    @cachedproperty
+    def _bug_notification_level_field(self):
+        """Return a custom form field for bug_notification_level."""
+        # We rebuild the items that we show in the field so that the
+        # labels shown are human readable and specific to the +subscribe
+        # form. The BugNotificationLevel descriptions are too generic.
+        bug_notification_level_terms = [
+            SimpleTerm(
+                level, level.name,
+                self._bug_notification_level_descriptions[level])
+            # We reorder the items so that COMMENTS comes first. We also
+            # drop the NOTHING option since it just makes the UI
+            # confusing.
+            for level in sorted(BugNotificationLevel.items, reverse=True)
+                if level != BugNotificationLevel.NOTHING
+            ]
+        bug_notification_vocabulary = SimpleVocabulary(
+            bug_notification_level_terms)
+
+        if self.current_user_subscription is not None:
+            default_value = (
+                self.current_user_subscription.bug_notification_level)
+        else:
+            default_value = BugNotificationLevel.COMMENTS
+
+        bug_notification_level_field = Choice(
+            __name__='bug_notification_level', title=_("Tell me when"),
+            vocabulary=bug_notification_vocabulary, required=True,
+            default=default_value)
+        return bug_notification_level_field
+
+    def _setUpBugNotificationLevelField(self):
+        """Set up the bug_notification_level field."""
+        self.form_fields = self.form_fields.omit('bug_notification_level')
+        self.form_fields += formlib.form.Fields(
+            self._bug_notification_level_field)
+        self.form_fields['bug_notification_level'].custom_widget = (
+            CustomWidgetFactory(RadioWidget))
+
+
 class BugSubscriptionSubscribeSelfView(LaunchpadFormView,
-                                       ReturnToReferrerMixin):
+                                       ReturnToReferrerMixin,
+                                       AdvancedSubscriptionMixin):
     """A view to handle the +subscribe page for a bug."""
 
     schema = IBugSubscription
@@ -139,12 +201,6 @@
         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
@@ -156,42 +212,17 @@
         return self.context.bug.getSubscriptionForPerson(self.user)
 
     @cachedproperty
-    def _bug_notification_level_field(self):
-        """Return a custom form field for bug_notification_level."""
-        # We rebuild the items that we show in the field so that the
-        # labels shown are human readable and specific to the +subscribe
-        # form. The BugNotificationLevel descriptions are too generic.
-        bug_notification_level_terms = [
-            SimpleTerm(
-                level, level.name,
-                self._bug_notification_level_descriptions[level])
-            # We reorder the items so that COMMENTS comes first. We also
-            # drop the NOTHING option since it just makes the UI
-            # confusing.
-            for level in sorted(BugNotificationLevel.items, reverse=True)
-                if level != BugNotificationLevel.NOTHING
-            ]
-        bug_notification_vocabulary = SimpleVocabulary(
-            bug_notification_level_terms)
-
-        if self.current_user_subscription is not None:
-            default_value = (
-                self.current_user_subscription.bug_notification_level)
-        else:
-            default_value = BugNotificationLevel.COMMENTS
-
-        bug_notification_level_field = Choice(
-            __name__='bug_notification_level', title=_("Tell me when"),
-            vocabulary=bug_notification_vocabulary, required=True,
-            default=default_value)
-        return bug_notification_level_field
-
-    @cachedproperty
     def _update_subscription_term(self):
         return SimpleTerm(
             'update-subscription', 'update-subscription',
             'Update my current subscription')
 
+    def setUpFields(self):
+        """See `LaunchpadFormView`."""
+        super(BugSubscriptionSubscribeSelfView, self).setUpFields()
+        if self.user is None:
+            return
+
     @cachedproperty
     def _subscription_field(self):
         subscription_terms = []
@@ -235,12 +266,8 @@
             return
 
         if self._use_advanced_features:
-            self.form_fields = self.form_fields.omit('bug_notification_level')
             self.form_fields += formlib.form.Fields(self._subscription_field)
-            self.form_fields += formlib.form.Fields(
-                self._bug_notification_level_field)
-            self.form_fields['bug_notification_level'].custom_widget = (
-                CustomWidgetFactory(RadioWidget))
+            self._setUpBugNotificationLevelField()
         else:
             self.form_fields += formlib.form.Fields(self._subscription_field)
         self.form_fields['subscription'].custom_widget = CustomWidgetFactory(