Reviewers: mp+158224_code.launchpad.net,

Message:
Please take a look.

Description:
Make safe relation ids in environment view

Relation ids included spaces and other problematic chacracters in our
related DOM ids.  This branch cleans those up, and also does a quick CSS
cleanup to make some text legible again.

https://code.launchpad.net/~gary/juju-gui/bug1167295/+merge/158224

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/8640043/

Affected files:
   A [revision details]
   M app/templates/unit.handlebars
   M app/views/topology/relation.js
   M app/views/utils.js
   M lib/views/stylesheet.less
   M test/test_environment_view.js
   M test/test_topology_relation.js
   M test/test_utils.js


Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: test/test_environment_view.js
=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js	2013-04-04 14:45:42 +0000
+++ test/test_environment_view.js	2013-04-10 21:17:18 +0000
@@ -84,7 +84,7 @@
        Y = YUI(GlobalConfig).use([
          'juju-views', 'juju-tests-utils', 'juju-env',
          'node-event-simulate', 'juju-gui', 'slider',
-        'landscape', 'dump'
+        'landscape', 'dump', 'juju-view-utils'
        ], function(Y) {
          testUtils = Y.namespace('juju-tests.utils');
          views = Y.namespace('juju.views');
@@ -170,6 +170,13 @@
                      parseInt(this.getAttribute(e), 10))
                              .should.equal(true);
                }, line);
+
+          // Verify that the node id has been munged as expected from the
+          // relation id. This is particularly important for Juju Core.
+          var node = container.one(
+              '#' + views.utils.generateSafeDOMId('relation-0000000007'));
+          assert.isNotNull(node);
+          assert.isDefined(node);
          });

      it('must be able to render subordinate and normal services',
@@ -750,7 +757,9 @@
             db: db,
             env: env}).render();

-         var relation = container.one('#relation-0000000001 .rel-label'),
+         var relation = container.one(
+              '#' + views.utils.generateSafeDOMId('relation-0000000001') +
+              ' .rel-label'),
           dialog_btn,
           panel;

@@ -778,7 +787,9 @@
             env: env}).render();

           // Get a subordinate relation.
-         var relation = container.one('#relation-0000000007 .rel-label'),
+         var relation = container.one(
+              '#' + views.utils.generateSafeDOMId('relation-0000000007') +
+              ' .rel-label'),
           dialog_btn,
           panel;



Index: test/test_topology_relation.js
=== modified file 'test/test_topology_relation.js'
--- test/test_topology_relation.js	2013-01-23 21:56:36 +0000
+++ test/test_topology_relation.js	2013-04-10 21:17:18 +0000
@@ -4,7 +4,8 @@
    var Y, views, view, container, topo, db;

    before(function(done) {
-    Y =  
YUI(GlobalConfig).use(['juju-topology', 'node', 'node-event-simulate'],
+    Y = YUI(GlobalConfig).use(
+         
['juju-topology', 'node', 'node-event-simulate', 'juju-view-utils'],
          function(Y) {
            views = Y.namespace('juju.views');
            done();
@@ -87,7 +88,8 @@
        endpoints: [null, null]
      };
      view.removeRelation.call(fauxView, relation, fauxView, undefined);
-    assert.equal(requestedSelector, '#' + relationId);
+    assert.equal(
+        requestedSelector, '#' +  
views.utils.generateSafeDOMId(relationId));
    });

  });


Index: app/templates/unit.handlebars
=== modified file 'app/templates/unit.handlebars'
--- app/templates/unit.handlebars	2013-04-10 09:48:16 +0000
+++ app/templates/unit.handlebars	2013-04-10 20:37:33 +0000
@@ -55,7 +55,7 @@
  </div>

  {{#unless relations}}
-This unit has no relations.
+<div class="yui3-u">This unit has no relations.</div>
  {{else}}
  <div id="relations" class="yui3-g">
    <div class="yui3-u-1">


Index: app/views/utils.js
=== modified file 'app/views/utils.js'
--- app/views/utils.js	2013-04-10 09:40:06 +0000
+++ app/views/utils.js	2013-04-10 21:17:18 +0000
@@ -12,6 +12,41 @@
    var views = Y.namespace('juju.views'),
        utils = Y.namespace('juju.views.utils');

+  /*jshint bitwise: false*/
+  /**
+    Create a hash of a string. From stackoverflow: http://goo.gl/PEOgF
+
+    @method generateHash
+    @param {String} value The string to hash.
+    @return {Integer} The hash of the string.
+   */
+  var generateHash = function(value) {
+    return value.split('').reduce(
+        function(hash, character) {
+          hash = ((hash << 5) - hash) + character.charCodeAt(0);
+          return hash & hash;
+        },
+        0
+    );
+  };
+  /*jshint bitwise: true*/
+  utils.generateHash = generateHash;
+
+  /**
+    Create a stable, safe DOM id given an arbitrary string.
+    See details and discussion in
+    https://bugs.launchpad.net/juju-gui/+bug/1167295
+
+    @method generateSafeDOMId
+    @param {String} value The string to hash.
+    @return {String} The calculated DOM id.
+   */
+  var generateSafeDOMId = function(value) {
+    return (
+        value.replace(/\W/g, '_') + '-' + generateHash(value));
+  };
+  utils.generateSafeDOMId = generateSafeDOMId;
+
    var timestrings = {
      prefixAgo: null,
      prefixFromNow: null,


Index: app/views/topology/relation.js
=== modified file 'app/views/topology/relation.js'
--- app/views/topology/relation.js	2013-04-10 16:38:13 +0000
+++ app/views/topology/relation.js	2013-04-10 21:17:18 +0000
@@ -164,7 +164,7 @@
          return relation.source.id === service.id ||
              relation.target.id === service.id;
        }), function(relation) {
-        var rel_group = d3.select('#' + relation.id);
+        var rel_group = d3.select('#' +  
utils.generateSafeDOMId(relation.id));
          var connectors = relation.source
                    .getConnectorPair(relation.target);
          var s = connectors[0];
@@ -199,7 +199,7 @@

        enter.insert('g', 'g.service')
                .attr('id', function(d) {
-            return d.id;
+            return utils.generateSafeDOMId(d.id);
            })
                .attr('class', function(d) {
                  // Mark the rel-group as a subordinate relation if need be.
@@ -461,7 +461,7 @@
        // At this time, relations may have been redrawn, so here we have to
        // retrieve the relation DOM element again.
        var relationElement = view.get('container')
-        .one('#' + relation.relation_id);
+        .one('#' + utils.generateSafeDOMId(relation.relation_id));
        utils.addSVGClass(relationElement, 'to-remove pending-relation');
        env.remove_relation(relation.endpoints[0], relation.endpoints[1],
            Y.bind(this._removeRelationCallback, this, view,
@@ -747,7 +747,7 @@
        // Remove our pending relation from the DB, error or no.
        db.relations.remove(
            db.relations.getById(relation_id));
-      vis.select('#' + relation_id).remove();
+      vis.select('#' + utils.generateSafeDOMId(relation_id)).remove();
        if (ev.err) {
          db.notifications.add(
              new models.Notification({


Index: lib/views/stylesheet.less
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-04-09 14:18:49 +0000
+++ lib/views/stylesheet.less	2013-04-10 20:37:33 +0000
@@ -872,6 +872,7 @@
      background-clip: padding-box;
      outline: none;
      font-weight: normal;
+    letter-spacing: normal;
      .yui3-widget-bd {
          margin-bottom: 1em;
      }


Index: test/test_utils.js
=== modified file 'test/test_utils.js'
--- test/test_utils.js	2013-04-10 09:54:21 +0000
+++ test/test_utils.js	2013-04-10 21:17:18 +0000
@@ -12,6 +12,31 @@
            });
      });

+    it('can generate a hash', function() {
+      // We aren't testing the algorithm here, just basic hash  
characteristics.
+      // It's a number.
+      assert.strictEqual(views.utils.generateHash(''), 0);
+      assert.isNumber(views.utils.generateHash('kumquat'));
+      assert.isNumber(views.utils.generateHash('qumquat'));
+      // It's stable.
+      assert.strictEqual(
+          views.utils.generateHash('kumquat'),
+          views.utils.generateHash('kumquat'));
+      // Different values hash differently.
+      assert.notEqual(
+          views.utils.generateHash('kumquat'),
+          views.utils.generateHash('qumquat'));
+    });
+
+    it('can generate safe relation ids', function() {
+      var relationId;
+      relationId = 'foo:Bar relation-00000006!@#';
+      assert.strictEqual(
+          views.utils.generateSafeDOMId(relationId),
+          'foo_Bar_relation_00000006___-' +
+          views.utils.generateHash(relationId));
+    });
+
      it('should create a confirmation panel',
         function() {
            var confirmed = false;