Merge lp:~frankban/juju-gui/bug-1103204-remove-relation into lp:juju-gui/experimental

[Francesco Banconi]

Fix error while removing relations.

When removing a relation sometimes an exception
is raised: "Uncaught TypeError: Cannot call method
'one' of null".
I was able to reproduce the bug following these steps:
1) click on a relation, the remove relation modal
   dialog appears;
2) open the GUI in another tab, and force a delta stream
   event (e.g. add/remove another relation,
   destroy a service...);
3) on delta, the relation nodes are redrawn on both tabs,
   so the element stored in the dialog no longer exists
   in the DOM;
4) on the first tab, click to confirm the relation removal,
   you'll see the error.
In this branch, the relation node is retrieved again in
removeRelation using its relation id. This way we ensure
the element actually exists in the DOM.

https://codereview.appspot.com/7199043/

[Francesco Banconi (frankban)]

Reviewers: mp+144484_code.launchpad.net,

Message:
Please take a look.

Description:
Fix error while removing relations.

When removing a relation sometimes an exception
is raised: "Uncaught TypeError: Cannot call method
'one' of null".
I was able to reproduce the bug following these steps:
1) click on a relation, the remove relation modal
    dialog appears;
2) open the GUI in another tab, and force a delta stream
    event (e.g. add/remove another relation,
    destroy a service...);
3) on delta, the relation nodes are redrawn on both tabs,
    so the element stored in the dialog no longer exists
    in the DOM;
4) on the first tab, click to confirm the relation removal,
    you'll see the error.
In this branch, the relation node is retrieved again in
removeRelation using its relation id. This way we ensure
the element actually exists in the DOM.

https://code.launchpad.net/~frankban/juju-gui/bug-1103204-remove-relation/+merge/144484

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/views/topology/relation.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: app/views/topology/relation.js
=== modified file 'app/views/topology/relation.js'
--- app/views/topology/relation.js 2013-01-22 20:25:32 +0000
+++ app/views/topology/relation.js 2013-01-23 12:16:30 +0000
@@ -439,16 +439,19 @@
          self.addRelation(); // Will clear the state.
        }
      },
- removeRelation: function(d, context, view, confirmButton) {
+ removeRelation: function(d, view, confirmButton) {
        var env = this.get('component').get('env');
        var endpoints = d.endpoints;
- var relationElement = Y.one(context.parentNode).one('.relation');
+ var relationId = d.relation_id;
+ // 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('#' + relationId);
        utils.addSVGClass(relationElement, 'to-remove pending-relation');
        env.remove_relation(
            endpoints[0][0] + ':' + endpoints[0][1].name,
            endpoints[1][0] + ':' + endpoints[1][1].name,
            Y.bind(this._removeRelationCallback, this, view,
- relationElement, d.relation_id, confirmButton));
+ relationElement, relationId, confirmButton));
      },

      _removeRelationCallback: function(view,
@@ -491,7 +494,7 @@
              ev.preventDefault();
              var confirmButton = ev.target;
              confirmButton.set('disabled', true);
- view.removeRelation(d, context, view, confirmButton);
+ view.removeRelation(d, view, confirmButton);
            },
            this)));
      },

[Brad Crittenden (bac)]

Looks good to land Francesco. Nice work figuring it out.

https://codereview.appspot.com/7199043/

[Francesco Banconi (frankban)]

*** Submitted:

Fix error while removing relations.

When removing a relation sometimes an exception
is raised: "Uncaught TypeError: Cannot call method
'one' of null".
I was able to reproduce the bug following these steps:
1) click on a relation, the remove relation modal
    dialog appears;
2) open the GUI in another tab, and force a delta stream
    event (e.g. add/remove another relation,
    destroy a service...);
3) on delta, the relation nodes are redrawn on both tabs,
    so the element stored in the dialog no longer exists
    in the DOM;
4) on the first tab, click to confirm the relation removal,
    you'll see the error.
In this branch, the relation node is retrieved again in
removeRelation using its relation id. This way we ensure
the element actually exists in the DOM.

R=bac, benji
CC=
https://codereview.appspot.com/7199043

https://codereview.appspot.com/7199043/

=== modified file 'app/views/topology/relation.js'
--- app/views/topology/relation.js	2013-01-22 20:25:32 +0000
+++ app/views/topology/relation.js	2013-01-23 12:27:19 +0000
@@ -439,16 +439,19 @@
         self.addRelation(); // Will clear the state.
       }
     },
-    removeRelation: function(d, context, view, confirmButton) {
+    removeRelation: function(d, view, confirmButton) {
       var env = this.get('component').get('env');
       var endpoints = d.endpoints;
-      var relationElement = Y.one(context.parentNode).one('.relation');
+      var relationId = d.relation_id;
+      // 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('#' + relationId);
       utils.addSVGClass(relationElement, 'to-remove pending-relation');
       env.remove_relation(
           endpoints[0][0] + ':' + endpoints[0][1].name,
           endpoints[1][0] + ':' + endpoints[1][1].name,
           Y.bind(this._removeRelationCallback, this, view,
-          relationElement, d.relation_id, confirmButton));
+          relationElement, relationId, confirmButton));
     },
 
     _removeRelationCallback: function(view,
@@ -491,7 +494,7 @@
             ev.preventDefault();
             var confirmButton = ev.target;
             confirmButton.set('disabled', true);
-            view.removeRelation(d, context, view, confirmButton);
+            view.removeRelation(d, view, confirmButton);
           },
           this)));
     },