launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03900
[Merge] lp:~danilo/launchpad/bug-772754-other-subscribers-loading into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/bug-772754-other-subscribers-loading into lp:launchpad with lp:~danilo/launchpad/bug-772754-other-subscribers-activity as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-loading/+merge/64186
= Bug 772754: Other subscribers list, part 5 =
Warning: oversized branch. Perhaps should have split it up into server-side and JS branches.
This is part of ongoing work for providing the "other subscribers" list as indicated in mockup https://launchpadlibrarian.net/71552495/all-in-one.png attached to bug 772754 by Gary.
This branch builds on top of the previous branches and provides finally ties in all the SubscribersList JS code with the actual subscribers loading.
To do the loading, method getDirectSubscribersWithDetails is added to provide both subscriber Person records and BugSubscription records in one go (for the sake of bug_notification_level, which is needed to determine which section a subscriber goes to).
For 'Maybe notified' subscribers, we are using existing APIs (getIndirectSubscribers).
Output of these methods is further hooked up into view +bug-portlet-subscriber-details which provides JSON suitable for direct use by the loading JS.
It is comprehensively tested (I believe). An integration test would be nice, but definitely for a later branch.
Considering the branch is over-sized already, I didn't bother looking into model/bug.py lint issues (except that I made sure I introduced no new ones).
== Tests ==
lp/bugs/javascript/tests/test_subscribers_list.html
== Demo and Q/A ==
N/A
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/configure.zcml
lib/lp/bugs/browser/bugsubscription.py
lib/lp/bugs/browser/configure.zcml
lib/lp/bugs/browser/tests/test_bugsubscription_views.py
lib/lp/bugs/interfaces/bug.py
lib/lp/bugs/javascript/subscribers_list.js
lib/lp/bugs/javascript/tests/test_subscribers_list.js
lib/lp/bugs/model/bug.py
lib/lp/bugs/model/tests/test_bug.py
lib/lp/bugs/templates/bug-portlet-subscribers.pt
lib/lp/bugs/templates/bugtask-index.pt
./lib/lp/bugs/model/bug.py
52: 'SQLRaw' imported but unused
52: 'Join' imported but unused
52: 'Exists' imported but unused
171: 'get_bug_privacy_filter' imported but unused
52: 'Count' imported but unused
303: E301 expected 1 blank line, found 0
2607: E225 missing whitespace around operator
make: *** [lint] Error 7
--
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-loading/+merge/64186
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-772754-other-subscribers-loading into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2011-06-10 13:53:33 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2011-06-10 13:53:35 +0000
@@ -26,6 +26,7 @@
SimpleTerm,
SimpleVocabulary,
)
+from zope.security.proxy import removeSecurityProxy
from canonical.launchpad import _
from canonical.launchpad.webapp import (
@@ -327,7 +328,7 @@
if self._use_advanced_features:
self.widgets['bug_notification_level'].widget_class = (
'bug-notification-level-field')
- if (len(self.form_fields['subscription'].field.vocabulary)==1):
+ if (len(self.form_fields['subscription'].field.vocabulary) == 1):
# We hide the subscription widget if the user isn't
# subscribed, since we know who the subscriber is and we
# don't need to present them with a single radio button.
@@ -581,6 +582,58 @@
key=(lambda subscription: subscription.person.displayname))]
+class BugPortletSubscribersWithDetails(LaunchpadView):
+ """A view that returns a JSON dump of the subscriber details for a bug."""
+
+ @property
+ def subscriber_data_js(self):
+ """Return subscriber_ids in a form suitable for JavaScript use."""
+ data = []
+ details = list(self.context.getDirectSubscribersWithDetails())
+ for person, subscription in details:
+ if person == self.user:
+ # Skip the current user viewing the page.
+ continue
+ can_edit = self.user is not None and self.user.inTeam(person)
+ subscriber = {
+ 'name': person.name,
+ 'display_name': person.displayname,
+ 'web_link': canonical_url(person, rootsite='mainsite'),
+ 'is_team': person.is_team,
+ 'can_edit': can_edit,
+ }
+ record = {
+ 'subscriber': subscriber,
+ 'subscription_level': str(
+ removeSecurityProxy(subscription.bug_notification_level)),
+ }
+ data.append(record)
+
+ others = list(self.context.getIndirectSubscribers())
+ for person in others:
+ if person == self.user:
+ # Skip the current user viewing the page.
+ continue
+ subscriber = {
+ 'name': person.name,
+ 'display_name': person.displayname,
+ 'web_link': canonical_url(person, rootsite='mainsite'),
+ 'is_team': person.is_team,
+ 'can_edit': False,
+ }
+ record = {
+ 'subscriber': subscriber,
+ 'subscription_level': 'Maybe',
+ }
+ data.append(record)
+ return dumps(data)
+
+ def render(self):
+ """Override the default render() to return only JSON."""
+ self.request.response.setHeader('content-type', 'application/json')
+ return self.subscriber_data_js
+
+
class BugPortletSubcribersIds(LaunchpadView, BugViewMixin):
"""A view that returns a JSON dump of the subscriber IDs for a bug."""
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-06-10 13:53:33 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-06-10 13:53:35 +0000
@@ -1100,6 +1100,12 @@
name="+bug-portlet-subscribers-ids"
class="lp.bugs.browser.bugsubscription.BugPortletSubcribersIds"
permission="zope.Public"/>
+ <browser:page
+ for="lp.bugs.interfaces.bug.IBug"
+ name="+bug-portlet-subscribers-details"
+ class="
+ lp.bugs.browser.bugsubscription.BugPortletSubscribersWithDetails"
+ permission="zope.Public"/>
<browser:navigation
module="lp.bugs.browser.bug"
classes="
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-06-02 15:34:06 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-06-10 13:53:35 +0000
@@ -5,11 +5,15 @@
__metaclass__ = type
+from simplejson import dumps
+
from canonical.launchpad.ftests import LaunchpadFormHarness
+from canonical.launchpad.webapp import canonical_url
from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.bugs.browser.bugsubscription import (
BugPortletSubcribersIds,
+ BugPortletSubscribersWithDetails,
BugSubscriptionListView,
BugSubscriptionSubscribeSelfView,
)
@@ -479,3 +483,146 @@
self.bug.default_bugtask, name="+mute",
form={'field.actions.unmute': 'Unmute bug mail'})
self.assertFalse(self.bug.isMuted(self.person))
+
+
+class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
+ """Tests for IBug:+bug-portlet-subscribers-details view."""
+ layer = LaunchpadFunctionalLayer
+
+ def test_content_type(self):
+ bug = self.factory.makeBug()
+
+ # It works even for anonymous users, so no log-in is needed.
+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+ harness.view.render()
+
+ self.assertEqual(
+ harness.request.response.getHeader('content-type'),
+ 'application/json')
+
+ def _makeBugWithNoSubscribers(self):
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ # Unsubscribe the bug reporter to ensure we have no subscribers.
+ bug.unsubscribe(bug.owner, bug.owner)
+ return bug
+
+ def test_data_no_subscriptions(self):
+ bug = self._makeBugWithNoSubscribers()
+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+ self.assertEqual(dumps([]), harness.view.subscriber_data_js)
+
+ def test_data_person_subscription(self):
+ # A subscriber_data_js returns JSON string of a list
+ # containing all subscriber information needed for
+ # subscribers_list.js subscribers loading.
+ bug = self._makeBugWithNoSubscribers()
+ subscriber = self.factory.makePerson(
+ name='user', displayname='Subscriber Name')
+ with person_logged_in(subscriber):
+ bug.subscribe(subscriber, subscriber,
+ level=BugNotificationLevel.LIFECYCLE)
+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'user',
+ 'display_name': 'Subscriber Name',
+ 'is_team': False,
+ 'can_edit': False,
+ 'web_link': canonical_url(subscriber),
+ },
+ 'subscription_level': "Lifecycle",
+ }
+ self.assertEqual(
+ dumps([expected_result]), harness.view.subscriber_data_js)
+
+ def test_data_team_subscription(self):
+ # For a team subscription, subscriber_data_js has is_team set
+ # to true.
+ bug = self._makeBugWithNoSubscribers()
+ subscriber = self.factory.makeTeam(
+ name='team', displayname='Team Name')
+ with person_logged_in(subscriber.teamowner):
+ bug.subscribe(subscriber, subscriber.teamowner,
+ level=BugNotificationLevel.LIFECYCLE)
+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': False,
+ 'web_link': canonical_url(subscriber),
+ },
+ 'subscription_level': "Lifecycle",
+ }
+ self.assertEqual(
+ dumps([expected_result]), harness.view.subscriber_data_js)
+
+ def test_data_team_subscription_owner_looks(self):
+ # For a team subscription, subscriber_data_js has can_edit
+ # set to true for team owner.
+ bug = self._makeBugWithNoSubscribers()
+ subscriber = self.factory.makeTeam(
+ name='team', displayname='Team Name')
+ with person_logged_in(subscriber.teamowner):
+ bug.subscribe(subscriber, subscriber.teamowner,
+ level=BugNotificationLevel.LIFECYCLE)
+ harness = LaunchpadFormHarness(
+ bug, BugPortletSubscribersWithDetails)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': True,
+ 'web_link': canonical_url(subscriber),
+ },
+ 'subscription_level': "Lifecycle",
+ }
+ with person_logged_in(subscriber.teamowner):
+ self.assertEqual(
+ dumps([expected_result]), harness.view.subscriber_data_js)
+
+ def test_data_team_subscription_member_looks(self):
+ # For a team subscription, subscriber_data_js has can_edit
+ # set to true for team member.
+ bug = self._makeBugWithNoSubscribers()
+ member = self.factory.makePerson()
+ subscriber = self.factory.makeTeam(
+ name='team', displayname='Team Name', members=[member])
+ with person_logged_in(subscriber.teamowner):
+ bug.subscribe(subscriber, subscriber.teamowner,
+ level=BugNotificationLevel.LIFECYCLE)
+ harness = LaunchpadFormHarness(
+ bug, BugPortletSubscribersWithDetails)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': True,
+ 'web_link': canonical_url(subscriber),
+ },
+ 'subscription_level': "Lifecycle",
+ }
+ with person_logged_in(subscriber.teamowner):
+ self.assertEqual(
+ dumps([expected_result]), harness.view.subscriber_data_js)
+
+ def test_data_person_subscription_user_excluded(self):
+ # With the subscriber logged in, he is not included in the results.
+ bug = self._makeBugWithNoSubscribers()
+ subscriber = self.factory.makePerson(
+ name='a-person', displayname='Subscriber Name')
+
+ with person_logged_in(subscriber):
+ bug.subscribe(subscriber, subscriber,
+ level=BugNotificationLevel.LIFECYCLE)
+ harness = LaunchpadFormHarness(
+ bug, BugPortletSubscribersWithDetails)
+ self.assertEqual(dumps([]), harness.view.subscriber_data_js)
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-06-06 13:54:07 +0000
+++ lib/lp/bugs/configure.zcml 2011-06-10 13:53:35 +0000
@@ -744,6 +744,7 @@
date_last_updated
getBugTask
getDirectSubscribers
+ getDirectSubscribersWithDetails
getDirectSubscriptions
getIndirectSubscribers
is_complete
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-05-24 08:58:38 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-06-10 13:53:35 +0000
@@ -522,6 +522,15 @@
Direct subscribers have an entry in the BugSubscription table.
"""
+ def getDirectSubscribersWithDetails():
+ """Get direct subscribers and their subscriptions for the bug.
+
+ Those with muted bug subscriptions are excluded from results.
+
+ :returns: A ResultSet of tuples (Person, BugSubscription)
+ representing a subscriber and their bug subscription.
+ """
+
def getIndirectSubscribers(recipients=None, level=None):
"""Return IPersons that are indirectly subscribed to this bug.
=== modified file 'lib/lp/bugs/javascript/subscribers_list.js'
--- lib/lp/bugs/javascript/subscribers_list.js 2011-06-10 13:53:33 +0000
+++ lib/lp/bugs/javascript/subscribers_list.js 2011-06-10 13:53:35 +0000
@@ -119,6 +119,168 @@
}
+/**
+ * Load subscribers for a bug from Launchpad and put them in the web page.
+ *
+ * Uses SubscribersList class to manage actual node construction
+ * and handling, and is mostly in charge of communication with Launchpad.
+ *
+ * Loading is triggered automatically on instance construction.
+ *
+ * @class BugSubscribersLoader
+ * @param config {Object} Defines `container_box' CSS selector for the
+ * SubscribersList container box, `bug' holding bug metadata (at
+ * least with `web_link') and `subscribers_details_view' holding
+ * a relative URI to load subscribers' details from.
+ */
+function BugSubscribersLoader(config) {
+ var sl = this.subscribers_list = new SubscribersList(config);
+ if (!Y.Lang.isValue(config.bug) ||
+ !Y.Lang.isString(config.bug.web_link)) {
+ Y.error(
+ "No bug specified in `config' or bug.web_link is invalid.");
+ }
+ this.bug = config.bug;
+
+ if (!Y.Lang.isString(config.subscribers_details_view)) {
+ Y.error(
+ "No config.subscribers_details_view specified to load " +
+ "other bug subscribers from.");
+ }
+ this.subscribers_portlet_uri = (
+ this.bug.web_link + config.subscribers_details_view);
+
+ this.error_handler = new Y.lp.client.FormErrorHandler();
+ this.error_handler.showError = function (error_msg) {
+ sl.stopActivity("Problem loading subscribers. " + error_msg);
+ };
+
+ // Allow tests to override lp_client.
+ if (Y.Lang.isValue(config.lp_client)) {
+ this.lp_client = config.lp_client;
+ } else {
+ this.lp_client = new Y.lp.client.Launchpad();
+ }
+
+ this._loadSubscribers();
+}
+namespace.BugSubscribersLoader = BugSubscribersLoader;
+
+/**
+ * Adds a subscriber along with the unsubscribe callback if needed.
+ *
+ * @method _addSubscriber
+ * @param subscriber {Object} A common subscriber object passed
+ * directly to SubscribersList.addSubscriber().
+ * If subscriber.can_edit === true, adds an unsubscribe callback
+ * as returned by this._getUnsubscribeCallback().
+ * @param level {String} A bug subscription level (one of
+ * subscriber_levels values). When level doesn't match any of the
+ * supported levels, 'Maybe' is used instead.
+ */
+BugSubscribersLoader.prototype._addSubscriber = function(subscriber, level) {
+ if (!subscriber_levels.hasOwnProperty(level)) {
+ // Default to 'subscribed at unknown level' for unrecognized
+ // subscription levels.
+ level = 'Maybe';
+ }
+
+ var unsubscribe_callback = this._getUnsubscribeCallback();
+
+ if (subscriber.can_edit === true) {
+ this.subscribers_list.addSubscriber(subscriber, level, {
+ unsubscribe_callback: unsubscribe_callback});
+ } else {
+ this.subscribers_list.addSubscriber(subscriber, level);
+ }
+};
+
+/**
+ * Load bug subscribers from the list of subscribers and add subscriber rows
+ * for them.
+ *
+ * @method _loadSubscribersFromList
+ * @param details {List} List of subscribers with their subscription levels.
+ */
+BugSubscribersLoader.prototype._loadSubscribersFromList = function(
+ details) {
+ if (!Y.Lang.isArray(details)) {
+ Y.error('Got non-array "'+ details +
+ '" in _loadSubscribersFromList().');
+ }
+ var index, subscriber, level;
+ for (index = 0; index < details.length; index++) {
+ subscriber = details[index].subscriber;
+ if (!Y.Lang.isObject(details[index])) {
+ Y.error('Subscriber details at index ' + index + ' (' +
+ details[index] + ') are not an object.');
+ }
+ this._addSubscriber(subscriber,
+ details[index].subscription_level);
+ }
+};
+
+/**
+ * Load subscribers from the JSON portlet with details, adding them
+ * to the actual subscribers list managed by this class.
+ *
+ * JSON string in the portlet should be of the following form:
+ *
+ * [ { "subscriber": {
+ * "name": "foobar",
+ * "display_name": "Foo Bar",
+ * "can_edit": true/false,
+ * "is_team": false/false,
+ * "web_link": "https://launchpad.dev/~foobar";
+ * },
+ * "subscription_level": "Details"},
+ * { "subscriber": ... }
+ * ]
+ * JSON itself is parsed by lp_client.get().
+ *
+ * Uses SubscribersList startActivity/stopActivity methods to indicate
+ * progress and/or any errors it hits.
+ *
+ * @method _loadSubscribers
+ */
+BugSubscribersLoader.prototype._loadSubscribers = function() {
+ var sl = this.subscribers_list;
+ var loader = this;
+
+ // Fetch the person and add a subscription.
+ function on_success(subscribers) {
+ loader._loadSubscribersFromList(subscribers);
+ loader.subscribers_list.stopActivity();
+ }
+
+ var config = { on: {
+ success: on_success,
+ failure: this.error_handler.getFailureHandler()
+ } };
+
+ sl.startActivity("Loading subscribers...");
+ this.lp_client.get(this.subscribers_portlet_uri, config);
+};
+
+/**
+ * Return a function object that accepts SubscribersList and subscriber
+ * objects as parameters.
+ *
+ * @method _getUnsubscribeCallback
+ */
+BugSubscribersLoader.prototype._getUnsubscribeCallback = function() {
+ return function(subscribers_list, subscriber) {
+ subscribers_list.indicateSubscriberActivity(subscriber);
+ // Simulated unsubscribing action for prototyping the UI.
+ setTimeout(function() {
+ subscribers_list.stopSubscriberActivity(
+ subscriber, true, function() {
+ subscribers_list.removeSubscriber(subscriber);
+ });
+ }, 2400);
+ };
+};
+
/**
* Manages entire subscribers' list for a single bug.
=== modified file 'lib/lp/bugs/javascript/tests/test_subscribers_list.js'
--- lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-10 13:53:33 +0000
+++ lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-10 13:53:35 +0000
@@ -1586,6 +1586,388 @@
}));
+/**
+ * Set-up all the nodes required for BugSubscriberLoader.
+ */
+function setUpLoader(root_node, config) {
+ // Set-up subscribers list node.
+ var node = Y.Node.create('<div></div>')
+ .set('id', 'other-subscribers-container');
+ var container_config = {
+ container_box: '#other-subscribers-container'
+ };
+ root_node.appendChild(node);
+ if (Y.Lang.isValue(config)) {
+ config = Y.mix(container_config, config);
+ } else {
+ config = container_config;
+ }
+ return new module.BugSubscribersLoader(config);
+}
+
+/**
+ * Test BugSubscribersLoader class construction.
+ */
+suite.add(new Y.Test.Case({
+ name: 'BugSubscribersLoader() construction test',
+
+ _should: {
+ error: {
+ test_BugSubscribersLoader_container_error:
+ new Error(
+ 'Container node must be specified in config.container_box.'),
+ test_BugSubscribersLoader_bug_error:
+ new Error(
+ "No bug specified in `config' or bug.web_link is invalid."),
+ test_BugSubscribersLoader_bug_web_link_error:
+ new Error(
+ "No bug specified in `config' or bug.web_link is invalid."),
+ test_BugSubscribersLoader_portlet_link_error:
+ new Error(
+ "No config.subscribers_details_view specified to load " +
+ "other bug subscribers from.")
+ }
+ },
+
+ setUp: function() {
+ this.root = Y.Node.create('<div></div>');
+ Y.one('body').appendChild(this.root);
+ },
+
+ tearDown: function() {
+ this.root.remove();
+ },
+
+ test_BugSubscribersLoader_container_error: function() {
+ // If no container node to hold the subscribers list is specified,
+ // it fails with an error.
+ var loader =
+ new module.BugSubscribersLoader({container_box: '#not-found'});
+ },
+
+ test_BugSubscribersLoader_bug_error: function() {
+ // Bug needs to be passed in as well.
+ // setUpLoader constructs the container node for us.
+ var config = {};
+ setUpLoader(this.root, config);
+ },
+
+ test_BugSubscribersLoader_bug_web_link_error: function() {
+ // Fails if the passed in bug has no web_link attribute defined.
+ var config = { bug: {} };
+ setUpLoader(this.root, config);
+ },
+
+ test_BugSubscribersLoader_portlet_link_error: function() {
+ // Fails if the passed in config has no passed in
+ // portlet URI for loading bug subscribers details.
+ var config = { bug: { web_link: '' } };
+ setUpLoader(this.root, config);
+ },
+
+ test_BugSubscribersLoader: function() {
+ // With all the parameters specified, it returns an instance
+ // with subscribers_portlet_uri, subscribers_list, error_handler,
+ // and calls the _loadSubscribers() method.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+
+ // Save original method for restoring later.
+ var old_load = module.BugSubscribersLoader.prototype._loadSubscribers;
+
+ var loading_started = false;
+ module.BugSubscribersLoader.prototype._loadSubscribers = function() {
+ loading_started = true;
+ };
+ var loader = setUpLoader(this.root, config);
+ Y.Assert.areEqual('/base/+details', loader.subscribers_portlet_uri);
+ Y.Assert.isNotNull(loader.subscribers_list);
+ Y.Assert.isTrue(
+ loader.subscribers_list instanceof module.SubscribersList);
+ Y.Assert.isNotNull(loader.error_handler);
+ Y.Assert.isTrue(loading_started);
+
+ // Restore original method.
+ module.BugSubscribersLoader.prototype._loadSubscribers = old_load;
+ }
+}));
+
+
+/**
+ * Test BugSubscribersLoader subscribers loading and helper methods.
+ */
+suite.add(new Y.Test.Case({
+ name: 'BugSubscribersLoader() subscribers loading test',
+
+ _should: {
+ error: {
+ test_loadSubscribersFromList_not_list_error:
+ new Error('Got non-array "Not-a-list" in ' +
+ '_loadSubscribersFromList().'),
+ test_loadSubscribersFromList_no_objects_error:
+ new Error('Subscriber details at index 0 (Subscriber)' +
+ ' are not an object.')
+ }
+ },
+
+ setUp: function() {
+ this.root = Y.Node.create('<div></div>');
+ Y.one('body').appendChild(this.root);
+ },
+
+ tearDown: function() {
+ this.root.remove();
+ },
+
+ test_addSubscriber_wraps_list_addSubscriber: function() {
+ // addSubscriber wraps the SubscribersList.addSubscriber.
+ // When no can_edit is set on the subscriber, no unsubscribe
+ // callback is added.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var subscriber = { name: "user" };
+ var level = "Discussion";
+ var call_passed_through = false;
+ // Save the old method for restoring later.
+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
+ module.SubscribersList.prototype.addSubscriber = function(
+ passed_subscriber, passed_level) {
+ call_passed_through = true;
+ Y.Assert.areSame(subscriber, passed_subscriber);
+ Y.Assert.areSame(level, passed_level);
+ };
+ var loader = setUpLoader(this.root, config);
+ loader._addSubscriber(subscriber, level);
+ Y.Assert.isTrue(call_passed_through);
+
+ // Restore the real method.
+ module.SubscribersList.prototype.addSubscriber = old_addSub;
+ },
+
+ test_addSubscriber_normalizes_level: function() {
+ // addSubscriber normalizes the subscription level to 'Maybe'
+ // when it's otherwise unknown subscription level.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var subscriber = { name: "user" };
+ var level = "Not a level";
+
+ // Save the old method for restoring later.
+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
+ module.SubscribersList.prototype.addSubscriber = function(
+ passed_subscriber, passed_level, passed_config) {
+ Y.Assert.areSame('Maybe', passed_level);
+ Y.Assert.isUndefined(passed_config);
+ };
+ var loader = setUpLoader(this.root, config);
+ loader._addSubscriber(subscriber, level);
+
+ // Restore the real method.
+ module.SubscribersList.prototype.addSubscriber = old_addSub;
+ },
+
+ test_addSubscriber_unsubscribe_callback: function() {
+ // addSubscriber sets the unsubscribe callback to function
+ // returned by BugSubscribersLoader._getUnsubscribeCallback()
+ // if subscriber.can_edit === true.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var subscriber = { name: "user", can_edit: true };
+ var unsubscribe_callback = function() {};
+
+ // Save old methods for restoring later.
+ var old_getUnsub = module.BugSubscribersLoader.prototype
+ ._getUnsubscribeCallback;
+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
+
+ // Make _getUnsubscribeCallback return the new callback.
+ module.BugSubscribersLoader.prototype._getUnsubscribeCallback =
+ function() {
+ return unsubscribe_callback;
+ };
+
+ // Assert in addSubscriber that it's being passed the new
+ // callback in the config parameter.
+ module.SubscribersList.prototype.addSubscriber = function(
+ passed_subscriber, passed_level, passed_config) {
+ Y.Assert.areSame(unsubscribe_callback,
+ passed_config.unsubscribe_callback);
+ };
+
+ var loader = setUpLoader(this.root, config);
+ loader._addSubscriber(subscriber);
+
+ // Restore original methods.
+ module.BugSubscribersLoader.prototype._getUnsubscribeCallback =
+ old_getUnsub;
+ module.SubscribersList.prototype.addSubscriber = old_addSub;
+ },
+
+
+ test_loadSubscribersFromList: function() {
+ // Accepts a list of dicts with 'subscriber' and 'subscription_level'
+ // fields, passing them directly to _addSubscriber() method.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var data = [{ subscriber: { name: "Subscriber 1" },
+ subscription_level: "Lifecycle" },
+ { subscriber: { name: "Subscriber 2" },
+ subscription_level: "Unknown" }];
+
+ // Save the original method for restoring later.
+ var old_addSub = module.BugSubscribersLoader.prototype._addSubscriber;
+
+ var call_count = 0;
+ module.BugSubscribersLoader.prototype._addSubscriber =
+ function(subscriber, level) {
+ call_count++;
+ if (call_count === 1) {
+ Y.Assert.areEqual("Subscriber 1", subscriber.name);
+ Y.Assert.areEqual("Lifecycle", level);
+ } else if (call_count === 2) {
+ Y.Assert.areEqual("Subscriber 2", subscriber.name);
+ Y.Assert.areEqual("Unknown", level);
+ }
+ };
+
+ var loader = setUpLoader(this.root, config);
+ loader._loadSubscribersFromList(data);
+
+ // Two subscribers have been processed total.
+ Y.Assert.areEqual(2, call_count);
+
+ // Restore the original method.
+ module.BugSubscribersLoader.prototype._addSubscriber = old_addSub;
+ },
+
+ test_loadSubscribersFromList_not_list_error: function() {
+ // When the data is not a list, it throws an error.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var data = "Not-a-list";
+
+ var loader = setUpLoader(this.root, config);
+ loader._loadSubscribersFromList(data);
+ },
+
+ test_loadSubscribersFromList_no_objects_error: function() {
+ // When the data is not a list of objects, it throws an error.
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+ var data = ["Subscriber"];
+
+ var loader = setUpLoader(this.root, config);
+ loader._loadSubscribersFromList(data);
+ },
+
+ test_loadSubscribers_success: function() {
+ // Testing successful operation of _loadSubscribers.
+ var details = [
+ { subscriber: { name: "subscriber" },
+ subscription_level: "Details" }
+ ];
+
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+
+ // Override loadSubscribersList to ensure it gets called with
+ // the right parameters.
+ var old_loadSubsList =
+ module.BugSubscribersLoader.prototype._loadSubscribersFromList;
+ var loading_done = false;
+ module.BugSubscribersLoader.prototype._loadSubscribersFromList =
+ function(my_details) {
+ Y.Assert.areSame(details, my_details);
+ loading_done = true;
+ };
+
+ var loader = setUpLoader(this.root, config);
+
+ // Mock lp_client for testing.
+ loader.lp_client = {
+ get: function(uri, get_config) {
+ // Assert that there is activity in progress.
+ var node = loader.subscribers_list.container_node
+ .one('.global-activity-indicator');
+ Y.Assert.isNotNull(node);
+ // Call the success handler.
+ get_config.on.success(details);
+ }
+ };
+ // Re-run _loadSubscribers with our mock methods in place.
+ loader._loadSubscribers();
+
+ // Assert that _loadSubscribersList was run in the process.
+ Y.Assert.isTrue(loading_done);
+
+ // And activity node was removed when everything was done.
+ var node = loader.subscribers_list.container_node
+ .one('.global-activity-indicator');
+ Y.Assert.isNull(node);
+
+ // Restore original method.
+ module.BugSubscribersLoader.prototype._loadSubscribersFromList =
+ old_loadSubsList;
+ },
+
+ test_loadSubscribers_failure: function() {
+ // On failure to load, activity indication is set to an error
+ // message received from the server.
+ var details = [
+ { subscriber: { name: "subscriber" },
+ subscription_level: "Details" }
+ ];
+
+ var config = {
+ bug: { web_link: '/base' },
+ subscribers_details_view: '/+details'
+ };
+
+ var loader = setUpLoader(this.root, config);
+
+ // Mock lp_client for testing erroring out with 'BOOM'.
+ loader.lp_client = {
+ get: function(uri, get_config) {
+ // Assert that there is activity in progress.
+ var node = loader.subscribers_list.container_node
+ .one('.global-activity-indicator');
+ Y.Assert.isNotNull(node);
+ // Call the success handler.
+ get_config.on.failure(1,{ status: 403,
+ statusText: 'BOOM',
+ responseText: '' });
+ }
+ };
+ // Re-run _loadSubscribers with our mock methods in place.
+ loader._loadSubscribers();
+
+ // And activity node is there with an error message.
+ var node = loader.subscribers_list.container_node
+ .one('.global-activity-indicator');
+ Y.Assert.isNotNull(node);
+ Y.Assert.areEqual('file:///@@/error', node.one('img').get('src'));
+ Y.Assert.areEqual(' Problem loading subscribers. 403 BOOM',
+ node.one('span').get('text'));
+ }
+}));
+
+
var handle_complete = function(data) {
status_node = Y.Node.create(
'<p id="complete">Test status: complete</p>');
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-06-10 13:53:33 +0000
+++ lib/lp/bugs/model/bug.py 2011-06-10 13:53:35 +0000
@@ -939,6 +939,17 @@
recipients.addDirectSubscriber(subscriber)
return subscriptions.subscribers.sorted
+ def getDirectSubscribersWithDetails(self):
+ """See `IBug`."""
+ results = Store.of(self).find(
+ (Person, BugSubscription),
+ BugSubscription.person_id == Person.id,
+ BugSubscription.bug_id == self.id,
+ Not(In(BugSubscription.person_id,
+ Select(BugMute.person_id, BugMute.bug_id == self.id)))
+ ).order_by(Person.displayname)
+ return results
+
def getIndirectSubscribers(self, recipients=None, level=None):
"""See `IBug`.
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2011-06-07 06:24:04 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2011-06-10 13:53:35 +0000
@@ -202,6 +202,37 @@
set(subscribers), set(direct_subscribers),
"Subscribers did not match expected value.")
+ def test_get_direct_subscribers_with_details(self):
+ # getDirectSubscribersWithDetails() returns both
+ # Person and BugSubscription records in one go.
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ # Unsubscribe bug owner so it doesn't taint the result.
+ bug.unsubscribe(bug.owner, bug.owner)
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ subscription = bug.subscribe(
+ subscriber, subscriber, level=BugNotificationLevel.LIFECYCLE)
+
+ self.assertContentEqual(
+ [(subscriber, subscription)],
+ bug.getDirectSubscribersWithDetails())
+
+ def test_get_direct_subscribers_with_details_mute_excludes(self):
+ # getDirectSubscribersWithDetails excludes muted subscriptions.
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ # Unsubscribe bug owner so it doesn't taint the result.
+ bug.unsubscribe(bug.owner, bug.owner)
+ subscriber = self.factory.makePerson()
+ with person_logged_in(subscriber):
+ bug.subscribe(
+ subscriber, subscriber, level=BugNotificationLevel.LIFECYCLE)
+ bug.mute(subscriber, subscriber)
+
+ self.assertContentEqual(
+ [], bug.getDirectSubscribersWithDetails())
+
def test_subscribers_from_dupes_uses_level(self):
# When getSubscribersFromDuplicates() is passed a `level`
# parameter it will include only subscribers subscribed to
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-05-20 21:09:40 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-06-10 13:53:35 +0000
@@ -10,6 +10,7 @@
<div tal:define="context_menu context/menu:context">
<div tal:content="structure context_menu/addsubscriber/render" />
</div>
+ <div id="other-bug-subscribers"></div>
<a id="subscribers-ids-link"
tal:define="bug context/bug|context"
tal:attributes="href bug/fmt:url/+bug-portlet-subscribers-ids"></a>
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-05-20 21:09:40 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-06-10 13:53:35 +0000
@@ -43,7 +43,7 @@
</tal:if>
<script type="text/javascript">
LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
- 'lp.bugs.bugtask_index.portlets',
+ 'lp.bugs.bugtask_index.portlets', 'lp.bugs.subscribers_list',
'lp.code.branchmergeproposal.diff', 'lp.comments.hide',
function(Y) {
Y.lp.bugs.bugtask_index.portlets.use_advanced_subscriptions =
@@ -55,6 +55,12 @@
Y.on('domready', function() {
LP.cache.comment_context = LP.cache.bug;
Y.lp.comments.hide.setup_hide_controls();
+ var sl = new Y.lp.bugs.subscribers_list.BugSubscribersLoader({
+ container_box: '#other-bug-subscribers',
+ bug: LP.cache.bug,
+ subscribers_details_view:
+ '/+bug-portlet-subscribers-details'
+ });
});
});
</script>
Follow ups
