Discussion on #5721

0 views
Skip to first unread message

Brian Rosner

unread,
Oct 10, 2007, 8:25:38 PM10/10/07
to django-d...@googlegroups.com
I wanted to raise some discussion about the problem descibed in [1]. I
also came across this and did something that made it work. However it
wasn't exactly a good fix. I looked into the issue in depth and
discovered some odd things.

inline_formset uses formset_for_model which ends up using
form_for_model. This works well for forms that are not bound to an
instance. To make forms with instances work in a formset there is a
function called initial_data which returns a dictionary mapping field
names to values of the given instance. This is called in the __init__
method of BaseModelFormSet. Then when the formset builds the change
forms it uses the form class built with form_for_model.

The problem here is that django.db.models.fields.FileField's formfield
method that is called through the formfield_callback is never able to
make the field not required when there is initial data (which would
have come from form_for_instance). It looks like as it stands that
formfield_callback functions will never be able to check if there is
initial data in the form in change forms that are bound to an object.

I wanted to fix this, but it seems I am hitting a wall. As I was
trying to fix this I began to see some functionality overlapping with
how generic foreign keys might want to integrate.

Basically, I was attempting to write a new function,
formset_for_instance that would be given an instance and I just hard
coded in the fetching of data which only works in my particular
environment. It would also use form_for_instance to create forms for
just the change_forms. Then I had to overload the _get_change_forms to
make it all work, sorta.

I hope someone understands what I am getting at. You can pretty
disregard my implementation, but hopefully I have explained the problem
well enough to warrent some ideas on how I might go about implementing
something.

[1]: http://code.djangoproject.com/ticket/5721

--
Brian Rosner


Joseph Kocherhans

unread,
Oct 10, 2007, 9:38:46 PM10/10/07
to django-d...@googlegroups.com
On 10/10/07, Brian Rosner <bro...@gmail.com> wrote:
>
> I hope someone understands what I am getting at.

I do. You've pretty much discovered one of the reasons why I didn't
try to write formset_for_instances or something similar in the first
place. I just spent about a half hour thinking about it again and came
to the same conclusions. If you want to come up with a patch, I'll
definitely take a look and help try to iron it out, but as you've
already discovered, it's tricky. FWIW I'm working on a replacement for
form_for_instance that uses initial_data, but it'll probably be awhile
before I finish it up.

The oldforms file widgets have 2 separate html inputs, a hidden one
for the current file value, and second for the actual file input. I
think getting the file widget/field to render and check for such a
hidden field might be a cleaner and easier approach. The initial_data
function may need to be tweaked slightly to get this to work. I don't
remember what it does with file fields.

For instance, html for an oldforms file field looks like this:

<input type="file" id="id_photo_file" class="vImageUploadField"
name="photo_file" />
<input type="hidden" id="id_photo" name="photo"
value="img/photos/2007/10/10/blah.jpg" />

Joseph

Brian Rosner

unread,
Oct 11, 2007, 1:57:22 AM10/11/07
to Django developers

On Oct 10, 7:38 pm, "Joseph Kocherhans" <jkocherh...@gmail.com> wrote:


> On 10/10/07, Brian Rosner <bros...@gmail.com> wrote:
>
>
>
> > I hope someone understands what I am getting at.
>
> I do. You've pretty much discovered one of the reasons why I didn't
> try to write formset_for_instances or something similar in the first
> place. I just spent about a half hour thinking about it again and came
> to the same conclusions. If you want to come up with a patch, I'll
> definitely take a look and help try to iron it out, but as you've
> already discovered, it's tricky. FWIW I'm working on a replacement for
> form_for_instance that uses initial_data, but it'll probably be awhile
> before I finish it up.

Take a look at http://code.djangoproject.com/ticket/5733. It is a
start. Perhaps you could expand it out? It doesn't seem to be 100%
complete either.

>
> The oldforms file widgets have 2 separate html inputs, a hidden one
> for the current file value, and second for the actual file input. I
> think getting the file widget/field to render and check for such a
> hidden field might be a cleaner and easier approach. The initial_data
> function may need to be tweaked slightly to get this to work. I don't
> remember what it does with file fields.

Yeah that is a much better way of doing it. It seems that the
formfield methods in the database Field's classes shouldn't be messing
with initial.

Reply all
Reply to author
Forward
0 new messages