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

added fq() for passing in an object or multiple filters #241

Merged

Conversation

ni-do
Copy link
Contributor

@ni-do ni-do commented Mar 31, 2021

this feature wraps the matchFilter() on the query to be able to pass in either an object with 'field' and 'value' properties or an array with such objects.
test cases are contained within the given tests.

regards, ni-do

@kibertoad
Copy link
Collaborator

Apologies for long delay in review! Will check it tomorrow.

@kibertoad kibertoad self-requested a review April 12, 2021 20:06
@ni-do
Copy link
Contributor Author

ni-do commented Apr 23, 2021

Apologies for long delay in review! Will check it tomorrow.

Did you already have the time to review it? Would be great to bring it to the master branch.

background of this feature is: i'm migrating from https://github.com/godong9/solr-node client, which is no longer maintained. i could continue implementing further adaptations to ease up the migration from the deprecated client.

lib/query.js Outdated
*/
Query.prototype.fq = function (filters) {
const self = this
if (filters instanceof Array) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Array.isArray is the standard way to check this, I believe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

@kibertoad
Copy link
Collaborator

@ni-do Wrote one super-small comment, will merge and publish the change after it is addressed! Please accept my apologies for the delay, was extremely swamped lately, but it's definitely better now, I'll do my best to review any future PRs way faster.

@kibertoad
Copy link
Collaborator

@ni-do I will merge and publish new version as soon as CI passes, I have another request, though. Considering that we mostly rely on examples as our documentation, could you also add one using the new method to https://github.com/lbdremy/solr-node-client/tree/master/examples? That would help our users to discover the new method :)

Separate PR would be fine, I don't want to delay publishing of this one any more.

@kibertoad kibertoad merged commit e74e486 into lbdremy:master Apr 28, 2021
@kibertoad
Copy link
Collaborator

Released in 0.9.0

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

2 participants