Home » Database » DB_DataObject_FormBuilder » Bug #8484

[2006-08-16 18:46 UTC] dtlmhn at dtl dot net

Description:
------------
The method, DB_DataObject_FormBuilder::_generateForm()
contains numerous instances of the following code:

$formValues[$key] = $this->_do->$key;

I haven't followed it all the way through, but I take it
that this is the fundamental linkage between the
DB_DataObject subclass's values and the HTML_QuickForm
form field values. However, this method of accessing the
DO's values completely bypasses one of the primary
features of DB_DataObject: the ability to wrap field
values in accessors. That is, the above code should be
something like:

if (method_exists($this->_do, 'get'.$key))
$formValues[$key] = $this->_do->{'get'.$key}();
else
$formValues[$key] = $this->_do->$key;

or actually just

$formValues[$key] = $this->_do->{'get'.$key}();

should work since DB_DO makes use of the PHP 'overloading'
mechanism.

This apparent oversight has grave issues for
behind-the-scenes processing of form data. For example,
suppose $k == 'phone' and in the database the phone number
is just stored as a string of digits (which is, after all,
what a phone number is -- the rest is presentation). Then
getphone() might do something like:

return '('.substr($this->phone, 0, 3).') '
.substr($this->phone, 3, 3).' '
.substr($this->phone, 6);

As is, FormBuilder misses this and just presents the user
with a form field populated with an unformatted string of
digits.

The solution is to simply store the user-entered (or
possibly pre-processed via setphone() method) field in the
database, but this means FormBuilder essentially forces
linkage between the model and the presentation.

Oddly, this error is not made in the FB processForm()
method, which does the following when setting the DO's
fields from the form:

// See if a setter method exists in the DataObject - if
so, use that one
if (method_exists($this->_do, 'set' . $field)) {
$this->_do->{'set'.$field}($value);
} else {
// Otherwise, just set the property 'normally'...
$this->_do->$field = $value;
}

Should it not work both ways?

I considered reporting this as a feature request, however,
this is really behavior I consider to be incorrect, and
therefore chose to report it as a bug instead.

[2006-08-16 18:56 UTC] dtlmhn at dtl dot net

Addendum: Scenario and expected behavior vs. actual
behavior:

Setup a complete DB_DataObject_FormBuilder page attached
to a database with a table that contains a field, 'phone'.

Update the table's DB_DataObject subclass so it contains
the following two methods:

function setphone($phone)
{
trigger_error('setphone');
}

function getphone()
{
trigger_error('getphone');
}

Now, what is expected is that upon loading the page the
method getphone() will trigger an error, and upon
processing the submitted form (via FB's processForm
method) the method setphone() will trigger an error.

However, what actually happens is that setphone() triggers
an error, but getphone() does not because it is never
called.