Code

Opened 6 years ago

Closed 6 years ago

Last modified 6 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: UI/UX:

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 6 years ago.
first patch-trail

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by Bernd Schlapsi

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 6 years ago by kmtracey

  • Triage Stage changed from Unreviewed to Accepted

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

comment:3 Changed 6 years ago by brosner

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 Changed 6 years ago by brosner

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 Changed 6 years ago by kmtracey

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 Changed 6 years ago by 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?

Changed 6 years ago by Bernd Schlapsi

first patch-trail

comment:8 follow-up: Changed 6 years ago by Bernd Schlapsi

  • Has patch set

comment:9 Changed 6 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:10 in reply to: ↑ 8 Changed 6 years ago by kmtracey

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.