Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add url, diagram for 3 relations for inclusion #4007

Merged
merged 1 commit into from Oct 28, 2019

Conversation

@agnes512
Copy link
Contributor

agnes512 commented Oct 25, 2019

After triaging some community issues and the inclusion blog, I'd like to enhance our docs for inclusion to have less confusion.

  • add high level diagram to explain the idea and relation of inclusionResolvers and inclusionResolver
  • add graphQL-like diagram to show the idea of how it gets related models.
  • add url GET ... as query examples in case users don't know how to query it with controller/url

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@agnes512 agnes512 requested review from bajtos and raymondfeng as code owners Oct 25, 2019
Copy link
Contributor

dhmlau left a comment

A few minor comments / suggestions. They apply to both BelongsTo and HasMany md files. Thanks.

@@ -328,6 +328,13 @@ on constructor to avoid "Circular dependency" error (see

## Querying related models

LoopBack 4 creates a different inclusion resolver for each relation type. Each

This comment has been minimized.

Copy link
@dhmlau

dhmlau Oct 25, 2019

Contributor

maybe we call it LoopBack instead of LoopBack 4?

This comment has been minimized.

Copy link
@agnes512

agnes512 Oct 27, 2019

Author Contributor

We don't mention anything about inclusion resolver in LB3. Would it be better if I change it to LB4? or

Different from LB3, LB4 creates a different inclusion resolver for each relation type to query related models.

?

@@ -336,12 +343,18 @@ belongs to a `Customer`.

After setting up the relation in the repository class, the inclusion resolver
allows users to retrieve all orders along with their related customers through
the following code:
the following code in repository level:

This comment has been minimized.

Copy link
@dhmlau

dhmlau Oct 25, 2019

Contributor

in repository level -> in the repository class?

Copy link
Contributor

jannyHou left a comment

Good diagram. LGTM, just a question about the url.

or use APIs with controllers:

```
GET http://localhost:3000/orders?filter[include][][relation]=customer

This comment has been minimized.

Copy link
@jannyHou

jannyHou Oct 28, 2019

Contributor

is it an extra []? I am looking at the similar include filter in LB3 https://loopback.io/doc/en/lb2/Include-filter.html#rest-examples:

/customers?filter[include][reviews]=author

This comment has been minimized.

Copy link
@agnes512

agnes512 Oct 28, 2019

Author Contributor

No, /customers?filter[include][reviews]=author wouldn't work because include.filter is not a function.

And different queries between LB3 and LB4 make me wonder if we need a querying data page for LB4 as well. Cause right now the only way to check the query syntax is to go to the LB3 site.

This comment has been minimized.

Copy link
@nabdelgadir

nabdelgadir Oct 28, 2019

Contributor

The include filter for LB2 is different from the LB4 one. In LB4 it's {include: [{relation: 'customer'}]}, whereas in LB2 (from my understanding) it's {include: {customer: [/* ... */]}}.

@agnes512 agnes512 force-pushed the inclusion-diagram branch from 7db429a to 49f38db Oct 28, 2019
@agnes512 agnes512 merged commit bc8d46f into master Oct 28, 2019
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 92.098%
Details
@agnes512 agnes512 deleted the inclusion-diagram branch Oct 28, 2019
@agnes512 agnes512 mentioned this pull request Nov 7, 2019
14 of 20 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.