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(postgrest): Expand documentation for contains and containedBy methods #824

Merged
merged 3 commits into from Feb 17, 2024

Conversation

jocubeit
Copy link
Contributor

@jocubeit jocubeit commented Feb 4, 2024

What kind of change does this PR introduce?

Previous documentation provides code examples that are syntactically incorrect and do not explain how to provide inclusive and exclusive ranges as a value.

This PR does not introduce any code changes, only code documentation changes.

Additional context

Documentation in VS Code for contains and containedBy methods was confusing, and looked like an obvious typo. While performing a deeper dive into the methods themselves I discovered I could pass a string with parenthesis or brackets, or an array object as a value. This was not exposed in the code comments for the documentation. I felt compelled to update it.

Previous documentation provides code examples that are syntactically incorrect and do not explain how to provide inclusive and exclusive ranges as a value.
@jocubeit jocubeit changed the title Expand documentation for contains() and containedBy() methods Expand documentation for contains and containedBy methods Feb 4, 2024
@jocubeit jocubeit changed the title Expand documentation for contains and containedBy methods fix: expand documentation for contains and containedBy methods Feb 4, 2024
@jocubeit jocubeit changed the title fix: expand documentation for contains and containedBy methods docs: expand documentation for contains and containedBy methods Feb 4, 2024
Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. This PR is great. Just to take this PR a step further, maybe we can address the different data types that contains and containedBy can be used against. Both of these filters can be used against an array and range types (and json types with array). Maybe it's better to explain this and

The JS docs explains it well, and yes, will bring this over to the Flutter docs as well.
https://supabase.com/docs/reference/javascript/containedby?example=on-jsonb-columns

I think we can add an example in each method to show how to use them with array columns.

And then for the range example, I actually like the [1,2) sample that we currently have, because it shows that you can use inclusive and exclusive range in a single call. Maybe we can just say something like use "use brackets for inclusive range and parenthesis for exclusive range", and just have the current example that we have.

Comment on lines 258 to 261
/// await supabase
/// .from('users')
/// .select()
/// .contains('age_range', '[1,2]');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this chunk is a typo since we have the same thing just above.

Comment on lines 296 to 299
/// await supabase
/// .from('users')
/// .select()
/// .containedBy('age_range', '[1,2]');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this one seems like a duplicate as well.

@dshukertjr
Copy link
Member

I have updated the docs on contains and containedBy with more intuitive examples. It would be great if we could bring these examples over to the comment docs on supabase_flutter if you're interested!

https://supabase.com/docs/reference/dart/contains

@dshukertjr dshukertjr changed the title docs: expand documentation for contains and containedBy methods docs: Expand documentation for contains and containedBy methods Feb 17, 2024
@dshukertjr dshukertjr changed the title docs: Expand documentation for contains and containedBy methods docs(postgrest): Expand documentation for contains and containedBy methods Feb 17, 2024
Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some edits based on the examples provided in the docs. Thanks for the PR!

@dshukertjr dshukertjr merged commit e241e76 into supabase:main Feb 17, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants