Validate Input for Create & Update

[johannesschobel]

Dear @doug-martin ,

i was trying to validate input for my CreateX and UpdateX mutations. Note that i use the default ValidationPipe provided by @nestjs/common and class-validator package.

My ValidationPipe is defined as follows:

app.useGlobalPipes(
    new ValidationPipe({
      transform: true,
      whitelist: true,
      forbidNonWhitelisted: true,
      skipMissingProperties: false,
      forbidUnknownValues: true
    })
  );

This definition, in turn, states, that ONLY defined attributes may pass, unknown properties automatically result in errors and so on. As i register the ValidationPipe as a GlobalPipe it is automatically applied to all endpoints! I can confirm, that this works for me.

While this may be better from the securities point of view, this results in the following issue: As the data sent to the backend is "wrapped" in the input param, it cannot be validated. Note, that the ValidationPipe will exit with a failure, as it detects an undeclared field input.

As i do not want to miss out on this feature, i digged into your library and found that it is possible to write custom ArgTypes for Create/Update - which is awesome 👍 .

I then created my own Type like this:

import { Type } from 'class-transformer';
import { ValidateNested } from 'class-validator';
import { ArgsType, Field } from 'type-graphql';
import { CreateIdentityInput } from './modules/identity/ui/graphql/inputs/create-identity.input';

@ArgsType()
export class CreateOneUserArgs extends CreateOneArgsType(
  CreateUserInput
) {
  @Type(() => CreateUserInput)
  @ValidateNested()
  @Field({
    description: 'the resource to be created'
  })
  input!: CreateUserInput;
}

What this basically does:

• @Type(() => CreateUserInput) maps the content of the variable to respective class (i.e., it is mapped to the CreateUserInput class).
• @ValidateNested() tells the class-validator package, to ALSO validate the rules in the CreateUserInput class (e.g., the field email must be a valid email, username must be at least 5 chars long, ...)

---

So my question is:
Would it be ok for you to merge such changes into your own code-base? I guess, the Validation feature would be a significant improvement for your library. What do you think? I would be willing to make a PR - because it actually would save me a lot of time instead of creating my own custom ArgsType()?

From the first glimpse, those chances may be directly applied to the CreateOneArgsType and UpdateOneArgsType. For the CreateManyArgsType and UpdateManyArgsType we may need to use the @ValidateNested({ each: true }) option, in order to validate each element individually..

All the best,
Johannes

[doug-martin]

Thanks for the detailed issue @johannesschobel! Yes I think this would be a great addition.

[johannesschobel]

Shall i make a PR for you? Or will you work on this?
All the best

[doug-martin]

Yeah if you want to make a PR that would be great!

[johannesschobel]

Dear @doug-martin ,

i just wanted to create a PR for this issue. My first approach was like this:

import { Class } from '@nestjs-query/core';
import { Field, ArgsType } from 'type-graphql';
import { ValidateNested } from 'class-validator';
impor { Type } from 'class-transformer';

export interface CreateOneArgsType<C> {
  input: C;
}

export function CreateOneArgsType<C>(ItemClass: Class<C>): Class<CreateOneArgsType<C>> {
  @ArgsType()
  class CreateOneArgs implements CreateOneArgsType<C> {
    @Type(() => C) // <-- this fails because 'C' only refers to a type, but is being used as a value here.
    @ValidateNested()
    @Field(() => ItemClass, { description: 'The record to create' })
    input!: C;
  }
  return CreateOneArgs;
}

However, this does not work, because 'C' only refers to a type, but is being used as a value here.

Any idea how to cope with this?
All the best

[doug-martin]

Yep,you need to pass in the ItemClass, similar to how it is passed to field.

If you take a look at https://github.com/doug-martin/nestjs-query/blob/master/packages/query-graphql/src/types/query/query-args.type.ts#L46 you'll see I had to do the same thing.

[johannesschobel]

ok, i don't have any idea how to test this properly.. because i would need the ValidationPipe, make a proper request to the endpoint, send correct and wrong data, check the responses and so on.. or do you have any better idea how to do this in a more sane way? 😆

[doug-martin]

For the unit tests I've been adding spys just to the decorators to make sure everything is wired up properly.

For e2e I haven't automated that yet, its on my todo list. But I usually add something to the example and test manually.

[doug-martin]

You might also checkout https://github.com/doug-martin/nestjs-query/blob/master/packages/query-graphql/__tests__/types/query/query-args.type.spec.ts

In there I use class validator just to ensure everything is getting validated as expected.

[johannesschobel]

my first approach would have also been to use the ./example application within the package. However, this specifically targets a dedicated version of @nestjs-query/x in the package.json file.

Maybe we can just use

"@nestjs-query/core": "./../../packages/core"`

in the package.json file to "link" it to the local dev-version?

[doug-martin]

Oh, yeah when developing run npm run bootstrap from the root directory which install all dependencies properly and link the packages together.

[johannesschobel]

yeah, the linking is not the problem, i guess. its more that the package.json file in the /example folder does not use the proper "development" version of the package in the root folder but specifies the 0.2.1 version, which would then be pulled from NPM (without my local changes).

wait.. i may be an idiot on this one.. lol.. gimme a second

[johannesschobel]

ok.. i cannot get the first initial proposal of this issue to work in the example application.. It's not working with the custom ArgsType for me.. i will try again tomorrow with a clean install..

However, in my own "real application" i am building it works fine with the custom argstype.. in the demo application it wont work.. maybe some kind of dependency problem (version!)

[doug-martin]

If you can provide a branch I may be able to help.

[izundo-viennv]

@johannesschobel @doug-martin How's it going, guys? I'm waiting for your PR

[doug-martin]

Published under @nestjs-query/[email protected]

[johannesschobel]

oh man.. that's awesome! thanks a lot