Move treestatus.js to a model class.

Move treestatus.js to a model class.

This converts the treestatus JS file into a model HTML file.

BUG=400392
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179854

[dsinclair, 2014-08-06 20:59:12]

PTAL.

[ojan, 2014-08-06 22:34:17]

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/tree-status.html (right):

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/tree-status.html:8: function TreeStatus() {
Lets pass in the project name as a constructor on this element. That way
urlByName doesn't need to take it as an argument and can just be renamed to url,
which means ct-tree-status doesn't need to take a project attribute either.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/tree-status.html:19:
TreeStatus.prototype.fetchTreeStatus = function(statusURL, resultSpan) {
This method appears to be unused. Delete it? I think it's just a leftover from
garden-o-matic I forgot to delete.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-tree-status.html (right):

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-tree-status.html:54: update: function() {
Now that we have a proper model class, lets get this component out of the
business of doing network requests. We're trying to move towards a design where
the model is the thing that does the network request. So, ct-unexpected-failures
will create the TreeStatus model instance and pass it in as a property on this
element. Then ct-unexpected-failures won't call update on this element at all.
It'll call update on the model, which will propagate down to here through data
binding magic.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-tree-status.html:61: updateStatus: function(status)
{
Can you move most of this logic into the model class?

[dsinclair, 2014-08-07 18:09:23]

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/tree-status.html (right):

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/tree-status.html:8: function TreeStatus() {
On 2014/08/06 22:34:17, ojan-only-code-yellow-reviews wrote:
> Lets pass in the project name as a constructor on this element. That way
> urlByName doesn't need to take it as an argument and can just be renamed to
url,
> which means ct-tree-status doesn't need to take a project attribute either.

Done.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/tree-status.html:19:
TreeStatus.prototype.fetchTreeStatus = function(statusURL, resultSpan) {
On 2014/08/06 22:34:17, ojan-only-code-yellow-reviews wrote:
> This method appears to be unused. Delete it? I think it's just a leftover from
> garden-o-matic I forgot to delete.

Done.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-tree-status.html (right):

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-tree-status.html:54: update: function() {
On 2014/08/06 22:34:17, ojan-only-code-yellow-reviews wrote:
> Now that we have a proper model class, lets get this component out of the
> business of doing network requests. We're trying to move towards a design
where
> the model is the thing that does the network request. So,
ct-unexpected-failures
> will create the TreeStatus model instance and pass it in as a property on this
> element. Then ct-unexpected-failures won't call update on this element at all.
> It'll call update on the model, which will propagate down to here through data
> binding magic.

Done.

https://codereview.chromium.org/443243002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-tree-status.html:61: updateStatus: function(status)
{
On 2014/08/06 22:34:17, ojan-only-code-yellow-reviews wrote:
> Can you move most of this logic into the model class?

Done.

[ojan, 2014-08-07 18:30:49]

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
File Tools/GardeningServer/model/tree-status.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
Tools/GardeningServer/model/tree-status.html:11: this.status = {
this.status = 'unknown';

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
Tools/GardeningServer/model/tree-status.html:17: TreeStatus.prototype.url =
function() {
Now that I look at this, there's no real reason for this to be a function
anymore. Lets just put the following line in the constructor and make this a
property on the TreeStatus:
this.url = "http://{1}-status.appspot.com".assign(project);

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-tree-status.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-tree-status.html:9: <polymer-element
name="ct-tree-status" attributes="treeStatus">
Nit: how about just calling the attribute "status"?

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (left):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:18: ct-tree-status {
This styling was intentionally out here. You could easily imagine a different
app reusing the ct-tree-status component and wanting to style it differently
(e.g. not clip and instead just show the whole status).

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:19: <div
id="ctTreeStatuses"></div>
I don't think you need the domReady business if you make the treeStatuses an
object instead of an array.

<ct-tree-status status="{{ treeStatuses["chromium"] }}></ct-tree-status>
<ct-tree-status status="{{ treeStatuses["blink"] }}></ct-tree-status>

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:59: for (var i = 0; i <
this.treeStatuses.length; i++)
If treeStatuses is an object, you can still iterate. sugarjs has a nice iterator
for objects:
Object.keys(this.treeStatuses, function(tree, status) { ... });

[dsinclair, 2014-08-08 13:43:47]

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
File Tools/GardeningServer/model/tree-status.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
Tools/GardeningServer/model/tree-status.html:11: this.status = {
On 2014/08/07 18:30:48, ojan-only-code-yellow-reviews wrote:
> this.status = 'unknown';

Done.

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/mo...
Tools/GardeningServer/model/tree-status.html:17: TreeStatus.prototype.url =
function() {
On 2014/08/07 18:30:48, ojan-only-code-yellow-reviews wrote:
> Now that I look at this, there's no real reason for this to be a function
> anymore. Lets just put the following line in the constructor and make this a
> property on the TreeStatus:
> this.url = "http://{1}-status.appspot.com".assign(project);

Done.

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-tree-status.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-tree-status.html:9: <polymer-element
name="ct-tree-status" attributes="treeStatus">
On 2014/08/07 18:30:48, ojan-only-code-yellow-reviews wrote:
> Nit: how about just calling the attribute "status"?

Done.

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (left):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:18: ct-tree-status {
On 2014/08/07 18:30:49, ojan-only-code-yellow-reviews wrote:
> This styling was intentionally out here. You could easily imagine a different
> app reusing the ct-tree-status component and wanting to style it differently
> (e.g. not clip and instead just show the whole status).

Done.

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:19: <div
id="ctTreeStatuses"></div>
On 2014/08/07 18:30:49, ojan-only-code-yellow-reviews wrote:
> I don't think you need the domReady business if you make the treeStatuses an
> object instead of an array.
> 
> <ct-tree-status status="{{ treeStatuses["chromium"] }}></ct-tree-status>
> <ct-tree-status status="{{ treeStatuses["blink"] }}></ct-tree-status>

Done.

https://codereview.chromium.org/443243002/diff/40001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-unexpected-failures.html:59: for (var i = 0; i <
this.treeStatuses.length; i++)
On 2014/08/07 18:30:48, ojan-only-code-yellow-reviews wrote:
> If treeStatuses is an object, you can still iterate. sugarjs has a nice
iterator
> for objects:
> Object.keys(this.treeStatuses, function(tree, status) { ... });

Done.

[ojan, 2014-08-08 18:26:16]

lgtm

[commit-bot: I haz the power, 2014-08-08 18:26:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dsinclair@chromium.org/443243002/80001

[commit-bot: I haz the power, 2014-08-08 18:30:00]

Change committed as 179854