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

WIP: Search for Community using elasticsearch #222

Merged
merged 169 commits into from Nov 12, 2014

Conversation

surenm
Copy link
Member

@surenm surenm commented Sep 15, 2014

First cuts for search feature for Community

@@ -1,2 +1,3 @@
web: bin/puma -C config/puma.rb
worker: bin/rake jobs:work
elastisearch: elasticsearch
Copy link
Member

Choose a reason for hiding this comment

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

Typo. elastisearch => elasticsearch

@zachallaun
Copy link
Member

@madhuvishy @surenm (@davidbalbert, I'd love you opinion as well)

I'd love to start thinking about our query syntax a bit more. The Elasticsearch query DSL is quite extensive, and it would be cool to take advantage of more of it.

In particular, I'd love to support narrowing queries based on subforums, thread titles, and post authors.

Proposed query syntax (the email formatting of this might suck – look at it on GitHub for a pretty table):

query result
author:Zach posts where author name matches "Zach"
author:Zach foo "foo" in thread title/post body where author name matches "Zach"
author:(Zach Allaun) foo same as above, except that author name matches "Zach Allaun"
author:Zach subforum:(New York) posts in New York subforum where author name matches "Zach"
author:Zach subforum:(New York) foo "foo" in thread title/post body in New York subforum where author name matches "Zach"
etc.

The basic idea of the above is key:val term1 term2 term3. The key:val paris narrow the set of documents we search for the terms in. It would also be awesome if we could refactor/extend our existing client autocomplete code to provide autocomplete for both valid keys that we support (author, subforum, etc.) and then for valid values for those keys (user names, subforum names, etc.).

Another thing to consider is which part of our code should know about this mini-query language: the client or the server? Should the client be sending the server the string

"author:(Zach Allaun) subforum:(New York) foo"

or should it parse the query and send a data structure

{
  "author": "Zach Allaun",
  "subform": "New York",
  "terms": ["foo"]
}

I think we should go with the second option and do the parsing on the client. The client is going to want to know the syntax anyways so that it can help users make correct queries. But I don't have strong opinions about this yet.

@zzmoss
Copy link
Contributor

zzmoss commented Sep 15, 2014

On searching with facets:

This is definitely the next thing we wanted to work on. We thought we could put the DSL construction logic on the server side, but it does make sense that the client already knows for auto completion.

@surenm
Copy link
Member Author

surenm commented Sep 16, 2014

+1 on using query DSL stuff and it was part of the plan to do this first before going to intent discovery.

Like @madhuvishy said we had originally intended to do a dumb client against a search server/autocomplete endpoint which takes care of all intent discovery but the client side approach could be appropriate for community since a lot of action happens client side.

I don't know which is a better method either but from my previous experiences I have always constructed the DSL on server side with varying levels of intelligence on the client side.



(defcomponent search-bar [app owner]
(display-name [_] "Autocomplete")
Copy link
Member

Choose a reason for hiding this comment

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

We should make this "SearchBar" or something.

@davidbalbert
Copy link
Member

Style bug:

In Firefox, the server error box looks weird.

Edit: Fixed. -Zach

:query "foo bar baz"})
;; =>
"/api/search?q=foo bar baz&filters[author]=Zach Allaun"

Copy link
Member

Choose a reason for hiding this comment

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

This looks like it was scratch, right? I think we should get rid of it.

@davidbalbert
Copy link
Member

Ok, this is good to go. We're deploying :)

@davidbalbert davidbalbert merged commit 029c1fe into recursecenter:master Nov 12, 2014
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.

None yet

5 participants