Skip to content

correcting these parameters to be arrays #37

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

Merged

Conversation

ggarnhart
Copy link
Contributor

What kind of change does this PR introduce?

When using the .contains function, I noticed I was unable to pass an array object. As noted in the documentation here, the expected parameter for value should be an array, but like when passing a value to the .in function.

What is the current behavior?

The current incorrectly accepts a singular value for the .contains function. When making a call, an error will occur, suggesting that an array needs to be passed.

What is the new behavior?

The new behavior allows for an array to be passed. The array is processed as any other list is when passed (much like the .in functionality)

@grdsdev
Copy link
Contributor

grdsdev commented Dec 26, 2022

Hi @ggarnhart thanks for bringing this into the discussion. There are other forms of using this filter on the documentation link you posted.

const { data, error } = await supabase
  .from('reservations')
  .select()
  .contains('during', '[2000-01-01 13:00, 2000-01-01 13:30)')

and

const { data, error } = await supabase
  .from('users')
  .select('name')
  .contains('address', { postcode: 90210 })

So using a single value is better. For supporting an array, we could implement URLQueryRepresentable on the Array, example:

extension Array: URLQueryRepresentable where Element: URLQueryRepresentable {
    var queryValue: String {
        self.map(\.queryValue).joined(separator: ",")
    }
}

For supporting the dictionary example, we could use the same approach, and conform Dictionary to URLQueryRepresentable.

@ggarnhart
Copy link
Contributor Author

ggarnhart commented Dec 26, 2022

Love that idea -- and thanks for the clarification! Apologies, new to Swift as of a few weeks ago :)

I'll push up a change in a few

@grdsdev
Copy link
Contributor

grdsdev commented Dec 27, 2022

Hey @ggarnhart thanks for the PR, I made a few changes to your implementation, please take a look and let me know what you think.

I can merge the PR once you're ready.

@ggarnhart
Copy link
Contributor Author

Looks awesome, @GRSouza! Thanks for the help :)

Merge whenever 🔥

@grdsdev grdsdev merged commit f18b065 into supabase-community:master Dec 28, 2022
@ggarnhart ggarnhart deleted the fixContainsAndArrayFunctions branch December 28, 2022 15:11
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.

2 participants