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

Added a new cookbook about file uploading #5375

Merged
merged 2 commits into from
Jun 19, 2015

Conversation

javiereguiluz
Copy link
Member

Q A
Doc fix? no
New docs? yes
Applies to all
Fixed tickets #2346

This PR is completely based on the work made by @saro0h in #4018. All the credit for the original work goes to her. I just picked her work and applied the suggestions made by @weaverryan @xabbuh and @wouterj to make it mergeable.

If you agree with this PR, please merge it as soon as possible because this is a very important topic and we're a bit late on this doc. Thanks!

Note that the type of the ``brochure`` column is ``string`` instead of ``binary``
or ``blob`` because it just stores the PDF file name instead of the file contents.

Then, add a new ``brochure`` field to the form that manages ``Product`` entities::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... manage the Product entity::

@weaverryan
Copy link
Member

👍 from me after the changes. It's simpler than most ways this is explained, and I like that.

*/
public function newAction(Request $request)
{
//...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well show the $form = $this->createForm() part - then this entry is totally complete

@javiereguiluz
Copy link
Member Author

@weaverryan thanks for your review! I've made all the suggested changes.

@weaverryan
Copy link
Member

Thanks Javier! Now, I also think we should remove http://symfony.com/doc/current/cookbook/doctrine/file_uploads.html entirely and then here, maybe add a note /link about how you might move the upload logic to a service and maybe even use a Doctrine listener if you are in fact using Doctrine. What do you think about that? The other article is not great - this is better (even though it's shorter).

@weaverryan weaverryan merged commit 4a7709b into symfony:2.3 Jun 19, 2015
weaverryan added a commit that referenced this pull request Jun 19, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Added a new cookbook about file uploading

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | all
| Fixed tickets | #2346

This PR is completely based on the work made by @saro0h in #4018. All the credit for the original work goes to her. I just picked her work and applied the suggestions made by @weaverryan @xabbuh and @wouterj to make it mergeable.

If you agree with this PR, please merge it as soon as possible because this is a very important topic and we're a bit late on this doc. Thanks!

Commits
-------

4a7709b Fixed all the issues spotted by Ryan
20ae21b Added a new cookbook about file uploading
xabbuh added a commit that referenced this pull request May 29, 2016
…outerJ)

This PR was squashed before being merged into the 2.3 branch (closes #6040).

Discussion
----------

Remove old File Upload article + improve the new one

| Q | A
| --- | ---
| Doc fix? | yes
| New docs? | yes
| Applies to | 2.3+
| Fixed tickets | #5375

The old file upload article wasn't good, a new one was written by @javiereguiluz but the old one remained online. This PR removes the old one and documents the missing bits in the new article.

Commits
-------

888c61c Remove old File Upload article + improve the new one
@javiereguiluz javiereguiluz deleted the upload_file branch May 24, 2018 15:59
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.

2 participants