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

Add support for $and #163

Merged
merged 1 commit into from
Jun 29, 2018
Merged

Add support for $and #163

merged 1 commit into from
Jun 29, 2018

Conversation

vonagam
Copy link
Contributor

@vonagam vonagam commented Jun 29, 2018

My use case for this method is a permissions hook. I want to add restrictions to which record user might find. It may be convenient to do by replacing params.query with {$and: [originalQuery, restrictionsQuery]} in a before hook. Without $and it is not trivial to combine two arbitrary queries.

@daffl
Copy link
Member

daffl commented Jun 29, 2018

Thank you for the pull request! How do the queries look like? Not opposed to adding this operator but normally combining them (e.g. with Object.assign({}, orginalQuery, restrictionsQUery) should accomplish the same thing.

@vonagam
Copy link
Contributor Author

vonagam commented Jun 29, 2018

First, example:
originalQuery is {category: "bar"}.
restrictionsQuery is {category: "foo"}.
We expect that there will be no results as there is no intersection.
But if we assign them then we will show foo category.

Second, i checked and it seems that this syntax:
.find([originalQuery, restrictionsQuery]) already works (maybe unintentionally because Object.keys works on arrays too).
So for my use case current functionality is enough.
(Still if it is unintentional as there is no docs or tests, i would risk to lose this way in future releases)

@vonagam
Copy link
Contributor Author

vonagam commented Jun 29, 2018

And this is just simplest example. More complex example is if both original and restrictions query contain $or field.

@daffl
Copy link
Member

daffl commented Jun 29, 2018

Makes sense, otherwise you'd have to do a some merging around in that case. If you could add documentation for this in https://github.com/feathersjs-ecosystem/feathers-knex#querying we can get it released.

@vonagam
Copy link
Contributor Author

vonagam commented Jun 29, 2018

Added simplest description. Don't know if specified REST API url right (array with nested objects...).

@daffl daffl merged commit adf7092 into feathersjs-ecosystem:master Jun 29, 2018
@daffl
Copy link
Member

daffl commented Jun 29, 2018

Perfect. Released as v3.3.0, thank you!

@vonagam
Copy link
Contributor Author

vonagam commented Jun 29, 2018

Thanks for super fast response.

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.

2 participants