543940 - [shipping] support many and none appversion in the query url

Status: RESOLVED FIXED

(Webtools Graveyard :: Elmo, defect, P3)

Description

[Staś Małolepszy :stas]

A spin-off of bug 543504

We should support:

?av=fx3.6&av=fx3.5&av=fennec1.0
?status=pending
?status=pending&av=fx3.6&av=fx3.5&av=fennec1.0

Comment 1

[Staś Małolepszy :stas]

Attachment: Patch A.1

I was about to attach a WIP patch, but I made a couple of edits and I think this works pretty OK.

This makes Shipping's dashboard support:

/shipping/dashboard
/shipping/dashboard?av=fx3.6
/shipping/dashboard?av=fx3.6&av=tb3.1

Comment 2

[Staś Małolepszy :stas]

Attachment: Patch A.2 A nicer-looking loop in shipping.views.dashboard

This loops over av's once, not three times.

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 426672 [details] [diff] [review]
Patch A.2 A nicer-looking loop in shipping.views.dashboard

r=me with nits.

I don't like getting the appvers or milestones still, I anticipate us changing that code once or twice again. Still, this should fix the problem for now.

My main culprit with my other approach is the subtitles, for which I need the str(appversion) and that needs more thought in my head.

Nits: A few lines should get a select_related.

for mstone in mstones.select_related("appver__tree"):

for appver in appvers.select_related("tree"):

av_or_m_list should also end up with appver__tree, which need a third layer if you're in ms.

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 426672 [details] [diff] [review]
Patch A.2 A nicer-looking loop in shipping.views.dashboard

Review of attachment 426672 [details] [diff] [review]:
-----------------------------------------------------------------

Did a triage pass through our bugs with patches that have an r+.

Moving this over to an r-, as this patch doesn't apply anymore. It's not that it couldn't be resurrected along the same lines though.

Do we still want this?

Comment 6

[Axel Hecht [:Pike]]

Yes, we still want this, but stas isn't going to work on this any time soon, unassigning.

Comment 7

[Staś Małolepszy :stas]

Attachment: 2013 patch

After 1192 days, here's a new patch :)

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 751625 [details] [diff] [review]
2013 patch

Review of attachment 751625 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits:

Please use a check-in comment that references the bug, and doesn't do [PATCH].

If you start the comment with "fixes bug XXXX - message", bots will close this bug on landing. Not that I like the message format.

Comment 9

[[github robot]]

Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/cd296aab9ee26c24317d678928debb191bbe860f
fixes bug 543940 - Allow more than one appversion in the dashboard's URL, r=Pike