feat(cli): update controller template to enable filter for findById endpoint#4114
feat(cli): update controller template to enable filter for findById endpoint#4114
Conversation
|
I also have a question, in https://github.com/strongloop/loopback-next/blob/master/examples/todo-list/src/controllers/todo-list.controller.ts#L80, we use the flag |
573bc4a to
1cc20c6
Compare
|
@agnes512 see code in buildSchema When that flag is added, the openapi spec of the response schema will include relational schemas, otherwise the included items in response will be treated as unknown properties :p |
| }) | ||
| async findById(@param.path.<%= idType %>('id') id: <%= idType %>): Promise<<%= modelName %>> { | ||
| return this.<%= repositoryNameCamel %>.findById(id); | ||
| async findById( |
There was a problem hiding this comment.
based on your question in comment, the response schema should also be updated to
content: {
`application/json`: {
schema: getModelSchemaRef(<%= modelName %>, includeRelations),
}
}
| }) | ||
| async findById(@param.path.number('id') id: number): Promise<ProductReview> { | ||
| return this.barRepository.findById(id); | ||
| async findById( |
There was a problem hiding this comment.
| }) | ||
| async findById(@param.path.number('id') id: number): Promise<ProductReview> { | ||
| return this.barRepository.findById(id); | ||
| async findById( |
There was a problem hiding this comment.
bajtos
left a comment
There was a problem hiding this comment.
Great initiative! The changes look mostly good, I have one comment to consider.
| schema: {type: 'array', items: getModelSchemaRef(<%= modelName %>)}, | ||
| schema: { | ||
| type: 'array', | ||
| items: getModelSchemaRef(<%= modelName %>, {includeRelations: true}), |
| return this.<%= repositoryNameCamel %>.findById(id); | ||
| async findById( | ||
| @param.path.<%= idType %>('id') id: <%= idType %>, | ||
| @param.query.object('filter', getFilterSchemaFor(<%= modelName %>)) filter?: Filter<<%= modelName %>> |
There was a problem hiding this comment.
Do we support all filter properties in findById? For example, I find it weird to combine findById with where or limit/skip filter properties, because findById always returns a single record matching the given id only. Should we perhaps describe only filter.include and filter.fields properties in the schema?
There was a problem hiding this comment.
See also #3336 Filter on findById is ignoring the where clause.
I am concerned that if we describe filter parameter of findById method as accepting all filter properties, then our users will try to set those (unsupported) properties and get surprised/confused.
There was a problem hiding this comment.
9a7913b to
efb2e16
Compare
efb2e16 to
cc59367
Compare
As LB4 supports inclusion, I think it's fair to allow users to traverse related models with
findById()endpoint with CLI generated controllers.Also, the todoList example enables such filters. I think they should be consistent as we have a note in the site:
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈