Code review comment for lp:~bcsaller/juju-gui/update-reductions

Revision history for this message
Benjamin Saller (bcsaller) wrote :

Reviewers: mp+155972_code.launchpad.net,

Message:
Please take a look.

Description:
Remove some db.update calls

Given the current model we can't remove all calls
to db.fire('update') but we can remove them from
anything using the topology. Change 'update' events
(which redispatch) so simply call topo.update().

service and charm views still redispatch.

https://code.launchpad.net/~bcsaller/juju-gui/update-reductions/+merge/155972

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/views/topology/relation.js
   M app/views/topology/service.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-03-22 10:28:31 +0000
+++ app/views/topology/relation.js 2013-03-28 14:22:42 +0000
@@ -470,7 +470,8 @@

      _removeRelationCallback: function(view,
              relationElement, relationId, confirmButton, ev) {
- var db = this.get('component').get('db');
+ var topo = this.get('component'),
+ db = topo.get('db');
        var service = this.get('model');
        if (ev.err) {
          db.notifications.add(
@@ -486,7 +487,7 @@
          // Remove the relation from the DB.
          db.relations.remove(db.relations.getById(relationId));
          // Redraw the graph and reattach events.
- db.fire('update');
+ topo.update();
        }
        view.get('rmrelation_dialog').hide();
        view.get('rmrelation_dialog').destroy();
@@ -697,19 +698,20 @@
       * role: 'client or server'
       * }]
       */
- addRelationEnd: function(endpoints, view, context) {
+ addRelationEnd: function(endpoints, module) {
        // Redisplay all services
- view.cancelRelationBuild();
+ module.cancelRelationBuild();

        // Get the vis, and links, build the new relation.
- var vis = view.get('component').vis;
- var env = view.get('component').get('env');
- var db = view.get('component').get('db');
- var source = view.get('addRelationStart_service');
+ var topo = module.get('component');
+ var vis = topo.vis;
+ var env = topo.get('env');
+ var db = topo.get('db');
+ var source = module.get('addRelationStart_service');
        var relation_id = 'pending-' + endpoints[0][0] + endpoints[1][0];

        if (endpoints[0][0] === endpoints[1][0]) {
- view.set('currentServiceClickAction', 'hideServiceMenu');
+ module.set('currentServiceClickAction', 'hideServiceMenu');
          return;
        }

@@ -724,21 +726,20 @@

        // Firing the update event on the db will properly redraw the
        // graph and reattach events.
- //db.fire('update');
- view.get('component').bindAllD3Events();
- view.update();
+ topo.update();
+ topo.bindAllD3Events();

        // Fire event to add relation in juju.
        // This needs to specify interface in the future.
        env.add_relation(endpoints[0], endpoints[1],
- Y.bind(this._addRelationCallback, this, view, relation_id)
+ Y.bind(this._addRelationCallback, this, module, relation_id)
        );
- view.set('currentServiceClickAction', 'hideServiceMenu');
+ module.set('currentServiceClickAction', 'hideServiceMenu');
      },

- _addRelationCallback: function(view, relation_id, ev) {
+ _addRelationCallback: function(module, relation_id, ev) {
        console.log('addRelationCallback reached');
- var topo = view.get('component');
+ var topo = module.get('component');
        var db = topo.get('db');
        var vis = topo.vis;
        // Remove our pending relation from the DB, error or no.
@@ -772,10 +773,8 @@
            display_name: endpoints[0][1].name
          });
        }
- // Redraw the graph and reattach events.
- //db.fire('update');
- view.get('component').bindAllD3Events();
- view.update();
+ topo.update();
+ topo.bindAllD3Events();
      },

      /*

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-03-27 18:01:33 +0000
+++ app/views/topology/service.js 2013-03-27 22:22:46 +0000
@@ -1060,7 +1060,8 @@

      _destroyCallback: function(service, btn, ev) {
        var getModelURL = this.get('component').get('getModelURL'),
- db = this.get('component').get('db');
+ topo = this.get('component'),
+ db = topo.get('db');
        if (ev.err) {
          db.notifications.add(
              new models.Notification({
@@ -1076,7 +1077,7 @@
            relation.destroy();
          });
          service.destroy();
- db.fire('update');
+ topo.update();
        }
        this.get('destroy_dialog').hide();
        btn.set('disabled', false);

« Back to merge proposal