Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#9418 closed (fixed)

upload_to doesn't work like excepted

Reported by: Bernd Schlapsi Owned by: nobody
Component: File uploads/storage Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

django-9418.diff (1.0 KB ) - added by Bernd Schlapsi 16 years ago.
first patch-trail

Download all attachments as: .zip

Change History (11)

comment:1 by Bernd Schlapsi, 16 years ago

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

comment:2 by Karen Tracey, 16 years ago

Triage Stage: UnreviewedAccepted

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

comment:3 by Brian Rosner, 16 years ago

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.

comment:4 by Brian Rosner, 16 years ago

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

comment:5 by Karen Tracey, 16 years ago

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?

comment:6 by Bernd Schlapsi, 16 years ago

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?

by Bernd Schlapsi, 16 years ago

Attachment: django-9418.diff added

first patch-trail

comment:8 by Bernd Schlapsi, 16 years ago

Has patch: set

comment:9 by Karen Tracey, 16 years ago

Resolution: fixed
Status: newclosed

(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.

in reply to:  8 comment:10 by Karen Tracey, 16 years ago

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.

Note: See TracTickets for help on using tickets.
Back to Top