Skip to content

Conversation

yharaskrik
Copy link

Basic mongoose implementation, working branch.

@Bnaya
Copy link

Bnaya commented Dec 3, 2019

Awesome!

@yharaskrik
Copy link
Author

Basic joins are now possible via the crud-mongoose package.

Requirements for joins to work:

Mongoose schema must be setup to use virtuals
Virtual for the join that is requested must be setup, see here: https://mongoosejs.com/docs/populate.html

@Bnaya
Copy link

Bnaya commented Dec 20, 2019

if this feature is mostly ready, would be awesome to have it published as separate package until merge, as canary or so,
For users to provide feedback, etc

Thanks!

@yharaskrik
Copy link
Author

Hey @Bnaya,

There is still some unit testing I would like to write but I could publish it as a separate package for now! I will update this soon! Just heading home for Christmas but would like to get that done soon!

@yharaskrik
Copy link
Author

I just published a 0.0.1 version to npm here

https://www.npmjs.com/package/nest-crud-mongoose

I need to update the readme to reflect mongoose instead of typeORM which I will try and get done today.

Feel free to comment on this issue with any questions or concerns. I will comment again when the docs after been updated.

@yharaskrik
Copy link
Author

Updated.

@yharaskrik
Copy link
Author

@Bnaya have you used the package at all?

@Bnaya
Copy link

Bnaya commented Dec 23, 2019

I've created this project template based on your integration tests code + in memory mongo
https://codesandbox.io/s/github/Bnaya/demo-mongoose-implementation-nest-crud/

- 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
@yharaskrik
Copy link
Author

I have rebased my code onto what is released on nestjsx/crud but am having issues with the mongoose documents not being converted to POJOs now, the request just returns a JSON object of the document not the value of the document itself. Not sure what changed with this but toJSON isn't being called correctly. I will try and figure it out soon.

@yharaskrik
Copy link
Author

@zMotivat0r is there something that would have changed in 4.3.x that would have caused the requests to not toJSON the mongoose Documents? All of my tests passed before but now the Documents are returned as they are represented in memory instead of the value of the document itself.

I had to remove all references of authFilter and the getTake and getSkip methods plus I had to remove the code that prefixed the operators with a $ as they before the operators were just eq or lt, now they are $eq. None of those changes should have changed the behavior around the return objects. I can try and force them to all return as lean() so they are plain JSON objects but then the _id field is returned as an object buffer as well whereas before it was being returned as the string representation of the buffer (as it should) so that the front end gets back the mongo ObjectID for the record.

Thanks for any input you have..

@michaelyali
Copy link
Member

getTake and getSkip have been removed to the abstract class.
Regarding the response - I've added the CrudResponseInterceptor, please see in crud/interceptors - there is classToPlain calls

@yharaskrik
Copy link
Author

Yup, noticed those methods were added to abstract thus were not needed, that is nice, the intercetpor is probably what is doing it! Thanks I will take a look.

- remove getTake and getSkip, included in abstract service now
- fix extra $ for operators
@yharaskrik
Copy link
Author

So I am going to need to spend some more time on this, for now the interceptors need to be turned off like in my spec files for it to work because of the way mongoose serializes the Document object.

…ain objects

- `crud-response.interceptor.ts` will handle Mongoose Documents now
- mongoose `.toObject()` will return getters and virtuals
@yharaskrik
Copy link
Author

yharaskrik commented Dec 24, 2019

0.0.5 released, has support for using the nestjsx/crud interceptors and custom serialization now.

peerDependencies now include @nestjsx/crud@^4.3.3

@lafeuil
Copy link
Contributor

lafeuil commented Dec 30, 2019

I have tried your MongooseCrudService with the CrudAuth decorator. The MongooseCrudService does not include the filter in the query. After comparing with the OrmCrudService, this uses the parsed.search variable previously calculate to construct the query.

I think, in the createBuilder function, you should define the defaultSearch variable by calling the queryFilterToSearch function with parsed.search :

  public async createBuilder<K>(
    fn: (...args) => DocumentQuery<K, T>,
    parsed: ParsedRequestParams,
    options: CrudRequestOptions,
    many = true,
  ): Promise<{ builder: DocumentQuery<K, T>; take?: number; skip?: number }> {
    // get select fields
    const select = this.getSelect(parsed, options.query);
    // default search condition
    const defaultSearch = this.queryFilterToSearch(parsed.search);

    const builder = fn(defaultSearch);
    // select fields
    builder.select(select);

@yharaskrik
Copy link
Author

Hey @lafeuil, thanks for letting me know! I will take a look at that today, could you provide me with the code for the Crud controller you were using in the example you provided?

@lafeuil
Copy link
Contributor

lafeuil commented Dec 30, 2019

For example, if you want to filter the posts by the user that is authenticated :

@Crud({
  model: {
    type: Post,
  },
})
@CrudAuth({
  filter: (user) => {
    return { userId: user.id };
  },
@Controller('posts')
export class PostsController implements CrudController<Post> {
...
}

Example in the integration TypeOrm :

@CrudAuth({
filter: (user: User) => ({
userId: user.id,
}),

You can login a user automatically with the AuthGuard in the integration TypeOrm :

import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { UsersService } from './users';
import { USER_REQUEST_KEY } from './constants';
@Injectable()
export class AuthGuard implements CanActivate {
constructor(private usersService: UsersService) {}
async canActivate(ctx: ExecutionContext): Promise<boolean> {
const req = ctx.switchToHttp().getRequest();
req[USER_REQUEST_KEY] = await this.usersService.findOne(1);
return true;
}
}

@yharaskrik
Copy link
Author

Awesome thank you! Will take a look at that today!

@yharaskrik
Copy link
Author

Hey @zMotivat0r ,

Just wondering if you would know what is going on here as it is the crud-request code causing it.

When I run this test in the mongoose package:

it('should return an entities with limit and page', (done) => {
        const query = qb
          .setLimit(3)
          .setPage(1)
          .sortBy({ field: '_id', order: 'DESC' })
          .query();
        return request(server)
          .get('/users')
          .query(query)
          .end((_, res) => {
            expect(res.status).toBe(200);
            expect(res.body.data.length).toBe(3);
            expect(res.body.count).toBe(3);
            expect(res.body.total).toBe(9);
            expect(res.body.page).toBe(1);
            expect(res.body.pageCount).toBe(3);
            done();
          });
      });

The parsed.search property on the Request object is { '$and': [ undefined, {} ] }. This throws an error with mongoose when trying to use it to query on model.count(parsed.search).

For the time being I wrote some code to prevent invalid values to be used for building the query here:

parsed.search = Object.keys(parsed.search).reduce((prev: { [key: string]: any[] }, cur: string) => {
      const queries = parsed.search[cur].filter(query => isObjectFull(query));
      if (isArrayFull(queries)) {
        prev[cur] = queries;
      }

      return prev;
    }, {});

But would like to just be able to use parsed.search and have the crud-request code not provide undefined or {} in the $and key of the parsed.search param.

@yharaskrik
Copy link
Author

For example, if you want to filter the posts by the user that is authenticated :

@Crud({
  model: {
    type: Post,
  },
})
@CrudAuth({
  filter: (user) => {
    return { userId: user.id };
  },
@Controller('posts')
export class PostsController implements CrudController<Post> {
...
}

Example in the integration TypeOrm :

@CrudAuth({
filter: (user: User) => ({
userId: user.id,
}),

You can login a user automatically with the AuthGuard in the integration TypeOrm :

import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { UsersService } from './users';
import { USER_REQUEST_KEY } from './constants';
@Injectable()
export class AuthGuard implements CanActivate {
constructor(private usersService: UsersService) {}
async canActivate(ctx: ExecutionContext): Promise<boolean> {
const req = ctx.switchToHttp().getRequest();
req[USER_REQUEST_KEY] = await this.usersService.findOne(1);
return true;
}
}

Hey @lafeuil , my apologies I have not had time to look at this yet, will try and look tomorrow if I can find the time.

@michaelyali michaelyali changed the base branch from master to v5 January 29, 2020 03:36
@michaelyali michaelyali merged commit 8dfeae1 into nestjsx:v5 Jan 29, 2020
@9173860
Copy link

9173860 commented Mar 3, 2020

@yharaskrik Hi, Thanks for the package! I found search or filter in query is not working and seems not been implemented.

Is this related to the following issue?

Hey @zMotivat0r ,

Just wondering if you would know what is going on here as it is the crud-request code causing it.

When I run this test in the mongoose package:

it('should return an entities with limit and page', (done) => {
        const query = qb
          .setLimit(3)
          .setPage(1)
          .sortBy({ field: '_id', order: 'DESC' })
          .query();
        return request(server)
          .get('/users')
          .query(query)
          .end((_, res) => {
            expect(res.status).toBe(200);
            expect(res.body.data.length).toBe(3);
            expect(res.body.count).toBe(3);
            expect(res.body.total).toBe(9);
            expect(res.body.page).toBe(1);
            expect(res.body.pageCount).toBe(3);
            done();
          });
      });

The parsed.search property on the Request object is { '$and': [ undefined, {} ] }. This throws an error with mongoose when trying to use it to query on model.count(parsed.search).

For the time being I wrote some code to prevent invalid values to be used for building the query here:

parsed.search = Object.keys(parsed.search).reduce((prev: { [key: string]: any[] }, cur: string) => {
      const queries = parsed.search[cur].filter(query => isObjectFull(query));
      if (isArrayFull(queries)) {
        prev[cur] = queries;
      }

      return prev;
    }, {});

But would like to just be able to use parsed.search and have the crud-request code not provide undefined or {} in the $and key of the parsed.search param.

@yharaskrik
Copy link
Author

@9173860 I believe in the version that was merged into v5 did have search and filter working, the comment you reference is talking about some code that i haven't pushed yet.

Could you open a new issue with more info about them problem you are having? Thanks a lot.

@9173860
Copy link

9173860 commented Mar 4, 2020

@9173860 I believe in the version that was merged into v5 did have search and filter working, the comment you reference is talking about some code that i haven't pushed yet.

Could you open a new issue with more info about them problem you are having? Thanks a lot.

Thanks for the reply. I've opened #435 for reporting it with a demo.

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

Successfully merging this pull request may close these issues.

6 participants