Skip to content

Commit 84d3087

Browse files
magsoutmagsout
committed
Merge pull request #418 from webcompat/372/1
Enable issues "pagination" dropdown. Fixes #372.
2 parents 3670986 + 520961a commit 84d3087

File tree

8 files changed

+223
-42
lines changed

8 files changed

+223
-42
lines changed

‎grunt-tasks/concat.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ module.exports = function(grunt) {
77
dist: {
88
src: [
99
'<%= jsPath %>/vendor/jquery-1.11.0.min.js',
10+
'<%= jsPath %>/vendor/jquery.deparam.js',
1011
'<%= jsPath %>/vendor/lodash.underscore-min.js',
1112
'<%= jsPath %>/vendor/backbone-min.js',
1213
'<%= jsPath %>/vendor/moment-min.js',

‎tests/functional/lib/issue-list.js

+35
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,41 @@ define([
8787
assert.include(className, 'is-disabled', 'Going back from first next click should have disabled prev button');
8888
})
8989
.end();
90+
},
91+
92+
'pagination dropdown tests': function() {
93+
return this.remote
94+
.setFindTimeout(intern.config.wc.pageLoadTimeout)
95+
.get(require.toUrl(url))
96+
.findByCssSelector('.js-dropdown-pagination').isDisplayed()
97+
.then(function (isDisplayed) {
98+
assert.equal(isDisplayed, true, 'pagination dropdown container is visible.');
99+
})
100+
.end()
101+
.findByCssSelector('.js-dropdown-pagination .js-dropdown-toggle').click()
102+
.end()
103+
.findByCssSelector('.js-dropdown-pagination').getAttribute('class')
104+
.then(function (className) {
105+
assert.include(className, 'is-active', 'clicking dropdown adds is-active class');
106+
})
107+
.end()
108+
.findByCssSelector('.js-dropdown-pagination .js-dropdown-options').isDisplayed()
109+
.then(function (isDisplayed) {
110+
assert.equal(isDisplayed, true, 'dropdown options are visible.');
111+
})
112+
.end()
113+
.findByCssSelector('.js-dropdown-pagination li.Dropdown-item:nth-child(3) > a:nth-child(1)').click()
114+
.end()
115+
.findByCssSelector('.js-dropdown-pagination .Dropdown-label').getVisibleText()
116+
.then(function (text) {
117+
assert.include(text, 'Show 100', 'Clicking first option updated dropdown label');
118+
})
119+
.end()
120+
.findByCssSelector('.IssueItem:nth-child(51)').isDisplayed()
121+
.then(function (isDisplayed) {
122+
assert.equal(isDisplayed, true, 'More than 50 issues were loaded.');
123+
})
124+
.end();
90125
}
91126
});
92127
});

‎webcompat/api/endpoints.py

+6-15
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,7 @@ def edit_issue(number):
6767
@api.route('/issues')
6868
def proxy_issues():
6969
'''API endpoint to list all issues from GitHub.'''
70-
if request.args.get('page'):
71-
params = {'page': request.args.get('page')}
72-
else:
73-
params = None
70+
params = request.args.copy()
7471

7572
if g.user:
7673
issues = github.raw_request('GET', 'repos/{0}'.format(ISSUES_PATH),
@@ -105,12 +102,9 @@ def get_issue_category(issue_category):
105102
* needsdiagnosis
106103
* sitewait
107104
'''
108-
params = {}
109105
category_list = ['contactready', 'needsdiagnosis', 'sitewait']
110106
issues_path = 'repos/{0}'.format(ISSUES_PATH)
111-
112-
if request.args.get('page'):
113-
params.update({'page': request.args.get('page')})
107+
params = request.args.copy()
114108

115109
if issue_category in category_list:
116110
params.update({'labels': issue_category})
@@ -128,9 +122,9 @@ def get_issue_category(issue_category):
128122
# For paginated results on the /issues page, see /issues/search/untriaged.
129123
elif issue_category == 'untriaged':
130124
if g.user:
131-
issues = github.raw_request('GET', issues_path)
125+
issues = github.raw_request('GET', issues_path, params=params)
132126
else:
133-
issues = proxy_request('get')
127+
issues = proxy_request('get', params=params)
134128
# Do not send random JSON to filter_untriaged
135129
if issues.status_code == 200:
136130
return (filter_untriaged(json.loads(issues.content)),
@@ -163,17 +157,14 @@ def get_search_results(query_string=None):
163157
'''
164158
search_uri = 'https://api.github.com/search/issues'
165159
# TODO: handle sort and order parameters.
166-
params = {}
160+
params = request.args.copy()
167161

168162
if query_string is None:
169-
query_string = request.args.get('q')
163+
query_string = params.get('q')
170164
# restrict results to our repo.
171165
query_string += " repo:{0}".format(REPO_PATH)
172166
params.update({'q': query_string})
173167

174-
if request.args.get('page'):
175-
params.update({'page': request.args.get('page')})
176-
177168
if g.user:
178169
request_headers = get_request_headers(g.request_headers)
179170
results = github.raw_request('GET', 'search/issues', params=params,

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

+63-24
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,20 @@ issueList.DropdownView = Backbone.View.extend({
3333
},
3434
selectDropdownOption: function(e) {
3535
var option = $(e.target);
36+
var paramKey = option.data('paramKey');
37+
var paramValue = option.data('paramValue');
3638
option.addClass('is-active')
3739
.siblings().removeClass('is-active');
38-
// TODO: persist in localStorage for page refreshes?
40+
3941
this.updateDropdownTitle(option);
42+
43+
// persist value of selection to be used on subsequent page loads
44+
if ('localStorage' in window) {
45+
window.localStorage.setItem(paramKey, paramValue);
46+
}
47+
48+
// fire an event so other views can react to dropdown changes
49+
wcEvents.trigger('dropdown:change', paramKey, paramValue);
4050
e.preventDefault();
4151
},
4252
updateDropdownTitle: function(optionElm) {
@@ -56,6 +66,7 @@ issueList.FilterView = Backbone.View.extend({
5666

5767
issueList.events.on('filter:activate', _.bind(this.toggleFilter, this));
5868

69+
// TODO(miket): update with paramKey & paramValue
5970
var options = [
6071
{title: "View all open issues", params: ""},
6172
{title: "View all issues", params: "filter=all"}
@@ -175,14 +186,16 @@ issueList.SortingView = Backbone.View.extend({
175186
events: {},
176187
initialize: function() {
177188
this.paginationModel = new Backbone.Model({
178-
dropdownTitle: "Show 25",
189+
// TODO(miket): persist selected page limit to survive page loads
190+
dropdownTitle: "Show 50",
179191
dropdownOptions: [
180-
{title: "Show 25", params: ""},
181-
{title: "Show 50", params: ""},
182-
{title: "Show 100", params: ""}
192+
{title: "Show 25", paramKey: "per_page", paramValue: "25"},
193+
{title: "Show 50", paramKey: "per_page", paramValue: "50"},
194+
{title: "Show 100", paramKey: "per_page", paramValue: "100"}
183195
]
184196
});
185197

198+
// TODO(miket): update model to have paramKey and paramValue
186199
this.sortModel = new Backbone.Model({
187200
dropdownTitle: "Newest",
188201
dropdownOptions: [
@@ -196,19 +209,19 @@ issueList.SortingView = Backbone.View.extend({
196209
this.initSubViews();
197210
},
198211
initSubViews: function() {
199-
/* Commenting out for now, see Issues #312, #266
200212
this.paginationDropdown = new issueList.DropdownView({
201213
model: this.paginationModel
202214
});
215+
/* Commenting out for now, see Issues #312, #266
203216
this.sortDropdown = new issueList.DropdownView({
204217
model: this.sortModel
205218
}); */
206219
},
207220
template: _.template($('#issuelist-sorting-tmpl').html()),
208221
render: function() {
209222
this.$el.html(this.template());
210-
/* Commenting out for now, see Issues #312, #266
211223
this.paginationDropdown.setElement(this.$el.find('.js-dropdown-pagination')).render();
224+
/* Commenting out for now, see Issues #312, #266
212225
this.sortDropdown.setElement(this.$el.find('.js-dropdown-sort')).render(); */
213226
return this;
214227
}
@@ -238,20 +251,23 @@ issueList.IssueView = Backbone.View.extend({
238251
events: {
239252
'click .js-issue-label': 'labelSearch',
240253
},
254+
_isLoggedIn: $('body').data('username'),
255+
_pageLimit: null,
241256
initialize: function() {
242257
this.issues = new issueList.IssueCollection();
243258
// check to see if we should pre-filter results
244-
this.checkParams();
259+
this.loadIssues();
245260

246261
// set up event listeners.
247262
issueList.events.on('issues:update', _.bind(this.updateIssues, this));
248263
issueList.events.on('paginate:next', _.bind(this.requestNextPage, this));
249264
issueList.events.on('paginate:previous', _.bind(this.requestPreviousPage, this));
265+
wcEvents.on('dropdown:change', _.bind(this.updateModelParams, this));
250266
},
251267
template: _.template($('#issuelist-issue-tmpl').html()),
252-
checkParams: function() {
253-
// Assumes a URI like: /?untriaged=1 and activates the untriaged filter,
254-
// for example.
268+
loadIssues: function() {
269+
// First checks URL params, e.g., /?untriaged=1 and activates the untriaged filter,
270+
// or loads default unsorted/unfiltered issues
255271
var category;
256272
var filterRegex = /\?(untriaged|needsdiagnosis|contactready|sitewait|closed)=1/;
257273
if (category = window.location.search.match(filterRegex)) {
@@ -267,6 +283,7 @@ issueList.IssueView = Backbone.View.extend({
267283
}
268284
},
269285
fetchAndRenderIssues: function() {
286+
//assumes this.issues.url has already been set to something meaninful.
270287
var headers = {headers: {'Accept': 'application/json'}};
271288
this.issues.fetch(headers).success(_.bind(function() {
272289
this.render(this.issues);
@@ -285,13 +302,15 @@ issueList.IssueView = Backbone.View.extend({
285302
wcEvents.trigger('flash:error', {message: message, timeout: timeout});
286303
});
287304
},
305+
getPageLimit: function() {
306+
return this._pageLimit;
307+
},
288308
render: function(issues) {
289309
this.$el.html(this.template({
290310
issues: issues.toJSON()
291311
}));
292312
return this;
293313
},
294-
_isLoggedIn: $('body').data('username'),
295314
initPaginationLinks: function(issues) {
296315
// if either the next or previous page numbers are null
297316
// disable the buttons and add .is-disabled classes.
@@ -332,41 +351,61 @@ issueList.IssueView = Backbone.View.extend({
332351
e.preventDefault();
333352
},
334353
requestNextPage: function() {
335-
var newUrl;
336354
if (this.issues.getNextPageNumber()) {
337355
// chop off the last character, which is the page number
338356
// TODO: this feels gross. ideally we should get the URL from the
339357
// link header and send that to an API endpoint which then requests
340358
// it from GitHub.
341-
newUrl = this.issues.url.slice(0,-1) + this.issues.getNextPageNumber();
342-
this.issues.url = newUrl;
343-
this.fetchAndRenderIssues();
359+
this.updateModelParams("page", this.issues.getNextPageNumber());
344360
}
345361
},
346362
requestPreviousPage: function() {
347-
var newUrl;
348363
if (this.issues.getPreviousPageNumber()) {
349-
newUrl = this.issues.url.slice(0,-1) + this.issues.getPreviousPageNumber();
350-
this.issues.url = newUrl;
351-
this.fetchAndRenderIssues();
364+
this.updateModelParams("page", this.issues.getPreviousPageNumber());
352365
}
353366
},
354367
updateIssues: function(category) {
355368
// depending on what category was clicked (or if a search came in),
356369
// update the collection instance url property and fetch the issues.
357370
var labelCategories = ['closed', 'contactready', 'needsdiagnosis', 'sitewait'];
358371

372+
//TODO(miket): make generic getModelParams method which can get the latest state
373+
// merge param objects and serialize
374+
var paramsBag = $.extend({page: 1}, this.getPageLimit());
375+
var params = $.param(paramsBag);
376+
359377
// note: if query is the empty string, it will load all issues from the
360378
// '/api/issues' endpoint (which I think we want).
361379
if (category && category.query) {
362-
this.issues.url = '/api/issues/search?q=' + category.query + '&page=1';
380+
params = $.param($.extend(paramsBag, {q: category.query}));
381+
this.issues.url = '/api/issues/search?' + params;
363382
} else if (_.contains(labelCategories, category)) {
364-
this.issues.url = '/api/issues/category/' + category + '?page=1';
383+
this.issues.url = '/api/issues/category/' + category + '?' + params;
365384
} else if (category === "untriaged") {
366-
this.issues.url = '/api/issues/search/untriaged?page=1';
385+
this.issues.url = '/api/issues/search/untriaged?' + params;
367386
} else {
368-
this.issues.url = '/api/issues?page=1';
387+
this.issues.url = '/api/issues?' + params;
388+
}
389+
this.fetchAndRenderIssues();
390+
},
391+
updateModelParams: function(paramKey, paramValue) {
392+
var modelUrl = this.issues.url.split('?');
393+
var modelPath = modelUrl[0];
394+
var modelParams = modelUrl[1];
395+
396+
var updateParams = {};
397+
updateParams[paramKey] = paramValue;
398+
399+
// merge old params with passed in param data
400+
// $.extend will update existing object keys, and add new ones
401+
var newParams = $.extend($.deparam(modelParams), updateParams);
402+
403+
if (paramKey === 'per_page') {
404+
this._pageLimit = paramValue;
369405
}
406+
407+
// construct new model URL and re-request issues
408+
this.issues.url = modelPath + '?' + $.param(newParams);
370409
this.fetchAndRenderIssues();
371410
}
372411
});

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ issueList.Issue = issues.Issue.extend({});
120120

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

173174
if (page = (this.linkHeader.hasOwnProperty(relation) &&
174175
this.linkHeader[relation].match(re))) {

0 commit comments

Comments
 (0)