localdata/localdata-api

API/server for LocalData

localdata-api's Issues

CRUD should be tested

Nearly all database schema changes should include a migration

If we make a change to a model, we should also write a corresponding migration that will update all relevant records. That way, we'll have to worry much less about making those changes since we'll know that they're consistent. The migration should be tested (we can create a sample test-- that will be an issue if we go forward).

Some model changes (eg adding a field that will be empty by default) won't require a migration, so this doesn't apply in those cases.

cc @prashtx

Tests are missing SQL fixtures for objects table

We have SQL for the features tests, but not for the objects (aka the /parcels endpoint) tests.

Start a new survey version

There can be different versions of a survey -- 2012, 2013, 2014 (time-based).

Add a public/private flag to surveys

Track user terms agreement

The User model should track if the user has agreed to the terms.

We should know the date they agreed to and the version they agreed to.

SESSION_SECRET isn't used?

Looks like we use settings.SECRET instead of settings.SESSION_SECRET, but the latter is still around. We should clarify that.

CSV: a column name has a space

We got a report that a column in the vanguard CSV export has a space in it. Is that true, and if so, why? / fix it!

Delete

Need to deactivate the delete endpoint and integrate with auth

tileBase isn't in settings.js or test.settings.js

Readme doesn't accurately describe how to get server running locally with https

What I did:

• Created certificates per readme
• Double-clicked the cert and added it to keychain as trusted
• Restarted Chrome
• Ran PORT=3443 bin/fakeroku 3000
• Went to https://localhost:3443/
• Got this error: An error has occurred: {"code":"ECONNREFUSED","errno":"ECONNREFUSED","syscall":"connect"}
  (no logs from fakeroku)
• Started lib/server.js while fakeroku was running
• Now I get a 500 at https://localhost:3443/, but neither server nor fakeroku log anything
• https://localhost:3000 gives a certificate error (`Error 107 (net::ERR_SSL_PROTOCOL_ERROR): SSL protocol error.)

Can the server even be accessed over plain http? If not, the README should be updated to reflect this requirement.

We should include a sample schema for the database

Allow queries by object ID

Imagine you want to get all responses for a parcel.

I'm envisioning GET /surveys/:id:/objects/:object_id:

Can we simplify our collection names?

We call them responseCollection, surveyCollection, etc. Can we simplify that be removing Collection for consistency and style?

Responses should be in geojson format

With this, we'll be able to serve geodata in a consistent format. This'll also let us take advantage of 2dsphere indexes in Mongo 2.4, which support more geometries.

This will require changes to the dashboard and mobile client.

From the GeoJSON spec:

• A feature object must have a member with the name "geometry". The value of the geometry member is a geometry object as defined above or a JSON null value.
• A feature object must have a member with the name "properties". The value of the properties member is an object (any JSON object or a JSON null value).
• If a feature has a commonly used identifier, that identifier should be included as a member of the feature object with the name "id".

From the MongoDB 2dsphere index docs:

{ <location field> : { type : "<GeoJSON type>" ,
                       coordinates : <coordinates>
} } 

Shapefile export for Vanguard is not working

Haven't confirmed yet, just adding this to follow up later.

Let users update responses.

Think this is best built in as .put

We'd need to ensure that the user owns the survey to edit.

Use winston for testing?

Should we use winston for logging? Per @bnchdrff's suggestion in #9

Try combing response count with bounds calculation

The response count and response bounds seem to be two of our slower computations, and we do them both when we get a survey object. We can probably calculate the count in the same aggregation operation that computes the bounds. That ought to save a scan.

KML/CSV export breaks for large response sets

If we ask Mongoose for very large result sets, we encounter a parseError. For the .../responses and .../responses.geojson endpoints, we require the client to specify a range. For CSV and KML export, we might need to request the data from the database in chunks.

Ideally, we can stream the data in and out, so that we don't risk creating extremely large data structures in memory.

Delete

Users should be able to delete a survey. For now, let's have a DEL to /surveys/[id] just email us or something equally simple. We'll just tell them that deletions take 24-48 hours to complete.

Don't log the word "error" unless we really mean it

We log an error when someone tries to log in with an incorrect password or a username that doesn't exist. That error generates an e-mail alert, even though the event is not a big deal.

Let's log a warning, or just a statement about invalid credentials, since it's not really an exceptional situation.

Add object_id query to /responses

Let's say you have multiple surveys about the same object. It would be nice to get all of the related data about that object in feed.

Proposal:

/api/responses?object_id=123 would return every response for that object_id, ordered by recency. It would only show responses for surveys you have permission to see. It could also include ancillary data about the surveys represented.

Sample data format:

{
  responses: [...],
  surveys: { ... keyed by ID ...}
}

Tests do not deal with order of results well

We don't enforce any ordering on orgs. When we create two orgs in the tests, we verify by matching the results up with our original array. Since the order could be different, we sometimes fail the tests.

We should sort the original array and the API response, to confirm that each submitted org has been properly created.

Use foreman to load appropriate environment variables for test/development

Right now we export environment variables to the shell before starting the server for local development. The settings.js file pulls from those environment variables. For safety and ease of running tests, we use run tests with a separate settings-test.js, which applies some hard-coded values instead of relying on the environment for everything.

This causes a complication in the code, because we need a single point where we choose a settings file, and then we need to pass those settings around to modules that rely on them. If we relied entirely on the environment, we could just require('./settings') wherever we need access to configuration values.

This is less of an issue with maintenance scripts, but we still risk using incorrect values from the environment and making changes to the wrong database.

I propose that we never export config values into the shell environment. Instead, we can have a set of local environment files: local.env, test.env, dev.env, remote-mongo.env, demo.env, etc. The test.env file can possibly be checked in, if we can avoid adding API keys to it. Then we can use commands like foreman run -e test.env ... to run our tests, and foreman run -e local.env maintenance/ensureStructure.js to ensure indexes on the local development db. We can potentially set the maintenance/utility scripts up as executables that take/require an argument specifying the environment file.

@hampelm: thoughts, comments?

Stats should only show most recent results

People expect that the stats reflect the current stat of the dataset -- eg only the newest result for each feature. Otherwise we get an inaccurate double-counting of some features.

Enforce nicer column names

Column names based off the question are confusing. We should have strong defaults and make it possible to change bad names.

Shapefile export: geo_info and source columns store JSON

We can cut geo_info and source should be mobile.

Show different response details based on public/private flag

If a survey is marked private and a user isn't an owner, only show truncated survey results.

Otherwise, send full results.

Survey model should accept geocoded survey location

But it shouldn't be required.

Shapefile export: missing 'collector' column

The collector's name should be a column separate in shapefile export. It's currently stored as json in the source column.

Export: shapefile includes json in the geo_info, source columns

Instead, columns should be named and contain address (for object name -- general enough for now, I think?) and collector (name of the collector)

Human readable name should be in export

Re-add "export most recent" feature

(if it has been disabled)

Add tests for the tile server proxied paths

It's bleeding edge right now, but we should only use that as an excuse for a little bit ;)

Users should be able to update their lists

And only their lists.

Users should be able to delete their lists

And only their lists.

Users should only see their own lists

Export to Excel

We should be able to use the Open XML SDK for Javascript to export directly to an Excel XLSX file.

Create a stay-awake endpoint

Let's create an API method that initiates some very minimal interaction with the MongoDB server. Then we can use Pingdom or similar to keep the app<-->mongo connection warm.

Ensure users can't edit forms if they don't own the survey

They should still be able to retrieve the forms .

Don't send details for private surveys

If a survey is private and a user doesn't have permission to access it, send a 403.

error from large requests renders the API unresponsive

When we request very large data sets through Mongoose (~ 25k responses, for example), we encounter Error: connection closed due to parseError. The app does not crash, but it looks like we no longer have database connectivity.

We should try to recover the connection. If we can't, then we should crash the app.

Create a mongoose model for lists

There are two settings files

/settings.js and /lib/settings.js

Something is causing a parseError from mongo code

Possibly a very large request. We should not allow clients to make a request that will cause problems for the API. For getting raw responses, we force the client to use paging parameters. We might need to enforce an upper limit on the count parameter.

Export date format request

We have a request to get the datetime in the format 2013/07/15 13:18:24 aka YYYY/MM/DD HH:MM:SS. That's arguably better for machine processing than our current format, which is Fri Mar 08 2013 16:58:55 GMT+0000 (UTC).

The open question -- how do we represent time zones appropriately? Ack!

What happened to our sample environment file?

new dev: tests fail

What I did:

• checked out localdata-api
• installed mongodb from ubuntu 12.04 defualt repo
• installed postgis from ppa:ubuntugis/ppa
• created postgres db and user, granted access, executed CREATE EXTENSION postgis and CREATE EXTENSION postgis_topology in new db as pg user
• edited setenv_local.sh
• ran ./node_modules/mocha/bin/mocha --ui tdd --reporter spec

What I got:

... skip ...
  ✖ 10 of 83 tests failed:

  1) Email Send an email:
     AssertionError: expected [Error: Email failed: 400
<ErrorResponse xmlns="http://ses.amazonaws.com/doc/2010-12-01/">
  <Error>
    <Type>Sender</Type>
    <Code>MessageRejected</Code>
    <Message>Email address is not verified.</Message>
  </Error>
  <RequestId>3f862dca-23f3-11e3-b1d5-61160a494759</RequestId>
</ErrorResponse>
] to not exist
   expected [Error: Email failed: 400
  <ErrorResponse xmlns="http://ses.amazonaws.com/doc/2010-12-01/">
    <Error>
      <Type>Sender</Type>
      <Code>MessageRejected</Code>
      <Message>Email address is not verified.</Message>
    </Error>
    <RequestId>3f862dca-23f3-11e3-b1d5-61160a494759</RequestId>
  </ErrorResponse>
  ] to not exist
      at /home/bc/Desktop/codez/localdata/localdata-api/test/email.js:21:18
      at /home/bc/Desktop/codez/localdata/localdata-api/lib/email.js:34:5
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/nodemailer/lib/engines/ses.js:130:17)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  2) Shapefile GET shapfile for a survey:
     AssertionError: expected 500 to equal 202
   expected 500 to equal 202
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/export-shapefile.js:119:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  3) Parcels GET Get parcels inside a bounding box:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:80:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  4) Parcels GET Get GeoJSON parcels inside a bounding box:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:101:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  5) Parcels GET Get parcels at a point:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:143:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  6) Parcels GET Point query with no results:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:179:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  7) Parcels GET Bounding box query with no results:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:197:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  8) Parcels GET ETag and 304 response:
     AssertionError: expected 500 to equal 200
   expected 500 to equal 200
      at Object.Assertion.equal (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/should/lib/should.js:303:10)
      at Request._callback (/home/bc/Desktop/codez/localdata/localdata-api/test/parcels.js:217:36)
      at Request.self.callback (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:148:22)
      at Request.EventEmitter.emit (events.js:98:17)
      at Request.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:876:14)
      at Request.EventEmitter.emit (events.js:117:20)
      at IncomingMessage.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/node_modules/request/index.js:827:12)
      at IncomingMessage.EventEmitter.emit (events.js:117:20)
      at _stream_readable.js:883:14
      at process._tickCallback (node.js:415:13)

  9) Responses POST Posting a file to /surveys/123/responses:
     Error: Callback was already called.
      at /home/bc/Desktop/codez/localdata/localdata-api/node_modules/async/lib/async.js:22:31
      at ClientRequest.<anonymous> (/home/bc/Desktop/codez/localdata/localdata-api/lib/controllers/responses.js:675:13)
      at ClientRequest.EventEmitter.emit (events.js:117:20)
      at CleartextStream.socketErrorListener (http.js:1487:9)
      at CleartextStream.EventEmitter.emit (events.js:95:17)
      at Socket.onerror (tls.js:1355:17)
      at Socket.EventEmitter.emit (events.js:117:20)
      at onwriteError (_stream_writable.js:223:10)
      at onwrite (_stream_writable.js:241:5)
      at WritableState.onwrite (_stream_writable.js:92:5)
      at fireErrorCallbacks (net.js:423:13)
      at Socket._destroy (net.js:457:3)
      at Object.afterWrite (net.js:700:10)

  10) Static Mobile client "before all" hook:
     Error: connect ECONNREFUSED
      at errnoException (net.js:883:11)
      at Object.afterConnect [as oncomplete] (net.js:874:19)

Check survey stats endpoint for a large data set

We've run into issues requesting large response sets all at once (> 10k, I believe). Most of our code fetches responses in chunks, but the stats code gets the entire set.

Can you test this against a very large data set (20k or 30k), to see if we run into any low-level mongodb issues?

If that works ok, can you confirm that we don't run into memory issues on Heroku? If it's a performance issue, then it's not super critical. But if there's a chance of a crash, then we should adjust the code.

/cc @hampelm