API Pagination & Submissions Page Performance #97
Conversation
|
You mention both 500 and 5000? |
|
5000, not 500 |
| var offset = req.query.offset; | ||
|
|
||
| // default to 100 for limit | ||
| if (isNaN(limit) || !Number.isInteger(limit) || limit < 1) { |
There was a problem hiding this comment.
When will Number.isInteger be false? I think parseInt either returns a number or NaN.
There was a problem hiding this comment.
Good point. Will remove.
| } | ||
|
|
||
| // check if valid offset. we paginate with valid offset. | ||
| if (typeof offset !== 'undefined' && offset !== null) { |
There was a problem hiding this comment.
offset != null is a good way to check for both null and undefined values.
| @@ -7,7 +7,7 @@ var aggregate = require('../helpers/aggregate-submissions'); | |||
| */ | |||
| module.exports = function (req, res, next) { | |||
|
|
|||
| aggregate(req.params.formName, errorCallback, aggregateCallback); | |||
| aggregate(req, errorCallback, aggregateCallback); | |||
There was a problem hiding this comment.
What's the background on the paired callbacks? Convention would be a single callback with err, data, data2, ... args.
There was a problem hiding this comment.
Good point. Will do.
| @@ -5,9 +5,34 @@ var async = require('async'); | |||
|
|
|||
| var ASYNC_LIMIT = 10; | |||
|
|
|||
| module.exports = function (formName, errorCallback, aggregateCallback) { | |||
| module.exports = function (req, errorCallback, aggregateCallback) { | |||
There was a problem hiding this comment.
Since this is a "helper", I would keep req out of the signature and move limit and offset args just before the callback(s), allowing it to be more cleanly used elsewhere.
| @@ -36,6 +62,13 @@ module.exports = function (formName, errorCallback, aggregateCallback) { | |||
| }); | |||
| return; | |||
| } | |||
|
|
|||
| // if offset, we do pagination | |||
| if (typeof offset !== 'undefined' && offset !== null) { | |||
There was a problem hiding this comment.
Same thing? This is a convention I adopted years ago from what the Coffeescript compiler would do.
| submissionDirs = submissionDirs.slice(offset, offset + limit); | ||
| } | ||
|
|
||
| // no results | ||
| if (submissionDirs.length === 0) { | ||
| aggregateCallback([]); |
There was a problem hiding this comment.
No need to short-circuit, since submissionDirs == [] at this point, and eachLimit will skip straight to the callback.
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "open-map-kit-server", | |||
| "version": "0.7.0", | |||
| "version": "0.8.0", | |||
There was a problem hiding this comment.
This probably shouldn't be bumped in a pull request, preferring to tag a release using npm version ... off of the primary branch once it's ready.
|
👍 |
|
There is something wrong with Github. This PR was indeed merged. For a few minutes, Github was giving me 500 notices, and now this seems to be an inconsistent state. You will see in Git that this has been merged into master. So, closing... |
I've made a couple of improvements that allows the UI to get submissions for forms with a large number of responses.
API Pagination
I've added pagination to the REST API. I'm using the most common nomenclature of using query params of
limitandoffset. For example:This will get me 10 submissions starting at the index of 25. Responses are sorted alphabetically by the UUID.
Submissions Page Fetch Limit
Right now, I've limited the submissions page to fetch at maximum 5000 submissions to be shown in the table. Any more takes a bit too long. I experimented with iteratively paginating more data into the JQuery Datatable, but this nonetheless needs to modify the DOM, so it can't be done in a thread.
We easily hit the limitations of working in the browser. The right solution would be to implement pagination of a table with the REST API. I'm not seeing a straightforward way of doing this with JQuery Datatables, so we'd want to go with something else.
This gives room for a good smaller project of revamping the submissions table UI. Perhaps a React project?
So, right now, if the number of responses is over 5000, we just get 5000 and notify the user in a toast at the bottom. You can get the complete dataset by downloading the JSON, CSV, or OSM. Also note that the CSV download actually hits the server's CSV endpoint instead of having the browser do the work.