[html-formfu] DBIC-FormFu extension - Carl?

[Carl Franks]

On 25/09/2007, Mario Minati <mario.minati at googlemail.com> wrote:
> On Wednesday 19 September 2007 12:16:01 Carl Franks wrote:
> > On 19/09/2007, Mario Minati <mario.minati at googlemail.com> wrote:
> > > On Wednesday 19 September 2007 11:02:47 Carl Franks wrote:
> > > > On 18/09/2007, Mario Minati <mario.minati at googlemail.com> wrote:
> > > > > If you have the following form fields:
> > > > >     private_street
> > > > >     private_city
> > > > >     private_email
> > > > >     office_street
> > > > >     office_city
> > > > >     office_email
> > > > >
> > > > > You most likely would like to save both datasets in same table:
> > > > >     my $private = $user->new_related( 'data', { type => 'private' }
> > > > > ); $private->populate_from_formfu( $form, { prefix_col => 'private_'
> > > > > } ); my $office = $user->new_related( 'data', { type => 'office' } );
> > > > > $office->populate_from_formfu( $form, { prefix_col => 'office_' } );
> > > >
> > > > I think a more elegant solution would be by adding support for
> > > > 'nested' params, e.g.
> > > >
> > > > with the following input:
> > > >     "private.street" => x
> > > >     "private.city" => y
> > > >     "private.email" => z
> > > >
> > > > $form->param('private') would return a hashref:
> > > > {
> > > >     street => x,
> > > >     city => y,
> > > >     email => z,
> > > > }
> > >
> > > This idea is more general and useful.
> > >
> > > > (or use the alternative syntax, "private[street]", "private[city]",
> > > > etc.)
> > >
> > > Can we get HTML problems with parameter names including '.' or '[]' (I
> > > favour '.' as it would allow chaining.)
> >
> > There shouldn't be any problems, this approach is already used by
> > CGI::Expand and Catalyst::Plugin::Params::Nested.
> >
> > (The idea was pinched from RoR, which uses it extensively)
> >
> > > > Then do something like:
> > > >
> > > >     $relationship->populate_from_formfu(
> > > >         $form,
> > > >         { param => 'private' } );
> > > >
> > > > Also, it's maybe not immediately helpful in this example, but I'd like
> > > > to break out the logic from Element/Dojo/Repeatable.pm into a core
> > > > Element/Repeatable.pm element.
> > > > This could help with some relationships, as it would provide automatic
> > > > handling of multiple fields by adding an increasing numerical suffix
> > > > to the fieldname.
> > >
> > > Would you do a more deeply change in FormFu, like adding an additional
> > > parameter to fieldset like
> > > - fieldset
> > >     my_parameters_belong_to: private
> > >     elements:
> > >       - type: Text
> > >         name: just_name
> > >
> > > which will lead to 'private.just_name' in HTML code.
> >
> > Yes, this approach.
> >
> > Though I'd probably just name it:
> >     nested: private
>
> After playing and thinking a bit I came over the following points I would like
> to discuss:

btw, both notations support chained names:
    foo.bar.baz
    foo[bar][baz]

Have a form-level nested() which is just a bool test.
Have a separate method to set which notation to use, and have it
default to one of them (I'm not really bothered which).

I imagine HTML::FormFu::Element::nested_name() looking something like this:

sub nested_name {
    my $self = shift;

    croak 'cannot set nested_name' if @_;

    if ( defined $self->name && $self->nested ) {
        my @names = ( $self->name );
        while ( defined $self->parent ) {
            my $self = $self->parent;
            push @names, $self->name
                if defined $self->name;
        }
        if ( $self->form->nested_dotted ) {
            return join ".", reverse @names;
        }
        else {
            my $name = pop @names;
            map { $name .= "[$_}" } reverse @names;
            return $name;
        }
    }
}

> 1.
> We need an additional accessor for the nested name (e.g. processed_name) which
> is given to render and which is used in the templates.
> This has to be done as functions use the name to find elements, so I can't
> just rewrite the name.

Probably.
Though I'm still debating on whether $field->render->name() should
return 'name' or 'nested_name'.
Maybe for now, just go with the simpler change, and change the
templates to use 'nested_name' instead of 'name'.

> 2.
> This might lead into a caveat. Considering a form with to fieldsets for
> addresses names just 'street', 'city', ... and with the a nested Attribute in
> their respective fieldsets. The user cannot access these fields by get_field
> unless we provide a way to use the processed nested name to access the field.
> I propose to autodiscover the way to find the fields by looking at the given
> name and use the processed nested names when there is a '.' or '[]' inside
> the given string.

Extend HTML::FormFu::Util::_get_elements() to support a 'nested_name' argument.

    if ( exists $args->{nested_name} ) {
        @$elements
            = grep { defined $_->nested_name && $_->nested_name eq
$args->{nested_name} } @$elements;
    }

If you want to find a field without using it's full name, they you
should probably be doing:
    $fieldset->get_field($field_name);

> 3.
> At the moment I calculate the nested names in 'process' as I
> extended 'process' in Element and late all children from Element that
> redefine 'process' call it's ancestor.
> Do you know of a better place to do this name preprocessing?

Probably just have a nested_name() method which calculates it on demand.

> 4.
> The param processing is going to be tricky, as like in Catalyst someone might
> have already expanded our parameter names into arrays/hashes, we have to take
> care of that.
> All elements and constraints need to use the processed nested name to access
> their parameter values.
> The param functon needs to be changed as it currently expects a list of names
> for which to get the values, but we no might give an argument 'nested', so it
> has to change to only accept an array ref of names or one name and the ash
> ref with the nested name to prepend.

I think we ought to demand that if you're using $form->nested(1) that
you must also use either Catalyst::Plugin::Params::Nested or
CGI::Expand, so that the input is already in a predictable state.

Carl