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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Docs] Update "Relationship filtering" section in querying docs #895

Merged
merged 3 commits into from
Dec 19, 2021

Conversation

bradjones1
Copy link
Contributor

  • The header I think should be an H3, otherwise it doesn't show up in the navigation and gets lost.
  • In my testing, it looks like the correct key is records in the relationship filter object... it's inconsistent between the two examples.

- The header I think should be an H3, otherwise it doesn't show up in the navigation and gets lost.
- In my testing, it looks like the correct key is `records` in the relationship filter object... it's inconsistent between the two examples.
@dgeb
Copy link
Member

dgeb commented Dec 17, 2021

Thanks for the PR @bradjones1 :)

In my testing, it looks like the correct key is records in the relationship filter object... it's inconsistent between the two examples.

For hasOne relationships the filter checks record not records. For hasMany relationships, records is checked.

If you're seeing different behavior, can you specify which version of orbit and which orbit source (memory / jsonapi / etc) you're using?

@bradjones1
Copy link
Contributor Author

Thanks for the gut-check on that. I actually moved to getting the records in question via an include from a related entity, so I don't have that code anymore. I think I might have just had myself turned in a knot.

I think this still should be under an H3 though ;-)

@dgeb
Copy link
Member

dgeb commented Dec 18, 2021

Ah thanks for the clarification on the filter.

The header I think should be an H3, otherwise it doesn't show up in the navigation and gets lost.

I can see what you mean that this rather important heading gets lost in the side nav. The issue with just changing this one H4 -> H3 is that there is another section just above, Attribute filtering, that should be at the same level as Relationship filtering. I think we can make this work by doing the following:

  • Drop the header ### Comparison operators for filtering entirely, since it adds little, but keep the section's brief contents.
  • Change #### Attribute filtering -> ### Attribute filtering

Would you mind making these two additions to your PR?

@bradjones1
Copy link
Contributor Author

Done, thanks for the review.

@dgeb dgeb merged commit 8fa9703 into orbitjs:main Dec 19, 2021
@dgeb
Copy link
Member

dgeb commented Dec 19, 2021

Looks great, thanks @bradjones1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants