Code review comment for lp:~brian-murray/launchpad/display-dupe-in-portlet-dupe-subscribers

Brian,

this is a _very_ useful improvement, I think!

> When a bug has a lot of duplicates and subscribers from duplicates
> it would be useful to know which duplicate you were subscribed to
> without checking every single duplicate bug to find out.
>
> Instead you can mouse over your name in the subscription and find
> out to which one you are subscribed.
>
> Test modified:
>
> bin/test -cvvt bugsubscription.txt xx-bug-personal-subscriptions.txt
>
> I'd be happy to continue using the macro
> "bug/@@+bug-portlet-subscribers-content/subscriber-row" if you've any
> ideas on how to replace the title in it. I couldn't figure out a way
> to do that.

I think you replace the model class properties display_subscribed_by and
display_duplicate_subscribed_by by methods displayDuplicateSubscribedBy()
of the browser classes
lp.bugs.browser.bugsubscription.BugPortletDuplicateSubcribersContents
and lp.bugs.browser.bugsubscription.BugPortletSubcribersContents

But the current variant is fine for me too.

> [Robert:]
>
> The table formatted awkwardly.
> Can you confirm:
> (dup subs, devel, old proposal, current proposal)
> (0, 24, 24, 25)
> (1, 26, 27, 26)
> (2, 28, 29, 28)
> (3, 29, 32, 29)
>
> This is certainly better. It appears to still scale with duplication
> subscriptions which is a problem. (And the query count is still quite
> large: for a portlet like this, 10-15 queries is what I'd expect).
>
> I haven't reviewed the code yet, but on the sheer performance size,
> these query counts (which are only a surrogate) are at least no-worse.
>
> If you want to grab an OCR to review this your friday, please do :
> don't block on me looking at the actual change - whomever reviews it
> can advise on the tastefulness of your approach to the scaling issue.

So we have one more query for zero duplicate subscribers, and the same
number of queries for one or more subscribers. I think this is fine.

Reducing the number of queries is something for another branch.
(pre-loading the subscriber and subscribed_by objects in the query
that retrieves the subscriptions comes to mind)