doug-martin / nestjs-query

Easy CRUD for GraphQL.
https://doug-martin.github.io/nestjs-query
MIT License
822 stars 142 forks source link

Pass DTO class or Entity class? #1494

Open thehappycoder opened 2 years ago

thehappycoder commented 2 years ago

I've noticed that InjectQueryService and QueryService expect DTOs to be passed, according to the code.

In the documentation though, examples hint that entities should be passed

@InjectQueryService(TodoItemEntity) service: QueryService<TodoItemEntity>

Practically, it seems that InjectQueryService definitely only works with an entity while QueryService seems not to mind either DTO or entity.

smolinari commented 2 years ago

It should be the entity for the injection and for the decorator the output DTO. And, if you use the AssemblerQueryService (extend from it) you would need to give it both the entity and the output DTO. Something like this:

@Injectable()
@QueryService(UserDTO)
export class UserService extends AssemblerQueryService<UserDTO, UserEntity> {
  constructor(
    readonly assembler: UserAssembler,
    @InjectQueryService(UserEntity) private readonly userService: QueryService<UserEntity>
  ) {
    super(assembler, userService)
  }

Scott

andefred commented 2 years ago

@smolinari I've been struggling with this same thing and can't get this straight because code and documentation directly contradicts each other. Let's say the following scenario you want to have a custom resolver extending the CRUDResolver as well as a custom service.

from the docs the todo-item.service.ts should look like this:

import { Model } from 'mongoose';
import { QueryService } from '@nestjs-query/core';
import { InjectModel } from '@nestjs/mongoose';
import { MongooseQueryService } from '@nestjs-query/query-mongoose';
import { TodoItemEntity } from './todo-item.entity';

@QueryService(TodoItemEntity)
export class TodoItemService extends MongooseQueryService<TodoItemEntity> {
  constructor(@InjectModel(TodoItemEntity.name) model: Model<TodoItemEntity>) {
    super(model);
  }

  async markAllAsCompleted(): Promise<number> {
    const entities = await this.query({ filter: { completed: { is: true } } });

    const { updatedCount } = await this.updateMany(
      { completed: true }, // update
      { id: { in: entities.map((e) => e.id) } } // filter
    );
    // do some other business logic
    return updatedCount;
  }
}

and then the resolver should look like

import { QueryService, InjectQueryService } from '@nestjs-query/core';
import { CRUDResolver } from '@nestjs-query/query-graphql';
import { Resolver } from '@nestjs/graphql';
import { TodoItemDTO } from './todo-item.dto';
import { TodoItemEntity } from './todo-item.entity';

@Resolver(() => TodoItemDTO)
export class TodoItemResolver extends CRUDResolver(TodoItemDTO) {
  constructor(
    @InjectQueryService(TodoItemEntity)
    readonly service: QueryService<TodoItemEntity>
  ) {
    super(service);
  }
}

However, the above creates type check errors saying that the TodoItemResolver incorrectly extends the base class. And if you change it to TodoItemDTO instead then it gives the compilation error: Nest can't resolve dependencies of the TodoItemResolver (?). Please make sure that the argument TodoItemDTOQueryService at index [0] is available in the TodoItemModule context

Thanks in advance!

andefred commented 2 years ago

After posting this I figured out what was missing. It turns out that the @InjectQueryService should after all have the TodoItemDTO class and same for the QueryService type.

The problem is then that the custom service must be named exactly TodoItemDTOQueryService for it to work.

Here is the full functioning example for others who come here: todo-item-dto-query.service.ts

import { Model } from 'mongoose';
import { QueryService } from '@nestjs-query/core';
import { InjectModel } from '@nestjs/mongoose';
import { MongooseQueryService } from '@nestjs-query/query-mongoose';
import { TodoItemEntity } from './todo-item.entity';

@QueryService(TodoItemEntity)
export class TodoItemDTOQueryService extends MongooseQueryService<
  TodoItemEntity
> {
  constructor(@InjectModel(TodoItemEntity.name) model: Model<TodoItemEntity>) {
    super(model);
  }

  async markAllAsCompleted(): Promise<number> {
    const entities = await this.query({ filter: { completed: { is: true } } });

    const { updatedCount } = await this.updateMany(
      { completed: true }, // update
      { id: { in: entities.map((e) => e.id) } } // filter
    );
    // do some other business logic
    return updatedCount;
  }
}

todo-item.resolver.ts

import { Filter, InjectQueryService, QueryService } from '@nestjs-query/core';
import { ConnectionType, CRUDResolver } from '@nestjs-query/query-graphql';
import { Args, Query, Resolver } from '@nestjs/graphql';
import { TodoItemDTO } from './todo-item.dto';
import { TodoItemConnection, TodoItemQuery } from './types';
import { CreateTodoItemDTO } from './todo-item-input.dto';

@Resolver(() => TodoItemDTO)
export class TodoItemResolver extends CRUDResolver(TodoItemDTO, {
  CreateDTOClass: CreateTodoItemDTO,
  enableAggregate: true,
  enableSubscriptions: true,
  enableTotalCount: true,
}) {
  constructor(
    @InjectQueryService(TodoItemDTO)
    readonly service: QueryService<TodoItemDTO>
  ) {
    super(service);
  }

  // Set the return type to the TodoItemConnection
  @Query(() => TodoItemConnection)
  completedTodoItems(
    @Args() query: TodoItemQuery
  ): Promise<ConnectionType<TodoItemDTO>> {
    // add the completed filter the user provided filter
    const filter: Filter<TodoItemDTO> = {
      ...query.filter,
      ...{ completed: { is: true } },
    };

    return TodoItemConnection.createFromPromise((q) => this.service.query(q), {
      ...query,
      ...{ filter },
    });
  }

  // Set the return type to the TodoItemConnection
  @Query(() => TodoItemConnection)
  uncompletedTodoItems(
    @Args() query: TodoItemQuery
  ): Promise<ConnectionType<TodoItemDTO>> {
    // add the completed filter the user provided filter
    const filter: Filter<TodoItemDTO> = {
      ...query.filter,
      ...{ completed: { is: false } },
    };

    return TodoItemConnection.createFromPromise((q) => this.service.query(q), {
      ...query,
      ...{ filter },
    });
  }
}
smolinari commented 2 years ago

@andefred

If you are creating your own TodoItem service, what is the purpose of injecting the base query service?

And, what you found to work or rather your deductions about service naming can't be correct. I don't follow any special naming conventions for my services and everything works flawlessly.

Scott

andefred commented 2 years ago

@smolinari I'm not sure I understand your question. I'm thinking that the whole point of having a custom service is to modify the existing service, and not write your own service from scratch. Maybe I just want to add one small additional method that hooks into the other service methods etc. And then if I have my own service, then of course it is the one that should be injected into the Resolver.

And using the code directly from the documentation does not work at as is. Ie using the Entity in the query service doesn't work. But using the code I provided above works, and it was only working once I changed the service name to the "correct" name. Also, using a custom service works well when using the autogenerated resolver too, but that worked with whatever service name you want.

smolinari commented 2 years ago

I'm thinking that the whole point of having a custom service is to modify the existing service

Correct, However, by calling the base query service in the resolver, your additional method is not at all callable. So, for instance, if you wanted to create a "markAllAsCompleted" mutation, you couldn't call your service method to make it happen.

Scott