1288530 - Add a "classifier" field to PerformanceAlert, referencing the user model

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect)

Description

[William Lachance (:wlach)]

It would be nice if we could know who was classifying performance alerts, especially if we're going to implement some kind of "autoclassification" for them (see bug 1285322). I think we can do this by adding a "classifier" field to the PerformanceAlert model, referencing the user's table (see the jobnote model for an example of this: https://github.com/mozilla/treeherder/blob/bfc5076/treeherder/model/models.py#L729). Then when the user initiates a modification request on the performance alert, we can update this field (you will need to add some functionality to the PerformancAlert viewset for this: https://github.com/mozilla/treeherder/blob/bfc5076/treeherder/webapp/api/performance_data.py#L241).

On a UI level, perhaps we could show the classifier when the user hovers over the alert text using a tooltip.

Comment 1

[GitHub Autolander Bot]

Attachment: [treeherder] crosscent:Bug1288530 > mozilla:master

Comment 2

[crosscent]

:camd told me that there could be a mailing list I can mail so that people know a new field is being added and the migration can go smoother, but we aren't sure which one to mail to.

Anyway, here are the steps I took, and please let me know if I'm missing something!

* Create a new field ``classifier``
* Create a new method ``update`` for PerformanceAlertViewSet, so that if ``related_summary_id`` is included in POST data, classifier can automatically be identified
* Create a new field ``ClassifierField``, since we don't have an endpoint for retrieving user data. I thought this would be the best way to to take a values that have different input/output format
* Add uib-tooltip to display classifier e-mail.

Comment 3

[crosscent]

Comment on attachment 8773852 [details] [review]
[treeherder] crosscent:Bug1288530 > mozilla:master

I've changed the custom field to SlugRelatedField.

Comment 4

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/5da09daa72121d031e51ddfa4adca99d838eb4ac
Bug 1288530 - Add a "classifier" field to PerformanceAlert, referencing the user model (#1718)

Comment 5

[William Lachance (:wlach)]

Hey, so I merged this (see above) and it seems to work well on stage. However, after playing with it a bit I think it would be better if we actually set the classification on *any change*, not just those based on reassigning/downstreaming the alert. Correspondingly, we would show the tooltip on the text of the alert itself (not the reassigned/downstream status span). Note that in the case that the alert state is "untriaged", we should not claim it is autoclassified (though we should show the user information if there is any). Roy, would you be interested in doing an updated PR for this? Sorry for not catching this earlier.

Comment 6

[crosscent]

I can definitely provide an updated PR for this. I'm not very familiar with modifying Alerts, so could you elaborate on what other changes are?

Comment 7

[William Lachance (:wlach)]

I'll comment on the old PR, the changes should be quite straightforward. :)

Comment 8

[GitHub Autolander Bot]

Attachment: [treeherder] crosscent:Bug1288530 > mozilla:master

Comment 9

[crosscent]

Hey Will, thanks for pointing out all the changes to make the new PR. What I meant was what other changes were possible when modifying an Alert, but you've already answered me on IRC! I forgot about "acknowledged."

Anyway, I have attached a new PR, and this is my first time creating a new PR on top of a previous commit, so please let me know if the commit isn't correct. I am also wondering if the nested ternary condition should be refactored out to a separate function. Thanks for all your help!

Comment 10

[William Lachance (:wlach)]

Comment on attachment 8773852 [details] [review]
[treeherder] crosscent:Bug1288530 > mozilla:master

You requested the review on the wrong PR. :) Also, please rebase the new one against master.

Comment 11

[crosscent]

Comment on attachment 8775347 [details] [review]
[treeherder] crosscent:Bug1288530 > mozilla:master

Thanks for the detailed commands! I think I'm getting a little better with Git everyday now. I hope I got the correct PR for review this time.

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/ef46fa00eb0919e92f11ffdfe6a6926541ab409c
Bug 1288530 - Update classifier unconditionally, and move classifier tooltip in AlertView (#1742)

Comment 13

[William Lachance (:wlach)]

Comment on attachment 8775347 [details] [review]
[treeherder] crosscent:Bug1288530 > mozilla:master

Looks great, merged.