[Findit] Adding support for triaging suspected builds from flake analysis

appengine/findit/model/base_triaged_model.py

+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from google.appengine.ext import ndb
+from google.appengine.api import users
+
+from common import appengine_util
+from common import time_util
+from model import triage_status
+
+
+class TriageResult(ndb.Model):
+  # The user who updated this result.
+  user_name = ndb.StringProperty(default=None, indexed=False)
+
+  # The time this triage result was determined.
+  triaged_time = ndb.DateTimeProperty(indexed=False, auto_now=True)
+
+  # The result of the analysis as correct or not. If not triaged, the value
+  # should be None. Other traige result codes are up to the child class to set.
+  triage_result = ndb.IntegerProperty(indexed=True, default=None)
+
+  # Record which version of that generated the analysis this result is to be a
+  # part of. Since not all triaged models are versioned, record the version
+  # number as a string for those that are versioned or Findit's version for
+  # those that aren't.
+  version = ndb.StringProperty(indexed=False)

[chanli, 2016/10/14 21:25:09]

This is very confusing. I don't think you should mix them together.  For
WfAnalysis, we only have Findit version, and I agree that for
master_flake_analysis, we should also have the version of the analysis, so maybe
have 2 different parameters?

[lijeffrey, 2016/10/14 23:11:31]

Done.

Created findit_version and version_number.

+
+  # Other information about this result. For example, cl_info for suspected CLs
+  # or the suspected range for a flake analysis.
+  suspect_info = ndb.StringProperty(default=None, indexed=False)

[chanli, 2016/10/14 21:25:09]

Based on the comments, this value should not be a string?

[lijeffrey, 2016/10/14 23:11:31]

Changed to Json property

+
+
+class TriagedModel(ndb.Model):
+  """The parent class for models that can have traige results."""
+
+  def UpdateTriageResult(self, triage_result, suspect_info, user_name,
+                         version=None):
+    result = TriageResult()
+    result.user_name = user_name
+    result.triage_result = triage_result
+    result.version = (
+        str(version) if version else appengine_util.GetCurrentVersion())
+    result.suspect_info = suspect_info
+    self.triage_history.append(result)
+
+  # Record the triage result history.
+  triage_history = ndb.LocalStructuredProperty(

[chanli, 2016/10/14 21:25:09]

Nit: Do you ming move this to the end of the class? Just not get it surrounded
by functions.

[lijeffrey, 2016/10/14 23:11:31]

Done.

+      TriageResult, repeated=True, indexed=False, compressed=True)
+
+  @staticmethod
+  def GetTriageHistory(analysis):
+    """Gets the triage history of a WfAnalysis as a list.

[chanli, 2016/10/14 21:25:09]

This is not necessarily a WfAnalysis, right? Actually only master_flake_analysis
inherits TriagedModel. And why make it static? Is there anything wrong to make
it as below?

def GetTriageHistory(self):
 # proess and return self.triage_history

[lijeffrey, 2016/10/14 23:11:31]

Oh yes sorry I got ahead of myself. That is the eventual outcome, WfAnalysis
isn't hooked up to use this new model yet

[chanli, 2016/10/15 01:47:21]

I actually have a second question here: why make the function static? 

+
+    Args:
+      analysis (WfAnalysis): An instance of WfAnalysis assumed to inherit
+      TriagedModel.
+
+    Returns:
+      A list of dicts representing the triage history of the analysis.
+    """
+    if (not users.is_current_user_admin() or
+        not analysis.completed or
+        not analysis.triage_history):  # pragma: no cover
+      return None
+
+    triage_history = []
+    for triage_record in analysis.triage_history:
+      triage_history.append({
+          'triaged_time': time_util.FormatDatetime(triage_record.triaged_time),
+          'user_name': triage_record.user_name,
+          'suspect_info': triage_record.suspect_info,
+          'triage_result': (
+              triage_status.TRIAGE_STATUS_TO_DESCRIPTION.get(
+                  triage_record.triage_result)),
+          'version': triage_record.version,
+      })
+
+    return triage_history