From 502fdc9c405ce57b6917756102d708a9d5d694f9 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 17:49:51 +0200 Subject: [PATCH 01/15] Concat label code into list JS, #783 --- grunt-tasks/concat.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/grunt-tasks/concat.js b/grunt-tasks/concat.js index 3fd65cc28..d25bca498 100644 --- a/grunt-tasks/concat.js +++ b/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,10 +33,10 @@ 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', ], @@ -43,6 +44,7 @@ module.exports = function(grunt) { }, issueList: { src: [ + '<%= jsPath %>/lib/labels.js', '<%= jsPath %>/lib/models/issue.js', '<%= jsPath %>/lib/issue-list.js' ], From 41237f691f4c88c641ae4b54a2df8628feb142a1 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 17:53:57 +0200 Subject: [PATCH 02/15] When issue lists rely on the label model, non-auth users should also get the full list from GitHub, #783 --- webcompat/api/endpoints.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/webcompat/api/endpoints.py b/webcompat/api/endpoints.py index 537d15f3b..a24190e69 100644 --- a/webcompat/api/endpoints.py +++ b/webcompat/api/endpoints.py @@ -264,14 +264,10 @@ def get_repo_labels(): Cached for 10 minutes. ''' - 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) + 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') From a5bd51067d1daf13b6e17be56854768d098e8efe Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 18:06:12 +0200 Subject: [PATCH 03/15] #783 Template fixes to include label js in the right place and remove some obsolete prefix handling --- webcompat/templates/index.html | 1 + webcompat/templates/issue-list.html | 7 ++++--- webcompat/templates/issue.html | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/webcompat/templates/index.html b/webcompat/templates/index.html index 82f0c8f95..2832f6a3e 100644 --- a/webcompat/templates/index.html +++ b/webcompat/templates/index.html @@ -105,6 +105,7 @@

Join The Team

{%- if config.PRODUCTION or config.DEVELOPMENT -%} {%- else -%} + {%- endif -%} diff --git a/webcompat/templates/issue-list.html b/webcompat/templates/issue-list.html index 8f0aeeff8..1e131f60a 100644 --- a/webcompat/templates/issue-list.html +++ b/webcompat/templates/issue-list.html @@ -75,9 +75,9 @@

- - <% _.each(issue.labels, function(label) { %> - <%= label.name %> + + <% _.each(issue.labels, function(label) { console.log(label); %> + <%= label.name %> <% }); %>
@@ -136,6 +136,7 @@

<%= dropdownTitle %>

{% else %} + {%- endif %} diff --git a/webcompat/templates/issue.html b/webcompat/templates/issue.html index 47f849761..dd6916ddd 100644 --- a/webcompat/templates/issue.html +++ b/webcompat/templates/issue.html @@ -33,7 +33,7 @@

<% _.each(labels, function(label) { %> - <%= label.name.replace(/(browser|status)-/, '') %> + <%= label.name %> <% }); %> @@ -145,9 +145,9 @@

<% _.each(labels, function(label) { %> <% }); %> @@ -170,10 +170,10 @@

{% else %} + - {%- endif %} From dafaba4fb31b3399b5a36c5d4bf0f9fd16048b30 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 18:14:29 +0200 Subject: [PATCH 04/15] #783 simplify click-on-label search for the new label model --- webcompat/static/js/lib/issue-list.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/webcompat/static/js/lib/issue-list.js b/webcompat/static/js/lib/issue-list.js index 244d6bd75..ed26ec59c 100644 --- a/webcompat/static/js/lib/issue-list.js +++ b/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(); From 92f978f58cc7343886ceff912ddf5ce081887d6c Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 18:17:29 +0200 Subject: [PATCH 05/15] #783 Updating issue model to use the new label list model --- webcompat/static/js/lib/models/issue.js | 61 ++----------------------- 1 file changed, 5 insertions(+), 56 deletions(-) diff --git a/webcompat/static/js/lib/models/issue.js b/webcompat/static/js/lib/models/issue.js index 87a4ef6fe..6a1ea65d2 100644 --- a/webcompat/static/js/lib/models/issue.js +++ b/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,36 +160,9 @@ 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); From d0aec72dd209948e2135014d50f6ad6688db4202 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 18:18:59 +0200 Subject: [PATCH 06/15] set labels correctly from POST response, fixes #784 --- webcompat/api/endpoints.py | 2 +- webcompat/static/js/lib/models/issue.js | 5 +++-- webcompat/templates/issue-list.html | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/webcompat/api/endpoints.py b/webcompat/api/endpoints.py index a24190e69..7179fcef4 100644 --- a/webcompat/api/endpoints.py +++ b/webcompat/api/endpoints.py @@ -250,7 +250,7 @@ def modify_labels(number): ''' try: labels = proxy_request('put', '/{0}/labels'.format(number), - data=request.data) + data=request.data) return (labels.content, labels.status_code, get_headers(labels)) except GitHubError as e: print('GitHubError: ', e.response.status_code) diff --git a/webcompat/static/js/lib/models/issue.js b/webcompat/static/js/lib/models/issue.js index 6a1ea65d2..d32e8133a 100644 --- a/webcompat/static/js/lib/models/issue.js +++ b/webcompat/static/js/lib/models/issue.js @@ -164,8 +164,9 @@ md.renderer.rules.link_open = function (tokens, idx, options, env, self) { 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.'; diff --git a/webcompat/templates/issue-list.html b/webcompat/templates/issue-list.html index 1e131f60a..5cc22ddb0 100644 --- a/webcompat/templates/issue-list.html +++ b/webcompat/templates/issue-list.html @@ -76,7 +76,7 @@
- <% _.each(issue.labels, function(label) { console.log(label); %> + <% _.each(issue.labels, function(label) { %> <%= label.name %> <% }); %>
From 4a4e9a31869bbf35845cdde6f1c13df69ef3aa99 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 22:53:51 +0200 Subject: [PATCH 07/15] #783 The new label model and related adjustments in label code --- webcompat/static/js/lib/labels.js | 153 +++++++++++++++++++++++------- 1 file changed, 117 insertions(+), 36 deletions(-) diff --git a/webcompat/static/js/lib/labels.js b/webcompat/static/js/lib/labels.js index c62fae91d..9963e72ac 100644 --- a/webcompat/static/js/lib/labels.js +++ b/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 + var 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'), @@ -63,19 +151,13 @@ issues.LabelsView = Backbone.View.extend({ 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 +166,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 +188,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,16 +251,15 @@ 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) { + _.each(toHide, function(name) { $('input[name=' + escape(name) + ']').closest('.LabelEditor-item').hide(); }); }, 100) From 510fd34c2684aff1e586f7b486450b31ca7285ca Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 23:29:44 +0200 Subject: [PATCH 08/15] jshint-fixes --- webcompat/static/js/lib/labels.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/webcompat/static/js/lib/labels.js b/webcompat/static/js/lib/labels.js index 9963e72ac..9065b3497 100644 --- a/webcompat/static/js/lib/labels.js +++ b/webcompat/static/js/lib/labels.js @@ -52,7 +52,7 @@ issues.LabelList = Backbone.Model.extend({ 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 - var theLabel = labelsArray[i].name || labelsArray[i]; + theLabel = labelsArray[i].name || labelsArray[i]; matches = theLabel.match(this.get('namespaceRegex')); if(matches) { namespaceMap[matches[2]] = matches[1]; @@ -67,10 +67,10 @@ issues.LabelList = Backbone.Model.extend({ list[i] = labelsArray[i]; list[i].remoteName = list[i].name; } else { - list[i] = {'name': theLabel} + list[i] = {'name': theLabel}; } } - }; + } this.set('labels', list); this.set('namespaceMap', namespaceMap); }, @@ -149,7 +149,6 @@ 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.labelEditor = new issues.LabelEditorView({ model: issues.allLabels, From 93e1de8c9020f81e8f014c87586ae7b2a3ab47cb Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Thu, 22 Oct 2015 23:46:51 +0200 Subject: [PATCH 09/15] (just to undo a bogus whitespace change) --- webcompat/api/endpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webcompat/api/endpoints.py b/webcompat/api/endpoints.py index 8c40821a8..b9dafc15b 100644 --- a/webcompat/api/endpoints.py +++ b/webcompat/api/endpoints.py @@ -247,7 +247,7 @@ def modify_labels(number): ''' try: labels = proxy_request('put', '/{0}/labels'.format(number), - data=request.data) + data=request.data) return (labels.content, labels.status_code, get_headers(labels)) except GitHubError as e: print('GitHubError: ', e.response.status_code) From 997f147f3b6954a60079db3d33a3c1877df6595d Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 12:15:28 +0200 Subject: [PATCH 10/15] Use proxy_request if user is not logged in, #783 --- webcompat/api/endpoints.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/webcompat/api/endpoints.py b/webcompat/api/endpoints.py index b9dafc15b..b36bb915d 100644 --- a/webcompat/api/endpoints.py +++ b/webcompat/api/endpoints.py @@ -259,8 +259,12 @@ def get_repo_labels(): '''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) + if g.user: + path = 'repos/{0}/labels'.format(REPO_PATH) + labels = github.raw_request('GET', path, headers=request_headers) + else: + 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)) From 3096da6a11c5f94748513bb4175210f7aa0b10b0 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 12:26:39 +0200 Subject: [PATCH 11/15] Move LabelList model to separate file --- grunt-tasks/concat.js | 3 + grunt-tasks/jshint.js | 3 +- webcompat/static/js/lib/labels.js | 102 ------------------ webcompat/static/js/lib/models/label-list.js | 107 +++++++++++++++++++ webcompat/templates/index.html | 1 + webcompat/templates/issue-list.html | 1 + webcompat/templates/issue.html | 1 + 7 files changed, 115 insertions(+), 103 deletions(-) create mode 100644 webcompat/static/js/lib/models/label-list.js diff --git a/grunt-tasks/concat.js b/grunt-tasks/concat.js index d25bca498..6186bb877 100644 --- a/grunt-tasks/concat.js +++ b/grunt-tasks/concat.js @@ -24,6 +24,7 @@ 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' @@ -33,6 +34,7 @@ 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', @@ -44,6 +46,7 @@ module.exports = function(grunt) { }, issueList: { src: [ + '<%= jsPath %>/lib/models/label-list.js', '<%= jsPath %>/lib/labels.js', '<%= jsPath %>/lib/models/issue.js', '<%= jsPath %>/lib/issue-list.js' diff --git a/grunt-tasks/jshint.js b/grunt-tasks/jshint.js index 125661dbf..248243a24 100644 --- a/grunt-tasks/jshint.js +++ b/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', diff --git a/webcompat/static/js/lib/labels.js b/webcompat/static/js/lib/labels.js index e851b9d3a..6e9582896 100644 --- a/webcompat/static/js/lib/labels.js +++ b/webcompat/static/js/lib/labels.js @@ -4,108 +4,6 @@ var issues = issues || {}; -/** -* 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(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 diff --git a/webcompat/static/js/lib/models/label-list.js b/webcompat/static/js/lib/models/label-list.js new file mode 100644 index 000000000..e5b2a9673 --- /dev/null +++ b/webcompat/static/js/lib/models/label-list.js @@ -0,0 +1,107 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + + var issues = issues || {}; + +/** +* 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(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); + } +}); diff --git a/webcompat/templates/index.html b/webcompat/templates/index.html index 2832f6a3e..c9bb0fe32 100644 --- a/webcompat/templates/index.html +++ b/webcompat/templates/index.html @@ -105,6 +105,7 @@

Join The Team

{%- if config.PRODUCTION or config.DEVELOPMENT -%} {%- else -%} + diff --git a/webcompat/templates/issue-list.html b/webcompat/templates/issue-list.html index 5cc22ddb0..1c0714a26 100644 --- a/webcompat/templates/issue-list.html +++ b/webcompat/templates/issue-list.html @@ -136,6 +136,7 @@

<%= dropdownTitle %>

{% else %} + diff --git a/webcompat/templates/issue.html b/webcompat/templates/issue.html index dd6916ddd..763c52080 100644 --- a/webcompat/templates/issue.html +++ b/webcompat/templates/issue.html @@ -170,6 +170,7 @@

{% else %} + From cea85423de9bc31e306d8e1a829df0e7c4e11590 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 12:29:40 +0200 Subject: [PATCH 12/15] Changed backend returns labels even without auth - fix test #783 --- tests/test_api_urls.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_api_urls.py b/tests/test_api_urls.py index ad01e81fb..657984f97 100644 --- a/tests/test_api_urls.py +++ b/tests/test_api_urls.py @@ -63,12 +63,11 @@ 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_search_wrong_parameter(self): '''API with wrong parameter returns JSON 404.''' From cd7051c758599b29666937b7930d53aa0818b367 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 13:07:25 +0200 Subject: [PATCH 13/15] Restoring some code to avoid noop HTTP requests #783 --- webcompat/static/js/lib/models/issue.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/webcompat/static/js/lib/models/issue.js b/webcompat/static/js/lib/models/issue.js index d32e8133a..913c3156a 100644 --- a/webcompat/static/js/lib/models/issue.js +++ b/webcompat/static/js/lib/models/issue.js @@ -160,9 +160,14 @@ md.renderer.rules.link_open = function (tokens, idx, options, env, self) { }); }, updateLabels: function(labelsArray) { - var labels = new issues.LabelList({'labels':labelsArray, - url: '/api/issues/' + this.get('number') + '/labels'}); - labels.save(null, { + // Save ourselves some requests in case nothing has changed. + if (!$.isArray(labelsArray) || + _.isEqual(labelsArray.sort(), _.pluck(this.get('labels'), 'name').sort())) { + return; + } + 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 var updatedLabels = new issues.LabelList({'labels': response.get('labels')}); From dea2935efab0c38f9ffb27e2a6ae6424f67d7799 Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 13:21:35 +0200 Subject: [PATCH 14/15] Don't allow non-auth label setting --- tests/test_api_urls.py | 5 +++++ webcompat/api/endpoints.py | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_api_urls.py b/tests/test_api_urls.py index 657984f97..93d4b95b1 100644 --- a/tests/test_api_urls.py +++ b/tests/test_api_urls.py @@ -69,6 +69,11 @@ def test_api_labels_without_auth(self): self.assertEqual(rv.status_code, 200) self.assertEqual(rv.content_type, 'application/json') + 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, body='[]') + self.assertEqual(rv.status_code, 403) + def test_api_search_wrong_parameter(self): '''API with wrong parameter returns JSON 404.''' rv = self.app.get('/api/issues/search?z=foobar', environ_base=headers) diff --git a/webcompat/api/endpoints.py b/webcompat/api/endpoints.py index b36bb915d..6e8003095 100644 --- a/webcompat/api/endpoints.py +++ b/webcompat/api/endpoints.py @@ -246,9 +246,12 @@ 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) From 5b602767e63d8038abfc394ae8de8dbe29237a5b Mon Sep 17 00:00:00 2001 From: "Hallvord R. M. Steen" Date: Sat, 24 Oct 2015 13:56:41 +0200 Subject: [PATCH 15/15] Fix new test, used wrong keyword --- tests/test_api_urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api_urls.py b/tests/test_api_urls.py index 93d4b95b1..67296d6f7 100644 --- a/tests/test_api_urls.py +++ b/tests/test_api_urls.py @@ -71,7 +71,7 @@ def test_api_labels_without_auth(self): 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, body='[]') + 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):