658163 - Inconsistent featured version limiting

Status: VERIFIED FIXED

(Socorro :: General, task)

Description

[Lonnen :lonnen]

Socorro limits the number of versions that can be featured, but in the admin panel the products have inconsistent behavior when adding new featured versions. With a limit of 4 I get the following behavior:

Camino -- warns that there are already 4 but sets a check indicating a 5th despite the warning. After the 5th it just gives the warning but no additional versions are set as featured

Fennec -- no warnings at all and I'm at 8 checked so far

Firefox -- same as Camino

SeaMonkey -- warns at 4, does not check a 5th like Camino.

Thunderbird -- same as Fennec, 8 so far and no warning

Comment 1

[Lonnen :lonnen]

Spent some time digging and found that the branch_model getFeaturedVersions() and getFeaturedVersionsExcludingVersionCount() calls are in agreement with each other but different than the versions marked as featured in the returned info from getBranchData().

Comment 2

[Lonnen :lonnen]

The branch_model inconsistently requires `pv.start_date <= '(today)' AND pv.end_date >= '(today)'` when it calls the database, which can result in inaccurate counts of featured versions.

Specifically, when the table is populated initially there is no date filtering, but when a user sets a new featured version it checks against the subset in that date range. In the case of Thunderbird, this lets me mark nearly 35 consecutive versions as 'featured' before choosing 3 that fall in that date range.

When we populate the menu items we use that date range, so having the option to mark another version as featured seems unexpected when we don't provide a convenient way to get to them through the UI. We could add the date range restriction to what populates the admin panel, or we could remove the date range restriction from the other commands. Not sure how to proceed, so I've cc'd others for advice.

Comment 3

[Lonnen :lonnen]

Attachment: Limits branches in the admin panel to only current branches

Since the available versions are effectively limited to unexpired versions elsewhere in the web app, this patch restricts the tables in the admin panel to display unexpired versions only.

I know we are about to discuss some larger changes to the concept of featured versions, so this may not be needed, though it gives us an option in case we want a temporary band aid while we develop the better solution.

Comment 4

[Robert Kaiser]

Comment on attachment 545280 [details] [diff] [review]
Limits branches in the admin panel to only current branches

With this, is there still a way to get to the "non-current" versions to make them current again in case of an error in settings?

Comment 5

[Lonnen :lonnen]

Attachment: Removes the the ability to mark versions as featured if they are out of date.

Revised patch. Presents out of date versions in their own section of the admin panel, and prevents users from marking a version as featured if it is out of date.

Comment 6

[Lonnen :lonnen]

Checked in as r3278 (http://code.google.com/p/socorro/source/detail?r=3278).

Comment 7

[Lonnen :lonnen]

The previous patch was comparing a date as a string with a date as a number, which coincidentally worked sometimes but was usually wrong.

Checked in as r3279 (http://code.google.com/p/socorro/source/detail?r=3278).

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

While testing this, I spun off bug 673183.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://crash-stats-dev.allizom.org/admin; felt like I've tested this pretty well (adding/deleting/updating/featuring/un-featuring).