Add support for loading QA data into charm detail

Add support for loading QA data into charm detail

- Update the store with the api call to the /qa.
- Add support to the charm detail view to load qa data when the Quality tab is
selected.
- Add code to summarize the scoring for the template to make life easy for it.
- Add tests for the scoring summary and clicking the quality tab.

https://code.launchpad.net/~rharding/juju-gui/qa_tab/+merge/156392

(do not edit description out of merge proposal)

[rharding, 2013-04-01 19:34:39]

Please take a look.

[jeff.pihach, 2013-04-01 19:49:23]

LGTM Thanks - with trivial.

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/templates/b...
File app/subapps/browser/templates/browser_qa.handlebars (right):

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/templates/b...
app/subapps/browser/templates/browser_qa.handlebars:7: <h3>{{this.description}}
<span class="heading_notes">{{this.score}}/{{this.questions.length}}
points</span></h3>
semantic pedantic: should a description not be in a <p> instead of a heading?

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/views/charm...
app/subapps/browser/views/charm.js:104: tab.after('selectionChange',
function(ev) {
do we want to destroy this event handler at any time? Or are we not worried
about it in this context?

https://codereview.appspot.com/8171047/diff/1/test/data/qa.json
File test/data/qa.json (right):

https://codereview.appspot.com/8171047/diff/1/test/data/qa.json#newcode1
test/data/qa.json:1: {
If/when this data gets stale it would be nice if there was some documentation
somewhere which says where it's from and how to generate new.

[rharding, 2013-04-01 20:06:51]

Please take a look.

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/templates/b...
File app/subapps/browser/templates/browser_qa.handlebars (right):

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/templates/b...
app/subapps/browser/templates/browser_qa.handlebars:7: <h3>{{this.description}}
<span class="heading_notes">{{this.score}}/{{this.questions.length}}
points</span></h3>
On 2013/04/01 19:49:23, jeff.pihach wrote:
> semantic pedantic: should a description not be in a <p> instead of a heading?

The description is really just a formatted .name property. "quality_assessment"
is the name but "Quality Assessment" is the description.

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8171047/diff/1/app/subapps/browser/views/charm...
app/subapps/browser/views/charm.js:104: tab.after('selectionChange',
function(ev) {
On 2013/04/01 19:49:23, jeff.pihach wrote:
> do we want to destroy this event handler at any time? Or are we not worried
> about it in this context?

Sure, I was thinking the tabview is destroyed, but you're right that this event
won't be detached. Updated.

[rharding, 2013-04-01 20:08:25]

Comments below, updated code coming.

https://codereview.appspot.com/8171047/diff/1/test/data/qa.json
File test/data/qa.json (right):

https://codereview.appspot.com/8171047/diff/1/test/data/qa.json#newcode1
test/data/qa.json:1: {
On 2013/04/01 19:49:23, jeff.pihach wrote:
> If/when this data gets stale it would be nice if there was some documentation
> somewhere which says where it's from and how to generate new.

I can't drop a comment above the JSON or it won't be valid. I've added a new
"_comment": "....." attribute with a demo url from staging to fetch the content.
It'll have to be manually updated/maintained. It's the best I can do.

[j.c.sackett, 2013-04-01 20:57:39]

Looks good, some questions below.

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/template...
File app/subapps/browser/templates/browser_qa.handlebars (right):

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/template...
app/subapps/browser/templates/browser_qa.handlebars:7: <h3>{{this.description}}
<span class="heading_notes">{{this.score}}/{{this.questions.length}}
points</span></h3>
Can't you skip the "this" on these? I think within a hashblock, you can simply
do the attr name, so {{ description }} instead of {{ this.description }}

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/views/ch...
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/views/ch...
app/subapps/browser/views/charm.js:119: break;
As long as you're putting in places to flesh out, is there a reason "Hooks" and
Readme aren't here? Since they're both part of the tabs, and at least hooks will
need a function of some sort to set everything up.

https://codereview.appspot.com/8171047/diff/2002/test/test_browser_charm_deta...
File test/test_browser_charm_details.js (right):

https://codereview.appspot.com/8171047/diff/2002/test/test_browser_charm_deta...
test/test_browser_charm_details.js:237: assert(1);
What's the point of this? If there's no assert, you'll still a test passing, and
this will always pass, won't it?

[rharding, 2013-04-01 23:27:25]

Comments replied to. Updated code coming next.

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/template...
File app/subapps/browser/templates/browser_qa.handlebars (right):

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/template...
app/subapps/browser/templates/browser_qa.handlebars:7: <h3>{{this.description}}
<span class="heading_notes">{{this.score}}/{{this.questions.length}}
points</span></h3>
On 2013/04/01 20:57:39, j.c.sackett wrote:
> Can't you skip the "this" on these? I think within a hashblock, you can simply
> do the attr name, so {{ description }} instead of {{ this.description }}

Yes, I can. removed.

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/views/ch...
File app/subapps/browser/views/charm.js (right):

https://codereview.appspot.com/8171047/diff/2002/app/subapps/browser/views/ch...
app/subapps/browser/views/charm.js:119: break;
On 2013/04/01 20:57:39, j.c.sackett wrote:
> As long as you're putting in places to flesh out, is there a reason "Hooks"
and
> Readme aren't here? Since they're both part of the tabs, and at least hooks
will
> need a function of some sort to set everything up.

This is only going to be for tabs that need extra work on click to load
initially. Some tabs, like the hooks one, do not need to catch/load on click.
They're already set. 

The readme is handled on initial load since it's the default view.

https://codereview.appspot.com/8171047/diff/2002/test/test_browser_charm_deta...
File test/test_browser_charm_details.js (right):

https://codereview.appspot.com/8171047/diff/2002/test/test_browser_charm_deta...
test/test_browser_charm_details.js:237: assert(1);
On 2013/04/01 20:57:39, j.c.sackett wrote:
> What's the point of this? If there's no assert, you'll still a test passing,
and
> this will always pass, won't it?

This is just me feeling dirty with a test without any assert. If it fails to hit
the callback, the point of the test, it's just a timeout error. I just added
this line noise as a pointer to "look here...this is where the key point of the
test is". 

If it's preferred I can remove it. It just made sense for clarity in my mind,
but might not for others.

[rharding, 2013-04-01 23:29:43]

Please take a look.

[j.c.sackett, 2013-04-02 14:32:40]

LGTM.

As to the always passing assert, I would just ask that you put a comment in
there, or down the road someone's going to assume its wrong and try to figure
out what it's supposed to be testing.

[rharding, 2013-04-02 15:59:17]

*** Submitted:

Add support for loading QA data into charm detail

- Update the store with the api call to the /qa.
- Add support to the charm detail view to load qa data when the Quality tab is
selected.
- Add code to summarize the scoring for the template to make life easy for it.
- Add tests for the scoring summary and clicking the quality tab.

R=jeff.pihach, j.c.sackett
CC=
https://codereview.appspot.com/8171047