Enable issues "pagination" dropdown. Fixes #372.

grunt-tasks/concat.js

@@ -7,6 +7,7 @@ module.exports = function(grunt) {
       dist: {
         src: [
             '<%= jsPath %>/vendor/jquery-1.11.0.min.js',
+            '<%= jsPath %>/vendor/jquery.deparam.js',
             '<%= jsPath %>/vendor/lodash.underscore-min.js',
             '<%= jsPath %>/vendor/backbone-min.js',
             '<%= jsPath %>/vendor/moment-min.js',

tests/functional/lib/issue-list.js

@@ -87,6 +87,41 @@ define([
           assert.include(className, 'is-disabled', 'Going back from first next click should have disabled prev button');
         })
         .end();
+    },
+
+    'pagination dropdown tests': function() {
+      return this.remote
+        .setFindTimeout(intern.config.wc.pageLoadTimeout)
+        .get(require.toUrl(url))
+        .findByCssSelector('.js-dropdown-pagination').isDisplayed()
+        .then(function (isDisplayed) {
+          assert.equal(isDisplayed, true, 'pagination dropdown container is visible.');
+        })
+        .end()
+        .findByCssSelector('.js-dropdown-pagination .js-dropdown-toggle').click()
+        .end()
+        .findByCssSelector('.js-dropdown-pagination').getAttribute('class')
+        .then(function (className) {
+          assert.include(className, 'is-active', 'clicking dropdown adds is-active class');
+        })
+        .end()
+        .findByCssSelector('.js-dropdown-pagination .js-dropdown-options').isDisplayed()
+        .then(function (isDisplayed) {
+          assert.equal(isDisplayed, true, 'dropdown options are visible.');
+        })
+        .end()
+        .findByCssSelector('.js-dropdown-pagination li.Dropdown-item:nth-child(3) > a:nth-child(1)').click()
+        .end()
+        .findByCssSelector('.js-dropdown-pagination .Dropdown-label').getVisibleText()
+        .then(function (text) {
+          assert.include(text, 'Show 100', 'Clicking first option updated dropdown label');
+        })
+        .end()
+        .findByCssSelector('.IssueItem:nth-child(51)').isDisplayed()
+        .then(function (isDisplayed) {
+          assert.equal(isDisplayed, true, 'More than 50 issues were loaded.');
+        })
+        .end();
     }
   });
 });

webcompat/api/endpoints.py

@@ -67,10 +67,7 @@ def edit_issue(number):
 @api.route('/issues')
 def proxy_issues():
     '''API endpoint to list all issues from GitHub.'''
-    if request.args.get('page'):
-        params = {'page': request.args.get('page')}
-    else:
-        params = None
+    params = request.args.copy()
 
     if g.user:
         issues = github.raw_request('GET', 'repos/{0}'.format(ISSUES_PATH),
@@ -105,12 +102,9 @@ def get_issue_category(issue_category):
     * needsdiagnosis
     * sitewait
     '''
-    params = {}
     category_list = ['contactready', 'needsdiagnosis', 'sitewait']
     issues_path = 'repos/{0}'.format(ISSUES_PATH)
-
-    if request.args.get('page'):
-        params.update({'page': request.args.get('page')})
+    params = request.args.copy()
 
     if issue_category in category_list:
         params.update({'labels': issue_category})
@@ -128,9 +122,9 @@ def get_issue_category(issue_category):
     # For paginated results on the /issues page, see /issues/search/untriaged.
     elif issue_category == 'untriaged':
         if g.user:
-            issues = github.raw_request('GET', issues_path)
+            issues = github.raw_request('GET', issues_path, params=params)
         else:
-            issues = proxy_request('get')
+            issues = proxy_request('get', params=params)
         # Do not send random JSON to filter_untriaged
         if issues.status_code == 200:
             return (filter_untriaged(json.loads(issues.content)),
@@ -163,17 +157,14 @@ def get_search_results(query_string=None):
     '''
     search_uri = 'https://api.github.com/search/issues'
     # TODO: handle sort and order parameters.
-    params = {}
+    params = request.args.copy()
 
     if query_string is None:
-        query_string = request.args.get('q')
+        query_string = params.get('q')
         # restrict results to our repo.
     query_string += " repo:{0}".format(REPO_PATH)
     params.update({'q': query_string})
 
-    if request.args.get('page'):
-        params.update({'page': request.args.get('page')})
-
     if g.user:
         request_headers = get_request_headers(g.request_headers)
         results = github.raw_request('GET', 'search/issues', params=params,

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

@@ -33,10 +33,20 @@ issueList.DropdownView = Backbone.View.extend({
   },
   selectDropdownOption: function(e) {
     var option = $(e.target);
+    var paramKey = option.data('paramKey');
+    var paramValue = option.data('paramValue');
     option.addClass('is-active')
           .siblings().removeClass('is-active');
-    // TODO: persist in localStorage for page refreshes?
+
     this.updateDropdownTitle(option);
+
+    // persist value of selection to be used on subsequent page loads
+    if ('localStorage' in window) {
+      window.localStorage.setItem(paramKey, paramValue);
+    }
+
+    // fire an event so other views can react to dropdown changes
+    wcEvents.trigger('dropdown:change', paramKey, paramValue);
     e.preventDefault();
   },
   updateDropdownTitle: function(optionElm) {
@@ -56,6 +66,7 @@ issueList.FilterView = Backbone.View.extend({
 
     issueList.events.on('filter:activate', _.bind(this.toggleFilter, this));
 
+    // TODO(miket): update with paramKey & paramValue
     var options = [
       {title: "View all open issues", params: ""},
       {title: "View all issues", params: "filter=all"}
@@ -175,14 +186,16 @@ issueList.SortingView = Backbone.View.extend({
   events: {},
   initialize: function() {
     this.paginationModel = new Backbone.Model({
-      dropdownTitle: "Show 25",
+      // TODO(miket): persist selected page limit to survive page loads
+      dropdownTitle: "Show 50",
       dropdownOptions: [
-        {title: "Show 25", params: ""},
-        {title: "Show 50", params: ""},
-        {title: "Show 100", params: ""}
+        {title: "Show 25", paramKey: "per_page", paramValue: "25"},
+        {title: "Show 50", paramKey: "per_page", paramValue: "50"},
+        {title: "Show 100", paramKey: "per_page", paramValue: "100"}
       ]
     });
 
+    // TODO(miket): update model to have paramKey and paramValue
     this.sortModel = new Backbone.Model({
       dropdownTitle: "Newest",
       dropdownOptions: [
@@ -196,19 +209,19 @@ issueList.SortingView = Backbone.View.extend({
     this.initSubViews();
   },
   initSubViews: function() {
-    /* Commenting out for now, see Issues #312, #266
     this.paginationDropdown = new issueList.DropdownView({
       model: this.paginationModel
     });
+    /* Commenting out for now, see Issues #312, #266
     this.sortDropdown = new issueList.DropdownView({
       model: this.sortModel
     }); */
   },
   template: _.template($('#issuelist-sorting-tmpl').html()),
   render: function() {
     this.$el.html(this.template());
-    /* Commenting out for now, see Issues #312, #266
     this.paginationDropdown.setElement(this.$el.find('.js-dropdown-pagination')).render();
+    /* Commenting out for now, see Issues #312, #266
     this.sortDropdown.setElement(this.$el.find('.js-dropdown-sort')).render(); */
     return this;
   }
@@ -238,20 +251,23 @@ issueList.IssueView = Backbone.View.extend({
   events: {
     'click .js-issue-label': 'labelSearch',
   },
+  _isLoggedIn: $('body').data('username'),
+  _pageLimit: null,
   initialize: function() {
     this.issues = new issueList.IssueCollection();
     // check to see if we should pre-filter results
-    this.checkParams();
+    this.loadIssues();
 
     // set up event listeners.
     issueList.events.on('issues:update', _.bind(this.updateIssues, this));
     issueList.events.on('paginate:next', _.bind(this.requestNextPage, this));
     issueList.events.on('paginate:previous', _.bind(this.requestPreviousPage, this));
+    wcEvents.on('dropdown:change', _.bind(this.updateModelParams, this));
   },
   template: _.template($('#issuelist-issue-tmpl').html()),
-  checkParams: function() {
-    // Assumes a URI like: /?untriaged=1 and activates the untriaged filter,
-    // for example.
+  loadIssues: function() {
+    // First checks URL params, e.g., /?untriaged=1 and activates the untriaged filter,
+    // or loads default unsorted/unfiltered issues
     var category;
     var filterRegex = /\?(untriaged|needsdiagnosis|contactready|sitewait|closed)=1/;
     if (category = window.location.search.match(filterRegex)) {
@@ -267,6 +283,7 @@ issueList.IssueView = Backbone.View.extend({
     }
   },
   fetchAndRenderIssues: function() {
+    //assumes this.issues.url has already been set to something meaninful.
     var headers = {headers: {'Accept': 'application/json'}};
     this.issues.fetch(headers).success(_.bind(function() {
       this.render(this.issues);
@@ -285,13 +302,15 @@ issueList.IssueView = Backbone.View.extend({
       wcEvents.trigger('flash:error', {message: message, timeout: timeout});
     });
   },
+  getPageLimit: function() {
+    return this._pageLimit;
+  },
   render: function(issues) {
     this.$el.html(this.template({
       issues: issues.toJSON()
     }));
     return this;
   },
-  _isLoggedIn: $('body').data('username'),
   initPaginationLinks: function(issues) {
     // if either the next or previous page numbers are null
     // disable the buttons and add .is-disabled classes.
@@ -332,41 +351,61 @@ issueList.IssueView = Backbone.View.extend({
     e.preventDefault();
   },
   requestNextPage: function() {
-    var newUrl;
     if (this.issues.getNextPageNumber()) {
       // chop off the last character, which is the page number
       // TODO: this feels gross. ideally we should get the URL from the
       // link header and send that to an API endpoint which then requests
       // it from GitHub.
-      newUrl = this.issues.url.slice(0,-1) + this.issues.getNextPageNumber();
-      this.issues.url = newUrl;
-      this.fetchAndRenderIssues();
+      this.updateModelParams("page", this.issues.getNextPageNumber());
     }
   },
   requestPreviousPage: function() {
-    var newUrl;
     if (this.issues.getPreviousPageNumber()) {
-      newUrl = this.issues.url.slice(0,-1) + this.issues.getPreviousPageNumber();
-      this.issues.url = newUrl;
-      this.fetchAndRenderIssues();
+      this.updateModelParams("page", this.issues.getPreviousPageNumber());
     }
   },
   updateIssues: function(category) {
     // depending on what category was clicked (or if a search came in),
     // update the collection instance url property and fetch the issues.
     var labelCategories = ['closed', 'contactready', 'needsdiagnosis', 'sitewait'];
 
+    //TODO(miket): make generic getModelParams method which can get the latest state
+    // merge param objects and serialize
+    var paramsBag = $.extend({page: 1}, this.getPageLimit());
+    var params = $.param(paramsBag);
+
     // note: if query is the empty string, it will load all issues from the
     // '/api/issues' endpoint (which I think we want).
     if (category && category.query) {
-      this.issues.url = '/api/issues/search?q=' + category.query + '&page=1';
+      params = $.param($.extend(paramsBag, {q: category.query}));
+      this.issues.url = '/api/issues/search?' + params;
     } else if (_.contains(labelCategories, category)) {
-      this.issues.url = '/api/issues/category/' + category + '?page=1';
+      this.issues.url = '/api/issues/category/' + category + '?' + params;
     } else if (category === "untriaged") {
-      this.issues.url = '/api/issues/search/untriaged?page=1';
+      this.issues.url = '/api/issues/search/untriaged?' + params;
     } else {
-      this.issues.url = '/api/issues?page=1';
+      this.issues.url = '/api/issues?' + params;
+    }
+    this.fetchAndRenderIssues();
+  },
+  updateModelParams: function(paramKey, paramValue) {
+    var modelUrl = this.issues.url.split('?');
+    var modelPath = modelUrl[0];
+    var modelParams = modelUrl[1];
+
+    var updateParams = {};
+    updateParams[paramKey] = paramValue;
+
+    // merge old params with passed in param data
+    // $.extend will update existing object keys, and add new ones
+    var newParams = $.extend($.deparam(modelParams), updateParams);
+
+    if (paramKey === 'per_page') {
+      this._pageLimit = paramValue;
     }
+
+    // construct new model URL and re-request issues
+    this.issues.url = modelPath + '?' + $.param(newParams);
     this.fetchAndRenderIssues();
   }
 });

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

@@ -120,7 +120,8 @@ issueList.Issue = issues.Issue.extend({});
 
 issueList.IssueCollection = Backbone.Collection.extend({
   model: issueList.Issue,
-  url: '/api/issues?page=1',
+  // TODO(miket): less hard-coding of default params
+  url: '/api/issues?page=1&per_page=50',
   parse: function(response, jqXHR) {
     if (jqXHR.xhr.getResponseHeader('Link') != null) {
       //external code can access the parsed header via this.linkHeader
@@ -168,7 +169,7 @@ issueList.IssueCollection = Backbone.Collection.extend({
     }
     var page;
     // we only return the page number
-    var re = new RegExp('page=(\\d)');
+    var re = new RegExp('[&?]page=(\\d)');
 
     if (page = (this.linkHeader.hasOwnProperty(relation) &&
                 this.linkHeader[relation].match(re))) {