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

We should strip leading spaces in search queries #3655

Open
anna-parker opened this issue Feb 10, 2025 · 5 comments
Open

We should strip leading spaces in search queries #3655

anna-parker opened this issue Feb 10, 2025 · 5 comments

Comments

@anna-parker
Copy link
Contributor

This is live on production, I cannot search by submissionId - can be tried by pasting the submissionId of any sequence into search:

e.g. https://main.loculus.org/seq/LOC_000PB42.1

Image

@anna-parker anna-parker added the bug Something isn't working label Feb 10, 2025
@fhennig
Copy link
Contributor

fhennig commented Feb 10, 2025

this one works: https://main.loculus.org/ebola-sudan/search?visibility_submissionId=true&submissionId=OQ673032.1

You can also find the one you've searched for if you drop the ".L"

@theosanderson
Copy link
Member

This is because of a leading space in the search query when you copy-pasted (as @fhennig pointed out, you can see it in your URL) we should strip these

@theosanderson theosanderson changed the title Search by submissionId is broken We should strip leading spaces in search options Feb 10, 2025
@theosanderson theosanderson changed the title We should strip leading spaces in search options We should strip leading spaces in search queries Feb 10, 2025
@chaoran-chen
Copy link
Member

@theosanderson, do we want to strip leading spaces (1) from all search queries or (2) only from those where we perform a sub-string search? (2) seems quite safe to me, but I'm not sure about (1) – what if someone does have a leading space in their data?

@theosanderson
Copy link
Member

Yes - good Q. I think my vote would be everything. Stripping is potentially dangerous but not stripping is also dangerous! (we might well not have set this field as substring). I think my vote would be strip everything and in 2 years when it catches someone out, implement configurability. But either would work for this case so I don't feel strongly - we could start with your idea.

@theosanderson theosanderson removed the bug Something isn't working label Feb 10, 2025
@theosanderson
Copy link
Member

Reposting from Slack: Actually the thing that gets copied is \r\n which is a newline sequence - that then gets converted to space when pasted into the search box - so probably room for improvement on the searchbox side too

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

No branches or pull requests

4 participants