Code review comment for lp://staging/~gary/juju-gui/bug1157138

Revision history for this message
Gary Poster (gary) wrote :

Reviewers: mp+154099_code.launchpad.net,

Message:
Please take a look.

Description:
Fix bug 1157138

Make service displayName dynamic so that newly created services
accurately change their display when transitioning from ghost to real.

https://code.launchpad.net/~gary/juju-gui/bug1157138/+merge/154099

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/models/models.js
   M test/test_fakebackend.js
   M test/test_model.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_fakebackend.js
=== modified file 'test/test_fakebackend.js'
--- test/test_fakebackend.js 2013-03-12 13:50:44 +0000
+++ test/test_fakebackend.js 2013-03-19 14:14:15 +0000
@@ -100,15 +100,19 @@
        assert.isTrue(Y.Lang.isString(attrs.clientId));
        delete attrs.clientId;
        assert.deepEqual(attrs, {
+ aggregated_status: undefined,
          charm: 'cs:precise/wordpress-10',
          config: undefined,
+ constraints: undefined,
          destroyed: false,
+ displayName: 'wordpress',
          exposed: false,
          id: 'wordpress',
          initialized: true,
          name: 'wordpress',
- displayName: 'wordpress',
- subordinate: undefined
+ pending: false,
+ subordinate: false,
+ unit_count: undefined
        });
        var units = fakebackend.db.units.get_units_for_service(service);
        assert.lengthOf(units, 1);

Index: test/test_model.js
=== modified file 'test/test_model.js'
--- test/test_model.js 2013-03-08 18:57:11 +0000
+++ test/test_model.js 2013-03-19 14:19:14 +0000
@@ -360,9 +360,17 @@
    });

    it('returns a display name for a service', function() {
- var services = new models.ServiceList();
- assert.equal('mysql', services.createDisplayName('service-mysql'));
- assert.equal('mysql', services.createDisplayName('mysql'));
+ var service = new models.Service({id: 'mysql', exposed: false});
+ assert.equal('mysql', service.get('displayName'));
+ service = new models.Service({id: 'service-mysql', exposed: false});
+ assert.equal('mysql', service.get('displayName'));
+ });
+
+ it('updates the display name when the id changes', function() {
+ var service = new models.Service({id: 'service-mysql', exposed:
false});
+ assert.equal('mysql', service.get('displayName'));
+ service.set('id', 'service-flibbertigibbet');
+ assert.equal('flibbertigibbet', service.get('displayName'));
    });

    it('returns a display name for a unit', function() {

Index: app/models/models.js
=== modified file 'app/models/models.js'
--- app/models/models.js 2013-03-11 17:44:48 +0000
+++ app/models/models.js 2013-03-19 14:19:14 +0000
@@ -77,9 +77,17 @@
    });
    models.Environment = Environment;

- var Service = Y.Base.create('service', Y.Model, [], {
+ var Service = Y.Base.create('service', Y.Model, [], {}, {
      ATTRS: {
- displayName: {},
+ displayName: {
+ /**
+ Dynamically calculate a display name that accounts for Juju Core
name
+ prefixes.
+ */
+ getter: function() {
+ return this.get('id').replace('service-', '');
+ }
+ },
        name: {},
        charm: {},
        config: {},
@@ -101,31 +109,6 @@

    var ServiceList = Y.Base.create('serviceList', Y.ModelList, [], {
      model: Service,
- /**
- * Create a display name that can be used in the views as an entity
label
- * agnostic from juju type.
- *
- * @method createDisplayName
- * @param {String} name The name to modify.
- * @return {String} A display name.
- */
- createDisplayName: function(name) {
- return name.replace('service-', '');
- },
-
- _setDefaultsAndCalculatedValues: function(obj) {
- obj.set('displayName', this.createDisplayName(obj.get('id')));
- },
-
- add: function() {
- var result = ServiceUnitList.superclass.add.apply(this, arguments);
- if (Y.Lang.isArray(result)) {
- Y.Array.each(result, this._setDefaultsAndCalculatedValues, this);
- } else {
- this._setDefaultsAndCalculatedValues(result);
- }
- return result;
- },

      process_delta: function(action, data) {
        _process_delta(this, action, data, {exposed: false});

« Back to merge proposal