Closed Bug 1131862 Opened 5 years ago Closed 5 years ago

Use the new bzapi target_milestone_detail to display only the active milestones

Categories

(Tree Management :: Bugherder, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(5 files)

In bug 1120782, it was requested that the list of active milestones be pruned for several products, so that the list shown in mcMerge was more manageable. 

However it was then discovered that bzapi returned _all_ milestones, even the inactive ones.

Bug 1124255 then added an additional property to https://bugzilla.mozilla.org/bzapi/configuration called target_milestone_detail - which gives not only the name found in target_milestone, but also:
* id
* is_active
* sortkey

We should switch to this, so we can filter on is_active == true, and only display the active milestones in the mcMerge UI.
https://bugzilla.mozilla.org/bzapi/configuration

     "target_milestone" : [
        "---",
        "2.2 S12 (15may)",
...
        "DeveloperPhone"
     ],
     "target_milestone_detail" : [
        {
           "id" : 2336,
           "is_active" : true,
           "name" : "---",
           "sortkey" : 0
        },
        {
           "id" : 6718,
           "is_active" : true,
           "name" : "2.2 S12 (15may)",
           "sortkey" : 945
        },
...
        {
           "id" : 2337,
           "is_active" : false,
           "name" : "DeveloperPhone",
           "sortkey" : 10310
        }
     ],

Handled here:
https://hg.mozilla.org/webtools/tbpl/file/ee042d68178e/mcmerge/js/ConfigurationData.js#l44
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Attachment #8562820 - Flags: review?(graeme.mccutcheon)
For consistency with ConfigurationData.milestones
Attachment #8562821 - Flags: review?(graeme.mccutcheon)
Doesn't make sense to iterate through products twice. Moves the testsuiteFlagID
calculation up front, then generates 'milestones' and 'hasTestsuiteFlag' as part
of the same loop.
Attachment #8562822 - Flags: review?(graeme.mccutcheon)
The bzapi config target_milestone property on each product returns all
milestones for that product, including the inactive ones. A new property has
been added to the API, of form:

     "target_milestone_detail" : [
        {
           "id" : 2336,
           "is_active" : true,
           "name" : "---",
           "sortkey" : 0
        },

We now use target_milestone_detail instead of target_milestone, and filter on
is_active to find only the active milestones. We use map() to flattern the list
of activate milestone names, so existing consumers do not need to be adjusted.
Attachment #8562823 - Flags: review?(graeme.mccutcheon)
Check for the 'product' property up-front, rather than having conditionals on it
throughout. Also create a variable for products, to clean up the references.
Attachment #8562825 - Flags: review?(graeme.mccutcheon)
Attachment #8562820 - Flags: review?(graeme.mccutcheon) → review+
Attachment #8562821 - Flags: review?(graeme.mccutcheon) → review+
Comment on attachment 8562825 [details] [diff] [review]
Part 3: Return early if 'products' missing from bzapi config

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

::: mcmerge/js/ConfigurationData.js
@@ +49,1 @@
>      // Parse Milestones 

Get this whitespace while you're here?
Attachment #8562825 - Flags: review?(graeme.mccutcheon) → review+
Attachment #8562822 - Flags: review?(graeme.mccutcheon) → review+
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #7)
> Get this whitespace while you're here?

Nevermind, I see you got it in the next patch.
Comment on attachment 8562823 [details] [diff] [review]
Part 5: Display only the active milestones

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

"Bug 1131862 - mcMerge: Display online the active milestones" - "only" ?
Attachment #8562823 - Flags: review?(graeme.mccutcheon) → review+
Depends on: 1136433
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.