On Tue, Nov 17, 2009 at 08:08:18PM -0000, Brad Crittenden wrote:
> Review: Needs Information code
> Hi Graham,

Hi Brad, thanks for the review.

> Thanks for this fix.  I note there are two issues.  One, the icon and
> subscriber name now appear on different lines and for the life of me I
> cannot figure out why.

That's not a result of this branch; you can see the same behaviour on
edge at the moment. I'll file a bug about it.

> 
> Also I'm bothered that a change of this size has no tests.  You could
> do a view test to check the results of your new view.  What about
> Windmill tests?
> 

You're quite right. I've added a view test for the JSON dump (See below
for diff). The changes should still pass the current windmill test
suite, though IIRC the inline subscribers part of the tests is currently
failing anyway, so it's difficult to tell. I'll run it through windmill
before landing.

> Perhaps it is just the current state of affairs but the fragility of
> this change bothers me.
> 

I don't think the change is particularly fragile. I designed it
specifically so that if fetching the IDs fails we'll just fall back to
loading the portlet anyway. The only time someone will have a noticeable
problem in that case is if they try to subscribe and unsubscribe quickly
in succession (in which case the subscribe / unsubscribe link may not
update properly) or if they try to unsubscribe from a dupe, in which
case they'll have to do it via the old +subscribe form rather than via
AJAX.

Can you be more specific about the fragility that concerns you?

> I've removed the 'js' portion from this review as you may want to get
> an expert to do that portion.

I'll ask Gavin to take a look since he's OCR today.


Incremental diff of changes
---------------------------

=== added file 'lib/lp/bugs/browser/tests/bug-subscription-views.txt'
--- lib/lp/bugs/browser/tests/bug-subscription-views.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/bug-subscription-views.txt	2009-11-18 11:50:28 +0000
@@ -0,0 +1,40 @@
+Bug subscription views
+======================
+
+Getting subscriber CSS IDs
+--------------------------
+
+It's possible to get a mapping of bug subscriber names to CSS IDs using
+the +bug-portlet-subscribers-ids view.
+
+    >>> from lp.bugs.interfaces.bug import IBugSet
+    >>> bug_15 = getUtility(IBugSet).get(15)
+
+    >>> subscriber_ids_view = create_initialized_view(
+    ...     bug_15, '+bug-portlet-subscribers-ids')
+
+The view's subscriber_ids_js property returns a JSON struct of the
+person name to CSS ID mappings for the bug's subscribers.
+
+    >>> print subscriber_ids_view.subscriber_ids_js
+    {"name16": "subscriber-16"}
+
+If bug 15 is marked as a duplicate of another bug, its subscribers will
+be included in that bugs subscriber_ids_js mapping.
+
+    >>> bug_13 = getUtility(IBugSet).get(13)
+    >>> subscriber_ids_view = create_initialized_view(
+    ...     bug_13, '+bug-portlet-subscribers-ids')
+
+    >>> print subscriber_ids_view.subscriber_ids_js
+    {"name12": "subscriber-12"}
+
+    >>> login('<email address hidden>')
+    >>> bug_15.duplicateof = bug_13
+    >>> bug_13 = getUtility(IBugSet).get(13)
+
+    >>> subscriber_ids_view = create_initialized_view(
+    ...     bug_13, '+bug-portlet-subscribers-ids')
+
+    >>> print subscriber_ids_view.subscriber_ids_js
+    {"name12": "subscriber-12", "name16": "subscriber-16"}