Search for available upgrades for services

Search for available upgrades for services

This searches the charm store for available upgrades to services when they're loaded.  If an upgrade is available, the service is marked as such, and the most recent charm id is added as an attribute, providing a quick means to link to a charm in the store.  This is under the feature flag `upgradeCharm` as part of the same work, so QA with /:flags:/upgradeCharm and inspect the models in the db - you'll have to use improv or a deployment of an old charm.

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

(do not edit description out of merge proposal)

[matthew.scott, 2013-08-20 23:21:52]

Please take a look.

[rharding, 2013-08-21 16:14:50]

Per our call then I'll wait for a final ok based on where the updates head. 

Comments are below. Per the call, I think it'd be great if the store had the
logic of detecting if an update is available, or figuring out the latest version
of a charm since it seems to fit into the store's responsibilities more and
should make for cleaner testing lines. Cleaner testing that we can find updated
charms in the store, and a that given that, the model-controller can respect
that.

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

https://codereview.appspot.com/12744049/diff/1/app/models/charm.js#newcode33
app/models/charm.js:33: var charmIdRe =
/^(?:(\w+):)?(?:~(\S+)\/)?(\w+)\/(\S+?)-(\d+|HEAD)$/;
thanks for this update. Missed that one.

https://codereview.appspot.com/12744049/diff/1/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/12744049/diff/1/app/models/models.js#newcode199
app/models/models.js:199: upgrade_available: {
docs strings on the attrs?

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js
File test/test_model_controller.js (right):

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js#...
test/test_model_controller.js:250: function(done) {
does this not fit on the previous line?

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js#...
test/test_model_controller.js:260: window.flags.upgradeCharm = true;
we need to clean this up. It's not part of afterEach.

[jeff.pihach, 2013-08-21 16:41:14]

LGTM with trivial and QA OK!

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js
File test/test_model_controller.js (right):

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js#...
test/test_model_controller.js:260: window.flags.upgradeCharm = true;
should this not be reset in the after?

[matthew.scott, 2013-08-21 23:27:08]

I've moved some of the upgrade-available logic to the store.  Reproposing.

https://codereview.appspot.com/12744049/diff/1/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/12744049/diff/1/app/models/models.js#newcode199
app/models/models.js:199: upgrade_available: {
On 2013/08/21 16:14:51, rharding wrote:
> docs strings on the attrs? 

Done.

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js
File test/test_model_controller.js (right):

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js#...
test/test_model_controller.js:250: function(done) {
On 2013/08/21 16:14:51, rharding wrote:
> does this not fit on the previous line?

Done.

https://codereview.appspot.com/12744049/diff/1/test/test_model_controller.js#...
test/test_model_controller.js:260: window.flags.upgradeCharm = true;
On 2013/08/21 16:14:51, rharding wrote:
> we need to clean this up. It's not part of afterEach. 

Done.

[matthew.scott, 2013-08-21 23:28:49]

Please take a look.

[rharding, 2013-08-22 00:03:40]

LGTM, one note below and wonder if a test to the store for the new method makes
sense?

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

https://codereview.appspot.com/12744049/diff/9001/app/config-debug.js#newcode44
app/config-debug.js:44: sandbox: false,
did you mean to commit this?

[matthew.scott, 2013-08-22 14:58:56]

Thanks for the reviews.

On 2013/08/22 00:03:40, rharding wrote:
> LGTM, one note below and wonder if a test to the store for the new method
makes
> sense?

Added some tests more directly around the added promise.

> 
> https://codereview.appspot.com/12744049/diff/9001/app/config-debug.js
> File app/config-debug.js (right):
> 
>
https://codereview.appspot.com/12744049/diff/9001/app/config-debug.js#newcode44
> app/config-debug.js:44: sandbox: false,
> did you mean to commit this?

Oops, fixed.

[matthew.scott, 2013-08-22 15:09:30]

*** Submitted:

Search for available upgrades for services

This searches the charm store for available upgrades to services when they're
loaded.  If an upgrade is available, the service is marked as such, and the most
recent charm id is added as an attribute, providing a quick means to link to a
charm in the store.  This is under the feature flag `upgradeCharm` as part of
the same work, so QA with /:flags:/upgradeCharm and inspect the models in the db
- you'll have to use improv or a deployment of an old charm.

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