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

eng(supabase): add query supabase transformer tests #408

Conversation

devj3ns
Copy link
Contributor

@devj3ns devj3ns commented Aug 23, 2024

This PR implements most of the tests for the QuerySupabaseTransformer. (There are still some tests missing, where I am currently not sure how to properly implement them.)

I added an age field to the DemoModel class to use it for the lt/gt/lte/gte tests.

Because the _uri property of the PostgrestBuilder is private and therefore cannot be directly accessed, I created an extension on the PostgrestBuilder which returns the URI by using the overrideSearchParams method. While this works, I am open for suggestions to improving this!

@devj3ns devj3ns requested a review from tshedor as a code owner August 23, 2024 09:25
Copy link
Collaborator

@tshedor tshedor left a comment

Choose a reason for hiding this comment

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

@devj3ns This is great. I agree exposing uri is less ideal, but given the small implementation you've taken to reach it, I think it's OK. The implementation I'm taking in the transformer is a little brittle IMO instead of just using the first-class .eq methods. In future updates, the Supabase team may expose something more programmatic like the JavaScript filters that we could leverage in Dart. For now, I think this covers our use cases.

Could you please get those remaining tests to pass and then I'll merge?

group('with query', () {
group('eq', () {
test('by field', () {
final query = Query.where('name', 'Jens');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice

@tshedor tshedor mentioned this pull request Aug 23, 2024
29 tasks
@tshedor tshedor merged commit e0193dc into GetDutchie:supabase-integration Aug 25, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants