upload_to doesn't work like excepted

[Bernd Schlapsi]

Hello,

I am using the latest svn-revision 9244 and tried to use a callable in the upload_to parameter of my ImageField.

The explanation in the django-docs for the instance-parameter of acallable says the following:

An instance of the model where the FileField is defined. More specifically, this is the particular instance where the current file is being attached.

If I understand this right, I have access to all the input data of this instance (excepting the id). If I have the following models.py

01: from django.db import models
02:
03: def get_storage_path(instance, filename):
04:    import os.path
05:    print "instance: %s" % (instance)
06:    print "desc: %s" % (instance.desc)
07:    return os.path.join('uploads', filename)
08:
09: class ImageModel(models.Model):
10:    image = models.ImageField('image', upload_to=get_storage_path)
11:    desc = models.CharField(max_length=100)
12:
13:    def __unicode__(self):
14:        return u'%s - %s' % (self.image, self.desc)

the print statement in line 6 should print all the data I inserted into this variable in the admin. Is that right? In my case this is always a empty string:

[20/Oct/2008 21:52:47] "GET /admin/hp/imagemodel/add/ HTTP/1.1" 2002822
[20/Oct/2008 21:52:47] "GET /admin/jsi18n/ HTTP/1.1" 200 1915
instance:  -
desc:
[20/Oct/2008 21:52:54] "POST /admin/hp/imagemodel/add/ HTTP/1.1" 302 0
[20/Oct/2008 21:52:54] "GET /admin/hp/imagemodel/ HTTP/1.1" 200 1866

But this is only true if I change the order of the two fields in my model. It seems that the order in the model is important for this behavior. But in my production case I subclass a model of an reusable app (photologue) and so I have no impact for the right order of my fields!
I tend to think it's a bug and file-type fields should be saved after non-file fields.

[Bernd Schlapsi]

Can anyone reproduce this behavior? Does no one else have this problem?

[Karen Tracey]

Yes, I recreated it. I have not had time to look into fixing it though.

[Brian Rosner]

I am not sure if there is going to be a good fix to this. The reason why descr is empty is because get_storage_path is called before the model form puts descr on the instance. If you move your ImageField last it will work. It surely should at least be examined to see what is possible here. I believe there is a possibility of special casing FieldFields similarly to m2m fields.

[Brian Rosner]

Looking at the code again, I would suspect that self.image in the model won't be set until after the callable is ran.

[Karen Tracey]

I was thinking it might work to check for the field type in the loop through the fields in save_instance, and for file type fields just add them to a list instead of calling save_form_data. Then at the end of the existing loop, loop through the accumulated list of file fields and call save_form_data for them.

If we can't fix it the doc needs to be updated to note that a callable upload_to only has access to field data for fields declared prior to it in the model.

Spot in the docs is pointed to in this thread: ​http://groups.google.com/group/django-users/browse_thread/thread/e8a19e17bcf488fa/d9be40ddcee8beae?

[Bernd Schlapsi]

I hope this "bug"/issue could be fixed.
I know I could change the order of the field. But my example is not my production code. In my production code I am using table inheritance and the image field is in the parent model.

Could you mention the module/file where this code live in, so I could learn a bit more of the django-internals?

[Karen Tracey]

Sorry, the code I was talking about is here:

​http://code.djangoproject.com/browser/django/tags/releases/1.0/django/forms/models.py#L25

[Bernd Schlapsi]

first patch-trail

[Karen Tracey]

(In [9335]) [1.0.X] Fixed #9418 -- When saving a model form, defer saving of file-type fields until after other fields, so that callable upload_to methods can use data from the other fields. Thanks to Bernd Schlapsi for the report and initial patch.

[9334] from trunk.

[Karen Tracey]

Replying to Bernd Schlapsi:
Thanks for the patch. I committed a variant with a variable name that more closely matches the rest of the code style and added a test. Also it's not necessary to test for ImageFields specifically since they're a subclass of FileFields. Fix went into both trunk and 1.0.X brach, I don't know why the trunk commit didn't get recorded in the ticket.