hotfix update route, add morgan and error logging line #84
Conversation
paolobueno
requested review from
austincunningham,
JameelB,
witmicko and
wtrocki
as code owners
| @@ -95,6 +95,12 @@ export class ApiController<T> { | |||
| return Bluebird.reject(error); | |||
| } | |||
|
|
|||
| req.body.id = req.body.id || req.params.id; | |||
| if (!req.body) { | |||
There was a problem hiding this comment.
Do you mean if (!req.body.id) {?
| @@ -95,6 +95,12 @@ export class ApiController<T> { | |||
| return Bluebird.reject(error); | |||
| } | |||
|
|
|||
| req.body.id = req.body.id || req.params.id; | |||
There was a problem hiding this comment.
I'm not sure we should be flexible here. Which way do we prefer?
Documentation states that api for put is PUT /. If we want to expose id as param let's update this.
There was a problem hiding this comment.
Let's have PUT /:id, it's the regular way to go since it's a request to modify the resource at the endpoint to a specific entity.
see section 9.6 here
| export class MongoDbRepository<T extends { id: string }> implements PagingDataRepository<T> { | ||
| export class MongoDbRepository<T extends { | ||
| id: string, | ||
| _id?: string | ObjectID |
There was a problem hiding this comment.
[Question] Why we need that? It looks strange.
There was a problem hiding this comment.
When this repository is grabbing the data from mongo it comes with _id: ObjectID('...'), but it's serialized to a string when we try to save it again and mongo complains that it won't change it.
We can either delete data._id like I did or try to wrap it again in an ObjectID constructor and try to update it back to the same value. I had some trouble trying out the second option.
There was a problem hiding this comment.
I think we have two options. Move all queries to use _id. or remove _id completely from every query. Second option seems better to me and align with the was sync work.
I think I also used wrong syntax for update https://stackoverflow.com/questions/22356110/update-field-inside-document-by-id-in-mongodb
Another thing will be to grab that obj into interface. This looks really strange .
|
Changes Unknown when pulling 6127654 on fix-update into ** on master**. |
Motivation
Client expected to send update requests to
PUT /{entity}/:idwhile server code only supported it on the root of the entity path, we can support both uses.Description
PUT /:id