yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

Matthew Scott has proposed merging lp:~makyo/juju-gui/ambiguous-relations into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)

For more details, see:
https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616

Implement ambiguous relation menu for adding rels

Displays a menu of choices when adding a relation between two services would result in ambiguity, otherwise simply adds a relation.  Also, a bit of minor test clean-up in charm config, due to floating point inequality.

https://codereview.appspot.com/6736051/

-- 
https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/ambiguous-relations into lp:juju-gui.

=== modified file 'app/templates/overview.handlebars'
--- app/templates/overview.handlebars	2012-10-11 10:49:21 +0000
+++ app/templates/overview.handlebars	2012-10-19 17:36:21 +0000
@@ -1,7 +1,7 @@
 <div id="overview">
     <div id="canvas" class="crosshatch-background">
         <div id="service-menu">
-            <div id="service-menu-triangle">&nbsp;</div>
+            <div class="triangle">&nbsp;</div>
             <ul>
                 <li class="view-service">View</li>
                 <li class="add-relation">Build Relation</li>
@@ -9,6 +9,10 @@
                 <li class="destroy-service">Destroy</li>
             </ul>
         </div>
+        <div id="ambiguous-relation-menu">
+            <div class="triangle">&nbsp;</div>
+            <ul/>
+        </div>
     </div>
     <div id="overview-tasks">
         <div class="controls yui3-skin-sam">

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2012-10-18 17:52:44 +0000
+++ app/views/environment.js	2012-10-19 17:36:21 +0000
@@ -1016,7 +1016,7 @@
           if (curr_action === 'show_service') {
             this.set('currentServiceClickAction', 'addRelationStart');
           } else if (curr_action === 'addRelationStart' ||
-              curr_action === 'addRelationEnd') {
+              curr_action === 'ambiguousAddRelationTest') {
             this.set('currentServiceClickAction', 'toggleControlPanel');
           } // Otherwise do nothing.
         },
@@ -1073,7 +1073,7 @@
           // If we landed on a rect, add relation, otherwise, cancel.
           if (rect) {
             self.service_click_actions
-            .addRelationEnd(endpoint, self, rect);
+            .ambiguousAddRelationTest(endpoint, self, rect);
           } else {
             // TODO clean up, abstract
             self.cancelRelationBuild();
@@ -1395,15 +1395,16 @@
             var db = view.get('db'),
                 app = view.get('app'),
                 service = view.serviceForBox(m),
+                endpoints = models.getEndpoints(
+                    service, app.serviceEndpoints, db),
 
                 /* Transform endpoints into a list of
                  * relatable services (to the service in m)
                  */
                 possible_relations = Y.Array.map(
                     Y.Array.flatten(Y.Object.values(
-                        models.getEndpoints(
-                            service, app.serviceEndpoints, db))),
-                            function(ep) {return ep.service;}),
+                        endpoints)),
+                    function(ep) {return ep.service;}),
                 invalidRelationTargets = {};
 
             // Iterate services and invert the possibles list.
@@ -1430,15 +1431,87 @@
 
             // Store start service in attrs.
             view.set('addRelationStart_service', m);
+            // Store possible endpoints.
+            view.set('addRelationStart_possibleEndpoints', endpoints);
             // Set click action.
-            view.set('currentServiceClickAction', 'addRelationEnd');
+            view.set('currentServiceClickAction', 'ambiguousAddRelationTest');
+          },
+
+          /*
+           * Test if the pending relation is ambiguous.  Display a menu if so,
+           * create the relation if not.
+           */
+          ambiguousAddRelationTest: function(m, view, context) {
+            var endpoints = view.get('addRelationStart_possibleEndpoints'),
+                container = view.get('container');
+            if (endpoints[m.id].length > 1) {
+              // Display menu with available endpoints.
+              var menu = container.one('#ambiguous-relation-menu'),
+                  list = menu.one('ul');
+              list.empty();
+
+              // Add a cancel item.
+              var none_item = Y.Node.create('<li>None</li>');
+              none_item.on('click', function(evt) {
+                menu.removeClass('active');
+                view.cancelRelationBuild();
+              });
+              list.append(none_item);
+
+              // Add each endpoint choice, and bind an an event to 'click' to
+              // add the specified relation.
+              endpoints[m.id].forEach(function(endpoint) {
+                var li = Y.Node.create('<li>' + endpoint[0].service + ':' +
+                    endpoint[0].name + ' &rarr; ' +
+                    endpoint[1].service + ':' +
+                    endpoint[1].name + '</li>');
+                li.on('click', function(evt) {
+                  var endpoints_item = [
+                    [endpoint[0].service, {
+                      name: endpoint[0].name,
+                      role: 'server' }],
+                    [endpoint[1].service, {
+                      name: endpoint[1].name,
+                      role: 'client' }]];
+                  menu.removeClass('active');
+                  view.service_click_actions
+                    .addRelationEnd(endpoints_item, view, context);
+                });
+                list.append(li);
+              });
+
+              // Display the menu at the service endpoint.
+              var tr = view.zoom.translate(),
+                  z = view.zoom.scale();
+              menu.setStyle('top', m.y * z + tr[1]);
+              menu.setStyle('left', m.x * z + m.w * z + tr[0]);
+              menu.addClass('active');
+            } else {
+              // Create a relation with the only available endpoint.
+              var ep = endpoints[m.id][0],
+                  endpoints_item = [
+                    [ep[0].service, {
+                      name: ep[0].name,
+                      role: 'server' }],
+                    [ep[1].service, {
+                      name: ep[1].name,
+                      role: 'client' }]];
+              view.service_click_actions
+                .addRelationEnd(endpoints_item, view, context);
+            }
           },
 
           /*
            * Fired when clicking the second service is clicked in the
            * add relation flow.
+           *
+           * :param endpoints: array of two endpoints, each in the form
+           *   ['service name', {
+           *     name: 'endpoint type',
+           *     role: 'client or server'
+           *   }]
            */
-          addRelationEnd: function(m, view, context) {
+          addRelationEnd: function(endpoints, view, context) {
             // Redisplay all services
             view.cancelRelationBuild();
 
@@ -1447,9 +1520,9 @@
                 env = view.get('env'),
                 db = view.get('db'),
                 source = view.get('addRelationStart_service'),
-                relation_id = 'pending:' + source.id + m.id;
+                relation_id = 'pending:' + endpoints[0][0] + endpoints[1][0];
 
-            if (m.id === source.id) {
+            if (endpoints[0][0] === endpoints[1][0]) {
               view.set('currentServiceClickAction', 'toggleControlPanel');
               return;
             }
@@ -1459,10 +1532,7 @@
             db.relations.create({
               relation_id: relation_id,
               display_name: 'pending',
-              endpoints: [
-                [source.id, {name: 'pending', role: 'server'}],
-                [m.id, {name: 'pending', role: 'client'}]
-              ],
+              endpoints: endpoints,
               pending: true
             });
 
@@ -1473,8 +1543,8 @@
             // Fire event to add relation in juju.
             // This needs to specify interface in the future.
             env.add_relation(
-                source.id,
-                m.id,
+                endpoints[0][0] + ':' + endpoints[0][1].name,
+                endpoints[1][0] + ':' + endpoints[1][1].name,
                 Y.bind(this._addRelationCallback, this, view, relation_id)
             );
             view.set('currentServiceClickAction', 'toggleControlPanel');

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-10-18 17:50:22 +0000
+++ lib/views/stylesheet.less	2012-10-19 17:36:21 +0000
@@ -142,7 +142,7 @@
 }
 
 
-#service-menu {
+#service-menu, #ambiguous-relation-menu {
     @border_radius: 20px;
     @background_color: #282421;
 
@@ -166,7 +166,7 @@
         display: block;
     }
 
-    #service-menu-triangle {
+    .triangle {
         position: absolute;
         top: 62px;
         left: -12px;

=== modified file 'test/test_application_notifications.js'
--- test/test_application_notifications.js	2012-10-15 18:18:52 +0000
+++ test/test_application_notifications.js	2012-10-19 17:36:21 +0000
@@ -310,7 +310,11 @@
              };
 
        views.environment.prototype.service_click_actions.addRelationEnd
-           .apply(view, [{id: 1}, view]);
+           .apply(view, [
+         [
+          ['s1', {name: 'n', role: 'client'}],
+          ['s2', {name: 'n', role: 'server'}]],
+         view]);
 
        assertNotificationNumber('1');
 

=== modified file 'test/test_charm_configuration.js'
--- test/test_charm_configuration.js	2012-10-18 00:27:33 +0000
+++ test/test_charm_configuration.js	2012-10-19 17:36:21 +0000
@@ -232,7 +232,8 @@
     // The simulate module does not support firing scroll events so we call
     // the associated method directly.
     view._moveTooltip();
-    tooltip.get('boundingBox').getY().should.equal(originalY - 10);
+    Math.floor(tooltip.get('boundingBox').getY())
+      .should.equal(Math.floor(originalY - 10));
   });
 
   it('must hide the tooltip when its field scrolls away', function() {

=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js	2012-10-15 10:07:49 +0000
+++ test/test_environment_view.js	2012-10-19 17:36:21 +0000
@@ -299,28 +299,46 @@
 
     it('must be able to add a relation from the control panel',
        function() {
+         var view = new views.environment({
+           container: container,
+           app: {serviceEndpoints: {}},
+           db: db,
+           env: env
+         }).render();
+         var service = container.one('.service'),
+         add_rel = container.one('#service-menu .add-relation'),
+         after_evt;
+
          // Mock endpoints
          var existing = models.getEndpoints;
          models.getEndpoints = function() {
-           return {requires: [],
-             provides: []};
+           var endpoints = {},
+               serviceName = service.one('.name')
+                 .getDOMNode().firstChild.nodeValue,
+               nextServiceName = service.next().one('.name')
+                 .getDOMNode().firstChild.nodeValue;
+           endpoints[nextServiceName] = [
+             [
+              {
+                service: serviceName,
+                name: 'relName',
+                type: 'relType'
+              },
+              {
+                service: nextServiceName,
+                name: 'relName',
+                type: 'relType'
+              }
+             ]
+           ];
+           return endpoints;
          };
 
-         var view = new views.environment({
-           container: container,
-           app: {serviceEndpoints: {}},
-           db: db,
-           env: env
-         }).render();
-         var service = container.one('.service'),
-         add_rel = container.one('#service-menu .add-relation'),
-         after_evt;
-
          service.simulate('click');
          add_rel.simulate('click');
          container.all('.selectable-service')
                .size()
-               .should.equal(1);
+               .should.equal(2);
          service.next().simulate('click');
          container.all('.selectable-service').size()
             .should.equal(0);