-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Search page #3357
Search page #3357
Conversation
Generated by 🚫 Danger |
This looks amazing!!! I've downloaded it and am compiling now. |
i'm pushing it to unstable too in case that installs faster than my laptop (its having me upgrade and compile some recently updated gems) |
Ok I like this a lot - is it running on unstable now as you expect? Is there a possibility of an "all" category, or is that too complicated? I think we can probably merge this now if you think it's ready! |
e2be09d
to
e7c19fd
Compare
I just pushed some small tweaks, @jeff. Sorry, can you try again? The reason it doesn't have the 'all' pages right now is that we don't have any partial to use to show the all type of results on the search view... unless we use just a table for now. We can do this later today, no problem! That was on the list :D about the unstable, I'm seeing the old search page here, on yours as well? 🤔 |
and it's also not showing the 'Show all' option on the typeahead... it doesn't look like it's our code there on unstable xD |
Ah, so I see https://unstable.publiclab.org/search/balloon doesn't exist anymore. Maybe we should redirect it to https://unstable.publiclab.org/search/notes/balloon until we think through an "all" search? Thanks! |
ah i think somebody overwrote unstable... maybe check in the chatroom? |
Actually i realized that there may not be a quick way to integrate users and notes... maybe the default "all" view would be just notes + pages + questions... OR we should just default to notes. |
sorry, in meetings today but trying to respond between! Having staff retreat -- staff say hello! |
I was able to test it on unstable before it seemed to have restarted, and saw the new interface with buttons for different types. It was great. |
Hmm, i do see an error - when i type the phrase (with quotation marks): https://unstable.publiclab.org/search/notes/'open%20call'?order=natural&type=boolean the Questions and Tags buttons don't seem to work on that page; it just goes to https://unstable.publiclab.org/search/questions/ However, without quotations, like searching for |
hi @jeff, try again now!! the only problem is that by setting the typeahead items to 15 we can't see the 'show all' button. I think 10 it's better! we can discuss more the details tomorrow, see you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-)
Follow up issues maybe
- Separate the search type button group from ordering button group? (Put them in button groups)
- Asterisks?
config/routes.rb
Outdated
get "search/wikis/:query", :to => "search#wikis" | ||
get "search/profiles/:query", :to => "search#profiles" | ||
get "search/questions/:query", :to => "search#questions" | ||
get "search/places/:query", :to => "search#places" | ||
get "search/tags/:query", :to => "search#tags" | ||
get "search/", :to => "search#new" | ||
get "search/:query", :to => "search#notes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
* Decouple DocResult from SearchService * Add new /search page * Fix tests * removing 'returns' * Fixing notes ans wikis parameters * Fixing notes ans wikis parameters [2] * small tweaks * change search/notes to search/* default and add html_safe to search/ queries * we need to fix those search scripts * add type to search node.search method and fix urls
fixes #987
fixes #2421
@jywarren , sorry for this big PR again, breaking into small Pull Request is a skill itself! But we believe but if you read this first it will help you with the review! If you could switch to our branch it would be great (or maybe trying on the unstable?), because the iteration on the /search and on the typeahead would be good to be reviewed live.
We decoupled DocResult from SearchService. Everyone now can use the SearchService endpoints by calling ExecuteSearch.by(:endpoint, search_criteria). We moved the DocResult packaging to the api/search.rb Class since this is the one who deals with the API. That's gonna be documented next week :)
The confusing thing that was happening that the pages and notes were due to the old queries on the SearchService. Right now, the typeahead results and the /search results are not using the same methods, for exampe. For now, we created a new endpoint for the wikis (old 'pages'). Maybe we can unify them all under a general search_nodes in the future, but that's something for later.
The new /search page is working and we can filter the Notes and Wikis results from sort_by options. The navbar search and the form on /search use now the same methods. Before the /search was only returning Notes... now the results are consistent. It includes now the static pages as well.
We just need to finish some small UI details but we thought it would be better to open this PR to ask for your review and open a new one tomorrow maybe.
As Camilla mentioned before, this is our last work on the coding. Along the way, we wrote some docs and we plan to create detailed issues of the tasks that we couldn't finish.