Show revision result in the project's commits page

Summary: If revision result is available for that commit, show it beside the build result. If it is not available, then use "unknown".

Test Plan:
ran locally + unit tests
{F527506}

Reviewers: kylec, anupc

Reviewed By: anupc

Subscribers: changesbot, kylec, wwu, anupc

Tags: #changes_ui

Differential Revision: https://tails.corp.dropbox.com/D232121

changes/api/project_commit_index.py

@@ -12,6 +12,7 @@
 from changes.models.build import Build
 from changes.models.project import Project, ProjectOptionsHelper
 from changes.models.revision import Revision
+from changes.models.revisionresult import RevisionResult
 from changes.models.source import Source
 
 
@@ -74,16 +75,30 @@ def get(self, project_id):
             builds_map = self.get_builds_for_commits(
                 commits, project, args.all_builds)
 
+        revision_result_map = self.get_revision_result_map(project, [c['id'] for c in commits])
+
         results = []
         for result in commits:
             if args.all_builds:
                 result['builds'] = builds_map.get(result['id'], [])
             else:
                 result['build'] = builds_map.get(result['id'])
+            if result['id'] in revision_result_map:
+                result['revisionResult'] = self.serialize(revision_result_map[result['id']])
             results.append(result)
 
         return self.respond(results, serialize=False, links=page_links)
 
+    def get_revision_result_map(self, project, shas):
+        revision_results = RevisionResult.query.filter(
+            RevisionResult.revision_sha.in_(shas),
+            RevisionResult.project_id == project.id,
+        )
+        result_map = {}
+        for result in revision_results:
+            result_map[result.revision_sha] = result
+        return result_map
+
     def get_commits_from_vcs(self, repo, vcs, offset, limit, paths, parent, branch):
         vcs_log = list(vcs.log(
             offset=offset,

changes/api/serializer/__init__.py

@@ -2,3 +2,6 @@
 
 # register models
 from . import models  # NOQA
+
+# register VCS
+from . import vcs  # NOQA

changes/api/serializer/models/revisionresult.py

@@ -1,15 +1,15 @@
+from __future__ import absolute_import
+
 from changes.api.serializer import Crumbler, register
-from changes.vcs.base import RevisionResult
+from changes.models.revisionresult import RevisionResult
 
 
 @register(RevisionResult)
-class RevisionCrumbler(Crumbler):
+class RevisionResultCrumbler(Crumbler):
     def crumble(self, instance, attrs):
         return {
-            'id': instance.id,
-            'sha': instance.id,  # Having both id and sha is a bit distasteful. We should try to fix this.
-            'message': instance.message,
-            'author': None,  # We don't return author information
-            'dateCreated': instance.author_date,
-            'dateCommitted': instance.committer_date,
+            'id': instance.id.hex,
+            'revisionSha': instance.revision_sha,
+            'build': instance.build,
+            'result': instance.result,
         }

changes/api/serializer/vcs/__init__.py

@@ -0,0 +1,3 @@
+from changes.utils.imports import import_submodules
+
+import_submodules(globals(), __name__, __path__)

changes/api/serializer/vcs/revisionresult.py

@@ -0,0 +1,15 @@
+from changes.api.serializer import Crumbler, register
+from changes.vcs.base import RevisionResult
+
+
+@register(RevisionResult)
+class RevisionCrumbler(Crumbler):
+    def crumble(self, instance, attrs):
+        return {
+            'id': instance.id,
+            'sha': instance.id,  # Having both id and sha is a bit distasteful. We should try to fix this.
+            'message': instance.message,
+            'author': None,  # We don't return author information
+            'dateCreated': instance.author_date,
+            'dateCommitted': instance.committer_date,
+        }

tests/changes/api/serializer/models/test_revisionresult.py

@@ -0,0 +1,25 @@
+from changes.api.serializer import serialize
+from changes.testutils.cases import TestCase
+
+
+class RevisionResultCrumblerTestCase(TestCase):
+
+    def test_simple(self):
+        build = self.create_build(self.create_project())
+        revision_result = self.create_revision_result(build=build, revision_sha=build.source.revision_sha, project_id=build.project_id)
+
+        data = serialize(revision_result)
+        assert data['id'] == revision_result.id.hex
+        assert data['revisionSha'] == revision_result.revision_sha
+        assert data['build']['id'] == build.id.hex
+        assert data['result']['id'] == 'unknown'
+
+    def test_no_build(self):
+        project = self.create_project()
+        revision_result = self.create_revision_result(revision_sha='a' * 40, project_id=project.id)
+
+        data = serialize(revision_result)
+        assert data['id'] == revision_result.id.hex
+        assert data['revisionSha'] == 'a' * 40
+        assert data['build'] is None
+        assert data['result']['id'] == 'unknown'

tests/changes/api/test_project_commit_index.py

@@ -3,7 +3,8 @@
 from datetime import datetime
 from uuid import uuid4
 
-from changes.constants import Status
+from changes.constants import Result, Status
+from changes.models.revisionresult import RevisionResult as RevisionResultModel
 from changes.testutils import APITestCase
 from changes.vcs.base import Vcs, RevisionResult
 
@@ -19,6 +20,7 @@ def test_simple(self):
 
         source = self.create_source(project, revision_sha=revision1.sha)
         build = self.create_build(project, source=source, status=Status.finished)
+        revision_result1 = self.create_revision_result(build=build, result=Result.passed, project=project, revision_sha=revision1.sha)
 
         path = '/api/0/projects/{0}/commits/'.format(fake_project_id.hex)
 
@@ -28,6 +30,7 @@ def test_simple(self):
         path = '/api/0/projects/{0}/commits/'.format(project.id.hex)
 
         resp = self.client.get(path)
+
         assert resp.status_code == 200
         data = self.unserialize(resp)
         assert len(data) == 2
@@ -36,6 +39,11 @@ def test_simple(self):
         assert data[1]['id'] == revision1.sha
         assert data[1]['build']['id'] == build.id.hex
 
+        assert 'revisionResult' not in data[0]
+
+        revision_result = RevisionResultModel.query.get(data[1]['revisionResult']['id'])
+        assert revision_result == revision_result1
+
         resp = self.client.get(path + '?per_page=1&page=1')
         assert resp.status_code == 200
         data = self.unserialize(resp)

webapp/display/changes/build_conditions.js

@@ -84,6 +84,31 @@ export var get_runnable_condition = function(runnable) {
   return result_condition[result] || COND_UNKNOWN;
 }
 
+/*
+ * Similar to get_runnable_condition, but take in statusId and
+ * resultId directly.
+ */
+export var convert_status_and_result_to_condition = function(statusId, resultId) {
+  if (statusId === 'in_progress' || statusId === 'queued' || statusId === 'pending_allocation' || statusId == 'allocated') {
+    if (resultId === 'failed') {
+      return COND_WAITING_WITH_FAILURES;
+    }
+    return COND_WAITING;
+  }
+
+  const result_condition = {
+      'passed':              COND_PASSED,
+      'quarantined_passed':  COND_PASSED,
+      'failed':              COND_FAILED,
+      'quarantined_failed':  COND_FAILED,
+      'skipped':             COND_UNKNOWN,
+      'quarantined_skipped': COND_UNKNOWN,
+      'aborted':             COND_FAILED_ABORTED,
+      'infra_failed':        COND_FAILED_INFRA,
+  };
+  return result_condition[resultId] || COND_UNKNOWN;
+}
+
 /*
  * Combines the conditions of a bunch of runnables into a single summary
  * condition: if any failed or haven't finished, returns failed/waiting, etc.
@@ -209,7 +234,7 @@ export var ConditionDot = React.createClass({
       };
 
       var style = {
-        fontSize: font_sizes[this.props.size], 
+        fontSize: font_sizes[this.props.size],
       };
       if (this.props.size === 'medium') {
         style = _.extend(style, {marginLeft: 2, marginRight: 6});
@@ -231,12 +256,12 @@ export var ConditionDot = React.createClass({
 
       // cap num at 99 unless rendering the large widget
       var num = this.props.num;
-      if (this.props.size !== 'large' && 
+      if (this.props.size !== 'large' &&
           (_.isFinite(num) && num > 99)) {
         num = '99+';
       }
 
-      if ((_.isString(num) && num.length === 1) || 
+      if ((_.isString(num) && num.length === 1) ||
           (_.isFinite(num) && num > 0 && num < 9)) {
         classes.push('singleDigit');
       }
@@ -271,29 +296,29 @@ Examples.add('ConditionDot', __ => {
       <ConditionDot className="marginRightS" condition="passed" size="large" num={222} />
     </div>,
     <div>
-      <ConditionDot 
-        className="marginRightS" 
-        condition="passed" 
-        size="small" 
-        multiIndicator={true} 
+      <ConditionDot
+        className="marginRightS"
+        condition="passed"
+        size="small"
+        multiIndicator={true}
       />
-      <ConditionDot 
-        className="marginRightS" 
-        condition="passed" 
-        size="smaller" 
-        multiIndicator={true} 
+      <ConditionDot
+        className="marginRightS"
+        condition="passed"
+        size="smaller"
+        multiIndicator={true}
       />
-      <ConditionDot 
-        className="marginRightS" 
-        condition="passed" 
-        size="medium" 
-        multiIndicator={true} 
+      <ConditionDot
+        className="marginRightS"
+        condition="passed"
+        size="medium"
+        multiIndicator={true}
       />
-      <ConditionDot 
-        className="marginRightS" 
-        condition="passed" 
-        size="large" 
-        multiIndicator={true} 
+      <ConditionDot
+        className="marginRightS"
+        condition="passed"
+        size="large"
+        multiIndicator={true}
       />
     </div>,
     <div>

webapp/pages/project_page/commits_tab.js

@@ -8,7 +8,10 @@ import { ChangesChart } from 'es6!display/changes/charts';
 import { Grid } from 'es6!display/grid';
 import { MissingBuildStatus, SingleBuildStatus } from 'es6!display/changes/builds';
 import { TimeText, display_duration } from 'es6!display/time';
-import { get_runnable_condition, is_waiting } from 'es6!display/changes/build_conditions';
+import { convert_status_and_result_to_condition, get_runnable_condition,
+         get_runnable_condition_short_text, is_waiting,
+         ConditionDot,
+       } from 'es6!display/changes/build_conditions';
 
 import InteractiveData from 'es6!pages/helpers/interactive_data';
 
@@ -158,17 +161,19 @@ var CommitsTab = React.createClass({
     var grid_data = _.map(data_to_show, c => this.turnIntoRow(c, project_info));
 
     var cellClasses = [
-      'buildWidgetCell', 
-      'wide easyClick', 
-      'bluishGray nowrap', 
-      'bluishGray nowrap', 
-      'nowrap', 
-      'nowrap', 
+      'buildWidgetCell',
+      'buildWidgetCell',
+      'wide easyClick',
+      'bluishGray nowrap',
+      'bluishGray nowrap',
+      'nowrap',
+      'nowrap',
       'nowrap'
     ];
 
     var headers = [
-      'Result',
+      'Revision',
+      'Build',
       'Name',
       'Time',
       'Tests Ran',
@@ -178,7 +183,7 @@ var CommitsTab = React.createClass({
     ];
 
     return <Grid
-      colnum={7}
+      colnum={8}
       data={grid_data}
       cellClasses={cellClasses}
       headers={headers}
@@ -233,8 +238,16 @@ var CommitsTab = React.createClass({
       />
     }
 
+    let revisionResultId = c.revisionResult ? c.revisionResult.result.id : "unknown";
+    let revisionCondition = convert_status_and_result_to_condition(c.status, revisionResultId)
+    let label = get_runnable_condition_short_text(revisionCondition);
+    let markup = <SimpleTooltip label={label} placement="right">
+      <span><ConditionDot condition={revisionCondition} /></span>
+    </SimpleTooltip>;
+
     // TODO: if there are any comments, show a comment icon on the right
     return [
+      markup,
       build_widget,
       title,
       duration,