Upgrade Service implementation

Upgrade Service implementation

Adds the ability to upgrade a service per the UX mocks.

This is an incremental branch.  This does not include:

* Updating the inspector after clicking upgrade (fetching the service again, even after the RPC has returned, shows the old version in the service)
* Paginating long lists of downgrades (diff was getting too long)
* Any way of notifying pyjuju users that they can't upgrade via the UI, except an error notification (env isn't available to check when we build the inspector)

https://code.launchpad.net/~makyo/juju-gui/upgrade-charm-1100904-2/+merge/182793

(do not edit description out of merge proposal)

[matthew.scott, 2013-08-29 01:19:52]

Please take a look.

[jeff.pihach, 2013-08-29 13:56:24]

This is coming together nicely!
LGTM with some cleanup and a slight change wrt the upgradeTo data location in
inspector.js

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js
File app/config-debug.js (right):

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js#newcode45
app/config-debug.js:45: sandbox: false,
I'm guessing these weren't supposed to be proposed?

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js#newcode117
app/store/charm.js:117: console.log(charmID);
remove

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode887
app/views/inspector.js:887: upgradeTo = ev.currentTarget.getData('upgradeto');
I feel like this should be stored on the service not on the element you click to
upgrade.

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode912
app/views/inspector.js:912: // report new charm URL properly with GetService.
ticket #?

https://codereview.appspot.com/13234044/diff/1/lib/views/juju-inspector.less
File lib/views/juju-inspector.less (right):

https://codereview.appspot.com/13234044/diff/1/lib/views/juju-inspector.less#...
lib/views/juju-inspector.less:760: .status-unit-header.upgrade-service {
no sprite?

[matthew.scott, 2013-08-29 14:05:12]

Just some replies for now, will repropose withchanges in a bit.

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js
File app/config-debug.js (right):

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js#newcode45
app/config-debug.js:45: sandbox: false,
On 2013/08/29 13:56:24, jeff.pihach wrote:
> I'm guessing these weren't supposed to be proposed?

ARGH, yes, sorry :(

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode887
app/views/inspector.js:887: upgradeTo = ev.currentTarget.getData('upgradeto');
On 2013/08/29 13:56:24, jeff.pihach wrote:
> I feel like this should be stored on the service not on the element you click
to
> upgrade.

The upgrade_to attribute on the service stores the HEAD revision if the service
is not up to date.  This upgradeTo (admittedly poorly named, will rename to
setCharmTo) reflects a possible downgrade. Given that there are n_versions - 1
other charms a user can set for a service, and thus n_versions - 1 other links
the user could click; this seemed more efficient.  Let me know if this clears
things up; if you still disagree, I'll see what I can do.

https://codereview.appspot.com/13234044/diff/1/lib/views/juju-inspector.less
File lib/views/juju-inspector.less (right):

https://codereview.appspot.com/13234044/diff/1/lib/views/juju-inspector.less#...
lib/views/juju-inspector.less:760: .status-unit-header.upgrade-service {
On 2013/08/29 13:56:24, jeff.pihach wrote:
> no sprite?

None of the others are sprited, so I figured that was the direction we were
heading.  I propose deferring until we can sprite them all, at least in this
file (there are several).

[matthew.scott, 2013-08-29 15:13:07]

Thanks for the review!

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js#newcode117
app/store/charm.js:117: console.log(charmID);
On 2013/08/29 13:56:24, jeff.pihach wrote:
> remove

Done.

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode912
app/views/inspector.js:912: // report new charm URL properly with GetService.
On 2013/08/29 13:56:24, jeff.pihach wrote:
> ticket #?

Done.

[matthew.scott, 2013-08-29 15:15:41]

Please take a look.

[rharding, 2013-08-29 15:16:09]

LGTM with notes below. Ignore my category question stuff. The code's a bit
confusing with category vs status and such. It initially looked like you were
creating the category idea, but upon doing farther it goes back a ways.

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js
File app/config-debug.js (right):

https://codereview.appspot.com/13234044/diff/1/app/config-debug.js#newcode45
app/config-debug.js:45: sandbox: false,
revert the config changes

https://codereview.appspot.com/13234044/diff/1/app/models/model-controller.js
File app/models/model-controller.js (right):

https://codereview.appspot.com/13234044/diff/1/app/models/model-controller.js...
app/models/model-controller.js:164: service.set('upgrade_to', 'cs:' + latestId);
is this always going to be true? We have charm.get('scheme') above, can we pull
that back in. That way if the if() check is adjusted in the future we don't need
to update the prefix here.

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js
File app/store/charm.js (right):

https://codereview.appspot.com/13234044/diff/1/app/store/charm.js#newcode117
app/store/charm.js:117: console.log(charmID);
remove debug

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode887
app/views/inspector.js:887: upgradeTo = ev.currentTarget.getData('upgradeto');
does getData in YUI pull data-? I thought it only pulled if it was set using
setData on the dom object and that you needed to getAttribute('data-..')?

https://codereview.appspot.com/13234044/diff/1/app/views/viewlets/service-ove...
File app/views/viewlets/service-overview.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/viewlets/service-ove...
app/views/viewlets/service-overview.js:41: @method categoryName
What category here? This isn't the charm models category data. It seems to be a
statusName method?

https://codereview.appspot.com/13234044/diff/1/test/test_inspector_overview.js
File test/test_inspector_overview.js (right):

https://codereview.appspot.com/13234044/diff/1/test/test_inspector_overview.j...
test/test_inspector_overview.js:504: it('upgrades services', function(done) {
attempts to upgrade on click

[matthew.scott, 2013-08-29 16:24:02]

Thanks for the reviews

https://codereview.appspot.com/13234044/diff/1/app/models/model-controller.js
File app/models/model-controller.js (right):

https://codereview.appspot.com/13234044/diff/1/app/models/model-controller.js...
app/models/model-controller.js:164: service.set('upgrade_to', 'cs:' + latestId);
On 2013/08/29 15:16:09, rharding wrote:
> is this always going to be true? We have charm.get('scheme') above, can we
pull
> that back in. That way if the if() check is adjusted in the future we don't
need
> to update the prefix here.

Done.

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13234044/diff/1/app/views/inspector.js#newcode887
app/views/inspector.js:887: upgradeTo = ev.currentTarget.getData('upgradeto');
On 2013/08/29 15:16:09, rharding wrote:
> does getData in YUI pull data-? I thought it only pulled if it was set using
> setData on the dom object and that you needed to getAttribute('data-..')?

I checked the docs before. It first tries to pull arbitrary data from the Y.Node
instance if it exists.  If not, it falls back to node.getAttribute('data-' +
key).

[matthew.scott, 2013-08-29 17:10:55]

*** Submitted:

Upgrade Service implementation

Adds the ability to upgrade a service per the UX mocks.

This is an incremental branch.  This does not include:

* Updating the inspector after clicking upgrade (fetching the service again,
even after the RPC has returned, shows the old version in the service)
* Paginating long lists of downgrades (diff was getting too long)
* Any way of notifying pyjuju users that they can't upgrade via the UI, except
an error notification (env isn't available to check when we build the inspector)

R=jeff.pihach, rharding
CC=
https://codereview.appspot.com/13234044