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.

Sign up for GitHub

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

Jump to bottom

Provide Validation for FileInterceptor/UploadedFile to Ensure a File is Actually Uploaded #4752

Closed
WonderPanda opened this issue May 7, 2020 · 17 comments
Closed

Provide Validation for FileInterceptor/UploadedFile to Ensure a File is Actually Uploaded #4752

WonderPanda opened this issue May 7, 2020 · 17 comments
Labels
effort1: hours good first issue 👍 scope: common type: enhancement 🐺

Comments

@WonderPanda
Copy link
Contributor

Feature Request

Provide a built in way to ensure that when using @FileInterceptor that it is easy to validate the scenario where no file was actually sent to the backend

Is your feature request related to a problem? Please describe.

The current @UploadedFile implementation doesn't allow it to participate in Pipes validation. Even if it did, many users would have to implement a custom Pipe to ensure that the file was set in UserLand code. This seems like a common enough scenario that it would make sense for NestJS to support an out of the box solution

Describe the solution you'd like

One of:

  • Extend the existing @FileInterceptor so that it takes (optional) additional configuration that indicates an error should be thrown if no file/files are found at the specified fields.
  • Update the @UploadedFile decorator so that it can participate in Pipes validation and then provide a built in Pipe eg ParseFile

What is the motivation / use case for changing the behavior?

Seamless out of the box experience for dealing with file uploading. Less duplication in user land code to cover common community scenarios

Notes

I'd be happy to open up a PR for this if there is some guidance regarding which approach would be preferred
@WonderPanda WonderPanda added needs triage This issue has not been looked into type: enhancement 🐺 labels May 7, 2020
@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented May 8, 2020 •
edited
Loading

  • Extend the existing @FileInterceptor so that it takes (optional) additional configuration that indicates an error should be thrown if no file/files are found at the specified fields.
  • Update the @uploadedfile decorator so that it can participate in Pipes validation and then provide a built in Pipe eg ParseFile

I like both ideas 🙂 The second one should be easier to implement though.

Would you like to create a PR with eg ParseFile you mentioned? I'll chime in then and make sure that pipes are evaluated for files too.

@kamilmysliwiec kamilmysliwiec added scope: common and removed needs triage This issue has not been looked into labels May 8, 2020
@rubiin
Copy link
Contributor

rubiin commented May 26, 2020

@WonderPanda both of the ideas look great.

@WonderPanda
Copy link
Contributor Author

Apologies guys life has been super busy the last couple weeks as I'm transitioning jobs at the moment. I'll look at getting a PR up for this this week

@Langstra
Copy link

@WonderPanda really interested in this, both ideas seem great!

@mertcanekiz
Copy link

Any updates on this?

@JeromeGill
Copy link

Is anyone working on this?

@kamilmysliwiec
Copy link
Member

@JeromeGill @mertcanekiz @Langstra PRs are more than welcome! There's no PR targeting this issue atm

@omer-morad-ni
Copy link

Hi,

@kamilmysliwiec is this issue still need a PR?
Or someone is already working on one?

I would love to give it a shot :)

@kamilmysliwiec
Copy link
Member

@omer-morad-ni yes, PR would be more than welcome

@patryk-bernasiewicz
Copy link

@omer-morad-ni any word on your idea? :)

@omer-morad-ni
Copy link

Sorry for the late response!
I did not have time to progress so much, I was loaded with a lot of work in the last month 😭

lucavb added a commit to lucavb/nest that referenced this issue Feb 7, 2021
@lucavb
feat(platform-express): Allowing pipes for FileInterceptor
31d91fe 
also adding types for multer so that we can specify the interfaces

nestjs#4752
lucavb added a commit to lucavb/nest that referenced this issue Feb 7, 2021
@lucavb
feat(platform-express): Allowing pipes for FileInterceptor
30b028c 
also adding types for multer so that we can specify the interfaces

nestjs#4752
@lucavb lucavb mentioned this issue Feb 7, 2021
Merged
3 tasks
lucavb added a commit to lucavb/nest that referenced this issue Feb 7, 2021
@lucavb
feat(platform-express): Allowing pipes for FileInterceptor
fda6664 
nestjs#4752
lucavb added a commit to lucavb/nest that referenced this issue Feb 7, 2021
@lucavb
feat(platform-express): Allowing pipes for UploadedFile
f3864d6 
nestjs#4752
lucavb added a commit to lucavb/nest that referenced this issue Feb 7, 2021
@lucavb
feat(common): Allowing pipes for UploadedFile / UploadedFiles
b3c53fc 
nestjs#4752
lucavb added a commit to lucavb/nest that referenced this issue Feb 8, 2021
@lucavb
feat(common): Allowing pipes for UploadedFile / UploadedFiles
2ceddd6 
nestjs#4752
lucavb added a commit to lucavb/nest that referenced this issue Mar 31, 2021
@lucavb
fix(core): marking files as pipeable
5ad26b5 
The two decorators @uploadedfile and @uploadedfiles have been marked as pipeable,
however this had no affect as this method was preventing their execution.

Original Issue nestjs#4752
Bug fixed by this commit nestjs#6344 (comment)
@lucavb lucavb mentioned this issue Mar 31, 2021
Merged
3 tasks
@mdaj06
Copy link

mdaj06 commented Aug 12, 2021

@kamilmysliwiec is this issue still open, if so i could help out!

@kamilmysliwiec
Copy link
Member

Still open yes! @mdaj06

@mdaj06
Copy link

mdaj06 commented Aug 12, 2021

Shall I have a go at it @kamilmysliwiec ?

@kamilmysliwiec
Copy link
Member

Sounds great!

mightyYaroslav pushed a commit to mightyYaroslav/nest that referenced this issue Aug 28, 2021
@daYaroslav
feat(platform-express): file interceptor no files validation
6b258f2 
This adds an option for file interceptor to throw if no files are found

Closes nestjs#4752
@mightyYaroslav mightyYaroslav mentioned this issue Aug 28, 2021
Closed
12 tasks
@coolsamk7 coolsamk7 mentioned this issue Jan 19, 2022
Closed
12 tasks
This was referenced May 31, 2022
Closed
Closed
Merged
@dzcpy
Copy link

dzcpy commented Jun 23, 2022

Any solutions so far?

@kamilmysliwiec
Copy link
Member

This will be added in v9.0.0

@nestjs nestjs locked and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
effort1: hours good first issue 👍 scope: common type: enhancement 🐺
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(platform-express): file interceptor no files validation mightyYaroslav/nest
10 participants
@dzcpy @Langstra @mertcanekiz @JeromeGill @WonderPanda @rubiin @patryk-bernasiewicz @kamilmysliwiec @mdaj06 @omer-morad-ni