Code review comment for lp:~brian-murray/launchpad/bug-912137

Revision history for this message
j.c.sackett (jcsackett) wrote :

Brian--

Thanks for this--it's good, it just needs one fix.

As we discussed on IRC, this has the side effect of new subscribers added via the UI being added to the bottom of the list. Because subscriber lists can quite long, new ones should be added to the top so the user can see that they were in fact added.

You could update addSubscriber to accept a parameter that indicates whether it's adding subscribers from a list of existing ones--which should be appended, or is adding a brand new subscriber which should be prepended. If you default this argument to indicate appending, then you should just need to update the call site that is bound to the `Subscribe someone else` button.

review: Needs Fixing

« Back to merge proposal