Skip to content

Commit 4555010

Browse files
author
Mike Taylor
committed
Merge pull request #661 from /issues/660/1
Fixes #660. Move all label namespace handling to models.
2 parents 77ddeab + 7b58903 commit 4555010

File tree

6 files changed

+165
-74
lines changed

6 files changed

+165
-74
lines changed

grunt-tasks/concat.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ module.exports = function(grunt) {
77
dist: {
88
src: [
99
'<%= jsPath %>/vendor/jquery-1.11.2.min.js',
10-
'<%= jsPath %>/vendor/lodash.underscore-min.js',
10+
'<%= jsPath %>/vendor/lodash.custom.min.js',
1111
'<%= jsPath %>/vendor/backbone-min.js',
1212
'<%= jsPath %>/vendor/moment-min.js',
1313
'<%= jsPath %>/vendor/prism.js',

webcompat/static/js/lib/labels.js

+20-3
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,23 @@ issues.AllLabels = Backbone.Model.extend({
99
url: function() {
1010
return '/api/issues/labels';
1111
},
12+
// See also issues.Issue#removeNamespaces
13+
removeNamespaces: function(labelsArray) {
14+
// Return a copy of labelsArray with the namespaces removed.
15+
var namespaceRegex = /(browser|closed|os|status)-/i;
16+
var labelsCopy = _.cloneDeep(labelsArray);
17+
return _.map(labelsCopy, function(labelObject) {
18+
labelObject.name = labelObject.name.replace(namespaceRegex, '');
19+
return labelObject;
20+
});
21+
},
1222
parse: function(response) {
13-
this.set({labels: response});
23+
this.set({
24+
// Store a copy of the original response, so we can reconstruct
25+
// the labels before talking back to the API.
26+
namespacedLabels: response,
27+
labels: this.removeNamespaces(response)
28+
});
1429
}
1530
});
1631

@@ -31,7 +46,7 @@ issues.LabelsView = Backbone.View.extend({
3146
subTemplate: _.template([
3247
'<% _.each(labels, function(label) { %>',
3348
'<span class="Label Label--badge" style="background-color:#<%=label.color%>">',
34-
'<%= label.name.replace(/(browser|status)-/, "") %>',
49+
'<%= label.name %>',
3550
'</span>',
3651
'<% }); %>'].join('')),
3752
render: function() {
@@ -53,6 +68,8 @@ issues.LabelsView = Backbone.View.extend({
5368
model: this.allLabels,
5469
issueView: this,
5570
});
71+
// Stash the allLabels model so we can get it from Issue model later
72+
this.model.set('repoLabels', this.allLabels);
5673
if (this._isLoggedIn) {
5774
this.allLabels.fetch(headersBag).success(_.bind(function(){
5875
this.issueLabels = this.getIssueLabels();
@@ -69,7 +86,7 @@ issues.LabelsView = Backbone.View.extend({
6986
this.$el.find('.LabelEditor-launcher').after(this.labelEditor.render().el);
7087
var toBeChecked = _.intersection(this.getIssueLabels(), this.repoLabels);
7188
_.each(toBeChecked, function(labelName) {
72-
$('[name=' + labelName.replace(/(browser|status)-/, '') + ']').prop('checked', true);
89+
$('[name=' + labelName + ']').prop('checked', true);
7390
});
7491
}
7592
});

webcompat/static/js/lib/models/issue.js

+41-30
Original file line numberDiff line numberDiff line change
@@ -32,29 +32,40 @@
3232
this.set('stateClass', 'close');
3333
return 'Closed';
3434
}
35-
if (labelsNames.indexOf('status-sitewait') > -1) {
35+
if (labelsNames.indexOf('sitewait') > -1) {
3636
this.set('stateClass', 'sitewait');
3737
return 'Site Contacted';
3838
}
39-
if (labelsNames.indexOf('status-contactready') > -1) {
39+
if (labelsNames.indexOf('contactready') > -1) {
4040
this.set('stateClass', 'ready');
4141
return 'Ready for Outreach';
4242
}
43-
if (labelsNames.indexOf('status-needsdiagnosis') > -1) {
43+
if (labelsNames.indexOf('needsdiagnosis') > -1) {
4444
this.set('stateClass', 'need');
4545
return 'Needs Diagnosis';
4646
}
4747
//New is the default value.
4848
this.set('stateClass', 'new');
4949
return 'New Issue';
5050
},
51+
// See also issues.AllLabels#removeNamespaces
52+
removeNamespaces: function(labelsArray) {
53+
// Return a copy of labelsArray with the namespaces removed.
54+
var namespaceRegex = /(browser|closed|os|status)-/i;
55+
var labelsCopy = _.cloneDeep(labelsArray);
56+
return _.map(labelsCopy, function(labelObject) {
57+
labelObject.name = labelObject.name.replace(namespaceRegex, '');
58+
return labelObject;
59+
});
60+
},
5161
parse: function(response) {
62+
var labels = this.removeNamespaces(response.labels);
5263
this.set({
5364
body: md.render(response.body),
5465
commentNumber: response.comments,
5566
createdAt: response.created_at.slice(0, 10),
56-
issueState: this.getState(response.state, response.labels),
57-
labels: response.labels,
67+
issueState: this.getState(response.state, labels),
68+
labels: labels,
5869
number: response.number,
5970
reporter: response.user.login,
6071
reporterAvatar: response.user.avatar_url,
@@ -83,40 +94,40 @@
8394
});
8495
},
8596
updateLabels: function(labelsArray) {
86-
// maybe this should be in a shared config file outside of python/JS
87-
var statusLabels = ['contactready', 'needscontact', 'needsdiagnosis', 'sitewait', ' closed-duplicate', 'closed-fixed', 'closed-invalid'];
88-
var browserLabels = ['chrome', 'firefox', 'ie', 'opera', 'safari', 'vivaldi'];
89-
var osLabels = ['android', 'fxos', 'ios', 'linux', 'mac', 'win'];
90-
// we check if we need to append the correct string before sending stuff back
91-
for (var i = labelsArray.length - 1; i >= 0; i--) {
92-
if (statusLabels.indexOf(labelsArray[i]) !== -1) {
93-
labelsArray[i] = 'status-'.concat(labelsArray[i]);
94-
} else if (browserLabels.indexOf(labelsArray[i]) !== -1) {
95-
labelsArray[i] = 'browser-'.concat(labelsArray[i]);
96-
} else if (osLabels.indexOf(labelsArray[i]) !== -1) {
97-
labelsArray[i] = 'os-'.concat(labelsArray[i]);
98-
}
99-
}
100-
var self = this;
101-
if (!$.isArray(labelsArray)) {
102-
return;
103-
}
97+
var namespaceRegex = '^(browser|closed|os|status)-';
98+
var repoLabelsArray = _.pluck(this.get('repoLabels').get('namespacedLabels'),
99+
'name');
104100

105-
// save ourselves a request if nothing has changed.
106-
if (_.isEqual(labelsArray.sort(),
107-
_.pluck(this.get('labels'), 'name').sort())) {
101+
// Save ourselves some requests in case nothing has changed.
102+
if (!$.isArray(labelsArray) ||
103+
_.isEqual(labelsArray.sort(), _.pluck(this.get('labels'), 'name').sort())) {
108104
return;
109105
}
110106

107+
// Reconstruct the namespaced labels by comparing the "new" labels
108+
// against the original namespaced labels from the repo.
109+
//
110+
// for each label in the labels array
111+
// filter over each repoLabel in the repoLabelsArray
112+
// if a regex from namespaceRegex + label matches against repoLabel
113+
// return that (and flatten the result because it's now an array of N arrays)
114+
var labelsToUpdate = _.flatten(_.map(labelsArray, function(label) {
115+
return _.filter(repoLabelsArray, function(repoLabel) {
116+
if (new RegExp(namespaceRegex + label + '$', 'i').test(repoLabel)) {
117+
return repoLabel;
118+
}
119+
});
120+
}));
121+
111122
$.ajax({
112123
contentType: 'application/json',
113-
data: JSON.stringify(labelsArray),
124+
data: JSON.stringify(labelsToUpdate),
114125
type: 'POST',
115126
url: '/api/issues/' + this.get('number') + '/labels',
116-
success: function(response) {
127+
success: _.bind(function(response) {
117128
//update model after success
118-
self.set('labels', response);
119-
},
129+
this.set('labels', response);
130+
}, this),
120131
error: function() {
121132
var msg = 'There was an error setting labels.';
122133
wcEvents.trigger('flash:error', {message: msg, timeout: 2000});

0 commit comments

Comments
 (0)