Django

Code

Ticket #9418 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

upload_to doesn't work like excepted

Reported by: Bernd Schlapsi Assigned to: nobody
Milestone: Component: File uploads/storage
Version: 1.0 Keywords:
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

django-9418.diff (1.0 kB) - added by Bernd Schlapsi on 11/02/08 14:57:44.
first patch-trail

Change History

10/31/08 04:18:46 changed by Bernd Schlapsi

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

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

10/31/08 07:33:24 changed by kmtracey

  • stage changed from Unreviewed to Accepted.

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

10/31/08 07:39:48 changed 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.

10/31/08 07:41:01 changed 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.

10/31/08 07:53:32 changed 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?

10/31/08 11:28:51 changed 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?

10/31/08 11:33:26 changed by kmtracey

Sorry, the code I was talking about is here:

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

11/02/08 14:57:44 changed by Bernd Schlapsi

  • attachment django-9418.diff added.

first patch-trail

(follow-up: ↓ 10 ) 11/02/08 14:58:09 changed by Bernd Schlapsi

  • has_patch set to 1.

11/04/08 13:54:29 changed by kmtracey

  • status changed from new to closed.
  • resolution set to fixed.

(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 ) 11/04/08 15:58:10 changed 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/Change #9418 (upload_to doesn't work like excepted)




Change Properties
Action