nest icon indicating copy to clipboard operation
nest copied to clipboard

feat(common): adding the fieldname to validation errors

Open sezanzeb opened this issue 2 years ago • 5 comments
trafficstars

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ ] Bugfix
  • [x] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Other... Please describe:

What is the current behavior?

Issue Number: #12357

It's impossible to know based on the BadRequest response payload which field is causing problems

What is the new behavior?

The error message contains the field name

Does this PR introduce a breaking change?

  • [x] Yes
  • [ ] No

Other information

sezanzeb avatar Sep 18 '23 07:09 sezanzeb

Pull Request Test Coverage Report for Build 4bfe98f8-c530-4503-87ed-d0e96d8d1b74

  • 3 of 4 (75.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 92.653%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/common/pipes/parse-array.pipe.ts 3 4 75.0%
<!-- Total: 3 4
Totals Coverage Status
Change from base Build df58eebf-9777-4621-ad6a-397c0c18d45a: 0.04%
Covered Lines: 6469
Relevant Lines: 6982

💛 - Coveralls

coveralls avatar Sep 18 '23 07:09 coveralls

Should we also add the metadata.type so if there are multiple foo fields the user knows which one specifically caused the error (body, query, param, custom)?

jmcdo29 avatar Sep 29 '23 21:09 jmcdo29

I have started using those pipes together with a custom pipe to extract values from a nested payload.

If my body is like

{
  "foo": {
    "bar": "qux"
  }
}

I'll have to use something like @Body('foo', GetBar, ParseIntPipe)

This would cause the error message to be Validation failed (numeric string is expected in "foo").

I would much rather like this to be something like Validation failed (numeric string is expected in "foo.bar")

One solution I can think of, which would conveniently work for my use-case, is to create a custom decorator, like @BodyFoo('bar'), and to add a new optional field to metadata called name. My custom decorator could then overwrite metadata.name with "foo.bar".

sezanzeb avatar Oct 04 '23 07:10 sezanzeb

Another one might be to support using an array instead of a string: @Body(['foo', 'bar'], ParseIntPipe) for nested attributes.

sezanzeb avatar Oct 04 '23 07:10 sezanzeb

A third option would be to overwrite the name via the ParseIntPipeOptions, but I wouldn't be a huge fan of that one, because it adds redundant strings in the controllers class.

Any thoughts?

sezanzeb avatar Oct 04 '23 07:10 sezanzeb