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

grunt-tasks/concat.js

@@ -24,6 +24,7 @@ module.exports = function(grunt) {
       },
       diagnose: {
         src: [
+            '<%= jsPath %>/lib/labels.js',
             '<%= jsPath %>/lib/models/issue.js',
             '<%= jsPath %>/lib/diagnose.js'
         ],
@@ -32,17 +33,18 @@ module.exports = function(grunt) {
       issues: {
         src: [
             '<%= jsPath %>/vendor/qr.min.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/labels.js',
           '<%= jsPath %>/lib/models/issue.js',
           '<%= jsPath %>/lib/issue-list.js'
         ],

webcompat/api/endpoints.py

@@ -256,15 +256,12 @@ def modify_labels(number):
 
 @api.route('/issues/labels')
 def get_repo_labels():
-    '''XHR endpoint to get all possible labels in a repo.'''
-    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)
+    '''XHR endpoint to get all possible labels in a repo.
+    '''
+    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))
 
 
 @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,31 +4,119 @@
 
  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;
+/**
+* A LabelList is a list of labels.
+*
+* It takes care of all namespace prefixing and unprefixing, so that
+* the rest of the app doesn't ever need to worry about those details.
+* To initialize, either pass in a list of labels as an array of strings
+* or an array of objects:
+*
+* new issues.LabelList({labels: ['firefox', 'ie', 'chrome']});
+*
+* new issues.LabelList({labels: [{name:'status-worksforme', url:'...',
+*    color:'cccccc'}]});
+*
+* Or a URL to a JSON file describing the labels:
+*
+* new issues.LabelList({url:'/path/to/labels.json'});
+*/
+
+issues.LabelList = Backbone.Model.extend({
+  initialize: function() {
+    this.set('namespaceRegex', /(browser|closed|os|status)-(.+)/i);
+    this.set('defaultLabelURL', '/api/issues/labels');
+    // The templating engine needs objects that have JS properties, it won't call
+    // get('labels'). Setting a property here makes sure we can pass the model
+    // directly to a template() method
+    this.on('change:labels', function(){
+      this.labels = this.get('labels');
     });
+    // if we're initialized with {labels:array-of-objects}, process the data
+    var inputLabelData = this.get('labels');
+    this.set('labels', []);
+    if(inputLabelData) {
+      this.parse(inputLabelData);
+    } else {
+      // No input data, let's fetch it from a URL
+      if(!this.get('url')) {
+        // default to "all labels" URL
+        this.set('url', this.get('defaultLabelURL'));
+      }
+      var headersBag = {headers: {'Accept': 'application/json'}};
+      this.fetch(headersBag); // This will trigger parse() on response
+    }
   },
-  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)
-    });
+  parse: function(labelsArray){
+    var list = [];
+    var namespaceMap = {};
+    for(var i = 0, matches, theLabel; i < labelsArray.length; i++){
+      // We assume we either have an object with .name or an array of strings
+      theLabel = labelsArray[i].name || labelsArray[i];
+      matches = theLabel.match(this.get('namespaceRegex'));
+      if(matches) {
+        namespaceMap[matches[2]] = matches[1];
+        list[i] = {
+          'name': matches[2],
+          'url': labelsArray[i].url,
+          'color': labelsArray[i].color,
+          'remoteName': matches[0]
+         };
+      }else {
+        if(typeof theLabel === 'object') {
+          list[i] = labelsArray[i];
+          list[i].remoteName = list[i].name;
+        } else {
+          list[i] = {'name': theLabel};
+        }
+      }
+    }
+    this.set('labels', list);
+    this.set('namespaceMap', namespaceMap);
+  },
+  // toPrefixed takes a local label name and maps it
+  // to the prefixed repository form. Also handles an array
+  // of label names (Note: not arrays of objects)
+  toPrefixed: function (input) {
+    if (typeof input === 'string') {
+      if(issues.allLabels.get('namespaceMap')[input]) {
+        return issues.allLabels.get('namespaceMap')[input] + '-' + input;
+      }
+      return input;
+    } else {
+      // This is not a string, we assume it's an array
+      return input.map(function(label){
+        return issues.allLabels.toPrefixed(label);
+      });
+    }
+  },
+  url: function() {
+    return this.get('url');
+  },
+  // Returns a simple array of unprefixed labels - strings only
+  toArray: function(){
+    return _.pluck(this.get('labels'), 'name');
+  },
+  // To save the model to the server, we need to make
+  // sure we apply the prefixes the server expects.
+  // The JSON serialization will take care of it.
+  toJSON: function(){
+    var labelsArray = _.pluck(this.get('labels'), 'name');
+    return issues.allLabels.toPrefixed(labelsArray);
   }
 });
 
+// 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'),
   el: $('.Label-wrapper'),
@@ -61,21 +149,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 +165,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 +187,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 +250,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,13 @@ 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.';

webcompat/templates/index.html

@@ -105,6 +105,7 @@ <h2>Join The Team</h2>
 {%- if config.PRODUCTION or config.DEVELOPMENT -%}
 <script src="{{ url_for('static', filename='js/diagnose.min.js')|bust_cache }}"></script>
 {%- else -%}
+<script src="{{ url_for('static', filename='js/lib/labels.js') }}"></script>
 <script src="{{ url_for('static', filename='js/lib/models/issue.js') }}"></script>
 <script src="{{ url_for('static', filename='js/lib/diagnose.js') }}"></script>
 {%- endif -%}

webcompat/templates/issue-list.html

@@ -75,9 +75,9 @@
               </p>
             </div>
             <div class="wc-IssueItem-section">
-              <span data-labels-map='<%= JSON.stringify(issue.labelsMap) %>' class="wc-IssueItem-label js-issue-label">
+              <span class="wc-IssueItem-label js-issue-label">
               <% _.each(issue.labels, function(label) { %>
-                <a href="#" class="wc-Labels" title="Labels : <%= label.name %>"><%= label.name %></a>
+                <a href="#" class="wc-Labels" data-remotename="<%= label.remoteName %>" title="Labels : <%= label.name %>"><%= label.name %></a>
               <% }); %></span>
             </div>
           </div>
@@ -136,6 +136,7 @@ <h1 class="wc-Dropdown-label"><%= dropdownTitle %></h1> <span class="wc-Icon wc-
 {%- if config.PRODUCTION or config.DEVELOPMENT -%}
 <script src="{{ url_for('static', filename='js/issue-list.min.js')|bust_cache }}"></script>
 {% else %}
+<script src="{{ url_for('static', filename='js/lib/labels.js') }}"></script>
 <script src="{{ url_for('static', filename='js/lib/models/issue.js') }}"></script>
 <script src="{{ url_for('static', filename='js/lib/issue-list.js') }}"></script>
 {%- endif %}