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

Potential issue with MongoId fields #1

Open
GeorgeMSchaffer opened this issue Jan 17, 2019 · 2 comments
Open

Potential issue with MongoId fields #1

GeorgeMSchaffer opened this issue Jan 17, 2019 · 2 comments
Labels
bug Something isn't working

Comments

@GeorgeMSchaffer
Copy link

Hi there, there seems to be an issue with fields prefixed with an "_" such as the _id as field, when I run:

const filter = buildMongoConditionsFromFilters({},{
_id: "5c3fa5a6c0db55422042e379",
});

it results in:
"id" is not a valid operator on field "".
at handleFieldOperator ​./node_modules/@entria/graphql-mongo-helpers/dist/buildMongoConditionsFromFilters.js:39​
at ​​​Object.keys.reduce​​​ ​./node_modules/@entria/graphql-mongo-helpers/dist/buildMongoConditionsFromFilters.js:84​
at ​​​buildConditionsObject​​​ ​./node_modules/@entria/graphql-mongo-helpers/dist/buildMongoConditionsFromFilters.js:60​
at ​​​buildMongoConditionsFromFilters​​​ ​./node_modules/@entria/graphql-mongo-helpers/dist/buildMongoConditionsFromFilters.js:120​
at ​​​Object.​​​ ​scratchpad.js:4:0​"

This seems to stem from support from the way Mongo operators are handled. Fields are split on '_' to derive the operators. Since "id".split('') results in ['',id], "id" is assumed to be the operator and it causes to fail the valid operator test. This would also affect came cased fields but i saw from the comments that they are not supported.

Of course, i could be misreading the whole thing :) If not let me know if you would be interested in a pulll request. I am thinking easy way to solve it was thinking of adding another if statement to buildCondtionsObject to handle fields that start with "_". I also see a way to solve this and add support for camel casing without affecting backwards compatibility, but it would be larger refactor. Let me know your thoughts.

@sibelius sibelius added the bug Something isn't working label Jan 17, 2019
@sibelius
Copy link
Member

can you add a failing test?

jean-leonco added a commit that referenced this issue Dec 8, 2020
chore(ci-and-misc): update deps, ts, lint and add workflows
@Tsugami
Copy link

Tsugami commented Dec 17, 2021

the same problem occurred to me some time ago, I don't remember how I solved it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants
@GeorgeMSchaffer @sibelius @Tsugami and others