launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

The proposal to merge lp:~adeuring/launchpad/bug-834303 into lp:launchpad has been updated.

Description changed to:

This branch fixes bug 834303: Archive:+subscriptions times out with
many subscribers.

The cause of the timeout is Storm needing really much time to load
15000 or 20000 ArchiveSubscriber records.

As lifeless noted in a bug comment, the fix is obvious: To show the
data in batches.

This leads to another problem, noted by Anthony Lenton in another comment
on the bug: The data was sorted by the creation time of a subscription,
while an important use case is to change the subscription status of
a subscriber. Looking them up by sifting through more than 100 pages
to look up a subscriber by name would be really painful.

The fix is obvious: Sort by name. Anthony suggested to use the display
name; after an IRC chat with him I switched instead to sorting by LP
user name. This allows us to use StormRangeFactory, which uses the
sort value of the first/last displayed result row as the "batch value"
in URLs. This allows to manually edit a URL to quickly look up the
subscription status of a given user. Not as nice as a decent search
form, but at least a convenient work-around, as long as we don't have
a search form and the related result page...

In theory, we could use Person.displayname for batching with
StormRangeFactory too, but the related URLs would look even more
insane than the URLs for sorting by LP user name: We would need
Person.id as an additional sort parameter, and the sort value
-- which appears in the URL -- may contain spaces, and when you
change a URL manually, you should be aware of the meaning of the
second sort parameter, Person.id. To compare the URLs:

sort by Person.name:

https://launchpad.dev/~cprov/+archive/p3a/+subscriptions?batch=5&memo=[%22stevea%22]&start=5

sort by Person.displayname, Person.id

https://launchpad.dev/~cprov/+archive/p3a/+subscriptions?batch=5&memo=[%22Steve+Alexander%22,1234]&start=5

tests:

./bin/test soyuz -vvt test_archivesubscriptionview
./bin/test soyuz -vvt archivesubscriber.txt

no lint

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-834303/+merge/90304
-- 
https://code.launchpad.net/~adeuring/launchpad/bug-834303/+merge/90304
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-834303 into lp:launchpad.