[patch] Manipulators ignore default values, fields can't be omitted from forms

[Antti Kaihola]

My model has several fields with default values. I'd like to be able to omit those fields from a form and have defaults used when creating a new object.

See ​http://groups.google.com/group/django-users/browse_frm/thread/5e134e5aa43bafe5/d50c9ebb0935cb20 and ​http://simon.bofh.ms/logger/django/2006/1/12 for discussion.

[brenocon@…]

The specific user-level error seems to happen here:

m = Item.AddManipulator()
m.do_html2python(new_data)

that step adds in ALL of the model's fields to new_data, even if they weren't there before. If a field wasn't there before, it doesn't fill in the default value, instead [None] (new_data is a MultiValueDict.)

There seem to be 2 places in the manipulator logic that try to use the default value, but since new_data has already been clobbered with None values for all fields, neither place seems to get called in this situation. (1) at AutomaticManipulator.save() there is logic to use get_default() in some cases; and also (2) save() sometimes calls Field.get_manipulator_new_data(), and the base class form of that method also has logic to use get_default().

(1): django/db/models/manipulators.py:84 or so
(2): django/db/models/fields/init__.py:274
(line numbers as of rev [2893])

It seems to me that the problem is do_html2python is being destructive. It should be AutomaticManipulator.save()'s responsibility to add in missing values; Manipulator.do_html2python should only convert things already inside new_data. Since AM.save() already has logic for get_default()'ing, I presume this is the logic the authors had in mind.

Since Manipulator.do_html2python() seems to be fighting AM.save() I'm marking this as a bug... maybe respecting-defaults functionality used to exist before?

This all might be less confusing with a callchain illustration. The user programmer does 2 calls which in turn call other things...

(1)
Manipulator.do_2html -> FormField.convert_post_data(new_data)

then you next call:
(2)
AM.save() -> ?Field.get_default(); and also
AM.save() -> Field.get_manipulator_new_data(new_data) -> Field.get_defaiult()
(I think these are model Fields, not FormFields, but i'm not sure)

There's something I don't understand going on in FormField.convert_post_data(). It is a second place that's checking for whether field names exist in new_data. (AM.save() being the other place that I know of.) I couldn't make heads or tails of convert_post_data() or the assumptions it was making of other objects it was using; therefore I just put the fix into do_html2python().

My patch is attached. It ensures that fields' default values get assigned when saving via the AddManipulator. I'd write test code but I don't know how with form data and all else... if I get more time to work on this (spent forever already!) I'll try to do so.

I'm wondering if this bug has something to do with #1584, since the logic dealing with that is hidden up in the AM.save() code that was not being called correctly. (Though I haven't checked thoroughly enough to know.)

Finally.. it seems note the reporter of #908 had a similar problem to this bug.

[brenocon@…]

patch so new_data doesnt get bogus [None] value filled in by do_html2python

[brenocon@…]

crap some of my formatting got screwed up. the user-level code that seems erroneous to me is:

m = Item.AddManipulator()
m.do_html2python(new_data)

The call chains are:

(1) Manipulator.do_2html -> FormField.convert_post_data(new_data)

(2) AM.save()

• - - -> Field.get_default(); and also
  
• - - -> Field.get_manipulator_new_data(new_data) -> Field.get_default()

(i'm not actually sure these are Field's, I'm guessing)

[brenocon@…]

I take back what I said near the bottom of my big post, my patch doesn't seem to affect #1584 at all.

[James Bennett <ubernostrum@…>]

This feels like we need to decide, and document more clearly up-front, what default really means, because it ends up being confusing -- there's the way it actually works, and there's the way that a lot of people seem to assume it works, and they're two different things. Both make sense, but we need to settle on which one is best and document it aggressively.

For the record, here are the two different interpretations, which differ on when exactly the value should be filled in:

1. The way it actually works is that the default value is pre-filled in the FormWrapper instance, so that when the form is displayed to a user, the value will be in there -- the user can then leave it as-is, or change it if desired.
2. The way many people seem to assume it works is that the value will be filled after the fact if the form comes in with no value for the field in question.

The disadvantage of (1) is that, in order to take advantage of the default value, you must actually include the field in what the end user sees; otherwise, the field comes back with no value and the only reasonable thing for Django to assume is that the user manually changed the value from its default to blank; often this will end up tripping a validation error, because the undisplayed field is usually going to be required.

The disadvantage of (2) is that it we could end up having to do lots of second-guessing about input values -- did the user really mean to submit an empty string (which is not inconceivable), in which case it should be left alone, or did the user leave the field alone (or was the field left out of the end user's display), in which case it should be replaced with the default. That seems like it could be quite a can of worms.

[anonymous]

i dont see a problem with submiting empty strings

if the string is empty it gets a variable in the post request,
if the field is missing it gets no variable in the request

[jeff@…]

I definitely would go for #2. In my mind, it makes much more sense.

[Chris Beaven]

Manipulators as they are now, are on the way out. This doesn't seem worth fixing before they go.