reactioncommerce / reaction

Mailchimp Open Commerce is an API-first, headless commerce platform built using Node.js, React, GraphQL. Deployed via Docker and Kubernetes.
https://mailchimp.com/developer/open-commerce/
GNU General Public License v3.0
12.32k stars 2.17k forks source link

Add search functionality #105

Closed bstocks closed 8 years ago

bstocks commented 10 years ago

As a user and as a shop administrator, I want to be able to search for products by tag/keyword across the whole store.

eahefnawy commented 10 years ago

After a bit of research I've found meteor-easy-search. Seems to be working well and pretty easy to set up. But it raises a question: Do we need elastic search at this point?

eahefnawy commented 10 years ago

I'm currently exploring your codebase while working on adding this feature. If you have any notes on this feature submit them here.

aaronjudd commented 10 years ago

I don't think it's worth supporting Elastic Search, I think a pure mongo query is likely to be just as performant when we're dealing with relatively small collections (say under 10000 products). Elastic Search add complexity in configuration and in maintainence, particular for our end goal (a PaaS solution for Reaction).

Really all this package is doing, other than the ElasticSearch is a creating mongo queries. As much as possible we'd like to have to core contain our own code (plus iron-router, collections, cfs). I'd expect later that developers might want to create add-on packages - so this might meteor package might be a good foundation for an add on package, rather than core functionality.

Core we'd like to see: Typeahead with autocompletion of tags Typeahead with autocompletion of product titles, variant options no match to type ahead - then a fuzzy query. Dropdown results (or some other modern presentation) rather than a results page. Pricing, image, title in results.

eahefnawy commented 10 years ago

Yeah that's what I thought. It's a complexity that is not needed. But do you mean that the search feature should be a package in the app gallery instead of reaction-core?

Another question that comes to mind is this: if we're not gonna use results pages and depend completely on dropdowns, then there won't be any routing specific for search results, right? Just plain old meteor reactivity. It would be helpful if you could reference example sites that implement this feature as closely as possible to what you have in mind.

eahefnawy commented 10 years ago

For auto completion there's meteor-autocomplete, and for fuzzy search there's fuzzy-search. Or you want to make our own solutions from scratch to contain our own core code? cause I'm guessing that might take some serious development time!

anotherangrymouse commented 10 years ago

Though I agree that ElasticSearch may be a little heavy from the outset and that Mongo search should be pretty good out the box, there are, unless I am completely mistaken, restrictions on the amount of full-text searchable fields per collection.

I've been told that you can only have one field per doc that can be full-text searched. When an ecommerce solution grows, this could become a bit of an issue.

If your db/app is complex and needs full text searching (thinking ebook store), ElasticSearch is a better solution and works extremely well with JSON/BSON. It also allows you to focus on the "Node.js way" of solutions that do a specific, focused-job extremely well and there's no denying that ElasticSearch does search extremely well.

but in the short term, MongoDB's search functionality should be fine and as an out of the box solution seems well suited to a Meteor app!

eahefnawy commented 10 years ago

ElasticSearch is definitely better than it's absence with big commerce sites. Maybe we could add it later on. But probably won't be part of reaction-core.

aaronjudd commented 10 years ago

Looks like there aren't limits on Mongodb text search, at least that I see in the http://docs.mongodb.org/manual/core/index-text/ We'll just need to setup up text indexes for anything we want part of a full text search.

eahefnawy commented 10 years ago

A basic search feature is now finished at this repo.

Enhancements:

Feel free to test it and check what I'm missing.

We also need to discuss how users will install/inject the searchBox template into their app without modifying core. We need to decide on this for all relevant rapps.

anotherangrymouse commented 10 years ago

@aaronjudd Just heard back from MongoDB people and there is indeed a limit. This is a quote from Francesca Krihely "Yes, a collection can have at most JUST ONE text index". I was originally told this "gotcha" from the people at MongoLabs.

In the docs http://docs.mongodb.org/manual/core/index-text/ under the heading "Create Text Index" it says "A collection can have at most one text index.". The key here is collections. Just something to keep in mind building solutions for text search using MongoDB.

aldeed commented 10 years ago

@eahefnawy, a few comments:

eahefnawy commented 10 years ago

I've noticed the weird indentation on github, but they look well indented on my editor. So I thought it's a github thing. using ?ts=2 at the end of the url would fix that problem. I'm not sure if there's anything that could be done from my end.

Thanks so much for your input. I'll make sure I make these changes asap. Please keep the feedback coming, I could really learn a lot from your experience. :)

aldeed commented 10 years ago

Oh, yeah, I was just viewing it on github. The problem is that it uses a mixture of tabs and spaces for indentation. If you convert to using only spaces, it should look fine on github. If you use ST3, you select all and go to View > Indentation > Convert indentation to spaces. Most IDEs have some similar option. This is another thing that adding the .editorconfig file helps with since it explicitly tells the IDE to always use spaces.

aaronjudd commented 10 years ago

@aldeed I actually include Morten's .editorconfig in reaction, but not in all the packages. Yes - it should be 2 ( also always used 4 until meteor). Coffeescript also recommends 2.

aaronjudd commented 10 years ago

@anotherangrymouse thanks - took me three times reading their doc to find that one line! I guess we (I) need to test more because after re-reading that doc I'm a little uncertain if the you are indexing a particular field, or indexing the whole collection for text search (it sort of appears that its the collection). If that's true, I think we're still okay.

ScyDev commented 8 years ago

This is ready? It's two years old and breaks with "Uncaught TypeError: ReactionCore.registerPackage is not a function".

I'm working on it to make it compatible with 0.12.

aaronjudd commented 8 years ago

ready as in: "the issue is ready to be worked".... in our road map. The existing reaction-search package was probably won't work without some upgrades, and probably isn't really in line with how we are going to do it now. We'll be adding into /reaction/packages by v0.13

ramusus commented 8 years ago

@ScyDev you may take a look https://github.com/ramusus/reaction-filtration/ it supports simple search and filtration functionality. You can integrate with current dev version, using this PR reactioncommerce#743

aaronjudd commented 8 years ago

Closing in favor of #903 and #277