[Bug]: Defining additional UpdateDtos breaks Schema

[johannesschobel]

Dear @doug-martin ,

while working on custom resolver methods, i stumbled upon an issue, i was able to reproduce in your example repository (shipped in this library).

You can take a look at the "how to reproduce" by looking at my commits here

master...johannesschobel:issue/update-dto

Description

I wanted to add a dedicated completeTodoItem mutation that only takes the completed parameter. Note that the original updateTodoItem also takes the name, and completed was set to optional. The dedicated completeTodoItem mutation should take this parameter as required.

• I created a new Input DTO in order to provide sophisticated validations
• I created a new InputType() that uses this dto
• I created a new Mutation in my Resolver that uses this newly created InputType.

Note that i also committed the initial schema.gql file to see the actual changes in this file!

As you can see, the new mutation is added correctly. Also, the newly created InputType as well as the dto are added correctly. However, the dto for the regular UpdateOneTodoItem is also changed to this newly created dto - which is, obviously, wrong.

I hope you can reproduce the issue? Maybe i am doing something wrong here? As the docs were not quite accurate in this point, i was not sure if my approach is correct. I would be willing to document this feature, if we can figure out how to deal with the latter?

All the best and thanks for your time!

[doug-martin]

Yep, I see the issue.

In https://github.com/doug-martin/nestjs-query/blob/master/packages/query-graphql/src/types/update-one-input.type.ts#L23 we set the InputType name. I think the fix would be to update it to be abstract and let the implementing code specify the InputType name.

This would apply for all the create, update and delete types. Ill try to take a look at this over the next couple of days unless you want to tackle it.

[johannesschobel]

Dear @doug-martin ,

thanks for confirming this bug. Glad i did nothing wrong here 😄

Basically, you would change it to something liket his, right?

@InputType()
export class CompleteTodoItemInputType extends UpdateOneInputType(
+ 'CompleteTodoItemInput',
  TodoItemDTO, 
  CompleteTodoItemDTO
) {}

Am I correct?
All the best,
Johannes

[doug-martin]

@johannesschobel I'm working on this now.

It looks like its going to be updated to

@InputType()
export class CompleteTodoItemInputType extends UpdateOneInputType(
  TodoItemDTO, 
  CompleteTodoItemDTO
) {}

and the UpdateOneInputType will be changed to

export function UpdateOneInputType<DTO, U extends DeepPartial<DTO>>(
  DTOClass: Class<DTO>,
  UpdateType: Class<U>,
): Class<UpdateOneInputType<DTO, U>> {
  const metadataStorage = getMetadataStorage();
  const existing = metadataStorage.getUpdateOneInputType<DTO, U>(DTOClass);
  if (existing) {
    return existing;
  }
  const { baseName } = getDTONames(DTOClass);
-  @InputType(`UpdateOne${baseName}Input`)
+  @InputType({ isAbstract: true })
  class UpdateOneInput implements UpdateOneInputType<DTO, U> {
    @IsNotEmpty()
    @Field(() => ID, { description: 'The id of the record to update' })
    id!: string | number;

    @Type(() => UpdateType)
    @ValidateNested()
    @Field(() => UpdateType, { description: 'The update to apply.' })
    update!: U;
  }
  metadataStorage.addUpdateOneInputType(DTOClass, UpdateOneInput);
  return UpdateOneInput;
}

[doug-martin]

@johannesschobel I've pushed up my changes to https://github.com/doug-martin/nestjs-query/tree/issue72 I'll finish cleaning up the docs and stuff tomorrow.

I ended up changing the the UpdateOneInputType method to the following

export function UpdateOneInputType<U>(UpdateType: Class<U>): Class<UpdateOneInputType<U>> {
  @InputType({ isAbstract: true })
  class UpdateOneInput implements UpdateOneInputType<U> {
    @IsNotEmpty()
    @Field(() => ID, { description: 'The id of the record to update' })
    id!: string | number;

    @Type(() => UpdateType)
    @ValidateNested()
    @Field(() => UpdateType, { description: 'The update to apply.' })
    update!: U;
  }
  return UpdateOneInput;
}

Once I publish you should be able to just extend the type like so

@InputType()
export class CompleteTodoItemInputType extends UpdateOneInputType( CompleteTodoItemDTO) {}

And the graphql type will be named CompleteTodoItemInputType.

Let me know what you think and if I should make any additional changes.

Stay safe and healthy!

-Doug

[johannesschobel]

Dear @doug-martin ,
thanks for sharing your thoughts on this and discussing the further roadmap with me!
Actually, i like the idea of just creating it as an abstract class! For the "dummy" implementation (i.e., no custom type was defined), the abstract class is extended with no particular changes, right?

So basically - for the developer using your awesome library, nothing changes ;)

All the best and stay safe,
Johannes

[doug-martin]

This is published under v0.8.0 I had to make breaking changes to the create and update input types, since the no longer required the "base" dto.

So your update type would change from

@InputType()
export class CompleteTodoItemInputType extends UpdateOneInputType(TodoItemDTO, CompleteTodoItemDTO) {}

To

@InputType()
export class CompleteTodoItemInputType extends UpdateOneInputType(CompleteTodoItemDTO) {}

Small change but still breaking. You can checkout the types documentation for more info.

Thanks again for the bug report!

[johannesschobel]

Dear @doug-martin
thank you very much for the new release.
The time needed to adapt to the latest changes will be minimal - thank you for keeping the public API so clean and slim! Very much appreciated!

Can you please release the new version on npm? It has been tagged on GitHub but not released on NPM so far..

All the best and thank you very much for your time and effort working on this library!

[doug-martin]

Sorry about that its published!

[johannesschobel]

you are awesome.. happy eastern - wish you all the best!
will try to update the lib next week!

[johannesschobel]

Dear @doug-martin,
just wanted to give a short feedback, that i managed to upgrade to the latest version of your library. Works like a charm! Thanks a lot for your awesome support and the time invested in this library.