Form->loadDataFrom() and FormField->setValue()

553 views
Skip to first unread message

Ingo Schommer

unread,
Oct 8, 2008, 7:42:53 PM10/8/08
to silverst...@googlegroups.com
I'm just trying to load a ComponentSet from a relationship into an
empty CheckboxSetField.
This fails because if you tell Form->loadDataFrom() not to load "blank
values", which is the default setting and widely used.
The method determines the field as "blank" because __get() doesn't
return anything.

Unit test currently failing (not on trunk yet):
http://open.silverstripe.com/changeset/63917

Each FormField->setValue() call has two parameters:
- $value: usually a string suitable for value display and saving
- $obj: The full object or array from the loadDataFrom() call
CheckboxSetField->setValue() takes its value from $obj rather than
$value, but never gets called in the first place because of the faulty
"blank" check.

A couple of solution approaches:
- Let each field handle its own definition of a blank value, and pass
the $loadBlanks preference into FormField->setValue() - seems like a
lot of adjustment and error prone in subclasses
- Let FormFields which use the second $obj parameter for setValue()
implement an interface, perhaps ComplexValueField?
- Use __call() in addition to __get() to determine if a field is
considered "blank", but don't set complex values like ComponentSets
through $value parameter

Any thoughts?

-------
Ingo Schommer | Senior Developer
SilverStripe
http://silverstripe.com

Phone: +64 4 978 7330 ext 42
Skype: chillu23

Sam Minnee

unread,
Oct 8, 2008, 9:48:59 PM10/8/08
to SilverStripe Development
The idea of loadBlanks = false is "Don't try and get data out of the
object that isn't actually contained in that object." So, if the
Member object doesn't have a ZipCode data field, then you should leave
the ZipCode TextField alone.

Since relation methods are a common way of providing a piece of data
on an object, it makes sense that Form::loadDataFrom() will look first
in the fields and then in the methods. However, we also need to
tighten up what "blanks" actually mean.

To this end, $loadBlanks should probably be renamed to
$clearMissingFields which more accurately describes what it's trying
to do.

In terms of implementation:
* For arrays, we should use array_key_exists() instead of isset() to
determine whether to set the field.
* For objects, we should call: isset($obj->$fieldName) || ($obj-
>hasMethod('hasField') && $obj->hasField($fieldName))


Reply all
Reply to author
Forward
0 new messages