feat(repository): implement inclusion resolver for hasOne relation #3764
Conversation
agnes512
force-pushed
the
hasone-resolver
branch
4 times, most recently
from
1dcc829 to
72415c4
Compare
agnes512
force-pushed
the
hasone-resolver
branch
2 times, most recently
from
48b9193 to
357bb70
Compare
...ages/repository-tests/src/crud/relations/acceptance/has-one.inclusion-resolver.acceptance.ts
Outdated
Show resolved
Hide resolved
docs/site/hasOne-relation.md
Outdated
|
|
||
| ## Querying related models | ||
|
|
||
| We introduce the concept of `inclusion resolver` in `Has Many Relation` and |
There was a problem hiding this comment.
Should we have the same intro as the other relations for consistency:
LoopBack 4 has the concept of an `inclusion resolver` in relations, which helps
to query data through an `include` filter. An inclusion resolver is a function
that can fetch target models for the given list of source model instances.
LoopBack 4 creates a different inclusion resolver for each relation type.
?
There was a problem hiding this comment.
I don't want to repeat (copy) it three times in three different relations 😂 I was thinking that they might check Has Many and BelongsTo already.
There was a problem hiding this comment.
If it's the same content, should we move the content to the top level? https://loopback.io/doc/en/lb4/Relations.html.
IMHO, it might not be the right assumption that users would read the docs for all the relations page. :) Besides, when we have more relations, it might not be a good idea to copy and paste the same paragraph everywhere too. Just my 2 cents.
There was a problem hiding this comment.
I thought about that before. But I found that the content in Relation.html are high-level stuff. That's why I put them inside of each relation.
There was a problem hiding this comment.
IMHO, it might not be the right assumption that users would read the docs for all the relations page. :)
I agree that it might not be the right assumption that users will read the docs for all of them (they might just be interested in hasOne).
Besides, when we have more relations, it might not be a good idea to copy and paste the same paragraph everywhere too. Just my 2 cents.
I think for the meantime since we only have three, it should be fine?
There was a problem hiding this comment.
+1 to introduce the InclusionResolver concept at relations level. Each of the specific relations can have its own page. We can have links to refer to the InclusionResolver.
packages/repository/src/relations/has-one/has-one-repository.factory.ts
Outdated
Show resolved
Hide resolved
agnes512
force-pushed
the
hasone-resolver
branch
2 times, most recently
from
3f95dd6 to
9b5eaf5
Compare
docs/site/Relations.md
Outdated
| @@ -31,7 +31,10 @@ introduction of [repositories](Repositories.md), we aim to simplify the approach | |||
| to relations by creating constrained repositories. This means that certain | |||
| constraints need to be honoured by the target model repository based on the | |||
| relation definition, and thus we produce a constrained version of it as a | |||
| navigational property on the source repository. | |||
| navigational property on the source repository. Besides, we also introduce the | |||
There was a problem hiding this comment.
suggestion: Additionally, we introduce...?
Co-authored-by: Miroslav <[email protected]>
agnes512
force-pushed
the
hasone-resolver
branch
from
9b5eaf5 to
8682168
Compare
resolves #3449
This PR implements the inclusionResolver for hasOne relation:
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 👈