Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Bad example of file type valiator for multer.fileFilter #1270

Open
incNick opened this issue Mar 18, 2016 · 10 comments
Open

Bad example of file type valiator for multer.fileFilter #1270

incNick opened this issue Mar 18, 2016 · 10 comments
Assignees
Milestone

Comments

@incNick
Copy link

incNick commented Mar 18, 2016

Source in file modules/users/server/controllers/users/users.profile.server.controller.js, some content like:

  var upload = multer(config.uploads.profileUpload).single('newProfilePicture');
  var profileUploadFileFilter = require(path.resolve('./config/lib/multer')).profileUploadFileFilter;

  // Filtering to upload only images
  upload.fileFilter = profileUploadFileFilter;

I just create one other controller use for save upload files, and reference above code, just found the fileFilter always not work. Correct way should be:

  var multerConfig = config.uploads.profileUpload;
  multerConfig.fileFilter = require(path.resolve('./config/lib/multer')).profileUploadFileFilter;
  var upload = multer(multerConfig).single('newProfilePicture');
@lirantal
Copy link
Member

@incNick would you like to submit a PR to propose a fix and explain it?

@incNick
Copy link
Author

incNick commented Mar 18, 2016

aHa, in fact I'm first time use github and also one new player of Node & Express so I don't know how to submit a PR.
Bur after read the source of multer, looks like the filter option only can assign when construct instance, so run multerInstance.fileFilter = something nothing change.

I'm sorry of can't provide you more help :(

@simison
Copy link
Member

simison commented Mar 18, 2016

@incNick, that's no problemo! This is a great change to get you started with Open Source contributions, Git and GitHub! :-) Up for a challenge? I'll spare some 5 minutes to write quick instructions. Probably easier if I write it for an app, since it's easier to get started with than command line (but lemme know if you'd like to try command line instead):

  1. Click Fork button on top-right corner at GitHub
  2. Open your fork https://github.com/incNick/mean
  3. Install https://desktop.github.com/
  4. Click image from your forked repository on GitHub. It opens the app and asks where to save your copy.
  5. At the app, click image to create a new branch
  6. Name branch something like "fix-multer-filetype-valiator"
  7. Open files, do your changes, make sure tests work ('npm test')
  8. When you're done, then again in the app, prepare the commit:
    image
      1. Choose "Uncommited Changes"
      1. Choose files (or click blue areas in files to commit only some lines)
      1. Write title, e.g. Fix(users): changes to multer filetype validator
      1. Write description, see guidelines (might look scary but don't worry if something isn't exactly as it should). Remember to reference this issue in your description by writing "Bad example of file type valiator for multer.fileFilter #1270"
      1. Commit changes to your branch
      1. Admire your first commit :-)
      1. Create a Pull request:

image

Volá! :-)

If you need to do changes, just commit and push to the same branch and they'll appear at the Pull Request here.

This was with OSX, should be quite similar with Windows or Linux clients.

Feel free to ask any questions here or just add me on any chat app. I'm always eager to get new people into open source community.

@incNick
Copy link
Author

incNick commented Mar 18, 2016

@simison sure, I think you are right, go further then need learn more :)
I just public my branch fix-multer-filetype-validator there, but I'm not sure you can see it.
Hope you have one very nice weekend.

@trendzetter
Copy link
Contributor

@incNick I have created the PR for you #1272

@trendzetter
Copy link
Contributor

If only I could reproduce the problem. The filetype-validator seems to work here, anyone else seeing the issue?

@trendzetter
Copy link
Contributor

I was able to reproduce the issue, see updates in pr #1272

@jamviking
Copy link

@simison thanks for the lesson...am a very newbie myself. Wish I had seen it before I attempted my first PR for the generator.

@lirantal
Copy link
Member

@simison kudos on that lovely invite for contributing on GitHub!
@trendzetter let's talk on your open PR for status

@mleanos
Copy link
Member

mleanos commented Oct 15, 2016

#1465

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

No branches or pull requests

6 participants