Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse file pipe fix (multiple files validation) #10737

Merged
merged 2 commits into from Feb 1, 2023

Conversation

mahkassem
Copy link
Contributor

@mahkassem mahkassem commented Dec 18, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • 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?

The parse file validation pipe doesn't work if multiple files were passed.

Issue Number: N/A

What is the new behavior?

  • ParseFilePipe will now handle multiple files validations where the IUploadValidatorOptions will be applied to each file.
  • It still works with single file as it already was.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I'm aware of the existing pull-request that introduces a similar solution, I tried to help with it but it seems to be abandoned, so I create this one to help ship this improvement quicker.

@mahkassem mahkassem changed the title Parse file pipe fix (support multiple files) Parse file pipe fix (multiple files) Dec 18, 2022
@coveralls
Copy link

Pull Request Test Coverage Report for Build 0bdc1286-e6fc-42e3-8026-8982426e0fcd

  • 6 of 9 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 93.364%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/common/pipes/file/parse-file.pipe.ts 6 9 66.67%
Totals Coverage Status
Change from base Build 0701e559-e0cf-4feb-9bad-f5fcb514c782: -0.04%
Covered Lines: 6205
Relevant Lines: 6646

💛 - Coveralls

@mahkassem mahkassem changed the title Parse file pipe fix (multiple files) Parse file pipe fix (multiple files validation) Dec 18, 2022
@kamilmysliwiec
Copy link
Member

LGTM

@kamilmysliwiec kamilmysliwiec merged commit 32b0d9c into nestjs:master Feb 1, 2023
@micalevisk
Copy link
Member

micalevisk commented Feb 1, 2023

note that this PR had introduced a regression because of the eager import of class-validator which is an optional package for @nestjs/common as long as you don't use few features -- fixed at 91f7190

We might clarify that on CONTRIBUTING.md

@mahkassem
Copy link
Contributor Author

note that this PR had introduced a regression because of the eager import of class-validator which is an optional package for @nestjs/common as long as you don't use few features -- fixed at 91f7190

We might clarify that on CONTRIBUTING.md

Thank you, I will be careful with any future PRs

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

Successfully merging this pull request may close these issues.

None yet

5 participants