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

Adds _search query parameter #493

Merged
merged 4 commits into from
Dec 4, 2024
Merged

Adds _search query parameter #493

merged 4 commits into from
Dec 4, 2024

Conversation

djperrefort
Copy link
Member

Adds ability to perform general searches via the _search parameter. Two design choices to note:

  1. I've generally excluded boolean and time fields from the search. Since these values have a limited set/range of values, I find them less useful to include in a search.
  2. I've prefixed the query parameter with an underscore to avoid possible naming conflicts with query parameters that map to model fields. As an extension of this. I'm thinking of also renaming the ordering argument to _ordering.

@djperrefort djperrefort requested a review from Comeani November 26, 2024 20:17
@Comeani
Copy link
Contributor

Comeani commented Dec 3, 2024

I've taken a look at this and it generally looks good, and I think what you said about ordering makes sense. Right now were are using a filters parameter in the client to do a similar thing, but it appears to be stricter (looking for exact matches to the provided value, for example). Is the intention of this primarily to broaden the behavior to include prefixes for more advanced search like regex, contains, starts-with, etc. ?

@Comeani
Copy link
Contributor

Comeani commented Dec 3, 2024

Yassin is answering my question now, adding detail about shifting the responsibility for the search from the client to the server.

Copy link
Contributor

@Comeani Comeani left a comment

Choose a reason for hiding this comment

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

Looks good to me, lets chat about any corresponding changes to how the client's factory methods get what they need from the server when this is in place.

@djperrefort djperrefort enabled auto-merge (squash) December 4, 2024 19:30
@djperrefort djperrefort merged commit 627390c into main Dec 4, 2024
17 checks passed
@djperrefort djperrefort deleted the search_filter branch December 4, 2024 19:32
@djperrefort djperrefort linked an issue Dec 4, 2024 that may be closed by this pull request
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.

Add support for searching records
2 participants