Make API responses consistent

[sethvincent]

There are a few examples of inconsistent api responses.

We should do a look through and see if there are other similar inconsistencies, make a list here, and try to update them all at once. These could later be broken into separate issues if needed.

List:

• /api/teams 200 get response is an array of objects, while /api/clients 200 get response is an object with a clients property which is an array of objects
• user OSM id should always be named the same thing and be either a string or an integer. for instance, in the individual team response in the member array the id is a string and represents the osm id but in the moderator array both id and osm_id exist and both are integers
• catch errors in route handlers and provide specific, readable errors instead of whatever the db or other dependency is throwing, for example instead of "insert into \"team\" (\"bio\", \"hashtag\", \"name\") values ($1, $2, $3) returning *, ST_asGeoJSON(\"location\") as \"location\" - duplicate key value violates unique constraint \"team_hashtag_unique\"" we should say something more clear like The hashtag `hashtagname` is already taken. (obvs hashtag will be replaced by tags but this is just an example)

[vgeorge]

In addition to the above, I believe all API responses should return a JSON object on success and error for consistency. There is a number of endpoints returning just a string "OK".

[vgeorge]

Tagging this as a high priority bug, most endpoints are returning the db query as described above. The API must return a message that doesn't expose the queries.

[vgeorge]

Status update on the items listed by Seth:

1. We did changes to some endpoints to provide pagination and make responses more consistent. But there are endpoints like /teams that are used by different pages and refactoring those pages doesn't look straightforward at the moment. One possible approach to avoid breaking stuff is to add a paginated param to all list routes that should have pagination. If this parameter is not present or equal to false, the route would return results as in version 1.

2. A possible approach for avoiding type errors in responses is adding a validation middleware that checks res.body after executing the route logic. This should be fairly easy to accomplish by using Yup schemas, but applying schemas for all routes might be a lot of work. For the moment we should try adding more test coverage with type-checking for routes that might be problematic.

3. We need to update all occurrences of throw Boom.badRequest(err.message) (like in here) with an appropriate error message like throw Boom.badRequest('Team name is already taken').

cc @batpad @kamicut @willemarcel @LanesGood