GitbookIO / filterable

Parse and convert GitHub-like search queries in Node.JS
Apache License 2.0
61 stars 8 forks source link

Adding "is:" operator feature #12

Closed estilles closed 8 years ago

estilles commented 9 years ago

Implementing issue #8.

This is a work in progress!

Opening early PR for discussion, as per @SamyPesse's suggestion.

Currently implemented:

Still pending:

estilles commented 9 years ago

Once you merge PR #14 I will rebase this and continue working on it. Will continue working on this, but will not include unit tests for FilterablePlugin. It's current structure makes it difficult to test without refactoring.

estilles commented 9 years ago

I refactored the splitGroup() function from query.js into a separate module because I'm going to need the same functionality in mongoose.js. I should be done with this tonight. :-)

Disregard. Found a less intrusive was of doing this. :-)

estilles commented 9 years ago

@SamyPesse, would you be willing expose the filter variable in FilterablePlugin in order to make to a bit more testable? We can call it something like .__filterable, to indicate that it's intended for private use.

Since the query returned by .search() is empty until it's actually executed, and the actual query isn't created until well into the wrapQuery function, it's very difficult, if not impossible to test.

While I would very much prefer the test if the actual Mongoose query is being generated correctly, by exposing the filter var we can at least test if the FilterablePlugin options are being correctly parse and a good Filterable instance is being created.

I suspect this is the reason you currently don't have any tests for FilterablePlugin. Since this PR updates FilterablePlugin I would like to include tests for this new functionality.

If you can think of any other way to do this, please let me know. If you're amenable to this I will include it in this PR, otherwise I will test manually and submit the PR without unit tests for FilterablePlugin.

estilles commented 8 years ago

Wow! Didn't think this was going to get merged. Thanks!

SamyPesse commented 8 years ago

@JohnnyEstilles Yes, sorry for the delay ... Thank you for the PR. I refactor all the module.

There is no mongoose plugin anymore (it can easily be built as an external module), the API has changed.

estilles commented 8 years ago

Thanks @SamyPesse! I noticed the rewrite. I still have a lot do uses for this (outside of Mongoose).