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

Update src/Symfony/Component/Form/Extension/Core/Type/FileType.php #5028

Closed
wants to merge 2 commits into from

Conversation

stfalcon
Copy link
Contributor

@stfalcon stfalcon commented Jul 24, 2012

@webmozart
Copy link
Contributor

@webmozart webmozart commented Jul 24, 2012

Thank you for the PR! Could you please add a test case?

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Jul 25, 2012

Could you please add a test case?

And what to check? I added one static option :)

@webmozart
Copy link
Contributor

@webmozart webmozart commented Jul 25, 2012

Whatever was the reason for adding the option ;) The test should fail when the option is not added and succeed when the option is there.

Probably it is sufficient to create a new field of type "file" in the test which comes prefilled with a File object.

$file = $this->getMock('Symfony\Component\HttpFoundation\File\File');
$this->factory->create('file', $file)

@stfalcon stfalcon closed this Aug 27, 2012
@stfalcon stfalcon reopened this Nov 12, 2012
@stfalcon stfalcon closed this Nov 15, 2012
@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 15, 2012

sorry, it's bug in VichUploaderBundle

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 21, 2012

or not :)

@stfalcon stfalcon reopened this Nov 21, 2012
@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 22, 2012

@bschussek done! it was really a bug with FileType

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 22, 2012

@stof who can merge it? I want close this bug dustin10/VichUploaderBundle#27 :)

@stof
Copy link
Member

@stof stof commented Nov 23, 2012

@stfalcon the rule is that only @fabpot merges PRs on symfony.

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 23, 2012

@fabpot do you have a minute :)? it's simple PR but many people wait for it

@fvilpoix
Copy link

@fvilpoix fvilpoix commented Nov 26, 2012

Hello,

Since this update, I've got a 500 error instead of validation failure when I try to post a form without a required file.

Warning: Missing argument 1 for Symfony\Component\HttpFoundation\File\File::__construct(), called in […]/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/FormType.php on line 172 and defined in […]/vendor/symfony/symfony/src/Symfony/Component/HttpFoundation/File/File.php line 38

@piotrantosik
Copy link
Contributor

@piotrantosik piotrantosik commented Nov 26, 2012

I have same error as @fvilpoix

@choomz
Copy link

@choomz choomz commented Nov 27, 2012

I have the same error on my side. This change means that we must necessarily provide Symfony File objects on forms input and output ?

I fail to see how this can work as in this case, nothing is passed to the File constructor.

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 28, 2012

Hmm. @bschussek, "Houston, we have a problem"

@Sybio
Copy link

@Sybio Sybio commented Nov 30, 2012

+1 same error !

@stfalcon
Copy link
Contributor Author

@stfalcon stfalcon commented Nov 30, 2012

@fabpot can you revert changes? and reopen this issue.

@dchaffin
Copy link

@dchaffin dchaffin commented Nov 30, 2012

I am getting this same error.

@dchaffin
Copy link

@dchaffin dchaffin commented Nov 30, 2012

I did find this 'fix':

#6134

@frodosghost
Copy link

@frodosghost frodosghost commented Dec 3, 2012

Same error.

@cedric-pouilleux
Copy link

@cedric-pouilleux cedric-pouilleux commented Dec 3, 2012

Yes, I have the same error, when i edit, and when the field file is empty :(

@gzankevich
Copy link

@gzankevich gzankevich commented Dec 4, 2012

The fix posted by dchaffin works for me. It's a little worrying that a security update (2.1.4) ended up breaking functionality.

@ghost
Copy link

@ghost ghost commented Dec 7, 2012

Same error and the fix posted by dchaffin doesn't works for me :/

@alainivars
Copy link

@alainivars alainivars commented Dec 11, 2012

Same error to minoz and the fix posted by dchaffin doesn't works for me to

@webmozart
Copy link
Contributor

@webmozart webmozart commented Dec 13, 2012

Fixed in #6319

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

Successfully merging this pull request may close these issues.