Implemented @mojodna suggestions

posm · Dec 6, 2016 · 8302148 · mojodna · Dec 6, 2016 · hallahan
1 parent 86f704f
commit 8302148
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 39 deletions.
diff --git a/api/odk/controllers/get-csv-submissions.js b/api/odk/controllers/get-csv-submissions.js
@@ -8,14 +8,17 @@ var json2csv = require('json2csv');
  */
 module.exports = function (req, res, next) {
 
-    aggregate(req, errorCallback, aggregateCallback);
-
-    function errorCallback(err) {
-        res.status(err.status).json(err);
-    }
-
-    function aggregateCallback(aggregate) {
-
+    var opts = {
+        formName: req.params.formName,
+        limit: req.query.limit,
+        offset: req.query.offset
+    };
+
+    aggregate(opts, errorCallback, function (err, aggregate) {
+        if (err) {
+            res.status(err.status).json(err);
+            return;
+        }
         try {
             var csv = json2csv({
                 data: aggregate,
@@ -33,8 +36,7 @@ module.exports = function (req, res, next) {
                 err: err
             });
         }
-
-    }
+    });
 
 };
 
diff --git a/api/odk/controllers/get-json-submissions.js b/api/odk/controllers/get-json-submissions.js
@@ -7,14 +7,18 @@ var aggregate = require('../helpers/aggregate-submissions');
  */
 module.exports = function (req, res, next) {
 
-    aggregate(req, errorCallback, aggregateCallback);
+    var opts = {
+        formName: req.params.formName,
+        limit: req.query.limit,
+        offset: req.query.offset
+    };
 
-    function errorCallback(err) {
-        res.status(err.status).json(err);
-    }
-
-    function aggregateCallback(aggregate) {
+    aggregate(opts, function(err, aggregate) {
+        if (err) {
+            res.status(err.status).json(err);
+            return;
+        }
         res.status(200).json(aggregate);
-    }
+    });
 
 };
diff --git a/api/odk/helpers/aggregate-submissions.js b/api/odk/helpers/aggregate-submissions.js
@@ -5,21 +5,21 @@ var async = require('async');
 
 var ASYNC_LIMIT = 10;
 
-module.exports = function (req, errorCallback, aggregateCallback) {
-    var formName = req.params.formName;
-    var limit = parseInt(req.query.limit);
-    var offset = req.query.offset;
+module.exports = function (opts, cb) {
+    var formName = opts.formName;
+    var limit = parseInt(opts.limit);
+    var offset = opts.offset;
 
     // default to 100 for limit
-    if (isNaN(limit) || !Number.isInteger(limit) || limit < 1) {
+    if (isNaN(limit) || limit < 1) {
         limit = 100;
     }
 
     // check if valid offset. we paginate with valid offset.
-    if (typeof offset !== 'undefined' && offset !== null) {
+    if (offset != null) {
         offset = parseInt(offset);
         if (isNaN(offset) || !Number.isInteger(offset) || offset < 0) {
-            errorCallback({
+            cb({
                 status: 400,
                 err: 'BAD_OFFSET',
                 msg: 'If you specify an offset for pagination, it must be an integer greater than 0.',
@@ -31,8 +31,8 @@ module.exports = function (req, errorCallback, aggregateCallback) {
     }
     // otherwise we don't have an offset
 
-    if (typeof formName === 'undefined' || formName === null) {
-        errorCallback({
+    if (formName == null) {
+        cb({
             status: 400,
             err: 'MISSING_PARAM',
             msg: 'You must specify a parameter for the formName in this end point.',
@@ -48,14 +48,14 @@ module.exports = function (req, errorCallback, aggregateCallback) {
         if (err) {
             if (err.errno === -2) {
                 // trying to open a directory that is not there.
-                errorCallback({
+                cb({
                     status: 404,
                     msg: 'You are trying to aggregate the ODK submissions for a form that has no submissions. Please submit at least one survey to see data. Also, check to see if you spelled the form name correctly. Form name: ' + formName,
                     err: err
                 });
                 return;
             }
-            errorCallback({
+            cb({
                 status: 500,
                 msg: 'Problem reading submissions directory.',
                 err: err
@@ -64,16 +64,10 @@ module.exports = function (req, errorCallback, aggregateCallback) {
         }
 
         // if offset, we do pagination
-        if (typeof offset !== 'undefined' && offset !== null) {
+        if (offset != null) {
             submissionDirs = submissionDirs.slice(offset, offset + limit);
         }
 
-        // no results
-        if (submissionDirs.length === 0) {
-            aggregateCallback([]);
-            return;
-        }
-
         async.eachLimit(submissionDirs, ASYNC_LIMIT, function (submissionDir, callback) {
             // If it's not a directory, we just skip processing that path.
             if (submissionDir[0] === '.' || submissionDir.indexOf('.txt') > 0) {
@@ -91,7 +85,7 @@ module.exports = function (req, errorCallback, aggregateCallback) {
                     callback(); // ok submission
                 });
             } catch(e) {
-                callback({
+                cb({
                     status: 500,
                     msg: 'Problem reading data.json file in submission directory. dataFile: ' + dataFile,
                     err: e
@@ -100,11 +94,11 @@ module.exports = function (req, errorCallback, aggregateCallback) {
         }, function (err) {
             // an error occurred...
             if (err) {
-                errorCallback(err);
+                cb(err);
             }
             // it was a success
             else {
-                aggregateCallback(aggregate);
+                cb(null, aggregate);
             }
         });
     });

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "open-map-kit-server",
-  "version": "0.8.0",
+  "version": "0.7.0",
   "description": "OpenMapKit Server is the lightweight server component of OpenMapKit that handles the collection and aggregation of OpenStreetMap and OpenDataKit data.",
   "main": "index.js",
   "scripts": {