New label code - fixes #783, #784, #797, #799

grunt-tasks/concat.js

@@ -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'
         ],
@@ -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'
         ],

grunt-tasks/jshint.js

@@ -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',

tests/test_api_urls.py

@@ -63,12 +63,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.'''

webcompat/api/endpoints.py

@@ -246,25 +246,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')
 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')

webcompat/static/js/lib/issue-list.js

@@ -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();

webcompat/static/js/lib/labels.js

@@ -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'),
@@ -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() {
@@ -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);
     });
@@ -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();
@@ -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)
 });

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

@@ -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,
@@ -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.';