Merge lp://staging/~gary/juju-gui/bug1228166 into lp://staging/juju-gui/experimental

Reviewers: mp+187311_code.launchpad.net,

Message:
Please take a look.

Description:
Data-bind unit details view

Our unit detail view was not data-bound: it would render and never
update. This branch fixes that, except for the relation information,
which is not really available at the moment (in terms of errors) so I
decided that could live to fight another day.

To qa, deploy a service, turn off the simulator (to avoid surprises),
open the unit view of unit 0, and try the following in the console,
looking on in the unit details view as you do.

myunit = app.db.units.item(0)
myunit.private_address = '10.0.0.1'
myunit.public_address = '10.0.0.2'
myunit.open_ports = [80]
myunit.open_ports = [8080]
myunit.open_ports = [443]
myunit.open_ports = [443, 8080]
myunit.agent_state_info = 'great!'
app.db.environment.set('annotations', {'landscape-url':
'http://landscape.example.com','landscape-computers':
'/computers/criteria/environment:test'})
myunit.annotations = {'landscape-computer': 'kumquat'}

(note that the app.dp.environment.set call does not trigger a change in
the view: it sets up data for the next mutation, which does.)

Thank you!

https://code.launchpad.net/~gary/juju-gui/bug1228166/+merge/187311

(do not edit description out of merge proposal)

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

Affected files (+235, -55 lines):
   A [revision details]
   M app/templates/unitOverview.handlebars
   M app/views/viewlet-manager.js
   M app/views/viewlets/unit-details.js
   M test/test_unit_detail_viewlet.js

Code LGTM with trivial and will QA and report back.

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js
File app/views/viewlets/unit-details.js (right):

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode40
app/views/viewlets/unit-details.js:40: if (value) {
if everything requires a value I would do:

if (!value) { return; }

just to avoid the indentation throughout the whole function (personal
preference)

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode89
app/views/viewlets/unit-details.js:89: 'get': function(model) {
What is this for? Where is it used?

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js
File test/test_unit_detail_viewlet.js (right):

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js#newcode67
test/test_unit_detail_viewlet.js:67:
assert.strictEqual(node.getContent(), '');
getContent() is deprecated in favour of getHTML()

https://codereview.appspot.com/13841044/

QA NOT OK - trivial

ip is not a link unless ports are visible
the resulting links afterwards do not match the ports you click on, and
the ip has no port added.

https://codereview.appspot.com/13841044/

QA OK after discussion Thanks

https://codereview.appspot.com/13841044/

On 2013/09/24 18:37:48, jeff.pihach wrote:
> Code LGTM with trivial and will QA and report back.

Thank you very much for review and qa!

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js
> File app/views/viewlets/unit-details.js (right):

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode40
> app/views/viewlets/unit-details.js:40: if (value) {
> if everything requires a value I would do:

> if (!value) { return; }

> just to avoid the indentation throughout the whole function (personal
> preference)

Yeah, I dunno. I changed it to match your preference this time. :-)

https://codereview.appspot.com/13841044/diff/1/app/views/viewlets/unit-details.js#newcode89
> app/views/viewlets/unit-details.js:89: 'get': function(model) {
> What is this for? Where is it used?

We discussed, and with Ben decided to move the functionality into
databinding and remove the extension feature I was using.

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js
> File test/test_unit_detail_viewlet.js (right):

https://codereview.appspot.com/13841044/diff/1/test/test_unit_detail_viewlet.js#newcode67
> test/test_unit_detail_viewlet.js:67:
assert.strictEqual(node.getContent(), '');
> getContent() is deprecated in favour of getHTML()

Ah! Thanks, changed.

https://codereview.appspot.com/13841044/

*** Submitted:

Data-bind unit details view

Our unit detail view was not data-bound: it would render and never
update. This branch fixes that, except for the relation information,
which is not really available at the moment (in terms of errors) so I
decided that could live to fight another day.

R=jeff.pihach
CC=
https://codereview.appspot.com/13841044

https://codereview.appspot.com/13841044/