Swarming UI: Bold requested dimensions in bot dimensions

appengine/swarming/ui/res/imp/taskpage/task-page.html

 <!--
   Copyright 2016 The LUCI Authors. All rights reserved.
   Use of this source code is governed under the Apache License, Version 2.0
   that can be found in the LICENSE file.
 
   This in an HTML Import-able file that contains the definition
   of the following elements:
 
     <task-page>
 
@@ skipping 45 matching lines @@
       }
 
       .right {
         margin-top: 8px;
       }
 
       .break-all {
         word-break: break-all;
       }
 
+      .bolded {
+        font-weight: bold;
+      }
+
       .expand {
         min-width: 3em;
         vertical-align: middle;
         padding: .5em;
       }
 
       .code {
         font-family: monospace;
       }
 
@@ skipping 45 matching lines @@
       .full-width-container > paper-checkbox {
         position: absolute;
         bottom: 10px;
         width: 140px;
         left: 5px;
       }
 
       .italic {
         font-style: italic;
       }
+
+      span~span:before {

[Ryan Tseng, 2017/06/06 00:27:55]

This should be restricted to certain classes.  ie span.dim~span.dim

[cwpayton, 2017/06/06 16:04:08]

Done.

+        content: " | ";
+      }
     </style>
 
     <url-param name="id"
       value="{{task_id}}">
     </url-param>
     <url-param name="try_detail"
       value="{{_try_detail}}">
     </url-param>
     <url-param name="request_detail"
       value="{{_request_detail}}">
@@ skipping 194 matching lines @@
                     <a
                         title="The list of bots that matches the list of dimensions"
                         href$="[[_botListLink(_request.properties.dimensions)]]">Bots</a>
                      <a
                         title="The list of tasks that matches the list of dimensions"
                         href$="[[_taskListLink(_request.properties.dimensions)]]">Tasks</a>
                   </td>
                 </tr>
                 <template is="dom-repeat" items="{{_request.properties.dimensions}}" as="dimension">
                   <tr>
-                    <td><b>[[dimension.key]]:</b> [[_alias(dimension)]]</td>
+                    <td><b>[[dimension.key]]:</b> [[_aliasRequest(dimension)]]</td>
                   </tr>
                 </template>
                 <tr>
                   <td>Isolated Inputs</td>
                   <td>
                     <a href$="[[_isolateLink(_request.properties.inputs_ref)]]">
                       [[_request.properties.inputs_ref.isolated]]
                     </a>
                   </td>
                 </tr>
@@ skipping 120 matching lines @@
                   <td>Bot assigned to task</td>
                   <td><a href$="[[_botLink(_result.bot_id)]]">[[_result.bot_id]]</td>
                 </tr>
                 <tr>
                   <td rowspan$="[[_rowspan(_result.bot_dimensions)]]">
                     <a>Bot Dimensions</a>
                   </td>
                 </tr>
                 <template is="dom-repeat" items="[[_result.bot_dimensions]]" as="dimension">
                   <tr>
-                    <td><b>[[dimension.key]]:</b> [[_alias(dimension)]]</td>
+                    <td bgcolor$="[[_highlight(_request.properties.dimensions, dimension.key)]]">

[kjlubick, 2017/06/06 12:26:54]

Super minor nit: you only need the $= for certain attributes like href, hidden,
etc. Otherwise, just use = (aka a property binding)

https://www.polymer-project.org/1.0/docs/devguide/data-binding#native-binding

[cwpayton, 2017/06/06 16:04:08]

Turns out bgcolor needs a $= instead of a simple =. I made the change and the
highlight stopped working, so I switched it back to $=.

+                      <b>[[dimension.key]]:</b>
+                      <template 
+                        is="dom-repeat" 
+                        items="[[_aliasResult(_request.properties.dimensions, dimension)]]" 
+                        as="value">
+                        <span class$="[[value.class]]">[[value.name]]</span>
+                      </template>
+                    </td>
                   </tr>
                 </template>
 
                 <tr>
                   <td>Exit code</td>
                   <td>[[_result.exit_code]]</td>
                 </tr>
                 <tr>
                   <td>Try number</td>
                   <td>[[_result.try_number]]</td>
@@ skipping 232 matching lines @@
           type: Object,
         },
         _stdout: {
           type: String,
         }
       },
 
       observers: [
         "_similarLoad(_request,_result,_auth_headers)"],
 
-      _alias: function(dim) {
-        var values = dim.value;
+      _aliasRequest: function(request) {
+        var values = request.value;
         if (!Array.isArray(values)) {
           values = [values];
         }
-        if (swarming.alias.has(dim.key)) {
-          values.forEach(function(v, i){
-            values[i] = swarming.alias.apply(v, dim.key);
+        if (swarming.alias.has(request.key)) {
+          values.forEach(function(v, i) {
+            values[i] = swarming.alias.apply(v, request.key);
           });
         }
         return values.join(" | ");
       },
 
+      _aliasResult: function(request, result) {

[kjlubick, 2017/06/06 12:26:54]

Please rename request -> request_dims and result -> dim or similar

Elsewhere request means the entire request object and result means the result
object;  we would prefer to not confuse the reader.

[cwpayton, 2017/06/06 16:04:08]

Done.

+        // request is the entire _request.properties.dimensions array, but we are

[Sergey Berezin, 2017/06/06 00:14:05]

nit: please reformat to fit 80 chars.

[cwpayton, 2017/06/06 16:04:08]

Done.

+        // only passed in one key at a time from _result.bot_dimensions as the
+        // result variable. Therefore we need to loop through the key-value pairs
+        // of request to determine whether the current bot dimension was requested.
+        var dimensions = [];
+        var target = null;
+        request.forEach(function(dim, i) {
+          if (dim.key == result.key) target = i;

[kjlubick, 2017/06/06 12:26:54]

Use triple equals unless there is a good reason not to (and then document what
that good reason is).

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+        });
+        var values = result.value;
+        if (!Array.isArray(values)) {

[kjlubick, 2017/06/06 12:26:54]

Just curious, is this logic just for safety or is the dimension's value
occasionally not an array?

[cwpayton, 2017/06/06 16:04:08]

This check was written by someone else before me, so I assume they ran into a
problem where if result only had one value, that value was simply passed in
as-is instead of inside of an array, so I left it in.

+          values = [values];
+        }
+        values.forEach(function(v, i) {
+          // Adjust the proper alias, if necessary
+          if (swarming.alias.has(result.key)){
+            values[i] = swarming.alias.apply(v, result.key);
+          }
+          // Set class attribute to bolded if the value was included in the
+          // requested dimensions
+          var bold = "";
+          if (target != null && request[target].value == v) bold = "bolded"

[kjlubick, 2017/06/06 12:26:54]

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+          dimensions.push({"name": values[i], "class": bold});
+        });
+
+        // Return an array of objects that map each value to its corresponding
+        // class name based on whether it should be bolded.
+        return dimensions;

[kjlubick, 2017/06/06 12:26:54]

Minor naming suggestion (take it or leave it), dimensions -> boldMap

[cwpayton, 2017/06/06 16:04:08]

Done.

+      },
+
       _bytes: function(sizeInBytes) {
         return sk.human.bytes(sizeInBytes);
       },
 
       _canCancelTask: function(result, permissions) {
         return result && result.state === "PENDING" && permissions.cancel_task;
       },
 
       _cancelTask: function() {
         var url = "/api/swarming/v1/task/" + this.task_id +"/cancel";
@@ skipping 106 matching lines @@
         if (!template) {
           return undefined;
         }
         return template.replace("%s", id);
       },
 
       _hasActualCIPDPackages: function(result) {
         return result && result.cipd_pins && result.cipd_pins.packages;
       },
 
+      _highlight: function(request, key) {
+        var highlight = false;
+        request.forEach(function (dim) {
+          if (dim.key == key) highlight = true;

[kjlubick, 2017/06/06 12:26:54]

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+        });
+        return highlight ? "Yellow" : "White";
+      },
+
       _internalClass: function(failure) {
         if (failure) {
           return "exception";
         }
         return "";
       },
 
       _isolateLink: function(ref) {
         if (!ref || !ref.isolatedserver) {
           return undefined;
@@ skipping 183 matching lines @@
 
         this._getJsonAsync("_other_running", "/api/swarming/v1/tasks/count", "_busyRunningCount", this._auth_headers, taskCountParams);
         // change running to pending
         taskCountParams.state = ["PENDING"];
         this._getJsonAsync("_other_pending", "/api/swarming/v1/tasks/count", "_busyPendingCount", this._auth_headers, taskCountParams);
       }
     });
   })();
   </script>
 </dom-module>