Skip to content

Commit

Permalink
Merge pull request #796 from webcompat/labelworks
Browse files Browse the repository at this point in the history 
New label code - fixes #783, #784, #797, #799
  • Loading branch information
Mike Taylor committed Oct 26, 2015
2 parents 4f9adb8 + 5b60276 commit d6b97ea
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 117 deletions.
7 changes: 6 additions & 1 deletion grunt-tasks/concat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ module.exports = function(grunt) {
},
diagnose: {
src: [
'<%= jsPath %>/lib/models/label-list.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/models/issue.js',
'<%= jsPath %>/lib/diagnose.js'
],
Expand All @@ -32,17 +34,20 @@ module.exports = function(grunt) {
issues: {
src: [
'<%= jsPath %>/vendor/qr.min.js',
'<%= jsPath %>/lib/models/label-list.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/models/issue.js',
'<%= jsPath %>/lib/models/comment.js',
'<%= jsPath %>/lib/comments.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/qrcode.js',
'<%= jsPath %>/lib/issues.js',
],
dest: '<%= jsPath %>/issues.js'
},
issueList: {
src: [
'<%= jsPath %>/lib/models/label-list.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/models/issue.js',
'<%= jsPath %>/lib/issue-list.js'
],
Expand Down
3 changes: 2 additions & 1 deletion grunt-tasks/jshint.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ module.exports = function(grunt) {
'<%= jsPath %>/lib/diagnose.js',
'<%= jsPath %>/lib/flash-message.js',
'<%= jsPath %>/lib/homepage.js',
'<%= jsPath %>/lib/models/label-list.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/issues.js',
'<%= jsPath %>/lib/issue-list.js',
'<%= jsPath %>/lib/labels.js',
'<%= jsPath %>/lib/models/comment.js',
'<%= jsPath %>/lib/models/issue.js',
'<%= jsPath %>/lib/qrCode.js',
Expand Down
10 changes: 7 additions & 3 deletions tests/test_api_urls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,16 @@ def test_api_wrong_category(self):
self.assertEqual(json_body['status'], 404)

def test_api_labels_without_auth(self):
'''API access to labels without auth returns JSON 401.'''
'''API access to labels without auth returns JSON 200.'''
rv = self.app.get('/api/issues/labels', environ_base=headers)
json_body = json.loads(rv.data)
self.assertEqual(rv.status_code, 401)
self.assertEqual(rv.status_code, 200)
self.assertEqual(rv.content_type, 'application/json')
self.assertEqual(json_body['status'], 401)

def test_api_set_labels_without_auth(self):
'''API setting labels without auth returns JSON 403 error code.'''
rv = self.app.post('/api/issues/1/labels', environ_base=headers, data='[]')
self.assertEqual(rv.status_code, 403)

def test_api_search_wrong_parameter(self):
'''API with wrong parameter returns JSON 404.'''
Expand Down
20 changes: 12 additions & 8 deletions webcompat/api/endpoints.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,25 +249,29 @@ def modify_labels(number):
can't normally edit labels for an issue.
'''
try:
labels = proxy_request('put', '/{0}/labels'.format(number),
data=request.data)
return (labels.content, labels.status_code, get_headers(labels))
if g.user:
labels = proxy_request('put', '/{0}/labels'.format(number),
data=request.data)
return (labels.content, labels.status_code, get_headers(labels))
else:
abort(403)
except GitHubError as e:
print('GitHubError: ', e.response.status_code)
return (':(', e.response.status_code)

@api.route('/issues/labels')
@mockable_response
def get_repo_labels():
'''XHR endpoint to get all possible labels in a repo.'''
'''XHR endpoint to get all possible labels in a repo.
'''
request_headers = get_request_headers(g.request_headers)
if g.user:
request_headers = get_request_headers(g.request_headers)
path = 'repos/{0}/labels'.format(REPO_PATH)
labels = github.raw_request('GET', path, headers=request_headers)
return (labels.content, labels.status_code, get_headers(labels))
else:
# only authed users should be hitting this endpoint
abort(401)
path = 'https://api.github.com/repos/{0}/labels'.format(REPO_PATH)
labels = proxy_request('get', uri=path, headers=request_headers)
return (labels.content, labels.status_code, get_headers(labels))


@api.route('/rate_limit')
Expand Down
5 changes: 2 additions & 3 deletions webcompat/static/js/lib/issue-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,8 @@ issueList.IssueView = Backbone.View.extend({
// "search:update" event to populate the view with search results
// for the given label.
var target = $(e.target);
var labelsMap = target.parent().data('labelsMap');
var clickedLabel = target.text();
var labelFilter = 'label:' + labelsMap[clickedLabel];
var clickedLabel = target.data('remotename');
var labelFilter = 'label:' + clickedLabel;
issueList.events.trigger('search:update', labelFilter);
issueList.events.trigger('issues:update', {query: labelFilter});
e.preventDefault();
Expand Down
63 changes: 21 additions & 42 deletions webcompat/static/js/lib/labels.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,16 @@

var issues = issues || {};

// Read-only Model of all labels in the repo
issues.AllLabels = Backbone.Model.extend({
url: function() {
return '/api/issues/labels';
},
// See also issues.Issue#removeNamespaces
removeNamespaces: function(labelsArray) {
// Return a copy of labelsArray with the namespaces removed.
var namespaceRegex = /(browser|closed|os|status)-/i;
var labelsCopy = _.cloneDeep(labelsArray);
return _.map(labelsCopy, function(labelObject) {
labelObject.name = labelObject.name.replace(namespaceRegex, '');
return labelObject;
});
},
parse: function(response) {
this.set({
// Store a copy of the original response, so we can reconstruct
// the labels before talking back to the API.
namespacedLabels: response,
labels: this.removeNamespaces(response)
});
}
});
// We need a complete list of labels for certain operations,
// especially namespace mapping. If the list we're handling
// doesn't happen to contain all the labels initially, it
// can't get prefixing/unprefixing right when labels in previously
// unseen namespaces are added in their local name form.
// Hence, we set up a single, globally accessible "all labels" model
// This is set up as early as possible to avoid timing issues
if(!issues.allLabels) {
issues.allLabels = new issues.LabelList();
}

issues.LabelsView = Backbone.View.extend({
_isLoggedIn: $('body').data('username'),
Expand Down Expand Up @@ -61,21 +47,14 @@ issues.LabelsView = Backbone.View.extend({
this.$el.html(this.template(this.model.toJSON()));
},
fetchLabels: function() {
var headersBag = {headers: {'Accept': 'application/json'}};
this.editorButton = $('.LabelEditor-launcher');
this.allLabels = new issues.AllLabels();
this.labelEditor = new issues.LabelEditorView({
model: this.allLabels,
model: issues.allLabels,
issueView: this,
});
// Stash the allLabels model so we can get it from Issue model later
this.model.set('repoLabels', this.allLabels);
if (this._isLoggedIn) {
this.allLabels.fetch(headersBag).success(_.bind(function(){
this.issueLabels = this.getIssueLabels();
this.repoLabels = _.pluck(this.labelEditor.model.get('labels'), 'name');
this.editorButton.show();
}, this));
this.issueLabels = this.getIssueLabels();
this.editorButton.show();
}
},
getIssueLabels: function() {
Expand All @@ -84,7 +63,7 @@ issues.LabelsView = Backbone.View.extend({
editLabels: function() {
this.editorButton.addClass('is-active');
this.$el.find('.LabelEditor-launcher').after(this.labelEditor.render().el);
var toBeChecked = _.intersection(this.getIssueLabels(), this.repoLabels);
var toBeChecked = _.intersection(this.getIssueLabels(), issues.allLabels.toArray());
_.each(toBeChecked, function(labelName) {
$('[name="' + labelName + '"]').prop('checked', true);
});
Expand All @@ -106,7 +85,7 @@ issues.LabelEditorView = Backbone.View.extend({
},
template: _.template($('#label-editor-tmpl').html()),
render: function() {
this.$el.html(this.template(this.model.toJSON()));
this.$el.html(this.template(this.model));
this.resizeEditorHeight();
_.defer(_.bind(function() {
this.$el.find('.LabelEditor-search').focus();
Expand Down Expand Up @@ -169,17 +148,17 @@ issues.LabelEditorView = Backbone.View.extend({
return s.replace(/[-\/\\^$*+?:.()|[\]{}]/g, '\\$&');
};
var re = new RegExp('^' + escape(e.target.value), 'i');
var matches = _.pluck(_.filter(this.model.get('labels'), function(label) {
return re.test(label.name);
}), 'name');
var toHide = _.filter(this.model.toArray(), function(label) {
return !re.test(label);
});

// make sure everything is showing
$('.LabelEditor-item').show();

// hide the non-filter matches
var hidden = _.difference(_.pluck(this.model.get('labels'), 'name'), matches);
_.each(hidden, function(name) {
$('input[name="' + escape(name) + '"]').closest('.LabelEditor-item').hide();
_.each(toHide, function(name) {
$('input[name=' + escape(name) + ']').closest('.LabelEditor-item').hide();

});
}, 100)
});
61 changes: 8 additions & 53 deletions webcompat/static/js/lib/models/issue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,39 +124,15 @@ md.renderer.rules.link_open = function (tokens, idx, options, env, self) {
this.set('stateClass', 'new');
return 'New Issue';
},
// See also issues.AllLabels#removeNamespaces
removeNamespaces: function(labelsArray) {
// Return a copy of labelsArray with the namespaces removed.
var labelsCopy = _.cloneDeep(labelsArray);
return _.map(labelsCopy, _.bind(function(labelObject) {
labelObject.name = labelObject.name.replace(this._namespaceRegex, '');
return labelObject;
}, this));
},
getLabelsMap: function(labelsArray) {
/* Create a mapping between a unnamespaced labels and namespaced labels,
i.e., {'contactready': 'status-contactready'} */
var labelsMap = {};
var tmp = _.groupBy(labelsArray, function(labelObj) {
return labelObj.name;
});

_.forEach(tmp, _.bind(function(val, key) {
labelsMap[val[0].name.replace(this._namespaceRegex, '')] = key;
}, this));

tmp = null;
return labelsMap;
},
parse: function(response) {
var labels = this.removeNamespaces(response.labels);
var labelList = new issues.LabelList({'labels':response.labels});
var labels = labelList.get('labels');
this.set({
body: md.render(response.body),
commentNumber: response.comments,
createdAt: response.created_at.slice(0, 10),
issueState: this.getState(response.state, labels),
labels: labels,
labelsMap: this.getLabelsMap(response.labels),
number: response.number,
reporter: response.user.login,
reporterAvatar: response.user.avatar_url,
Expand Down Expand Up @@ -184,39 +160,18 @@ md.renderer.rules.link_open = function (tokens, idx, options, env, self) {
});
},
updateLabels: function(labelsArray) {
var namespaceRegex = '^(browser|closed|os|status)-';
var repoLabelsArray = _.pluck(this.get('repoLabels').get('namespacedLabels'),
'name');

// Save ourselves some requests in case nothing has changed.
if (!$.isArray(labelsArray) ||
_.isEqual(labelsArray.sort(), _.pluck(this.get('labels'), 'name').sort())) {
return;
}

// Reconstruct the namespaced labels by comparing the "new" labels
// against the original namespaced labels from the repo.
//
// for each label in the labels array
// filter over each repoLabel in the repoLabelsArray
// if a regex from namespaceRegex + label matches against repoLabel
// return that (and flatten the result because it's now an array of N arrays)
var labelsToUpdate = _.flatten(_.map(labelsArray, function(label) {
return _.filter(repoLabelsArray, function(repoLabel) {
if (new RegExp(namespaceRegex + label + '$', 'i').test(repoLabel)) {
return repoLabel;
}
});
}));

$.ajax({
contentType: 'application/json',
data: JSON.stringify(labelsToUpdate),
type: 'POST',
url: '/api/issues/' + this.get('number') + '/labels',
var labels = new issues.LabelList({'labels':labelsArray,
url: '/api/issues/' + this.get('number') + '/labels'});
labels.save(null, {
success: _.bind(function(response) {
//update model after success
this.set('labels', response);
// update model after success
var updatedLabels = new issues.LabelList({'labels': response.get('labels')});
this.set('labels', updatedLabels.get('labels'));
}, this),
error: function() {
var msg = 'There was an error setting labels.';
Expand Down
Loading

0 comments on commit d6b97ea

Please sign in to comment.