launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03877
lp:~wallyworld/launchpad/blueprint-subscriptions-tales-refactor into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/blueprint-subscriptions-tales-refactor into lp:launchpad with lp:~wallyworld/launchpad/expose-blueprint-subscribe-methods as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/blueprint-subscriptions-tales-refactor/+merge/63949
This is branch #2 of a series of branches to add an ajax implementation for subscribing/unsubscribing to/from blueprints. The work is needed to fix bug 50875 - allow a team to be unsubscribed from a blueprint.
== Implementation ==
Refactor the tales used to render the subscribers portal and introduce new views needed to provide the content for the ajax calls. There is no ajax functionality in this branch - blueprints subscribing works using separate page loads just as it always has. This branch covers the main server side code and template changes necessary to make all the new stuff work.
== Tests ==
Add tests for the new views:
SpecificationPortletSubscribersContentsTestCase
test_sorted_subscriptions()
TestSpecificationPortletSubcribersIds
test_subscriber_ids()
Also run existing blueprints tests: bin/test -vvt blueprint
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/blueprints/browser/configure.zcml
lib/lp/blueprints/browser/specification.py
lib/lp/blueprints/browser/specificationsubscription.py
lib/lp/blueprints/browser/tests/test_specificationsubscription.py
lib/lp/blueprints/templates/specification-portlet-subscribers-content.pt
lib/lp/blueprints/templates/specification-portlet-subscribers.pt
./lib/lp/blueprints/browser/specification.py
204: E251 no spaces around keyword / parameter equals
--
https://code.launchpad.net/~wallyworld/launchpad/blueprint-subscriptions-tales-refactor/+merge/63949
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/blueprint-subscriptions-tales-refactor into lp:launchpad.
=== modified file 'lib/lp/blueprints/browser/configure.zcml'
--- lib/lp/blueprints/browser/configure.zcml 2011-03-08 03:03:42 +0000
+++ lib/lp/blueprints/browser/configure.zcml 2011-06-09 03:09:11 +0000
@@ -277,9 +277,6 @@
name="+listing-compact"
template="../templates/specification-listing-compact.pt"/>
<browser:page
- name="+portlet-subscribers"
- template="../templates/specification-portlet-subscribers.pt"/>
- <browser:page
name="+portlet-dependencies"
template="../templates/specification-portlet-dependencies.pt"/>
<browser:page
@@ -291,6 +288,23 @@
</browser:pages>
<browser:page
for="lp.blueprints.interfaces.specification.ISpecification"
+ name="+portlet-subscribers"
+ class="lp.blueprints.browser.specificationsubscription.SpecificationPortletSubcribersContents"
+ template="../templates/specification-portlet-subscribers.pt"
+ permission="zope.Public"/>
+ <browser:page
+ for="lp.blueprints.interfaces.specification.ISpecification"
+ name="+blueprint-portlet-subscribers-content"
+ class="lp.blueprints.browser.specificationsubscription.SpecificationPortletSubcribersContents"
+ template="../templates/specification-portlet-subscribers-content.pt"
+ permission="zope.Public"/>
+ <browser:page
+ for="lp.blueprints.interfaces.specification.ISpecification"
+ name="+blueprint-portlet-subscribers-ids"
+ class="lp.blueprints.browser.specificationsubscription.SpecificationPortletSubcribersIds"
+ permission="zope.Public"/>
+ <browser:page
+ for="lp.blueprints.interfaces.specification.ISpecification"
class="lp.blueprints.browser.specificationdependency.SpecificationDependencyTreeView"
permission="zope.Public"
name="+deptree"
=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py 2011-06-09 03:09:01 +0000
+++ lib/lp/blueprints/browser/specification.py 2011-06-09 03:09:11 +0000
@@ -413,7 +413,7 @@
links = ['edit', 'people', 'status', 'priority',
'whiteboard', 'proposegoal',
'milestone', 'requestfeedback', 'givefeedback', 'subscription',
- 'subscribeanother',
+ 'addsubscriber',
'linkbug', 'unlinkbug', 'linkbranch',
'adddependency', 'removedependency',
'dependencytree', 'linksprint', 'supersede',
@@ -464,7 +464,7 @@
text = 'Change status'
return Link('+status', text, icon='edit')
- def subscribeanother(self):
+ def addsubscriber(self):
"""Return the 'Subscribe someone else' Link."""
text = 'Subscribe someone else'
return Link('+addsubscriber', text, icon='add')
@@ -1026,6 +1026,7 @@
transitively.
"""
get_related_specs_fn = attrgetter('dependencies')
+
def link_nodes_fn(node, dependency):
self.link(dependency, node)
self.walkSpecsMakingNodes(spec, get_related_specs_fn, link_nodes_fn)
@@ -1033,6 +1034,7 @@
def addBlockedNodes(self, spec):
"""Add nodes for specs that the given spec blocks, transitively."""
get_related_specs_fn = attrgetter('blocked_specs')
+
def link_nodes_fn(node, blocked_spec):
self.link(node, blocked_spec)
self.walkSpecsMakingNodes(spec, get_related_specs_fn, link_nodes_fn)
@@ -1106,7 +1108,7 @@
size='9.2,9', # Width fits of 2 col layout, 1024x768.
ratio='compress',
ranksep=0.25,
- nodesep=0.01 # Separation between nodes
+ nodesep=0.01 # Separation between nodes
)
# Global node and edge attributes.
@@ -1453,4 +1455,3 @@
return "%s %s" % (
format_link(completer), date_formatter.displaydate())
return render
-
=== modified file 'lib/lp/blueprints/browser/specificationsubscription.py'
--- lib/lp/blueprints/browser/specificationsubscription.py 2010-11-23 23:22:27 +0000
+++ lib/lp/blueprints/browser/specificationsubscription.py 2011-06-09 03:09:11 +0000
@@ -9,11 +9,18 @@
'SpecificationSubscriptionEditView',
]
+from simplejson import dumps
+from zope.component import getUtility
+
+from lazr.delegates import delegates
from canonical.launchpad import _
from canonical.launchpad.webapp import (
canonical_url,
)
+from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.interfaces import ILaunchBag
+from canonical.launchpad.webapp.publisher import LaunchpadView
from lp.app.browser.launchpadform import (
action,
LaunchpadEditFormView,
@@ -22,6 +29,8 @@
from lp.blueprints.interfaces.specificationsubscription import (
ISpecificationSubscription,
)
+from lp.registry.model.person import person_sort_key
+from lp.services.propertycache import cachedproperty
class SpecificationSubscriptionAddView(LaunchpadFormView):
@@ -55,3 +64,85 @@
@property
def cancel_url(self):
return canonical_url(self.context.specification)
+
+
+class SpecificationPortletSubcribersContents(LaunchpadView):
+ """View for the contents for the subscribers portlet."""
+
+ @property
+ def sorted_subscriptions(self):
+ """Get the list of subscriptions to the specification.
+
+ The list is sorted such that subscriptions you can unsubscribe appear
+ before all other subscriptions.
+ """
+ sort_key = lambda sub: person_sort_key(sub.person)
+ subscriptions = sorted(self.context.subscriptions, key=sort_key)
+
+ can_unsubscribe = []
+ cannot_unsubscribe = []
+ for subscription in subscriptions:
+ if not check_permission('launchpad.View', subscription.person):
+ continue
+ if subscription.person == self.user:
+ can_unsubscribe = [subscription] + can_unsubscribe
+ elif subscription.canBeUnsubscribedByUser(self.user):
+ can_unsubscribe.append(subscription)
+ else:
+ cannot_unsubscribe.append(subscription)
+
+ sorted_subscriptions = can_unsubscribe + cannot_unsubscribe
+ result = [
+ SubscriptionAttrDecorator(subscription)
+ for subscription in sorted_subscriptions]
+ return result
+
+ @property
+ def current_user_subscription_class(self):
+ is_subscribed = self.context.isSubscribed(self.user)
+ if is_subscribed:
+ return 'subscribed-true'
+ else:
+ return 'subscribed-false'
+
+
+class SpecificationPortletSubcribersIds(LaunchpadView):
+ """A view returning a JSON dump of the subscriber IDs for a blueprint."""
+
+ @cachedproperty
+ def subscriber_ids(self):
+ """Return a dictionary mapping a css_name to user name."""
+ subscribers = set(self.context.subscribers)
+
+ # The current user has to be in subscribers_id so
+ # in case the id is needed for a new subscription.
+ user = getUtility(ILaunchBag).user
+ if user is not None:
+ subscribers.add(user)
+
+ ids = {}
+ for sub in subscribers:
+ ids[sub.name] = 'subscriber-%s' % sub.id
+ return ids
+
+ @property
+ def subscriber_ids_js(self):
+ """Return subscriber_ids in a form suitable for JavaScript use."""
+ return dumps(self.subscriber_ids)
+
+ def render(self):
+ """Override the default render() to return only JSON."""
+ self.request.response.setHeader('content-type', 'application/json')
+ return self.subscriber_ids_js
+
+
+class SubscriptionAttrDecorator:
+ """A SpecificationSubscription with added attributes for HTML/JS."""
+ delegates(ISpecificationSubscription, 'subscription')
+
+ def __init__(self, subscription):
+ self.subscription = subscription
+
+ @property
+ def css_name(self):
+ return 'subscriber-%s' % self.subscription.person.id
=== added file 'lib/lp/blueprints/browser/tests/test_specificationsubscription.py'
--- lib/lp/blueprints/browser/tests/test_specificationsubscription.py 1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationsubscription.py 2011-06-09 03:09:11 +0000
@@ -0,0 +1,54 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.testing.views import create_initialized_view
+
+
+class SpecificationPortletSubscribersContentsTestCase(TestCaseWithFactory):
+ """Tests for SpecificationPortletSubscribersContents view."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_sorted_subscriptions(self):
+ # SpecificationPortletSubscribersContents.sorted_subscriptions
+ spec = self.factory.makeSpecification()
+ subscriber = self.factory.makePerson(displayname="Joe")
+ subscriber1 = self.factory.makePerson(displayname="Mark")
+ subscriber2 = self.factory.makePerson(displayname="Fred")
+ with person_logged_in(subscriber):
+ sub1 = spec.subscribe(subscriber, subscriber)
+ sub2 = spec.subscribe(subscriber1, subscriber)
+ sub3 = spec.subscribe(subscriber2, subscriber)
+ view = create_initialized_view(
+ spec, name="+blueprint-portlet-subscribers-content")
+ # Loop over the results of sorted_subscriptions to extract the
+ # subscriptions from their SubscriptionAttrDecorator instances.
+ sorted_subscriptions = [
+ decorator.subscription for decorator in
+ view.sorted_subscriptions]
+ self.assertEqual([sub1, sub3, sub2], sorted_subscriptions)
+
+
+class TestSpecificationPortletSubcribersIds(TestCaseWithFactory):
+ # Tests for SpecificationPortletSubcribersIds view.
+ layer = LaunchpadFunctionalLayer
+
+ def test_subscriber_ids(self):
+ spec = self.factory.makeSpecification()
+ subscriber = self.factory.makePerson()
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ spec.subscribe(subscriber, subscriber)
+ view = create_initialized_view(
+ spec, name="+blueprint-portlet-subscribers-ids")
+ subscriber_ids = dict(
+ (subscriber.name, 'subscriber-%s' % subscriber.id)
+ for subscriber in [person, subscriber])
+ self.assertEqual(subscriber_ids, view.subscriber_ids)
=== added file 'lib/lp/blueprints/templates/specification-portlet-subscribers-content.pt'
--- lib/lp/blueprints/templates/specification-portlet-subscribers-content.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/templates/specification-portlet-subscribers-content.pt 2011-06-09 03:09:11 +0000
@@ -0,0 +1,63 @@
+<div
+ tal:omit-tag=""
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ class="portletBody"
+ tal:define="
+ blueprint context;
+ subscriptions view/sorted_subscriptions">
+
+ <div id="subscribers">
+ <h2>Subscribers</h2>
+ <div id="subscribers-links">
+ <tal:comment replace="nothing">
+ This is messy, but the portlet has to convey both subscribers and
+ whether or not they are essential. Also, in the case where a person
+ can say if the subscription is essential, that subscription
+ essentiality indicator needs to be clickable to edit, otherwise not.
+ Hence all the duplication.
+ </tal:comment>
+ <div class='subscriber' tal:repeat="subscription subscriptions"
+ tal:attributes="id string:${subscription/css_name}">
+ <a tal:condition="subscription/required:launchpad.Edit"
+ tal:attributes="href subscription/fmt:url">
+ <img
+ tal:condition="not:subscription/person/is_valid_person"
+ alt="(inactive)"
+ src="/@@/person-inactive"
+ title="No longer uses Launchpad."
+ />
+ <tal:active condition="subscription/person/is_valid_person">
+ <img
+ tal:condition="subscription/essential"
+ alt="(essential)"
+ src="/@@/subscriber-essential"
+ title="Has asked to take part in all discussions of this blueprint."
+ />
+ <img
+ tal:condition="not: subscription/essential"
+ alt=""
+ src="/@@/subscriber-inessential"
+ title="Normal subscriber."
+ />
+ </tal:active>
+ </a>
+ <tal:no_edit condition="not: subscription/required:launchpad.Edit">
+ <img tal:condition="subscription/essential"
+ src="/@@/subscriber-essential" />
+ <img tal:condition="not: subscription/essential"
+ src="/@@/subscriber-inessential" />
+ </tal:no_edit>
+ <a tal:attributes="href subscription/person/fmt:url"
+ tal:content="subscription/person/fmt:displayname"
+ />
+ </div>
+ </div>
+ <div id="none-subscribers"
+ tal:condition="not:subscriptions">
+ No subscribers.
+ </div>
+
+ </div>
+</div>
=== modified file 'lib/lp/blueprints/templates/specification-portlet-subscribers.pt'
--- lib/lp/blueprints/templates/specification-portlet-subscribers.pt 2009-09-18 21:17:07 +0000
+++ lib/lp/blueprints/templates/specification-portlet-subscribers.pt 2011-06-09 03:09:11 +0000
@@ -1,63 +1,17 @@
-<tal:root
+<div
xmlns:tal="http://xml.zope.org/namespaces/tal";
xmlns:metal="http://xml.zope.org/namespaces/metal";
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
- omit-tag="">
-
-<div class="portlet"
- tal:define="subscriptions context/subscriptions">
-
- <ul tal:define="context_menu context/menu:context">
- <li tal:content="structure context_menu/subscription/render" />
- <li tal:content="structure context_menu/subscribeanother/render" />
- </ul>
-
- <div id="portlet-subscribers" tal:condition="subscriptions">
- <h2>Subscribers</h2>
- <div class="portletBody">
- <tal:comment replace="nothing">
- This is messy, but the portlet has to convey both subscribers and
- whether or not they are essential. Also, in the case where a person
- can say if the subscription is essential, that subscription
- essentiality indicator needs to be clickable to edit, otherwise not.
- Hence all the duplication.
- </tal:comment>
- <div tal:repeat="subscription subscriptions"
- class="subscriber">
- <a tal:condition="subscription/required:launchpad.Edit"
- tal:attributes="href subscription/fmt:url">
- <img
- tal:condition="not:subscription/person/is_valid_person"
- alt="(inactive)"
- src="/@@/person-inactive"
- title="No longer uses Launchpad."
- />
- <tal:active condition="subscription/person/is_valid_person">
- <img
- tal:condition="subscription/essential"
- alt="(essential)"
- src="/@@/subscriber-essential"
- title="Has asked to take part in all discussions of this blueprint."
- />
- <img
- tal:condition="not: subscription/essential"
- alt=""
- src="/@@/subscriber-inessential"
- title="Normal subscriber."
- />
- </tal:active>
- </a>
- <tal:no_edit condition="not: subscription/required:launchpad.Edit">
- <img tal:condition="subscription/essential"
- src="/@@/subscriber-essential" />
- <img tal:condition="not: subscription/essential"
- src="/@@/subscriber-inessential" />
- </tal:no_edit>
- <a tal:attributes="href subscription/person/fmt:url"
- tal:content="subscription/person/fmt:displayname"
- />
- </div>
- </div>
+ class="portlet vertical"
+ id="portlet-subscribers"
+ metal:define-macro="custom"
+>
+ <div class="section" tal:define="context_menu context/menu:context"
+ metal:define-slot="heading">
+ <div
+ tal:attributes="class view/current_user_subscription_class"
+ tal:content="structure context_menu/subscription/render" />
+ <div tal:content="structure context_menu/addsubscriber/render" />
</div>
+ <div tal:replace="structure context/@@+blueprint-portlet-subscribers-content" />
</div>
-</tal:root>
