[Zope3-dev] Re: [Zope3-checkins] CVS: Zope3/src/zope/schema - _bootstrapfields.py:1.19.2.1

[Fred L. Drake, Jr.]

This is a long message.  Sorry.  But I think it's worth reading for
those playing with the nastiness that is the current forms machinery.

In a checkin message, Garrett Smith writes:
 > - Changed haveData to hasData.
 > 
 > - Propogated the use of Field.missing_value as an alternative to
 >   explicit checks for None and '' (empty string).

Excellent.

 > - Modified the update logic for objects to skip fields that are not
 >   present in the request form. Prior to this change, unspecified values
 >   (i.e. values not in the request form) would cause default values to be
 >   set for corresponding object fields.

Hmm.  I haven't read the code changes yet.  How are you determining
what was sent to the browser?  Are you assuming that self-posting
forms always have the same set of widgets?

This assumption is made many places, and might be tolerable, but I'm
not convinced.  Much of the form management support feels very weak.
It's true for the standard add/edit/schemadisplay forms, but may not
be true for others; fields could easily be displayed conditionally,
and so appear and disappear at "partial submit" events based on other
state in the form.

 > - Exposed missing_value in initializer for Field - developers can now
 >   specify a missing value for a schema field.

Excellent.

 > - Changed the default implementation of field validation. Prior to this
 >   change, validation failed if a required value was missing. Now validation 
 >   is limited to validating non-missing values and the check for required
 >   values is performed elsewhere.

Probably ok.  What's the motivation?

 > - Text fields have a missing_value of '' (empty string) instead of None.

Argh.  I know there is a popular position that this is the right thing
to do, but it doesn't feel right to me.  Using None as the default
missing_value makes it easier to detect careless bugs.

 > - Widget related error classes have been revamped:
 > 
 >   - WidgetInputError, the base class for all widget related errors, now
 >     provides two attributes: widget, the widget associated with the
 >     error, and error_msg, a string describing the error,
 > 
 >   - WidgetInputError implements 'args' so it can be rendered like other
 >     errors.
 > 
 >   - MissingInputError uses a standard error message.
 > 
 >   - All uses of WidgetInputError and its subclasses have been updated
 >     to use the new class.

Yay!

 > - Deleted class zope.app.browser.form.widget.PossiblyEmptyMeansMissing.
 >   This capability is now handled by setting a field's missing_value to '' 
 >   (empty string).

Excellent.

 > - Renamed the 'force' argument used in various setUpWidget methods in 
 >   utility.py to 'ignore_unspecified'. This argument is set to True to
 >   ensure that fields that are not in a form are not used to update their
 >   corresponding widget. This argument should be true during object
 >   updates (see editview.py) to ensure that unspecified fields are not
 >   updated.

Definately better than "force"!

 > - Removed the 'strict' argument from applyWidgetChanges since no one was
 >   using it and there's no clear application for it.

Yeehaa!  Isn't YAGNI wonderful?  ;-)


Now for the nitty-gritty:

 > - Changed the default implementation of hasData (was haveData) to return
 >   True only if the one of the following was true:
 > 
 >   - setData had been called for the widget. Per IWidget, values passed to
 >     setData override all other data sources for a widget.
 > 
 >   - There is a value in the request form that corresponds to the widget
 >     field.
 > 
 >   Prior to this change, hasData would return False if the widget's data
 >   equaled the field's missing value. E.g., a text widget with an empty
 >   string would return False, indicating that it has no data, even though
 >   the empty string, a legitmate value, was submitted in the request.
 > 
 >   This change required a handful of other changes to reflect the new
 >   logic.
 > 
 >   The bulk of this change effected widget.py. To see how the new logic
 >   implementation of hasData effected widgets, see test_browserwidget.py
 >   and test_utility.py.
...
 > - Changed widget's _showData:
 > 
 >   - Renamed to getUnconvertedData to clarify the method meaning and signal
 >     its use as a public method.
 > 
 >   - Positioned the method as a complement to getData, which returns data
 >     in its converted form. getUnconvertedData returns data in its
 >     unconvereted form using _unconvert.

Some of us here have beaten the IBrowserWidget API around a bit, and
are unhappy with the current interface.  I did manage to write an
implementation of IBrowserWidget interface that mapped to something
the unhappy among us felt was better, but not everyone agreed that
there was a problem in the existing interface.  (There was some
flexibility for a small incremental improvement, but we didn't feel it
was enough to actually accomplish what we wanted.)

The biggest problem is that getData(), haveData(), and setData() sound
like they're closely related, and they simply aren't.  Here's how we
ended up describing them:

getData()
    Return the converted value that's the result of the request.  Not
    that setData() only affects this is haveData() is false; once the
    field is part of the request, this should always come from the
    form.

haveData() (hasData() in your branch)
    Whether or not the field was present in the request that caused
    the form to be generated.  For complex widgets, it's almost
    impossible to get away without a marker to say "I'm here",
    especially if a valid state is to have no other input from the
    browser (since empty string fields can be dropped as a misguided
    "optimization" by the browser).

setData()
    Set the initial value of the widget for it's initial display.
    When a form is re-rendered due to a "partial submit", this value
    is ignored, and the value from the request is used instead.  This
    requires *really* knowing what the value was on the browser when
    the submit button was pressed---even if it was an empty string.

You're changing the interpretation of haveData() in a fundamental
way.  It may work for most of the standard widgets, but I'm very wary
of this change.  (But like I said, I've not read the code yet.)

Now, I'm not ready to take up a jihad to fix what I think is wrong
with the form machinery.  But some clarity and improved documentation
seems essential if we going to make this stuff easier to work with.
The difficulty of understanding what's going on is an enormous entry
barrier even for some very smart people.  Changing the
get/have/setData() behavior needs very careful review.  The effects
to code will spread well beyond the Zope 3 core, and not just in Zope
Corporation projects.


  -Fred

-- 
Fred L. Drake, Jr.  <fred at zope.com>
PythonLabs at Zope Corporation