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

Jaybell/crud mongoose query params #509

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

yharaskrik
Copy link

No description provided.

yharaskrik and others added 11 commits May 13, 2020 20:27
- basic CRUD operations
- missing: aggregate functionality and more advanced operators
- joins with projections work by using mongoose populate
- can deeply nest joins to recursively populate through virtuals
- `getMany` will return an array of JSON objects now instead of array of Mongoose documents
- remove getTake and getSkip, included in abstract service now
- fix extra $ for operators
…ain objects

- `crud-response.interceptor.ts` will handle Mongoose Documents now
- mongoose `.toObject()` will return getters and virtuals
Just a typo fix for tsconfig.json reference path: from 'crid-mongoose' to 'crud-mongoose'.
- Implements most filter conditions with transform functions to mongoose
- Follows order of important when or and filter conditions exist together
- TODO: nested queries
@yharaskrik
Copy link
Author

Hey @arrrrny coming back to this after a while I fixed #435 first but I will try and look into your populate issue and decoupling from typeORM

@yharaskrik
Copy link
Author

This still needs quite a few unit tests to test the filter, or and search on GET's but the issues mentioned in #435 should be fixed.

@arrrrny
Copy link

arrrrny commented May 14, 2020

@yharaskrik thanks bro

@michaelyali
Copy link
Member

@yharaskrik please ping me when I can merge it

@yharaskrik
Copy link
Author

@zMotivat0r will do. I would like to get the test coverage up a bit before that happens just to make sure that the filters are being built correctly. And to test that populate issue.

@michaelyali
Copy link
Member

Awesome

- create DeepPartial type in utils
- use DeepPartial and ObjectLiteral from utils instead of from typeORM
- add operator map injection token
- add mongoose operator map to map operators in @nestjsx/crud to operators in mongo
- test mongoose crud service
- rework request interceptor to allow for 1 operator to map to more than one operator if needed
@yharaskrik
Copy link
Author

I had to rework how the crud request interceptor works a small amount to ensure that some of the operators that are allowed can be used with mongoose.

There is now an OPERATOR_MAP optional injection token that can customize the crud request parsing. Since mongoose does not use SQL to query some operators do not map 1:1 like in TypeORM.

Can be provided like this:

{
   provide: OPERATOR_MAP,
   useValue: MONGOOSE_OPERATOR_MAP,
}

This injection token is optional and if not provided will use the operators as is to query the DB.

This will also allow for more Databases if they happen to use different syntax.

One side effect of this that may need to be addressed is that the operators that are used after transformation with the operator map may need to be whitelisted in the validate operator function in the interceptor.

This also decouples from TypeORM and adds DeepPartial to the utils.

Something to note: The operator map is only needed for mongoose if you are using operators that are not directly supported by mongoose. And not all operators are supported yet, the ones currently supported are:

  1. eq
  2. ne
  3. gt
  4. lt
  5. gte
  6. lte
  7. in
  8. notin
  9. isnull
  10. notnull
  11. between
  12. starts
  13. end
  14. cont
  15. excl

Not sure if this is quite the best way to go, open to suggestions of how we can breach the gap between NoSQL query language (currently mongodb) and SQL. With the changes that I made I do not think we would be able to use more than one DB type with this package, although not sure if that was possible before or not either.

@AliYusuf95
Copy link

Is there any roadmap to merge this?

@helgetan
Copy link

helgetan commented Sep 2, 2020

Any Updates here, would love to see this!

@fasenderos
Copy link

Is there any news on this?

@rewiko
Copy link
Collaborator

rewiko commented Nov 28, 2021

I am trying to help and have forked this repo (can't reach the owner of this repo and do not have the credential for the npm repo).

See #710 (comment)

We can hopefully merge it via rewiko#5

@nind0932
Copy link

it has been two years!
Why this is not merged?

@afilp
Copy link

afilp commented May 18, 2023

We would like to use mongoose with NestJS, is it possible that you merge this PR? Thanks a lot.

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.

10 participants