Open sethvincent opened 5 years ago
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".
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.
Status update on the items listed by Seth:
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.
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.
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
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 aclients
property which is an array of objects"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 likeThe hashtag `hashtagname` is already taken
. (obvshashtag
will be replaced bytags
but this is just an example)