Conversation
| expectValidJsonSchema(jsonSchema); | ||
| }); | ||
|
|
||
| it('properly converts nested array property when json schema provided', () => { |
There was a problem hiding this comment.
Do we have any tests on querying/filtering the nested arrays?
There was a problem hiding this comment.
@agnes512 @loopback/repository-json-schema doesn't have it. If we add tests, need to do them in the acceptance tests for connectors.
There was a problem hiding this comment.
I mentioned it is because when I was triaging loopbackio/loopback-connector-postgresql#441, I realized that not all connectors support arrays/nested arrays. It'd be good if we can have them documented and tested. Just some thoughts 😄
There was a problem hiding this comment.
I realized that not all connectors support arrays/nested arrays.
Yeah that's the problem, the original issue #4754 is not trying to support the full feature, it's a fix for the misuse of decorator.
It'd be good if we can have them documented and tested.
It's not a trivial thing to test across connectors, let's do it in a separate story.
| jsonSchema: { | ||
| type: 'array', | ||
| items: {type: 'string'}, |
There was a problem hiding this comment.
I have another question of the design. Do we need to specify these two fields? if so, why do we need type: 'array' as we already use the decorator?
There was a problem hiding this comment.
@agnes512 the jsonSchema here is for the sub array property,
e.g. For Array<Array<string>>, the json schema describes Array<string>
If the current doc confuses you:
To define a nested array property, you must provide the
jsonSchemafield to describe the sub-array property.
Any suggestion on how to make it more clear?
There was a problem hiding this comment.
Ah I see, then I am good with it 👍
agnes512
left a comment
There was a problem hiding this comment.
Hopefully we can have some tests for different connectors in the future :D
Fixes #4754
To define a nested array property with
@property.array(), you must provide a json schema to describe the sub array item:A few issues this PR addresses:
We didn't allow nested array property before due to
https://github.com/strongloop/loopback-next/blob/ef155eeddf11895bac8ddc491cb87c1e5b7398d0/packages/repository-json-schema/src/build-schema.ts#L260
which is not correct. I tested with todo app, when decorate the nested array property well, creating & retrieving an instance works properly.
It's not possible to infer the sub type of nested array property, that's why the json schema field is needed(Thank you @deepakrkris for your suggestion in defining a nested array model property using @property.array decorator crashes the app #4754 (comment)). I added a new check for that field to avoid triggering the subsequent error '"items" property must be present if "type" is an array' instead.
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 👈