Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create vendor agnostic JSON format between issue data sources and consumers #74

Merged
merged 14 commits into from
Oct 19, 2016
3 changes: 3 additions & 0 deletions client/chromez-behaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,8 @@ ChromeZBehaviors.AJAXBehavior = {
}

function clone(data) {
if (data instanceof Object && data.clone instanceof Function) {
return data.clone();
}
return JSON.parse(JSON.stringify(data));
}
4 changes: 2 additions & 2 deletions client/crbug-lib.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(function() {

if (window.CrBug) {
if (window.Crbug) {
return;
}

Expand All @@ -12,7 +12,7 @@ function queryURL(query) {
return 'https://bugs.chromium.org/p/chromium/issues/list?can=2&q=' + encodeURIComponent(queryString(query));
}

window.CrBug = {
window.Crbug = {
queryString,
queryURL,
};
Expand Down
29 changes: 14 additions & 15 deletions client/cz-component-dash.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
<paper-item-body two-line>
<div>Owned by us</div>
<div secondary class='component'>
<template is="dom-repeat" items="{{issues(component.teamsIssues, updateSLO)}}">
<template is="dom-repeat" items="{{summary(component.teamsIssueList, updateSLO)}}">
<a href="{{crbugLink(component.name, item.query)}}">
<span class="bubble" style="background-color: {{item.color}}">{{item.level}}</span>
<span class="summary">{{item.summary}}</span>
Expand All @@ -66,7 +66,7 @@
<paper-item-body two-line>
<div>Owned by others</div>
<div secondary class='component'>
<template is="dom-repeat" items="{{issues(component.othersIssues, updateSLO)}}">
<template is="dom-repeat" items="{{summary(component.othersIssueList, updateSLO)}}">
<a href="{{crbugLink(component.name, item.query)}}">
<span class="bubble" style="background-color: {{item.color}}">{{item.level}}</span>
<span class="summary">{{item.summary}}</span>
Expand All @@ -80,7 +80,7 @@
<paper-item-body two-line>
<div>Unowned</div>
<div secondary class='component'>
<template is="dom-repeat" items="{{issues(component.unownedIssues, updateSLO)}}">
<template is="dom-repeat" items="{{summary(component.unownedIssueList, updateSLO)}}">
<a href="{{crbugLink(component.name, item.query)}}">
<span class="bubble" style="background-color: {{item.color}}">{{item.level}}</span>
<span class="summary">{{item.summary}}</span>
Expand Down Expand Up @@ -127,35 +127,34 @@
}.bind(this));
},

issues: function(issues, updateSLO) {
return issues.summary(updateSLO);
summary: function(issueList, updateSLO) {
return issueList.summary(updateSLO);
},

updateComponentIssues: function(component) {
registerSource('cz-issues', {component: component.name}, function(data) {
registerSource('cz-crbug-issues', {component: component.name}, function(issueList) {
var teamsIssueList = new IssueList();
var othersIssueList = new IssueList();
var unownedIssueList = new IssueList();
for (var i = 0; i < data.length; i++) {
var issue = new Issue(data[i]);
for (var issue of issueList) {
if (issue.owner == null) {
unownedIssueList.append(issue);
unownedIssueList.push(issue);
} else if (this.users.indexOf(issue.owner) === -1) {
othersIssueList.append(issue);
othersIssueList.push(issue);
} else {
teamsIssueList.append(issue);
teamsIssueList.push(issue);
}
}
this.set('component.teamsIssues', teamsIssueList);
this.set('component.othersIssues', othersIssueList);
this.set('component.unownedIssues', unownedIssueList);
this.set('component.teamsIssueList', teamsIssueList);
this.set('component.othersIssueList', othersIssueList);
this.set('component.unownedIssueList', unownedIssueList);
}.bind(this));
},

crbugLink: function(component, query) {
var componentQuery = clone(query || {});
componentQuery.component = component;
return CrBug.queryURL(componentQuery);
return Crbug.queryURL(componentQuery);
},
});
</script>
Expand Down
18 changes: 13 additions & 5 deletions client/cz-issues.html → client/cz-crbug-issues.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
<link rel="import" href="../bower_components/iron-ajax/iron-ajax.html">
<script src="../bower_components/papaparse/papaparse.min.js"></script>
<script src="chromez-behaviors.js"></script>
<script src="crbug-lib.js"></script>
<script src="issues-lib.js"></script>

<dom-module id="cz-issues">
<dom-module id="cz-crbug-issues">

<template>
<template is="dom-repeat" items="{{searchQueries}}">
Expand All @@ -21,20 +21,28 @@

<script>
Polymer({
is: "cz-issues",
is: "cz-crbug-issues",
behaviors: [ChromeZBehaviors.AJAXBehavior],

onResponse: function(data) {
data = JSON.parse(data);
if (data.items == undefined)
return [];
return data.items.filter(a => a.ID !== "");
return new IssueList(data.items.filter(a => a.ID !== "").map(item => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the crbug-specific constructor into the issue library?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issues-lib is supposed to be vendor agnostic code, I don't think it makes sense to put Crbug specific code in there. This function performs the transformation from Crbug issue data into generic issue data. Would you prefer it live in crbug-lib.js?

return new Issue({
id: item.id,
owner: item.owner ? item.owner.name : null,
summary: item.summary,
lastUpdatedString: item.updated,
labels: item.labels,
});
}));
},

registerQuery: function(query, callback) {
var params = {
site: 'issues',
q: CrBug.queryString(query),
q: Crbug.queryString(query),
};
this.addCallbackToQuery(query, callback, params);
}
Expand Down
22 changes: 9 additions & 13 deletions client/cz-issue-priority-dash.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@
items="{{users}}"
as="user"
filter="filterUsers"
observe="issues">
observe="issueList">
<paper-item>
<paper-item-body two-line>
<div>{{user.user}}</div>
<div secondary class='priorities'>
<template is="dom-repeat" items="{{priorities(user.issues, updateSLO)}}">
<template is="dom-repeat" items="{{summary(user.issueList, updateSLO)}}">
<a href="{{crbugLink(user.email, item.query)}}">
<span class="bubble" style="background-color: {{item.color}}">{{item.level}}</span>
<span class="summary">{{item.summary}}</span>
Expand Down Expand Up @@ -89,9 +89,9 @@
},

filterUsers(item) {
if (item.issues == undefined)
if (item.issueList == undefined)
return false;
return item.issues.length() > 0;
return item.issueList.length > 0;
},

attached: function() {
Expand All @@ -103,25 +103,21 @@
}.bind(this));
},

priorities: function(issues, updateSLO) {
return issues.summary(updateSLO);
summary: function(issueList, updateSLO) {
return issueList.summary(updateSLO);
},

crbugLink: function(email, query) {
var userQuery = clone(query);
userQuery.owner = email;
return CrBug.queryURL(userQuery);
return Crbug.queryURL(userQuery);
},

usersChanged: function(users) {
// TODO: this probably only works for all-or-nothing changes to users right now.
users.forEach(function(user, idx) {
registerSource('cz-issues', {owner: user.email}, function(data) {
var issueList = new IssueList();
for (var i = 0; i < data.length; i++) {
issueList.append(new Issue(data[i]));
}
this.set('users.' + idx + '.issues', issueList);
registerSource('cz-crbug-issues', {owner: user.email}, function(issueList) {
this.set('users.' + idx + '.issueList', issueList);
}.bind(this));
}.bind(this));
}
Expand Down
39 changes: 19 additions & 20 deletions client/cz-milestones-dash.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@
<div class='card-flex'>
<template is="dom-repeat" items="{{users}}"
filter="filterUsers" sort="sortUsers"
observe="data">
observe="issueList">
<paper-item>
<paper-item-body two-line>
<div>{{item.user}}</div>
<!-- TODO(suzyh): convert to templatized HTML -->
<div secondary inner-h-t-m-l="[[milestones(item.data, releases)]]" class='milestones'></div>
<div secondary inner-h-t-m-l="[[milestones(item.issueList, releases)]]" class='milestones'></div>
</paper-item-body>
</paper-item>
</template>
Expand All @@ -61,26 +61,26 @@
},

filterUsers(item) {
if (item.data == undefined)
if (item.issueList == undefined)
return false;
return (item.data.filter(a => a.M != undefined).length > 0);
return (item.issueList.filter(a => a.M != undefined).length > 0);
},

sortUsers(a, b) {
if (a.data == undefined || b.data == undefined)
if (a.issueList == undefined || b.issueList == undefined)
return 0;
var minMilestoneA = undefined
for (var i = 0; i < a.data.length; i++) {
var milestone = Number(a.data[i].M);
for (var i = 0; i < a.issueList.length; i++) {
var milestone = Number(a.issueList[i].M);
if (milestone == 0)
continue;
if (minMilestoneA == undefined || milestone < minMilestoneA)
minMilestoneA = milestone;
}

var minMilestoneB = undefined
for (var i = 0; i < b.data.length; i++) {
var milestone = Number(b.data[i].M);
for (var i = 0; i < b.issueList.length; i++) {
var milestone = Number(b.issueList[i].M);
if (milestone == 0)
continue;
if (minMilestoneB == undefined || milestone < minMilestoneB)
Expand All @@ -103,14 +103,14 @@
}.bind(this));
},

milestones: function(data, releases) {
milestones: function(issueList, releases) {
var div = "<div style='color: white; border-radius: 4px; padding: 0px 6px;";
if (data == undefined)
if (issueList == undefined)
return "pending"
var milestones = {};
data = data.filter(a => a.M);
data.forEach(a => a.M = Number(a.M) ? Math.max(Number(a.M), releases.stable) : a.M);
data.forEach(a => milestones[a.M] ? milestones[a.M]++ : milestones[a.M] = 1);
issueList = issueList.filter(a => a.M);
issueList.forEach(a => a.M = Number(a.M) ? Math.max(Number(a.M), releases.stable) : a.M);
issueList.forEach(a => milestones[a.M] ? milestones[a.M]++ : milestones[a.M] = 1);
var result = "";
var keys = Object.keys(milestones).sort();
for (var key of keys) {
Expand All @@ -131,17 +131,16 @@
usersChanged: function(users) {
// TODO: this probably only works for all-or-nothing changes to users right now.
users.forEach(function(user, idx) {
registerSource('cz-issues', {owner: user.email}, function(data) {
for (var i = 0; i < data.length; i++) {
for (var j = 0; j < data[i].labels.length; j++) {
var label = data[i].labels[j];
registerSource('cz-crbug-issues', {owner: user.email}, function(issueList) {
for (var issue of issueList) {
for (var label of issue.labels) {
if (label.substring(0, 2).toLowerCase() == 'm-') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this 'm-xx' label processing belongs in Issue.create as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M doesn't really make sense for issues other than monorail, nor is it used by any other dash. I'd rather leave this behaviour scoped to this particular dash.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair call.

data[i].M = label.substring(2);
issue.M = label.substring(2);
break;
}
}
}
this.set('users.' + idx + '.data', data);
this.set('users.' + idx + '.issueList', issueList);
}.bind(this));
}.bind(this));
}
Expand Down
4 changes: 2 additions & 2 deletions client/cz-regression-dash.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
if (this.components.length == 0) {
return;
}
registerSource('cz-issues', {type: 'Bug-Regression', component: this.components}, function(issues) {
registerSource('cz-crbug-issues', {type: 'Bug-Regression', component: this.components}, function(issues) {
var regressionVersions = [];
for (var issue of issues) {
var version = null;
Expand All @@ -124,7 +124,7 @@
},

crbugLink: function(components) {
return CrBug.queryURL({type: 'Bug-Regression', component: components});
return Crbug.queryURL({type: 'Bug-Regression', component: components});
},

joinSpace: list => list.join(' '),
Expand Down
Loading