Skip to content
This repository has been archived by the owner on May 13, 2023. It is now read-only.

Add head and count option #16

Merged
merged 8 commits into from
Jan 25, 2021
Merged

Add head and count option #16

merged 8 commits into from
Jan 25, 2021

Conversation

dshukertjr
Copy link
Member

What kind of change does this PR introduce?

This feature is postgrest-dart equivalent of this feature in postgrest-js
supabase/postgrest-js#147

What is the current behavior?

There are not head or count options

What is the new behavior?

head and count options are added

@dshukertjr dshukertjr changed the title [WIP] Add head and count option Add head and count option Jan 22, 2021
@dshukertjr
Copy link
Member Author

@phamhieu @
Hi! I have added a head and count option that was added to postgrest-js recently. Let me know what you think about it! Feedback is always welcome.
Let me know if you don't like some of the decisions that I have made with the code. I can always update it.

lib/postgrest.dart Outdated Show resolved Hide resolved
lib/src/builder.dart Outdated Show resolved Hide resolved
@phamhieu
Copy link
Member

phamhieu commented Jan 24, 2021

Hi @dshukertjr thanks for your contribution 👍

I have a proposal for head and count implementation. Let me know your thought.

Instead of modify select, insert, update, delete and rpc, why don't we modify execute() to support these new options

Future<PostgrestResponse> execute({bool head = false, CountOption count}) async {
  if (head) {
     method = 'HEAD';
  }

  if (count != null) {
      if (headers['Prefer'] == null) {
          headers['Prefer'] = 'count=${count.name()}'; 
      }
     else { 
          headers['Prefer'] += ',count=${count.name()}'; 
     }
   }
   
  ...
}

by doing this:

  • less change on the code base
  • It can also support all PostgrestQueryBuilder methods: select, insert, update, delete and rpc
  • prevent breaking change for select with custom columns. select(columns: 'messages(*)')

@dshukertjr
Copy link
Member Author

dshukertjr commented Jan 24, 2021

Hi @dshukertjr thanks for your contribution 👍

I have a proposal for head and count implementation. Let me know your thought.

Instead of modify select, insert, update, delete and rpc, why don't we modify execute() to support these new options

Future<PostgrestResponse> execute({bool head = false, CountOption count}) async {
  if (head) {
     method = 'HEAD';
  }

  if (count != null) {
      if (headers['Prefer'] == null) {
          headers['Prefer'] = 'count=${count.name()}'; 
      }
     else { 
          headers['Prefer'] += ',count=${count.name()}'; 
     }
   }
   
  ...
}

by doing this:

  • less change on the code base
  • It can also support all PostgrestQueryBuilder methods: select, insert, update, delete and rpc
  • prevent breaking change for select with custom columns. select(columns: 'messages(*)')

@phamhieu
I like this approach. Let me update the code now!

@dshukertjr
Copy link
Member Author

@phamhieu I have updated the code! Thanks for all the great suggestions, and let me know if you have more!

lib/src/builder.dart Outdated Show resolved Hide resolved
lib/src/builder.dart Outdated Show resolved Hide resolved
@phamhieu
Copy link
Member

Excellent work! @dshukertjr

There're minor changes and cleanup. But overall looks good to me.

@phamhieu phamhieu merged commit 57b798e into supabase:master Jan 25, 2021
@dshukertjr
Copy link
Member Author

@phamhieu
Thank you so much for all of your support!

@phamhieu
Copy link
Member

Thanks @dshukertjr, amazing job!

Btw, I'm writing gotrue-dart library right now. Porting from https://github.com/supabase/gotrue-js
Let me know if you are interested to contribute. I can add you in. As it's still under-development I haven't made it public

@dshukertjr
Copy link
Member Author

dshukertjr commented Jan 25, 2021

@phamhieu
I was going to ask someone if there is a gotrue-dart being developed! I would love to contribute to gotrue-dart as well!

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

Successfully merging this pull request may close these issues.

None yet

2 participants