Launchpad itself

lib/lp/bugs/javascript/subscribers_list.js (+258/-2)
lib/lp/bugs/javascript/tests/test_subscribers_list.html (+2/-0)
lib/lp/bugs/javascript/tests/test_subscribers_list.js (+437/-17)

To merge this branch:

bzr merge lp://staging/~danilo/launchpad/bug-772754-other-subscribers-subscribers

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Benji York (community)	code	2011-06-10	Approve on 2011-06-13
Review via email: mp+64179@code.staging.launchpad.net

Description of the change

= Bug 772754: Other subscribers list, part 3 =

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 continues on the previous branches to provide methods to add/remove subscribers and add an unsubscribe action for a subscriber when needed.

It is comprehensively tested.

== 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/javascript/subscribers_list.js
  lib/lp/bugs/javascript/tests/test_subscribers_list.html
  lib/lp/bugs/javascript/tests/test_subscribers_list.js

Revision history for this message

Benji York (benji) wrote on 2011-06-13:

This branch looks good. Here are a couple of small thoughts I had while
looking over the branch:

checkSubscriptionLevel doesn't really need to return anything (and the
return value isn't used.

Unlike "nomal" HTML, YUI can understand self-closing divs, so on line
531 of the diff (in test_subscribers_list.js) so "<div></div>" can be
replaced with "<div/>" on on lines 312, 464, 531, and 715 of the diff
(in test_subscribers_list.js).

review: Approve (code)

Revision history for this message

Данило Шеган (danilo) wrote on 2011-06-14:

Thanks for the review.

У пон, 13. 06 2011. у 14:38 +0000, Benji York пише:
> checkSubscriptionLevel doesn't really need to return anything (and the
> return value isn't used.

Good point, fixed.

> Unlike "nomal" HTML, YUI can understand self-closing divs, so on line
> 531 of the diff (in test_subscribers_list.js) so "<div></div>" can be
> replaced with "<div/>" on on lines 312, 464, 531, and 715 of the diff
> (in test_subscribers_list.js).

Yeah, I didn't know that about YUI. Brad just pointed that out in a
previous review so I fixed it in the code. Forgot about the tests
though, so now those are fixed as well :)