FormSets don't work yet

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Try and use a FormSet via inlineformset_factory (or I assume other 
formset_factory)
2. the formset tries to speak djangodb instead of googledb
3.

What is the expected output? What do you see instead?
FormSets should work with the GAE db models

What version of the product are you using? On what operating system?
0.9.2

Please provide any additional information below.

My first bug report that isn't a trivial typo style fix :-)  This one will 
require the FormSet classes to be ported from django.

Original issue reported on code.google.com by norm...@gmail.com on 14 Dec 2008 at 3:48

[GoogleCodeExporter]

And I hoped that nobody would notice this one. :P

I don't yet know when I can fix this. Would you like to have a try? The patched
Django repo is here:
http://www.bitbucket.org/wkornewald/django-app-engine/

The intro on how to contribute is here:
http://code.google.com/p/app-engine-patch/wiki/Contributing

Original comment by wkornew...@gmail.com on 14 Dec 2008 at 4:57

• Changed state: Accepted

[GoogleCodeExporter]

I'll see what I can do.  I have loading in a working state already :-)

Original comment by norm...@gmail.com on 14 Dec 2008 at 6:01

[GoogleCodeExporter]

Here's my port of FormSet functionality to GAE db.

There are two additional bonus items you can keep as part of ragendja or throw 
away 
if you don't like them:
 - SelfReferenceModelChoiceField: gives you a ModelChoiceField (aka Drop Down) that 
displays the data as a 'treeview'.
 - FormWithSets: gives you the ability to use generic.create_update.update_object to 
display a master form and multiple subFormSets that all link to the primary 
object.  
Very useful for m2m resolving tables!

Original comment by norm...@gmail.com on 14 Dec 2008 at 9:02

Attachments:

• forms.py

[GoogleCodeExporter]

Thanks for the patch! I've committed an initial untested version. I simplified 
your
code a little bit in a few places because we support some of the Model._meta
variables. I also ported the rest of the functions used by ModelForm, so we 
have a
native version, but it looks like the fields aren't ordered correctly, yet. 
I'll have
a look at this at some later point, but could you please test whether FormSets 
work
with the sample app:

http://www.bitbucket.org/wkornewald/appenginepatch-sample/

Thanks!

Original comment by wkornew...@gmail.com on 15 Dec 2008 at 2:07

[GoogleCodeExporter]

Fixed the field order bug, so could you please test the FormSet based on the new
source in either the sample or the django-app-engine repo (both should work)? 
Thanks!

Original comment by wkornew...@gmail.com on 15 Dec 2008 at 4:05

[GoogleCodeExporter]

I don't yet understand exactly what your two bonus items do. Do you have 
screenshots,
so I can see it in action? Also, could you please document those features, so I 
can
integrate them in ragendja? Thanks! Sorry for all those small posts. Busy day.

Original comment by wkornew...@gmail.com on 15 Dec 2008 at 7:01

[GoogleCodeExporter]

Here's a patch to make the changes complete.

Original comment by norm...@gmail.com on 15 Dec 2008 at 7:17

Attachments:

• patch

[GoogleCodeExporter]

SelfReferenceModelChoiceField is a ModelChoiceField (i.e. renders to html as a 
select 
with options) that expects the model used to have a SelfReference field.

It treats that field as a Parent reference, and displays the data passed as a 
tree.
i.e. (.'s actually render as   just worried about spacing in this text)
Europe
..United Kingdom
....England
....Scotland
....Wales
..France
..Germany
America
..New York
....New York

FormWithSets can be used to show a single form and multiple formsets as a 
single form 
(this makes it possible to use with the generic update_object, because it 
doesn't 
take any formsets as parameters).  It's basically an object that acts like a 
form (to 
make the generic view happy), but can also show all the formsets you want.

Original comment by norm...@gmail.com on 15 Dec 2008 at 7:23

[GoogleCodeExporter]

btw, I use the recursetag [1] to render the SelfReference data in my view 
template.

[1] http://www.djangosnippets.org/snippets/592/

Original comment by norm...@gmail.com on 15 Dec 2008 at 7:26

[GoogleCodeExporter]

Are you sure that this is correct:

-    data = {'key_name': instance.key().name()}
+    data = {'key_name': instance.key()}

Since the attribute is called 'key_name' I'd say it should be the key().name()
instead of just key().

Original comment by wkornew...@gmail.com on 15 Dec 2008 at 7:30

[GoogleCodeExporter]

key().name() was returning a blank string, so save was failing.  I want the 
str(of 
key) at this point.  i.e. the internal value that can be passed to the model 
constructor

Original comment by norm...@gmail.com on 15 Dec 2008 at 7:33

[GoogleCodeExporter]

But str(key) shouldn't be passed as the key_name to the constructor. That 
wouldn't be
correct. I guess we'll have to construct models with ragendja.dbutils.db_create 
or
something like that, so they always have a key_name.

Original comment by wkornew...@gmail.com on 15 Dec 2008 at 7:42

[GoogleCodeExporter]

nono, it works(tm).  I meant to reconstruct the model, you pass model(dbkey) 
(where 
dbkey is the long horrible base32 string.

I'm busy putting together a patch to the sample app.

Original comment by norm...@gmail.com on 15 Dec 2008 at 7:45

[GoogleCodeExporter]

Here's a patch demo'ing the models.  I don't like the calling convention of the 
FormWithSets - you can't label the subforms, etc yet.  It may well end up being 
a 
list of dictionaries (or *args), so that each form gets it's own 
label/prefix/sub-
settings, etc.

Original comment by norm...@gmail.com on 15 Dec 2008 at 8:30

Attachments:

• patch

[GoogleCodeExporter]

woooah, my brain just did a back-flip.  The other option is to create a 
ModelField 
that will render a formset, hmmmm

Original comment by norm...@gmail.com on 15 Dec 2008 at 8:33

[GoogleCodeExporter]

As it turns out, it's practically impossible to create a field that knows about 
it's 
model's instance.  So I ended up cleaning up FormWithSets so that it renders 
almost 
the same as normal forms.  Attached is a patch that I'm happy with.  I _think_ 
most 
of the places attrs can go, would.

Original comment by norm...@gmail.com on 15 Dec 2008 at 11:35

Attachments:

• patch

[GoogleCodeExporter]

fix for label_prefix

Original comment by norm...@gmail.com on 15 Dec 2008 at 11:43

Attachments:

• patch

[GoogleCodeExporter]

and now saving works too :-)

Original comment by norm...@gmail.com on 15 Dec 2008 at 11:58

Attachments:

• patch

[GoogleCodeExporter]

OK, I've changed app-engine-patch to simulate the key instead of the key_name 
as the
pk and now it's practically something like:
data = {'key': instance.key()}
This allows for distinguishing models and it always generates correct code.

I applied your patch locally and it works. At least now I understand what
FormWithSets does and I think it's really useful.

Instead of passing the formsets via a dict (**formsets) it would be better to 
pass a
tuple of (name,formset) pairs, so the order will always be correct.

Is pretty_name really needed? Form names are often too ugly, anyway, and they 
need a
translation in many cases. Also, what if someone doesn't want a formset caption?

There are lots of HTML validation errors. For example, it generates code like 
this:

<p><label for="id_Pets">Pets:</label> <table name="Pets" id="id_Pets"><input
type="hidden" name="Pets-TOTAL_FORMS" ...

Labels with a "for" attribute should have an input field. Instead of a label I'd
suggest that you use something like <fieldset><caption>Pets</caption>...

Tables don't support a "name" attribute.

Tables mustn't be placed within a <p>. That one probably comes from the 
form.as_p
template code. Is it possible to somehow not generate the <p> tag around the 
table?

Please place the two hidden input fields in that table inside tr->td elements 
or move
them out of the table.

Since the template uses form.as_p I think the pets form should use as_p(), too.

Please download an HTML validator plugin for Firefox and fix all bugs. After 
that I
can incorporate your patch. BTW, you have a Contract model, but it's not used 
anywhere.

Original comment by wkornew...@gmail.com on 16 Dec 2008 at 9:52

[GoogleCodeExporter]

BTW, just wondering. Isn't that feature already implemented in the admin 
interface? I
mean, there I can edit model instances and their related items directly, too. Of
course, the admin interface hasn't been ported, yet, but maybe those parts 
still work
or maybe it would be easier to port the functionality in the admin interface and
reuse that?

Original comment by wkornew...@gmail.com on 16 Dec 2008 at 2:41

[GoogleCodeExporter]

Okay, I've updated the patch to address the issues:

 - includes a fix for model.__repr__

 - changed to pass via tuples, also supported via Forms really easily, eg:

class PersonForm(forms.ModelForm):
    Pets = FormSetField(Pet)
    Employers = FormSetField(Contract, fk_name='employee')
    Employees = FormSetField(Contract, fk_name='employer')
    class Meta:
        model = Person
EventForm=FormWithSets(EventForm)

   the constructor on FormSetField can be passed args for 
inlineformset_factory(model=Meta.model) and Field, it's intelligent enough to 
figure 
out what goes where.

 - pretty_name is there because it's the same way that the django's BoundField does 
labels.  So I'm trying to make it work in the least surprising way.

 - HTML validation errors are now all fixed in all rendering modes: table, ul, p

 - Currently django's formset only supports as_table.  It also has a XXX 'todo'.  I 
have implemented a 'pivot' that will alter the table so that it only has a 
single set 
of headers, and one row per item.

 - Contract model is for testing multiple references to the same model with fk_name.  
It's now in full use.

 - Yes this is already in the admin interface.  Partly I want to be able to use the 
functionality outside of admin, and secondly because admin doesn't "just work" 
at the 
moment.  Django formsets seem to be really immature, because of the lack of 
generic 
support for them.  FormWithSets is a way to try and add that.

Original comment by norm...@gmail.com on 16 Dec 2008 at 9:53

Attachments:

• patch

[GoogleCodeExporter]

Great! Thanks a lot! Would you like to add the documentation to our wiki (you 
know
the code much better than me)? Which email should I add to the project? You can 
send
it to me privately. I've also given you write access to all repos. Please make 
sure
that changes to the appenginepatch folder are done in the appenginepatch repo. 
The
appenginepatch folder in the sample is just a manual copy of the real 
appenginepatch
repository (with .hg* removed) and I'd mistakenly overwrite all changes on the 
next
update.

Original comment by wkornew...@gmail.com on 16 Dec 2008 at 11:32

[GoogleCodeExporter]

Original comment by wkornew...@gmail.com on 16 Dec 2008 at 11:32

• Changed state: Fixed

[GoogleCodeExporter]

Oh, I noticed a bug. The order of the forms in the rendered HTML (employees, 
pets,
employers) is not the same as the one you set in the view (pets, employers,
employees). Could you please have a look?

Original comment by wkornew...@gmail.com on 17 Dec 2008 at 8:06