newforms: Implement FileField and ImageField

The FileInput widget supports <input type="file">.

Then, on form_for_model() shouldn't it use that widget instead of Char ? (On my last try it used a simple text box with no "Browse" button). Gonna try later anyway ;)

same problem with "forms.models.form_for_model" (revision 4325).

Output for the FileField:

<tr><th><label for="id_data">Data:</label></th><td><input type="text" name="data" id="id_data" /></td></tr>

FYI:

>>> File._meta.fields[2].name
'data'
>>> File._meta.fields[2]
<django.db.models.fields.FileField object at 0xb778c6ac>

>>> f = forms.models.form_for_model(File).fields['data']
>>> f.widget
<django.newforms.widgets.TextInput object at 0xb66ebb2c>
>>> f.widget.input_type
'text'

Even with this small patch in "db/models/fields/init.py:

class FileField(Field):
...
    def formfield(self, initial=None):
        return forms.CharField(required=not self.blank, widget=forms.FileInput, label=capfirst(self.verbose_name), initial=initial)

... the field doesn't pass the validation. Output for the FileField was in oldforms:

<tr><th><label for="id_data_file">Data:</label></th><td><input type="file" id="id_data_file" name="data_file" /><input type="hidden" id="id_data" name="data" value="" /></td></tr>

(don't we need the hidden input in newforms?)

This is what I have in my template:

<form enctype="multipart/form-data" action="" method="post">
  <table class="form">
    {{ form }}
  </table>
  <input type="submit" value="Add File" />
</form>

On my setup it doesn't seem to work without the hidden field.. ;(

Milestone Version 1.0 deleted

quick and dirty FileField? support that appears to work

So, I jumped the gun to newforms without realizing FileFields? wouldn't work right. Since newforms is otherwise delightful, I have whipped up a stop-gap implementation for myself and anyone else who misses their FileFields?. I'm somewhat assuming that the newforms developers have a grand plan in mind for how this should actually work, and assuming this patch will be eventually mooted. As such, I haven't gone super-fancy. If you would like a fancier patch, feel free to let me know and I'll see what I can do.

Er, and I'm going to need to upload a new version of the patch because I accidentally left in part of some other patches I have applied. (I'm also running rediff this time...)

fixed quickie implementation for FileFields?

remove additional commit conditional check that shouldn't have existed

What would be great was if the filenames were decided before saving the models, then saved as that filename after all models are saved.

diff file with what you have to do to patch v4457

Folks, fixed2-quickie-file-uploads.diff didn't work for me, because it was for newforms-admin branch version 4421, and I'm using newforms-admin branch version 4457. So our tech lead fiddled, and made fixed2-quickie-file-uploads.diff work for 4457. Now, I'm not sure how to create a patch file, so I'm just sending the diff.

Mae

Hi anonymous, the state "Accepted" should be left as is, it means "This has to be worked out and fixed". It should move then to "Ready for check-in" or be closed as invalid. If you look at this diagram you'll see there's no way back to Unreviwed!!

About the patch, if you are using a SVN checkout, simply run "svn diff > my_own_patch.patch" or "svn diff <your modified files> > my_own_patch.patch" that should leave you with a "my_own_patch.patch" file which contains the differences between your local copy and your current revision number. So please do 'svn up' before anything.

ALSO, make sure your patch applies to trunk! If not, leave a patch for trunk and if you like, another for newforms-admin. But make sure you don't brake patches for trunk, after all, unless otherwise specified a patch is to be applied to trunk.

PS: Removed the "patch needs improvement" as the patch on trunk applies fine, someone will have to deal with the conflict (adrian?) on newforms-admin if it gets checked-in, or it will need to be improved if adrian merges his branch before this ticket gets in.

diff.txt as patch, checked against trunk.

Checked against trunk, made file saving overridable in filefield as newforms_save_file

Patch does not work with prefixed forms.

required validation + formfield_save_file

My bad, did work with prefixed forms just forgot to prefix the instanciated forms.

Added validation for required filefields.

Made the patch work more like filefields works in oldforms by making saving of files a method (formfield_save_file) of the fieldtype, instead of calling save_FIELD_file directly.

Added tests and updated to trunk

Removed one comment too many, fixed

The patch seems fine and has no known issues, can this be set as "Read for check-in"?

Oops, validation of filefields has errors. How can the field know if it is a modelform or instanceform?

Needs testing with form_for_instance.

ImageField? with validation added

Better validationerrors

Added one more validator, fixed formfield_save_file, and better tests

Seems to work fine now.

Corrected for multiple model save() , like manipulators in #4609, updated for rev 4664

I assume support@microsoft.com doesn't *really* want to hear about this.

I'd like to point out that there is the FileInput? widget, which is fine for _uploading_ a file, but for _displaying_ a file or an image, on should display a link/thumbnail/whatever.

So I made this little widget:

from django.newforms.widgets import Widget, smart_unicode
import os

class File(Widget):
    def render(self, name, value, attrs=None):
        if value is None or value=='': return u'No file uploaded'
            
        if value != '': value = smart_unicode(value) # Only add the 'value' attribute if a value is non-empty.
        
        return u'<a href="%s">%s</a>' % (value,os.path.basename(value))

It's very basic, and needs to be passed the absolute url of a file (easy for models using get_FIELD_url). But it works.

To use it, I define a separate field that uses it, and I manually set it to the model's data.

django.newforms.models.save_instance() should ignore fields that are not in the data. This will make it easy to make your own forms manually that only handle some of the fields, then set the rest of the fields explicitly in your view. For example the automatically filled in width and height fields you can specify for a model ImageField? should not be attempted to be set since they will get set by some other code.

I will attach my updated patch that has these two if-statements added :P

updated to handle fields not handled by the form

Refactored code to handle fields not handled by the form, and save files after all other fields

As I think putting the current value out to the web could lead to issues and may not be secure enough (user can skip a required field or submit anything to the script, NEVER trust user-data).

So I rewrote the patch, to not need this additional <input> anymore:

• newforms.form_for_instance() sets formfield.required to False for every FileField? that has some file attached
• Forms that are written by the user should imitate this behavior if they want to use FileFields? (perhaps a generic function can be added to newforms.models to accomplish this?)

Note about the implementation: This can be done using a different variable, too. I tried using formfield.has_file_attached. But you have a problem here with newforms.Field.clean(). It checks whether the value is in EMPTY_VALUES and raises an Exception regardless of formfield.has_file_attached. So you have to skip super(FileField?, self).clean(value), which I considered hackish. (You could set self.required before calling this and reset it afterwards, but I think this is worse.)

I tried to update the tests as well, but am not sure if they work (No clue how to call them).

Attachment follows.

Alternative implementation, without the need to have some hidden field containing the current value

Uploaded the wrong patch, sorry...this is the right one

The 4722-newforms-file-imagefield.2.diff patch doesn't work for me.

I get a "TypeError?: <lambda>() takes exactly 3 arguments (4 given)" at /django/db/models/fields/init.py in formfield_save_file, line 668. I tried both, patching the latest SVN and patching revision 4722.

Do you use any other patches? Does 4700-newforms-file-imagefield.2.diff work? I don't see a problem, as the code works here (tested with 4722 and HEAD) and looks currect (see below).

save_$FIELDNAME_file() is defined in contribute_to_class(), line 638 using:

lambda instance, filename, raw_contents, save=True: instance._save_FIELD_file(self, filename, raw_contents, save)

This takes 3 parameters as the error states (the first is "self", so I didn't count it).

It is called in formfield_save_file() (defined on line 665), using:

func = getattr(new_object, 'save_%s_file' % self.name)
func(new_data[field_name]["filename"], new_data[field_name]["content"], save)

I only see the 3 needed parameters here, so where did the 4th came from?

Could you perhaps look at you code to see if it differs and/or post a stacktrace? As I only removed the "[0]" from this method I'm very confused by this, the previous patch seemed to work fine and I didn't change any API or logic in this part of the code.

Oh my! I had a "from mymodule.django.models import EnhancedImageField? as ImageField?" in my model code. That class was not aware of the optional "save" argument and therefore caused the problem when used together with newforms. Lesson learned: When working with non-api-stable software make sure you read the changelogs and diffs carefully. ;-)

The last patch against 4722 does not work for me. When uploading the form is not validated and complains I'm not providing the field that is hidden (the one that does not end in _file). I'm not sure if I should change something.

The original patch by Øyvind Saltvik 4700 works just fine.

So the 4700 one is good to go?:

4700-newforms-file-imagefield.2.diff (11.6 kB) - added by Øyvind Saltvik <oyvind.saltvik...> on 03/10/07 12:11:36.
    Refactored code to handle fields not handled by the form, and save files after all other fields

The 4700-patch has the problems stated in comment #31. It relies on user-input (the second form-field) to do validation. This is not clean and can lead to validation-problems. As the djangoboook says: "Never — under any circumstances — trust data from the browser." (http://www.djangobook.com/en/beta/chapter20/)

4722 does mostly the same, but custom Forms need to set required to False on (already provided) FileField?'s itself. And of course it skips the additional hidden form-field, so I think the above error might be caused by some mixture of the old forms (or old patch) and the rewritten one. -> There is no check for the hidden field anymore.

Please consider adding 4722, as newforms is a good place to remove such unclean things from the code.

Btw.: I think both patches do not apply against the current trunk. I will provide an updated patch soon.

Patch without additional hidden field for current trunk (rev 4921)

Patch for current trunk, changes: clean_data -> cleaned_data and new formfield-method for model-fields

New patch, as the old deleted some tests (missed this first, sorry)

Diff against revision 5268.

Can confirm the patch works -- after much headscratching: Don't forget to pass FILES to the 'form-factory':

Here's how I got it to work:

ImageForm = forms.models.form_for_model(Image)
request.POST.update(request.FILES)
f = ImageForm(request.POST)

Works beautifully. Only two issues I see, save() seems to save the file to disk before the database, which leaves filenames without an ID. Second, as somebody already pointed out, manually adding request.FILES to form data seems to be necessary. Newforms should take care of that.

Slight modifications to FileField? and ImageField? implementation; against [5428]

I've just attached a slightly modified version of the FileField/ImageField? patch. This version of the patch:

• Modifies forms to take a "files" argument, for storage of file data. This means you no longer have to copy and update your POST dictionary on submit. You just call MyForm?(request.POST, request.FILES)
• Introduces an "UploadedFile?" temporary object to store the submitted file data.
• Modifies the internals of the form_for_model/instance save method to make field saving more generic, rather than making a special case of FileField?. This should make writing custom fields with unusual save requirements much easier.

This patch now has 2 potential backwards incompatibilities:

1. If you are relying upon auto_id being the 2nd argument on a form (rather than explicitly naming auto_id=False), your rendered forms will start adding label tags.
2. If you have written any custom code using value_from_datadict, you will need to modify those implementations to handle the new files argument.

Russel's patch plus a few lines to integrate with newforms-admin

This patch integrates Russel's patch with the admin of newforms-admin.

Just got a couple of issues with this latest patch.

No need to set widget in line 232, since it is already set by the formfield in the field.

Testing the code gives a error if you have not null constraints on other fields, because save is called when saving the file to disk before those values are set on the model.

request.FILES is passed even if it is not needed, not sure if that affects efficiency in any way.

Patch that fixes the issues i mentioned

Cleaner passing of request.POST and request.FILES in add_view and change_view

A new patch updated to latest newform-admin branch. Not-required ImageField? bug fixed. Patch made with Fidel Ramos

A new patch updated to latest newform-admin branch. Not-required ImageField bug fixed. Patch made with Fidel Ramos

There's a bug in this patch. The clean method of ImageField? is defined as:

class ImageField(FileField):
    def clean(self, data):
        """
        Checks that the file-upload field data contains a valid image (GIF, JPG,
        PNG, possibly others -- whatever the Python Imaging Library supports).
        """
        f = super(ImageField, self).clean(data)
        from PIL import Image
        from cStringIO import StringIO
        try:
            Image.open(StringIO(f.content))
        except IOError: # Python Imaging Library doesn't recognize it as an image
            raise ValidationError(gettext(u"Upload a valid image. The file you uploaded was either not an image or a corrupted image."))
        return f

I have a newforms with a not required ImageField?, but if it's validated without uploading a file it fails with the following exception:

Exception Type:  	AttributeError
Exception Value: 	'NoneType' object has no attribute 'content'
Exception Location: 	/usr/lib/python2.4/site-packages/django/newforms/fields.py in clean, line 396

The fix is checking that we have a file before passing it to PIL. My colleague Manuel Saelices has just uploaded an updated patch where this bug is fixed. Note there are some minor problems when applying the patch to django trunk, because it was made in the newforms-admin branch, to keep compatibility with previous patches.

when will this patch be included into trunk?

cheers, stefan

patch against current trunk

Updated to newforms admin 5741, proposal of new model field kwarg to allow some fields to be saved last. save_last=True

Updated patch to newforms-admin revision 5741.

Added a new kwarg to model fields to allow some fields to be saved last.

Example model

class Model(models.Model):
    file = models.FileField(upload_to='somedir', save_last=True)
    description = models.CharField(maxlength=100)
    category = models.ForeignKey(Category)

Would cause the field file to have access to those other fields set on the model and be able to save the model instance without missing required data if you need a pk/foreignkeys when saving the file .

This is looking good. Ticket #2070 which deals with streaming files just got approved for checkin. I would think that this would need to take some of the changes that ticket makes into consideration before this can get committed.

Save on commit seems like a better idea.

Missing paranthesis in last patch

cleaner code

Validation using ugettext instead of gettext

Save later using non weak signals instead of save_instance modification.

Forgot to call save on the last patch. Calling save caused a eternal loop ofcourse. Don't program past bedtime. Sorry for all the mess.

This one actually works and has comments.

Posting a new patch for specialized saving using a custom function, example follows.

class Model(models.Model):
    file = models.FileField(upload_to='files', save_to_function=lambda field, instance, filename: '%s_%s' % (instance._get_pk_val(), filename))

Save to using a function

Valid example, requires null=True .

class Model(models.Model):
    file = models.FileField(null=True, upload_to='files', save_to_function=lambda field, instance, filename: '%s_%s' % (instance._get_pk_val(), filename))

File and Image fields, patch against trunk [5779]

I'm of the opinion that this ticket should implement filefield and imagefield in newforms... and nothing else. Implementing file save-triggered functions and the like should be a separate ticket as it's new functionality that isn't necessarily required ot resolve this ticket.

The most recent patch by russellm, filefield-5779, applies against trunk 5784 and works if I say 'x = MyForm?(request.POST, request.FILES)'.

(In [5819]) Fixed #3297 -- Implemented FileField? and ImageField? for newforms. Thanks to the many users that contributed to and tested this patch.

(In [5828]) Merged revisions 4350-4357,4359-4365,4371-4372,4374-4377,4380-4386,4388,4390-4391,4400-4402,4404-4408,4410,4412-4419,4426-4427,4430-4432,4434,4441,4443-4444,4446-4447,4450,4452-4453,4455-4458,4476,4503,4546,4564-4569,4580-4586,4617,4630,4641-4643,4653-4655,4657,4669,4673-4675,4694-4696,4713-4714,4720-4723,4725-4732,4735-4741,4750,4755,4758,4769-4770,4776-4777,4783-4795,4798,4805-4808,4810,4813-4815,4817,4824,4836,4838-4843,4851-4855,4869,4872,4882-4884,4906,4916,4935-4936,4940-4944,4946-4953,4962-4963,4969,4971-4973,4990,4994-4997,5000-5003,5006-5008,5013-5014,5019-5024,5026-5036,5046-5047,5054-5059,5062,5079,5081-5083,5090,5100-5101,5114,5122-5123,5126,5128,5134-5136,5148-5149,5151,5157,5174,5178,5183-5185,5192-5195,5197-5200,5203,5205,5208,5214,5223-5224,5226-5227,5229-5230,5235-5236,5238-5244,5246-5249,5251,5254-5262,5266-5280,5282-5284,5286,5301,5307,5309-5310,5312,5314-5317,5319-5332,5334-5345,5372-5378,5381-5382,5384,5386,5388-5390,5393-5397,5399-5400,5416,5419-5430,5440-5441,5444-5448,5461-5464,5467,5473-5481,5487-5489,5491-5492,5498-5499,5507-5510,5512,5527,5529,5531-5535,5540-5541,5546,5570,5572-5574,5576-5578,5580-5581,5583-5589,5591,5595-5597,5601-5608,5613,5626-5826 via svnmerge from http://code.djangoproject.com/svn/django/trunk

........

r5626 | russellm | 2007-07-07 10:16:23 +0800 (Sat, 07 Jul 2007) | 2 lines

Added some uncredited authors that worked on the Oracle branch.

........

r5629 | mtredinnick | 2007-07-08 01:15:54 +0800 (Sun, 08 Jul 2007) | 8 lines

Changed HttpRequest.path to be a Unicode object. It has already been URL-decoded by the time we see it anyway, so keeping it as a UTF-8 bytestring was causing unnecessary problems.

Also added handling for non-ASCII URL fragments in feed creation (the portion that was outside the control of the Feed class was messed up).

........

r5630 | mtredinnick | 2007-07-08 02:24:27 +0800 (Sun, 08 Jul 2007) | 4 lines

Fixed #4772 -- Fixed reverse URL creation to work with non-ASCII arguments. Also included a test for non-ASCII strings in URL patterns, although that already worked correctly.

........

r5631 | mtredinnick | 2007-07-08 02:39:23 +0800 (Sun, 08 Jul 2007) | 3 lines

Corrected misleading comment from [5619]. Not sure what I was smoking at the time.

........

r5632 | mtredinnick | 2007-07-08 08:39:32 +0800 (Sun, 08 Jul 2007) | 5 lines

Fixed reverse URL lookup using functions when the original URL pattern was a string. This is now just as fragile as it was prior to [5609], but works in a few cases that people were relying on, apparently.

........

r5636 | mtredinnick | 2007-07-08 19:22:53 +0800 (Sun, 08 Jul 2007) | 4 lines

Fixed