launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01433
[Merge] lp:~gmb/launchpad/launchpadformify-subscribe-view-bug-651240 into lp:launchpad/devel
Graham Binns has proposed merging lp:~gmb/launchpad/launchpadformify-subscribe-view-bug-651240 into lp:launchpad/devel with lp:~gmb/launchpad/extract-subscription-view-bug-651240 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
#653122 Make the new Bug +subscribe view into a LaunchpadFormView
https://bugs.launchpad.net/bugs/653122
This branch fixes bug 651240 by turning BugSubscriptionSubscribeSelfView from a LaunchpadView into a LaunchpadFormView. The work here is pretty straightforward - converting everything to use FormFields instead of custom widget attributes on the view - and will be used in subsequent branches to add new features to BugSubscriptions.
--
https://code.launchpad.net/~gmb/launchpad/launchpadformify-subscribe-view-bug-651240/+merge/37850
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/launchpadformify-subscribe-view-bug-651240 into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2010-10-07 13:43:57 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2010-10-07 13:43:58 +0000
@@ -15,6 +15,7 @@
from lazr.delegates import delegates
from simplejson import dumps
+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
@@ -25,9 +26,11 @@
SimpleVocabulary,
)
+from canonical.launchpad import _
from canonical.launchpad.webapp import (
action,
canonical_url,
+ custom_widget,
LaunchpadFormView,
LaunchpadView,
)
@@ -36,6 +39,9 @@
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
class BugSubscriptionAddView(LaunchpadFormView):
@@ -74,10 +80,11 @@
page_title = label
-class BugSubscriptionSubscribeSelfView(LaunchpadView,
- ReturnToReferrerMixin):
+class BugSubscriptionSubscribeSelfView(LaunchpadFormView):
"""A view to handle the +subscribe page for a bug."""
+ schema = IBugSubscription
+
@property
def next_url(self):
"""Provided so returning to the page they came from works."""
@@ -85,7 +92,7 @@
# XXX bdmurray 2010-09-30 bug=98437: work around zope's test
# browser setting referer to localhost.
- if referer and referer != 'localhost':
+ if referer and referer not in ('localhost', self.request.getURL()):
next_url = referer
else:
next_url = canonical_url(self.context)
@@ -93,20 +100,30 @@
cancel_url = next_url
- def initialize(self):
- """Set up the needed widgets."""
- bug = self.context.bug
-
- # See render() for how this flag is used.
- self._redirecting_to_bug_list = False
-
+ @cachedproperty
+ def _subscribers_for_current_user(self):
+ """Return a dict of the subscribers for the current user."""
+ persons_for_user = {}
+ person_count = 0
+ bug = self.context.bug
+ for person in bug.getSubscribersForPerson(self.user):
+ if person.id not in persons_for_user:
+ persons_for_user[person.id] = person
+ person_count += 1
+
+ self._subscriber_count_for_current_user = person_count
+ return persons_for_user.values()
+
+ def setUpFields(self):
+ """See `LaunchpadFormView`."""
+ super(BugSubscriptionSubscribeSelfView, self).setUpFields()
+ bug = self.context.bug
if self.user is None:
return
- # Set up widgets in order to handle subscription requests.
subscription_terms = []
self_subscribed = False
- for person in bug.getSubscribersForPerson(self.user):
+ for person in self._subscribers_for_current_user:
if person.id == self.user.id:
subscription_terms.append(
SimpleTerm(
@@ -125,16 +142,18 @@
SimpleTerm(
self.user, self.user.name, 'Subscribe me to this bug'))
subscription_vocabulary = SimpleVocabulary(subscription_terms)
- person_field = Choice(
- __name__='subscription',
- vocabulary=subscription_vocabulary, required=True)
- self.subscription_widget = CustomWidgetFactory(RadioWidget)
- setUpWidget(
- self, 'subscription', person_field, IInputWidget, value=self.user)
-
- self.handleSubscriptionRequest()
-
- def userIsSubscribed(self):
+ default_subscription_value = subscription_vocabulary.getTermByToken(
+ self.user.name).value
+ subscription_field = Choice(
+ __name__='subscription', title=_("Subscription options"),
+ vocabulary=subscription_vocabulary, required=True,
+ default=default_subscription_value)
+ self.form_fields += formlib.form.Fields(subscription_field)
+ self.form_fields['subscription'].custom_widget = CustomWidgetFactory(
+ RadioWidget)
+
+ @cachedproperty
+ def user_is_subscribed(self):
"""Is the user subscribed to this bug?"""
return (
self.context.bug.isSubscribed(self.user) or
@@ -146,7 +165,7 @@
The warning should tell the user that, when unsubscribing, they
will also be unsubscribed from dupes of this bug.
"""
- if self.userIsSubscribed():
+ if self.user_is_subscribed:
return True
bug = self.context.bug
@@ -156,37 +175,16 @@
return False
- def render(self):
- """Render the bug list if the user has permission to see the bug."""
- # Prevent normal rendering when redirecting to the bug list
- # after unsubscribing from a private bug, because rendering the
- # bug page would raise Unauthorized errors!
- if self._redirecting_to_bug_list:
- return u''
- elif self._isSubscriptionRequest() and self.request.get('next_url'):
- self.request.response.redirect(self.request.get('next_url'))
- return u''
- else:
- return LaunchpadView.render(self)
-
- def handleSubscriptionRequest(self):
- """Subscribe or unsubscribe the user from the bug, if requested."""
- if not self._isSubscriptionRequest():
- return
-
- subscription_person = self.subscription_widget.getInputValue()
-
- # 'subscribe' appears in the request whether the request is to
- # subscribe or unsubscribe. Since "subscribe someone else" is
- # handled by a different view we can assume that 'subscribe' +
- # current user as a parameter means "subscribe the current
- # user", and any other kind of 'subscribe' request actually
- # means "unsubscribe". (Yes, this *is* very confusing!)
- if ('subscribe' in self.request.form and
+ @action('Continue', name='continue')
+ def subscribe_action(self, action, data):
+ """Handle subscription requests."""
+ subscription_person = self.widgets['subscription'].getInputValue()
+ if (not self.user_is_subscribed and
(subscription_person == self.user)):
self._handleSubscribe()
else:
self._handleUnsubscribe(subscription_person)
+ self.request.response.redirect(self.next_url)
def _isSubscriptionRequest(self):
"""Return True if the form contains subscribe/unsubscribe input."""
@@ -194,7 +192,7 @@
self.user and
self.request.method == 'POST' and
'cancel' not in self.request.form and
- self.subscription_widget.hasValidInput())
+ self.widgets['subscription'].hasValidInput())
def _handleSubscribe(self):
"""Handle a subscribe request."""
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions.txt 2010-10-07 13:43:57 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions.txt 2010-10-07 13:43:58 +0000
@@ -68,7 +68,7 @@
>>> submit.click()
>>> browser.url
- 'http://bugs.launchpad.dev/firefox/+bug/1/'
+ 'http://bugs.launchpad.dev/firefox/+bug/1'
>>> for tag in find_tags_by_class(browser.contents, 'informational message'):
... print tag.renderContents()
=== modified file 'lib/lp/bugs/templates/bug-subscription.pt'
--- lib/lp/bugs/templates/bug-subscription.pt 2010-10-07 13:43:57 +0000
+++ lib/lp/bugs/templates/bug-subscription.pt 2010-10-07 13:43:58 +0000
@@ -15,13 +15,13 @@
<div id="maincontent">
<div id="nonportlets" class="readable">
- <h1 tal:define="subscribed view/userIsSubscribed">
+ <h1 tal:define="subscribed view/user_is_subscribed">
<tal:action condition="subscribed">Unsubscribe from</tal:action>
<tal:action condition="not:subscribed">Subscribe to</tal:action>
bug #<span tal:replace="context/bug/id" />
</h1>
- <p tal:condition="not: view/userIsSubscribed">
+ <p tal:condition="not: view/user_is_subscribed">
If you subscribe to a bug it shows up on your
personal pages, you receive copies of all comments by email,
and notification of any changes to the status of the bug upstream
@@ -34,33 +34,8 @@
duplicates.
</p>
- <form action="" method="POST">
-
- <div class="field">
- <div>
- <div tal:content="structure view/subscription_widget">
- <input type="radio" />
- </div>
- </div>
-
- </div>
-
- <div class="actions">
- <input type="hidden" name="next_url"
- tal:condition="view/next_url | nothing"
- tal:attributes="value view/next_url" />
- <input tal:condition="not: view/userIsSubscribed"
- type="submit"
- name="subscribe"
- value="Continue" />
- <input tal:condition="view/userIsSubscribed"
- type="submit"
- name="unsubscribe"
- value="Continue" />
- or <a tal:condition="view/cancel_url | nothing"
- tal:attributes="href view/cancel_url">Cancel</a>
- </div>
- </form>
+ <div metal:use-macro="context/@@launchpad_form/form">
+ </div>
</div>
</div>
