Coping with nulls v. empty strings

[James Masters]

This struck me right at the beginning but have been waiting until I
could actually test it. If I populate a form with fields from a db
record and one of those fields is null, then the form naturally
displays this as the empty string. So that when I update that record,
the field is changed from null to empty string.

The requirement here is probably different depending upon application
and specific field but in general for me, I think that if a field can
be null and the form is blank for that field, then it should set it to
null.

Is there anything that handles this or some parameter than can be set
to make a field and/or a form behave in a set way for blank fields as
required?

thanks,

James.

[Zbigniew Lukasiak]

This is an interesting problem - but I think that the correct solution
should let the user to set the field to "" if he chooses to.
Conflating "" to NULL is as wrong as conflating NULL to "". At the
server side we can have a convention that undef means NULL - so the
question is what the browsers send.

--
Zbigniew Lukasiak
http://brudnopis.blogspot.com/

[John Siracusa]

On Fri, May 9, 2008 at 11:22 AM, James Masters <ja...@mastersgames.com> wrote:
> This struck me right at the beginning but have been waiting until I
> could actually test it. If I populate a form with fields from a db
> record and one of those fields is null, then the form naturally
> displays this as the empty string. So that when I update that record,
> the field is changed from null to empty string.

This all depends on what you pass to params(). Example:

% perl -MRose::HTML::Form -de 0

main::(-e:1): 0
DB<1> $f = Rose::HTML::Form->new;

DB<2> $f->add_fields(t => { type => 'text' });

DB<3> $f->params({ t => undef });

DB<4> $f->init_fields;

DB<5> x $f->field_value('t');
0 undef
DB<6> $f->params({ t => '' });

DB<7> $f->init_fields;

DB<8> x $f->field_value('t');
0 ''
DB<9> $f->params({});

DB<10> $f->init_fields;

DB<11> x $f->field_value('t');
0 undef

So if you really want '' to be coerced to NULL, then change all ''
param values to undef before passing the params hash to the form's
params() method.

-John

[James Masters]

> So if you really want '' to be coerced to NULL, then change all ''
> param values to undef before passing the params hash to the form's
> params() method.

Thanks, I see, yes I will have a play with params; that might be the
simplest solution for me although I don't think it's the best solution
in general for RHTMLO.

I am still slightly concerned from the generic point of view, that a
user of RHTMLO doesn't inadvertantly do something bad. It seems to me
to be good practice that a field that isn't amended, should not be
changed. And more concerning, sometimes databases have fields where
NULL definitively means something different to '' in which case the
user certainly wouldn't want NULL silently changed to ''.

Therefore, I am wondering if it might be best if RHTMLO by default
does not update a field where the form field is '' but the existing db
record has it set to NULL. I don't think params or field attributes
will help with this. Could object_from_form() be modified to behave
like this by default or to behave like this if instructed? I've had a
look at it and it appears that if the following line:

$object->$name($field->internal_value);

is changed to something like:

unless (!defined($object->$name) and defined($field->internal_value)
and !$field->internal_value) { #unless dbval NULL and formval ''
$object->$name($field->internal_value);
}

this would do it.

James.

[James Masters]

In case it's useful for anyone else, I have successfully implemented
the "don't set db field value to blank if db field is presently NULL"
policy by inserting the following before the object_from_form call.

foreach my $field ($form->fields) {
my $name = $field->local_name;
if($existingdbobject->can($name)) {
if (!defined($existingdbobject->$name) and defined($field-
>internal_value) and !$field->internal_value) {
$field->input_value(undef);
}
}
}

[John Siracusa]

On Fri, May 9, 2008 at 2:49 PM, James Masters <ja...@mastersgames.com> wrote:
> Therefore, I am wondering if it might be best if RHTMLO by default
> does not update a field where the form field is '' but the existing db
> record has it set to NULL.

The method in Rose::HTML::Form is called object_from_form(), and as
such it is generic to the point of being unrelated to databases
entirely. So it's certainly not going to gain any features based on
what to do in the case when a db record has a NULL :)

What I think you want instead is a db_object_form_form() and
init_with_db_object() pair of methods that understands and handles
Rose::DB::Object-derived objects.

I'm planning to add such methods to RHTMLO eventually. I've been
testing something similar in my own code, which I've posted before
(when the list was on SF.net) and I'll post it again here (see below).
It does some funky stuff with a.b.c nested forms and fields
corresponding to $a->b->c foreign keys and relationships in RDBO
objects, which may or may not be to your liking. But it does have the
'' to undef coercion, plus some custom boolean handling. Feel free to
modify it to suite your needs.

-John

---

package My::HTML::Form;

use strict;

use base 'Rose::HTML::Form';

use Carp;
use Scalar::Util();

use Rose::DB::Object::Util;
use Rose::HTML::Form::Constants qw(FF_SEPARATOR);

use EVA::HTML::Errors qw(FORM_SET_FIELD_ERROR);

# Variables for use in regexes
our $FF_SEPARATOR_RE = quotemeta FF_SEPARATOR;
our $FF_SEPARATOR = FF_SEPARATOR;

sub db_object_from_form
{
my($self) = shift;

my($class, $object);

if(@_ == 1)
{
$class = shift;

if(ref $class)
{
$object = $class;
$class = ref $object;
}
}
elsif(@_)
{
my %args = @_;

$class = $args{'class'};
$object = $args{'object'};
}
else
{
croak "Missing required object class argument";
}

$object ||= $class->new();

my $parent_object = $object;

unless($object->isa('Rose::DB::Object'))
{
croak "$object is not a Rose::DB::Object-derived object";
}

my $meta = $object->meta;

foreach my $field ($self->fields)
{
my $name = $field->name;

next unless($self->param_exists_for_field($name));

$object = $parent_object;

my $partial_name = '';

if($name =~ /$FF_SEPARATOR_RE/o)
{
my $nibble = $name;
my $obj = $object;

while($nibble =~ s/^([^$FF_SEPARATOR]+)(?:$FF_SEPARATOR_RE)//o)
{
my $related = $1;

last unless($obj->can($related));

if(Rose::DB::Object::Util::has_loaded_related($obj, $related))
{
$obj = $obj->$related()
}
else
{
my $new_obj;
eval
{
$new_obj = $obj->$related();

unless($new_obj)
{
if(my $fk = $obj->meta->foreign_key($related))
{
$new_obj = $fk->class->new;
}
elsif(my $rel = $obj->meta->relationship($related))
{
my $class = $rel->can('foreign_class') ?
$rel->foreign_class : $rel->class;

$new_obj = $class->new;
}

$obj->$related($new_obj);
}
};

if($@ || !$new_obj)
{
# Restore failed segment
$nibble = "$related$FF_SEPARATOR$nibble";
last;
}

$obj = $new_obj;
}
}

if($nibble =~ /$FF_SEPARATOR_RE/o)
{
$name = $field->local_name;
}
else
{
$name = $nibble;
$object = $obj;
}
}
else
{
$name = $field->local_name;
}

if($object->can($name))
{
# Checkboxes setting boolean columns
if($field->isa('Rose::HTML::Form::Field::Checkbox') &&
$meta->column($name) && $meta->column($name)->type eq 'boolean')
{
#$Debug && warn "$object->$name(", $field->is_on, ")\n";
$object->$name($field->is_on);
}
else # everything else
{
my $value = $field->internal_value;
$value = undef unless(length $value);

#$Debug && warn "$object->$name($value)\n";
eval { $object->$name($value) };

if($@)
{
# this looks like an error that should not be user-visible
# $self->error("Could not set $object->$name($value) - $@");
$self->error_id(FORM_SET_FIELD_ERROR);
warn "Could not set $object->$name($value) - $@";
return undef;
}
}
}
}

return $parent_object;
}

sub init_with_db_object
{
my($self, $object) = @_;

croak "Missing required object argument" unless($object);

$self->clear();

my $selected_object;

foreach my $field (sort { $a->name cmp $b->name } $self->fields)
{
my $name = $field->name;

$selected_object = $object;

if($name =~ /$FF_SEPARATOR_RE/o)
{
my $nibble = $name;
my $tmp_obj = $selected_object;

while($nibble =~ s/^([^$FF_SEPARATOR]+)$FF_SEPARATOR_RE//o)
{
my $related = $1;
last unless($tmp_obj->can($related));

if(Rose::DB::Object::Util::has_loaded_related($tmp_obj, $related))
{
$tmp_obj = $tmp_obj->$related()
}
else
{
my $new_obj;
eval { $new_obj = $tmp_obj->$related() };

if($@ || !$new_obj)
{
# Restore failed segment
$nibble = "$related$FF_SEPARATOR$nibble";
last;
}

$tmp_obj = $new_obj;
}
}

if($nibble =~ /$FF_SEPARATOR_RE/o)
{
$name = $field->local_name;
}
else
{
$name = $nibble;
$selected_object = $tmp_obj;
}
}
else
{
$name = $field->local_name;
}

if($selected_object->can($name))
{
#$Debug && warn field($name) = $selected_object->$name = ",
$selected_object->$name(), "\n";
$field->input_value(scalar $selected_object->$name());
}
}
}

1;

[James Masters]

I see, yes, that will be much more correct, thanks.