Code review comment for lp:~makyo/juju-gui/coords

Revision history for this message
Madison Scott-Clary (makyo) wrote :

Reviewers: mp+191034_code.launchpad.net,

Message:
Please take a look.

Description:
Fix initial coordinates for annotated services

As services with annotations have those annotations removed after
placement, further updates to the DB were causing them to be treated as
new services in a short-circuit || check. They were initially placed at
the annotated positions, then re-positioned as default new services on
any further updates to the database (e.g.: GetCharm, etc.). This treats
them as old services as soon as they've been received by the first delta
with their annotations intact by utilizing the already-existing
hasBeenPositioned flag which bypasses that check; no more
auto-positioning. Asserts added to the appropriate test.

https://code.launchpad.net/~makyo/juju-gui/coords/+merge/191034

(do not edit description out of merge proposal)

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

Affected files (+9, -0 lines):
   A [revision details]
   M app/views/topology/service.js
   M test/test_environment_view.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-10-07 19:38:23 +0000
+++ test/test_environment_view.js 2013-10-14 19:00:03 +0000
@@ -637,8 +637,11 @@
              }
            ]]
        };
+ db.reset();
        db.onDelta({ data: tmp_data });
        view.update();
+
assert.equal(db.services.getById('wordpressa').get('hasBeenPositioned'),
+ true);

        view.topo.servicePointOutside = function() {
          assert(false,
@@ -655,6 +658,7 @@
              }
            ]]
        };
+ db.reset();
        db.onDelta({ data: tmp_data });
        db.services.getById('wordpressb').set('hasBeenPositioned', true);
        view.update();

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-10-11 20:13:50 +0000
+++ app/views/topology/service.js 2013-10-14 19:07:38 +0000
@@ -110,6 +110,9 @@
          if (!d.inDrag) {
            var useTransitions = self.get('useTransitions') && !fromGhost;
            self.drag.call(this, d, self, {x: x, y: y}, useTransitions);
+ // When selecting new services in update this value allows the
+ // filter function to skip over annotated services.
+ d.model.set('hasBeenPositioned', true);
          }
        }});

« Back to merge proposal