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 |
agnes512
force-pushed
the
update-controller-template
branch
from
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 |
| @@ -112,8 +112,11 @@ export class <%= className %>Controller { | |||
| }, | |||
| }, | |||
| }) | |||
| 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),
}
}
| @@ -122,8 +122,11 @@ export class ProductReviewController { | |||
| }, | |||
| }, | |||
| }) | |||
| async findById(@param.path.number('id') id: number): Promise<ProductReview> { | |||
| return this.barRepository.findById(id); | |||
| async findById( | |||
There was a problem hiding this comment.
| @@ -291,8 +294,11 @@ export class ProductReviewController { | |||
| }, | |||
| }, | |||
| }) | |||
| async findById(@param.path.number('id') id: number): Promise<ProductReview> { | |||
| return this.barRepository.findById(id); | |||
| async findById( | |||
There was a problem hiding this comment.
| 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.
agnes512
force-pushed
the
update-controller-template
branch
from
9a7913b to
efb2e16
Compare
agnes512
force-pushed
the
update-controller-template
branch
from
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 👈