Merge lp:~danilo/launchpad/bug-772754-other-subscribers-loading into lp:launchpad

Proposed by Данило Шеган
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 13243
Proposed branch: lp:~danilo/launchpad/bug-772754-other-subscribers-loading
Merge into: lp:launchpad
Prerequisite: lp:~danilo/launchpad/bug-772754-other-subscribers-activity
Diff against target: 933 lines (+781/-2)
11 files modified
lib/lp/bugs/browser/bugsubscription.py (+54/-1)
lib/lp/bugs/browser/configure.zcml (+6/-0)
lib/lp/bugs/browser/tests/test_bugsubscription_views.py (+147/-0)
lib/lp/bugs/configure.zcml (+1/-0)
lib/lp/bugs/interfaces/bug.py (+9/-0)
lib/lp/bugs/javascript/subscribers_list.js (+162/-0)
lib/lp/bugs/javascript/tests/test_subscribers_list.js (+352/-0)
lib/lp/bugs/model/bug.py (+11/-0)
lib/lp/bugs/model/tests/test_bug.py (+31/-0)
lib/lp/bugs/templates/bug-portlet-subscribers.pt (+1/-0)
lib/lp/bugs/templates/bugtask-index.pt (+7/-1)
To merge this branch: bzr merge lp:~danilo/launchpad/bug-772754-other-subscribers-loading
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+64186@code.launchpad.net

Description of the change

= 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.

Note how this doesn't really remove any of the existing subscribers loading, so you end up with two subscribers lists after this.

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).

Another thing I am unsure about is if I should move BugSubscribersLoader into a separate JS module. SubscribersList can be useful for more than just bug subscriptions (fwiw, it could work well on the MP page like this one).

== Tests ==

lp/bugs/javascript/tests/test_subscribers_list.html

bin/test -cvvt BugPortletSubscribersWithDetailsTests -t test_get_direct_subscribers_with_details

== 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

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
2--- lib/lp/bugs/browser/bugsubscription.py 2011-06-15 11:08:00 +0000
3+++ lib/lp/bugs/browser/bugsubscription.py 2011-06-15 11:08:14 +0000
4@@ -26,6 +26,7 @@
5 SimpleTerm,
6 SimpleVocabulary,
7 )
8+from zope.security.proxy import removeSecurityProxy
9
10 from canonical.launchpad import _
11 from canonical.launchpad.webapp import (
12@@ -327,7 +328,7 @@
13 if self._use_advanced_features:
14 self.widgets['bug_notification_level'].widget_class = (
15 'bug-notification-level-field')
16- if (len(self.form_fields['subscription'].field.vocabulary)==1):
17+ if (len(self.form_fields['subscription'].field.vocabulary) == 1):
18 # We hide the subscription widget if the user isn't
19 # subscribed, since we know who the subscriber is and we
20 # don't need to present them with a single radio button.
21@@ -581,6 +582,58 @@
22 key=(lambda subscription: subscription.person.displayname))]
23
24
25+class BugPortletSubscribersWithDetails(LaunchpadView):
26+ """A view that returns a JSON dump of the subscriber details for a bug."""
27+
28+ @property
29+ def subscriber_data_js(self):
30+ """Return subscriber_ids in a form suitable for JavaScript use."""
31+ data = []
32+ details = list(self.context.getDirectSubscribersWithDetails())
33+ for person, subscription in details:
34+ if person == self.user:
35+ # Skip the current user viewing the page.
36+ continue
37+ can_edit = self.user is not None and self.user.inTeam(person)
38+ subscriber = {
39+ 'name': person.name,
40+ 'display_name': person.displayname,
41+ 'web_link': canonical_url(person, rootsite='mainsite'),
42+ 'is_team': person.is_team,
43+ 'can_edit': can_edit,
44+ }
45+ record = {
46+ 'subscriber': subscriber,
47+ 'subscription_level': str(
48+ removeSecurityProxy(subscription.bug_notification_level)),
49+ }
50+ data.append(record)
51+
52+ others = list(self.context.getIndirectSubscribers())
53+ for person in others:
54+ if person == self.user:
55+ # Skip the current user viewing the page.
56+ continue
57+ subscriber = {
58+ 'name': person.name,
59+ 'display_name': person.displayname,
60+ 'web_link': canonical_url(person, rootsite='mainsite'),
61+ 'is_team': person.is_team,
62+ 'can_edit': False,
63+ }
64+ record = {
65+ 'subscriber': subscriber,
66+ 'subscription_level': 'Maybe',
67+ }
68+ data.append(record)
69+ return dumps(data)
70+
71+ def render(self):
72+ """Override the default render() to return only JSON."""
73+ self.request.response.setHeader('content-type', 'application/json')
74+ return self.subscriber_data_js
75+
76+
77 class BugPortletSubcribersIds(LaunchpadView, BugViewMixin):
78 """A view that returns a JSON dump of the subscriber IDs for a bug."""
79
80
81=== modified file 'lib/lp/bugs/browser/configure.zcml'
82--- lib/lp/bugs/browser/configure.zcml 2011-06-15 11:08:00 +0000
83+++ lib/lp/bugs/browser/configure.zcml 2011-06-15 11:08:14 +0000
84@@ -1100,6 +1100,12 @@
85 name="+bug-portlet-subscribers-ids"
86 class="lp.bugs.browser.bugsubscription.BugPortletSubcribersIds"
87 permission="zope.Public"/>
88+ <browser:page
89+ for="lp.bugs.interfaces.bug.IBug"
90+ name="+bug-portlet-subscribers-details"
91+ class="
92+ lp.bugs.browser.bugsubscription.BugPortletSubscribersWithDetails"
93+ permission="zope.Public"/>
94 <browser:navigation
95 module="lp.bugs.browser.bug"
96 classes="
97
98=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
99--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-06-02 15:34:06 +0000
100+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-06-15 11:08:14 +0000
101@@ -5,11 +5,15 @@
102
103 __metaclass__ = type
104
105+from simplejson import dumps
106+
107 from canonical.launchpad.ftests import LaunchpadFormHarness
108+from canonical.launchpad.webapp import canonical_url
109 from canonical.testing.layers import LaunchpadFunctionalLayer
110
111 from lp.bugs.browser.bugsubscription import (
112 BugPortletSubcribersIds,
113+ BugPortletSubscribersWithDetails,
114 BugSubscriptionListView,
115 BugSubscriptionSubscribeSelfView,
116 )
117@@ -479,3 +483,146 @@
118 self.bug.default_bugtask, name="+mute",
119 form={'field.actions.unmute': 'Unmute bug mail'})
120 self.assertFalse(self.bug.isMuted(self.person))
121+
122+
123+class BugPortletSubscribersWithDetailsTests(TestCaseWithFactory):
124+ """Tests for IBug:+bug-portlet-subscribers-details view."""
125+ layer = LaunchpadFunctionalLayer
126+
127+ def test_content_type(self):
128+ bug = self.factory.makeBug()
129+
130+ # It works even for anonymous users, so no log-in is needed.
131+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
132+ harness.view.render()
133+
134+ self.assertEqual(
135+ harness.request.response.getHeader('content-type'),
136+ 'application/json')
137+
138+ def _makeBugWithNoSubscribers(self):
139+ bug = self.factory.makeBug()
140+ with person_logged_in(bug.owner):
141+ # Unsubscribe the bug reporter to ensure we have no subscribers.
142+ bug.unsubscribe(bug.owner, bug.owner)
143+ return bug
144+
145+ def test_data_no_subscriptions(self):
146+ bug = self._makeBugWithNoSubscribers()
147+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
148+ self.assertEqual(dumps([]), harness.view.subscriber_data_js)
149+
150+ def test_data_person_subscription(self):
151+ # A subscriber_data_js returns JSON string of a list
152+ # containing all subscriber information needed for
153+ # subscribers_list.js subscribers loading.
154+ bug = self._makeBugWithNoSubscribers()
155+ subscriber = self.factory.makePerson(
156+ name='user', displayname='Subscriber Name')
157+ with person_logged_in(subscriber):
158+ bug.subscribe(subscriber, subscriber,
159+ level=BugNotificationLevel.LIFECYCLE)
160+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
161+
162+ expected_result = {
163+ 'subscriber': {
164+ 'name': 'user',
165+ 'display_name': 'Subscriber Name',
166+ 'is_team': False,
167+ 'can_edit': False,
168+ 'web_link': canonical_url(subscriber),
169+ },
170+ 'subscription_level': "Lifecycle",
171+ }
172+ self.assertEqual(
173+ dumps([expected_result]), harness.view.subscriber_data_js)
174+
175+ def test_data_team_subscription(self):
176+ # For a team subscription, subscriber_data_js has is_team set
177+ # to true.
178+ bug = self._makeBugWithNoSubscribers()
179+ subscriber = self.factory.makeTeam(
180+ name='team', displayname='Team Name')
181+ with person_logged_in(subscriber.teamowner):
182+ bug.subscribe(subscriber, subscriber.teamowner,
183+ level=BugNotificationLevel.LIFECYCLE)
184+ harness = LaunchpadFormHarness(bug, BugPortletSubscribersWithDetails)
185+
186+ expected_result = {
187+ 'subscriber': {
188+ 'name': 'team',
189+ 'display_name': 'Team Name',
190+ 'is_team': True,
191+ 'can_edit': False,
192+ 'web_link': canonical_url(subscriber),
193+ },
194+ 'subscription_level': "Lifecycle",
195+ }
196+ self.assertEqual(
197+ dumps([expected_result]), harness.view.subscriber_data_js)
198+
199+ def test_data_team_subscription_owner_looks(self):
200+ # For a team subscription, subscriber_data_js has can_edit
201+ # set to true for team owner.
202+ bug = self._makeBugWithNoSubscribers()
203+ subscriber = self.factory.makeTeam(
204+ name='team', displayname='Team Name')
205+ with person_logged_in(subscriber.teamowner):
206+ bug.subscribe(subscriber, subscriber.teamowner,
207+ level=BugNotificationLevel.LIFECYCLE)
208+ harness = LaunchpadFormHarness(
209+ bug, BugPortletSubscribersWithDetails)
210+
211+ expected_result = {
212+ 'subscriber': {
213+ 'name': 'team',
214+ 'display_name': 'Team Name',
215+ 'is_team': True,
216+ 'can_edit': True,
217+ 'web_link': canonical_url(subscriber),
218+ },
219+ 'subscription_level': "Lifecycle",
220+ }
221+ with person_logged_in(subscriber.teamowner):
222+ self.assertEqual(
223+ dumps([expected_result]), harness.view.subscriber_data_js)
224+
225+ def test_data_team_subscription_member_looks(self):
226+ # For a team subscription, subscriber_data_js has can_edit
227+ # set to true for team member.
228+ bug = self._makeBugWithNoSubscribers()
229+ member = self.factory.makePerson()
230+ subscriber = self.factory.makeTeam(
231+ name='team', displayname='Team Name', members=[member])
232+ with person_logged_in(subscriber.teamowner):
233+ bug.subscribe(subscriber, subscriber.teamowner,
234+ level=BugNotificationLevel.LIFECYCLE)
235+ harness = LaunchpadFormHarness(
236+ bug, BugPortletSubscribersWithDetails)
237+
238+ expected_result = {
239+ 'subscriber': {
240+ 'name': 'team',
241+ 'display_name': 'Team Name',
242+ 'is_team': True,
243+ 'can_edit': True,
244+ 'web_link': canonical_url(subscriber),
245+ },
246+ 'subscription_level': "Lifecycle",
247+ }
248+ with person_logged_in(subscriber.teamowner):
249+ self.assertEqual(
250+ dumps([expected_result]), harness.view.subscriber_data_js)
251+
252+ def test_data_person_subscription_user_excluded(self):
253+ # With the subscriber logged in, he is not included in the results.
254+ bug = self._makeBugWithNoSubscribers()
255+ subscriber = self.factory.makePerson(
256+ name='a-person', displayname='Subscriber Name')
257+
258+ with person_logged_in(subscriber):
259+ bug.subscribe(subscriber, subscriber,
260+ level=BugNotificationLevel.LIFECYCLE)
261+ harness = LaunchpadFormHarness(
262+ bug, BugPortletSubscribersWithDetails)
263+ self.assertEqual(dumps([]), harness.view.subscriber_data_js)
264
265=== modified file 'lib/lp/bugs/configure.zcml'
266--- lib/lp/bugs/configure.zcml 2011-06-06 13:54:07 +0000
267+++ lib/lp/bugs/configure.zcml 2011-06-15 11:08:14 +0000
268@@ -744,6 +744,7 @@
269 date_last_updated
270 getBugTask
271 getDirectSubscribers
272+ getDirectSubscribersWithDetails
273 getDirectSubscriptions
274 getIndirectSubscribers
275 is_complete
276
277=== modified file 'lib/lp/bugs/interfaces/bug.py'
278--- lib/lp/bugs/interfaces/bug.py 2011-05-24 08:58:38 +0000
279+++ lib/lp/bugs/interfaces/bug.py 2011-06-15 11:08:14 +0000
280@@ -522,6 +522,15 @@
281 Direct subscribers have an entry in the BugSubscription table.
282 """
283
284+ def getDirectSubscribersWithDetails():
285+ """Get direct subscribers and their subscriptions for the bug.
286+
287+ Those with muted bug subscriptions are excluded from results.
288+
289+ :returns: A ResultSet of tuples (Person, BugSubscription)
290+ representing a subscriber and their bug subscription.
291+ """
292+
293 def getIndirectSubscribers(recipients=None, level=None):
294 """Return IPersons that are indirectly subscribed to this bug.
295
296
297=== modified file 'lib/lp/bugs/javascript/subscribers_list.js'
298--- lib/lp/bugs/javascript/subscribers_list.js 2011-06-15 11:08:00 +0000
299+++ lib/lp/bugs/javascript/subscribers_list.js 2011-06-15 11:08:14 +0000
300@@ -121,6 +121,168 @@
301 }
302
303
304+/**
305+ * Load subscribers for a bug from Launchpad and put them in the web page.
306+ *
307+ * Uses SubscribersList class to manage actual node construction
308+ * and handling, and is mostly in charge of communication with Launchpad.
309+ *
310+ * Loading is triggered automatically on instance construction.
311+ *
312+ * @class BugSubscribersLoader
313+ * @param config {Object} Defines `container_box' CSS selector for the
314+ * SubscribersList container box, `bug' holding bug metadata (at
315+ * least with `web_link') and `subscribers_details_view' holding
316+ * a relative URI to load subscribers' details from.
317+ */
318+function BugSubscribersLoader(config) {
319+ var sl = this.subscribers_list = new SubscribersList(config);
320+ if (!Y.Lang.isValue(config.bug) ||
321+ !Y.Lang.isString(config.bug.web_link)) {
322+ Y.error(
323+ "No bug specified in `config' or bug.web_link is invalid.");
324+ }
325+ this.bug = config.bug;
326+
327+ if (!Y.Lang.isString(config.subscribers_details_view)) {
328+ Y.error(
329+ "No config.subscribers_details_view specified to load " +
330+ "other bug subscribers from.");
331+ }
332+ this.subscribers_portlet_uri = (
333+ this.bug.web_link + config.subscribers_details_view);
334+
335+ this.error_handler = new Y.lp.client.FormErrorHandler();
336+ this.error_handler.showError = function (error_msg) {
337+ sl.stopActivity("Problem loading subscribers. " + error_msg);
338+ };
339+
340+ // Allow tests to override lp_client.
341+ if (Y.Lang.isValue(config.lp_client)) {
342+ this.lp_client = config.lp_client;
343+ } else {
344+ this.lp_client = new Y.lp.client.Launchpad();
345+ }
346+
347+ this._loadSubscribers();
348+}
349+namespace.BugSubscribersLoader = BugSubscribersLoader;
350+
351+/**
352+ * Adds a subscriber along with the unsubscribe callback if needed.
353+ *
354+ * @method _addSubscriber
355+ * @param subscriber {Object} A common subscriber object passed
356+ * directly to SubscribersList.addSubscriber().
357+ * If subscriber.can_edit === true, adds an unsubscribe callback
358+ * as returned by this._getUnsubscribeCallback().
359+ * @param level {String} A bug subscription level (one of
360+ * subscriber_levels values). When level doesn't match any of the
361+ * supported levels, 'Maybe' is used instead.
362+ */
363+BugSubscribersLoader.prototype._addSubscriber = function(subscriber, level) {
364+ if (!subscriber_levels.hasOwnProperty(level)) {
365+ // Default to 'subscribed at unknown level' for unrecognized
366+ // subscription levels.
367+ level = 'Maybe';
368+ }
369+
370+ var unsubscribe_callback = this._getUnsubscribeCallback();
371+
372+ if (subscriber.can_edit === true) {
373+ this.subscribers_list.addSubscriber(subscriber, level, {
374+ unsubscribe_callback: unsubscribe_callback});
375+ } else {
376+ this.subscribers_list.addSubscriber(subscriber, level);
377+ }
378+};
379+
380+/**
381+ * Load bug subscribers from the list of subscribers and add subscriber rows
382+ * for them.
383+ *
384+ * @method _loadSubscribersFromList
385+ * @param details {List} List of subscribers with their subscription levels.
386+ */
387+BugSubscribersLoader.prototype._loadSubscribersFromList = function(
388+ details) {
389+ if (!Y.Lang.isArray(details)) {
390+ Y.error('Got non-array "'+ details +
391+ '" in _loadSubscribersFromList().');
392+ }
393+ var index, subscriber, level;
394+ for (index = 0; index < details.length; index++) {
395+ subscriber = details[index].subscriber;
396+ if (!Y.Lang.isObject(details[index])) {
397+ Y.error('Subscriber details at index ' + index + ' (' +
398+ details[index] + ') are not an object.');
399+ }
400+ this._addSubscriber(subscriber,
401+ details[index].subscription_level);
402+ }
403+};
404+
405+/**
406+ * Load subscribers from the JSON portlet with details, adding them
407+ * to the actual subscribers list managed by this class.
408+ *
409+ * JSON string in the portlet should be of the following form:
410+ *
411+ * [ { "subscriber": {
412+ * "name": "foobar",
413+ * "display_name": "Foo Bar",
414+ * "can_edit": true/false,
415+ * "is_team": false/false,
416+ * "web_link": "https://launchpad.dev/~foobar"
417+ * },
418+ * "subscription_level": "Details"},
419+ * { "subscriber": ... }
420+ * ]
421+ * JSON itself is parsed by lp_client.get().
422+ *
423+ * Uses SubscribersList startActivity/stopActivity methods to indicate
424+ * progress and/or any errors it hits.
425+ *
426+ * @method _loadSubscribers
427+ */
428+BugSubscribersLoader.prototype._loadSubscribers = function() {
429+ var sl = this.subscribers_list;
430+ var loader = this;
431+
432+ // Fetch the person and add a subscription.
433+ function on_success(subscribers) {
434+ loader._loadSubscribersFromList(subscribers);
435+ loader.subscribers_list.stopActivity();
436+ }
437+
438+ var config = { on: {
439+ success: on_success,
440+ failure: this.error_handler.getFailureHandler()
441+ } };
442+
443+ sl.startActivity("Loading subscribers...");
444+ this.lp_client.get(this.subscribers_portlet_uri, config);
445+};
446+
447+/**
448+ * Return a function object that accepts SubscribersList and subscriber
449+ * objects as parameters.
450+ *
451+ * @method _getUnsubscribeCallback
452+ */
453+BugSubscribersLoader.prototype._getUnsubscribeCallback = function() {
454+ return function(subscribers_list, subscriber) {
455+ subscribers_list.indicateSubscriberActivity(subscriber);
456+ // Simulated unsubscribing action for prototyping the UI.
457+ setTimeout(function() {
458+ subscribers_list.stopSubscriberActivity(
459+ subscriber, true, function() {
460+ subscribers_list.removeSubscriber(subscriber);
461+ });
462+ }, 2400);
463+ };
464+};
465+
466
467 /**
468 * Manages entire subscribers' list for a single bug.
469
470=== modified file 'lib/lp/bugs/javascript/tests/test_subscribers_list.js'
471--- lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-15 11:08:00 +0000
472+++ lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-15 11:08:14 +0000
473@@ -1585,6 +1585,358 @@
474 }));
475
476
477+/**
478+ * Set-up all the nodes required for BugSubscriberLoader.
479+ */
480+function setUpLoader(root_node, config, barebone) {
481+ // Set-up subscribers list node.
482+ var node = Y.Node.create('<div />')
483+ .set('id', 'other-subscribers-container');
484+ var container_config = {
485+ container_box: '#other-subscribers-container'
486+ };
487+ if (barebone !== true) {
488+ container_config.bug = { web_link: '/base' };
489+ container_config.subscribers_details_view = '/+details';
490+ }
491+ root_node.appendChild(node);
492+ if (Y.Lang.isValue(config)) {
493+ config = Y.mix(container_config, config);
494+ } else {
495+ config = container_config;
496+ }
497+ return new module.BugSubscribersLoader(config);
498+}
499+
500+/**
501+ * Test BugSubscribersLoader class construction.
502+ */
503+suite.add(new Y.Test.Case({
504+ name: 'BugSubscribersLoader() construction test',
505+
506+ _should: {
507+ error: {
508+ test_BugSubscribersLoader_container_error:
509+ new Error(
510+ 'Container node must be specified in config.container_box.'),
511+ test_BugSubscribersLoader_bug_error:
512+ new Error(
513+ "No bug specified in `config' or bug.web_link is invalid."),
514+ test_BugSubscribersLoader_bug_web_link_error:
515+ new Error(
516+ "No bug specified in `config' or bug.web_link is invalid."),
517+ test_BugSubscribersLoader_portlet_link_error:
518+ new Error(
519+ "No config.subscribers_details_view specified to load " +
520+ "other bug subscribers from.")
521+ }
522+ },
523+
524+ setUp: function() {
525+ this.root = Y.Node.create('<div />');
526+ Y.one('body').appendChild(this.root);
527+ },
528+
529+ tearDown: function() {
530+ this.root.remove();
531+ },
532+
533+ test_BugSubscribersLoader_container_error: function() {
534+ // If no container node to hold the subscribers list is specified,
535+ // it fails with an error.
536+ var loader =
537+ new module.BugSubscribersLoader({container_box: '#not-found'});
538+ },
539+
540+ test_BugSubscribersLoader_bug_error: function() {
541+ // Bug needs to be passed in as well.
542+ // setUpLoader constructs the container node for us.
543+ var config = {};
544+ setUpLoader(this.root, config, true);
545+ },
546+
547+ test_BugSubscribersLoader_bug_web_link_error: function() {
548+ // Fails if the passed in bug has no web_link attribute defined.
549+ var config = { bug: {} };
550+ setUpLoader(this.root, config, true);
551+ },
552+
553+ test_BugSubscribersLoader_portlet_link_error: function() {
554+ // Fails if the passed in config has no passed in
555+ // portlet URI for loading bug subscribers details.
556+ var config = { bug: { web_link: '' } };
557+ setUpLoader(this.root, config, true);
558+ },
559+
560+ test_BugSubscribersLoader: function() {
561+ // With all the parameters specified, it returns an instance
562+ // with subscribers_portlet_uri, subscribers_list, error_handler,
563+ // and calls the _loadSubscribers() method.
564+ var config = {
565+ bug: { web_link: '/base' },
566+ subscribers_details_view: '/+details'
567+ };
568+
569+ // Save original method for restoring later.
570+ var old_load = module.BugSubscribersLoader.prototype._loadSubscribers;
571+
572+ var loading_started = false;
573+ module.BugSubscribersLoader.prototype._loadSubscribers = function() {
574+ loading_started = true;
575+ };
576+ var loader = setUpLoader(this.root, config);
577+ Y.Assert.areEqual('/base/+details', loader.subscribers_portlet_uri);
578+ Y.Assert.isNotNull(loader.subscribers_list);
579+ Y.Assert.isTrue(
580+ loader.subscribers_list instanceof module.SubscribersList);
581+ Y.Assert.isNotNull(loader.error_handler);
582+ Y.Assert.isTrue(loading_started);
583+
584+ // Restore original method.
585+ module.BugSubscribersLoader.prototype._loadSubscribers = old_load;
586+ }
587+}));
588+
589+
590+/**
591+ * Test BugSubscribersLoader subscribers loading and helper methods.
592+ */
593+suite.add(new Y.Test.Case({
594+ name: 'BugSubscribersLoader() subscribers loading test',
595+
596+ _should: {
597+ error: {
598+ test_loadSubscribersFromList_not_list_error:
599+ new Error('Got non-array "Not-a-list" in ' +
600+ '_loadSubscribersFromList().'),
601+ test_loadSubscribersFromList_no_objects_error:
602+ new Error('Subscriber details at index 0 (Subscriber)' +
603+ ' are not an object.')
604+ }
605+ },
606+
607+ setUp: function() {
608+ this.root = Y.Node.create('<div />');
609+ Y.one('body').appendChild(this.root);
610+ },
611+
612+ tearDown: function() {
613+ this.root.remove();
614+ },
615+
616+ test_addSubscriber_wraps_list_addSubscriber: function() {
617+ // addSubscriber wraps the SubscribersList.addSubscriber.
618+ // When no can_edit is set on the subscriber, no unsubscribe
619+ // callback is added.
620+ var subscriber = { name: "user" };
621+ var level = "Discussion";
622+ var call_passed_through = false;
623+ // Save the old method for restoring later.
624+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
625+ module.SubscribersList.prototype.addSubscriber = function(
626+ passed_subscriber, passed_level) {
627+ call_passed_through = true;
628+ Y.Assert.areSame(subscriber, passed_subscriber);
629+ Y.Assert.areSame(level, passed_level);
630+ };
631+ var loader = setUpLoader(this.root);
632+ loader._addSubscriber(subscriber, level);
633+ Y.Assert.isTrue(call_passed_through);
634+
635+ // Restore the real method.
636+ module.SubscribersList.prototype.addSubscriber = old_addSub;
637+ },
638+
639+ test_addSubscriber_normalizes_level: function() {
640+ // addSubscriber normalizes the subscription level to 'Maybe'
641+ // when it's otherwise unknown subscription level.
642+ var subscriber = { name: "user" };
643+ var level = "Not a level";
644+
645+ // Save the old method for restoring later.
646+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
647+ module.SubscribersList.prototype.addSubscriber = function(
648+ passed_subscriber, passed_level, passed_config) {
649+ Y.Assert.areSame('Maybe', passed_level);
650+ Y.Assert.isUndefined(passed_config);
651+ };
652+ var loader = setUpLoader(this.root);
653+ loader._addSubscriber(subscriber, level);
654+
655+ // Restore the real method.
656+ module.SubscribersList.prototype.addSubscriber = old_addSub;
657+ },
658+
659+ test_addSubscriber_unsubscribe_callback: function() {
660+ // addSubscriber sets the unsubscribe callback to function
661+ // returned by BugSubscribersLoader._getUnsubscribeCallback()
662+ // if subscriber.can_edit === true.
663+ var subscriber = { name: "user", can_edit: true };
664+ var unsubscribe_callback = function() {};
665+
666+ // Save old methods for restoring later.
667+ var old_getUnsub = module.BugSubscribersLoader.prototype
668+ ._getUnsubscribeCallback;
669+ var old_addSub = module.SubscribersList.prototype.addSubscriber;
670+
671+ // Make _getUnsubscribeCallback return the new callback.
672+ module.BugSubscribersLoader.prototype._getUnsubscribeCallback =
673+ function() {
674+ return unsubscribe_callback;
675+ };
676+
677+ // Assert in addSubscriber that it's being passed the new
678+ // callback in the config parameter.
679+ module.SubscribersList.prototype.addSubscriber = function(
680+ passed_subscriber, passed_level, passed_config) {
681+ Y.Assert.areSame(unsubscribe_callback,
682+ passed_config.unsubscribe_callback);
683+ };
684+
685+ var loader = setUpLoader(this.root);
686+ loader._addSubscriber(subscriber);
687+
688+ // Restore original methods.
689+ module.BugSubscribersLoader.prototype._getUnsubscribeCallback =
690+ old_getUnsub;
691+ module.SubscribersList.prototype.addSubscriber = old_addSub;
692+ },
693+
694+
695+ test_loadSubscribersFromList: function() {
696+ // Accepts a list of dicts with 'subscriber' and 'subscription_level'
697+ // fields, passing them directly to _addSubscriber() method.
698+ var data = [{ subscriber: { name: "Subscriber 1" },
699+ subscription_level: "Lifecycle" },
700+ { subscriber: { name: "Subscriber 2" },
701+ subscription_level: "Unknown" }];
702+
703+ // Save the original method for restoring later.
704+ var old_addSub = module.BugSubscribersLoader.prototype._addSubscriber;
705+
706+ var call_count = 0;
707+ module.BugSubscribersLoader.prototype._addSubscriber =
708+ function(subscriber, level) {
709+ call_count++;
710+ if (call_count === 1) {
711+ Y.Assert.areEqual("Subscriber 1", subscriber.name);
712+ Y.Assert.areEqual("Lifecycle", level);
713+ } else if (call_count === 2) {
714+ Y.Assert.areEqual("Subscriber 2", subscriber.name);
715+ Y.Assert.areEqual("Unknown", level);
716+ }
717+ };
718+
719+ var loader = setUpLoader(this.root);
720+ loader._loadSubscribersFromList(data);
721+
722+ // Two subscribers have been processed total.
723+ Y.Assert.areEqual(2, call_count);
724+
725+ // Restore the original method.
726+ module.BugSubscribersLoader.prototype._addSubscriber = old_addSub;
727+ },
728+
729+ test_loadSubscribersFromList_not_list_error: function() {
730+ // When the data is not a list, it throws an error.
731+ var data = "Not-a-list";
732+
733+ var loader = setUpLoader(this.root);
734+ loader._loadSubscribersFromList(data);
735+ },
736+
737+ test_loadSubscribersFromList_no_objects_error: function() {
738+ // When the data is not a list of objects, it throws an error.
739+ var data = ["Subscriber"];
740+
741+ var loader = setUpLoader(this.root);
742+ loader._loadSubscribersFromList(data);
743+ },
744+
745+ test_loadSubscribers_success: function() {
746+ // Testing successful operation of _loadSubscribers.
747+ var details = [
748+ { subscriber: { name: "subscriber" },
749+ subscription_level: "Details" }
750+ ];
751+
752+ // Override loadSubscribersList to ensure it gets called with
753+ // the right parameters.
754+ var old_loadSubsList =
755+ module.BugSubscribersLoader.prototype._loadSubscribersFromList;
756+ var loading_done = false;
757+ module.BugSubscribersLoader.prototype._loadSubscribersFromList =
758+ function(my_details) {
759+ Y.Assert.areSame(details, my_details);
760+ loading_done = true;
761+ };
762+
763+ var loader = setUpLoader(this.root);
764+
765+ // Mock lp_client for testing.
766+ loader.lp_client = {
767+ get: function(uri, get_config) {
768+ // Assert that there is activity in progress.
769+ var node = loader.subscribers_list.container_node
770+ .one('.global-activity-indicator');
771+ Y.Assert.isNotNull(node);
772+ // Call the success handler.
773+ get_config.on.success(details);
774+ }
775+ };
776+ // Re-run _loadSubscribers with our mock methods in place.
777+ loader._loadSubscribers();
778+
779+ // Assert that _loadSubscribersList was run in the process.
780+ Y.Assert.isTrue(loading_done);
781+
782+ // And activity node was removed when everything was done.
783+ var node = loader.subscribers_list.container_node
784+ .one('.global-activity-indicator');
785+ Y.Assert.isNull(node);
786+
787+ // Restore original method.
788+ module.BugSubscribersLoader.prototype._loadSubscribersFromList =
789+ old_loadSubsList;
790+ },
791+
792+ test_loadSubscribers_failure: function() {
793+ // On failure to load, activity indication is set to an error
794+ // message received from the server.
795+ var details = [
796+ { subscriber: { name: "subscriber" },
797+ subscription_level: "Details" }
798+ ];
799+
800+ var loader = setUpLoader(this.root);
801+
802+ // Mock lp_client for testing erroring out with 'BOOM'.
803+ loader.lp_client = {
804+ get: function(uri, get_config) {
805+ // Assert that there is activity in progress.
806+ var node = loader.subscribers_list.container_node
807+ .one('.global-activity-indicator');
808+ Y.Assert.isNotNull(node);
809+ // Call the success handler.
810+ get_config.on.failure(1,{ status: 403,
811+ statusText: 'BOOM',
812+ responseText: '' });
813+ }
814+ };
815+ // Re-run _loadSubscribers with our mock methods in place.
816+ loader._loadSubscribers();
817+
818+ // And activity node is there with an error message.
819+ var node = loader.subscribers_list.container_node
820+ .one('.global-activity-indicator');
821+ Y.Assert.isNotNull(node);
822+ Y.Assert.areEqual('file:///@@/error', node.one('img').get('src'));
823+ Y.Assert.areEqual(' Problem loading subscribers. 403 BOOM',
824+ node.one('span').get('text'));
825+ }
826+}));
827+
828+
829 var handle_complete = function(data) {
830 window.status = '::::' + JSON.stringify(data);
831 };
832
833=== modified file 'lib/lp/bugs/model/bug.py'
834--- lib/lp/bugs/model/bug.py 2011-06-15 11:08:00 +0000
835+++ lib/lp/bugs/model/bug.py 2011-06-15 11:08:14 +0000
836@@ -940,6 +940,17 @@
837 recipients.addDirectSubscriber(subscriber)
838 return subscriptions.subscribers.sorted
839
840+ def getDirectSubscribersWithDetails(self):
841+ """See `IBug`."""
842+ results = Store.of(self).find(
843+ (Person, BugSubscription),
844+ BugSubscription.person_id == Person.id,
845+ BugSubscription.bug_id == self.id,
846+ Not(In(BugSubscription.person_id,
847+ Select(BugMute.person_id, BugMute.bug_id == self.id)))
848+ ).order_by(Person.displayname)
849+ return results
850+
851 def getIndirectSubscribers(self, recipients=None, level=None):
852 """See `IBug`.
853
854
855=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
856--- lib/lp/bugs/model/tests/test_bug.py 2011-06-07 06:24:04 +0000
857+++ lib/lp/bugs/model/tests/test_bug.py 2011-06-15 11:08:14 +0000
858@@ -202,6 +202,37 @@
859 set(subscribers), set(direct_subscribers),
860 "Subscribers did not match expected value.")
861
862+ def test_get_direct_subscribers_with_details(self):
863+ # getDirectSubscribersWithDetails() returns both
864+ # Person and BugSubscription records in one go.
865+ bug = self.factory.makeBug()
866+ with person_logged_in(bug.owner):
867+ # Unsubscribe bug owner so it doesn't taint the result.
868+ bug.unsubscribe(bug.owner, bug.owner)
869+ subscriber = self.factory.makePerson()
870+ with person_logged_in(subscriber):
871+ subscription = bug.subscribe(
872+ subscriber, subscriber, level=BugNotificationLevel.LIFECYCLE)
873+
874+ self.assertContentEqual(
875+ [(subscriber, subscription)],
876+ bug.getDirectSubscribersWithDetails())
877+
878+ def test_get_direct_subscribers_with_details_mute_excludes(self):
879+ # getDirectSubscribersWithDetails excludes muted subscriptions.
880+ bug = self.factory.makeBug()
881+ with person_logged_in(bug.owner):
882+ # Unsubscribe bug owner so it doesn't taint the result.
883+ bug.unsubscribe(bug.owner, bug.owner)
884+ subscriber = self.factory.makePerson()
885+ with person_logged_in(subscriber):
886+ bug.subscribe(
887+ subscriber, subscriber, level=BugNotificationLevel.LIFECYCLE)
888+ bug.mute(subscriber, subscriber)
889+
890+ self.assertContentEqual(
891+ [], bug.getDirectSubscribersWithDetails())
892+
893 def test_subscribers_from_dupes_uses_level(self):
894 # When getSubscribersFromDuplicates() is passed a `level`
895 # parameter it will include only subscribers subscribed to
896
897=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
898--- lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-05-20 21:09:40 +0000
899+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-06-15 11:08:14 +0000
900@@ -10,6 +10,7 @@
901 <div tal:define="context_menu context/menu:context">
902 <div tal:content="structure context_menu/addsubscriber/render" />
903 </div>
904+ <div id="other-bug-subscribers"></div>
905 <a id="subscribers-ids-link"
906 tal:define="bug context/bug|context"
907 tal:attributes="href bug/fmt:url/+bug-portlet-subscribers-ids"></a>
908
909=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
910--- lib/lp/bugs/templates/bugtask-index.pt 2011-05-20 21:09:40 +0000
911+++ lib/lp/bugs/templates/bugtask-index.pt 2011-06-15 11:08:14 +0000
912@@ -43,7 +43,7 @@
913 </tal:if>
914 <script type="text/javascript">
915 LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
916- 'lp.bugs.bugtask_index.portlets',
917+ 'lp.bugs.bugtask_index.portlets', 'lp.bugs.subscribers_list',
918 'lp.code.branchmergeproposal.diff', 'lp.comments.hide',
919 function(Y) {
920 Y.lp.bugs.bugtask_index.portlets.use_advanced_subscriptions =
921@@ -55,6 +55,12 @@
922 Y.on('domready', function() {
923 LP.cache.comment_context = LP.cache.bug;
924 Y.lp.comments.hide.setup_hide_controls();
925+ var sl = new Y.lp.bugs.subscribers_list.BugSubscribersLoader({
926+ container_box: '#other-bug-subscribers',
927+ bug: LP.cache.bug,
928+ subscribers_details_view:
929+ '/+bug-portlet-subscribers-details'
930+ });
931 });
932 });
933 </script>