Milo: Print raw json for buildbot build properties

milo/appengine/buildbot/build.go

Index: milo/appengine/buildbot/build.go
diff --git a/milo/appengine/buildbot/build.go b/milo/appengine/buildbot/build.go
index 289b53592dbd5f3146e9d0fa95120449cf5a1416..559830da8cdee65aed2eb304bd5af360a25c3240 100644
--- a/milo/appengine/buildbot/build.go
+++ b/milo/appengine/buildbot/build.go
@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -315,46 +316,26 @@ func components(b *buildbotBuild) (result []*resp.BuildComponent) {
 
 // parseProp returns a representation of v based off k, and a boolean to

[nodir, 2017/04/11 07:13:19]

there is no k

[nodir, 2017/04/11 07:13:19]

high level: i think JSON encoding should be in a template func, not here
because this logic is a part of presentation

[hinoka, 2017/04/13 21:01:59]

This was done this was because previously rietveld links required knowing both
the "rietveld" and "issue" fields (to match buildbot behavior).  This has been
since moved to the "Source" section.

 // specify whether or not to hide it altogether.

[nodir, 2017/04/11 07:13:19]

after I read this, i thought the function returns true if a property should be
hidden. 
maybe "whether or not to display it"? or more explicit ", and false to specify
to hide it altogether"

[hinoka, 2017/04/13 21:01:59]

Removed block.

-func parseProp(prop map[string]Prop, k string, v interface{}) (string, bool) {
-	switch k {
-	case "requestedAt":
-		if vf, ok := v.(float64); ok {
-			return time.Unix(int64(vf), 0).UTC().Format(time.RFC3339), true
-		}
-	case "buildbucket":
-		var b map[string]interface{}
-		json.Unmarshal([]byte(v.(string)), &b)
-		if b["build"] == nil {
-			return "", false
-		}
-		build := b["build"].(map[string]interface{})
-		id := build["id"]
-		if id == nil {
-			return "", false
-		}
-		return fmt.Sprintf("https://cr-buildbucket.appspot.com/b/%s", id.(string)), true
-	case "issue":
-		if rv, ok := prop["rietveld"]; ok {
-			rietveld := rv.Value.(string)
-			// Issue could be a float, int, or string.
-			switch v := v.(type) {
-			case float64:
-				return fmt.Sprintf("%s/%d", rietveld, int(v)), true
-			default: // Probably int or string
-				return fmt.Sprintf("%s/%v", rietveld, v), true
-			}
-		}
-		return fmt.Sprintf("%d", int(v.(float64))), true
-	case "rietveld":
+func parseProp(v interface{}) (string, bool) {
+	if vs, ok := v.(string); ok && vs == "" {
+		return "", false // Value is empty, don't show it.

[nodir, 2017/04/11 07:13:19]

why we are hiding empty strings? They are legit values

[hinoka, 2017/04/13 21:01:59]

They why is because it seems kind of useless to display them.  But if you think
they should be there i can put it back.

[nodir, 2017/04/13 21:08:20]

Yeah, I think they should be there. Imagine someone triggers a build a
specifies a="" property. They would think they are doing something wrong if they
don't see the property in the list

+	}
+	if v == nil {
 		return "", false
-	default:
-		if vs, ok := v.(string); ok && vs == "" {
-			return "", false // Value is empty, don't show it.
-		}
-		if v == nil {
-			return "", false
+	}
+	// if v is a whole number, force it into an int.  json.Marshal() would turn
+	// it into what looks like a float instead.  We want this to remain and
+	// int instead of a number.
+	if vf, ok := v.(float64); ok {
+		if math.Floor(vf) == vf {
+			return fmt.Sprintf("%d", int64(vf)), true
 		}
 	}
+	// return the json representation of the value.
+	b, err := json.Marshal(v)
+	if err == nil {
+		return string(b), true
+	}
 	return fmt.Sprintf("%v", v), true
 }
 
@@ -383,7 +364,7 @@ func properties(b *buildbotBuild) (result []*resp.PropertyGroup) {
 		if _, ok := groups[groupName]; !ok {
 			groups[groupName] = &resp.PropertyGroup{GroupName: groupName}
 		}
-		vs, ok := parseProp(allProps, key, value)
+		vs, ok := parseProp(value)
 		if !ok {
 			continue
 		}