Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Input filter operator _nin is not yet implemented #2638

Closed
adalidda opened this issue Sep 20, 2020 · 7 comments
Closed

Input filter operator _nin is not yet implemented #2638

adalidda opened this issue Sep 20, 2020 · 7 comments

Comments

@adalidda
Copy link

Hi,

I just notice that in the current version, in the input filter, while the operator _in is implemented, the operator _nin is not yet implemented.

Thank you
Adalidda

@eric-burel
Copy link
Contributor

For the record you can nest _not and _in to simulate _nin.
Happy to accept a PR with _nin someday.

Note that this part of the code is being transitioned from Meteor to a NPM package at the moment, we'll be back at adding feature when it's done.

@adalidda
Copy link
Author

Thank You @eric-burel

@ErikDakoda
Copy link
Contributor

@eric-burel I just noticed that in vulcan-lib/lib/server/graphql/typedefs.js in String_Selector and others many of the selector options are commented out. I needed to use _gt for a string input parameter, so I uncommented it, and the selector worked just fine, returning documents filtered correctly.

Can you let me know what the status of this is? It actually appears that _gte and _lte are even unit tested in vulcan:lib/mongoParams/keep multiple filters.

@eric-burel
Copy link
Contributor

Good question, ping @SachaG

I've noticed that but I am not sure why, I have started to convert the whole thing in TS but haven't run much filters yet https://github.com/VulcanJS/vulcan-npm/blob/devel/packages/mongo/mongoParams.ts

@eric-burel
Copy link
Contributor

eric-burel commented Dec 3, 2020

They live here now: https://github.com/VulcanJS/vulcan-npm/blob/devel/packages/graphql/server/defaultSchema.ts

I remember asking the same for Date_selector. I think they indeed just lack testing, if you have unit tests for them then I think we're good activating them.
Let me know because I'll need to reenable them on Vulcan NPM too until we have unified both.

The main security threat is not the options in themselves, but the ability to remove document that you shouldn't have been able to filter, and it is correctly handled and unit tested in current version. So adding operators shouldn't be a problem as long as you confirm they work as expected.

@kevinashworth
Copy link
Contributor

kevinashworth commented Dec 31, 2020

EDIT: Oops. Ignore what's below. I see Erik already changed things!

Should this issue be marked as resolved?


Sacha had previously said it was commented out because it wasn't implemented yet. See https://vulcanjs.slack.com/archives/C02L990AF/p1595281668431400

The minimal change to get _nin working for me was only for a string (_id) so just to uncomment its first occurence in typedefs.js. Seems to me that now uncommenting most of the selectors would work fine. I don't see the "ilike" or "contain_all" operators being in place, but otherwise I count maybe a dozen selectors to uncomment in that file.

@ErikDakoda
Copy link
Contributor

Implemented in PR #2672

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants