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

feature: new search params #1918

Merged
merged 2 commits into from
Mar 13, 2024
Merged

feature: new search params #1918

merged 2 commits into from
Mar 13, 2024

Conversation

jokd
Copy link
Contributor

@jokd jokd commented Dec 7, 2023

Fixes #1917

@jokd jokd changed the title feature: new search param feature: new search params Dec 7, 2023
@jokd
Copy link
Contributor Author

jokd commented Dec 7, 2023

Added queryType option for search control which will be sent as a url param for the search endpoint to handle. It could for instance be 'beginwith' or 'contains' to solve #1926.
Certain changes to the origo server search is obviously needed for it to work.

@jokd
Copy link
Contributor Author

jokd commented Dec 7, 2023

I made some changes to origo server to try this out: https://github.com/origo-map/origo-server/tree/added-querytype-to-search

@asemoller
Copy link
Contributor

Tested this with the associated changes in origo server and it works like a charm!
You're a ⭐!

@jokd
Copy link
Contributor Author

jokd commented Dec 11, 2023

Tested this with the associated changes in origo server and it works like a charm! You're a ⭐!

Good to hear! The search function on Origo server could use some tlc to prevent sql injection but that is perhaps another issue.

Copy link
Contributor

@steff-o steff-o left a comment

Choose a reason for hiding this comment

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

LGTM.

Implementation seems OK, but it actually introduces more features than advertised as the "queryType"-parameter is out of scope of the original issue. I expect it to be well documented.

And next time use URLSearchParams instead of that hideous string concatenation. I know that it would have required rewriting some old stuff to make it work in this case.

@Flodkvist Flodkvist merged commit 03cf143 into master Mar 13, 2024
4 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.

Another search param when hitting enter
4 participants