[Findit] UI change and triage change for cl level trige.

appengine/findit/handlers/build_failure.py

Index: appengine/findit/handlers/build_failure.py
diff --git a/appengine/findit/handlers/build_failure.py b/appengine/findit/handlers/build_failure.py
index 0903748af7a67a736523c22b3905f7553ed1b09f..b50133f7c5911736f7c24f4707cc9a85d468ed09 100644
--- a/appengine/findit/handlers/build_failure.py
+++ b/appengine/findit/handlers/build_failure.py
@@ -15,9 +15,11 @@ from handlers import handlers_util
 from handlers import result_status
 from handlers.result_status import NO_TRY_JOB_REASON_MAP
 from model import analysis_status
+from model.result_status import RESULT_STATUS_TO_DESCRIPTION
+from model.suspected_cl_status import CL_STATUS_TO_DESCRIPTION
 from model.wf_analysis import WfAnalysis
+from model.wf_suspected_cl import WfSuspectedCL
 from model.wf_try_job import WfTryJob
-from model.result_status import RESULT_STATUS_TO_DESCRIPTION
 from waterfall import build_failure_analysis_pipelines
 from waterfall import buildbot
 from waterfall import waterfall_config
@@ -33,6 +35,19 @@ def _FormatDatetime(dt):
     return dt.strftime('%Y-%m-%d %H:%M:%S UTC')
 
 
+def _GetCLDict(analysis, cl_info):
+  if not cl_info:
+    return None

[lijeffrey, 2016/09/23 05:09:09]

nit: how about instead of returning None, return {} since this function says it
returns a dict?

[chanli, 2016/09/24 01:09:40]

Done.

+
+  cl_keys = cl_info.split('/')
+  repo_name = cl_keys[0]
+  revision = cl_keys[1]
+  for cl in analysis.suspected_cls:
+    if cl['repo_name'] == repo_name and cl['revision'] == revision:
+      return cl
+  return None
+
+
 def _GetTriageHistory(analysis):
   if (not users.is_current_user_admin() or
       not analysis.completed or
@@ -41,12 +56,15 @@ def _GetTriageHistory(analysis):
 
   triage_history = []
   for triage_record in analysis.triage_history:
+
     triage_history.append({
         'triage_time': _FormatDatetime(
             datetime.utcfromtimestamp(triage_record['triage_timestamp'])),
         'user_name': triage_record['user_name'],
-        'result_status': RESULT_STATUS_TO_DESCRIPTION.get(
-            triage_record['result_status']),
+        'triaged_cl': _GetCLDict(analysis, triage_record.get('triaged_cl')),
+        'result_status': (
+            RESULT_STATUS_TO_DESCRIPTION.get(triage_record.get('result_status'))
+            or CL_STATUS_TO_DESCRIPTION.get(triage_record.get('cl_status'))),
         'version': triage_record.get('version'),
     })
 
@@ -302,6 +320,30 @@ def _PopulateHeuristicDataForCompileFailure(analysis, data):
       data['suspected_cls_by_heuristic'] = compile_failure['suspected_cls']
 
 
+def _GetAllSuspectedCLsAndCheckStatus(
+    master_name, builder_name, build_number, analysis):
+  build_key = '%s/%s/%d' % (master_name, builder_name, build_number)

[lijeffrey, 2016/09/23 05:09:09]

nit: how about import base_build_model and use CreateBuildId here instead?

[stgao, 2016/09/23 19:35:41]

This encoding and decoding should be abstracted in one place as I commented in
the other CL.

[chanli, 2016/09/24 01:09:40]

Done.

[chanli, 2016/09/24 01:09:40]

Done.

+  suspected_cls = analysis.suspected_cls

[stgao, 2016/09/23 19:35:41]

Should we do a deep copy first before modifying?

[chanli, 2016/09/24 01:09:40]

Oops, Done.

+  if not suspected_cls:
+    return []
+
+  for cl in suspected_cls:
+    cl['status'] = None
+    cl_object = WfSuspectedCL.Get(cl['repo_name'], cl['revision'])

[stgao, 2016/09/23 19:35:41]

Can we map var name to its class_name for easier reading?

[chanli, 2016/09/24 01:09:40]

Done.

+    # TODO(chanli): Write a script to convert legacy data before enable
+    # this change.
+    if not cl_object:
+      continue
+
+    build = cl_object.builds.get(build_key)
+    import json
+    print json.dumps(build, indent =2)

[lijeffrey, 2016/09/23 05:09:09]

nit: remove print before committing

[chanli, 2016/09/24 01:09:40]

Done.

+    if build:
+      cl['status'] = build['status']
+
+  return suspected_cls
+
+
 class BuildFailure(BaseHandler):
   PERMISSION_LEVEL = Permission.ANYONE
 
@@ -340,19 +382,16 @@ class BuildFailure(BaseHandler):
             analysis.triage_reference_analysis_build_number
     }
 
-  def _PrepareDataForCompileFailure(self, analysis):
-    data = self._PrepareCommonDataForFailure(analysis)
+  def _PrepareDataForCompileFailure(self, analysis, data):
 
     # Check result from heuristic analysis.
     _PopulateHeuristicDataForCompileFailure(analysis, data)
     # Check result from try job.
     data['try_job'] = _PrepareTryJobDataForCompileFailure(analysis)
 
-    return data
-
-  def _PrepareDataForTestFailures(self, analysis, build_info,
+  def _PrepareDataForTestFailures(self, analysis, build_info, data,
                                   show_debug_info=False):
-    data = self._PrepareCommonDataForFailure(analysis)
+
     data['status_message_map'] = result_status.STATUS_MESSAGE_MAP
 
     organized_results = _GetOrganizedAnalysisResultBySuspectedCL(
@@ -402,17 +441,24 @@ class BuildFailure(BaseHandler):
           force_try_job=force_try_job,
           queue_name=constants.WATERFALL_ANALYSIS_QUEUE)
 
+    data = self._PrepareCommonDataForFailure(analysis)
+    data['suspected_cls'] = _GetAllSuspectedCLsAndCheckStatus(
+        master_name, builder_name, build_number, analysis)
+
     if analysis.failure_type == failure_type.COMPILE:
+      self._PrepareDataForCompileFailure(analysis, data)
       return {
-          'template': 'waterfall/compile_failure.html',
-          'data': self._PrepareDataForCompileFailure(analysis),
+        'template': 'waterfall/compile_failure.html',
+        'data': data
       }
     else:
+      self._PrepareDataForTestFailures(
+        analysis, build_info, data, self._ShowDebugInfo())
       return {
-          'template': 'waterfall/test_failure.html',
-          'data': self._PrepareDataForTestFailures(analysis, build_info,
-                                                   self._ShowDebugInfo()),
+        'template': 'waterfall/test_failure.html',
+        'data': data
       }
 
+

[lijeffrey, 2016/09/23 05:09:09]

nit: too many blank lines

[chanli, 2016/09/24 01:09:40]

Done.

   def HandlePost(self):  # pragma: no cover
-    return self.HandleGet()
+    return self.HandleGet()