[Findit] Add fracas analysis result feedback page for manual triage.

[Findit] Add fracas analysis result feedback page for manual triage.

Open the dashboard: https://findit-for-me.googleplex.com/crash/fracas-dashboard

Click the signature of a crash, the page will be redirected to the feedback page of this crash.

Sheriff can triage 4 results findit provided:
regression_range,
suspected_cls,
suspected_project,
suspected_components

BUG=615267

[Sharu Jiang, 2016-06-15 23:13:16]

Patchset #1 (id:1) has been deleted

[Sharu Jiang, 2016-06-15 23:13:26]

Patchset #1 (id:20001) has been deleted

[Sharu Jiang, 2016-06-15 23:15:00]

Patchset #1 (id:40001) has been deleted

[Sharu Jiang, 2016-06-15 23:42:02]

Description was changed from

==========
Fix tests.


Add tests.


Add tests.


Add features.


Merge branch 'only-dashboard' into dashboard


FREEZE.unindexed


Add time_util


Add dashboard test.


Fix nits.


update dashboard.


Add real culprit input.


Fix nits.


Merge branch 'master' of https://chromium.googlesource.com/infra/infra into
dashboard


experiments.


Modify handlers.


add one more.


intermediate.


intermediate.

BUG=
==========

to

==========
[Findit] Add fracas analysis result feedback page for manual triage.

Open the dashboard: https://findit-for-me.googleplex.com/crash/fracas-dashboard

Click the signature of a crash, the page will be redirected to the feedback page
of this crash.

Sheriff can triage 4 results findit provided:
regression_range,
suspected_cls,
suspected_project,
suspected_components

BUG=615267
==========

[Sharu Jiang, 2016-06-15 23:42:02]

katesonia@chromium.org changed reviewers:
+ stgao@chromium.org

[Sharu Jiang, 2016-06-16 00:55:21]

Patchset #1 (id:60001) has been deleted

[Sharu Jiang, 2016-06-16 00:59:31]

PTAL :)

[stgao, 2016-06-16 17:32:05]

This CL is too big for review as it is, because it includes quite a few
features. Please break it down into a couple of CLs by isolated features.

As we discussed before, a CL should be as small as possible, and it implements a
feature at a time.
If you have difficulty in isolating features, please let me know and we could
discuss.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/results.py:40: line_parts.append('frame #%d' %
frame.index)
Could we avoid including the change from the other CL?

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
File appengine/findit/handlers/crash/fracas_result_feedback.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:21: from
handlers.result_status import NO_TRY_JOB_REASON_MAP
Can you please clean up the code when coping from the other handler for try-job
or failures on waterfall?

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:46: PERMISSION_LEVEL =
Permission.ANYONE
Why it is anyone? Shouldn't it be just corp user?

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:56:
analysis.result['regression_range'] if 'regression_range' in
This could be simplified by dict.get('key', 'default_value').

Same for all below.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:58:
'culprit_regression_range': analysis.culprit_regression_range,
What's the difference between 'regression_range' and 'culprit_regression_range'?

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:88: """Triggers
analysis of a build failure on demand and return current result.
Not clean up again for the copyover from another handler.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/model/...
File appengine/findit/model/crash/crash_analysis.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/model/...
appengine/findit/model/crash/crash_analysis.py:102: setattr(self, key, value)
What if the key is not an attribute of the entity?

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
File appengine/findit/templates/crash/fracas_result_feedback.html (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:124: alert('Please
fill the culprit if you found it.')
No alert. Too many alerts are not user-friendly.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:124: alert('Please
fill the culprit if you found it.')
And those input fields are not saved anywhere.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:223: {% for
metadata in historical_metadata %}
This seems not the right usage of a <thead> in a <table>.

[Sharu Jiang, 2016-06-21 20:28:50]

The comments were addressed in https://codereview.chromium.org/2075153003/ 

:)

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/crash/...
File appengine/findit/crash/results.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/crash/...
appengine/findit/crash/results.py:40: line_parts.append('frame #%d' %
frame.index)
On 2016/06/16 17:32:04, stgao wrote:
> Could we avoid including the change from the other CL?

Yes, I uploaded separate cl for this, after rebase, this will be gone.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
File appengine/findit/handlers/crash/fracas_result_feedback.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:21: from
handlers.result_status import NO_TRY_JOB_REASON_MAP
On 2016/06/16 17:32:05, stgao wrote:
> Can you please clean up the code when coping from the other handler for
try-job
> or failures on waterfall?

Done.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:46: PERMISSION_LEVEL =
Permission.ANYONE
On 2016/06/16 17:32:05, stgao wrote:
> Why it is anyone? Shouldn't it be just corp user?

Done.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:56:
analysis.result['regression_range'] if 'regression_range' in
On 2016/06/16 17:32:05, stgao wrote:
> This could be simplified by dict.get('key', 'default_value').
> 
> Same for all below.

Done.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:58:
'culprit_regression_range': analysis.culprit_regression_range,
On 2016/06/16 17:32:05, stgao wrote:
> What's the difference between 'regression_range' and
'culprit_regression_range'?

regression_range is the result findit found.

culprit_regression_range is what sheriff manually found.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/handle...
appengine/findit/handlers/crash/fracas_result_feedback.py:88: """Triggers
analysis of a build failure on demand and return current result.
On 2016/06/16 17:32:05, stgao wrote:
> Not clean up again for the copyover from another handler.

Oops...sorry.
Done.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/model/...
File appengine/findit/model/crash/crash_analysis.py (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/model/...
appengine/findit/model/crash/crash_analysis.py:102: setattr(self, key, value)
On 2016/06/16 17:32:05, stgao wrote:
> What if the key is not an attribute of the entity?

This will be addressed in another cl which contains the triage feedback
functions.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
File appengine/findit/templates/crash/fracas_result_feedback.html (right):

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:124: alert('Please
fill the culprit if you found it.')
On 2016/06/16 17:32:05, stgao wrote:
> No alert. Too many alerts are not user-friendly.

Done.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:124: alert('Please
fill the culprit if you found it.')
On 2016/06/16 17:32:05, stgao wrote:
> And those input fields are not saved anywhere.

Actually it will be saved by pressing enter, but I will change it to a save
button.

https://codereview.chromium.org/2067373002/diff/80001/appengine/findit/templa...
appengine/findit/templates/crash/fracas_result_feedback.html:223: {% for
metadata in historical_metadata %}
On 2016/06/16 17:32:05, stgao wrote:
> This seems not the right usage of a <thead> in a <table>.

Because UMA right now only stores 20 days' data, so Fracas can only give us 20
versions data, so a row won't be too long.